* [PATCH] pcie: enable Extended tag field capability
@ 2024-10-17 13:33 Marcin Juszkiewicz
2024-10-17 16:58 ` Michael S. Tsirkin
0 siblings, 1 reply; 5+ messages in thread
From: Marcin Juszkiewicz @ 2024-10-17 13:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Marcel Apfelbaum, Marcin Juszkiewicz
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) |
---
base-commit: f774a677507966222624a9b2859f06ede7608100
change-id: 20241017-pcie-extend-a6a9de74dbd0
Best regards,
--
Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] pcie: enable Extended tag field capability
2024-10-17 13:33 [PATCH] pcie: enable Extended tag field capability Marcin Juszkiewicz
@ 2024-10-17 16:58 ` Michael S. Tsirkin
2024-10-17 19:18 ` Marcin Juszkiewicz
2024-10-18 10:24 ` Marcin Juszkiewicz
0 siblings, 2 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2024-10-17 16:58 UTC (permalink / raw)
To: Marcin Juszkiewicz; +Cc: qemu-devel, Marcel Apfelbaum
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pcie: enable Extended tag field capability
2024-10-17 16:58 ` Michael S. Tsirkin
@ 2024-10-17 19:18 ` Marcin Juszkiewicz
2024-10-18 10:24 ` Marcin Juszkiewicz
1 sibling, 0 replies; 5+ messages in thread
From: Marcin Juszkiewicz @ 2024-10-17 19:18 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, Marcel Apfelbaum
W dniu 17.10.2024 o 18:58, Michael S. Tsirkin pisze:
>> 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.
Thanks for review. Will look how to handle it better then.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pcie: enable Extended tag field capability
2024-10-17 16:58 ` Michael S. Tsirkin
2024-10-17 19:18 ` Marcin Juszkiewicz
@ 2024-10-18 10:24 ` Marcin Juszkiewicz
2024-10-22 7:34 ` Igor Mammedov
1 sibling, 1 reply; 5+ messages in thread
From: Marcin Juszkiewicz @ 2024-10-18 10:24 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, Marcel Apfelbaum
W dniu 17.10.2024 o 18:58, Michael S. Tsirkin pisze:
>> 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.
Started looking and wonder how to pass it from MachineClass level down
to pcie.c/pcie_cap_v1_fill() level.
hw/arm/sbsa-ref.c (the machine I know best) has create_pcie() which
allocates PCI_HOST_BRIDGE and then creates 2 pcie devices (default_nic
(e1000e) and bochs-display gfx).
If I add "pcie_uses_ext_tags = true" to SBSAMachineState then I need to
have it stored in PCIBus structure so pci_create_simple() will know.
This function would copy it into "dev" (PCIDevice) to make it pass to
pcie_cap_v1_fill() function.
But that's not right way because other PCIe devices are created in other
places.
Need to dig deeper.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pcie: enable Extended tag field capability
2024-10-18 10:24 ` Marcin Juszkiewicz
@ 2024-10-22 7:34 ` Igor Mammedov
0 siblings, 0 replies; 5+ messages in thread
From: Igor Mammedov @ 2024-10-22 7:34 UTC (permalink / raw)
To: Marcin Juszkiewicz; +Cc: Michael S. Tsirkin, qemu-devel, Marcel Apfelbaum
On Fri, 18 Oct 2024 12:24:05 +0200
Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org> wrote:
> W dniu 17.10.2024 o 18:58, Michael S. Tsirkin pisze:
> >> 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.
>
> Started looking and wonder how to pass it from MachineClass level down
> to pcie.c/pcie_cap_v1_fill() level.
see as an example:
fa905f65c5549 hw/nvme: add machine compatibility parameter to enable msix exclusive bar
> hw/arm/sbsa-ref.c (the machine I know best) has create_pcie() which
> allocates PCI_HOST_BRIDGE and then creates 2 pcie devices (default_nic
> (e1000e) and bochs-display gfx).
>
> If I add "pcie_uses_ext_tags = true" to SBSAMachineState then I need to
> have it stored in PCIBus structure so pci_create_simple() will know.
> This function would copy it into "dev" (PCIDevice) to make it pass to
> pcie_cap_v1_fill() function.
>
> But that's not right way because other PCIe devices are created in other
> places.
>
> Need to dig deeper.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-10-22 7:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-17 13:33 [PATCH] pcie: enable Extended tag field capability Marcin Juszkiewicz
2024-10-17 16:58 ` Michael S. Tsirkin
2024-10-17 19:18 ` Marcin Juszkiewicz
2024-10-18 10:24 ` Marcin Juszkiewicz
2024-10-22 7:34 ` Igor Mammedov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).