[PATCH 0/4] arm fixes (WxFT, bcm2835_fb)

[PATCH 0/4] arm fixes (WxFT, bcm2835_fb)

[Alex Bennée]

These are the arm specific fixes from my last series. All reviewed and
ready to pull.

Alex.

Alex Bennée (4):
  target/arm: hold bql when SEV sets the event_register
  target/arm: don't pass env to the sev helper
  target/arm: simplify wfxt_timer_cb to only trigger cpu_interrupt
  target/arm: fix bcm2835 migration issue

 target/arm/tcg/helper-defs.h   |  2 +-
 hw/display/bcm2835_fb.c        | 11 +++++++++++
 system/cpus.c                  |  2 +-
 target/arm/cpu.c               | 20 +++-----------------
 target/arm/tcg/op_helper.c     |  7 +++++--
 target/arm/tcg/translate-a64.c |  2 +-
 target/arm/tcg/translate.c     |  2 +-
 7 files changed, 23 insertions(+), 23 deletions(-)

-- 
2.47.3

[PATCH 1/4] target/arm: hold bql when SEV sets the event_register

[Alex Bennée]

There is a race in qemu_process_cpu_events and sleeping on halt_cond
where we can miss the setting of the event_register which would cause
arm_cpu_has_work to report we shouldn't sleep. As by definition the
bql has to be held when we go to sleep against halt_cond guard the
kicking of the cpus by the SEV helper with the BQL to ensure things
are properly serialised.

The other places that set event_register are fine as they are either
in the same vCPU context or are already serialised by the BQL.

Also use qemu_cond_wait_bql like the other halt_cond users.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4660
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4670
Cc: qemu-stable@nongnu.org
Fixes: d238858bff60 ("target/arm: Implement WFE, SEV and SEVONPEND for Cortex-M")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20261029161310.2065290-4-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 system/cpus.c              | 2 +-
 target/arm/tcg/op_helper.c | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/system/cpus.c b/system/cpus.c
index e11a5aab6a6..985732217fb 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -412,7 +412,7 @@ void qemu_process_cpu_events(CPUState *cpu)
             slept = true;
             qemu_plugin_vcpu_idle_cb(cpu);
         }
-        qemu_cond_wait(cpu->halt_cond, &bql);
+        qemu_cond_wait_bql(cpu->halt_cond);
     }
     if (slept) {
         qemu_plugin_vcpu_resume_cb(cpu);
diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
index 6a673e5ae6e..5788a094123 100644
--- a/target/arm/tcg/op_helper.c
+++ b/target/arm/tcg/op_helper.c
@@ -470,7 +470,10 @@ void HELPER(wfit)(CPUARMState *env, uint32_t rd)
 
 void HELPER(sev)(CPUARMState *env)
 {
-    CPUState *cs = env_cpu(env);
+    CPUState *cs;
+
+    BQL_LOCK_GUARD();
+
     CPU_FOREACH(cs) {
         ARMCPU *target_cpu = ARM_CPU(cs);
         target_cpu->env.event_register = true;
-- 
2.47.3

Re: [PATCH 1/4] target/arm: hold bql when SEV sets the event_register

[Lorand Marton]

On Fri, 30 Oct 2026 at 14:10, Alex Bennée <alex.bennee@linaro.org> wrote:
> There is a race in qemu_process_cpu_events and sleeping on halt_cond
> where we can miss the setting of the event_register which would cause
> arm_cpu_has_work to report we shouldn't sleep. As by definition the
> bql has to be held when we go to sleep against halt_cond guard the
> kicking of the cpus by the SEV helper with the BQL to ensure things
> are properly serialised.

I hit this race with a bare-metal test in which two CPUs pass a turn
back and forth 500000 times with WFE and SEV (virt, -smp 2,
semihosting).

On master (af1179b406) it hangs in 10 of 10 runs with -cpu max and in
10 of 10 with cortex-a72 (no result after 20 s; a passing run takes
2 s). A second test, in which a CPU waits in WFE for a broadcast TLBI
and an SEV, hangs in 23 of 100 runs with cortex-a53.

With this series applied on the same master, the first test passes
150 of 150 runs (50 each with max, cortex-a72 and cortex-a53) and the
second 600 of 600 (200 each).

Tested-by: Lorand Marton <lorandmarton.dev@gmail.com>

The series has no test case now; I can post these two as
tests/tcg/aarch64/system tests if that helps.

Thanks,
Lorand

Re: [PATCH 1/4] target/arm: hold bql when SEV sets the event_register

[Alex Bennée]

Lorand Marton <lorandmarton.dev@gmail.com> writes:

> On Fri, 30 Oct 2026 at 14:10, Alex Bennée <alex.bennee@linaro.org> wrote:
>> There is a race in qemu_process_cpu_events and sleeping on halt_cond
>> where we can miss the setting of the event_register which would cause
>> arm_cpu_has_work to report we shouldn't sleep. As by definition the
>> bql has to be held when we go to sleep against halt_cond guard the
>> kicking of the cpus by the SEV helper with the BQL to ensure things
>> are properly serialised.
>
> I hit this race with a bare-metal test in which two CPUs pass a turn
> back and forth 500000 times with WFE and SEV (virt, -smp 2,
> semihosting).
>
> On master (af1179b406) it hangs in 10 of 10 runs with -cpu max and in
> 10 of 10 with cortex-a72 (no result after 20 s; a passing run takes
> 2 s). A second test, in which a CPU waits in WFE for a broadcast TLBI
> and an SEV, hangs in 23 of 100 runs with cortex-a53.
>
> With this series applied on the same master, the first test passes
> 150 of 150 runs (50 each with max, cortex-a72 and cortex-a53) and the
> second 600 of 600 (200 each).
>
> Tested-by: Lorand Marton <lorandmarton.dev@gmail.com>
>
> The series has no test case now; I can post these two as
> tests/tcg/aarch64/system tests if that helps.

Are they hand written? I did include the test cases pulled from one of
the bugs here:

  20261029161310.2065290-1-alex.bennee@linaro.org

but I suspect the violate the current AI policy.

>
> Thanks,
> Lorand

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 2/4] target/arm: don't pass env to the sev helper

[Alex Bennée]

It was pointed out during reviewing a recent fix that cs is only
needed as a loop variable for CPU_FOREACH so passing env is just
wasteful.

Fixes: d238858bff60 ("target/arm: Implement WFE, SEV and SEVONPEND for Cortex-M")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20261029161310.2065290-5-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/arm/tcg/helper-defs.h   | 2 +-
 target/arm/tcg/op_helper.c     | 2 +-
 target/arm/tcg/translate-a64.c | 2 +-
 target/arm/tcg/translate.c     | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/arm/tcg/helper-defs.h b/target/arm/tcg/helper-defs.h
index 269ac515065..4f9736e6796 100644
--- a/target/arm/tcg/helper-defs.h
+++ b/target/arm/tcg/helper-defs.h
@@ -60,7 +60,7 @@ DEF_HELPER_1(yield, void, env)
 DEF_HELPER_1(pre_hvc, void, env)
 DEF_HELPER_2(pre_smc, void, env, i32)
 DEF_HELPER_1(vesb, void, env)
-DEF_HELPER_1(sev, void, env)
+DEF_HELPER_0(sev, void)
 
 DEF_HELPER_3(cpsr_write, void, env, i32, i32)
 DEF_HELPER_2(cpsr_write_eret, void, env, i32)
diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
index 5788a094123..4a31ce979c6 100644
--- a/target/arm/tcg/op_helper.c
+++ b/target/arm/tcg/op_helper.c
@@ -468,7 +468,7 @@ void HELPER(wfit)(CPUARMState *env, uint32_t rd)
 #endif
 }
 
-void HELPER(sev)(CPUARMState *env)
+void HELPER(sev)(void)
 {
     CPUState *cs;
 
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 0c712e7ac2e..941254983e9 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -2148,7 +2148,7 @@ static bool trans_SEV(DisasContext *s, arg_SEV *a)
      * SEV is a NOP for user-mode emulation.
      */
 #ifndef CONFIG_USER_ONLY
-    gen_helper_sev(tcg_env);
+    gen_helper_sev();
 #endif
     return true;
 }
diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index 6983153ffc6..c1970f93c8f 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -3282,7 +3282,7 @@ static bool trans_SEV(DisasContext *s, arg_SEV *a)
 #ifndef CONFIG_USER_ONLY
     if (arm_dc_feature(s, ARM_FEATURE_M) ||
         arm_dc_feature(s, ARM_FEATURE_V6K)) {
-        gen_helper_sev(tcg_env);
+        gen_helper_sev();
     }
 #endif
     return true;
-- 
2.47.3

[PATCH 3/4] target/arm: simplify wfxt_timer_cb to only trigger cpu_interrupt

[Alex Bennée]

HALT_WFE will always be cleared when leaving halt via
arm_cpu_leaving_halt so we don't need to worry about atomic games in
the asynchronous timer callback. Furthermore, a WFET timeout or
event-stream expiration is not an event that sets the event register;
doing so causes a subsequent WFE to wake up immediately.

As we only need CPUState to call cpu_interrupt we can also simplify
the user data and avoid needless casting to get to it.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Fixes: 01b223f895a7 ("target/arm: enable WFE sleeping for A-profile")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20261029161310.2065290-6-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/arm/cpu.c | 20 +++-----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 510fc9eeb3b..c690e2c8937 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -885,26 +885,12 @@ static void arm_cpu_leaving_halt(CPUState *cs)
 }
 #endif
 
-/*
- * Unlike almost everything else that messes with the halt_reason and
- * event_register details the timer callbacks are not in the vCPU
- * context.
- *
- * To prevent races we atomically consume a HALT_WFE and set the event
- * register. Either way we trigger the an exit event.
- */
 static void arm_wfxt_timer_cb(void *opaque)
 {
-    ARMCPU *cpu = opaque;
-    CPUState *cs = CPU(cpu);
-    CPUARMState *env = &cpu->env;
-
-    if (qatomic_cmpxchg(&env->halt_reason, HALT_WFE, NOT_HALTED)) {
-        qatomic_set(&env->event_register, true);
-    }
+    CPUState *cs = opaque;
 
     /*
-     * We expect the CPU to be halted; this will cause arm_cpu_is_work()
+     * We expect the CPU to be halted; this will cause arm_cpu_has_work()
      * to return true (so we will come out of halt even with no other
      * pending interrupt), and the TCG accelerator's cpu_exec_interrupt()
      * function auto-clears the CPU_INTERRUPT_EXITTB flag for us.
@@ -2302,7 +2288,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
      */
     if (tcg_enabled() && arm_feature(env, ARM_FEATURE_V6K)) {
         cpu->wfxt_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
-                                       arm_wfxt_timer_cb, cpu);
+                                       arm_wfxt_timer_cb, cs);
     }
 #endif
 
-- 
2.47.3

Re: [PATCH 3/4] target/arm: simplify wfxt_timer_cb to only trigger cpu_interrupt

[Peter Maydell]

On Fri, 30 Oct 2026 at 14:10, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> HALT_WFE will always be cleared when leaving halt via
> arm_cpu_leaving_halt so we don't need to worry about atomic games in
> the asynchronous timer callback. Furthermore, a WFET timeout or
> event-stream expiration is not an event that sets the event register;

No, this is still not true. WFET timeouts don't set the
event register, but event-stream expiration does.

-- PMM

Re: [PATCH 3/4] target/arm: simplify wfxt_timer_cb to only trigger cpu_interrupt

[Alex Bennée]

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

> On Fri, 30 Oct 2026 at 14:10, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> HALT_WFE will always be cleared when leaving halt via
>> arm_cpu_leaving_halt so we don't need to worry about atomic games in
>> the asynchronous timer callback. Furthermore, a WFET timeout or
>> event-stream expiration is not an event that sets the event register;
>
> No, this is still not true. WFET timeouts don't set the
> event register, but event-stream expiration does.

Oops I missed that comment, how about something like this:

--8<---------------cut here---------------start------------->8---
modified   target/arm/cpu.h
@@ -984,8 +984,12 @@ struct ArchCPU {
      * pmu_op_finish() - it does not need other handling during migration
      */
     QEMUTimer *pmu_timer;
-    /* Timer used for WFxT timeouts OR event stream events */
+    /*
+     * Timer used for WFxT timeouts OR event stream events,
+     * wfxt_triggers_event set depending on what will come first.
+     */
     QEMUTimer *wfxt_timer;
+    bool wfxt_triggers_event;
 
     /* GPIO outputs for generic timer */
     qemu_irq gt_timer_outputs[NUM_GTIMERS];
modified   target/arm/cpu.c
@@ -885,26 +885,17 @@ static void arm_cpu_leaving_halt(CPUState *cs)
 }
 #endif
 
-/*
- * Unlike almost everything else that messes with the halt_reason and
- * event_register details the timer callbacks are not in the vCPU
- * context.
- *
- * To prevent races we atomically consume a HALT_WFE and set the event
- * register. Either way we trigger the an exit event.
- */
 static void arm_wfxt_timer_cb(void *opaque)
 {
     ARMCPU *cpu = opaque;
     CPUState *cs = CPU(cpu);
     CPUARMState *env = &cpu->env;
 
-    if (qatomic_cmpxchg(&env->halt_reason, HALT_WFE, NOT_HALTED)) {
+    if (cpu->wfxt_triggers_event) {
         qatomic_set(&env->event_register, true);
     }
-
     /*
-     * We expect the CPU to be halted; this will cause arm_cpu_is_work()
+     * We expect the CPU to be halted; this will cause arm_cpu_has_work()
      * to return true (so we will come out of halt even with no other
      * pending interrupt), and the TCG accelerator's cpu_exec_interrupt()
      * function auto-clears the CPU_INTERRUPT_EXITTB flag for us.
modified   target/arm/tcg/op_helper.c
@@ -377,6 +377,7 @@ void HELPER(wfi)(CPUARMState *env, uint32_t insn_len)
     return;
 #else
     CPUState *cs = env_cpu(env);
+    ARMCPU *cpu = env_archcpu(env);
     uint32_t excp;
     int target_el = check_wfx_trap(env, false, &excp);
 
@@ -398,6 +399,7 @@ void HELPER(wfi)(CPUARMState *env, uint32_t insn_len)
                         target_el);
     }
 
+    cpu->wfxt_triggers_event = false;
     qatomic_set(&env->halt_reason, HALT_WFI);
     cs->exception_index = EXCP_HLT;
     cs->halted = 1;
@@ -461,6 +463,7 @@ void HELPER(wfit)(CPUARMState *env, uint32_t rd)
     } else {
         timer_mod(cpu->wfxt_timer, nexttick);
     }
+    cpu->wfxt_triggers_event = false;
     qatomic_set(&env->halt_reason, HALT_WFI);
     cs->exception_index = EXCP_HLT;
     cs->halted = 1;
@@ -720,12 +723,14 @@ void HELPER(wfet)(CPUARMState *env, uint32_t rd)
     next_event = gt_calc_next_event_stream(env);
     if (next_event > 0 && next_event < nexttick) {
         timer_mod(cpu->wfxt_timer, next_event);
+        cpu->wfxt_triggers_event = true;
     } else {
         if (nexttick == INT64_MAX) {
             timer_mod_ns(cpu->wfxt_timer, INT64_MAX);
         } else {
             timer_mod(cpu->wfxt_timer, nexttick);
         }
+        cpu->wfxt_triggers_event = false;
     }
 
     qatomic_set(&env->halt_reason, HALT_WFE);
--8<---------------cut here---------------end--------------->8---


>
> -- PMM

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 4/4] target/arm: fix bcm2835 migration issue

[Alex Bennée]

We don't take care to force re-validation of the config on migration
leading to a crash when the defaults don't match the migration stream.
The other frame-buffers seem to do a similar thing in post-loads to
address this.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4668
Fixes: 5e9c2a8dac19 ("bcm2835_fb: add framebuffer device for Raspberry Pi")
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20261029161310.2065290-9-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/display/bcm2835_fb.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/display/bcm2835_fb.c b/hw/display/bcm2835_fb.c
index bd58f625fcd..c5731521b57 100644
--- a/hw/display/bcm2835_fb.c
+++ b/hw/display/bcm2835_fb.c
@@ -352,10 +352,21 @@ static const MemoryRegionOps bcm2835_fb_ops = {
     .valid.max_access_size = 4,
 };
 
+static int bcm2835_fb_post_load(void *opaque, int version_id)
+{
+    BCM2835FBState *s = opaque;
+
+    bcm2835_fb_validate_config(&s->config);
+    s->invalidate = true;
+    qemu_console_resize(s->con, s->config.xres, s->config.yres);
+    return 0;
+}
+
 static const VMStateDescription vmstate_bcm2835_fb = {
     .name = TYPE_BCM2835_FB,
     .version_id = 1,
     .minimum_version_id = 1,
+    .post_load = bcm2835_fb_post_load,
     .fields = (const VMStateField[]) {
         VMSTATE_BOOL(lock, BCM2835FBState),
         VMSTATE_BOOL(invalidate, BCM2835FBState),
-- 
2.47.3

Re: [PATCH 0/4] arm fixes (WxFT, bcm2835_fb)

[Peter Maydell]

On Fri, 30 Oct 2026 at 14:10, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> These are the arm specific fixes from my last series. All reviewed and
> ready to pull.
>
> Alex.
>
> Alex Bennée (4):
>   target/arm: hold bql when SEV sets the event_register
>   target/arm: don't pass env to the sev helper
>   target/arm: simplify wfxt_timer_cb to only trigger cpu_interrupt
>   target/arm: fix bcm2835 migration issue

I've applied patches 1, 2 and 4 to target-arm.next; thanks.

-- PMM