Re: [Qemu-devel] [PATCH 06/17] ppc: Create cpu_ppc_set_papr() helper

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 5234 bytes --]

On Thu, Mar 17, 2016 at 01:33:31PM +0100, Cédric Le Goater wrote:
> On 03/17/2016 03:34 AM, David Gibson wrote:
> > On Mon, Mar 14, 2016 at 05:56:29PM +0100, Cédric Le Goater wrote:
> >> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >>
> >> And move the code adjusting the MSR mask and calling kvmppc_set_papr()
> >> to it. This allows us to add a few more things such as disabling setting
> >> of MSR:HV and appropriate LPCR bits which will be used when fixing
> >> the exception model.
> >>
> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > 
> > I'm a little nervous about applying this before 2.6.  This affects the
> > value of the LPCR which is used to control exception behaviour in some
> > cases.  I'm pretty sure the current behaviour is wrong, but we do know
> > it doesn't break horribly for existing machines, which we'd have to
> > retest with the new behaviour.
> 
> Yes. I agree.
> 
> > I'm certainly willing to hear a case for this if it makes other
> > patches in the series significantly easier though.
> 
> I think we should split this patch in two. Put the cpu_ppc_set_papr() helper 
> and the MSR change in the first one and keep the LPCR changes for the second. 
> The latter belong to another set of fixes related the exception
> models.

Yes, I think that makes sense.

> 
> C.
> 
> >> ---
> >>  hw/ppc/spapr.c              | 11 ++---------
> >>  target-ppc/cpu.h            |  1 +
> >>  target-ppc/translate_init.c | 37 ++++++++++++++++++++++++++++++++++++-
> >>  3 files changed, 39 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 43708a2a9086..9c01872ce4d3 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -1612,15 +1612,8 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> >>      /* Set time-base frequency to 512 MHz */
> >>      cpu_ppc_tb_init(env, TIMEBASE_FREQ);
> >>  
> >> -    /* PAPR always has exception vectors in RAM not ROM. To ensure this,
> >> -     * MSR[IP] should never be set.
> >> -     */
> >> -    env->msr_mask &= ~(1 << 6);
> >> -
> >> -    /* Tell KVM that we're in PAPR mode */
> >> -    if (kvm_enabled()) {
> >> -        kvmppc_set_papr(cpu);
> >> -    }
> >> +    /* Enable PAPR mode in TCG or KVM */
> >> +    cpu_ppc_set_papr(cpu);
> >>  
> >>      if (cpu->max_compat) {
> >>          Error *local_err = NULL;
> >> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> >> index 9ce301f18922..a7da0d3e95a9 100644
> >> --- a/target-ppc/cpu.h
> >> +++ b/target-ppc/cpu.h
> >> @@ -1268,6 +1268,7 @@ void store_booke_tcr (CPUPPCState *env, target_ulong val);
> >>  void store_booke_tsr (CPUPPCState *env, target_ulong val);
> >>  void ppc_tlb_invalidate_all (CPUPPCState *env);
> >>  void ppc_tlb_invalidate_one (CPUPPCState *env, target_ulong addr);
> >> +void cpu_ppc_set_papr(PowerPCCPU *cpu);
> >>  #endif
> >>  #endif
> >>  
> >> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> >> index 46dabe58783a..093ef036320d 100644
> >> --- a/target-ppc/translate_init.c
> >> +++ b/target-ppc/translate_init.c
> >> @@ -8496,8 +8496,43 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
> >>      pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> >>      pcc->threads_per_core = 8;
> >>  }
> >> -#endif /* defined (TARGET_PPC64) */
> >>  
> >> +#if !defined(CONFIG_USER_ONLY)
> >> +
> >> +void cpu_ppc_set_papr(PowerPCCPU *cpu)
> >> +{
> >> +    CPUPPCState *env = &cpu->env;
> >> +    ppc_spr_t *lpcr = &env->spr_cb[SPR_LPCR];
> >> +
> >> +    /* PAPR always has exception vectors in RAM not ROM. To ensure this,
> >> +     * MSR[IP] should never be set.
> >> +     *
> >> +     * We also disallow setting of MSR_HV
> >> +     */
> >> +    env->msr_mask &= ~((1ull << MSR_EP) | MSR_HVB);
> >> +
> >> +    /* Set emulated LPCR to not send interrupts to hypervisor. Note that
> >> +     * under KVM, the actual HW LPCR will be set differently by KVM itself,
> >> +     * the settings below ensure proper operations with TCG in absence of
> >> +     * a real hypervisor
> >> +     */
> >> +    lpcr->default_value &= ~(LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_KBV);
> >> +    lpcr->default_value |= LPCR_LPES0 | LPCR_LPES1;
> >> +
> >> +    /* We should be followed by a CPU reset but update the active value
> >> +     * just in case...
> >> +     */
> >> +    env->spr[SPR_LPCR] = lpcr->default_value;
> >> +
> >> +    /* Tell KVM that we're in PAPR mode */
> >> +    if (kvm_enabled()) {
> >> +        kvmppc_set_papr(cpu);
> >> +    }
> >> +}
> >> +
> >> +#endif /* !defined(CONFIG_USER_ONLY) */
> >> +
> >> +#endif /* defined (TARGET_PPC64) */
> >>  
> >>  /*****************************************************************************/
> >>  /* Generic CPU instantiation routine                                         */
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]