Re: [Qemu-devel] [PATCH v2 4/9] target-mips: add RI and XI fields to TLB entry

[Leon Alrae]

On 15/10/2014 13:24, Yongbok Kim wrote:
> 
> On 08/07/2014 08:57, Leon Alrae wrote:
>> In Revision 3 of the architecture, the RI and XI bits were added to
>> the TLB
>> to enable more secure access of memory pages. These bits (along with
>> the Dirty
>> bit) allow the implementation of read-only, write-only, no-execute access
>> policies for mapped pages.
>>
>> Signed-off-by: Leon Alrae <address@hidden>
>> ---
>>   target-mips/cpu.h       |   11 +++++++++++
>>   target-mips/helper.c    |   11 ++++++++++-
>>   target-mips/op_helper.c |    8 ++++++++
>>   3 files changed, 29 insertions(+), 1 deletions(-)
>>
>> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
>> index 4f6aa5b..5afafd7 100644
>> --- a/target-mips/cpu.h
>> +++ b/target-mips/cpu.h
>> @@ -30,6 +30,10 @@ struct r4k_tlb_t {
>>       uint_fast16_t V1:1;
>>       uint_fast16_t D0:1;
>>       uint_fast16_t D1:1;
>> +    uint_fast16_t XI0:1;
>> +    uint_fast16_t XI1:1;
>> +    uint_fast16_t RI0:1;
>> +    uint_fast16_t RI1:1;
>>       target_ulong PFN[2];
>>   };
>>   @@ -229,6 +233,13 @@ struct CPUMIPSState {
>>   #define CP0VPEOpt_DWX0    0
>>       target_ulong CP0_EntryLo0;
>>       target_ulong CP0_EntryLo1;
>> +#if defined(TARGET_MIPS64)
>> +# define CP0EnLo_RI 63
>> +# define CP0EnLo_XI 62
>> +#else
>> +# define CP0EnLo_RI 31
>> +# define CP0EnLo_XI 30
>> +#endif
>>       target_ulong CP0_Context;
>>       target_ulong CP0_KScratch[MIPS_KSCRATCH_NUM];
>>       int32_t CP0_PageMask;
>> diff --git a/target-mips/helper.c b/target-mips/helper.c
>> index 9871273..6aa8c8a 100644
>> --- a/target-mips/helper.c
>> +++ b/target-mips/helper.c
>> @@ -27,6 +27,8 @@
>>   #include "sysemu/kvm.h"
>>     enum {
>> +    TLBRET_XI = -6,
>> +    TLBRET_RI = -5,
>>       TLBRET_DIRTY = -4,
>>       TLBRET_INVALID = -3,
>>       TLBRET_NOMATCH = -2,
>> @@ -85,8 +87,15 @@ int r4k_map_address (CPUMIPSState *env, hwaddr
>> *physical, int *prot,
>>               /* TLB match */
>>               int n = !!(address & mask & ~(mask >> 1));
>>               /* Check access rights */
>> -            if (!(n ? tlb->V1 : tlb->V0))
>> +            if (!(n ? tlb->V1 : tlb->V0)) {
>>                   return TLBRET_INVALID;
>> +            }
>> +            if (rw == MMU_INST_FETCH && (n ? tlb->XI1 : tlb->XI0)) {
>> +                return TLBRET_XI;
>> +            }
>> +            if (rw == MMU_DATA_LOAD && (n ? tlb->RI1 : tlb->RI0)) {
>> +                return TLBRET_RI;
> 
> PC relative loads are allowed where execute is allowed (even though RI
> is 1).
> Rather than just return RI here have to check XI and its OP code.

This is true only for MIPS16 PC-relative loads. New R6 PC-relative loads
do cause TLBRI exceptions. Thus in context of Release 6 current
implementation is correct. I agree this will need to be corrected for
MIPS16, but not necessarily in this patchset.

Regards,
Leon