Re: [kvm-devel] [PATCH 1/6] Use correct types to enable > 2G support

[Anthony L. <ali...@us...>]

Fabrice Bellard wrote:
> Anthony Liguori wrote:
>> +    /* above 4giga memory allocation */
>> +    if (above_4g_mem_size > 0) {
>> +        ram_addr = qemu_ram_alloc(above_4g_mem_size);
>> +        cpu_register_physical_memory(0x100000000, above_4g_mem_size, 
>> ram_addr);
>> +    }
>> +
>
> Why do you need this ? All the RAM can be registered with a single 
> call.  I fear you need to do that because of KVM RAM handling 
> limitations.

On the x86, there is a rather large hole at the top of memory.  
Currently, we do separate allocations around this whole.    You can't 
get away from doing multiple cpu_register_physical_memory calls here.  
We've discussed just allocating a single chunk with qemu_ram_alloc since 
so many places in QEMU assume that you can do phys_ram_base + PA.

I think I'll change this too into a single qemu_ram_alloc.  That will 
fix the bug with KVM when using -kernel and large memory anyway :-)

>> Index: qemu/osdep.c
>> ===================================================================
>> --- qemu.orig/osdep.c    2008-01-30 13:47:00.000000000 -0600
>> +++ qemu/osdep.c    2008-01-30 13:47:31.000000000 -0600
>> @@ -113,7 +113,7 @@
>>              int64_t free_space;
>>              int ram_mb;
>>  
>> -            extern int ram_size;
>> +            extern int64_t ram_size;
>>              free_space = (int64_t)stfs.f_bavail * stfs.f_bsize;
>>              if ((ram_size + 8192 * 1024) >= free_space) {
>>                  ram_mb = (ram_size / (1024 * 1024));
>> @@ -202,7 +202,7 @@
>>  #ifdef _BSD
>>      return valloc(size);
>>  #else
>> -    return memalign(4096, size);
>> +    return memalign(TARGET_PAGE_SIZE, size);
>>  #endif
>>  }
>
> No fully correct because it is intended to be the host page size.

Indeed.  I'm dropping this.  It was added for the ia64 port and since 
that's not included in this patch set, I'll let them fix it properly 
when they submit support for ia64.

>> +extern int64_t ram_size;
>
> I agree with the fact that ram_size should be 64 bit. Maybe each 
> machine could test the value and emit an error message if it is too 
> big. Maybe an uint64_t would be better though.

uint64_t is probably more reasonable.  I wouldn't begin to know what the 
appropriate amount of ram was for each machine though so I'll let the 
appropriate people handle that :-)

Regards,

Anthony Liguori

> Fabrice.