| Message ID | 20230401144429.88673-5-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce new methods for verifying ownership in vfio PCI hot reset | expand |
Hi, On 4/1/23 16:44, Yi Liu wrote: > This is needed by the vfio-pci driver to report affected devices in the > hot reset for a given device. > > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > Tested-by: Yanting Jiang <yanting.jiang@intel.com> > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > --- > drivers/iommu/iommufd/device.c | 12 ++++++++++++ > drivers/vfio/iommufd.c | 14 ++++++++++++++ > include/linux/iommufd.h | 3 +++ > include/linux/vfio.h | 13 +++++++++++++ > 4 files changed, 42 insertions(+) > > diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c > index 25115d401d8f..04a57aa1ae2c 100644 > --- a/drivers/iommu/iommufd/device.c > +++ b/drivers/iommu/iommufd/device.c > @@ -131,6 +131,18 @@ void iommufd_device_unbind(struct iommufd_device *idev) > } > EXPORT_SYMBOL_NS_GPL(iommufd_device_unbind, IOMMUFD); > > +struct iommufd_ctx *iommufd_device_to_ictx(struct iommufd_device *idev) > +{ > + return idev->ictx; > +} > +EXPORT_SYMBOL_NS_GPL(iommufd_device_to_ictx, IOMMUFD); > + > +u32 iommufd_device_to_id(struct iommufd_device *idev) > +{ > + return idev->obj.id; > +} > +EXPORT_SYMBOL_NS_GPL(iommufd_device_to_id, IOMMUFD); > + > static int iommufd_device_setup_msi(struct iommufd_device *idev, > struct iommufd_hw_pagetable *hwpt, > phys_addr_t sw_msi_start) > diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c > index 88b00c501015..809f2dd73b9e 100644 > --- a/drivers/vfio/iommufd.c > +++ b/drivers/vfio/iommufd.c > @@ -66,6 +66,20 @@ void vfio_iommufd_unbind(struct vfio_device *vdev) > vdev->ops->unbind_iommufd(vdev); > } > > +struct iommufd_ctx *vfio_iommufd_physical_ictx(struct vfio_device *vdev) > +{ > + if (!vdev->iommufd_device) > + return NULL; > + return iommufd_device_to_ictx(vdev->iommufd_device); > +} > +EXPORT_SYMBOL_GPL(vfio_iommufd_physical_ictx); > + > +void vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id) > +{ > + if (vdev->iommufd_device) > + *id = iommufd_device_to_id(vdev->iommufd_device); since there is no return value, may be worth to add at least a WARN_ON in case of !vdev->iommufd_device > +} > +EXPORT_SYMBOL_GPL(vfio_iommufd_physical_devid); > /* > * The physical standard ops mean that the iommufd_device is bound to the > * physical device vdev->dev that was provided to vfio_init_group_dev(). Drivers > diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h > index 1129a36a74c4..ac96df406833 100644 > --- a/include/linux/iommufd.h > +++ b/include/linux/iommufd.h > @@ -24,6 +24,9 @@ void iommufd_device_unbind(struct iommufd_device *idev); > int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id); > void iommufd_device_detach(struct iommufd_device *idev); > > +struct iommufd_ctx *iommufd_device_to_ictx(struct iommufd_device *idev); > +u32 iommufd_device_to_id(struct iommufd_device *idev); > + > struct iommufd_access_ops { > u8 needs_pin_pages : 1; > void (*unmap)(void *data, unsigned long iova, unsigned long length); > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > index 3188d8a374bd..97a1174b922f 100644 > --- a/include/linux/vfio.h > +++ b/include/linux/vfio.h > @@ -113,6 +113,8 @@ struct vfio_device_ops { > }; > > #if IS_ENABLED(CONFIG_IOMMUFD) > +struct iommufd_ctx *vfio_iommufd_physical_ictx(struct vfio_device *vdev); > +void vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id); > int vfio_iommufd_physical_bind(struct vfio_device *vdev, > struct iommufd_ctx *ictx, u32 *out_device_id); > void vfio_iommufd_physical_unbind(struct vfio_device *vdev); > @@ -122,6 +124,17 @@ int vfio_iommufd_emulated_bind(struct vfio_device *vdev, > void vfio_iommufd_emulated_unbind(struct vfio_device *vdev); > int vfio_iommufd_emulated_attach_ioas(struct vfio_device *vdev, u32 *pt_id); > #else > +static inline struct iommufd_ctx * > +vfio_iommufd_physical_ictx(struct vfio_device *vdev) > +{ > + return NULL; > +} > + > +static inline void > +vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id) > +{ > +} > + > #define vfio_iommufd_physical_bind \ > ((int (*)(struct vfio_device *vdev, struct iommufd_ctx *ictx, \ > u32 *out_device_id)) NULL) besides Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric
On Tue, 4 Apr 2023 17:28:40 +0200 Eric Auger <eric.auger@redhat.com> wrote: > Hi, > > On 4/1/23 16:44, Yi Liu wrote: > > This is needed by the vfio-pci driver to report affected devices in the > > hot reset for a given device. > > > > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > > Tested-by: Yanting Jiang <yanting.jiang@intel.com> > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > > --- > > drivers/iommu/iommufd/device.c | 12 ++++++++++++ > > drivers/vfio/iommufd.c | 14 ++++++++++++++ > > include/linux/iommufd.h | 3 +++ > > include/linux/vfio.h | 13 +++++++++++++ > > 4 files changed, 42 insertions(+) > > > > diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c > > index 25115d401d8f..04a57aa1ae2c 100644 > > --- a/drivers/iommu/iommufd/device.c > > +++ b/drivers/iommu/iommufd/device.c > > @@ -131,6 +131,18 @@ void iommufd_device_unbind(struct iommufd_device *idev) > > } > > EXPORT_SYMBOL_NS_GPL(iommufd_device_unbind, IOMMUFD); > > > > +struct iommufd_ctx *iommufd_device_to_ictx(struct iommufd_device *idev) > > +{ > > + return idev->ictx; > > +} > > +EXPORT_SYMBOL_NS_GPL(iommufd_device_to_ictx, IOMMUFD); > > + > > +u32 iommufd_device_to_id(struct iommufd_device *idev) > > +{ > > + return idev->obj.id; > > +} > > +EXPORT_SYMBOL_NS_GPL(iommufd_device_to_id, IOMMUFD); > > + > > static int iommufd_device_setup_msi(struct iommufd_device *idev, > > struct iommufd_hw_pagetable *hwpt, > > phys_addr_t sw_msi_start) > > diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c > > index 88b00c501015..809f2dd73b9e 100644 > > --- a/drivers/vfio/iommufd.c > > +++ b/drivers/vfio/iommufd.c > > @@ -66,6 +66,20 @@ void vfio_iommufd_unbind(struct vfio_device *vdev) > > vdev->ops->unbind_iommufd(vdev); > > } > > > > +struct iommufd_ctx *vfio_iommufd_physical_ictx(struct vfio_device *vdev) > > +{ > > + if (!vdev->iommufd_device) > > + return NULL; > > + return iommufd_device_to_ictx(vdev->iommufd_device); > > +} > > +EXPORT_SYMBOL_GPL(vfio_iommufd_physical_ictx); > > + > > +void vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id) > > +{ > > + if (vdev->iommufd_device) > > + *id = iommufd_device_to_id(vdev->iommufd_device); > since there is no return value, may be worth to add at least a WARN_ON > in case of !vdev->iommufd_device Yeah, this is bizarre and makes the one caller of this interface very awkward. We later go on to define IOMMUFD_INVALID_ID, so this should simply return that in the case of no iommufd_device and skip this unnecessary pointer passing. Thanks, Alex > > +} > > +EXPORT_SYMBOL_GPL(vfio_iommufd_physical_devid); > > /* > > * The physical standard ops mean that the iommufd_device is bound to the > > * physical device vdev->dev that was provided to vfio_init_group_dev(). Drivers > > diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h > > index 1129a36a74c4..ac96df406833 100644 > > --- a/include/linux/iommufd.h > > +++ b/include/linux/iommufd.h > > @@ -24,6 +24,9 @@ void iommufd_device_unbind(struct iommufd_device *idev); > > int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id); > > void iommufd_device_detach(struct iommufd_device *idev); > > > > +struct iommufd_ctx *iommufd_device_to_ictx(struct iommufd_device *idev); > > +u32 iommufd_device_to_id(struct iommufd_device *idev); > > + > > struct iommufd_access_ops { > > u8 needs_pin_pages : 1; > > void (*unmap)(void *data, unsigned long iova, unsigned long length); > > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > > index 3188d8a374bd..97a1174b922f 100644 > > --- a/include/linux/vfio.h > > +++ b/include/linux/vfio.h > > @@ -113,6 +113,8 @@ struct vfio_device_ops { > > }; > > > > #if IS_ENABLED(CONFIG_IOMMUFD) > > +struct iommufd_ctx *vfio_iommufd_physical_ictx(struct vfio_device *vdev); > > +void vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id); > > int vfio_iommufd_physical_bind(struct vfio_device *vdev, > > struct iommufd_ctx *ictx, u32 *out_device_id); > > void vfio_iommufd_physical_unbind(struct vfio_device *vdev); > > @@ -122,6 +124,17 @@ int vfio_iommufd_emulated_bind(struct vfio_device *vdev, > > void vfio_iommufd_emulated_unbind(struct vfio_device *vdev); > > int vfio_iommufd_emulated_attach_ioas(struct vfio_device *vdev, u32 *pt_id); > > #else > > +static inline struct iommufd_ctx * > > +vfio_iommufd_physical_ictx(struct vfio_device *vdev) > > +{ > > + return NULL; > > +} > > + > > +static inline void > > +vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id) > > +{ > > +} > > + > > #define vfio_iommufd_physical_bind \ > > ((int (*)(struct vfio_device *vdev, struct iommufd_ctx *ictx, \ > > u32 *out_device_id)) NULL) > besides > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Eric >
> From: Alex Williamson <alex.williamson@redhat.com> > Sent: Wednesday, April 5, 2023 5:49 AM > On Tue, 4 Apr 2023 17:28:40 +0200 > Eric Auger <eric.auger@redhat.com> wrote: > > > Hi, > > > > On 4/1/23 16:44, Yi Liu wrote: > > > This is needed by the vfio-pci driver to report affected devices in the > > > hot reset for a given device. > > > > > > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > > > Tested-by: Yanting Jiang <yanting.jiang@intel.com> > > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > > > --- > > > drivers/iommu/iommufd/device.c | 12 ++++++++++++ > > > drivers/vfio/iommufd.c | 14 ++++++++++++++ > > > include/linux/iommufd.h | 3 +++ > > > include/linux/vfio.h | 13 +++++++++++++ > > > 4 files changed, 42 insertions(+) > > > > > > diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c > > > index 25115d401d8f..04a57aa1ae2c 100644 > > > --- a/drivers/iommu/iommufd/device.c > > > +++ b/drivers/iommu/iommufd/device.c > > > @@ -131,6 +131,18 @@ void iommufd_device_unbind(struct iommufd_device > *idev) > > > } > > > EXPORT_SYMBOL_NS_GPL(iommufd_device_unbind, IOMMUFD); > > > > > > +struct iommufd_ctx *iommufd_device_to_ictx(struct iommufd_device *idev) > > > +{ > > > + return idev->ictx; > > > +} > > > +EXPORT_SYMBOL_NS_GPL(iommufd_device_to_ictx, IOMMUFD); > > > + > > > +u32 iommufd_device_to_id(struct iommufd_device *idev) > > > +{ > > > + return idev->obj.id; > > > +} > > > +EXPORT_SYMBOL_NS_GPL(iommufd_device_to_id, IOMMUFD); > > > + > > > static int iommufd_device_setup_msi(struct iommufd_device *idev, > > > struct iommufd_hw_pagetable *hwpt, > > > phys_addr_t sw_msi_start) > > > diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c > > > index 88b00c501015..809f2dd73b9e 100644 > > > --- a/drivers/vfio/iommufd.c > > > +++ b/drivers/vfio/iommufd.c > > > @@ -66,6 +66,20 @@ void vfio_iommufd_unbind(struct vfio_device *vdev) > > > vdev->ops->unbind_iommufd(vdev); > > > } > > > > > > +struct iommufd_ctx *vfio_iommufd_physical_ictx(struct vfio_device *vdev) > > > +{ > > > + if (!vdev->iommufd_device) > > > + return NULL; > > > + return iommufd_device_to_ictx(vdev->iommufd_device); > > > +} > > > +EXPORT_SYMBOL_GPL(vfio_iommufd_physical_ictx); > > > + > > > +void vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id) > > > +{ > > > + if (vdev->iommufd_device) > > > + *id = iommufd_device_to_id(vdev->iommufd_device); > > since there is no return value, may be worth to add at least a WARN_ON > > in case of !vdev->iommufd_device This may be a user-triggerable warning if the input device is not bound to iommufd. > Yeah, this is bizarre and makes the one caller of this interface very > awkward. We later go on to define IOMMUFD_INVALID_ID, so this should > simply return that in the case of no iommufd_device and skip this > unnecessary pointer passing. Thanks, Ok. then it can return invalid id when !CONFIG_IOMMUFD. Also Needs to wait for the decision in the thread that is talking errr-code. Regards, Yi Liu > Alex > > > > +} > > > +EXPORT_SYMBOL_GPL(vfio_iommufd_physical_devid); > > > /* > > > * The physical standard ops mean that the iommufd_device is bound to the > > > * physical device vdev->dev that was provided to vfio_init_group_dev(). Drivers > > > diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h > > > index 1129a36a74c4..ac96df406833 100644 > > > --- a/include/linux/iommufd.h > > > +++ b/include/linux/iommufd.h > > > @@ -24,6 +24,9 @@ void iommufd_device_unbind(struct iommufd_device *idev); > > > int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id); > > > void iommufd_device_detach(struct iommufd_device *idev); > > > > > > +struct iommufd_ctx *iommufd_device_to_ictx(struct iommufd_device *idev); > > > +u32 iommufd_device_to_id(struct iommufd_device *idev); > > > + > > > struct iommufd_access_ops { > > > u8 needs_pin_pages : 1; > > > void (*unmap)(void *data, unsigned long iova, unsigned long length); > > > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > > > index 3188d8a374bd..97a1174b922f 100644 > > > --- a/include/linux/vfio.h > > > +++ b/include/linux/vfio.h > > > @@ -113,6 +113,8 @@ struct vfio_device_ops { > > > }; > > > > > > #if IS_ENABLED(CONFIG_IOMMUFD) > > > +struct iommufd_ctx *vfio_iommufd_physical_ictx(struct vfio_device *vdev); > > > +void vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id); > > > int vfio_iommufd_physical_bind(struct vfio_device *vdev, > > > struct iommufd_ctx *ictx, u32 *out_device_id); > > > void vfio_iommufd_physical_unbind(struct vfio_device *vdev); > > > @@ -122,6 +124,17 @@ int vfio_iommufd_emulated_bind(struct vfio_device > *vdev, > > > void vfio_iommufd_emulated_unbind(struct vfio_device *vdev); > > > int vfio_iommufd_emulated_attach_ioas(struct vfio_device *vdev, u32 *pt_id); > > > #else > > > +static inline struct iommufd_ctx * > > > +vfio_iommufd_physical_ictx(struct vfio_device *vdev) > > > +{ > > > + return NULL; > > > +} > > > + > > > +static inline void > > > +vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id) > > > +{ > > > +} > > > + > > > #define vfio_iommufd_physical_bind \ > > > ((int (*)(struct vfio_device *vdev, struct iommufd_ctx *ictx, \ > > > u32 *out_device_id)) NULL) > > besides > > > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > > > Eric > >
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index 25115d401d8f..04a57aa1ae2c 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -131,6 +131,18 @@ void iommufd_device_unbind(struct iommufd_device *idev) } EXPORT_SYMBOL_NS_GPL(iommufd_device_unbind, IOMMUFD); +struct iommufd_ctx *iommufd_device_to_ictx(struct iommufd_device *idev) +{ + return idev->ictx; +} +EXPORT_SYMBOL_NS_GPL(iommufd_device_to_ictx, IOMMUFD); + +u32 iommufd_device_to_id(struct iommufd_device *idev) +{ + return idev->obj.id; +} +EXPORT_SYMBOL_NS_GPL(iommufd_device_to_id, IOMMUFD); + static int iommufd_device_setup_msi(struct iommufd_device *idev, struct iommufd_hw_pagetable *hwpt, phys_addr_t sw_msi_start) diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c index 88b00c501015..809f2dd73b9e 100644 --- a/drivers/vfio/iommufd.c +++ b/drivers/vfio/iommufd.c @@ -66,6 +66,20 @@ void vfio_iommufd_unbind(struct vfio_device *vdev) vdev->ops->unbind_iommufd(vdev); } +struct iommufd_ctx *vfio_iommufd_physical_ictx(struct vfio_device *vdev) +{ + if (!vdev->iommufd_device) + return NULL; + return iommufd_device_to_ictx(vdev->iommufd_device); +} +EXPORT_SYMBOL_GPL(vfio_iommufd_physical_ictx); + +void vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id) +{ + if (vdev->iommufd_device) + *id = iommufd_device_to_id(vdev->iommufd_device); +} +EXPORT_SYMBOL_GPL(vfio_iommufd_physical_devid); /* * The physical standard ops mean that the iommufd_device is bound to the * physical device vdev->dev that was provided to vfio_init_group_dev(). Drivers diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h index 1129a36a74c4..ac96df406833 100644 --- a/include/linux/iommufd.h +++ b/include/linux/iommufd.h @@ -24,6 +24,9 @@ void iommufd_device_unbind(struct iommufd_device *idev); int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id); void iommufd_device_detach(struct iommufd_device *idev); +struct iommufd_ctx *iommufd_device_to_ictx(struct iommufd_device *idev); +u32 iommufd_device_to_id(struct iommufd_device *idev); + struct iommufd_access_ops { u8 needs_pin_pages : 1; void (*unmap)(void *data, unsigned long iova, unsigned long length); diff --git a/include/linux/vfio.h b/include/linux/vfio.h index 3188d8a374bd..97a1174b922f 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -113,6 +113,8 @@ struct vfio_device_ops { }; #if IS_ENABLED(CONFIG_IOMMUFD) +struct iommufd_ctx *vfio_iommufd_physical_ictx(struct vfio_device *vdev); +void vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id); int vfio_iommufd_physical_bind(struct vfio_device *vdev, struct iommufd_ctx *ictx, u32 *out_device_id); void vfio_iommufd_physical_unbind(struct vfio_device *vdev); @@ -122,6 +124,17 @@ int vfio_iommufd_emulated_bind(struct vfio_device *vdev, void vfio_iommufd_emulated_unbind(struct vfio_device *vdev); int vfio_iommufd_emulated_attach_ioas(struct vfio_device *vdev, u32 *pt_id); #else +static inline struct iommufd_ctx * +vfio_iommufd_physical_ictx(struct vfio_device *vdev) +{ + return NULL; +} + +static inline void +vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id) +{ +} + #define vfio_iommufd_physical_bind \ ((int (*)(struct vfio_device *vdev, struct iommufd_ctx *ictx, \ u32 *out_device_id)) NULL)