Re: [Qemu-devel] [PATCH v2 6/9] target-microblaze: Convert pvr-full to a CPU property

[Alistair Francis]

On Mon, Jun 15, 2015 at 12:41 PM, Peter Crosthwaite
<address@hidden> wrote:
> On Thu, Jun 4, 2015 at 11:42 PM, Alistair Francis
> <address@hidden> wrote:
>> Originally the pvr-full 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>
>> ---
>> V2:
>>  - Rename DTS mapping
>>
>>  target-microblaze/cpu-qom.h |    1 +
>>  target-microblaze/cpu.c     |    5 +++--
>>  target-microblaze/helper.c  |    4 ++--
>>  3 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
>> index 7da25fa..74d70de 100644
>> --- a/target-microblaze/cpu-qom.h
>> +++ b/target-microblaze/cpu-qom.h
>> @@ -68,6 +68,7 @@ typedef struct MicroBlazeCPU {
>>          bool dcache_writeback;
>>          bool endi;
>>          char *version;
>> +        bool pvr_full;
>>      } cfg;
>>
>>      CPUMBState env;
>> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
>> index a6e1872..8a86aa5 100644
>> --- a/target-microblaze/cpu.c
>> +++ b/target-microblaze/cpu.c
>> @@ -129,8 +129,7 @@ static void mb_cpu_realizefn(DeviceState *dev, Error 
>> **errp)
>>
>>      qemu_init_vcpu(cs);
>>
>> -    env->pvr.regs[0] = PVR0_PVR_FULL_MASK \
>> -                       | PVR0_USE_BARREL_MASK \
>> +    env->pvr.regs[0] = PVR0_USE_BARREL_MASK \
>>                         | PVR0_USE_DIV_MASK \
>>                         | PVR0_USE_HW_MUL_MASK \
>>                         | PVR0_USE_EXC_MASK \
>> @@ -166,6 +165,7 @@ static void mb_cpu_realizefn(DeviceState *dev, Error 
>> **errp)
>>                          (cpu->cfg.use_mmu ? PVR0_USE_MMU_MASK : 0);
>>                          (cpu->cfg.endi ? PVR0_ENDI_MASK : 0) |
>>                          (version_code << 16);
>> +                        (cpu->cfg.pvr_full ? PVR0_PVR_FULL_MASK : 0);
>>
>>      env->pvr.regs[2] |= (cpu->cfg.use_fpu ? PVR2_USE_FPU_MASK : 0) |
>>                          (cpu->cfg.use_fpu > 1 ? PVR2_USE_FPU2_MASK : 0);
>> @@ -227,6 +227,7 @@ static Property mb_properties[] = {
>>                       false),
>>      DEFINE_PROP_BOOL("endianness", MicroBlazeCPU, cfg.endi, false),
>>      DEFINE_PROP_STRING("version", MicroBlazeCPU, cfg.version),
>> +    DEFINE_PROP_BOOL("pvr", MicroBlazeCPU, cfg.pvr_full, true),
>
>
> From UG984, C_PVR config option has 3 encodings (0-2). Even if we do
> not support the functional difference for basic vs none (0 v 1) on
> this setting, the prop should match the encoding scheme. This should
> be uint8_t where 2 means full version PVR.

Thanks Peter, you are correct. I just assumed it was a bool, thanks for
double checking. Fixed in version 3.

Thanks,

Alistair

>
> Regards,
> Peter
>
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>
>> diff --git a/target-microblaze/helper.c b/target-microblaze/helper.c
>> index 5156c12..c3e1d79 100644
>> --- a/target-microblaze/helper.c
>> +++ b/target-microblaze/helper.c
>> @@ -58,8 +58,8 @@ int mb_cpu_handle_mmu_fault(CPUState *cs, vaddr address, 
>> int rw,
>>      mmu_available = 0;
>>      if (cpu->cfg.use_mmu) {
>>          mmu_available = 1;
>> -        if ((env->pvr.regs[0] & PVR0_PVR_FULL_MASK)
>> -            && (env->pvr.regs[11] & PVR11_USE_MMU) != PVR11_USE_MMU) {
>> +        if (cpu->cfg.pvr_full &&
>> +            (env->pvr.regs[11] & PVR11_USE_MMU) != PVR11_USE_MMU) {
>>              mmu_available = 0;
>>          }
>>      }
>> --
>> 1.7.1
>>
>>
>