diff mbox series

Should pcie-pci-bridge use 32-bit non-prefetchable memory?

Message ID 6a0b9345-a447-4cf9-ba9a-88c420e7ab79@linaro.org
State New
Headers show
Series Should pcie-pci-bridge use 32-bit non-prefetchable memory? | expand

Commit Message

Marcin Juszkiewicz Sept. 11, 2023, 10:17 a.m. UTC
I am working on aarch64/sbsa-ref machine so people can have virtual
machine to test their OS against something reminding standards compliant
system.

One of tools I use is BSA ACS (Base System Architecture - Architecture
Compliance Suite) [1] written by Arm. It runs set of tests to check does
system conforms to BSA specification.

1. https://github.com/ARM-software/bsa-acs

To run tests I use my "boot-sbsa-ref.sh" script [2] which allows me to
run exactly same setup each time.

2. https://github.com/hrw/sbsa-ref-status/blob/main/boot-sbsa-ref.sh

Since we have ITS support in whole stack (qemu, tf-a, edk2) I use
overcomplicated PCIe setup:

-device igb
-device pcie-root-port,id=root_port_for_switch1,chassis=0,slot=0
  -device x3130-upstream,id=upstream_port1,bus=root_port_for_switch1
   -device xio3130-downstream,id=downstream_port1,bus=upstream_port1,chassis=1,slot=0
    -device ac97,bus=downstream_port1
-device pcie-root-port,id=root_port_for_nvme1,chassis=2,slot=0
  -device nvme,serial=deadbeef,bus=root_port_for_nvme1
-device pcie-root-port,id=root_port_for_igb,chassis=3,slot=0
  -device igb,bus=root_port_for_igb
-device pcie-root-port,id=root_port_for_xhci,chassis=4,slot=0
  -device qemu-xhci,bus=root_port_for_xhci
-device pcie-root-port,id=root_port_for_rng,chassis=5,slot=0
  -device virtio-rng-pci,bus=root_port_for_rng
-device pcie-root-port,id=root_port_for_pci,chassis=6,slot=0
  -device pcie-pci-bridge,id=pci,bus=root_port_for_pci
   -device es1370,bus=pci,addr=9
   -device e1000,bus=pci,addr=10
-device pxb-pcie,id=pxb1,bus_nr=1
  -device pcie-root-port,id=root_port_for_ahci,bus=pxb1,chassis=10,slot=0
   -device ahci,bus=root_port_for_ahci

BSA ACS test 841 checks do Type-1 PCIe devices have 32-bit
non-prefetchable memory. And fails on pcie-pci-bridge:

Operating System View:
  841 : NP type-1 PCIe supp 32-bit only                START

        BDF - 0x400
        BDF - 0x500
        BDF - 0x600
        BDF - 0x700
        BDF - 0x800
        BDF - 0x900
        BDF - 0x10000
        BDF - 0x30000
        Skipping Non Type-1 headers
        BDF - 0x40000
        Skipping Non Type-1 headers
        BDF - 0x50000
        Skipping Non Type-1 headers
        BDF - 0x60000
        Skipping Non Type-1 headers
        BDF - 0x70000
        NP type-1 pcie is not 32-bit mem type
        Failed on PE -    0
        PCI_MM_04
        Checkpoint --  1                           : Result:  FAIL

0x70000 is pcie-pci-bridge card.

I opened issue for BSA ACS [3] and asked where the problem is.

3. https://github.com/ARM-software/bsa-acs/issues/197

Got quote from BSA (Arm Base System Architecture) [4]
chapter E.2 PCI Express Memory Space:

> When PCI Express memory space is mapped as normal memory, the system
> must support unaligned accesses to that region.
> PCI Type 1 headers, used in PCI-to-PCI bridges, and therefore in root
> ports and switches, have to be programmed with the address space
> resources claimed by the given bridge.
> For non-prefetchable (NP) memory, Type 1 headers only support 32-bit
> addresses. This implies that endpoints on the other end of a
> PCI-to-PCI bridge only support 32-bit NP BARs

4. https://developer.arm.com/documentation/den0094/latest/


I looked at code and tried to switch pcie-pci-bridge to 32-bit:

------------------------------------------------------------------------

------------------------------------------------------------------------

With it, test 841 passes.

The difference in "lspci -vvvv" output suggests that this region address
was 32-bit in both cases:

-       Region 0: Memory at 81c00000 (64-bit, non-prefetchable) [size=256]
+       Region 0: Memory at 81c00000 (32-bit, non-prefetchable) [size=256]

Any ideas how to continue from here?

Comments

Michael S. Tsirkin Sept. 11, 2023, 11:32 a.m. UTC | #1
On Mon, Sep 11, 2023 at 12:17:13PM +0200, Marcin Juszkiewicz wrote:
> I am working on aarch64/sbsa-ref machine so people can have virtual
> machine to test their OS against something reminding standards compliant
> system.
> 
> One of tools I use is BSA ACS (Base System Architecture - Architecture
> Compliance Suite) [1] written by Arm. It runs set of tests to check does
> system conforms to BSA specification.
> 
> 1. https://github.com/ARM-software/bsa-acs
> 
> To run tests I use my "boot-sbsa-ref.sh" script [2] which allows me to
> run exactly same setup each time.
> 
> 2. https://github.com/hrw/sbsa-ref-status/blob/main/boot-sbsa-ref.sh
> 
> Since we have ITS support in whole stack (qemu, tf-a, edk2) I use
> overcomplicated PCIe setup:
> 
> -device igb
> -device pcie-root-port,id=root_port_for_switch1,chassis=0,slot=0
>  -device x3130-upstream,id=upstream_port1,bus=root_port_for_switch1
>   -device xio3130-downstream,id=downstream_port1,bus=upstream_port1,chassis=1,slot=0
>    -device ac97,bus=downstream_port1
> -device pcie-root-port,id=root_port_for_nvme1,chassis=2,slot=0
>  -device nvme,serial=deadbeef,bus=root_port_for_nvme1
> -device pcie-root-port,id=root_port_for_igb,chassis=3,slot=0
>  -device igb,bus=root_port_for_igb
> -device pcie-root-port,id=root_port_for_xhci,chassis=4,slot=0
>  -device qemu-xhci,bus=root_port_for_xhci
> -device pcie-root-port,id=root_port_for_rng,chassis=5,slot=0
>  -device virtio-rng-pci,bus=root_port_for_rng
> -device pcie-root-port,id=root_port_for_pci,chassis=6,slot=0
>  -device pcie-pci-bridge,id=pci,bus=root_port_for_pci
>   -device es1370,bus=pci,addr=9
>   -device e1000,bus=pci,addr=10
> -device pxb-pcie,id=pxb1,bus_nr=1
>  -device pcie-root-port,id=root_port_for_ahci,bus=pxb1,chassis=10,slot=0
>   -device ahci,bus=root_port_for_ahci
> 
> BSA ACS test 841 checks do Type-1 PCIe devices have 32-bit
> non-prefetchable memory. And fails on pcie-pci-bridge:
> 
> Operating System View:
>  841 : NP type-1 PCIe supp 32-bit only                START
> 
>        BDF - 0x400
>        BDF - 0x500
>        BDF - 0x600
>        BDF - 0x700
>        BDF - 0x800
>        BDF - 0x900
>        BDF - 0x10000
>        BDF - 0x30000
>        Skipping Non Type-1 headers
>        BDF - 0x40000
>        Skipping Non Type-1 headers
>        BDF - 0x50000
>        Skipping Non Type-1 headers
>        BDF - 0x60000
>        Skipping Non Type-1 headers
>        BDF - 0x70000
>        NP type-1 pcie is not 32-bit mem type
>        Failed on PE -    0
>        PCI_MM_04
>        Checkpoint --  1                           : Result:  FAIL
> 
> 0x70000 is pcie-pci-bridge card.
> 
> I opened issue for BSA ACS [3] and asked where the problem is.
> 
> 3. https://github.com/ARM-software/bsa-acs/issues/197
> 
> Got quote from BSA (Arm Base System Architecture) [4]
> chapter E.2 PCI Express Memory Space:
> 
> > When PCI Express memory space is mapped as normal memory, the system
> > must support unaligned accesses to that region.
> > PCI Type 1 headers, used in PCI-to-PCI bridges, and therefore in root
> > ports and switches, have to be programmed with the address space
> > resources claimed by the given bridge.
> > For non-prefetchable (NP) memory, Type 1 headers only support 32-bit
> > addresses. This implies that endpoints on the other end of a
> > PCI-to-PCI bridge only support 32-bit NP BARs
> 
> 4. https://developer.arm.com/documentation/den0094/latest/
> 
> 
> I looked at code and tried to switch pcie-pci-bridge to 32-bit:
> 
> ------------------------------------------------------------------------
> diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
> index 2301b2ca0b..45199d2fa0 100644
> --- a/hw/pci-bridge/pcie_pci_bridge.c
> +++ b/hw/pci-bridge/pcie_pci_bridge.c
> @@ -82,7 +82,7 @@ static void pcie_pci_bridge_realize(PCIDevice *d, Error **errp)
>          }
>      }
>      pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
> -                     PCI_BASE_ADDRESS_MEM_TYPE_64, &pcie_br->shpc_bar);
> +                     PCI_BASE_ADDRESS_MEM_TYPE_32, &pcie_br->shpc_bar);
>      return;
> 
>  msi_error:
> 
> ------------------------------------------------------------------------
> 
> With it, test 841 passes.
> 
> The difference in "lspci -vvvv" output suggests that this region address
> was 32-bit in both cases:
> 
> -       Region 0: Memory at 81c00000 (64-bit, non-prefetchable) [size=256]
> +       Region 0: Memory at 81c00000 (32-bit, non-prefetchable) [size=256]
> 
> Any ideas how to continue from here?

It's a bug in BSA spec then. PCI Express allows 64 bit non prefetcheable
BARs in devices even though the top 32 bit just goes unused on all
systems.

Try to raise this with them?

but the express spec also says:

On PCI Express systems that meet the criteria enumerated below, setting the Prefetchable bit in a candidate BAR
will still permit correct operation even if the BAR’s range includes some locations that have read side-effects or
cannot tolerate write merging. This is primarily due to the fact that PCI Express Memory Reads always contain an
explicit length, and PCI Express Switches never prefetch or do byte merging. Generally only 64-bit BARs are good
candidates, since only Legacy Endpoints are permitted to set the Prefetchable bit in 32-bit BARs, and most
scalable platforms map all 32-bit Memory BARs into non-prefetchable Memory Space regardless of the
Prefetchable bit value.
Here are criteria that are sufficient to guarantee correctness for a given candidate BAR:
• The entire path from the host to the adapter is over PCI Express.
• No conventional PCI or PCI-X devices do peer-to-peer reads to the range mapped by the BAR.
• The PCI Express Host Bridge does no byte merging. (This is believed to be true on most platforms.)
• Any locations with read side-effects are never the target of Memory Reads with the TH bit Set. See
§ Section 2.2.5 .
• The range mapped by the BAR is never the target of a speculative Memory Read, either Host initiated or
peer-to-peer.

I need to look into implications of the TH bit, but maybe it's ok to map
SHPC prefetcheable.
diff mbox series

Patch

diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
index 2301b2ca0b..45199d2fa0 100644
--- a/hw/pci-bridge/pcie_pci_bridge.c
+++ b/hw/pci-bridge/pcie_pci_bridge.c
@@ -82,7 +82,7 @@  static void pcie_pci_bridge_realize(PCIDevice *d, Error **errp)
          }
      }
      pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
-                     PCI_BASE_ADDRESS_MEM_TYPE_64, &pcie_br->shpc_bar);
+                     PCI_BASE_ADDRESS_MEM_TYPE_32, &pcie_br->shpc_bar);
      return;

  msi_error: