Re: [Qemu-devel] [PATCH v4 09/29] target-ppc: Introduce and reuse generalized init_proc_book3s_64()

[Tom Musta]

On 6/3/2014 4:27 AM, Alexey Kardashevskiy wrote:
> At the moment every POWER CPU family has its own init_proc_POWERX function.
> E500 already has common init function so we try to do the same thing.
> 
> This introduces BOOK3S_CPU_TYPE enum with 2 values - 970 and POWER5+.
> 
> This introduces generalized init_proc_book3s_64() which accepts a CPU type
> as a parameter.
> 
> This uses new init function for 970 and POWER5+ CPU classes.
> 
> 970 and POWER5+ use the same CPU class initialization except 3 things:
> 1. logical partitioning is controlled by LPCR (POWER5+) and HID4 (970)
> SPRs;
> 2. 970 does not have EAR (External Access Register) SPR and PowerISA 2.03
> defines one so keep it only for POWER5+;
> 3. POWER5+ does not have ALTIVEC so insns_flags does not have PPC_ALTIVEC
> flag set and gen_spr_book3s_altivec() won't init ALTIVEC for POWER5+.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
>  target-ppc/translate_init.c | 85 
> ++++++++++++++-------------------------------
>  1 file changed, 27 insertions(+), 58 deletions(-)
> 
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index de920a0..301f5ff 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -7266,6 +7266,11 @@ POWERPC_FAMILY(e600)(ObjectClass *oc, void *data)
>  #define POWERPC970_HID5_INIT 0x00000000
>  #endif
>  
> +enum BOOK3S_CPU_TYPE {
> +    BOOK3S_CPU_970,
> +    BOOK3S_CPU_POWER5PLUS,
> +};
> +
>  static int check_pow_970 (CPUPPCState *env)
>  {
>      if (env->spr[SPR_HID0] & 0x01C00000) {
> @@ -7485,7 +7490,16 @@ static void gen_spr_power5p_ear(CPUPPCState *env)
>                   0x00000000);
>  }
>  
> -static void init_proc_970 (CPUPPCState *env)
> +static void gen_spr_power5p_lpar(CPUPPCState *env)
> +{
> +    /* Logical partitionning */
> +    spr_register_kvm(env, SPR_LPCR, "LPCR",
> +                     SPR_NOACCESS, SPR_NOACCESS,
> +                     &spr_read_generic, &spr_write_generic,
> +                     KVM_REG_PPC_LPCR, 0x00000000);
> +}
> +
> +static void init_proc_book3s_64(CPUPPCState *env, int version)
>  {
>      gen_spr_ne_601(env);
>      gen_tbl(env);
> @@ -7500,9 +7514,13 @@ static void init_proc_970 (CPUPPCState *env)
>      gen_spr_book3s_common(env);
>      gen_spr_970_pmu_hypv(env);
>      gen_spr_970_pmu_user(env);
> -    gen_spr_970_lpar(env);
>  
> -    gen_spr_power5p_ear(env);
> +    if (version >= BOOK3S_CPU_POWER5PLUS) {
> +        gen_spr_power5p_lpar(env);
> +        gen_spr_power5p_ear(env);
> +    } else {
> +        gen_spr_970_lpar(env);
> +    }
>  
>      gen_spr_970_dbg(env);
>  #if !defined(CONFIG_USER_ONLY)
> @@ -7515,6 +7533,11 @@ static void init_proc_970 (CPUPPCState *env)
>      ppc970_irq_init(env);
>  }
>  
> +static void init_proc_970(CPUPPCState *env)
> +{
> +    init_proc_book3s_64(env, BOOK3S_CPU_970);
> +}
> +
>  POWERPC_FAMILY(970)(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> @@ -7571,61 +7594,7 @@ static int check_pow_970FX (CPUPPCState *env)
>  
>  static void init_proc_power5plus(CPUPPCState *env)
>  {
> -    gen_spr_ne_601(env);
> -    gen_spr_7xx(env);
> -    /* Time base */
> -    gen_tbl(env);
> -    /* Hardware implementation registers */
> -    /* XXX : not implemented */
> -    spr_register(env, SPR_HID0, "HID0",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_clear,
> -                 0x60000000);
> -    /* XXX : not implemented */
> -    spr_register(env, SPR_HID1, "HID1",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_generic,
> -                 0x00000000);
> -    /* XXX : not implemented */
> -    spr_register(env, SPR_970_HID5, "HID5",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_generic,
> -                 POWERPC970_HID5_INIT);
> -    /* Memory management */
> -    /* XXX: not correct */
> -    gen_low_BATs(env);
> -    spr_register(env, SPR_HIOR, "SPR_HIOR",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_hior, &spr_write_hior,
> -                 0x00000000);
> -    spr_register(env, SPR_CTRL, "SPR_CTRL",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 SPR_NOACCESS, &spr_write_generic,
> -                 0x00000000);
> -    spr_register(env, SPR_UCTRL, "SPR_UCTRL",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, SPR_NOACCESS,
> -                 0x00000000);
> -    spr_register(env, SPR_VRSAVE, "SPR_VRSAVE",
> -                 &spr_read_generic, &spr_write_generic,
> -                 &spr_read_generic, &spr_write_generic,
> -                 0x00000000);
> -    /* Logical partitionning */
> -    spr_register_kvm(env, SPR_LPCR, "LPCR",
> -                     SPR_NOACCESS, SPR_NOACCESS,
> -                     &spr_read_generic, &spr_write_generic,
> -                     KVM_REG_PPC_LPCR, 0x00000000);
> -#if !defined(CONFIG_USER_ONLY)
> -    env->slb_nr = 64;
> -#endif
> -    init_excp_970(env);
> -    env->dcache_line_size = 128;
> -    env->icache_line_size = 128;
> -    /* Allocate hardware IRQ controller */
> -    ppc970_irq_init(env);
> -    /* Can't find information on what this should be on reset.  This
> -     * value is the one used by 74xx processors. */
> -    vscr_init(env, 0x00010000);
> +    init_proc_book3s_64(env, BOOK3S_CPU_POWER5PLUS);
>  }
>  
>  POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
> 


Reviewed-by: Tom Musta <address@hidden>