Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

[Peter Maydell]

On 23 January 2015 at 21:44, Greg Bellows <address@hidden> wrote:
> On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell
> <address@hidden> wrote:
>> +typedef enum ARMMMUIdx {
>> +    ARMMMUIdx_S12NSE0 = 0,
>> +    ARMMMUIdx_S12NSE1 = 1,
>> +    ARMMMUIdx_S1E2 = 2,
>> +    ARMMMUIdx_S1E3 = 3,
>> +    ARMMMUIdx_S1SE0 = 4,
>> +    ARMMMUIdx_S1SE1 = 5,
>> +    ARMMMUIdx_S2NS = 6,
>> +    /* Indexes below here don't have TLBs and are used only for AT system
>> +     * instructions or for the first stage of an S12 page table walk.
>> +     */
>> +    ARMMMUIdx_S1NSE0 = 7,
>> +    ARMMMUIdx_S1NSE1 = 8,
>> +} ARMMMUIdx;
>> +
>>  #define MMU_MODE0_SUFFIX _user
>>  #define MMU_MODE1_SUFFIX _kernel
>>  #define MMU_USER_IDX 0
>> +
>> +/* Return the exception level we're running at if this is our mmu_idx */
>> +static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
>> +{
>> +    assert(mmu_idx < ARMMMUIdx_S2NS);
>> +    return mmu_idx & 3;
>> +}
>> +
>> +/* Determine the current mmu_idx to use for normal loads/stores */
>>  static inline int cpu_mmu_index (CPUARMState *env)
>>  {
>> -    return arm_current_el(env);
>> +    int el = arm_current_el(env);
>> +
>> +    if (el < 3 && arm_is_secure_below_el3(env)) {
>
> We bypass the secure check for EL3 but not EL2.  We should either
> circumvent both EL2 & 3 or neither.

Not sure what you mean here. The point of this code is to return
a suitable MMU idx for the current CPU state. If we are in
EL2 or EL3 then just "el" is correct (being ARMMMUIdx_S1E2 and
ARMMMUIdx_S1E3). We can't be in this condition with el == 2
because if we're in EL2 then the NS bit must be set (there's
no such thing as Secure EL2) and so arm_is_secure_below_el3()
will have returned false. So we know here that el is 0 or 1,
and this addition below:

>> +        return ARMMMUIdx_S1SE0 + el;

means we end up with either _S1SE0 or _S1SE1, as required.

(the condition could equally well be written
   if (el < 2 && arm_is_secure_below_el3(env))
 as the two are true in exactly the same set of cases. < 3 seemed
 marginally better to me, since it's expressing "if we're secure
and not in EL3" which is the set of cases where we need to
change the mmu_idx from the 0/1/2/3 values.)

>> +    }
>> +    return el;

Anything else (_S12NSE0, _S12NSE1, _S1E2, _S1E3) is this return.

>>  }

thanks
-- PMM