[PATCH v6 06/14] iommufd/viommu: Add iommufd_viommu_get_vdev_id helper

Nicolin Chen posted 14 patches 1 year ago
There is a newer version of this series
[PATCH v6 06/14] iommufd/viommu: Add iommufd_viommu_get_vdev_id helper
Posted by Nicolin Chen 1 year ago
This is a reverse search v.s. iommufd_viommu_find_dev, as drivers may want
to convert a struct device pointer (physical) to its virtual device ID for
an event injection to the user space VM.

Again, this avoids exposing more core structures to the drivers, than the
iommufd_viommu alone.

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 include/linux/iommufd.h        |  9 +++++++++
 drivers/iommu/iommufd/driver.c | 24 ++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
index 8948b1836940..05cb393aff0a 100644
--- a/include/linux/iommufd.h
+++ b/include/linux/iommufd.h
@@ -190,6 +190,8 @@ struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
 					     enum iommufd_object_type type);
 struct device *iommufd_viommu_find_dev(struct iommufd_viommu *viommu,
 				       unsigned long vdev_id);
+int iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
+			       struct device *dev, unsigned long *vdev_id);
 #else /* !CONFIG_IOMMUFD_DRIVER_CORE */
 static inline struct iommufd_object *
 _iommufd_object_alloc(struct iommufd_ctx *ictx, size_t size,
@@ -203,6 +205,13 @@ iommufd_viommu_find_dev(struct iommufd_viommu *viommu, unsigned long vdev_id)
 {
 	return NULL;
 }
+
+static inline int iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
+					     struct device *dev,
+					     unsigned long *vdev_id)
+{
+	return -ENOENT;
+}
 #endif /* CONFIG_IOMMUFD_DRIVER_CORE */
 
 /*
diff --git a/drivers/iommu/iommufd/driver.c b/drivers/iommu/iommufd/driver.c
index 2d98b04ff1cb..185c4fde8987 100644
--- a/drivers/iommu/iommufd/driver.c
+++ b/drivers/iommu/iommufd/driver.c
@@ -49,5 +49,29 @@ struct device *iommufd_viommu_find_dev(struct iommufd_viommu *viommu,
 }
 EXPORT_SYMBOL_NS_GPL(iommufd_viommu_find_dev, "IOMMUFD");
 
+/* Return -ENOENT if device is not associated to the vIOMMU */
+int iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
+			       struct device *dev, unsigned long *vdev_id)
+{
+	struct iommufd_vdevice *vdev;
+	unsigned long index;
+	int rc = -ENOENT;
+
+	if (WARN_ON_ONCE(!vdev_id))
+		return -EINVAL;
+
+	xa_lock(&viommu->vdevs);
+	xa_for_each(&viommu->vdevs, index, vdev) {
+		if (vdev->dev == dev) {
+			*vdev_id = (unsigned long)vdev->id;
+			rc = 0;
+			break;
+		}
+	}
+	xa_unlock(&viommu->vdevs);
+	return rc;
+}
+EXPORT_SYMBOL_NS_GPL(iommufd_viommu_get_vdev_id, "IOMMUFD");
+
 MODULE_DESCRIPTION("iommufd code shared with builtin modules");
 MODULE_LICENSE("GPL");
-- 
2.43.0
Re: [PATCH v6 06/14] iommufd/viommu: Add iommufd_viommu_get_vdev_id helper
Posted by Jason Gunthorpe 11 months, 3 weeks ago
On Fri, Jan 24, 2025 at 04:30:35PM -0800, Nicolin Chen wrote:
> This is a reverse search v.s. iommufd_viommu_find_dev, as drivers may want
> to convert a struct device pointer (physical) to its virtual device ID for
> an event injection to the user space VM.
> 
> Again, this avoids exposing more core structures to the drivers, than the
> iommufd_viommu alone.
> 
> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>  include/linux/iommufd.h        |  9 +++++++++
>  drivers/iommu/iommufd/driver.c | 24 ++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

> +	xa_lock(&viommu->vdevs);
> +	xa_for_each(&viommu->vdevs, index, vdev) {
> +		if (vdev->dev == dev) {
> +			*vdev_id = (unsigned long)vdev->id;

I don't think we need this cast

Jason
Re: [PATCH v6 06/14] iommufd/viommu: Add iommufd_viommu_get_vdev_id helper
Posted by Nicolin Chen 11 months, 3 weeks ago
On Tue, Feb 18, 2025 at 11:31:54AM -0400, Jason Gunthorpe wrote:
> On Fri, Jan 24, 2025 at 04:30:35PM -0800, Nicolin Chen wrote:
> > This is a reverse search v.s. iommufd_viommu_find_dev, as drivers may want
> > to convert a struct device pointer (physical) to its virtual device ID for
> > an event injection to the user space VM.
> > 
> > Again, this avoids exposing more core structures to the drivers, than the
> > iommufd_viommu alone.
> > 
> > Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> > ---
> >  include/linux/iommufd.h        |  9 +++++++++
> >  drivers/iommu/iommufd/driver.c | 24 ++++++++++++++++++++++++
> >  2 files changed, 33 insertions(+)
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> 
> > +	xa_lock(&viommu->vdevs);
> > +	xa_for_each(&viommu->vdevs, index, vdev) {
> > +		if (vdev->dev == dev) {
> > +			*vdev_id = (unsigned long)vdev->id;
> 
> I don't think we need this cast

The left side is ulong for xarray index, while the right side is
__aligned_u64 for uAPI. Could there be a gcc warning when somebody
builds the kernel having a BITS_PER_LONG=32?

iommufd_vdevice_alloc_ioctl() does test vdev->id against ULONG_MAX
though..

Thanks
Nicolin
Re: [PATCH v6 06/14] iommufd/viommu: Add iommufd_viommu_get_vdev_id helper
Posted by Jason Gunthorpe 11 months, 3 weeks ago
On Wed, Feb 19, 2025 at 09:17:18PM -0800, Nicolin Chen wrote:
> On Tue, Feb 18, 2025 at 11:31:54AM -0400, Jason Gunthorpe wrote:
> > On Fri, Jan 24, 2025 at 04:30:35PM -0800, Nicolin Chen wrote:
> > > This is a reverse search v.s. iommufd_viommu_find_dev, as drivers may want
> > > to convert a struct device pointer (physical) to its virtual device ID for
> > > an event injection to the user space VM.
> > > 
> > > Again, this avoids exposing more core structures to the drivers, than the
> > > iommufd_viommu alone.
> > > 
> > > Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> > > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> > > ---
> > >  include/linux/iommufd.h        |  9 +++++++++
> > >  drivers/iommu/iommufd/driver.c | 24 ++++++++++++++++++++++++
> > >  2 files changed, 33 insertions(+)
> > 
> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> > 
> > > +	xa_lock(&viommu->vdevs);
> > > +	xa_for_each(&viommu->vdevs, index, vdev) {
> > > +		if (vdev->dev == dev) {
> > > +			*vdev_id = (unsigned long)vdev->id;
> > 
> > I don't think we need this cast
> 
> The left side is ulong for xarray index, while the right side is
> __aligned_u64 for uAPI. Could there be a gcc warning when somebody
> builds the kernel having a BITS_PER_LONG=32?

No. The kernel is full of these implicit casts

Jason