On Thu, 21 Jul 2011 18:49:44 +0200 Alexander Graf <ag...@suse.de> wrote:
> > On 21.07.2011, at 18:38, Scott Wood wrote: > > > On Thu, 21 Jul 2011 03:27:12 +0200 > > Alexander Graf <ag...@suse.de> 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
