From 51d736cd71a6515808b705010ec7e38695cb7a01 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Sun, 21 Sep 2025 00:05:13 +0800 Subject: rust/qdev: use addr_of! in QDevProp We want a &raw pointer, so unsafe { &_ } is not needed. Suggested-by: Zhao Liu Signed-off-by: Manos Pitsidianakis Signed-off-by: Zhao Liu Signed-off-by: Paolo Bonzini Link: https://lore.kernel.org/r/20250920160520.3699591-6-zhao1.liu@intel.com --- rust/hw/core/src/qdev.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'rust/hw/core/src/qdev.rs') diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs index 71b9ef141c..3ee5b24262 100644 --- a/rust/hw/core/src/qdev.rs +++ b/rust/hw/core/src/qdev.rs @@ -6,7 +6,7 @@ use std::{ ffi::{c_int, c_void, CStr, CString}, - ptr::NonNull, + ptr::{addr_of, NonNull}, }; use chardev::Chardev; @@ -129,17 +129,17 @@ pub unsafe trait QDevProp { /// Use [`bindings::qdev_prop_bool`] for `bool`. unsafe impl QDevProp for bool { - const VALUE: *const bindings::PropertyInfo = unsafe { &bindings::qdev_prop_bool }; + const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_bool); } /// Use [`bindings::qdev_prop_uint64`] for `u64`. unsafe impl QDevProp for u64 { - const VALUE: *const bindings::PropertyInfo = unsafe { &bindings::qdev_prop_uint64 }; + const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_uint64); } /// Use [`bindings::qdev_prop_chr`] for [`chardev::CharBackend`]. unsafe impl QDevProp for chardev::CharBackend { - const VALUE: *const bindings::PropertyInfo = unsafe { &bindings::qdev_prop_chr }; + const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_chr); } /// Trait to define device properties. -- cgit 1.4.1 From bed2a37b2096e1d002f3b8e881c0f6eff863ff14 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Sun, 21 Sep 2025 00:05:14 +0800 Subject: rust/qdev: Refine the documentation for QDevProp trait Refine the documentation to clarify: * `unsfae` requires that `VALUE` must be valid. * using `*const` instead of `&` because the latter will cause compiler error. Signed-off-by: Manos Pitsidianakis Signed-off-by: Zhao Liu Signed-off-by: Paolo Bonzini Link: https://lore.kernel.org/r/20250920160520.3699591-7-zhao1.liu@intel.com --- rust/hw/core/src/qdev.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'rust/hw/core/src/qdev.rs') diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs index 3ee5b24262..2735e2b2c1 100644 --- a/rust/hw/core/src/qdev.rs +++ b/rust/hw/core/src/qdev.rs @@ -109,9 +109,16 @@ unsafe extern "C" fn rust_resettable_exit_fn( /// /// # Safety /// -/// This trait is marked as `unsafe` because currently having a `const` refer to -/// an `extern static` as a reference instead of a raw pointer results in this -/// compiler error: +/// This trait is marked as `unsafe` because `VALUE` must be a valid raw +/// reference to a [`bindings::PropertyInfo`]. +/// +/// Note we could not use a regular reference: +/// +/// ```text +/// const VALUE: &bindings::PropertyInfo = ... +/// ``` +/// +/// because this results in the following compiler error: /// /// ```text /// constructing invalid value: encountered reference to `extern` static in `const` @@ -119,7 +126,7 @@ unsafe extern "C" fn rust_resettable_exit_fn( /// /// This is because the compiler generally might dereference a normal reference /// during const evaluation, but not in this case (if it did, it'd need to -/// dereference the raw pointer so this would fail to compile). +/// dereference the raw pointer so using a `*const` would also fail to compile). /// /// It is the implementer's responsibility to provide a valid /// [`bindings::PropertyInfo`] pointer for the trait implementation to be safe. -- cgit 1.4.1 From b4221e88a9a497c3d72c4dfad1bcc12468f3fd01 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sun, 21 Sep 2025 00:05:15 +0800 Subject: rust/qdev: Support property info for more common types Add a helper macro to implement QDevProp trait for u8/u16/u32/usize/i32 /i64. Signed-off-by: Zhao Liu Signed-off-by: Paolo Bonzini Link: https://lore.kernel.org/r/20250920160520.3699591-8-zhao1.liu@intel.com --- rust/hw/core/src/qdev.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) (limited to 'rust/hw/core/src/qdev.rs') diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs index 2735e2b2c1..85422e0379 100644 --- a/rust/hw/core/src/qdev.rs +++ b/rust/hw/core/src/qdev.rs @@ -134,20 +134,24 @@ pub unsafe trait QDevProp { const VALUE: *const bindings::PropertyInfo; } -/// Use [`bindings::qdev_prop_bool`] for `bool`. -unsafe impl QDevProp for bool { - const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_bool); -} - -/// Use [`bindings::qdev_prop_uint64`] for `u64`. -unsafe impl QDevProp for u64 { - const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_uint64); +macro_rules! impl_qdev_prop { + ($type:ty,$info:ident) => { + unsafe impl $crate::qdev::QDevProp for $type { + const VALUE: *const $crate::bindings::PropertyInfo = + addr_of!($crate::bindings::$info); + } + }; } -/// Use [`bindings::qdev_prop_chr`] for [`chardev::CharBackend`]. -unsafe impl QDevProp for chardev::CharBackend { - const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_chr); -} +impl_qdev_prop!(bool, qdev_prop_bool); +impl_qdev_prop!(u8, qdev_prop_uint8); +impl_qdev_prop!(u16, qdev_prop_uint16); +impl_qdev_prop!(u32, qdev_prop_uint32); +impl_qdev_prop!(u64, qdev_prop_uint64); +impl_qdev_prop!(usize, qdev_prop_usize); +impl_qdev_prop!(i32, qdev_prop_int32); +impl_qdev_prop!(i64, qdev_prop_int64); +impl_qdev_prop!(chardev::CharBackend, qdev_prop_chr); /// Trait to define device properties. /// -- cgit 1.4.1 From 9686aa9a05d9ef25c13ddc0af0123e2fc569b0ac Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sun, 21 Sep 2025 00:05:16 +0800 Subject: rust/qdev: Support bit property in #property macro Add BIT_INFO to QDevProp trait, so that bit related property info could be bound to u32 & u64. Then add "bit=*" field in #property attributes macro to allow device to configure bit property. In addtion, convert the #property field parsing from `if-else` pattern to `match` pattern, to help readability. And note, the `bitnr` member of `Property` struct is generated by manual TokenStream construction, instead of conditional repetition (like #(bitnr: #bitnr,)?) since `quote` doesn't support this. In addtion, rename VALUE member of QDevProp trait to BASE_INFO. And update the test cases about qdev property. Signed-off-by: Zhao Liu Signed-off-by: Paolo Bonzini Link: https://lore.kernel.org/r/20250920160520.3699591-9-zhao1.liu@intel.com --- rust/hw/core/src/qdev.rs | 19 ++++++++++++------- rust/qemu-macros/src/lib.rs | 18 +++++++++++++++--- rust/qemu-macros/src/tests.rs | 8 +++++--- 3 files changed, 32 insertions(+), 13 deletions(-) (limited to 'rust/hw/core/src/qdev.rs') diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs index 85422e0379..9c82e1716c 100644 --- a/rust/hw/core/src/qdev.rs +++ b/rust/hw/core/src/qdev.rs @@ -109,8 +109,8 @@ unsafe extern "C" fn rust_resettable_exit_fn( /// /// # Safety /// -/// This trait is marked as `unsafe` because `VALUE` must be a valid raw -/// reference to a [`bindings::PropertyInfo`]. +/// This trait is marked as `unsafe` because `BASE_INFO` and `BIT_INFO` must be +/// valid raw references to [`bindings::PropertyInfo`]. /// /// Note we could not use a regular reference: /// @@ -131,14 +131,19 @@ unsafe extern "C" fn rust_resettable_exit_fn( /// It is the implementer's responsibility to provide a valid /// [`bindings::PropertyInfo`] pointer for the trait implementation to be safe. pub unsafe trait QDevProp { - const VALUE: *const bindings::PropertyInfo; + const BASE_INFO: *const bindings::PropertyInfo; + const BIT_INFO: *const bindings::PropertyInfo = { + panic!("invalid type for bit property"); + }; } macro_rules! impl_qdev_prop { - ($type:ty,$info:ident) => { + ($type:ty,$info:ident$(, $bit_info:ident)?) => { unsafe impl $crate::qdev::QDevProp for $type { - const VALUE: *const $crate::bindings::PropertyInfo = + const BASE_INFO: *const $crate::bindings::PropertyInfo = addr_of!($crate::bindings::$info); + $(const BIT_INFO: *const $crate::bindings::PropertyInfo = + addr_of!($crate::bindings::$bit_info);)? } }; } @@ -146,8 +151,8 @@ macro_rules! impl_qdev_prop { impl_qdev_prop!(bool, qdev_prop_bool); impl_qdev_prop!(u8, qdev_prop_uint8); impl_qdev_prop!(u16, qdev_prop_uint16); -impl_qdev_prop!(u32, qdev_prop_uint32); -impl_qdev_prop!(u64, qdev_prop_uint64); +impl_qdev_prop!(u32, qdev_prop_uint32, qdev_prop_bit); +impl_qdev_prop!(u64, qdev_prop_uint64, qdev_prop_bit64); impl_qdev_prop!(usize, qdev_prop_usize); impl_qdev_prop!(i32, qdev_prop_int32); impl_qdev_prop!(i64, qdev_prop_int64); diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs index 37e1b723bd..3e21b67b47 100644 --- a/rust/qemu-macros/src/lib.rs +++ b/rust/qemu-macros/src/lib.rs @@ -179,6 +179,7 @@ impl Parse for DevicePropertyName { #[derive(Default, Debug)] struct DeviceProperty { rename: Option, + bitnr: Option, defval: Option, } @@ -187,6 +188,7 @@ impl DeviceProperty { use attrs::{set, with, Attrs}; let mut parser = Attrs::new(); parser.once("rename", with::eq(set::parse(&mut self.rename))); + parser.once("bit", with::eq(set::parse(&mut self.bitnr))); parser.once("default", with::eq(set::parse(&mut self.defval))); a.parse_args_with(&mut parser) } @@ -222,7 +224,11 @@ fn derive_device_or_error(input: DeriveInput) -> Result {{ @@ -252,14 +258,20 @@ fn derive_device_or_error(input: DeriveInput) -> Result::VALUE }; + let qdev_prop = if bitnr.is_none() { + quote! { <#field_ty as ::hwcore::QDevProp>::BASE_INFO } + } else { + quote! { <#field_ty as ::hwcore::QDevProp>::BIT_INFO } + }; + let bitnr = bitnr.unwrap_or(syn::Expr::Verbatim(quote! { 0 })); let set_default = defval.is_some(); let defval = defval.unwrap_or(syn::Expr::Verbatim(quote! { 0 })); properties_expanded.push(quote! { ::hwcore::bindings::Property { name: ::std::ffi::CStr::as_ptr(#prop_name), - info: #qdev_prop , + info: #qdev_prop, offset: ::core::mem::offset_of!(#name, #field_name) as isize, + bitnr: #bitnr, set_default: #set_default, defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: #defval as u64 }, ..::common::Zeroable::ZERO diff --git a/rust/qemu-macros/src/tests.rs b/rust/qemu-macros/src/tests.rs index 00a106612f..ec137132ae 100644 --- a/rust/qemu-macros/src/tests.rs +++ b/rust/qemu-macros/src/tests.rs @@ -60,7 +60,7 @@ fn test_derive_device() { migrate_clock: bool, } }, - "Expected one of `default` or `rename`" + "Expected one of `bit`, `default` or `rename`" ); // Check that repeated attributes are not allowed: derive_compile_fail!( @@ -106,8 +106,9 @@ fn test_derive_device() { const PROPERTIES: &'static [::hwcore::bindings::Property] = &[ ::hwcore::bindings::Property { name: ::std::ffi::CStr::as_ptr(c"migrate_clock"), - info: ::VALUE, + info: ::BASE_INFO, offset: ::core::mem::offset_of!(DummyState, migrate_clock) as isize, + bitnr: 0, set_default: true, defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: true as u64 }, ..::common::Zeroable::ZERO @@ -133,8 +134,9 @@ fn test_derive_device() { const PROPERTIES: &'static [::hwcore::bindings::Property] = &[ ::hwcore::bindings::Property { name: ::std::ffi::CStr::as_ptr(c"migrate-clk"), - info: ::VALUE, + info: ::BASE_INFO, offset: ::core::mem::offset_of!(DummyState, migrate_clock) as isize, + bitnr: 0, set_default: true, defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: true as u64 }, ..::common::Zeroable::ZERO -- cgit 1.4.1 From 0a7fe8d407009840bee0e6d95005714ede51d0b8 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sun, 21 Sep 2025 00:05:20 +0800 Subject: rust/qdev: Drop declare_properties & define_property macros After HPET's #property conversion, there's no use case for declare_properties & define_property. So get rid of them for now. In future, if there's something that #property really cannot resolve, they can be brought back. Reviewed-by: Manos Pitsidianakis Signed-off-by: Zhao Liu Signed-off-by: Paolo Bonzini Link: https://lore.kernel.org/r/20250920160520.3699591-13-zhao1.liu@intel.com --- rust/hw/core/src/qdev.rs | 53 ------------------------------------------------ 1 file changed, 53 deletions(-) (limited to 'rust/hw/core/src/qdev.rs') diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs index 9c82e1716c..a4493dbf01 100644 --- a/rust/hw/core/src/qdev.rs +++ b/rust/hw/core/src/qdev.rs @@ -248,59 +248,6 @@ impl DeviceClass { } } -#[macro_export] -macro_rules! define_property { - ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, bit = $bitnr:expr, default = $defval:expr$(,)*) => { - $crate::bindings::Property { - // use associated function syntax for type checking - name: ::std::ffi::CStr::as_ptr($name), - info: $prop, - offset: ::std::mem::offset_of!($state, $field) as isize, - bitnr: $bitnr, - set_default: true, - defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 }, - ..::common::zeroable::Zeroable::ZERO - } - }; - ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, default = $defval:expr$(,)*) => { - $crate::bindings::Property { - // use associated function syntax for type checking - name: ::std::ffi::CStr::as_ptr($name), - info: $prop, - offset: ::std::mem::offset_of!($state, $field) as isize, - set_default: true, - defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 }, - ..::common::zeroable::Zeroable::ZERO - } - }; - ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty$(,)*) => { - $crate::bindings::Property { - // use associated function syntax for type checking - name: ::std::ffi::CStr::as_ptr($name), - info: $prop, - offset: ::std::mem::offset_of!($state, $field) as isize, - set_default: false, - ..::common::zeroable::Zeroable::ZERO - } - }; -} - -#[macro_export] -macro_rules! declare_properties { - ($ident:ident, $($prop:expr),*$(,)*) => { - pub static $ident: [$crate::bindings::Property; { - let mut len = 0; - $({ - _ = stringify!($prop); - len += 1; - })* - len - }] = [ - $($prop),*, - ]; - }; -} - unsafe impl ObjectType for DeviceState { type Class = DeviceClass; const TYPE_NAME: &'static CStr = -- cgit 1.4.1