On Mon, Oct 05, 2026 at 08:31:32PM -0500, Tushar Dave wrote:
> From: Yishai Hadas <[email protected]>
>
> To support P2P on Guest we must expose to the guest OS the actual
> PCIe topology and configuration as set by the hypervisor. The ACS
> control must be set accordingly to have functional P2P. A mismatch
> between the hypervisor's ACS configuration and the guest-visible ACS
> Control register can prevent P2P between devices from working as
> expected.
>
> Extend both root port and downstream port to accept ACS Control bits
> that match the hypervisor configuration and expose them to the guest.
>
> Signed-off-by: Yishai Hadas <[email protected]>
> Co-developed-by: Tushar Dave <[email protected]>
> Signed-off-by: Tushar Dave <[email protected]>
> ---
> hw/pci-bridge/pcie_root_port.c | 13 +++++++++++--
> hw/pci-bridge/xio3130_downstream.c | 13 +++++++++++--
> hw/pci/pcie.c | 17 ++++++++++++++---
> include/hw/pci/pcie.h | 5 +++--
> include/hw/pci/pcie_port.h | 1 +
> 5 files changed, 40 insertions(+), 9 deletions(-)
>
> diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> index 7c3e78010b..e7cce8c0ee 100644
> --- a/hw/pci-bridge/pcie_root_port.c
> +++ b/hw/pci-bridge/pcie_root_port.c
> @@ -46,6 +46,7 @@ static void rp_write_config(PCIDevice *d, uint32_t address,
> static void rp_reset_hold(Object *obj, ResetType type)
> {
> PCIDevice *d = PCI_DEVICE(obj);
> + PCIEPort *p = PCIE_PORT(d);
> DeviceState *qdev = DEVICE(obj);
>
> rp_aer_vector_update(d);
> @@ -53,7 +54,7 @@ static void rp_reset_hold(Object *obj, ResetType type)
> pcie_cap_deverr_reset(d);
> pcie_cap_slot_reset(d);
> pcie_cap_arifwd_reset(d);
> - pcie_acs_reset(d);
> + pcie_acs_reset(d, p->acs_ctrl);
> pcie_aer_root_reset(d);
> pci_bridge_reset(qdev);
> pci_bridge_disable_base_limit(d);
> @@ -118,11 +119,18 @@ static void rp_realize(PCIDevice *d, Error **errp)
> rp_aer_vector_update(d);
>
> if (rpc->acs_offset) {
> - pcie_acs_init(d, rpc->acs_offset);
> + rc = pcie_acs_init(d, rpc->acs_offset, p->acs_ctrl, errp);
> + if (rc < 0) {
> + goto err;
> + }
> + } else if (p->acs_ctrl) {
> + error_setg(errp, "acs-ctrl is not supported by this root port");
> + goto err;
> }
> return;
>
> err:
> + pcie_aer_exit(d);
> pcie_chassis_del_slot(s);
> err_pcie_cap:
> pcie_cap_exit(d);
> @@ -151,6 +159,7 @@ 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),
> };
>
> 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 6a9df5c174..e243dfdf7d 100644
> --- a/hw/pci-bridge/xio3130_downstream.c
> +++ b/hw/pci-bridge/xio3130_downstream.c
> @@ -58,11 +58,12 @@ static void xio3130_downstream_write_config(PCIDevice *d,
> uint32_t address,
> static void xio3130_downstream_reset(DeviceState *qdev)
> {
> PCIDevice *d = PCI_DEVICE(qdev);
> + PCIEPort *p = PCIE_PORT(d);
>
> pcie_cap_deverr_reset(d);
> pcie_cap_slot_reset(d);
> pcie_cap_arifwd_reset(d);
> - pcie_acs_reset(d);
> + pcie_acs_reset(d, p->acs_ctrl);
> pci_bridge_reset(qdev);
> }
>
> @@ -115,12 +116,19 @@ static void xio3130_downstream_realize(PCIDevice *d,
> Error **errp)
> }
>
> if (d->cap_present & QEMU_PCIE_CAP_ACS) {
> - pcie_acs_init(d, XIO3130_ACS_OFFSET);
> + rc = pcie_acs_init(d, XIO3130_ACS_OFFSET, p->acs_ctrl, errp);
> + if (rc < 0) {
> + goto err;
> + }
> + } else if (p->acs_ctrl) {
> + error_setg(errp, "acs-ctrl requires x-pcie-acs to be on");
> + goto err;
Tying a stable QAPI to an unstable one like this is not really
reasonable.
> }
>
> return;
>
> err:
> + pcie_aer_exit(d);
> pcie_chassis_del_slot(s);
> err_pcie_cap:
> pcie_cap_exit(d);
> @@ -146,6 +154,7 @@ static const Property xio3130_downstream_props[] = {
> QEMU_PCIE_SLTCAP_PCP_BITNR, true),
> DEFINE_PROP_BIT("x-pcie-acs", PCIDevice, cap_present,
> QEMU_PCIE_ACS_BITNR, true),
> + DEFINE_PROP_UINT16("acs-ctrl", PCIEPort, acs_ctrl, 0),
> };
>
> static const VMStateDescription vmstate_xio3130_downstream = {
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 42bcb9206d..af24ec7fd9 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -1260,7 +1260,8 @@ void pcie_ats_init(PCIDevice *dev, uint16_t offset,
> bool aligned)
> }
>
> /* ACS (Access Control Services) */
> -void pcie_acs_init(PCIDevice *dev, uint16_t offset)
> +int pcie_acs_init(PCIDevice *dev, uint16_t offset, uint16_t ctrl_bits,
> + Error **errp)
> {
> bool is_downstream = pci_is_express_downstream_port(dev);
> uint16_t cap_bits = 0;
> @@ -1284,16 +1285,26 @@ void pcie_acs_init(PCIDevice *dev, uint16_t offset)
> */
> 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) {
> + error_setg(errp,
> + "acs-ctrl 0x%hx has bits not supported by this port "
> + "(supported: 0x%hx)", ctrl_bits, cap_bits);
> + return -EINVAL;
> + }
> }
>
> pci_set_word(dev->config + offset + PCI_ACS_CAP, cap_bits);
> pci_set_word(dev->wmask + offset + PCI_ACS_CTRL, cap_bits);
> + pci_set_word(dev->config + offset + PCI_ACS_CTRL, ctrl_bits);
> +
> + return 0;
> }
>
> -void pcie_acs_reset(PCIDevice *dev)
> +void pcie_acs_reset(PCIDevice *dev, uint16_t val)
> {
> if (dev->exp.acs_cap) {
> - pci_set_word(dev->config + dev->exp.acs_cap + PCI_ACS_CTRL, 0);
> + pci_set_word(dev->config + dev->exp.acs_cap + PCI_ACS_CTRL, val);
> }
> }
>
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index ec25e7a7de..678ad73f7f 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -137,8 +137,9 @@ bool pcie_insert_capability(PCIDevice *dev, uint16_t
> cap_id, uint8_t cap_ver,
> uint16_t offset, uint16_t size);
> void pcie_sync_bridge_lnk(PCIDevice *dev);
>
> -void pcie_acs_init(PCIDevice *dev, uint16_t offset);
> -void pcie_acs_reset(PCIDevice *dev);
> +int pcie_acs_init(PCIDevice *dev, uint16_t offset, uint16_t ctrl_bits,
> + Error **errp);
> +void pcie_acs_reset(PCIDevice *dev, uint16_t val);
>
> void pcie_ari_init(PCIDevice *dev, uint16_t offset);
> void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t
> ser_num);
> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> index b28af067a6..2ad9a563d9 100644
> --- a/include/hw/pci/pcie_port.h
> +++ b/include/hw/pci/pcie_port.h
> @@ -36,6 +36,7 @@ struct PCIEPort {
>
> /* pci express switch port */
> uint8_t port;
> + uint16_t acs_ctrl;
> };
>
> void pcie_port_init_reg(PCIDevice *d);
> --
> 2.34.1
