Re: [RFC V2 06/10] hw/pci: Add PCIe Streamlined Virtual Channel (SVC) capability.

[Junjie Cao <junjie.cao@intel.com>]

Hi Shrihari,

On Wed, 26 Aug 2026 11:04:06 +0530, Shrihari E S wrote:
> +            /*
> +             * SVC3 is only meant for UIO TLPs and The non-UIO protocol 0000
> +             * should be for VC0 only. Also checking the TC-VC mapping.
> +             */
> +            if ((i == 3) || (res_ctrl & PCI_SVC_VC0_PROTOCOL) ||
> +                 !(res_ctrl & BIT(i))) {
> +                valid = false;
> +            }

PCI_SVC_VC0_PROTOCOL is (0x0 << 8), so "res_ctrl & PCI_SVC_VC0_PROTOCOL"
is always 0 and the protocol-0000 rejection this comment describes can
never fire.  I tested it: wrote 0x80000010 (VC Enable | TC/VC map bit 4,
protocol select = 0000) to the VC4 Resource Control register of a
cxl-rp; the readback keeps VC Enable set (and, going by the code,
pcie_svc_update_map() then marks uio_opt_svc), where per the comment
the write should have been rejected.  Protocol == 0000 needs the
whole field tested, e.g. !(res_ctrl & PCI_SVC_VC_PROTOCOL_SELECTED).

The UIO branch has the same pattern:

> +        if (res_ctrl & SVC_UIO_PROTOCOL_SELECTED) {

Matches any protocol value with bit 9 set -- including the
vendor-defined 1111b -- not just 0010b.  Comparing the extracted 4-bit
field against the expected value in both branches would close that.

Same run: Port Cap1 reads back EVCC=7, but the init loop only populates
sets 0, 3 and 4 -- the rest read VC ID 000b, the same as VC0, which
7.9.29.6 wants unique.  Was EVCC=2 with sets 1/2 carrying 3/4 the shape
you were after?  (Quoting 6.2 throughout -- newest spec I have.)

> +void pcie_svc_cap_reset(PCIDevice *dev)
> +{
[...]
> +    pci_set_long(dev->config + offset + PCI_SVC_CTL_OFFSET, 0);
> +    pci_set_long(dev->config + offset + PCI_SVC_STA_OFFSET, 0);
> +}

The init function sets USE_VC_MFVC and this clears it again, before
a cold-plugged guest ever runs -- SVC status reads 0 on all four port
types at first boot.  0 is the conformant value anyway (7.9.29.5, and
the comment above the set says as much), so I'd drop the set at init
rather than restore the bit here.

The asymmetry does bite for the rest of the state: a guest-set VC
Enable in RES_CTRL(3)/(4) survives a system reset while CTL/STA are
cleared, and the dev->exp.svc shadow flags set by
pcie_svc_update_map() aren't cleared either.

On the register defines: v1 ended with routing these through
pci_regs.h plus a note on when the kernel header picks them up; they
went into QEMU's pcie_regs.h instead.  Still the plan for a later
spin, or intentional?

Many thanks,
Junjie