Re: [PATCH v2 7/9] powerpc/64s: do not allocate lppaca if we are not virtualized

[Nicholas Piggin]

On Sat, 14 Oct 2017 09:47:59 +1100
Paul Mackerras  wrote:

> On Sun, Aug 13, 2017 at 11:33:44AM +1000, Nicholas Piggin wrote:
> > The "lppaca" is a structure registered with the hypervisor. This
> > is unnecessary when running on non-virtualised platforms. One field
> > from the lppaca (pmcregs_in_use) is also used by the host, so move
> > the host part out into the paca (lppaca field is still updated in
> > guest mode).  
> 
> There is an error in the patch, see below...
> 
> > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
> > b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > index c52184a8efdf..b838348e3a2b 100644
> > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > @@ -99,8 +99,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
> > mtspr   SPRN_SPRG_VDSO_WRITE,r3
> >  
> > /* Reload the host's PMU registers */
> > -   ld  r3, PACALPPACAPTR(r13)  /* is the host using the PMU? */
> > -   lbz r4, LPPACA_PMCINUSE(r3)
> > +   lbz r4, PACA_PMCINUSE(r13) /* is the host using the PMU? */
> > cmpwi   r4, 0
> > beq 23f /* skip if not */
> >  BEGIN_FTR_SECTION
> > @@ -1671,7 +1670,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> > mtspr   SPRN_MMCRA, r7
> > isync
> > beq 21f /* if no VPA, save PMU stuff anyway */
> > -   lbz r7, LPPACA_PMCINUSE(r8)
> > +   lbz r7, PACA_PMCINUSE(r13)  
> 
> We really do need to check the guest's flag not the host's here, since
> we're deciding whether to save the PMU state to the vcpu struct.

Okay I'll fix that up.

Thanks,
Nick

Re: [PATCH v2 7/9] powerpc/64s: do not allocate lppaca if we are not virtualized

[Paul Mackerras]

On Sun, Aug 13, 2017 at 11:33:44AM +1000, Nicholas Piggin wrote:
> The "lppaca" is a structure registered with the hypervisor. This
> is unnecessary when running on non-virtualised platforms. One field
> from the lppaca (pmcregs_in_use) is also used by the host, so move
> the host part out into the paca (lppaca field is still updated in
> guest mode).

There is an error in the patch, see below...

> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
> b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index c52184a8efdf..b838348e3a2b 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -99,8 +99,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
>   mtspr   SPRN_SPRG_VDSO_WRITE,r3
>  
>   /* Reload the host's PMU registers */
> - ld  r3, PACALPPACAPTR(r13)  /* is the host using the PMU? */
> - lbz r4, LPPACA_PMCINUSE(r3)
> + lbz r4, PACA_PMCINUSE(r13) /* is the host using the PMU? */
>   cmpwi   r4, 0
>   beq 23f /* skip if not */
>  BEGIN_FTR_SECTION
> @@ -1671,7 +1670,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
>   mtspr   SPRN_MMCRA, r7
>   isync
>   beq 21f /* if no VPA, save PMU stuff anyway */
> - lbz r7, LPPACA_PMCINUSE(r8)
> + lbz r7, PACA_PMCINUSE(r13)

We really do need to check the guest's flag not the host's here, since
we're deciding whether to save the PMU state to the vcpu struct.

Paul.