[PATCH] target/arm: Make WFI a NOP for userspace emulators

[PATCH] target/arm: Make WFI a NOP for userspace emulators

[Peter Maydell]

The WFI insn is not system-mode only, though it doesn't usually make
a huge amount of sense for userspace code to execute it.  Currently
if you try it in qemu-arm then the helper function will raise an
EXCP_HLT exception, which is not covered by the switch in cpu_loop()
and results in an abort:

qemu: unhandled CPU exception 0x10001 - aborting
R00=00000001 R01=408003e4 R02=408003ec R03=000102ec
R04=00010a28 R05=00010158 R06=00087460 R07=00010158
R08=00000000 R09=00000000 R10=00085b7c R11=408002a4
R12=408002b8 R13=408002a0 R14=0001057c R15=000102f8
PSR=60000010 -ZC- A usr32
qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x7fcbfa4f0a12

Make the WFI helper function return immediately in the usermode
emulator. This turns WFI into a NOP, which is OK because:
 * architecturally "WFI is a NOP" is a permitted implementation
 * aarch64 Linux kernels use the SCTLR_EL1.nTWI bit to trap
   userspace WFI and NOP it (though aarch32 kernels currently
   just let WFI do whatever it would do)

We could in theory make the translate.c code special case user-mode
emulation and NOP the insn entirely rather than making the helper
do nothing, but because no real world code will be trying to
execute WFI we don't care about efficiency and the helper provides
a single place where we can make the change rather than having
to touch multiple places in translate.c and translate-a64.c.

Fixes: https://bugs.launchpad.net/qemu/+bug/1926759
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/op_helper.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 65cb37d088f..1a475aa51eb 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -286,6 +286,17 @@ static inline int check_wfx_trap(CPUARMState *env, bool is_wfe)
 
 void HELPER(wfi)(CPUARMState *env, uint32_t insn_len)
 {
+#ifdef CONFIG_USER_ONLY
+    /*
+     * WFI in the user-mode emulator is technically permitted but not
+     * something any real-world code would do. AArch64 Linux kernels
+     * trap it via SCTRL_EL1.nTWI and make it an (expensive) NOP;
+     * AArch32 kernels don't trap it so it will delay a bit.
+     * For QEMU, make it NOP here, because trying to raise EXCP_HLT
+     * would trigger an abort.
+     */
+    return;
+#else
     CPUState *cs = env_cpu(env);
     int target_el = check_wfx_trap(env, false);
 
@@ -310,6 +321,7 @@ void HELPER(wfi)(CPUARMState *env, uint32_t insn_len)
     cs->exception_index = EXCP_HLT;
     cs->halted = 1;
     cpu_loop_exit(cs);
+#endif
 }
 
 void HELPER(wfe)(CPUARMState *env)
-- 
2.20.1

Re: [PATCH] target/arm: Make WFI a NOP for userspace emulators

[Richard Henderson]

On 4/30/21 9:22 AM, Peter Maydell wrote:
> The WFI insn is not system-mode only, though it doesn't usually make
> a huge amount of sense for userspace code to execute it.  Currently
> if you try it in qemu-arm then the helper function will raise an
> EXCP_HLT exception, which is not covered by the switch in cpu_loop()
> and results in an abort:
> 
> qemu: unhandled CPU exception 0x10001 - aborting
> R00=00000001 R01=408003e4 R02=408003ec R03=000102ec
> R04=00010a28 R05=00010158 R06=00087460 R07=00010158
> R08=00000000 R09=00000000 R10=00085b7c R11=408002a4
> R12=408002b8 R13=408002a0 R14=0001057c R15=000102f8
> PSR=60000010 -ZC- A usr32
> qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x7fcbfa4f0a12
> 
> Make the WFI helper function return immediately in the usermode
> emulator. This turns WFI into a NOP, which is OK because:
>   * architecturally "WFI is a NOP" is a permitted implementation
>   * aarch64 Linux kernels use the SCTLR_EL1.nTWI bit to trap
>     userspace WFI and NOP it (though aarch32 kernels currently
>     just let WFI do whatever it would do)
> 
> We could in theory make the translate.c code special case user-mode
> emulation and NOP the insn entirely rather than making the helper
> do nothing, but because no real world code will be trying to
> execute WFI we don't care about efficiency and the helper provides
> a single place where we can make the change rather than having
> to touch multiple places in translate.c and translate-a64.c.
> 
> Fixes:https://bugs.launchpad.net/qemu/+bug/1926759
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/op_helper.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)

You could also ifdef this out in translate, in tb_stop for  DISAS_WFI. But 
either way,

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


r~

Re: [PATCH] target/arm: Make WFI a NOP for userspace emulators

[Peter Maydell]

On Fri, 30 Apr 2021 at 18:18, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/30/21 9:22 AM, Peter Maydell wrote:
> > The WFI insn is not system-mode only, though it doesn't usually make
> > a huge amount of sense for userspace code to execute it.  Currently
> > if you try it in qemu-arm then the helper function will raise an
> > EXCP_HLT exception, which is not covered by the switch in cpu_loop()
> > and results in an abort:
> >
> > qemu: unhandled CPU exception 0x10001 - aborting
> > R00=00000001 R01=408003e4 R02=408003ec R03=000102ec
> > R04=00010a28 R05=00010158 R06=00087460 R07=00010158
> > R08=00000000 R09=00000000 R10=00085b7c R11=408002a4
> > R12=408002b8 R13=408002a0 R14=0001057c R15=000102f8
> > PSR=60000010 -ZC- A usr32
> > qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x7fcbfa4f0a12
> >
> > Make the WFI helper function return immediately in the usermode
> > emulator. This turns WFI into a NOP, which is OK because:
> >   * architecturally "WFI is a NOP" is a permitted implementation
> >   * aarch64 Linux kernels use the SCTLR_EL1.nTWI bit to trap
> >     userspace WFI and NOP it (though aarch32 kernels currently
> >     just let WFI do whatever it would do)
> >
> > We could in theory make the translate.c code special case user-mode
> > emulation and NOP the insn entirely rather than making the helper
> > do nothing, but because no real world code will be trying to
> > execute WFI we don't care about efficiency and the helper provides
> > a single place where we can make the change rather than having
> > to touch multiple places in translate.c and translate-a64.c.
> >
> > Fixes:https://bugs.launchpad.net/qemu/+bug/1926759
> > Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> > ---
> >   target/arm/op_helper.c | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
>
> You could also ifdef this out in translate, in tb_stop for  DISAS_WFI.

You'd need to do it in both translate.c and translate-a64.c if
you did it there, though.

thanks
-- PMM