Re: [Qemu-devel] [RFC v1 3/3] target-microblaze: Convert use-fpu to a CPU property

[Peter Crosthwaite]

On Wed, May 13, 2015 at 11:08 PM, Alistair Francis
<address@hidden> wrote:
> Originally the use-fpu PVR bits were manually set for each machine. This
> is a hassle and difficult to read, instead set them based on the CPU
> properties.
>
> Signed-off-by: Alistair Francis <address@hidden>
> ---
>
>  hw/microblaze/petalogix_ml605_mmu.c |    4 ++--
>  target-microblaze/cpu-qom.h         |    1 +
>  target-microblaze/cpu.c             |   13 +++++++++++--
>  target-microblaze/translate.c       |    6 +++---
>  4 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
> b/hw/microblaze/petalogix_ml605_mmu.c
> index 48c264b..f4e1cc5 100644
> --- a/hw/microblaze/petalogix_ml605_mmu.c
> +++ b/hw/microblaze/petalogix_ml605_mmu.c
> @@ -71,9 +71,8 @@ static void machine_cpu_reset(MicroBlazeCPU *cpu)
>      env->pvr.regs[10] = 0x0e000000; /* virtex 6 */
>      /* setup pvr to match kernel setting */
>      env->pvr.regs[5] |= PVR5_DCACHE_WRITEBACK_MASK;
> -    env->pvr.regs[0] |= PVR0_USE_FPU_MASK | PVR0_ENDI;
> +    env->pvr.regs[0] |= PVR0_ENDI;
>      env->pvr.regs[0] = (env->pvr.regs[0] & ~PVR0_VERSION_MASK) | (0x14 << 8);
> -    env->pvr.regs[2] ^= PVR2_USE_FPU2_MASK;

So I think this here will clear PVR2_FPU2 ...

>      env->pvr.regs[4] = 0xc56b8000;
>      env->pvr.regs[5] = 0xc56be000;
>  }
> @@ -95,6 +94,7 @@ petalogix_ml605_init(MachineState *machine)
>
>      /* init CPUs */
>      cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
> +    object_property_set_int(OBJECT(cpu), 2, "xlnx.use-fpu", &error_abort);

But here you are setting use-fpu to 2. Is this a functional change for
ML605 board?

>      object_property_set_bool(OBJECT(cpu), true, "realized", &error_abort);
>
>      /* Attach emulated BRAM through the LMB.  */
> diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
> index 7bc5b81..ce3a574 100644
> --- a/target-microblaze/cpu-qom.h
> +++ b/target-microblaze/cpu-qom.h
> @@ -62,6 +62,7 @@ typedef struct MicroBlazeCPU {
>      /* Microblaze Configuration Settings */
>      struct {
>          bool stackproc;
> +        uint8_t usefpu;
>      } cfg;
>
>      CPUMBState env;
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index c08da19..c3a9a5d 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -89,10 +89,18 @@ static void mb_cpu_reset(CPUState *s)
>                          | PVR2_USE_DIV_MASK \
>                          | PVR2_USE_HW_MUL_MASK \
>                          | PVR2_USE_MUL64_MASK \
> -                        | PVR2_USE_FPU_MASK \
> -                        | PVR2_USE_FPU2_MASK \

If I have this straight, this suggests the property default value
should be 2, and the ml605 board setting should be 1.

>                          | PVR2_FPU_EXC_MASK \
>                          | 0;
> +
> +    if (cpu->cfg.usefpu) {
> +        env->pvr.regs[0] |= PVR0_USE_FPU_MASK;
> +        env->pvr.regs[2] |= PVR2_USE_FPU_MASK;
> +
> +        if (cpu->cfg.usefpu > 1) {
> +            env->pvr.regs[2] |= PVR2_USE_FPU2_MASK;
> +        }
> +    }

This should be handled at realize time. See pl330_realize for example
of realize-time PVR ("cfg" in that case) population.

To avoid churn, it might be easiest to convert the PVRs in chunks one
by one (e.g. one patch for PVR0, then PVR2 etc etc. The existing |ing
can be moved into realize for settings not worth propertyifying.

> +
>      env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
>      env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);
>
> @@ -154,6 +162,7 @@ static Property mb_properties[] = {
>      DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
>      DEFINE_PROP_BOOL("xlnx.use-stack-protection", MicroBlazeCPU, 
> cfg.stackproc,
>                       true),
> +    DEFINE_PROP_UINT8("xlnx.use-fpu", MicroBlazeCPU, cfg.usefpu, 1),

No need for xlnx prefix (FDT generic should trim the prefix).

Regards,
Peter

>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
> index 4068946..36cfc5c 100644
> --- a/target-microblaze/translate.c
> +++ b/target-microblaze/translate.c
> @@ -1415,11 +1415,11 @@ static int dec_check_fpuv2(DisasContext *dc)
>
>      r = dc->cpu->env.pvr.regs[2] & PVR2_USE_FPU2_MASK;
>
> -    if (!r && (dc->tb_flags & MSR_EE_FLAG)) {
> +    if ((dc->cpu->cfg.usefpu != 2) && (dc->tb_flags & MSR_EE_FLAG)) {
>          tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_FPU);
>          t_gen_raise_exception(dc, EXCP_HW_EXCP);
>      }
> -    return r;
> +    return (dc->cpu->cfg.usefpu == 2) ? 0 : PVR2_USE_FPU2_MASK;
>  }
>
>  static void dec_fpu(DisasContext *dc)
> @@ -1428,7 +1428,7 @@ static void dec_fpu(DisasContext *dc)
>
>      if ((dc->tb_flags & MSR_EE_FLAG)
>            && (dc->cpu->env.pvr.regs[2] & PVR2_ILL_OPCODE_EXC_MASK)
> -          && !((dc->cpu->env.pvr.regs[2] & PVR2_USE_FPU_MASK))) {
> +          && (dc->cpu->cfg.usefpu != 1)) {
>          tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_ILLEGAL_OP);
>          t_gen_raise_exception(dc, EXCP_HW_EXCP);
>          return;
> --
> 1.7.1
>
>