On Wed, 14 Jan 2026 at 18:36, Shameer Kolothum <[email protected]> wrote: > > 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; > }
Yes, this works Tested-by: Zhangfei Gao <[email protected]>
