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

[Yongbok Kim <yongbok.kim@imgtec.com>]

On 24/10/2014 15:16, Leon Alrae wrote:
> 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 <leon.alrae@imgtec.com>
>>> ---
>>>    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
>
Agreed.

Regards,
Yongbok