Re: [PATCH v6 09/19] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa()

[Mark Cave-Ayland <mark.caveayland@nutanix.com>]

On 26/08/2025 11:01, Xiaoyao Li wrote:

> On 8/22/2025 8:11 PM, Mark Cave-Ayland wrote:
>> All isapc machines must have 32-bit CPUs and so the RAM split logic 
>> can be hardcoded
>> accordingly.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>> ---
>>   hw/i386/pc_piix.c | 58 ++++-------------------------------------------
>>   1 file changed, 4 insertions(+), 54 deletions(-)
>>
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 816124c027..fc94937ad4 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -444,69 +444,19 @@ static void pc_init_isa(MachineState *machine)
>>       GSIState *gsi_state;
>>       MemoryRegion *ram_memory;
>>       MemoryRegion *rom_memory = system_memory;
>> -    ram_addr_t lowmem;
>>       uint64_t hole64_size = 0;
>>       /*
>> -     * 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.
>> -     *
>> -     *  - Note that Xen has its own ram setup code in xen_ram_init(),
>> -     *    called via xen_hvm_init_pc().
>> -     *
>> -     * 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)
>> +     * There is no RAM split for the isapc machine
>>        */
>>       if (xen_enabled()) {
>>           xen_hvm_init_pc(pcms, &ram_memory);
>>       } else {
>>           ram_memory = machine->ram;
>> -        if (!pcms->max_ram_below_4g) {
>> -            pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
>> -        }
>> -        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 & (1 * GiB - 1)) {
>> -                    warn_report("Large machine and max_ram_below_4g "
>> -                                "(%" PRIu64 ") not a multiple of 1G; "
>> -                                "possible bad performance.",
>> -                                pcms->max_ram_below_4g);
>> -                }
>> -            }
>> -        }
>> -        if (machine->ram_size >= lowmem) {
>> -            x86ms->above_4g_mem_size = machine->ram_size - lowmem;
>> -            x86ms->below_4g_mem_size = lowmem;
>> -        } else {
>> -            x86ms->above_4g_mem_size = 0;
>> -            x86ms->below_4g_mem_size = machine->ram_size;
>> -        }
>> +        pcms->max_ram_below_4g = 4 * GiB;
>> +        x86ms->above_4g_mem_size = 0;
>> +        x86ms->below_4g_mem_size = machine->ram_size;
> 
> I think we need to sanity check the machine->ram_size is not bigger than 
> 4G, and error out if it exceeds.

Amazingly there is currently no limit for the isapc machine, but I shall 
add it in for v7.

>>       }
>>       /*


ATB,

Mark.