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);
+    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)",
+    .get = get_acs_ctrl,
+    .set = set_acs_ctrl,
+};
+
 void pcie_pasid_common_init(PCIDevice *dev, uint16_t offset,
                             uint8_t pasid_width, bool exec_perm, bool priv_mod)
 {
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index 1ab7b3eee5..b1d5e8ae20 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -26,6 +26,7 @@
 #include "hw/pci/pcie_aer.h"
 #include "hw/pci/pcie_sriov.h"
 #include "hw/core/hotplug.h"
+#include "hw/core/qdev-properties.h"
 
 typedef struct PCIEPort PCIEPort;
 typedef struct PCIESlot PCIESlot;
@@ -86,6 +87,8 @@ struct PCIExpressDevice {
 
 #define COMPAT_PROP_PCP "power_controller_present"
 
+extern const PropertyInfo qdev_prop_acs_ctrl;
+
 /* PCI express capability helper functions */
 int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type,
                   uint8_t port, Error **errp);
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index 2ad9a563d9..ef2c2ec2cd 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -37,6 +37,7 @@ struct PCIEPort {
     /* pci express switch port */
     uint8_t     port;
     uint16_t    acs_ctrl;
+    bool        acs_ctrl_configured;
 };
 
 void pcie_port_init_reg(PCIDevice *d);
-- 
2.34.1


Reply via email to