Re: [Qemu-devel] i386: pc: align gpa<->hpa on 1GB boundary (v3)

[Paolo Bonzini <pbonzini@redhat.com>]

Il 06/11/2013 12:59, Igor Mammedov ha scritto:
> On Tue, 5 Nov 2013 23:55:43 -0200
> Marcelo Tosatti <mtosatti@redhat.com> wrote:
> 
>>
>>
>> v2: condition enablement of new mapping to new machine types (Paolo)
>> v3: fix changelog
>>
>> -----
>>
>>
>> Align guest physical address and host physical address
>> beyond guest 4GB on a 1GB boundary.
>>
>> Otherwise 1GB TLBs cannot be cached for the range.
>>
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 0c313fe..534e067 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1116,7 +1116,7 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
> [...]
>> +        /*
>> +         *
>> +         * If 1GB hugepages are used to back guest RAM, map guest address
>> +         * space in the range [ramsize,ramsize+holesize] to the ram block
>> +         * range [holestart, 4GB]
>> +         *
>> +         *                      0      h     4G     [ramsize,ramsize+holesize]
>> +         *
>> +         * guest-addr-space     [      ]     [      ][xxx]
>> +         *                                /----------/
>> +         * contiguous-ram-block [      ][xxx][     ]
>> +         *
>> +         * So that memory beyond 4GB is aligned on a 1GB boundary,
>> +         * at the host physical address space.
>> +         *
>> +         */
>> +        if (guest_info->gb_align) {
> 'gb_align' is one shot usage, it would be better to just add it as an argument
> to pc_memory_init(). That would allow to avoid extending PcGuestInfo needlessly,
> since gb_align isn't reused.

No, Marcelo's way is better.  pc_memory_init already has too many
arguments, moving them to PcGuestInfo (which ultimately might become
properties of the /machine QOM object) is the right thing to do.

Paolo

>> +            unsigned long holesize = 0x100000000ULL - below_4g_mem_size;
>> +
>> +            memory_region_init_alias(ram_above_4g, NULL, "ram-above-4g", ram,
>> +                                    0x100000000ULL,
>> +                                    above_4g_mem_size - holesize);
>> +            memory_region_add_subregion(system_memory, 0x100000000ULL,
>>                                      ram_above_4g);
> [...]
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 6083839..00afe4a 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -20,6 +20,7 @@ typedef struct PcPciInfo {
>>  struct PcGuestInfo {
>>      bool has_pci_info;
>>      bool isapc_ram_fw;
>> +    bool gb_align;
>>      FWCfgState *fw_cfg;
>>  };
> it doesn't apply anymore.
>