[PATCH v2] PCI/ATS: Disable ATS when ACS TB is set upstream

[PATCH v2] PCI/ATS: Disable ATS when ACS TB is set upstream

[Danny Gonzalez]

Enabling ATS allows an endpoint to cache translations and initiate DMA
requests with Translated addresses, which an upstream bridge with ACS
Translation Blocking (PCI_ACS_TB) set will block as ACS Violations.
Because pci_enable_acs() preserves existing PCI_ACS_CTRL bits (e.g., from
firmware, a kexec kernel, or "pci=config_acs=") without relaxing them,
PCI_ACS_TB may remain set on upstream bridges.

Add pci_ats_check_acs() and call it from pci_dma_configure() after
pci_enable_acs() to walk upstream bridges and clear dev->ats_cap if any
bridge has PCI_ACS_TB enabled, leaving the endpoint to use Untranslated
addresses.

Tested by booting a machine with an endpoint advertising the ATS Capability
behind a Root Port with PCI_ACS_TB set; verified ATS is disabled and the
system boots cleanly.

Signed-off-by: Danny Gonzalez <digonzal@google.com>
---

v2 (based on sashiko review):
 - Move the upstream ACS TB check from pci_ats_supported() into
   pci_ats_check_acs() called after pci_enable_acs()
 - Use pci_acs_enabled() instead of reading PCI_ACS_CTRL directly

 drivers/pci/ats.c        | 20 ++++++++++++++++++++
 drivers/pci/pci-driver.c |  1 +
 drivers/pci/pci.h        |  2 ++
 3 files changed, 23 insertions(+)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 9cb23780093d..0395612f3987 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -31,6 +31,26 @@ void pci_ats_init(struct pci_dev *dev)
 	dev->ats_cap = pos;
 }
 
+void pci_ats_check_acs(struct pci_dev *dev)
+{
+	struct pci_dev *bridge;
+
+	if (!dev->ats_cap || dev->is_virtfn || dev->ats_stu)
+		return;
+
+	/* Do not allow ATS if any upstream bridge blocks translation */
+	for (bridge = pci_upstream_bridge(dev); bridge;
+	     bridge = pci_upstream_bridge(bridge)) {
+		if ((bridge->acs_capabilities & PCI_ACS_TB) &&
+		    pci_acs_enabled(bridge, PCI_ACS_TB)) {
+			pci_warn_once(dev, "ATS disabled: upstream bridge %s has ACS Translation Blocking enabled\n",
+				 pci_name(bridge));
+			dev->ats_cap = 0;
+			return;
+		}
+	}
+}
+
 /**
  * pci_ats_supported - check if the device can use ATS
  * @dev: the PCI device
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e16aa59dd7ac..9a1856def5bd 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1713,6 +1713,7 @@ static int pci_dma_configure(struct device *dev)
 	 * quirks.
 	 */
 	pci_enable_acs(to_pci_dev(dev));
+	pci_ats_check_acs(to_pci_dev(dev));
 
 	pci_put_host_bridge_device(bridge);
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index ba3c3fddddc2..cf2c93761d81 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -961,9 +961,11 @@ static inline void pcie_walk_rcec(struct pci_dev *rcec,
 #ifdef CONFIG_PCI_ATS
 /* Address Translation Service */
 void pci_ats_init(struct pci_dev *dev);
+void pci_ats_check_acs(struct pci_dev *dev);
 void pci_restore_ats_state(struct pci_dev *dev);
 #else
 static inline void pci_ats_init(struct pci_dev *d) { }
+static inline void pci_ats_check_acs(struct pci_dev *d) { }
 static inline void pci_restore_ats_state(struct pci_dev *dev) { }
 #endif /* CONFIG_PCI_ATS */
 
-- 
2.56.0.313.g2479322e68-goog

Re: [PATCH v2] PCI/ATS: Disable ATS when ACS TB is set upstream

[Bjorn Helgaas]

[+cc other folks interested in ATS]

On Wed, Oct 28, 2026 at 11:36:51PM +0000, Danny Gonzalez wrote:
> Enabling ATS allows an endpoint to cache translations and initiate DMA
> requests with Translated addresses, which an upstream bridge with ACS
> Translation Blocking (PCI_ACS_TB) set will block as ACS Violations.
> Because pci_enable_acs() preserves existing PCI_ACS_CTRL bits (e.g., from
> firmware, a kexec kernel, or "pci=config_acs=") without relaxing them,
> PCI_ACS_TB may remain set on upstream bridges.
> 
> Add pci_ats_check_acs() and call it from pci_dma_configure() after
> pci_enable_acs() to walk upstream bridges and clear dev->ats_cap if any
> bridge has PCI_ACS_TB enabled, leaving the endpoint to use Untranslated
> addresses.

> Tested by booting a machine with an endpoint advertising the ATS Capability
> behind a Root Port with PCI_ACS_TB set; verified ATS is disabled and the
> system boots cleanly.
> 
> Signed-off-by: Danny Gonzalez <digonzal@google.com>
> ---
> 
> v2 (based on sashiko review):
>  - Move the upstream ACS TB check from pci_ats_supported() into
>    pci_ats_check_acs() called after pci_enable_acs()
>  - Use pci_acs_enabled() instead of reading PCI_ACS_CTRL directly
> 
>  drivers/pci/ats.c        | 20 ++++++++++++++++++++
>  drivers/pci/pci-driver.c |  1 +
>  drivers/pci/pci.h        |  2 ++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index 9cb23780093d..0395612f3987 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -31,6 +31,26 @@ void pci_ats_init(struct pci_dev *dev)
>  	dev->ats_cap = pos;
>  }
>  
> +void pci_ats_check_acs(struct pci_dev *dev)
> +{
> +	struct pci_dev *bridge;
> +
> +	if (!dev->ats_cap || dev->is_virtfn || dev->ats_stu)
> +		return;
> +
> +	/* Do not allow ATS if any upstream bridge blocks translation */
> +	for (bridge = pci_upstream_bridge(dev); bridge;
> +	     bridge = pci_upstream_bridge(bridge)) {
> +		if ((bridge->acs_capabilities & PCI_ACS_TB) &&
> +		    pci_acs_enabled(bridge, PCI_ACS_TB)) {
> +			pci_warn_once(dev, "ATS disabled: upstream bridge %s has ACS Translation Blocking enabled\n",
> +				 pci_name(bridge));
> +			dev->ats_cap = 0;
> +			return;
> +		}
> +	}

This works by making pci_ats_supported() return "false".  Can we do
this checking inside pci_ats_supported() instead?  Then we wouldn't
need a new function, and I think it would be easier to understand the
control flow that way.

> +}
> +
>  /**
>   * pci_ats_supported - check if the device can use ATS
>   * @dev: the PCI device
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index e16aa59dd7ac..9a1856def5bd 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1713,6 +1713,7 @@ static int pci_dma_configure(struct device *dev)
>  	 * quirks.
>  	 */
>  	pci_enable_acs(to_pci_dev(dev));
> +	pci_ats_check_acs(to_pci_dev(dev));
>  	pci_put_host_bridge_device(bridge);
>  
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index ba3c3fddddc2..cf2c93761d81 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -961,9 +961,11 @@ static inline void pcie_walk_rcec(struct pci_dev *rcec,
>  #ifdef CONFIG_PCI_ATS
>  /* Address Translation Service */
>  void pci_ats_init(struct pci_dev *dev);
> +void pci_ats_check_acs(struct pci_dev *dev);
>  void pci_restore_ats_state(struct pci_dev *dev);
>  #else
>  static inline void pci_ats_init(struct pci_dev *d) { }
> +static inline void pci_ats_check_acs(struct pci_dev *d) { }
>  static inline void pci_restore_ats_state(struct pci_dev *dev) { }
>  #endif /* CONFIG_PCI_ATS */
>  
> -- 
> 2.56.0.313.g2479322e68-goog
> 

Re: [PATCH v2] PCI/ATS: Disable ATS when ACS TB is set upstream

[Jason Gunthorpe]

On Tue, Nov 03, 2026 at 12:16:16PM -0600, Bjorn Helgaas wrote:
> [+cc other folks interested in ATS]
> 
> On Wed, Oct 28, 2026 at 11:36:51PM +0000, Danny Gonzalez wrote:
> > Enabling ATS allows an endpoint to cache translations and initiate DMA
> > requests with Translated addresses, which an upstream bridge with ACS
> > Translation Blocking (PCI_ACS_TB) set will block as ACS Violations.
> > Because pci_enable_acs() preserves existing PCI_ACS_CTRL bits (e.g., from
> > firmware, a kexec kernel, or "pci=config_acs=") without relaxing them,
> > PCI_ACS_TB may remain set on upstream bridges.
> > 
> > Add pci_ats_check_acs() and call it from pci_dma_configure() after
> > pci_enable_acs() to walk upstream bridges and clear dev->ats_cap if any
> > bridge has PCI_ACS_TB enabled, leaving the endpoint to use Untranslated
> > addresses.
> 
> > Tested by booting a machine with an endpoint advertising the ATS Capability
> > behind a Root Port with PCI_ACS_TB set; verified ATS is disabled and the
> > system boots cleanly.
> > 
> > Signed-off-by: Danny Gonzalez <digonzal@google.com>
> > ---
> > 
> > v2 (based on sashiko review):
> >  - Move the upstream ACS TB check from pci_ats_supported() into
> >    pci_ats_check_acs() called after pci_enable_acs()
> >  - Use pci_acs_enabled() instead of reading PCI_ACS_CTRL directly
> > 
> >  drivers/pci/ats.c        | 20 ++++++++++++++++++++
> >  drivers/pci/pci-driver.c |  1 +
> >  drivers/pci/pci.h        |  2 ++
> >  3 files changed, 23 insertions(+)
> > 
> > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> > index 9cb23780093d..0395612f3987 100644
> > --- a/drivers/pci/ats.c
> > +++ b/drivers/pci/ats.c
> > @@ -31,6 +31,26 @@ void pci_ats_init(struct pci_dev *dev)
> >  	dev->ats_cap = pos;
> >  }
> >  
> > +void pci_ats_check_acs(struct pci_dev *dev)
> > +{
> > +	struct pci_dev *bridge;
> > +
> > +	if (!dev->ats_cap || dev->is_virtfn || dev->ats_stu)
> > +		return;
> > +
> > +	/* Do not allow ATS if any upstream bridge blocks translation */
> > +	for (bridge = pci_upstream_bridge(dev); bridge;
> > +	     bridge = pci_upstream_bridge(bridge)) {
> > +		if ((bridge->acs_capabilities & PCI_ACS_TB) &&
> > +		    pci_acs_enabled(bridge, PCI_ACS_TB)) {
> > +			pci_warn_once(dev, "ATS disabled: upstream bridge %s has ACS Translation Blocking enabled\n",
> > +				 pci_name(bridge));
> > +			dev->ats_cap = 0;
> > +			return;

Isn't this pci_acs_path_enabled() ?

> This works by making pci_ats_supported() return "false".  Can we do
> this checking inside pci_ats_supported() instead?  Then we wouldn't
> need a new function, and I think it would be easier to understand the
> control flow that way.

Yeah, and I don't think this is great:

> > @@ -1713,6 +1713,7 @@ static int pci_dma_configure(struct device *dev)
> >  	 * quirks.
> >  	 */
> >  	pci_enable_acs(to_pci_dev(dev));
> > +	pci_ats_check_acs(to_pci_dev(dev));
> >  	pci_put_host_bridge_device(bridge);

We shouldn't have to leak these details outside the PCI core..

The idea seems sound at least.

Jason