From d1368344bc9bb251080507940f2bad16048d2687 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Wed, 19 Mar 2025 19:31:10 +0000 Subject: rust: Kconfig: Factor out whether HPET is Rust or C Currently we require everywhere that wants to know if there is an HPET device to check for "CONFIG_HPET || CONFIG_X_HPET_RUST". Factor out whether the HPET device is Rust or C into a separate Kconfig stanza, so that CONFIG_HPET means "there is an HPET", and whether this has pulled in CONFIG_X_HPET_RUST or CONFIG_HPET_C is something the rest of QEMU can ignore. Signed-off-by: Peter Maydell Link: https://lore.kernel.org/r/20250319193110.1565578-3-peter.maydell@linaro.org Signed-off-by: Paolo Bonzini --- rust/hw/timer/Kconfig | 1 - 1 file changed, 1 deletion(-) (limited to 'rust/hw') diff --git a/rust/hw/timer/Kconfig b/rust/hw/timer/Kconfig index 42e421317a..afd9803350 100644 --- a/rust/hw/timer/Kconfig +++ b/rust/hw/timer/Kconfig @@ -1,3 +1,2 @@ config X_HPET_RUST bool - default y if PC && HAVE_RUST -- cgit 1.4.1 From 7bda68e8e2b0b836639557ddb13d761bdd15a104 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 28 Feb 2025 14:02:03 +0100 Subject: qdev, rust/hpet: fix type of HPET "timers" property Signed-off-by: Paolo Bonzini --- hw/core/qdev-properties.c | 37 +++++++++++++++++++++++++++++++++++++ include/hw/qdev-properties.h | 1 + rust/hw/timer/hpet/src/hpet.rs | 6 +++--- 3 files changed, 41 insertions(+), 3 deletions(-) (limited to 'rust/hw') diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index c04df3b337..147b3ffd16 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -442,6 +442,43 @@ const PropertyInfo qdev_prop_uint64_checkmask = { .set = set_uint64_checkmask, }; +/* --- pointer-size integer --- */ + +static void get_usize(Object *obj, Visitor *v, const char *name, void *opaque, + Error **errp) +{ + const Property *prop = opaque; + +#if HOST_LONG_BITS == 32 + uint32_t *ptr = object_field_prop_ptr(obj, prop); + visit_type_uint32(v, name, ptr, errp); +#else + uint64_t *ptr = object_field_prop_ptr(obj, prop); + visit_type_uint64(v, name, ptr, errp); +#endif +} + +static void set_usize(Object *obj, Visitor *v, const char *name, void *opaque, + Error **errp) +{ + const Property *prop = opaque; + +#if HOST_LONG_BITS == 32 + uint32_t *ptr = object_field_prop_ptr(obj, prop); + visit_type_uint32(v, name, ptr, errp); +#else + uint64_t *ptr = object_field_prop_ptr(obj, prop); + visit_type_uint64(v, name, ptr, errp); +#endif +} + +const PropertyInfo qdev_prop_usize = { + .type = "usize", + .get = get_usize, + .set = set_usize, + .set_default_value = qdev_propinfo_set_default_value_uint, +}; + /* --- string --- */ static void release_string(Object *obj, const char *name, void *opaque) diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index 15fcec5260..2c99856caa 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -52,6 +52,7 @@ extern const PropertyInfo qdev_prop_bool; extern const PropertyInfo qdev_prop_uint8; extern const PropertyInfo qdev_prop_uint16; extern const PropertyInfo qdev_prop_uint32; +extern const PropertyInfo qdev_prop_usize; extern const PropertyInfo qdev_prop_int32; extern const PropertyInfo qdev_prop_uint64; extern const PropertyInfo qdev_prop_uint64_checkmask; diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs index 20e0afdfca..63c1971f0b 100644 --- a/rust/hw/timer/hpet/src/hpet.rs +++ b/rust/hw/timer/hpet/src/hpet.rs @@ -12,7 +12,7 @@ use std::{ use qemu_api::{ bindings::{ address_space_memory, address_space_stl_le, qdev_prop_bit, qdev_prop_bool, - qdev_prop_uint32, qdev_prop_uint8, + qdev_prop_uint32, qdev_prop_usize, }, c_str, cell::{BqlCell, BqlRefCell}, @@ -859,8 +859,8 @@ qemu_api::declare_properties! { c_str!("timers"), HPETState, num_timers, - unsafe { &qdev_prop_uint8 }, - u8, + unsafe { &qdev_prop_usize }, + usize, default = HPET_MIN_TIMERS ), qemu_api::define_property!( -- cgit 1.4.1 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 ++++++++- rust/wrapper.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 'rust/hw') 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)] diff --git a/rust/wrapper.h b/rust/wrapper.h index d927ad6799..d4fec54657 100644 --- a/rust/wrapper.h +++ b/rust/wrapper.h @@ -65,3 +65,4 @@ typedef enum memory_order { #include "exec/memattrs.h" #include "qemu/timer.h" #include "exec/address-spaces.h" +#include "hw/char/pl011.h" -- 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') 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 From 64acc23c9793e86f2811345f3c122bf3ece8088b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 21 Mar 2025 14:17:52 +0100 Subject: rust: hpet: fix decoding of timer registers Due to a missing "& 0x18", timer registers are not decoded correctly. This breaks the tests/functional/test_x86_64_tuxrun.py functional test. Fixes: 519088b7cf6 ("rust: hpet: decode HPET registers into enums", 2025-03-06) Reported-by: Peter Maydell Tested-by: Peter Maydell Reviewed-by: Peter Maydell Signed-off-by: Paolo Bonzini --- rust/hw/timer/hpet/src/hpet.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/hw') diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs index 63c1971f0b..3ae3ec25f1 100644 --- a/rust/hw/timer/hpet/src/hpet.rs +++ b/rust/hw/timer/hpet/src/hpet.rs @@ -776,7 +776,7 @@ impl HPETState { let timer_id: usize = ((addr - 0x100) / 0x20) as usize; if timer_id <= self.num_timers.get() { // TODO: Add trace point - trace_hpet_ram_[read|write]_timer_id(timer_id) - TimerRegister::try_from(addr) + TimerRegister::try_from(addr & 0x18) .map(|reg| HPETRegister::Timer(&self.timers[timer_id], reg)) } else { // TODO: Add trace point - trace_hpet_timer_id_out_of_range(timer_id) -- cgit 1.4.1