Re: [PATCH] pcie: enable Extended tag field capability

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

On Thu, Oct 17, 2024 at 03:33:44PM +0200, Marcin Juszkiewicz wrote:
> PCI has 32 transactions, PCI Express devices can handle 256.
> 
> SBSA ACS checks for this capability to be enabled on Arm server systems.
> 
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---
> SBSA Reference Platform work goes on so I am looking at PCIe related tests.
> 
> SBSA ACS has test 824 which checks for PCIe device capabilities. BSA
> specification [1] (SBSA is on top of BSA) in section F.3.2 lists
> expected values for Device Capabilities Register:
> 
> Device Capabilities Register     Requirement
> Role based error reporting       RCEC and RCiEP: Hardwired to 1
> Endpoint L0s acceptable latency  RCEC and RCiEP: Hardwired to 0
> L1 acceptable latency            RCEC and RCiEP: Hardwired to 0
> Captured slot power limit scale  RCEC and RCiEP: Hardwired to 0
> Captured slot power limit value  RCEC and RCiEP: Hardwired to 0
> Max payload size                 value must be compliant with PCIe spec
> Phantom functions                RCEC and RCiEP: Recommendation is to
>                                  hardwire this bit to 0.
> Extended tag field               Hardwired to 1
> 
> 1. https://developer.arm.com/documentation/den0094/c/
> 
> QEMU leaves 'Extended tag field' with 0 as value:
> 
> Capabilities: [e0] Express (v1) Root Complex Integrated Endpoint, IntMsgNum 0
>         DevCap: MaxPayload 128 bytes, PhantFunc 0
>                 ExtTag- RBE+ FLReset- TEE-IO-
> 
> >From what I read PCI has 32 transactions, PCI Express devices can handle
> 256 with Extended tag enabled (spec mentions also larger values but I
> lack PCIe knowledge).
> ---
>  hw/pci/pcie.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 4b2f0805c6..54c0f1ec67 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -86,7 +86,8 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t version)
>       * Specification, Revision 1.1., or subsequent PCI Express Base
>       * Specification revisions.
>       */
> -    pci_set_long(exp_cap + PCI_EXP_DEVCAP, PCI_EXP_DEVCAP_RBER);
> +    pci_set_long(exp_cap + PCI_EXP_DEVCAP, PCI_EXP_DEVCAP_RBER |
> +                 PCI_EXP_DEVCAP_EXT_TAG);
>  
>      pci_set_long(exp_cap + PCI_EXP_LNKCAP,
>                   (port << PCI_EXP_LNKCAP_PN_SHIFT) |


We can't change capabilities unconditionally.
It needs at least a machine type compat thing.

> ---
> base-commit: f774a677507966222624a9b2859f06ede7608100
> change-id: 20241017-pcie-extend-a6a9de74dbd0
> 
> Best regards,
> -- 
> Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>