This state continues the cleanups of the HPET state, moving fields out of
BqlCells and into HPETRegisters and HPETTimerRegisters.  It also restores
the old migration format and shows an interesting trick: HPETTimer is now
a very simple object that handles the "unsafe" backreference from the
timer to the HPETState, but it also implements ToMigrationStateShared
and is stored in the HPETState as Migratable<[HPETTimer; N]>.  I find
it pretty cool that the composition works naturally.

The less beautiful part is that I had to modify Timer::init_full for
this to compile.  It's probably time to work on the final design for
initialization, because this is becoming very ad hoc and the differences
between timer, MemoryRegion and Clock initialization have no real
justification.

I'm leaving out the conversion to Mutex because, as Zhao noticed, it
has a deadlock - the timer callback tries to grab the HPET mutex inside
the BQL, where as the vCPU tries to grab the BQL inside the HPET mutex.
This is not present in the C code only because... it doesn't take the
lock at all in places where it should.  In particular hpet_timer() reads
and writes t->cmp and t->cmp64 outside the lock, while hpet_ram_write()
does so within the lock via hpet_set_timer().

Patch 4 ("rust: migration: implement ToMigrationState for Timer") is
still incomplete, and provided here as a starting point.

Please review!

Paolo

Paolo Bonzini (4):
  rust/hpet: move hidden registers to HPETTimerRegisters
  rust/hpet: move hpet_offset to HPETRegisters
  rust/hpet: remove BqlRefCell around HPETTimer
  rust: migration: implement ToMigrationState for Timer

Zhao Liu (1):
  rust/hpet: Apply Migratable<> wrapper and ToMigrationState

 rust/hw/timer/hpet/src/device.rs | 377 ++++++++++++++++++-------------
 rust/migration/src/migratable.rs |  31 +++
 rust/util/src/timer.rs           |  22 +-
 3 files changed, 264 insertions(+), 166 deletions(-)

-- 
2.51.1

On Mon, Nov 17, 2025 at 09:47:47AM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Nov 2025 09:47:47 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 0/5] rust/hpet: complete moving state out of HPETTimer
> X-Mailer: git-send-email 2.51.1
> 
> This state continues the cleanups of the HPET state, moving fields out of
> BqlCells and into HPETRegisters and HPETTimerRegisters.  It also restores
> the old migration format and shows an interesting trick: HPETTimer is now
> a very simple object that handles the "unsafe" backreference from the
> timer to the HPETState, but it also implements ToMigrationStateShared
> and is stored in the HPETState as Migratable<[HPETTimer; N]>.  I find
> it pretty cool that the composition works naturally.
> 
> The less beautiful part is that I had to modify Timer::init_full for
> this to compile.  It's probably time to work on the final design for
> initialization, because this is becoming very ad hoc and the differences
> between timer, MemoryRegion and Clock initialization have no real
> justification.

<Just some rough thoughts/understanding>

Yes, Timer requires Pin<> and it seems MemoryRegion also should requires
it because of callback...

MemoryRegion requires MaybeUninitField, but it would be not necessary
for HPET, since HPETTimer has already initialized its other field before
calling init_full(). But as a general interface, MaybeUninitField could
be also useful for Timer, as we can't require how device initialize its
field.

Clock's ParentInit<> is also different. We don't need to require
Timer::init_full must be called in init(). MemoryRegion may also don't
need it And some C device even calls memory_region_init_io() in
realize().

Regards,
Zhao