From 7b72c7dd0d4dbbcc8a909533a5c70e9935b201c3 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 15 Oct 2024 11:14:18 +0200 Subject: meson: pass rustc_args when building all crates rustc_args is needed to smooth the difference in warnings between the various versions of rustc. Always include those arguments. Reviewed-by: Junjie Mao Reviewed-by: Kevin Wolf Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/device_class.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index 1ea95beb78..b6b68cf9ce 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -16,10 +16,12 @@ macro_rules! device_class_init { ) { let mut dc = ::core::ptr::NonNull::new(klass.cast::<$crate::bindings::DeviceClass>()).unwrap(); - dc.as_mut().realize = $realize_fn; - dc.as_mut().vmsd = &$vmsd; - $crate::bindings::device_class_set_legacy_reset(dc.as_mut(), $legacy_reset_fn); - $crate::bindings::device_class_set_props(dc.as_mut(), $props.as_mut_ptr()); + unsafe { + dc.as_mut().realize = $realize_fn; + dc.as_mut().vmsd = &$vmsd; + $crate::bindings::device_class_set_legacy_reset(dc.as_mut(), $legacy_reset_fn); + $crate::bindings::device_class_set_props(dc.as_mut(), $props.as_mut_ptr()); + } } }; } -- cgit 1.4.1 From 00ed18de3537c783add0be5ea1e0a84979f6f63b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 15 Oct 2024 14:31:54 +0200 Subject: rust: do not use --no-size_t-is-usize This is not necessary and makes it harder to write code that is portable between 32- and 64-bit systems: it adds extra casts even though size_of, align_of or offset_of already return the right type. Reviewed-by: Junjie Mao Reviewed-by: Kevin Wolf Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- meson.build | 1 - rust/qemu-api/src/definitions.rs | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/meson.build b/meson.build index bed895091b..4bf397ef3b 100644 --- a/meson.build +++ b/meson.build @@ -3954,7 +3954,6 @@ if have_rust and have_system '--no-doc-comments', '--use-core', '--with-derive-default', - '--no-size_t-is-usize', '--no-layout-tests', '--no-prepend-enum-name', '--allowlist-file', meson.project_source_root() + '/include/.*', diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 60bd3f8aaa..0b681c593f 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -81,13 +81,13 @@ macro_rules! type_info { } else { ::core::ptr::null_mut() }, - instance_size: ::core::mem::size_of::<$t>() as $crate::bindings::size_t, - instance_align: ::core::mem::align_of::<$t>() as $crate::bindings::size_t, + instance_size: ::core::mem::size_of::<$t>(), + instance_align: ::core::mem::align_of::<$t>(), instance_init: <$t as $crate::definitions::ObjectImpl>::INSTANCE_INIT, instance_post_init: <$t as $crate::definitions::ObjectImpl>::INSTANCE_POST_INIT, instance_finalize: <$t as $crate::definitions::ObjectImpl>::INSTANCE_FINALIZE, abstract_: <$t as $crate::definitions::ObjectImpl>::ABSTRACT, - class_size: ::core::mem::size_of::<<$t as $crate::definitions::ObjectImpl>::Class>() as $crate::bindings::size_t, + class_size: ::core::mem::size_of::<<$t as $crate::definitions::ObjectImpl>::Class>(), class_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::Class>::CLASS_INIT, class_base_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::Class>::CLASS_BASE_INIT, class_data: ::core::ptr::null_mut(), -- cgit 1.4.1 From 2eb6274d12b92eb57dc6fa0516c4248509f4a66a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 18 Oct 2024 11:38:41 +0200 Subject: rust: remove uses of #[no_mangle] Mangled symbols do not cause any issue; disabling mangling is only useful if C headers reference the Rust function, which is not the case here. Reviewed-by: Junjie Mao Reviewed-by: Kevin Wolf Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device.rs | 4 ---- rust/hw/char/pl011/src/device_class.rs | 2 -- rust/hw/char/pl011/src/memory_ops.rs | 2 -- rust/qemu-api/src/definitions.rs | 1 - rust/qemu-api/src/device_class.rs | 2 -- 5 files changed, 11 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index c7193b41be..0347a027c5 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -514,7 +514,6 @@ pub const IRQMASK: [u32; 6] = [ /// We expect the FFI user of this function to pass a valid pointer, that has /// the same size as [`PL011State`]. We also expect the device is /// readable/writeable from one thread at any time. -#[no_mangle] pub unsafe extern "C" fn pl011_can_receive(opaque: *mut c_void) -> c_int { unsafe { debug_assert!(!opaque.is_null()); @@ -530,7 +529,6 @@ pub unsafe extern "C" fn pl011_can_receive(opaque: *mut c_void) -> c_int { /// readable/writeable from one thread at any time. /// /// The buffer and size arguments must also be valid. -#[no_mangle] pub unsafe extern "C" fn pl011_receive( opaque: *mut core::ffi::c_void, buf: *const u8, @@ -554,7 +552,6 @@ pub unsafe extern "C" fn pl011_receive( /// We expect the FFI user of this function to pass a valid pointer, that has /// the same size as [`PL011State`]. We also expect the device is /// readable/writeable from one thread at any time. -#[no_mangle] pub unsafe extern "C" fn pl011_event(opaque: *mut core::ffi::c_void, event: QEMUChrEvent) { unsafe { debug_assert!(!opaque.is_null()); @@ -589,7 +586,6 @@ pub unsafe extern "C" fn pl011_create( /// We expect the FFI user of this function to pass a valid pointer, that has /// the same size as [`PL011State`]. We also expect the device is /// readable/writeable from one thread at any time. -#[no_mangle] pub unsafe extern "C" fn pl011_init(obj: *mut Object) { unsafe { debug_assert!(!obj.is_null()); diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs index b7ab31af02..2ad80451e8 100644 --- a/rust/hw/char/pl011/src/device_class.rs +++ b/rust/hw/char/pl011/src/device_class.rs @@ -46,7 +46,6 @@ qemu_api::device_class_init! { /// We expect the FFI user of this function to pass a valid pointer, that has /// the same size as [`PL011State`]. We also expect the device is /// readable/writeable from one thread at any time. -#[no_mangle] pub unsafe extern "C" fn pl011_realize(dev: *mut DeviceState, _errp: *mut *mut Error) { unsafe { assert!(!dev.is_null()); @@ -60,7 +59,6 @@ pub unsafe extern "C" fn pl011_realize(dev: *mut DeviceState, _errp: *mut *mut E /// We expect the FFI user of this function to pass a valid pointer, that has /// the same size as [`PL011State`]. We also expect the device is /// readable/writeable from one thread at any time. -#[no_mangle] pub unsafe extern "C" fn pl011_reset(dev: *mut DeviceState) { unsafe { assert!(!dev.is_null()); diff --git a/rust/hw/char/pl011/src/memory_ops.rs b/rust/hw/char/pl011/src/memory_ops.rs index 8d066ebf6d..5a5320e66c 100644 --- a/rust/hw/char/pl011/src/memory_ops.rs +++ b/rust/hw/char/pl011/src/memory_ops.rs @@ -22,7 +22,6 @@ pub static PL011_OPS: MemoryRegionOps = MemoryRegionOps { }, }; -#[no_mangle] unsafe extern "C" fn pl011_read( opaque: *mut core::ffi::c_void, addr: hwaddr, @@ -44,7 +43,6 @@ unsafe extern "C" fn pl011_read( } } -#[no_mangle] unsafe extern "C" fn pl011_write( opaque: *mut core::ffi::c_void, addr: hwaddr, diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 0b681c593f..49ac59af12 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -53,7 +53,6 @@ macro_rules! module_init { #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")] pub static LOAD_MODULE: extern "C" fn() = { extern "C" fn __load() { - #[no_mangle] unsafe extern "C" fn $func() { $body } diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index b6b68cf9ce..2219b9f73d 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -9,7 +9,6 @@ use crate::bindings::Property; #[macro_export] macro_rules! device_class_init { ($func:ident, props => $props:ident, realize_fn => $realize_fn:expr, legacy_reset_fn => $legacy_reset_fn:expr, vmsd => $vmsd:ident$(,)*) => { - #[no_mangle] pub unsafe extern "C" fn $func( klass: *mut $crate::bindings::ObjectClass, _: *mut ::core::ffi::c_void, @@ -103,7 +102,6 @@ macro_rules! declare_properties { ] } - #[no_mangle] pub static mut $ident: $crate::device_class::Properties = $crate::device_class::Properties(::std::sync::OnceLock::new(), _make_properties); }; } -- cgit 1.4.1 From 4f7521916d12e07d25d5175f2da9614624344a7b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 18 Oct 2024 15:03:01 +0200 Subject: rust: modernize link_section usage for ELF platforms Some newer ABI implementations do not provide .ctors; and while some linkers rewrite .ctors into .init_array, not all of them do. Use the newer .init_array ABI, which works more reliably, and apply it to all non-Apple, non-Windows platforms. This is similar to how the ctor crate operates; without this change, "#[derive(Object)]" does not work on Fedora 41. Reviewed-by: Junjie Mao Reviewed-by: Kevin Wolf Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/qemu-api-macros/src/lib.rs | 7 +++++-- rust/qemu-api/src/definitions.rs | 14 ++++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs index 59aba592d9..70e3f92046 100644 --- a/rust/qemu-api-macros/src/lib.rs +++ b/rust/qemu-api-macros/src/lib.rs @@ -16,8 +16,11 @@ pub fn derive_object(input: TokenStream) -> TokenStream { let expanded = quote! { #[allow(non_upper_case_globals)] #[used] - #[cfg_attr(target_os = "linux", link_section = ".ctors")] - #[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")] + #[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 #module_static: extern "C" fn() = { extern "C" fn __register() { diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 49ac59af12..3323a665d9 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -31,8 +31,11 @@ pub trait Class { macro_rules! module_init { ($func:expr, $type:expr) => { #[used] - #[cfg_attr(target_os = "linux", link_section = ".ctors")] - #[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")] + #[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 __load() { @@ -48,8 +51,11 @@ macro_rules! module_init { // NOTE: To have custom identifiers for the ctor func we need to either supply // them directly as a macro argument or create them with a proc macro. #[used] - #[cfg_attr(target_os = "linux", link_section = ".ctors")] - #[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")] + #[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 __load() { -- cgit 1.4.1 From cde3c425d16f6d0c8e6f47940ef5152b9021f3f2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 18 Oct 2024 16:30:56 +0200 Subject: rust: build integration test for the qemu_api crate Adjust the integration test to compile with a subset of QEMU object files, and make it actually create an object of the class it defines. Follow the Rust filesystem conventions, where tests go in tests/ if they use the library in the same way any other code would. Reviewed-by: Junjie Mao Reviewed-by: Kevin Wolf Signed-off-by: Paolo Bonzini --- meson.build | 10 +++++- rust/qemu-api/meson.build | 26 +++++++++++++-- rust/qemu-api/src/lib.rs | 3 -- rust/qemu-api/src/tests.rs | 49 ---------------------------- rust/qemu-api/tests/tests.rs | 78 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 56 deletions(-) delete mode 100644 rust/qemu-api/src/tests.rs create mode 100644 rust/qemu-api/tests/tests.rs (limited to 'rust/qemu-api/src') diff --git a/meson.build b/meson.build index 4bf397ef3b..ac0f03d2e7 100644 --- a/meson.build +++ b/meson.build @@ -3345,7 +3345,15 @@ if have_rust and have_system # Prohibit code that is forbidden in Rust 2024 rustc_args += ['-D', 'unsafe_op_in_unsafe_fn'] - add_project_arguments(rustc_args, native: false, language: 'rust') + + # Apart from procedural macros, our Rust executables will often link + # with C code, so include all the libraries that C code needs. This + # is safe; https://github.com/rust-lang/rust/pull/54675 says that + # passing -nodefaultlibs to the linker "was more ideological to + # start with than anything". + add_project_arguments(rustc_args + ['-C', 'default-linker-libraries'], + native: false, language: 'rust') + add_project_arguments(rustc_args, native: true, language: 'rust') endif diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build index 42ea815fa5..1fc3607802 100644 --- a/rust/qemu-api/meson.build +++ b/rust/qemu-api/meson.build @@ -14,11 +14,31 @@ _qemu_api_rs = static_library( '--cfg', 'MESON', # '--cfg', 'feature="allocator"', ], - dependencies: [ - qemu_api_macros, - ], ) qemu_api = declare_dependency( link_with: _qemu_api_rs, + dependencies: qemu_api_macros, ) + +# Rust executables do not support objects, so add an intermediate step. +rust_qemu_api_objs = static_library( + 'rust_qemu_api_objs', + objects: [libqom.extract_all_objects(recursive: false), + libhwcore.extract_all_objects(recursive: false)]) + +test('rust-qemu-api-integration', + executable( + 'rust-qemu-api-integration', + 'tests/tests.rs', + override_options: ['rust_std=2021', 'build.rust_std=2021'], + rust_args: ['--test'], + install: false, + dependencies: [qemu_api, qemu_api_macros], + link_whole: [rust_qemu_api_objs, libqemuutil]), + args: [ + '--test', + '--format', 'pretty', + ], + protocol: 'rust', + suite: ['unit', 'rust']) diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index e72fb4b4bb..6bc68076aa 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -30,9 +30,6 @@ unsafe impl Sync for bindings::VMStateDescription {} pub mod definitions; pub mod device_class; -#[cfg(test)] -mod tests; - use std::alloc::{GlobalAlloc, Layout}; #[cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)] diff --git a/rust/qemu-api/src/tests.rs b/rust/qemu-api/src/tests.rs deleted file mode 100644 index df54edbd4e..0000000000 --- a/rust/qemu-api/src/tests.rs +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2024, Linaro Limited -// Author(s): Manos Pitsidianakis -// SPDX-License-Identifier: GPL-2.0-or-later - -use crate::{ - bindings::*, declare_properties, define_property, device_class_init, vm_state_description, -}; - -#[test] -fn test_device_decl_macros() { - // Test that macros can compile. - vm_state_description! { - VMSTATE, - name: c"name", - unmigratable: true, - } - - #[repr(C)] - pub struct DummyState { - pub char_backend: CharBackend, - pub migrate_clock: bool, - } - - declare_properties! { - DUMMY_PROPERTIES, - define_property!( - c"chardev", - DummyState, - char_backend, - unsafe { &qdev_prop_chr }, - CharBackend - ), - define_property!( - c"migrate-clk", - DummyState, - migrate_clock, - unsafe { &qdev_prop_bool }, - bool - ), - } - - device_class_init! { - dummy_class_init, - props => DUMMY_PROPERTIES, - realize_fn => None, - reset_fn => None, - vmsd => VMSTATE, - } -} diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs new file mode 100644 index 0000000000..aa1e0568c6 --- /dev/null +++ b/rust/qemu-api/tests/tests.rs @@ -0,0 +1,78 @@ +// Copyright 2024, Linaro Limited +// Author(s): Manos Pitsidianakis +// SPDX-License-Identifier: GPL-2.0-or-later + +use core::ffi::CStr; + +use qemu_api::{ + bindings::*, + declare_properties, define_property, + definitions::{Class, ObjectImpl}, + device_class_init, vm_state_description, +}; + +#[test] +fn test_device_decl_macros() { + // Test that macros can compile. + vm_state_description! { + VMSTATE, + name: c"name", + unmigratable: true, + } + + #[repr(C)] + #[derive(qemu_api_macros::Object)] + pub struct DummyState { + pub _parent: DeviceState, + pub migrate_clock: bool, + } + + #[repr(C)] + pub struct DummyClass { + pub _parent: DeviceClass, + } + + declare_properties! { + DUMMY_PROPERTIES, + define_property!( + c"migrate-clk", + DummyState, + migrate_clock, + unsafe { &qdev_prop_bool }, + bool + ), + } + + device_class_init! { + dummy_class_init, + props => DUMMY_PROPERTIES, + realize_fn => None, + legacy_reset_fn => None, + vmsd => VMSTATE, + } + + impl ObjectImpl for DummyState { + type Class = DummyClass; + const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self }; + const TYPE_NAME: &'static CStr = c"dummy"; + const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_DEVICE); + const ABSTRACT: bool = false; + const INSTANCE_INIT: Option = None; + const INSTANCE_POST_INIT: Option = None; + const INSTANCE_FINALIZE: Option = None; + } + + impl Class for DummyClass { + const CLASS_INIT: Option< + unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void), + > = Some(dummy_class_init); + const CLASS_BASE_INIT: Option< + unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void), + > = None; + } + + unsafe { + module_call_init(module_init_type::MODULE_INIT_QOM); + object_unref(object_new(DummyState::TYPE_NAME.as_ptr()) as *mut _); + } +} -- cgit 1.4.1 From e90d470733d3d2ca185a0b379ef8773601f60cfb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 21 Oct 2024 13:24:22 +0200 Subject: rust: cleanup module_init!, use it from #[derive(Object)] Remove the duplicate code by using the module_init! macro; at the same time, simplify how module_init! is used, by taking inspiration from the implementation of #[derive(Object)]. Reviewed-by: Junjie Mao Reviewed-by: Kevin Wolf Signed-off-by: Paolo Bonzini --- rust/qemu-api-macros/src/lib.rs | 33 ++++---------------- rust/qemu-api/src/definitions.rs | 65 +++++++++++++++++----------------------- 2 files changed, 32 insertions(+), 66 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs index 70e3f92046..a4bc5d01ee 100644 --- a/rust/qemu-api-macros/src/lib.rs +++ b/rust/qemu-api-macros/src/lib.rs @@ -3,43 +3,20 @@ // SPDX-License-Identifier: GPL-2.0-or-later use proc_macro::TokenStream; -use quote::{format_ident, quote}; +use quote::quote; use syn::{parse_macro_input, DeriveInput}; #[proc_macro_derive(Object)] pub fn derive_object(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); - let name = input.ident; - let module_static = format_ident!("__{}_LOAD_MODULE", name); let expanded = quote! { - #[allow(non_upper_case_globals)] - #[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 #module_static: extern "C" fn() = { - extern "C" fn __register() { - unsafe { - ::qemu_api::bindings::type_register_static(&<#name as ::qemu_api::definitions::ObjectImpl>::TYPE_INFO); - } - } - - extern "C" fn __load() { - unsafe { - ::qemu_api::bindings::register_module_init( - Some(__register), - ::qemu_api::bindings::module_init_type::MODULE_INIT_QOM - ); - } + ::qemu_api::module_init! { + MODULE_INIT_QOM => unsafe { + ::qemu_api::bindings::type_register_static(&<#name as ::qemu_api::definitions::ObjectImpl>::TYPE_INFO); } - - __load - }; + } }; TokenStream::from(expanded) diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 3323a665d9..064afe6054 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -29,51 +29,40 @@ pub trait Class { #[macro_export] macro_rules! module_init { - ($func:expr, $type:expr) => { - #[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 __load() { - unsafe { - $crate::bindings::register_module_init(Some($func), $type); - } - } - - __load - }; - }; - (qom: $func:ident => $body:block) => { - // NOTE: To have custom identifiers for the ctor func we need to either supply - // them directly as a macro argument or create them with a proc macro. - #[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 __load() { - unsafe extern "C" fn $func() { + ($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 } - unsafe { - $crate::bindings::register_module_init( - Some($func), - $crate::bindings::module_init_type::MODULE_INIT_QOM, - ); + extern "C" fn ctor_fn() { + unsafe { + $crate::bindings::register_module_init( + Some(init_fn), + $crate::bindings::module_init_type::$type, + ); + } } - } - __load + ctor_fn + }; }; }; + + // shortcut because it's quite common that $body needs unsafe {} + ($type:ident => unsafe $body:block) => { + $crate::module_init! { + $type => { unsafe { $body } } + } + }; } #[macro_export] -- cgit 1.4.1 From 03a573b960dae735847e6b6877b5cc3d07466e94 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 18 Oct 2024 10:45:00 +0200 Subject: rust: clean up define_property macro Use the "struct update" syntax to initialize most of the fields to zero, and simplify the handmade type-checking of $name. Reviewed-by: Junjie Mao Reviewed-by: Kevin Wolf Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/device_class.rs | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index 2219b9f73d..aab6048409 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -29,44 +29,27 @@ macro_rules! device_class_init { macro_rules! define_property { ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr, default = $defval:expr$(,)*) => { $crate::bindings::Property { - name: { - #[used] - static _TEMP: &::core::ffi::CStr = $name; - _TEMP.as_ptr() - }, + // use associated function syntax for type checking + name: ::core::ffi::CStr::as_ptr($name), info: $prop, offset: ::core::mem::offset_of!($state, $field) .try_into() .expect("Could not fit offset value to type"), - bitnr: 0, - bitmask: 0, set_default: true, - defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval.into() }, - arrayoffset: 0, - arrayinfo: ::core::ptr::null(), - arrayfieldsize: 0, - link_type: ::core::ptr::null(), + defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 }, + ..unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() } } }; ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr$(,)*) => { $crate::bindings::Property { - name: { - #[used] - static _TEMP: &::core::ffi::CStr = $name; - _TEMP.as_ptr() - }, + // use associated function syntax for type checking + name: ::core::ffi::CStr::as_ptr($name), info: $prop, offset: ::core::mem::offset_of!($state, $field) .try_into() .expect("Could not fit offset value to type"), - bitnr: 0, - bitmask: 0, set_default: false, - defval: $crate::bindings::Property__bindgen_ty_1 { i: 0 }, - arrayoffset: 0, - arrayinfo: ::core::ptr::null(), - arrayfieldsize: 0, - link_type: ::core::ptr::null(), + ..unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() } } }; } -- cgit 1.4.1 From c92c447ff04ca6a5b80da2930d87637b34713b8c Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 18 Oct 2024 15:22:59 +0200 Subject: rust: make properties array immutable Now that device_class_set_props() takes a const pointer, the only part of "define_property!" that needs to be non-const is the call to try_into(). This in turn will only break if offset_of returns a value with the most significant bit set (i.e. a struct size that is >=2^31 or >= 2^63, respectively on 32- and 64-bit system), which is impossible. Just use a cast and clean everything up to remove the run-time initialization. This also removes a use of OnceLock, which was only stabilized in 1.70.0. Reviewed-by: Kevin Wolf Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/device_class.rs | 42 ++++++++------------------------------- 1 file changed, 8 insertions(+), 34 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index aab6048409..4b14cb3ffd 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -2,10 +2,6 @@ // Author(s): Manos Pitsidianakis // SPDX-License-Identifier: GPL-2.0-or-later -use std::sync::OnceLock; - -use crate::bindings::Property; - #[macro_export] macro_rules! device_class_init { ($func:ident, props => $props:ident, realize_fn => $realize_fn:expr, legacy_reset_fn => $legacy_reset_fn:expr, vmsd => $vmsd:ident$(,)*) => { @@ -19,7 +15,7 @@ macro_rules! device_class_init { dc.as_mut().realize = $realize_fn; dc.as_mut().vmsd = &$vmsd; $crate::bindings::device_class_set_legacy_reset(dc.as_mut(), $legacy_reset_fn); - $crate::bindings::device_class_set_props(dc.as_mut(), $props.as_mut_ptr()); + $crate::bindings::device_class_set_props(dc.as_mut(), $props.as_ptr()); } } }; @@ -32,9 +28,7 @@ macro_rules! define_property { // use associated function syntax for type checking name: ::core::ffi::CStr::as_ptr($name), info: $prop, - offset: ::core::mem::offset_of!($state, $field) - .try_into() - .expect("Could not fit offset value to type"), + offset: ::core::mem::offset_of!($state, $field) as isize, set_default: true, defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 }, ..unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() } @@ -45,47 +39,27 @@ macro_rules! define_property { // use associated function syntax for type checking name: ::core::ffi::CStr::as_ptr($name), info: $prop, - offset: ::core::mem::offset_of!($state, $field) - .try_into() - .expect("Could not fit offset value to type"), + offset: ::core::mem::offset_of!($state, $field) as isize, set_default: false, ..unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() } } }; } -#[repr(C)] -pub struct Properties(pub OnceLock<[Property; N]>, pub fn() -> [Property; N]); - -impl Properties { - pub fn as_mut_ptr(&mut self) -> *mut Property { - _ = self.0.get_or_init(self.1); - self.0.get_mut().unwrap().as_mut_ptr() - } -} - #[macro_export] macro_rules! declare_properties { ($ident:ident, $($prop:expr),*$(,)*) => { - - const fn _calc_prop_len() -> usize { + pub static $ident: [$crate::bindings::Property; { let mut len = 1; $({ _ = stringify!($prop); len += 1; })* len - } - const PROP_LEN: usize = _calc_prop_len(); - - fn _make_properties() -> [$crate::bindings::Property; PROP_LEN] { - [ - $($prop),*, - unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() }, - ] - } - - pub static mut $ident: $crate::device_class::Properties = $crate::device_class::Properties(::std::sync::OnceLock::new(), _make_properties); + }] = [ + $($prop),*, + unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() }, + ]; }; } -- cgit 1.4.1 From 6e50bde1e1c8edc70145fb87b21b0d0843250600 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 18 Oct 2024 10:51:10 +0200 Subject: rust: provide safe wrapper for MaybeUninit::zeroed() MaybeUninit::zeroed() is handy, but it introduces unsafe (and has a pretty heavy syntax in general). Introduce a trait that provides the same functionality while staying within safe Rust. In addition, MaybeUninit::zeroed() is not available as a "const" function until Rust 1.75.0, so this also prepares for having handwritten implementations of the trait until we can assume that version. Reviewed-by: Junjie Mao Reviewed-by: Kevin Wolf Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device_class.rs | 4 ++-- rust/hw/char/pl011/src/memory_ops.rs | 8 ++++---- rust/qemu-api/meson.build | 1 + rust/qemu-api/src/device_class.rs | 8 ++++---- rust/qemu-api/src/lib.rs | 1 + rust/qemu-api/src/zeroable.rs | 23 +++++++++++++++++++++++ 6 files changed, 35 insertions(+), 10 deletions(-) create mode 100644 rust/qemu-api/src/zeroable.rs (limited to 'rust/qemu-api/src') diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs index 2ad80451e8..08c846aa48 100644 --- a/rust/hw/char/pl011/src/device_class.rs +++ b/rust/hw/char/pl011/src/device_class.rs @@ -4,7 +4,7 @@ use core::ptr::NonNull; -use qemu_api::{bindings::*, definitions::ObjectImpl}; +use qemu_api::{bindings::*, definitions::ObjectImpl, zeroable::Zeroable}; use crate::device::PL011State; @@ -12,7 +12,7 @@ use crate::device::PL011State; pub static VMSTATE_PL011: VMStateDescription = VMStateDescription { name: PL011State::TYPE_INFO.name, unmigratable: true, - ..unsafe { ::core::mem::MaybeUninit::::zeroed().assume_init() } + ..Zeroable::ZERO }; qemu_api::declare_properties! { diff --git a/rust/hw/char/pl011/src/memory_ops.rs b/rust/hw/char/pl011/src/memory_ops.rs index 5a5320e66c..fc69922fbf 100644 --- a/rust/hw/char/pl011/src/memory_ops.rs +++ b/rust/hw/char/pl011/src/memory_ops.rs @@ -2,9 +2,9 @@ // Author(s): Manos Pitsidianakis // SPDX-License-Identifier: GPL-2.0-or-later -use core::{mem::MaybeUninit, ptr::NonNull}; +use core::ptr::NonNull; -use qemu_api::bindings::*; +use qemu_api::{bindings::*, zeroable::Zeroable}; use crate::device::PL011State; @@ -14,11 +14,11 @@ pub static PL011_OPS: MemoryRegionOps = MemoryRegionOps { read_with_attrs: None, write_with_attrs: None, endianness: device_endian::DEVICE_NATIVE_ENDIAN, - valid: unsafe { MaybeUninit::::zeroed().assume_init() }, + valid: Zeroable::ZERO, impl_: MemoryRegionOps__bindgen_ty_2 { min_access_size: 4, max_access_size: 4, - ..unsafe { MaybeUninit::::zeroed().assume_init() } + ..Zeroable::ZERO }, }; diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build index 1fc3607802..1b0fd40637 100644 --- a/rust/qemu-api/meson.build +++ b/rust/qemu-api/meson.build @@ -5,6 +5,7 @@ _qemu_api_rs = static_library( 'src/lib.rs', 'src/definitions.rs', 'src/device_class.rs', + 'src/zeroable.rs', ], {'.' : bindings_rs}, ), diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index 4b14cb3ffd..aa6088d9d3 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -31,7 +31,7 @@ macro_rules! define_property { offset: ::core::mem::offset_of!($state, $field) as isize, set_default: true, defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 }, - ..unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() } + ..$crate::zeroable::Zeroable::ZERO } }; ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr$(,)*) => { @@ -41,7 +41,7 @@ macro_rules! define_property { info: $prop, offset: ::core::mem::offset_of!($state, $field) as isize, set_default: false, - ..unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() } + ..$crate::zeroable::Zeroable::ZERO } }; } @@ -58,7 +58,7 @@ macro_rules! declare_properties { len }] = [ $($prop),*, - unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() }, + $crate::zeroable::Zeroable::ZERO, ]; }; } @@ -79,7 +79,7 @@ macro_rules! vm_state_description { $vname.as_ptr() },)* unmigratable: true, - ..unsafe { ::core::mem::MaybeUninit::<$crate::bindings::VMStateDescription>::zeroed().assume_init() } + ..$crate::zeroable::Zeroable::ZERO }; } } diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index 6bc68076aa..e94a15bb82 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -29,6 +29,7 @@ unsafe impl Sync for bindings::VMStateDescription {} pub mod definitions; pub mod device_class; +pub mod zeroable; use std::alloc::{GlobalAlloc, Layout}; diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs new file mode 100644 index 0000000000..45ec95c9f7 --- /dev/null +++ b/rust/qemu-api/src/zeroable.rs @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +/// Encapsulates the requirement that +/// `MaybeUninit::::zeroed().assume_init()` does not cause +/// undefined behavior. +/// +/// # Safety +/// +/// Do not add this trait to a type unless all-zeroes is +/// a valid value for the type. In particular, remember that raw +/// pointers can be zero, but references and `NonNull` cannot +/// unless wrapped with `Option<>`. +pub unsafe trait Zeroable: Default { + /// SAFETY: If the trait was added to a type, then by definition + /// this is safe. + const ZERO: Self = unsafe { ::core::mem::MaybeUninit::::zeroed().assume_init() }; +} + +unsafe impl Zeroable for crate::bindings::Property__bindgen_ty_1 {} +unsafe impl Zeroable for crate::bindings::Property {} +unsafe impl Zeroable for crate::bindings::VMStateDescription {} +unsafe impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_1 {} +unsafe impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_2 {} -- cgit 1.4.1 From 0a65e4124ad9c6dab594d738cac31fd32d19402c Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Fri, 25 Oct 2024 07:55:50 +0200 Subject: rust: add definitions for vmstate Add a new qemu_api module, `vmstate`. Declare a bunch of Rust macros declared that are equivalent in spirit to the C macros in include/migration/vmstate.h. For example the Rust of equivalent of the C macro: VMSTATE_UINT32(field_name, struct_name) is: vmstate_uint32!(field_name, StructName) This breathtaking development will allow us to reach feature parity between the Rust and C pl011 implementations. Extracted from a patch by Manos Pitsidianakis (https://lore.kernel.org/qemu-devel/20241024-rust-round-2-v1-4-051e7a25b978@linaro.org/). Signed-off-by: Manos Pitsidianakis Signed-off-by: Paolo Bonzini --- rust/qemu-api/meson.build | 1 + rust/qemu-api/src/device_class.rs | 21 --- rust/qemu-api/src/lib.rs | 3 + rust/qemu-api/src/vmstate.rs | 360 ++++++++++++++++++++++++++++++++++++++ rust/qemu-api/tests/tests.rs | 11 +- 5 files changed, 370 insertions(+), 26 deletions(-) create mode 100644 rust/qemu-api/src/vmstate.rs (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build index 1b0fd40637..3b849f7c41 100644 --- a/rust/qemu-api/meson.build +++ b/rust/qemu-api/meson.build @@ -5,6 +5,7 @@ _qemu_api_rs = static_library( 'src/lib.rs', 'src/definitions.rs', 'src/device_class.rs', + 'src/vmstate.rs', 'src/zeroable.rs', ], {'.' : bindings_rs}, diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index aa6088d9d3..3d40256f60 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -62,24 +62,3 @@ macro_rules! declare_properties { ]; }; } - -#[macro_export] -macro_rules! vm_state_description { - ($(#[$outer:meta])* - $name:ident, - $(name: $vname:expr,)* - $(unmigratable: $um_val:expr,)* - ) => { - #[used] - $(#[$outer])* - pub static $name: $crate::bindings::VMStateDescription = $crate::bindings::VMStateDescription { - $(name: { - #[used] - static VMSTATE_NAME: &::core::ffi::CStr = $vname; - $vname.as_ptr() - },)* - unmigratable: true, - ..$crate::zeroable::Zeroable::ZERO - }; - } -} diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index e94a15bb82..10ab3d7e63 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -26,9 +26,12 @@ unsafe impl Send for bindings::Property {} unsafe impl Sync for bindings::Property {} unsafe impl Sync for bindings::TypeInfo {} unsafe impl Sync for bindings::VMStateDescription {} +unsafe impl Sync for bindings::VMStateField {} +unsafe impl Sync for bindings::VMStateInfo {} pub mod definitions; pub mod device_class; +pub mod vmstate; pub mod zeroable; use std::alloc::{GlobalAlloc, Layout}; diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs new file mode 100644 index 0000000000..0c1197277f --- /dev/null +++ b/rust/qemu-api/src/vmstate.rs @@ -0,0 +1,360 @@ +// Copyright 2024, Linaro Limited +// Author(s): Manos Pitsidianakis +// SPDX-License-Identifier: GPL-2.0-or-later + +//! Helper macros to declare migration state for device models. +//! +//! Some macros are direct equivalents to the C macros declared in +//! `include/migration/vmstate.h` while +//! [`vmstate_subsections`](crate::vmstate_subsections) and +//! [`vmstate_fields`](crate::vmstate_fields) are meant to be used when +//! declaring a device model state struct. + +#[doc(alias = "VMSTATE_UNUSED_BUFFER")] +#[macro_export] +macro_rules! vmstate_unused_buffer { + ($field_exists_fn:expr, $version_id:expr, $size:expr) => {{ + $crate::bindings::VMStateField { + name: c"unused".as_ptr(), + err_hint: ::core::ptr::null(), + offset: 0, + size: $size, + start: 0, + num: 0, + num_offset: 0, + size_offset: 0, + info: unsafe { ::core::ptr::addr_of!($crate::bindings::vmstate_info_unused_buffer) }, + flags: VMStateFlags::VMS_BUFFER, + vmsd: ::core::ptr::null(), + version_id: $version_id, + struct_version_id: 0, + field_exists: $field_exists_fn, + } + }}; +} + +#[doc(alias = "VMSTATE_UNUSED_V")] +#[macro_export] +macro_rules! vmstate_unused_v { + ($version_id:expr, $size:expr) => {{ + $crate::vmstate_unused_buffer!(None, $version_id, $size) + }}; +} + +#[doc(alias = "VMSTATE_UNUSED")] +#[macro_export] +macro_rules! vmstate_unused { + ($size:expr) => {{ + $crate::vmstate_unused_v!(0, $size) + }}; +} + +#[doc(alias = "VMSTATE_SINGLE_TEST")] +#[macro_export] +macro_rules! vmstate_single_test { + ($field_name:ident, $struct_name:ty, $field_exists_fn:expr, $version_id:expr, $info:expr, $size:expr) => {{ + $crate::bindings::VMStateField { + name: ::core::concat!(::core::stringify!($field_name), 0) + .as_bytes() + .as_ptr() as *const ::core::ffi::c_char, + err_hint: ::core::ptr::null(), + offset: ::core::mem::offset_of!($struct_name, $field_name), + size: $size, + start: 0, + num: 0, + num_offset: 0, + size_offset: 0, + info: unsafe { $info }, + flags: VMStateFlags::VMS_SINGLE, + vmsd: ::core::ptr::null(), + version_id: $version_id, + struct_version_id: 0, + field_exists: $field_exists_fn, + } + }}; +} + +#[doc(alias = "VMSTATE_SINGLE")] +#[macro_export] +macro_rules! vmstate_single { + ($field_name:ident, $struct_name:ty, $version_id:expr, $info:expr, $size:expr) => {{ + $crate::vmstate_single_test!($field_name, $struct_name, None, $version_id, $info, $size) + }}; +} + +#[doc(alias = "VMSTATE_UINT32_V")] +#[macro_export] +macro_rules! vmstate_uint32_v { + ($field_name:ident, $struct_name:ty, $version_id:expr) => {{ + $crate::vmstate_single!( + $field_name, + $struct_name, + $version_id, + ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint32), + ::core::mem::size_of::() + ) + }}; +} + +#[doc(alias = "VMSTATE_UINT32")] +#[macro_export] +macro_rules! vmstate_uint32 { + ($field_name:ident, $struct_name:ty) => {{ + $crate::vmstate_uint32_v!($field_name, $struct_name, 0) + }}; +} + +#[doc(alias = "VMSTATE_INT32_V")] +#[macro_export] +macro_rules! vmstate_int32_v { + ($field_name:ident, $struct_name:ty, $version_id:expr) => {{ + $crate::vmstate_single!( + $field_name, + $struct_name, + $version_id, + ::core::ptr::addr_of!($crate::bindings::vmstate_info_int32), + ::core::mem::size_of::() + ) + }}; +} + +#[doc(alias = "VMSTATE_INT32")] +#[macro_export] +macro_rules! vmstate_int32 { + ($field_name:ident, $struct_name:ty) => {{ + $crate::vmstate_int32_v!($field_name, $struct_name, 0) + }}; +} + +#[doc(alias = "VMSTATE_ARRAY")] +#[macro_export] +macro_rules! vmstate_array { + ($field_name:ident, $struct_name:ty, $length:expr, $version_id:expr, $info:expr, $size:expr) => {{ + $crate::bindings::VMStateField { + name: ::core::concat!(::core::stringify!($field_name), 0) + .as_bytes() + .as_ptr() as *const ::core::ffi::c_char, + err_hint: ::core::ptr::null(), + offset: ::core::mem::offset_of!($struct_name, $field_name), + size: $size, + start: 0, + num: $length as _, + num_offset: 0, + size_offset: 0, + info: unsafe { $info }, + flags: VMStateFlags::VMS_ARRAY, + vmsd: ::core::ptr::null(), + version_id: $version_id, + struct_version_id: 0, + field_exists: None, + } + }}; +} + +#[doc(alias = "VMSTATE_UINT32_ARRAY_V")] +#[macro_export] +macro_rules! vmstate_uint32_array_v { + ($field_name:ident, $struct_name:ty, $length:expr, $version_id:expr) => {{ + $crate::vmstate_array!( + $field_name, + $struct_name, + $length, + $version_id, + ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint32), + ::core::mem::size_of::() + ) + }}; +} + +#[doc(alias = "VMSTATE_UINT32_ARRAY")] +#[macro_export] +macro_rules! vmstate_uint32_array { + ($field_name:ident, $struct_name:ty, $length:expr) => {{ + $crate::vmstate_uint32_array_v!($field_name, $struct_name, $length, 0) + }}; +} + +#[doc(alias = "VMSTATE_STRUCT_POINTER_V")] +#[macro_export] +macro_rules! vmstate_struct_pointer_v { + ($field_name:ident, $struct_name:ty, $version_id:expr, $vmsd:expr, $type:ty) => {{ + $crate::bindings::VMStateField { + name: ::core::concat!(::core::stringify!($field_name), 0) + .as_bytes() + .as_ptr() as *const ::core::ffi::c_char, + err_hint: ::core::ptr::null(), + offset: ::core::mem::offset_of!($struct_name, $field_name), + size: ::core::mem::size_of::<*const $type>(), + start: 0, + num: 0, + num_offset: 0, + size_offset: 0, + info: ::core::ptr::null(), + flags: VMStateFlags(VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_POINTER.0), + vmsd: unsafe { $vmsd }, + version_id: $version_id, + struct_version_id: 0, + field_exists: None, + } + }}; +} + +#[doc(alias = "VMSTATE_ARRAY_OF_POINTER")] +#[macro_export] +macro_rules! vmstate_array_of_pointer { + ($field_name:ident, $struct_name:ty, $num:expr, $version_id:expr, $info:expr, $type:ty) => {{ + $crate::bindings::VMStateField { + name: ::core::concat!(::core::stringify!($field_name), 0) + .as_bytes() + .as_ptr() as *const ::core::ffi::c_char, + version_id: $version_id, + num: $num as _, + info: unsafe { $info }, + size: ::core::mem::size_of::<*const $type>(), + flags: VMStateFlags(VMStateFlags::VMS_ARRAY.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0), + offset: ::core::mem::offset_of!($struct_name, $field_name), + err_hint: ::core::ptr::null(), + start: 0, + num_offset: 0, + size_offset: 0, + vmsd: ::core::ptr::null(), + struct_version_id: 0, + field_exists: None, + } + }}; +} + +#[doc(alias = "VMSTATE_ARRAY_OF_POINTER_TO_STRUCT")] +#[macro_export] +macro_rules! vmstate_array_of_pointer_to_struct { + ($field_name:ident, $struct_name:ty, $num:expr, $version_id:expr, $vmsd:expr, $type:ty) => {{ + $crate::bindings::VMStateField { + name: ::core::concat!(::core::stringify!($field_name), 0) + .as_bytes() + .as_ptr() as *const ::core::ffi::c_char, + version_id: $version_id, + num: $num as _, + vmsd: unsafe { $vmsd }, + size: ::core::mem::size_of::<*const $type>(), + flags: VMStateFlags( + VMStateFlags::VMS_ARRAY.0 + | VMStateFlags::VMS_STRUCT.0 + | VMStateFlags::VMS_ARRAY_OF_POINTER.0, + ), + offset: ::core::mem::offset_of!($struct_name, $field_name), + err_hint: ::core::ptr::null(), + start: 0, + num_offset: 0, + size_offset: 0, + vmsd: ::core::ptr::null(), + struct_version_id: 0, + field_exists: None, + } + }}; +} + +#[doc(alias = "VMSTATE_CLOCK_V")] +#[macro_export] +macro_rules! vmstate_clock_v { + ($field_name:ident, $struct_name:ty, $version_id:expr) => {{ + $crate::vmstate_struct_pointer_v!( + $field_name, + $struct_name, + $version_id, + ::core::ptr::addr_of!($crate::bindings::vmstate_clock), + $crate::bindings::Clock + ) + }}; +} + +#[doc(alias = "VMSTATE_CLOCK")] +#[macro_export] +macro_rules! vmstate_clock { + ($field_name:ident, $struct_name:ty) => {{ + $crate::vmstate_clock_v!($field_name, $struct_name, 0) + }}; +} + +#[doc(alias = "VMSTATE_ARRAY_CLOCK_V")] +#[macro_export] +macro_rules! vmstate_array_clock_v { + ($field_name:ident, $struct_name:ty, $num:expr, $version_id:expr) => {{ + $crate::vmstate_array_of_pointer_to_struct!( + $field_name, + $struct_name, + $num, + $version_id, + ::core::ptr::addr_of!($crate::bindings::vmstate_clock), + $crate::bindings::Clock + ) + }}; +} + +#[doc(alias = "VMSTATE_ARRAY_CLOCK")] +#[macro_export] +macro_rules! vmstate_array_clock { + ($field_name:ident, $struct_name:ty, $num:expr) => {{ + $crate::vmstate_array_clock_v!($field_name, $struct_name, $name, 0) + }}; +} + +/// Helper macro to declare a list of +/// ([`VMStateField`](`crate::bindings::VMStateField`)) into a static and return +/// a pointer to the array of values it created. +#[macro_export] +macro_rules! vmstate_fields { + ($($field:expr),*$(,)*) => {{ + static _FIELDS: &[$crate::bindings::VMStateField] = &[ + $($field),*, + $crate::bindings::VMStateField { + name: ::core::ptr::null(), + err_hint: ::core::ptr::null(), + offset: 0, + size: 0, + start: 0, + num: 0, + num_offset: 0, + size_offset: 0, + info: ::core::ptr::null(), + flags: VMStateFlags::VMS_END, + vmsd: ::core::ptr::null(), + version_id: 0, + struct_version_id: 0, + field_exists: None, + } + ]; + _FIELDS.as_ptr() + }} +} + +/// A transparent wrapper type for the `subsections` field of +/// [`VMStateDescription`](crate::bindings::VMStateDescription). +/// +/// This is necessary to be able to declare subsection descriptions as statics, +/// because the only way to implement `Sync` for a foreign type (and `*const` +/// pointers are foreign types in Rust) is to create a wrapper struct and +/// `unsafe impl Sync` for it. +/// +/// This struct is used in the +/// [`vm_state_subsections`](crate::vmstate_subsections) macro implementation. +#[repr(transparent)] +pub struct VMStateSubsectionsWrapper(pub &'static [*const crate::bindings::VMStateDescription]); + +unsafe impl Sync for VMStateSubsectionsWrapper {} + +/// Helper macro to declare a list of subsections +/// ([`VMStateDescription`](`crate::bindings::VMStateDescription`)) into a +/// static and return a pointer to the array of pointers it created. +#[macro_export] +macro_rules! vmstate_subsections { + ($($subsection:expr),*$(,)*) => {{ + static _SUBSECTIONS: $crate::vmstate::VMStateSubsectionsWrapper = $crate::vmstate::VMStateSubsectionsWrapper(&[ + $({ + static _SUBSECTION: $crate::bindings::VMStateDescription = $subsection; + ::core::ptr::addr_of!(_SUBSECTION) + }),*, + ::core::ptr::null() + ]); + _SUBSECTIONS.0.as_ptr() + }} +} diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs index aa1e0568c6..37c4dd44f8 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -8,17 +8,18 @@ use qemu_api::{ bindings::*, declare_properties, define_property, definitions::{Class, ObjectImpl}, - device_class_init, vm_state_description, + device_class_init, + zeroable::Zeroable, }; #[test] fn test_device_decl_macros() { // Test that macros can compile. - vm_state_description! { - VMSTATE, - name: c"name", + pub static VMSTATE: VMStateDescription = VMStateDescription { + name: c"name".as_ptr(), unmigratable: true, - } + ..Zeroable::ZERO + }; #[repr(C)] #[derive(qemu_api_macros::Object)] -- cgit 1.4.1 From 9f7d4520d679364f7ca95b7ddb899ff084e7d7c6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 24 Oct 2024 13:53:59 +0200 Subject: rust: use std::os::raw instead of core::ffi core::ffi::c_* types were introduced in Rust 1.64.0. Use the older types in std::os::raw, which are now aliases of the types in core::ffi. There is no need to compile QEMU as no_std, so this is acceptable as long as we support a version of Debian with Rust 1.63.0. Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- meson.build | 3 +-- rust/hw/char/pl011/src/device.rs | 35 +++++++++++++---------------------- rust/hw/char/pl011/src/lib.rs | 4 ++-- rust/hw/char/pl011/src/memory_ops.rs | 14 +++----------- rust/qemu-api/src/definitions.rs | 2 +- rust/qemu-api/src/device_class.rs | 6 +++--- rust/qemu-api/src/lib.rs | 11 +++++++---- rust/qemu-api/src/vmstate.rs | 10 +++++----- rust/qemu-api/tests/tests.rs | 9 ++++----- 9 files changed, 39 insertions(+), 55 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/meson.build b/meson.build index ac0f03d2e7..8ee4b7dd15 100644 --- a/meson.build +++ b/meson.build @@ -3953,14 +3953,13 @@ if have_rust and have_system bindgen_args = [ '--disable-header-comment', '--raw-line', '// @generated', - '--ctypes-prefix', 'core::ffi', + '--ctypes-prefix', 'std::os::raw', '--formatter', 'rustfmt', '--generate-block', '--generate-cstr', '--impl-debug', '--merge-extern-blocks', '--no-doc-comments', - '--use-core', '--with-derive-default', '--no-layout-tests', '--no-prepend-enum-name', diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index 788b47203b..036757f7f3 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -2,9 +2,10 @@ // Author(s): Manos Pitsidianakis // SPDX-License-Identifier: GPL-2.0-or-later -use core::{ - ffi::{c_int, c_uchar, c_uint, c_void, CStr}, - ptr::{addr_of, addr_of_mut, NonNull}, +use core::ptr::{addr_of, addr_of_mut, NonNull}; +use std::{ + ffi::CStr, + os::raw::{c_int, c_uchar, c_uint, c_void}, }; use qemu_api::{ @@ -117,11 +118,10 @@ pub struct PL011Class { } impl qemu_api::definitions::Class for PL011Class { - const CLASS_INIT: Option< - unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void), - > = Some(crate::device_class::pl011_class_init); + const CLASS_INIT: Option = + Some(crate::device_class::pl011_class_init); const CLASS_BASE_INIT: Option< - unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void), + unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void), > = None; } @@ -176,11 +176,7 @@ impl PL011State { } } - pub fn read( - &mut self, - offset: hwaddr, - _size: core::ffi::c_uint, - ) -> std::ops::ControlFlow { + pub fn read(&mut self, offset: hwaddr, _size: c_uint) -> std::ops::ControlFlow { use RegisterOffset::*; std::ops::ControlFlow::Break(match RegisterOffset::try_from(offset) { @@ -562,11 +558,7 @@ pub unsafe extern "C" fn pl011_can_receive(opaque: *mut c_void) -> c_int { /// readable/writeable from one thread at any time. /// /// The buffer and size arguments must also be valid. -pub unsafe extern "C" fn pl011_receive( - opaque: *mut core::ffi::c_void, - buf: *const u8, - size: core::ffi::c_int, -) { +pub unsafe extern "C" fn pl011_receive(opaque: *mut c_void, buf: *const u8, size: c_int) { unsafe { debug_assert!(!opaque.is_null()); let mut state = NonNull::new_unchecked(opaque.cast::()); @@ -585,7 +577,7 @@ pub unsafe extern "C" fn pl011_receive( /// We expect the FFI user of this function to pass a valid pointer, that has /// the same size as [`PL011State`]. We also expect the device is /// readable/writeable from one thread at any time. -pub unsafe extern "C" fn pl011_event(opaque: *mut core::ffi::c_void, event: QEMUChrEvent) { +pub unsafe extern "C" fn pl011_event(opaque: *mut c_void, event: QEMUChrEvent) { unsafe { debug_assert!(!opaque.is_null()); let mut state = NonNull::new_unchecked(opaque.cast::()); @@ -656,11 +648,10 @@ pub unsafe extern "C" fn pl011_luminary_init(obj: *mut Object) { } impl qemu_api::definitions::Class for PL011LuminaryClass { - const CLASS_INIT: Option< - unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void), - > = None; + const CLASS_INIT: Option = + None; const CLASS_BASE_INIT: Option< - unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void), + unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void), > = None; } diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs index fb33110d3d..69e96d7285 100644 --- a/rust/hw/char/pl011/src/lib.rs +++ b/rust/hw/char/pl011/src/lib.rs @@ -46,8 +46,8 @@ pub mod device; pub mod device_class; pub mod memory_ops; -pub const TYPE_PL011: &::core::ffi::CStr = c"pl011"; -pub const TYPE_PL011_LUMINARY: &::core::ffi::CStr = c"pl011_luminary"; +pub const TYPE_PL011: &::std::ffi::CStr = c"pl011"; +pub const TYPE_PL011_LUMINARY: &::std::ffi::CStr = c"pl011_luminary"; /// Offset of each register from the base memory address of the device. /// diff --git a/rust/hw/char/pl011/src/memory_ops.rs b/rust/hw/char/pl011/src/memory_ops.rs index fc69922fbf..169d485a4d 100644 --- a/rust/hw/char/pl011/src/memory_ops.rs +++ b/rust/hw/char/pl011/src/memory_ops.rs @@ -3,6 +3,7 @@ // SPDX-License-Identifier: GPL-2.0-or-later use core::ptr::NonNull; +use std::os::raw::{c_uint, c_void}; use qemu_api::{bindings::*, zeroable::Zeroable}; @@ -22,11 +23,7 @@ pub static PL011_OPS: MemoryRegionOps = MemoryRegionOps { }, }; -unsafe extern "C" fn pl011_read( - opaque: *mut core::ffi::c_void, - addr: hwaddr, - size: core::ffi::c_uint, -) -> u64 { +unsafe extern "C" fn pl011_read(opaque: *mut c_void, addr: hwaddr, size: c_uint) -> u64 { assert!(!opaque.is_null()); let mut state = unsafe { NonNull::new_unchecked(opaque.cast::()) }; let val = unsafe { state.as_mut().read(addr, size) }; @@ -43,12 +40,7 @@ unsafe extern "C" fn pl011_read( } } -unsafe extern "C" fn pl011_write( - opaque: *mut core::ffi::c_void, - addr: hwaddr, - data: u64, - _size: core::ffi::c_uint, -) { +unsafe extern "C" fn pl011_write(opaque: *mut c_void, addr: hwaddr, data: u64, _size: c_uint) { unsafe { assert!(!opaque.is_null()); let mut state = NonNull::new_unchecked(opaque.cast::()); diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 064afe6054..26597934bb 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -4,7 +4,7 @@ //! Definitions required by QEMU when registering a device. -use ::core::ffi::{c_void, CStr}; +use std::{ffi::CStr, os::raw::c_void}; use crate::bindings::{Object, ObjectClass, TypeInfo}; diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index 3d40256f60..cb4573ca6e 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -7,7 +7,7 @@ macro_rules! device_class_init { ($func:ident, props => $props:ident, realize_fn => $realize_fn:expr, legacy_reset_fn => $legacy_reset_fn:expr, vmsd => $vmsd:ident$(,)*) => { pub unsafe extern "C" fn $func( klass: *mut $crate::bindings::ObjectClass, - _: *mut ::core::ffi::c_void, + _: *mut ::std::os::raw::c_void, ) { let mut dc = ::core::ptr::NonNull::new(klass.cast::<$crate::bindings::DeviceClass>()).unwrap(); @@ -26,7 +26,7 @@ macro_rules! define_property { ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr, default = $defval:expr$(,)*) => { $crate::bindings::Property { // use associated function syntax for type checking - name: ::core::ffi::CStr::as_ptr($name), + name: ::std::ffi::CStr::as_ptr($name), info: $prop, offset: ::core::mem::offset_of!($state, $field) as isize, set_default: true, @@ -37,7 +37,7 @@ macro_rules! define_property { ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr$(,)*) => { $crate::bindings::Property { // use associated function syntax for type checking - name: ::core::ffi::CStr::as_ptr($name), + name: ::std::ffi::CStr::as_ptr($name), info: $prop, offset: ::core::mem::offset_of!($state, $field) as isize, set_default: false, diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index 10ab3d7e63..ed840ee2f7 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -34,7 +34,10 @@ pub mod device_class; pub mod vmstate; pub mod zeroable; -use std::alloc::{GlobalAlloc, Layout}; +use std::{ + alloc::{GlobalAlloc, Layout}, + os::raw::c_void, +}; #[cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)] extern "C" { @@ -48,8 +51,8 @@ extern "C" { #[cfg(not(HAVE_GLIB_WITH_ALIGNED_ALLOC))] extern "C" { - fn qemu_memalign(alignment: usize, size: usize) -> *mut ::core::ffi::c_void; - fn qemu_vfree(ptr: *mut ::core::ffi::c_void); + fn qemu_memalign(alignment: usize, size: usize) -> *mut c_void; + fn qemu_vfree(ptr: *mut c_void); } extern "C" { @@ -114,7 +117,7 @@ impl Default for QemuAllocator { } // Sanity check. -const _: [(); 8] = [(); ::core::mem::size_of::<*mut ::core::ffi::c_void>()]; +const _: [(); 8] = [(); ::core::mem::size_of::<*mut c_void>()]; unsafe impl GlobalAlloc for QemuAllocator { unsafe fn alloc(&self, layout: Layout) -> *mut u8 { diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs index 0c1197277f..4e06e40505 100644 --- a/rust/qemu-api/src/vmstate.rs +++ b/rust/qemu-api/src/vmstate.rs @@ -56,7 +56,7 @@ macro_rules! vmstate_single_test { $crate::bindings::VMStateField { name: ::core::concat!(::core::stringify!($field_name), 0) .as_bytes() - .as_ptr() as *const ::core::ffi::c_char, + .as_ptr() as *const ::std::os::raw::c_char, err_hint: ::core::ptr::null(), offset: ::core::mem::offset_of!($struct_name, $field_name), size: $size, @@ -133,7 +133,7 @@ macro_rules! vmstate_array { $crate::bindings::VMStateField { name: ::core::concat!(::core::stringify!($field_name), 0) .as_bytes() - .as_ptr() as *const ::core::ffi::c_char, + .as_ptr() as *const ::std::os::raw::c_char, err_hint: ::core::ptr::null(), offset: ::core::mem::offset_of!($struct_name, $field_name), size: $size, @@ -181,7 +181,7 @@ macro_rules! vmstate_struct_pointer_v { $crate::bindings::VMStateField { name: ::core::concat!(::core::stringify!($field_name), 0) .as_bytes() - .as_ptr() as *const ::core::ffi::c_char, + .as_ptr() as *const ::std::os::raw::c_char, err_hint: ::core::ptr::null(), offset: ::core::mem::offset_of!($struct_name, $field_name), size: ::core::mem::size_of::<*const $type>(), @@ -206,7 +206,7 @@ macro_rules! vmstate_array_of_pointer { $crate::bindings::VMStateField { name: ::core::concat!(::core::stringify!($field_name), 0) .as_bytes() - .as_ptr() as *const ::core::ffi::c_char, + .as_ptr() as *const ::std::os::raw::c_char, version_id: $version_id, num: $num as _, info: unsafe { $info }, @@ -231,7 +231,7 @@ macro_rules! vmstate_array_of_pointer_to_struct { $crate::bindings::VMStateField { name: ::core::concat!(::core::stringify!($field_name), 0) .as_bytes() - .as_ptr() as *const ::core::ffi::c_char, + .as_ptr() as *const ::std::os::raw::c_char, version_id: $version_id, num: $num as _, vmsd: unsafe { $vmsd }, diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs index 37c4dd44f8..c7089f0cf2 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -2,7 +2,7 @@ // Author(s): Manos Pitsidianakis // SPDX-License-Identifier: GPL-2.0-or-later -use core::ffi::CStr; +use std::{ffi::CStr, os::raw::c_void}; use qemu_api::{ bindings::*, @@ -64,11 +64,10 @@ fn test_device_decl_macros() { } impl Class for DummyClass { - const CLASS_INIT: Option< - unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void), - > = Some(dummy_class_init); + const CLASS_INIT: Option = + Some(dummy_class_init); const CLASS_BASE_INIT: Option< - unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void), + unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void), > = None; } -- cgit 1.4.1 From 718e255f0a97cf43939ae2e90ba4673ae9a8bd2f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 25 Oct 2024 08:23:53 +0200 Subject: rust: introduce a c_str macro This allows CStr constants to be defined easily on Rust 1.63.0, while checking that there are no embedded NULs. c"" literals were only stabilized in Rust 1.77.0. Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device.rs | 5 ++-- rust/hw/char/pl011/src/device_class.rs | 18 +++++------- rust/hw/char/pl011/src/lib.rs | 6 ++-- rust/qemu-api/meson.build | 4 +++ rust/qemu-api/src/c_str.rs | 53 ++++++++++++++++++++++++++++++++++ rust/qemu-api/src/lib.rs | 1 + rust/qemu-api/src/vmstate.rs | 2 +- rust/qemu-api/tests/tests.rs | 8 ++--- 8 files changed, 78 insertions(+), 19 deletions(-) create mode 100644 rust/qemu-api/src/c_str.rs (limited to 'rust/qemu-api/src') diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index 036757f7f3..2d225d544d 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -10,6 +10,7 @@ use std::{ use qemu_api::{ bindings::{self, *}, + c_str, definitions::ObjectImpl, }; @@ -135,7 +136,7 @@ impl PL011State { /// location/instance. All its fields are expected to hold unitialized /// values with the sole exception of `parent_obj`. unsafe fn init(&mut self) { - const CLK_NAME: &CStr = c"clk"; + const CLK_NAME: &CStr = c_str!("clk"); let dev = addr_of_mut!(*self).cast::(); // SAFETY: @@ -598,7 +599,7 @@ pub unsafe extern "C" fn pl011_create( let dev: *mut DeviceState = qdev_new(PL011State::TYPE_INFO.name); let sysbus: *mut SysBusDevice = dev.cast::(); - qdev_prop_set_chr(dev, c"chardev".as_ptr(), chr); + qdev_prop_set_chr(dev, c_str!("chardev").as_ptr(), chr); sysbus_realize_and_unref(sysbus, addr_of!(error_fatal) as *mut *mut Error); sysbus_mmio_map(sysbus, 0, addr); sysbus_connect_irq(sysbus, 0, irq); diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs index 6a554ad792..a707fde138 100644 --- a/rust/hw/char/pl011/src/device_class.rs +++ b/rust/hw/char/pl011/src/device_class.rs @@ -2,14 +2,12 @@ // Author(s): Manos Pitsidianakis // SPDX-License-Identifier: GPL-2.0-or-later -use core::{ - ffi::{c_int, c_void}, - ptr::NonNull, -}; +use core::ptr::NonNull; +use std::os::raw::{c_int, c_void}; use qemu_api::{ - bindings::*, vmstate_clock, vmstate_fields, vmstate_int32, vmstate_subsections, vmstate_uint32, - vmstate_uint32_array, vmstate_unused, zeroable::Zeroable, + bindings::*, c_str, vmstate_clock, vmstate_fields, vmstate_int32, vmstate_subsections, + vmstate_uint32, vmstate_uint32_array, vmstate_unused, zeroable::Zeroable, }; use crate::device::{PL011State, PL011_FIFO_DEPTH}; @@ -24,7 +22,7 @@ extern "C" fn pl011_clock_needed(opaque: *mut c_void) -> bool { /// Migration subsection for [`PL011State`] clock. pub static VMSTATE_PL011_CLOCK: VMStateDescription = VMStateDescription { - name: c"pl011/clock".as_ptr(), + name: c_str!("pl011/clock").as_ptr(), version_id: 1, minimum_version_id: 1, needed: Some(pl011_clock_needed), @@ -48,7 +46,7 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, version_id: c_int) -> c_int { } pub static VMSTATE_PL011: VMStateDescription = VMStateDescription { - name: c"pl011".as_ptr(), + name: c_str!("pl011").as_ptr(), version_id: 2, minimum_version_id: 2, post_load: Some(pl011_post_load), @@ -79,14 +77,14 @@ pub static VMSTATE_PL011: VMStateDescription = VMStateDescription { qemu_api::declare_properties! { PL011_PROPERTIES, qemu_api::define_property!( - c"chardev", + c_str!("chardev"), PL011State, char_backend, unsafe { &qdev_prop_chr }, CharBackend ), qemu_api::define_property!( - c"migrate-clk", + c_str!("migrate-clk"), PL011State, migrate_clock, unsafe { &qdev_prop_bool }, diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs index 69e96d7285..cd0a49acb9 100644 --- a/rust/hw/char/pl011/src/lib.rs +++ b/rust/hw/char/pl011/src/lib.rs @@ -42,12 +42,14 @@ extern crate bilge; extern crate bilge_impl; extern crate qemu_api; +use qemu_api::c_str; + pub mod device; pub mod device_class; pub mod memory_ops; -pub const TYPE_PL011: &::std::ffi::CStr = c"pl011"; -pub const TYPE_PL011_LUMINARY: &::std::ffi::CStr = c"pl011_luminary"; +pub const TYPE_PL011: &::std::ffi::CStr = c_str!("pl011"); +pub const TYPE_PL011_LUMINARY: &::std::ffi::CStr = c_str!("pl011_luminary"); /// Offset of each register from the base memory address of the device. /// diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build index 3b849f7c41..c950b008d5 100644 --- a/rust/qemu-api/meson.build +++ b/rust/qemu-api/meson.build @@ -3,6 +3,7 @@ _qemu_api_rs = static_library( structured_sources( [ 'src/lib.rs', + 'src/c_str.rs', 'src/definitions.rs', 'src/device_class.rs', 'src/vmstate.rs', @@ -18,6 +19,9 @@ _qemu_api_rs = static_library( ], ) +rust.test('rust-qemu-api-tests', _qemu_api_rs, + suite: ['unit', 'rust']) + qemu_api = declare_dependency( link_with: _qemu_api_rs, dependencies: qemu_api_macros, diff --git a/rust/qemu-api/src/c_str.rs b/rust/qemu-api/src/c_str.rs new file mode 100644 index 0000000000..4cd96da0b4 --- /dev/null +++ b/rust/qemu-api/src/c_str.rs @@ -0,0 +1,53 @@ +// Copyright 2024 Red Hat, Inc. +// Author(s): Paolo Bonzini +// SPDX-License-Identifier: GPL-2.0-or-later + +#[macro_export] +/// Given a string constant _without_ embedded or trailing NULs, return +/// a `CStr`. +/// +/// Needed for compatibility with Rust <1.77. +macro_rules! c_str { + ($str:expr) => {{ + const STRING: &str = concat!($str, "\0"); + const BYTES: &[u8] = STRING.as_bytes(); + + // "for" is not allowed in const context... oh well, + // everybody loves some lisp. This could be turned into + // a procedural macro if this is a problem; alternatively + // Rust 1.72 makes CStr::from_bytes_with_nul a const function. + const fn f(b: &[u8], i: usize) { + if i == b.len() - 1 { + } else if b[i] == 0 { + panic!("c_str argument contains NUL") + } else { + f(b, i + 1) + } + } + f(BYTES, 0); + + // SAFETY: absence of NULs apart from the final byte was checked above + unsafe { std::ffi::CStr::from_bytes_with_nul_unchecked(BYTES) } + }}; +} + +#[cfg(test)] +mod tests { + use std::ffi::CStr; + + use crate::c_str; + + #[test] + fn test_cstr_macro() { + let good = c_str!("🦀"); + let good_bytes = b"\xf0\x9f\xa6\x80\0"; + assert_eq!(good.to_bytes_with_nul(), good_bytes); + } + + #[test] + fn test_cstr_macro_const() { + const GOOD: &CStr = c_str!("🦀"); + const GOOD_BYTES: &[u8] = b"\xf0\x9f\xa6\x80\0"; + assert_eq!(GOOD.to_bytes_with_nul(), GOOD_BYTES); + } +} diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index ed840ee2f7..e6bd953e10 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -29,6 +29,7 @@ unsafe impl Sync for bindings::VMStateDescription {} unsafe impl Sync for bindings::VMStateField {} unsafe impl Sync for bindings::VMStateInfo {} +pub mod c_str; pub mod definitions; pub mod device_class; pub mod vmstate; diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs index 4e06e40505..9c252ce18e 100644 --- a/rust/qemu-api/src/vmstate.rs +++ b/rust/qemu-api/src/vmstate.rs @@ -15,7 +15,7 @@ macro_rules! vmstate_unused_buffer { ($field_exists_fn:expr, $version_id:expr, $size:expr) => {{ $crate::bindings::VMStateField { - name: c"unused".as_ptr(), + name: c_str!("unused").as_ptr(), err_hint: ::core::ptr::null(), offset: 0, size: $size, diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs index c7089f0cf2..381ac84657 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -6,7 +6,7 @@ use std::{ffi::CStr, os::raw::c_void}; use qemu_api::{ bindings::*, - declare_properties, define_property, + c_str, declare_properties, define_property, definitions::{Class, ObjectImpl}, device_class_init, zeroable::Zeroable, @@ -16,7 +16,7 @@ use qemu_api::{ fn test_device_decl_macros() { // Test that macros can compile. pub static VMSTATE: VMStateDescription = VMStateDescription { - name: c"name".as_ptr(), + name: c_str!("name").as_ptr(), unmigratable: true, ..Zeroable::ZERO }; @@ -36,7 +36,7 @@ fn test_device_decl_macros() { declare_properties! { DUMMY_PROPERTIES, define_property!( - c"migrate-clk", + c_str!("migrate-clk"), DummyState, migrate_clock, unsafe { &qdev_prop_bool }, @@ -55,7 +55,7 @@ fn test_device_decl_macros() { impl ObjectImpl for DummyState { type Class = DummyClass; const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self }; - const TYPE_NAME: &'static CStr = c"dummy"; + const TYPE_NAME: &'static CStr = c_str!("dummy"); const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_DEVICE); const ABSTRACT: bool = false; const INSTANCE_INIT: Option = None; -- cgit 1.4.1 From f3518400882022ddcbe1148abf2165917a7b4640 Mon Sep 17 00:00:00 2001 From: Junjie Mao Date: Thu, 24 Oct 2024 12:25:15 +0200 Subject: rust: introduce alternative implementation of offset_of! offset_of! was stabilized in Rust 1.77.0. Use an alternative implemenation that was found on the Rust forums, and whose author agreed to license as MIT for use in QEMU. The alternative allows only one level of field access, but apart from this can be used just by replacing core::mem::offset_of! with qemu_api::offset_of!. The actual implementation of offset_of! is done in a declarative macro, but for simplicity and to avoid introducing an extra level of indentation, the trigger is a procedural macro #[derive(offsets)]. The procedural macro is perhaps a bit overengineered, but it helps introducing some idioms that will be useful in the future as well. Signed-off-by: Junjie Mao Co-developed-by: Paolo Bonzini Signed-off-by: Paolo Bonzini --- rust/Cargo.lock | 1 + rust/hw/char/pl011/src/device.rs | 2 +- rust/qemu-api-macros/Cargo.toml | 2 +- rust/qemu-api-macros/src/lib.rs | 75 +++++++++++- rust/qemu-api/Cargo.toml | 6 +- rust/qemu-api/build.rs | 9 ++ rust/qemu-api/meson.build | 12 +- rust/qemu-api/src/device_class.rs | 8 +- rust/qemu-api/src/lib.rs | 4 + rust/qemu-api/src/offset_of.rs | 161 ++++++++++++++++++++++++++ rust/qemu-api/src/vmstate.rs | 10 +- rust/qemu-api/tests/tests.rs | 1 + subprojects/packagefiles/syn-2-rs/meson.build | 1 + 13 files changed, 274 insertions(+), 18 deletions(-) create mode 100644 rust/qemu-api/src/offset_of.rs (limited to 'rust/qemu-api/src') diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 9f43b33e8b..c0c6069247 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -93,6 +93,7 @@ name = "qemu_api" version = "0.1.0" dependencies = [ "qemu_api_macros", + "version_check", ] [[package]] diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index 2d225d544d..bca727e37f 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -55,7 +55,7 @@ impl DeviceId { } #[repr(C)] -#[derive(Debug, qemu_api_macros::Object)] +#[derive(Debug, qemu_api_macros::Object, qemu_api_macros::offsets)] /// PL011 Device Model in QEMU pub struct PL011State { pub parent_obj: SysBusDevice, diff --git a/rust/qemu-api-macros/Cargo.toml b/rust/qemu-api-macros/Cargo.toml index f8d6d03609..a8f7377106 100644 --- a/rust/qemu-api-macros/Cargo.toml +++ b/rust/qemu-api-macros/Cargo.toml @@ -19,4 +19,4 @@ proc-macro = true [dependencies] proc-macro2 = "1" quote = "1" -syn = "2" +syn = { version = "2", features = ["extra-traits"] } diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs index a4bc5d01ee..cf99ac04b8 100644 --- a/rust/qemu-api-macros/src/lib.rs +++ b/rust/qemu-api-macros/src/lib.rs @@ -3,8 +3,34 @@ // SPDX-License-Identifier: GPL-2.0-or-later use proc_macro::TokenStream; -use quote::quote; -use syn::{parse_macro_input, DeriveInput}; +use proc_macro2::Span; +use quote::{quote, quote_spanned}; +use syn::{ + parse_macro_input, parse_quote, punctuated::Punctuated, token::Comma, Data, DeriveInput, Field, + Fields, Ident, Type, Visibility, +}; + +struct CompileError(String, Span); + +impl From for proc_macro2::TokenStream { + fn from(err: CompileError) -> Self { + let CompileError(msg, span) = err; + quote_spanned! { span => compile_error!(#msg); } + } +} + +fn is_c_repr(input: &DeriveInput, msg: &str) -> Result<(), CompileError> { + let expected = parse_quote! { #[repr(C)] }; + + if input.attrs.iter().any(|attr| attr == &expected) { + Ok(()) + } else { + Err(CompileError( + format!("#[repr(C)] required for {}", msg), + input.ident.span(), + )) + } +} #[proc_macro_derive(Object)] pub fn derive_object(input: TokenStream) -> TokenStream { @@ -21,3 +47,48 @@ pub fn derive_object(input: TokenStream) -> TokenStream { TokenStream::from(expanded) } + +fn get_fields(input: &DeriveInput) -> Result<&Punctuated, CompileError> { + if let Data::Struct(s) = &input.data { + if let Fields::Named(fs) = &s.fields { + Ok(&fs.named) + } else { + Err(CompileError( + "Cannot generate offsets for unnamed fields.".to_string(), + input.ident.span(), + )) + } + } else { + Err(CompileError( + "Cannot generate offsets for union or enum.".to_string(), + input.ident.span(), + )) + } +} + +#[rustfmt::skip::macros(quote)] +fn derive_offsets_or_error(input: DeriveInput) -> Result { + is_c_repr(&input, "#[derive(offsets)]")?; + + let name = &input.ident; + let fields = get_fields(&input)?; + let field_names: Vec<&Ident> = fields.iter().map(|f| f.ident.as_ref().unwrap()).collect(); + let field_types: Vec<&Type> = fields.iter().map(|f| &f.ty).collect(); + let field_vis: Vec<&Visibility> = fields.iter().map(|f| &f.vis).collect(); + + Ok(quote! { + ::qemu_api::with_offsets! { + struct #name { + #(#field_vis #field_names: #field_types,)* + } + } + }) +} + +#[proc_macro_derive(offsets)] +pub fn derive_offsets(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as DeriveInput); + let expanded = derive_offsets_or_error(input).unwrap_or_else(Into::into); + + TokenStream::from(expanded) +} diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml index e092f61e8f..cc716d75d4 100644 --- a/rust/qemu-api/Cargo.toml +++ b/rust/qemu-api/Cargo.toml @@ -16,9 +16,13 @@ categories = [] [dependencies] qemu_api_macros = { path = "../qemu-api-macros" } +[build-dependencies] +version_check = "~0.9" + [features] default = [] allocator = [] [lints.rust] -unexpected_cfgs = { level = "warn", check-cfg = ['cfg(MESON)', 'cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)'] } +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(MESON)', 'cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)', + 'cfg(has_offset_of)'] } diff --git a/rust/qemu-api/build.rs b/rust/qemu-api/build.rs index 419b154c2d..20f8f718b9 100644 --- a/rust/qemu-api/build.rs +++ b/rust/qemu-api/build.rs @@ -4,6 +4,8 @@ use std::path::Path; +use version_check as rustc; + fn main() { if !Path::new("src/bindings.rs").exists() { panic!( @@ -11,4 +13,11 @@ fn main() { (`ninja bindings.rs`) and copy them to src/bindings.rs, or build through meson." ); } + + // Check for available rustc features + if rustc::is_min_version("1.77.0").unwrap_or(false) { + println!("cargo:rustc-cfg=has_offset_of"); + } + + println!("cargo:rerun-if-changed=build.rs"); } diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build index c950b008d5..6f637af7b1 100644 --- a/rust/qemu-api/meson.build +++ b/rust/qemu-api/meson.build @@ -1,3 +1,9 @@ +_qemu_api_cfg = ['--cfg', 'MESON'] +# _qemu_api_cfg += ['--cfg', 'feature="allocator"'] +if rustc.version().version_compare('>=1.77.0') + _qemu_api_cfg += ['--cfg', 'has_offset_of'] +endif + _qemu_api_rs = static_library( 'qemu_api', structured_sources( @@ -6,6 +12,7 @@ _qemu_api_rs = static_library( 'src/c_str.rs', 'src/definitions.rs', 'src/device_class.rs', + 'src/offset_of.rs', 'src/vmstate.rs', 'src/zeroable.rs', ], @@ -13,10 +20,7 @@ _qemu_api_rs = static_library( ), override_options: ['rust_std=2021', 'build.rust_std=2021'], rust_abi: 'rust', - rust_args: [ - '--cfg', 'MESON', - # '--cfg', 'feature="allocator"', - ], + rust_args: _qemu_api_cfg, ) rust.test('rust-qemu-api-tests', _qemu_api_rs, diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index cb4573ca6e..56608c7f7f 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -23,23 +23,23 @@ macro_rules! device_class_init { #[macro_export] macro_rules! define_property { - ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr, default = $defval:expr$(,)*) => { + ($name:expr, $state:ty, $field:ident, $prop:expr, $type:expr, default = $defval:expr$(,)*) => { $crate::bindings::Property { // use associated function syntax for type checking name: ::std::ffi::CStr::as_ptr($name), info: $prop, - offset: ::core::mem::offset_of!($state, $field) as isize, + offset: $crate::offset_of!($state, $field) as isize, set_default: true, defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 }, ..$crate::zeroable::Zeroable::ZERO } }; - ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr$(,)*) => { + ($name:expr, $state:ty, $field:ident, $prop:expr, $type:expr$(,)*) => { $crate::bindings::Property { // use associated function syntax for type checking name: ::std::ffi::CStr::as_ptr($name), info: $prop, - offset: ::core::mem::offset_of!($state, $field) as isize, + offset: $crate::offset_of!($state, $field) as isize, set_default: false, ..$crate::zeroable::Zeroable::ZERO } diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index e6bd953e10..aa8d16ec94 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -32,6 +32,7 @@ unsafe impl Sync for bindings::VMStateInfo {} pub mod c_str; pub mod definitions; pub mod device_class; +pub mod offset_of; pub mod vmstate; pub mod zeroable; @@ -169,3 +170,6 @@ unsafe impl GlobalAlloc for QemuAllocator { } } } + +#[cfg(has_offset_of)] +pub use core::mem::offset_of; diff --git a/rust/qemu-api/src/offset_of.rs b/rust/qemu-api/src/offset_of.rs new file mode 100644 index 0000000000..075e98f986 --- /dev/null +++ b/rust/qemu-api/src/offset_of.rs @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: MIT + +/// This macro provides the same functionality as `core::mem::offset_of`, +/// except that only one level of field access is supported. The declaration +/// of the struct must be wrapped with `with_offsets! { }`. +/// +/// It is needed because `offset_of!` was only stabilized in Rust 1.77. +#[cfg(not(has_offset_of))] +#[macro_export] +macro_rules! offset_of { + ($Container:ty, $field:ident) => { + <$Container>::OFFSET_TO__.$field + }; +} + +/// A wrapper for struct declarations, that allows using `offset_of!` in +/// versions of Rust prior to 1.77 +#[macro_export] +macro_rules! with_offsets { + // This method to generate field offset constants comes from: + // + // https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=10a22a9b8393abd7b541d8fc844bc0df + // + // used under MIT license with permission of Yandros aka Daniel Henry-Mantilla + ( + $(#[$struct_meta:meta])* + $struct_vis:vis + struct $StructName:ident { + $( + $(#[$field_meta:meta])* + $field_vis:vis + $field_name:ident : $field_ty:ty + ),* + $(,)? + } + ) => ( + #[cfg(not(has_offset_of))] + const _: () = { + struct StructOffsetsHelper(std::marker::PhantomData); + const END_OF_PREV_FIELD: usize = 0; + + // populate StructOffsetsHelper with associated consts, + // one for each field + $crate::with_offsets! { + @struct $StructName + @names [ $($field_name)* ] + @tys [ $($field_ty ,)*] + } + + // now turn StructOffsetsHelper's consts into a single struct, + // applying field visibility. This provides better error messages + // than if offset_of! used StructOffsetsHelper:: directly. + pub + struct StructOffsets { + $( + $field_vis + $field_name: usize, + )* + } + impl $StructName { + pub + const OFFSET_TO__: StructOffsets = StructOffsets { + $( + $field_name: StructOffsetsHelper::<$StructName>::$field_name, + )* + }; + } + }; + ); + + ( + @struct $StructName:ident + @names [] + @tys [] + ) => (); + + ( + @struct $StructName:ident + @names [$field_name:ident $($other_names:tt)*] + @tys [$field_ty:ty , $($other_tys:tt)*] + ) => ( + #[allow(non_local_definitions)] + #[allow(clippy::modulo_one)] + impl StructOffsetsHelper<$StructName> { + #[allow(nonstandard_style)] + const $field_name: usize = { + const ALIGN: usize = std::mem::align_of::<$field_ty>(); + const TRAIL: usize = END_OF_PREV_FIELD % ALIGN; + END_OF_PREV_FIELD + (if TRAIL == 0 { 0usize } else { ALIGN - TRAIL }) + }; + } + const _: () = { + const END_OF_PREV_FIELD: usize = + StructOffsetsHelper::<$StructName>::$field_name + + std::mem::size_of::<$field_ty>() + ; + $crate::with_offsets! { + @struct $StructName + @names [$($other_names)*] + @tys [$($other_tys)*] + } + }; + ); +} + +#[cfg(test)] +mod tests { + use crate::offset_of; + + #[repr(C)] + struct Foo { + a: u16, + b: u32, + c: u64, + d: u16, + } + + #[repr(C)] + struct Bar { + pub a: u16, + pub b: u64, + c: Foo, + d: u64, + } + + crate::with_offsets! { + #[repr(C)] + struct Bar { + pub a: u16, + pub b: u64, + c: Foo, + d: u64, + } + } + + #[repr(C)] + pub struct Baz { + b: u32, + a: u8, + } + crate::with_offsets! { + #[repr(C)] + pub struct Baz { + b: u32, + a: u8, + } + } + + #[test] + fn test_offset_of() { + const OFFSET_TO_C: usize = offset_of!(Bar, c); + + assert_eq!(offset_of!(Bar, a), 0); + assert_eq!(offset_of!(Bar, b), 8); + assert_eq!(OFFSET_TO_C, 16); + assert_eq!(offset_of!(Bar, d), 40); + + assert_eq!(offset_of!(Baz, b), 0); + assert_eq!(offset_of!(Baz, a), 4); + } +} diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs index 9c252ce18e..bedcf1e8f3 100644 --- a/rust/qemu-api/src/vmstate.rs +++ b/rust/qemu-api/src/vmstate.rs @@ -58,7 +58,7 @@ macro_rules! vmstate_single_test { .as_bytes() .as_ptr() as *const ::std::os::raw::c_char, err_hint: ::core::ptr::null(), - offset: ::core::mem::offset_of!($struct_name, $field_name), + offset: $crate::offset_of!($struct_name, $field_name), size: $size, start: 0, num: 0, @@ -135,7 +135,7 @@ macro_rules! vmstate_array { .as_bytes() .as_ptr() as *const ::std::os::raw::c_char, err_hint: ::core::ptr::null(), - offset: ::core::mem::offset_of!($struct_name, $field_name), + offset: $crate::offset_of!($struct_name, $field_name), size: $size, start: 0, num: $length as _, @@ -183,7 +183,7 @@ macro_rules! vmstate_struct_pointer_v { .as_bytes() .as_ptr() as *const ::std::os::raw::c_char, err_hint: ::core::ptr::null(), - offset: ::core::mem::offset_of!($struct_name, $field_name), + offset: $crate::offset_of!($struct_name, $field_name), size: ::core::mem::size_of::<*const $type>(), start: 0, num: 0, @@ -212,7 +212,7 @@ macro_rules! vmstate_array_of_pointer { info: unsafe { $info }, size: ::core::mem::size_of::<*const $type>(), flags: VMStateFlags(VMStateFlags::VMS_ARRAY.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0), - offset: ::core::mem::offset_of!($struct_name, $field_name), + offset: $crate::offset_of!($struct_name, $field_name), err_hint: ::core::ptr::null(), start: 0, num_offset: 0, @@ -241,7 +241,7 @@ macro_rules! vmstate_array_of_pointer_to_struct { | VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0, ), - offset: ::core::mem::offset_of!($struct_name, $field_name), + offset: $crate::offset_of!($struct_name, $field_name), err_hint: ::core::ptr::null(), start: 0, num_offset: 0, diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs index 381ac84657..7442f69564 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -21,6 +21,7 @@ fn test_device_decl_macros() { ..Zeroable::ZERO }; + #[derive(qemu_api_macros::offsets)] #[repr(C)] #[derive(qemu_api_macros::Object)] pub struct DummyState { diff --git a/subprojects/packagefiles/syn-2-rs/meson.build b/subprojects/packagefiles/syn-2-rs/meson.build index a53335f309..9f56ce1c24 100644 --- a/subprojects/packagefiles/syn-2-rs/meson.build +++ b/subprojects/packagefiles/syn-2-rs/meson.build @@ -24,6 +24,7 @@ _syn_rs = static_library( '--cfg', 'feature="printing"', '--cfg', 'feature="clone-impls"', '--cfg', 'feature="proc-macro"', + '--cfg', 'feature="extra-traits"', ], dependencies: [ quote_dep, -- cgit 1.4.1 From bb42965dd417042d171ecf98cb470c430b592134 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 18 Oct 2024 11:53:19 +0200 Subject: rust: do not use MaybeUninit::zeroed() MaybeUninit::zeroed() is handy but is not available as a "const" function until Rust 1.75.0. Remove the default implementation of Zeroable::ZERO, and write by hand the definitions for those types that need it. It may be possible to add automatic implementation of the trait, via a procedural macro and/or a trick similar to offset_of!, but do it the easy way for now. Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/zeroable.rs | 91 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 77 insertions(+), 14 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs index 45ec95c9f7..13cdb2ccba 100644 --- a/rust/qemu-api/src/zeroable.rs +++ b/rust/qemu-api/src/zeroable.rs @@ -1,23 +1,86 @@ // SPDX-License-Identifier: GPL-2.0-or-later +use std::ptr; + /// Encapsulates the requirement that -/// `MaybeUninit::::zeroed().assume_init()` does not cause -/// undefined behavior. +/// `MaybeUninit::::zeroed().assume_init()` does not cause undefined +/// behavior. This trait in principle could be implemented as just: +/// +/// ``` +/// const ZERO: Self = unsafe { +/// ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() +/// }, +/// ``` +/// +/// The need for a manual implementation is only because `zeroed()` cannot +/// be used as a `const fn` prior to Rust 1.75.0. Once we can assume a new +/// enough version of the compiler, we could provide a `#[derive(Zeroable)]` +/// macro to check at compile-time that all struct fields are Zeroable, and +/// use the above blanket implementation of the `ZERO` constant. /// /// # Safety /// -/// Do not add this trait to a type unless all-zeroes is -/// a valid value for the type. In particular, remember that raw -/// pointers can be zero, but references and `NonNull` cannot -/// unless wrapped with `Option<>`. +/// Because the implementation of `ZERO` is manual, it does not make +/// any assumption on the safety of `zeroed()`. However, other users of the +/// trait could use it that way. Do not add this trait to a type unless +/// all-zeroes is a valid value for the type. In particular, remember that +/// raw pointers can be zero, but references and `NonNull` cannot pub unsafe trait Zeroable: Default { - /// SAFETY: If the trait was added to a type, then by definition - /// this is safe. - const ZERO: Self = unsafe { ::core::mem::MaybeUninit::::zeroed().assume_init() }; + const ZERO: Self; +} + +unsafe impl Zeroable for crate::bindings::Property__bindgen_ty_1 { + const ZERO: Self = Self { i: 0 }; +} + +unsafe impl Zeroable for crate::bindings::Property { + const ZERO: Self = Self { + name: ptr::null(), + info: ptr::null(), + offset: 0, + bitnr: 0, + bitmask: 0, + set_default: false, + defval: Zeroable::ZERO, + arrayoffset: 0, + arrayinfo: ptr::null(), + arrayfieldsize: 0, + link_type: ptr::null(), + }; +} + +unsafe impl Zeroable for crate::bindings::VMStateDescription { + const ZERO: Self = Self { + name: ptr::null(), + unmigratable: false, + early_setup: false, + version_id: 0, + minimum_version_id: 0, + priority: crate::bindings::MigrationPriority::MIG_PRI_DEFAULT, + pre_load: None, + post_load: None, + pre_save: None, + post_save: None, + needed: None, + dev_unplug_pending: None, + fields: ptr::null(), + subsections: ptr::null(), + }; } -unsafe impl Zeroable for crate::bindings::Property__bindgen_ty_1 {} -unsafe impl Zeroable for crate::bindings::Property {} -unsafe impl Zeroable for crate::bindings::VMStateDescription {} -unsafe impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_1 {} -unsafe impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_2 {} +unsafe impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_1 { + const ZERO: Self = Self { + min_access_size: 0, + max_access_size: 0, + unaligned: false, + accepts: None, + }; +} + +unsafe impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_2 { + const ZERO: Self = Self { + min_access_size: 0, + max_access_size: 0, + unaligned: false, + }; +} -- cgit 1.4.1 From ce4a144cf817d401592918faa86f4903d3c78c3e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 25 Oct 2024 09:20:16 +0200 Subject: rust: do not use --generate-cstr --generate-cstr is a good idea and generally the right thing to do, but it is not available in Debian 12 and Ubuntu 22.04. Work around the absence. Signed-off-by: Paolo Bonzini --- meson.build | 4 +++- rust/hw/char/pl011/src/device.rs | 1 + rust/qemu-api/src/device_class.rs | 10 ++++++++++ rust/qemu-api/tests/tests.rs | 4 ++-- 4 files changed, 16 insertions(+), 3 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/meson.build b/meson.build index 11d7c3464c..5b85d4275b 100644 --- a/meson.build +++ b/meson.build @@ -3961,13 +3961,15 @@ common_all = static_library('common', dependencies: common_ss.all_dependencies()) if have_rust + # We would like to use --generate-cstr, but it is only available + # starting with bindgen 0.66.0. The oldest supported versions + # are in Ubuntu 22.04 (0.59.1) and Debian 12 (0.60.1). bindgen_args = [ '--disable-header-comment', '--raw-line', '// @generated', '--ctypes-prefix', 'std::os::raw', '--formatter', 'rustfmt', '--generate-block', - '--generate-cstr', '--impl-debug', '--merge-extern-blocks', '--no-doc-comments', diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index bca727e37f..2a85960b81 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -12,6 +12,7 @@ use qemu_api::{ bindings::{self, *}, c_str, definitions::ObjectImpl, + device_class::TYPE_SYS_BUS_DEVICE, }; use crate::{ diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index 56608c7f7f..0ba798d3e3 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -2,6 +2,10 @@ // Author(s): Manos Pitsidianakis // SPDX-License-Identifier: GPL-2.0-or-later +use std::ffi::CStr; + +use crate::bindings; + #[macro_export] macro_rules! device_class_init { ($func:ident, props => $props:ident, realize_fn => $realize_fn:expr, legacy_reset_fn => $legacy_reset_fn:expr, vmsd => $vmsd:ident$(,)*) => { @@ -62,3 +66,9 @@ macro_rules! declare_properties { ]; }; } + +// workaround until we can use --generate-cstr in bindgen. +pub const TYPE_DEVICE: &CStr = + unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_DEVICE) }; +pub const TYPE_SYS_BUS_DEVICE: &CStr = + unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_SYS_BUS_DEVICE) }; diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs index 7442f69564..43a4827de1 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -8,7 +8,7 @@ use qemu_api::{ bindings::*, c_str, declare_properties, define_property, definitions::{Class, ObjectImpl}, - device_class_init, + device_class, device_class_init, zeroable::Zeroable, }; @@ -57,7 +57,7 @@ fn test_device_decl_macros() { type Class = DummyClass; const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self }; const TYPE_NAME: &'static CStr = c_str!("dummy"); - const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_DEVICE); + const PARENT_TYPE_NAME: Option<&'static CStr> = Some(device_class::TYPE_DEVICE); const ABSTRACT: bool = false; const INSTANCE_INIT: Option = None; const INSTANCE_POST_INIT: Option = None; -- cgit 1.4.1