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

Gautham R Shenoy
2016-05-19T14:26:28Z

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.