Re: [PATCH 3/5] rust/hpet: remove BqlRefCell around HPETTimer

[Paolo Bonzini <pbonzini@redhat.com>]

On 11/19/25 16:17, Zhao Liu wrote:
>> -    fn init_timer_with_cell(cell: &BqlRefCell<Self>) {
>> -        let mut timer = cell.borrow_mut();
>> -        // SAFETY: HPETTimer is only used as part of HPETState, which is
>> -        // always pinned.
>> -        let qemu_timer = unsafe { Pin::new_unchecked(&mut timer.qemu_timer) };
>> -        qemu_timer.init_full(None, CLOCK_VIRTUAL, Timer::NS, 0, timer_handler, cell);
>> +    fn init_timer(timer: Pin<&mut Self>) {
>> +        Timer::init_full(
>> +            timer,
>> +            None,
>> +            CLOCK_VIRTUAL,
>> +            Timer::NS,
>> +            0,
>> +            timer_handler,
>> +            |t| &mut t.qemu_timer,
>> +        );
>>       }
> 
> I find this way could also work for BqlRefCell case:
> 
>      fn init_timer_with_cell(cell: &mut BqlRefCell<Self>) {
>          // SAFETY: HPETTimer is only used as part of HPETState, which is
>          // always pinned.
>          let timer = unsafe { Pin::new_unchecked(cell) };
>          Timer::init_full(
>              timer,
>              None,
>              CLOCK_VIRTUAL,
>              Timer::NS,
>              0,
>              timer_handler,
>              |t| {
>                  assert!(bql::is_locked());
>                  &mut t.get_mut().qemu_timer
>              },
>          );
>      }

Yes, but I'm still not sure what the final shape will be... I don't like 
this too much.

> 
> So any other non-lockless timer can also use this interface to
> initialize their BqlRefCell<>.
> 
> (BTW, I find BqlRefCell::get_mut() / as_ref() missed bql::is_locked().
>   right?)

as_ptr() doesn't need it because the pointer can be dereferenced later.

As to get_mut()... I think if you are the only owner, you should have 
the guarantee of being able to modify the content freely.  This is true 
even if your &mut came from interior mutability.

So, in the above case you only need &mut t.get_mut().qemu_timer.

> I think it may be better to add doc about how to use this for
> BqlRefCell<> case since there'll be no example after this patch.