On Fri, 20 Jun 2025 18:01:08 +0100
Peter Maydell <peter.mayd...@linaro.org> wrote:
> On Fri, 20 Jun 2025 at 16:15, Igor Mammedov <imamm...@redhat.com> wrote:
> >
> > Reading QEMU_CLOCK_VIRTUAL is thread-safe.
> >
> > with CLI
> > -M q35,hpet=on -cpu host -enable-kvm -smp 240,sockets=4
> > patch makes WS2025 guest, that was able to boot in 4/2min, to boot in
> > 2/1min.
> >
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > ---
> > hw/timer/hpet.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> > index 0fd1337a15..1ebd715529 100644
> > --- a/hw/timer/hpet.c
> > +++ b/hw/timer/hpet.c
> > @@ -681,6 +681,8 @@ static void hpet_init(Object *obj)
> >
> > /* HPET Area */
> > memory_region_init_io(&s->iomem, obj, &hpet_ram_ops, s, "hpet",
> > HPET_LEN);
> > + memory_region_enable_lockless_ro_io(&s->iomem);
> > + s->iomem.disable_reentrancy_guard = true;
> > sysbus_init_mmio(sbd, &s->iomem);
>
> Is this sequence possible?
While unlikely (what I observe from Linux/Windows guest,
they enable timer 1st and only then start readers),
but yes it still possible.
The more convoluted is the switch into disabled state, where
1:
reader is already in enabled branch and preempted before
reading clock:
case HPET_COUNTER:
if (hpet_enabled(s)) {
<yield>
cur_tick = hpet_get_ticks(s);
2:
writer saves s->hpet_counter = <now>
3:
reader resumes and reads newer 'now'
4:
on next counter read, reader switches to disabled branch
and sees timer jump back to older s->hpet_counter value.
and that shouldn't happen.
for this to work s->hpet_counter needs to catch up
the latest read timer value.
Let me try and see what could be done with it.
>
> thread A
> takes the BQL
> enters hpet_ram_write() for HPET_CFG to set ENABLE
> executes s->config = new_val (setting the ENABLE bit in it)
> context switch before it gets to the point of setting
> s->hpet_offset
>
> thread B
> enters hpet_ram_read() for HPET_COUNTER (which it can now
> do because it doesn't need the BQL)
> hpet_enabled() returns true (it tests s->config)
> calls hpet_get_ticks which adds hpet_offset to the clock,
> but hpet_offset has not been correctly set by A yet
>
> thanks
> -- PMM
>
