Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Pranjal Shrivastava <praan@google.com>]

On Wed, May 20, 2026 at 01:31:17PM -0300, Jason Gunthorpe wrote:
> On Wed, May 20, 2026 at 04:24:04PM +0000, Pranjal Shrivastava wrote:
> > On Wed, May 20, 2026 at 11:51:42AM -0300, Jason Gunthorpe wrote:
> > > On Wed, May 20, 2026 at 02:24:47PM +0000, Pranjal Shrivastava wrote:
> > > 
> > > > However, I'm thinking what's the right thing to do if pci_enable_ats() 
> > > > fails in attach_commit():
> > > > 
> > > > 1. Fail to attach but unmerging the invs_array entry is complicated.
> > > 
> > > Is it? We should be able to keep the the original invs and not free
> > > it, just put the pointer back?
> > 
> > Reverting the invs pointer itself is straightforward, but IIUC the 
> > current implementation of arm_smmu_install_new_domain_invs() is a
> > destructive update as it calls kfree_rcu() on the old list immediately
> > after the swap.
> > 
> > Although, I guess we could refactor the RCU lifecycle to defer the 
> > kfree_rcu until after pci_enable_ats() succeeds in the commit phase?
> 
> Yeah..
> 
> Though really pci_enable_ats is not working the way we want, writing
> the enable register can't actually fail - it is all the sanity
> checking for kernel bugs that is the problem here.
> 
> How about call pci_ats_supported() early in attach and fail, then if
> pci_enable_ats() fails just WARN_ON and keep going, kernel bug.
> 
> Re-organize pci_enable_ats() so it relies on pci_ats_supported() for
> all the sanity checks and cannot fail if supported is true.

Hmm, IIUC, the current situation says: 

pci_ats_supported ==> ATS cap check
pci_prepare_ats ==> ATS enablement prep 

Thus, I feel if the prepare fails, we should expect enable to fail. But
if prepare succeeds but enable didn't, that's the place we should
WARN_ON and move on.. because we (IOMMU) did the best we could have..

With the updated variants of pci_ats_support / pci_prepare_ats (in
patches 1 & 2), I believe we cover all the cases now..

And we plan to call prepare_ats only if (pci_ats_supported == true)

Thus, if prepare_ats failed, we fail at probe. And if probe succeeds but
enable ATS failed, that's something the IOMMU can't help with, hence we
WARN and move on? Does that sound good?

Thanks,
Praan