From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ingo Molnar Subject: Re: [PATCH 1/3 V4] kvm tools: Add memory gap for larger RAM sizes Date: Thu, 12 May 2011 09:34:07 +0200 Message-ID: <20110512073407.GC8163@elte.hu> References: <4dcaa893.5925e30a.0f9e.125c@mx.google.com> <4DCB2EA1.2080805@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: levinsasha928@gmail.com, penberg@kernel.org, prasadjoshi124@gmail.com, avi@redhat.com, gorcunov@gmail.com, kvm@vger.kernel.org To: Asias He Return-path: Received: from mx2.mail.elte.hu ([157.181.151.9]:53452 "EHLO mx2.mail.elte.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751426Ab1ELHeT (ORCPT ); Thu, 12 May 2011 03:34:19 -0400 Content-Disposition: inline In-Reply-To: <4DCB2EA1.2080805@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: * Asias He wrote: > On 05/11/2011 11:17 PM, levinsasha928@gmail.com wrote: > > From: Sasha Levin > >=20 > > e820 is expected to leave a memory gap within the low 32 > > bits of RAM space. From the documentation of e820_setup_gap(): > > /* > > * Search for the biggest gap in the low 32 bits of the e820 > > * memory space. We pass this space to PCI to assign MMIO resource= s > > * for hotplug or unconfigured devices in. > > * Hopefully the BIOS let enough space left. > > */ > >=20 > > Not leaving such gap causes errors and hangs during the boot > > process. > >=20 > > This patch adds a memory gap between 0xe0000000 and 0x100000000 > > when using more than 0xe0000000 bytes for guest RAM. > >=20 > > This patch updates the e820 table, slot allocations > > used for KVM_SET_USER_MEMORY_REGION. > >=20 > > Changes in V2: > > - Allocate RAM with the gap to avoid altering the translation code= =2E > > - New patch description. > >=20 > > Changes in V3: > > - Remove unnecessary casts. > >=20 > > Changes in V4: > > - Rewrite kvm__init_ram(). > > - Document the 64bit gap within the code. > >=20 > > Signed-off-by: Sasha Levin > > --- > > tools/kvm/bios.c | 27 ++++++++++++---- > > tools/kvm/include/kvm/e820.h | 2 +- > > tools/kvm/include/kvm/kvm.h | 2 + > > tools/kvm/kvm.c | 66 ++++++++++++++++++++++++++++++= +++++++---- > > 4 files changed, 82 insertions(+), 15 deletions(-) > >=20 > > diff --git a/tools/kvm/bios.c b/tools/kvm/bios.c > > index 2199c0c..3cd9b24 100644 > > --- a/tools/kvm/bios.c > > +++ b/tools/kvm/bios.c > > @@ -61,8 +61,6 @@ static void e820_setup(struct kvm *kvm) > > size =3D guest_flat_to_host(kvm, E820_MAP_SIZE); > > mem_map =3D guest_flat_to_host(kvm, E820_MAP_START); > > =20 > > - *size =3D E820_MEM_AREAS; > > - > > mem_map[i++] =3D (struct e820_entry) { > > .addr =3D REAL_MODE_IVT_BEGIN, > > .size =3D EBDA_START - REAL_MODE_IVT_BEGIN, > > @@ -78,13 +76,28 @@ static void e820_setup(struct kvm *kvm) > > .size =3D MB_BIOS_END - MB_BIOS_BEGIN, > > .type =3D E820_MEM_RESERVED, > > }; > > - mem_map[i++] =3D (struct e820_entry) { > > - .addr =3D BZ_KERNEL_START, > > - .size =3D kvm->ram_size - BZ_KERNEL_START, > > - .type =3D E820_MEM_USABLE, > > - }; > > + if (kvm->ram_size < KVM_32BIT_GAP_START) { > > + mem_map[i++] =3D (struct e820_entry) { > > + .addr =3D BZ_KERNEL_START, > > + .size =3D kvm->ram_size - BZ_KERNEL_START, > > + .type =3D E820_MEM_USABLE, > > + }; > > + } else { > > + mem_map[i++] =3D (struct e820_entry) { > > + .addr =3D BZ_KERNEL_START, > > + .size =3D KVM_32BIT_GAP_START - BZ_KERNEL_START, > > + .type =3D E820_MEM_USABLE, > > + }; > > + mem_map[i++] =3D (struct e820_entry) { > > + .addr =3D 0x100000000ULL, > > + .size =3D kvm->ram_size - KVM_32BIT_GAP_START, > > + .type =3D E820_MEM_USABLE, > > + }; > > + } > > =20 > > BUILD_BUG_ON(i > E820_MEM_AREAS); > > + > > + *size =3D i; > > } > > =20 > > /** > > diff --git a/tools/kvm/include/kvm/e820.h b/tools/kvm/include/kvm/e= 820.h > > index 252ae1f..e0f5f2a 100644 > > --- a/tools/kvm/include/kvm/e820.h > > +++ b/tools/kvm/include/kvm/e820.h > > @@ -8,7 +8,7 @@ > > #define E820_MEM_USABLE 1 > > #define E820_MEM_RESERVED 2 > > =20 > > -#define E820_MEM_AREAS 4 > > +#define E820_MEM_AREAS 5 > > =20 > > struct e820_entry { > > u64 addr; /* start of memory segment */ > > diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kv= m.h > > index 3dab78d..5e2e64c 100644 > > --- a/tools/kvm/include/kvm/kvm.h > > +++ b/tools/kvm/include/kvm/kvm.h > > @@ -8,6 +8,8 @@ > > #include > > =20 > > #define KVM_NR_CPUS (255) > > +#define KVM_32BIT_GAP_SIZE (512 << 20) > > +#define KVM_32BIT_GAP_START ((1ULL << 32) - KVM_32BIT_GAP_SIZE) > > =20 > > struct kvm { > > int sys_fd; /* For system ioctls(), i.e. /dev/kvm */ > > diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c > > index 65793f2..a3d3dd8 100644 > > --- a/tools/kvm/kvm.c > > +++ b/tools/kvm/kvm.c > > @@ -153,23 +153,64 @@ static bool kvm__cpu_supports_vm(void) > > return regs.ecx & (1 << feature); > > } > > =20 > > -void kvm__init_ram(struct kvm *self) > > +static void kvm_register_mem_slot(struct kvm *kvm, u32 slot, u64 g= uest_phys, u64 size, void *userspace_addr) > > { > > struct kvm_userspace_memory_region mem; > > int ret; > > =20 > > mem =3D (struct kvm_userspace_memory_region) { > > - .slot =3D 0, > > - .guest_phys_addr =3D 0x0UL, > > - .memory_size =3D self->ram_size, > > - .userspace_addr =3D (unsigned long) self->ram_start, > > + .slot =3D slot, > > + .guest_phys_addr =3D guest_phys, > > + .memory_size =3D size, > > + .userspace_addr =3D (u64)userspace_addr, >=20 >=20 > I am seeing: >=20 > CC kvm.o > kvm.c: In function =E2=80=98kvm_register_mem_slot=E2=80=99: > kvm.c:165:22: error: cast from pointer to integer of different size > [-Werror=3Dpointer-to-int-cast] > cc1: all warnings being treated as errors >=20 > make: *** [kvm.o] Error 1 >=20 > with this patch on 32-bit box. it's useful if you report the 'gcc -v' output for compiler warnings - s= o that=20 we can after some time have a mental picture of which GCC versions prod= uce=20 spurious warnings. Btw,. it would also be useful to add a 'make WERROR=3D0' option - that = way=20 warnings that trigger only on some GCC versions can be skipped during t= he build=20 while they will still be reported to us because the default 'make' fail= s. [ If you add that then please also send such a patch against tools/perf= / :-) ] Thanks, Ingo