diff mbox series

[v2,1/1] pcie: enable Extended tag field support

Message ID 20241023113820.486017-1-marcin.juszkiewicz@linaro.org (mailing list archive)
State New, archived
Headers show
Series [v2,1/1] pcie: enable Extended tag field support | expand

Commit Message

Marcin Juszkiewicz Oct. 23, 2024, 11:38 a.m. UTC
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).

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-

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/

This change enables Extended tag field. All versioned platforms should
have it disabled for older versions (tested with Arm/virt).

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 hw/core/machine.c    | 4 +++-
 hw/pci/pci.c         | 2 ++
 hw/pci/pcie.c        | 8 +++++++-
 include/hw/pci/pci.h | 2 ++
 4 files changed, 14 insertions(+), 2 deletions(-)

Comments

Marcin Juszkiewicz Oct. 31, 2024, 11:10 a.m. UTC | #1
W dniu 23.10.2024 o 13:38, Marcin Juszkiewicz pisze:
>  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).

Ping?


> 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-
> 
> 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/
> 
> This change enables Extended tag field. All versioned platforms should
> have it disabled for older versions (tested with Arm/virt).
> 
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---
>   hw/core/machine.c    | 4 +++-
>   hw/pci/pci.c         | 2 ++
>   hw/pci/pcie.c        | 8 +++++++-
>   include/hw/pci/pci.h | 2 ++
>   4 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index adaba17eba..8ccc74067a 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -34,7 +34,9 @@
>   #include "hw/virtio/virtio-iommu.h"
>   #include "audio/audio.h"
>   
> -GlobalProperty hw_compat_9_1[] = {};
> +GlobalProperty hw_compat_9_1[] = {
> +    { TYPE_PCI_DEVICE, "x-pcie-ext-tag", "false" },
> +};
>   const size_t hw_compat_9_1_len = G_N_ELEMENTS(hw_compat_9_1);
>   
>   GlobalProperty hw_compat_9_0[] = {
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 87da35ca9b..9a3b0e4a43 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -87,6 +87,8 @@ static Property pci_props[] = {
>                       QEMU_PCIE_ARI_NEXTFN_1_BITNR, false),
>       DEFINE_PROP_SIZE32("x-max-bounce-buffer-size", PCIDevice,
>                        max_bounce_buffer_size, DEFAULT_MAX_BOUNCE_BUFFER_SIZE),
> +    DEFINE_PROP_BIT("x-pcie-ext-tag", PCIDevice, cap_present,
> +                    QEMU_PCIE_EXT_TAG_BITNR, true),
>       DEFINE_PROP_END_OF_LIST()
>   };
>   
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 4b2f0805c6..9f369c2b6c 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -86,7 +86,13 @@ 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);
> +    uint32_t devcap = PCI_EXP_DEVCAP_RBER;
> +
> +    if (dev->cap_present & QEMU_PCIE_EXT_TAG) {
> +        devcap = PCI_EXP_DEVCAP_RBER | PCI_EXP_DEVCAP_EXT_TAG;
> +    }
> +
> +    pci_set_long(exp_cap + PCI_EXP_DEVCAP, devcap);
>   
>       pci_set_long(exp_cap + PCI_EXP_LNKCAP,
>                    (port << PCI_EXP_LNKCAP_PN_SHIFT) |
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index eb26cac810..5b14f9d375 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -213,6 +213,8 @@ enum {
>       QEMU_PCIE_ERR_UNC_MASK = (1 << QEMU_PCIE_ERR_UNC_MASK_BITNR),
>   #define QEMU_PCIE_ARI_NEXTFN_1_BITNR 12
>       QEMU_PCIE_ARI_NEXTFN_1 = (1 << QEMU_PCIE_ARI_NEXTFN_1_BITNR),
> +#define QEMU_PCIE_EXT_TAG_BITNR 13
> +    QEMU_PCIE_EXT_TAG = (1 << QEMU_PCIE_EXT_TAG_BITNR),
>   };
>   
>   typedef struct PCIINTxRoute {
Michael S. Tsirkin Oct. 31, 2024, 11:42 a.m. UTC | #2
On Thu, Oct 31, 2024 at 12:10:26PM +0100, Marcin Juszkiewicz wrote:
> W dniu 23.10.2024 o 13:38, Marcin Juszkiewicz pisze:
> >  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).
> 
> Ping?
> 

tagged, thanks!

> > 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-
> > 
> > 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/
> > 
> > This change enables Extended tag field. All versioned platforms should
> > have it disabled for older versions (tested with Arm/virt).
> > 
> > Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> > ---
> >   hw/core/machine.c    | 4 +++-
> >   hw/pci/pci.c         | 2 ++
> >   hw/pci/pcie.c        | 8 +++++++-
> >   include/hw/pci/pci.h | 2 ++
> >   4 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index adaba17eba..8ccc74067a 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -34,7 +34,9 @@
> >   #include "hw/virtio/virtio-iommu.h"
> >   #include "audio/audio.h"
> > -GlobalProperty hw_compat_9_1[] = {};
> > +GlobalProperty hw_compat_9_1[] = {
> > +    { TYPE_PCI_DEVICE, "x-pcie-ext-tag", "false" },
> > +};
> >   const size_t hw_compat_9_1_len = G_N_ELEMENTS(hw_compat_9_1);
> >   GlobalProperty hw_compat_9_0[] = {
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 87da35ca9b..9a3b0e4a43 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -87,6 +87,8 @@ static Property pci_props[] = {
> >                       QEMU_PCIE_ARI_NEXTFN_1_BITNR, false),
> >       DEFINE_PROP_SIZE32("x-max-bounce-buffer-size", PCIDevice,
> >                        max_bounce_buffer_size, DEFAULT_MAX_BOUNCE_BUFFER_SIZE),
> > +    DEFINE_PROP_BIT("x-pcie-ext-tag", PCIDevice, cap_present,
> > +                    QEMU_PCIE_EXT_TAG_BITNR, true),
> >       DEFINE_PROP_END_OF_LIST()
> >   };
> > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > index 4b2f0805c6..9f369c2b6c 100644
> > --- a/hw/pci/pcie.c
> > +++ b/hw/pci/pcie.c
> > @@ -86,7 +86,13 @@ 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);
> > +    uint32_t devcap = PCI_EXP_DEVCAP_RBER;
> > +
> > +    if (dev->cap_present & QEMU_PCIE_EXT_TAG) {
> > +        devcap = PCI_EXP_DEVCAP_RBER | PCI_EXP_DEVCAP_EXT_TAG;
> > +    }
> > +
> > +    pci_set_long(exp_cap + PCI_EXP_DEVCAP, devcap);
> >       pci_set_long(exp_cap + PCI_EXP_LNKCAP,
> >                    (port << PCI_EXP_LNKCAP_PN_SHIFT) |
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index eb26cac810..5b14f9d375 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -213,6 +213,8 @@ enum {
> >       QEMU_PCIE_ERR_UNC_MASK = (1 << QEMU_PCIE_ERR_UNC_MASK_BITNR),
> >   #define QEMU_PCIE_ARI_NEXTFN_1_BITNR 12
> >       QEMU_PCIE_ARI_NEXTFN_1 = (1 << QEMU_PCIE_ARI_NEXTFN_1_BITNR),
> > +#define QEMU_PCIE_EXT_TAG_BITNR 13
> > +    QEMU_PCIE_EXT_TAG = (1 << QEMU_PCIE_EXT_TAG_BITNR),
> >   };
> >   typedef struct PCIINTxRoute {
diff mbox series

Patch

diff --git a/hw/core/machine.c b/hw/core/machine.c
index adaba17eba..8ccc74067a 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -34,7 +34,9 @@ 
 #include "hw/virtio/virtio-iommu.h"
 #include "audio/audio.h"
 
-GlobalProperty hw_compat_9_1[] = {};
+GlobalProperty hw_compat_9_1[] = {
+    { TYPE_PCI_DEVICE, "x-pcie-ext-tag", "false" },
+};
 const size_t hw_compat_9_1_len = G_N_ELEMENTS(hw_compat_9_1);
 
 GlobalProperty hw_compat_9_0[] = {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 87da35ca9b..9a3b0e4a43 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -87,6 +87,8 @@  static Property pci_props[] = {
                     QEMU_PCIE_ARI_NEXTFN_1_BITNR, false),
     DEFINE_PROP_SIZE32("x-max-bounce-buffer-size", PCIDevice,
                      max_bounce_buffer_size, DEFAULT_MAX_BOUNCE_BUFFER_SIZE),
+    DEFINE_PROP_BIT("x-pcie-ext-tag", PCIDevice, cap_present,
+                    QEMU_PCIE_EXT_TAG_BITNR, true),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 4b2f0805c6..9f369c2b6c 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -86,7 +86,13 @@  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);
+    uint32_t devcap = PCI_EXP_DEVCAP_RBER;
+
+    if (dev->cap_present & QEMU_PCIE_EXT_TAG) {
+        devcap = PCI_EXP_DEVCAP_RBER | PCI_EXP_DEVCAP_EXT_TAG;
+    }
+
+    pci_set_long(exp_cap + PCI_EXP_DEVCAP, devcap);
 
     pci_set_long(exp_cap + PCI_EXP_LNKCAP,
                  (port << PCI_EXP_LNKCAP_PN_SHIFT) |
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index eb26cac810..5b14f9d375 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -213,6 +213,8 @@  enum {
     QEMU_PCIE_ERR_UNC_MASK = (1 << QEMU_PCIE_ERR_UNC_MASK_BITNR),
 #define QEMU_PCIE_ARI_NEXTFN_1_BITNR 12
     QEMU_PCIE_ARI_NEXTFN_1 = (1 << QEMU_PCIE_ARI_NEXTFN_1_BITNR),
+#define QEMU_PCIE_EXT_TAG_BITNR 13
+    QEMU_PCIE_EXT_TAG = (1 << QEMU_PCIE_EXT_TAG_BITNR),
 };
 
 typedef struct PCIINTxRoute {