On Sun, Oct 13, 2013 at 07:33:19PM +0200, Igor Mammedov wrote:
On Sun, 13 Oct 2013 19:46:09 +0300 "Michael S. Tsirkin" mst@redhat.com wrote:
On Sun, Oct 13, 2013 at 06:23:28PM +0200, Igor Mammedov wrote:
On Sun, 13 Oct 2013 18:59:20 +0300 "Michael S. Tsirkin" mst@redhat.com wrote:
On Sun, Oct 13, 2013 at 05:11:54PM +0200, Igor Mammedov wrote:
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:
- 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.
What are the compatibility worries, exactly?
The way I see it, if QEMU wants to be compatible it can't use memory hotplug. In that case RamSizeOver4G will have the old meaning which should work without issues.
QEMU can't know what BIOS version is used and error out in wrong case, and that could cause hard to guess what going wrong bugs in the guest.
- 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.
Hmm but you aren't passing in memory size - pcimem64-start merely tells bios where to put PCI devices. So how would it help?
If a lot of memory is reserved for memory hotplug (terabytes), that could put window behind what RamSizeOver4G could address it.
All we care about for a compatible bios is that it keep working in old hardware configurations. IMO there's no real need for old BIOS to work with e.g. memory hotplug - it's reasonable to ask users to update fitmware if they want to use this feature.
Fine with me if we don't care about strange hangs if something misused.
But RamSizeOver4G still doesn't allow correctly place PCI window if large amount of memory present/reserved.
I'm fine with adding a new fw cfg really. I am however not sure we should have a single new base64 flag. Hardware has a set of ranges where guest can put PCI devices - would we be better off with an interface that lists all these ranges?
I'm not saying we should but this needs a bit of thought.
> --- > 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
-- Regards, Igor
-- Regards, Igor