On 9/10/25 16:25, Igor Mammedov wrote:
Commit [1] made qemu fail with abort:
xen_evtchn_set_gsi: Assertion `bql_locked()' failed.
when running ./tests/functional/x86_64/test_kvm_xen.py tests.
To fix it make sure that BQL is held when manipulating IRQs.
Queued, but I think it's time to allow qemu_irq handlers to run outside
the BQL.
The idea would be to add qemu_allocate_irq_unlocked and
qemu_allocate_irqs_unlocked, and add a boolean to IRQState. If the
boolean is set (the default), qemu_set_irq would wrap the callback with
bql_lock()/bql_unlock().
Together with the MMIO support that you introduced, this would allow
making interrupt controllers BQL-free.
Easier thread-safety would also, but I digress, provide a good reason to
write even the simple devices in Rust---that's in my opinion an even
better reason than memory safety, for which we already have developed
lots of abstractions in QEMU.
Paolo
Fixes: 7defb58baf (hpet: switch to fine-grained device locking)
Reported-by: Daniel P. Berrangé <berra...@redhat.com>
Signed-off-by: Igor Mammedov <imamm...@redhat.com>
---
hw/timer/hpet.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 789a31d0a0..1acba4fa9d 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -40,6 +40,7 @@
#include "qom/object.h"
#include "qemu/lockable.h"
#include "qemu/seqlock.h"
+#include "qemu/main-loop.h"
#include "trace.h"
struct hpet_fw_config hpet_fw_cfg = {.count = UINT8_MAX};
@@ -222,12 +223,15 @@ static void update_irq(struct HPETTimer *timer, int set)
timer->fsb & 0xffffffff,
MEMTXATTRS_UNSPECIFIED,
NULL);
} else if (timer->config & HPET_TN_TYPE_LEVEL) {
+ BQL_LOCK_GUARD();
qemu_irq_raise(s->irqs[route]);
} else {
+ BQL_LOCK_GUARD();
qemu_irq_pulse(s->irqs[route]);
}
} else {
if (!timer_fsb_route(timer)) {
+ BQL_LOCK_GUARD();
qemu_irq_lower(s->irqs[route]);
}
}
@@ -534,10 +538,12 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
/* i8254 and RTC output pins are disabled
* when HPET is in legacy mode */
if (activating_bit(old_val, new_val, HPET_CFG_LEGACY)) {
+ BQL_LOCK_GUARD();
qemu_set_irq(s->pit_enabled, 0);
qemu_irq_lower(s->irqs[0]);
qemu_irq_lower(s->irqs[RTC_ISA_IRQ]);
} else if (deactivating_bit(old_val, new_val, HPET_CFG_LEGACY)) {
+ BQL_LOCK_GUARD();
qemu_irq_lower(s->irqs[0]);
qemu_set_irq(s->pit_enabled, 1);
qemu_set_irq(s->irqs[RTC_ISA_IRQ], s->rtc_irq_level);
@@ -683,11 +689,13 @@ static void hpet_handle_legacy_irq(void *opaque, int n,
int level)
if (n == HPET_LEGACY_PIT_INT) {
if (!hpet_in_legacy_mode(s)) {
+ BQL_LOCK_GUARD();
qemu_set_irq(s->irqs[0], level);
}
} else {
s->rtc_irq_level = level;
if (!hpet_in_legacy_mode(s)) {
+ BQL_LOCK_GUARD();
qemu_set_irq(s->irqs[RTC_ISA_IRQ], level);
}
}
