Message ID | 3d952137a7935608f9cc6b05bd561a58a0c5da16.1737754129.git.nicolinc@nvidia.com |
---|---|
State | Superseded |
Headers | show |
Series | iommufd: Add vIOMMU infrastructure (Part-3: vEVENTQ) | expand |
> 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>
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
> 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.)
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
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 --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");