diff mbox series

[v3,07/14] iommufd/viommu: Add iommufd_viommu_get_vdev_id helper

Message ID 21d7e63b97d81d0acf9127418a67efe386787261.1734477608.git.nicolinc@nvidia.com
State New
Headers show
Series iommufd: Add vIOMMU infrastructure (Part-3: vIRQ) | expand

Commit Message

Nicolin Chen Dec. 18, 2024, 5 a.m. UTC 
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.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 include/linux/iommufd.h        |  8 ++++++++
 drivers/iommu/iommufd/driver.c | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

Comments

Baolu Lu Dec. 19, 2024, 2:05 a.m. UTC | #1 
On 12/18/24 13:00, 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.
> 
> Signed-off-by: Nicolin Chen<nicolinc@nvidia.com>
> ---
>   include/linux/iommufd.h        |  8 ++++++++
>   drivers/iommu/iommufd/driver.c | 20 ++++++++++++++++++++
>   2 files changed, 28 insertions(+)
> 
> diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
> index b082676c9e43..ac1f1897d290 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);
> +unsigned long iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
> +					 struct device *dev);

Hi Nicolin,

This series overall looks good to me. But I have a question that might
be irrelevant to this series itself.

The iommufd provides both IOMMUFD_OBJ_DEVICE and IOMMUFD_OBJ_VDEVICE
objects. What is the essential difference between these two from
userspace's perspective? And, which object ID should the IOMMU device
driver provide when reporting other events in the future?

Currently, the IOMMUFD uAPI reports IOMMUFD_OBJ_DEVICE in the page
fault message, and IOMMUFD_OBJ_VDEVICE (if I understand it correctly) in
the vIRQ message. It will be more future-proof if this could be defined
clearly.

Thanks,
baolu
Nicolin Chen Dec. 19, 2024, 5:06 a.m. UTC | #2 
On Thu, Dec 19, 2024 at 10:05:53AM +0800, Baolu Lu wrote:
> On 12/18/24 13:00, 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.
> > 
> > Signed-off-by: Nicolin Chen<nicolinc@nvidia.com>
> > ---
> >   include/linux/iommufd.h        |  8 ++++++++
> >   drivers/iommu/iommufd/driver.c | 20 ++++++++++++++++++++
> >   2 files changed, 28 insertions(+)
> > 
> > diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
> > index b082676c9e43..ac1f1897d290 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);
> > +unsigned long iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
> > +					 struct device *dev);
> 
> Hi Nicolin,
> 
> This series overall looks good to me. But I have a question that might
> be irrelevant to this series itself.
> 
> The iommufd provides both IOMMUFD_OBJ_DEVICE and IOMMUFD_OBJ_VDEVICE
> objects. What is the essential difference between these two from
> userspace's perspective?

A quick answer is an IOMMUFD_OBJ_DEVICE being a host physical
device and an IOMMUFD_OBJ_VDEVICE being an IOMMUFD_OBJ_DEVICE
related to IOMMUFD_OBJ_VIOMMU. Two of them can be seen in two
different layers. May refer to this graph:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/userspace-api/iommufd.rst?h=v6.13-rc3#n150

> And, which object ID should the IOMMU device
> driver provide when reporting other events in the future?
>
> Currently, the IOMMUFD uAPI reports IOMMUFD_OBJ_DEVICE in the page
> fault message, and IOMMUFD_OBJ_VDEVICE (if I understand it correctly) in
> the vIRQ message. It will be more future-proof if this could be defined
> clearly.

A vIRQ is actually reported per-vIOMMU in this design. Although
in the this series the SMMU driver seems to report a per-device
vIRQ, it internally converts the vDEVICE to a virtual device ID
and packs the virtual device ID into a per-vIOMMU event:

+/**
+ * struct iommu_virq_arm_smmuv3 - ARM SMMUv3 Virtual IRQ
+ *                                (IOMMU_VIRQ_TYPE_ARM_SMMUV3)
+ * @evt: 256-bit ARM SMMUv3 Event record, little-endian.
+ *       (Refer to "7.3 Event records" in SMMUv3 HW Spec)
+ *
+ * StreamID field reports a virtual device ID. To receive a virtual IRQ for a
+ * device, a vDEVICE must be allocated via IOMMU_VDEVICE_ALLOC.
+ */
+struct iommu_virq_arm_smmuv3 {
+	__aligned_le64 evt[4];
 };

Thanks
Nicolin
diff mbox series

Patch

diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
index b082676c9e43..ac1f1897d290 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);
+unsigned long iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
+					 struct device *dev);
 #else /* !CONFIG_IOMMUFD_DRIVER_CORE */
 static inline struct iommufd_object *
 _iommufd_object_alloc(struct iommufd_ctx *ictx, size_t size,
@@ -203,6 +205,12 @@  iommufd_viommu_find_dev(struct iommufd_viommu *viommu, unsigned long vdev_id)
 {
 	return NULL;
 }
+
+static inline unsigned long
+iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu, struct device *dev)
+{
+	return 0;
+}
 #endif /* CONFIG_IOMMUFD_DRIVER_CORE */
 
 /*
diff --git a/drivers/iommu/iommufd/driver.c b/drivers/iommu/iommufd/driver.c
index 2d98b04ff1cb..e5d7397c0a6c 100644
--- a/drivers/iommu/iommufd/driver.c
+++ b/drivers/iommu/iommufd/driver.c
@@ -49,5 +49,25 @@  struct device *iommufd_viommu_find_dev(struct iommufd_viommu *viommu,
 }
 EXPORT_SYMBOL_NS_GPL(iommufd_viommu_find_dev, "IOMMUFD");
 
+/* Return 0 if device is not associated to the vIOMMU */
+unsigned long iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
+					 struct device *dev)
+{
+	struct iommufd_vdevice *vdev;
+	unsigned long vdev_id = 0;
+	unsigned long index;
+
+	xa_lock(&viommu->vdevs);
+	xa_for_each(&viommu->vdevs, index, vdev) {
+		if (vdev && vdev->dev == dev) {
+			vdev_id = (unsigned long)vdev->id;
+			break;
+		}
+	}
+	xa_unlock(&viommu->vdevs);
+	return vdev_id;
+}
+EXPORT_SYMBOL_NS_GPL(iommufd_viommu_get_vdev_id, "IOMMUFD");
+
 MODULE_DESCRIPTION("iommufd code shared with builtin modules");
 MODULE_LICENSE("GPL");