From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=56116 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PSWcZ-0002qY-JC for qemu-devel@nongnu.org; Tue, 14 Dec 2010 10:16:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PSWcX-0002hm-7c for qemu-devel@nongnu.org; Tue, 14 Dec 2010 10:16:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:2092) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PSWcW-0002hI-TG for qemu-devel@nongnu.org; Tue, 14 Dec 2010 10:16:53 -0500 From: Alex Williamson In-Reply-To: <4D073661.8010307@redhat.com> References: <20101213212059.2472.17879.stgit@s20.home> <20101213212436.2472.16686.stgit@s20.home> <4D073661.8010307@redhat.com> Content-Type: text/plain; charset="UTF-8" Date: Tue, 14 Dec 2010 08:16:45 -0700 Message-ID: <1292339805.2857.157.camel@x201> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH v4 2/2] RAM API: Make use of it for x86 PC List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: blauwirbel@gmail.com, qemu-devel@nongnu.org, kvm@vger.kernel.org On Tue, 2010-12-14 at 11:18 +0200, Avi Kivity wrote: > On 12/13/2010 11:24 PM, Alex Williamson wrote: > > Register the actual VM RAM using the new API > > > > > > @@ -913,14 +913,11 @@ void pc_memory_init(ram_addr_t ram_size, > > /* allocate RAM */ > > ram_addr = qemu_ram_alloc(NULL, "pc.ram", > > below_4g_mem_size + above_4g_mem_size); > > - cpu_register_physical_memory(0, 0xa0000, ram_addr); > > - cpu_register_physical_memory(0x100000, > > - below_4g_mem_size - 0x100000, > > - ram_addr + 0x100000); > > + ram_register(0, below_4g_mem_size, ram_addr); > > > > What's the impact of this? Won't it conflict with BIOS memory > registration? What about VGA? > > In terms of patch hygiene, it should be in a separate patch titled > "register 0xa0000-0x100000 as RAM" or something. It's a much more > drastic change than making use of the new RAM API. As we discussed in the v2 patch, the chipset can selectively switch regions within this range to point at VGA, ROM, or RAM, but there's always physical RAM backing the space, even when it's mapping isn't active. VGA and ROM will be overlay the RAM mapping. I'm fine with splitting this into two patches for debug-ability, but the change is reflective of following the RAM API and registering all of "RAM". Maybe it would be sufficient to make such a note explicit in this commit log? Thanks, Alex