From 0da41145ea40ecb657cf4c83d411e5463a901ec5 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 19:50:10 +0200 Subject: [PATCH] F-12066: pci: restore original COMMAND and clear windows on bridge error pci_program_bridge() used orig_cmd both as the saved COMMAND register value and as the accumulator for the decode bits enabled while programming. Error paths after a window was programmed (post-enum MMIO or IO alignment failures) restored that mutated value and left the programmed bridge windows active, so the bridge kept decoding address ranges the allocator rollback had just returned. Keep the two values separate: saved_cmd holds the original register content for the error path, new_cmd accumulates the decode bits and is written on success (seeded from saved_cmd, so the success path preserves the bits it did not manage, exactly as before). The error path now disables every bridge window (prefetch, MMIO, IO) before restoring saved_cmd. Test: test_program_bridge_oom_late_restore programs a prefetch window behind the bridge, then exhausts the MMIO pool so the post-enum MMIO alignment fails. Pre-fix the restored COMMAND was 0x0006 (original 0x0004 plus the MEM_SPACE bit for the discarded window) and the prefetch window stayed programmed (0x9000-0x900F); post-fix the original COMMAND is restored and all windows are disabled. --- src/pci.c | 32 ++++++++++++----- tools/unit-tests/unit-pci.c | 71 +++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 8 deletions(-) diff --git a/src/pci.c b/src/pci.c index d87b6b96..934f8ed3 100644 --- a/src/pci.c +++ b/src/pci.c @@ -623,7 +623,8 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, uint64_t prefetch_start; uint64_t mem_start; uint64_t io_start; - uint32_t orig_cmd; + uint32_t saved_cmd; + uint32_t new_cmd; uint8_t saved_bus; uint64_t saved_mem; uint64_t saved_pf; @@ -635,8 +636,12 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, saved_pf = info->mem_pf; saved_io = info->io; - orig_cmd = pci_config_read16(bus, dev, fun, PCI_COMMAND_OFFSET); + saved_cmd = pci_config_read16(bus, dev, fun, PCI_COMMAND_OFFSET); pci_config_write16(bus, dev, fun, PCI_COMMAND_OFFSET, 0); + /* decode bits are accumulated from the original value so the + * success path preserves the bits it did not manage; the error + * path restores saved_cmd itself */ + new_cmd = saved_cmd; /* curr_bus_number is one bus per bridge level; at 0xFF the next * increment wraps to 0, which would write SECONDARY_BUS 0 and @@ -699,7 +704,7 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, prefetch_start >> 16); pci_config_write16(bus, dev, fun, PCI_PREFETCH_LIMIT_OFF, (info->mem_pf - 1) >> 16); - orig_cmd |= PCI_COMMAND_MEM_SPACE; + new_cmd |= PCI_COMMAND_MEM_SPACE; } else { /* disable prefetch */ pci_config_write16(bus, dev, fun, PCI_PREFETCH_BASE_OFF, @@ -719,7 +724,7 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, mem_start >> 16); pci_config_write16(bus, dev, fun, PCI_MMIO_LIMIT_OFF, (info->mem - 1) >> 16); - orig_cmd |= PCI_COMMAND_MEM_SPACE; + new_cmd |= PCI_COMMAND_MEM_SPACE; } else { /* disable mem */ pci_config_write16(bus, dev, fun, PCI_MMIO_BASE_OFF, @@ -739,7 +744,7 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, io_start >> 8); pci_config_write8(bus, dev, fun, PCI_IO_LIMIT_OFF, (info->io - 1) >> 8); - orig_cmd |= PCI_COMMAND_IO_SPACE; + new_cmd |= PCI_COMMAND_IO_SPACE; } else { pci_config_write8(bus, dev, fun, PCI_IO_BASE_OFF, @@ -748,8 +753,8 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, 0x0); } - orig_cmd |= PCI_COMMAND_BUS_MASTER; - pci_config_write16(bus, dev, fun, PCI_COMMAND_OFFSET, orig_cmd); + new_cmd |= PCI_COMMAND_BUS_MASTER; + pci_config_write16(bus, dev, fun, PCI_COMMAND_OFFSET, new_cmd); pci_dump_bridge(bus,dev,fun); return 0; @@ -759,10 +764,21 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, info->mem = saved_mem; info->mem_pf = saved_pf; info->io = saved_io; + /* Disable every window that may have been programmed before the + * error: the allocator cursors are rolled back, so the bridge must + * not keep decoding the returned address ranges. */ + pci_config_write16(bus, dev, fun, PCI_PREFETCH_BASE_OFF, 0xffff); + pci_config_write16(bus, dev, fun, PCI_PREFETCH_LIMIT_OFF, 0x0); + pci_config_write16(bus, dev, fun, PCI_MMIO_BASE_OFF, 0xffff); + pci_config_write16(bus, dev, fun, PCI_MMIO_LIMIT_OFF, 0x0); + pci_config_write8(bus, dev, fun, PCI_IO_BASE_OFF, 0xff); + pci_config_write8(bus, dev, fun, PCI_IO_LIMIT_OFF, 0x0); pci_config_write8(bus, dev, fun, PCI_PRIMARY_BUS, 0); pci_config_write8(bus, dev, fun, PCI_SECONDARY_BUS, 0); pci_config_write8(bus, dev, fun, PCI_SUB_SEC_BUS, 0); - pci_config_write16(bus, dev, fun, PCI_COMMAND_OFFSET, orig_cmd); + /* restore the original COMMAND value, not the decode bits + * accumulated for the discarded windows */ + pci_config_write16(bus, dev, fun, PCI_COMMAND_OFFSET, saved_cmd); return -1; } diff --git a/tools/unit-tests/unit-pci.c b/tools/unit-tests/unit-pci.c index 67d5dd39..6d967d3e 100644 --- a/tools/unit-tests/unit-pci.c +++ b/tools/unit-tests/unit-pci.c @@ -1552,6 +1552,73 @@ START_TEST(test_program_bridge_oom_post_enum) } END_TEST +/* F-12066: on a post-enum error the bridge must be left fully + * disabled. The prefetch window is programmed (and the MEM_SPACE + * decode bit accumulated) before the MMIO post-enum alignment fails; + * the error path must restore the original COMMAND register value and + * disable every window, not restore a mutated COMMAND with the + * programmed prefetch window still active for an address range the + * allocator rollback just returned. */ +START_TEST(test_program_bridge_oom_late_restore) +{ + struct test_pci_topology t; + struct pci_enum_info info; + int br, ep, ret; + uint16_t cmd_before = 0x0004; /* master only: no decode bits */ + uint16_t cmd, pfbase, pflimit, mbase, mlimit; + uint8_t iobase, iolimit; + + test_pci_init(&t); + br = test_pci_add_bridge(&t, 1, 0, 0xAAAA, 0xBBBB, TEST_PCI_ROOT_BUS); + ep = test_pci_add_dev(&t, 0, 0, 0xCCCC, 0xDDDD, br); + /* 64KB prefetchable BAR: programs the bridge prefetch window */ + test_pci_dev_set_bar(&t, ep, 0, 0x10000, TEST_PCI_BAR_PF); + /* 64KB MMIO BAR: consumes the whole 1MB pool, so the post-enum + * MMIO alignment (aligned start == limit) fails after the + * prefetch window was programmed. */ + test_pci_dev_set_bar(&t, ep, 1, 0x10000, TEST_PCI_BAR_MMIO); + test_pci_commit(&t); + memcpy(&t.nodes[br].cfg[PCI_COMMAND_OFFSET], &cmd_before, 2); + + memset(&info, 0, sizeof(info)); + info.mem = 0x80000000; + info.mem_limit = 0x80100000; + info.mem_pf = 0x90000000; + info.mem_pf_limit = 0xFFFFFFFF; + info.io = 0x2000; + info.curr_bus_number = 0; + + ret = pci_program_bridge(0, 1, 0, &info); + ck_assert_int_eq(ret, -1); + + /* original COMMAND restored, not the value with MEM_SPACE added */ + cmd = pci_config_read16(0, 1, 0, PCI_COMMAND_OFFSET); + ck_assert_uint_eq(cmd, cmd_before); + + /* every bridge window disabled */ + pfbase = pci_config_read16(0, 1, 0, PCI_PREFETCH_BASE_OFF); + pflimit = pci_config_read16(0, 1, 0, PCI_PREFETCH_LIMIT_OFF); + ck_assert_uint_eq(pfbase, 0xFFFF); + ck_assert_uint_eq(pflimit, 0x0000); + mbase = pci_config_read16(0, 1, 0, PCI_MMIO_BASE_OFF); + mlimit = pci_config_read16(0, 1, 0, PCI_MMIO_LIMIT_OFF); + ck_assert_uint_eq(mbase, 0xFFFF); + ck_assert_uint_eq(mlimit, 0x0000); + iobase = pci_config_read8(0, 1, 0, PCI_IO_BASE_OFF); + iolimit = pci_config_read8(0, 1, 0, PCI_IO_LIMIT_OFF); + ck_assert_uint_eq(iobase, 0xFF); + ck_assert_uint_eq(iolimit, 0x00); + + /* allocator cursors rolled back */ + ck_assert_uint_eq(info.mem, 0x80000000); + ck_assert_uint_eq(info.mem_pf, 0x90000000); + ck_assert_uint_eq(info.io, 0x2000); + ck_assert_uint_eq(info.curr_bus_number, 0); + + test_pci_cleanup(&t); +} +END_TEST + /* test_program_bridge_bus_exhaustion: curr_bus_number is one bus per * bridge level; at 0xFF the next increment wraps to 0, writing * SECONDARY_BUS 0 and re-enumerating bus 0 over the already configured @@ -2078,6 +2145,10 @@ Suite *wolfboot_suite(void) tcase_add_test(tc_oom_post, test_program_bridge_oom_post_enum); suite_add_tcase(s, tc_oom_post); + TCase *tc_oom_late = tcase_create("bridge-oom-late-restore"); + tcase_add_test(tc_oom_late, test_program_bridge_oom_late_restore); + suite_add_tcase(s, tc_oom_late); + TCase *tc_bus_exhaust = tcase_create("bridge-bus-exhaustion"); tcase_add_test(tc_bus_exhaust, test_program_bridge_bus_exhaustion); suite_add_tcase(s, tc_bus_exhaust);