Re: [Qemu-devel] [PATCH v2] pc: allow raising low memory via max-ram-below-4g option

[Laszlo Ersek]

On 01/08/16 18:45, Igor Mammedov wrote:
> On Fri,  8 Jan 2016 13:58:03 +0100
> Gerd Hoffmann <address@hidden> wrote:
> 
>> This patch extends the functionality of the max-ram-below-4g option
>> to also allow increasing lowmem.  Use case: Give as much memory as
>> possible to legacy non-PAE guests.
>>
>> While being at it also rework the lowmem calculation logic and add a
>> longish comment describing how it works and what the compatibility
>> constrains are.
> CCing Laszlo as it might affect OVMF

Thanks a lot for the CC, Igor!

So I have to investigate this separately for i440fx and Q35.

(1) For i440fx, OVMF determines the base of the 32-bit PCI hole like this:

      PciBase = (TopOfLowRam < BASE_2GB) ? BASE_2GB : TopOfLowRam;

where TopOfLowRam is calculated from the CMOS registers 0x34 and 0x35.

*If* QEMU is still sticking with the idea of git commit ddaaefb4dd, that
is, the 32-bit PCI hole still starts immediately after the end of low
RAM, then this change should be fine for i440fx.

(The problem used to be the (TopOfLowRam > BASE_2GB) case, when OVMF
allowed BAR allocation right above the end of low RAM, but QEMU didn't
actually start the PCI hole until higher up.)

Gerd, can you confirm that this new logic for the lowmem/highmem split
doesn't affect the above?

In other words, as long as there is no "void" left between the top of
low RAM and the base of the PCI hole, it doesn't matter where exactly
the split is.

(2) For Q35, the OVMF code is different:

//
// A 3GB base will always fall into Q35's 32-bit PCI host aperture,
// regardless of the Q35 MMCONFIG BAR. Correspondingly, QEMU never lets
// the RAM below 4 GB exceed it.
//
PciBase = BASE_2GB + BASE_1GB;
ASSERT (TopOfLowRam <= PciBase);


(This is based on pc_q35_init() in QEMU.)

This patch doesn't change "hw/i386/pc_q35.c", so that looks fine.

The patch does change "hw/i386/pc.c", which I believe might still affect
Q35...

... Hm, as far as I understand pc_q35_init(), the change in
"hw/i386/pc.c" will only cause the default user limit to move *down*
half a gig. The previous default user limit was 4G (i.e., not a limit at
all), and the new default is 3.5 GB.

And, in any case, the user limit continues to *lower* the split only,
from the initial 0x80000000 (2GB) or 0xb0000000 (3GB). So Q35 looks good
too.

Bottom line, I think the patch should be fine -- famous last words -- as
long as the idea of git commit ddaaefb4dd is still intact in QEMU:
- in Q35 the split cannot be raised
- in i440fx the split *can* be raised, but OVMF deals with that, as
  long as QEMU's 32-bit PCI hole still starts right after the split.

... I propose to replace the "pc:" prefix in the subject with "piix:" or
"i440fx:".

Thanks
Laszlo


> 
>>
>> Signed-off-by: Gerd Hoffmann <address@hidden>
>> ---
>>  hw/i386/pc.c      |  2 +-
>>  hw/i386/pc_piix.c | 61 
>> +++++++++++++++++++++++++++++++++++--------------------
>>  2 files changed, 40 insertions(+), 23 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 459260b..1332269 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1887,7 +1887,7 @@ static void pc_machine_initfn(Object *obj)
>>                          pc_machine_get_hotplug_memory_region_size,
>>                          NULL, NULL, NULL, &error_abort);
>>  
>> -    pcms->max_ram_below_4g = 1ULL << 32; /* 4G */
>> +    pcms->max_ram_below_4g = 0xe0000000; /* 3.5G */
>>      object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, "size",
>>                          pc_machine_get_max_ram_below_4g,
>>                          pc_machine_set_max_ram_below_4g,
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 438cdae..3743736 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -87,29 +87,46 @@ static void pc_init1(MachineState *machine,
>>      PcGuestInfo *guest_info;
>>      ram_addr_t lowmem;
>>  
>> -    /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory).
>> -     * If it doesn't, we need to split it in chunks below and above 4G.
>> -     * In any case, try to make sure that guest addresses aligned at
>> -     * 1G boundaries get mapped to host addresses aligned at 1G boundaries.
>> -     * For old machine types, use whatever split we used historically to 
>> avoid
>> -     * breaking migration.
>> +    /*
>> +     * Calculate ram split, for memory below and above 4G.  It's a bit
>> +     * complicated for backward compatibility reasons ...
>> +     *
>> +     *  - Traditional split is 3.5G (lowmem = 0xe0000000).  This is the
>> +     *    default value for max_ram_below_4g now.
>> +     *
>> +     *  - Then, to gigabyte align the memory, we move the split to 3G
>> +     *    (lowmem = 0xc0000000).  But only in case we have to split in
>> +     *    the first place, i.e. ram_size is larger than (traditional)
>> +     *    lowmem.  And for new machine types (gigabyte_align = true)
>> +     *    only, for live migration compatibility reasons.
>> +     *
>> +     *  - Next the max-ram-below-4g option was added, which allowed to
>> +     *    reduce lowmem to a smaller value, to allow a larger PCI I/O
>> +     *    window below 4G.  qemu doesn't enforce gigabyte alignment here,
>> +     *    but prints a warning.
>> +     *
>> +     *  - Finally max-ram-below-4g got updated to also allow raising lowmem,
>> +     *    so legacy non-PAE guests can get as much memory as possible in
>> +     *    the 32bit address space below 4G.
>> +     *
>> +     * Examples:
>> +     *    qemu -M pc-1.7 -m 4G    (old default)    -> 3584M low,  512M high
>> +     *    qemu -M pc -m 4G        (new default)    -> 3072M low, 1024M high
>> +     *    qemu -M pc,max-ram-below-4g=2G -m 4G     -> 2048M low, 2048M high
>> +     *    qemu -M pc,max-ram-below-4g=4G -m 3968M  -> 3968M low (=4G-128M)
>>       */
>> -    if (machine->ram_size >= 0xe0000000) {
>> -        lowmem = pcmc->gigabyte_align ? 0xc0000000 : 0xe0000000;
>> -    } else {
>> -        lowmem = 0xe0000000;
>> -    }
>> -
>> -    /* Handle the machine opt max-ram-below-4g.  It is basically doing
>> -     * min(qemu limit, user limit).
>> -     */
>> -    if (lowmem > pcms->max_ram_below_4g) {
>> -        lowmem = pcms->max_ram_below_4g;
>> -        if (machine->ram_size - lowmem > lowmem &&
>> -            lowmem & ((1ULL << 30) - 1)) {
>> -            error_report("Warning: Large machine and 
>> max_ram_below_4g(%"PRIu64
>> -                         ") not a multiple of 1G; possible bad 
>> performance.",
>> -                         pcms->max_ram_below_4g);
>> +    lowmem = pcms->max_ram_below_4g;
>> +    if (machine->ram_size >= pcms->max_ram_below_4g) {
>> +        if (pcmc->gigabyte_align) {
>> +            if (lowmem > 0xc0000000) {
>> +                lowmem = 0xc0000000;
>> +            }
>> +            if (lowmem & ((1ULL << 30) - 1)) {
>> +                error_report("Warning: Large machine and max_ram_below_4g "
>> +                             "(%" PRIu64 ") not a multiple of 1G; "
>> +                             "possible bad performance.",
>> +                             pcms->max_ram_below_4g);
>> +            }
>>          }
>>      }
>>  
> 
>