Re: [PATCH v3 0/7] Make the rest of the VFIO driver interface use vfio_device

Alex Williamson
2022-05-05T18:56:23Z

On Wed, 4 May 2022 14:49:26 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Mon, May 02, 2022 at 02:31:30PM -0300, Jason Gunthorpe wrote:
> > Prior series have transformed other parts of VFIO from working on struct
> > device or struct vfio_group into working directly on struct
> > vfio_device. Based on that work we now have vfio_device's readily
> > available in all the drivers.
> >
> > Update the rest of the driver facing API to use vfio_device as an input.
> >
> > The following are switched from struct device to struct vfio_device:
> > vfio_register_notifier()
> > vfio_unregister_notifier()
> > vfio_pin_pages()
> > vfio_unpin_pages()
> > vfio_dma_rw()
> >
> > The following group APIs are obsoleted and removed by just using struct
> > vfio_device with the above:
> > vfio_group_pin_pages()
> > vfio_group_unpin_pages()
> > vfio_group_iommu_domain()
> > vfio_group_get_external_user_from_dev()
> >
> > To retain the performance of the new device APIs relative to their group
> > versions optimize how vfio_group_add_container_user() is used to avoid
> > calling it when the driver must already guarantee the device is open and
> > the container_users incrd.
> >
> > The remaining exported VFIO group interfaces are only used by kvm, and are
> > addressed by a parallel series.
> >
> > This series is based on Christoph's gvt rework here:
> >
> > https://lore.kernel.org/all/5a8b9f48-2c32-8177-1c18-e3bd7bfde558@intel.com/
> >
> > and so will need the PR merged first.
> >
> > I have a followup series that needs this.
> >
> > This is also part of the iommufd work - moving the driver facing interface
> > to vfio_device provides a much cleaner path to integrate with iommufd.
> >
> > v3:
> > - Based on VFIO's gvt/iommu merge
> > - Remove mention of mdev_legacy_get_vfio_device() from commit message
> > - Clarify commit message for vfio_dma_rw() conversion
> > - Talk about the open_count change in the commit message
> > - No code change
> > v2: https://lore.kernel.org/r/0-v2-6011bde8e0a1+5f-vfio_mdev_no_group_jgg@nvidia.com
> > - Based on Christoph's series so mdev_legacy_get_vfio_device() is removed
> > - Reflow indenting
> > - Use vfio_assert_device_open() and WARN_ON_ONCE instead of open coding
> > the assertion
> > v1: https://lore.kernel.org/r/0-v1-a8faf768d202+125dd-vfio_mdev_no_group_jgg@nvidia.com
>
> Hi Alex,
>
> This v3 is still good to go, it applies clean on top of the gvt series.

I wasn't cc'd on that version. It looks to me like we'd still prefer
to see acks from GVT-g maintainers, Zhenyu and Zhi.

Also, I was thinking of posting the below cleanup patch unless you'd
prefer to roll it in.

Regarding your other outstanding patches, I think all of these depend
on the IOMMU changes, please correct if there are any that can be
queued with only the GVT-g topic branch dependency:

Subject: [PATCH v3 0/8] Remove vfio_group from the struct file facing VFIO API
Date: Wed, 4 May 2022 16:14:38 -0300
https://lore.kernel.org/all/0-v3-f7729924a7ea+25e33-vfio_kvm_no_group_jgg@nvidia.com/

Subject: [PATCH] vfio: Delete container_q
Date: Fri, 29 Apr 2022 15:46:17 -0300
https://lore.kernel.org/all/0-v1-a1e8791d795b+6b-vfio_container_q_jgg@nvidia.com/

And I'm waiting for a respin based on comments for:

Subject: [PATCH v3 0/2] Remove vfio_device_get_from_dev()
Date: Wed, 4 May 2022 16:01:46 -0300
https://lore.kernel.org/all/0-v3-4adf6c1b8e7c+170-vfio_get_from_dev_jgg@nvidia.com/

If there are others I should be tracking, please let me know. Thanks,

Alex

commit e35ab17c48abcd8f6b40a35025d6d34a4d57f67a
Author: Alex Williamson <alex.williamson@redhat.com>
Date: Wed May 4 17:01:51 2022 -0600

vfio: Rename notifier function args

We typically use "device" for a struct vfio_device* whereas "dev" is
more likely a "struct device*".

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index b566ae3d320b..5de9a9892877 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -2104,7 +2104,7 @@ EXPORT_SYMBOL(vfio_set_irqs_validate_and_prepare);
/*
* Pin a set of guest PFNs and return their associated host PFNs for
local
* domain only.
- * @dev [in] : device
+ * @device [in] : vfio device
* @user_pfn [in]: array of user/guest PFNs to be pinned.
* @npage [in] : count of elements in user_pfn array. This count
should not
* be greater VFIO_PIN_PAGES_MAX_ENTRIES.
@@ -2112,15 +2112,16 @@
EXPORT_SYMBOL(vfio_set_irqs_validate_and_prepare);
* @phys_pfn[out]: array of host PFNs
* Return error or number of pages pinned.
*/
-int vfio_pin_pages(struct vfio_device *vdev, unsigned long *user_pfn,
int npage,
- int prot, unsigned long *phys_pfn)
+int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn,
+ int npage, int prot, unsigned long *phys_pfn)
{
struct vfio_container *container;
- struct vfio_group *group = vdev->group;
+ struct vfio_group *group = device->group;
struct vfio_iommu_driver *driver;
int ret;

- if (!user_pfn || !phys_pfn || !npage ||
!vfio_assert_device_open(vdev))
+ if (!user_pfn || !phys_pfn || !npage ||
+ !vfio_assert_device_open(device))
return -EINVAL;

if (npage > VFIO_PIN_PAGES_MAX_ENTRIES)
@@ -2144,27 +2145,27 @@ EXPORT_SYMBOL(vfio_pin_pages);

/*
* Unpin set of host PFNs for local domain only.
- * @dev [in] : device
+ * @device [in] : vfio device
* @user_pfn [in]: array of user/guest PFNs to be unpinned. Number of
user/guest
* PFNs should not be greater than
VFIO_PIN_PAGES_MAX_ENTRIES.
* @npage [in] : count of elements in user_pfn array. This count
should not
* be greater than VFIO_PIN_PAGES_MAX_ENTRIES.
* Return error or number of pages unpinned.
*/
-int vfio_unpin_pages(struct vfio_device *vdev, unsigned long *user_pfn,
+int vfio_unpin_pages(struct vfio_device *device, unsigned long
*user_pfn, int npage)
{
struct vfio_container *container;
struct vfio_iommu_driver *driver;
int ret;

- if (!user_pfn || !npage || !vfio_assert_device_open(vdev))
+ if (!user_pfn || !npage || !vfio_assert_device_open(device))
return -EINVAL;

if (npage > VFIO_PIN_PAGES_MAX_ENTRIES)
return -E2BIG;

- container = vdev->group->container;
+ container = device->group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->unpin_pages))
ret = driver->ops->unpin_pages(container->iommu_data,
user_pfn, @@ -2186,24 +2187,24 @@ EXPORT_SYMBOL(vfio_unpin_pages);
* As the read/write of user space memory is conducted via the CPUs
and is
* not a real device DMA, it is not necessary to pin the user space
memory. *
- * @vdev [in] : VFIO device
+ * @device [in] : VFIO device
* @user_iova [in] : base IOVA of a user space buffer
* @data [in] : pointer to kernel buffer
* @len [in] : kernel buffer length
* @write : indicate read or write
* Return error code on failure or 0 on success.
*/
-int vfio_dma_rw(struct vfio_device *vdev, dma_addr_t user_iova, void
*data, +int vfio_dma_rw(struct vfio_device *device, dma_addr_t
user_iova, void *data, size_t len, bool write)
{
struct vfio_container *container;
struct vfio_iommu_driver *driver;
int ret = 0;

- if (!data || len <= 0 || !vfio_assert_device_open(vdev))
+ if (!data || len <= 0 || !vfio_assert_device_open(device))
return -EINVAL;

- container = vdev->group->container;
+ container = device->group->container;
driver = container->iommu_driver;

if (likely(driver && driver->ops->dma_rw))
@@ -2211,6 +2212,7 @@ int vfio_dma_rw(struct vfio_device *vdev,
dma_addr_t user_iova, void *data, user_iova, data, len, write);
else
ret = -ENOTTY;
+
return ret;
}
EXPORT_SYMBOL(vfio_dma_rw);
@@ -2287,13 +2289,15 @@ static int vfio_register_group_notifier(struct
vfio_group *group, return ret;
}

-int vfio_register_notifier(struct vfio_device *dev, enum
vfio_notify_type type,
- unsigned long *events, struct
notifier_block *nb) +int vfio_register_notifier(struct vfio_device
*device,
+ enum vfio_notify_type type, unsigned long
*events,
+ struct notifier_block *nb)
{
- struct vfio_group *group = dev->group;
+ struct vfio_group *group = device->group;
int ret;

- if (!nb || !events || (*events == 0) ||
!vfio_assert_device_open(dev))
+ if (!nb || !events || (*events == 0) ||
+ !vfio_assert_device_open(device))
return -EINVAL;

switch (type) {
@@ -2310,14 +2314,14 @@ int vfio_register_notifier(struct vfio_device
*dev, enum vfio_notify_type type, }
EXPORT_SYMBOL(vfio_register_notifier);

-int vfio_unregister_notifier(struct vfio_device *dev,
+int vfio_unregister_notifier(struct vfio_device *device,
enum vfio_notify_type type,
struct notifier_block *nb)
{
- struct vfio_group *group = dev->group;
+ struct vfio_group *group = device->group;
int ret;

- if (!nb || !vfio_assert_device_open(dev))
+ if (!nb || !vfio_assert_device_open(device))
return -EINVAL;

switch (type) {
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 9a9981c26228..6195edd2edcd 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -148,11 +148,11 @@ extern long vfio_external_check_extension(struct
vfio_group *group,
#define VFIO_PIN_PAGES_MAX_ENTRIES (PAGE_SIZE/sizeof(unsigned
long))
-extern int vfio_pin_pages(struct vfio_device *vdev, unsigned long
*user_pfn, +extern int vfio_pin_pages(struct vfio_device *device,
unsigned long *user_pfn, int npage, int prot, unsigned long *phys_pfn);
-extern int vfio_unpin_pages(struct vfio_device *vdev, unsigned long
*user_pfn, +extern int vfio_unpin_pages(struct vfio_device *device,
unsigned long *user_pfn, int npage);
-extern int vfio_dma_rw(struct vfio_device *vdev, dma_addr_t user_iova,
+extern int vfio_dma_rw(struct vfio_device *device, dma_addr_t
user_iova, void *data, size_t len, bool write);

/* each type has independent events */
@@ -167,11 +167,11 @@ enum vfio_notify_type {
/* events for VFIO_GROUP_NOTIFY */
#define VFIO_GROUP_NOTIFY_SET_KVM BIT(0)

-extern int vfio_register_notifier(struct vfio_device *dev,
+extern int vfio_register_notifier(struct vfio_device *device,
enum vfio_notify_type type,
unsigned long *required_events,
struct notifier_block *nb);
-extern int vfio_unregister_notifier(struct vfio_device *dev,
+extern int vfio_unregister_notifier(struct vfio_device *device,
enum vfio_notify_type type,
struct notifier_block *nb);