Re: [Qemu-devel] [PATCH 2/6] Cascade m68k_features by m680xx_cpu_initfn() to improve readability

[Laurent Vivier]

Le 16/06/2019 à 16:22, Lucien Murray-Pitts a écrit :
> The m680XX_cpu_initfn functions have been rearranged to cascade starting from
> the base 68000, so that the 68010 then inherits from this, and so on until the
> 68060.
> 
> This makes it simpler to track features since in most cases the m68k were
> product enhancements on each other, with only a few instructions being 
> retired.
> 
> Because each cpu class inherits the previous CPU class, then for example
> the 68020 also has the feature 68010, and 68000 and so on upto the 68060.
> 
> - Added 68010 cpu class, and moved correct features into 68000/68010.
> - Added m68k_unset_feature to allow removing a feature in the inheritence
> - Created real m68030_cpu_initfn to replace the macro define
>   for more obvious calling/future expansion
> 
> Signed-off-by: Lucien Murray-Pitts <address@hidden>
> ---
>  target/m68k/cpu.c | 58 +++++++++++++++++++++++++++++------------------
>  1 file changed, 36 insertions(+), 22 deletions(-)
> 
> diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
> index cc770a8042..f3246d6e72 100644
> --- a/target/m68k/cpu.c
> +++ b/target/m68k/cpu.c
> @@ -42,6 +42,11 @@ static void m68k_set_feature(CPUM68KState *env, int 
> feature)
>      env->features |= (1u << feature);
>  }
>  
> +static void m68k_unset_feature(CPUM68KState *env, int feature)
> +{
> +    env->features &= (-1u - (1u << feature));
> +}
> +
>  /* CPUClass::reset() */
>  static void m68k_cpu_reset(CPUState *s)
>  {
> @@ -119,6 +124,21 @@ static void m68000_cpu_initfn(Object *obj)
>      m68k_set_feature(env, M68K_FEATURE_MOVEP);
>  }
>  
> +
> +/*
> + * Adds BKPT, MOVE-from-SR *now priv instr, and MOVEC, MOVES, RTD
> + */
> +static void m68010_cpu_initfn(Object *obj)
> +{
> +    M68kCPU *cpu = M68K_CPU(obj);
> +    CPUM68KState *env = &cpu->env;
> +
> +    m68000_cpu_initfn(obj);
> +    m68k_set_feature(env, M68K_FEATURE_RTD);
> +    m68k_set_feature(env, M68K_FEATURE_BKPT);
> +}
> +
> +
>  /*
>   * Adds BFCHG, BFCLR, BFEXTS, BFEXTU, BFFFO, BFINS, BFSET, BFTST, CAS, CAS2,
>   *      CHK2, CMP2, DIVSL, DIVUL, EXTB, PACK, TRAPcc, UNPK.
> @@ -130,9 +150,7 @@ static void m68020_cpu_initfn(Object *obj)
>      M68kCPU *cpu = M68K_CPU(obj);
>      CPUM68KState *env = &cpu->env;
>  
> -    m68k_set_feature(env, M68K_FEATURE_M68000);
> -    m68k_set_feature(env, M68K_FEATURE_USP);
> -    m68k_set_feature(env, M68K_FEATURE_WORD_INDEX);
> +    m68010_cpu_initfn(obj);
>      m68k_set_feature(env, M68K_FEATURE_QUAD_MULDIV);
>      m68k_set_feature(env, M68K_FEATURE_BRAL);
>      m68k_set_feature(env, M68K_FEATURE_BCCL);
> @@ -142,10 +160,7 @@ static void m68020_cpu_initfn(Object *obj)
>      m68k_set_feature(env, M68K_FEATURE_LONG_MULDIV);
>      m68k_set_feature(env, M68K_FEATURE_FPU);
>      m68k_set_feature(env, M68K_FEATURE_CAS);
> -    m68k_set_feature(env, M68K_FEATURE_BKPT);
> -    m68k_set_feature(env, M68K_FEATURE_RTD);
>      m68k_set_feature(env, M68K_FEATURE_CHK2);
> -    m68k_set_feature(env, M68K_FEATURE_MOVEP);
>  }
>  
>  /*
> @@ -156,7 +171,14 @@ static void m68020_cpu_initfn(Object *obj)
>   * NOTES:
>   *  5. Not valid on MC68EC030
>   */
> -#define m68030_cpu_initfn m68020_cpu_initfn
> +static void m68030_cpu_initfn(Object *obj)
> +{
> +    M68kCPU *cpu = M68K_CPU(obj);
> +    CPUM68KState *env = &cpu->env;
> +
> +    m68020_cpu_initfn(obj);
> +}
> +
>  
>  /*
>   * Adds: CINV, CPUSH
> @@ -180,7 +202,7 @@ static void m68040_cpu_initfn(Object *obj)
>      M68kCPU *cpu = M68K_CPU(obj);
>      CPUM68KState *env = &cpu->env;
>  
> -    m68020_cpu_initfn(obj);
> +    m68030_cpu_initfn(obj);
>      m68k_set_feature(env, M68K_FEATURE_M68040);
>  }
>  
> @@ -200,20 +222,11 @@ static void m68060_cpu_initfn(Object *obj)
>      M68kCPU *cpu = M68K_CPU(obj);
>      CPUM68KState *env = &cpu->env;
>  
> -    m68k_set_feature(env, M68K_FEATURE_M68000);
> -    m68k_set_feature(env, M68K_FEATURE_USP);
> -    m68k_set_feature(env, M68K_FEATURE_WORD_INDEX);
> -    m68k_set_feature(env, M68K_FEATURE_BRAL);
> -    m68k_set_feature(env, M68K_FEATURE_BCCL);
> -    m68k_set_feature(env, M68K_FEATURE_BITFIELD);
> -    m68k_set_feature(env, M68K_FEATURE_EXT_FULL);
> -    m68k_set_feature(env, M68K_FEATURE_SCALED_INDEX);
> -    m68k_set_feature(env, M68K_FEATURE_LONG_MULDIV);
> -    m68k_set_feature(env, M68K_FEATURE_FPU);
> -    m68k_set_feature(env, M68K_FEATURE_CAS);
> -    m68k_set_feature(env, M68K_FEATURE_BKPT);
> -    m68k_set_feature(env, M68K_FEATURE_RTD);
> -    m68k_set_feature(env, M68K_FEATURE_CHK2);
> +    m68040_cpu_initfn(obj);
> +    m68k_unset_feature(env, M68K_FEATURE_MOVEP);
> +
> +    /* Implemented as a software feature */
> +    m68k_unset_feature(env, M68K_FEATURE_QUAD_MULDIV);
>  }
>  
>  static void m5208_cpu_initfn(Object *obj)
> @@ -350,6 +363,7 @@ static const TypeInfo m68k_cpus_type_infos[] = {
>          .class_init = m68k_cpu_class_init,
>      },
>      DEFINE_M68K_CPU_TYPE("m68000", m68000_cpu_initfn),
> +    DEFINE_M68K_CPU_TYPE("m68010", m68010_cpu_initfn),
>      DEFINE_M68K_CPU_TYPE("m68020", m68020_cpu_initfn),
>      DEFINE_M68K_CPU_TYPE("m68030", m68030_cpu_initfn),
>      DEFINE_M68K_CPU_TYPE("m68040", m68040_cpu_initfn),
> 

Reviewed-by: Laurent Vivier <address@hidden>