Patchwork kvm: align ram_size to page boundary

login
register
mail settings
Submitter Avi Kivity
Date June 17, 2012, 8:47 a.m.
Message ID <1339922831-23002-1-git-send-email-avi@redhat.com>
Download mbox | patch
Permalink /patch/165330/
State New
Headers show

Comments

Avi Kivity - June 17, 2012, 8:47 a.m.
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(+)
Jan Kiszka - June 17, 2012, 11:03 a.m.
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
Avi Kivity - June 17, 2012, 11:30 a.m.
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)
Jan Kiszka - June 17, 2012, 11:47 a.m.
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
Avi Kivity - June 17, 2012, 11:51 a.m.
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.
Blue Swirl - June 17, 2012, 12:43 p.m.
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
>
>
>
Peter Maydell - June 17, 2012, 12:48 p.m.
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
Avi Kivity - June 17, 2012, 12:54 p.m.
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.
Blue Swirl - June 17, 2012, 1:06 p.m.
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
>
>
Avi Kivity - June 17, 2012, 1:14 p.m.
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.

Patch

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: