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 |
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 --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: