Hi Igor, Thank you for your guide to split the hotplug patch. Currently I'm looking at the Linux kernel codes related with huge page size.
> -----Original Message----- > From: Igor Mammedov [mailto:imamm...@redhat.com] > Sent: Tuesday, August 02, 2016 9:18 PM > To: Peter Maydell > Cc: Xiao Guangrong; Eduardo Habkost; Michael S. Tsirkin; QEMU Developers; > 정우석(CHUNG WOO SUK) MS SW; > qemu-arm; Shannon Zhao; Shannon Zhao; Paolo Bonzini; 김현철(KIM HYUNCHUL) MS SW; > 이광우(LEE KWANGWOO) MS > SW; Richard Henderson > Subject: Re: [Qemu-devel] [RFC PATCH 1/3] hw/arm/virt: add hotplug memory > support > > 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 OK. I'll try to split it. > > > > thanks > > -- PMM > > Best Regards, Kwangwoo Lee
