[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: |
address@hidden |
Subject: |
Re: [Qemu-devel] [RFC PATCH 1/3] hw/arm/virt: add hotplug memory support |
Date: |
Thu, 4 Aug 2016 23:30:28 +0000 |
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:address@hidden
> 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 <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
OK. I'll try to split it.
> >
> > thanks
> > -- PMM
> >
Best Regards,
Kwangwoo Lee