[PATCH v2 1/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM
Matthew Rosato
mjrosato at linux.ibm.com
Thu May 19 16:35:00 UTC 2022
On 5/19/22 12:23 PM, Tony Krowiak wrote:
> I made a few comments, but other than that this looks good to
> me:
>
> Reviewed-by: Tony Krowiak <akrowiak at linux.ibm.com>
>
...
> I'm not sure what version of the code on which the patch was rebased,
Was on top of Jason's vfio_group_locking series, but now would apply on
vfio-next since Alex pulled that series in to vfio-next.
> 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.
Yeah, I didn't notice those there. v3 will simply remove my get/put
additions and leave yours as-is.
...
>> vfio_ap_mdev_group_notifier;
>> - events = VFIO_GROUP_NOTIFY_SET_KVM;
>> + if (!vdev->kvm)
>> + return -EPERM;
>
> Perhaps -EINVAL or -EFAULT?
>
Whichever you'd prefer? If I don't hear back I'll just use -EINVAL in v3.
>> - 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,
With this patch matrix_mdev->kvm is set from the value in vdev->kvm
during vfio_ap_mdev_set_kvm (basically, doing the work that the notifier
was doing but instead of getting it from notifier data get it from the
vfio_device)
> 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.
>
More information about the intel-gvt-dev
mailing list