diff mbox series

PCI/ACS: Enforce pci=noats with Transaction Blocking

Message ID 162404966325.2362347.12176138291577486015.stgit@omen (mailing list archive)
State Accepted
Commit f8bd7c48ecb0e94d2b2cf1b21385ba27f0cd15d9
Delegated to: Bjorn Helgaas
Headers show
Series PCI/ACS: Enforce pci=noats with Transaction Blocking | expand

Commit Message

Alex Williamson June 18, 2021, 8:55 p.m. UTC
PCIe Address Translation Services (ATS) provides a mechanism for a
device to provide an on-device caching translation agent (device
iotlb).  We already have a means to disable support for this feature
via the pci=noats option.  For untrusted and externally facing
devices, we not only disable ATS support for the device, but we use
Access Control Services (ACS) Transaction Blocking to actively
prevent devices from sending TLPs with non-default AT field values.

Extend pci=noats to also make use of PCI_ACS_TB so that not only is
ATS disabled at the device, but blocked at the downstream ports.
This provides a means to further lock-down ATS for cases such as
device assignment, where it may not be the hardware configuration of
the device that makes it untrusted, but the driver running on the
device.

Cc: Rajat Jain <rajatja@google.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/pci/pci.c    |    4 ++--
 drivers/pci/quirks.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Alex Williamson Aug. 3, 2021, 6:49 p.m. UTC | #1
Bump.

On Fri, 18 Jun 2021 14:55:14 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> PCIe Address Translation Services (ATS) provides a mechanism for a
> device to provide an on-device caching translation agent (device
> iotlb).  We already have a means to disable support for this feature
> via the pci=noats option.  For untrusted and externally facing
> devices, we not only disable ATS support for the device, but we use
> Access Control Services (ACS) Transaction Blocking to actively
> prevent devices from sending TLPs with non-default AT field values.
> 
> Extend pci=noats to also make use of PCI_ACS_TB so that not only is
> ATS disabled at the device, but blocked at the downstream ports.
> This provides a means to further lock-down ATS for cases such as
> device assignment, where it may not be the hardware configuration of
> the device that makes it untrusted, but the driver running on the
> device.
> 
> Cc: Rajat Jain <rajatja@google.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/pci/pci.c    |    4 ++--
>  drivers/pci/quirks.c |    2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 68f57d86b243..5aa1bb2ddd80 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -915,8 +915,8 @@ static void pci_std_enable_acs(struct pci_dev *dev)
>  	/* Upstream Forwarding */
>  	ctrl |= (cap & PCI_ACS_UF);
>  
> -	/* Enable Translation Blocking for external devices */
> -	if (dev->external_facing || dev->untrusted)
> +	/* Enable Translation Blocking for external devices and noats */
> +	if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
>  		ctrl |= (cap & PCI_ACS_TB);
>  
>  	pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6d74386eadc2..d541076c083a 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5031,7 +5031,7 @@ static int pci_quirk_enable_intel_spt_pch_acs(struct pci_dev *dev)
>  	ctrl |= (cap & PCI_ACS_CR);
>  	ctrl |= (cap & PCI_ACS_UF);
>  
> -	if (dev->external_facing || dev->untrusted)
> +	if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
>  		ctrl |= (cap & PCI_ACS_TB);
>  
>  	pci_write_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, ctrl);
> 
>
Bjorn Helgaas Aug. 3, 2021, 11:28 p.m. UTC | #2
On Fri, Jun 18, 2021 at 02:55:14PM -0600, Alex Williamson wrote:
> PCIe Address Translation Services (ATS) provides a mechanism for a
> device to provide an on-device caching translation agent (device
> iotlb).  We already have a means to disable support for this feature
> via the pci=noats option.  For untrusted and externally facing
> devices, we not only disable ATS support for the device, but we use
> Access Control Services (ACS) Transaction Blocking to actively
> prevent devices from sending TLPs with non-default AT field values.
> 
> Extend pci=noats to also make use of PCI_ACS_TB so that not only is
> ATS disabled at the device, but blocked at the downstream ports.
> This provides a means to further lock-down ATS for cases such as
> device assignment, where it may not be the hardware configuration of
> the device that makes it untrusted, but the driver running on the
> device.
> 
> Cc: Rajat Jain <rajatja@google.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

Applied to pci/virtualization for v5.15, thanks!

> ---
>  drivers/pci/pci.c    |    4 ++--
>  drivers/pci/quirks.c |    2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 68f57d86b243..5aa1bb2ddd80 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -915,8 +915,8 @@ static void pci_std_enable_acs(struct pci_dev *dev)
>  	/* Upstream Forwarding */
>  	ctrl |= (cap & PCI_ACS_UF);
>  
> -	/* Enable Translation Blocking for external devices */
> -	if (dev->external_facing || dev->untrusted)
> +	/* Enable Translation Blocking for external devices and noats */
> +	if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
>  		ctrl |= (cap & PCI_ACS_TB);
>  
>  	pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6d74386eadc2..d541076c083a 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5031,7 +5031,7 @@ static int pci_quirk_enable_intel_spt_pch_acs(struct pci_dev *dev)
>  	ctrl |= (cap & PCI_ACS_CR);
>  	ctrl |= (cap & PCI_ACS_UF);
>  
> -	if (dev->external_facing || dev->untrusted)
> +	if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
>  		ctrl |= (cap & PCI_ACS_TB);
>  
>  	pci_write_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, ctrl);
> 
>
diff mbox series

Patch

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 68f57d86b243..5aa1bb2ddd80 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -915,8 +915,8 @@  static void pci_std_enable_acs(struct pci_dev *dev)
 	/* Upstream Forwarding */
 	ctrl |= (cap & PCI_ACS_UF);
 
-	/* Enable Translation Blocking for external devices */
-	if (dev->external_facing || dev->untrusted)
+	/* Enable Translation Blocking for external devices and noats */
+	if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
 		ctrl |= (cap & PCI_ACS_TB);
 
 	pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6d74386eadc2..d541076c083a 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5031,7 +5031,7 @@  static int pci_quirk_enable_intel_spt_pch_acs(struct pci_dev *dev)
 	ctrl |= (cap & PCI_ACS_CR);
 	ctrl |= (cap & PCI_ACS_UF);
 
-	if (dev->external_facing || dev->untrusted)
+	if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
 		ctrl |= (cap & PCI_ACS_TB);
 
 	pci_write_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, ctrl);