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 <mst@redhat.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
hw/pci/pcie.c | 58 +++++++++++++++++++++++++++++++++++++++++++
include/hw/pci/pcie.h | 2 ++
2 files changed, 60 insertions(+)
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index b302de6419..8568a062a5 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -1050,6 +1050,64 @@ static void pcie_ext_cap_set_next(PCIDevice *dev, uint16_t pos, uint16_t next)
pci_set_long(dev->config + pos, header);
}
+/*
+ * Insert a PCIe extended capability at a given offset.
+ *
+ * This helper only validates that the insertion does not overwrite an
+ * existing PCIe extended capability header, as corrupting a header would
+ * break the extended capability linked list.
+ *
+ * The caller must ensure that (offset, size) does not overlap with other
+ * registers or capability-specific register blocks. Overlaps with
+ * capability-specific registers are not checked and are considered a
+ * user-controlled override.
+ */
+bool pcie_insert_capability(PCIDevice *dev, uint16_t cap_id, uint8_t cap_ver,
+ uint16_t offset, uint16_t size)
+{
+ uint16_t prev = 0, next = 0;
+ uint16_t cur = pci_get_word(dev->config + PCI_CONFIG_SPACE_SIZE);
+
+ /* Walk the ext cap list to find insertion point */
+ while (cur) {
+ uint32_t hdr = pci_get_long(dev->config + cur);
+ next = PCI_EXT_CAP_NEXT(hdr);
+
+ /* Check we are not overwriting any existing CAP header area */
+ if (offset >= cur && offset < cur + PCI_EXT_CAP_ALIGN) {
+ return false;
+ }
+
+ prev = cur;
+ cur = next;
+ if (next == 0 || next > offset) {
+ break;
+ }
+ }
+
+ /* Make sure, next CAP header area is not over written either */
+ if (next && (offset + size) >= next) {
+ return false;
+ }
+
+ /* Insert new cap */
+ pci_set_long(dev->config + offset,
+ PCI_EXT_CAP(cap_id, cap_ver, cur));
+ if (prev) {
+ pcie_ext_cap_set_next(dev, prev, offset);
+ } else {
+ /* Insert at head (0x100) */
+ pci_set_word(dev->config + PCI_CONFIG_SPACE_SIZE, offset);
+ }
+
+ /* 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;
+}
+
/*
* Caller must supply valid (offset, size) such that the range wouldn't
* overlap with other capability or other registers.
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index c880ae1e04..d68bfa6257 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -133,6 +133,8 @@ uint16_t pcie_find_capability(PCIDevice *dev, uint16_t cap_id);
void pcie_add_capability(PCIDevice *dev,
uint16_t cap_id, uint8_t cap_ver,
uint16_t offset, uint16_t size);
+bool pcie_insert_capability(PCIDevice *dev, uint16_t cap_id, uint8_t cap_ver,
+ uint16_t offset, uint16_t size);
void pcie_sync_bridge_lnk(PCIDevice *dev);
void pcie_acs_init(PCIDevice *dev, uint16_t offset);
--
2.43.0On Sun, 11 Jan 2026 19:53:19 +0000
Shameer Kolothum <skolothumtho@nvidia.com> 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 <mst@redhat.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
Hi Shameer.
Random musings inline... Maybe I'm just failing in my spec grep skills.
> ---
> hw/pci/pcie.c | 58 +++++++++++++++++++++++++++++++++++++++++++
> include/hw/pci/pcie.h | 2 ++
> 2 files changed, 60 insertions(+)
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index b302de6419..8568a062a5 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -1050,6 +1050,64 @@ static void pcie_ext_cap_set_next(PCIDevice *dev, uint16_t pos, uint16_t next)
> pci_set_long(dev->config + pos, header);
> }
>
> +/*
> + * Insert a PCIe extended capability at a given offset.
> + *
> + * This helper only validates that the insertion does not overwrite an
> + * existing PCIe extended capability header, as corrupting a header would
> + * break the extended capability linked list.
> + *
> + * The caller must ensure that (offset, size) does not overlap with other
> + * registers or capability-specific register blocks. Overlaps with
> + * capability-specific registers are not checked and are considered a
> + * user-controlled override.
> + */
> +bool pcie_insert_capability(PCIDevice *dev, uint16_t cap_id, uint8_t cap_ver,
> + uint16_t offset, uint16_t size)
> +{
> + uint16_t prev = 0, next = 0;
> + uint16_t cur = pci_get_word(dev->config + PCI_CONFIG_SPACE_SIZE);
> +
> + /* Walk the ext cap list to find insertion point */
> + while (cur) {
> + uint32_t hdr = pci_get_long(dev->config + cur);
> + next = PCI_EXT_CAP_NEXT(hdr);
> +
> + /* Check we are not overwriting any existing CAP header area */
> + if (offset >= cur && offset < cur + PCI_EXT_CAP_ALIGN) {
> + return false;
> + }
> +
> + prev = cur;
> + cur = next;
> + if (next == 0 || next > offset) {
So this (sort of) relies on a thing I've never been able to find a clear
statement of in the PCIe spec. Does Next Capability Offset have to be
larger than the offset of the current record? I.e. Can we have
backwards pointers?
Meh, I think this is fine, it just came up before and I couldn't find
a reference to prove it!
More importantly, if it isn't a requirement and a rare device turns up
that has a backwards pointer, that just means there isn't a 'right'
point in the list to put this at, so any random choice is fine and
the next == 0 condition means we always fine an option.
> + break;
> + }
> + }
> +
> + /* Make sure, next CAP header area is not over written either */
Looks like one space too few.
> + if (next && (offset + size) >= next) {
> + return false;
> + }
> +
> + /* Insert new cap */
> + pci_set_long(dev->config + offset,
> + PCI_EXT_CAP(cap_id, cap_ver, cur));
> + if (prev) {
> + pcie_ext_cap_set_next(dev, prev, offset);
> + } else {
> + /* Insert at head (0x100) */
Comment is a little confusing as you aren't inserting the new capability
there. What I think this is actually doing is
/*
* Insert a Null Extended Capability (7.9.28 in the PCI 6.2 spec)
* at head when there are no extended capabilities and use that to
* point to the inserted capability at offset.
*/
> + pci_set_word(dev->config + PCI_CONFIG_SPACE_SIZE, offset);
> + }
> +
> + /* 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;
> +}
> -----Original Message-----
> From: Jonathan Cameron <jonathan.cameron@huawei.com>
> Sent: 14 January 2026 11:46
> To: Shameer Kolothum <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; ddutile@redhat.com;
> berrange@redhat.com; clg@redhat.com; alex@shazbot.org; Nathan Chen
> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; zhangfei.gao@linaro.org;
> zhenzhong.duan@intel.com; yi.l.liu@intel.com; Krishnakant Jaju
> <kjaju@nvidia.com>; Michael S . Tsirkin <mst@redhat.com>
> 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
>
>
> On Sun, 11 Jan 2026 19:53:19 +0000
> Shameer Kolothum <skolothumtho@nvidia.com> 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 <mst@redhat.com>
> > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> Hi Shameer.
Happy new year!
>
> Random musings inline... Maybe I'm just failing in my spec grep skills.
>
> > ---
> > hw/pci/pcie.c | 58
> +++++++++++++++++++++++++++++++++++++++++++
> > include/hw/pci/pcie.h | 2 ++
> > 2 files changed, 60 insertions(+)
> >
> > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > index b302de6419..8568a062a5 100644
> > --- a/hw/pci/pcie.c
> > +++ b/hw/pci/pcie.c
> > @@ -1050,6 +1050,64 @@ static void pcie_ext_cap_set_next(PCIDevice
> *dev, uint16_t pos, uint16_t next)
> > pci_set_long(dev->config + pos, header);
> > }
> >
> > +/*
> > + * Insert a PCIe extended capability at a given offset.
> > + *
> > + * This helper only validates that the insertion does not overwrite an
> > + * existing PCIe extended capability header, as corrupting a header would
> > + * break the extended capability linked list.
> > + *
> > + * The caller must ensure that (offset, size) does not overlap with other
> > + * registers or capability-specific register blocks. Overlaps with
> > + * capability-specific registers are not checked and are considered a
> > + * user-controlled override.
> > + */
> > +bool pcie_insert_capability(PCIDevice *dev, uint16_t cap_id, uint8_t
> cap_ver,
> > + uint16_t offset, uint16_t size)
> > +{
> > + uint16_t prev = 0, next = 0;
> > + uint16_t cur = pci_get_word(dev->config + PCI_CONFIG_SPACE_SIZE);
> > +
> > + /* Walk the ext cap list to find insertion point */
> > + while (cur) {
> > + uint32_t hdr = pci_get_long(dev->config + cur);
> > + next = PCI_EXT_CAP_NEXT(hdr);
> > +
> > + /* Check we are not overwriting any existing CAP header area */
> > + if (offset >= cur && offset < cur + PCI_EXT_CAP_ALIGN) {
> > + return false;
> > + }
> > +
> > + prev = cur;
> > + cur = next;
> > + if (next == 0 || next > offset) {
>
> So this (sort of) relies on a thing I've never been able to find a clear
> statement of in the PCIe spec. Does Next Capability Offset have to be
> larger than the offset of the current record? I.e. Can we have
> backwards pointers?
That’s right. I also couldn’t find a place in the spec that explicitly
says the list must be forward only. A device doing a backward walk
would be pretty odd, hopefully nothing like that exists in the wild.
> Meh, I think this is fine, it just came up before and I couldn't find
> a reference to prove it!
>
> More importantly, if it isn't a requirement and a rare device turns up
> that has a backwards pointer, that just means there isn't a 'right'
> point in the list to put this at, so any random choice is fine and
> the next == 0 condition means we always fine an option.
Yes.
>
> > + break;
> > + }
> > + }
> > +
> > + /* Make sure, next CAP header area is not over written either */
>
> Looks like one space too few.
>
> > + if (next && (offset + size) >= next) {
> > + return false;
> > + }
> > +
> > + /* Insert new cap */
> > + pci_set_long(dev->config + offset,
> > + PCI_EXT_CAP(cap_id, cap_ver, cur));
> > + if (prev) {
> > + pcie_ext_cap_set_next(dev, prev, offset);
> > + } else {
> > + /* Insert at head (0x100) */
>
> Comment is a little confusing as you aren't inserting the new capability
> there. What I think this is actually doing is
>
> /*
> * Insert a Null Extended Capability (7.9.28 in the PCI 6.2 spec)
> * at head when there are no extended capabilities and use that to
> * point to the inserted capability at offset.
> */
Sure. However, Zhangfei has reported a crash with this and I have
reworked the logic a bit to cover few corner cases. Based on his
tests I will update this.
Thanks,
Shameer
> > + pci_set_word(dev->config + PCI_CONFIG_SPACE_SIZE, offset);
> > + }
> > +
> > + /* 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;
> > +}
>
On Wed, Jan 14, 2026 at 12:26:29PM +0000, Shameer Kolothum wrote:
>
>
> > -----Original Message-----
> > From: Jonathan Cameron <jonathan.cameron@huawei.com>
> > Sent: 14 January 2026 11:46
> > To: Shameer Kolothum <skolothumtho@nvidia.com>
> > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> > eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> > <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; ddutile@redhat.com;
> > berrange@redhat.com; clg@redhat.com; alex@shazbot.org; Nathan Chen
> > <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> > smostafa@google.com; wangzhou1@hisilicon.com;
> > jiangkunkun@huawei.com; zhangfei.gao@linaro.org;
> > zhenzhong.duan@intel.com; yi.l.liu@intel.com; Krishnakant Jaju
> > <kjaju@nvidia.com>; Michael S . Tsirkin <mst@redhat.com>
> > 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
> >
> >
> > On Sun, 11 Jan 2026 19:53:19 +0000
> > Shameer Kolothum <skolothumtho@nvidia.com> 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 <mst@redhat.com>
> > > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> > Hi Shameer.
>
> Happy new year!
>
> >
> > Random musings inline... Maybe I'm just failing in my spec grep skills.
> >
> > > ---
> > > hw/pci/pcie.c | 58
> > +++++++++++++++++++++++++++++++++++++++++++
> > > include/hw/pci/pcie.h | 2 ++
> > > 2 files changed, 60 insertions(+)
> > >
> > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > index b302de6419..8568a062a5 100644
> > > --- a/hw/pci/pcie.c
> > > +++ b/hw/pci/pcie.c
> > > @@ -1050,6 +1050,64 @@ static void pcie_ext_cap_set_next(PCIDevice
> > *dev, uint16_t pos, uint16_t next)
> > > pci_set_long(dev->config + pos, header);
> > > }
> > >
> > > +/*
> > > + * Insert a PCIe extended capability at a given offset.
> > > + *
> > > + * This helper only validates that the insertion does not overwrite an
> > > + * existing PCIe extended capability header, as corrupting a header would
> > > + * break the extended capability linked list.
> > > + *
> > > + * The caller must ensure that (offset, size) does not overlap with other
> > > + * registers or capability-specific register blocks. Overlaps with
> > > + * capability-specific registers are not checked and are considered a
> > > + * user-controlled override.
> > > + */
> > > +bool pcie_insert_capability(PCIDevice *dev, uint16_t cap_id, uint8_t
> > cap_ver,
> > > + uint16_t offset, uint16_t size)
> > > +{
> > > + uint16_t prev = 0, next = 0;
> > > + uint16_t cur = pci_get_word(dev->config + PCI_CONFIG_SPACE_SIZE);
> > > +
> > > + /* Walk the ext cap list to find insertion point */
> > > + while (cur) {
> > > + uint32_t hdr = pci_get_long(dev->config + cur);
> > > + next = PCI_EXT_CAP_NEXT(hdr);
> > > +
> > > + /* Check we are not overwriting any existing CAP header area */
> > > + if (offset >= cur && offset < cur + PCI_EXT_CAP_ALIGN) {
> > > + return false;
> > > + }
> > > +
> > > + prev = cur;
> > > + cur = next;
> > > + if (next == 0 || next > offset) {
> >
> > So this (sort of) relies on a thing I've never been able to find a clear
> > statement of in the PCIe spec. Does Next Capability Offset have to be
> > larger than the offset of the current record? I.e. Can we have
> > backwards pointers?
>
> That’s right. I also couldn’t find a place in the spec that explicitly
> says the list must be forward only. A device doing a backward walk
> would be pretty odd, hopefully nothing like that exists in the wild.
Yes, there's no reason not to have such pointers, with either
PCIe or classical PCI capability.
> > Meh, I think this is fine, it just came up before and I couldn't find
> > a reference to prove it!
> >
> > More importantly, if it isn't a requirement and a rare device turns up
> > that has a backwards pointer, that just means there isn't a 'right'
> > point in the list to put this at, so any random choice is fine and
> > the next == 0 condition means we always fine an option.
>
> Yes.
>
> >
> > > + break;
> > > + }
> > > + }
> > > +
> > > + /* Make sure, next CAP header area is not over written either */
> >
> > Looks like one space too few.
> >
> > > + if (next && (offset + size) >= next) {
> > > + return false;
> > > + }
> > > +
> > > + /* Insert new cap */
> > > + pci_set_long(dev->config + offset,
> > > + PCI_EXT_CAP(cap_id, cap_ver, cur));
> > > + if (prev) {
> > > + pcie_ext_cap_set_next(dev, prev, offset);
> > > + } else {
> > > + /* Insert at head (0x100) */
> >
> > Comment is a little confusing as you aren't inserting the new capability
> > there. What I think this is actually doing is
> >
> > /*
> > * Insert a Null Extended Capability (7.9.28 in the PCI 6.2 spec)
> > * at head when there are no extended capabilities and use that to
> > * point to the inserted capability at offset.
> > */
>
> Sure. However, Zhangfei has reported a crash with this and I have
> reworked the logic a bit to cover few corner cases. Based on his
> tests I will update this.
>
> Thanks,
> Shameer
>
> > > + pci_set_word(dev->config + PCI_CONFIG_SPACE_SIZE, offset);
> > > + }
> > > +
> > > + /* 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;
> > > +}
> >
>
On Wed, 14 Jan 2026 07:35:01 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Jan 14, 2026 at 12:26:29PM +0000, Shameer Kolothum wrote:
> >
> >
> > > -----Original Message-----
> > > From: Jonathan Cameron <jonathan.cameron@huawei.com>
> > > Sent: 14 January 2026 11:46
> > > To: Shameer Kolothum <skolothumtho@nvidia.com>
> > > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> > > eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> > > <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; ddutile@redhat.com;
> > > berrange@redhat.com; clg@redhat.com; alex@shazbot.org; Nathan Chen
> > > <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> > > smostafa@google.com; wangzhou1@hisilicon.com;
> > > jiangkunkun@huawei.com; zhangfei.gao@linaro.org;
> > > zhenzhong.duan@intel.com; yi.l.liu@intel.com; Krishnakant Jaju
> > > <kjaju@nvidia.com>; Michael S . Tsirkin <mst@redhat.com>
> > > 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
> > >
> > >
> > > On Sun, 11 Jan 2026 19:53:19 +0000
> > > Shameer Kolothum <skolothumtho@nvidia.com> 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 <mst@redhat.com>
> > > > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> > > Hi Shameer.
> >
> > Happy new year!
> >
> > >
> > > Random musings inline... Maybe I'm just failing in my spec grep skills.
> > >
> > > > ---
> > > > hw/pci/pcie.c | 58
> > > +++++++++++++++++++++++++++++++++++++++++++
> > > > include/hw/pci/pcie.h | 2 ++
> > > > 2 files changed, 60 insertions(+)
> > > >
> > > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > > index b302de6419..8568a062a5 100644
> > > > --- a/hw/pci/pcie.c
> > > > +++ b/hw/pci/pcie.c
> > > > @@ -1050,6 +1050,64 @@ static void pcie_ext_cap_set_next(PCIDevice
> > > *dev, uint16_t pos, uint16_t next)
> > > > pci_set_long(dev->config + pos, header);
> > > > }
> > > >
> > > > +/*
> > > > + * Insert a PCIe extended capability at a given offset.
> > > > + *
> > > > + * This helper only validates that the insertion does not overwrite an
> > > > + * existing PCIe extended capability header, as corrupting a header would
> > > > + * break the extended capability linked list.
> > > > + *
> > > > + * The caller must ensure that (offset, size) does not overlap with other
> > > > + * registers or capability-specific register blocks. Overlaps with
> > > > + * capability-specific registers are not checked and are considered a
> > > > + * user-controlled override.
> > > > + */
> > > > +bool pcie_insert_capability(PCIDevice *dev, uint16_t cap_id, uint8_t
> > > cap_ver,
> > > > + uint16_t offset, uint16_t size)
> > > > +{
> > > > + uint16_t prev = 0, next = 0;
> > > > + uint16_t cur = pci_get_word(dev->config + PCI_CONFIG_SPACE_SIZE);
> > > > +
> > > > + /* Walk the ext cap list to find insertion point */
> > > > + while (cur) {
> > > > + uint32_t hdr = pci_get_long(dev->config + cur);
> > > > + next = PCI_EXT_CAP_NEXT(hdr);
> > > > +
> > > > + /* Check we are not overwriting any existing CAP header area */
> > > > + if (offset >= cur && offset < cur + PCI_EXT_CAP_ALIGN) {
> > > > + return false;
> > > > + }
> > > > +
> > > > + prev = cur;
> > > > + cur = next;
> > > > + if (next == 0 || next > offset) {
> > >
> > > So this (sort of) relies on a thing I've never been able to find a clear
> > > statement of in the PCIe spec. Does Next Capability Offset have to be
> > > larger than the offset of the current record? I.e. Can we have
> > > backwards pointers?
> >
> > That’s right. I also couldn’t find a place in the spec that explicitly
> > says the list must be forward only. A device doing a backward walk
> > would be pretty odd, hopefully nothing like that exists in the wild.
>
> Yes, there's no reason not to have such pointers, with either
> PCIe or classical PCI capability.
I think best we can do here is a comment saying this is 'best effort' attempt
to place it based on many devices using increasing addresses. (I can't claim
to have seen any that don't, but I've only looked a few dozen of my career :)
Jonathan
Hi, Shameer
On Mon, 12 Jan 2026 at 03:58, Shameer Kolothum <skolothumtho@nvidia.com> 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 <mst@redhat.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
The guest kernel fails to boot with para "ssidsize=16" with v7 series.
Without ssidsize, guest kernel can boot no problem.
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.
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)
Thanks
> ---
> hw/pci/pcie.c | 58 +++++++++++++++++++++++++++++++++++++++++++
> include/hw/pci/pcie.h | 2 ++
> 2 files changed, 60 insertions(+)
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index b302de6419..8568a062a5 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -1050,6 +1050,64 @@ static void pcie_ext_cap_set_next(PCIDevice *dev, uint16_t pos, uint16_t next)
> pci_set_long(dev->config + pos, header);
> }
>
> +/*
> + * Insert a PCIe extended capability at a given offset.
> + *
> + * This helper only validates that the insertion does not overwrite an
> + * existing PCIe extended capability header, as corrupting a header would
> + * break the extended capability linked list.
> + *
> + * The caller must ensure that (offset, size) does not overlap with other
> + * registers or capability-specific register blocks. Overlaps with
> + * capability-specific registers are not checked and are considered a
> + * user-controlled override.
> + */
> +bool pcie_insert_capability(PCIDevice *dev, uint16_t cap_id, uint8_t cap_ver,
> + uint16_t offset, uint16_t size)
> +{
> + uint16_t prev = 0, next = 0;
> + uint16_t cur = pci_get_word(dev->config + PCI_CONFIG_SPACE_SIZE);
> +
> + /* Walk the ext cap list to find insertion point */
> + while (cur) {
> + uint32_t hdr = pci_get_long(dev->config + cur);
> + next = PCI_EXT_CAP_NEXT(hdr);
> +
> + /* Check we are not overwriting any existing CAP header area */
> + if (offset >= cur && offset < cur + PCI_EXT_CAP_ALIGN) {
> + return false;
> + }
> +
> + prev = cur;
> + cur = next;
> + if (next == 0 || next > offset) {
> + break;
> + }
> + }
> +
> + /* Make sure, next CAP header area is not over written either */
> + if (next && (offset + size) >= next) {
> + return false;
> + }
> +
> + /* Insert new cap */
> + pci_set_long(dev->config + offset,
> + PCI_EXT_CAP(cap_id, cap_ver, cur));
> + if (prev) {
> + pcie_ext_cap_set_next(dev, prev, offset);
> + } else {
> + /* Insert at head (0x100) */
> + pci_set_word(dev->config + PCI_CONFIG_SPACE_SIZE, offset);
> + }
> +
> + /* 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;
> +}
> +
> /*
> * Caller must supply valid (offset, size) such that the range wouldn't
> * overlap with other capability or other registers.
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index c880ae1e04..d68bfa6257 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -133,6 +133,8 @@ uint16_t pcie_find_capability(PCIDevice *dev, uint16_t cap_id);
> void pcie_add_capability(PCIDevice *dev,
> uint16_t cap_id, uint8_t cap_ver,
> uint16_t offset, uint16_t size);
> +bool pcie_insert_capability(PCIDevice *dev, uint16_t cap_id, uint8_t cap_ver,
> + uint16_t offset, uint16_t size);
> void pcie_sync_bridge_lnk(PCIDevice *dev);
>
> void pcie_acs_init(PCIDevice *dev, uint16_t offset);
> --
> 2.43.0
>
Hi Zhangfei,
> -----Original Message-----
> From: Zhangfei Gao <zhangfei.gao@linaro.org>
> Sent: 14 January 2026 04:18
> To: Shameer Kolothum <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; ddutile@redhat.com;
> berrange@redhat.com; clg@redhat.com; alex@shazbot.org; Nathan Chen
> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> zhenzhong.duan@intel.com; yi.l.liu@intel.com; Krishnakant Jaju
> <kjaju@nvidia.com>; Michael S . Tsirkin <mst@redhat.com>
> 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
> <skolothumtho@nvidia.com> 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 <mst@redhat.com>
> > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
>
> 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;
}
On 1/14/26 11:36 AM, Shameer Kolothum wrote:
> Hi Zhangfei,
>
>> -----Original Message-----
>> From: Zhangfei Gao <zhangfei.gao@linaro.org>
>> Sent: 14 January 2026 04:18
>> To: Shameer Kolothum <skolothumtho@nvidia.com>
>> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
>> eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
>> <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; ddutile@redhat.com;
>> berrange@redhat.com; clg@redhat.com; alex@shazbot.org; Nathan Chen
>> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
>> smostafa@google.com; wangzhou1@hisilicon.com;
>> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
>> zhenzhong.duan@intel.com; yi.l.liu@intel.com; Krishnakant Jaju
>> <kjaju@nvidia.com>; Michael S . Tsirkin <mst@redhat.com>
>> 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
>> <skolothumtho@nvidia.com> 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 <mst@redhat.com>
>>> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
>> 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 I understand correctly you don't check the whole EXT CAP size but
just the header, correct?
> 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;
> }
Besides
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: 19 January 2026 16:01
> To: Shameer Kolothum <skolothumtho@nvidia.com>; Zhangfei Gao
> <zhangfei.gao@linaro.org>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> peter.maydell@linaro.org; Jason Gunthorpe <jgg@nvidia.com>; Nicolin Chen
> <nicolinc@nvidia.com>; ddutile@redhat.com; berrange@redhat.com;
> clg@redhat.com; alex@shazbot.org; Nathan Chen <nathanc@nvidia.com>;
> Matt Ochs <mochs@nvidia.com>; smostafa@google.com;
> wangzhou1@hisilicon.com; jiangkunkun@huawei.com;
> jonathan.cameron@huawei.com; zhenzhong.duan@intel.com;
> yi.l.liu@intel.com; Krishnakant Jaju <kjaju@nvidia.com>; Michael S . Tsirkin
> <mst@redhat.com>
> 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
>
>
> On 1/14/26 11:36 AM, Shameer Kolothum wrote:
> > Hi Zhangfei,
> >
> >> -----Original Message-----
> >> From: Zhangfei Gao <zhangfei.gao@linaro.org>
> >> Sent: 14 January 2026 04:18
> >> To: Shameer Kolothum <skolothumtho@nvidia.com>
> >> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> >> eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> >> <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>;
> ddutile@redhat.com;
> >> berrange@redhat.com; clg@redhat.com; alex@shazbot.org; Nathan Chen
> >> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> >> smostafa@google.com; wangzhou1@hisilicon.com;
> >> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> >> zhenzhong.duan@intel.com; yi.l.liu@intel.com; Krishnakant Jaju
> >> <kjaju@nvidia.com>; Michael S . Tsirkin <mst@redhat.com>
> >> 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
> >> <skolothumtho@nvidia.com> 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 <mst@redhat.com>
> >>> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> >> 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 I understand correctly you don't check the whole EXT CAP size but
> just the header, correct?
Yes. That’s correct.
> > 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;
> > }
> Besides
>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks,
Shameer
On Wed, 14 Jan 2026 at 18:36, Shameer Kolothum <skolothumtho@nvidia.com> wrote:
>
> Hi Zhangfei,
>
> > -----Original Message-----
> > From: Zhangfei Gao <zhangfei.gao@linaro.org>
> > Sent: 14 January 2026 04:18
> > To: Shameer Kolothum <skolothumtho@nvidia.com>
> > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> > eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> > <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; ddutile@redhat.com;
> > berrange@redhat.com; clg@redhat.com; alex@shazbot.org; Nathan Chen
> > <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> > smostafa@google.com; wangzhou1@hisilicon.com;
> > jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> > zhenzhong.duan@intel.com; yi.l.liu@intel.com; Krishnakant Jaju
> > <kjaju@nvidia.com>; Michael S . Tsirkin <mst@redhat.com>
> > 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
> > <skolothumtho@nvidia.com> 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 <mst@redhat.com>
> > > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> >
> > 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 <zhangfei.gao@linaro.org>
© 2016 - 2026 Red Hat, Inc.