Re: [PATCH 10/22] rust/hpet: Abstract HPETTimerRegisters struct

[Paolo Bonzini <pbonzini@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]

Il ven 14 nov 2025, 05:15 Zhao Liu <zhao1.liu@intel.com> ha scritto:

> Yes, this will reduce BQL context in lockless IO a lot. And I'll based
> on your patch to extract HPETTimer from BqlRefCell.
>
> > Preserving the old migration format can then be solved in two ways:
> >
> > 1) with a handwritten ToMigrationStateShared implementation for
> > HPETTimer (and marking the tn_regs array as #[migration_state(omit)])
>
> Yes, compared with 2), I also this is the better choice, which looks
> more common and could be an good example for other device.
>
> > 2) by also adding num_timers_save and the timer's expiration to
> > HPETRegisters and HPETTimerRegisters, respectively.
> >
> > I think I prefer the former, but I haven't written the code so it's
> > not my choice. :)
>

Yes, it is much better.

> @@ -181,6 +181,9 @@ fn timer_handler(timer_cell: &BqlRefCell<HPETTimer>) {
> >  #[repr(C)]
> >  #[derive(Debug, Default, ToMigrationState)]
> >  pub struct HPETTimerRegisters {
> > +    // Only needed for migration
> > +    index: u8,
>
> I didn't get the point why we need to migrate this "index" instead of
> HPETTimer::index?
>

Just because it's still migrating HPETTimerRegisters, but yes it's not
necessary with the custom ToMigrationStateShared, as you write below.

Paolo


> Anyway, if we continue to keep index in HPETTimerRegisters, we can make
> it have usize type with a "#[migration_state(try_into(u8))]" property.
>
> If we just HPETTimer::index for migration, I think it's possible to do
> type convertion in handwritten ToMigrationStateShared.
>
> Thanks,
> Zhao
>
>

[-- Attachment #2: Type: text/html, Size: 2738 bytes --]