On 20/8/26 09:29, Philippe Mathieu-Daudé wrote:
On 2026-08-19 23:40, Peter Maydell wrote:
On Wed, 19 Aug 2026 at 21:14, Richard Henderson
<[email protected]> wrote:
On 8/19/26 07:56, Philippe Mathieu-Daudé wrote:
Refactor the target-specific halt-to-execution transition
logic (disable WFE/WFI timers, clear the halt_reason flag)
into a separate arm_cpu_transition_halt_to_exec() function.
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
---
target/arm/cpu.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 77aa78f00e2..8e9b584559d 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -870,18 +870,26 @@ static bool
arm_cpu_internal_is_big_endian(CPUState *cs)
}
#ifdef CONFIG_TCG
+static void arm_cpu_transition_halt_to_exec(CPUState *cs)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+
+ assert(cpu_has_work(cs));
In reply to patch 11 you suggest removing this.
I wonder why you added it in the first place.
Does the accel loop enforce that we hold the BQL when
arm_cpu_exec_halt() is called?
For MTTCG:
mttcg_cpu_thread_fn()
{
bql_lock();
while (1)
qemu_process_cpu_events(cpu);
if (cpu_can_run(cpu)) {
bql_unlock();
tcg_cpu_exec(cpu);
cpu_exec(cpu);
if (cpu->halted) {
cpu_has_work_after_processing_async_events(cpu);
process_async_events(cpu);
x86_cpu_process_async_events(); // APIC takes BQL
cpu_has_work(cpu);
transition_halt_to_exec(cpu);
arm_cpu_transition_halt_to_exec(); // timer_lock
x86_cpu_transition_halt_to_exec();
...
bql_lock();
...
}
void x86_cpu_process_async_events()
{
/* (BQL not held) */
if (cpu_test_interrupt()) {
bql_lock();
apic_poll_irq();
apic_sync_vapic();
apic_update_irq();
cpu_interrupt();
cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
bql_unlock();
}
}
If not then the assertion
is racy, because something might get in and e.g. lower
an IRQ line so that cpu_has_work() is no longer true
(which would be fine -- it just means we made the decision
to wake up and that won vs the incoming interrupt).
On the one hand, if we don't have the BQL in has_work and
exec_halt then we should be a lot more careful about how
we code them (e.g. use of the right kind of atomics, and
there's no way the call to do_interrupt_all() in the x86
exec_halt can be safe without the BQL, surely).
On the other hand, if we do have the BQL in exec_halt
then why is the x86 implementation explicitly taking
the BQL when it calls apic_poll_irq()?
Trying harder, my understanding is apic_poll_irq() only requires
the BQL for the apic_update_irq() call, not apic_sync_vapic().
So maybe only apic_sync_vapic() belongs to process_async_events(),
leaving apic_update_irq() to transition_halt_to_exec().
Paolo does that sound like going in the good direction?
