From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47014) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdGqA-0001j9-PT for qemu-devel@nongnu.org; Mon, 04 Nov 2013 04:53:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VdGq4-0004HH-No for qemu-devel@nongnu.org; Mon, 04 Nov 2013 04:52:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48135) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdGq4-0004HD-FJ for qemu-devel@nongnu.org; Mon, 04 Nov 2013 04:52:52 -0500 Message-ID: <1383558605.2264.22.camel@localhost.localdomain> From: Marcel Apfelbaum Date: Mon, 04 Nov 2013 11:50:05 +0200 In-Reply-To: <20131104060608.GA3322@redhat.com> References: <20131104060608.GA3322@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] exec: limit system memory size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Peter Maydell , Jan Kiszka , qemu-devel@nongnu.org, Paolo Bonzini , Andreas =?ISO-8859-1?Q?F=E4rber?= , Richard Henderson On Mon, 2013-11-04 at 08:06 +0200, Michael S. Tsirkin wrote: > The page table logic in exec.c assumes > that memory addresses are at most TARGET_PHYS_ADDR_SPACE_BITS. > > But pci addresses are full 64 bit so if we try to render them ignoring > the extra bits, we get strange effects with sections overlapping each > other. > > To fix, simply limit the system memory size to > 1 << TARGET_PHYS_ADDR_SPACE_BITS, > pci addresses will be rendered within that. > > Signed-off-by: Michael S. Tsirkin > --- > exec.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/exec.c b/exec.c > index 030118e..c7a8df5 100644 > --- a/exec.c > +++ b/exec.c > @@ -1801,7 +1801,12 @@ void address_space_destroy_dispatch(AddressSpace *as) > static void memory_map_init(void) > { > system_memory = g_malloc(sizeof(*system_memory)); > - memory_region_init(system_memory, NULL, "system", INT64_MAX); > + > + assert(TARGET_PHYS_ADDR_SPACE_BITS <= 64); > + > + memory_region_init(system_memory, NULL, "system", > + TARGET_PHYS_ADDR_SPACE_BITS == 64 ? > + UINT64_MAX : (0x1ULL << TARGET_PHYS_ADDR_SPACE_BITS)); Michael, thanks again for the help. I am concerned that we cannot use all the UINT64_MAX address space. By the way, this patch makes the memory size aligned to page size, so the call to register_subpage (the asserted code) is diverted to register_multipage that does not have an assert. That leads me to another question? Maybe the fact that INT64_MAX is not aligned to page size makes all the trouble? What do you think? Regarding this patch: Maybe we should to add an assert inside memory_region_init in order to protect all the code that creates memory regions? And also maybe we should add a define MAX_MEMORY_REGION_SIZE to be used in all places we want a "maximum size" memory region? Thanks, Marcel > address_space_init(&address_space_memory, system_memory, "memory"); > > system_io = g_malloc(sizeof(*system_io));