Re: [PATCH rc v2] iommu/arm-smmu-v3: Fix inconsistent ATS state tracking

[Nicolin Chen <nicolinc@nvidia.com>]

On Mon, May 04, 2026 at 07:33:07PM +0000, Pranjal Shrivastava wrote:
> On Mon, May 04, 2026 at 11:01:42AM -0700, Nicolin Chen wrote:
> > On Mon, May 04, 2026 at 04:38:42PM +0000, Pranjal Shrivastava wrote:
> > I am thinking, maybe the call sites of pci_enable/disable_ats() can
> > check to_pci_dev(dev)->ats_enabled instead of master->ats_enabled?
> > 
> > Then, we keep master->ats_enabled as-is, so detach() can revert the
> > nr_ats_masters and ATS invalidation entry in domain->invs.
> 
> My suggestion in that case would be to update arm_smmu_ats_supported to
> check if the client is a VF and check if it's PF enables ATS:
> 
> static bool arm_smmu_ats_supported(struct arm_smmu_master *master)
> {
>     struct device *dev = master->dev;
>     struct pci_dev *pdev;
> 
>     if (!dev_is_pci(dev))
>         return false;
> 
>     pdev = to_pci_dev(dev);
>     if (!pci_ats_supported(pdev))
>         return false;
> 
>     /*
>      * If this is a VF, ATS only works if the PF has already enabled it
>      * with a valid STU.
>      */
>     if (pdev->is_virtfn && !pci_physfn(pdev)->ats_enabled)
>         return false;
> 
>     return true;
> }

I think that's okay to address the issue for now. But it assumes
that pci_enable_ats() will never change so it won't fail for any
other reason.

> and then in attach_prepare we can add the ats check for safety:
> 
>         if (!state->ats_enabled && master->ats_enabled) {
>                 pci_disable_ats(to_pci_dev(master->dev));
> 
> +               if (pdev->ats_enabled)
> +                       pci_disable_ats(to_pci_dev(master->dev));

master->ats_enabled is redundant if we check pdev->ats_enabled.

If we add a gate here, should add to pci_enable_ats() too?

Nicolin