> -----Original Message----- > From: Tushar Dave <[email protected]> > Sent: 22 September 2026 22:50 > To: [email protected] > Cc: [email protected]; [email protected]; [email protected]; > Yishai Hadas <[email protected]>; Alex Williamson > <[email protected]>; Jason Gunthorpe <[email protected]>; Shameer > Kolothum Thodi <[email protected]> > Subject: [PATCH 3/3] hw/pci: Make ACS Control read-only to guests > > GPUDirect RDMA using data-direct requires a specific ACS configuration > on PCIe Root Ports and Downstream Ports. > > While ACS can be configured via QEMU's 'acs-ctrl' property, the guest > kernel may overwrite ACS during standard programming. > > This change blocks all guest writes to the PCIe ACS Control register and > preserves QEMU-provided ACS settings across device resets on PCIe Root > Ports > and Downstream Ports. > > Signed-off-by: Tushar Dave <[email protected]> > --- > hw/pci-bridge/pcie_root_port.c | 3 +- > hw/pci-bridge/xio3130_downstream.c | 3 +- > hw/pci/pcie.c | 45 ++++++++++++++++++++++++++++-- > include/hw/pci/pcie.h | 3 ++ > include/hw/pci/pcie_port.h | 1 + > 5 files changed, 51 insertions(+), 4 deletions(-) > > diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c > index 3ae0b75146..784697b1ca 100644 > --- a/hw/pci-bridge/pcie_root_port.c > +++ b/hw/pci-bridge/pcie_root_port.c > @@ -156,7 +156,8 @@ static void rp_exit(PCIDevice *d) > static const Property rp_props[] = { > DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present, > QEMU_PCIE_SLTCAP_PCP_BITNR, true), > - DEFINE_PROP_UINT16("acs-ctrl", PCIEPort, acs_ctrl, 0), > + DEFINE_PROP_UNSIGNED_NODEFAULT("acs-ctrl", PCIEPort, acs_ctrl, > + qdev_prop_acs_ctrl, uint16_t), > }; > > static void rp_instance_post_init(Object *obj) > diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci- > bridge/xio3130_downstream.c > index 9829832c39..e8717be73a 100644 > --- a/hw/pci-bridge/xio3130_downstream.c > +++ b/hw/pci-bridge/xio3130_downstream.c > @@ -149,7 +149,8 @@ static void xio3130_downstream_exitfn(PCIDevice > *d) > static const Property xio3130_downstream_props[] = { > DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present, > QEMU_PCIE_SLTCAP_PCP_BITNR, true), > - DEFINE_PROP_UINT16("acs-ctrl", PCIEPort, acs_ctrl, 0), > + DEFINE_PROP_UNSIGNED_NODEFAULT("acs-ctrl", PCIEPort, acs_ctrl, > + qdev_prop_acs_ctrl, uint16_t), > }; > > static const VMStateDescription vmstate_xio3130_downstream = { > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index c36f8c58c6..08e21022e5 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -1265,6 +1265,8 @@ int pcie_acs_init(PCIDevice *dev, uint16_t offset, > uint16_t ctrl_bits, > { > bool is_downstream = pci_is_express_downstream_port(dev); > uint16_t cap_bits = 0; > + PCIEPort *p = PCIE_PORT(dev); This is a common helper. PCIE_PORT() will fail if a non-port dev is passed. > + bool configured = p->acs_ctrl_configured; > > /* For endpoints, only multifunction devs may have an ACS capability: */ > assert(is_downstream || > @@ -1286,7 +1288,7 @@ int pcie_acs_init(PCIDevice *dev, uint16_t offset, > uint16_t ctrl_bits, > cap_bits = PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | > PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT; > > - if (ctrl_bits & ~cap_bits) { > + if (configured && (ctrl_bits & ~cap_bits)) { > error_setg(errp, > "Unsupported ACS capabilities 0x%hx were supplied. " > "Supported capabilities are 0x%hx", > @@ -1296,7 +1298,17 @@ int pcie_acs_init(PCIDevice *dev, uint16_t offset, > uint16_t ctrl_bits, > } > > pci_set_word(dev->config + offset + PCI_ACS_CAP, cap_bits); > - pci_set_word(dev->wmask + offset + PCI_ACS_CTRL, cap_bits); > + > + if (is_downstream && configured) { > + /* > + * Block guest writes to ACS Control entirely to preserve QEMU > + * ACS settings > + */ > + pci_set_word(dev->wmask + offset + PCI_ACS_CTRL, 0); > + } else { > + pci_set_word(dev->wmask + offset + PCI_ACS_CTRL, cap_bits); > + } > + > pci_set_word(dev->config + offset + PCI_ACS_CTRL, ctrl_bits); > > return 0; > @@ -1309,6 +1321,35 @@ void pcie_acs_reset(PCIDevice *dev, uint16_t val) > } > } > > +static void set_acs_ctrl(Object *obj, Visitor *v, const char *name, > + void *opaque, Error **errp) > +{ > + PCIEPort *p = PCIE_PORT(obj); > + const Property *prop = opaque; > + uint16_t *ptr = object_field_prop_ptr(obj, prop); > + > + if (!visit_type_uint16(v, name, ptr, errp)) { > + return; > + } > + p->acs_ctrl_configured = true; > +} > + > +static void get_acs_ctrl(Object *obj, Visitor *v, const char *name, > + void *opaque, Error **errp) > +{ > + const Property *prop = opaque; > + uint16_t *ptr = object_field_prop_ptr(obj, prop); > + > + visit_type_uint16(v, name, ptr, errp); > +} > + > +const PropertyInfo qdev_prop_acs_ctrl = { > + .type = "uint16", > + .description = "PCIe ACS ctrl configuration (unset = not configured)", Please also say here that setting this, even to 0, makes ACS Control read only to the guest. Thanks, Shameer
