Hi Gautham, On 05/18/2016 11:55 AM, Gautham R Shenoy wrote: > 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. > Thanks for the review!
>> 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. > > > [..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. Ok. I'll do that. > >> >> fastsleep_workaround_at_exit: >> li r3,1 >> -- >> 2.4.11 >>
