RE: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest

Bharat.Bhushan
2014-07-17T16:32:15Z

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Thursday, July 17, 2014 9:58 PM
> To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org
> Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
> Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest
>
>
> On 17.07.14 18:24, Bharat.Bhushan@freescale.com wrote:
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:agraf@suse.de]
> >> Sent: Thursday, July 17, 2014 9:41 PM
> >> To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org
> >> Cc: kvm@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
> >> Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering
> >> guest
> >>
> >>
> >> On 16.07.14 08:02, Bharat Bhushan wrote:
> >>> SPRG3 is guest accessible and SPRG3 can be clobbered by host or
> >>> another guest, So this need to be restored when loading guest state.
> >>>
> >>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> >>> ---
> >>> arch/powerpc/kvm/booke_interrupts.S | 2 ++
> >>> 1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/arch/powerpc/kvm/booke_interrupts.S
> >>> b/arch/powerpc/kvm/booke_interrupts.S
> >>> index 2c6deb5ef..0d3403f 100644
> >>> --- a/arch/powerpc/kvm/booke_interrupts.S
> >>> +++ b/arch/powerpc/kvm/booke_interrupts.S
> >>> @@ -459,6 +459,8 @@ lightweight_exit:
> >>> * written directly to the shared area, so we
> >>> * need to reload them here with the guest's values.
> >>> */
> >>> + PPC_LD(r3, VCPU_SHARED_SPRG3, r5)
> >>> + mtspr SPRN_SPRG3, r3
> >> We also need to restore it when resuming the host, no?
> > I do not think host expect some meaningful value when returning from guest,
> same true for SPRG4-7.
> > So there seems no reason to save host values and restore them.
>
> Hmm - arch/powerpc/include/asm/reg.h says:
>
> * All 32-bit:
> * - SPRG3 current thread_info pointer
> * (virtual on BookE, physical on others)
>
> but I can indeed find no trace of usage anywhere. This at least needs to go into
> the patch description.

I will add a comment in code as well.

Thanks
-Bharat

>
>
> Alex