diff mbox series

[v6,07/14] iommufd/viommu: Add iommufd_viommu_report_event helper

Message ID 3d952137a7935608f9cc6b05bd561a58a0c5da16.1737754129.git.nicolinc@nvidia.com
State Superseded
Headers show
Series iommufd: Add vIOMMU infrastructure (Part-3: vEVENTQ) | expand

Commit Message

Nicolin Chen Jan. 25, 2025, 12:30 a.m. UTC 
Similar to iommu_report_device_fault, this allows IOMMU drivers to report
vIOMMU events from threaded IRQ handlers to user space hypervisors.

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 include/linux/iommufd.h        | 11 +++++++++
 drivers/iommu/iommufd/driver.c | 45 ++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

Comments

Tian, Kevin Feb. 18, 2025, 5:14 a.m. UTC | #1 
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Saturday, January 25, 2025 8:31 AM
> 
> Similar to iommu_report_device_fault, this allows IOMMU drivers to report
> vIOMMU events from threaded IRQ handlers to user space hypervisors.
> 
> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jason Gunthorpe Feb. 18, 2025, 3:35 p.m. UTC | #2 
On Fri, Jan 24, 2025 at 04:30:36PM -0800, Nicolin Chen wrote:
> +int iommufd_viommu_report_event(struct iommufd_viommu *viommu,
> +				enum iommu_veventq_type type, void *event_data,
> +				size_t data_len)
> +{
> +	struct iommufd_veventq *veventq;
> +	struct iommufd_vevent *vevent;
> +	int rc = 0;
> +
> +	if (WARN_ON_ONCE(!data_len || !event_data))
> +		return -EINVAL;
> +
> +	down_read(&viommu->veventqs_rwsem);
> +
> +	veventq = iommufd_viommu_find_veventq(viommu, type);
> +	if (!veventq) {
> +		rc = -EOPNOTSUPP;
> +		goto out_unlock_veventqs;
> +	}
> +
> +	if (atomic_read(&veventq->num_events) == veventq->depth) {
> +		vevent = &veventq->overflow;
> +		goto out_set_header;
> +	}
> +
> +	vevent = kmalloc(struct_size(vevent, event_data, data_len), GFP_KERNEL);
> +	if (!vevent) {
> +		rc = -ENOMEM;
> +		goto out_unlock_veventqs;

This should record an overflow too

Jason
Tian, Kevin Feb. 19, 2025, 6:58 a.m. UTC | #3 
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Tuesday, February 18, 2025 11:36 PM
> 
> On Fri, Jan 24, 2025 at 04:30:36PM -0800, Nicolin Chen wrote:
> > +int iommufd_viommu_report_event(struct iommufd_viommu *viommu,
> > +				enum iommu_veventq_type type, void
> *event_data,
> > +				size_t data_len)
> > +{
> > +	struct iommufd_veventq *veventq;
> > +	struct iommufd_vevent *vevent;
> > +	int rc = 0;
> > +
> > +	if (WARN_ON_ONCE(!data_len || !event_data))
> > +		return -EINVAL;
> > +
> > +	down_read(&viommu->veventqs_rwsem);
> > +
> > +	veventq = iommufd_viommu_find_veventq(viommu, type);
> > +	if (!veventq) {
> > +		rc = -EOPNOTSUPP;
> > +		goto out_unlock_veventqs;
> > +	}
> > +
> > +	if (atomic_read(&veventq->num_events) == veventq->depth) {
> > +		vevent = &veventq->overflow;
> > +		goto out_set_header;
> > +	}
> > +
> > +	vevent = kmalloc(struct_size(vevent, event_data, data_len),
> GFP_KERNEL);
> > +	if (!vevent) {
> > +		rc = -ENOMEM;
> > +		goto out_unlock_veventqs;
> 
> This should record an overflow too
> 

In that case probably we want to rename 'overflow' to 'lost_event'
which counts lost events for whatever reasons (overflow, oom, etc.)
Nicolin Chen Feb. 20, 2025, 9:16 p.m. UTC | #4 
On Wed, Feb 19, 2025 at 06:58:16AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Tuesday, February 18, 2025 11:36 PM
> > 
> > On Fri, Jan 24, 2025 at 04:30:36PM -0800, Nicolin Chen wrote:
> > > +int iommufd_viommu_report_event(struct iommufd_viommu *viommu,
> > > +				enum iommu_veventq_type type, void
> > *event_data,
> > > +				size_t data_len)
> > > +{
> > > +	struct iommufd_veventq *veventq;
> > > +	struct iommufd_vevent *vevent;
> > > +	int rc = 0;
> > > +
> > > +	if (WARN_ON_ONCE(!data_len || !event_data))
> > > +		return -EINVAL;
> > > +
> > > +	down_read(&viommu->veventqs_rwsem);
> > > +
> > > +	veventq = iommufd_viommu_find_veventq(viommu, type);
> > > +	if (!veventq) {
> > > +		rc = -EOPNOTSUPP;
> > > +		goto out_unlock_veventqs;
> > > +	}
> > > +
> > > +	if (atomic_read(&veventq->num_events) == veventq->depth) {
> > > +		vevent = &veventq->overflow;
> > > +		goto out_set_header;
> > > +	}
> > > +
> > > +	vevent = kmalloc(struct_size(vevent, event_data, data_len),
> > GFP_KERNEL);
> > > +	if (!vevent) {
> > > +		rc = -ENOMEM;
> > > +		goto out_unlock_veventqs;
> > 
> > This should record an overflow too
> > 
> 
> In that case probably we want to rename 'overflow' to 'lost_event'
> which counts lost events for whatever reasons (overflow, oom, etc.)

Naming-wise, I think it may sound better to call the overflow node
just an 'exception' that concludes with lost eventsfor different
reasons.

A complication is that this 'exception' would give userspace a very
implicit reason as the node just report the number of lost events
v.s. providing the reason to each lost event.

With this, maybe the IOMMU_VEVENTQ_FLAG_OVERFLOW in the uAPI should
be just IOMMU_VEVENTQ_FLAG_EXCEPTION? Any better idea?

Thanks
Nicolin
Jason Gunthorpe Feb. 21, 2025, 1:39 p.m. UTC | #5 
On Fri, Feb 21, 2025 at 04:27:32AM +0000, Tian, Kevin wrote:
> With that in mind we don't need provide reason for the static node,
> and IOMMU_EVENTQ_FLAG_LOST_EVENTS sounds closer to the
> real intention.

I also like lost events - it is simple and to the point

Jason
diff mbox series

Patch

diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
index 05cb393aff0a..60eff9272551 100644
--- a/include/linux/iommufd.h
+++ b/include/linux/iommufd.h
@@ -11,6 +11,7 @@ 
 #include <linux/refcount.h>
 #include <linux/types.h>
 #include <linux/xarray.h>
+#include <uapi/linux/iommufd.h>
 
 struct device;
 struct file;
@@ -192,6 +193,9 @@  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);
+int iommufd_viommu_report_event(struct iommufd_viommu *viommu,
+				enum iommu_veventq_type type, void *event_data,
+				size_t data_len);
 #else /* !CONFIG_IOMMUFD_DRIVER_CORE */
 static inline struct iommufd_object *
 _iommufd_object_alloc(struct iommufd_ctx *ictx, size_t size,
@@ -212,6 +216,13 @@  static inline int iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
 {
 	return -ENOENT;
 }
+
+static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu,
+					      enum iommu_veventq_type type,
+					      void *event_data, size_t data_len)
+{
+	return -EOPNOTSUPP;
+}
 #endif /* CONFIG_IOMMUFD_DRIVER_CORE */
 
 /*
diff --git a/drivers/iommu/iommufd/driver.c b/drivers/iommu/iommufd/driver.c
index 185c4fde8987..9e52ce66204c 100644
--- a/drivers/iommu/iommufd/driver.c
+++ b/drivers/iommu/iommufd/driver.c
@@ -73,5 +73,50 @@  int iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
 }
 EXPORT_SYMBOL_NS_GPL(iommufd_viommu_get_vdev_id, "IOMMUFD");
 
+/*
+ * Typically called in driver's threaded IRQ handler.
+ * The @type and @event_data must be defined in include/uapi/linux/iommufd.h
+ */
+int iommufd_viommu_report_event(struct iommufd_viommu *viommu,
+				enum iommu_veventq_type type, void *event_data,
+				size_t data_len)
+{
+	struct iommufd_veventq *veventq;
+	struct iommufd_vevent *vevent;
+	int rc = 0;
+
+	if (WARN_ON_ONCE(!data_len || !event_data))
+		return -EINVAL;
+
+	down_read(&viommu->veventqs_rwsem);
+
+	veventq = iommufd_viommu_find_veventq(viommu, type);
+	if (!veventq) {
+		rc = -EOPNOTSUPP;
+		goto out_unlock_veventqs;
+	}
+
+	if (atomic_read(&veventq->num_events) == veventq->depth) {
+		vevent = &veventq->overflow;
+		goto out_set_header;
+	}
+
+	vevent = kmalloc(struct_size(vevent, event_data, data_len), GFP_KERNEL);
+	if (!vevent) {
+		rc = -ENOMEM;
+		goto out_unlock_veventqs;
+	}
+	memcpy(vevent->event_data, event_data, data_len);
+	vevent->data_len = data_len;
+	atomic_inc(&veventq->num_events);
+
+out_set_header:
+	iommufd_vevent_handler(veventq, vevent);
+out_unlock_veventqs:
+	up_read(&viommu->veventqs_rwsem);
+	return rc;
+}
+EXPORT_SYMBOL_NS_GPL(iommufd_viommu_report_event, "IOMMUFD");
+
 MODULE_DESCRIPTION("iommufd code shared with builtin modules");
 MODULE_LICENSE("GPL");