qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC 04/13] rust: add bindings for gpio_{in|out} initialization


From: Zhao Liu
Subject: Re: [RFC 04/13] rust: add bindings for gpio_{in|out} initialization
Date: Thu, 16 Jan 2025 11:04:35 +0800

> and the structure of all the blanket implementation is always the same:
> 
> pub trait DeviceClassMethods: IsA<DeviceState> {...}
> impl<T> DeviceClassMethods for T where T: IsA<DeviceState> {}
> 
> pub trait DeviceMethods: ObjectDeref
> where
>     Self::Target: IsA<DeviceState>,
> {...}
> impl<R: ObjectDeref> DeviceMethods for R where R::Target: IsA<DeviceState> {}
> 
> impl<T> ClassInitImpl<DeviceClass> for T
> where
>     T: ClassInitImpl<ObjectClass> + DeviceImpl
> {...}
> 

Similiar to timer, now I've also worked out the gpio bindings (but one
thing that's different is that it uses `self` as an parameter) like:

* gpio_in and gpio_out:

/// Trait for methods of [`DeviceState`] and its subclasses.
pub trait DeviceMethods: ObjectDeref
where
    Self::Target: IsA<DeviceState>,
{
    fn init_gpio_in<F>(&self, lines_num: u32, _f: F)
    where
        F: for<'a> FnCall<(&'a Self::Target, u32, u32)>,
    {
        unsafe extern "C" fn rust_irq_handler<T, F: for<'a> FnCall<(&'a T, u32, 
u32)>>(
            opaque: *mut c_void,
            lines_num: c_int,
            level: c_int,
        ) {
            // SAFETY: the opaque was passed as a reference to `T`
            F::call((
                unsafe { &*(opaque.cast::<T>()) },
                lines_num as u32,
                level as u32,
            ))
        }

        let gpio_in_cb: unsafe extern "C" fn(*mut c_void, c_int, c_int) =
            rust_irq_handler::<Self::Target, F>;

        unsafe {
            qdev_init_gpio_in(
                self.upcast::<DeviceState>() as *const DeviceState as *mut 
DeviceState,
                Some(gpio_in_cb),
                lines_num as c_int,
            );
        }
    }

    fn init_gpio_out(&self, pins: &[InterruptSource]) {
        assert!(pins.len() > 0);

        unsafe {
            qdev_init_gpio_out(
                self.upcast::<DeviceState>() as *const DeviceState as *mut 
DeviceState,
                pins[0].as_ptr(),
                pins.len() as c_int,
            );
        }
    }
}

impl<R: ObjectDeref> DeviceMethods for R where R::Target: IsA<DeviceState> {}


* Use case in HPET:

impl HPETState {
    ...

    fn handle_legacy_irq(&self, irq: u32, level: u32) {
        if irq == HPET_LEGACY_PIT_INT {
            if !self.is_legacy_mode() {
                self.irqs[0].set(level != 0);
            }
        } else {
            self.rtc_irq_level.set(level as u8);
            if !self.is_legacy_mode() {
                self.irqs[RTC_ISA_IRQ].set(level != 0);
            }
        }
    }

    ...

    fn realize(&self) {
        ...
        self.init_gpio_in(2, HPETState::handle_legacy_irq);
        self.init_gpio_out(from_ref(&self.pit_enabled));
    }
}

---

I made the handler accept the inmuttable reference, but init_gpio_in()
is called in realize(), which now accepts the `&mut self`.

I think init_gpio_in() should be called in realize() so that realize()
needs to become safe in advance (before your plan).

The safe realize() mainly affects pl011, and before you formally
introduce char binding, if you don't mind, I can make this change to
pl011:

-    pub fn realize(&mut self) {
+    pub fn realize(&self) {
         // SAFETY: self.char_backend has the correct size and alignment for a
         // CharBackend object, and its callbacks are of the correct types.
         unsafe {
             qemu_chr_fe_set_handlers(
-                addr_of_mut!(self.char_backend),
+                addr_of!(self.char_backend) as *mut CharBackend,
                 Some(pl011_can_receive),
                 Some(pl011_receive),
                 Some(pl011_event),
                 None,
-                addr_of_mut!(*self).cast::<c_void>(),
+                addr_of!(*self).cast::<c_void>() as *mut c_void,
                 core::ptr::null_mut(),
                 true,
             );

Thanks,
Zhao





reply via email to

[Prev in Thread] Current Thread [Next in Thread]