* [PULL 01/27] rust: docs: document naming convention
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 02/27] rust: qom: add reference counting functionality Paolo Bonzini
` (26 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel
As agreed in the "vtables and procedural macros" thread on
the mailing list.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index 390aae43866..8cccca7a734 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -194,6 +194,50 @@ module status
interface either. Also, ``unsafe`` interfaces may be replaced by safe interfaces
later.
+Naming convention
+'''''''''''''''''
+
+C function names usually are prefixed according to the data type that they
+apply to, for example ``timer_mod`` or ``sysbus_connect_irq``. Furthermore,
+both function and structs sometimes have a ``qemu_`` or ``QEMU`` prefix.
+Generally speaking, these are all removed in the corresponding Rust functions:
+``QEMUTimer`` becomes ``timer::Timer``, ``timer_mod`` becomes ``Timer::modify``,
+``sysbus_connect_irq`` becomes ``SysBusDeviceMethods::connect_irq``.
+
+Sometimes however a name appears multiple times in the QOM class hierarchy,
+and the only difference is in the prefix. An example is ``qdev_realize`` and
+``sysbus_realize``. In such cases, whenever a name is not unique in
+the hierarchy, always add the prefix to the classes that are lower in
+the hierarchy; for the top class, decide on a case by case basis.
+
+For example:
+
+========================== =========================================
+``device_cold_reset()`` ``DeviceMethods::cold_reset()``
+``pci_device_reset()`` ``PciDeviceMethods::pci_device_reset()``
+``pci_bridge_reset()`` ``PciBridgeMethods::pci_bridge_reset()``
+========================== =========================================
+
+Here, the name is not exactly the same, but nevertheless ``PciDeviceMethods``
+adds the prefix to avoid confusion, because the functionality of
+``device_cold_reset()`` and ``pci_device_reset()`` is subtly different.
+
+In this case, however, no prefix is needed:
+
+========================== =========================================
+``device_realize()`` ``DeviceMethods::realize()``
+``sysbus_realize()`` ``SysbusDeviceMethods::sysbus_realize()``
+``pci_realize()`` ``PciDeviceMethods::pci_realize()``
+========================== =========================================
+
+Here, the lower classes do not add any functionality, and mostly
+provide extra compile-time checking; the basic *realize* functionality
+is the same for all devices. Therefore, ``DeviceMethods`` does not
+add the prefix.
+
+Whenever a name is unique in the hierarchy, instead, you should
+always remove the class name prefix.
+
Common pitfalls
'''''''''''''''
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 02/27] rust: qom: add reference counting functionality
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
2025-02-13 16:00 ` [PULL 01/27] rust: docs: document naming convention Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 03/27] rust: qom: add object creation functionality Paolo Bonzini
` (25 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Add a smart pointer that allows to add and remove references from
QOM objects. It's important to note that while all QOM objects have a
reference count, in practice not all of them have their lifetime guarded
by it. Embedded objects, specifically, are confined to the lifetime of
the owner.
When writing Rust bindings this is important, because embedded objects are
*never* used through the "Owned<>" smart pointer that is introduced here.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/qom.rs | 166 +++++++++++++++++++++++++++++++++--
rust/qemu-api/src/vmstate.rs | 6 +-
rust/qemu-api/tests/tests.rs | 13 ++-
3 files changed, 178 insertions(+), 7 deletions(-)
diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
index f50ee371aac..404446d57fc 100644
--- a/rust/qemu-api/src/qom.rs
+++ b/rust/qemu-api/src/qom.rs
@@ -56,6 +56,7 @@
use std::{
ffi::CStr,
fmt,
+ mem::ManuallyDrop,
ops::{Deref, DerefMut},
os::raw::c_void,
ptr::NonNull,
@@ -63,7 +64,13 @@
pub use bindings::{Object, ObjectClass};
-use crate::bindings::{self, object_dynamic_cast, object_get_class, object_get_typename, TypeInfo};
+use crate::{
+ bindings::{
+ self, object_dynamic_cast, object_get_class, object_get_typename, object_ref, object_unref,
+ TypeInfo,
+ },
+ cell::bql_locked,
+};
/// Marker trait: `Self` can be statically upcasted to `P` (i.e. `P` is a direct
/// or indirect parent of `Self`).
@@ -280,10 +287,10 @@ fn as_ptr<U: ObjectType>(&self) -> *const U
///
/// # Safety
///
- /// This method is unsafe because it overrides const-ness of `&self`.
- /// Bindings to C APIs will use it a lot, but otherwise it should not
- /// be necessary.
- unsafe fn as_mut_ptr<U: ObjectType>(&self) -> *mut U
+ /// This method is safe because only the actual dereference of the pointer
+ /// has to be unsafe. Bindings to C APIs will use it a lot, but care has
+ /// to be taken because it overrides the const-ness of `&self`.
+ fn as_mut_ptr<U: ObjectType>(&self) -> *mut U
where
Self::Target: IsA<U>,
{
@@ -610,6 +617,148 @@ unsafe impl ObjectType for Object {
unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_OBJECT) };
}
+/// A reference-counted pointer to a QOM object.
+///
+/// `Owned<T>` wraps `T` with automatic reference counting. It increases the
+/// reference count when created via [`Owned::from`] or cloned, and decreases
+/// it when dropped. This ensures that the reference count remains elevated
+/// as long as any `Owned<T>` references to it exist.
+///
+/// `Owned<T>` can be used for two reasons:
+/// * because the lifetime of the QOM object is unknown and someone else could
+/// take a reference (similar to `Arc<T>`, for example): in this case, the
+/// object can escape and outlive the Rust struct that contains the `Owned<T>`
+/// field;
+///
+/// * to ensure that the object stays alive until after `Drop::drop` is called
+/// on the Rust struct: in this case, the object will always die together with
+/// the Rust struct that contains the `Owned<T>` field.
+///
+/// Child properties are an example of the second case: in C, an object that
+/// is created with `object_initialize_child` will die *before*
+/// `instance_finalize` is called, whereas Rust expects the struct to have valid
+/// contents when `Drop::drop` is called. Therefore Rust structs that have
+/// child properties need to keep a reference to the child object. Right now
+/// this can be done with `Owned<T>`; in the future one might have a separate
+/// `Child<'parent, T>` smart pointer that keeps a reference to a `T`, like
+/// `Owned`, but does not allow cloning.
+///
+/// Note that dropping an `Owned<T>` requires the big QEMU lock to be taken.
+#[repr(transparent)]
+#[derive(PartialEq, Eq, Hash, PartialOrd, Ord)]
+pub struct Owned<T: ObjectType>(NonNull<T>);
+
+// The following rationale for safety is taken from Linux's kernel::sync::Arc.
+
+// SAFETY: It is safe to send `Owned<T>` to another thread when the underlying
+// `T` is `Sync` because it effectively means sharing `&T` (which is safe
+// because `T` is `Sync`); additionally, it needs `T` to be `Send` because any
+// thread that has an `Owned<T>` may ultimately access `T` using a
+// mutable reference when the reference count reaches zero and `T` is dropped.
+unsafe impl<T: ObjectType + Send + Sync> Send for Owned<T> {}
+
+// SAFETY: It is safe to send `&Owned<T>` to another thread when the underlying
+// `T` is `Sync` because it effectively means sharing `&T` (which is safe
+// because `T` is `Sync`); additionally, it needs `T` to be `Send` because any
+// thread that has a `&Owned<T>` may clone it and get an `Owned<T>` on that
+// thread, so the thread may ultimately access `T` using a mutable reference
+// when the reference count reaches zero and `T` is dropped.
+unsafe impl<T: ObjectType + Sync + Send> Sync for Owned<T> {}
+
+impl<T: ObjectType> Owned<T> {
+ /// Convert a raw C pointer into an owned reference to the QOM
+ /// object it points to. The object's reference count will be
+ /// decreased when the `Owned` is dropped.
+ ///
+ /// # Panics
+ ///
+ /// Panics if `ptr` is NULL.
+ ///
+ /// # Safety
+ ///
+ /// The caller must indeed own a reference to the QOM object.
+ /// The object must not be embedded in another unless the outer
+ /// object is guaranteed to have a longer lifetime.
+ ///
+ /// A raw pointer obtained via [`Owned::into_raw()`] can always be passed
+ /// back to `from_raw()` (assuming the original `Owned` was valid!),
+ /// since the owned reference remains there between the calls to
+ /// `into_raw()` and `from_raw()`.
+ pub unsafe fn from_raw(ptr: *const T) -> Self {
+ // SAFETY NOTE: while NonNull requires a mutable pointer, only
+ // Deref is implemented so the pointer passed to from_raw
+ // remains const
+ Owned(NonNull::new(ptr as *mut T).unwrap())
+ }
+
+ /// Obtain a raw C pointer from a reference. `src` is consumed
+ /// and the reference is leaked.
+ #[allow(clippy::missing_const_for_fn)]
+ pub fn into_raw(src: Owned<T>) -> *mut T {
+ let src = ManuallyDrop::new(src);
+ src.0.as_ptr()
+ }
+
+ /// Increase the reference count of a QOM object and return
+ /// a new owned reference to it.
+ ///
+ /// # Safety
+ ///
+ /// The object must not be embedded in another, unless the outer
+ /// object is guaranteed to have a longer lifetime.
+ pub unsafe fn from(obj: &T) -> Self {
+ unsafe {
+ object_ref(obj.as_object_mut_ptr().cast::<c_void>());
+
+ // SAFETY NOTE: while NonNull requires a mutable pointer, only
+ // Deref is implemented so the reference passed to from_raw
+ // remains shared
+ Owned(NonNull::new_unchecked(obj.as_mut_ptr()))
+ }
+ }
+}
+
+impl<T: ObjectType> Clone for Owned<T> {
+ fn clone(&self) -> Self {
+ // SAFETY: creation method is unsafe; whoever calls it has
+ // responsibility that the pointer is valid, and remains valid
+ // throughout the lifetime of the `Owned<T>` and its clones.
+ unsafe { Owned::from(self.deref()) }
+ }
+}
+
+impl<T: ObjectType> Deref for Owned<T> {
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ // SAFETY: creation method is unsafe; whoever calls it has
+ // responsibility that the pointer is valid, and remains valid
+ // throughout the lifetime of the `Owned<T>` and its clones.
+ // With that guarantee, reference counting ensures that
+ // the object remains alive.
+ unsafe { &*self.0.as_ptr() }
+ }
+}
+impl<T: ObjectType> ObjectDeref for Owned<T> {}
+
+impl<T: ObjectType> Drop for Owned<T> {
+ fn drop(&mut self) {
+ assert!(bql_locked());
+ // SAFETY: creation method is unsafe, and whoever calls it has
+ // responsibility that the pointer is valid, and remains valid
+ // throughout the lifetime of the `Owned<T>` and its clones.
+ unsafe {
+ object_unref(self.as_object_mut_ptr().cast::<c_void>());
+ }
+ }
+}
+
+impl<T: IsA<Object>> fmt::Debug for Owned<T> {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ self.deref().debug_fmt(f)
+ }
+}
+
/// Trait for methods exposed by the Object class. The methods can be
/// called on all objects that have the trait `IsA<Object>`.
///
@@ -641,6 +790,13 @@ fn get_class(&self) -> &'static <Self::Target as ObjectType>::Class {
klass
}
+
+ /// Convenience function for implementing the Debug trait
+ fn debug_fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ f.debug_tuple(&self.typename())
+ .field(&(self as *const Self))
+ .finish()
+ }
}
impl<R: ObjectDeref> ObjectMethods for R where R::Target: IsA<Object> {}
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 6ac432cf52f..11d21b8791c 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -29,6 +29,8 @@
pub use crate::bindings::{VMStateDescription, VMStateField};
use crate::{
bindings::{self, VMStateFlags},
+ prelude::*,
+ qom::Owned,
zeroable::Zeroable,
};
@@ -191,7 +193,8 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateFlags
/// * a transparent wrapper for any of the above (`Cell`, `UnsafeCell`,
/// [`BqlCell`](crate::cell::BqlCell), [`BqlRefCell`](crate::cell::BqlRefCell)
/// * a raw pointer to any of the above
-/// * a `NonNull` pointer or a `Box` for any of the above
+/// * a `NonNull` pointer, a `Box` or an [`Owned`](crate::qom::Owned) for any of
+/// the above
/// * an array of any of the above
///
/// In order to support other types, the trait `VMState` must be implemented
@@ -398,6 +401,7 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
// Unlike C pointers, Box is always non-null therefore there is no need
// to specify VMS_ALLOC.
impl_vmstate_pointer!(Box<T> where T: VMState);
+impl_vmstate_pointer!(Owned<T> where T: VMState + ObjectType);
// Arrays using the underlying type's VMState plus
// VMS_ARRAY/VMS_ARRAY_OF_POINTER
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 5c3e75ed3d5..5f6096a572e 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -15,7 +15,7 @@
declare_properties, define_property,
prelude::*,
qdev::{DeviceClass, DeviceImpl, DeviceState, Property},
- qom::{ClassInitImpl, ObjectImpl, ParentField},
+ qom::{ClassInitImpl, ObjectImpl, Owned, ParentField},
vmstate::VMStateDescription,
zeroable::Zeroable,
};
@@ -138,6 +138,17 @@ fn test_object_new() {
}
}
+#[test]
+#[allow(clippy::redundant_clone)]
+/// Create, clone and then drop an instance.
+fn test_clone() {
+ init_qom();
+ let p: *mut DummyState = unsafe { object_new(DummyState::TYPE_NAME.as_ptr()).cast() };
+ let p = unsafe { Owned::from_raw(p) };
+ assert_eq!(p.clone().typename(), "dummy");
+ drop(p);
+}
+
#[test]
/// Try invoking a method on an object.
fn test_typename() {
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 03/27] rust: qom: add object creation functionality
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
2025-02-13 16:00 ` [PULL 01/27] rust: docs: document naming convention Paolo Bonzini
2025-02-13 16:00 ` [PULL 02/27] rust: qom: add reference counting functionality Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 04/27] rust: callbacks: allow passing optional callbacks as () Paolo Bonzini
` (24 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
The basic object lifecycle test can now be implemented using safe code!
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/char/pl011/src/device.rs | 23 ++++++++++++---------
rust/qemu-api/src/prelude.rs | 1 +
rust/qemu-api/src/qom.rs | 23 +++++++++++++++++++--
rust/qemu-api/tests/tests.rs | 35 ++++++++++++--------------------
4 files changed, 48 insertions(+), 34 deletions(-)
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 8050ede9c85..f5db114b0c7 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -10,11 +10,11 @@
use qemu_api::{
bindings::{
- error_fatal, hwaddr, memory_region_init_io, qdev_init_clock_in, qdev_new,
- qdev_prop_set_chr, qemu_chr_fe_accept_input, qemu_chr_fe_ioctl, qemu_chr_fe_set_handlers,
- qemu_chr_fe_write_all, qemu_irq, sysbus_connect_irq, sysbus_mmio_map,
- sysbus_realize_and_unref, CharBackend, Chardev, Clock, ClockEvent, MemoryRegion,
- QEMUChrEvent, CHR_IOCTL_SERIAL_SET_BREAK,
+ error_fatal, hwaddr, memory_region_init_io, qdev_init_clock_in, qdev_prop_set_chr,
+ qemu_chr_fe_accept_input, qemu_chr_fe_ioctl, qemu_chr_fe_set_handlers,
+ qemu_chr_fe_write_all, qemu_irq, sysbus_connect_irq, sysbus_mmio_map, sysbus_realize,
+ CharBackend, Chardev, Clock, ClockEvent, MemoryRegion, QEMUChrEvent,
+ CHR_IOCTL_SERIAL_SET_BREAK,
},
c_str, impl_vmstate_forward,
irq::InterruptSource,
@@ -705,15 +705,18 @@ pub fn post_load(&self, _version_id: u32) -> Result<(), ()> {
irq: qemu_irq,
chr: *mut Chardev,
) -> *mut DeviceState {
+ let pl011 = PL011State::new();
unsafe {
- let dev: *mut DeviceState = qdev_new(PL011State::TYPE_NAME.as_ptr());
- let sysbus: *mut SysBusDevice = dev.cast::<SysBusDevice>();
-
+ let dev = pl011.as_mut_ptr::<DeviceState>();
qdev_prop_set_chr(dev, c_str!("chardev").as_ptr(), chr);
- sysbus_realize_and_unref(sysbus, addr_of_mut!(error_fatal));
+
+ let sysbus = pl011.as_mut_ptr::<SysBusDevice>();
+ sysbus_realize(sysbus, addr_of_mut!(error_fatal));
sysbus_mmio_map(sysbus, 0, addr);
sysbus_connect_irq(sysbus, 0, irq);
- dev
+
+ // return the pointer, which is kept alive by the QOM tree; drop owned ref
+ pl011.as_mut_ptr()
}
}
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index 2dc86e19b29..3df6a5c21ec 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -12,6 +12,7 @@
pub use crate::qom::ObjectCast;
pub use crate::qom::ObjectCastMut;
pub use crate::qom::ObjectDeref;
+pub use crate::qom::ObjectClassMethods;
pub use crate::qom::ObjectMethods;
pub use crate::qom::ObjectType;
diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
index 404446d57fc..3e63cb30ca6 100644
--- a/rust/qemu-api/src/qom.rs
+++ b/rust/qemu-api/src/qom.rs
@@ -66,8 +66,8 @@
use crate::{
bindings::{
- self, object_dynamic_cast, object_get_class, object_get_typename, object_ref, object_unref,
- TypeInfo,
+ self, object_dynamic_cast, object_get_class, object_get_typename, object_new, object_ref,
+ object_unref, TypeInfo,
},
cell::bql_locked,
};
@@ -759,6 +759,24 @@ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
}
}
+/// Trait for class methods exposed by the Object class. The methods can be
+/// called on all objects that have the trait `IsA<Object>`.
+///
+/// The trait should only be used through the blanket implementation,
+/// which guarantees safety via `IsA`
+pub trait ObjectClassMethods: IsA<Object> {
+ /// Return a new reference counted instance of this class
+ fn new() -> Owned<Self> {
+ assert!(bql_locked());
+ // SAFETY: the object created by object_new is allocated on
+ // the heap and has a reference count of 1
+ unsafe {
+ let obj = &*object_new(Self::TYPE_NAME.as_ptr());
+ Owned::from_raw(obj.unsafe_cast::<Self>())
+ }
+ }
+}
+
/// Trait for methods exposed by the Object class. The methods can be
/// called on all objects that have the trait `IsA<Object>`.
///
@@ -799,4 +817,5 @@ fn debug_fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
}
}
+impl<T> ObjectClassMethods for T where T: IsA<Object> {}
impl<R: ObjectDeref> ObjectMethods for R where R::Target: IsA<Object> {}
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 5f6096a572e..10748fba197 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -3,8 +3,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
use std::{
- ffi::CStr,
- os::raw::c_void,
+ ffi::{c_void, CStr},
ptr::{addr_of, addr_of_mut},
};
@@ -15,7 +14,7 @@
declare_properties, define_property,
prelude::*,
qdev::{DeviceClass, DeviceImpl, DeviceState, Property},
- qom::{ClassInitImpl, ObjectImpl, Owned, ParentField},
+ qom::{ClassInitImpl, ObjectImpl, ParentField},
vmstate::VMStateDescription,
zeroable::Zeroable,
};
@@ -132,10 +131,8 @@ fn init_qom() {
/// Create and immediately drop an instance.
fn test_object_new() {
init_qom();
- unsafe {
- object_unref(object_new(DummyState::TYPE_NAME.as_ptr()).cast());
- object_unref(object_new(DummyChildState::TYPE_NAME.as_ptr()).cast());
- }
+ drop(DummyState::new());
+ drop(DummyChildState::new());
}
#[test]
@@ -143,8 +140,7 @@ fn test_object_new() {
/// Create, clone and then drop an instance.
fn test_clone() {
init_qom();
- let p: *mut DummyState = unsafe { object_new(DummyState::TYPE_NAME.as_ptr()).cast() };
- let p = unsafe { Owned::from_raw(p) };
+ let p = DummyState::new();
assert_eq!(p.clone().typename(), "dummy");
drop(p);
}
@@ -153,12 +149,8 @@ fn test_clone() {
/// Try invoking a method on an object.
fn test_typename() {
init_qom();
- let p: *mut DummyState = unsafe { object_new(DummyState::TYPE_NAME.as_ptr()).cast() };
- let p_ref: &DummyState = unsafe { &*p };
- assert_eq!(p_ref.typename(), "dummy");
- unsafe {
- object_unref(p_ref.as_object_mut_ptr().cast::<c_void>());
- }
+ let p = DummyState::new();
+ assert_eq!(p.typename(), "dummy");
}
// a note on all "cast" tests: usually, especially for downcasts the desired
@@ -173,24 +165,23 @@ fn test_typename() {
/// Test casts on shared references.
fn test_cast() {
init_qom();
- let p: *mut DummyState = unsafe { object_new(DummyState::TYPE_NAME.as_ptr()).cast() };
+ let p = DummyState::new();
+ let p_ptr: *mut DummyState = p.as_mut_ptr();
+ let p_ref: &mut DummyState = unsafe { &mut *p_ptr };
- let p_ref: &DummyState = unsafe { &*p };
let obj_ref: &Object = p_ref.upcast();
- assert_eq!(addr_of!(*obj_ref), p.cast());
+ assert_eq!(addr_of!(*obj_ref), p_ptr.cast());
let sbd_ref: Option<&SysBusDevice> = obj_ref.dynamic_cast();
assert!(sbd_ref.is_none());
let dev_ref: Option<&DeviceState> = obj_ref.downcast();
- assert_eq!(addr_of!(*dev_ref.unwrap()), p.cast());
+ assert_eq!(addr_of!(*dev_ref.unwrap()), p_ptr.cast());
// SAFETY: the cast is wrong, but the value is only used for comparison
unsafe {
let sbd_ref: &SysBusDevice = obj_ref.unsafe_cast();
- assert_eq!(addr_of!(*sbd_ref), p.cast());
-
- object_unref(p_ref.as_object_mut_ptr().cast::<c_void>());
+ assert_eq!(addr_of!(*sbd_ref), p_ptr.cast());
}
}
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 04/27] rust: callbacks: allow passing optional callbacks as ()
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (2 preceding siblings ...)
2025-02-13 16:00 ` [PULL 03/27] rust: qom: add object creation functionality Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 05/27] rust: qdev: add clock creation Paolo Bonzini
` (23 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
In some cases, callbacks are optional. Using "Some(function)" and "None"
does not work well, because when someone writes "None" the compiler does
not know what to use for "F" in "Option<F>".
Therefore, adopt () to mean a "null" callback. It is possible to enforce
that a callback is valid by adding a "let _: () = F::ASSERT_IS_SOME" before
the invocation of F::call.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/callbacks.rs | 97 ++++++++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)
diff --git a/rust/qemu-api/src/callbacks.rs b/rust/qemu-api/src/callbacks.rs
index 314f9dce962..9642a16eb89 100644
--- a/rust/qemu-api/src/callbacks.rs
+++ b/rust/qemu-api/src/callbacks.rs
@@ -79,6 +79,31 @@
/// call_it(&move |_| String::from(x), "hello workd");
/// ```
///
+/// `()` can be used to indicate "no function":
+///
+/// ```
+/// # use qemu_api::callbacks::FnCall;
+/// fn optional<F: for<'a> FnCall<(&'a str,), String>>(_f: &F, s: &str) -> Option<String> {
+/// if F::IS_SOME {
+/// Some(F::call((s,)))
+/// } else {
+/// None
+/// }
+/// }
+///
+/// assert!(optional(&(), "hello world").is_none());
+/// ```
+///
+/// Invoking `F::call` will then be a run-time error.
+///
+/// ```should_panic
+/// # use qemu_api::callbacks::FnCall;
+/// # fn call_it<F: for<'a> FnCall<(&'a str,), String>>(_f: &F, s: &str) -> String {
+/// # F::call((s,))
+/// # }
+/// let s: String = call_it(&(), "hello world"); // panics
+/// ```
+///
/// # Safety
///
/// Because `Self` is a zero-sized type, all instances of the type are
@@ -93,10 +118,70 @@ pub unsafe trait FnCall<Args, R = ()>: 'static + Sync + Sized {
/// Rust 1.79.0+.
const ASSERT_ZERO_SIZED: () = { assert!(mem::size_of::<Self>() == 0) };
+ /// Referring to this constant asserts that the `Self` type is an actual
+ /// function type, which can be used to catch incorrect use of `()`
+ /// at compile time.
+ ///
+ /// # Examples
+ ///
+ /// ```compile_fail
+ /// # use qemu_api::callbacks::FnCall;
+ /// fn call_it<F: for<'a> FnCall<(&'a str,), String>>(_f: &F, s: &str) -> String {
+ /// let _: () = F::ASSERT_IS_SOME;
+ /// F::call((s,))
+ /// }
+ ///
+ /// let s: String = call_it((), "hello world"); // does not compile
+ /// ```
+ ///
+ /// Note that this can be more simply `const { assert!(F::IS_SOME) }` in
+ /// Rust 1.79.0 or newer.
+ const ASSERT_IS_SOME: () = { assert!(Self::IS_SOME) };
+
+ /// `true` if `Self` is an actual function type and not `()`.
+ ///
+ /// # Examples
+ ///
+ /// You can use `IS_SOME` to catch this at compile time:
+ ///
+ /// ```compile_fail
+ /// # use qemu_api::callbacks::FnCall;
+ /// fn call_it<F: for<'a> FnCall<(&'a str,), String>>(_f: &F, s: &str) -> String {
+ /// const { assert!(F::IS_SOME) }
+ /// F::call((s,))
+ /// }
+ ///
+ /// let s: String = call_it((), "hello world"); // does not compile
+ /// ```
+ const IS_SOME: bool;
+
+ /// `false` if `Self` is an actual function type, `true` if it is `()`.
+ fn is_none() -> bool {
+ !Self::IS_SOME
+ }
+
+ /// `true` if `Self` is an actual function type, `false` if it is `()`.
+ fn is_some() -> bool {
+ Self::IS_SOME
+ }
+
/// Call the function with the arguments in args.
fn call(a: Args) -> R;
}
+/// `()` acts as a "null" callback. Using `()` and `function` is nicer
+/// than `None` and `Some(function)`, because the compiler is unable to
+/// infer the type of just `None`. Therefore, the trait itself acts as the
+/// option type, with functions [`FnCall::is_some`] and [`FnCall::is_none`].
+unsafe impl<Args, R> FnCall<Args, R> for () {
+ const IS_SOME: bool = false;
+
+ /// Call the function with the arguments in args.
+ fn call(_a: Args) -> R {
+ panic!("callback not specified")
+ }
+}
+
macro_rules! impl_call {
($($args:ident,)* ) => (
// SAFETY: because each function is treated as a separate type,
@@ -106,6 +191,8 @@ unsafe impl<F, $($args,)* R> FnCall<($($args,)*), R> for F
where
F: 'static + Sync + Sized + Fn($($args, )*) -> R,
{
+ const IS_SOME: bool = true;
+
#[inline(always)]
fn call(a: ($($args,)*)) -> R {
let _: () = Self::ASSERT_ZERO_SIZED;
@@ -141,4 +228,14 @@ fn do_test_call<'a, F: FnCall<(&'a str,), String>>(_f: &F) -> String {
fn test_call() {
assert_eq!(do_test_call(&str::to_owned), "hello world")
}
+
+ // The `_f` parameter is unused but it helps the compiler infer `F`.
+ fn do_test_is_some<'a, F: FnCall<(&'a str,), String>>(_f: &F) {
+ assert!(F::is_some());
+ }
+
+ #[test]
+ fn test_is_some() {
+ do_test_is_some(&str::to_owned);
+ }
}
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 05/27] rust: qdev: add clock creation
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (3 preceding siblings ...)
2025-02-13 16:00 ` [PULL 04/27] rust: callbacks: allow passing optional callbacks as () Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 06/27] rust: qom: allow initializing interface vtables Paolo Bonzini
` (22 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Add a Rust version of qdev_init_clock_in, which can be used in
instance_init. There are a couple differences with the C
version:
- in Rust the object keeps its own reference to the clock (in addition to
the one embedded in the NamedClockList), and the reference is dropped
automatically by instance_finalize(); this is encoded in the signature
of DeviceClassMethods::init_clock_in, which makes the lifetime of the
clock independent of that of the object it holds. This goes unnoticed
in the C version and is due to the existence of aliases.
- also, anything that happens during instance_init uses the pinned_init
framework to operate on a partially initialized object, and is done
through class methods (i.e. through DeviceClassMethods rather than
DeviceMethods) because the device does not exist yet. Therefore, Rust
code *must* create clocks from instance_init, which is stricter than C.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/char/pl011/src/device.rs | 43 +++++--------
rust/qemu-api/src/prelude.rs | 2 +
rust/qemu-api/src/qdev.rs | 107 ++++++++++++++++++++++++++++++-
rust/qemu-api/src/vmstate.rs | 4 +-
4 files changed, 125 insertions(+), 31 deletions(-)
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index f5db114b0c7..37936a328b8 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -10,17 +10,16 @@
use qemu_api::{
bindings::{
- error_fatal, hwaddr, memory_region_init_io, qdev_init_clock_in, qdev_prop_set_chr,
- qemu_chr_fe_accept_input, qemu_chr_fe_ioctl, qemu_chr_fe_set_handlers,
- qemu_chr_fe_write_all, qemu_irq, sysbus_connect_irq, sysbus_mmio_map, sysbus_realize,
- CharBackend, Chardev, Clock, ClockEvent, MemoryRegion, QEMUChrEvent,
- CHR_IOCTL_SERIAL_SET_BREAK,
+ error_fatal, hwaddr, memory_region_init_io, qdev_prop_set_chr, qemu_chr_fe_accept_input,
+ qemu_chr_fe_ioctl, qemu_chr_fe_set_handlers, qemu_chr_fe_write_all, qemu_irq,
+ sysbus_connect_irq, sysbus_mmio_map, sysbus_realize, CharBackend, Chardev, MemoryRegion,
+ QEMUChrEvent, CHR_IOCTL_SERIAL_SET_BREAK,
},
c_str, impl_vmstate_forward,
irq::InterruptSource,
prelude::*,
- qdev::{DeviceImpl, DeviceState, Property},
- qom::{ClassInitImpl, ObjectImpl, ParentField},
+ qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property},
+ qom::{ClassInitImpl, ObjectImpl, Owned, ParentField},
sysbus::{SysBusDevice, SysBusDeviceClass},
vmstate::VMStateDescription,
};
@@ -131,7 +130,7 @@ pub struct PL011State {
#[doc(alias = "irq")]
pub interrupts: [InterruptSource; IRQMASK.len()],
#[doc(alias = "clk")]
- pub clock: NonNull<Clock>,
+ pub clock: Owned<Clock>,
#[doc(alias = "migrate_clk")]
pub migrate_clock: bool,
}
@@ -485,8 +484,6 @@ 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_str!("clk");
-
// SAFETY:
//
// self and self.iomem are guaranteed to be valid at this point since callers
@@ -506,22 +503,16 @@ unsafe fn init(&mut self) {
// SAFETY:
//
- // self.clock is not initialized at this point; but since `NonNull<_>` is Copy,
- // we can overwrite the undefined value without side effects. This is
- // safe since all PL011State instances are created by QOM code which
- // calls this function to initialize the fields; therefore no code is
- // able to access an invalid self.clock value.
- unsafe {
- let dev: &mut DeviceState = self.upcast_mut();
- self.clock = NonNull::new(qdev_init_clock_in(
- dev,
- CLK_NAME.as_ptr(),
- None, /* pl011_clock_update */
- addr_of_mut!(*self).cast::<c_void>(),
- ClockEvent::ClockUpdate.0,
- ))
- .unwrap();
- }
+ // self.clock is not initialized at this point; but since `Owned<_>` is
+ // not Drop, we can overwrite the undefined value without side effects;
+ // it's not sound but, because for all PL011State instances are created
+ // by QOM code which calls this function to initialize the fields, at
+ // leastno code is able to access an invalid self.clock value.
+ self.clock = self.init_clock_in("clk", &Self::clock_update, ClockEvent::ClockUpdate);
+ }
+
+ const fn clock_update(&self, _event: ClockEvent) {
+ /* pl011_trace_baudrate_change(s); */
}
fn post_init(&self) {
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index 3df6a5c21ec..87e3ce90f26 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -7,6 +7,8 @@
pub use crate::cell::BqlCell;
pub use crate::cell::BqlRefCell;
+pub use crate::qdev::DeviceMethods;
+
pub use crate::qom::IsA;
pub use crate::qom::Object;
pub use crate::qom::ObjectCast;
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index f4c75c752f1..176c69a5600 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -4,14 +4,20 @@
//! Bindings to create devices and access device functionality from Rust.
-use std::{ffi::CStr, ptr::NonNull};
+use std::{
+ ffi::{CStr, CString},
+ os::raw::c_void,
+ ptr::NonNull,
+};
-pub use bindings::{DeviceClass, DeviceState, Property};
+pub use bindings::{Clock, ClockEvent, DeviceClass, DeviceState, Property};
use crate::{
bindings::{self, Error},
+ callbacks::FnCall,
+ cell::bql_locked,
prelude::*,
- qom::{ClassInitImpl, ObjectClass},
+ qom::{ClassInitImpl, ObjectClass, Owned},
vmstate::VMStateDescription,
};
@@ -143,3 +149,98 @@ unsafe impl ObjectType for DeviceState {
unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_DEVICE) };
}
qom_isa!(DeviceState: Object);
+
+/// Trait for methods exposed by the [`DeviceState`] class. The methods can be
+/// called on all objects that have the trait `IsA<DeviceState>`.
+///
+/// The trait should only be used through the blanket implementation,
+/// which guarantees safety via `IsA`.
+pub trait DeviceMethods: ObjectDeref
+where
+ Self::Target: IsA<DeviceState>,
+{
+ /// Add an input clock named `name`. Invoke the callback with
+ /// `self` as the first parameter for the events that are requested.
+ ///
+ /// The resulting clock is added as a child of `self`, but it also
+ /// stays alive until after `Drop::drop` is called because C code
+ /// keeps an extra reference to it until `device_finalize()` calls
+ /// `qdev_finalize_clocklist()`. Therefore (unlike most cases in
+ /// which Rust code has a reference to a child object) it would be
+ /// possible for this function to return a `&Clock` too.
+ #[inline]
+ fn init_clock_in<F: for<'a> FnCall<(&'a Self::Target, ClockEvent)>>(
+ &self,
+ name: &str,
+ _cb: &F,
+ events: ClockEvent,
+ ) -> Owned<Clock> {
+ fn do_init_clock_in(
+ dev: *mut DeviceState,
+ name: &str,
+ cb: Option<unsafe extern "C" fn(*mut c_void, ClockEvent)>,
+ events: ClockEvent,
+ ) -> Owned<Clock> {
+ assert!(bql_locked());
+
+ // SAFETY: the clock is heap allocated, but qdev_init_clock_in()
+ // does not gift the reference to its caller; so use Owned::from to
+ // add one. The callback is disabled automatically when the clock
+ // is unparented, which happens before the device is finalized.
+ unsafe {
+ let cstr = CString::new(name).unwrap();
+ let clk = bindings::qdev_init_clock_in(
+ dev,
+ cstr.as_ptr(),
+ cb,
+ dev.cast::<c_void>(),
+ events.0,
+ );
+
+ Owned::from(&*clk)
+ }
+ }
+
+ let cb: Option<unsafe extern "C" fn(*mut c_void, ClockEvent)> = if F::is_some() {
+ unsafe extern "C" fn rust_clock_cb<T, F: for<'a> FnCall<(&'a T, ClockEvent)>>(
+ opaque: *mut c_void,
+ event: ClockEvent,
+ ) {
+ // SAFETY: the opaque is "this", which is indeed a pointer to T
+ F::call((unsafe { &*(opaque.cast::<T>()) }, event))
+ }
+ Some(rust_clock_cb::<Self::Target, F>)
+ } else {
+ None
+ };
+
+ do_init_clock_in(self.as_mut_ptr(), name, cb, events)
+ }
+
+ /// Add an output clock named `name`.
+ ///
+ /// The resulting clock is added as a child of `self`, but it also
+ /// stays alive until after `Drop::drop` is called because C code
+ /// keeps an extra reference to it until `device_finalize()` calls
+ /// `qdev_finalize_clocklist()`. Therefore (unlike most cases in
+ /// which Rust code has a reference to a child object) it would be
+ /// possible for this function to return a `&Clock` too.
+ #[inline]
+ fn init_clock_out(&self, name: &str) -> Owned<Clock> {
+ unsafe {
+ let cstr = CString::new(name).unwrap();
+ let clk = bindings::qdev_init_clock_out(self.as_mut_ptr(), cstr.as_ptr());
+
+ Owned::from(&*clk)
+ }
+ }
+}
+
+impl<R: ObjectDeref> DeviceMethods for R where R::Target: IsA<DeviceState> {}
+
+unsafe impl ObjectType for Clock {
+ type Class = ObjectClass;
+ const TYPE_NAME: &'static CStr =
+ unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_CLOCK) };
+}
+qom_isa!(Clock: Object);
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 11d21b8791c..164effc6553 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -470,11 +470,11 @@ macro_rules! vmstate_clock {
$crate::assert_field_type!(
$struct_name,
$field_name,
- core::ptr::NonNull<$crate::bindings::Clock>
+ $crate::qom::Owned<$crate::bindings::Clock>
);
$crate::offset_of!($struct_name, $field_name)
},
- size: ::core::mem::size_of::<*const $crate::bindings::Clock>(),
+ size: ::core::mem::size_of::<*const $crate::qdev::Clock>(),
flags: VMStateFlags(VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_POINTER.0),
vmsd: unsafe { ::core::ptr::addr_of!($crate::bindings::vmstate_clock) },
..$crate::zeroable::Zeroable::ZERO
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 06/27] rust: qom: allow initializing interface vtables
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (4 preceding siblings ...)
2025-02-13 16:00 ` [PULL 05/27] rust: qdev: add clock creation Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 07/27] rust: qdev: make ObjectImpl a supertrait of DeviceImpl Paolo Bonzini
` (21 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Unlike regular classes, interface vtables can only be obtained via
object_class_dynamic_cast. Provide a wrapper that allows accessing
the vtable and pass it to a ClassInitImpl implementation, for example
ClassInitImpl<ResettableClass>.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/prelude.rs | 1 +
rust/qemu-api/src/qom.rs | 45 ++++++++++++++++++++++++++++++++++--
2 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index 87e3ce90f26..254edb476dd 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -9,6 +9,7 @@
pub use crate::qdev::DeviceMethods;
+pub use crate::qom::InterfaceType;
pub use crate::qom::IsA;
pub use crate::qom::Object;
pub use crate::qom::ObjectCast;
diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
index 3e63cb30ca6..3d5ab2d9018 100644
--- a/rust/qemu-api/src/qom.rs
+++ b/rust/qemu-api/src/qom.rs
@@ -66,8 +66,8 @@
use crate::{
bindings::{
- self, object_dynamic_cast, object_get_class, object_get_typename, object_new, object_ref,
- object_unref, TypeInfo,
+ self, object_class_dynamic_cast, object_dynamic_cast, object_get_class,
+ object_get_typename, object_new, object_ref, object_unref, TypeInfo,
},
cell::bql_locked,
};
@@ -263,6 +263,47 @@ unsafe fn as_object_mut_ptr(&self) -> *mut Object {
}
}
+/// Trait exposed by all structs corresponding to QOM interfaces.
+/// Unlike `ObjectType`, it is implemented on the class type (which provides
+/// the vtable for the interfaces).
+///
+/// # Safety
+///
+/// `TYPE` must match the contents of the `TypeInfo` as found in the C code;
+/// right now, interfaces can only be declared in C.
+pub unsafe trait InterfaceType: Sized {
+ /// The name of the type, which can be passed to
+ /// `object_class_dynamic_cast()` to obtain the pointer to the vtable
+ /// for this interface.
+ const TYPE_NAME: &'static CStr;
+
+ /// Initialize the vtable for the interface; the generic argument `T` is the
+ /// type being initialized, while the generic argument `U` is the type that
+ /// lists the interface in its `TypeInfo`.
+ ///
+ /// # Panics
+ ///
+ /// Panic if the incoming argument if `T` does not implement the interface.
+ fn interface_init<
+ T: ObjectType + ClassInitImpl<Self> + ClassInitImpl<U::Class>,
+ U: ObjectType,
+ >(
+ klass: &mut U::Class,
+ ) {
+ unsafe {
+ // SAFETY: upcasting to ObjectClass is always valid, and the
+ // return type is either NULL or the argument itself
+ let result: *mut Self = object_class_dynamic_cast(
+ (klass as *mut U::Class).cast(),
+ Self::TYPE_NAME.as_ptr(),
+ )
+ .cast();
+
+ <T as ClassInitImpl<Self>>::class_init(result.as_mut().unwrap())
+ }
+ }
+}
+
/// This trait provides safe casting operations for QOM objects to raw pointers,
/// to be used for example for FFI. The trait can be applied to any kind of
/// reference or smart pointers, and enforces correctness through the [`IsA`]
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 07/27] rust: qdev: make ObjectImpl a supertrait of DeviceImpl
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (5 preceding siblings ...)
2025-02-13 16:00 ` [PULL 06/27] rust: qom: allow initializing interface vtables Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 08/27] rust: qdev: switch from legacy reset to Resettable Paolo Bonzini
` (20 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
In practice it has to be implemented always in order to access an
implementation of ClassInitImpl<ObjectClass>. Make the relationship
explicit in the code.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/qdev.rs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 176c69a5600..34d24da4b63 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -17,12 +17,12 @@
callbacks::FnCall,
cell::bql_locked,
prelude::*,
- qom::{ClassInitImpl, ObjectClass, Owned},
+ qom::{ClassInitImpl, ObjectClass, ObjectImpl, Owned},
vmstate::VMStateDescription,
};
/// Trait providing the contents of [`DeviceClass`].
-pub trait DeviceImpl {
+pub trait DeviceImpl: ObjectImpl {
/// _Realization_ is the second stage of device creation. It contains
/// all operations that depend on device properties and can fail (note:
/// this is not yet supported for Rust devices).
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 08/27] rust: qdev: switch from legacy reset to Resettable
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (6 preceding siblings ...)
2025-02-13 16:00 ` [PULL 07/27] rust: qdev: make ObjectImpl a supertrait of DeviceImpl Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 09/27] rust: bindings: add Send and Sync markers for types that have bindings Paolo Bonzini
` (19 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
meson.build | 1 +
rust/hw/char/pl011/src/device.rs | 10 ++-
rust/qemu-api/src/qdev.rs | 111 ++++++++++++++++++++++++-------
rust/qemu-api/tests/tests.rs | 5 +-
4 files changed, 99 insertions(+), 28 deletions(-)
diff --git a/meson.build b/meson.build
index 18cf9e2913b..16c76c493f3 100644
--- a/meson.build
+++ b/meson.build
@@ -4073,6 +4073,7 @@ if have_rust
'MigrationPriority',
'QEMUChrEvent',
'QEMUClockType',
+ 'ResetType',
'device_endian',
'module_init_type',
]
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 37936a328b8..1d0390b4fbe 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -18,7 +18,7 @@
c_str, impl_vmstate_forward,
irq::InterruptSource,
prelude::*,
- qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property},
+ qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl},
qom::{ClassInitImpl, ObjectImpl, Owned, ParentField},
sysbus::{SysBusDevice, SysBusDeviceClass},
vmstate::VMStateDescription,
@@ -171,7 +171,10 @@ fn vmsd() -> Option<&'static VMStateDescription> {
Some(&device_class::VMSTATE_PL011)
}
const REALIZE: Option<fn(&Self)> = Some(Self::realize);
- const RESET: Option<fn(&Self)> = Some(Self::reset);
+}
+
+impl ResettablePhasesImpl for PL011State {
+ const HOLD: Option<fn(&Self, ResetType)> = Some(Self::reset_hold);
}
impl PL011Registers {
@@ -622,7 +625,7 @@ pub fn realize(&self) {
}
}
- pub fn reset(&self) {
+ pub fn reset_hold(&self, _type: ResetType) {
self.regs.borrow_mut().reset();
}
@@ -737,3 +740,4 @@ impl ObjectImpl for PL011Luminary {
}
impl DeviceImpl for PL011Luminary {}
+impl ResettablePhasesImpl for PL011Luminary {}
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 34d24da4b63..64ba3d90981 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -10,10 +10,10 @@
ptr::NonNull,
};
-pub use bindings::{Clock, ClockEvent, DeviceClass, DeviceState, Property};
+pub use bindings::{Clock, ClockEvent, DeviceClass, DeviceState, Property, ResetType};
use crate::{
- bindings::{self, Error},
+ bindings::{self, Error, ResettableClass},
callbacks::FnCall,
cell::bql_locked,
prelude::*,
@@ -21,8 +21,70 @@
vmstate::VMStateDescription,
};
+/// Trait providing the contents of the `ResettablePhases` struct,
+/// which is part of the QOM `Resettable` interface.
+pub trait ResettablePhasesImpl {
+ /// If not None, this is called when the object enters reset. It
+ /// can reset local state of the object, but it must not do anything that
+ /// has a side-effect on other objects, such as raising or lowering an
+ /// [`InterruptSource`](crate::irq::InterruptSource), or reading or
+ /// writing guest memory. It takes the reset's type as argument.
+ const ENTER: Option<fn(&Self, ResetType)> = None;
+
+ /// If not None, this is called when the object for entry into reset, once
+ /// every object in the system which is being reset has had its
+ /// `ResettablePhasesImpl::ENTER` method called. At this point devices
+ /// can do actions that affect other objects.
+ ///
+ /// If in doubt, implement this method.
+ const HOLD: Option<fn(&Self, ResetType)> = None;
+
+ /// If not None, this phase is called when the object leaves the reset
+ /// state. Actions affecting other objects are permitted.
+ const EXIT: Option<fn(&Self, ResetType)> = None;
+}
+
+/// # Safety
+///
+/// We expect the FFI user of this function to pass a valid pointer that
+/// can be downcasted to type `T`. We also expect the device is
+/// readable/writeable from one thread at any time.
+unsafe extern "C" fn rust_resettable_enter_fn<T: ResettablePhasesImpl>(
+ obj: *mut Object,
+ typ: ResetType,
+) {
+ let state = NonNull::new(obj).unwrap().cast::<T>();
+ T::ENTER.unwrap()(unsafe { state.as_ref() }, typ);
+}
+
+/// # Safety
+///
+/// We expect the FFI user of this function to pass a valid pointer that
+/// can be downcasted to type `T`. We also expect the device is
+/// readable/writeable from one thread at any time.
+unsafe extern "C" fn rust_resettable_hold_fn<T: ResettablePhasesImpl>(
+ obj: *mut Object,
+ typ: ResetType,
+) {
+ let state = NonNull::new(obj).unwrap().cast::<T>();
+ T::HOLD.unwrap()(unsafe { state.as_ref() }, typ);
+}
+
+/// # Safety
+///
+/// We expect the FFI user of this function to pass a valid pointer that
+/// can be downcasted to type `T`. We also expect the device is
+/// readable/writeable from one thread at any time.
+unsafe extern "C" fn rust_resettable_exit_fn<T: ResettablePhasesImpl>(
+ obj: *mut Object,
+ typ: ResetType,
+) {
+ let state = NonNull::new(obj).unwrap().cast::<T>();
+ T::EXIT.unwrap()(unsafe { state.as_ref() }, typ);
+}
+
/// Trait providing the contents of [`DeviceClass`].
-pub trait DeviceImpl: ObjectImpl {
+pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl {
/// _Realization_ is the second stage of device creation. It contains
/// all operations that depend on device properties and can fail (note:
/// this is not yet supported for Rust devices).
@@ -31,13 +93,6 @@ pub trait DeviceImpl: ObjectImpl {
/// with the function pointed to by `REALIZE`.
const REALIZE: Option<fn(&Self)> = None;
- /// If not `None`, the parent class's `reset` method is overridden
- /// with the function pointed to by `RESET`.
- ///
- /// Rust does not yet support the three-phase reset protocol; this is
- /// usually okay for leaf classes.
- const RESET: Option<fn(&Self)> = None;
-
/// An array providing the properties that the user can set on the
/// device. Not a `const` because referencing statics in constants
/// is unstable until Rust 1.83.0.
@@ -65,29 +120,36 @@ fn vmsd() -> Option<&'static VMStateDescription> {
T::REALIZE.unwrap()(unsafe { state.as_ref() });
}
-/// # Safety
-///
-/// We expect the FFI user of this function to pass a valid pointer that
-/// can be downcasted to type `T`. We also expect the device is
-/// readable/writeable from one thread at any time.
-unsafe extern "C" fn rust_reset_fn<T: DeviceImpl>(dev: *mut DeviceState) {
- let mut state = NonNull::new(dev).unwrap().cast::<T>();
- T::RESET.unwrap()(unsafe { state.as_mut() });
+unsafe impl InterfaceType for ResettableClass {
+ const TYPE_NAME: &'static CStr =
+ unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_RESETTABLE_INTERFACE) };
+}
+
+impl<T> ClassInitImpl<ResettableClass> for T
+where
+ T: ResettablePhasesImpl,
+{
+ fn class_init(rc: &mut ResettableClass) {
+ if <T as ResettablePhasesImpl>::ENTER.is_some() {
+ rc.phases.enter = Some(rust_resettable_enter_fn::<T>);
+ }
+ if <T as ResettablePhasesImpl>::HOLD.is_some() {
+ rc.phases.hold = Some(rust_resettable_hold_fn::<T>);
+ }
+ if <T as ResettablePhasesImpl>::EXIT.is_some() {
+ rc.phases.exit = Some(rust_resettable_exit_fn::<T>);
+ }
+ }
}
impl<T> ClassInitImpl<DeviceClass> for T
where
- T: ClassInitImpl<ObjectClass> + DeviceImpl,
+ T: ClassInitImpl<ObjectClass> + ClassInitImpl<ResettableClass> + DeviceImpl,
{
fn class_init(dc: &mut DeviceClass) {
if <T as DeviceImpl>::REALIZE.is_some() {
dc.realize = Some(rust_realize_fn::<T>);
}
- if <T as DeviceImpl>::RESET.is_some() {
- unsafe {
- bindings::device_class_set_legacy_reset(dc, Some(rust_reset_fn::<T>));
- }
- }
if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
dc.vmsd = vmsd;
}
@@ -98,6 +160,7 @@ fn class_init(dc: &mut DeviceClass) {
}
}
+ ResettableClass::interface_init::<T, DeviceState>(dc);
<T as ClassInitImpl<ObjectClass>>::class_init(&mut dc.parent_class);
}
}
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 10748fba197..92dbfb8a0c8 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -13,7 +13,7 @@
cell::{self, BqlCell},
declare_properties, define_property,
prelude::*,
- qdev::{DeviceClass, DeviceImpl, DeviceState, Property},
+ qdev::{DeviceClass, DeviceImpl, DeviceState, Property, ResettablePhasesImpl},
qom::{ClassInitImpl, ObjectImpl, ParentField},
vmstate::VMStateDescription,
zeroable::Zeroable,
@@ -61,6 +61,8 @@ impl ObjectImpl for DummyState {
const ABSTRACT: bool = false;
}
+impl ResettablePhasesImpl for DummyState {}
+
impl DeviceImpl for DummyState {
fn properties() -> &'static [Property] {
&DUMMY_PROPERTIES
@@ -101,6 +103,7 @@ impl ObjectImpl for DummyChildState {
const ABSTRACT: bool = false;
}
+impl ResettablePhasesImpl for DummyChildState {}
impl DeviceImpl for DummyChildState {}
impl ClassInitImpl<DummyClass> for DummyChildState {
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 09/27] rust: bindings: add Send and Sync markers for types that have bindings
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (7 preceding siblings ...)
2025-02-13 16:00 ` [PULL 08/27] rust: qdev: switch from legacy reset to Resettable Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 10/27] rust: bindings for MemoryRegionOps Paolo Bonzini
` (18 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
This is needed for the MemoryRegionOps<T> to be declared as static;
Rust requires static elements to be Sync.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/bindings.rs | 46 +++++++++++++++++++++++++++++++++++
rust/qemu-api/src/irq.rs | 3 +++
2 files changed, 49 insertions(+)
diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
index 8a9b821bb91..b71220113ef 100644
--- a/rust/qemu-api/src/bindings.rs
+++ b/rust/qemu-api/src/bindings.rs
@@ -21,9 +21,55 @@
#[cfg(not(MESON))]
include!(concat!(env!("OUT_DIR"), "/bindings.inc.rs"));
+// SAFETY: these are implemented in C; the bindings need to assert that the
+// BQL is taken, either directly or via `BqlCell` and `BqlRefCell`.
+unsafe impl Send for BusState {}
+unsafe impl Sync for BusState {}
+
+unsafe impl Send for CharBackend {}
+unsafe impl Sync for CharBackend {}
+
+unsafe impl Send for Chardev {}
+unsafe impl Sync for Chardev {}
+
+unsafe impl Send for Clock {}
+unsafe impl Sync for Clock {}
+
+unsafe impl Send for DeviceState {}
+unsafe impl Sync for DeviceState {}
+
+unsafe impl Send for MemoryRegion {}
+unsafe impl Sync for MemoryRegion {}
+
+unsafe impl Send for ObjectClass {}
+unsafe impl Sync for ObjectClass {}
+
+unsafe impl Send for Object {}
+unsafe impl Sync for Object {}
+
+unsafe impl Send for SysBusDevice {}
+unsafe impl Sync for SysBusDevice {}
+
+// SAFETY: this is a pure data struct
+unsafe impl Send for CoalescedMemoryRange {}
+unsafe impl Sync for CoalescedMemoryRange {}
+
+// SAFETY: these are constants and vtables; the Send and Sync requirements
+// are deferred to the unsafe callbacks that they contain
+unsafe impl Send for MemoryRegionOps {}
+unsafe impl Sync for MemoryRegionOps {}
+
unsafe impl Send for Property {}
unsafe impl Sync for Property {}
+
+unsafe impl Send for TypeInfo {}
unsafe impl Sync for TypeInfo {}
+
+unsafe impl Send for VMStateDescription {}
unsafe impl Sync for VMStateDescription {}
+
+unsafe impl Send for VMStateField {}
unsafe impl Sync for VMStateField {}
+
+unsafe impl Send for VMStateInfo {}
unsafe impl Sync for VMStateInfo {}
diff --git a/rust/qemu-api/src/irq.rs b/rust/qemu-api/src/irq.rs
index 378e5202951..638545c3a64 100644
--- a/rust/qemu-api/src/irq.rs
+++ b/rust/qemu-api/src/irq.rs
@@ -43,6 +43,9 @@ pub struct InterruptSource<T = bool>
_marker: PhantomData<T>,
}
+// SAFETY: the implementation asserts via `BqlCell` that the BQL is taken
+unsafe impl<T> Sync for InterruptSource<T> where c_int: From<T> {}
+
impl InterruptSource<bool> {
/// Send a low (`false`) value to the interrupt sink.
pub fn lower(&self) {
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 10/27] rust: bindings for MemoryRegionOps
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (8 preceding siblings ...)
2025-02-13 16:00 ` [PULL 09/27] rust: bindings: add Send and Sync markers for types that have bindings Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 11/27] rust: irq: define ObjectType for IRQState Paolo Bonzini
` (17 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 1 +
rust/hw/char/pl011/src/device.rs | 51 +++----
rust/hw/char/pl011/src/lib.rs | 1 -
rust/hw/char/pl011/src/memory_ops.rs | 34 -----
rust/qemu-api/meson.build | 1 +
rust/qemu-api/src/lib.rs | 1 +
rust/qemu-api/src/memory.rs | 191 +++++++++++++++++++++++++++
rust/qemu-api/src/sysbus.rs | 7 +-
rust/qemu-api/src/zeroable.rs | 1 +
9 files changed, 227 insertions(+), 61 deletions(-)
delete mode 100644 rust/hw/char/pl011/src/memory_ops.rs
create mode 100644 rust/qemu-api/src/memory.rs
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index 8cccca7a734..a5399db50b5 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -180,6 +180,7 @@ module status
``cell`` stable
``c_str`` complete
``irq`` complete
+``memory`` stable
``module`` complete
``offset_of`` stable
``qdev`` stable
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 1d0390b4fbe..5e4e75133c8 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -10,13 +10,14 @@
use qemu_api::{
bindings::{
- error_fatal, hwaddr, memory_region_init_io, qdev_prop_set_chr, qemu_chr_fe_accept_input,
- qemu_chr_fe_ioctl, qemu_chr_fe_set_handlers, qemu_chr_fe_write_all, qemu_irq,
- sysbus_connect_irq, sysbus_mmio_map, sysbus_realize, CharBackend, Chardev, MemoryRegion,
- QEMUChrEvent, CHR_IOCTL_SERIAL_SET_BREAK,
+ error_fatal, qdev_prop_set_chr, qemu_chr_fe_accept_input, qemu_chr_fe_ioctl,
+ qemu_chr_fe_set_handlers, qemu_chr_fe_write_all, qemu_irq, sysbus_connect_irq,
+ sysbus_mmio_map, sysbus_realize, CharBackend, Chardev, QEMUChrEvent,
+ CHR_IOCTL_SERIAL_SET_BREAK,
},
c_str, impl_vmstate_forward,
irq::InterruptSource,
+ memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
prelude::*,
qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl},
qom::{ClassInitImpl, ObjectImpl, Owned, ParentField},
@@ -26,7 +27,6 @@
use crate::{
device_class,
- memory_ops::PL011_OPS,
registers::{self, Interrupt},
RegisterOffset,
};
@@ -487,20 +487,24 @@ 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) {
+ static PL011_OPS: MemoryRegionOps<PL011State> = MemoryRegionOpsBuilder::<PL011State>::new()
+ .read(&PL011State::read)
+ .write(&PL011State::write)
+ .native_endian()
+ .impl_sizes(4, 4)
+ .build();
+
// SAFETY:
//
// self and self.iomem are guaranteed to be valid at this point since callers
// must make sure the `self` reference is valid.
- unsafe {
- memory_region_init_io(
- addr_of_mut!(self.iomem),
- addr_of_mut!(*self).cast::<Object>(),
- &PL011_OPS,
- addr_of_mut!(*self).cast::<c_void>(),
- Self::TYPE_NAME.as_ptr(),
- 0x1000,
- );
- }
+ MemoryRegion::init_io(
+ unsafe { &mut *addr_of_mut!(self.iomem) },
+ addr_of_mut!(*self),
+ &PL011_OPS,
+ "pl011",
+ 0x1000,
+ );
self.regs = Default::default();
@@ -525,7 +529,7 @@ fn post_init(&self) {
}
}
- pub fn read(&mut self, offset: hwaddr, _size: u32) -> u64 {
+ pub fn read(&self, offset: hwaddr, _size: u32) -> u64 {
match RegisterOffset::try_from(offset) {
Err(v) if (0x3f8..0x400).contains(&(v >> 2)) => {
let device_id = self.get_class().device_id;
@@ -540,7 +544,7 @@ pub fn read(&mut self, offset: hwaddr, _size: u32) -> u64 {
if update_irq {
self.update();
unsafe {
- qemu_chr_fe_accept_input(&mut self.char_backend);
+ qemu_chr_fe_accept_input(addr_of!(self.char_backend) as *mut _);
}
}
result.into()
@@ -548,7 +552,7 @@ pub fn read(&mut self, offset: hwaddr, _size: u32) -> u64 {
}
}
- pub fn write(&mut self, offset: hwaddr, value: u64) {
+ pub fn write(&self, offset: hwaddr, value: u64, _size: u32) {
let mut update_irq = false;
if let Ok(field) = RegisterOffset::try_from(offset) {
// qemu_chr_fe_write_all() calls into the can_receive
@@ -561,14 +565,15 @@ pub fn write(&mut self, offset: hwaddr, value: u64) {
// XXX this blocks entire thread. Rewrite to use
// qemu_chr_fe_write and background I/O callbacks
unsafe {
- qemu_chr_fe_write_all(&mut self.char_backend, &ch, 1);
+ qemu_chr_fe_write_all(addr_of!(self.char_backend) as *mut _, &ch, 1);
}
}
- update_irq = self
- .regs
- .borrow_mut()
- .write(field, value as u32, &mut self.char_backend);
+ update_irq = self.regs.borrow_mut().write(
+ field,
+ value as u32,
+ addr_of!(self.char_backend) as *mut _,
+ );
} else {
eprintln!("write bad offset {offset} value {value}");
}
diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
index 3c72f1221ff..1bf46c65af2 100644
--- a/rust/hw/char/pl011/src/lib.rs
+++ b/rust/hw/char/pl011/src/lib.rs
@@ -18,7 +18,6 @@
mod device;
mod device_class;
-mod memory_ops;
pub use device::pl011_create;
diff --git a/rust/hw/char/pl011/src/memory_ops.rs b/rust/hw/char/pl011/src/memory_ops.rs
deleted file mode 100644
index 432d3263898..00000000000
--- a/rust/hw/char/pl011/src/memory_ops.rs
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright 2024, Linaro Limited
-// Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
-// 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};
-
-use crate::device::PL011State;
-
-pub static PL011_OPS: MemoryRegionOps = MemoryRegionOps {
- read: Some(pl011_read),
- write: Some(pl011_write),
- read_with_attrs: None,
- write_with_attrs: None,
- endianness: device_endian::DEVICE_NATIVE_ENDIAN,
- valid: Zeroable::ZERO,
- impl_: MemoryRegionOps__bindgen_ty_2 {
- min_access_size: 4,
- max_access_size: 4,
- ..Zeroable::ZERO
- },
-};
-
-unsafe extern "C" fn pl011_read(opaque: *mut c_void, addr: hwaddr, size: c_uint) -> u64 {
- let mut state = NonNull::new(opaque).unwrap().cast::<PL011State>();
- unsafe { state.as_mut() }.read(addr, size)
-}
-
-unsafe extern "C" fn pl011_write(opaque: *mut c_void, addr: hwaddr, data: u64, _size: c_uint) {
- let mut state = NonNull::new(opaque).unwrap().cast::<PL011State>();
- unsafe { state.as_mut() }.write(addr, data);
-}
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 60944a657de..80eafc7f6bd 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -22,6 +22,7 @@ _qemu_api_rs = static_library(
'src/cell.rs',
'src/c_str.rs',
'src/irq.rs',
+ 'src/memory.rs',
'src/module.rs',
'src/offset_of.rs',
'src/prelude.rs',
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 3cf9371cff0..e4316b21cf2 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -19,6 +19,7 @@
pub mod callbacks;
pub mod cell;
pub mod irq;
+pub mod memory;
pub mod module;
pub mod offset_of;
pub mod qdev;
diff --git a/rust/qemu-api/src/memory.rs b/rust/qemu-api/src/memory.rs
new file mode 100644
index 00000000000..963d689c27d
--- /dev/null
+++ b/rust/qemu-api/src/memory.rs
@@ -0,0 +1,191 @@
+// Copyright 2024 Red Hat, Inc.
+// Author(s): Paolo Bonzini <pbonzini@redhat.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! Bindings for `MemoryRegion` and `MemoryRegionOps`
+
+use std::{
+ ffi::{CStr, CString},
+ marker::{PhantomData, PhantomPinned},
+ os::raw::{c_uint, c_void},
+ ptr::addr_of,
+};
+
+pub use bindings::hwaddr;
+
+use crate::{
+ bindings::{self, device_endian, memory_region_init_io},
+ callbacks::FnCall,
+ prelude::*,
+ zeroable::Zeroable,
+};
+
+pub struct MemoryRegionOps<T>(
+ bindings::MemoryRegionOps,
+ // Note: quite often you'll see PhantomData<fn(&T)> mentioned when discussing
+ // covariance and contravariance; you don't need any of those to understand
+ // this usage of PhantomData. Quite simply, MemoryRegionOps<T> *logically*
+ // holds callbacks that take an argument of type &T, except the type is erased
+ // before the callback is stored in the bindings::MemoryRegionOps field.
+ // The argument of PhantomData is a function pointer in order to represent
+ // that relationship; while that will also provide desirable and safe variance
+ // for T, variance is not the point but just a consequence.
+ PhantomData<fn(&T)>,
+);
+
+// SAFETY: When a *const T is passed to the callbacks, the call itself
+// is done in a thread-safe manner. The invocation is okay as long as
+// T itself is `Sync`.
+unsafe impl<T: Sync> Sync for MemoryRegionOps<T> {}
+
+#[derive(Clone)]
+pub struct MemoryRegionOpsBuilder<T>(bindings::MemoryRegionOps, PhantomData<fn(&T)>);
+
+unsafe extern "C" fn memory_region_ops_read_cb<T, F: for<'a> FnCall<(&'a T, hwaddr, u32), u64>>(
+ opaque: *mut c_void,
+ addr: hwaddr,
+ size: c_uint,
+) -> u64 {
+ F::call((unsafe { &*(opaque.cast::<T>()) }, addr, size))
+}
+
+unsafe extern "C" fn memory_region_ops_write_cb<T, F: for<'a> FnCall<(&'a T, hwaddr, u64, u32)>>(
+ opaque: *mut c_void,
+ addr: hwaddr,
+ data: u64,
+ size: c_uint,
+) {
+ F::call((unsafe { &*(opaque.cast::<T>()) }, addr, data, size))
+}
+
+impl<T> MemoryRegionOpsBuilder<T> {
+ #[must_use]
+ pub const fn read<F: for<'a> FnCall<(&'a T, hwaddr, u32), u64>>(mut self, _f: &F) -> Self {
+ self.0.read = Some(memory_region_ops_read_cb::<T, F>);
+ self
+ }
+
+ #[must_use]
+ pub const fn write<F: for<'a> FnCall<(&'a T, hwaddr, u64, u32)>>(mut self, _f: &F) -> Self {
+ self.0.write = Some(memory_region_ops_write_cb::<T, F>);
+ self
+ }
+
+ #[must_use]
+ pub const fn big_endian(mut self) -> Self {
+ self.0.endianness = device_endian::DEVICE_BIG_ENDIAN;
+ self
+ }
+
+ #[must_use]
+ pub const fn little_endian(mut self) -> Self {
+ self.0.endianness = device_endian::DEVICE_LITTLE_ENDIAN;
+ self
+ }
+
+ #[must_use]
+ pub const fn native_endian(mut self) -> Self {
+ self.0.endianness = device_endian::DEVICE_NATIVE_ENDIAN;
+ self
+ }
+
+ #[must_use]
+ pub const fn valid_sizes(mut self, min: u32, max: u32) -> Self {
+ self.0.valid.min_access_size = min;
+ self.0.valid.max_access_size = max;
+ self
+ }
+
+ #[must_use]
+ pub const fn valid_unaligned(mut self) -> Self {
+ self.0.valid.unaligned = true;
+ self
+ }
+
+ #[must_use]
+ pub const fn impl_sizes(mut self, min: u32, max: u32) -> Self {
+ self.0.impl_.min_access_size = min;
+ self.0.impl_.max_access_size = max;
+ self
+ }
+
+ #[must_use]
+ pub const fn impl_unaligned(mut self) -> Self {
+ self.0.impl_.unaligned = true;
+ self
+ }
+
+ #[must_use]
+ pub const fn build(self) -> MemoryRegionOps<T> {
+ MemoryRegionOps::<T>(self.0, PhantomData)
+ }
+
+ #[must_use]
+ pub const fn new() -> Self {
+ Self(bindings::MemoryRegionOps::ZERO, PhantomData)
+ }
+}
+
+impl<T> Default for MemoryRegionOpsBuilder<T> {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
+/// A safe wrapper around [`bindings::MemoryRegion`]. Compared to the
+/// underlying C struct it is marked as pinned because the QOM tree
+/// contains a pointer to it.
+pub struct MemoryRegion {
+ inner: bindings::MemoryRegion,
+ _pin: PhantomPinned,
+}
+
+impl MemoryRegion {
+ // inline to ensure that it is not included in tests, which only
+ // link to hwcore and qom. FIXME: inlining is actually the opposite
+ // of what we want, since this is the type-erased version of the
+ // init_io function below. Look into splitting the qemu_api crate.
+ #[inline(always)]
+ unsafe fn do_init_io(
+ slot: *mut bindings::MemoryRegion,
+ owner: *mut Object,
+ ops: &'static bindings::MemoryRegionOps,
+ name: &'static str,
+ size: u64,
+ ) {
+ unsafe {
+ let cstr = CString::new(name).unwrap();
+ memory_region_init_io(
+ slot,
+ owner.cast::<Object>(),
+ ops,
+ owner.cast::<c_void>(),
+ cstr.as_ptr(),
+ size,
+ );
+ }
+ }
+
+ pub fn init_io<T: IsA<Object>>(
+ &mut self,
+ owner: *mut T,
+ ops: &'static MemoryRegionOps<T>,
+ name: &'static str,
+ size: u64,
+ ) {
+ unsafe {
+ Self::do_init_io(&mut self.inner, owner.cast::<Object>(), &ops.0, name, size);
+ }
+ }
+
+ pub(crate) const fn as_mut_ptr(&self) -> *mut bindings::MemoryRegion {
+ addr_of!(self.inner) as *mut _
+ }
+}
+
+unsafe impl ObjectType for MemoryRegion {
+ type Class = bindings::MemoryRegionClass;
+ const TYPE_NAME: &'static CStr =
+ unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_MEMORY_REGION) };
+}
+qom_isa!(MemoryRegion: Object);
diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs
index e6762b5c145..c27dbf79e43 100644
--- a/rust/qemu-api/src/sysbus.rs
+++ b/rust/qemu-api/src/sysbus.rs
@@ -2,7 +2,7 @@
// Author(s): Paolo Bonzini <pbonzini@redhat.com>
// SPDX-License-Identifier: GPL-2.0-or-later
-use std::{ffi::CStr, ptr::addr_of};
+use std::ffi::CStr;
pub use bindings::{SysBusDevice, SysBusDeviceClass};
@@ -10,6 +10,7 @@
bindings,
cell::bql_locked,
irq::InterruptSource,
+ memory::MemoryRegion,
prelude::*,
qdev::{DeviceClass, DeviceState},
qom::ClassInitImpl,
@@ -42,10 +43,10 @@ pub trait SysBusDeviceMethods: ObjectDeref
/// important, since whoever creates the sysbus device will refer to the
/// region with a number that corresponds to the order of calls to
/// `init_mmio`.
- fn init_mmio(&self, iomem: &bindings::MemoryRegion) {
+ fn init_mmio(&self, iomem: &MemoryRegion) {
assert!(bql_locked());
unsafe {
- bindings::sysbus_init_mmio(self.as_mut_ptr(), addr_of!(*iomem) as *mut _);
+ bindings::sysbus_init_mmio(self.as_mut_ptr(), iomem.as_mut_ptr());
}
}
diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
index 7b04947cb6c..75742b50d4e 100644
--- a/rust/qemu-api/src/zeroable.rs
+++ b/rust/qemu-api/src/zeroable.rs
@@ -100,3 +100,4 @@ fn default() -> Self {
impl_zeroable!(crate::bindings::VMStateDescription);
impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_1);
impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_2);
+impl_zeroable!(crate::bindings::MemoryRegionOps);
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 11/27] rust: irq: define ObjectType for IRQState
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (9 preceding siblings ...)
2025-02-13 16:00 ` [PULL 10/27] rust: bindings for MemoryRegionOps Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 12/27] rust: chardev, qdev: add bindings to qdev_prop_set_chr Paolo Bonzini
` (16 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel
This is a small preparation in order to use an Owned<IRQState> for the argument
to sysbus_connect_irq.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/irq.rs | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/rust/qemu-api/src/irq.rs b/rust/qemu-api/src/irq.rs
index 638545c3a64..835b027d5e5 100644
--- a/rust/qemu-api/src/irq.rs
+++ b/rust/qemu-api/src/irq.rs
@@ -5,11 +5,12 @@
//! Bindings for interrupt sources
use core::ptr;
-use std::{marker::PhantomData, os::raw::c_int};
+use std::{ffi::CStr, marker::PhantomData, os::raw::c_int};
use crate::{
- bindings::{qemu_set_irq, IRQState},
+ bindings::{self, qemu_set_irq},
prelude::*,
+ qom::ObjectClass,
};
/// Interrupt sources are used by devices to pass changes to a value (typically
@@ -21,7 +22,8 @@
/// method sends a `true` value to the sink. If the guest has to see a
/// different polarity, that change is performed by the board between the
/// device and the interrupt controller.
-///
+pub type IRQState = bindings::IRQState;
+
/// Interrupts are implemented as a pointer to the interrupt "sink", which has
/// type [`IRQState`]. A device exposes its source as a QOM link property using
/// a function such as [`SysBusDeviceMethods::init_irq`], and
@@ -91,3 +93,10 @@ fn default() -> Self {
}
}
}
+
+unsafe impl ObjectType for IRQState {
+ type Class = ObjectClass;
+ const TYPE_NAME: &'static CStr =
+ unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_IRQ) };
+}
+qom_isa!(IRQState: Object);
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 12/27] rust: chardev, qdev: add bindings to qdev_prop_set_chr
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (10 preceding siblings ...)
2025-02-13 16:00 ` [PULL 11/27] rust: irq: define ObjectType for IRQState Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 13/27] rust: pl011: convert pl011_create to safe Rust Paolo Bonzini
` (15 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Because the argument to the function is an Owned<Chardev>, this also
adds an ObjectType implementation to Chardev.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/char/pl011/src/device.rs | 3 ++-
rust/qemu-api/meson.build | 1 +
rust/qemu-api/src/chardev.rs | 19 +++++++++++++++++++
rust/qemu-api/src/lib.rs | 1 +
rust/qemu-api/src/qdev.rs | 9 +++++++++
5 files changed, 32 insertions(+), 1 deletion(-)
create mode 100644 rust/qemu-api/src/chardev.rs
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 5e4e75133c8..4e959073711 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -12,9 +12,10 @@
bindings::{
error_fatal, qdev_prop_set_chr, qemu_chr_fe_accept_input, qemu_chr_fe_ioctl,
qemu_chr_fe_set_handlers, qemu_chr_fe_write_all, qemu_irq, sysbus_connect_irq,
- sysbus_mmio_map, sysbus_realize, CharBackend, Chardev, QEMUChrEvent,
+ sysbus_mmio_map, sysbus_realize, CharBackend, QEMUChrEvent,
CHR_IOCTL_SERIAL_SET_BREAK,
},
+ chardev::Chardev,
c_str, impl_vmstate_forward,
irq::InterruptSource,
memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 80eafc7f6bd..45e30324b29 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -20,6 +20,7 @@ _qemu_api_rs = static_library(
'src/bitops.rs',
'src/callbacks.rs',
'src/cell.rs',
+ 'src/chardev.rs',
'src/c_str.rs',
'src/irq.rs',
'src/memory.rs',
diff --git a/rust/qemu-api/src/chardev.rs b/rust/qemu-api/src/chardev.rs
new file mode 100644
index 00000000000..74cfb634e5f
--- /dev/null
+++ b/rust/qemu-api/src/chardev.rs
@@ -0,0 +1,19 @@
+// Copyright 2024 Red Hat, Inc.
+// Author(s): Paolo Bonzini <pbonzini@redhat.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! Bindings for character devices
+
+use std::ffi::CStr;
+
+use crate::{bindings, prelude::*};
+
+pub type Chardev = bindings::Chardev;
+pub type ChardevClass = bindings::ChardevClass;
+
+unsafe impl ObjectType for Chardev {
+ type Class = ChardevClass;
+ const TYPE_NAME: &'static CStr =
+ unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_CHARDEV) };
+}
+qom_isa!(Chardev: Object);
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index e4316b21cf2..2a338a888a0 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -18,6 +18,7 @@
pub mod c_str;
pub mod callbacks;
pub mod cell;
+pub mod chardev;
pub mod irq;
pub mod memory;
pub mod module;
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 64ba3d90981..73343e10b96 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -16,6 +16,7 @@
bindings::{self, Error, ResettableClass},
callbacks::FnCall,
cell::bql_locked,
+ chardev::Chardev,
prelude::*,
qom::{ClassInitImpl, ObjectClass, ObjectImpl, Owned},
vmstate::VMStateDescription,
@@ -297,6 +298,14 @@ fn init_clock_out(&self, name: &str) -> Owned<Clock> {
Owned::from(&*clk)
}
}
+
+ fn prop_set_chr(&self, propname: &str, chr: &Owned<Chardev>) {
+ assert!(bql_locked());
+ let c_propname = CString::new(propname).unwrap();
+ unsafe {
+ bindings::qdev_prop_set_chr(self.as_mut_ptr(), c_propname.as_ptr(), chr.as_mut_ptr());
+ }
+ }
}
impl<R: ObjectDeref> DeviceMethods for R where R::Target: IsA<DeviceState> {}
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 13/27] rust: pl011: convert pl011_create to safe Rust
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (11 preceding siblings ...)
2025-02-13 16:00 ` [PULL 12/27] rust: chardev, qdev: add bindings to qdev_prop_set_chr Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 14/27] i386/fw_cfg: move hpet_cfg definition to hpet.c Paolo Bonzini
` (14 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Not a major change but, as a small but significant step in creating
qdev bindings, show how pl011_create can be written without "unsafe"
calls (apart from converting pointers to references).
This also provides a starting point for creating Error** bindings.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/char/pl011/src/device.rs | 37 ++++++++++++++++----------------
rust/qemu-api/src/sysbus.rs | 34 ++++++++++++++++++++++++++---
2 files changed, 49 insertions(+), 22 deletions(-)
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 4e959073711..fe73771021e 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -10,14 +10,12 @@
use qemu_api::{
bindings::{
- error_fatal, qdev_prop_set_chr, qemu_chr_fe_accept_input, qemu_chr_fe_ioctl,
- qemu_chr_fe_set_handlers, qemu_chr_fe_write_all, qemu_irq, sysbus_connect_irq,
- sysbus_mmio_map, sysbus_realize, CharBackend, QEMUChrEvent,
- CHR_IOCTL_SERIAL_SET_BREAK,
+ qemu_chr_fe_accept_input, qemu_chr_fe_ioctl, qemu_chr_fe_set_handlers,
+ qemu_chr_fe_write_all, CharBackend, QEMUChrEvent, CHR_IOCTL_SERIAL_SET_BREAK,
},
chardev::Chardev,
- c_str, impl_vmstate_forward,
- irq::InterruptSource,
+ impl_vmstate_forward,
+ irq::{IRQState, InterruptSource},
memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
prelude::*,
qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl},
@@ -698,26 +696,27 @@ pub fn post_load(&self, _version_id: u32) -> Result<(), ()> {
/// # Safety
///
-/// We expect the FFI user of this function to pass a valid pointer for `chr`.
+/// We expect the FFI user of this function to pass a valid pointer for `chr`
+/// and `irq`.
#[no_mangle]
pub unsafe extern "C" fn pl011_create(
addr: u64,
- irq: qemu_irq,
+ irq: *mut IRQState,
chr: *mut Chardev,
) -> *mut DeviceState {
- let pl011 = PL011State::new();
- unsafe {
- let dev = pl011.as_mut_ptr::<DeviceState>();
- qdev_prop_set_chr(dev, c_str!("chardev").as_ptr(), chr);
+ // SAFETY: The callers promise that they have owned references.
+ // They do not gift them to pl011_create, so use `Owned::from`.
+ let irq = unsafe { Owned::<IRQState>::from(&*irq) };
+ let chr = unsafe { Owned::<Chardev>::from(&*chr) };
- let sysbus = pl011.as_mut_ptr::<SysBusDevice>();
- sysbus_realize(sysbus, addr_of_mut!(error_fatal));
- sysbus_mmio_map(sysbus, 0, addr);
- sysbus_connect_irq(sysbus, 0, irq);
+ let dev = PL011State::new();
+ dev.prop_set_chr("chardev", &chr);
+ dev.sysbus_realize();
+ dev.mmio_map(0, addr);
+ dev.connect_irq(0, &irq);
- // return the pointer, which is kept alive by the QOM tree; drop owned ref
- pl011.as_mut_ptr()
- }
+ // The pointer is kept alive by the QOM tree; drop the owned ref
+ dev.as_mut_ptr()
}
#[repr(C)]
diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs
index c27dbf79e43..1f66a5f1e09 100644
--- a/rust/qemu-api/src/sysbus.rs
+++ b/rust/qemu-api/src/sysbus.rs
@@ -2,18 +2,18 @@
// Author(s): Paolo Bonzini <pbonzini@redhat.com>
// SPDX-License-Identifier: GPL-2.0-or-later
-use std::ffi::CStr;
+use std::{ffi::CStr, ptr::addr_of_mut};
pub use bindings::{SysBusDevice, SysBusDeviceClass};
use crate::{
bindings,
cell::bql_locked,
- irq::InterruptSource,
+ irq::{IRQState, InterruptSource},
memory::MemoryRegion,
prelude::*,
qdev::{DeviceClass, DeviceState},
- qom::ClassInitImpl,
+ qom::{ClassInitImpl, Owned},
};
unsafe impl ObjectType for SysBusDevice {
@@ -60,6 +60,34 @@ fn init_irq(&self, irq: &InterruptSource) {
bindings::sysbus_init_irq(self.as_mut_ptr(), irq.as_ptr());
}
}
+
+ // TODO: do we want a type like GuestAddress here?
+ fn mmio_map(&self, id: u32, addr: u64) {
+ assert!(bql_locked());
+ let id: i32 = id.try_into().unwrap();
+ unsafe {
+ bindings::sysbus_mmio_map(self.as_mut_ptr(), id, addr);
+ }
+ }
+
+ // Owned<> is used here because sysbus_connect_irq (via
+ // object_property_set_link) adds a reference to the IRQState,
+ // which can prolong its life
+ fn connect_irq(&self, id: u32, irq: &Owned<IRQState>) {
+ assert!(bql_locked());
+ let id: i32 = id.try_into().unwrap();
+ unsafe {
+ bindings::sysbus_connect_irq(self.as_mut_ptr(), id, irq.as_mut_ptr());
+ }
+ }
+
+ fn sysbus_realize(&self) {
+ // TODO: return an Error
+ assert!(bql_locked());
+ unsafe {
+ bindings::sysbus_realize(self.as_mut_ptr(), addr_of_mut!(bindings::error_fatal));
+ }
+ }
}
impl<R: ObjectDeref> SysBusDeviceMethods for R where R::Target: IsA<SysBusDevice> {}
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 14/27] i386/fw_cfg: move hpet_cfg definition to hpet.c
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (12 preceding siblings ...)
2025-02-13 16:00 ` [PULL 13/27] rust: pl011: convert pl011_create to safe Rust Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 15/27] rust/qdev: add the macro to define bit property Paolo Bonzini
` (13 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
HPET device needs to access and update hpet_cfg variable, but now it is
defined in hw/i386/fw_cfg.c and Rust code can't access it.
Move hpet_cfg definition to hpet.c (and rename it to hpet_fw_cfg). This
allows Rust HPET device implements its own global hpet_fw_cfg variable,
and will further reduce the use of unsafe C code access and calls in the
Rust HPET implementation.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250210030051.2562726-2-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/hw/timer/hpet.h | 2 +-
hw/i386/fw_cfg.c | 6 ++++--
hw/timer/hpet.c | 16 +++++++++-------
3 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/include/hw/timer/hpet.h b/include/hw/timer/hpet.h
index 71e8c62453d..c2656f7f0be 100644
--- a/include/hw/timer/hpet.h
+++ b/include/hw/timer/hpet.h
@@ -73,7 +73,7 @@ struct hpet_fw_config
struct hpet_fw_entry hpet[8];
} QEMU_PACKED;
-extern struct hpet_fw_config hpet_cfg;
+extern struct hpet_fw_config hpet_fw_cfg;
#define TYPE_HPET "hpet"
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index 91bf1df0f2e..d08aefa0291 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -26,7 +26,9 @@
#include CONFIG_DEVICES
#include "target/i386/cpu.h"
-struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
+#if !defined(CONFIG_HPET) && !defined(CONFIG_X_HPET_RUST)
+struct hpet_fw_config hpet_fw_cfg = {.count = UINT8_MAX};
+#endif
const char *fw_cfg_arch_key_name(uint16_t key)
{
@@ -149,7 +151,7 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
#endif
fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, 1);
- fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
+ fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_fw_cfg, sizeof(hpet_fw_cfg));
/* allocate memory for the NUMA channel: one (64bit) word for the number
* of nodes, one word for each VCPU->node and one word for each node to
* hold the amount of memory.
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 1c8c6c69ef5..dcff18a9871 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -40,6 +40,8 @@
#include "qom/object.h"
#include "trace.h"
+struct hpet_fw_config hpet_fw_cfg = {.count = UINT8_MAX};
+
#define HPET_MSI_SUPPORT 0
OBJECT_DECLARE_SIMPLE_TYPE(HPETState, HPET)
@@ -278,7 +280,7 @@ static int hpet_post_load(void *opaque, int version_id)
/* Push number of timers into capability returned via HPET_ID */
s->capability &= ~HPET_ID_NUM_TIM_MASK;
s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT;
- hpet_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
+ hpet_fw_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
/* Derive HPET_MSI_SUPPORT from the capability of the first timer. */
s->flags &= ~(1 << HPET_MSI_SUPPORT);
@@ -665,8 +667,8 @@ static void hpet_reset(DeviceState *d)
s->hpet_counter = 0ULL;
s->hpet_offset = 0ULL;
s->config = 0ULL;
- hpet_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
- hpet_cfg.hpet[s->hpet_id].address = sbd->mmio[0].addr;
+ hpet_fw_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
+ hpet_fw_cfg.hpet[s->hpet_id].address = sbd->mmio[0].addr;
/* to document that the RTC lowers its output on reset as well */
s->rtc_irq_level = 0;
@@ -708,17 +710,17 @@ static void hpet_realize(DeviceState *dev, Error **errp)
if (!s->intcap) {
warn_report("Hpet's intcap not initialized");
}
- if (hpet_cfg.count == UINT8_MAX) {
+ if (hpet_fw_cfg.count == UINT8_MAX) {
/* first instance */
- hpet_cfg.count = 0;
+ hpet_fw_cfg.count = 0;
}
- if (hpet_cfg.count == 8) {
+ if (hpet_fw_cfg.count == 8) {
error_setg(errp, "Only 8 instances of HPET is allowed");
return;
}
- s->hpet_id = hpet_cfg.count++;
+ s->hpet_id = hpet_fw_cfg.count++;
for (i = 0; i < HPET_NUM_IRQ_ROUTES; i++) {
sysbus_init_irq(sbd, &s->irqs[i]);
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 15/27] rust/qdev: add the macro to define bit property
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (13 preceding siblings ...)
2025-02-13 16:00 ` [PULL 14/27] i386/fw_cfg: move hpet_cfg definition to hpet.c Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 16/27] rust/irq: Add a helper to convert [InterruptSource] to pointer Paolo Bonzini
` (12 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
HPET device (Rust device) needs to define the bit type property.
Add a variant of define_property macro to define bit type property.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250210030051.2562726-3-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/qdev.rs | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 73343e10b96..c44a22876b9 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -168,6 +168,18 @@ fn class_init(dc: &mut DeviceClass) {
#[macro_export]
macro_rules! define_property {
+ ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, bit = $bitnr:expr, default = $defval:expr$(,)*) => {
+ $crate::bindings::Property {
+ // use associated function syntax for type checking
+ name: ::std::ffi::CStr::as_ptr($name),
+ info: $prop,
+ offset: $crate::offset_of!($state, $field) as isize,
+ bitnr: $bitnr,
+ set_default: true,
+ defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 },
+ ..$crate::zeroable::Zeroable::ZERO
+ }
+ };
($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, default = $defval:expr$(,)*) => {
$crate::bindings::Property {
// use associated function syntax for type checking
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 16/27] rust/irq: Add a helper to convert [InterruptSource] to pointer
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (14 preceding siblings ...)
2025-02-13 16:00 ` [PULL 15/27] rust/qdev: add the macro to define bit property Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 17/27] rust: add bindings for gpio_{in|out} initialization Paolo Bonzini
` (11 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
This is useful when taking an InterruptSource slice and passing it to C
function.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250210030051.2562726-4-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/irq.rs | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/rust/qemu-api/src/irq.rs b/rust/qemu-api/src/irq.rs
index 835b027d5e5..672eec1430f 100644
--- a/rust/qemu-api/src/irq.rs
+++ b/rust/qemu-api/src/irq.rs
@@ -83,6 +83,12 @@ pub fn set(&self, level: T) {
pub(crate) const fn as_ptr(&self) -> *mut *mut IRQState {
self.cell.as_ptr()
}
+
+ #[allow(dead_code)]
+ pub(crate) const fn slice_as_ptr(slice: &[Self]) -> *mut *mut IRQState {
+ assert!(!slice.is_empty());
+ slice[0].as_ptr()
+ }
}
impl Default for InterruptSource {
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 17/27] rust: add bindings for gpio_{in|out} initialization
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (15 preceding siblings ...)
2025-02-13 16:00 ` [PULL 16/27] rust/irq: Add a helper to convert [InterruptSource] to pointer Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 18/27] rust: add bindings for memattrs Paolo Bonzini
` (10 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Wrap qdev_init_gpio_{in|out} as methods in DeviceMethods. And for
qdev_init_gpio_in, based on FnCall, it can support idiomatic Rust
callback without the need for C style wrapper.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250210030051.2562726-5-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/irq.rs | 1 -
rust/qemu-api/src/qdev.rs | 47 +++++++++++++++++++++++++++++++++++----
2 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/rust/qemu-api/src/irq.rs b/rust/qemu-api/src/irq.rs
index 672eec1430f..d1c9dc96eff 100644
--- a/rust/qemu-api/src/irq.rs
+++ b/rust/qemu-api/src/irq.rs
@@ -84,7 +84,6 @@ pub(crate) const fn as_ptr(&self) -> *mut *mut IRQState {
self.cell.as_ptr()
}
- #[allow(dead_code)]
pub(crate) const fn slice_as_ptr(slice: &[Self]) -> *mut *mut IRQState {
assert!(!slice.is_empty());
slice[0].as_ptr()
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index c44a22876b9..3a7aa4def62 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -6,17 +6,18 @@
use std::{
ffi::{CStr, CString},
- os::raw::c_void,
+ os::raw::{c_int, c_void},
ptr::NonNull,
};
pub use bindings::{Clock, ClockEvent, DeviceClass, DeviceState, Property, ResetType};
use crate::{
- bindings::{self, Error, ResettableClass},
+ bindings::{self, qdev_init_gpio_in, qdev_init_gpio_out, Error, ResettableClass},
callbacks::FnCall,
cell::bql_locked,
chardev::Chardev,
+ irq::InterruptSource,
prelude::*,
qom::{ClassInitImpl, ObjectClass, ObjectImpl, Owned},
vmstate::VMStateDescription,
@@ -28,8 +29,8 @@ pub trait ResettablePhasesImpl {
/// If not None, this is called when the object enters reset. It
/// can reset local state of the object, but it must not do anything that
/// has a side-effect on other objects, such as raising or lowering an
- /// [`InterruptSource`](crate::irq::InterruptSource), or reading or
- /// writing guest memory. It takes the reset's type as argument.
+ /// [`InterruptSource`], or reading or writing guest memory. It takes the
+ /// reset's type as argument.
const ENTER: Option<fn(&Self, ResetType)> = None;
/// If not None, this is called when the object for entry into reset, once
@@ -318,6 +319,44 @@ fn prop_set_chr(&self, propname: &str, chr: &Owned<Chardev>) {
bindings::qdev_prop_set_chr(self.as_mut_ptr(), c_propname.as_ptr(), chr.as_mut_ptr());
}
}
+
+ fn init_gpio_in<F: for<'a> FnCall<(&'a Self::Target, u32, u32)>>(
+ &self,
+ num_lines: u32,
+ _cb: F,
+ ) {
+ let _: () = F::ASSERT_IS_SOME;
+
+ unsafe extern "C" fn rust_irq_handler<T, F: for<'a> FnCall<(&'a T, u32, u32)>>(
+ opaque: *mut c_void,
+ line: c_int,
+ level: c_int,
+ ) {
+ // SAFETY: the opaque was passed as a reference to `T`
+ F::call((unsafe { &*(opaque.cast::<T>()) }, line as u32, level as u32))
+ }
+
+ let gpio_in_cb: unsafe extern "C" fn(*mut c_void, c_int, c_int) =
+ rust_irq_handler::<Self::Target, F>;
+
+ unsafe {
+ qdev_init_gpio_in(
+ self.as_mut_ptr::<DeviceState>(),
+ Some(gpio_in_cb),
+ num_lines as c_int,
+ );
+ }
+ }
+
+ fn init_gpio_out(&self, pins: &[InterruptSource]) {
+ unsafe {
+ qdev_init_gpio_out(
+ self.as_mut_ptr::<DeviceState>(),
+ InterruptSource::slice_as_ptr(pins),
+ pins.len() as c_int,
+ );
+ }
+ }
}
impl<R: ObjectDeref> DeviceMethods for R where R::Target: IsA<DeviceState> {}
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 18/27] rust: add bindings for memattrs
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (16 preceding siblings ...)
2025-02-13 16:00 ` [PULL 17/27] rust: add bindings for gpio_{in|out} initialization Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 19/27] rust: add bindings for timer Paolo Bonzini
` (9 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
The MemTxAttrs structure contains bitfield members, and bindgen is
unable to generate an equivalent macro definition for
MEMTXATTRS_UNSPECIFIED.
Therefore, manually define a global constant variable
MEMTXATTRS_UNSPECIFIED to support calls from Rust code.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250125125137.1223277-6-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/wrapper.h | 1 +
rust/qemu-api/src/memory.rs | 16 ++++++++++++++--
rust/qemu-api/src/zeroable.rs | 1 +
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/rust/wrapper.h b/rust/wrapper.h
index a9bc67af0d5..54839ce0f51 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -62,3 +62,4 @@ typedef enum memory_order {
#include "qapi/error.h"
#include "migration/vmstate.h"
#include "chardev/char-serial.h"
+#include "exec/memattrs.h"
diff --git a/rust/qemu-api/src/memory.rs b/rust/qemu-api/src/memory.rs
index 963d689c27d..682951ab44e 100644
--- a/rust/qemu-api/src/memory.rs
+++ b/rust/qemu-api/src/memory.rs
@@ -2,7 +2,7 @@
// Author(s): Paolo Bonzini <pbonzini@redhat.com>
// SPDX-License-Identifier: GPL-2.0-or-later
-//! Bindings for `MemoryRegion` and `MemoryRegionOps`
+//! Bindings for `MemoryRegion`, `MemoryRegionOps` and `MemTxAttrs`
use std::{
ffi::{CStr, CString},
@@ -11,7 +11,7 @@
ptr::addr_of,
};
-pub use bindings::hwaddr;
+pub use bindings::{hwaddr, MemTxAttrs};
use crate::{
bindings::{self, device_endian, memory_region_init_io},
@@ -189,3 +189,15 @@ unsafe impl ObjectType for MemoryRegion {
unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_MEMORY_REGION) };
}
qom_isa!(MemoryRegion: Object);
+
+/// A special `MemTxAttrs` constant, used to indicate that no memory
+/// attributes are specified.
+///
+/// Bus masters which don't specify any attributes will get this,
+/// which has all attribute bits clear except the topmost one
+/// (so that we can distinguish "all attributes deliberately clear"
+/// from "didn't specify" if necessary).
+pub const MEMTXATTRS_UNSPECIFIED: MemTxAttrs = MemTxAttrs {
+ unspecified: true,
+ ..Zeroable::ZERO
+};
diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
index 75742b50d4e..9f009606b1a 100644
--- a/rust/qemu-api/src/zeroable.rs
+++ b/rust/qemu-api/src/zeroable.rs
@@ -101,3 +101,4 @@ fn default() -> Self {
impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_1);
impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_2);
impl_zeroable!(crate::bindings::MemoryRegionOps);
+impl_zeroable!(crate::bindings::MemTxAttrs);
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 19/27] rust: add bindings for timer
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (17 preceding siblings ...)
2025-02-13 16:00 ` [PULL 18/27] rust: add bindings for memattrs Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 20/27] rust/timer/hpet: define hpet_fw_cfg Paolo Bonzini
` (8 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Add timer bindings to help handle idiomatic Rust callbacks.
Additionally, wrap QEMUClockType in ClockType binding to avoid unsafe
calls in device code.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250210030051.2562726-7-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 1 +
meson.build | 7 +++
rust/wrapper.h | 1 +
rust/qemu-api/meson.build | 1 +
rust/qemu-api/src/lib.rs | 1 +
rust/qemu-api/src/timer.rs | 98 ++++++++++++++++++++++++++++++++++++++
6 files changed, 109 insertions(+)
create mode 100644 rust/qemu-api/src/timer.rs
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index a5399db50b5..90958e5a306 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -186,6 +186,7 @@ module status
``qdev`` stable
``qom`` stable
``sysbus`` stable
+``timer`` stable
``vmstate`` proof of concept
``zeroable`` stable
================ ======================
diff --git a/meson.build b/meson.build
index 16c76c493f3..8ed10b6624e 100644
--- a/meson.build
+++ b/meson.build
@@ -4087,6 +4087,13 @@ if have_rust
foreach enum : c_bitfields
bindgen_args += ['--bitfield-enum', enum]
endforeach
+ c_nocopy = [
+ 'QEMUTimer',
+ ]
+ # Used to customize Drop trait
+ foreach struct : c_nocopy
+ bindgen_args += ['--no-copy', struct]
+ endforeach
# TODO: Remove this comment when the clang/libclang mismatch issue is solved.
#
diff --git a/rust/wrapper.h b/rust/wrapper.h
index 54839ce0f51..a35bfbd1760 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -63,3 +63,4 @@ typedef enum memory_order {
#include "migration/vmstate.h"
#include "chardev/char-serial.h"
#include "exec/memattrs.h"
+#include "qemu/timer.h"
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 45e30324b29..2e9c1078b9b 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -30,6 +30,7 @@ _qemu_api_rs = static_library(
'src/qdev.rs',
'src/qom.rs',
'src/sysbus.rs',
+ 'src/timer.rs',
'src/vmstate.rs',
'src/zeroable.rs',
],
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 2a338a888a0..ed1a8f9a2b4 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -26,6 +26,7 @@
pub mod qdev;
pub mod qom;
pub mod sysbus;
+pub mod timer;
pub mod vmstate;
pub mod zeroable;
diff --git a/rust/qemu-api/src/timer.rs b/rust/qemu-api/src/timer.rs
new file mode 100644
index 00000000000..a593538917a
--- /dev/null
+++ b/rust/qemu-api/src/timer.rs
@@ -0,0 +1,98 @@
+// Copyright (C) 2024 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use std::os::raw::{c_int, c_void};
+
+use crate::{
+ bindings::{self, qemu_clock_get_ns, timer_del, timer_init_full, timer_mod, QEMUClockType},
+ callbacks::FnCall,
+};
+
+pub type Timer = bindings::QEMUTimer;
+pub type TimerListGroup = bindings::QEMUTimerListGroup;
+
+impl Timer {
+ pub const MS: u32 = bindings::SCALE_MS;
+ pub const US: u32 = bindings::SCALE_US;
+ pub const NS: u32 = bindings::SCALE_NS;
+
+ pub fn new() -> Self {
+ Default::default()
+ }
+
+ const fn as_mut_ptr(&self) -> *mut Self {
+ self as *const Timer as *mut _
+ }
+
+ pub fn init_full<'timer, 'opaque: 'timer, T, F>(
+ &'timer mut self,
+ timer_list_group: Option<&TimerListGroup>,
+ clk_type: ClockType,
+ scale: u32,
+ attributes: u32,
+ _cb: F,
+ opaque: &'opaque T,
+ ) where
+ F: for<'a> FnCall<(&'a T,)>,
+ {
+ let _: () = F::ASSERT_IS_SOME;
+
+ /// timer expiration callback
+ unsafe extern "C" fn rust_timer_handler<T, F: for<'a> FnCall<(&'a T,)>>(
+ opaque: *mut c_void,
+ ) {
+ // SAFETY: the opaque was passed as a reference to `T`.
+ F::call((unsafe { &*(opaque.cast::<T>()) },))
+ }
+
+ let timer_cb: unsafe extern "C" fn(*mut c_void) = rust_timer_handler::<T, F>;
+
+ // SAFETY: the opaque outlives the timer
+ unsafe {
+ timer_init_full(
+ self,
+ if let Some(g) = timer_list_group {
+ g as *const TimerListGroup as *mut _
+ } else {
+ ::core::ptr::null_mut()
+ },
+ clk_type.id,
+ scale as c_int,
+ attributes as c_int,
+ Some(timer_cb),
+ (opaque as *const T).cast::<c_void>() as *mut c_void,
+ )
+ }
+ }
+
+ pub fn modify(&self, expire_time: u64) {
+ unsafe { timer_mod(self.as_mut_ptr(), expire_time as i64) }
+ }
+
+ pub fn delete(&self) {
+ unsafe { timer_del(self.as_mut_ptr()) }
+ }
+}
+
+impl Drop for Timer {
+ fn drop(&mut self) {
+ self.delete()
+ }
+}
+
+pub struct ClockType {
+ id: QEMUClockType,
+}
+
+impl ClockType {
+ pub fn get_ns(&self) -> u64 {
+ // SAFETY: cannot be created outside this module, therefore id
+ // is valid
+ (unsafe { qemu_clock_get_ns(self.id) }) as u64
+ }
+}
+
+pub const CLOCK_VIRTUAL: ClockType = ClockType {
+ id: QEMUClockType::QEMU_CLOCK_VIRTUAL,
+};
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 20/27] rust/timer/hpet: define hpet_fw_cfg
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (18 preceding siblings ...)
2025-02-13 16:00 ` [PULL 19/27] rust: add bindings for timer Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 21/27] rust/timer/hpet: add basic HPET timer and HPETState Paolo Bonzini
` (7 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Define HPETFwEntry structure with the same memory layout as
hpet_fw_entry in C.
Further, define the global hpet_cfg variable in Rust which is the
same as the C version. This hpet_cfg variable in Rust will replace
the C version one and allows both Rust code and C code to access it.
The Rust version of hpet_cfg is self-contained, avoiding unsafe
access to C code.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250210030051.2562726-8-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/Cargo.lock | 8 ++++
rust/Cargo.toml | 1 +
rust/hw/meson.build | 1 +
rust/hw/timer/hpet/Cargo.toml | 18 ++++++++
rust/hw/timer/hpet/meson.build | 18 ++++++++
rust/hw/timer/hpet/src/fw_cfg.rs | 71 ++++++++++++++++++++++++++++++++
rust/hw/timer/hpet/src/lib.rs | 10 +++++
rust/hw/timer/meson.build | 1 +
rust/qemu-api/src/zeroable.rs | 6 ++-
9 files changed, 132 insertions(+), 2 deletions(-)
create mode 100644 rust/hw/timer/hpet/Cargo.toml
create mode 100644 rust/hw/timer/hpet/meson.build
create mode 100644 rust/hw/timer/hpet/src/fw_cfg.rs
create mode 100644 rust/hw/timer/hpet/src/lib.rs
create mode 100644 rust/hw/timer/meson.build
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index c0c6069247a..79e142723b8 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -37,6 +37,14 @@ version = "1.12.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b"
+[[package]]
+name = "hpet"
+version = "0.1.0"
+dependencies = [
+ "qemu_api",
+ "qemu_api_macros",
+]
+
[[package]]
name = "itertools"
version = "0.11.0"
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 5b0cb559286..5041d6291fd 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -4,6 +4,7 @@ members = [
"qemu-api-macros",
"qemu-api",
"hw/char/pl011",
+ "hw/timer/hpet",
]
[workspace.lints.rust]
diff --git a/rust/hw/meson.build b/rust/hw/meson.build
index 860196645e7..9749d4adfc9 100644
--- a/rust/hw/meson.build
+++ b/rust/hw/meson.build
@@ -1 +1,2 @@
subdir('char')
+subdir('timer')
diff --git a/rust/hw/timer/hpet/Cargo.toml b/rust/hw/timer/hpet/Cargo.toml
new file mode 100644
index 00000000000..147f216e725
--- /dev/null
+++ b/rust/hw/timer/hpet/Cargo.toml
@@ -0,0 +1,18 @@
+[package]
+name = "hpet"
+version = "0.1.0"
+edition = "2021"
+authors = ["Zhao Liu <zhao1.liu@intel.com>"]
+license = "GPL-2.0-or-later"
+description = "IA-PC High Precision Event Timer emulation in Rust"
+rust-version = "1.63.0"
+
+[lib]
+crate-type = ["staticlib"]
+
+[dependencies]
+qemu_api = { path = "../../../qemu-api" }
+qemu_api_macros = { path = "../../../qemu-api-macros" }
+
+[lints]
+workspace = true
diff --git a/rust/hw/timer/hpet/meson.build b/rust/hw/timer/hpet/meson.build
new file mode 100644
index 00000000000..c2d7c0532ca
--- /dev/null
+++ b/rust/hw/timer/hpet/meson.build
@@ -0,0 +1,18 @@
+_libhpet_rs = static_library(
+ 'hpet',
+ files('src/lib.rs'),
+ override_options: ['rust_std=2021', 'build.rust_std=2021'],
+ rust_abi: 'rust',
+ dependencies: [
+ qemu_api,
+ qemu_api_macros,
+ ],
+)
+
+rust_devices_ss.add(when: 'CONFIG_X_HPET_RUST', if_true: [declare_dependency(
+ link_whole: [_libhpet_rs],
+ # Putting proc macro crates in `dependencies` is necessary for Meson to find
+ # them when compiling the root per-target static rust lib.
+ dependencies: [qemu_api_macros],
+ variables: {'crate': 'hpet'},
+)])
diff --git a/rust/hw/timer/hpet/src/fw_cfg.rs b/rust/hw/timer/hpet/src/fw_cfg.rs
new file mode 100644
index 00000000000..849e277d483
--- /dev/null
+++ b/rust/hw/timer/hpet/src/fw_cfg.rs
@@ -0,0 +1,71 @@
+// Copyright (C) 2024 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#![allow(dead_code)]
+
+use std::ptr::addr_of_mut;
+
+use qemu_api::{cell::bql_locked, impl_zeroable, zeroable::Zeroable};
+
+/// Each `HPETState` represents a Event Timer Block. The v1 spec supports
+/// up to 8 blocks. QEMU only uses 1 block (in PC machine).
+const HPET_MAX_NUM_EVENT_TIMER_BLOCK: usize = 8;
+
+#[repr(C, packed)]
+#[derive(Copy, Clone, Default)]
+pub struct HPETFwEntry {
+ pub event_timer_block_id: u32,
+ pub address: u64,
+ pub min_tick: u16,
+ pub page_prot: u8,
+}
+impl_zeroable!(HPETFwEntry);
+
+#[repr(C, packed)]
+#[derive(Copy, Clone, Default)]
+pub struct HPETFwConfig {
+ pub count: u8,
+ pub hpet: [HPETFwEntry; HPET_MAX_NUM_EVENT_TIMER_BLOCK],
+}
+impl_zeroable!(HPETFwConfig);
+
+#[allow(non_upper_case_globals)]
+#[no_mangle]
+pub static mut hpet_fw_cfg: HPETFwConfig = HPETFwConfig {
+ count: u8::MAX,
+ ..Zeroable::ZERO
+};
+
+impl HPETFwConfig {
+ pub(crate) fn assign_hpet_id() -> usize {
+ assert!(bql_locked());
+ // SAFETY: all accesses go through these methods, which guarantee
+ // that the accesses are protected by the BQL.
+ let mut fw_cfg = unsafe { *addr_of_mut!(hpet_fw_cfg) };
+
+ if fw_cfg.count == u8::MAX {
+ // first instance
+ fw_cfg.count = 0;
+ }
+
+ if fw_cfg.count == 8 {
+ // TODO: Add error binding: error_setg()
+ panic!("Only 8 instances of HPET is allowed");
+ }
+
+ let id: usize = fw_cfg.count.into();
+ fw_cfg.count += 1;
+ id
+ }
+
+ pub(crate) fn update_hpet_cfg(hpet_id: usize, timer_block_id: u32, address: u64) {
+ assert!(bql_locked());
+ // SAFETY: all accesses go through these methods, which guarantee
+ // that the accesses are protected by the BQL.
+ let mut fw_cfg = unsafe { *addr_of_mut!(hpet_fw_cfg) };
+
+ fw_cfg.hpet[hpet_id].event_timer_block_id = timer_block_id;
+ fw_cfg.hpet[hpet_id].address = address;
+ }
+}
diff --git a/rust/hw/timer/hpet/src/lib.rs b/rust/hw/timer/hpet/src/lib.rs
new file mode 100644
index 00000000000..44e9f3fb8ab
--- /dev/null
+++ b/rust/hw/timer/hpet/src/lib.rs
@@ -0,0 +1,10 @@
+// Copyright (C) 2024 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! # HPET QEMU Device Model
+//!
+//! This library implements a device model for the IA-PC HPET (High
+//! Precision Event Timers) device in QEMU.
+
+pub mod fw_cfg;
diff --git a/rust/hw/timer/meson.build b/rust/hw/timer/meson.build
new file mode 100644
index 00000000000..22a84f15536
--- /dev/null
+++ b/rust/hw/timer/meson.build
@@ -0,0 +1 @@
+subdir('hpet')
diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
index 9f009606b1a..cd424e6ea05 100644
--- a/rust/qemu-api/src/zeroable.rs
+++ b/rust/qemu-api/src/zeroable.rs
@@ -56,6 +56,7 @@ pub unsafe trait Zeroable: Default {
/// ## Differences with `core::mem::zeroed`
///
/// `const_zero` zeroes padding bits, while `core::mem::zeroed` doesn't
+#[macro_export]
macro_rules! const_zero {
// This macro to produce a type-generic zero constant is taken from the
// const_zero crate (v0.1.1):
@@ -77,10 +78,11 @@ union TypeAsBytes {
}
/// A wrapper to implement the `Zeroable` trait through the `const_zero` macro.
+#[macro_export]
macro_rules! impl_zeroable {
($type:ty) => {
- unsafe impl Zeroable for $type {
- const ZERO: Self = unsafe { const_zero!($type) };
+ unsafe impl $crate::zeroable::Zeroable for $type {
+ const ZERO: Self = unsafe { $crate::const_zero!($type) };
}
};
}
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 21/27] rust/timer/hpet: add basic HPET timer and HPETState
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (19 preceding siblings ...)
2025-02-13 16:00 ` [PULL 20/27] rust/timer/hpet: define hpet_fw_cfg Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 22/27] rust/timer/hpet: add qom and qdev APIs support Paolo Bonzini
` (6 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Add the HPETTimer and HPETState (HPET timer block), along with their
basic methods and register definitions.
This is in preparation for supporting the QAPI interfaces.
Note, wrap all items in HPETState that may be changed in the callback
called by C code into the BqlCell/BqlRefCell.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250210030051.2562726-9-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/wrapper.h | 1 +
rust/hw/timer/hpet/src/hpet.rs | 629 +++++++++++++++++++++++++++++++++
rust/hw/timer/hpet/src/lib.rs | 1 +
3 files changed, 631 insertions(+)
create mode 100644 rust/hw/timer/hpet/src/hpet.rs
diff --git a/rust/wrapper.h b/rust/wrapper.h
index a35bfbd1760..d927ad6799d 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -64,3 +64,4 @@ typedef enum memory_order {
#include "chardev/char-serial.h"
#include "exec/memattrs.h"
#include "qemu/timer.h"
+#include "exec/address-spaces.h"
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
new file mode 100644
index 00000000000..795610f8e88
--- /dev/null
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -0,0 +1,629 @@
+// Copyright (C) 2024 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#![allow(dead_code)]
+
+use std::ptr::{addr_of_mut, null_mut, NonNull};
+
+use qemu_api::{
+ bindings::{address_space_memory, address_space_stl_le},
+ cell::{BqlCell, BqlRefCell},
+ irq::InterruptSource,
+ memory::{MemoryRegion, MEMTXATTRS_UNSPECIFIED},
+ prelude::*,
+ qom::ParentField,
+ sysbus::SysBusDevice,
+ timer::{Timer, CLOCK_VIRTUAL},
+};
+
+/// Register space for each timer block (`HPET_BASE` is defined in hpet.h).
+const HPET_REG_SPACE_LEN: u64 = 0x400; // 1024 bytes
+
+/// Minimum recommended hardware implementation.
+const HPET_MIN_TIMERS: usize = 3;
+/// Maximum timers in each timer block.
+const HPET_MAX_TIMERS: usize = 32;
+
+/// Flags that HPETState.flags supports.
+const HPET_FLAG_MSI_SUPPORT_SHIFT: usize = 0;
+
+const HPET_NUM_IRQ_ROUTES: usize = 32;
+const HPET_LEGACY_PIT_INT: u32 = 0; // HPET_LEGACY_RTC_INT isn't defined here.
+const RTC_ISA_IRQ: usize = 8;
+
+const HPET_CLK_PERIOD: u64 = 10; // 10 ns
+const FS_PER_NS: u64 = 1000000; // 1000000 femtoseconds == 1 ns
+
+/// General Capabilities and ID Register
+const HPET_CAP_REG: u64 = 0x000;
+/// Revision ID (bits 0:7). Revision 1 is implemented (refer to v1.0a spec).
+const HPET_CAP_REV_ID_VALUE: u64 = 0x1;
+const HPET_CAP_REV_ID_SHIFT: usize = 0;
+/// Number of Timers (bits 8:12)
+const HPET_CAP_NUM_TIM_SHIFT: usize = 8;
+/// Counter Size (bit 13)
+const HPET_CAP_COUNT_SIZE_CAP_SHIFT: usize = 13;
+/// Legacy Replacement Route Capable (bit 15)
+const HPET_CAP_LEG_RT_CAP_SHIFT: usize = 15;
+/// Vendor ID (bits 16:31)
+const HPET_CAP_VENDER_ID_VALUE: u64 = 0x8086;
+const HPET_CAP_VENDER_ID_SHIFT: usize = 16;
+/// Main Counter Tick Period (bits 32:63)
+const HPET_CAP_CNT_CLK_PERIOD_SHIFT: usize = 32;
+
+/// General Configuration Register
+const HPET_CFG_REG: u64 = 0x010;
+/// Overall Enable (bit 0)
+const HPET_CFG_ENABLE_SHIFT: usize = 0;
+/// Legacy Replacement Route (bit 1)
+const HPET_CFG_LEG_RT_SHIFT: usize = 1;
+/// Other bits are reserved.
+const HPET_CFG_WRITE_MASK: u64 = 0x003;
+
+/// General Interrupt Status Register
+const HPET_INT_STATUS_REG: u64 = 0x020;
+
+/// Main Counter Value Register
+const HPET_COUNTER_REG: u64 = 0x0f0;
+
+/// Timer N Configuration and Capability Register (masked by 0x18)
+const HPET_TN_CFG_REG: u64 = 0x000;
+/// bit 0, 7, and bits 16:31 are reserved.
+/// bit 4, 5, 15, and bits 32:64 are read-only.
+const HPET_TN_CFG_WRITE_MASK: u64 = 0x7f4e;
+/// Timer N Interrupt Type (bit 1)
+const HPET_TN_CFG_INT_TYPE_SHIFT: usize = 1;
+/// Timer N Interrupt Enable (bit 2)
+const HPET_TN_CFG_INT_ENABLE_SHIFT: usize = 2;
+/// Timer N Type (Periodic enabled or not, bit 3)
+const HPET_TN_CFG_PERIODIC_SHIFT: usize = 3;
+/// Timer N Periodic Interrupt Capable (support Periodic or not, bit 4)
+const HPET_TN_CFG_PERIODIC_CAP_SHIFT: usize = 4;
+/// Timer N Size (timer size is 64-bits or 32 bits, bit 5)
+const HPET_TN_CFG_SIZE_CAP_SHIFT: usize = 5;
+/// Timer N Value Set (bit 6)
+const HPET_TN_CFG_SETVAL_SHIFT: usize = 6;
+/// Timer N 32-bit Mode (bit 8)
+const HPET_TN_CFG_32BIT_SHIFT: usize = 8;
+/// Timer N Interrupt Rout (bits 9:13)
+const HPET_TN_CFG_INT_ROUTE_MASK: u64 = 0x3e00;
+const HPET_TN_CFG_INT_ROUTE_SHIFT: usize = 9;
+/// Timer N FSB Interrupt Enable (bit 14)
+const HPET_TN_CFG_FSB_ENABLE_SHIFT: usize = 14;
+/// Timer N FSB Interrupt Delivery (bit 15)
+const HPET_TN_CFG_FSB_CAP_SHIFT: usize = 15;
+/// Timer N Interrupt Routing Capability (bits 32:63)
+const HPET_TN_CFG_INT_ROUTE_CAP_SHIFT: usize = 32;
+
+/// Timer N Comparator Value Register (masked by 0x18)
+const HPET_TN_CMP_REG: u64 = 0x008;
+
+/// Timer N FSB Interrupt Route Register (masked by 0x18)
+const HPET_TN_FSB_ROUTE_REG: u64 = 0x010;
+
+const fn hpet_next_wrap(cur_tick: u64) -> u64 {
+ (cur_tick | 0xffffffff) + 1
+}
+
+const fn hpet_time_after(a: u64, b: u64) -> bool {
+ ((b - a) as i64) < 0
+}
+
+const fn ticks_to_ns(value: u64) -> u64 {
+ value * HPET_CLK_PERIOD
+}
+
+const fn ns_to_ticks(value: u64) -> u64 {
+ value / HPET_CLK_PERIOD
+}
+
+// Avoid touching the bits that cannot be written.
+const fn hpet_fixup_reg(new: u64, old: u64, mask: u64) -> u64 {
+ (new & mask) | (old & !mask)
+}
+
+const fn activating_bit(old: u64, new: u64, shift: usize) -> bool {
+ let mask: u64 = 1 << shift;
+ (old & mask == 0) && (new & mask != 0)
+}
+
+const fn deactivating_bit(old: u64, new: u64, shift: usize) -> bool {
+ let mask: u64 = 1 << shift;
+ (old & mask != 0) && (new & mask == 0)
+}
+
+fn timer_handler(timer_cell: &BqlRefCell<HPETTimer>) {
+ timer_cell.borrow_mut().callback()
+}
+
+/// HPET Timer Abstraction
+#[repr(C)]
+#[derive(Debug, Default)]
+#[cfg_attr(has_offset_of, derive(qemu_api_macros::offsets))]
+pub struct HPETTimer {
+ /// timer N index within the timer block (`HPETState`)
+ #[doc(alias = "tn")]
+ index: usize,
+ qemu_timer: Option<Box<Timer>>,
+ /// timer block abstraction containing this timer
+ state: Option<NonNull<HPETState>>,
+
+ // Memory-mapped, software visible timer registers
+ /// Timer N Configuration and Capability Register
+ config: u64,
+ /// Timer N Comparator Value Register
+ cmp: u64,
+ /// Timer N FSB Interrupt Route Register
+ fsb: u64,
+
+ // Hidden register state
+ /// comparator (extended to counter width)
+ cmp64: u64,
+ /// Last value written to comparator
+ period: u64,
+ /// timer pop will indicate wrap for one-shot 32-bit
+ /// mode. Next pop will be actual timer expiration.
+ wrap_flag: u8,
+ /// last value armed, to avoid timer storms
+ last: u64,
+}
+
+impl HPETTimer {
+ fn init(&mut self, index: usize, state_ptr: *mut HPETState) -> &mut Self {
+ *self = HPETTimer::default();
+ self.index = index;
+ self.state = NonNull::new(state_ptr);
+ self
+ }
+
+ fn init_timer_with_state(&mut self) {
+ self.qemu_timer = Some(Box::new({
+ let mut t = Timer::new();
+ t.init_full(
+ None,
+ CLOCK_VIRTUAL,
+ Timer::NS,
+ 0,
+ timer_handler,
+ &self.get_state().timers[self.index],
+ );
+ t
+ }));
+ }
+
+ fn get_state(&self) -> &HPETState {
+ // SAFETY:
+ // the pointer is convertible to a reference
+ unsafe { self.state.unwrap().as_ref() }
+ }
+
+ fn is_int_active(&self) -> bool {
+ self.get_state().is_timer_int_active(self.index)
+ }
+
+ const fn is_fsb_route_enabled(&self) -> bool {
+ self.config & (1 << HPET_TN_CFG_FSB_ENABLE_SHIFT) != 0
+ }
+
+ const fn is_periodic(&self) -> bool {
+ self.config & (1 << HPET_TN_CFG_PERIODIC_SHIFT) != 0
+ }
+
+ const fn is_int_enabled(&self) -> bool {
+ self.config & (1 << HPET_TN_CFG_INT_ENABLE_SHIFT) != 0
+ }
+
+ const fn is_32bit_mod(&self) -> bool {
+ self.config & (1 << HPET_TN_CFG_32BIT_SHIFT) != 0
+ }
+
+ const fn is_valset_enabled(&self) -> bool {
+ self.config & (1 << HPET_TN_CFG_SETVAL_SHIFT) != 0
+ }
+
+ fn clear_valset(&mut self) {
+ self.config &= !(1 << HPET_TN_CFG_SETVAL_SHIFT);
+ }
+
+ /// True if timer interrupt is level triggered; otherwise, edge triggered.
+ const fn is_int_level_triggered(&self) -> bool {
+ self.config & (1 << HPET_TN_CFG_INT_TYPE_SHIFT) != 0
+ }
+
+ /// calculate next value of the general counter that matches the
+ /// target (either entirely, or the low 32-bit only depending on
+ /// the timer mode).
+ fn calculate_cmp64(&self, cur_tick: u64, target: u64) -> u64 {
+ if self.is_32bit_mod() {
+ let mut result: u64 = cur_tick.deposit(0, 32, target);
+ if result < cur_tick {
+ result += 0x100000000;
+ }
+ result
+ } else {
+ target
+ }
+ }
+
+ const fn get_individual_route(&self) -> usize {
+ ((self.config & HPET_TN_CFG_INT_ROUTE_MASK) >> HPET_TN_CFG_INT_ROUTE_SHIFT) as usize
+ }
+
+ fn get_int_route(&self) -> usize {
+ if self.index <= 1 && self.get_state().is_legacy_mode() {
+ // If LegacyReplacement Route bit is set, HPET specification requires
+ // timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O APIC,
+ // timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O APIC.
+ //
+ // If the LegacyReplacement Route bit is set, the individual routing
+ // bits for timers 0 and 1 (APIC or FSB) will have no impact.
+ //
+ // FIXME: Consider I/O APIC case.
+ if self.index == 0 {
+ 0
+ } else {
+ RTC_ISA_IRQ
+ }
+ } else {
+ // (If the LegacyReplacement Route bit is set) Timer 2-n will be
+ // routed as per the routing in the timer n config registers.
+ // ...
+ // If the LegacyReplacement Route bit is not set, the individual
+ // routing bits for each of the timers are used.
+ self.get_individual_route()
+ }
+ }
+
+ fn set_irq(&mut self, set: bool) {
+ let route = self.get_int_route();
+
+ if set && self.is_int_enabled() && self.get_state().is_hpet_enabled() {
+ if self.is_fsb_route_enabled() {
+ // SAFETY:
+ // the parameters are valid.
+ unsafe {
+ address_space_stl_le(
+ addr_of_mut!(address_space_memory),
+ self.fsb >> 32, // Timer N FSB int addr
+ self.fsb as u32, // Timer N FSB int value, truncate!
+ MEMTXATTRS_UNSPECIFIED,
+ null_mut(),
+ );
+ }
+ } else if self.is_int_level_triggered() {
+ self.get_state().irqs[route].raise();
+ } else {
+ self.get_state().irqs[route].pulse();
+ }
+ } else if !self.is_fsb_route_enabled() {
+ self.get_state().irqs[route].lower();
+ }
+ }
+
+ fn update_irq(&mut self, set: bool) {
+ // If Timer N Interrupt Enable bit is 0, "the timer will
+ // still operate and generate appropriate status bits, but
+ // will not cause an interrupt"
+ self.get_state()
+ .update_int_status(self.index as u32, set && self.is_int_level_triggered());
+ self.set_irq(set);
+ }
+
+ fn arm_timer(&mut self, tick: u64) {
+ let mut ns = self.get_state().get_ns(tick);
+
+ // Clamp period to reasonable min value (1 us)
+ if self.is_periodic() && ns - self.last < 1000 {
+ ns = self.last + 1000;
+ }
+
+ self.last = ns;
+ self.qemu_timer.as_ref().unwrap().modify(self.last);
+ }
+
+ fn set_timer(&mut self) {
+ let cur_tick: u64 = self.get_state().get_ticks();
+
+ self.wrap_flag = 0;
+ self.cmp64 = self.calculate_cmp64(cur_tick, self.cmp);
+ if self.is_32bit_mod() {
+ // HPET spec says in one-shot 32-bit mode, generate an interrupt when
+ // counter wraps in addition to an interrupt with comparator match.
+ if !self.is_periodic() && self.cmp64 > hpet_next_wrap(cur_tick) {
+ self.wrap_flag = 1;
+ self.arm_timer(hpet_next_wrap(cur_tick));
+ return;
+ }
+ }
+ self.arm_timer(self.cmp64);
+ }
+
+ fn del_timer(&mut self) {
+ // Just remove the timer from the timer_list without destroying
+ // this timer instance.
+ self.qemu_timer.as_ref().unwrap().delete();
+
+ if self.is_int_active() {
+ // For level-triggered interrupt, this leaves interrupt status
+ // register set but lowers irq.
+ self.update_irq(true);
+ }
+ }
+
+ /// Configuration and Capability Register
+ fn set_tn_cfg_reg(&mut self, shift: u32, len: u32, val: u64) {
+ // TODO: Add trace point - trace_hpet_ram_write_tn_cfg(addr & 4)
+ let old_val: u64 = self.config;
+ let mut new_val: u64 = old_val.deposit(shift, len, val);
+ new_val = hpet_fixup_reg(new_val, old_val, HPET_TN_CFG_WRITE_MASK);
+
+ // Switch level-type interrupt to edge-type.
+ if deactivating_bit(old_val, new_val, HPET_TN_CFG_INT_TYPE_SHIFT) {
+ // Do this before changing timer.config; otherwise, if
+ // HPET_TN_FSB is set, update_irq will not lower the qemu_irq.
+ self.update_irq(false);
+ }
+
+ self.config = new_val;
+
+ if activating_bit(old_val, new_val, HPET_TN_CFG_INT_ENABLE_SHIFT) && self.is_int_active() {
+ self.update_irq(true);
+ }
+
+ if self.is_32bit_mod() {
+ self.cmp = u64::from(self.cmp as u32); // truncate!
+ self.period = u64::from(self.period as u32); // truncate!
+ }
+
+ if self.get_state().is_hpet_enabled() {
+ self.set_timer();
+ }
+ }
+
+ /// Comparator Value Register
+ fn set_tn_cmp_reg(&mut self, shift: u32, len: u32, val: u64) {
+ let mut length = len;
+ let mut value = val;
+
+ // TODO: Add trace point - trace_hpet_ram_write_tn_cmp(addr & 4)
+ if self.is_32bit_mod() {
+ // High 32-bits are zero, leave them untouched.
+ if shift != 0 {
+ // TODO: Add trace point - trace_hpet_ram_write_invalid_tn_cmp()
+ return;
+ }
+ length = 64;
+ value = u64::from(value as u32); // truncate!
+ }
+
+ if !self.is_periodic() || self.is_valset_enabled() {
+ self.cmp = self.cmp.deposit(shift, length, value);
+ }
+
+ if self.is_periodic() {
+ self.period = self.period.deposit(shift, length, value);
+ }
+
+ self.clear_valset();
+ if self.get_state().is_hpet_enabled() {
+ self.set_timer();
+ }
+ }
+
+ /// FSB Interrupt Route Register
+ fn set_tn_fsb_route_reg(&mut self, shift: u32, len: u32, val: u64) {
+ self.fsb = self.fsb.deposit(shift, len, val);
+ }
+
+ fn reset(&mut self) {
+ self.del_timer();
+ self.cmp = u64::MAX; // Comparator Match Registers reset to all 1's.
+ self.config = (1 << HPET_TN_CFG_PERIODIC_CAP_SHIFT) | (1 << HPET_TN_CFG_SIZE_CAP_SHIFT);
+ if self.get_state().has_msi_flag() {
+ self.config |= 1 << HPET_TN_CFG_FSB_CAP_SHIFT;
+ }
+ // advertise availability of ioapic int
+ self.config |=
+ (u64::from(self.get_state().int_route_cap)) << HPET_TN_CFG_INT_ROUTE_CAP_SHIFT;
+ self.period = 0;
+ self.wrap_flag = 0;
+ }
+
+ /// timer expiration callback
+ fn callback(&mut self) {
+ let period: u64 = self.period;
+ let cur_tick: u64 = self.get_state().get_ticks();
+
+ if self.is_periodic() && period != 0 {
+ while hpet_time_after(cur_tick, self.cmp64) {
+ self.cmp64 += period;
+ }
+ if self.is_32bit_mod() {
+ self.cmp = u64::from(self.cmp64 as u32); // truncate!
+ } else {
+ self.cmp = self.cmp64;
+ }
+ self.arm_timer(self.cmp64);
+ } else if self.wrap_flag != 0 {
+ self.wrap_flag = 0;
+ self.arm_timer(self.cmp64);
+ }
+ self.update_irq(true);
+ }
+}
+
+/// HPET Event Timer Block Abstraction
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+pub struct HPETState {
+ parent_obj: ParentField<SysBusDevice>,
+ iomem: MemoryRegion,
+
+ // HPET block Registers: Memory-mapped, software visible registers
+ /// General Capabilities and ID Register
+ capability: BqlCell<u64>,
+ /// General Configuration Register
+ config: BqlCell<u64>,
+ /// General Interrupt Status Register
+ #[doc(alias = "isr")]
+ int_status: BqlCell<u64>,
+ /// Main Counter Value Register
+ #[doc(alias = "hpet_counter")]
+ counter: BqlCell<u64>,
+
+ // Internal state
+ /// Capabilities that QEMU HPET supports.
+ /// bit 0: MSI (or FSB) support.
+ flags: u32,
+
+ /// Offset of main counter relative to qemu clock.
+ hpet_offset: BqlCell<u64>,
+ hpet_offset_saved: bool,
+
+ irqs: [InterruptSource; HPET_NUM_IRQ_ROUTES],
+ rtc_irq_level: BqlCell<u32>,
+ pit_enabled: InterruptSource,
+
+ /// Interrupt Routing Capability.
+ /// This field indicates to which interrupts in the I/O (x) APIC
+ /// the timers' interrupt can be routed, and is encoded in the
+ /// bits 32:64 of timer N's config register:
+ #[doc(alias = "intcap")]
+ int_route_cap: u32,
+
+ /// HPET timer array managed by this timer block.
+ #[doc(alias = "timer")]
+ timers: [BqlRefCell<HPETTimer>; HPET_MAX_TIMERS],
+ num_timers: BqlCell<usize>,
+
+ /// Instance id (HPET timer block ID).
+ hpet_id: BqlCell<usize>,
+}
+
+impl HPETState {
+ const fn has_msi_flag(&self) -> bool {
+ self.flags & (1 << HPET_FLAG_MSI_SUPPORT_SHIFT) != 0
+ }
+
+ fn is_legacy_mode(&self) -> bool {
+ self.config.get() & (1 << HPET_CFG_LEG_RT_SHIFT) != 0
+ }
+
+ fn is_hpet_enabled(&self) -> bool {
+ self.config.get() & (1 << HPET_CFG_ENABLE_SHIFT) != 0
+ }
+
+ fn is_timer_int_active(&self, index: usize) -> bool {
+ self.int_status.get() & (1 << index) != 0
+ }
+
+ fn get_ticks(&self) -> u64 {
+ ns_to_ticks(CLOCK_VIRTUAL.get_ns() + self.hpet_offset.get())
+ }
+
+ fn get_ns(&self, tick: u64) -> u64 {
+ ticks_to_ns(tick) - self.hpet_offset.get()
+ }
+
+ fn handle_legacy_irq(&self, irq: u32, level: u32) {
+ if irq == HPET_LEGACY_PIT_INT {
+ if !self.is_legacy_mode() {
+ self.irqs[0].set(level != 0);
+ }
+ } else {
+ self.rtc_irq_level.set(level);
+ if !self.is_legacy_mode() {
+ self.irqs[RTC_ISA_IRQ].set(level != 0);
+ }
+ }
+ }
+
+ fn init_timer(&self) {
+ let raw_ptr: *mut HPETState = self as *const HPETState as *mut HPETState;
+
+ for (index, timer) in self.timers.iter().enumerate() {
+ timer
+ .borrow_mut()
+ .init(index, raw_ptr)
+ .init_timer_with_state();
+ }
+ }
+
+ fn update_int_status(&self, index: u32, level: bool) {
+ self.int_status
+ .set(self.int_status.get().deposit(index, 1, u64::from(level)));
+ }
+
+ /// General Configuration Register
+ fn set_cfg_reg(&self, shift: u32, len: u32, val: u64) {
+ let old_val = self.config.get();
+ let mut new_val = old_val.deposit(shift, len, val);
+
+ new_val = hpet_fixup_reg(new_val, old_val, HPET_CFG_WRITE_MASK);
+ self.config.set(new_val);
+
+ if activating_bit(old_val, new_val, HPET_CFG_ENABLE_SHIFT) {
+ // Enable main counter and interrupt generation.
+ self.hpet_offset
+ .set(ticks_to_ns(self.counter.get()) - CLOCK_VIRTUAL.get_ns());
+
+ for timer in self.timers.iter().take(self.num_timers.get()) {
+ let mut t = timer.borrow_mut();
+
+ if t.is_int_enabled() && t.is_int_active() {
+ t.update_irq(true);
+ }
+ t.set_timer();
+ }
+ } else if deactivating_bit(old_val, new_val, HPET_CFG_ENABLE_SHIFT) {
+ // Halt main counter and disable interrupt generation.
+ self.counter.set(self.get_ticks());
+
+ for timer in self.timers.iter().take(self.num_timers.get()) {
+ timer.borrow_mut().del_timer();
+ }
+ }
+
+ // i8254 and RTC output pins are disabled when HPET is in legacy mode
+ if activating_bit(old_val, new_val, HPET_CFG_LEG_RT_SHIFT) {
+ self.pit_enabled.set(false);
+ self.irqs[0].lower();
+ self.irqs[RTC_ISA_IRQ].lower();
+ } else if deactivating_bit(old_val, new_val, HPET_CFG_LEG_RT_SHIFT) {
+ // TODO: Add irq binding: qemu_irq_lower(s->irqs[0])
+ self.irqs[0].lower();
+ self.pit_enabled.set(true);
+ self.irqs[RTC_ISA_IRQ].set(self.rtc_irq_level.get() != 0);
+ }
+ }
+
+ /// General Interrupt Status Register: Read/Write Clear
+ fn set_int_status_reg(&self, shift: u32, _len: u32, val: u64) {
+ let new_val = val << shift;
+ let cleared = new_val & self.int_status.get();
+
+ for (index, timer) in self.timers.iter().take(self.num_timers.get()).enumerate() {
+ if cleared & (1 << index) != 0 {
+ timer.borrow_mut().update_irq(false);
+ }
+ }
+ }
+
+ /// Main Counter Value Register
+ fn set_counter_reg(&self, shift: u32, len: u32, val: u64) {
+ if self.is_hpet_enabled() {
+ // TODO: Add trace point -
+ // trace_hpet_ram_write_counter_write_while_enabled()
+ //
+ // HPET spec says that writes to this register should only be
+ // done while the counter is halted. So this is an undefined
+ // behavior. There's no need to forbid it, but when HPET is
+ // enabled, the changed counter value will not affect the
+ // tick count (i.e., the previously calculated offset will
+ // not be changed as well).
+ }
+ self.counter
+ .set(self.counter.get().deposit(shift, len, val));
+ }
+}
diff --git a/rust/hw/timer/hpet/src/lib.rs b/rust/hw/timer/hpet/src/lib.rs
index 44e9f3fb8ab..d6ac0b2521a 100644
--- a/rust/hw/timer/hpet/src/lib.rs
+++ b/rust/hw/timer/hpet/src/lib.rs
@@ -8,3 +8,4 @@
//! Precision Event Timers) device in QEMU.
pub mod fw_cfg;
+pub mod hpet;
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 22/27] rust/timer/hpet: add qom and qdev APIs support
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (20 preceding siblings ...)
2025-02-13 16:00 ` [PULL 21/27] rust/timer/hpet: add basic HPET timer and HPETState Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 23/27] i386: enable rust hpet for pc when rust is enabled Paolo Bonzini
` (5 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Implement QOM & QAPI support for HPET device.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250210030051.2562726-10-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/timer/hpet/src/fw_cfg.rs | 2 -
rust/hw/timer/hpet/src/hpet.rs | 278 ++++++++++++++++++++++++++++++-
rust/hw/timer/hpet/src/lib.rs | 4 +
3 files changed, 273 insertions(+), 11 deletions(-)
diff --git a/rust/hw/timer/hpet/src/fw_cfg.rs b/rust/hw/timer/hpet/src/fw_cfg.rs
index 849e277d483..bef03727ea3 100644
--- a/rust/hw/timer/hpet/src/fw_cfg.rs
+++ b/rust/hw/timer/hpet/src/fw_cfg.rs
@@ -2,8 +2,6 @@
// Author(s): Zhao Liu <zhai1.liu@intel.com>
// SPDX-License-Identifier: GPL-2.0-or-later
-#![allow(dead_code)]
-
use std::ptr::addr_of_mut;
use qemu_api::{cell::bql_locked, impl_zeroable, zeroable::Zeroable};
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index 795610f8e88..75ff5b3e8d6 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -2,21 +2,33 @@
// Author(s): Zhao Liu <zhai1.liu@intel.com>
// SPDX-License-Identifier: GPL-2.0-or-later
-#![allow(dead_code)]
-
-use std::ptr::{addr_of_mut, null_mut, NonNull};
+use std::{
+ ffi::CStr,
+ ptr::{addr_of_mut, null_mut, NonNull},
+ slice::from_ref,
+};
use qemu_api::{
- bindings::{address_space_memory, address_space_stl_le},
+ bindings::{
+ address_space_memory, address_space_stl_le, qdev_prop_bit, qdev_prop_bool,
+ qdev_prop_uint32, qdev_prop_uint8,
+ },
+ c_str,
cell::{BqlCell, BqlRefCell},
irq::InterruptSource,
- memory::{MemoryRegion, MEMTXATTRS_UNSPECIFIED},
+ memory::{
+ hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder, MEMTXATTRS_UNSPECIFIED,
+ },
prelude::*,
- qom::ParentField,
+ qdev::{DeviceImpl, DeviceMethods, DeviceState, Property, ResetType, ResettablePhasesImpl},
+ qom::{ObjectImpl, ObjectType, ParentField},
+ qom_isa,
sysbus::SysBusDevice,
timer::{Timer, CLOCK_VIRTUAL},
};
+use crate::fw_cfg::HPETFwConfig;
+
/// Register space for each timer block (`HPET_BASE` is defined in hpet.h).
const HPET_REG_SPACE_LEN: u64 = 0x400; // 1024 bytes
@@ -139,8 +151,7 @@ fn timer_handler(timer_cell: &BqlRefCell<HPETTimer>) {
/// HPET Timer Abstraction
#[repr(C)]
-#[derive(Debug, Default)]
-#[cfg_attr(has_offset_of, derive(qemu_api_macros::offsets))]
+#[derive(Debug, Default, qemu_api_macros::offsets)]
pub struct HPETTimer {
/// timer N index within the timer block (`HPETState`)
#[doc(alias = "tn")]
@@ -451,11 +462,41 @@ fn callback(&mut self) {
}
self.update_irq(true);
}
+
+ const fn read(&self, addr: hwaddr, _size: u32) -> u64 {
+ let shift: u64 = (addr & 4) * 8;
+
+ match addr & !4 {
+ HPET_TN_CFG_REG => self.config >> shift, // including interrupt capabilities
+ HPET_TN_CMP_REG => self.cmp >> shift, // comparator register
+ HPET_TN_FSB_ROUTE_REG => self.fsb >> shift,
+ _ => {
+ // TODO: Add trace point - trace_hpet_ram_read_invalid()
+ // Reserved.
+ 0
+ }
+ }
+ }
+
+ fn write(&mut self, addr: hwaddr, value: u64, size: u32) {
+ let shift = ((addr & 4) * 8) as u32;
+ let len = std::cmp::min(size * 8, 64 - shift);
+
+ match addr & !4 {
+ HPET_TN_CFG_REG => self.set_tn_cfg_reg(shift, len, value),
+ HPET_TN_CMP_REG => self.set_tn_cmp_reg(shift, len, value),
+ HPET_TN_FSB_ROUTE_REG => self.set_tn_fsb_route_reg(shift, len, value),
+ _ => {
+ // TODO: Add trace point - trace_hpet_ram_write_invalid()
+ // Reserved.
+ }
+ }
+ }
}
/// HPET Event Timer Block Abstraction
#[repr(C)]
-#[derive(qemu_api_macros::offsets)]
+#[derive(qemu_api_macros::Object, qemu_api_macros::offsets)]
pub struct HPETState {
parent_obj: ParentField<SysBusDevice>,
iomem: MemoryRegion,
@@ -626,4 +667,223 @@ fn set_counter_reg(&self, shift: u32, len: u32, val: u64) {
self.counter
.set(self.counter.get().deposit(shift, len, val));
}
+
+ unsafe fn init(&mut self) {
+ static HPET_RAM_OPS: MemoryRegionOps<HPETState> =
+ MemoryRegionOpsBuilder::<HPETState>::new()
+ .read(&HPETState::read)
+ .write(&HPETState::write)
+ .native_endian()
+ .valid_sizes(4, 8)
+ .impl_sizes(4, 8)
+ .build();
+
+ // SAFETY:
+ // self and self.iomem are guaranteed to be valid at this point since callers
+ // must make sure the `self` reference is valid.
+ MemoryRegion::init_io(
+ unsafe { &mut *addr_of_mut!(self.iomem) },
+ addr_of_mut!(*self),
+ &HPET_RAM_OPS,
+ "hpet",
+ HPET_REG_SPACE_LEN,
+ );
+ }
+
+ fn post_init(&self) {
+ self.init_mmio(&self.iomem);
+ for irq in self.irqs.iter() {
+ self.init_irq(irq);
+ }
+ }
+
+ fn realize(&self) {
+ if self.int_route_cap == 0 {
+ // TODO: Add error binding: warn_report()
+ println!("Hpet's hpet-intcap property not initialized");
+ }
+
+ self.hpet_id.set(HPETFwConfig::assign_hpet_id());
+
+ if self.num_timers.get() < HPET_MIN_TIMERS {
+ self.num_timers.set(HPET_MIN_TIMERS);
+ } else if self.num_timers.get() > HPET_MAX_TIMERS {
+ self.num_timers.set(HPET_MAX_TIMERS);
+ }
+
+ self.init_timer();
+ // 64-bit General Capabilities and ID Register; LegacyReplacementRoute.
+ self.capability.set(
+ HPET_CAP_REV_ID_VALUE << HPET_CAP_REV_ID_SHIFT |
+ 1 << HPET_CAP_COUNT_SIZE_CAP_SHIFT |
+ 1 << HPET_CAP_LEG_RT_CAP_SHIFT |
+ HPET_CAP_VENDER_ID_VALUE << HPET_CAP_VENDER_ID_SHIFT |
+ ((self.num_timers.get() - 1) as u64) << HPET_CAP_NUM_TIM_SHIFT | // indicate the last timer
+ (HPET_CLK_PERIOD * FS_PER_NS) << HPET_CAP_CNT_CLK_PERIOD_SHIFT, // 10 ns
+ );
+
+ self.init_gpio_in(2, HPETState::handle_legacy_irq);
+ self.init_gpio_out(from_ref(&self.pit_enabled));
+ }
+
+ fn reset_hold(&self, _type: ResetType) {
+ let sbd = self.upcast::<SysBusDevice>();
+
+ for timer in self.timers.iter().take(self.num_timers.get()) {
+ timer.borrow_mut().reset();
+ }
+
+ self.counter.set(0);
+ self.config.set(0);
+ self.pit_enabled.set(true);
+ self.hpet_offset.set(0);
+
+ HPETFwConfig::update_hpet_cfg(
+ self.hpet_id.get(),
+ self.capability.get() as u32,
+ sbd.mmio[0].addr,
+ );
+
+ // to document that the RTC lowers its output on reset as well
+ self.rtc_irq_level.set(0);
+ }
+
+ fn timer_and_addr(&self, addr: hwaddr) -> Option<(&BqlRefCell<HPETTimer>, hwaddr)> {
+ let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
+
+ // TODO: Add trace point - trace_hpet_ram_[read|write]_timer_id(timer_id)
+ if timer_id > self.num_timers.get() {
+ // TODO: Add trace point - trace_hpet_timer_id_out_of_range(timer_id)
+ None
+ } else {
+ // Keep the complete address so that HPETTimer's read and write could
+ // detect the invalid access.
+ Some((&self.timers[timer_id], addr & 0x1F))
+ }
+ }
+
+ fn read(&self, addr: hwaddr, size: u32) -> u64 {
+ let shift: u64 = (addr & 4) * 8;
+
+ // address range of all TN regs
+ // TODO: Add trace point - trace_hpet_ram_read(addr)
+ if (0x100..=0x3ff).contains(&addr) {
+ match self.timer_and_addr(addr) {
+ None => 0, // Reserved,
+ Some((timer, tn_addr)) => timer.borrow_mut().read(tn_addr, size),
+ }
+ } else {
+ match addr & !4 {
+ HPET_CAP_REG => self.capability.get() >> shift, /* including HPET_PERIOD 0x004 */
+ // (CNT_CLK_PERIOD field)
+ HPET_CFG_REG => self.config.get() >> shift,
+ HPET_COUNTER_REG => {
+ let cur_tick: u64 = if self.is_hpet_enabled() {
+ self.get_ticks()
+ } else {
+ self.counter.get()
+ };
+
+ // TODO: Add trace point - trace_hpet_ram_read_reading_counter(addr & 4,
+ // cur_tick)
+ cur_tick >> shift
+ }
+ HPET_INT_STATUS_REG => self.int_status.get() >> shift,
+ _ => {
+ // TODO: Add trace point- trace_hpet_ram_read_invalid()
+ // Reserved.
+ 0
+ }
+ }
+ }
+ }
+
+ fn write(&self, addr: hwaddr, value: u64, size: u32) {
+ let shift = ((addr & 4) * 8) as u32;
+ let len = std::cmp::min(size * 8, 64 - shift);
+
+ // TODO: Add trace point - trace_hpet_ram_write(addr, value)
+ if (0x100..=0x3ff).contains(&addr) {
+ match self.timer_and_addr(addr) {
+ None => (), // Reserved.
+ Some((timer, tn_addr)) => timer.borrow_mut().write(tn_addr, value, size),
+ }
+ } else {
+ match addr & !0x4 {
+ HPET_CAP_REG => {} // General Capabilities and ID Register: Read Only
+ HPET_CFG_REG => self.set_cfg_reg(shift, len, value),
+ HPET_INT_STATUS_REG => self.set_int_status_reg(shift, len, value),
+ HPET_COUNTER_REG => self.set_counter_reg(shift, len, value),
+ _ => {
+ // TODO: Add trace point - trace_hpet_ram_write_invalid()
+ // Reserved.
+ }
+ }
+ }
+ }
+}
+
+qom_isa!(HPETState: SysBusDevice, DeviceState, Object);
+
+unsafe impl ObjectType for HPETState {
+ // No need for HPETClass. Just like OBJECT_DECLARE_SIMPLE_TYPE in C.
+ type Class = <SysBusDevice as ObjectType>::Class;
+ const TYPE_NAME: &'static CStr = crate::TYPE_HPET;
+}
+
+impl ObjectImpl for HPETState {
+ type ParentType = SysBusDevice;
+
+ const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
+ const INSTANCE_POST_INIT: Option<fn(&Self)> = Some(Self::post_init);
+}
+
+// TODO: Make these properties user-configurable!
+qemu_api::declare_properties! {
+ HPET_PROPERTIES,
+ qemu_api::define_property!(
+ c_str!("timers"),
+ HPETState,
+ num_timers,
+ unsafe { &qdev_prop_uint8 },
+ u8,
+ default = HPET_MIN_TIMERS
+ ),
+ qemu_api::define_property!(
+ c_str!("msi"),
+ HPETState,
+ flags,
+ unsafe { &qdev_prop_bit },
+ u32,
+ bit = HPET_FLAG_MSI_SUPPORT_SHIFT as u8,
+ default = false,
+ ),
+ qemu_api::define_property!(
+ c_str!("hpet-intcap"),
+ HPETState,
+ int_route_cap,
+ unsafe { &qdev_prop_uint32 },
+ u32,
+ default = 0
+ ),
+ qemu_api::define_property!(
+ c_str!("hpet-offset-saved"),
+ HPETState,
+ hpet_offset_saved,
+ unsafe { &qdev_prop_bool },
+ bool,
+ default = true
+ ),
+}
+
+impl DeviceImpl for HPETState {
+ fn properties() -> &'static [Property] {
+ &HPET_PROPERTIES
+ }
+
+ const REALIZE: Option<fn(&Self)> = Some(Self::realize);
+}
+
+impl ResettablePhasesImpl for HPETState {
+ const HOLD: Option<fn(&Self, ResetType)> = Some(Self::reset_hold);
}
diff --git a/rust/hw/timer/hpet/src/lib.rs b/rust/hw/timer/hpet/src/lib.rs
index d6ac0b2521a..5e7c961c289 100644
--- a/rust/hw/timer/hpet/src/lib.rs
+++ b/rust/hw/timer/hpet/src/lib.rs
@@ -7,5 +7,9 @@
//! This library implements a device model for the IA-PC HPET (High
//! Precision Event Timers) device in QEMU.
+use qemu_api::c_str;
+
pub mod fw_cfg;
pub mod hpet;
+
+pub const TYPE_HPET: &::std::ffi::CStr = c_str!("hpet");
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 23/27] i386: enable rust hpet for pc when rust is enabled
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (21 preceding siblings ...)
2025-02-13 16:00 ` [PULL 22/27] rust/timer/hpet: add qom and qdev APIs support Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-17 11:03 ` Zhao Liu
2025-02-13 16:00 ` [PULL 24/27] rust: qemu_api: add a documentation header for all modules Paolo Bonzini
` (4 subsequent siblings)
27 siblings, 1 reply; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Add HPET configuration in PC's Kconfig options, and select HPET device
(Rust version) if Rust is supported.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250210030051.2562726-11-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
configs/devices/i386-softmmu/default.mak | 1 +
hw/i386/pc.c | 2 +-
hw/timer/Kconfig | 2 +-
rust/hw/Kconfig | 1 +
rust/hw/timer/Kconfig | 2 ++
tests/qtest/meson.build | 3 ++-
6 files changed, 8 insertions(+), 3 deletions(-)
create mode 100644 rust/hw/timer/Kconfig
diff --git a/configs/devices/i386-softmmu/default.mak b/configs/devices/i386-softmmu/default.mak
index 4faf2f0315e..9ef343cace0 100644
--- a/configs/devices/i386-softmmu/default.mak
+++ b/configs/devices/i386-softmmu/default.mak
@@ -6,6 +6,7 @@
#CONFIG_APPLESMC=n
#CONFIG_FDC=n
#CONFIG_HPET=n
+#CONFIG_X_HPET_RUST=n
#CONFIG_HYPERV=n
#CONFIG_ISA_DEBUG=n
#CONFIG_ISA_IPMI_BT=n
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0eb52d315bb..22641e6ddca 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1701,7 +1701,7 @@ static void pc_machine_initfn(Object *obj)
pcms->sata_enabled = true;
pcms->i8042_enabled = true;
pcms->max_fw_size = 8 * MiB;
-#ifdef CONFIG_HPET
+#if defined(CONFIG_HPET) || defined(CONFIG_X_HPET_RUST)
pcms->hpet_enabled = true;
#endif
pcms->fd_bootchk = true;
diff --git a/hw/timer/Kconfig b/hw/timer/Kconfig
index c96fd5d97ae..9ac00845340 100644
--- a/hw/timer/Kconfig
+++ b/hw/timer/Kconfig
@@ -11,7 +11,7 @@ config A9_GTIMER
config HPET
bool
- default y if PC
+ default y if PC && !HAVE_RUST
config I8254
bool
diff --git a/rust/hw/Kconfig b/rust/hw/Kconfig
index 4d934f30afe..36f92ec0287 100644
--- a/rust/hw/Kconfig
+++ b/rust/hw/Kconfig
@@ -1,2 +1,3 @@
# devices Kconfig
source char/Kconfig
+source timer/Kconfig
diff --git a/rust/hw/timer/Kconfig b/rust/hw/timer/Kconfig
new file mode 100644
index 00000000000..afd98033503
--- /dev/null
+++ b/rust/hw/timer/Kconfig
@@ -0,0 +1,2 @@
+config X_HPET_RUST
+ bool
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 68316dbdc1a..8a6243382a1 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -103,7 +103,8 @@ qtests_i386 = \
config_all_devices.has_key('CONFIG_VIRTIO_PCI') and \
slirp.found() ? ['virtio-net-failover'] : []) + \
(unpack_edk2_blobs and \
- config_all_devices.has_key('CONFIG_HPET') and \
+ (config_all_devices.has_key('CONFIG_HPET') or \
+ config_all_devices.has_key('CONFIG_X_HPET_RUST')) and \
config_all_devices.has_key('CONFIG_PARALLEL') ? ['bios-tables-test'] : []) + \
qtests_pci + \
qtests_cxl + \
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PULL 23/27] i386: enable rust hpet for pc when rust is enabled
2025-02-13 16:00 ` [PULL 23/27] i386: enable rust hpet for pc when rust is enabled Paolo Bonzini
@ 2025-02-17 11:03 ` Zhao Liu
2025-02-17 13:15 ` Paolo Bonzini
0 siblings, 1 reply; 33+ messages in thread
From: Zhao Liu @ 2025-02-17 11:03 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
Hi Paolo,
> --- a/hw/timer/Kconfig
> +++ b/hw/timer/Kconfig
> @@ -11,7 +11,7 @@ config A9_GTIMER
>
> config HPET
> bool
> - default y if PC
> + default y if PC && !HAVE_RUST
+
+config X_HPET_RUST
+ bool
+ default y if PC && HAVE_RUST
> config I8254
> bool
This patch doesn't enable the configuration option of Rust HPET for PC.
Is it because the Rust HPET would break live migration, so we have to
disable it?
Thanks,
Zhao
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PULL 23/27] i386: enable rust hpet for pc when rust is enabled
2025-02-17 11:03 ` Zhao Liu
@ 2025-02-17 13:15 ` Paolo Bonzini
0 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-17 13:15 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel
On 2/17/25 12:03, Zhao Liu wrote:
> Hi Paolo,
>
>> --- a/hw/timer/Kconfig
>> +++ b/hw/timer/Kconfig
>> @@ -11,7 +11,7 @@ config A9_GTIMER
>>
>> config HPET
>> bool
>> - default y if PC
>> + default y if PC && !HAVE_RUST
>
> +
> +config X_HPET_RUST
> + bool
> + default y if PC && HAVE_RUST
>
>> config I8254
>> bool
>
> This patch doesn't enable the configuration option of Rust HPET for PC.
>
> Is it because the Rust HPET would break live migration, so we have to
> disable it?
No, something like this is missing:
diff --git a/rust/hw/timer/Kconfig b/rust/hw/timer/Kconfig
index afd98033503..42e421317a5 100644
--- a/rust/hw/timer/Kconfig
+++ b/rust/hw/timer/Kconfig
@@ -1,2 +1,3 @@
config X_HPET_RUST
bool
+ default y if PC && HAVE_RUST
I haven't checked if this passes CI yet.
Paolo
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PULL 24/27] rust: qemu_api: add a documentation header for all modules
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (22 preceding siblings ...)
2025-02-13 16:00 ` [PULL 23/27] i386: enable rust hpet for pc when rust is enabled Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 25/27] rust: vmstate: remove redundant link targets Paolo Bonzini
` (3 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/assertions.rs | 4 ++++
rust/qemu-api/src/bindings.rs | 2 ++
rust/qemu-api/src/c_str.rs | 8 ++++++++
rust/qemu-api/src/offset_of.rs | 7 +++++++
rust/qemu-api/src/prelude.rs | 2 ++
rust/qemu-api/src/sysbus.rs | 2 ++
rust/qemu-api/src/zeroable.rs | 2 ++
7 files changed, 27 insertions(+)
diff --git a/rust/qemu-api/src/assertions.rs b/rust/qemu-api/src/assertions.rs
index 6e420469806..fa1a18de6fe 100644
--- a/rust/qemu-api/src/assertions.rs
+++ b/rust/qemu-api/src/assertions.rs
@@ -2,9 +2,13 @@
// Author(s): Paolo Bonzini <pbonzini@redhat.com>
// SPDX-License-Identifier: GPL-2.0-or-later
+#![doc(hidden)]
//! This module provides macros to check the equality of types and
//! the type of `struct` fields. This can be useful to ensure that
//! types match the expectations of C code.
+//!
+//! Documentation is hidden because it only exposes macros, which
+//! are exported directly from `qemu_api`.
// Based on https://stackoverflow.com/questions/64251852/x/70978292#70978292
// (stackoverflow answers are released under MIT license).
diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
index b71220113ef..d2868639ff6 100644
--- a/rust/qemu-api/src/bindings.rs
+++ b/rust/qemu-api/src/bindings.rs
@@ -15,6 +15,8 @@
clippy::missing_safety_doc
)]
+//! `bindgen`-generated declarations.
+
#[cfg(MESON)]
include!("bindings.inc.rs");
diff --git a/rust/qemu-api/src/c_str.rs b/rust/qemu-api/src/c_str.rs
index 4cd96da0b45..3fa61b59c76 100644
--- a/rust/qemu-api/src/c_str.rs
+++ b/rust/qemu-api/src/c_str.rs
@@ -2,6 +2,14 @@
// Author(s): Paolo Bonzini <pbonzini@redhat.com>
// SPDX-License-Identifier: GPL-2.0-or-later
+#![doc(hidden)]
+//! This module provides a macro to define a constant of type
+//! [`CStr`](std::ffi::CStr), for compatibility with versions of
+//! Rust that lack `c""` literals.
+//!
+//! Documentation is hidden because it only exposes macros, which
+//! are exported directly from `qemu_api`.
+
#[macro_export]
/// Given a string constant _without_ embedded or trailing NULs, return
/// a `CStr`.
diff --git a/rust/qemu-api/src/offset_of.rs b/rust/qemu-api/src/offset_of.rs
index 075e98f986b..373229bbde9 100644
--- a/rust/qemu-api/src/offset_of.rs
+++ b/rust/qemu-api/src/offset_of.rs
@@ -1,5 +1,12 @@
// SPDX-License-Identifier: MIT
+#![doc(hidden)]
+//! This module provides macros that emulate the functionality of
+//! `core::mem::offset_of` on older versions of Rust.
+//!
+//! Documentation is hidden because it only exposes macros, which
+//! are exported directly from `qemu_api`.
+
/// 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! { }`.
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index 254edb476dd..fbf0ee23e0b 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -2,6 +2,8 @@
// Author(s): Paolo Bonzini <pbonzini@redhat.com>
// SPDX-License-Identifier: GPL-2.0-or-later
+//! Commonly used traits and types for QEMU.
+
pub use crate::bitops::IntegerExt;
pub use crate::cell::BqlCell;
diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs
index 1f66a5f1e09..fa36e12178f 100644
--- a/rust/qemu-api/src/sysbus.rs
+++ b/rust/qemu-api/src/sysbus.rs
@@ -2,6 +2,8 @@
// Author(s): Paolo Bonzini <pbonzini@redhat.com>
// SPDX-License-Identifier: GPL-2.0-or-later
+//! Bindings to access `sysbus` functionality from Rust.
+
use std::{ffi::CStr, ptr::addr_of_mut};
pub use bindings::{SysBusDevice, SysBusDeviceClass};
diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
index cd424e6ea05..a2356cb2f24 100644
--- a/rust/qemu-api/src/zeroable.rs
+++ b/rust/qemu-api/src/zeroable.rs
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+//! Defines a trait for structs that can be safely initialized with zero bytes.
+
/// Encapsulates the requirement that
/// `MaybeUninit::<Self>::zeroed().assume_init()` does not cause undefined
/// behavior. This trait in principle could be implemented as just:
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 25/27] rust: vmstate: remove redundant link targets
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (23 preceding siblings ...)
2025-02-13 16:00 ` [PULL 24/27] rust: qemu_api: add a documentation header for all modules Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 26/27] rust: fix doctests Paolo Bonzini
` (2 subsequent siblings)
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 164effc6553..c6dfb609356 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -191,10 +191,9 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateFlags
/// * scalar types (integer and `bool`)
/// * the C struct `QEMUTimer`
/// * a transparent wrapper for any of the above (`Cell`, `UnsafeCell`,
-/// [`BqlCell`](crate::cell::BqlCell), [`BqlRefCell`](crate::cell::BqlRefCell)
+/// [`BqlCell`], [`BqlRefCell`]
/// * a raw pointer to any of the above
-/// * a `NonNull` pointer, a `Box` or an [`Owned`](crate::qom::Owned) for any of
-/// the above
+/// * a `NonNull` pointer, a `Box` or an [`Owned`] for any of the above
/// * an array of any of the above
///
/// In order to support other types, the trait `VMState` must be implemented
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 26/27] rust: fix doctests
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (24 preceding siblings ...)
2025-02-13 16:00 ` [PULL 25/27] rust: vmstate: remove redundant link targets Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-13 16:00 ` [PULL 27/27] ui/sdl2: reenable the SDL2 Windows keyboard hook procedure Paolo Bonzini
2025-02-14 16:58 ` [PULL 00/27] rust, Windows patches for 2025-02-13 Stefan Hajnoczi
27 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel
Doctests were not being run by CI, and have broken. Fix them.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
.gitlab-ci.d/buildtest.yml | 6 ++++++
rust/qemu-api/src/vmstate.rs | 2 +-
rust/qemu-api/src/zeroable.rs | 2 +-
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 4265a577834..00f4bfcd9f3 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -131,6 +131,12 @@ build-system-fedora-rust-nightly:
CONFIGURE_ARGS: --disable-docs --enable-rust --enable-strict-rust-lints
TARGETS: aarch64-softmmu
MAKE_CHECK_ARGS: check-build
+ after_script:
+ - source scripts/ci/gitlab-ci-section
+ - section_start test "Running Rust doctests"
+ - cd build
+ - pyvenv/bin/meson devenv -w ../rust ${CARGO-cargo} test --doc -p qemu_api
+
allow_failure: true
check-system-fedora:
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index c6dfb609356..24a4dc81e7f 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -294,7 +294,7 @@ pub const fn with_varray_multiply(mut self, num: u32) -> VMStateField {
/// # Examples
///
/// ```
-/// # use qemu_api::vmstate::impl_vmstate_forward;
+/// # use qemu_api::impl_vmstate_forward;
/// pub struct Fifo([u8; 16]);
/// impl_vmstate_forward!(Fifo);
/// ```
diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
index a2356cb2f24..47b6977828d 100644
--- a/rust/qemu-api/src/zeroable.rs
+++ b/rust/qemu-api/src/zeroable.rs
@@ -7,7 +7,7 @@
/// behavior. This trait in principle could be implemented as just:
///
/// ```
-/// pub unsafe trait Zeroable {
+/// pub unsafe trait Zeroable: Default {
/// const ZERO: Self = unsafe { ::core::mem::MaybeUninit::<Self>::zeroed().assume_init() };
/// }
/// ```
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PULL 27/27] ui/sdl2: reenable the SDL2 Windows keyboard hook procedure
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (25 preceding siblings ...)
2025-02-13 16:00 ` [PULL 26/27] rust: fix doctests Paolo Bonzini
@ 2025-02-13 16:00 ` Paolo Bonzini
2025-02-15 17:35 ` Michael Tokarev
2025-02-14 16:58 ` [PULL 00/27] rust, Windows patches for 2025-02-13 Stefan Hajnoczi
27 siblings, 1 reply; 33+ messages in thread
From: Paolo Bonzini @ 2025-02-13 16:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Volker Rümelin
From: Volker Rümelin <vr_qemu@t-online.de>
Windows only:
The libSDL2 Windows message loop needs the libSDL2 Windows low
level keyboard hook procedure to grab the left and right Windows
keys correctly. Reenable the SDL2 Windows keyboard hook procedure.
Since SDL2 2.30.4 the SDL2 keyboard hook procedure also filters
out the special left Control key event for every Alt Gr key event
on keyboards with an international layout. This means the QEMU low
level keyboard hook procedure is no longer needed. Remove the QEMU
Windows keyboard hook procedure.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2139
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2323
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Link: https://lore.kernel.org/r/20241231115950.6732-1-vr_qemu@t-online.de
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
ui/sdl2.c | 26 --------------------------
ui/meson.build | 4 ----
2 files changed, 30 deletions(-)
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 445eb1dd9f9..cda4293a53e 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -32,7 +32,6 @@
#include "system/runstate.h"
#include "system/runstate-action.h"
#include "system/system.h"
-#include "ui/win32-kbd-hook.h"
#include "qemu/log.h"
#include "qemu-main.h"
@@ -263,7 +262,6 @@ static void sdl_grab_start(struct sdl2_console *scon)
}
SDL_SetWindowGrab(scon->real_window, SDL_TRUE);
gui_grab = 1;
- win32_kbd_set_grab(true);
sdl_update_caption(scon);
}
@@ -271,7 +269,6 @@ static void sdl_grab_end(struct sdl2_console *scon)
{
SDL_SetWindowGrab(scon->real_window, SDL_FALSE);
gui_grab = 0;
- win32_kbd_set_grab(false);
sdl_show_cursor(scon);
sdl_update_caption(scon);
}
@@ -372,19 +369,6 @@ static int get_mod_state(void)
}
}
-static void *sdl2_win32_get_hwnd(struct sdl2_console *scon)
-{
-#ifdef CONFIG_WIN32
- SDL_SysWMinfo info;
-
- SDL_VERSION(&info.version);
- if (SDL_GetWindowWMInfo(scon->real_window, &info)) {
- return info.info.win.window;
- }
-#endif
- return NULL;
-}
-
static void handle_keydown(SDL_Event *ev)
{
int win;
@@ -609,10 +593,6 @@ static void handle_windowevent(SDL_Event *ev)
sdl2_redraw(scon);
break;
case SDL_WINDOWEVENT_FOCUS_GAINED:
- win32_kbd_set_grab(gui_grab);
- if (qemu_console_is_graphic(scon->dcl.con)) {
- win32_kbd_set_window(sdl2_win32_get_hwnd(scon));
- }
/* fall through */
case SDL_WINDOWEVENT_ENTER:
if (!gui_grab && (qemu_input_is_absolute(scon->dcl.con) || absolute_enabled)) {
@@ -628,9 +608,6 @@ static void handle_windowevent(SDL_Event *ev)
scon->ignore_hotkeys = get_mod_state();
break;
case SDL_WINDOWEVENT_FOCUS_LOST:
- if (qemu_console_is_graphic(scon->dcl.con)) {
- win32_kbd_set_window(NULL);
- }
if (gui_grab && !gui_fullscreen) {
sdl_grab_end(scon);
}
@@ -870,10 +847,7 @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o)
#ifdef SDL_HINT_VIDEO_X11_NET_WM_BYPASS_COMPOSITOR /* only available since SDL 2.0.8 */
SDL_SetHint(SDL_HINT_VIDEO_X11_NET_WM_BYPASS_COMPOSITOR, "0");
#endif
-#ifndef CONFIG_WIN32
- /* QEMU uses its own low level keyboard hook procedure on Windows */
SDL_SetHint(SDL_HINT_GRAB_KEYBOARD, "1");
-#endif
#ifdef SDL_HINT_ALLOW_ALT_TAB_WHILE_GRABBED
SDL_SetHint(SDL_HINT_ALLOW_ALT_TAB_WHILE_GRABBED, "0");
#endif
diff --git a/ui/meson.build b/ui/meson.build
index 28c7381dd10..35fb04cadf3 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -120,10 +120,6 @@ if gtk.found()
endif
if sdl.found()
- if host_os == 'windows'
- system_ss.add(files('win32-kbd-hook.c'))
- endif
-
sdl_ss = ss.source_set()
sdl_ss.add(sdl, sdl_image, pixman, glib, files(
'sdl2-2d.c',
--
2.48.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PULL 00/27] rust, Windows patches for 2025-02-13
2025-02-13 16:00 [PULL 00/27] rust, Windows patches for 2025-02-13 Paolo Bonzini
` (26 preceding siblings ...)
2025-02-13 16:00 ` [PULL 27/27] ui/sdl2: reenable the SDL2 Windows keyboard hook procedure Paolo Bonzini
@ 2025-02-14 16:58 ` Stefan Hajnoczi
27 siblings, 0 replies; 33+ messages in thread
From: Stefan Hajnoczi @ 2025-02-14 16:58 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 116 bytes --]
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/10.0 for any user-visible changes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread