BOOKE KVM calling load_up_fpu from C?

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, February 13, 2013 12:03 AM
> To: Bhushan Bharat-R65777
> Cc: Michael Neuling; Wood Scott-B07421; linuxppc-dev at lists.ozlabs.org
> Subject: Re: BOOKE KVM calling load_up_fpu from C?
> 
> On 02/12/2013 03:01:07 AM, Bhushan Bharat-R65777 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Michael Neuling [mailto:mikey at neuling.org]
> > > Sent: Tuesday, February 12, 2013 9:46 AM
> > > To: Bhushan Bharat-R65777
> > > Cc: Wood Scott-B07421; linuxppc-dev at lists.ozlabs.org
> > > Subject: Re: BOOKE KVM calling load_up_fpu from C?
> > >
> > > Bhushan Bharat-R65777 <R65777 at freescale.com> wrote:
> > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Michael Neuling [mailto:mikey at neuling.org]
> > > > > Sent: Tuesday, February 12, 2013 9:16 AM
> > > > > To: Bhushan Bharat-R65777
> > > > > Cc: Wood Scott-B07421; linuxppc-dev at lists.ozlabs.org
> > > > > Subject: Re: BOOKE KVM calling load_up_fpu from C?
> > > > >
> > > > > Look further down...
> > > > >
> > > > > #ifdef CONFIG_PPC32
> > > > > 	mfspr	r5,SPRN_SPRG_THREAD		/* current
> > task's THREAD (phys) */
> > > > > 	lwz	r4,THREAD_FPEXC_MODE(r5)
> > > > > 	ori	r9,r9,MSR_FP		/* enable FP for
> > current */
> > > > > 	or	r9,r9,r4
> > > > > #else
> > > > > 	ld	r4,PACACURRENT(r13)
> > > > > 	addi	r5,r4,THREAD		/* Get THREAD */
> > > > > 	lwz	r4,THREAD_FPEXC_MODE(r5)
> > > > > 	ori	r12,r12,MSR_FP
> > > > > 	or	r12,r12,r4
> > > > > 	std	r12,_MSR(r1)
> > > > > #endif
> > > > >
> > > > > R12 is loaded with SRR1 in the exception prolog before
> > load_up_fpu is
> > > called.
> > > >
> > > > Yes it is SRR1 not MSR.
> > >
> > > Yes, SRR1 == the MSR of the user process, not the current MSR.
> > >
> > > > Also on 32bit it looks like that R9 is assumed to have SRR1.
> > >
> > > Yep that too.
> > >
> > > So any idea how it's suppose to work or is it broken?
> >
> > To me this looks wrong. And this seems to works because the
> > thread->reg->msr is not actually used to write SRR1 (and eventually
> > the thread MSR) when doing rfi to enter guest. Infact
> > Guest(shadow_msr) MSR is used as SRR1 and which will have proper MSR
> > (including FP set).
> >
> > But Yes, Scott is right person to comment, So let us wait for him
> > comment.
> 
> I don't think it's actually a problem on 32-bit, since r9 is modified but never
> actually used for anything.

Is not the epilog loads srr1 in r9 and load_up_fpu() changes r9 and then r9 is written back in srr1 ?


>  On 64-bit, though, there's a store to the caller's
> stack frame (yuck) which the kvm/booke.h caller is not prepared for.

So if caller is using r12 then it can lead to come corruption, right ?

>  Indeed,
> book3s's kvmppc_load_up_fpu creates an interrupt-like stack frame, but does not
> load r9 or r12.
> 
> It would be really nice if assumptions like these were put in a code comment
> above load_up_fpu...  and if we didn't have so many random differences between
> 32-bit and 64-bit. :-P

:)

Thanks
-Bharat

> 
> -Scott