qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC PATCH 1/3] hw/arm/virt: add hotplug memory support


From: Igor Mammedov
Subject: Re: [Qemu-devel] [RFC PATCH 1/3] hw/arm/virt: add hotplug memory support
Date: Tue, 2 Aug 2016 14:18:19 +0200

On Tue, 2 Aug 2016 08:59:46 +0100
Peter Maydell <address@hidden> wrote:

> On 1 August 2016 at 10:14, Igor Mammedov <address@hidden> wrote:
> > On Mon, 1 Aug 2016 09:13:34 +0100
> > Peter Maydell <address@hidden> wrote:  
> >> On 1 August 2016 at 08:46, Igor Mammedov <address@hidden> 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
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]