On Thu, 11 Feb 2021 09:21:26 -0500 Tony Krowiak akrowiak@linux.ibm.com wrote:
Yes, it makes sense. I guess I didn't look closely at your suggestion when I said it was exactly what I implemented after agreeing with Connie. I had a slight difference in my implementation:
static void vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev *matrix_mdev) { struct kvm *kvm;
mutex_lock(&matrix_dev->lock);
if (matrix_mdev->kvm) { kvm = matrix_mdev->kvm; mutex_unlock(&matrix_dev->lock);
The problem with this one is that as soon as we drop the lock here, another thread can in theory execute the critical section below, which drops our reference to kvm via kvm_put_kvm(kvm). Thus when we enter kvm_arch_crypto_clear_mask(), even if we are guaranteed to have a non-null pointer, the pointee is not guaranteed to be around. So like Connie suggested, you better take another reference to kvm in the first critical section.
Regards, Halil
kvm_arch_crypto_clear_masks(kvm); mutex_lock(&matrix_dev->lock); kvm->arch.crypto.pqap_hook = NULL; vfio_ap_mdev_reset_queues(matrix_mdev->mdev); matrix_mdev->kvm = NULL; kvm_put_kvm(kvm); }
mutex_unlock(&matrix_dev->lock); }