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

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

2016-05-19 Thread Paul Mackerras

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.

This is a nice cleanup.  The one minor comment I have is that since
power7_restore_hyp_resource has some unusual entry requirements (such
as requiring cr3 to be set a certain way), those requirements should
be documented in the comment just about the function entry point.  I
didn't see any unusual exit conditions, but if there are any they
should be documented too.

Reviewed-by: Paul Mackerras

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

2016-05-19 Thread Paul Mackerras

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.

This is a nice cleanup.  The one minor comment I have is that since
power7_restore_hyp_resource has some unusual entry requirements (such
as requiring cr3 to be set a certain way), those requirements should
be documented in the comment just about the function entry point.  I
didn't see any unusual exit conditions, but if there are any they
should be documented too.

Reviewed-by: Paul Mackerras

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

2016-05-19 Thread 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.

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

2016-05-19 Thread 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.

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

2016-05-18 Thread Shreyas B Prabhu

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)
>> -addir1,r1,INT_FRAME_SIZE
>> -mtcrr3
>> -mfspr   r3,SPRN_SRR1/* Return SRR1 */
>> -mtspr   SPRN_SRR1,r4
>> -mtspr   SPRN_SRR0,r5
>> -rfid
>> +mtlrr17
>> +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
>>

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

2016-05-18 Thread Shreyas B Prabhu

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)
>> -addir1,r1,INT_FRAME_SIZE
>> -mtcrr3
>> -mfspr   r3,SPRN_SRR1/* Return SRR1 */
>> -mtspr   SPRN_SRR1,r4
>> -mtspr   SPRN_SRR0,r5
>> -rfid
>> +mtlrr17
>> +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
>>

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

2016-05-18 Thread Gautham R Shenoy

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)
> - addir1,r1,INT_FRAME_SIZE
> - mtcrr3
> - mfspr   r3,SPRN_SRR1/* Return SRR1 */
> - mtspr   SPRN_SRR1,r4
> - mtspr   SPRN_SRR0,r5
> - rfid
> + mtlrr17
> + 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
>

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

2016-05-18 Thread Gautham R Shenoy

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)
> - addir1,r1,INT_FRAME_SIZE
> - mtcrr3
> - mfspr   r3,SPRN_SRR1/* Return SRR1 */
> - mtspr   SPRN_SRR1,r4
> - mtspr   SPRN_SRR0,r5
> - rfid
> + mtlrr17
> + 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
>

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

2016-05-03 Thread Shreyas B. Prabhu

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-

Before this patch -
power7_wakeup_tb_loss:
restore hypervisor state
if (thread needed by kvm)
goto kvm_start_guest
restore nvgprs, cr, pc
rfid to process context

power7_wakeup_loss:
restore nvgprs, cr, pc
rfid to process context

reset vector:
if (waking from deep idle states)
goto power7_wakeup_tb_loss
else
if (thread needed by kvm)
goto kvm_start_guest
goto power7_wakeup_loss

After this patch -
power7_wakeup_tb_loss:
restore hypervisor state
return

power7_restore_hyp_resource():
if (waking from deep idle states)
goto power7_wakeup_tb_loss
return

power7_wakeup_loss:
restore nvgprs, cr, pc
rfid to process context

reset vector:
power7_restore_hyp_resource()
if (thread needed by kvm)
goto kvm_start_guest
goto power7_wakeup_loss

Signed-off-by: Shreyas B. Prabhu 
---
 arch/powerpc/kernel/exceptions-64s.S | 29 +++-
 arch/powerpc/kernel/idle_power7.S| 67 
 2 files changed, 41 insertions(+), 55 deletions(-)

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)
-   b   power7_wakeup_tb_loss
+   blt cr3,2f
+   b   power7_wakeup_loss
+2: b   power7_wakeup_noloss
 
 9:
 END_FTR_SECTION_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
diff --git a/arch/powerpc/kernel/idle_power7.S 
b/arch/powerpc/kernel/idle_power7.S
index 6b3404b..82d164b 100644
--- a/arch/powerpc/kernel/idle_power7.S
+++ b/arch/powerpc/kernel/idle_power7.S
@@ -258,6 +258,35 @@ _GLOBAL(power7_winkle)
b   power7_powersave_common
/* No return */
 
+/*
+ * Called from reset vector. Check whether we have woken up with
+ * hypervisor state loss. If yes, restore hypervisor state and return
+ * back to reset vector.
+ */
+_GLOBAL(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,power7_wakeup_tb_loss   /* Either sleep or Winkle */
+
+   /*
+* We fall through here if PACA_THREAD_IDLE_STATE shows we are waking
+* up from nap. At this stage CR3 shouldn't contains 'gt' since that
+* indicates we are waking with hypervisor state loss from nap.
+*/
+   bgt cr3,.
+
+   blr
+
 _GLOBAL(power7_wakeup_tb_loss)
ld  r2,PACATOC(r13);
ld  r1,PACAR1(r13)
@@ -266,11 +295,13 @@ _GLOBAL(power7_wakeup_tb_loss)
 * and they are restored before switching to the process context. Hence
 * until they are restored, they are free to be used.
 *
-* Save SRR1 in a NVGPR as it might be clobbered in opal_call_realmode
-* (called in CHECK_HMI_INTERRUPT). SRR1 is required to determine the
-* wakeup reason if we branch to kvm_start_guest.
+* Save SRR1 and LR in NVGPRs as they might be clobbered in
+* opal_call_realmode (called in CHECK_HMI_INTERRUPT). SRR1 is required
+* to determine the wakeup reason if we branch to kvm_start_guest. LR
+

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

2016-05-03 Thread Shreyas B. Prabhu

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-

Before this patch -
power7_wakeup_tb_loss:
restore hypervisor state
if (thread needed by kvm)
goto kvm_start_guest
restore nvgprs, cr, pc
rfid to process context

power7_wakeup_loss:
restore nvgprs, cr, pc
rfid to process context

reset vector:
if (waking from deep idle states)
goto power7_wakeup_tb_loss
else
if (thread needed by kvm)
goto kvm_start_guest
goto power7_wakeup_loss

After this patch -
power7_wakeup_tb_loss:
restore hypervisor state
return

power7_restore_hyp_resource():
if (waking from deep idle states)
goto power7_wakeup_tb_loss
return

power7_wakeup_loss:
restore nvgprs, cr, pc
rfid to process context

reset vector:
power7_restore_hyp_resource()
if (thread needed by kvm)
goto kvm_start_guest
goto power7_wakeup_loss

Signed-off-by: Shreyas B. Prabhu 
---
 arch/powerpc/kernel/exceptions-64s.S | 29 +++-
 arch/powerpc/kernel/idle_power7.S| 67 
 2 files changed, 41 insertions(+), 55 deletions(-)

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)
-   b   power7_wakeup_tb_loss
+   blt cr3,2f
+   b   power7_wakeup_loss
+2: b   power7_wakeup_noloss
 
 9:
 END_FTR_SECTION_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
diff --git a/arch/powerpc/kernel/idle_power7.S 
b/arch/powerpc/kernel/idle_power7.S
index 6b3404b..82d164b 100644
--- a/arch/powerpc/kernel/idle_power7.S
+++ b/arch/powerpc/kernel/idle_power7.S
@@ -258,6 +258,35 @@ _GLOBAL(power7_winkle)
b   power7_powersave_common
/* No return */
 
+/*
+ * Called from reset vector. Check whether we have woken up with
+ * hypervisor state loss. If yes, restore hypervisor state and return
+ * back to reset vector.
+ */
+_GLOBAL(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,power7_wakeup_tb_loss   /* Either sleep or Winkle */
+
+   /*
+* We fall through here if PACA_THREAD_IDLE_STATE shows we are waking
+* up from nap. At this stage CR3 shouldn't contains 'gt' since that
+* indicates we are waking with hypervisor state loss from nap.
+*/
+   bgt cr3,.
+
+   blr
+
 _GLOBAL(power7_wakeup_tb_loss)
ld  r2,PACATOC(r13);
ld  r1,PACAR1(r13)
@@ -266,11 +295,13 @@ _GLOBAL(power7_wakeup_tb_loss)
 * and they are restored before switching to the process context. Hence
 * until they are restored, they are free to be used.
 *
-* Save SRR1 in a NVGPR as it might be clobbered in opal_call_realmode
-* (called in CHECK_HMI_INTERRUPT). SRR1 is required to determine the
-* wakeup reason if we branch to kvm_start_guest.
+* Save SRR1 and LR in NVGPRs as they might be clobbered in
+* opal_call_realmode (called in CHECK_HMI_INTERRUPT). SRR1 is required
+* to determine the wakeup reason if we branch to kvm_start_guest. LR
+* is required to return

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

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

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

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

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

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

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

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

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

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

10 matches

Site Navigation

Mail list logo

Footer information