I made a few comments, but other than that this looks good to
me:
Reviewed-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
I'm not sure what version of the code on which the patch was rebased,
but in the
latest master branch from our repository the kvm_get_kvm(kvm) function is
called inside of the if block below. I'm fine with moving outside of the block, but
I don't see a corresponding removal of it from inside the block.
vfio_ap_mdev_group_notifier;
- events = VFIO_GROUP_NOTIFY_SET_KVM;
+ if (!vdev->kvm)
+ return -EPERM;
Perhaps -EINVAL or -EFAULT?
- ret = vfio_register_notifier(vdev, VFIO_GROUP_NOTIFY, &events,
- &matrix_mdev->group_notifier);
+ ret = vfio_ap_mdev_set_kvm(matrix_mdev, vdev->kvm);
if (ret)
return ret;
@@ -1415,12 +1400,11 @@ static int vfio_ap_mdev_open_device(struct vfio_device *vdev)
ret = vfio_register_notifier(vdev, VFIO_IOMMU_NOTIFY, &events,
&matrix_mdev->iommu_notifier);
if (ret)
- goto out_unregister_group;
+ goto err_kvm;
return 0;
-out_unregister_group:
- vfio_unregister_notifier(vdev, VFIO_GROUP_NOTIFY,
- &matrix_mdev->group_notifier);
+err_kvm:
+ vfio_ap_mdev_unset_kvm(matrix_mdev);
return ret;
}
@@ -1431,8 +1415,6 @@ static void vfio_ap_mdev_close_device(struct vfio_device *vdev)
vfio_unregister_notifier(vdev, VFIO_IOMMU_NOTIFY,
&matrix_mdev->iommu_notifier);
- vfio_unregister_notifier(vdev, VFIO_GROUP_NOTIFY,
- &matrix_mdev->group_notifier);
vfio_ap_mdev_unset_kvm(matrix_mdev);
I'm not sure if this matters, but the vfio_ap_mdev_unset_kvm(matrix_mdev)
function uses the KVM pointer stored in matrix_mdev->kvm. I can't imagine
the KVM pointer stored in vdev->kvm being different than matrix_mdev->kvm,
but thought I should point it out. Previously, this function was called by the
notifier handler which did not have access to the KVM pointer which is why it
was retrieved from matrix_mdev->kvm. Even if the vdev->kvm and
matrix_mdev->kvm did not match, we should probably go ahead and call
the unset function anyway to remove access to AP resources for the guest and
reset the queues.