The A32_BANKED_REG_{GET,SET} macros are only used inside target/arm;

move their definitions to cpregs.h. There's no need to have them

defined in all the code that includes cpu.h.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

---

target/arm/cpregs.h | 28 ++++++++++++++++++++++++++++

target/arm/cpu.h | 27 ---------------------------

2 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpregs.h

+++ b/target/arm/cpregs.h

@@ -XXX,XX +XXX,XX @@ static inline bool arm_cpreg_traps_in_nv(const ARMCPRegInfo *ri)

return ri->opc1 == 4 || ri->opc1 == 5;

}

+/* Macros for accessing a specified CP register bank */

+#define A32_BANKED_REG_GET(_env, _regname, _secure) \

+ ((_secure) ? (_env)->cp15._regname##_s : (_env)->cp15._regname##_ns)

+

+#define A32_BANKED_REG_SET(_env, _regname, _secure, _val) \

+ do { \

+ if (_secure) { \

+ (_env)->cp15._regname##_s = (_val); \

+ } else { \

+ (_env)->cp15._regname##_ns = (_val); \

+ } \

+ } while (0)

+

+/*

+ * Macros for automatically accessing a specific CP register bank depending on

+ * the current secure state of the system. These macros are not intended for

+ * supporting instruction translation reads/writes as these are dependent

+ * solely on the SCR.NS bit and not the mode.

+ */

+#define A32_BANKED_CURRENT_REG_GET(_env, _regname) \

+ A32_BANKED_REG_GET((_env), _regname, \

+ (arm_is_secure(_env) && !arm_el_is_aa64((_env), 3)))

+

+#define A32_BANKED_CURRENT_REG_SET(_env, _regname, _val) \

+ A32_BANKED_REG_SET((_env), _regname, \

+ (arm_is_secure(_env) && !arm_el_is_aa64((_env), 3)), \

+ (_val))

+

#endif /* TARGET_ARM_CPREGS_H */

diff --git a/target/arm/cpu.h b/target/arm/cpu.h

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpu.h

+++ b/target/arm/cpu.h

@@ -XXX,XX +XXX,XX @@ static inline bool access_secure_reg(CPUARMState *env)

return ret;

}

-/* Macros for accessing a specified CP register bank */

-#define A32_BANKED_REG_GET(_env, _regname, _secure) \

- ((_secure) ? (_env)->cp15._regname##_s : (_env)->cp15._regname##_ns)

-

-#define A32_BANKED_REG_SET(_env, _regname, _secure, _val) \

- do { \

- if (_secure) { \

- (_env)->cp15._regname##_s = (_val); \

- } else { \

- (_env)->cp15._regname##_ns = (_val); \

- } \

- } while (0)

-

-/* Macros for automatically accessing a specific CP register bank depending on

- * the current secure state of the system. These macros are not intended for

- * supporting instruction translation reads/writes as these are dependent

- * solely on the SCR.NS bit and not the mode.

- */

-#define A32_BANKED_CURRENT_REG_GET(_env, _regname) \

- A32_BANKED_REG_GET((_env), _regname, \

- (arm_is_secure(_env) && !arm_el_is_aa64((_env), 3)))

-

-#define A32_BANKED_CURRENT_REG_SET(_env, _regname, _val) \

- A32_BANKED_REG_SET((_env), _regname, \

- (arm_is_secure(_env) && !arm_el_is_aa64((_env), 3)), \

- (_val))

-

uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,

uint32_t cur_el, bool secure);

--

2.43.0

We would like to move arm_el_is_aa64() to internals.h; however, it is

used by access_secure_reg(). Make that function not be inline, so

that it can stay in cpu.h.

access_secure_reg() is used only in two places:

* in hflags.c

* in the user-mode arm emulators, to decide whether to store

the TLS value in the secure or non-secure banked field

The second of these is not on a super-hot path that would care about

the inlining (and incidentally will always use the NS banked field

because our user-mode CPUs never set ARM_FEATURE_EL3); put the

definition of access_secure_reg() in hflags.c, near its only use

inside target/arm.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

---

target/arm/cpu.h | 12 +++---------

target/arm/tcg/hflags.c | 9 +++++++++

2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpu.h

+++ b/target/arm/cpu.h

@@ -XXX,XX +XXX,XX @@ static inline bool arm_el_is_aa64(CPUARMState *env, int el)

return aa64;

}

-/* Function for determining whether guest cp register reads and writes should

+/*

+ * Function for determining whether guest cp register reads and writes should

* access the secure or non-secure bank of a cp register. When EL3 is

* operating in AArch32 state, the NS-bit determines whether the secure

* instance of a cp register should be used. When EL3 is AArch64 (or if

* it doesn't exist at all) then there is no register banking, and all

* accesses are to the non-secure version.

*/

-static inline bool access_secure_reg(CPUARMState *env)

-{

- bool ret = (arm_feature(env, ARM_FEATURE_EL3) &&

- !arm_el_is_aa64(env, 3) &&

- !(env->cp15.scr_el3 & SCR_NS));

-

- return ret;

-}

+bool access_secure_reg(CPUARMState *env);

uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,

uint32_t cur_el, bool secure);

diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/tcg/hflags.c

+++ b/target/arm/tcg/hflags.c

@@ -XXX,XX +XXX,XX @@ static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t sctlr)

#endif

}

+bool access_secure_reg(CPUARMState *env)

+{

+ bool ret = (arm_feature(env, ARM_FEATURE_EL3) &&

+ !arm_el_is_aa64(env, 3) &&

+ !(env->cp15.scr_el3 & SCR_NS));

+

+ return ret;

+}

+

static CPUARMTBFlags rebuild_hflags_common(CPUARMState *env, int fp_el,

ARMMMUIdx mmu_idx,

CPUARMTBFlags flags)

--

2.43.0

At the top of linux-user/aarch64/cpu_loop.c we define a set of

macros for reading and writing data and code words, but we never

use these macros. Delete them.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

---

linux-user/aarch64/cpu_loop.c | 48 -----------------------------------

1 file changed, 48 deletions(-)

diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c

index XXXXXXX..XXXXXXX 100644

--- a/linux-user/aarch64/cpu_loop.c

+++ b/linux-user/aarch64/cpu_loop.c

@@ -XXX,XX +XXX,XX @@

#include "target/arm/syndrome.h"

#include "target/arm/cpu-features.h"

-#define get_user_code_u32(x, gaddr, env) \

- ({ abi_long __r = get_user_u32((x), (gaddr)); \

- if (!__r && bswap_code(arm_sctlr_b(env))) { \

- (x) = bswap32(x); \

- } \

- __r; \

- })

-

-#define get_user_code_u16(x, gaddr, env) \

- ({ abi_long __r = get_user_u16((x), (gaddr)); \

- if (!__r && bswap_code(arm_sctlr_b(env))) { \

- (x) = bswap16(x); \

- } \

- __r; \

- })

-

-#define get_user_data_u32(x, gaddr, env) \

- ({ abi_long __r = get_user_u32((x), (gaddr)); \

- if (!__r && arm_cpu_bswap_data(env)) { \

- (x) = bswap32(x); \

- } \

- __r; \

- })

-

-#define get_user_data_u16(x, gaddr, env) \

- ({ abi_long __r = get_user_u16((x), (gaddr)); \

- if (!__r && arm_cpu_bswap_data(env)) { \

- (x) = bswap16(x); \

- } \

- __r; \

- })

-

-#define put_user_data_u32(x, gaddr, env) \

- ({ typeof(x) __x = (x); \

- if (arm_cpu_bswap_data(env)) { \

- __x = bswap32(__x); \

- } \

- put_user_u32(__x, (gaddr)); \

- })

-

-#define put_user_data_u16(x, gaddr, env) \

- ({ typeof(x) __x = (x); \

- if (arm_cpu_bswap_data(env)) { \

- __x = bswap16(__x); \

- } \

- put_user_u16(__x, (gaddr)); \

- })

-

/* AArch64 main loop */

void cpu_loop(CPUARMState *env)

{

--

2.43.0

In linux-user/arm/cpu_loop.c we define a full set of get/put

macros for both code and data (since the endianness handling

is different between the two). However the only one we actually

use is get_user_code_u32(). Remove the rest.

We leave a comment noting how data-side accesses should be handled

for big-endian, because that's a subtle point and we just removed the

macros that were effectively documenting it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

---

linux-user/arm/cpu_loop.c | 43 ++++-----------------------------------

1 file changed, 4 insertions(+), 39 deletions(-)

diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c

index XXXXXXX..XXXXXXX 100644

--- a/linux-user/arm/cpu_loop.c

+++ b/linux-user/arm/cpu_loop.c

@@ -XXX,XX +XXX,XX @@

__r; \

})

-#define get_user_code_u16(x, gaddr, env) \

- ({ abi_long __r = get_user_u16((x), (gaddr)); \

- if (!__r && bswap_code(arm_sctlr_b(env))) { \

- (x) = bswap16(x); \

- } \

- __r; \

- })

-

-#define get_user_data_u32(x, gaddr, env) \

- ({ abi_long __r = get_user_u32((x), (gaddr)); \

- if (!__r && arm_cpu_bswap_data(env)) { \

- (x) = bswap32(x); \

- } \

- __r; \

- })

-

-#define get_user_data_u16(x, gaddr, env) \

- ({ abi_long __r = get_user_u16((x), (gaddr)); \

- if (!__r && arm_cpu_bswap_data(env)) { \

- (x) = bswap16(x); \

- } \

- __r; \

- })

-

-#define put_user_data_u32(x, gaddr, env) \

- ({ typeof(x) __x = (x); \

- if (arm_cpu_bswap_data(env)) { \

- __x = bswap32(__x); \

- } \

- put_user_u32(__x, (gaddr)); \

- })

-

-#define put_user_data_u16(x, gaddr, env) \

- ({ typeof(x) __x = (x); \

- if (arm_cpu_bswap_data(env)) { \

- __x = bswap16(__x); \

- } \

- put_user_u16(__x, (gaddr)); \

- })

+/*

+ * Note that if we need to do data accesses here, they should do a

+ * bswap if arm_cpu_bswap_data() returns true.

+ */

/*

* Similar to code in accel/tcg/user-exec.c, but outside the execution loop.

--

2.43.0

The arm_cpu_data_is_big_endian() and related functions are now used

only in target/arm; they can be moved to internals.h.

The motivation here is that we would like to move arm_current_el()

to internals.h.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

---

target/arm/cpu.h | 48 ------------------------------------------

target/arm/internals.h | 48 ++++++++++++++++++++++++++++++++++++++++++

2 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpu.h

+++ b/target/arm/cpu.h

@@ -XXX,XX +XXX,XX @@ static inline bool arm_sctlr_b(CPUARMState *env)

uint64_t arm_sctlr(CPUARMState *env, int el);

-static inline bool arm_cpu_data_is_big_endian_a32(CPUARMState *env,

- bool sctlr_b)

-{

-#ifdef CONFIG_USER_ONLY

- /*

- * In system mode, BE32 is modelled in line with the

- * architecture (as word-invariant big-endianness), where loads

- * and stores are done little endian but from addresses which

- * are adjusted by XORing with the appropriate constant. So the

- * endianness to use for the raw data access is not affected by

- * SCTLR.B.

- * In user mode, however, we model BE32 as byte-invariant

- * big-endianness (because user-only code cannot tell the

- * difference), and so we need to use a data access endianness

- * that depends on SCTLR.B.

- */

- if (sctlr_b) {

- return true;

- }

-#endif

- /* In 32bit endianness is determined by looking at CPSR's E bit */

- return env->uncached_cpsr & CPSR_E;

-}

-

-static inline bool arm_cpu_data_is_big_endian_a64(int el, uint64_t sctlr)

-{

- return sctlr & (el ? SCTLR_EE : SCTLR_E0E);

-}

-

-/* Return true if the processor is in big-endian mode. */

-static inline bool arm_cpu_data_is_big_endian(CPUARMState *env)

-{

- if (!is_a64(env)) {

- return arm_cpu_data_is_big_endian_a32(env, arm_sctlr_b(env));

- } else {

- int cur_el = arm_current_el(env);

- uint64_t sctlr = arm_sctlr(env, cur_el);

- return arm_cpu_data_is_big_endian_a64(cur_el, sctlr);

- }

-}

-

#include "exec/cpu-all.h"

/*

@@ -XXX,XX +XXX,XX @@ static inline bool bswap_code(bool sctlr_b)

#endif

}

-#ifdef CONFIG_USER_ONLY

-static inline bool arm_cpu_bswap_data(CPUARMState *env)

-{

- return TARGET_BIG_ENDIAN ^ arm_cpu_data_is_big_endian(env);

-}

-#endif

-

void cpu_get_tb_cpu_state(CPUARMState *env, vaddr *pc,

uint64_t *cs_base, uint32_t *flags);

diff --git a/target/arm/internals.h b/target/arm/internals.h

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/internals.h

+++ b/target/arm/internals.h

@@ -XXX,XX +XXX,XX @@ static inline FloatRoundMode arm_rmode_to_sf(ARMFPRounding rmode)

return arm_rmode_to_sf_map[rmode];

}

+static inline bool arm_cpu_data_is_big_endian_a32(CPUARMState *env,

+ bool sctlr_b)

+{

+#ifdef CONFIG_USER_ONLY

+ /*

+ * In system mode, BE32 is modelled in line with the

+ * architecture (as word-invariant big-endianness), where loads

+ * and stores are done little endian but from addresses which

+ * are adjusted by XORing with the appropriate constant. So the

+ * endianness to use for the raw data access is not affected by

+ * SCTLR.B.

+ * In user mode, however, we model BE32 as byte-invariant

+ * big-endianness (because user-only code cannot tell the

+ * difference), and so we need to use a data access endianness

+ * that depends on SCTLR.B.

+ */

+ if (sctlr_b) {

+ return true;

+ }

+#endif

+ /* In 32bit endianness is determined by looking at CPSR's E bit */

+ return env->uncached_cpsr & CPSR_E;

+}

+

+static inline bool arm_cpu_data_is_big_endian_a64(int el, uint64_t sctlr)

+{

+ return sctlr & (el ? SCTLR_EE : SCTLR_E0E);

+}

+

+/* Return true if the processor is in big-endian mode. */

+static inline bool arm_cpu_data_is_big_endian(CPUARMState *env)

+{

+ if (!is_a64(env)) {

+ return arm_cpu_data_is_big_endian_a32(env, arm_sctlr_b(env));

+ } else {

+ int cur_el = arm_current_el(env);

+ uint64_t sctlr = arm_sctlr(env, cur_el);

+ return arm_cpu_data_is_big_endian_a64(cur_el, sctlr);

+ }

+}

+

+#ifdef CONFIG_USER_ONLY

+static inline bool arm_cpu_bswap_data(CPUARMState *env)

+{

+ return TARGET_BIG_ENDIAN ^ arm_cpu_data_is_big_endian(env);

+}

+#endif

+

static inline void aarch64_save_sp(CPUARMState *env, int el)

{

if (env->pstate & PSTATE_SP) {

--

2.43.0

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.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@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(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpu.h

+++ b/target/arm/cpu.h

@@ -XXX,XX +XXX,XX @@ uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, ARMSecuritySpace space);

uint64_t arm_hcr_el2_eff(CPUARMState *env);

uint64_t arm_hcrx_el2_eff(CPUARMState *env);

-/* 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;

-}

-

/*

* Function for determining whether guest cp register reads and writes should

* access the secure or non-secure bank of a cp register. When EL3 is

@@ -XXX,XX +XXX,XX @@ static inline bool arm_v7m_is_handler_mode(CPUARMState *env)

return env->v7m.exception != 0;

}

-/* Return the current Exception Level (as per ARMv8; note that this differs

- * from the ARMv7 Privilege Level).

- */

-static inline int arm_current_el(CPUARMState *env)

-{

- if (arm_feature(env, ARM_FEATURE_M)) {

- return arm_v7m_is_handler_mode(env) ||

- !(env->v7m.control[env->v7m.secure] & 1);

- }

-

- if (is_a64(env)) {

- return extract32(env->pstate, 2, 2);

- }

-

- switch (env->uncached_cpsr & 0x1f) {

- case ARM_CPU_MODE_USR:

- return 0;

- case ARM_CPU_MODE_HYP:

- return 2;

- case ARM_CPU_MODE_MON:

- return 3;

- default:

- if (arm_is_secure(env) && !arm_el_is_aa64(env, 3)) {

- /* If EL3 is 32-bit then all secure privileged modes run in

- * EL3

- */

- return 3;

- }

-

- return 1;

- }

-}

-

/**

* write_list_to_cpustate

* @cpu: ARMCPU

diff --git a/target/arm/internals.h b/target/arm/internals.h

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/internals.h

+++ b/target/arm/internals.h

@@ -XXX,XX +XXX,XX @@ static inline FloatRoundMode arm_rmode_to_sf(ARMFPRounding rmode)

return arm_rmode_to_sf_map[rmode];

}

+/* 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;

+}

+

+/*

+ * Return the current Exception Level (as per ARMv8; note that this differs

+ * from the ARMv7 Privilege Level).

+ */

+static inline int arm_current_el(CPUARMState *env)

+{

+ if (arm_feature(env, ARM_FEATURE_M)) {

+ return arm_v7m_is_handler_mode(env) ||

+ !(env->v7m.control[env->v7m.secure] & 1);

+ }

+

+ if (is_a64(env)) {

+ return extract32(env->pstate, 2, 2);

+ }

+

+ switch (env->uncached_cpsr & 0x1f) {

+ case ARM_CPU_MODE_USR:

+ return 0;

+ case ARM_CPU_MODE_HYP:

+ return 2;

+ case ARM_CPU_MODE_MON:

+ return 3;

+ default:

+ if (arm_is_secure(env) && !arm_el_is_aa64(env, 3)) {

+ /* If EL3 is 32-bit then all secure privileged modes run in EL3 */

+ return 3;

+ }

+

+ return 1;

+ }

+}

+

static inline bool arm_cpu_data_is_big_endian_a32(CPUARMState *env,

bool sctlr_b)

{

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/arm_gicv3_cpuif.c

+++ b/hw/intc/arm_gicv3_cpuif.c

@@ -XXX,XX +XXX,XX @@

#include "cpu.h"

#include "target/arm/cpregs.h"

#include "target/arm/cpu-features.h"

+#include "target/arm/internals.h"

#include "system/tcg.h"

#include "system/qtest.h"

diff --git a/target/arm/arch_dump.c b/target/arm/arch_dump.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/arch_dump.c

+++ b/target/arm/arch_dump.c

@@ -XXX,XX +XXX,XX @@

#include "elf.h"

#include "system/dump.h"

#include "cpu-features.h"

+#include "internals.h"

/* struct user_pt_regs from arch/arm64/include/uapi/asm/ptrace.h */

struct aarch64_user_regs {

--

2.43.0

The definition of SCR_EL3.RW says that its effective value is 1 if:

- EL2 is implemented and does not support AArch32, and SCR_EL3.NS is 1

- the effective value of SCR_EL3.{EEL2,NS} is {1,0} (i.e. we are

Secure and Secure EL2 is disabled)

We implement the second of these in arm_el_is_aa64(), but forgot the

first.

Provide a new function arm_scr_rw_eff() to return the effective

value of SCR_EL3.RW, and use it in arm_el_is_aa64() and the other

places that currently look directly at the bit value.

(scr_write() enforces that the RW bit is RAO/WI if neither EL1 nor

EL2 have AArch32 support, but if EL1 does but EL2 does not then the

bit must still be writeable.)

This will mean that if code at EL3 attempts to perform an exception

return to AArch32 EL2 when EL2 is AArch64-only we will correctly

handle this as an illegal exception return: it will be caught by the

"return to an EL which is configured for a different register width"

check in HELPER(exception_return).

We do already have some CPU types which don't implement AArch32

above EL0, so this is technically a bug; it doesn't seem worth

backporting to stable because no sensible guest code will be

deliberately attempting to set the RW bit to a value corresponding

to an unimplemented execution state and then checking that we

did the right thing.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

---

target/arm/internals.h | 26 +++++++++++++++++++++++---

target/arm/helper.c | 4 ++--

2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/internals.h

+++ b/target/arm/internals.h

@@ -XXX,XX +XXX,XX @@ static inline FloatRoundMode arm_rmode_to_sf(ARMFPRounding rmode)

return arm_rmode_to_sf_map[rmode];

}

+/* Return the effective value of SCR_EL3.RW */

+static inline bool arm_scr_rw_eff(CPUARMState *env)

+{

+ /*

+ * SCR_EL3.RW has an effective value of 1 if:

+ * - we are NS and EL2 is implemented but doesn't support AArch32

+ * - we are S and EL2 is enabled (in which case it must be AArch64)

+ */

+ ARMCPU *cpu = env_archcpu(env);

+

+ if (env->cp15.scr_el3 & SCR_RW) {

+ return true;

+ }

+ if (env->cp15.scr_el3 & SCR_NS) {

+ return arm_feature(env, ARM_FEATURE_EL2) &&

+ !cpu_isar_feature(aa64_aa32_el2, cpu);

+ } else {

+ return env->cp15.scr_el3 & SCR_EEL2;

+ }

+}

+

/* Return true if the specified exception level is running in AArch64 state. */

static inline bool arm_el_is_aa64(CPUARMState *env, int el)

{

@@ -XXX,XX +XXX,XX @@ static inline bool arm_el_is_aa64(CPUARMState *env, int el)

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 (arm_feature(env, ARM_FEATURE_EL3)) {

+ aa64 = aa64 && arm_scr_rw_eff(env);

}

if (el == 2) {

diff --git a/target/arm/helper.c b/target/arm/helper.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,

uint64_t hcr_el2;

if (arm_feature(env, ARM_FEATURE_EL3)) {

- rw = ((env->cp15.scr_el3 & SCR_RW) == SCR_RW);

+ rw = arm_scr_rw_eff(env);

} else {

/*

* Either EL2 is the highest EL (and so the EL2 register width

@@ -XXX,XX +XXX,XX @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)

switch (new_el) {

case 3:

- is_aa64 = (env->cp15.scr_el3 & SCR_RW) != 0;

+ is_aa64 = arm_scr_rw_eff(env);

break;

case 2:

hcr = arm_hcr_el2_eff(env);

--

2.43.0

When EL1 doesn't support AArch32, the HCR_EL2.RW bit is supposed to

be RAO/WI. Enforce the RAO/WI behaviour.

Note that we handle "reset value should honour RES1 bits" in the same

way that SCR_EL3 does, via a reset function.

We do already have some CPU types which don't implement AArch32

above EL0, so this is technically a bug; it doesn't seem worth

backporting to stable because no sensible guest code will be

deliberately attempting to set the RW bit to a value corresponding

to an unimplemented execution state and then checking that we

did the right thing.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

---

target/arm/helper.c | 12 ++++++++++++

1 file changed, 12 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ static void do_hcr_write(CPUARMState *env, uint64_t value, uint64_t valid_mask)

/* Clear RES0 bits. */

value &= valid_mask;

+ /* RW is RAO/WI if EL1 is AArch64 only */

+ if (!cpu_isar_feature(aa64_aa32_el1, cpu)) {

+ value |= HCR_RW;

+ }

+

/*

* These bits change the MMU setup:

* HCR_VM enables stage 2 translation

@@ -XXX,XX +XXX,XX @@ static void hcr_writelow(CPUARMState *env, const ARMCPRegInfo *ri,

do_hcr_write(env, value, MAKE_64BIT_MASK(32, 32));

}

+static void hcr_reset(CPUARMState *env, const ARMCPRegInfo *ri)

+{

+ /* hcr_write will set the RES1 bits on an AArch64-only CPU */

+ hcr_write(env, ri, 0);

+}

+

/*

* Return the effective value of HCR_EL2, at the given security state.

* Bits that are not included here:

@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el2_cp_reginfo[] = {

.opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,

.access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.hcr_el2),

.nv2_redirect_offset = 0x78,

+ .resetfn = hcr_reset,

.writefn = hcr_write, .raw_writefn = raw_write },

{ .name = "HCR", .state = ARM_CP_STATE_AA32,

.type = ARM_CP_ALIAS | ARM_CP_IO,

--

2.43.0

We already call env_archcpu() multiple times within the

exception_return helper function, and we're about to want to

add another use of the ARMCPU pointer. Add a local variable

cpu so we can call env_archcpu() just once.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

---

target/arm/tcg/helper-a64.c | 7 ++++---

1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/tcg/helper-a64.c

+++ b/target/arm/tcg/helper-a64.c

@@ -XXX,XX +XXX,XX @@ static void cpsr_write_from_spsr_elx(CPUARMState *env,

void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)

{

+ ARMCPU *cpu = env_archcpu(env);

int cur_el = arm_current_el(env);

unsigned int spsr_idx = aarch64_banked_spsr_index(cur_el);

uint32_t spsr = env->banked_spsr[spsr_idx];

@@ -XXX,XX +XXX,XX @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)

}

bql_lock();

- arm_call_pre_el_change_hook(env_archcpu(env));

+ arm_call_pre_el_change_hook(cpu);

bql_unlock();

if (!return_to_aa64) {

@@ -XXX,XX +XXX,XX @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)

int tbii;

env->aarch64 = true;

- spsr &= aarch64_pstate_valid_mask(&env_archcpu(env)->isar);

+ spsr &= aarch64_pstate_valid_mask(&cpu->isar);

pstate_write(env, spsr);

if (!arm_singlestep_active(env)) {

env->pstate &= ~PSTATE_SS;

@@ -XXX,XX +XXX,XX @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)

aarch64_sve_change_el(env, cur_el, new_el, return_to_aa64);

bql_lock();

- arm_call_el_change_hook(env_archcpu(env));

+ arm_call_el_change_hook(cpu);

bql_unlock();

return;

--

2.43.0

In the Arm ARM, rule R_TYTWB states that returning to AArch32

is an illegal exception return if:

* AArch32 is not supported at any exception level

* the target EL is configured for AArch64 via SCR_EL3.RW

or HCR_EL2.RW or via CPU state at reset

We check the second of these, but not the first (which can only be

relevant for the case of a return to EL0, because if AArch32 is not

supported at one of the higher ELs then the RW bits will have an

effective value of 1 and the the "configured for AArch64" condition

will hold also).

Add the missing condition. Although this is technically a bug

(because we have one AArch64-only CPU: a64fx) it isn't worth

backporting to stable because no sensible guest code will

deliberately try to return to a nonexistent execution state

to check that it gets an illegal exception return.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

---

target/arm/tcg/helper-a64.c | 5 +++++

1 file changed, 5 insertions(+)

diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/tcg/helper-a64.c

+++ b/target/arm/tcg/helper-a64.c

@@ -XXX,XX +XXX,XX @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)

goto illegal_return;

}

+ if (!return_to_aa64 && !cpu_isar_feature(aa64_aa32, cpu)) {

+ /* Return to AArch32 when CPU is AArch64-only */

+ goto illegal_return;

+ }

+

if (new_el == 1 && (arm_hcr_el2_eff(env) & HCR_TGE)) {

goto illegal_return;

}

--

2.43.0

I'm down as the only listed maintainer for quite a lot of Arm SoC and

board types. In some cases this is only as the "maintainer of last

resort" and I'm not in practice doing anything beyond patch review

and the odd bit of tidyup.

Move these entries in MAINTAINERS from "Maintained" to "Odd Fixes",

to better represent reality. Entries for other boards and SoCs where

I do more actively care (or where there is a listed co-maintainer)

remain as they are.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Message-id: 20250307152838.3226398-1-peter.maydell@linaro.org

---

MAINTAINERS | 14 +++++++-------

1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS

index XXXXXXX..XXXXXXX 100644

--- a/MAINTAINERS

+++ b/MAINTAINERS

@@ -XXX,XX +XXX,XX @@ F: docs/system/arm/kzm.rst

Integrator CP

M: Peter Maydell <peter.maydell@linaro.org>

L: qemu-arm@nongnu.org

-S: Maintained

+S: Odd Fixes

F: hw/arm/integratorcp.c

F: hw/misc/arm_integrator_debug.c

F: include/hw/misc/arm_integrator_debug.h

@@ -XXX,XX +XXX,XX @@ F: docs/system/arm/mps2.rst

Musca

M: Peter Maydell <peter.maydell@linaro.org>

L: qemu-arm@nongnu.org

-S: Maintained

+S: Odd Fixes

F: hw/arm/musca.c

F: docs/system/arm/musca.rst

@@ -XXX,XX +XXX,XX @@ F: tests/functional/test_aarch64_raspi4.py

Real View

M: Peter Maydell <peter.maydell@linaro.org>

L: qemu-arm@nongnu.org

-S: Maintained

+S: Odd Fixes

F: hw/arm/realview*

F: hw/cpu/realview_mpcore.c

F: hw/intc/realview_gic.c

@@ -XXX,XX +XXX,XX @@ F: tests/functional/test_arm_collie.py

Stellaris

M: Peter Maydell <peter.maydell@linaro.org>

L: qemu-arm@nongnu.org

-S: Maintained

+S: Odd Fixes

F: hw/*/stellaris*

F: hw/display/ssd03*

F: include/hw/input/gamepad.h

@@ -XXX,XX +XXX,XX @@ F: docs/system/arm/stm32.rst

Versatile Express

M: Peter Maydell <peter.maydell@linaro.org>

L: qemu-arm@nongnu.org

-S: Maintained

+S: Odd Fixes

F: hw/arm/vexpress.c

F: hw/display/sii9022.c

F: docs/system/arm/vexpress.rst

@@ -XXX,XX +XXX,XX @@ F: tests/functional/test_arm_vexpress.py

Versatile PB

M: Peter Maydell <peter.maydell@linaro.org>

L: qemu-arm@nongnu.org

-S: Maintained

+S: Odd Fixes

F: hw/*/versatile*

F: hw/i2c/arm_sbcon_i2c.c

F: include/hw/i2c/arm_sbcon_i2c.h

@@ -XXX,XX +XXX,XX @@ F: include/hw/hyperv/vmbus*.h

OMAP

M: Peter Maydell <peter.maydell@linaro.org>

L: qemu-arm@nongnu.org

-S: Maintained

+S: Odd Fixes

F: hw/*/omap*

F: include/hw/arm/omap.h

F: docs/system/arm/sx1.rst

--

2.43.0

From: Paolo Bonzini <pbonzini@redhat.com>

The guest does not control whether characters are sent on the UART.

Sending them before the guest happens to boot will now result in a

"guest error" log entry that is only because of timing, even if the

guest _would_ later setup the receiver correctly.

This reverts the bulk of commit abf2b6a028670bd2890bb3aee7e103fe53e4b0df,

and instead adds a comment about why we don't check the enable bits.

Cc: Philippe Mathieu-Daudé <philmd@linaro.org>

Cc: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20250311153717.206129-1-pbonzini@redhat.com

[PMM: expanded comment]

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

hw/char/pl011.c | 19 ++++++++++---------

1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/hw/char/pl011.c b/hw/char/pl011.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/char/pl011.c

+++ b/hw/char/pl011.c

@@ -XXX,XX +XXX,XX @@ static int pl011_can_receive(void *opaque)

unsigned fifo_depth = pl011_get_fifo_depth(s);

unsigned fifo_available = fifo_depth - s->read_count;

- if (!(s->cr & CR_UARTEN)) {

- qemu_log_mask(LOG_GUEST_ERROR,

- "PL011 receiving data on disabled UART\n");

- }

- if (!(s->cr & CR_RXE)) {

- qemu_log_mask(LOG_GUEST_ERROR,

- "PL011 receiving data on disabled RX UART\n");

- }

- trace_pl011_can_receive(s->lcr, s->read_count, fifo_depth, fifo_available);

+ /*

+ * In theory we should check the UART and RX enable bits here and

+ * return 0 if they are not set (so the guest can't receive data

+ * until you have enabled the UART). In practice we suspect there

+ * is at least some guest code out there which has been tested only

+ * on QEMU and which never bothers to enable the UART because we

+ * historically never enforced that. So we effectively keep the

+ * UART continuously enabled regardless of the enable bits.

+ */

+ trace_pl011_can_receive(s->lcr, s->read_count, fifo_depth, fifo_available);

return fifo_available;

}

--

2.43.0

From: Joe Komlodi <komlodi@google.com>

On ARM hosts with CTR_EL0.DIC and CTR_EL0.IDC set, this would only cause

an ISB to be executed during cache maintenance, which could lead to QEMU

executing TBs containing garbage instructions.

This seems to be because the ISB finishes executing instructions and

flushes the pipeline, but the ISB doesn't guarantee that writes from the

executed instructions are committed. If a small enough TB is created, it's

possible that the writes setting up the TB aren't committed by the time the

TB is executed.

This function is intended to be a port of the gcc implementation

(https://github.com/gcc-mirror/gcc/blob/85b46d0795ac76bc192cb8f88b646a647acf98c1/libgcc/config/aarch64/sync-cache.c#L67)

which makes the first DSB unconditional, so we can fix the synchronization

issue by doing that as well.

Cc: qemu-stable@nongnu.org

Fixes: 664a79735e4deb1 ("util: Specialize flush_idcache_range for aarch64")

Signed-off-by: Joe Komlodi <komlodi@google.com>

Message-id: 20250310203622.1827940-2-komlodi@google.com

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

util/cacheflush.c | 4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/util/cacheflush.c b/util/cacheflush.c

index XXXXXXX..XXXXXXX 100644

--- a/util/cacheflush.c

+++ b/util/cacheflush.c

@@ -XXX,XX +XXX,XX @@ void flush_idcache_range(uintptr_t rx, uintptr_t rw, size_t len)

for (p = rw & -dcache_lsize; p < rw + len; p += dcache_lsize) {

asm volatile("dc\tcvau, %0" : : "r" (p) : "memory");

}

- asm volatile("dsb\tish" : : : "memory");

}

+ /* DSB unconditionally to ensure any outstanding writes are committed. */

+ asm volatile("dsb\tish" : : : "memory");

+

/*

* If CTR_EL0.DIC is enabled, Instruction cache cleaning to the Point

* of Unification is not required for instruction to data coherence.

--

2.43.0

From: Richard Henderson <richard.henderson@linaro.org>

The check for fp_excp_el in assert_fp_access_checked is

incorrect. For SME, with StreamingMode enabled, the access

is really against the streaming mode vectors, and access

to the normal fp registers is allowed to be disabled.

C.f. sme_enabled_check.

Convert sve_access_checked to match, even though we don't

currently check the exception state.

Cc: qemu-stable@nongnu.org

Fixes: 3d74825f4d6 ("target/arm: Add SME enablement checks")

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20250307190415.982049-2-richard.henderson@linaro.org

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

target/arm/tcg/translate-a64.h | 2 +-

target/arm/tcg/translate.h | 10 +++++++---

target/arm/tcg/translate-a64.c | 17 +++++++++--------

3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/target/arm/tcg/translate-a64.h b/target/arm/tcg/translate-a64.h

index XXXXXXX..XXXXXXX 100644