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

[Nicolin Chen <nicolinc@nvidia.com>]

On Wed, Jan 22, 2025 at 09:33:35AM +0000, Tian, Kevin wrote:
> > From: Nicolin Chen <nicolinc@nvidia.com>
> > Sent: Wednesday, January 22, 2025 3:16 PM
> > 
> > On Tue, Jan 21, 2025 at 08:21:28PM -0400, Jason Gunthorpe wrote:
> > > On Tue, Jan 21, 2025 at 01:40:05PM -0800, Nicolin Chen wrote:
> > > > > There is also the minor detail of what happens if the hypervisor HW
> > > > > queue overflows - I don't know the answer here. It is security
> > > > > concerning since the VM can spam DMA errors at high rate. :|
> > > >
> > > > In my view, the hypervisor queue is the vHW queue for the VM, so
> > > > it should act like a HW, which means it's up to the guest kernel
> > > > driver that handles the high rate DMA errors..
> > >
> > > I'm mainly wondering what happens if the single physical kernel
> > > event queue overflows because it is DOS'd by a VM and the hypervisor
> > > cannot drain it fast enough?
> > >
> > > I haven't looked closely but is there some kind of rate limiting or
> > > otherwise to mitigate DOS attacks on the shared event queue from VMs?
> > 
> > SMMUv3 reads the event out of the physical kernel event queue,
> > and adds that to faultq or veventq or prints it out. So, it'd
> > not overflow because of DOS? And all other drivers should do
> > the same?
> > 
> 
> "add that to faultq or eventq" could take time or the irqthread
> could be preempted for various reasons then there is always an
> window within which an overflow condition could occur due to
> the smmu driver incapable of fetching pending events timely.

Oh, I see..

> On VT-d the driver could disable reporting non-recoverable fault
> for a given device via a control bit in the PASID entry, but I didn't
> see a similar knob for PRQ.

ARM has an event suppressing CD.R bit to disable event recording
for a device. However, the stage-1 CD is controlled by the guest
kernel or VMM having the control of the ram..

ARM seems to also have an interesting event merging feature:
 STE.MEV, bit [83]
 Merge Events arising from terminated transactions from this stream.
   0b0 Do not merge similar fault records
   0b1 Permit similar fault records to be merged
 The SMMU might be able to reduce the usage of the Event queue by
 coalescing fault records that share the same page granule of address,
 access type and SubstreamID.
   Setting MEV == 1 does not guarantee that faults will be coalesced.
   Setting MEV == 0 causes a physical SMMU to prevent coalescing of
   fault records, however, a hypervisor might not honour this setting
   if it deems a guest to be too verbose.
 Note: Software must expect, and be able to deal with, coalesced fault
 records even when MEV == 0.

Yet, the driver doesn't seem to care setting it at this moment.

Thanks
Nicolin