On Tue, 17 Mar 2026 at 12:25, Zenghui Yu <[email protected]> wrote: > > As per the pseudo code from DDI0487 M.a.a (on J1-16021) AArch64.S1Walk(): > > // Check descriptor AF bit > elsif (descriptor<10> == '0' && walkparams.ha == '0' && > (!accdesc.acctype IN {AccessType_DC, AccessType_IC} || > boolean IMPLEMENTATION_DEFINED "Generate access flag fault on > IC/DC operations")) then > fault.statuscode = Fault_AccessFlag; > > an access flag fault should be generated for AccessType_AT, if the AF bit > is 0 and !param.ha.
Did you find this by code inspection, or because it caused a guest to go wrong? We should probably Cc: [email protected] > Fixes: efebeec13d07 ("target/arm: Skip AF and DB updates for AccessType_AT") > Signed-off-by: Zenghui Yu <[email protected]> > --- > target/arm/ptw.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index 8b8dc09e72..572048d560 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -2118,6 +2118,12 @@ static bool get_phys_addr_lpae(CPUARMState *env, > S1Translate *ptw, > descaddr &= ~(hwaddr)(page_size - 1); > descaddr |= (address & (page_size - 1)); > > + /* Check descriptor AF bit */ > + if (!(descriptor & (1 << 10)) && !param.ha) { > + fi->type = ARMFault_AccessFlag; > + goto do_fault; > + } > + > /* > * For AccessType_AT, DB is not updated (AArch64.SetDirtyFlag), > * and it is IMPLEMENTATION DEFINED whether AF is updated > @@ -2127,15 +2133,9 @@ static bool get_phys_addr_lpae(CPUARMState *env, > S1Translate *ptw, > /* > * Access flag. > * If HA is enabled, prepare to update the descriptor below. > - * Otherwise, pass the access fault on to software. > */ > - if (!(descriptor & (1 << 10))) { > - if (param.ha) { > - new_descriptor |= 1 << 10; /* AF */ > - } else { > - fi->type = ARMFault_AccessFlag; > - goto do_fault; > - } > + if (!(descriptor & (1 << 10)) && param.ha) { > + new_descriptor |= 1 << 10; /* AF */ > } This is definitely correct behaviour for AT insns. The only thing I'm wondering about is what we should do for the ptw->in_debug == true case. Before commit efebeec13d07 we were checking !ptw->in_debug to skip both the testing and the updating of the access flag; now we will skip the update (definitely correct) but will fail debug accesses if the access flag is not set. There's an argument that if you're doing a debugger access it's not very helpful for it to not work just because the OS happens to have cleared the access flag on the page table entry (we also ignore permissions on debug accesses, though there are some code paths where we do enforce permissions, not really intentionally). On the other hand, we don't and I think have never suppressed AccessFlag faults for the get_phys_addr_v6 short-descriptor tables, so honouring them here too is at least consistent. Richard, what do you think? thanks -- PMM
