Re: [PATCH 3/3] s390/vfio-ap: r/w lock for PQAP interception handler function pointer

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

 





On 6/15/21 4:55 AM, Christian Borntraeger wrote:
On 10.06.21 00:46, Tony Krowiak wrote:
The function pointer to the interception handler for the PQAP instruction
can get changed during the interception process. Let's add a
semaphore to struct kvm_s390_crypto to control read/write access to the
function pointer contained therein.

The semaphore must be locked for write access by the vfio_ap device driver
when notified that the KVM pointer has been set or cleared. It must be
locked for read access by the interception framework when the PQAP
instruction is intercepted.

Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>

Given that patch 2 is still  under discussion. Can this patch go without patch 2?

Here's what I think I'm going to do. Patches 1 and 3 are related to the patch I posted to resolve the FIXME in the vfio_ap_mdev_remove() callback in a patch posted by Jason Gunthorpe (Message ID: <6-v1-d88406ed308e+418-vfio3_jgg@xxxxxxxxxx>).
Patch 2 in this series was precipitated by a comment Jason made that was
not directly related to that fix, so I will put a two patch series
with patch 1 and 3, then test them and post them here before proceeding
to resolving the issues in patch 2 which I will post separately.

---
  arch/s390/include/asm/kvm_host.h      |  6 +++---
  arch/s390/kvm/kvm-s390.c              |  1 +
  arch/s390/kvm/priv.c                  |  6 +++---
  drivers/s390/crypto/vfio_ap_ops.c     | 14 ++++++++++----
  drivers/s390/crypto/vfio_ap_private.h |  2 +-
  5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 8925f3969478..58edaa3f9602 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -803,14 +803,14 @@ struct kvm_s390_cpu_model {
      unsigned short ibc;
  };
  -struct kvm_s390_module_hook {
+struct kvm_s390_crypto_hook {
      int (*hook)(struct kvm_vcpu *vcpu);
-    struct module *owner;
  };
    struct kvm_s390_crypto {
      struct kvm_s390_crypto_cb *crycb;
-    struct kvm_s390_module_hook *pqap_hook;
+    struct rw_semaphore pqap_hook_rwsem;
+    struct kvm_s390_crypto_hook *pqap_hook;
      __u32 crycbd;
      __u8 aes_kw;
      __u8 dea_kw;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 1296fc10f80c..418d910df569 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2606,6 +2606,7 @@ static void kvm_s390_crypto_init(struct kvm *kvm)
  {
      kvm->arch.crypto.crycb = &kvm->arch.sie_page2->crycb;
      kvm_s390_set_crycb_format(kvm);
+    init_rwsem(&kvm->arch.crypto.pqap_hook_rwsem);
        if (!test_kvm_facility(kvm, 76))
          return;
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index 9928f785c677..bbbd84ffe239 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -657,15 +657,15 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
       * Verify that the hook callback is registered, lock the owner
       * and call the hook.
       */
+ down_read(&vcpu->kvm->arch.crypto.pqap_hook_rwsem);
      if (vcpu->kvm->arch.crypto.pqap_hook) {
-        if (!try_module_get(vcpu->kvm->arch.crypto.pqap_hook->owner))
-            return -EOPNOTSUPP;
          ret = vcpu->kvm->arch.crypto.pqap_hook->hook(vcpu);
- module_put(vcpu->kvm->arch.crypto.pqap_hook->owner);
          if (!ret && vcpu->run->s.regs.gprs[1] & 0x00ff0000)
              kvm_s390_set_psw_cc(vcpu, 3);
+ up_read(&vcpu->kvm->arch.crypto.pqap_hook_rwsem);
          return ret;
      }
+    up_read(&vcpu->kvm->arch.crypto.pqap_hook_rwsem);
      /*
       * A vfio_driver must register a hook.
       * No hook means no driver to enable the SIE CRYCB and no queues.
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index d65a5728153b..2998c1b53ab9 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -342,7 +342,6 @@ static int vfio_ap_mdev_create(struct mdev_device *mdev)
      init_rwsem(&matrix_mdev->rwsem);
      mdev_set_drvdata(mdev, matrix_mdev);
      matrix_mdev->pqap_hook.hook = handle_pqap;
-    matrix_mdev->pqap_hook.owner = THIS_MODULE;
        mutex_lock(&matrix_dev->lock);
      list_add(&matrix_mdev->node, &matrix_dev->mdev_list);
@@ -1063,7 +1062,6 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
      down_write(&matrix_mdev->rwsem);
      matrix_mdev->kvm = kvm;
      kvm_get_kvm(kvm);
-    kvm->arch.crypto.pqap_hook = &matrix_mdev->pqap_hook;
      up_write(&matrix_mdev->rwsem);
        /*
@@ -1071,6 +1069,10 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
       * masks for the KVM guest
       */
      if (kvm->arch.crypto.crycbd) {
+ down_write(&matrix_mdev->kvm->arch.crypto.pqap_hook_rwsem);
+        kvm->arch.crypto.pqap_hook = &matrix_mdev->pqap_hook;
+ up_write(&matrix_mdev->kvm->arch.crypto.pqap_hook_rwsem);
+
          down_read(&matrix_mdev->matrix.rwsem);
          kvm_arch_crypto_set_masks(kvm,
                        matrix_mdev->matrix.apm,
@@ -1122,11 +1124,15 @@ static int vfio_ap_mdev_iommu_notifier(struct notifier_block *nb,
  static void vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev *matrix_mdev)
  {
      if (matrix_mdev->kvm) {
-        if (matrix_mdev->kvm->arch.crypto.crycbd)
+        if (matrix_mdev->kvm->arch.crypto.crycbd) {
+ down_write(&matrix_mdev->kvm->arch.crypto.pqap_hook_rwsem);
+            matrix_mdev->kvm->arch.crypto.pqap_hook = NULL;
+ up_write(&matrix_mdev->kvm->arch.crypto.pqap_hook_rwsem);
+
              kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
+        }
            down_write(&matrix_mdev->rwsem);
-        matrix_mdev->kvm->arch.crypto.pqap_hook = NULL;
          vfio_ap_mdev_reset_queues(matrix_mdev->mdev);
          kvm_put_kvm(matrix_mdev->kvm);
          matrix_mdev->kvm = NULL;
diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h
index a163ac04ff8a..3d6afd0faaaf 100644
--- a/drivers/s390/crypto/vfio_ap_private.h
+++ b/drivers/s390/crypto/vfio_ap_private.h
@@ -90,7 +90,7 @@ struct ap_matrix_mdev {
      struct notifier_block iommu_notifier;
      struct rw_semaphore rwsem;
      struct kvm *kvm;
-    struct kvm_s390_module_hook pqap_hook;
+    struct kvm_s390_crypto_hook pqap_hook;
      struct mdev_device *mdev;
  };





[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