From cc3d262aa93a42e19c38f6acb6d0f6012a71eb4b Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 21 Mar 2025 11:25:23 +0000 Subject: rust: pl011: Check size of state struct at compile time The PL011 device's C implementation exposes its PL011State struct to users of the device, and one common usage pattern is to embed that struct into the user's own state struct. (The internals of the struct are technically visible to the C user of the device, but in practice are treated as implementation details.) This means that the Rust version of the state struct must not be larger than the C version's struct; otherwise it will trip a runtime assertion in object_initialize_type() when the C user attempts to in-place initialize the type. Add a compile-time assertion on the Rust side, so that if we accidentally make the Rust device state larger we know immediately that we need to expand the padding in the C version of the struct. Reviewed-by: Zhao Liu Signed-off-by: Peter Maydell Link: https://lore.kernel.org/r/20250321112523.1774131-4-peter.maydell@linaro.org Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'rust/hw/char/pl011/src') diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index f137b49fea..bf88e0b00a 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -2,7 +2,7 @@ // Author(s): Manos Pitsidianakis // SPDX-License-Identifier: GPL-2.0-or-later -use std::{ffi::CStr, ptr::addr_of_mut}; +use std::{ffi::CStr, mem::size_of, ptr::addr_of_mut}; use qemu_api::{ chardev::{CharBackend, Chardev, Event}, @@ -12,6 +12,7 @@ use qemu_api::{ prelude::*, qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl}, qom::{ObjectImpl, Owned, ParentField}, + static_assert, sysbus::{SysBusDevice, SysBusDeviceImpl}, vmstate::VMStateDescription, }; @@ -124,6 +125,12 @@ pub struct PL011State { pub migrate_clock: bool, } +// Some C users of this device embed its state struct into their own +// structs, so the size of the Rust version must not be any larger +// than the size of the C one. If this assert triggers you need to +// expand the padding_for_rust[] array in the C PL011State struct. +static_assert!(size_of::() <= size_of::()); + qom_isa!(PL011State : SysBusDevice, DeviceState, Object); #[repr(C)] -- cgit 1.4.1 From f7b87e464c8e9d30661fb9f519ed14fb053cd415 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Tue, 18 Mar 2025 21:02:19 +0800 Subject: rust/vmstate: Include complete crate path of VMStateFlags in vmstate_clock The use of "bindings::*" masks incomplete path of VMStateFlags. Include complete crate path of VMStateFlags in vmstate_clock, and clean up "bindings::*" in device_class.rs of pl011. Signed-off-by: Zhao Liu Link: https://lore.kernel.org/r/20250318130219.1799170-16-zhao1.liu@intel.com Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device_class.rs | 8 ++++++-- rust/qemu-api/src/vmstate.rs | 5 ++++- 2 files changed, 10 insertions(+), 3 deletions(-) (limited to 'rust/hw/char/pl011/src') diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs index 0b2076ddaa..b4d4a7eb43 100644 --- a/rust/hw/char/pl011/src/device_class.rs +++ b/rust/hw/char/pl011/src/device_class.rs @@ -8,8 +8,12 @@ use std::{ }; use qemu_api::{ - bindings::*, c_str, prelude::*, vmstate_clock, vmstate_fields, vmstate_of, vmstate_struct, - vmstate_subsections, vmstate_unused, zeroable::Zeroable, + bindings::{qdev_prop_bool, qdev_prop_chr}, + c_str, + prelude::*, + vmstate::VMStateDescription, + vmstate_clock, vmstate_fields, vmstate_of, vmstate_struct, vmstate_subsections, vmstate_unused, + zeroable::Zeroable, }; use crate::device::{PL011Registers, PL011State}; diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs index 9740931fb1..1b2b12eadd 100644 --- a/rust/qemu-api/src/vmstate.rs +++ b/rust/qemu-api/src/vmstate.rs @@ -487,7 +487,10 @@ macro_rules! vmstate_clock { $crate::offset_of!($struct_name, $field_name) }, size: ::core::mem::size_of::<*const $crate::qdev::Clock>(), - flags: VMStateFlags(VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_POINTER.0), + flags: $crate::bindings::VMStateFlags( + $crate::bindings::VMStateFlags::VMS_STRUCT.0 + | $crate::bindings::VMStateFlags::VMS_POINTER.0, + ), vmsd: unsafe { ::core::ptr::addr_of!($crate::bindings::vmstate_clock) }, ..$crate::zeroable::Zeroable::ZERO } -- cgit 1.4.1