Re: [PATCH] target/arm: Two fixes for secure ptw

[Richard Henderson <richard.henderson@linaro.org>]

On 11/4/22 00:19, Peter Maydell wrote:
>> @@ -2671,6 +2671,13 @@ static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
>>       bool is_secure = ptw->in_secure;
>>       ARMMMUIdx s1_mmu_idx;
>>
>> +    /*
>> +     * The page table entries may downgrade secure to non-secure, but
>> +     * cannot upgrade an non-secure translation regime's attributes
>> +     * to secure.
>> +     */
>> +    result->f.attrs.secure = is_secure;
>> +
>>       switch (mmu_idx) {
>>       case ARMMMUIdx_Phys_S:
>>       case ARMMMUIdx_Phys_NS:
>> @@ -2712,12 +2719,6 @@ static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
>>           break;
>>       }
>>
>> -    /*
>> -     * The page table entries may downgrade secure to non-secure, but
>> -     * cannot upgrade an non-secure translation regime's attributes
>> -     * to secure.
>> -     */
>> -    result->f.attrs.secure = is_secure;
>>       result->f.attrs.user = regime_is_user(env, mmu_idx);
> 
> Do we also need to move this setting of attrs.user ?
> get_phys_addr_disabled() doesn't set that either.

I don't think so.  The only cases which don't pass through the setting of attrs.user are 
the two Phys mmu_idx.  Which was by design per the comment up there about artificially 
deciding which EL regime they belong to.


r~