Re: [PULL 24/28] hpet: switch to fine-grained device locking

["Daniel P. Berrangé" <berrange@redhat.com>]

Hi,

This patches causes a regression making QEMU  abort in the KVM Xen
functional test.

To reproduce please run 'make check-functional-x86_64', or more
specifically run this single test:

 QEMU_TEST_QEMU_BINARY=./build/qemu-system-x86_64 PYTHONPATH=./python:./tests/functional ./tests/functional/x86_64/test_kvm_xen.py

though I recommend you first add this series:

  https://lists.nongnu.org/archive/html/qemu-devel/2025-09/msg01540.html

as that fixes an infinite loop in the functional test console
code on QEMU abnormal premature exit.

In the test logs we see the following on the serial console:

  2025-09-08 15:26:32,286: Cannot get hvm parameter CONSOLE_EVTCHN (18): -22!

and then the following on stder:

  qemu-system-x86_64: ../hw/i386/kvm/xen_evtchn.c:1619: xen_evtchn_set_gsi: Assertion `bql_locked()' failed.

The QEMU command line was:

  2025-09-08 15:26:32,097 - DEBUG: VM launch command: './build/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -chardev socket,id=console,fd=10 -serial chardev:console -accel kvm,xen-version=0x4000a,kernel-irqchip=split -smp 2 -kernel /var/home/berrange/.cache/qemu/download/ec0ad7bb8c33c5982baee0a75505fe7dbf29d3ff5d44258204d6307c6fe0132a -append "printk.time=0 root=/dev/xvda console=ttyS0 quiet xen_emul_unplug=ide-disks" -drive file=/var/home/berrange/.cache/qemu/download/b11045d649006c649c184e93339aaa41a8fe20a1a86620af70323252eb29e40b,if=none,snapshot=on,format=raw,id=drv0 -device xen-disk,drive=drv0,vdev=xvda -device virtio-net-pci,netdev=unet -netdev user,id=unet,hostfwd=:127.0.0.1:0-:22'

On Fri, Aug 29, 2025 at 02:59:31PM +0200, Paolo Bonzini wrote:
> From: Igor Mammedov <imammedo@redhat.com>
> 
> as a step towards lock-less HPET counter read,
> use per device locking instead of BQL.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Link: https://lore.kernel.org/r/20250814160600.2327672-4-imammedo@redhat.com
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/timer/hpet.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> index cb48cc151f1..ab5aa59ae4e 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -38,6 +38,7 @@
>  #include "hw/timer/i8254.h"
>  #include "system/address-spaces.h"
>  #include "qom/object.h"
> +#include "qemu/lockable.h"
>  #include "trace.h"
>  
>  struct hpet_fw_config hpet_fw_cfg = {.count = UINT8_MAX};
> @@ -69,6 +70,7 @@ struct HPETState {
>      SysBusDevice parent_obj;
>      /*< public >*/
>  
> +    QemuMutex lock;
>      MemoryRegion iomem;
>      uint64_t hpet_offset;
>      bool hpet_offset_saved;
> @@ -428,6 +430,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
>      trace_hpet_ram_read(addr);
>      addr &= ~4;
>  
> +    QEMU_LOCK_GUARD(&s->lock);
>      /*address range of all global regs*/
>      if (addr <= 0xff) {
>          switch (addr) {
> @@ -482,6 +485,7 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
>      int len = MIN(size * 8, 64 - shift);
>      uint64_t old_val, new_val, cleared;
>  
> +    QEMU_LOCK_GUARD(&s->lock);
>      trace_hpet_ram_write(addr, value);
>      addr &= ~4;
>  
> @@ -679,8 +683,10 @@ static void hpet_init(Object *obj)
>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>      HPETState *s = HPET(obj);
>  
> +    qemu_mutex_init(&s->lock);
>      /* HPET Area */
>      memory_region_init_io(&s->iomem, obj, &hpet_ram_ops, s, "hpet", HPET_LEN);
> +    memory_region_enable_lockless_io(&s->iomem);
>      sysbus_init_mmio(sbd, &s->iomem);
>  }
>  
> -- 
> 2.51.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|