Re: [PATCH v6 1/3] PCI: Add pci_ats_required() for CXL.cache capable devices

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, May 21, 2026 at 01:34:20PM -0700, Nicolin Chen wrote:
> Controlled by IOMMU drivers, ATS can be enabled "on demand", when a given
> PASID on a device is attached to an I/O page table. This is working, even
> when a device has no translation on its RID (i.e., RID is IOMMU bypassed).
> 
> However, certain PCIe devices require non-PASID ATS on their RID even when
> the RID is IOMMU bypassed. Call this "ATS always on" in IOMMU term.
> 
> For example, CXL spec r4.0 notes in sec 3.2.5.13 Memory Type on CXL.cache:
>  "To source requests on CXL.cache, devices need to get the Host Physical
>   Address (HPA) from the Host by means of an ATS request on CXL.io."
> 
> In other words, the CXL.cache capability requires ATS; otherwise, it can't
> access host physical memory.
> 
> Introduce a new pci_ats_required() helper for the IOMMU driver to scan a
> PCI device and shift ATS policies between "on demand" and "always on".
> 
> Add the support for CXL.cache devices first. Pre-CXL devices will be added
> in quirks.c file.
> 
> Note that pci_ats_required() validates against pci_ats_supported(), so we
> ensure that untrusted devices (e.g. external ports) will not be always on.
> This maintains the existing ATS security policy regarding potential side-
> channel attacks via ATS.
> 
> Cc: linux-cxl@vger.kernel.org
> Suggested-by: Vikram Sethi <vsethi@nvidia.com>
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Tested-by: Nirmoy Das <nirmoyd@nvidia.com>
> Acked-by: Nirmoy Das <nirmoyd@nvidia.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ...

> +bool pci_ats_required(struct pci_dev *pdev)
> +{
> +	if (!pci_ats_supported(pdev))
> +		return false;
> +
> +	/* A VF inherits its PF's requirement for ATS function */
> +	if (pdev->is_virtfn)
> +		pdev = pci_physfn(pdev);
> +
> +	return pci_cxl_ats_required(pdev);

I acked this before I saw this sashiko feedback, which looks like a
legit issue to me:

  Will this VF inheritance logic ever be reached?

  According to the PCIe SR-IOV specification (section 9.3.3.1), VFs do
  not implement the ATS Extended Capability, which means pdev->ats_cap
  is always 0 for VFs.

  Because of this, pci_ats_supported(pdev) will unconditionally return
  false for any VF. This causes the function to return false before it
  can ever reach the pdev->is_virtfn check.

  Could this prevent VFs from correctly enabling the ATS always on
  feature and leave them unable to access host memory without
  triggering IOMMU faults?

(From https://sashiko.dev/#/patchset/cover.1779304390.git.nicolinc%40nvidia.com)

I withdraw my ack for now until we figure out if it's a real issue.

> +}
> +EXPORT_SYMBOL_GPL(pci_ats_required);
> +
>  #ifdef CONFIG_PCI_PRI
>  void pci_pri_init(struct pci_dev *pdev)
>  {
> -- 
> 2.43.0
>