Re: [PATCH v2 2/9] powerpc/kvm: make hypervisor state restore a function

[Gautham R Shenoy]

Hi Shreyas,

On Wed, May 18, 2016 at 12:37:56PM +0530, Shreyas B Prabhu wrote:

[..snip..]
> >> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> >> index 7716ceb..7ebfbb0 100644
> >> --- a/arch/powerpc/kernel/exceptions-64s.S
> >> +++ b/arch/powerpc/kernel/exceptions-64s.S
> >> @@ -107,25 +107,8 @@ BEGIN_FTR_SECTION
> >>  	beq	9f
> >>
> >>  	cmpwi	cr3,r13,2
> >> +	bl	power7_restore_hyp_resource
> >>
> >> -	/*
> >> -	 * Check if last bit of HSPGR0 is set. This indicates whether we are
> >> -	 * waking up from winkle.
> >> -	 */
> >> -	GET_PACA(r13)
> >> -	clrldi	r5,r13,63
> >> -	clrrdi	r13,r13,1
> >> -	cmpwi	cr4,r5,1
> >> -	mtspr	SPRN_HSPRG0,r13
> >> -
> >> -	lbz	r0,PACA_THREAD_IDLE_STATE(r13)
> >> -	cmpwi   cr2,r0,PNV_THREAD_NAP
> >> -	bgt     cr2,8f				/* Either sleep or Winkle */
> >> -
> >> -	/* Waking up from nap should not cause hypervisor state loss */
> >> -	bgt	cr3,.
> >> -
> >> -	/* Waking up from nap */
> >>  	li	r0,PNV_THREAD_RUNNING
> >>  	stb	r0,PACA_THREAD_IDLE_STATE(r13)	/* Clear thread state */
> >>
> >> @@ -143,13 +126,9 @@ BEGIN_FTR_SECTION
> >>
> >>  	/* Return SRR1 from power7_nap() */
> >>  	mfspr	r3,SPRN_SRR1
> >> -	beq	cr3,2f
> >> -	b	power7_wakeup_noloss
> >> -2:	b	power7_wakeup_loss
> >> -
> >> -	/* Fast Sleep wakeup on PowerNV */
> >> -8:	GET_PACA(r13)
> > 
> > In the old code, we do a GET_PACA(r13) before invoking the
> > power7_wakeup_tb_loss. In the new code we don't. Can you explain
> > this omission ?
> 
> GET_PACA(13) is the called in the beginning of
> power7_restore_hyp_resource. So r13 contains pointer to PACA when
> power7_wakeup_tb_loss invoked later in the same function.

Ah, I see it now. So the GET_PACA(r13) at 8: was anyway redundant in
the older code.

You can add my Reviewed-by: to this patch.

--
Thanks and Regards
gautham.