Re: [PATCH v5 08/14] iommufd/viommu: Add iommufd_viommu_report_event helper

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Jan 13, 2025 at 12:44:37PM -0800, Nicolin Chen wrote:

> > You'd want to push a special event when the first overflow happens and
> > probably also report a counter so userspace can know how many events
> > got lost.
> 
> How about this:
> 
> enum iommufd_veventq_header_type {
> 	IOMMU_VEVENTQ_HEADER_TYPE_V1,
> };

You don't need another format tag, just describe it in the driver tag.

> enum iommu_hwpt_pgfault_flags {
> 	IOMMU_VEVENT_HEADER_FLAGS_OVERFLOW = (1 << 0),
> };
> 
> struct iommufd_vevent_header_v1 {
> 	__u64 flags;
> 	__u32 num_events;
> 	__u32 num_overflows; // valid if flag_overflow is set
> };

num_overflows is hard, I'd just keep a counter.

> > This seems most robust and simplest to implement..
> > 
> > I think I'd implement it by having a static overflow list entry so no
> > memory allocation is needed and just keep moving that entry to the
> > back of the list every time an event is lost. This way it will cover
> > lost events due to memory outages too
> 
> Could double-adding the same static node to the list happen and
> corrupt the list?

It has to be done carefully, but under a lock you can make this work

> > For old formats like the fault queue you could return EOVERFLOW
> > whenever the sequence number becomes discontiguous or it sees the
> > overflow event..
> 
> So, IOMMU_OBJ_FAULT is safe to return EOVERFLOW via read(), as
> you mentioned that it is self-limited right?

The idea for fault is that we wouldn't hit it, but if we add a limit,
or hit a memory allocation error, then it could happen

Jason