Re: [PATCH v9 1/3] s390x: KVM: ipte lock for SCA access should be contained in KVM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 5/12/22 13:32, Janosch Frank wrote:
On 5/6/22 11:24, Pierre Morel wrote:
The former check to chose between SIIF or not SIIF can be done
using the sclp.has_siif instead of accessing per vCPU structures

Maybe replace this paragraph with:
We can check if SIIF is enabled by testing the sclp_info struct instead of testing the sie control block eca variable. sclp.has_ssif is the only requirement to set ECA_SII anyway so we can go straight to the source for that.

Reviewed-by: Janosch Frank <frankja@xxxxxxxxxxxxx>

OK, thanks,

Regards,
Pierre



When accessing the SCA, ipte lock and ipte_unlock do not need
to access any vcpu structures but only the KVM structure.

Let's simplify the ipte handling.

Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
  arch/s390/kvm/gaccess.c | 96 ++++++++++++++++++++---------------------
  arch/s390/kvm/gaccess.h |  6 +--
  arch/s390/kvm/priv.c    |  6 +--
  3 files changed, 54 insertions(+), 54 deletions(-)

diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index d53a183c2005..0e1f6dd31882 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -262,77 +262,77 @@ struct aste {
      /* .. more fields there */
  };
-int ipte_lock_held(struct kvm_vcpu *vcpu)
+int ipte_lock_held(struct kvm *kvm)
  {
-    if (vcpu->arch.sie_block->eca & ECA_SII) {
+    if (sclp.has_siif) {
          int rc;
-        read_lock(&vcpu->kvm->arch.sca_lock);
-        rc = kvm_s390_get_ipte_control(vcpu->kvm)->kh != 0;
-        read_unlock(&vcpu->kvm->arch.sca_lock);
+        read_lock(&kvm->arch.sca_lock);
+        rc = kvm_s390_get_ipte_control(kvm)->kh != 0;
+        read_unlock(&kvm->arch.sca_lock);
          return rc;
      }
-    return vcpu->kvm->arch.ipte_lock_count != 0;
+    return kvm->arch.ipte_lock_count != 0;
  }
-static void ipte_lock_simple(struct kvm_vcpu *vcpu)
+static void ipte_lock_simple(struct kvm *kvm)
  {
      union ipte_control old, new, *ic;
-    mutex_lock(&vcpu->kvm->arch.ipte_mutex);
-    vcpu->kvm->arch.ipte_lock_count++;
-    if (vcpu->kvm->arch.ipte_lock_count > 1)
+    mutex_lock(&kvm->arch.ipte_mutex);
+    kvm->arch.ipte_lock_count++;
+    if (kvm->arch.ipte_lock_count > 1)
          goto out;
  retry:
-    read_lock(&vcpu->kvm->arch.sca_lock);
-    ic = kvm_s390_get_ipte_control(vcpu->kvm);
+    read_lock(&kvm->arch.sca_lock);
+    ic = kvm_s390_get_ipte_control(kvm);
      do {
          old = READ_ONCE(*ic);
          if (old.k) {
-            read_unlock(&vcpu->kvm->arch.sca_lock);
+            read_unlock(&kvm->arch.sca_lock);
              cond_resched();
              goto retry;
          }
          new = old;
          new.k = 1;
      } while (cmpxchg(&ic->val, old.val, new.val) != old.val);
-    read_unlock(&vcpu->kvm->arch.sca_lock);
+    read_unlock(&kvm->arch.sca_lock);
  out:
-    mutex_unlock(&vcpu->kvm->arch.ipte_mutex);
+    mutex_unlock(&kvm->arch.ipte_mutex);
  }
-static void ipte_unlock_simple(struct kvm_vcpu *vcpu)
+static void ipte_unlock_simple(struct kvm *kvm)
  {
      union ipte_control old, new, *ic;
-    mutex_lock(&vcpu->kvm->arch.ipte_mutex);
-    vcpu->kvm->arch.ipte_lock_count--;
-    if (vcpu->kvm->arch.ipte_lock_count)
+    mutex_lock(&kvm->arch.ipte_mutex);
+    kvm->arch.ipte_lock_count--;
+    if (kvm->arch.ipte_lock_count)
          goto out;
-    read_lock(&vcpu->kvm->arch.sca_lock);
-    ic = kvm_s390_get_ipte_control(vcpu->kvm);
+    read_lock(&kvm->arch.sca_lock);
+    ic = kvm_s390_get_ipte_control(kvm);
      do {
          old = READ_ONCE(*ic);
          new = old;
          new.k = 0;
      } while (cmpxchg(&ic->val, old.val, new.val) != old.val);
-    read_unlock(&vcpu->kvm->arch.sca_lock);
-    wake_up(&vcpu->kvm->arch.ipte_wq);
+    read_unlock(&kvm->arch.sca_lock);
+    wake_up(&kvm->arch.ipte_wq);
  out:
-    mutex_unlock(&vcpu->kvm->arch.ipte_mutex);
+    mutex_unlock(&kvm->arch.ipte_mutex);
  }
-static void ipte_lock_siif(struct kvm_vcpu *vcpu)
+static void ipte_lock_siif(struct kvm *kvm)
  {
      union ipte_control old, new, *ic;
  retry:
-    read_lock(&vcpu->kvm->arch.sca_lock);
-    ic = kvm_s390_get_ipte_control(vcpu->kvm);
+    read_lock(&kvm->arch.sca_lock);
+    ic = kvm_s390_get_ipte_control(kvm);
      do {
          old = READ_ONCE(*ic);
          if (old.kg) {
-            read_unlock(&vcpu->kvm->arch.sca_lock);
+            read_unlock(&kvm->arch.sca_lock);
              cond_resched();
              goto retry;
          }
@@ -340,15 +340,15 @@ static void ipte_lock_siif(struct kvm_vcpu *vcpu)
          new.k = 1;
          new.kh++;
      } while (cmpxchg(&ic->val, old.val, new.val) != old.val);
-    read_unlock(&vcpu->kvm->arch.sca_lock);
+    read_unlock(&kvm->arch.sca_lock);
  }
-static void ipte_unlock_siif(struct kvm_vcpu *vcpu)
+static void ipte_unlock_siif(struct kvm *kvm)
  {
      union ipte_control old, new, *ic;
-    read_lock(&vcpu->kvm->arch.sca_lock);
-    ic = kvm_s390_get_ipte_control(vcpu->kvm);
+    read_lock(&kvm->arch.sca_lock);
+    ic = kvm_s390_get_ipte_control(kvm);
      do {
          old = READ_ONCE(*ic);
          new = old;
@@ -356,25 +356,25 @@ static void ipte_unlock_siif(struct kvm_vcpu *vcpu)
          if (!new.kh)
              new.k = 0;
      } while (cmpxchg(&ic->val, old.val, new.val) != old.val);
-    read_unlock(&vcpu->kvm->arch.sca_lock);
+    read_unlock(&kvm->arch.sca_lock);
      if (!new.kh)
-        wake_up(&vcpu->kvm->arch.ipte_wq);
+        wake_up(&kvm->arch.ipte_wq);
  }
-void ipte_lock(struct kvm_vcpu *vcpu)
+void ipte_lock(struct kvm *kvm)
  {
-    if (vcpu->arch.sie_block->eca & ECA_SII)
-        ipte_lock_siif(vcpu);
+    if (sclp.has_siif)
+        ipte_lock_siif(kvm);
      else
-        ipte_lock_simple(vcpu);
+        ipte_lock_simple(kvm);
  }
-void ipte_unlock(struct kvm_vcpu *vcpu)
+void ipte_unlock(struct kvm *kvm)
  {
-    if (vcpu->arch.sie_block->eca & ECA_SII)
-        ipte_unlock_siif(vcpu);
+    if (sclp.has_siif)
+        ipte_unlock_siif(kvm);
      else
-        ipte_unlock_simple(vcpu);
+        ipte_unlock_simple(kvm);
  }
  static int ar_translation(struct kvm_vcpu *vcpu, union asce *asce, u8 ar, @@ -1075,7 +1075,7 @@ int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
      try_storage_prot_override = storage_prot_override_applicable(vcpu);
      need_ipte_lock = psw_bits(*psw).dat && !asce.r;
      if (need_ipte_lock)
-        ipte_lock(vcpu);
+        ipte_lock(vcpu->kvm);
      /*
       * Since we do the access further down ultimately via a move instruction        * that does key checking and returns an error in case of a protection @@ -1113,7 +1113,7 @@ int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
          rc = trans_exc(vcpu, rc, ga, ar, mode, prot);
  out_unlock:
      if (need_ipte_lock)
-        ipte_unlock(vcpu);
+        ipte_unlock(vcpu->kvm);
      if (nr_pages > ARRAY_SIZE(gpa_array))
          vfree(gpas);
      return rc;
@@ -1185,10 +1185,10 @@ int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva, u8 ar,
      rc = get_vcpu_asce(vcpu, &asce, gva, ar, mode);
      if (rc)
          return rc;
-    ipte_lock(vcpu);
+    ipte_lock(vcpu->kvm);
      rc = guest_range_to_gpas(vcpu, gva, ar, NULL, length, asce, mode,
                   access_key);
-    ipte_unlock(vcpu);
+    ipte_unlock(vcpu->kvm);
      return rc;
  }
@@ -1451,7 +1451,7 @@ int kvm_s390_shadow_fault(struct kvm_vcpu *vcpu, struct gmap *sg,
       * tables/pointers we read stay valid - unshadowing is however
       * always possible - only guest_table_lock protects us.
       */
-    ipte_lock(vcpu);
+    ipte_lock(vcpu->kvm);
      rc = gmap_shadow_pgt_lookup(sg, saddr, &pgt, &dat_protection, &fake);
      if (rc)
@@ -1485,7 +1485,7 @@ int kvm_s390_shadow_fault(struct kvm_vcpu *vcpu, struct gmap *sg,
      pte.p |= dat_protection;
      if (!rc)
          rc = gmap_shadow_page(sg, saddr, __pte(pte.val));
-    ipte_unlock(vcpu);
+    ipte_unlock(vcpu->kvm);
      mmap_read_unlock(sg->mm);
      return rc;
  }
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
index 1124ff282012..9408d6cc8e2c 100644
--- a/arch/s390/kvm/gaccess.h
+++ b/arch/s390/kvm/gaccess.h
@@ -440,9 +440,9 @@ int read_guest_real(struct kvm_vcpu *vcpu, unsigned long gra, void *data,
      return access_guest_real(vcpu, gra, data, len, 0);
  }
-void ipte_lock(struct kvm_vcpu *vcpu);
-void ipte_unlock(struct kvm_vcpu *vcpu);
-int ipte_lock_held(struct kvm_vcpu *vcpu);
+void ipte_lock(struct kvm *kvm);
+void ipte_unlock(struct kvm *kvm);
+int ipte_lock_held(struct kvm *kvm);
  int kvm_s390_check_low_addr_prot_real(struct kvm_vcpu *vcpu, unsigned long gra);
  /* MVPG PEI indication bits */
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index 5beb7a4a11b3..0e8603acc105 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -443,7 +443,7 @@ static int handle_ipte_interlock(struct kvm_vcpu *vcpu)
      vcpu->stat.instruction_ipte_interlock++;
      if (psw_bits(vcpu->arch.sie_block->gpsw).pstate)
          return kvm_s390_inject_program_int(vcpu, PGM_PRIVILEGED_OP);
-    wait_event(vcpu->kvm->arch.ipte_wq, !ipte_lock_held(vcpu));
+    wait_event(vcpu->kvm->arch.ipte_wq, !ipte_lock_held(vcpu->kvm));
      kvm_s390_retry_instr(vcpu);
      VCPU_EVENT(vcpu, 4, "%s", "retrying ipte interlock operation");
      return 0;
@@ -1472,7 +1472,7 @@ static int handle_tprot(struct kvm_vcpu *vcpu)
      access_key = (operand2 & 0xf0) >> 4;
      if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_DAT)
-        ipte_lock(vcpu);
+        ipte_lock(vcpu->kvm);
      ret = guest_translate_address_with_key(vcpu, address, ar, &gpa,
                             GACC_STORE, access_key);
@@ -1509,7 +1509,7 @@ static int handle_tprot(struct kvm_vcpu *vcpu)
      }
      if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_DAT)
-        ipte_unlock(vcpu);
+        ipte_unlock(vcpu->kvm);
      return ret;
  }


--
Pierre Morel
IBM Lab Boeblingen



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux