| Message ID | 20241209123717.99077-20-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rust: bundle of prerequisites for HPET implementation | expand |
On Mon, Dec 09, 2024 at 01:37:10PM +0100, Paolo Bonzini wrote: > Date: Mon, 9 Dec 2024 13:37:10 +0100 > From: Paolo Bonzini <pbonzini@redhat.com> > Subject: [PATCH 19/26] rust: rename qemu-api modules to follow C code a bit > more > X-Mailer: git-send-email 2.47.1 > > A full match would mean calling them qom::object and hw::core::qdev. For now, > keep the names shorter but still a bit easier to find. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > rust/hw/char/pl011/src/device.rs | 4 +- > rust/qemu-api-macros/src/lib.rs | 2 +- > rust/qemu-api/meson.build | 5 +- > rust/qemu-api/src/lib.rs | 5 +- > rust/qemu-api/src/module.rs | 43 +++++++++++ > rust/qemu-api/src/prelude.rs | 2 +- > .../qemu-api/src/{device_class.rs => qdev.rs} | 4 +- > rust/qemu-api/src/{definitions.rs => qom.rs} | 74 +++++++++---------- > rust/qemu-api/src/sysbus.rs | 2 +- > rust/qemu-api/tests/tests.rs | 5 +- > 10 files changed, 92 insertions(+), 54 deletions(-) > create mode 100644 rust/qemu-api/src/module.rs > rename rust/qemu-api/src/{device_class.rs => qdev.rs} (97%) > rename rust/qemu-api/src/{definitions.rs => qom.rs} (83%) > > --- a/rust/qemu-api/src/definitions.rs > +++ b/rust/qemu-api/src/qom.rs > @@ -2,7 +2,37 @@ > // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org> > // SPDX-License-Identifier: GPL-2.0-or-later > > -//! Definitions required by QEMU when registering a device. > +//! Bindings to access QOM functionality from Rust. > +//! > +//! This module provides automatic creation and registration of `TypeInfo` > +//! for classes that are written in Rust, and mapping between Rust traits > +//! and QOM vtables. > +//! > +//! # Structure of a class > +//! > +//! A concrete class only needs a struct holding instance state. The struct must > +//! implement the [`ObjectType`] and [`IsA`] traits, as well as any `*Impl` > +//! traits provided by its superclasses. In this commit, this comment is a bit ahead, but I think it's okay. qom and qdev are both good names. In addition, we can rename the files of PL011 as well. Perhaps device_class.rs could be merged into device.rs (and eventually renamed to pl011.rs). I guess you might be planning to keep it until the cleanup of vmstate and property is done. Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
On 12/12/24 10:52, Zhao Liu wrote: > On Mon, Dec 09, 2024 at 01:37:10PM +0100, Paolo Bonzini wrote: >> Date: Mon, 9 Dec 2024 13:37:10 +0100 >> From: Paolo Bonzini <pbonzini@redhat.com> >> Subject: [PATCH 19/26] rust: rename qemu-api modules to follow C code a bit >> more >> X-Mailer: git-send-email 2.47.1 >> >> A full match would mean calling them qom::object and hw::core::qdev. For now, >> keep the names shorter but still a bit easier to find. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> rust/hw/char/pl011/src/device.rs | 4 +- >> rust/qemu-api-macros/src/lib.rs | 2 +- >> rust/qemu-api/meson.build | 5 +- >> rust/qemu-api/src/lib.rs | 5 +- >> rust/qemu-api/src/module.rs | 43 +++++++++++ >> rust/qemu-api/src/prelude.rs | 2 +- >> .../qemu-api/src/{device_class.rs => qdev.rs} | 4 +- >> rust/qemu-api/src/{definitions.rs => qom.rs} | 74 +++++++++---------- >> rust/qemu-api/src/sysbus.rs | 2 +- >> rust/qemu-api/tests/tests.rs | 5 +- >> 10 files changed, 92 insertions(+), 54 deletions(-) >> create mode 100644 rust/qemu-api/src/module.rs >> rename rust/qemu-api/src/{device_class.rs => qdev.rs} (97%) >> rename rust/qemu-api/src/{definitions.rs => qom.rs} (83%) >> >> --- a/rust/qemu-api/src/definitions.rs >> +++ b/rust/qemu-api/src/qom.rs >> @@ -2,7 +2,37 @@ >> // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org> >> // SPDX-License-Identifier: GPL-2.0-or-later >> >> -//! Definitions required by QEMU when registering a device. >> +//! Bindings to access QOM functionality from Rust. >> +//! >> +//! This module provides automatic creation and registration of `TypeInfo` >> +//! for classes that are written in Rust, and mapping between Rust traits >> +//! and QOM vtables. >> +//! >> +//! # Structure of a class >> +//! >> +//! A concrete class only needs a struct holding instance state. The struct must >> +//! implement the [`ObjectType`] and [`IsA`] traits, as well as any `*Impl` >> +//! traits provided by its superclasses. > > In this commit, this comment is a bit ahead, but I think it's okay. > > qom and qdev are both good names. In addition, we can rename the files > of PL011 as well. Perhaps device_class.rs could be merged into device.rs > (and eventually renamed to pl011.rs). I guess you might be planning to > keep it until the cleanup of vmstate and property is done. Yeah, I don't have any specific plans but memory_ops will certainly go away. device_class doesn't do much, but keeping it separate is a reminder for things that are still there to be cleaned up. As to VMState, there are two parts. One is the vmstate_description macro, probably it has to be replaced with something else to incorporate the trampolines for pre_save/post_load/... I haven't looked at it but it should not be a lot of work. The second is VMStateFields, for which my idea is to implement a trait on types to retrieve a basic VMStateField (for example something like vmstate_uint32 would become an implementation of the VMState trait on u32). Then you'd write something like "vmstate_of!(Type, field).with_version_id(2)" (i.e. vmstate_of retrieves the basic field and fills in the offset, then you apply more changes on top). But that may take a while, and I think it cannot be done without the const_refs_to_static feature, which is only stable in 1.83.0. Paolo
> > In this commit, this comment is a bit ahead, but I think it's okay. > > > > qom and qdev are both good names. In addition, we can rename the files > > of PL011 as well. Perhaps device_class.rs could be merged into device.rs > > (and eventually renamed to pl011.rs). I guess you might be planning to > > keep it until the cleanup of vmstate and property is done. > > Yeah, I don't have any specific plans but memory_ops will certainly go away. This depends on MemoryRegionOps binding. I will see how much HPET can do, and I'm sorry for not being able to review the remaining patches yet in this week, especially the last two. I will continue next week (my machine will be powered down until next Monday :( ). > device_class doesn't do much, but keeping it separate is a reminder for > things that are still there to be cleaned up. > > As to VMState, there are two parts. One is the vmstate_description macro, > probably it has to be replaced with something else to incorporate the > trampolines for pre_save/post_load/... I haven't looked at it but it should > not be a lot of work. > > The second is VMStateFields, I found vmstate_array_of_pointer_to_struct missed a `info` field, and I could submit a patch to fix this nit next week (along with other cleanup you and other miantainers suggested for HPET). I'd also like to try apply zeroable to VMStateField to avoid missing any items... > for which my idea is to implement a trait on > types to retrieve a basic VMStateField (for example something like > vmstate_uint32 would become an implementation of the VMState trait on u32). This makes sense. > Then you'd write something like "vmstate_of!(Type, > field).with_version_id(2)" (i.e. vmstate_of retrieves the basic field and > fills in the offset, then you apply more changes on top). But that may take > a while, and I think it cannot be done without the const_refs_to_static > feature, which is only stable in 1.83.0. I also like this idea! Thanks, Zhao
Il ven 13 dic 2024, 10:01 Zhao Liu <zhao1.liu@intel.com> ha scritto: > I found vmstate_array_of_pointer_to_struct missed a `info` field, and I > could submit a patch to fix this nit next week (along with other cleanup > you and other miantainers suggested for HPET). > I wouldn't worry too much about VMState, it's not in its final form and anything that isn't needed to pass qtests can wait. Paolo > for which my idea is to implement a trait on > > types to retrieve a basic VMStateField (for example something like > > vmstate_uint32 would become an implementation of the VMState trait on > u32). > > This makes sense. > > > Then you'd write something like "vmstate_of!(Type, > > field).with_version_id(2)" (i.e. vmstate_of retrieves the basic field and > > fills in the offset, then you apply more changes on top). But that may > take > > a while, and I think it cannot be done without the const_refs_to_static > > feature, which is only stable in 1.83.0. > > I also like this idea! > > Thanks, > Zhao > > >
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index d9e9f35f456..3fed8b4ad25 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -11,10 +11,10 @@ use qemu_api::{ bindings::{self, *}, c_str, - definitions::ObjectImpl, - device_class::DeviceImpl, irq::InterruptSource, prelude::*, + qdev::DeviceImpl, + qom::ObjectImpl, }; use crate::{ diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs index cf99ac04b8f..74a8bc7503e 100644 --- a/rust/qemu-api-macros/src/lib.rs +++ b/rust/qemu-api-macros/src/lib.rs @@ -40,7 +40,7 @@ pub fn derive_object(input: TokenStream) -> TokenStream { let expanded = quote! { ::qemu_api::module_init! { MODULE_INIT_QOM => unsafe { - ::qemu_api::bindings::type_register_static(&<#name as ::qemu_api::definitions::ObjectImpl>::TYPE_INFO); + ::qemu_api::bindings::type_register_static(&<#name as ::qemu_api::qom::ObjectImpl>::TYPE_INFO); } } }; diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build index adcee661150..7ff408ad68e 100644 --- a/rust/qemu-api/meson.build +++ b/rust/qemu-api/meson.build @@ -19,11 +19,12 @@ _qemu_api_rs = static_library( 'src/bitops.rs', 'src/cell.rs', 'src/c_str.rs', - 'src/definitions.rs', - 'src/device_class.rs', 'src/irq.rs', + 'src/module.rs', 'src/offset_of.rs', 'src/prelude.rs', + 'src/qdev.rs', + 'src/qom.rs', 'src/sysbus.rs', 'src/vmstate.rs', 'src/zeroable.rs', diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index 9e007e16354..124bece0449 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -15,10 +15,11 @@ pub mod bitops; pub mod c_str; pub mod cell; -pub mod definitions; -pub mod device_class; pub mod irq; +pub mod module; pub mod offset_of; +pub mod qdev; +pub mod qom; pub mod sysbus; pub mod vmstate; pub mod zeroable; diff --git a/rust/qemu-api/src/module.rs b/rust/qemu-api/src/module.rs new file mode 100644 index 00000000000..fa5cea3598f --- /dev/null +++ b/rust/qemu-api/src/module.rs @@ -0,0 +1,43 @@ +// Copyright 2024, Linaro Limited +// Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org> +// SPDX-License-Identifier: GPL-2.0-or-later + +//! Macro to register blocks of code that run as QEMU starts up. + +#[macro_export] +macro_rules! module_init { + ($type:ident => $body:block) => { + const _: () = { + #[used] + #[cfg_attr( + not(any(target_vendor = "apple", target_os = "windows")), + link_section = ".init_array" + )] + #[cfg_attr(target_vendor = "apple", link_section = "__DATA,__mod_init_func")] + #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")] + pub static LOAD_MODULE: extern "C" fn() = { + extern "C" fn init_fn() { + $body + } + + extern "C" fn ctor_fn() { + unsafe { + $crate::bindings::register_module_init( + Some(init_fn), + $crate::bindings::module_init_type::$type, + ); + } + } + + ctor_fn + }; + }; + }; + + // shortcut because it's quite common that $body needs unsafe {} + ($type:ident => unsafe $body:block) => { + $crate::module_init! { + $type => { unsafe { $body } } + } + }; +} diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs index 1b8677b2d9a..5cc41f081f9 100644 --- a/rust/qemu-api/src/prelude.rs +++ b/rust/qemu-api/src/prelude.rs @@ -7,4 +7,4 @@ pub use crate::cell::BqlCell; pub use crate::cell::BqlRefCell; -pub use crate::definitions::ObjectType; +pub use crate::qom::ObjectType; diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/qdev.rs similarity index 97% rename from rust/qemu-api/src/device_class.rs rename to rust/qemu-api/src/qdev.rs index 285dfe582c7..1228dabaaaf 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/qdev.rs @@ -2,12 +2,14 @@ // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org> // SPDX-License-Identifier: GPL-2.0-or-later +//! Bindings to create devices and access device functionality from Rust. + use std::ffi::CStr; use crate::{ bindings::{self, DeviceClass, DeviceState, Error, ObjectClass, Property, VMStateDescription}, - definitions::ClassInitImpl, prelude::*, + qom::ClassInitImpl, zeroable::Zeroable, }; diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/qom.rs similarity index 83% rename from rust/qemu-api/src/definitions.rs rename to rust/qemu-api/src/qom.rs index 958ea34decc..9b316e07efa 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/qom.rs @@ -2,7 +2,37 @@ // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org> // SPDX-License-Identifier: GPL-2.0-or-later -//! Definitions required by QEMU when registering a device. +//! Bindings to access QOM functionality from Rust. +//! +//! This module provides automatic creation and registration of `TypeInfo` +//! for classes that are written in Rust, and mapping between Rust traits +//! and QOM vtables. +//! +//! # Structure of a class +//! +//! A concrete class only needs a struct holding instance state. The struct must +//! implement the [`ObjectType`] and [`IsA`] traits, as well as any `*Impl` +//! traits provided by its superclasses. +//! +//! An abstract class will also provide a struct for instance data, with the +//! same characteristics as for concrete classes, but it also needs additional +//! components to support virtual methods: +//! +//! * a struct for instance data, with the same characteristics as for concrete +//! classes. +//! +//! * a struct for class data, for example `DeviceClass`. This corresponds to +//! the C "class struct" and holds the vtable that is used by instances of the +//! class and its subclasses. It must start with its parent's class struct. +//! +//! * a trait for virtual method implementations, for example `DeviceImpl`. +//! Child classes implement this trait to provide their own behavior for +//! virtual methods. The trait's methods take `&self` to access instance data. +//! +//! * an implementation of [`ClassInitImpl`], for example +//! `ClassInitImpl<DeviceClass>`. This fills the vtable in the class struct, +//! typically with wrappers that call into the +//! [`DeviceImpl`](crate::qdev::DeviceImpl) implementations. use std::{ffi::CStr, os::raw::c_void}; @@ -136,7 +166,7 @@ pub trait ObjectImpl: ObjectType + ClassInitImpl<Self::Class> { /// /// Fortunately, this is almost never necessary. Instead, the Rust /// implementation of methods will usually come from a trait like -/// [`ObjectImpl`] or [`DeviceImpl`](crate::device_class::DeviceImpl). +/// [`ObjectImpl`] or [`DeviceImpl`](crate::qdev::DeviceImpl). /// `ClassInitImpl` then can be provided by blanket implementations /// that operate on all implementors of the `*Impl`* trait. For example: /// @@ -162,7 +192,7 @@ pub trait ClassInitImpl<T> { /// can change them to override virtual methods of a parent class. /// /// The virtual method implementations usually come from another - /// trait, for example [`DeviceImpl`](crate::device_class::DeviceImpl) + /// trait, for example [`DeviceImpl`](crate::qdev::DeviceImpl) /// when `T` is [`DeviceClass`](crate::bindings::DeviceClass). /// /// On entry, `klass`'s parent class is initialized, while the other fields @@ -174,44 +204,6 @@ pub trait ClassInitImpl<T> { fn class_init(klass: &mut T); } -#[macro_export] -macro_rules! module_init { - ($type:ident => $body:block) => { - const _: () = { - #[used] - #[cfg_attr( - not(any(target_vendor = "apple", target_os = "windows")), - link_section = ".init_array" - )] - #[cfg_attr(target_vendor = "apple", link_section = "__DATA,__mod_init_func")] - #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")] - pub static LOAD_MODULE: extern "C" fn() = { - extern "C" fn init_fn() { - $body - } - - extern "C" fn ctor_fn() { - unsafe { - $crate::bindings::register_module_init( - Some(init_fn), - $crate::bindings::module_init_type::$type, - ); - } - } - - ctor_fn - }; - }; - }; - - // shortcut because it's quite common that $body needs unsafe {} - ($type:ident => unsafe $body:block) => { - $crate::module_init! { - $type => { unsafe { $body } } - } - }; -} - /// # Safety /// /// We expect the FFI user of this function to pass a valid pointer that diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs index 5d15b317405..fa69cadd7c1 100644 --- a/rust/qemu-api/src/sysbus.rs +++ b/rust/qemu-api/src/sysbus.rs @@ -9,9 +9,9 @@ use crate::{ bindings::{self, DeviceClass}, cell::bql_locked, - definitions::ClassInitImpl, irq::InterruptSource, prelude::*, + qom::ClassInitImpl, }; unsafe impl ObjectType for SysBusDevice { diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs index 4ce1bd19247..fc57eb81290 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -7,10 +7,9 @@ use qemu_api::{ bindings::*, c_str, declare_properties, define_property, - definitions::ObjectImpl, - device_class::DeviceImpl, - impl_device_class, prelude::*, + qdev::DeviceImpl, + qom::ObjectImpl, zeroable::Zeroable, };
A full match would mean calling them qom::object and hw::core::qdev. For now, keep the names shorter but still a bit easier to find. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- rust/hw/char/pl011/src/device.rs | 4 +- rust/qemu-api-macros/src/lib.rs | 2 +- rust/qemu-api/meson.build | 5 +- rust/qemu-api/src/lib.rs | 5 +- rust/qemu-api/src/module.rs | 43 +++++++++++ rust/qemu-api/src/prelude.rs | 2 +- .../qemu-api/src/{device_class.rs => qdev.rs} | 4 +- rust/qemu-api/src/{definitions.rs => qom.rs} | 74 +++++++++---------- rust/qemu-api/src/sysbus.rs | 2 +- rust/qemu-api/tests/tests.rs | 5 +- 10 files changed, 92 insertions(+), 54 deletions(-) create mode 100644 rust/qemu-api/src/module.rs rename rust/qemu-api/src/{device_class.rs => qdev.rs} (97%) rename rust/qemu-api/src/{definitions.rs => qom.rs} (83%)