[v3,0/7] Make the rest of the VFIO driver interface use vfio_device

Message

Jason Gunthorpe May 2, 2022, 5:31 p.m. UTC

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/[e-mail]/

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/[e-mail]
 - 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/[e-mail]

Jason Gunthorpe (7):
  vfio: Make vfio_(un)register_notifier accept a vfio_device
  vfio/ccw: Remove mdev from struct channel_program
  vfio/mdev: Pass in a struct vfio_device * to vfio_pin/unpin_pages()
  vfio/mdev: Pass in a struct vfio_device * to vfio_dma_rw()
  drm/i915/gvt: Change from vfio_group_(un)pin_pages to
    vfio_(un)pin_pages
  vfio: Remove dead code
  vfio: Remove calls to vfio_group_add_container_user()

 .../driver-api/vfio-mediated-device.rst       |   4 +-
 drivers/gpu/drm/i915/gvt/gvt.h                |   5 +-
 drivers/gpu/drm/i915/gvt/kvmgt.c              |  51 ++--
 drivers/s390/cio/vfio_ccw_cp.c                |  47 +--
 drivers/s390/cio/vfio_ccw_cp.h                |   4 +-
 drivers/s390/cio/vfio_ccw_fsm.c               |   3 +-
 drivers/s390/cio/vfio_ccw_ops.c               |   7 +-
 drivers/s390/crypto/vfio_ap_ops.c             |  23 +-
 drivers/vfio/vfio.c                           | 288 ++----------------
 include/linux/vfio.h                          |  21 +-
 10 files changed, 102 insertions(+), 351 deletions(-)


base-commit: 676d7cda1a3c19872428a9bc818577a1aafafdd5

Comments

Jason Gunthorpe May 4, 2022, 5:49 p.m. UTC | #1

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/[e-mail]/
> 
> 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/[e-mail]
>  - 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/[e-mail]

Hi Alex,

This v3 is still good to go, it applies clean on top of the gvt series.

Thanks,
Jason

Alex Williamson May 5, 2022, 6:56 p.m. UTC | #2

On Wed, 4 May 2022 14:49:26 -0300
Jason Gunthorpe <[e-mail]> 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/[e-mail]/
> > 
> > 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/[e-mail]
> >  - 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/[e-mail]  
> 
> 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/[e-mail]/

Subject: [PATCH] vfio: Delete container_q
Date: Fri, 29 Apr 2022 15:46:17 -0300
https://lore.kernel.org/all/[e-mail]/

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/[e-mail]/

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

Alex

commit e35ab17c48abcd8f6b40a35025d6d34a4d57f67a
Author: Alex Williamson <[e-mail]>
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 <[e-mail]>

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);

Jason Gunthorpe May 5, 2022, 7:44 p.m. UTC | #3

On Thu, May 05, 2022 at 12:56:14PM -0600, Alex Williamson wrote:

> 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.

Somehow the entire To line got wiped out, still unclear why. Maybe
get_maintainers had a bad time. I'm watching for this now

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

If it is the style, I will 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/[e-mail]/

This one applies cleanly without the iommu series, I just confirmed it.
 
> Subject: [PATCH] vfio: Delete container_q
> Date: Fri, 29 Apr 2022 15:46:17 -0300
> https://lore.kernel.org/all/[e-mail]/

This one needs the iommu series

> 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/[e-mail]/

I will do this in a few hours hopefully

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

Nothing more for your tree, but FYI:

The enforced coherent series is going through Joerg's tree
https://lore.kernel.org/linux-iommu/[e-mail]/

The bug fix for the iommu series is pending Joerg:
https://lore.kernel.org/linux-iommu/[e-mail]/

I've got one more series to post which just needs a last pass over
now, maybe today, but that is looking grim again.

It is alot of series, it is hard to keep all this organized, thanks.

Jason