On 15/03/2019 19:15, Halil Pasic wrote:
On Wed, 13 Mar 2019 17:05:01 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
When the mediated device is open we setup the relation with KVM unset it
when the mediated device is released.
We ensure KVM is present on opening of the mediated device.
We ensure that KVM survives the mediated device, and establish a direct
survives?
what alternative do you prefer?
link from KVM to the mediated device to simplify the relationship.
Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
...snip...
static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
unsigned long action, void *data)
{
- int ret;
struct ap_matrix_mdev *matrix_mdev;
if (action != VFIO_GROUP_NOTIFY_SET_KVM)
return NOTIFY_OK;
matrix_mdev = container_of(nb, struct ap_matrix_mdev, group_notifier);
-
- if (!data) {
- matrix_mdev->kvm = NULL;
- return NOTIFY_OK;
- }
-
- ret = vfio_ap_mdev_set_kvm(matrix_mdev, data);
- if (ret)
- return NOTIFY_DONE;
-
- /* If there is no CRYCB pointer, then we can't copy the masks */
- if (!matrix_mdev->kvm->arch.crypto.crycbd)
- return NOTIFY_DONE;
-
- kvm_arch_crypto_set_masks(matrix_mdev->kvm, matrix_mdev->matrix.apm,
- matrix_mdev->matrix.aqm,
- matrix_mdev->matrix.adm);
+ matrix_mdev->kvm = data;
return NOTIFY_OK;
}
@@ -888,6 +873,12 @@ static int vfio_ap_mdev_open(struct mdev_device *mdev)
if (ret)
goto err_group;
+ /* We do not support opening the mediated device without KVM */
+ if (!matrix_mdev->kvm) {
+ ret = -ENODEV;
+ goto err_group;
+ }
+
matrix_mdev->iommu_notifier.notifier_call = vfio_ap_mdev_iommu_notifier;
events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
@@ -896,8 +887,15 @@ static int vfio_ap_mdev_open(struct mdev_device *mdev)
if (ret)
goto err_iommu;
+ ret = vfio_ap_mdev_set_kvm(matrix_mdev);
At this point the matrix_mdev->kvm ain't guaranteed to be valid IMHO. Or
am I wrong? If I'm right kvm_get_kvm(matrix_mdev->kvm) could be too late.
What about the if (!matrix_mdev->kvm) 10 lines above ?
+ if (ret)
+ goto err_kvm;
+
return 0;
+err_kvm:
+ vfio_unregister_notifier(mdev_dev(mdev), VFIO_IOMMU_NOTIFY,
+ &matrix_mdev->iommu_notifier);
err_iommu:
vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
&matrix_mdev->group_notifier);
@@ -906,19 +904,33 @@ static int vfio_ap_mdev_open(struct mdev_device *mdev)
return ret;
}
-static void vfio_ap_mdev_release(struct mdev_device *mdev)
+static int vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev *matrix_mdev)
{
- struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+ struct kvm *kvm = matrix_mdev->kvm;
if (matrix_mdev->kvm)
kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
This still conditional?
Yes, nothing to clear if there is no KVM.
-
+ vfio_ap_mdev_reset_queues(matrix_mdev->mdev);
I guess your intention was to move vfio_ap_mdev_reset_queues()
here from vfio_ap_mdev_release(), but you still have a
vfio_ap_mdev_reset_queues() call in vfio_ap_mdev_release().
+ matrix_mdev->kvm->arch.crypto.pqap_hook = NULL;
matrix_mdev->kvm = NULL;
+
+ kvm_put_kvm(kvm);
+ return 0;
+}
+
+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+ struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+ mutex_lock(&matrix_dev->lock);
+
vfio_ap_mdev_reset_queues(mdev);
right, this one will go away.
Thanks for reviewing.
Regards,
Pierre
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
