Re: [PATCH 2/3] target/s390x: refractor AP functionalities

[Thomas Huth]

In the subject: s/refractor/refactor/

Please also add a short patch description why you are doing these changes.
Signed-off-by: Steffen Eiden <sei...@linux.ibm.com>
---
  target/s390x/kvm/kvm.c | 18 +++++++++++-------
  1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 3ac7ec9acf..bd62a7f606 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -250,7 +250,7 @@ static void kvm_s390_enable_cmma(void)
      trace_kvm_enable_cmma(rc);
  }
  
-static void kvm_s390_set_attr(uint64_t attr)
+static void kvm_s390_set_crypto_attr(uint64_t attr)
  {
      struct kvm_device_attr attribute = {
          .group = KVM_S390_VM_CRYPTO,
@@ -275,7 +275,7 @@ static void kvm_s390_init_aes_kw(void)
      }
  
      if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
-            kvm_s390_set_attr(attr);
+            kvm_s390_set_crypto_attr(attr);
      }
  }
  
@@ -289,7 +289,7 @@ static void kvm_s390_init_dea_kw(void)
      }
  
      if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
-            kvm_s390_set_attr(attr);
+            kvm_s390_set_crypto_attr(attr);
      }
  }

I'd maybe move the renaming into a separate patch.

@@ -2296,6 +2296,11 @@ static int configure_cpu_subfunc(const S390FeatBitmap 
features)
      return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
  }
  
+static bool ap_available(void)
+{
+    return kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, 
KVM_S390_VM_CRYPTO_ENABLE_APIE);

The line is already quite long ... maybe put it on two lines to avoid 
checkpatch.pl warnings.

+}
+
  static int kvm_to_feat[][2] = {
      { KVM_S390_VM_CPU_FEAT_ESOP, S390_FEAT_ESOP },
      { KVM_S390_VM_CPU_FEAT_SIEF2, S390_FEAT_SIE_F2 },
@@ -2475,8 +2480,7 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, 
Error **errp)
          return;
      }
      /* for now, we can only provide the AP feature with HW support */
-    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO,
-        KVM_S390_VM_CRYPTO_ENABLE_APIE)) {
+    if (ap_available()) {
          set_bit(S390_FEAT_AP, model->features);
      }
  
@@ -2502,7 +2506,7 @@ static void kvm_s390_configure_apie(bool interpret)
                                  KVM_S390_VM_CRYPTO_DISABLE_APIE;
  
      if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
-        kvm_s390_set_attr(attr);
+        kvm_s390_set_crypto_attr(attr);
      }
  }
  
@@ -2556,7 +2560,7 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
          kvm_s390_enable_cmma();
      }
  
-    if (test_bit(S390_FEAT_AP, model->features)) {
+    if (ap_enabled(model->features)) {

You only introduce ap_enabled() in the next patch ... please move it here to 
avoid breaking bisection later.

 Thomas


          kvm_s390_configure_apie(true);
      }
  }