| Message ID | 1339922831-23002-1-git-send-email-avi@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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)
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
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.
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 > > >
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
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.
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 > >
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.
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:
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(+)