On 1/30/25 10:23, Peter Maydell wrote:
In system register access pseudocode the common pattern for AArch32 registers with access traps to EL3 is:at EL1 and EL2: if HaveEL(EL3) && !ELUsingAArch32(EL3) && (SCR_EL3.TERR == 1) then AArch64.AArch32SystemAccessTrap(EL3, 0x03); elsif HaveEL(EL3) && ELUsingAArch32(EL3) && (SCR.TERR == 1) then AArch32.TakeMonitorTrapException(); at EL3: if (PSTATE.M != M32_Monitor) && (SCR.TERR == 1) then AArch32.TakeMonitorTrapException(); (taking as an example the ERRIDR access pseudocode). This implements the behaviour of (in this case) SCR.TERR that "Accesses to the specified registers from modes other than Monitor mode generate a Monitor Trap exception" and of SCR_EL3.TERR that "Accesses of the specified Error Record registers at EL2 and EL1 are trapped to EL3, unless the instruction generates a higher priority exception". In QEMU we don't implement this pattern correctly in two ways: * in access_check_cp_reg() we turn the CP_ACCESS_TRAP_EL3 into an UNDEF, not a trap to Monitor mode * in the access functions, we check trap bits like SCR.TERR only when arm_current_el(env) < 3 -- this is correct for AArch64 EL3, but misses the "trap non-Monitor-mode execution at EL3 into Monitor mode" case for AArch32 EL3 In this commit we fix the first of these two issues, by making access_check_cp_reg() handle CP_ACCESS_TRAP_EL3 as a Monitor trap. This is a kind of exception that we haven't yet implemented(!), so we need a new EXCP_MON_TRAP for it. This diverges from the pseudocode approach, where every access check function explicitly checks for "if EL3 is AArch32" and takes a monitor trap; if we wanted to be closer to the pseudocode we could add a new CP_ACCESS_TRAP_MONITOR and make all the accessfns use it when appropriate. But because there are no non-standard cases in the pseudocode (i.e. where either it raises a Monitor trap that doesn't correspond to an AArch64 SystemAccessTrap or where it raises a SystemAccessTrap that doesn't correspond to a Monitor trap), handling this all in one place seems less likely to result in future bugs where we forgot again about this special case when writing an accessor. (The cc of stable here is because "hw/intc/arm_gicv3_cpuif: Don't downgrade monitor traps for AArch32 EL3" which is alsocc:stable will implicitly use the new EXCP_MON_TRAP code path.) Cc:qemu-sta...@nongnu.org Signed-off-by: Peter Maydell<peter.mayd...@linaro.org> ---
Reviewed-by: Richard Henderson <richard.hender...@linaro.org> r~
