* [PATCH v2 0/1] target/arm: Fix SCR_EL3 migration issue @ 2021-02-03 16:55 michael.nawrocki--- via 2021-02-03 16:55 ` [PATCH v2 1/1] target/arm: Fix SCR RES1 handling michael.nawrocki--- via 0 siblings, 1 reply; 4+ messages in thread From: michael.nawrocki--- via @ 2021-02-03 16:55 UTC (permalink / raw) To: qemu-arm; +Cc: peter.maydell, qemu-devel, Mike Nawrocki The SCR_EL3 register reset value (0) and the value produced when writing 0 via the scr_write function (set as writefn in the register struct) differ. This causes migration to fail. Ultimately, this is due to incorrect handling of context-dependent behavior of the RES1 bits of SCR_EL3. The FW and AW bits should be forced to 1 only if there is no support for AArch32 at EL1 or above. This patch improves the scr_write RES1 bit handling and adds a reset function which will initialize SCR_EL3 to 0x30 on AArch64-only CPUs, and 0 if AArch32 is supported at EL1 or above. Failing invocation: $ qemu-system-arm -machine vexpress-a9 -cpu cortex-a9 -nographic QEMU 5.2.0 monitor - type 'help' for more information (qemu) migrate "exec:cat > img" (qemu) q $ qemu-system-arm -machine vexpress-a9 -cpu cortex-a9 -nographic -incoming "exec:cat img" qemu-system-arm: error while loading state for instance 0x0 of device 'cpu' qemu-system-arm: load of migration failed: Operation not permitted Mike Nawrocki (1): target/arm: Fix SCR RES1 handling target/arm/cpu.h | 5 +++++ target/arm/helper.c | 16 ++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/1] target/arm: Fix SCR RES1 handling 2021-02-03 16:55 [PATCH v2 0/1] target/arm: Fix SCR_EL3 migration issue michael.nawrocki--- via @ 2021-02-03 16:55 ` michael.nawrocki--- via 2021-02-03 18:28 ` Richard Henderson 2021-02-08 16:41 ` Peter Maydell 0 siblings, 2 replies; 4+ messages in thread From: michael.nawrocki--- via @ 2021-02-03 16:55 UTC (permalink / raw) To: qemu-arm; +Cc: peter.maydell, qemu-devel, Mike Nawrocki The FW and AW bits of SCR_EL3 are RES1 only in some contexts. Force them to 1 only when there is no support for AArch32 at EL1 or above. The reset value will be 0x30 only if the CPU is AArch64-only; if there is support for AArch32 at EL1 or above, it will be reset to 0. Also adds helper function isar_feature_aa64_aa32_el1 to check if AArch32 is supported at EL1 or above. Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu> --- target/arm/cpu.h | 5 +++++ target/arm/helper.c | 16 ++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index d080239863..39633f73f3 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -4033,6 +4033,11 @@ static inline bool isar_feature_aa64_aa32(const ARMISARegisters *id) return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL0) >= 2; } +static inline bool isar_feature_aa64_aa32_el1(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL1) >= 2; +} + static inline bool isar_feature_aa64_sve(const ARMISARegisters *id) { return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, SVE) != 0; diff --git a/target/arm/helper.c b/target/arm/helper.c index 47e266d7e6..e529cdbfd0 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -2024,7 +2024,10 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) ARMCPU *cpu = env_archcpu(env); if (ri->state == ARM_CP_STATE_AA64) { - value |= SCR_FW | SCR_AW; /* these two bits are RES1. */ + if (arm_feature(env, ARM_FEATURE_AARCH64) && + !cpu_isar_feature(aa64_aa32_el1, cpu)) { + value |= SCR_FW | SCR_AW; /* these two bits are RES1. */ + } valid_mask &= ~SCR_NET; if (cpu_isar_feature(aa64_lor, cpu)) { @@ -2063,6 +2066,15 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) raw_write(env, ri, value); } +static void scr_reset(CPUARMState *env, const ARMCPRegInfo *ri) +{ + /* + * scr_write will set the RES1 bits on an AArch64-only CPU. + * The reset value will be 0x30 on an AArch64-only CPU and 0 otherwise. + */ + scr_write(env, ri, 0); +} + static CPAccessResult access_aa64_tid2(CPUARMState *env, const ARMCPRegInfo *ri, bool isread) @@ -5785,7 +5797,7 @@ static const ARMCPRegInfo el3_cp_reginfo[] = { { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0, .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3), - .resetvalue = 0, .writefn = scr_write }, + .resetfn = scr_reset, .writefn = scr_write }, { .name = "SCR", .type = ARM_CP_ALIAS | ARM_CP_NEWEL, .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0, .access = PL1_RW, .accessfn = access_trap_aa32s_el1, -- 2.20.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] target/arm: Fix SCR RES1 handling 2021-02-03 16:55 ` [PATCH v2 1/1] target/arm: Fix SCR RES1 handling michael.nawrocki--- via @ 2021-02-03 18:28 ` Richard Henderson 2021-02-08 16:41 ` Peter Maydell 1 sibling, 0 replies; 4+ messages in thread From: Richard Henderson @ 2021-02-03 18:28 UTC (permalink / raw) To: Mike Nawrocki, qemu-arm; +Cc: peter.maydell, qemu-devel On 2/3/21 6:55 AM, michael.nawrocki--- via wrote: > The FW and AW bits of SCR_EL3 are RES1 only in some contexts. Force them > to 1 only when there is no support for AArch32 at EL1 or above. > > The reset value will be 0x30 only if the CPU is AArch64-only; if there > is support for AArch32 at EL1 or above, it will be reset to 0. > > Also adds helper function isar_feature_aa64_aa32_el1 to check if AArch32 > is supported at EL1 or above. > > Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu> > --- > target/arm/cpu.h | 5 +++++ > target/arm/helper.c | 16 ++++++++++++++-- > 2 files changed, 19 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] target/arm: Fix SCR RES1 handling 2021-02-03 16:55 ` [PATCH v2 1/1] target/arm: Fix SCR RES1 handling michael.nawrocki--- via 2021-02-03 18:28 ` Richard Henderson @ 2021-02-08 16:41 ` Peter Maydell 1 sibling, 0 replies; 4+ messages in thread From: Peter Maydell @ 2021-02-08 16:41 UTC (permalink / raw) To: Mike Nawrocki; +Cc: qemu-arm, QEMU Developers On Wed, 3 Feb 2021 at 16:56, Mike Nawrocki <michael.nawrocki@gtri.gatech.edu> wrote: > > The FW and AW bits of SCR_EL3 are RES1 only in some contexts. Force them > to 1 only when there is no support for AArch32 at EL1 or above. > > The reset value will be 0x30 only if the CPU is AArch64-only; if there > is support for AArch32 at EL1 or above, it will be reset to 0. > > Also adds helper function isar_feature_aa64_aa32_el1 to check if AArch32 > is supported at EL1 or above. > > Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu> > --- > target/arm/cpu.h | 5 +++++ > target/arm/helper.c | 16 ++++++++++++++-- > 2 files changed, 19 insertions(+), 2 deletions(-) Applied to target-arm.next, thanks. -- PMM ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-08 21:49 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-02-03 16:55 [PATCH v2 0/1] target/arm: Fix SCR_EL3 migration issue michael.nawrocki--- via 2021-02-03 16:55 ` [PATCH v2 1/1] target/arm: Fix SCR RES1 handling michael.nawrocki--- via 2021-02-03 18:28 ` Richard Henderson 2021-02-08 16:41 ` Peter Maydell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).