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

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, May 19, 2026 at 07:23:35PM -0300, Jason Gunthorpe wrote:
> On Tue, May 19, 2026 at 02:36:49PM -0500, Bjorn Helgaas wrote:
> > 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.
> 
> It works like the acs quirks that are in the quirks file, which are
> also arguably only used by iommu too :)

True, although ACS has a lot more PCI-specific grunge in it, including
all the "pci=config_acs" and "pci=disable_acs_redir" stuff.

> I'm not keen on spreading lists of device ids for PCI quirks to iommu
> files, but it would be OK to move pci_ats_always_on() to
> iommu_ats_always_on() that calls the PCI quirk function.

Yeah, I guess it's fair to collect the device IDs in PCI since this is
about characteristics of the device.

If we leave stuff in drivers/pci/, I would prefer that part of it be
named to be purely informational, i.e., "CXL.cache_enabled" or
something similar that would also cover the NVIDIA devices.

"pci_ats_always_on()" doesn't sound right quite to me because it
presupposes the policy choice that IOMMU is going to make; that PCI
function doesn't actually turn ATS on, and it looks like the question
of enabling ATS depends on how the device is actually *used*.  E.g.,
if Cache_Enable is not set, is ATS required?

That raises the question of whether this is the right test:

  +   if (pci_read_config_word(pdev, offset + PCI_DVSEC_CXL_CAP, &cap))
  +           return false;
  +
  +   return cap & PCI_DVSEC_CXL_CACHE_CAPABLE;

That just says the device is *capable* of CXL.cache; should it check
whether CXL.cache is *enabled* instead?

Bjorn