linux-kernel - Re: [PATCH v5 08/14] iommufd/viommu: Add iommufd_viommu_report

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250121183611.GY5556@nvidia.com>
Date: Tue, 21 Jan 2025 14:36:11 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: kevin.tian@...el.com, corbet@....net, will@...nel.org, joro@...tes.org,
	suravee.suthikulpanit@....com, robin.murphy@....com,
	dwmw2@...radead.org, baolu.lu@...ux.intel.com, shuah@...nel.org,
	linux-kernel@...r.kernel.org, iommu@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org,
	linux-kselftest@...r.kernel.org, linux-doc@...r.kernel.org,
	eric.auger@...hat.com, jean-philippe@...aro.org, mdf@...nel.org,
	mshavit@...gle.com, shameerali.kolothum.thodi@...wei.com,
	smostafa@...gle.com, ddutile@...hat.com, yi.l.liu@...el.com,
	patches@...ts.linux.dev
Subject: Re: [PATCH v5 08/14] iommufd/viommu: Add iommufd_viommu_report_event
 helper

On Mon, Jan 20, 2025 at 12:52:09PM -0800, Nicolin Chen wrote:
> The counter of the number of events in the vEVENTQ could decrease
> when userspace reads the queue. But you were saying "the number of
> events that were sent into the queue", which is like a PROD index
> that would keep growing but reset to 0 after UINT_MAX?

yes

> > IOMMU_VEVENTQ_STATE_OVERFLOW with a 0 length event is seen if events
> > have been lost and no subsequent events are present. It exists to
> > ensure timely delivery of the overflow event to userspace. counter
> > will be the sequence number of the next successful event.
> 
> So userspace should first read the header to decide whether or not
> to read a vEVENT. If header is overflows, it should skip the vEVENT
> struct and read the next header?

Yes, but there won't be a next header. overflow would always be the
last thing in a read() response. If there is another event then
overflow is indicated by non-monotonic count.

> > If events are lost in the middle of the queue then flags will remain 0
> > but counter will become non-montonic. A counter delta > 1 indicates
> > that many events have been lost.
> 
> I don't quite get the "no subsequent events" v.s. "in the middle of
> the queue"..

I mean to supress specific overflow events to userspace if the counter already
fully indicates overflow.

The purpose of the overflow event is specifically, and only, to
indicate immediately that an overflow occured at the end of the queue,
and no additional events have been pushed since the overflow.

Without this we could loose an event and userspace may not realize
it for a long time.

> The producer is the driver calling iommufd_viommu_report_event that
> only produces a single vEVENT at a time. When the number of vEVENTs
> in the vEVENTQ hits the @veventq_depth, it won't insert new vEVENTs
> but add an overflow (or exception) node to the head of deliver list
> and increase the producer index so the next vEVENT that can find an
> empty space in the queue will have an index with a gap (delta >= 1)?

Yes, but each new overflow should move the single preallocated
overflow node back to the end of the list, and the read side should
skip the overflow node if it is not the last entry in the list

Jason