I think this is extremely useful to show where we could go in the task
of creating more idiomatic bindings.

On Tue, Jun 11, 2024 at 12:34 PM Manos Pitsidianakis
<manos.pitsidiana...@linaro.org> wrote:
> +fn main() {
> +    println!("cargo::rerun-if-env-changed=MESON_BUILD_DIR");
> +    println!("cargo::rerun-if-env-changed=MESON_BUILD_ROOT");
> +    println!("cargo::rerun-if-changed=src/generated.rs.inc");

Why do you need this rerun-if-changed?

> +pub const PL011_ARM_INFO: TypeInfo = TypeInfo {
> +    name: TYPE_PL011.as_ptr(),
> +    parent: TYPE_SYS_BUS_DEVICE.as_ptr(),
> +    instance_size: core::mem::size_of::<PL011State>(),
> +    instance_align: core::mem::align_of::<PL011State>(),
> +    instance_init: Some(pl011_init),
> +    instance_post_init: None,
> +    instance_finalize: None,
> +    abstract_: false,
> +    class_size: 0,
> +    class_init: Some(pl011_class_init),
> +    class_base_init: None,
> +    class_data: core::ptr::null_mut(),
> +    interfaces: core::ptr::null_mut(),
> +};

QOM is certainly a major part of what we need to do for idiomatic
bindings. This series includes both using QOM objects (chardev) and
defining them.

For using QOM objects, there is only one strategy that we need to
implement for both Chardev and DeviceState/SysBusDevice which is nice.
We can probably take inspiration from glib-rs, see for example
- https://docs.rs/glib/latest/glib/object/trait.Cast.html
- https://docs.rs/glib/latest/glib/object/trait.ObjectType.html
- https://docs.rs/glib/latest/glib/object/struct.ObjectRef.html

For definining new classes I think it's okay if Rust does not support
writing superclasses yet, only leaves.

I would make a QOM class written in Rust a struct that only contains
the new fields. The struct must implement Default and possibly Drop
(for finalize).

  struct PL011_Inner {
    iomem: MemoryRegion,
    readbuff: u32.
    ...
  }

and then a macro defines a wrapper struct that includes just two
fields, one for the superclass and one for the Rust struct.
instance_init can initialize the latter with Default::default().

  struct PL011 {
    parent_obj: qemu::bindings::SysBusDevice,
    private: PL011_Inner,
  }

"private" probably should be RefCell<PL011_Inner>, avoiding the unsafe

    state.as_mut().read(addr, size)

There should also be macros to define the wrappers for MMIO MemoryRegions.

> +    pub fn realize(&mut self) {
> +        unsafe {
> +            qemu_chr_fe_set_handlers(
> +                addr_of_mut!(self.char_backend),
> +                Some(pl011_can_receive),
> +                Some(pl011_receive),
> +                Some(pl011_event),
> +                None,
> +                addr_of_mut!(*self).cast::<c_void>(),
> +                core::ptr::null_mut(),
> +                true,
> +            );
> +        }

Probably each set of callbacks (MemoryRegion, Chardev) needs to be a
struct that implements a trait.

> +#[macro_export]
> +macro_rules! define_property {
> +    ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr, default = 
> $defval:expr) => {
> +        $crate::generated::Property {
> +            name: $name,
> +            info: $prop,
> +            offset: ::core::mem::offset_of!($state, $field) as _,
> +            bitnr: 0,
> +            bitmask: 0,
> +            set_default: true,
> +            defval: $crate::generated::Property__bindgen_ty_1 { u: 
> $defval.into() },
> +            arrayoffset: 0,
> +            arrayinfo: ::core::ptr::null(),
> +            arrayfieldsize: 0,
> +            link_type: ::core::ptr::null(),
> +        }
> +    };

Perhaps we can define macros similar to the C DEFINE_PROP_*, and const
functions can be used to enforce some kind of type safety.

pub const fn check_type<T>(_x: &T, y: T) -> T { y }

static VAR: i16 = 42i16;
static TEST: i8 = check_type(&VAR, 43i8);

pub fn main() { println!("{}", TEST); }

error[E0308]: mismatched types
 --> ff.rs:4:30
  |
4 | static TEST: i8 = check_type(&VAR, 43i8);
  |                   ---------- ^^^^ expected `&i8`, found `&i16`
  |                   |
  |                   arguments to this function are incorrect
  |
  = note: expected reference `&i8`
             found reference `&i16`

Anyhow I think this is already very useful, because as the
abstractions are developed, it is possible to see how the device code
evolves.

Paolo


Reply via email to