[6/6] target-ppc: Add PMC7/8 to 970

970 CPUs have PMC7/8. Create gen_spr_970 to avoid replicating
it 3 times, and simplify the existing code.

Signed-off-by: Anton Blanchard <anton@samba.org>
---
 target-ppc/translate_init.c | 89 ++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 50 deletions(-)

On 03/25/2014 01:40 PM, Anton Blanchard wrote:
> 970 CPUs have PMC7/8. Create gen_spr_970 to avoid replicating
> it 3 times, and simplify the existing code.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
>  target-ppc/translate_init.c | 89 ++++++++++++++++++++-------------------------
>  1 file changed, 39 insertions(+), 50 deletions(-)
> 
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 273e37d..50b2603 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -6747,12 +6747,13 @@ static void gen_spr_book3s (CPUPPCState *env)
>                   0x00000000);
>  }
>  
> -static void init_proc_970 (CPUPPCState *env)
> +static void gen_spr_970 (CPUPPCState *env)
>  {
> -    gen_spr_ne_601(env);
> -    gen_spr_book3s(env);
> -    /* Time base */
> -    gen_tbl(env);
> +    spr_register(env, SPR_HIOR, "SPR_HIOR",


+    spr_register(env, SPR_HIOR, "HIOR",

Can we please not add SPR_ in string name? Thanks.



> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 &spr_read_hior, &spr_write_hior,
> +                 0x00000000);
> +
>      /* Hardware implementation registers */
>      /* XXX : not implemented */
>      spr_register(env, SPR_HID0, "HID0",
> @@ -6769,13 +6770,40 @@ static void init_proc_970 (CPUPPCState *env)
>                   SPR_NOACCESS, SPR_NOACCESS,
>                   &spr_read_generic, &spr_write_generic,
>                   POWERPC970_HID5_INIT);
> +
> +    /* Performance monitors */
> +    /* XXX : not implemented */
> +    spr_register_kvm(env, SPR_BOOK3S_PMC7, "PMC7",
> +                     SPR_NOACCESS, SPR_NOACCESS,
> +                     &spr_read_generic, &spr_write_generic,
> +                     KVM_REG_PPC_PMC7, 0x00000000);
> +    /* XXX : not implemented */
> +    spr_register_kvm(env, SPR_BOOK3S_PMC8, "PMC8",
> +                     SPR_NOACCESS, SPR_NOACCESS,
> +                     &spr_read_generic, &spr_write_generic,
> +                     KVM_REG_PPC_PMC8, 0x00000000);
> +    /* XXX : not implemented */
> +    spr_register(env, SPR_BOOK3S_UPMC7, "UPMC7",
> +                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_ureg, SPR_NOACCESS,
> +                 0x00000000);
> +    /* XXX : not implemented */
> +    spr_register(env, SPR_BOOK3S_UPMC8, "UPMC8",
> +                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_ureg, SPR_NOACCESS,
> +                 0x00000000);
> +}
> +
> +static void init_proc_970 (CPUPPCState *env)
> +{
> +    gen_spr_ne_601(env);
> +    gen_spr_book3s(env);
> +    gen_spr_970(env);
> +    /* Time base */
> +    gen_tbl(env);
>      /* 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);
>  #if !defined(CONFIG_USER_ONLY)
>      env->slb_nr = 32;
>  #endif
> @@ -6831,31 +6859,12 @@ static void init_proc_970FX (CPUPPCState *env)
>  {
>      gen_spr_ne_601(env);
>      gen_spr_book3s(env);
> +    gen_spr_970(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,
> @@ -6923,32 +6932,12 @@ static void init_proc_970MP (CPUPPCState *env)
>  {
>      gen_spr_ne_601(env);
>      gen_spr_book3s(env);
> +    gen_spr_970(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);
> -    /* XXX : not implemented */
>      /* 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);
>      /* Logical partitionning */
>      spr_register_kvm(env, SPR_LPCR, "LPCR",
>                       SPR_NOACCESS, SPR_NOACCESS,
>

Am 27.03.2014 14:03, schrieb Alexey Kardashevskiy:
> On 03/25/2014 01:40 PM, Anton Blanchard wrote:
>> 970 CPUs have PMC7/8. Create gen_spr_970 to avoid replicating
>> it 3 times, and simplify the existing code.
>>
>> Signed-off-by: Anton Blanchard <anton@samba.org>
>> ---
>>  target-ppc/translate_init.c | 89 ++++++++++++++++++++-------------------------
>>  1 file changed, 39 insertions(+), 50 deletions(-)
>>
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 273e37d..50b2603 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -6747,12 +6747,13 @@ static void gen_spr_book3s (CPUPPCState *env)
>>                   0x00000000);
>>  }
>>  
>> -static void init_proc_970 (CPUPPCState *env)
>> +static void gen_spr_970 (CPUPPCState *env)
>>  {
>> -    gen_spr_ne_601(env);
>> -    gen_spr_book3s(env);
>> -    /* Time base */
>> -    gen_tbl(env);
>> +    spr_register(env, SPR_HIOR, "SPR_HIOR",
> 
> 
> +    spr_register(env, SPR_HIOR, "HIOR",
> 
> Can we please not add SPR_ in string name? Thanks.

As far I see, this is only being moved around, not added here.

If this is not user ABI, feel free to first drop it, then move it.
Similarly, adding the new ones in a third step would make review and
bisection easier.

Regards,
Andreas

On 25.03.14 03:40, Anton Blanchard wrote:
> 970 CPUs have PMC7/8. Create gen_spr_970 to avoid replicating
> it 3 times, and simplify the existing code.
>
> Signed-off-by: Anton Blanchard <anton@samba.org>

Don't you think we could just combine 970, 970FX and 970MP into the same 
class?


Alex