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

[David Gibson]

On Mon, Mar 14, 2016 at 05:56:29PM +0100, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt <address@hidden>
> 
> 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 <address@hidden>
> Reviewed-by: David Gibson <address@hidden>

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.

I'm certainly willing to hear a case for this if it makes other
patches in the series significantly easier though.

> ---
>  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

signature.asc
Description: PGP signature