Re: [Qemu-devel] [PATCH 3/7] target-mips: add CP0.PageGrain.ELPA support

[Leon Alrae <leon.alrae@imgtec.com>]

Hi James,

On 28/04/2015 16:08, James Hogan wrote:
>> diff --git a/target-mips/mips-defs.h b/target-mips/mips-defs.h
>> index 1784227..20aa87c 100644
>> --- a/target-mips/mips-defs.h
>> +++ b/target-mips/mips-defs.h
>> @@ -10,11 +10,11 @@
>>  
>>  #if defined(TARGET_MIPS64)
>>  #define TARGET_LONG_BITS 64
>> -#define TARGET_PHYS_ADDR_SPACE_BITS 36
>> +#define TARGET_PHYS_ADDR_SPACE_BITS 48
>>  #define TARGET_VIRT_ADDR_SPACE_BITS 42
>>  #else
>>  #define TARGET_LONG_BITS 32
>> -#define TARGET_PHYS_ADDR_SPACE_BITS 36
>> +#define TARGET_PHYS_ADDR_SPACE_BITS 40
> 
> Out of interest, is there a particular reason not to put this up to 59,
> the max supported by the architecture, rather than just what P5600 supports?

More bits we declare then more levels of page tables QEMU will have,
which means more time spent on page walking. Therefore I don't think
it's a good idea to put the architectural limit if we don't have to.

>>  #define TARGET_VIRT_ADDR_SPACE_BITS 32
>>  #endif
>>  
>> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
>> index 6bff927..4b1b0ec 100644
>> --- a/target-mips/op_helper.c
>> +++ b/target-mips/op_helper.c
>> @@ -1067,19 +1067,28 @@ void helper_mtc0_vpeopt(CPUMIPSState *env, target_ulong arg1)
>>      env->CP0_VPEOpt = arg1 & 0x0000ffff;
>>  }
>>  
>> +static inline target_ulong get_mtc0_entrylo_mask(const CPUMIPSState *env)
>> +{
>> +#if defined(TARGET_MIPS64)
>> +    return env->PAMask >> 6;
> 
> I think this case is suitable for dmtc0 EntryLo regardless of MIPS64/MIPS32?
> 
>> +#else
>> +    return (env->PAMask >> 6) & 0x3FFFFFFF;
> 
> mtc0 sets bits 61:30 of EntryLo to 0 on MIPS64 too, so this case is
> suitable for mtc0 regardless of MIPS64/MIPS32?

I must have assumed here that mtc0 and dmtc0 do the same thing in MIPS64
(apart from having RI/XI bits in different places). I'll fix it, thanks.

Leon