On Tue, 2 Aug 2016 08:59:46 +0100
Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 1 August 2016 at 10:14, Igor Mammedov <imamm...@redhat.com> wrote:
> > On Mon, 1 Aug 2016 09:13:34 +0100
> > Peter Maydell <peter.mayd...@linaro.org> wrote:
> >> On 1 August 2016 at 08:46, Igor Mammedov <imamm...@redhat.com> wrote:
> >> > Base alignment comes from max supported hugepage size,
> >>
> >> Max hugepage size for any host? (if so, should be defined
> >> in a common header somewhere)
> >> Max hugepage size for ARM hosts? (if so, why is TCG
> >> different from KVM?, and should still be in a common
> >> header somewhere)
> > It's the same for TCG but it probably doesn't matter much there,
> > main usage is to provide better performance with KVM.
> >
> > So I'd say it's host depended (for x86 it's 1Gb),
> > probably other values for ARM and PPC
>
> We probably don't want to make the memory layout depend
> on the host architecture, though :-(
Important here is not change it dynamically so it won't
break migration.
Otherwise it could be a value that makes a sense for
the use-case where performance matters most, i.e. KVM
which makes us to derive value from max supported page
size for a KVM host (meaning guest is the same arch)
In case of x86 value is constant hardcoded in target
specific code which applies to both KVM and TCG.
Perhaps there is a better way to handle it which I just
don't see.
>
> >>
> >> > while
> >> > size alignment should depend on backend's page size
> >>
> >> Which page size do you have in mind here? TARGET_PAGE_SIZE
> >> is often not the right answer, since it doesn't
> >> correspond either to the actual page size being used
> >> by the host kernel or to the actual page size used
> >> by the guest kernel...
> > alignment comes from here: memory_region_get_alignment()
> >
> > exec:c
> > MAX(page_size, QEMU_VMALLOC_ALIGN)
> > so it's either backend's page size or a min chunk QEMU
> > allocates memory to make KVM/valgrind/whatnot happy.
>
> Since that's always larger than TARGET_PAGE_SIZE
> why are we checking for an alignment here that's
> not actually sufficient to make things work?
You mean following hunk:
+ if (QEMU_ALIGN_UP(machine->maxram_size,
+ TARGET_PAGE_SIZE) != machine->maxram_size) {
It's a bit late to fix for x86 without breaking CLI,
side effect would be inability to hotplug upto maxmem if maxmem
is misaligned wrt used backend page size
ARCH_VIRT_HOTPLUG_MEM_ALIGN should be used instead of TARGET_PAGE_SIZE
PS:
I haven't reviewed series yet, but I'd split this patch in 3
to make review easier
1st - introduce machine level hotplug hooks
2nd - add MemoryHotplugState to VirtMachineState and initialize it
3rd - add virt_dimm_plug() handler
>
> thanks
> -- PMM
>
