Re: [PATCH v4 1/3] PCI: Allow ATS to be always on for CXL.cache capable devices

[Bjorn Helgaas <helgaas@kernel.org>]

On Sun, Apr 26, 2026 at 10:54:00PM -0700, Nicolin Chen wrote:
> Controlled by the IOMMU driver, ATS is usually 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., the RID is IOMMU
> bypassed).
> 
> However, certain PCIe devices require non-PASID ATS on their RID even when
> the RID is IOMMU bypassed. Call this "always on".
> 
> 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_always_on() 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_always_on() 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.

IMO this doesn't really fit in the PCI core.  ats.c encapsulates
discovery and provides interfaces to access the ATS Capability, but
the users of those interfaces are all outside the PCI core.

The decision to enable enable ATS for CXL.cache devices is fine but
it's really an IOMMU usage policy, and I think it should be
implemented in the IOMMU core.  All the pieces needed
(pci_ats_disabled(), pci_ats_supported(), pci_find_dvsec_capability())
are already exported.

One motivation for putting this in the PCI core was to use the quirk
infrastructure, but this series doesn't use any of that.  It doesn't
declare any fixups, e.g., DECLARE_PCI_FIXUP_FINAL, and it doesn't
update any state cached by the PCI core.

> +++ b/include/uapi/linux/pci_regs.h
> @@ -1349,6 +1349,7 @@
>  /* CXL r4.0, 8.1.3: PCIe DVSEC for CXL Device */
>  #define PCI_DVSEC_CXL_DEVICE				0
>  #define  PCI_DVSEC_CXL_CAP				0xA
> +#define   PCI_DVSEC_CXL_CACHE_CAPABLE			_BITUL(0)

This makes good sense, I'm fine with adding
PCI_DVSEC_CXL_CACHE_CAPABLE.

> +bool pci_ats_always_on(struct pci_dev *pdev)
> +{
> +	if (pci_ats_disabled() || !pci_ats_supported(pdev))
> +		return false;

Isn't this the same as:

  if (!pci_ats_supported(pdev))
    return false;

If pci_ats_disabled(), dev->ats_cap should be zero, so
pci_ats_supported() should always return false.

> +
> +	/* A VF inherits its PF's requirement for ATS function */
> +	if (pdev->is_virtfn)
> +		pdev = pci_physfn(pdev);
> +
> +	return pci_cxl_ats_always_on(pdev);
> +}
> +EXPORT_SYMBOL_GPL(pci_ats_always_on);