RE: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks

[Caraman Mihai Claudiu-B02008 <B02008@freescale.com>]

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
> Sent: Thursday, July 05, 2012 1:26 AM
> To: Alexander Graf
> Cc: Caraman Mihai Claudiu-B02008; <kvm-ppc@vger.kernel.org>; KVM list;
> linuxppc-dev; qemu-ppc@nongnu.org List
> Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM
> kernel hooks
> 
> On Wed, 2012-07-04 at 16:29 +0200, Alexander Graf wrote:
> 
> > > +#ifdef CONFIG_KVM_BOOKE_HV
> > > +#define KVM_BOOKE_HV_MFSPR(reg, spr)				\
> > > +	BEGIN_FTR_SECTION					\
> > > +		mfspr	reg, spr;			  	\
> > > +	END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> > > +#else
> > > +#define KVM_BOOKE_HV_MFSPR(reg, spr)
> > > +#endif
> >
> > Bleks - this is ugly. Do we really need to open-code the #ifdef here?
> > Can't the feature section code determine that the feature is disabled
> > and just always not include the code?
> 
> You can't but in any case I don't see the point of the conditional here,
> we'll eventually have to load srr1 no ? We can move the load up to here
> in all cases or can't we ? 

I like the idea, but there is a problem with addition macros which may clobber
r11 and PROLOG_ADDITION_MASKABLE_GEN is such a case.

> If really not, we could have it inside DO_KVM and be done with it no ?

32-bit exception prolog loads srr1 unconditionally, as Alex and Scott mentioned
earlier, so we will be suboptimal for this case.

-Mike