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

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, May 19, 2026 at 06:48:01PM -0500, Bjorn Helgaas wrote:
> 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.

Yeah, that's fair, so let's rename it to 

pci_translated_required()

ie the device requires translated requests to function. This is what
CXL.cache implies (IIRC I was told the spec specifically says this)

Requiring translated requests implies you have to enable ATS in the
system.

> 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?

We have no way to know..
 
> 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?

No, we talked about this with Dan in one of the versions... it is
better to over-enable ATS than under-enable. over-enable at best is a
NOP, or maybe a tiny performance loss, under-enable is a functional
failure.

If the CXL.cache is not enabled right now it could become enabled
later, after the iommu has already called this and made its
choice..

Thus lets not try to be too narrow here..

Thanks,
Jason