On Wed, Nov 04, 2026 at 07:27:09PM +0000, Jonathan Cameron wrote:
> On Thu, 29 Oct 2026 15:05:11 +0000
> Jim MacArthur <[email protected]> wrote:
>
> > From: Pierrick Bouvier <[email protected]>
> >
> > Adds the TEE flag in the legacy config space, and the IDE
> > (Integrity & Data Encryption) extended capability. These
> > will be present when rme-cap=on is specified for the
> > pcie-root-port device.
> >
> > Signed-off-by: Pierrick Bouvier <[email protected]>
> > Signed-off-by: Jim MacArthur <Jim [email protected]>
> > ---
> > hw/pci-bridge/gen_pcie_root_port.c | 21 ++++++++++++++++++++-
> > hw/pci/pcie.c | 19 +++++++++++++++++++
> > include/hw/pci/pcie.h | 3 +++
> > include/hw/pci/pcie_regs.h | 4 ++++
> > 4 files changed, 46 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/pci-bridge/gen_pcie_root_port.c
> > b/hw/pci-bridge/gen_pcie_root_port.c
> > index 5434d693d9..ab8087da87 100644
> > --- a/hw/pci-bridge/gen_pcie_root_port.c
> > +++ b/hw/pci-bridge/gen_pcie_root_port.c
> > @@ -26,13 +26,16 @@ OBJECT_DECLARE_SIMPLE_TYPE(GenPCIERootPort,
> > GEN_PCIE_ROOT_PORT)
> > #define GEN_PCIE_ROOT_PORT_AER_OFFSET 0x100
> > #define GEN_PCIE_ROOT_PORT_ACS_OFFSET \
> > (GEN_PCIE_ROOT_PORT_AER_OFFSET + PCI_ERR_SIZEOF)
> > -
> > +#define GEN_PCIE_ROOT_PORT_ACS_END \
> > + (GEN_PCIE_ROOT_PORT_ACS_OFFSET + PCI_ACS_SIZEOF)
>
> Can't you figure this out from root port class acs offset?
>
> Or if you want it in a fixed place follow that pattern and stash it
> in the root class during dev_rp_dev_class_init.
>
There seems to be quite a lot of cases where we're arranging capabilities in
sequential order in memory, but hard-coding the locations. I would rather all
of this be replaced with a dynamic pointer which records the next capability
offset, and it looks like there are attempts to do this with, e.g.
pcie_insert_capability, but that has a lot of caveats and only gets used once.
I'll have another think about this, and in the meantime if anyone knows a good
reason why we do hardcode this, that would be useful.
> > +
> > + if (grp->rme_caps) {
> Not arm specific. Can't see any harm in using a generic config name
> for this. Actually I'm not seeing a reason not to always include
> the ide cap. Maybe less so for setting up tee. Haven't really thought about
> that.
>
Good point, IDE/TEE could be separate named options quite easily. There might
be more Arm RME things included later, but that can be a third option.
Thanks for the review,
Jim
