Hi Zhangfei,
> -----Original Message-----
> From: Zhangfei Gao <[email protected]>
> Sent: 14 January 2026 04:18
> To: Shameer Kolothum <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Jason Gunthorpe
> <[email protected]>; Nicolin Chen <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; Nathan Chen
> <[email protected]>; Matt Ochs <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; Krishnakant Jaju
> <[email protected]>; Michael S . Tsirkin <[email protected]>
> Subject: Re: [PATCH v7 33/36] hw/pci: Add helper to insert PCIe extended
> capability at a fixed offset
>
> External email: Use caution opening links or attachments
>
>
> Hi, Shameer
>
> On Mon, 12 Jan 2026 at 03:58, Shameer Kolothum
> <[email protected]> wrote:
> >
> > Add pcie_insert_capability(), a helper to insert a PCIe extended
> > capability into an existing extended capability list at a
> > caller-specified offset.
> >
> > Unlike pcie_add_capability(), which always appends a capability to the
> > end of the list, this helper preserves the existing list ordering
> > while allowing insertion at an arbitrary offset.
> >
> > The helper only validates that the insertion does not overwrite an
> > existing PCIe extended capability header, since corrupting a header
> > would break the extended capability linked list. Validation of
> > overlaps with other configuration space registers or
> > capability-specific register blocks is left to the caller.
> >
> > Cc: Michael S. Tsirkin <[email protected]>
> > Signed-off-by: Shameer Kolothum <[email protected]>
>
> The guest kernel fails to boot with para "ssidsize=16" with v7 series.
> Without ssidsize, guest kernel can boot no problem.
Thanks for giving this a spin.
> However, pasid feature requires ssidsize.
> smmuv3_accel_get_viommu_flags
> if (s->ssidsize) {
> flags |= VIOMMU_FLAG_PASID_SUPPORTED;
>
> v6 does not has such issue, and does not requires ssidsize param.
As mentioned in cover letter this series has changed the way the overall
PASID is enabled. It now allows user to specify an offset to place the
PASID CAP for vfio-pci devices,
-device vfio-pci,host=0018:06:00.0,..,x-vpasid-cap-offset=xxx
If none is specified it will place it at the last offset as default.
And you need "ssidsize" to specify the SMMUv3 sub stream Id bits.
> log:
> ASSERT_EFI_ERROR (Status = Invalid Parameter) ASSERT [PciBusDxe]
> /home/linaro/work/edk2/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c(626):
> !(((INTN)(RETURN_STATUS)(Status)) < 0)
It could be that the pcie_insert_capability() helper added here is corrupting
the config space. Looking at it again, I can see that it is not handling a few
corner cases. Could you please replace the pcie_insert_capability() with
the below and retest.
Thanks,
Shameer
...
bool pcie_insert_capability(PCIDevice *dev, uint16_t cap_id, uint8_t cap_ver,
uint16_t offset, uint16_t size)
{
uint16_t pos = PCI_CONFIG_SPACE_SIZE, prev = 0;
uint32_t header;
assert(offset >= PCI_CONFIG_SPACE_SIZE);
assert(offset < (uint16_t)(offset + size));
assert((uint16_t)(offset + size) <= PCIE_CONFIG_SPACE_SIZE);
assert(size >= 8);
assert(pci_is_express(dev));
header = pci_get_long(dev->config + pos);
if (!header) {
/* No extended capability, check requested offset is at
PCI_CONFIG_SPACE_SIZE*/
if (offset != pos) {
return false;
}
pci_set_long(dev->config + pos, PCI_EXT_CAP(cap_id, cap_ver, 0));
goto out;
}
while (header && pos && offset >= pos) {
uint16_t next = PCI_EXT_CAP_NEXT(header);
/* Reject insertion inside an existing ECAP header (4 bytes) */
if (offset < pos + PCI_EXT_CAP_ALIGN) {
return false;
}
prev = pos;
pos = next;
header = pos ? pci_get_long(dev->config + pos) : 0;
}
pci_set_long(dev->config + offset, PCI_EXT_CAP(cap_id, cap_ver, pos));
if (prev) {
pcie_ext_cap_set_next(dev, prev, offset);
}
out:
/* Make capability read-only by default */
memset(dev->wmask + offset, 0, size);
memset(dev->w1cmask + offset, 0, size);
/* Check capability by default */
memset(dev->cmask + offset, 0xFF, size);
return true;
}
