[PATCH 0/8] fixes for 11.2 softfreeze (checkpatch, gdbstub, wfxt, bcm fb)

[PATCH 0/8] fixes for 11.2 softfreeze (checkpatch, gdbstub, wfxt, bcm fb)

[Alex Bennée]

A small selection of fixes as we approach softfreeze.

The core changes are tweaks to the WxFT emulation which fixes a
deadlock when the vCPU goes to sleep. There are some tidy ups in the
following patches including an import of the test case from one of the
bugs reported.

The is a quick fix for a gdbstub test regression I inadvertently added
in my last PR.

While checking the series I also ran into an issue with checkpatch
which I have a proposed fix for.

Finally there is a random bug fix for the bcm2835 frame buffer.

Alex.

Alex Bennée (8):
  scripts/checkpatch: validate fixes against origin/master
  tests/functional: fix call to common_tuxrun_gdb
  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
  tests/tcg: import arm wfe test cases for #4670
  tests/functional: remove @skipSlowTest from quanta_gsj test
  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 +-
 .../tcg/aarch64/system/wfe-sev-lock-aarch64.c | 259 ++++++++++++++++++
 tests/tcg/arm/system/wfe-sev-lock-arm.c       | 240 ++++++++++++++++
 scripts/checkpatch.pl                         |   4 +-
 tests/functional/arm/test_quanta_gsj.py       |   1 -
 tests/functional/s390x/test_tuxrun.py         |   1 -
 tests/tcg/aarch64/system/meson.build          |   7 +
 tests/tcg/arm/system/meson.build              |   7 +
 14 files changed, 538 insertions(+), 27 deletions(-)
 create mode 100644 tests/tcg/aarch64/system/wfe-sev-lock-aarch64.c
 create mode 100644 tests/tcg/arm/system/wfe-sev-lock-arm.c

-- 
2.47.3

[PATCH 1/8] scripts/checkpatch: validate fixes against origin/master

[Alex Bennée]

The current check for a referenced fix commit relies on the developer
keeping their master branch up to date which is not always grunted. If
you create your branches directly from origin/master this will keep
failing.

Given that origin/master should always be up to date (module the
developer eschewing convention) lets use that in the test instead.

checkpatch complains about the line lengths but this seems to be
standard for the perl script so I didn't try and fix it.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 scripts/checkpatch.pl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 82f62f60212..60e8b6bf7bc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1913,9 +1913,9 @@ sub process {
 				}
 				if (which("git") ne "" && -e "$gitroot") {
 					my $hash = defined($cid) ? $cid : $orig_commit;
-					`git merge-base --is-ancestor $hash master 2>/dev/null`;
+					`git merge-base --is-ancestor $hash origin/master 2>/dev/null`;
 					if ($? != 0) {
-						WARN("Fixes: commit $hash is not an ancestor of master\n" . $herecurr);
+						WARN("Fixes: commit $hash is not an ancestor of origin/master\n" . $herecurr);
 					}
 				}
 			}
-- 
2.47.3

[PATCH 2/8] tests/functional: fix call to common_tuxrun_gdb

[Alex Bennée]

I applied a last minute review fix to the api as it was not used but
evidently missed the one place where it had been set by copy & paste.

Fixes: e0a7139b2134 ("tests/functional: add tuxrun kvm gdb test for s390x")
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/functional/s390x/test_tuxrun.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/functional/s390x/test_tuxrun.py b/tests/functional/s390x/test_tuxrun.py
index 3821f985196..1dd662aaaf0 100755
--- a/tests/functional/s390x/test_tuxrun.py
+++ b/tests/functional/s390x/test_tuxrun.py
@@ -55,7 +55,6 @@ def test_s390_kvm_gdb(self):
                                rootfs_asset=self.ASSET_S390X_ROOTFS,
                                symbol_off="0x3ffe0000000",
                                drive="virtio-blk-ccw",
-                               haltmsg="Requesting system halt",
                                accel="kvm")
 
 if __name__ == '__main__':
-- 
2.47.3

[PATCH 3/8] 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")
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - drop pointless cs assignment.
---
 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

[PATCH 4/8] 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")
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 5/8] 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")
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

[PATCH 6/8] tests/tcg: import arm wfe test cases for #4670

[Alex Bennée]

The report for #4670 is a duplicate of #4660 (fixed in a previous
commit) but it came with some test cases that could trigger the issue.
It's more extensive than the basic test as it's trying to hit a race.

It's also a bit more complex as it needs to wake up the additional
vCPUs so they can take part.

As it doesn't take long to run it's probably worth adding.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
AI-used-for: writing the test case
---
 .../tcg/aarch64/system/wfe-sev-lock-aarch64.c | 259 ++++++++++++++++++
 tests/tcg/arm/system/wfe-sev-lock-arm.c       | 240 ++++++++++++++++
 tests/tcg/aarch64/system/meson.build          |   7 +
 tests/tcg/arm/system/meson.build              |   7 +
 4 files changed, 513 insertions(+)
 create mode 100644 tests/tcg/aarch64/system/wfe-sev-lock-aarch64.c
 create mode 100644 tests/tcg/arm/system/wfe-sev-lock-arm.c

diff --git a/tests/tcg/aarch64/system/wfe-sev-lock-aarch64.c b/tests/tcg/aarch64/system/wfe-sev-lock-aarch64.c
new file mode 100644
index 00000000000..6a767e7476c
--- /dev/null
+++ b/tests/tcg/aarch64/system/wfe-sev-lock-aarch64.c
@@ -0,0 +1,259 @@
+/*
+ * SMP ticket lock whose waiters sleep in WFE and whose releaser wakes
+ * them with SEV (the waiters re-read the owner with a plain load-acquire,
+ * so no exclusive monitor is armed while they wait).
+ *
+ * CPU0 starts the other CPUs with PSCI CPU_ON and then only watches.
+ * The other CPUs take and release one ticket lock with interrupts masked
+ * and no timer or event stream running, so a waiter halted in WFE can
+ * only be woken by the SEV that the releasing CPU executes.  If that
+ * wake-up is lost, the waiter whose ticket is now being served sleeps
+ * forever with the lock free.  CPU0 notices that no CPU has made
+ * progress for STALL_MS, reports the lock state, issues its own SEV to
+ * recover and fails the test at the end.
+ *
+ * Run with -smp 3 or more; up to MAX_CPUS are used.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <minilib.h>
+
+#define MAX_CPUS        8
+#define ITERATIONS      20000   /* lock acquisitions per worker CPU */
+#define HOLD_LOOPS      200     /* work done while holding the lock */
+#define STALL_MS        500     /* no progress for this long = stall */
+#define MAX_STALLS      20      /* give up after this many */
+
+#define PSCI_CPU_ON_64  0xc4000003ul
+#define NO_TICKET       0xffffffffu
+
+#define READ_ONCE(x)        __atomic_load_n(&(x), __ATOMIC_RELAXED)
+#define WRITE_ONCE(x, v)    __atomic_store_n(&(x), (v), __ATOMIC_RELAXED)
+
+/* from Linux's include/linux/stringify.h */
+#define __stringify_1(x...) #x
+#define __stringify(x...)   __stringify_1(x)
+
+#define read_sysreg(r) ({                                           \
+            uint64_t __val;                                         \
+            asm volatile("mrs %0, " __stringify(r) : "=r" (__val)); \
+            __val;                                                  \
+})
+
+typedef union {
+    uint32_t slock;
+    struct {
+        uint16_t owner;
+        uint16_t next;
+    } tickets;
+} ticket_lock_t;
+
+static ticket_lock_t lock;
+static uint32_t shared_count;
+static uint32_t progress[MAX_CPUS];
+static uint32_t waiting_for[MAX_CPUS];
+static uint32_t finished[MAX_CPUS];
+
+/* CPU0's EL1 MMU configuration, copied by the secondaries */
+struct boot_regs {
+    uint64_t vbar, mair, tcr, ttbr0, sctlr, cpacr;
+};
+static struct boot_regs __attribute__((used)) boot_regs;
+
+#define STACK_SIZE 4096
+static uint8_t stacks[MAX_CPUS][STACK_SIZE] __attribute__((aligned(16)));
+
+static uint32_t this_cpu(void)
+{
+    return read_sysreg(mpidr_el1) & 0xff;
+}
+
+static void ticket_lock(uint32_t cpu)
+{
+    uint32_t old, newval, fail;
+    uint16_t ticket, owner;
+
+    asm volatile("1: ldaxr   %w0, [%3]\n"
+                 "   add     %w1, %w0, %w4\n"
+                 "   stxr    %w2, %w1, [%3]\n"
+                 "   cbnz    %w2, 1b"
+                 : "=&r" (old), "=&r" (newval), "=&r" (fail)
+                 : "r" (&lock.slock), "r" (1 << 16)
+                 : "memory");
+
+    ticket = old >> 16;
+    owner = old & 0xffff;
+    WRITE_ONCE(waiting_for[cpu], ticket);
+    while (ticket != owner) {
+        asm volatile("wfe\n"
+                     "ldarh %w0, [%1]"
+                     : "=r" (owner) : "r" (&lock.tickets.owner) : "memory");
+    }
+    WRITE_ONCE(waiting_for[cpu], NO_TICKET);
+}
+
+static void ticket_unlock(void)
+{
+    uint16_t owner = lock.tickets.owner + 1;
+
+    asm volatile("stlrh %w0, [%1]\n"
+                 "dsb ishst\n"
+                 "sev"
+                 : : "r" (owner), "r" (&lock.tickets.owner) : "memory");
+}
+
+static void __attribute__((used)) secondary_main(void)
+{
+    uint32_t cpu = this_cpu();
+
+    for (uint32_t i = 0; i < ITERATIONS; i++) {
+        ticket_lock(cpu);
+        shared_count++;
+        /* hold the lock long enough for the others to go to sleep */
+        for (uint32_t spin = 0; spin < HOLD_LOOPS; spin++) {
+            asm volatile("" : : : "memory");
+        }
+        ticket_unlock();
+        WRITE_ONCE(progress[cpu], i + 1);
+    }
+    WRITE_ONCE(finished[cpu], 1);
+}
+
+/*
+ * Entered by PSCI CPU_ON at EL1 with the MMU off; x0 holds the context
+ * ID, which is the top of this CPU's stack.
+ */
+asm(".text\n"
+    ".type secondary_entry, %function\n"
+    "secondary_entry:\n"
+    "    mov sp, x0\n"
+    "    adrp x1, boot_regs\n"
+    "    add x1, x1, :lo12:boot_regs\n"
+    "    ldp x2, x3, [x1, #0]\n"
+    "    msr vbar_el1, x2\n"
+    "    msr mair_el1, x3\n"
+    "    ldp x2, x3, [x1, #16]\n"
+    "    msr tcr_el1, x2\n"
+    "    msr ttbr0_el1, x3\n"
+    "    ldp x2, x3, [x1, #32]\n"
+    "    msr cpacr_el1, x3\n"
+    "    isb\n"
+    "    dsb sy\n"
+    "    msr sctlr_el1, x2\n"
+    "    isb\n"
+    "    bl secondary_main\n"
+    "1:  wfi\n"
+    "    b 1b\n");
+
+static int64_t psci_cpu_on(uint64_t mpidr, uint64_t entry, uint64_t ctx)
+{
+    register uint64_t x0 asm("x0") = PSCI_CPU_ON_64;
+    register uint64_t x1 asm("x1") = mpidr;
+    register uint64_t x2 asm("x2") = entry;
+    register uint64_t x3 asm("x3") = ctx;
+
+    asm volatile("hvc #0"
+                 : "+r" (x0), "+r" (x1), "+r" (x2), "+r" (x3)
+                 :
+                 : "memory");
+    return x0;
+}
+
+static uint64_t read_cntvct(void)
+{
+    asm volatile("isb" : : : "memory");
+    return read_sysreg(cntvct_el0);
+}
+
+int main(void)
+{
+    uint32_t ncpus = 1, workers, stalls = 0, last_sum = 0;
+    uint64_t freq = read_sysreg(cntfrq_el0);
+    uint64_t start, now, last_change, stall_ticks, entry;
+
+    boot_regs.vbar = read_sysreg(vbar_el1);
+    boot_regs.mair = read_sysreg(mair_el1);
+    boot_regs.tcr = read_sysreg(tcr_el1);
+    boot_regs.ttbr0 = read_sysreg(ttbr0_el1);
+    boot_regs.sctlr = read_sysreg(sctlr_el1);
+    boot_regs.cpacr = read_sysreg(cpacr_el1);
+    asm volatile("dsb sy" : : : "memory");
+
+    asm("adrp %0, secondary_entry\n"
+        "add %0, %0, :lo12:secondary_entry" : "=r" (entry));
+
+    for (uint32_t cpu = 1; cpu < MAX_CPUS; cpu++) {
+        if (psci_cpu_on(cpu, entry,
+                        (uint64_t)&stacks[cpu][STACK_SIZE]) != 0) {
+            break;
+        }
+        ncpus++;
+    }
+    workers = ncpus - 1;
+    if (workers < 2) {
+        ml_printf("SKIP: need -smp 3 or more (found %d CPUs)\n", ncpus);
+        return 0;
+    }
+    ml_printf("%d CPUs, %d contending workers, %d iterations each\n",
+              ncpus, workers, ITERATIONS);
+
+    stall_ticks = freq / 1000 * STALL_MS;
+    start = last_change = read_cntvct();
+    for (;;) {
+        uint32_t sum = 0, done = 0, owner;
+
+        for (uint32_t cpu = 1; cpu < ncpus; cpu++) {
+            sum += READ_ONCE(progress[cpu]);
+            done += READ_ONCE(finished[cpu]);
+        }
+        if (done == workers) {
+            break;
+        }
+        now = read_cntvct();
+        if (sum != last_sum) {
+            last_sum = sum;
+            last_change = now;
+            continue;
+        }
+        if (now - last_change < stall_ticks) {
+            continue;
+        }
+
+        stalls++;
+        owner = READ_ONCE(lock.tickets.owner);
+        ml_printf("STALL %d: no progress for %d ms; lock owner %d next %d\n",
+                  stalls, STALL_MS, owner, READ_ONCE(lock.tickets.next));
+        for (uint32_t cpu = 1; cpu < ncpus; cpu++) {
+            if (READ_ONCE(waiting_for[cpu]) == owner) {
+                ml_printf("  cpu%d holds the next ticket %d and is still "
+                          "waiting: lost wake-up\n", cpu, owner);
+            }
+        }
+        if (stalls >= MAX_STALLS) {
+            ml_printf("FAIL: giving up after %d stalls\n", stalls);
+            return 1;
+        }
+        /* Recover so the rest of the run can be measured. */
+        asm volatile("dsb ishst\n"
+                     "sev" : : : "memory");
+        last_change = read_cntvct();
+    }
+
+    now = read_cntvct();
+    ml_printf("%d acquisitions in %d ms, %d stalls\n", shared_count,
+              (uint32_t)((now - start) / (freq / 1000)), stalls);
+    if (shared_count != workers * ITERATIONS) {
+        ml_printf("FAIL: count %d, expected %d\n", shared_count,
+                  workers * ITERATIONS);
+        return 1;
+    }
+    if (stalls) {
+        ml_printf("FAIL: %d lost wake-ups\n", stalls);
+        return 1;
+    }
+    ml_printf("PASS\n");
+    return 0;
+}
diff --git a/tests/tcg/arm/system/wfe-sev-lock-arm.c b/tests/tcg/arm/system/wfe-sev-lock-arm.c
new file mode 100644
index 00000000000..1e87744a695
--- /dev/null
+++ b/tests/tcg/arm/system/wfe-sev-lock-arm.c
@@ -0,0 +1,240 @@
+/*
+ * SMP ticket lock using WFE/SEV, in the style of the 32-bit Arm Linux
+ * arch_spin_lock()/arch_spin_unlock().
+ *
+ * CPU0 starts the other CPUs with PSCI CPU_ON and then only watches.
+ * The other CPUs take and release one ticket lock with IRQs masked and
+ * no timer running, so a waiter halted in WFE can only be woken by the
+ * SEV that the releasing CPU executes.  If that wake-up is lost, the
+ * waiter whose ticket is now being served sleeps forever with the lock
+ * free.  CPU0 notices that no CPU has made progress for STALL_MS,
+ * reports the lock state, issues its own SEV to recover and fails the
+ * test at the end.
+ *
+ * Run with -smp 3 or more; up to MAX_CPUS are used.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <minilib.h>
+
+#define MAX_CPUS        8
+#define ITERATIONS      20000   /* lock acquisitions per worker CPU */
+#define HOLD_LOOPS      200     /* work done while holding the lock */
+#define STALL_MS        500     /* no progress for this long = stall */
+#define MAX_STALLS      20      /* give up after this many */
+
+#define PSCI_CPU_ON     0x84000003u
+#define NO_TICKET       0xffffffffu
+
+#define READ_ONCE(x)        __atomic_load_n(&(x), __ATOMIC_RELAXED)
+#define WRITE_ONCE(x, v)    __atomic_store_n(&(x), (v), __ATOMIC_RELAXED)
+
+/* Same layout as Linux's arch_spinlock_t on a little-endian CPU */
+typedef union {
+    uint32_t slock;
+    struct {
+        uint16_t owner;
+        uint16_t next;
+    } tickets;
+} ticket_lock_t;
+
+static ticket_lock_t lock;
+static uint32_t shared_count;
+static uint32_t progress[MAX_CPUS];
+static uint32_t waiting_for[MAX_CPUS];
+static uint32_t finished[MAX_CPUS];
+
+#define STACK_SIZE 4096
+static uint8_t stacks[MAX_CPUS][STACK_SIZE] __attribute__((aligned(16)));
+
+static uint32_t this_cpu(void)
+{
+    uint32_t mpidr;
+
+    asm volatile("mrc p15, 0, %0, c0, c0, 5" : "=r" (mpidr));
+    return mpidr & 0xff;
+}
+
+static void ticket_lock(uint32_t cpu)
+{
+    uint32_t old, newval, fail;
+    uint16_t ticket, owner;
+
+    asm volatile("1: ldrex   %0, [%3]\n"
+                 "   add     %1, %0, %4\n"
+                 "   strex   %2, %1, [%3]\n"
+                 "   teq     %2, #0\n"
+                 "   bne     1b"
+                 : "=&r" (old), "=&r" (newval), "=&r" (fail)
+                 : "r" (&lock.slock), "I" (1 << 16)
+                 : "cc", "memory");
+
+    ticket = old >> 16;
+    owner = old & 0xffff;
+    WRITE_ONCE(waiting_for[cpu], ticket);
+    while (ticket != owner) {
+        asm volatile("wfe" : : : "memory");
+        owner = READ_ONCE(lock.tickets.owner);
+    }
+    WRITE_ONCE(waiting_for[cpu], NO_TICKET);
+    asm volatile("dmb ish" : : : "memory");
+}
+
+static void ticket_unlock(void)
+{
+    asm volatile("dmb ish" : : : "memory");
+    WRITE_ONCE(lock.tickets.owner, lock.tickets.owner + 1);
+    asm volatile("dsb ishst\n"
+                 "sev" : : : "memory");
+}
+
+static void __attribute__((used)) secondary_main(void)
+{
+    uint32_t cpu = this_cpu();
+
+    for (uint32_t i = 0; i < ITERATIONS; i++) {
+        ticket_lock(cpu);
+        shared_count++;
+        /* hold the lock long enough for the others to go to sleep */
+        for (uint32_t spin = 0; spin < HOLD_LOOPS; spin++) {
+            asm volatile("" : : : "memory");
+        }
+        ticket_unlock();
+        WRITE_ONCE(progress[cpu], i + 1);
+    }
+    WRITE_ONCE(finished[cpu], 1);
+}
+
+/*
+ * Entered by PSCI CPU_ON in SVC mode with the MMU off; r0 holds the
+ * context ID, which is the top of this CPU's stack.  Use the same
+ * vectors and translation table as CPU0 (mmu_setup is in boot.S).
+ */
+asm(".text\n"
+    ".type secondary_entry, %function\n"
+    "secondary_entry:\n"
+    "    ldr r1, =vector_table\n"
+    "    mcr p15, 0, r1, c12, c0, 0\n"
+    "    mov sp, r0\n"
+    "    bl mmu_setup\n"
+    "    bl secondary_main\n"
+    "1:  wfi\n"
+    "    b 1b\n"
+    "    .ltorg\n");
+
+static int32_t psci_cpu_on(uint32_t mpidr, uint32_t entry, uint32_t ctx)
+{
+    register uint32_t r0 asm("r0") = PSCI_CPU_ON;
+    register uint32_t r1 asm("r1") = mpidr;
+    register uint32_t r2 asm("r2") = entry;
+    register uint32_t r3 asm("r3") = ctx;
+
+    asm volatile(".arch_extension virt\n"
+                 "hvc #0"
+                 : "+r" (r0)
+                 : "r" (r1), "r" (r2), "r" (r3)
+                 : "memory");
+    return r0;
+}
+
+static uint64_t read_cntvct(void)
+{
+    uint32_t lo, hi;
+
+    asm volatile("isb\n"
+                 "mrrc p15, 1, %0, %1, c14" : "=r" (lo), "=r" (hi));
+    return ((uint64_t)hi << 32) | lo;
+}
+
+static uint32_t read_cntfrq(void)
+{
+    uint32_t frq;
+
+    asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (frq));
+    return frq;
+}
+
+int main(void)
+{
+    uint32_t ncpus = 1, workers, stalls = 0, last_sum = 0, entry;
+    uint32_t freq = read_cntfrq();
+    uint64_t start, now, last_change, stall_ticks;
+
+    asm("movw %0, #:lower16:secondary_entry\n"
+        "movt %0, #:upper16:secondary_entry" : "=r" (entry));
+
+    for (uint32_t cpu = 1; cpu < MAX_CPUS; cpu++) {
+        if (psci_cpu_on(cpu, entry,
+                        (uint32_t)&stacks[cpu][STACK_SIZE]) != 0) {
+            break;
+        }
+        ncpus++;
+    }
+    workers = ncpus - 1;
+    if (workers < 2) {
+        ml_printf("SKIP: need -smp 3 or more (found %d CPUs)\n", ncpus);
+        return 0;
+    }
+    ml_printf("%d CPUs, %d contending workers, %d iterations each\n",
+              ncpus, workers, ITERATIONS);
+
+    stall_ticks = (uint64_t)freq / 1000 * STALL_MS;
+    start = last_change = read_cntvct();
+    for (;;) {
+        uint32_t sum = 0, done = 0, owner;
+
+        for (uint32_t cpu = 1; cpu < ncpus; cpu++) {
+            sum += READ_ONCE(progress[cpu]);
+            done += READ_ONCE(finished[cpu]);
+        }
+        if (done == workers) {
+            break;
+        }
+        now = read_cntvct();
+        if (sum != last_sum) {
+            last_sum = sum;
+            last_change = now;
+            continue;
+        }
+        if (now - last_change < stall_ticks) {
+            continue;
+        }
+
+        stalls++;
+        owner = READ_ONCE(lock.tickets.owner);
+        ml_printf("STALL %d: no progress for %d ms; lock owner %d next %d\n",
+                  stalls, STALL_MS, owner, READ_ONCE(lock.tickets.next));
+        for (uint32_t cpu = 1; cpu < ncpus; cpu++) {
+            if (READ_ONCE(waiting_for[cpu]) == owner) {
+                ml_printf("  cpu%d holds the next ticket %d and is still "
+                          "waiting: lost wake-up\n", cpu, owner);
+            }
+        }
+        if (stalls >= MAX_STALLS) {
+            ml_printf("FAIL: giving up after %d stalls\n", stalls);
+            return 1;
+        }
+        /* Recover so the rest of the run can be measured. */
+        asm volatile("dsb ishst\n"
+                     "sev" : : : "memory");
+        last_change = read_cntvct();
+    }
+
+    now = read_cntvct();
+    ml_printf("%d acquisitions in %d ms, %d stalls\n", shared_count,
+              (uint32_t)((now - start) / (freq / 1000)), stalls);
+    if (shared_count != workers * ITERATIONS) {
+        ml_printf("FAIL: count %d, expected %d\n", shared_count,
+                  workers * ITERATIONS);
+        return 1;
+    }
+    if (stalls) {
+        ml_printf("FAIL: %d lost wake-ups\n", stalls);
+        return 1;
+    }
+    ml_printf("PASS\n");
+    return 0;
+}
diff --git a/tests/tcg/aarch64/system/meson.build b/tests/tcg/aarch64/system/meson.build
index 8710ebfdad3..18f2d0a3ff4 100644
--- a/tests/tcg/aarch64/system/meson.build
+++ b/tests/tcg/aarch64/system/meson.build
@@ -79,6 +79,13 @@ tests += {
                   '-semihosting-config', 'enable=on,arg=2',
                   qemu_base_args]
   },
+  'wfe-sev-lock-aarch64.c': {
+    'cflags': cflags,
+    'qemu_args': ['-serial', 'none', '-chardev', 'stdio,mux=on,id=stdio0',
+                  '-semihosting-config', 'enable=on,chardev=stdio0',
+                  '-mon', 'chardev=stdio0,mode=readline', '-smp', '4',
+                  qemu_def_args],
+  },
   'ttbr-baddr.c': {
     'cflags': cflags,
     'qemu_args': ['-M', 'virt,virtualization=on', '-cpu', 'max',
diff --git a/tests/tcg/arm/system/meson.build b/tests/tcg/arm/system/meson.build
index c20818f0893..cf6e71fee43 100644
--- a/tests/tcg/arm/system/meson.build
+++ b/tests/tcg/arm/system/meson.build
@@ -59,6 +59,13 @@ tests += {
                   qemu_def_args],
     'wrapper': [prog_run_with_input, 'X']
   },
+  'wfe-sev-lock-arm.c': {
+    'cflags': cflags,
+    'qemu_args': ['-serial', 'none', '-chardev', 'stdio,mux=on,id=stdio0',
+                  '-semihosting-config', 'enable=on,chardev=stdio0',
+                  '-mon', 'chardev=stdio0,mode=readline', '-smp', '4',
+                  qemu_def_args],
+  },
   '../../multiarch/system/memory.c': {
     'test_name': 'memory-record-replay',
     'qemu_args': qemu_def_args,
-- 
2.47.3

[PATCH 7/8] tests/functional: remove @skipSlowTest from quanta_gsj test

[Alex Bennée]

The full quanta_gsj test would have caught the regression caused by
the WFxT implementation and isn't that slow (<60s) it needs to be
gated.

Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/functional/arm/test_quanta_gsj.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/functional/arm/test_quanta_gsj.py b/tests/functional/arm/test_quanta_gsj.py
index cb0545f7bfa..0c0f5d85874 100755
--- a/tests/functional/arm/test_quanta_gsj.py
+++ b/tests/functional/arm/test_quanta_gsj.py
@@ -30,7 +30,6 @@ class EmcraftSf2Machine(LinuxKernelTest):
          '20200711-gsj-qemu-0/nuvoton-npcm730-gsj.dtb'),
         '3249b2da787d4b9ad4e61f315b160abfceb87b5e1895a7ce898ce7f40c8d4045')
 
-    @skipSlowTest()
     def test_arm_quanta_gsj(self):
         self.set_machine('quanta-gsj')
         image_path = self.uncompress(self.ASSET_IMAGE, format='gz')
-- 
2.47.3

[PATCH 8/8] 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
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---

Bellow is an AI imported qtest-ification of the test case with the bug
report. I didn't think it was worth adding as a commit (and this will
be dropped when merged) but in case anyone wants to see how it looks
and think if it meets the bar for code legibility and understanding.

--8<---------------cut here---------------start------------->8---
new file    tests/qtest/bcm2835-fb-test.c
@@ -0,0 +1,109 @@
+/*
+ * QTest testcase for BCM2835 Framebuffer
+ *
+ * Copyright (c) 2026 QEMU contributors
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "migration/migration-qmp.h"
+
+#define MBOX_ADDR 0x3f00b8a0
+#define PROP_BUFFER_ADDR 0x30000000
+
+static void test_bcm2835_fb_resize_migrate(void)
+{
+    g_autofree char *tmpdir = g_dir_make_tmp("qtest-bcm2835-fb-XXXXXX", NULL);
+    g_autofree char *migfile = g_build_filename(tmpdir, "fb.state", NULL);
+    g_autofree char *mig_uri = g_strdup_printf("file:%s", migfile);
+    g_autofree char *ppm_path = g_build_filename(tmpdir, "out.ppm", NULL);
+    QTestState *src, *dst;
+    QDict *rsp;
+    FILE *f;
+    char header[64];
+
+    /* Start source machine */
+    src = qtest_init("-accel tcg -M raspi3b -m 1G -display none -S");
+
+    /*
+     * Configure 1024x768 framebuffer via property mailbox.
+     * Buffer at 0x30000000:
+     * - size: 0x30 (48 bytes)
+     * - request code: 0
+     * - tag: 0x00048003 (SET_PHYSICAL_WIDTH_HEIGHT)
+     * - val buf size: 8
+     * - req/resp indicator: 8
+     * - width: 1024
+     * - height: 768
+     * - end tag: 0
+     */
+    qtest_writel(src, PROP_BUFFER_ADDR + 0x00, 0x30);
+    qtest_writel(src, PROP_BUFFER_ADDR + 0x04, 0x00000000);
+    qtest_writel(src, PROP_BUFFER_ADDR + 0x08, 0x00048003);
+    qtest_writel(src, PROP_BUFFER_ADDR + 0x0c, 8);
+    qtest_writel(src, PROP_BUFFER_ADDR + 0x10, 8);
+    qtest_writel(src, PROP_BUFFER_ADDR + 0x14, 1024);
+    qtest_writel(src, PROP_BUFFER_ADDR + 0x18, 768);
+    qtest_writel(src, PROP_BUFFER_ADDR + 0x1c, 0x00000000);
+
+    /* Post buffer address ORed with property channel (8) to Mailbox 1 */
+    qtest_writel(src, MBOX_ADDR, PROP_BUFFER_ADDR | 8);
+
+    /* Migrate source to file */
+    qtest_qmp_assert_success(src,
+        "{ 'execute': 'migrate', 'arguments': { 'uri': %s } }", mig_uri);
+    wait_for_migration_complete(src);
+    qtest_quit(src);
+
+    /* Start destination and load state */
+    dst = qtest_initf("-accel tcg -M raspi3b -m 1G -display none -S "
+                      "-incoming %s", mig_uri);
+    wait_for_migration_complete(dst);
+
+    /* Request screendump from destination */
+    rsp = qtest_qmp(dst,
+        "{ 'execute': 'screendump', 'arguments': { 'filename': %s } }",
+        ppm_path);
+    if (qdict_haskey(rsp, "error")) {
+        QDict *error = qdict_get_qdict(rsp, "error");
+        const char *class = qdict_get_str(error, "class");
+        if (g_str_equal(class, "CommandNotFound")) {
+            /* Built without pixman */
+            qobject_unref(rsp);
+            qtest_quit(dst);
+            unlink(migfile);
+            rmdir(tmpdir);
+            g_test_skip("screendump command not available (without pixman)");
+            return;
+        }
+    }
+    g_assert(!qdict_haskey(rsp, "error"));
+    qobject_unref(rsp);
+
+    /* Check PPM header for 1024x768 resolution */
+    f = fopen(ppm_path, "r");
+    g_assert_nonnull(f);
+    g_assert_nonnull(fgets(header, sizeof(header), f));
+    g_assert_cmpstr(header, ==, "P6\n");
+    g_assert_nonnull(fgets(header, sizeof(header), f));
+    g_assert_cmpstr(header, ==, "1024 768\n");
+    fclose(f);
+
+    qtest_quit(dst);
+
+    unlink(ppm_path);
+    unlink(migfile);
+    rmdir(tmpdir);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    qtest_add_func("/bcm2835/fb/resize_migrate",
+                   test_bcm2835_fb_resize_migrate);
+
+    return g_test_run();
+}
modified    tests/qtest/meson.build
@@ -283,7 +283,7 @@ qtests_aarch64 = \
     ['tpm-tis-device-test', 'tpm-tis-device-swtpm-test'] : []) +                                         \
   (config_all_devices.has_key('CONFIG_XLNX_ZYNQMP_ARM') ? ['xlnx-can-test', 'fuzz-xlnx-dp-test'] : []) + \
   (config_all_devices.has_key('CONFIG_XLNX_VERSAL') ? ['xlnx-canfd-test', 'xlnx-versal-trng-test'] : []) + \
-  (config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test', 'bcm2835-i2c-test', 'bcm2835-ic-test'] : []) +  \
+  (config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test', 'bcm2835-fb-test', 'bcm2835-i2c-test', 'bcm2835-ic-test'] : []) +  \
   (config_all_accel.has_key('CONFIG_TCG') and                                            \
    config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test', 'tpm-tis-i2c-swtpm-test'] : []) + \
   (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
@@ -424,6 +424,8 @@ qtests = {
   'ast2700-hace-test': [files('aspeed-hace-utils.c', 'ast2700-hace-test.c'),
                         crypto],
   'ast2700-smc-test': files('aspeed-smc-utils.c', 'ast2700-smc-test.c'),
+  'bcm2835-fb-test': files('migration/migration-qmp.c',
+                           'migration/migration-util.c'),
   'bios-tables-test': [io, 'boot-sector.c', 'acpi-utils.c', 'tpm-emu.c'],
   'cdrom-test': files('boot-sector.c'),
   'dbus-vmstate-test': files('migration/migration-qmp.c',
--8<---------------cut here---------------end--------------->8---
---
 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 3/8] target/arm: hold bql when SEV sets the event_register

[Richard Henderson]

On 10/29/26 11:13, Alex Bennée 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.
> 
> 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")
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> 
> ---
> v2
>    - drop pointless cs assignment.
> ---
>   system/cpus.c              | 2 +-
>   target/arm/tcg/op_helper.c | 5 ++++-
>   2 files changed, 5 insertions(+), 2 deletions(-)

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

r~

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

[Richard Henderson]

On 10/29/26 11:13, Alex Bennée wrote:
> 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")
> 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(-)

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

r~

Re: [PATCH 5/8] target/arm: simplify wfxt_timer_cb to only trigger cpu_interrupt

[Richard Henderson]

On 10/29/26 11:13, Alex Bennée 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;
> 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")
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   target/arm/cpu.c | 20 +++-----------------
>   1 file changed, 3 insertions(+), 17 deletions(-)

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

r~

Re: [PATCH 8/8] target/arm: fix bcm2835 migration issue

[Peter Maydell]

On Thu, 29 Oct 2026 at 16:13, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> 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
> 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

For the fix

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

thanks
-- PMM

Re: [PATCH 0/8] fixes for 11.2 softfreeze (checkpatch, gdbstub, wfxt, bcm fb)

[Peter Maydell]

On Thu, 29 Oct 2026 at 16:13, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> A small selection of fixes as we approach softfreeze.
>
> The core changes are tweaks to the WxFT emulation which fixes a
> deadlock when the vCPU goes to sleep. There are some tidy ups in the
> following patches including an import of the test case from one of the
> bugs reported.
>
> The is a quick fix for a gdbstub test regression I inadvertently added
> in my last PR.
>
> While checking the series I also ran into an issue with checkpatch
> which I have a proposed fix for.
>
> Finally there is a random bug fix for the bcm2835 frame buffer.
>
> Alex.
>
> Alex Bennée (8):
>   scripts/checkpatch: validate fixes against origin/master
>   tests/functional: fix call to common_tuxrun_gdb
>   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
>   tests/tcg: import arm wfe test cases for #4670
>   tests/functional: remove @skipSlowTest from quanta_gsj test
>   target/arm: fix bcm2835 migration issue

The WFE fixes are target-arm material -- could you post
them as their own series for review, not mixed in with
these other unrelated changes, please?

thanks
-- PMM

Re: [PATCH 5/8] target/arm: simplify wfxt_timer_cb to only trigger cpu_interrupt

[Peter Maydell]

On Thu, 29 Oct 2026 at 16:13, 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;
> doing so causes a subsequent WFE to wake up immediately.

It is the case that WFET timeouts don't set the event register.
But an event-stream expiration does: R_XRZRK lists
"An event from a Generic Timer event stream, see Event streams."
in its set of things that set the event register.

thanks
-- PMM

Re: [PATCH 7/8] tests/functional: remove @skipSlowTest from quanta_gsj test

[Peter Maydell]

On Thu, 29 Oct 2026 at 16:13, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The full quanta_gsj test would have caught the regression caused by
> the WFxT implementation and isn't that slow (<60s) it needs to be
> gated.
>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/functional/arm/test_quanta_gsj.py | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/tests/functional/arm/test_quanta_gsj.py b/tests/functional/arm/test_quanta_gsj.py
> index cb0545f7bfa..0c0f5d85874 100755
> --- a/tests/functional/arm/test_quanta_gsj.py
> +++ b/tests/functional/arm/test_quanta_gsj.py
> @@ -30,7 +30,6 @@ class EmcraftSf2Machine(LinuxKernelTest):
>           '20200711-gsj-qemu-0/nuvoton-npcm730-gsj.dtb'),
>          '3249b2da787d4b9ad4e61f315b160abfceb87b5e1895a7ce898ce7f40c8d4045')
>
> -    @skipSlowTest()
>      def test_arm_quanta_gsj(self):
>          self.set_machine('quanta-gsj')
>          image_path = self.uncompress(self.ASSET_IMAGE, format='gz')
> --
> 2.47.3


It looks like this was originally marked skipSlowtest() as the
evolution of what was originally commit b6eea069ab marking
it as AVOCADO_TIMEOUT_EXPECTED because of observed timeouts.
I guess we'll see if we fixed whatever that flakiness was.

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

(We do want not only to avoid running tests that are slow in themselves
but also to avoid accumulating a large number of tests that are
individually only moderately slow but together add up to the complete
run being slow. But I think we can afford this one.)

The other argument against using quanta-gsj as our test here
is that as discussed as part of a recent docs update:
https://lore.kernel.org/qemu-devel/20260627063208.33642-1-bin.meng@processmission.com/
it's no longer supported by OpenBMC.

I didn't get any response from the Nuvoton maintainers in
that thread, but I kind of favour deprecating the machine
type in QEMU.

thanks
-- PMM

Re: [PATCH 7/8] tests/functional: remove @skipSlowTest from quanta_gsj test

[Thomas Huth]

On 29/10/2026 17.13, Alex Bennée wrote:
> The full quanta_gsj test would have caught the regression caused by
> the WFxT implementation and isn't that slow (<60s) it needs to be
> gated.
> 
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   tests/functional/arm/test_quanta_gsj.py | 1 -
>   1 file changed, 1 deletion(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 7/8] tests/functional: remove @skipSlowTest from quanta_gsj test

[Thomas Huth]

On 29/10/2026 17.38, Peter Maydell wrote:
> On Thu, 29 Oct 2026 at 16:13, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> The full quanta_gsj test would have caught the regression caused by
>> the WFxT implementation and isn't that slow (<60s) it needs to be
>> gated.
>>
>> Suggested-by: Thomas Huth <thuth@redhat.com>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
...
> The other argument against using quanta-gsj as our test here
> is that as discussed as part of a recent docs update:
> https://lore.kernel.org/qemu-devel/20260627063208.33642-1-bin.meng@processmission.com/
> it's no longer supported by OpenBMC.
> 
> I didn't get any response from the Nuvoton maintainers in
> that thread, but I kind of favour deprecating the machine
> type in QEMU.
If nobody seems to care anymore, please go ahead and mark it as deprecated! 
We can still revert it in case someone speaks up later during the 
deprecation period.

  Thomas

Re: [PATCH 1/8] scripts/checkpatch: validate fixes against origin/master

[Pierrick Bouvier]

On 10/29/2026 8:13 AM, Alex Bennée wrote:
> The current check for a referenced fix commit relies on the developer
> keeping their master branch up to date which is not always grunted. If
> you create your branches directly from origin/master this will keep
> failing.
> 
> Given that origin/master should always be up to date (module the
> developer eschewing convention) lets use that in the test instead.
> 
> checkpatch complains about the line lengths but this seems to be
> standard for the perl script so I didn't try and fix it.
>

Or... you can use something else as origin, with origin/master not being
updated, and master as a tracking branch to upstream/master, and in this
case, this patch doesn't work.

Using master instead of origin/master has the advantage to not hardcode
what your remotes should be.

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  scripts/checkpatch.pl | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 82f62f60212..60e8b6bf7bc 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1913,9 +1913,9 @@ sub process {
>  				}
>  				if (which("git") ne "" && -e "$gitroot") {
>  					my $hash = defined($cid) ? $cid : $orig_commit;
> -					`git merge-base --is-ancestor $hash master 2>/dev/null`;
> +					`git merge-base --is-ancestor $hash origin/master 2>/dev/null`;
>  					if ($? != 0) {
> -						WARN("Fixes: commit $hash is not an ancestor of master\n" . $herecurr);
> +						WARN("Fixes: commit $hash is not an ancestor of origin/master\n" . $herecurr);
>  					}
>  				}
>  			}

Re: [PATCH 7/8] tests/functional: remove @skipSlowTest from quanta_gsj test

[Pierrick Bouvier]

On 10/29/2026 9:05 AM, Thomas Huth wrote:
> On 29/10/2026 17.38, Peter Maydell wrote:
>> On Thu, 29 Oct 2026 at 16:13, Alex Bennée <alex.bennee@linaro.org> wrote:
>>>
>>> The full quanta_gsj test would have caught the regression caused by
>>> the WFxT implementation and isn't that slow (<60s) it needs to be
>>> gated.
>>>
>>> Suggested-by: Thomas Huth <thuth@redhat.com>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
> ...
>> The other argument against using quanta-gsj as our test here
>> is that as discussed as part of a recent docs update:
>> https://lore.kernel.org/qemu-devel/20260627063208.33642-1-
>> bin.meng@processmission.com/
>> it's no longer supported by OpenBMC.
>>
>> I didn't get any response from the Nuvoton maintainers in
>> that thread, but I kind of favour deprecating the machine
>> type in QEMU.
> If nobody seems to care anymore, please go ahead and mark it as
> deprecated! We can still revert it in case someone speaks up later
> during the deprecation period.
> 
>  Thomas
> 
> 

For what it's worth, I also noticed that this test tends to timeout
randomly, even when running tests with -j1. So we might observe this in
our CI also, by enabling it by default.

Re: [PATCH 1/8] scripts/checkpatch: validate fixes against origin/master

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> writes:

> On 10/29/2026 8:13 AM, Alex Bennée wrote:
>> The current check for a referenced fix commit relies on the developer
>> keeping their master branch up to date which is not always grunted. If
>> you create your branches directly from origin/master this will keep
>> failing.
>> 
>> Given that origin/master should always be up to date (module the
>> developer eschewing convention) lets use that in the test instead.
>> 
>> checkpatch complains about the line lengths but this seems to be
>> standard for the perl script so I didn't try and fix it.
>>
>
> Or... you can use something else as origin, with origin/master not being
> updated, and master as a tracking branch to upstream/master, and in this
> case, this patch doesn't work.
>
> Using master instead of origin/master has the advantage to not hardcode
> what your remotes should be.

origin is the default when you clone a repo. We could get really complex
and query the tracking remote for the current branch. But if your deeply
forked then that obviates the test.

>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  scripts/checkpatch.pl | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 82f62f60212..60e8b6bf7bc 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -1913,9 +1913,9 @@ sub process {
>>  				}
>>  				if (which("git") ne "" && -e "$gitroot") {
>>  					my $hash = defined($cid) ? $cid : $orig_commit;
>> -					`git merge-base --is-ancestor $hash master 2>/dev/null`;
>> +					`git merge-base --is-ancestor $hash origin/master 2>/dev/null`;
>>  					if ($? != 0) {
>> -						WARN("Fixes: commit $hash is not an ancestor of master\n" . $herecurr);
>> +						WARN("Fixes: commit $hash is not an ancestor of origin/master\n" . $herecurr);
>>  					}
>>  				}
>>  			}

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 7/8] tests/functional: remove @skipSlowTest from quanta_gsj test

[Daniel P. Berrangé]

On Thu, Oct 29, 2026 at 09:14:21AM -0800, Pierrick Bouvier wrote:
> On 10/29/2026 9:05 AM, Thomas Huth wrote:
> > On 29/10/2026 17.38, Peter Maydell wrote:
> >> On Thu, 29 Oct 2026 at 16:13, Alex Bennée <alex.bennee@linaro.org> wrote:
> >>>
> >>> The full quanta_gsj test would have caught the regression caused by
> >>> the WFxT implementation and isn't that slow (<60s) it needs to be
> >>> gated.
> >>>
> >>> Suggested-by: Thomas Huth <thuth@redhat.com>
> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >>> ---
> > ...
> >> The other argument against using quanta-gsj as our test here
> >> is that as discussed as part of a recent docs update:
> >> https://lore.kernel.org/qemu-devel/20260627063208.33642-1-
> >> bin.meng@processmission.com/
> >> it's no longer supported by OpenBMC.
> >>
> >> I didn't get any response from the Nuvoton maintainers in
> >> that thread, but I kind of favour deprecating the machine
> >> type in QEMU.
> > If nobody seems to care anymore, please go ahead and mark it as
> > deprecated! We can still revert it in case someone speaks up later
> > during the deprecation period.
> > 
> >  Thomas
> > 
> > 
> 
> For what it's worth, I also noticed that this test tends to timeout
> randomly, even when running tests with -j1. So we might observe this in
> our CI also, by enabling it by default.

If it is unreliable then it should be tagged with skipFlakyTest,
and a gitlab issue filed with an example failure trace.

If no one steps up to fix the flaky test in a reasonable time
then it should later be deleted, as flaky tests should not be
allowed to linger for years.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH 1/8] scripts/checkpatch: validate fixes against origin/master

[Pierrick Bouvier]

On 10/29/2026 9:26 AM, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> writes:
> 
>> On 10/29/2026 8:13 AM, Alex Bennée wrote:
>>> The current check for a referenced fix commit relies on the developer
>>> keeping their master branch up to date which is not always grunted. If
>>> you create your branches directly from origin/master this will keep
>>> failing.
>>>
>>> Given that origin/master should always be up to date (module the
>>> developer eschewing convention) lets use that in the test instead.
>>>
>>> checkpatch complains about the line lengths but this seems to be
>>> standard for the perl script so I didn't try and fix it.
>>>
>>
>> Or... you can use something else as origin, with origin/master not being
>> updated, and master as a tracking branch to upstream/master, and in this
>> case, this patch doesn't work.
>>
>> Using master instead of origin/master has the advantage to not hardcode
>> what your remotes should be.
> 
> origin is the default when you clone a repo. We could get really complex
> and query the tracking remote for the current branch. But if your deeply
> forked then that obviates the test.
>

I am aware that origin is the default remote.
In my case, I always use "upstream" to reference the original project,
and "origin" for my personal fork. As a result, origin/master does not
always track upstream/master.

With git tracking branch, master can be setup to track whatever/master,
and not origin/master.

I would be surprised if I'm the only dev with such a setup, it's fairly
common for any fork based workflow, and why git has tracking branches.

This patch basically forces origin/master to be aligned with the
upstream, and nowhere else we enforce that, even if commit message says
the opposite.
```
Given that origin/master should always be up to date (module the
developer eschewing convention) lets use that in the test instead.
```

>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>  scripts/checkpatch.pl | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index 82f62f60212..60e8b6bf7bc 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -1913,9 +1913,9 @@ sub process {
>>>  				}
>>>  				if (which("git") ne "" && -e "$gitroot") {
>>>  					my $hash = defined($cid) ? $cid : $orig_commit;
>>> -					`git merge-base --is-ancestor $hash master 2>/dev/null`;
>>> +					`git merge-base --is-ancestor $hash origin/master 2>/dev/null`;
>>>  					if ($? != 0) {
>>> -						WARN("Fixes: commit $hash is not an ancestor of master\n" . $herecurr);
>>> +						WARN("Fixes: commit $hash is not an ancestor of origin/master\n" . $herecurr);
>>>  					}
>>>  				}
>>>  			}
> 

Re: [PATCH 1/8] scripts/checkpatch: validate fixes against origin/master

[Pierrick Bouvier]

On 10/29/2026 9:44 AM, Pierrick Bouvier wrote:
> On 10/29/2026 9:26 AM, Alex Bennée wrote:
>> Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> writes:
>>
>>> On 10/29/2026 8:13 AM, Alex Bennée wrote:
>>>> The current check for a referenced fix commit relies on the developer
>>>> keeping their master branch up to date which is not always grunted. If
>>>> you create your branches directly from origin/master this will keep
>>>> failing.
>>>>
>>>> Given that origin/master should always be up to date (module the
>>>> developer eschewing convention) lets use that in the test instead.
>>>>
>>>> checkpatch complains about the line lengths but this seems to be
>>>> standard for the perl script so I didn't try and fix it.
>>>>
>>>
>>> Or... you can use something else as origin, with origin/master not being
>>> updated, and master as a tracking branch to upstream/master, and in this
>>> case, this patch doesn't work.
>>>
>>> Using master instead of origin/master has the advantage to not hardcode
>>> what your remotes should be.
>>
>> origin is the default when you clone a repo. We could get really complex
>> and query the tracking remote for the current branch. But if your deeply
>> forked then that obviates the test.
>>
> 
> I am aware that origin is the default remote.
> In my case, I always use "upstream" to reference the original project,
> and "origin" for my personal fork. As a result, origin/master does not
> always track upstream/master.
> 
> With git tracking branch, master can be setup to track whatever/master,
> and not origin/master.
> 
> I would be surprised if I'm the only dev with such a setup, it's fairly
> common for any fork based workflow, and why git has tracking branches.
> 
> This patch basically forces origin/master to be aligned with the
> upstream, and nowhere else we enforce that, even if commit message says
> the opposite.
> ```
> Given that origin/master should always be up to date (module the
> developer eschewing convention) lets use that in the test instead.
> ```
>

The current patch is fine, I would just prefer to see it uses tracked
remote branch for master, instead of hardcoding origin/master (see git
branch -vv).

>>>
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> ---
>>>>  scripts/checkpatch.pl | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>>> index 82f62f60212..60e8b6bf7bc 100755
>>>> --- a/scripts/checkpatch.pl
>>>> +++ b/scripts/checkpatch.pl
>>>> @@ -1913,9 +1913,9 @@ sub process {
>>>>  				}
>>>>  				if (which("git") ne "" && -e "$gitroot") {
>>>>  					my $hash = defined($cid) ? $cid : $orig_commit;
>>>> -					`git merge-base --is-ancestor $hash master 2>/dev/null`;
>>>> +					`git merge-base --is-ancestor $hash origin/master 2>/dev/null`;
>>>>  					if ($? != 0) {
>>>> -						WARN("Fixes: commit $hash is not an ancestor of master\n" . $herecurr);
>>>> +						WARN("Fixes: commit $hash is not an ancestor of origin/master\n" . $herecurr);
>>>>  					}
>>>>  				}
>>>>  			}
>>
> 

Re: [PATCH 1/8] scripts/checkpatch: validate fixes against origin/master

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> writes:

> On 10/29/2026 9:26 AM, Alex Bennée wrote:
>> Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> writes:
>> 
>>> On 10/29/2026 8:13 AM, Alex Bennée wrote:
>>>> The current check for a referenced fix commit relies on the developer
>>>> keeping their master branch up to date which is not always grunted. If
>>>> you create your branches directly from origin/master this will keep
>>>> failing.
>>>>
>>>> Given that origin/master should always be up to date (module the
>>>> developer eschewing convention) lets use that in the test instead.
>>>>
>>>> checkpatch complains about the line lengths but this seems to be
>>>> standard for the perl script so I didn't try and fix it.
>>>>
>>>
>>> Or... you can use something else as origin, with origin/master not being
>>> updated, and master as a tracking branch to upstream/master, and in this
>>> case, this patch doesn't work.
>>>
>>> Using master instead of origin/master has the advantage to not hardcode
>>> what your remotes should be.
>> 
>> origin is the default when you clone a repo. We could get really complex
>> and query the tracking remote for the current branch. But if your deeply
>> forked then that obviates the test.
>>
>
> I am aware that origin is the default remote.
> In my case, I always use "upstream" to reference the original project,
> and "origin" for my personal fork. As a result, origin/master does not
> always track upstream/master.

So master in that case won't be correct either?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 1/8] scripts/checkpatch: validate fixes against origin/master

[Pierrick Bouvier]

On 10/29/2026 11:22 AM, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> writes:
> 
>> On 10/29/2026 9:26 AM, Alex Bennée wrote:
>>> Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> writes:
>>>
>>>> On 10/29/2026 8:13 AM, Alex Bennée wrote:
>>>>> The current check for a referenced fix commit relies on the developer
>>>>> keeping their master branch up to date which is not always grunted. If
>>>>> you create your branches directly from origin/master this will keep
>>>>> failing.
>>>>>
>>>>> Given that origin/master should always be up to date (module the
>>>>> developer eschewing convention) lets use that in the test instead.
>>>>>
>>>>> checkpatch complains about the line lengths but this seems to be
>>>>> standard for the perl script so I didn't try and fix it.
>>>>>
>>>>
>>>> Or... you can use something else as origin, with origin/master not being
>>>> updated, and master as a tracking branch to upstream/master, and in this
>>>> case, this patch doesn't work.
>>>>
>>>> Using master instead of origin/master has the advantage to not hardcode
>>>> what your remotes should be.
>>>
>>> origin is the default when you clone a repo. We could get really complex
>>> and query the tracking remote for the current branch. But if your deeply
>>> forked then that obviates the test.
>>>
>>
>> I am aware that origin is the default remote.
>> In my case, I always use "upstream" to reference the original project,
>> and "origin" for my personal fork. As a result, origin/master does not
>> always track upstream/master.
> 
> So master in that case won't be correct either?
> 

It's correct because it's setup as a tracking branch of upstream/master,
and not origin/master. See help for `git branch --set-upstream-to=` if
you're not familiar with it.

Thus why I'm suggesting to either leave master as it is (and correctly
update your personal local branch), or find which remote branch is
tracked for master (see git branch -vv) to not break existing workflows.