Re: [PATCH v4 1/2] PCI: Enable the enhanced ACS controls introduced by PCI_ACS_ECAP
From: Wei Wang
Date: Tue Feb 10 2026 - 10:02:27 EST
On 2/10/26 12:43 AM, Jonathan Cameron wrote:
On Sat, 7 Feb 2026 19:30:58 +0800
Wei Wang <wei.w.wang@xxxxxxxxxxx> wrote:
The ACS Enhanced Capability introduces several new access controls to
improve device isolation. These new controls are particularly important
for device passthrough in virtualization scenarios.
For example, a DMA transaction from a device may target a guest physical
address that lies within the memory aperture of the switch's upstream
port, but not within any memory aperture or BAR space of a downstream
port. In such cases, the switch would generate an Unsupported Request (UR)
response to the device, which is undesirable. Enabling Unclaimed Request
Redirect Control ensures that these DMA requests are forwarded upstream
instead of being rejected.
The ACS DSP and USP Memory Target Access Control and ACS I/O Request
Blocking features similarly enhance device isolation. Device grouping in
Linux assumes that devices are properly isolated. Therefore, enable these
controls by default if PCI_ACS_ECAP is supported by the hardware. As with
other basic ACS access controls, these new controls can be configured via
the "config_acs = " boot parameter.
Signed-off-by: Wei Wang <wei.w.wang@xxxxxxxxxxx>
Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Hi Wei Wang,
I'm late to the game here and may be missing something but
I think the validation and defines for the 2 bit fields are both
misleading and not sufficiently careful to reject values that
are not allowed. Maybe we do want to allow the reserved value
but if so use a 2 bit mask to validate, not two specific
values that the filed can take.
Hi Jonathan,
Thanks for your comments. Yes, I think it's better to reject the reserved encoding (0b11). I'll add masks and enum values for the encodings, and validate that the value is not 0b11.
#define PCI_ACS_DMAC_MASK 0x0300
#define PCI_ACS_UMAC_MASK 0x0C00
enum {
PCI_ACS_MAC_DA = 0x0,
PCI_ACS_MAC_RB = 0x1,
PCI_ACS_MAC_RR = 0x2,
PCI_ACS_MAC_RSVD = 0x3,
};
Thanks,
Jonathan
---This used of the RB and RR Bits seems misleading to me as they form parts
.../admin-guide/kernel-parameters.txt | 23 +++++++++++++------
drivers/pci/pci.c | 13 ++++++++++-
include/uapi/linux/pci_regs.h | 7 ++++++
3 files changed, 35 insertions(+), 8 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 7cfbf1102152..fd895716b07d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5301,13 +5301,22 @@ Kernel parameters
flags.
ACS Flags is defined as follows:
- bit-0 : ACS Source Validation
- bit-1 : ACS Translation Blocking
- bit-2 : ACS P2P Request Redirect
- bit-3 : ACS P2P Completion Redirect
- bit-4 : ACS Upstream Forwarding
- bit-5 : ACS P2P Egress Control
- bit-6 : ACS Direct Translated P2P
+ bit-0 : ACS Source Validation
+ bit-1 : ACS Translation Blocking
+ bit-2 : ACS P2P Request Redirect
+ bit-3 : ACS P2P Completion Redirect
+ bit-4 : ACS Upstream Forwarding
+ bit-5 : ACS P2P Egress Control
+ bit-6 : ACS Direct Translated P2P
+ bit-7 : ACS I/O Request Blocking
+ bit-9:8 : ACS DSP Memory Target Access Ctrl
+ 00 : Direct Request access enabled
+ 01 : Request blocking enabled
+ 10 : Request redirect enabled
+ 11 : Reserved
+ bit-11:10 : ACS USP Memory Target Access Ctrl
+ Same encoding as bit-9:8
+ bit-12 : ACS Unclaimed Request Redirect Ctrl
Each bit can be marked as:
'0' – force disabled
'1' – force enabled
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d2810eb97bda..1714e29ce099 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -942,7 +942,10 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
}
if (mask & ~(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | PCI_ACS_CR |
- PCI_ACS_UF | PCI_ACS_EC | PCI_ACS_DT)) {
+ PCI_ACS_UF | PCI_ACS_EC | PCI_ACS_DT | PCI_ACS_IB |
+ PCI_ACS_DMAC_RB | PCI_ACS_DMAC_RR |
+ PCI_ACS_UMAC_RB | PCI_ACS_UMAC_RR |
+ PCI_ACS_URRC)) {
of 2 bit field that can only take values 0, 1 and 2.
So if the command line set PCI_ACS_UMAC_RB and PCI_ACS_UMAC_RR it
would have hit the reserved value and that should be detected.
Hence I think you need to do this sort of masking + additional checks that these
2 bit fields have valid content.
Yes, will do.
pci_err(dev, "Invalid ACS flags specified\n");
return;
}
@@ -1002,6 +1005,14 @@ static void pci_std_enable_acs(struct pci_dev *dev, struct pci_acs *caps)
/* Upstream Forwarding */
caps->ctrl |= (caps->cap & PCI_ACS_UF);
+ /*
+ * Downstream and Upstream Port Memory Target Access Redirect,
+ * Redirect Unclaimed Request Redirect Control
+ */
+ if (caps->cap & PCI_ACS_ECAP)
+ caps->ctrl |= PCI_ACS_DMAC_RR | PCI_ACS_UMAC_RR |
+ PCI_ACS_URRC | PCI_ACS_IB;
+
/* Enable Translation Blocking for external devices and noats */
if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
caps->ctrl |= (caps->cap & PCI_ACS_TB);
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index ec1c54b5a310..593ef522d2ba 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -1016,6 +1016,7 @@
/* Access Control Service */
#define PCI_ACS_CAP 0x04 /* ACS Capability Register */
+#define PCI_ACS_ECAP 0x0080 /* ACS Enhanced Capability */
#define PCI_ACS_SV 0x0001 /* Source Validation */
#define PCI_ACS_TB 0x0002 /* Translation Blocking */
#define PCI_ACS_RR 0x0004 /* P2P Request Redirect */
@@ -1023,6 +1024,12 @@
#define PCI_ACS_UF 0x0010 /* Upstream Forwarding */
#define PCI_ACS_EC 0x0020 /* P2P Egress Control */
#define PCI_ACS_DT 0x0040 /* Direct Translated P2P */
+#define PCI_ACS_IB 0x0080 /* I/O Request Blocking */
+#define PCI_ACS_DMAC_RB 0x0100 /* DSP Memory Target Access Blocking */
+#define PCI_ACS_DMAC_RR 0x0200 /* DSP Memory Target Access Redirect */
+#define PCI_ACS_UMAC_RB 0x0400 /* USP Memory Target Access Blocking */
If I'm reading this write, those are specific values in the
ACS USP Memory Target Access Control, not what they appear to be here
which is bits that can be combined.
I think you need a _MASK and separate definitions for the 3 values that
are allowed.
Yes.
+#define PCI_ACS_UMAC_RR 0x0800 /* USP Memory Target Access Redirect */
+#define PCI_ACS_URRC 0x1000 /* Unclaimed Request Redirect Ctrl */
#define PCI_ACS_EGRESS_BITS 0x05 /* ACS Egress Control Vector Size */
#define PCI_ACS_CTRL 0x06 /* ACS Control Register */
#define PCI_ACS_EGRESS_CTL_V 0x08 /* ACS Egress Control Vector */