Hi Shreyas, On Tue, May 03, 2016 at 01:54:31PM +0530, Shreyas B. Prabhu wrote: > In the current code, when the thread wakes up in reset vector, some > of the state restore code and check for whether a thread needs to > branch to kvm is duplicated. Reorder the code such that this > duplication is avoided. > > At a higher level this is what the change looks like-
I have manually verified that the code flow in the new patch is has the same effect as whatever we were doing earlier. There a couple of comments inline. > 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 ? [..snip..] > @@ -420,33 +451,9 @@ common_exit: > > hypervisor_state_restored: > > - li r5,PNV_THREAD_RUNNING > - stb r5,PACA_THREAD_IDLE_STATE(r13) > - > mtspr SPRN_SRR1,r16 > -#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > - li r0,KVM_HWTHREAD_IN_KERNEL > - stb r0,HSTATE_HWTHREAD_STATE(r13) > - /* Order setting hwthread_state vs. testing hwthread_req */ > - sync > - lbz r0,HSTATE_HWTHREAD_REQ(r13) > - cmpwi r0,0 > - beq 6f > - b kvm_start_guest > -6: > -#endif > - > - REST_NVGPRS(r1) > - REST_GPR(2, r1) > - ld r3,_CCR(r1) > - ld r4,_MSR(r1) > - ld r5,_NIP(r1) > - addi r1,r1,INT_FRAME_SIZE > - mtcr r3 > - mfspr r3,SPRN_SRR1 /* Return SRR1 */ > - mtspr SPRN_SRR1,r4 > - mtspr SPRN_SRR0,r5 > - rfid > + mtlr r17 > + blr Perhaps you could add a comment against this blr to indicate that we go back to the reset vector right after the call to power7_restore_hyp_resource. > > fastsleep_workaround_at_exit: > li r3,1 > -- > 2.4.11 >