Re: [PATCH 03/20] target/arm: Convert get_phys_addr_lpae to access_perm

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

On 7/10/25 05:59, Peter Maydell wrote:
> On Mon, 7 Jul 2025 at 22:01, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/arm/ptw.c | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
>> index 39ecc093a5..7503d1de6f 100644
>> --- a/target/arm/ptw.c
>> +++ b/target/arm/ptw.c
>> @@ -1643,14 +1643,14 @@ static bool nv_nv1_enabled(CPUARMState *env, S1Translate *ptw)
>>    * @env: CPUARMState
>>    * @ptw: Current and next stage parameters for the walk.
>>    * @address: virtual address to get physical address for
>> - * @access_type: MMU_DATA_LOAD, MMU_DATA_STORE or MMU_INST_FETCH
>> + * @access_perm: PAGE_{READ, WRITE, EXEC}, or 0
>>    * @memop: memory operation feeding this access, or 0 for none
>>    * @result: set on translation success,
>>    * @fi: set to fault info if the translation fails
>>    */
>>   static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
>>                                  uint64_t address,
>> -                               MMUAccessType access_type, MemOp memop,
>> +                               unsigned access_perm, MemOp memop,
>>                                  GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
>>   {
>>       ARMCPU *cpu = env_archcpu(env);
>> @@ -1678,7 +1678,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
>>           int ps;
>>
>>           param = aa64_va_parameters(env, address, mmu_idx,
>> -                                   access_type != MMU_INST_FETCH,
>> +                                   !(access_perm & PAGE_EXEC),
>>                                      !arm_el_is_aa64(env, 1));
>>           level = 0;
> 
> This will treat a "don't check access permissions" call as
> a data-access (relevant for TBI), and means there's no way
> to say "do an address lookup for INST_FETCH but don't do the
> access-permission check". Is that what we want?
> We should at least comment this.
It does happen to be what we want for ats1a.
I can add a comment.

r~