This patch set fixes pci bar allocation. Cam Macdonell reported that there was something wrong with 64bit/32bit 512M BAR. This addresses his report.
Isaku Yamahata (2): seabios: pciinit: fix 64bit bar initilization. seabios: pciinit: fix overflow when bar allocation.
src/pciinit.c | 56 ++++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 36 insertions(+), 20 deletions(-)
When 64bit bar allocation failed, leave it untouched as 32bit bar case. There is no point to set higher bit to all 1, it is just leftover from debug code.
Signed-off-by: Isaku Yamahata yamahata@valinux.co.jp --- src/pciinit.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index b110531..f75e552 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -116,12 +116,8 @@ static int pci_bios_allocate_region(u16 bdf, int region_num)
int is_64bit = !(val & PCI_BASE_ADDRESS_SPACE_IO) && (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64; - if (is_64bit) { - if (size > 0) { - pci_config_writel(bdf, ofs + 4, 0); - } else { - pci_config_writel(bdf, ofs + 4, ~0); - } + if (is_64bit && size > 0) { + pci_config_writel(bdf, ofs + 4, 0); } return is_64bit; }
When allocating bar, overflow can occur. So add overflow check and don't allocate bar if overflowed. Overflow check is ugly, but necessary. Another suggested way is to change related variables u64 from u32 thus overflow can't occur because the related value are all u32 addressable. Anyway even with u64, it is necessary to the resulted value > max_u32.
Signed-off-by: Isaku Yamahata yamahata@valinux.co.jp --- src/pciinit.c | 48 ++++++++++++++++++++++++++++++++++-------------- 1 files changed, 34 insertions(+), 14 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index f75e552..9be895c 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -49,6 +49,12 @@ static void pci_set_io_region_addr(u16 bdf, int region_num, u32 addr) dprintf(1, "region %d: 0x%08x\n", region_num, addr); }
+static inline int pci_bar_overflow(u32 addr, u32 size) +{ + return (ALIGN(addr, size) < addr) || + (ALIGN(addr, size) + size < ALIGN(addr, size)); +} + /* * return value * 0: 32bit BAR @@ -78,7 +84,8 @@ static int pci_bios_allocate_region(u16 bdf, int region_num) if (val != 0) { if (val & PCI_BASE_ADDRESS_SPACE_IO) { paddr = &pci_bios_io_addr; - if (ALIGN(*paddr, size) + size >= 64 * 1024) { + if (pci_bar_overflow(*paddr, size) || + ALIGN(*paddr, size) + size >= 64 * 1024) { dprintf(1, "io region of (bdf 0x%x bar %d) can't be mapped.\n", bdf, region_num); @@ -90,7 +97,8 @@ static int pci_bios_allocate_region(u16 bdf, int region_num) /* If pci_bios_prefmem_addr == 0, keep old behaviour */ pci_bios_prefmem_addr != 0) { paddr = &pci_bios_prefmem_addr; - if (ALIGN(*paddr, size) + size >= BUILD_PCIPREFMEM_END) { + if (pci_bar_overflow(*paddr, size) || + ALIGN(*paddr, size) + size >= BUILD_PCIPREFMEM_END) { dprintf(1, "prefmem region of (bdf 0x%x bar %d) can't be mapped. " "decrease BUILD_PCIMEM_SIZE and recompile. size %x\n", @@ -99,7 +107,8 @@ static int pci_bios_allocate_region(u16 bdf, int region_num) } } else { paddr = &pci_bios_mem_addr; - if (ALIGN(*paddr, size) + size >= BUILD_PCIMEM_END) { + if (pci_bar_overflow(*paddr, size) || + ALIGN(*paddr, size) + size >= BUILD_PCIMEM_END) { dprintf(1, "mem region of (bdf 0x%x bar %d) can't be mapped. " "increase BUILD_PCIMEM_SIZE and recompile. size %x\n", @@ -152,6 +161,12 @@ static const struct pci_device_id pci_isa_bridge_tbl[] = { PCI_DEVICE_END };
+static inline int pci_addr_overflow(u32 old, u32 base, u32 addr, u32 end) +{ + /* if without overflow, old <= base <= addr <= end */ + return (old > base) || (base > addr) || (addr > end); +} + #define PCI_IO_ALIGN 4096 #define PCI_IO_SHIFT 8 #define PCI_MEMORY_ALIGN (1UL << 20) @@ -184,36 +199,41 @@ static void pci_bios_init_device_bridge(u16 bdf, void *arg) pci_bios_init_device_in_bus(secbus); }
- pci_bios_io_addr = ALIGN(pci_bios_io_addr, PCI_IO_ALIGN); - pci_bios_mem_addr = ALIGN(pci_bios_mem_addr, PCI_MEMORY_ALIGN); - pci_bios_prefmem_addr = - ALIGN(pci_bios_prefmem_addr, PCI_PREF_MEMORY_ALIGN); - - u32 io_end = pci_bios_io_addr; - if (io_end == io_base) { + u32 io_end = ALIGN(pci_bios_io_addr, PCI_IO_ALIGN); + if (pci_addr_overflow(io_old, io_base, pci_bios_io_addr, io_end) || + io_end == io_base) { pci_bios_io_addr = io_old; io_base = 0xffff; io_end = 1; + } else { + pci_bios_io_addr = io_end; } pci_config_writeb(bdf, PCI_IO_BASE, io_base >> PCI_IO_SHIFT); pci_config_writew(bdf, PCI_IO_BASE_UPPER16, 0); pci_config_writeb(bdf, PCI_IO_LIMIT, (io_end - 1) >> PCI_IO_SHIFT); pci_config_writew(bdf, PCI_IO_LIMIT_UPPER16, 0);
- u32 mem_end = pci_bios_mem_addr; - if (mem_end == mem_base) { + u32 mem_end = ALIGN(pci_bios_mem_addr, PCI_MEMORY_ALIGN); + if (pci_addr_overflow(mem_old, mem_base, pci_bios_mem_addr, mem_end) || + mem_end == mem_base) { pci_bios_mem_addr = mem_old; mem_base = 0xffffffff; mem_end = 1; + } else { + pci_bios_io_addr = mem_end; } pci_config_writew(bdf, PCI_MEMORY_BASE, mem_base >> PCI_MEMORY_SHIFT); pci_config_writew(bdf, PCI_MEMORY_LIMIT, (mem_end -1) >> PCI_MEMORY_SHIFT);
- u32 prefmem_end = pci_bios_prefmem_addr; - if (prefmem_end == prefmem_base) { + u32 prefmem_end = ALIGN(pci_bios_prefmem_addr, PCI_PREF_MEMORY_ALIGN); + if (pci_addr_overflow(prefmem_old, prefmem_base, + pci_bios_prefmem_addr, prefmem_end) || + prefmem_end == prefmem_base) { pci_bios_prefmem_addr = prefmem_old; prefmem_base = 0xffffffff; prefmem_end = 1; + } else { + pci_bios_prefmem_addr = prefmem_end; } pci_config_writew(bdf, PCI_PREF_MEMORY_BASE, prefmem_base >> PCI_PREF_MEMORY_SHIFT);
-
Isaku Yamahata