Re: [RFC 07/13] rust: add bindings for timer

[Paolo Bonzini <pbonzini@redhat.com>]

On 12/5/24 07:07, Zhao Liu wrote:
> +impl QEMUTimer {
> +    fn new() -> Self {
> +        QEMUTimer {
> +            expire_time: 0,
> +            timer_list: ::core::ptr::null_mut(),
> +            cb: None,
> +            opaque: ::core::ptr::null_mut(),
> +            next: ::core::ptr::null_mut(),
> +            attributes: 0,
> +            scale: 0,
> +        }
> +    }

Please implement Zeroable instead and make this just Self::ZERO.

> +    // TODO: Consider how to avoid passing in C style callbacks directly.
> +    fn timer_new_full<T: QEMUTimerImpl>(
> +        timer_list_group: Option<&mut QEMUTimerListGroup>,
> +        clk_type: QEMUClockType,
> +        scale: u32,
> +        attributes: u32,
> +        opaque: &mut T::Opaque,

This gets tricky when you have more than one timer per device.  With the right
infrastructure we can make this something like

     fn timer_init_full<'a, 'b: 'a, T, F: 'b Fn(&'b T)>(
         &'a mut self,
         timer_list_group: Option<&mut QEMUTimerListGroup>,
         clk_type: QEMUClockType,
         scale: u32,
         attributes: u32,
         f: &F,
         opaque: &'b T) {
         // SAFETY: the opaque outlives the timer
         unsafe {
             timer_init_full(...)
         }
     }

So I guess that's another thing I have to extract and post. :)

BTW don't bother with timer_new, only do the non-pointer timer_init.

> +    pub fn timer_del(&mut self) {
> +        unsafe { timer_del(self as *mut QEMUTimer) };
> +    }
> +}

Please also add a Drop implementation that calls timer_del.

> +pub fn qemu_clock_get_virtual_ns() -> u64 {
> +    // SAFETY:
> +    // Valid parameter.
> +    (unsafe { qemu_clock_get_ns(QEMUClockType::QEMU_CLOCK_VIRTUAL) }) as u64
> +}

Maybe something like

pub struct Clock {
     id: QEMUClockType
}

impl Clock {
     pub fn get_ns() -> u64 {
         // SAFETY: cannot be created outside this module, therefore id
         // is valid
         (unsafe { qemu_clock_get_ns(self.id) }) as u64
     }
}

pub static CLOCK_VIRTUAL: Clock = Clock { id: QEMUClockType::QEMU_CLOCK_VIRTUAL };
// etc.

and then the user can do timer::CLOCK_VIRTUAL::get_ns().

Paolo