[Qemu-devel] [PATCH] split memory allocation

[Qemu-devel] [PATCH] split memory allocation

[Glauber Costa]

Right now, kvm keeps the memory allocation split, so we can
handle different areas in different ways. This schema works with qemu
too, so it appears to be the common ground.

This patch proposes using this common ground for everyone, by spliting
raw qemu.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/pc.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 435c7d4..d6084ee 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -777,16 +777,24 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
     vmport_init();
 
     /* allocate RAM */
-    ram_addr = qemu_ram_alloc(ram_size);
-    cpu_register_physical_memory(0, below_4g_mem_size, ram_addr);
+    ram_addr = qemu_ram_alloc(0xa0000);
+    cpu_register_physical_memory(0, 0xa0000, ram_addr);
+
+    ram_addr = qemu_ram_alloc(0x100000 - 0xa0000);   /* hole */
+    ram_addr = qemu_ram_alloc(below_4g_mem_size - 0x100000);
+    cpu_register_physical_memory(0x100000,
+                 below_4g_mem_size - 0x100000,
+                 ram_addr);
 
     /* above 4giga memory allocation */
     if (above_4g_mem_size > 0) {
-        cpu_register_physical_memory((target_phys_addr_t) 0x100000000ULL,
+        ram_addr = qemu_ram_alloc(above_4g_mem_size);
+        cpu_register_physical_memory(0x100000000ULL,
                                      above_4g_mem_size,
-                                     ram_addr + below_4g_mem_size);
+                                     ram_addr);
     }
 
+
     /* allocate VGA RAM */
     vga_ram_addr = qemu_ram_alloc(vga_ram_size);
 
-- 
1.5.5.1

Re: [Qemu-devel] [PATCH] split memory allocation

[Aurelien Jarno]

Glauber Costa a écrit :
> Right now, kvm keeps the memory allocation split, so we can
> handle different areas in different ways. This schema works with qemu
> too, so it appears to be the common ground.
> 
> This patch proposes using this common ground for everyone, by spliting
> raw qemu.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  hw/pc.c |   16 ++++++++++++----
>  1 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 435c7d4..d6084ee 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -777,16 +777,24 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>      vmport_init();
>  
>      /* allocate RAM */
> -    ram_addr = qemu_ram_alloc(ram_size);
> -    cpu_register_physical_memory(0, below_4g_mem_size, ram_addr);
> +    ram_addr = qemu_ram_alloc(0xa0000);
> +    cpu_register_physical_memory(0, 0xa0000, ram_addr);
> +
> +    ram_addr = qemu_ram_alloc(0x100000 - 0xa0000);   /* hole */

What the point of allocating RAM for the memory hole if it is not mapped?

> +    ram_addr = qemu_ram_alloc(below_4g_mem_size - 0x100000);
> +    cpu_register_physical_memory(0x100000,
> +                 below_4g_mem_size - 0x100000,
> +                 ram_addr);
>  
>      /* above 4giga memory allocation */
>      if (above_4g_mem_size > 0) {
> -        cpu_register_physical_memory((target_phys_addr_t) 0x100000000ULL,
> +        ram_addr = qemu_ram_alloc(above_4g_mem_size);
> +        cpu_register_physical_memory(0x100000000ULL,
>                                       above_4g_mem_size,
> -                                     ram_addr + below_4g_mem_size);
> +                                     ram_addr);
>      }
>  
> +
>      /* allocate VGA RAM */
>      vga_ram_addr = qemu_ram_alloc(vga_ram_size);
>  


-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] split memory allocation

[Anthony Liguori]

Aurelien Jarno wrote:
> Glauber Costa a écrit :
>   
>> Right now, kvm keeps the memory allocation split, so we can
>> handle different areas in different ways. This schema works with qemu
>> too, so it appears to be the common ground.
>>
>> This patch proposes using this common ground for everyone, by spliting
>> raw qemu.
>>
>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>> ---
>>  hw/pc.c |   16 ++++++++++++----
>>  1 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 435c7d4..d6084ee 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -777,16 +777,24 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>>      vmport_init();
>>  
>>      /* allocate RAM */
>> -    ram_addr = qemu_ram_alloc(ram_size);
>> -    cpu_register_physical_memory(0, below_4g_mem_size, ram_addr);
>> +    ram_addr = qemu_ram_alloc(0xa0000);
>> +    cpu_register_physical_memory(0, 0xa0000, ram_addr);
>> +
>> +    ram_addr = qemu_ram_alloc(0x100000 - 0xa0000);   /* hole */
>>     
>
> What the point of allocating RAM for the memory hole if it is not mapped?
>   

If you don't, you violate the phys_ram_base + PA assumption for all of 
memory.  No code should rely on this but practically speaking, there is 
still some code in QEMU that does.

Since the RAM is never touched, it doesn't actually impact the RSS size 
so it's not all that important.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] split memory allocation

[Glauber Costa]

On Thu, Sep 11, 2008 at 09:35:44AM -0500, Anthony Liguori wrote:
> Aurelien Jarno wrote:
>> Glauber Costa a écrit :
>>   
>>> Right now, kvm keeps the memory allocation split, so we can
>>> handle different areas in different ways. This schema works with qemu
>>> too, so it appears to be the common ground.
>>>
>>> This patch proposes using this common ground for everyone, by spliting
>>> raw qemu.
>>>
>>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>>> ---
>>>  hw/pc.c |   16 ++++++++++++----
>>>  1 files changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/pc.c b/hw/pc.c
>>> index 435c7d4..d6084ee 100644
>>> --- a/hw/pc.c
>>> +++ b/hw/pc.c
>>> @@ -777,16 +777,24 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>>>      vmport_init();
>>>       /* allocate RAM */
>>> -    ram_addr = qemu_ram_alloc(ram_size);
>>> -    cpu_register_physical_memory(0, below_4g_mem_size, ram_addr);
>>> +    ram_addr = qemu_ram_alloc(0xa0000);
>>> +    cpu_register_physical_memory(0, 0xa0000, ram_addr);
>>> +
>>> +    ram_addr = qemu_ram_alloc(0x100000 - 0xa0000);   /* hole */
>>>     
>>
>> What the point of allocating RAM for the memory hole if it is not mapped?
>>   
>
> If you don't, you violate the phys_ram_base + PA assumption for all of  
> memory.  No code should rely on this but practically speaking, there is  
> still some code in QEMU that does.
>
> Since the RAM is never touched, it doesn't actually impact the RSS size  
> so it's not all that important.
>
> Regards,
>
> Anthony Liguori
>
Precisely.

We actually saw this a while ago. the kernel loader was broken due to that.

[Qemu-devel] Re: [PATCH] split memory allocation

[Anthony Liguori]

Glauber Costa wrote:
> Right now, kvm keeps the memory allocation split, so we can
> handle different areas in different ways. This schema works with qemu
> too, so it appears to be the common ground.
>
> This patch proposes using this common ground for everyone, by spliting
> raw qemu.
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  hw/pc.c |   16 ++++++++++++----
>  1 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 435c7d4..d6084ee 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -777,16 +777,24 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>      vmport_init();
>
>      /* allocate RAM */
> -    ram_addr = qemu_ram_alloc(ram_size);
> -    cpu_register_physical_memory(0, below_4g_mem_size, ram_addr);
> +    ram_addr = qemu_ram_alloc(0xa0000);
> +    cpu_register_physical_memory(0, 0xa0000, ram_addr);
> +
> +    ram_addr = qemu_ram_alloc(0x100000 - 0xa0000);   /* hole */
>   

It's not really a hole.  QEMU leaves a hole from 0xa0000 to 0xc0000 
because this is the VGA area.  KVM remaps this range to normal RAM when 
the guest sets the VGA card to have a linear framebuffer as an optimization.

Later, 0xc0000 to 0xd0000 gets registered as ROM (because it's the VGA 
BIOS), and 0xd0000 to about 0xe0000 is option ROM space, and 0xe0000 to 
0x100000 is where the BIOS gets mapped.

So the comment code be improved to explain what goes in this region and 
why we need to allocate ram for it in the first place.

Regards,

Anthony Liguori

> +    ram_addr = qemu_ram_alloc(below_4g_mem_size - 0x100000);
> +    cpu_register_physical_memory(0x100000,
> +                 below_4g_mem_size - 0x100000,
> +                 ram_addr);
>
>      /* above 4giga memory allocation */
>      if (above_4g_mem_size > 0) {
> -        cpu_register_physical_memory((target_phys_addr_t) 0x100000000ULL,
> +        ram_addr = qemu_ram_alloc(above_4g_mem_size);
> +        cpu_register_physical_memory(0x100000000ULL,
>                                       above_4g_mem_size,
> -                                     ram_addr + below_4g_mem_size);
> +                                     ram_addr);
>      }
>
> +
>      /* allocate VGA RAM */
>      vga_ram_addr = qemu_ram_alloc(vga_ram_size);
>
>   

[Qemu-devel] Re: [PATCH] split memory allocation

[Glauber Costa]

On Thu, Sep 11, 2008 at 09:43:19AM -0500, Anthony Liguori wrote:
> Glauber Costa wrote:
>> Right now, kvm keeps the memory allocation split, so we can
>> handle different areas in different ways. This schema works with qemu
>> too, so it appears to be the common ground.
>>
>> This patch proposes using this common ground for everyone, by spliting
>> raw qemu.
>>
>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>> ---
>>  hw/pc.c |   16 ++++++++++++----
>>  1 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 435c7d4..d6084ee 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -777,16 +777,24 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>>      vmport_init();
>>
>>      /* allocate RAM */
>> -    ram_addr = qemu_ram_alloc(ram_size);
>> -    cpu_register_physical_memory(0, below_4g_mem_size, ram_addr);
>> +    ram_addr = qemu_ram_alloc(0xa0000);
>> +    cpu_register_physical_memory(0, 0xa0000, ram_addr);
>> +
>> +    ram_addr = qemu_ram_alloc(0x100000 - 0xa0000);   /* hole */
>>   
>
> It's not really a hole.  QEMU leaves a hole from 0xa0000 to 0xc0000  
> because this is the VGA area.  KVM remaps this range to normal RAM when  
> the guest sets the VGA card to have a linear framebuffer as an 
> optimization.
>
> Later, 0xc0000 to 0xd0000 gets registered as ROM (because it's the VGA  
> BIOS), and 0xd0000 to about 0xe0000 is option ROM space, and 0xe0000 to  
> 0x100000 is where the BIOS gets mapped.
>
> So the comment code be improved to explain what goes in this region and  
> why we need to allocate ram for it in the first place.
>
150 % in agreement.
Wait for a new one.

> Regards,
>
> Anthony Liguori
>
>> +    ram_addr = qemu_ram_alloc(below_4g_mem_size - 0x100000);
>> +    cpu_register_physical_memory(0x100000,
>> +                 below_4g_mem_size - 0x100000,
>> +                 ram_addr);
>>
>>      /* above 4giga memory allocation */
>>      if (above_4g_mem_size > 0) {
>> -        cpu_register_physical_memory((target_phys_addr_t) 0x100000000ULL,
>> +        ram_addr = qemu_ram_alloc(above_4g_mem_size);
>> +        cpu_register_physical_memory(0x100000000ULL,
>>                                       above_4g_mem_size,
>> -                                     ram_addr + below_4g_mem_size);
>> +                                     ram_addr);
>>      }
>>
>> +
>>      /* allocate VGA RAM */
>>      vga_ram_addr = qemu_ram_alloc(vga_ram_size);
>>
>>   
>

Re: [Qemu-devel] [PATCH] split memory allocation

[Avi Kivity]

Glauber Costa wrote:
> Right now, kvm keeps the memory allocation split, so we can
> handle different areas in different ways. This schema works with qemu
> too, so it appears to be the common ground.
>
> This patch proposes using this common ground for everyone, by spliting
> raw qemu.
>   

I would rather see qemu switched to using memory slots.  The big chunk 
approach doesn't work well with memory hotplug (or with device hotplug 
when the device provides a memory resource).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Re: [PATCH] split memory allocation

[Avi Kivity]

Anthony Liguori wrote:
>
> It's not really a hole.  QEMU leaves a hole from 0xa0000 to 0xc0000 
> because this is the VGA area.  KVM remaps this range to normal RAM 
> when the guest sets the VGA card to have a linear framebuffer as an 
> optimization.
>

kvm doesn't remap it to normal RAM, it remaps it to a framebuffer.  So 
from the RAM viewpoint, it is a hole.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Re: [PATCH] split memory allocation

[Glauber Costa]

On Thu, Sep 11, 2008 at 06:29:38PM +0300, Avi Kivity wrote:
> Anthony Liguori wrote:
>>
>> It's not really a hole.  QEMU leaves a hole from 0xa0000 to 0xc0000  
>> because this is the VGA area.  KVM remaps this range to normal RAM  
>> when the guest sets the VGA card to have a linear framebuffer as an  
>> optimization.
>>
>
> kvm doesn't remap it to normal RAM, it remaps it to a framebuffer.  So  
> from the RAM viewpoint, it is a hole.

for VGA yes, but the said "hole" is much bigger than the vga space itself.
Not all of it is "hole".

>
> -- 
> error compiling committee.c: too many arguments to function
>

Re: [Qemu-devel] Re: [PATCH] split memory allocation

[Avi Kivity]

Glauber Costa wrote:
> for VGA yes, but the said "hole" is much bigger than the vga space itself.
> Not all of it is "hole".
>   

There is a hole in the hole?

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.