On Tue, Sep 8, 2026 at 10:40 AM Peter Maydell <[email protected]> wrote: > > On Mon, 7 Sept 2026 at 22:42, Paolo Bonzini <[email protected]> wrote: > > > > On 9/2/26 20:02, Philippe Mathieu-Daudé wrote: > > >>> 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? > > Unfortunately not, because apic_update_irq() is needed to call > > cpu_interrupt() and break out of halt. > > > > By the way... cpu_has_work_after_processing_async_events() is a bit > > long, what about simply cpu_poll_while_halted()? > > > > Answering Peter's question above, the way in which the code is protected > > against the BQL is basically two-fold: 1) kicking the CPU out of > > whatever it's doing, for example in tcg_handle_interrupt, 2) changing > > what other conditions affect cpu_has_work() only in the vCPU thread, > > e.g. with run_on_cpu. > > Ah, so it is called without the BQL held? That's quite unusual > for CPU methods, so we should clearly document that.
I agree. > I bet also that most targets don't expect it. Picking one at > random, the sparc has_work function calls e.g. cpu_interrupts_enabled() > which does not look like a function that is expecting to be called > without the BQL. Why? It only accesses fields for the local CPU. The problem is interprocedural communication, and only if it does not use cpu_interrupt(). Alpha -> ok, interrupts Arm -> see above AVR -> ok, interrupts + registers Hexagon -> ok, global registers writes use run_on_cpu afaict HPPA -> ok, interrupts i386 -> complex but ok - though it should use qatomic_load_acquire loongarch -> ok, interrupts + registers m68k -> ok, interrupts microblaze -> ok, interrupts mips -> i think ok, interrupts + registers openrisc -> ok, interrupts riscv -> sort of ok, uses BQL for register writes but should use qatomic_read/set as well rx -> ok, interrupts s390 -> probably wrong, s390_cpu_set_state sometimes not on CPU thread? So the only one that is blatantly wrong is s390; Arm is slightly wrong, i386 and riscv are technically wrong but have correct locking. > Is it OK for a has_work function to just take the BQL across > what it's doing if it doesn't want to be bothered with trying > to make the handling truly thread safe without a lock? That > seems to me like it would be a lot simpler and more reliable. The simplest way to write has_work is to make it use only cpu_test_interrupt and checks on local registers. That is, use the synchronization provided by common code. Paolo
