headers
Avi Kivity | 17 Jun 2012 10:47
Picon
Favicon

[PATCH] kvm: align ram_size to page boundary

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 <at> tls.msk.ru>
Signed-off-by: Avi Kivity <avi <at> 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
 <at>  <at>  -1311,6 +1311,8  <at>  <at>  int kvm_init(void)

     cpu_interrupt_handler = kvm_handle_interrupt;

+    ram_size = TARGET_PAGE_ALIGN(ram_size);
+
     return 0;

 err:
--

-- 
1.7.10.1
Permalink | Reply |
headers
Jan Kiszka | 17 Jun 2012 13:03
Picon

Re: [PATCH] kvm: align ram_size to page boundary

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 <at> tls.msk.ru>
> Signed-off-by: Avi Kivity <avi <at> 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
>  <at>  <at>  -1311,6 +1311,8  <at>  <at>  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
(Continue reading)

Permalink | Reply |
headers
Avi Kivity | 17 Jun 2012 13:30
Picon
Favicon

Re: [PATCH] kvm: align ram_size to page boundary

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 <at> tls.msk.ru>
>> Signed-off-by: Avi Kivity <avi <at> 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
>>  <at>  <at>  -1311,6 +1311,8  <at>  <at>  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.
(Continue reading)

Permalink | Reply |
headers
Jan Kiszka | 17 Jun 2012 13:47
Picon

Re: [PATCH] kvm: align ram_size to page boundary

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 <at> tls.msk.ru>
>>> Signed-off-by: Avi Kivity <avi <at> 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
>>>  <at>  <at>  -1311,6 +1311,8  <at>  <at>  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
(Continue reading)

Permalink | Reply |
headers
Avi Kivity | 17 Jun 2012 13:51
Picon
Favicon

Re: [PATCH] kvm: align ram_size to page boundary

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.
(Continue reading)

Permalink | Reply |
headers
Blue Swirl | 17 Jun 2012 14:43
Picon

Re: [PATCH] kvm: align ram_size to page boundary

On Sun, Jun 17, 2012 at 11:51 AM, Avi Kivity <avi <at> 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.
(Continue reading)

Permalink | Reply |
headers
Peter Maydell | 17 Jun 2012 14:48
Favicon
Gravatar

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

On 17 June 2012 13:43, Blue Swirl <blauwirbel <at> 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply |
headers
Avi Kivity | 17 Jun 2012 14:54
Picon
Favicon

Re: [PATCH] kvm: align ram_size to page boundary

On 06/17/2012 03:43 PM, Blue Swirl wrote:
> On Sun, Jun 17, 2012 at 11:51 AM, Avi Kivity <avi <at> 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
(Continue reading)

Permalink | Reply |
headers
Blue Swirl | 17 Jun 2012 15:06
Picon

Re: [PATCH] kvm: align ram_size to page boundary

On Sun, Jun 17, 2012 at 12:54 PM, Avi Kivity <avi <at> 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 <at> 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.
>>>
(Continue reading)

Permalink | Reply |
headers
Avi Kivity | 17 Jun 2012 15:14
Picon
Favicon

Re: [PATCH] kvm: align ram_size to page boundary

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
Permalink | Reply |
headers
Michael Tokarev | 6 Jul 2012 20:50
Picon

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

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
 <at>  <at>  -2671,7 +2675,15  <at>  <at>  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)
(Continue reading)

Permalink | Reply |

Gmane