Re: [Qemu-devel] [PATCH RESEND v1 4/8] target-microblaze: Convert version_mask to a CPU property

[Alistair Francis]

On Fri, Jun 5, 2015 at 12:54 PM, Edgar E. Iglesias
<address@hidden> wrote:
> On Fri, Jun 05, 2015 at 12:53:05PM +1000, Alistair Francis wrote:
>> On Fri, Jun 5, 2015 at 12:51 PM, Alistair Francis
>> <address@hidden> wrote:
>> > On Fri, Jun 5, 2015 at 10:42 AM, Edgar E. Iglesias
>> > <address@hidden> wrote:
>> >> On Thu, Jun 04, 2015 at 11:23:19AM +1000, Alistair Francis wrote:
>> >>> Originally the version_mask 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 |    3 ++-
>> >>>  target-microblaze/cpu-qom.h         |    1 +
>> >>>  target-microblaze/cpu.c             |    4 +++-
>> >>>  3 files changed, 6 insertions(+), 2 deletions(-)
>> >>>
>> >>> diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
>> >>> b/hw/microblaze/petalogix_ml605_mmu.c
>> >>> index 5f341c4..f52654c 100644
>> >>> --- a/hw/microblaze/petalogix_ml605_mmu.c
>> >>> +++ b/hw/microblaze/petalogix_ml605_mmu.c
>> >>> @@ -70,7 +70,7 @@ static void machine_cpu_reset(MicroBlazeCPU *cpu)
>> >>>
>> >>>      env->pvr.regs[10] = 0x0e000000; /* virtex 6 */
>> >>>      /* setup pvr to match kernel setting */
>> >>> -    env->pvr.regs[0] = (env->pvr.regs[0] & ~PVR0_VERSION_MASK) | (0x14 
>> >>> << 8);
>> >>> +    env->pvr.regs[0] |= (0x14 << 8);
>> >>>      env->pvr.regs[4] = 0xc56b8000;
>> >>>      env->pvr.regs[5] = 0xc56be000;
>> >>>  }
>> >>> @@ -99,6 +99,7 @@ petalogix_ml605_init(MachineState *machine)
>> >>>      object_property_set_bool(OBJECT(cpu), true, "dcache-writeback",
>> >>>                               &error_abort);
>> >>>      object_property_set_bool(OBJECT(cpu), true, "endi", &error_abort);
>> >>> +    object_property_set_bool(OBJECT(cpu), true, "version_mask", 
>> >>> &error_abort);
>> >>>      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 85b8f75..b6c6374 100644
>> >>> --- a/target-microblaze/cpu-qom.h
>> >>> +++ b/target-microblaze/cpu-qom.h
>> >>> @@ -67,6 +67,7 @@ typedef struct MicroBlazeCPU {
>> >>>          bool usemmu;
>> >>>          bool dcache_writeback;
>> >>>          bool endi;
>> >>> +        bool version_mask;
>> >>>      } cfg;
>> >>>
>> >>>      CPUMBState env;
>> >>> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
>> >>> index f40df43..849c737 100644
>> >>> --- a/target-microblaze/cpu.c
>> >>> +++ b/target-microblaze/cpu.c
>> >>> @@ -115,7 +115,8 @@ static void mb_cpu_realizefn(DeviceState *dev, Error 
>> >>> **errp)
>> >>>      env->pvr.regs[0] |= (cpu->cfg.stackprot ? PVR0_SPROT_MASK : 0) |
>> >>>                          (cpu->cfg.usefpu ? PVR0_USE_FPU_MASK : 0) |
>> >>>                          (cpu->cfg.usemmu ? PVR0_USE_MMU_MASK : 0) |
>> >>> -                        (cpu->cfg.endi ? PVR0_ENDI_MASK : 0);
>> >>> +                        (cpu->cfg.endi ? PVR0_ENDI_MASK : 0) |
>> >>> +                        (cpu->cfg.endi ? ~PVR0_VERSION_MASK : 0);
>> >>
>> >> This looks wrong...
>> >>
>> >> My guess is that this should be:
>> >> cpu->cfg.version_mask ? PVR0_VERSION_MASK : 0
>> >
>> > So looking at the spec more the version mask should be a two byte
>>
>> Woops, I meant one byte number.
>>
>> Thanks,
>>
>> Alistair
>>
>> > number that indicated the version. I can't see it in the DTS,  so I
>> > think it should be a settable number, not a bool.
>> >
>> > I'm not sure why it zeroes it out though,
>
> Aha I see now, the ml605 is forcing the CPU version to "8.00.b".
>
> Shouldn't this prop just be a string with the re-encoding to
> version field done by the CPU model?

Yeah, you are right. That is a much better idea

Thanks,

Alistair

>
> Cheers,
> Edgar
>
>
>> >
>> > Thanks,
>> >
>> > Alistair
>> >
>> >>
>> >> and that maybe the ml605_mmu board is trying to disable the version mask,
>> >> e.g setting it to false.
>> >>
>> >> Can you double check that it matches with dts and specs?
>> >>
>> >>
>> >>>
>> >>>      env->pvr.regs[2] |= (cpu->cfg.usefpu ? PVR2_USE_FPU_MASK : 0) |
>> >>>                          (cpu->cfg.usefpu > 1 ? PVR2_USE_FPU2_MASK : 0);
>> >>> @@ -176,6 +177,7 @@ static Property mb_properties[] = {
>> >>>      DEFINE_PROP_BOOL("dcache-writeback", MicroBlazeCPU, 
>> >>> cfg.dcache_writeback,
>> >>>                       false),
>> >>>      DEFINE_PROP_BOOL("endi", MicroBlazeCPU, cfg.endi, false),
>> >>> +    DEFINE_PROP_BOOL("version-mask", MicroBlazeCPU, cfg.version_mask, 
>> >>> false),
>> >>>      DEFINE_PROP_END_OF_LIST(),
>> >>>  };
>> >>>
>> >>> --
>> >>> 1.7.1
>> >>>
>> >>
>