Re: [PATCH v2 3/4] hw/pci: add DVSEC RMEDA capability to PCIe root port

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Sep 04, 2026 at 05:05:03PM +0100, Jim MacArthur wrote:
> From: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
> 
> After this we can see all the capabilities listed for the PCIe bridge:
> 
> 00:05.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port (prog-if 00 [Normal decode])
> 
> 	...
> 
> 	Capabilities: [54] Express (v2) Root Port (Slot+), IntMsgNum 0
> 
> 	...
> 
> 	Capabilities: [48] MSI-X: Enable+ Count=1 Masked-
> 		Vector table: BAR=0 offset=00000000
> 		PBA: BAR=0 offset=00000800
> 	Capabilities: [40] Subsystem: Red Hat, Inc. Device 0000
> 	Capabilities: [100 v2] Advanced Error Reporting
> 		UESta:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP-
> 			ECRC- UnsupReq- ACSViol- UncorrIntErr- BlockedTLP- AtomicOpBlocked- TLPBlockedErr-
> 			PoisonTLPBlocked- DMWrReqBlocked- IDECheck- MisIDETLP- PCRC_CHECK- TLPXlatBlocked-
> 		UEMsk:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP-
> 			ECRC- UnsupReq- ACSViol- UncorrIntErr+ BlockedTLP- AtomicOpBlocked- TLPBlockedErr+
> 			PoisonTLPBlocked- DMWrReqBlocked- IDECheck- MisIDETLP- PCRC_CHECK- TLPXlatBlocked-
> 		UESvrt:	DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+
> 			ECRC- UnsupReq- ACSViol- UncorrIntErr+ BlockedTLP- AtomicOpBlocked- TLPBlockedErr-
> 			PoisonTLPBlocked- DMWrReqBlocked- IDECheck- MisIDETLP- PCRC_CHECK- TLPXlatBlocked-
> 		CESta:	RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- CorrIntErr- HeaderOF-
> 		CEMsk:	RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr+ CorrIntErr+ HeaderOF+
> 		AERCap:	First Error Pointer: 00, ECRCGenCap+ ECRCGenEn- ECRCChkCap+ ECRCChkEn-
> 			MultHdrRecCap+ MultHdrRecEn- TLPPfxPres- HdrLogCap-
> 		HeaderLog: 00000000 00000000 00000000 00000000
> 		RootCmd: CERptEn+ NFERptEn+ FERptEn+
> 		RootSta: CERcvd- MultCERcvd- UERcvd- MultUERcvd-
> 			 FirstFatal- NonFatalMsg- FatalMsg- IntMsgNum 0
> 		ErrorSrc: ERR_COR: 0000 ERR_FATAL/NONFATAL: 0000
> 	Capabilities: [148 v1] Access Control Services
> 		ACSCap:	SrcValid+ TransBlk+ ReqRedir+ CmpltRedir+ UpstreamFwd+ EgressCtrl- DirectTrans+
> 		ACSCtl:	SrcValid+ TransBlk- ReqRedir+ CmpltRedir+ UpstreamFwd+ EgressCtrl- DirectTrans-
> 	Capabilities: [150 v1] Designated Vendor-Specific: Vendor=13b5 ID=ff01 Rev=0 Len=16 <?>
> 	Capabilities: [164 v1] Integrity & Data Encryption
> 		IDECap: Lnk=0 Sel=1 FlowThru- PartHdr- Aggr- PCPC- IDE_KM- Alg='AES-GCM-256-96b' TCs=1 TeeLim-
> 		IDECtl: FTEn-
> 		[170]SelectiveIDE#0 Cap: RID#=0
> 		[174]SelectiveIDE#0 Ctl: En- NPR- PR- CPL- PCRC- CFG- HdrEnc=no Alg='AES-GCM-256-96b' TC0 ID0
> 		[178]SelectiveIDE#0 Sta: insecure RecvChkFail-
> 		[17c]SelectiveIDE#0 RID: Valid- Base=0 Limit=0 SegBase=0
> 	Kernel driver in use: pcieport
> ---
>  hw/pci-bridge/gen_pcie_root_port.c | 30 ++++++++++++++++++++++++++++++
>  include/hw/pci/pcie_regs.h         |  4 ++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> index 452ebf69b3..035212508d 100644
> --- a/hw/pci-bridge/gen_pcie_root_port.c
> +++ b/hw/pci-bridge/gen_pcie_root_port.c
> @@ -104,6 +104,36 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
>      }
>  
>      uint32_t offset = GEN_PCIE_ROOT_PORT_ACS_END;
> +    /*
> +     * dvsec rme-da
> +     * https://developer.arm.com/documentation/den0129/latest/
> +     * Arm RME System Architecture
> +     * 0x0000 RMEDA_ECH See B3.2.6.2.1 RME-DA Extended Capability Header
> +     * 0x0004 RMEDA_HEAD1 See B3.2.6.2.2 RME-DA DVSEC Header 1
> +     * 0x0008 RMEDA_HEAD2 See B3.2.6.2.3 RME-DA DVSEC Header 2
> +     * 0x000C RMEDA_CTL1 See B3.2.6.2.4 RME-DA Control register 1
> +     * 0x0010 RMEDA_CTL2 See B3.2.6.2.5 RME-DA Control register 2
> +     */
> +    pcie_add_capability(d, PCI_EXT_CAP_ID_DVSEC, PCI_RMEDA_VER, offset,
> +                        PCI_RMEDA_SIZEOF);

don't put declarations after code.

> +    const uint32_t header1 = 0x010013b5;
> +    const uint32_t header2 = 0xFF01;

what are these things?

> +    const uint32_t ctl1 = 0x1; /* support tdisp */
> +    const uint32_t ctl2 = 0x0; /* unlocked */


so why do we need these vars? just open-code.

> +    pci_set_long(d->config + offset + 0x4, header1);
> +    pci_set_long(d->config + offset + 0x8, header2);
> +    pci_set_long(d->config + offset + 0xC, ctl1);
> +    pci_set_long(d->config + offset + 0x10, ctl2);
> +    d->wmask[offset + 0xC] = 0xff;
> +    d->wmask[offset + 0xC + 1] = 0xff;
> +    d->wmask[offset + 0xC + 2] = 0xff;
> +    d->wmask[offset + 0xC + 3] = 0xff;
> +    d->wmask[offset + 0x10] = 0xff;
> +    d->wmask[offset + 0x10 + 1] = 0xff;
> +    d->wmask[offset + 0x10 + 2] = 0xff;
> +    d->wmask[offset + 0x10 + 3] = 0xff;
> +    offset += PCI_RMEDA_SIZEOF;
> +
>      pcie_ide_init(d, offset);
>      offset += PCI_IDE_SIZEOF;

You can't unconditionally change config space, this will break
cross version migration.
You must use a property (beginning with "x-") and compat machinery.


Besides, having all root ports on all arches have this capability
looks very strange. For example not everything supports tdisp.

> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
> index aa21177411..233378fd79 100644
> --- a/include/hw/pci/pcie_regs.h
> +++ b/include/hw/pci/pcie_regs.h
> @@ -186,4 +186,8 @@ typedef enum PCIExpLinkWidth {
>  #define PCI_IDE_VER                     0x1
>  #define PCI_IDE_SIZEOF                  12
>  
> +/* RME-DA */
> +#define PCI_RMEDA_VER                   0x1
> +#define PCI_RMEDA_SIZEOF                20
> +
>  #endif /* QEMU_PCIE_REGS_H */
> 
> -- 
> 2.43.0