On 3/6/25 08:39, Peter Maydell wrote:
The functions arm_current_el() and arm_el_is_aa64() are used only in
target/arm and in hw/intc/arm_gicv3_cpuif.c. They're functions that
query internal state of the CPU. Move them out of cpu.h and into
internals.h.
This means we need to include internals.h in arm_gicv3_cpuif.c, but
this is justifiable because that file is implementing the GICv3 CPU
interface, which really is part of the CPU proper; we just ended up
implementing it in code in hw/intc/ for historical reasons.
The motivation for this move is that we'd like to change
arm_el_is_aa64() to add a condition that uses cpu_isar_feature();
but we don't want to include cpu-features.h in cpu.h.
Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
---
target/arm/cpu.h | 66 --------------------------------------
target/arm/internals.h | 67 +++++++++++++++++++++++++++++++++++++++
hw/intc/arm_gicv3_cpuif.c | 1 +
target/arm/arch_dump.c | 1 +
4 files changed, 69 insertions(+), 66 deletions(-)
Likewise, is there a good reason to keep arm_current_el inline?
I can see that a fair fraction of arm_el_is_aa64 calls have a constant second argument
(and most of those check el3). Therefore I can see that keeping that function inline can
eliminate quite a lot of tests.
Anyway,
Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
+/* Return true if the specified exception level is running in AArch64 state. */
+static inline bool arm_el_is_aa64(CPUARMState *env, int el)
+{
+ /*
+ * This isn't valid for EL0 (if we're in EL0, is_a64() is what you want,
+ * and if we're not in EL0 then the state of EL0 isn't well defined.)
+ */
+ assert(el >= 1 && el <= 3);
+ bool aa64 = arm_feature(env, ARM_FEATURE_AARCH64);
+
+ /*
+ * The highest exception level is always at the maximum supported
+ * register width, and then lower levels have a register width controlled
+ * by bits in the SCR or HCR registers.
+ */
+ if (el == 3) {
+ return aa64;
+ }
+
+ if (arm_feature(env, ARM_FEATURE_EL3) &&
+ ((env->cp15.scr_el3 & SCR_NS) || !(env->cp15.scr_el3 & SCR_EEL2))) {
+ aa64 = aa64 && (env->cp15.scr_el3 & SCR_RW);
+ }
+
+ if (el == 2) {
+ return aa64;
+ }
+
+ if (arm_is_el2_enabled(env)) {
+ aa64 = aa64 && (env->cp15.hcr_el2 & HCR_RW);
+ }
+
+ return aa64;
+}
I'll note that this would be clearer with early returns instead of &&.
E.g.
if (!arm_feature(env, ARM_FEATURE_AARCH64)) {
return false;
}
if (el == 3) {
return true;
}
if (arm_feature(env, ARM_FEATURE_EL3)
&& (env->cp15.scr_el3 & SCR_RW)
&& ((env->cp15.scr_el3 & SCR_NS)
|| !(env->cp15.scr_el3 & SCR_EEL2))) {
return false;
}
if (el == 2) {
return true;
}
...
But of course not changed with code movement.
r~
