> -----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 2/3] hw/pci: Expose ACS ctrl on Guest as configured by the > HYP > > 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 HYP. Please use "host" or "hypervisor" instead of HYP > > Otherwise, the behavior is considered as un-defined. > > It might fail by SW or HW. I think this is vague. Please spell out the failure. > > Extend both root port and downstream port to get acs ctrl bits that > should match the HYP and use them in the guest. > > Signed-off-by: Yishai Hadas <[email protected]> > Signed-off-by: Tushar Dave <[email protected]> > --- > hw/pci-bridge/pcie_root_port.c | 10 ++++++++-- > hw/pci-bridge/xio3130_downstream.c | 10 ++++++++-- > hw/pci/pcie.c | 18 +++++++++++++++--- > include/hw/pci/pcie.h | 6 +++--- > include/hw/pci/pcie_port.h | 1 + > 5 files changed, 35 insertions(+), 10 deletions(-) > > diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c > index 7c3e78010b..3ae0b75146 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,15 @@ 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; > + } > } If acs_offset is 0, there is no ACS capability and acs-ctrl is silently ignored. Please report an error in that case. > return; > > err: > + pcie_aer_exit(d); > pcie_chassis_del_slot(s); > err_pcie_cap: > pcie_cap_exit(d); > @@ -151,6 +156,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 87ba30eb36..9829832c39 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,16 @@ 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); Same here, with x-pcie-acs=off, acs-ctrl is silently ignored. > + if (rc < 0) { > + goto err; > + } > } > > return; > > err: > + pcie_aer_exit(d); > pcie_chassis_del_slot(s); > err_pcie_cap: > pcie_cap_exit(d); > @@ -144,6 +149,7 @@ 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), > }; > > static const VMStateDescription vmstate_xio3130_downstream = { > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 42bcb9206d..c36f8c58c6 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,27 @@ 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, > + "Unsupported ACS capabilities 0x%hx were supplied. " > + "Supported capabilities are 0x%hx", > + ctrl_bits & ~cap_bits, cap_bits); Nit: maybe "acs-ctrl 0x%hx has bits not supported by this port (supported: 0x%hx)" is clearer. Thanks, Shameer
