This patch set fixes PCI bar allocation when bar overflow occured. I checked if pmm_alloc facility can be used, but it doesn't suit for pci bar allocation. So I resulted in new API, pci_region which encapsulates region allocation and overflow checks. The first patch introduces pci_region, and the second patch fixes the overflow case with pci_region.
Isaku Yamahata (2): pci: introduce pci_region to manage pci io/memory/prefmemory regions. pciinit: use pci_region functions.
Makefile | 3 +- src/pci_region.c | 70 +++++++++++++++++++++++++++++++ src/pciinit.c | 122 ++++++++++++++++++++++++++--------------------------- src/util.h | 15 +++++++ 4 files changed, 147 insertions(+), 63 deletions(-) create mode 100644 src/pci_region.c
This patch adds helper functions to manage pci area.
Signed-off-by: Isaku Yamahata yamahata@valinux.co.jp --- Makefile | 3 +- src/pci_region.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/util.h | 15 +++++++++++ 3 files changed, 87 insertions(+), 1 deletions(-) create mode 100644 src/pci_region.c
diff --git a/Makefile b/Makefile index 9d412f1..1663a5d 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,8 @@ SRCBOTH=misc.c pmm.c stacks.c output.c util.c block.c floppy.c ata.c mouse.c \ SRC16=$(SRCBOTH) system.c disk.c font.c SRC32FLAT=$(SRCBOTH) post.c shadow.c memmap.c coreboot.c boot.c \ acpi.c smm.c mptable.c smbios.c pciinit.c optionroms.c mtrr.c \ - lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c dev-i440fx.c + lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c dev-i440fx.c \ + pci_region.c SRC32SEG=util.c output.c pci.c pcibios.c apm.c stacks.c
cc-option = $(shell if test -z "`$(1) $(2) -S -o /dev/null -xc \ diff --git a/src/pci_region.c b/src/pci_region.c new file mode 100644 index 0000000..a4e71d9 --- /dev/null +++ b/src/pci_region.c @@ -0,0 +1,70 @@ +// helper functions to manage pci io/memory/prefetch memory region +// +// Copyright (C) 2009 Isaku Yamahata <yamahata at valinux co jp> +// +// This file may be distributed under the terms of the GNU LGPLv3 license. +// +// + +#include "util.h" + +#define PCI_REGION_DISABLED (-1) + +void pci_region_init(struct pci_region *r, u32 start, u32 end) +{ + r->start = start; + r->end = end; + + r->cur_end = start; +} + +static u32 pci_region_alloc_align(struct pci_region *r, u32 size, u32 align) +{ + if (r->cur_end == PCI_REGION_DISABLED) { + return 0; + } + + u32 s = ALIGN(r->cur_end, align); + if (s > r->end || s < r->cur_end) { + return 0; + } + u32 e = s + size; + if (e > r->end || e < s) { + return 0; + } + r->cur_end = e; + return s; +} + +u32 pci_region_alloc(struct pci_region *r, u32 size) +{ + return pci_region_alloc_align(r, size, size); +} + +u32 pci_region_align(struct pci_region *r, u32 align) +{ + return pci_region_alloc_align(r, 0, align); +} + +void pci_region_revert(struct pci_region *r, u32 addr) +{ + r->cur_end = addr; +} + +u32 pci_region_disable(struct pci_region *r) +{ + return r->cur_end = PCI_REGION_DISABLED; +} + +u32 pci_region_addr(const struct pci_region *r) +{ + if (r->cur_end == PCI_REGION_DISABLED){ + return r->end; + } + return r->cur_end; +} + +u32 pci_region_size(const struct pci_region *r) +{ + return r->end - r->start; +} diff --git a/src/util.h b/src/util.h index 5cc9f17..ecd1c16 100644 --- a/src/util.h +++ b/src/util.h @@ -344,6 +344,21 @@ void qemu_prep_reset(void); void smm_save_and_copy(void); void smm_relocate_and_restore(void);
+// pci_region.c +struct pci_region { + u32 start; + u32 end; + + u32 cur_end; +}; +void pci_region_init(struct pci_region *r, u32 start, u32 end); +u32 pci_region_alloc(struct pci_region *r, u32 size); +u32 pci_region_align(struct pci_region *r, u32 align); +void pci_region_revert(struct pci_region *r, u32 addr); +u32 pci_region_disable(struct pci_region *r); +u32 pci_region_addr(const struct pci_region *r); +u32 pci_region_size(const struct pci_region *r); + // pciinit.c extern const u8 pci_irqs[4]; void pci_bios_allocate_regions(u16 bdf, void *arg);
This patch cleans up pci region allocation with pci_region. Now it is aware of overflow.
Signed-off-by: Isaku Yamahata yamahata@valinux.co.jp --- src/pciinit.c | 122 ++++++++++++++++++++++++++++----------------------------- 1 files changed, 60 insertions(+), 62 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index 0346423..2a01aaa 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -17,9 +17,10 @@
static void pci_bios_init_device_in_bus(int bus);
-static u32 pci_bios_io_addr; -static u32 pci_bios_mem_addr; -static u32 pci_bios_prefmem_addr; +static struct pci_region pci_bios_io_region; +static struct pci_region pci_bios_mem_region; +static struct pci_region pci_bios_prefmem_region; + /* host irqs corresponding to PCI irqs A-D */ const u8 pci_irqs[4] = { 10, 10, 11, 11 @@ -54,7 +55,7 @@ static void pci_set_io_region_addr(u16 bdf, int region_num, u32 addr) */ static int pci_bios_allocate_region(u16 bdf, int region_num) { - u32 *paddr; + struct pci_region *r; u32 ofs = pci_bar(bdf, region_num);
u32 old = pci_config_readl(bdf, ofs); @@ -74,41 +75,34 @@ static int pci_bios_allocate_region(u16 bdf, int region_num)
u32 size = (~(val & mask)) + 1; if (val != 0) { + const char *type; + const char *msg; if (val & PCI_BASE_ADDRESS_SPACE_IO) { - paddr = &pci_bios_io_addr; - if (ALIGN(*paddr, size) + size >= 64 * 1024) { - dprintf(1, - "io region of (bdf 0x%x bar %d) can't be mapped.\n", - bdf, region_num); - size = 0; - } + r = &pci_bios_io_region; + type = "io"; + msg = ""; } else if ((val & PCI_BASE_ADDRESS_MEM_PREFETCH) && - /* keep behaviour on bus = 0 */ - pci_bdf_to_bus(bdf) != 0 && - /* 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) { - dprintf(1, - "prefmem region of (bdf 0x%x bar %d) can't be mapped. " - "decrease BUILD_PCIMEM_SIZE and recompile. size %x\n", - bdf, region_num, BUILD_PCIPREFMEM_SIZE); - size = 0; - } + /* keep behaviour on bus = 0 */ + pci_bdf_to_bus(bdf) != 0 && + /* If pci_bios_prefmem_addr == 0, keep old behaviour */ + pci_region_addr(&pci_bios_prefmem_region) != 0) { + r = &pci_bios_prefmem_region; + type = "prefmem"; + msg = "decrease BUILD_PCIMEM_SIZE and recompile. size %x"; } else { - paddr = &pci_bios_mem_addr; - if (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", - bdf, region_num, BUILD_PCIMEM_SIZE); - size = 0; - } + r = &pci_bios_mem_region; + type = "mem"; + msg = "increase BUILD_PCIMEM_SIZE and recompile."; } - if (size > 0) { - *paddr = ALIGN(*paddr, size); - pci_set_io_region_addr(bdf, region_num, *paddr); - *paddr += size; + u32 addr = pci_region_alloc(r, size); + if (addr > 0) { + pci_set_io_region_addr(bdf, region_num, addr); + } else { + size = 0; + dprintf(1, + "%s region of (bdf 0x%x bar %d) can't be mapped. " + "%s size %x\n", + type, bdf, region_num, msg, pci_region_size(r)); } }
@@ -163,33 +157,34 @@ static void pci_bios_init_device_bridge(u16 bdf, void *arg) pci_bios_allocate_region(bdf, 1); pci_bios_allocate_region(bdf, PCI_ROM_SLOT);
- u32 io_old = pci_bios_io_addr; - u32 mem_old = pci_bios_mem_addr; - u32 prefmem_old = pci_bios_prefmem_addr; + u32 io_old = pci_region_addr(&pci_bios_io_region); + u32 mem_old = pci_region_addr(&pci_bios_mem_region); + u32 prefmem_old = pci_region_addr(&pci_bios_prefmem_region);
/* IO BASE is assumed to be 16 bit */ - 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); + if (pci_region_align(&pci_bios_io_region, PCI_IO_ALIGN) == 0) { + pci_region_disable(&pci_bios_io_region); + } + if (pci_region_align(&pci_bios_mem_region, PCI_MEMORY_ALIGN) == 0) { + pci_region_disable(&pci_bios_mem_region); + } + if (pci_region_align(&pci_bios_prefmem_region, + PCI_PREF_MEMORY_ALIGN) == 0) { + pci_region_disable(&pci_bios_prefmem_region); + }
- u32 io_base = pci_bios_io_addr; - u32 mem_base = pci_bios_mem_addr; - u32 prefmem_base = pci_bios_prefmem_addr; + u32 io_base = pci_region_addr(&pci_bios_io_region); + u32 mem_base = pci_region_addr(&pci_bios_mem_region); + u32 prefmem_base = pci_region_addr(&pci_bios_prefmem_region);
u8 secbus = pci_config_readb(bdf, PCI_SECONDARY_BUS); if (secbus > 0) { 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) { - pci_bios_io_addr = io_old; + u32 io_end = pci_region_align(&pci_bios_io_region, PCI_IO_ALIGN); + if (io_end == 0) { + pci_region_revert(&pci_bios_io_region, io_old); io_base = 0xffff; io_end = 1; } @@ -198,18 +193,19 @@ static void pci_bios_init_device_bridge(u16 bdf, void *arg) 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) { - pci_bios_mem_addr = mem_old; + u32 mem_end = pci_region_align(&pci_bios_mem_region, PCI_MEMORY_ALIGN); + if (mem_end == 0) { + pci_region_revert(&pci_bios_mem_region, mem_old); mem_base = 0xffffffff; mem_end = 1; } 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) { - pci_bios_prefmem_addr = prefmem_old; + u32 prefmem_end = pci_region_align(&pci_bios_prefmem_region, + PCI_PREF_MEMORY_ALIGN); + if (prefmem_end == 0) { + pci_region_revert(&pci_bios_prefmem_region, prefmem_old); prefmem_base = 0xffffffff; prefmem_end = 1; } @@ -406,9 +402,11 @@ pci_setup(void)
dprintf(3, "pci setup\n");
- pci_bios_io_addr = 0xc000; - pci_bios_mem_addr = BUILD_PCIMEM_START; - pci_bios_prefmem_addr = BUILD_PCIPREFMEM_START; + pci_region_init(&pci_bios_io_region, 0xc000, 64 * 1024); + pci_region_init(&pci_bios_mem_region, + BUILD_PCIMEM_START, BUILD_PCIMEM_END); + pci_region_init(&pci_bios_prefmem_region, + BUILD_PCIPREFMEM_START, BUILD_PCIPREFMEM_END);
pci_bios_init_bus();
On Mon, Oct 18, 2010 at 06:34:21PM +0900, Isaku Yamahata wrote:
This patch set fixes PCI bar allocation when bar overflow occured. I checked if pmm_alloc facility can be used, but it doesn't suit for pci bar allocation. So I resulted in new API, pci_region which encapsulates region allocation and overflow checks. The first patch introduces pci_region, and the second patch fixes the overflow case with pci_region.
Isaku Yamahata (2): pci: introduce pci_region to manage pci io/memory/prefmemory regions. pciinit: use pci_region functions.
Makefile | 3 +- src/pci_region.c | 70 +++++++++++++++++++++++++++++++ src/pciinit.c | 122 ++++++++++++++++++++++++++--------------------------- src/util.h | 15 +++++++ 4 files changed, 147 insertions(+), 63 deletions(-) create mode 100644 src/pci_region.c
Could you clarify what do you mean by bar overflow please?
On Mon, Oct 18, 2010 at 11:47:42AM +0200, Michael S. Tsirkin wrote:
On Mon, Oct 18, 2010 at 06:34:21PM +0900, Isaku Yamahata wrote:
This patch set fixes PCI bar allocation when bar overflow occured. I checked if pmm_alloc facility can be used, but it doesn't suit for pci bar allocation. So I resulted in new API, pci_region which encapsulates region allocation and overflow checks. The first patch introduces pci_region, and the second patch fixes the overflow case with pci_region.
Isaku Yamahata (2): pci: introduce pci_region to manage pci io/memory/prefmemory regions. pciinit: use pci_region functions.
Makefile | 3 +- src/pci_region.c | 70 +++++++++++++++++++++++++++++++ src/pciinit.c | 122 ++++++++++++++++++++++++++--------------------------- src/util.h | 15 +++++++ 4 files changed, 147 insertions(+), 63 deletions(-) create mode 100644 src/pci_region.c
Could you clarify what do you mean by bar overflow please?
This is originally raised by Cam. http://www.seabios.org/pipermail/seabios/2010-August/000888.html For example, pci_bios_bios() has *paddr += size. It may overflow and really does with huge bar(1G, 2G...) reported by Cam and Adnan.
This patch series is revised version of the following as Kevin has requested me to try to use pmm_malloc() in order to avoid ugly overflow check. http://www.seabios.org/pipermail/seabios/2010-July/000794.html
-
Isaku Yamahata -
Michael S. Tsirkin