From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56198) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUYea-0000p0-3J for qemu-devel@nongnu.org; Tue, 02 Aug 2016 08:18:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUYeT-0004z3-FJ for qemu-devel@nongnu.org; Tue, 02 Aug 2016 08:18:34 -0400 Date: Tue, 2 Aug 2016 14:18:19 +0200 From: Igor Mammedov Message-ID: <20160802141819.406aa754@nial.brq.redhat.com> In-Reply-To: References: <1468975744-12587-1-git-send-email-kwangwoo.lee@sk.com> <1468975744-12587-2-git-send-email-kwangwoo.lee@sk.com> <79b91f02b26441be993e372cef44002d@nmail01.hynixad.com> <20160801094607.057cef2f@nial.brq.redhat.com> <20160801111433.2af5c013@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 1/3] hw/arm/virt: add hotplug memory support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Xiao Guangrong , Eduardo Habkost , "Michael S. Tsirkin" , QEMU Developers , "woosuk.chung@sk.com" , qemu-arm , Shannon Zhao , Shannon Zhao , Paolo Bonzini , "hyunchul3.kim@sk.com" , "kwangwoo.lee@sk.com" , Richard Henderson On Tue, 2 Aug 2016 08:59:46 +0100 Peter Maydell wrote: > On 1 August 2016 at 10:14, Igor Mammedov wrote: > > On Mon, 1 Aug 2016 09:13:34 +0100 > > Peter Maydell wrote: > >> On 1 August 2016 at 08:46, Igor Mammedov 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 >