From b115363291ddae38205de918bd50586e76c7ada5 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 26 Aug 2026 18:59:48 +0200 Subject: [PATCH] pci: accept MMIO pools whose exclusive end is 4 GiB A pool ending exactly at 0x100000000 (e.g. 0xC0000000 + 0x40000000, the classic top-half 32-bit MMIO layout) was a working configuration: the old base + length - 1 initialization wrapped to 0xFFFFFFFF in 32-bit arithmetic. The overflow guard from the exclusive-limit fix rejected such pools with base + length > 0xFFFFFFFF, aborting enumeration - and the FSP caller discards the return value, so the platform would boot with no PCI BARs programmed. The limit fields cannot hold the exclusive end 0x100000000 while 32-bit, so widen mem_limit and mem_pf_limit (and the limit parameters of pci_enum_next_aligned32 and pci_align_check_up, plus the local in pci_program_bar) to 64-bit, and reject only pools whose end is above the 32-bit space. The initialization now casts to 64-bit before the addition so the sum cannot wrap. The T10xx PCIe setup initializes the same struct with the old inclusive base + length - 1 form; align it to the exclusive semantics the allocator enforces, or the last byte of the configured pool is unusable. New unit-pci-4gib build of the existing test file with the MMIO pool [0xC0000000, 0x100000000): pci_enum_do() must accept the pool and map a 1 MB BAR at the pool base. Fails on the old guard. --- hal/nxp_t10xx.c | 9 ++++--- include/pci.h | 6 +++-- src/pci.c | 20 ++++++++------- tools/unit-tests/Makefile | 16 ++++++++++-- tools/unit-tests/unit-pci.c | 51 +++++++++++++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 16 deletions(-) diff --git a/hal/nxp_t10xx.c b/hal/nxp_t10xx.c index 9fcd2f7e..292feaa0 100644 --- a/hal/nxp_t10xx.c +++ b/hal/nxp_t10xx.c @@ -1796,10 +1796,13 @@ static int hal_pcie_init(void) memset(&enum_info, 0, sizeof(enum_info)); enum_info.curr_bus_number = 0; enum_info.mem = CONFIG_PCIE_MEM_BUS; - enum_info.mem_limit = enum_info.mem + (CONFIG_PCIE_MEM_LENGTH - 1); + /* Pool limits are exclusive ends (the allocator accepts a + * region when its end is <= limit). */ + enum_info.mem_limit = (uint64_t)enum_info.mem + + CONFIG_PCIE_MEM_LENGTH; enum_info.mem_pf = (enum_info.mem + CONFIG_PCIE_MEM_PREFETCH_LENGTH); - enum_info.mem_pf_limit = enum_info.mem_pf + - (CONFIG_PCIE_MEM_PREFETCH_LENGTH - 1); + enum_info.mem_pf_limit = (uint64_t)enum_info.mem_pf + + CONFIG_PCIE_MEM_PREFETCH_LENGTH; enum_info.io = CONFIG_PCIE_IO_BASE; /* Setup PCIe Output Windows */ diff --git a/include/pci.h b/include/pci.h index 77c669d7..e4819871 100644 --- a/include/pci.h +++ b/include/pci.h @@ -90,10 +90,12 @@ typedef struct { struct pci_enum_info { uint32_t mem; - uint32_t mem_limit; + /* Exclusive pool ends. 64-bit: a pool may end exactly at 4 GiB + * (0x100000000), which a 32-bit field cannot represent. */ + uint64_t mem_limit; uint32_t io; uint32_t mem_pf; - uint32_t mem_pf_limit; + uint64_t mem_pf_limit; uint8_t curr_bus_number; }; diff --git a/src/pci.c b/src/pci.c index 797cc6b5..b2376f38 100644 --- a/src/pci.c +++ b/src/pci.c @@ -109,7 +109,7 @@ static inline uint32_t align_down(uint32_t address, uint32_t alignment) { } static int pci_align_check_up(uint32_t address, uint32_t alignment, - uint32_t limit, uint32_t *aligned) + uint64_t limit, uint32_t *aligned) { uint32_t a; a = align_up(address, alignment); @@ -364,7 +364,7 @@ static int pci_enum_is_mmio(uint32_t value) } static int pci_enum_next_aligned32(uint32_t address, uint32_t *next, - uint32_t align, uint32_t limit) + uint32_t align, uint64_t limit) { uintptr_t addr; @@ -422,7 +422,7 @@ static int pci_program_bar(uint8_t bus, uint8_t dev, uint8_t fun, uint8_t bar_off; int is_prefetch; uint32_t *base; - uint32_t limit; + uint64_t limit; uint32_t reg; int is_mmio; int ret = 0; @@ -937,11 +937,13 @@ int pci_enum_do(void) * when its end is <= limit (pci_enum_next_aligned32, the BAR end * check, pci_align_check_up) and the IO limit is the 16-bit IO * ceiling, not the last usable address. A region ending exactly - * at base + length must fit, so initialize base + length; reject - * a pool whose end would wrap the 32-bit address space. */ - if ((uint64_t)PCI_MMIO32_BASE + PCI_MMIO32_LENGTH > 0xFFFFFFFFULL || + * at base + length must fit, so initialize base + length. The + * limit fields are 64-bit because a pool may end exactly at + * 0x100000000 (4 GiB), the top of the 32-bit space; reject only + * pools whose end is above it. */ + if ((uint64_t)PCI_MMIO32_BASE + PCI_MMIO32_LENGTH > 0x100000000ULL || (uint64_t)PCI_MMIO32_PREFETCH_BASE + - PCI_MMIO32_PREFETCH_LENGTH > 0xFFFFFFFFULL) + PCI_MMIO32_PREFETCH_LENGTH > 0x100000000ULL) { PCI_DEBUG_PRINTF("PCI MMIO pool overflows the 32-bit address " "space\r\n"); @@ -949,9 +951,9 @@ int pci_enum_do(void) } enum_info.mem = PCI_MMIO32_BASE; - enum_info.mem_limit = enum_info.mem + PCI_MMIO32_LENGTH; + enum_info.mem_limit = (uint64_t)enum_info.mem + PCI_MMIO32_LENGTH; enum_info.mem_pf = PCI_MMIO32_PREFETCH_BASE; - enum_info.mem_pf_limit = enum_info.mem_pf + + enum_info.mem_pf_limit = (uint64_t)enum_info.mem_pf + PCI_MMIO32_PREFETCH_LENGTH; enum_info.io = PCI_IO32_BASE; enum_info.curr_bus_number = 0; diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 39dcc8f3..6ff20bac 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -56,8 +56,8 @@ endif TESTS:=unit-parser unit-parser-large-header unit-fdt unit-extflash unit-string \ unit-spi-flash unit-aes128 \ unit-uart-flash \ - unit-aes256 unit-chacha20 unit-pci unit-mock-state unit-sectorflags \ - unit-max-space \ + unit-aes256 unit-chacha20 unit-pci unit-pci-4gib unit-mock-state \ + unit-sectorflags unit-max-space \ unit-image unit-image-hybrid unit-image-rsa unit-nvm unit-nvm-flagshome unit-enc-nvm \ unit-enc-nvm-flagshome unit-delta unit-gzip unit-update-flash unit-update-flash-delta \ unit-update-flash-hook \ @@ -619,6 +619,18 @@ unit-chacha20: ../../include/target.h unit-extflash.c unit-pci: unit-pci.c ../../src/pci.c gcc -o $@ $< $(CFLAGS) -DWOLFBOOT_USE_PCI $(LDFLAGS) +# unit-pci-4gib reruns the same test file with the MMIO pool ending +# exactly at the 4 GiB boundary ([0xC0000000, 0x100000000)), which a +# 32-bit pool limit cannot represent. Only the pool-end test runs. +unit-pci-4gib: unit-pci.c ../../src/pci.c + gcc -o $@ $< $(CFLAGS) -DWOLFBOOT_USE_PCI \ + -DUNIT_TEST_PCI_POOL_4GIB \ + -DPCI_MMIO32_BASE=0xC0000000ULL \ + -DPCI_MMIO32_LENGTH=0x40000000ULL \ + -DPCI_MMIO32_PREFETCH_BASE=0x80000000ULL \ + -DPCI_MMIO32_PREFETCH_LENGTH=0x40000000ULL \ + $(LDFLAGS) + # linux_loader.c is x86 32bit only and pulls in inline asm guarded on 32bit; # build standalone with -m32 and without coverage (no 32bit gcov/check libs). unit-linux-loader-e820: ../../include/target.h unit-linux-loader-e820.c diff --git a/tools/unit-tests/unit-pci.c b/tools/unit-tests/unit-pci.c index f75dfd74..6dd2527e 100644 --- a/tools/unit-tests/unit-pci.c +++ b/tools/unit-tests/unit-pci.c @@ -1731,6 +1731,36 @@ START_TEST(test_enum_do_pool_fill) } END_TEST +#ifdef UNIT_TEST_PCI_POOL_4GIB +/* Compiled only in the unit-pci-4gib build, where the MMIO pool is + * [0xC0000000, 0x100000000): its exclusive end sits exactly on the + * 4 GiB boundary. A 32-bit limit field cannot represent that end, so + * the pool may be rejected only when the end is above the boundary, + * and a BAR must still be mappable from such a pool. */ +START_TEST (test_pool_end_4gib) +{ + struct test_pci_topology t; + int dev_node; + uint32_t bar_val; + int ret; + + test_pci_init(&t); + dev_node = test_pci_add_dev(&t, 0, 0, 0x1234, 0x5678, TEST_PCI_ROOT_BUS); + test_pci_dev_set_bar(&t, dev_node, 0, 0x00100000, TEST_PCI_BAR_MMIO); + test_pci_commit(&t); + + ret = pci_enum_do(); + ck_assert_int_eq(ret, 0); + + /* The BAR is allocated at the pool base */ + bar_val = pci_config_read32(0, 0, 0, PCI_BAR0_OFFSET); + ck_assert_uint_eq(bar_val, 0xC0000000); + + test_pci_cleanup(&t); +} +END_TEST +#endif /* UNIT_TEST_PCI_POOL_4GIB */ + /* test_enum_do_nested_bridges: end-to-end nested bridge enumeration */ START_TEST(test_enum_do_nested_bridges) @@ -2059,6 +2089,7 @@ Suite *wolfboot_suite(void) return s; } +#ifndef UNIT_TEST_PCI_POOL_4GIB int main(void) { int fails; @@ -2069,3 +2100,23 @@ int main(void) srunner_free(sr); return fails; } +#else +/* The 4 GiB-end pool build runs only the pool-end test: the rest of + * the suite assumes the default 128 MB pool layout. */ +int main(void) +{ + int fails; + Suite *s = suite_create("pci-pool-4gib"); + TCase *tc = tcase_create("pool-end-4gib"); + SRunner *sr; + + tcase_add_test(tc, test_pool_end_4gib); + suite_add_tcase(s, tc); + + sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +} +#endif /* UNIT_TEST_PCI_POOL_4GIB */