[SeaBIOS] [PATCH v4 1/2] pciinit: refactor mem init code

Michael S. Tsirkin mst at redhat.com
Mon Jul 29 13:30:13 CEST 2013


On Mon, Jul 29, 2013 at 01:23:21PM +0200, Igor Mammedov wrote:
> On Mon, 29 Jul 2013 12:33:46 +0300
> "Michael S. Tsirkin" <mst at redhat.com> wrote:
> 
> > Refactor memory initialization code:
> > - split window initialization from other setup
> > - pass all window data around by pointer
> > 
> > This is in preparation for getting window data
> > from qemu.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> > ---
> >  src/pciinit.c | 102 +++++++++++++++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 79 insertions(+), 23 deletions(-)
> > 
> > diff --git a/src/pciinit.c b/src/pciinit.c
> > index 6d7f8fa..ef5e33b 100644
> > --- a/src/pciinit.c
> > +++ b/src/pciinit.c
> > @@ -59,6 +59,13 @@ struct pci_bus {
> >      struct pci_device *bus_dev;
> >  };
> >  
> > +struct pci_mem {
> > +    u64 start32;
> > +    u64 end32;
> > +    u64 start64;
> > +    u64 end64;
> > +};
> > +
> >  static u32 pci_bar(struct pci_device *pci, int region_num)
> >  {
> >      if (region_num != PCI_ROM_SLOT) {
> > @@ -357,17 +364,22 @@ static void pci_enable_default_vga(void)
> >   * Platform device initialization
> >   ****************************************************************/
> >  
> > +void i440fx_setup(struct pci_device *dev, void *arg)
> > +{
> > +    pci_slot_get_irq = piix_pci_slot_get_irq;
> > +}
> > +
> >  void i440fx_mem_addr_setup(struct pci_device *dev, void *arg)
> >  {
> > +    struct pci_mem *mem = arg;
> > +
> >      if (RamSize <= 0x80000000)
> > -        pcimem_start = 0x80000000;
> > +        mem->start32 = 0x80000000;
> >      else if (RamSize <= 0xc0000000)
> > -        pcimem_start = 0xc0000000;
> > -
> > -    pci_slot_get_irq = piix_pci_slot_get_irq;
> > +        mem->start32 = 0xc0000000;
> >  }
> >  
> > -void mch_mem_addr_setup(struct pci_device *dev, void *arg)
> > +void mch_setup(struct pci_device *dev, void *arg)
> >  {
> >      u64 addr = Q35_HOST_BRIDGE_PCIEXBAR_ADDR;
> >      u32 size = Q35_HOST_BRIDGE_PCIEXBAR_SIZE;
> > @@ -381,13 +393,20 @@ void mch_mem_addr_setup(struct pci_device *dev, void *arg)
> >      pci_config_writel(bdf, Q35_HOST_BRIDGE_PCIEXBAR, lower);
> >      add_e820(addr, size, E820_RESERVED);
> >  
> > -    /* setup pci i/o window (above mmconfig) */
> > -    pcimem_start = addr + size;
> > -
> >      pci_slot_get_irq = mch_pci_slot_get_irq;
> >  }
> >  
> > -static const struct pci_device_id pci_platform_tbl[] = {
> > +void mch_mem_addr_setup(struct pci_device *dev, void *arg)
> > +{
> > +    struct pci_mem *mem = arg;
> > +    u64 addr = Q35_HOST_BRIDGE_PCIEXBAR_ADDR;
> > +    u32 size = Q35_HOST_BRIDGE_PCIEXBAR_SIZE;
> > +
> > +    /* setup pci i/o window (above mmconfig) */
> > +    mem->start32 = addr + size;
> > +}
> > +
> > +static const struct pci_device_id pci_mem_addr_setup_tbl[] = {
> >      PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441,
> >                 i440fx_mem_addr_setup),
> >      PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_Q35_MCH,
> > @@ -395,11 +414,34 @@ static const struct pci_device_id pci_platform_tbl[] = {
> >      PCI_DEVICE_END
> >  };
> >  
> > +static const struct pci_device_id pci_platform_setup_tbl[] = {
> > +    PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441,
> > +               i440fx_setup),
> > +    PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_Q35_MCH,
> > +               mch_setup),
> > +    PCI_DEVICE_END
> > +};
> > +
> > +static void pci_bios_init_mem_addr(struct pci_mem *mem)
> > +{
> > +    struct pci_device *pci;
> > +
> > +    /* Default in case no device found */
> > +    mem->start32 = RamSize;
> > +    mem->end32 = BUILD_PCIMEM_END;
> > +    mem->start64 = 0x100000000LL + RamSizeOver4G;
> > +    mem->end64 = 0;
> perhaps mem->end64 = mem->start64 to avoid negative diff if it's ever used?

An empty range is as likely to crash guest, no?
I haven't actually tried.

Below we panic on mem->end64 < mem->start64, this
won't trigger if end == start.


> > +
> > +    foreachpci(pci) {
> > +        pci_init_device(pci_mem_addr_setup_tbl, pci, mem);
> > +    }
> > +}
> > +
> >  static void pci_bios_init_platform(void)
> >  {
> >      struct pci_device *pci;
> >      foreachpci(pci) {
> > -        pci_init_device(pci_platform_tbl, pci, NULL);
> > +        pci_init_device(pci_platform_setup_tbl, pci, NULL);
> >      }
> >  }
> >  
> > @@ -676,7 +718,7 @@ static int pci_bios_check_devices(struct pci_bus *busses)
> >   ****************************************************************/
> >  
> >  // Setup region bases (given the regions' size and alignment)
> > -static int pci_bios_init_root_regions(struct pci_bus *bus)
> > +static int pci_bios_init_root_regions(struct pci_bus *bus, struct pci_mem *mem)
> >  {
> >      bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
> >  
> > @@ -690,13 +732,13 @@ static int pci_bios_init_root_regions(struct pci_bus *bus)
> >      }
> >      u64 sum = pci_region_sum(r_end);
> >      u64 align = pci_region_align(r_end);
> > -    r_end->base = ALIGN_DOWN((pcimem_end - sum), align);
> > +    r_end->base = ALIGN_DOWN((mem->end32 - sum), align);
> >      sum = pci_region_sum(r_start);
> >      align = pci_region_align(r_start);
> >      r_start->base = ALIGN_DOWN((r_end->base - sum), align);
> >  
> > -    if ((r_start->base < pcimem_start) ||
> > -         (r_start->base > pcimem_end))
> > +    if ((r_start->base < mem->start32) ||
> > +         (r_start->base > mem->end32))
> >          // Memory range requested is larger than available.
> >          return -1;
> >      return 0;
> > @@ -755,9 +797,9 @@ static void pci_region_map_entries(struct pci_bus *busses, struct pci_region *r)
> >      }
> >  }
> >  
> > -static void pci_bios_map_devices(struct pci_bus *busses)
> > +static void pci_bios_map_devices(struct pci_bus *busses, struct pci_mem *mem)
> >  {
> > -    if (pci_bios_init_root_regions(busses)) {
> > +    if (pci_bios_init_root_regions(busses, mem)) {
> >          struct pci_region r64_mem, r64_pref;
> >          r64_mem.list.first = NULL;
> >          r64_pref.list.first = NULL;
> > @@ -766,7 +808,7 @@ static void pci_bios_map_devices(struct pci_bus *busses)
> >          pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_PREFMEM],
> >                                           &r64_pref);
> >  
> > -        if (pci_bios_init_root_regions(busses))
> > +        if (pci_bios_init_root_regions(busses, mem))
> >              panic("PCI: out of 32bit address space\n");
> >  
> >          u64 sum_mem = pci_region_sum(&r64_mem);
> > @@ -774,16 +816,24 @@ static void pci_bios_map_devices(struct pci_bus *busses)
> >          u64 align_mem = pci_region_align(&r64_mem);
> >          u64 align_pref = pci_region_align(&r64_pref);
> >  
> > -        r64_mem.base = ALIGN(0x100000000LL + RamSizeOver4G, align_mem);
> > +        r64_mem.base = ALIGN(mem->start64, align_mem);
> > +        /*
> > +         * We don't know where the window ends, so pack all 64 bit memory
> > +         * at start of window.
> > +         */
> >          r64_pref.base = ALIGN(r64_mem.base + sum_mem, align_pref);
> > -        pcimem64_start = r64_mem.base;
> > -        pcimem64_end = r64_pref.base + sum_pref;
> > +
> > +        mem->start64 = r64_mem.base;
> > +        mem->end64 = r64_pref.base + sum_pref;
> > +
> > +        if (mem->end64 < mem->start64)
> > +            panic("PCI: out of 64bit address space\n");
> >  
> >          pci_region_map_entries(busses, &r64_mem);
> >          pci_region_map_entries(busses, &r64_pref);
> >      } else {
> >          // no bars mapped high -> drop 64bit window (see dsdt)
> > -        pcimem64_start = 0;
> > +        mem->start64 = mem->end64 = 0;
> >      }
> >      // Map regions on each device.
> >      int bus;
> > @@ -802,6 +852,8 @@ static void pci_bios_map_devices(struct pci_bus *busses)
> >  void
> >  pci_setup(void)
> >  {
> > +    struct pci_mem mem;
> > +
> >      if (!CONFIG_QEMU)
> >          return;
> >  
> > @@ -816,7 +868,7 @@ pci_setup(void)
> >      dprintf(1, "=== PCI device probing ===\n");
> >      pci_probe_devices();
> >  
> > -    pcimem_start = RamSize;
> > +    pci_bios_init_mem_addr(&mem);
> >      pci_bios_init_platform();
> >  
> >      dprintf(1, "=== PCI new allocation pass #1 ===\n");
> > @@ -830,7 +882,11 @@ pci_setup(void)
> >          return;
> >  
> >      dprintf(1, "=== PCI new allocation pass #2 ===\n");
> > -    pci_bios_map_devices(busses);
> > +    pci_bios_map_devices(busses, &mem);
> > +    pcimem_start = mem.start32;
> > +    pcimem_end = mem.end32;
> > +    pcimem64_start = mem.start64;
> > +    pcimem64_end = mem.end64;
> >  
> >      pci_bios_init_devices();
> >  



More information about the SeaBIOS mailing list