| Message ID | 20241023113820.486017-1-marcin.juszkiewicz@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/1] pcie: enable Extended tag field support | expand |
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 {
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 --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 {
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(-)