On 17/06/26 11:45PM, Junjie Cao wrote:
Hi Shrihari,
On Tue, 9 Jun 2026 16:28:35 +0530, Shrihari E S wrote:
+ pcie_config_uio_svc(d, errp);
pcie_cap_arifwd_init(d);
pcie_cap_deverr_init(d);
pcie_cap_slot_init(d, s);
In rp_realize() (and the two xio3130 ports) pcie_config_uio_svc() runs
before pcie_aer_init(). pcie_svc_cap_init() falls back to offset 0x100
when no extended capability exists yet, so SVC is placed at 0x100 at
this point -- and then pcie_aer_init() puts AER at the same 0x100 and
overwrites it, while dev->exp.svc_cap still points there.
The visible effect is that "-device pcie-root-port,x-uio-svc=on" comes
up with no SVC capability in the extended config space (the chain is
just AER -> ACS), and no error is reported, so the request is silently
dropped. I reproduced this by walking the extended capability list of
the root port over ECAM: SVC (0x35) never appears, even though the
property was set.
+ rc = pcie_config_uio_svc(pci_dev, errp);
+ if (p->flitmode && rc >= 0) {
+ crp->uio_capable = true;
+ }
On cxl-rp this runs a second time: cxl_rp_realize() first calls the
parent rp_realize() (which already calls pcie_config_uio_svc() per the
hunk above) and then calls it again directly. The first call collides
with AER at 0x100 as above; the second runs after the extended caps
exist, so SVC ends up correctly at 0x200. So cxl-rp happens to work,
but via a double init.
Just moving the call to after pcie_aer_init() isn't quite enough on
its own: the CXL ports (cxl-rp/usp/dsp) build their DVSECs at fixed
offsets and then add SVC themselves at the end, so if the shared
parent rp_realize() also adds SVC before those DVSECs are built, the
cxl-rp ends up with its DVSECs off the chain.
What worked for me was to move the SVC call after the ECAP inits in
the pure-PCIe paths and skip it in the parent when the device is CXL,
letting the CXL realize functions keep adding it last as they already
do:
- rp_realize(): move pcie_config_uio_svc() to after pcie_acs_init(),
guarded by if (!pci_is_cxl(d)) so cxl-rp's own call (which runs
after build_dvsecs()) remains the one that places SVC.
- xio3130_upstream/downstream realize(): move the call to after
pcie_aer_init().
- cxl-rp/usp/dsp: unchanged, they already add it last.
With that, plain pcie-root-port, the xio3130 switch ports and the CXL
ports all come up with SVC at 0x200, and cxl-rp keeps its four DVSECs.
Hi Junjie,
Thank you for pointing this bug, got missed in our testing. Will check
and move the svc_init() function properly in the next version of the patch.
One thing to note is that this only gets the capability enumerated --
I haven't verified the SVC behaviour beyond that. Looking at
pcie_svc_cap_write_config()/pcie_svc_apply_gating(), writing the SVC
Enable bit just copies ctrl/status into the shadow struct; no VC
Resource Status bit (the per-VC negotiation bit, SVC_VC_NEGO) is ever
set, and the SVC Status register stays zero. So a guest that enables
SVC and polls for negotiation wouldn't see anything change. If the
data-plane series on top is expected to drive that, fine -- I just
wanted to flag that as things stand the enable is observationally a
no-op, in case a driver is expected to key off it.
Many thanks,
Junjie
Yeah this patch series is meant for enumeration (probe) puprpose only
and yes we have plan to add data plane support as well.
Thanks,
Shrihari
