Re: [Qemu-devel] [PATCH v2] map 64-bit PCI BARs at location provided by emulator

[Igor Mammedov <imammedo@redhat.com>]

On Sun, 13 Oct 2013 15:31:23 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Sun, Oct 13, 2013 at 02:13:44PM +0200, Igor Mammedov wrote:
> > Currently 64-bit PCI BARs are unconditionally mapped by BIOS right
> > over 4G + RamSizeOver4G location, which doesn't allow to reserve
> > extra space before 64-bit PCI window. For memory hotplug an extra
> > RAM space might be reserved after present 64-bit RAM end and BIOS
> > should map 64-bit PCI BARs after it.
> > 
> > Introduce "etc/pcimem64-start" romfile to provide BIOS a hint
> > where it should start mapping of 64-bit PCI BARs. If romfile is
> > missing, BIOS reverts to legacy behavior and starts mapping right
> > after high memory.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v2:
> >   * place 64-bit window behind high RAM end if "etc/pcimem64-start"
> >     points below it.
> 
> Hmm I had an alternative suggestion of passing smbios
> tables in from QEMU (seabios already has a mechanism for this).
> We could then simply increase the existing RamSize variable.
> 
> What do you think about this idea?
There is several issues with RamSizeOver4G I see:
1. As Gerd pointed out it is also used to setup a part of e820 tables.
   It possibly could be also changed see "[Qemu-devel] [RfC PATCH] e820: pass
   high memory too" but it will be more complicated to keep seabios
   backward/forward compatible if we use RamSizeOver4G for passing.

2. It doesn't scale if we are going to use more than 1Tb future.
   Extending RamSizeOver4G value via additional CMOS port wasn't welcomed
   either, see
http://www.seabios.org/pipermail/seabios/2010-September/000911.html

It looks like fw_cfg interface is preferred one and "etc/pcimem64-start"
approach doesn't have backward/forward compatibility issues.

> 
> 
> > ---
> >  src/fw/pciinit.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> > index b29db99..798309b 100644
> > --- a/src/fw/pciinit.c
> > +++ b/src/fw/pciinit.c
> > @@ -18,6 +18,8 @@
> >  #include "paravirt.h" // RamSize
> >  #include "string.h" // memset
> >  #include "util.h" // pci_setup
> > +#include "byteorder.h" // le64_to_cpu
> > +#include "romfile.h" // romfile_loadint
> >  
> >  #define PCI_DEVICE_MEM_MIN     0x1000
> >  #define PCI_BRIDGE_IO_MIN      0x1000
> > @@ -764,6 +766,15 @@ static void pci_bios_map_devices(struct pci_bus *busses)
> >  {
> >      if (pci_bios_init_root_regions(busses)) {
> >          struct pci_region r64_mem, r64_pref;
> > +        u64 ram64_end = 0x100000000ULL + RamSizeOver4G;
> > +        u64 base64 = le64_to_cpu(romfile_loadint("etc/pcimem64-start",
> > +                                 ram64_end));
> > +        if (base64 < ram64_end) {
> > +            dprintf(1, "ignorig etc/pcimem64-start [0x%llx] below present RAM, "
> > +                       "placing 64-bit PCI window behind RAM end: %llx",
> > +                        base64, ram64_end);
> > +            base64 = ram64_end;
> > +        }
> >          r64_mem.list.first = NULL;
> >          r64_pref.list.first = NULL;
> >          pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_MEM],
> > @@ -779,7 +790,7 @@ 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(base64, align_mem);
> >          r64_pref.base = ALIGN(r64_mem.base + sum_mem, align_pref);
> >          pcimem64_start = r64_mem.base;
> >          pcimem64_end = r64_pref.base + sum_pref;
> > -- 
> > 1.8.3.1
> 


-- 
Regards,
  Igor