From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:57276) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QjxKE-0000R0-1C for qemu-devel@nongnu.org; Thu, 21 Jul 2011 13:46:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QjxK9-0007i8-T6 for qemu-devel@nongnu.org; Thu, 21 Jul 2011 13:46:18 -0400 Received: from ch1ehsobe006.messaging.microsoft.com ([216.32.181.186]:33497 helo=ch1outboundpool.messaging.microsoft.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QjxK9-0007hu-MQ for qemu-devel@nongnu.org; Thu, 21 Jul 2011 13:46:13 -0400 Date: Thu, 21 Jul 2011 12:46:05 -0500 From: Scott Wood Message-ID: <20110721124605.2be28f75@schlenkerla.am.freescale.net> In-Reply-To: <9F5BB83D-7B43-49BE-9993-FD82EF06934D@suse.de> References: <1311211654-14326-1-git-send-email-agraf@suse.de> <1311211654-14326-2-git-send-email-agraf@suse.de> <20110721113840.49798d8c@schlenkerla.am.freescale.net> <9F5BB83D-7B43-49BE-9993-FD82EF06934D@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 01/23] PPC: Add secondary CPU spinning code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: QEMU-devel Developers On Thu, 21 Jul 2011 18:49:44 +0200 Alexander Graf wrote: > > On 21.07.2011, at 18:38, Scott Wood wrote: > > > On Thu, 21 Jul 2011 03:27:12 +0200 > > Alexander Graf wrote: > > > >> When directly starting an SMP system with -kernel on PPC e500, we need to > >> simulate the spin table code from u-boot. This code adds a small c file > >> plus generated .elf file that enable secondary CPUs to spin just like they > >> would with u-boot. > > > > Why not just handle the spin table as an MMIO region? > > > > Besides being simpler, it avoids any spinning overhead if the guest doesn't > > spin up all the cpus. > > Hmm - sounds like a nice idea. We'd have to reserve the region in the dt, but overall I agree that it might end up being simpler. The region needs to be reserved in the dt regardless. > > You're jumping through a lot of hoops to (nominally) do this in C. > > Yeah, but at the end of the day C is a lot more readable than asm IMHO :). I'm not so sure in this case. > >> + info->pir = pir; > > > > While I'm fine with not allowing the guest to set PIR (the ISA says it's > > read-only in virtualized implementations), I'm not sure we should be > > overwriting the spintable value here. > > I merely do what the u-boot code is doing. Never mind, ePAPR does say to update the spin table with the actual PIR prior to entry. > >> + entry = (void*)(unsigned long)info->addr; > > > > You're ignoring addr_hi, and you should create an IMA appropriate for the > > guest's chosen entry point rather than assume it's in the first 64M. > > Hm - we have a linear map of 256MB. But yes, maybe creating a mapping instead of just assuming it'll be within there is a good idea :). In the KVM direct-mapped memory case there might not be any memory at all in the first 256M. We've got a patch in our queue to choose an appropriate IMA at runtime. > > And don't forget about r3_hi and r6_hi on future 64-bit targets. > > The code as is is 100% 32-bit. Right, but it'd be nice to minimize the things that silently break on 64-bit. -Scott