On Sat, 5 Oct 2024 at 21:06, Richard Henderson <richard.henderson@xxxxxxxxxx> wrote: > > Zero is the safe do-nothing value for callers to use. > > Signed-off-by: Richard Henderson <richard.henderson@xxxxxxxxxx> > --- > target/arm/internals.h | 3 ++- > target/arm/helper.c | 4 ++-- > target/arm/ptw.c | 2 +- > 3 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/target/arm/internals.h b/target/arm/internals.h > index 2b16579fa5..a6088d551c 100644 > --- a/target/arm/internals.h > +++ b/target/arm/internals.h > @@ -1461,6 +1461,7 @@ bool get_phys_addr(CPUARMState *env, vaddr address, > * @env: CPUARMState > * @address: virtual address to get physical address for > * @access_type: 0 for read, 1 for write, 2 for execute > + * @memop: memory operation feeding this access, or 0 for none > * @mmu_idx: MMU index indicating required translation regime > * @space: security space for the access > * @result: set on translation success. > @@ -1470,7 +1471,7 @@ bool get_phys_addr(CPUARMState *env, vaddr address, > * a Granule Protection Check on the resulting address. > */ > bool get_phys_addr_with_space_nogpc(CPUARMState *env, vaddr address, > - MMUAccessType access_type, > + MMUAccessType access_type, MemOp memop, > ARMMMUIdx mmu_idx, ARMSecuritySpace space, > GetPhysAddrResult *result, > ARMMMUFaultInfo *fi) > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 3f77b40734..f2f329e00a 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -3602,8 +3602,8 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, > * I_MXTJT: Granule protection checks are not performed on the final address > * of a successful translation. > */ > - ret = get_phys_addr_with_space_nogpc(env, value, access_type, mmu_idx, ss, > - &res, &fi); > + ret = get_phys_addr_with_space_nogpc(env, value, access_type, 0, > + mmu_idx, ss, &res, &fi); 0 is the correct thing here, because AT operations are effectively assuming an aligned address (cf AArch64.AT() setting "aligned = true" in the Arm ARM pseudocode). Is there a way to write this as something other than "0" as an indication of "we've definitely thought about this callsite and it's not just 0 for back-compat behaviour" ? Otherwise we could add a brief comment. Otherwise Reviewed-by: Peter Maydell <peter.maydell@xxxxxxxxxx> thanks -- PMM
