On Thu, 24 Oct 2024 at 18:14, Alex Bennée <alex.ben...@linaro.org> wrote:
>
> Manos Pitsidianakis <manos.pitsidiana...@linaro.org> writes:
>
> > Add a new derive procedural macro to declare device models. Add
> > corresponding DeviceImpl trait after already existing ObjectImpl trait.
> > At the same time, add instance_init, instance_post_init,
> > instance_finalize methods to the ObjectImpl trait and call them from the
> > ObjectImplUnsafe trait, which is generated by the procedural macro.
> >
> > This allows all the boilerplate device model registration to be handled
> > by macros, and all pertinent details to be declared through proc macro
> > attributes or trait associated constants and methods.
> >
> > The device class can now be generated automatically and the name can be
> > optionally overridden:
> >
> > ------------------------ >8 ------------------------
> > #[repr(C)]
> > #[derive(Debug, qemu_api_macros::Object, qemu_api_macros::Device)]
> > #[device(class_name_override = PL011Class)]
> > /// PL011 Device Model in QEMU
> > pub struct PL011State {
> > ------------------------ >8 ------------------------
> >
> > Properties are now marked as field attributes:
> >
> > ------------------------ >8 ------------------------
> > #[property(name = c"chardev", qdev_prop = qdev_prop_chr)]
> > pub char_backend: CharBackend,
> > ------------------------ >8 ------------------------
> >
> > Object methods (instance_init, etc) methods are now trait methods:
> >
> > ------------------------ >8 ------------------------
> > /// Trait a type must implement to be registered with QEMU.
> > pub trait ObjectImpl {
> > type Class: ClassImpl;
> > const TYPE_NAME: &'static CStr;
> > const PARENT_TYPE_NAME: Option<&'static CStr>;
> > const ABSTRACT: bool;
> >
> > unsafe fn instance_init(&mut self) {}
> > fn instance_post_init(&mut self) {}
> > fn instance_finalize(&mut self) {}
> > }
> > ------------------------ >8 ------------------------
> >
> > Device methods (realize/reset etc) are now safe idiomatic trait methods:
> >
> > ------------------------ >8 ------------------------
> > /// Implementation methods for device types.
> > pub trait DeviceImpl: ObjectImpl {
> > fn realize(&mut self) {}
> > fn reset(&mut self) {}
> > }
> > ------------------------ >8 ------------------------
> >
> > The derive Device macro is responsible for creating all the extern "C" FFI
> > functions that QEMU needs to call these methods.
> >
> > Signed-off-by: Manos Pitsidianakis <manos.pitsidiana...@linaro.org>
> > ---
> > rust/hw/char/pl011/src/device.rs | 124 +++-----
> > rust/hw/char/pl011/src/device_class.rs | 70 -----
> > rust/hw/char/pl011/src/lib.rs | 1 -
> > rust/qemu-api-macros/src/device.rs | 433
> > ++++++++++++++++++++++++++
> > rust/qemu-api-macros/src/lib.rs | 45 +--
> > rust/qemu-api-macros/src/object.rs | 107 +++++++
> > rust/qemu-api-macros/src/symbols.rs | 55 ++++
> > rust/qemu-api-macros/src/utilities.rs | 152 +++++++++
> > rust/qemu-api/meson.build | 3 +-
> > rust/qemu-api/src/definitions.rs | 97 ------
> > rust/qemu-api/src/device_class.rs | 128 --------
> > rust/qemu-api/src/lib.rs | 6 +-
> > rust/qemu-api/src/objects.rs | 90 ++++++
> > rust/qemu-api/src/tests.rs | 49 ---
> > subprojects/packagefiles/syn-2-rs/meson.build | 1 +
> > 15 files changed, 902 insertions(+), 459 deletions(-)
>
> My initial reaction is these patches are getting quite large because we
> are doing several things at once. Is it possible to split them so:
>
> - we bring in one macro at a time (with unit tests)
> - we then convert pl011 to the new idiom
Sure, I think it's possible to split them on some level. (Make changes
on the existing Object derive macro and then introduce the Device
macro) We can't unfortunately do changes without pl011 not being
updated though, since it uses the code.
As for unit tests, the only way to test proc macros is to match
generated code output to expected output. Otherwise they are
indirectly tested by the device not breaking other qemu tests. What do
you think should be our unit testing approach for this instance?
