Re: [PATCH rc v4 3/4] iommu/arm-smmu-v3: Mark STE EATS safe when computing the update sequence

[Mostafa Saleh <smostafa@google.com>]

On Thu, Dec 18, 2025 at 02:01:29PM -0400, Jason Gunthorpe wrote:
> On Thu, Dec 18, 2025 at 09:32:43AM -0800, Nicolin Chen wrote:
> > > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > > index 12a9669bcc83..a3b29ad20a82 100644
> > > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > > @@ -1095,6 +1095,15 @@ void arm_smmu_get_ste_update_safe(__le64 *safe_bits)
> > > >  	 *  fault records even when MEV == 0.
> > > >  	 */
> > > >  	safe_bits[1] |= cpu_to_le64(STRTAB_STE_1_MEV);
> > > > +
> > > > +	/*
> > > > +	 * EATS is used to reject and control the ATS behavior of the device. If
> > > > +	 * we are changing it away from 0 then we already trust the device to
> > > > +	 * use ATS properly and we have sequenced the device's ATS enable in PCI
> > > > +	 * config space to prevent it from issuing ATS while we are changing
> > > > +	 * EATS.
> > > > +	 */
> > > 
> > > I am not sure about this one, Is it only about trusting the device?
> 
> Yes. The purpose of EATS=0 is to prevent the device from using ATS at
> all - critically including using translated TLPs eg because it is an
> untrusted device and the OS wants to prevent it from attacking the
> system with direct access to physical memory.
> 
> If the device is trusted then once we disable ATS it must stop issuing
> ATS, so the EATS=0 should never trigger a fault.
> 
> > > I’d be worried about cases where we switch domains, that means that
> > > briefly the HW observers EATS=1 while it was not intended, especially
> > > that EATS is in a different DWORD from S2TTB and CDptr. 
> 
> Well, no, it means EATS is enabled a little bit earlier or disabled a
> little bit later, it doesn't mean it was not intended.
> 
> The point is our rules for ATS say that the ATC is empty at this
> moment and the device is not permitted to do any ATS fetches because
> we won't issue any flushes.
> 
> Thus there can be no concurrent ATS traffic and we don't need to
> exactly sequence EATS with the translation.
> 
> With virtualization the hypervisor is still the exclusive owner of ATS
> and guarentees that EATS enable/disable is sequences correctly with
> ATC invalidation.

I see, thanks a lot for the explanation!
I was mainly worried about the virtualization case as the VMM can
influence ATS enable from the vSTE.
Looking at the code in arm-smmu-v3-iommufd and iommufd/device.c, it
seems ATS enable will only change at attach.
Looking into arm_smmu_attach_commit(), I see the ATC is invalidated
after the STE is installed, which means that userspace can transiently
observe the old domain ATC. I think that's fine at the moment because
when binding to a VFIO driver, it will attach to an empty domain.

Thanks,
Mostafa

> 
> > I think the last line that driver controls pci_enable/disable_ats()
> > should justify the whole thing? Are you worried about device still
> > doing ATS after pci_disable_ats()?
> 
> Exactly right, and we can't worry about that because it says the whole
> ATC coherency system is broken.
> 
> Jason