Re: [PATCH 3/5] rust/hpet: remove BqlRefCell around HPETTimer
Paolo Bonzini <[email protected]>
| Newsgroups | org.nongnu.qemu-rust,org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
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.