From 7db653b29210d9e7ba210f3233bce93a7040c051 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 26 Aug 2026 17:16:39 +0200 Subject: [PATCH] F-11031: initialize PCI MMIO pool limits as exclusive ends pci_enum_do() set mem_limit/mem_pf_limit to base + length - 1, i.e. the last usable byte, while every consumer of the limits compares them as exclusive ends: pci_enum_next_aligned32() rejects a start >= limit, the BAR end check rejects a region whose end is > limit, and pci_align_check_up() rejects an aligned start >= limit. The IO pool limit (PCI_IO32_LIMIT) is already the exclusive 16-bit ceiling. With the inclusive-style init the pool effectively lost its last byte and a BAR that exactly fills a configured pool (e.g. a 128 MB non-prefetchable MMIO BAR on the default 128 MB pool) was skipped instead of mapped. Initialize the MMIO and prefetch limits as base + length and reject a pool whose end would wrap the 32-bit address space (custom PCI_MMIO32_BASE/LENGTH definitions), computed in 64 bits so the check holds on every host word size. unit-pci gains test_enum_do_pool_fill, which drives the real pci_enum_do() over a 128 MB BAR that exactly fills the default pool; pre-fix the BAR was restored to its original value (never mapped). Verification: - Built: gcc (host) unit-pci with -DWOLFBOOT_USE_PCI: clean. - Tested: unit-pci 29/29; pre-fix the new test failed with the BAR restored to 0 instead of programmed at 0x80000000. - Pitfalls: single- and multi-BAR allocations under a partially filled pool are unaffected (region end <= base + length still fits); the overflow guard only rejects pools that cannot be represented in 32-bit address space. - Style: cstyle-check.sh flag output on src/pci.c identical to the pre-change file; the new test adds one C99-decl line in the suite registration, the class the existing registrations already trip. - Message: F-11031: prefix, no co-author trailers. --- src/pci.c | 19 +++++++++++++++++-- tools/unit-tests/unit-pci.c | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/pci.c b/src/pci.c index 357bf9cc..64240ee2 100644 --- a/src/pci.c +++ b/src/pci.c @@ -926,11 +926,26 @@ int pci_enum_do(void) struct pci_enum_info enum_info; int ret; + /* Pool limits are exclusive ends: the allocator accepts a region + * 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 || + (uint64_t)PCI_MMIO32_PREFETCH_BASE + + PCI_MMIO32_PREFETCH_LENGTH > 0xFFFFFFFFULL) + { + PCI_DEBUG_PRINTF("PCI MMIO pool overflows the 32-bit address " + "space\r\n"); + return -1; + } + enum_info.mem = PCI_MMIO32_BASE; - enum_info.mem_limit = enum_info.mem + (PCI_MMIO32_LENGTH - 1); + enum_info.mem_limit = enum_info.mem + PCI_MMIO32_LENGTH; enum_info.mem_pf = PCI_MMIO32_PREFETCH_BASE; enum_info.mem_pf_limit = enum_info.mem_pf + - (PCI_MMIO32_PREFETCH_LENGTH - 1); + PCI_MMIO32_PREFETCH_LENGTH; enum_info.io = PCI_IO32_BASE; enum_info.curr_bus_number = 0; diff --git a/tools/unit-tests/unit-pci.c b/tools/unit-tests/unit-pci.c index 3dd28c0e..bff46596 100644 --- a/tools/unit-tests/unit-pci.c +++ b/tools/unit-tests/unit-pci.c @@ -1631,6 +1631,38 @@ START_TEST(test_enum_do_full) } END_TEST +/* test_enum_do_pool_fill: a BAR that exactly fills the configured MMIO + * pool [PCI_MMIO32_BASE, PCI_MMIO32_BASE + PCI_MMIO32_LENGTH) must be + * mapped. The pool limits are exclusive ends: the allocator accepts a + * region when its end is <= limit (pci_enum_next_aligned32, the BAR + * end check, pci_align_check_up) and the IO pool limit is the 16-bit + * ceiling, not the last usable address. Initializing the MMIO limits + * as base + length - 1 rejects this BAR and strands the last byte of + * the pool. */ +START_TEST(test_enum_do_pool_fill) +{ + 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); + /* 128 MB MMIO BAR: exactly the default pool size */ + test_pci_dev_set_bar(&t, dev_node, 0, 0x08000000, TEST_PCI_BAR_MMIO); + test_pci_commit(&t); + + ret = pci_enum_do(); + ck_assert_int_eq(ret, 0); + + /* The BAR must be programmed at the pool base */ + bar_val = pci_config_read32(0, 0, 0, PCI_BAR0_OFFSET); + ck_assert_uint_eq(bar_val, 0x80000000); + + test_pci_cleanup(&t); +} +END_TEST + /* test_enum_do_nested_bridges: end-to-end nested bridge enumeration */ START_TEST(test_enum_do_nested_bridges) @@ -1936,6 +1968,10 @@ Suite *wolfboot_suite(void) tcase_add_test(tc_enum_nested, test_enum_do_nested_bridges); suite_add_tcase(s, tc_enum_nested); + TCase *tc_enum_pool = tcase_create("enum-do-pool-fill"); + tcase_add_test(tc_enum_pool, test_enum_do_pool_fill); + suite_add_tcase(s, tc_enum_pool); + TCase *tc_rw8 = tcase_create("config-rw-8bit-positions"); tcase_add_test(tc_rw8, test_config_rw_8bit_all_positions); suite_add_tcase(s, tc_rw8);