[Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Avi Kivity]

kvm is not able to execute out of partial pages; align the RAM size
so partial pages aren't present.

Reported-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Avi Kivity <avi@redhat.com>
---
 kvm-all.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/kvm-all.c b/kvm-all.c
index 4ea7d85..482768f 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1311,6 +1311,8 @@ int kvm_init(void)
 
     cpu_interrupt_handler = kvm_handle_interrupt;
 
+    ram_size = TARGET_PAGE_ALIGN(ram_size);
+
     return 0;
 
 err:
-- 
1.7.10.1

Re: [Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

On 2012-06-17 10:47, Avi Kivity wrote:
> kvm is not able to execute out of partial pages; align the RAM size
> so partial pages aren't present.
> 
> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  kvm-all.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 4ea7d85..482768f 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1311,6 +1311,8 @@ int kvm_init(void)
>  
>      cpu_interrupt_handler = kvm_handle_interrupt;
>  
> +    ram_size = TARGET_PAGE_ALIGN(ram_size);
> +
>      return 0;
>  
>  err:

I think this should rather go into generic code. What sense does it make
to have partial pages with TCG?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Avi Kivity]

On 06/17/2012 02:03 PM, Jan Kiszka wrote:
> On 2012-06-17 10:47, Avi Kivity wrote:
>> kvm is not able to execute out of partial pages; align the RAM size
>> so partial pages aren't present.
>> 
>> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
>> Signed-off-by: Avi Kivity <avi@redhat.com>
>> ---
>>  kvm-all.c |    2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 4ea7d85..482768f 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -1311,6 +1311,8 @@ int kvm_init(void)
>>  
>>      cpu_interrupt_handler = kvm_handle_interrupt;
>>  
>> +    ram_size = TARGET_PAGE_ALIGN(ram_size);
>> +
>>      return 0;
>>  
>>  err:
> 
> I think this should rather go into generic code. 

To be honest, I put this in kvm-specific code because vl.c doesn't have
TARGET_PAGE_ALIGN.  Maybe we should have machine->page_size or
machine->ram_alignment.

> What sense does it make
> to have partial pages with TCG?

Why impose an artificial restriction?

(answer: to reduce differences among various accelerators)

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

Re: [Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]

On 2012-06-17 13:30, Avi Kivity wrote:
> On 06/17/2012 02:03 PM, Jan Kiszka wrote:
>> On 2012-06-17 10:47, Avi Kivity wrote:
>>> kvm is not able to execute out of partial pages; align the RAM size
>>> so partial pages aren't present.
>>>
>>> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
>>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>> ---
>>>  kvm-all.c |    2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> index 4ea7d85..482768f 100644
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -1311,6 +1311,8 @@ int kvm_init(void)
>>>  
>>>      cpu_interrupt_handler = kvm_handle_interrupt;
>>>  
>>> +    ram_size = TARGET_PAGE_ALIGN(ram_size);
>>> +
>>>      return 0;
>>>  
>>>  err:
>>
>> I think this should rather go into generic code. 
> 
> To be honest, I put this in kvm-specific code because vl.c doesn't have
> TARGET_PAGE_ALIGN.  Maybe we should have machine->page_size or
> machine->ram_alignment.
> 
>> What sense does it make
>> to have partial pages with TCG?
> 
> Why impose an artificial restriction?

Beca...

> 
> (answer: to reduce differences among various accelerators)
> 

Oh, you found the answer. :)

At least, it should be enforce for the x86 target, independent of the
accelerator.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Avi Kivity]

On 06/17/2012 02:47 PM, Jan Kiszka wrote:
>>>
>>> I think this should rather go into generic code. 
>> 
>> To be honest, I put this in kvm-specific code because vl.c doesn't have
>> TARGET_PAGE_ALIGN.  Maybe we should have machine->page_size or
>> machine->ram_alignment.
>> 
>>> What sense does it make
>>> to have partial pages with TCG?
>> 
>> Why impose an artificial restriction?
> 
> Beca...
> 
>> 
>> (answer: to reduce differences among various accelerators)
>> 
> 
> Oh, you found the answer. :)

Reducing round-trips across the Internet.

> 
> At least, it should be enforce for the x86 target, independent of the
> accelerator.

Yeah.  So there's machine->page_size or machine->ram_alignment.  Not
sure which is best.

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

Re: [Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Blue Swirl]

On Sun, Jun 17, 2012 at 11:51 AM, Avi Kivity <avi@redhat.com> wrote:
> On 06/17/2012 02:47 PM, Jan Kiszka wrote:
>>>>
>>>> I think this should rather go into generic code.
>>>
>>> To be honest, I put this in kvm-specific code because vl.c doesn't have
>>> TARGET_PAGE_ALIGN.  Maybe we should have machine->page_size or
>>> machine->ram_alignment.
>>>
>>>> What sense does it make
>>>> to have partial pages with TCG?
>>>
>>> Why impose an artificial restriction?
>>
>> Beca...
>>
>>>
>>> (answer: to reduce differences among various accelerators)
>>>
>>
>> Oh, you found the answer. :)
>
> Reducing round-trips across the Internet.
>
>>
>> At least, it should be enforce for the x86 target, independent of the
>> accelerator.
>
> Yeah.  So there's machine->page_size or machine->ram_alignment.  Not
> sure which is best.

The boards should make sure that the amount of RAM is feasible with
the board memory slots. It's not possible to put 256kb SIMMs to a slot
that expects 1GB DIMMs. We can allow some flexibility there though,
I'm not sure if the current chipsets would support very much memory if
we followed the docs to the letter.

Maybe strtosz() should just enforce 1MB granularity.

What about ballooning (memory hotplug?), can that reduce the memory by
smaller amount than page size?

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

Re: [Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Peter Maydell]

On 17 June 2012 13:43, Blue Swirl <blauwirbel@gmail.com> wrote:
> The boards should make sure that the amount of RAM is feasible with
> the board memory slots. It's not possible to put 256kb SIMMs to a slot
> that expects 1GB DIMMs. We can allow some flexibility there though,
> I'm not sure if the current chipsets would support very much memory if
> we followed the docs to the letter.

Last time I tried to propose a means for boards to specify
their memory restrictions it got shot down for being insufficiently
general :-)

> Maybe strtosz() should just enforce 1MB granularity.

That seems even more random a restriction than whole-page requirements.

-- PMM

Re: [Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Avi Kivity]

On 06/17/2012 03:43 PM, Blue Swirl wrote:
> On Sun, Jun 17, 2012 at 11:51 AM, Avi Kivity <avi@redhat.com> wrote:
>> On 06/17/2012 02:47 PM, Jan Kiszka wrote:
>>>>>
>>>>> I think this should rather go into generic code.
>>>>
>>>> To be honest, I put this in kvm-specific code because vl.c doesn't have
>>>> TARGET_PAGE_ALIGN.  Maybe we should have machine->page_size or
>>>> machine->ram_alignment.
>>>>
>>>>> What sense does it make
>>>>> to have partial pages with TCG?
>>>>
>>>> Why impose an artificial restriction?
>>>
>>> Beca...
>>>
>>>>
>>>> (answer: to reduce differences among various accelerators)
>>>>
>>>
>>> Oh, you found the answer. :)
>>
>> Reducing round-trips across the Internet.
>>
>>>
>>> At least, it should be enforce for the x86 target, independent of the
>>> accelerator.
>>
>> Yeah.  So there's machine->page_size or machine->ram_alignment.  Not
>> sure which is best.
> 
> The boards should make sure that the amount of RAM is feasible with
> the board memory slots. It's not possible to put 256kb SIMMs to a slot
> that expects 1GB DIMMs. We can allow some flexibility there though,
> I'm not sure if the current chipsets would support very much memory if
> we followed the docs to the letter.

Right. And generally memory modules are sized a power of two, creating
the silly "mega == 1048576" movement.

> 
> Maybe strtosz() should just enforce 1MB granularity.

strtosz() is much too general.  We could do it in vl.c without trouble.
 However, it takes away our ability to emulate a "640k should be enough
for everyone" machine.

> 
> What about ballooning (memory hotplug?), can that reduce the memory by
> smaller amount than page size?

Ballooning removes individual pages, that has no effect on the slot size.

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

Re: [Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Blue Swirl]

On Sun, Jun 17, 2012 at 12:54 PM, Avi Kivity <avi@redhat.com> wrote:
> On 06/17/2012 03:43 PM, Blue Swirl wrote:
>> On Sun, Jun 17, 2012 at 11:51 AM, Avi Kivity <avi@redhat.com> wrote:
>>> On 06/17/2012 02:47 PM, Jan Kiszka wrote:
>>>>>>
>>>>>> I think this should rather go into generic code.
>>>>>
>>>>> To be honest, I put this in kvm-specific code because vl.c doesn't have
>>>>> TARGET_PAGE_ALIGN.  Maybe we should have machine->page_size or
>>>>> machine->ram_alignment.
>>>>>
>>>>>> What sense does it make
>>>>>> to have partial pages with TCG?
>>>>>
>>>>> Why impose an artificial restriction?
>>>>
>>>> Beca...
>>>>
>>>>>
>>>>> (answer: to reduce differences among various accelerators)
>>>>>
>>>>
>>>> Oh, you found the answer. :)
>>>
>>> Reducing round-trips across the Internet.
>>>
>>>>
>>>> At least, it should be enforce for the x86 target, independent of the
>>>> accelerator.
>>>
>>> Yeah.  So there's machine->page_size or machine->ram_alignment.  Not
>>> sure which is best.
>>
>> The boards should make sure that the amount of RAM is feasible with
>> the board memory slots. It's not possible to put 256kb SIMMs to a slot
>> that expects 1GB DIMMs. We can allow some flexibility there though,
>> I'm not sure if the current chipsets would support very much memory if
>> we followed the docs to the letter.
>
> Right. And generally memory modules are sized a power of two, creating
> the silly "mega == 1048576" movement.
>
>>
>> Maybe strtosz() should just enforce 1MB granularity.
>
> strtosz() is much too general.  We could do it in vl.c without trouble.
>  However, it takes away our ability to emulate a "640k should be enough
> for everyone" machine.

Then how about current max of target page sizes: 8k? No machine should
want less than that.

>
>>
>> What about ballooning (memory hotplug?), can that reduce the memory by
>> smaller amount than page size?
>
> Ballooning removes individual pages, that has no effect on the slot size.
>
> --
> error compiling committee.c: too many arguments to function
>
>

Re: [Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Avi Kivity]

On 06/17/2012 04:06 PM, Blue Swirl wrote:

>> strtosz() is much too general.  We could do it in vl.c without trouble.
>>  However, it takes away our ability to emulate a "640k should be enough
>> for everyone" machine.
> 
> Then how about current max of target page sizes: 8k? No machine should
> want less than that.

Okay by me, but I can hear the we-should-have-a-generic-mechanism crowd
charging their megaphone batteries.

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

Re: [Qemu-devel] [PATCH] kvm: align ram_size to page boundary

[Michael Tokarev]

On 17.06.2012 17:14, Avi Kivity wrote:
> On 06/17/2012 04:06 PM, Blue Swirl wrote:
> 
>>> strtosz() is much too general.  We could do it in vl.c without trouble.
>>>  However, it takes away our ability to emulate a "640k should be enough
>>> for everyone" machine.
>>
>> Then how about current max of target page sizes: 8k? No machine should
>> want less than that.
> 
> Okay by me, but I can hear the we-should-have-a-generic-mechanism crowd
> charging their megaphone batteries.

So, is there some bottom line in that?  I think I'll put a (temp)
fix/workaround for the debian package to require memory size to be
a multiple of 8K, and to produce a warning if that requiriment hasn't
met.

Something like this:

diff --git a/vl.c b/vl.c
index 5d9fc55..db98a4a 100644
--- a/vl.c
+++ b/vl.c
@@ -2671,7 +2675,15 @@ int main(int argc, char **argv, char **envp)
                     fprintf(stderr, "qemu: ram size too large\n");
                     exit(1);
                 }
-                ram_size = value;
+#define RAM_SIZE_GRANULARITY (8*1024)
+                ram_size = value / RAM_SIZE_GRANULARITY;
+                ram_size *= RAM_SIZE_GRANULARITY;
+                if (ram_size != value) {
+                    fprintf(stderr,
+                            "warning: requested memory size (%" PRIu64 " bytes) "
+                            "truncated to %" PRIu64 " bytes\n",
+                            value, (uint64_t)ram_size);
+                }
                 break;
             }
             case QEMU_OPTION_mempath:


With this patch, running
  qemu-system-x86_64 -m 1.4g
produces the following:

warning: requested memory size (1503238553 bytes) truncated to 1503232000 bytes


Thanks,

/mjt