From cb7ada5409f171dae364f206a7fe3ff30fcba7cb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 12 Nov 2024 11:52:23 +0100 Subject: rust: allow using build-root bindings.rs from cargo Right now, using cargo with QEMU requires copying by hand the bindings.rs to the source tree. Instead, we can use an include file to escape the cage of cargo's mandated source directory structure. By running cargo within meson's "devenv" and adding a MESON_BUILD_ROOT environment variable, it is easy for build.rs to find the file. However, the file must be symlinked into cargo's output directory for rust-analyzer to find it. Suggested-by: Junjie Mao Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/bindings.rs | 29 +++++++++++++++++++++++++++++ rust/qemu-api/src/lib.rs | 22 ---------------------- 2 files changed, 29 insertions(+), 22 deletions(-) create mode 100644 rust/qemu-api/src/bindings.rs (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs new file mode 100644 index 0000000..0b76ec5 --- /dev/null +++ b/rust/qemu-api/src/bindings.rs @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#![allow( + dead_code, + improper_ctypes_definitions, + improper_ctypes, + non_camel_case_types, + non_snake_case, + non_upper_case_globals, + unsafe_op_in_unsafe_fn, + clippy::missing_const_for_fn, + clippy::too_many_arguments, + clippy::approx_constant, + clippy::use_self, + clippy::useless_transmute, + clippy::missing_safety_doc +)] + +#[cfg(MESON)] +include!("bindings.inc.rs"); + +#[cfg(not(MESON))] +include!(concat!(env!("OUT_DIR"), "/bindings.inc.rs")); + +unsafe impl Send for Property {} +unsafe impl Sync for Property {} +unsafe impl Sync for TypeInfo {} +unsafe impl Sync for VMStateDescription {} +unsafe impl Sync for VMStateField {} +unsafe impl Sync for VMStateInfo {} diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index aa8d16e..440aff3 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -4,31 +4,9 @@ #![cfg_attr(not(MESON), doc = include_str!("../README.md"))] -#[allow( - dead_code, - improper_ctypes_definitions, - improper_ctypes, - non_camel_case_types, - non_snake_case, - non_upper_case_globals, - unsafe_op_in_unsafe_fn, - clippy::missing_const_for_fn, - clippy::too_many_arguments, - clippy::approx_constant, - clippy::use_self, - clippy::useless_transmute, - clippy::missing_safety_doc, -)] #[rustfmt::skip] pub mod bindings; -unsafe impl Send for bindings::Property {} -unsafe impl Sync for bindings::Property {} -unsafe impl Sync for bindings::TypeInfo {} -unsafe impl Sync for bindings::VMStateDescription {} -unsafe impl Sync for bindings::VMStateField {} -unsafe impl Sync for bindings::VMStateInfo {} - pub mod c_str; pub mod definitions; pub mod device_class; -- cgit v1.1 From 2f9eec8f72673f97766eba1682a75f06f16302cb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 6 Nov 2024 11:42:00 +0100 Subject: rust: build: establish a baseline of lints across all crates Many lints that default to allow can be helpful in detecting bugs or keeping the code style homogeneous. Add them liberally, though perhaps not as liberally as in hw/char/pl011/src/lib.rs. In particular, enabling entire groups can be problematic because of bitrot when new links are added in the future. For Clippy, this is actually a feature that is only present in Cargo 1.74.0 but, since we are not using Cargo to *build* QEMU, only developers will need a new-enough cargo and only to run tools such as clippy. The requirement does not apply to distros that are building QEMU. Reviewed-by: Junjie Mao Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/bindings.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs index 0b76ec5..8a9b821 100644 --- a/rust/qemu-api/src/bindings.rs +++ b/rust/qemu-api/src/bindings.rs @@ -7,10 +7,10 @@ non_snake_case, non_upper_case_globals, unsafe_op_in_unsafe_fn, + clippy::pedantic, + clippy::restriction, + clippy::style, clippy::missing_const_for_fn, - clippy::too_many_arguments, - clippy::approx_constant, - clippy::use_self, clippy::useless_transmute, clippy::missing_safety_doc )] -- cgit v1.1 From 8a88b55f698ad660a11d24925cedcff106053ff4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 5 Nov 2024 18:44:56 +0100 Subject: rust: fix doc test syntax Allow "cargo test --doc" to pass. Reviewed-by: Junjie Mao Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/zeroable.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs index 13cdb2c..6125aee 100644 --- a/rust/qemu-api/src/zeroable.rs +++ b/rust/qemu-api/src/zeroable.rs @@ -7,9 +7,9 @@ use std::ptr; /// behavior. This trait in principle could be implemented as just: /// /// ``` -/// const ZERO: Self = unsafe { -/// ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() -/// }, +/// pub unsafe trait Zeroable: Default { +/// const ZERO: Self = unsafe { ::core::mem::MaybeUninit::::zeroed().assume_init() }; +/// } /// ``` /// /// The need for a manual implementation is only because `zeroed()` cannot -- cgit v1.1 From a3057c52f45c280bffc22980f535799a12500d66 Mon Sep 17 00:00:00 2001 From: Junjie Mao Date: Thu, 17 Oct 2024 22:32:44 +0800 Subject: rust/qemu-api: Fix fragment-specifiers in define_property macro For the matcher of macro, "expr" is used for expressions, while "ident" is used for variable/function names, and "ty" matches types. In define_property macro, $field is a member name of type $state, so it should be defined as "ident", though offset_of! doesn't complain about this. $type is the type of $field, since it is not used in the macro, so that no type mismatch error is triggered either. Fix fragment-specifiers of $field and $type. Signed-off-by: Junjie Mao Co-developed-by: Zhao Liu Signed-off-by: Zhao Liu Link: https://lore.kernel.org/r/20241017143245.1248589-2-zhao1.liu@intel.com Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/device_class.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index 0ba798d..922bbce 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -27,7 +27,7 @@ macro_rules! device_class_init { #[macro_export] macro_rules! define_property { - ($name:expr, $state:ty, $field:ident, $prop:expr, $type:expr, default = $defval:expr$(,)*) => { + ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, default = $defval:expr$(,)*) => { $crate::bindings::Property { // use associated function syntax for type checking name: ::std::ffi::CStr::as_ptr($name), @@ -38,7 +38,7 @@ macro_rules! define_property { ..$crate::zeroable::Zeroable::ZERO } }; - ($name:expr, $state:ty, $field:ident, $prop:expr, $type:expr$(,)*) => { + ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty$(,)*) => { $crate::bindings::Property { // use associated function syntax for type checking name: ::std::ffi::CStr::as_ptr($name), -- cgit v1.1 From 8e194c0ea5cdbae05b77125a582f9927678121ee Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 15 Nov 2024 13:26:58 +0100 Subject: rust: cell: add BQL-enforcing Cell variant QEMU objects usually have their pointer shared with the "outside world" very early in their lifetime, for example when they create their MemoryRegions. Because at this point it is not valid anymore to create a &mut reference to the device, individual parts of the device struct must be made mutable in a controlled manner. QEMU's Big Lock (BQL) effectively turns multi-threaded code into single-threaded code while device code runs, as long as the BQL is not released while the device is borrowed (because C code could sneak in and mutate the device). We can then introduce custom interior mutability primitives that are semantically similar to the standard library's (single-threaded) Cell and RefCell, but account for QEMU's threading model. Accessing the "BqlCell" or borrowing the "BqlRefCell" requires proving that the BQL is held, and attempting to access without the BQL is a runtime panic, similar to RefCell's already-borrowed panic. With respect to naming I also considered omitting the "Bql" prefix or moving it to the module, e.g. qemu_api::bql::{Cell, RefCell}. However, this could easily lead to mistakes and confusion; for example rustc could suggest the wrong import, leading to subtle bugs. As a start introduce the an equivalent of Cell. Almost all of the code was taken from Rust's standard library, while removing unstable features and probably-unnecessary functionality that constitute a large of the original code. A lot of what's left is documentation, as well as unit tests in the form of doctests. These are not yet integrated in "make check" but can be run with "cargo test --doc". Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/cell.rs | 298 ++++++++++++++++++++++++++++++++++++++++++++++ rust/qemu-api/src/lib.rs | 1 + 2 files changed, 299 insertions(+) create mode 100644 rust/qemu-api/src/cell.rs (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/cell.rs b/rust/qemu-api/src/cell.rs new file mode 100644 index 0000000..2e4ea8d --- /dev/null +++ b/rust/qemu-api/src/cell.rs @@ -0,0 +1,298 @@ +// SPDX-License-Identifier: MIT +// +// This file is based on library/core/src/cell.rs from +// Rust 1.82.0. +// +// Permission is hereby granted, free of charge, to any +// person obtaining a copy of this software and associated +// documentation files (the "Software"), to deal in the +// Software without restriction, including without +// limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of +// the Software, and to permit persons to whom the Software +// is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice +// shall be included in all copies or substantial portions +// of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF +// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED +// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A +// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR +// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +//! BQL-protected mutable containers. +//! +//! Rust memory safety is based on this rule: Given an object `T`, it is only +//! possible to have one of the following: +//! +//! - Having several immutable references (`&T`) to the object (also known as +//! **aliasing**). +//! - Having one mutable reference (`&mut T`) to the object (also known as +//! **mutability**). +//! +//! This is enforced by the Rust compiler. However, there are situations where +//! this rule is not flexible enough. Sometimes it is required to have multiple +//! references to an object and yet mutate it. In particular, QEMU objects +//! usually have their pointer shared with the "outside world very early in +//! their lifetime", for example when they create their +//! [`MemoryRegion`s](crate::bindings::MemoryRegion). Therefore, individual +//! parts of a device must be made mutable in a controlled manner through the +//! use of cell types. +//! +//! This module provides a way to do so via the Big QEMU Lock. While +//! [`BqlCell`] is essentially the same single-threaded primitive that is +//! available in `std::cell`, the BQL allows it to be used from a multi-threaded +//! context and to share references across threads, while maintaining Rust's +//! safety guarantees. For this reason, unlike its `std::cell` counterpart, +//! `BqlCell` implements the `Sync` trait. +//! +//! BQL checks are performed in debug builds but can be optimized away in +//! release builds, providing runtime safety during development with no overhead +//! in production. +//! +//! Warning: While `BqlCell` is similar to its `std::cell` counterpart, the two +//! are not interchangeable. Using `std::cell` types in QEMU device +//! implementations is usually incorrect and can lead to thread-safety issues. +//! +//! ## `BqlCell` +//! +//! [`BqlCell`] implements interior mutability by moving values in and out of +//! the cell. That is, an `&mut T` to the inner value can never be obtained as +//! long as the cell is shared. The value itself cannot be directly obtained +//! without copying it, cloning it, or replacing it with something else. This +//! type provides the following methods, all of which can be called only while +//! the BQL is held: +//! +//! - For types that implement [`Copy`], the [`get`](BqlCell::get) method +//! retrieves the current interior value by duplicating it. +//! - For types that implement [`Default`], the [`take`](BqlCell::take) method +//! replaces the current interior value with [`Default::default()`] and +//! returns the replaced value. +//! - All types have: +//! - [`replace`](BqlCell::replace): replaces the current interior value and +//! returns the replaced value. +//! - [`set`](BqlCell::set): this method replaces the interior value, +//! dropping the replaced value. + +use std::{cell::UnsafeCell, cmp::Ordering, fmt, mem}; + +use crate::bindings; + +// TODO: When building doctests do not include the actual BQL, because cargo +// does not know how to link them to libqemuutil. This can be fixed by +// running rustdoc from "meson test" instead of relying on cargo. +pub fn bql_locked() -> bool { + // SAFETY: the function does nothing but return a thread-local bool + !cfg!(MESON) || unsafe { bindings::bql_locked() } +} + +/// A mutable memory location that is protected by the Big QEMU Lock. +/// +/// # Memory layout +/// +/// `BqlCell` has the same in-memory representation as its inner type `T`. +#[repr(transparent)] +pub struct BqlCell { + value: UnsafeCell, +} + +// SAFETY: Same as for std::sync::Mutex. In the end this *is* a Mutex, +// except it is stored out-of-line +unsafe impl Send for BqlCell {} +unsafe impl Sync for BqlCell {} + +impl Clone for BqlCell { + #[inline] + fn clone(&self) -> BqlCell { + BqlCell::new(self.get()) + } +} + +impl Default for BqlCell { + /// Creates a `BqlCell`, with the `Default` value for T. + #[inline] + fn default() -> BqlCell { + BqlCell::new(Default::default()) + } +} + +impl PartialEq for BqlCell { + #[inline] + fn eq(&self, other: &BqlCell) -> bool { + self.get() == other.get() + } +} + +impl Eq for BqlCell {} + +impl PartialOrd for BqlCell { + #[inline] + fn partial_cmp(&self, other: &BqlCell) -> Option { + self.get().partial_cmp(&other.get()) + } +} + +impl Ord for BqlCell { + #[inline] + fn cmp(&self, other: &BqlCell) -> Ordering { + self.get().cmp(&other.get()) + } +} + +impl From for BqlCell { + /// Creates a new `BqlCell` containing the given value. + fn from(t: T) -> BqlCell { + BqlCell::new(t) + } +} + +impl fmt::Debug for BqlCell { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.get().fmt(f) + } +} + +impl fmt::Display for BqlCell { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.get().fmt(f) + } +} + +impl BqlCell { + /// Creates a new `BqlCell` containing the given value. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// ``` + #[inline] + pub const fn new(value: T) -> BqlCell { + BqlCell { + value: UnsafeCell::new(value), + } + } + + /// Sets the contained value. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// + /// c.set(10); + /// ``` + #[inline] + pub fn set(&self, val: T) { + self.replace(val); + } + + /// Replaces the contained value with `val`, and returns the old contained + /// value. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let cell = BqlCell::new(5); + /// assert_eq!(cell.get(), 5); + /// assert_eq!(cell.replace(10), 5); + /// assert_eq!(cell.get(), 10); + /// ``` + #[inline] + pub fn replace(&self, val: T) -> T { + assert!(bql_locked()); + // SAFETY: This can cause data races if called from multiple threads, + // but it won't happen as long as C code accesses the value + // under BQL protection only. + mem::replace(unsafe { &mut *self.value.get() }, val) + } + + /// Unwraps the value, consuming the cell. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// let five = c.into_inner(); + /// + /// assert_eq!(five, 5); + /// ``` + pub fn into_inner(self) -> T { + assert!(bql_locked()); + self.value.into_inner() + } +} + +impl BqlCell { + /// Returns a copy of the contained value. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// + /// let five = c.get(); + /// ``` + #[inline] + pub fn get(&self) -> T { + assert!(bql_locked()); + // SAFETY: This can cause data races if called from multiple threads, + // but it won't happen as long as C code accesses the value + // under BQL protection only. + unsafe { *self.value.get() } + } +} + +impl BqlCell { + /// Returns a raw pointer to the underlying data in this cell. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// + /// let ptr = c.as_ptr(); + /// ``` + #[inline] + pub const fn as_ptr(&self) -> *mut T { + self.value.get() + } +} + +impl BqlCell { + /// Takes the value of the cell, leaving `Default::default()` in its place. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// let five = c.take(); + /// + /// assert_eq!(five, 5); + /// assert_eq!(c.into_inner(), 0); + /// ``` + pub fn take(&self) -> T { + self.replace(Default::default()) + } +} diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index 440aff3..b04d110 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -8,6 +8,7 @@ pub mod bindings; pub mod c_str; +pub mod cell; pub mod definitions; pub mod device_class; pub mod offset_of; -- cgit v1.1 From c596199f639cdf4cc021c9bc076e4a1e64e9d50f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 15 Nov 2024 12:20:35 +0100 Subject: rust: cell: add BQL-enforcing RefCell variant Similar to the existing BqlCell, introduce a custom interior mutability primitive that resembles RefCell but accounts for QEMU's threading model. Borrowing the RefCell requires proving that the BQL is held, and attempting to access without the BQL is a runtime panic. Almost all of the code was taken from Rust's standard library, while removing unstable features and probably-unnecessary functionality that amounts to 60% of the original code. A lot of what's left is documentation, as well as unit tests in the form of doctests. These are not yet integrated in "make check" but can be run with "cargo test --doc". Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/cell.rs | 544 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 534 insertions(+), 10 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/cell.rs b/rust/qemu-api/src/cell.rs index 2e4ea8d..28349de 100644 --- a/rust/qemu-api/src/cell.rs +++ b/rust/qemu-api/src/cell.rs @@ -46,20 +46,30 @@ //! parts of a device must be made mutable in a controlled manner through the //! use of cell types. //! -//! This module provides a way to do so via the Big QEMU Lock. While -//! [`BqlCell`] is essentially the same single-threaded primitive that is -//! available in `std::cell`, the BQL allows it to be used from a multi-threaded -//! context and to share references across threads, while maintaining Rust's -//! safety guarantees. For this reason, unlike its `std::cell` counterpart, -//! `BqlCell` implements the `Sync` trait. +//! [`BqlCell`] and [`BqlRefCell`] allow doing this via the Big QEMU Lock. +//! While they are essentially the same single-threaded primitives that are +//! available in `std::cell`, the BQL allows them to be used from a +//! multi-threaded context and to share references across threads, while +//! maintaining Rust's safety guarantees. For this reason, unlike +//! their `std::cell` counterparts, `BqlCell` and `BqlRefCell` implement the +//! `Sync` trait. //! //! BQL checks are performed in debug builds but can be optimized away in //! release builds, providing runtime safety during development with no overhead //! in production. //! -//! Warning: While `BqlCell` is similar to its `std::cell` counterpart, the two -//! are not interchangeable. Using `std::cell` types in QEMU device -//! implementations is usually incorrect and can lead to thread-safety issues. +//! The two provide different ways of handling interior mutability. +//! `BqlRefCell` is best suited for data that is primarily accessed by the +//! device's own methods, where multiple reads and writes can be grouped within +//! a single borrow and a mutable reference can be passed around. Instead, +//! [`BqlCell`] is a better choice when sharing small pieces of data with +//! external code (especially C code), because it provides simple get/set +//! operations that can be used one at a time. +//! +//! Warning: While `BqlCell` and `BqlRefCell` are similar to their `std::cell` +//! counterparts, they are not interchangeable. Using `std::cell` types in +//! QEMU device implementations is usually incorrect and can lead to +//! thread-safety issues. //! //! ## `BqlCell` //! @@ -80,8 +90,37 @@ //! returns the replaced value. //! - [`set`](BqlCell::set): this method replaces the interior value, //! dropping the replaced value. +//! +//! ## `BqlRefCell` +//! +//! [`BqlRefCell`] uses Rust's lifetimes to implement "dynamic borrowing", a +//! process whereby one can claim temporary, exclusive, mutable access to the +//! inner value: +//! +//! ```ignore +//! fn clear_interrupts(&self, val: u32) { +//! // A mutable borrow gives read-write access to the registers +//! let mut regs = self.registers.borrow_mut(); +//! let old = regs.interrupt_status(); +//! regs.update_interrupt_status(old & !val); +//! } +//! ``` +//! +//! Borrows for `BqlRefCell`s are tracked at _runtime_, unlike Rust's native +//! reference types which are entirely tracked statically, at compile time. +//! Multiple immutable borrows are allowed via [`borrow`](BqlRefCell::borrow), +//! or a single mutable borrow via [`borrow_mut`](BqlRefCell::borrow_mut). The +//! thread will panic if these rules are violated or if the BQL is not held. -use std::{cell::UnsafeCell, cmp::Ordering, fmt, mem}; +use std::{ + cell::{Cell, UnsafeCell}, + cmp::Ordering, + fmt, + marker::PhantomData, + mem, + ops::{Deref, DerefMut}, + ptr::NonNull, +}; use crate::bindings; @@ -93,6 +132,15 @@ pub fn bql_locked() -> bool { !cfg!(MESON) || unsafe { bindings::bql_locked() } } +fn bql_block_unlock(increase: bool) { + if cfg!(MESON) { + // SAFETY: this only adjusts a counter + unsafe { + bindings::bql_block_unlock(increase); + } + } +} + /// A mutable memory location that is protected by the Big QEMU Lock. /// /// # Memory layout @@ -296,3 +344,479 @@ impl BqlCell { self.replace(Default::default()) } } + +/// A mutable memory location with dynamically checked borrow rules, +/// protected by the Big QEMU Lock. +/// +/// See the [module-level documentation](self) for more. +/// +/// # Memory layout +/// +/// `BqlRefCell` starts with the same in-memory representation as its +/// inner type `T`. +#[repr(C)] +pub struct BqlRefCell { + // It is important that this is the first field (which is not the case + // for std::cell::BqlRefCell), so that we can use offset_of! on it. + // UnsafeCell and repr(C) both prevent usage of niches. + value: UnsafeCell, + borrow: Cell, + // Stores the location of the earliest currently active borrow. + // This gets updated whenever we go from having zero borrows + // to having a single borrow. When a borrow occurs, this gets included + // in the panic message + #[cfg(feature = "debug_cell")] + borrowed_at: Cell>>, +} + +// Positive values represent the number of `BqlRef` active. Negative values +// represent the number of `BqlRefMut` active. Right now QEMU's implementation +// does not allow to create `BqlRefMut`s that refer to distinct, nonoverlapping +// components of a `BqlRefCell` (e.g., different ranges of a slice). +// +// `BqlRef` and `BqlRefMut` are both two words in size, and so there will likely +// never be enough `BqlRef`s or `BqlRefMut`s in existence to overflow half of +// the `usize` range. Thus, a `BorrowFlag` will probably never overflow or +// underflow. However, this is not a guarantee, as a pathological program could +// repeatedly create and then mem::forget `BqlRef`s or `BqlRefMut`s. Thus, all +// code must explicitly check for overflow and underflow in order to avoid +// unsafety, or at least behave correctly in the event that overflow or +// underflow happens (e.g., see BorrowRef::new). +type BorrowFlag = isize; +const UNUSED: BorrowFlag = 0; + +#[inline(always)] +const fn is_writing(x: BorrowFlag) -> bool { + x < UNUSED +} + +#[inline(always)] +const fn is_reading(x: BorrowFlag) -> bool { + x > UNUSED +} + +impl BqlRefCell { + /// Creates a new `BqlRefCell` containing `value`. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlRefCell; + /// + /// let c = BqlRefCell::new(5); + /// ``` + #[inline] + pub const fn new(value: T) -> BqlRefCell { + BqlRefCell { + value: UnsafeCell::new(value), + borrow: Cell::new(UNUSED), + #[cfg(feature = "debug_cell")] + borrowed_at: Cell::new(None), + } + } +} + +// This ensures the panicking code is outlined from `borrow_mut` for +// `BqlRefCell`. +#[inline(never)] +#[cold] +#[cfg(feature = "debug_cell")] +fn panic_already_borrowed(source: &Cell>>) -> ! { + // If a borrow occurred, then we must already have an outstanding borrow, + // so `borrowed_at` will be `Some` + panic!("already borrowed at {:?}", source.take().unwrap()) +} + +#[inline(never)] +#[cold] +#[cfg(not(feature = "debug_cell"))] +fn panic_already_borrowed() -> ! { + panic!("already borrowed") +} + +impl BqlRefCell { + #[inline] + #[allow(clippy::unused_self)] + fn panic_already_borrowed(&self) -> ! { + #[cfg(feature = "debug_cell")] + { + panic_already_borrowed(&self.borrowed_at) + } + #[cfg(not(feature = "debug_cell"))] + { + panic_already_borrowed() + } + } + + /// Immutably borrows the wrapped value. + /// + /// The borrow lasts until the returned `BqlRef` exits scope. Multiple + /// immutable borrows can be taken out at the same time. + /// + /// # Panics + /// + /// Panics if the value is currently mutably borrowed. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlRefCell; + /// + /// let c = BqlRefCell::new(5); + /// + /// let borrowed_five = c.borrow(); + /// let borrowed_five2 = c.borrow(); + /// ``` + /// + /// An example of panic: + /// + /// ```should_panic + /// use qemu_api::cell::BqlRefCell; + /// + /// let c = BqlRefCell::new(5); + /// + /// let m = c.borrow_mut(); + /// let b = c.borrow(); // this causes a panic + /// ``` + #[inline] + #[track_caller] + pub fn borrow(&self) -> BqlRef<'_, T> { + if let Some(b) = BorrowRef::new(&self.borrow) { + // `borrowed_at` is always the *first* active borrow + if b.borrow.get() == 1 { + #[cfg(feature = "debug_cell")] + self.borrowed_at.set(Some(std::panic::Location::caller())); + } + + bql_block_unlock(true); + + // SAFETY: `BorrowRef` ensures that there is only immutable access + // to the value while borrowed. + let value = unsafe { NonNull::new_unchecked(self.value.get()) }; + BqlRef { value, borrow: b } + } else { + self.panic_already_borrowed() + } + } + + /// Mutably borrows the wrapped value. + /// + /// The borrow lasts until the returned `BqlRefMut` or all `BqlRefMut`s + /// derived from it exit scope. The value cannot be borrowed while this + /// borrow is active. + /// + /// # Panics + /// + /// Panics if the value is currently borrowed. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlRefCell; + /// + /// let c = BqlRefCell::new("hello".to_owned()); + /// + /// *c.borrow_mut() = "bonjour".to_owned(); + /// + /// assert_eq!(&*c.borrow(), "bonjour"); + /// ``` + /// + /// An example of panic: + /// + /// ```should_panic + /// use qemu_api::cell::BqlRefCell; + /// + /// let c = BqlRefCell::new(5); + /// let m = c.borrow(); + /// + /// let b = c.borrow_mut(); // this causes a panic + /// ``` + #[inline] + #[track_caller] + pub fn borrow_mut(&self) -> BqlRefMut<'_, T> { + if let Some(b) = BorrowRefMut::new(&self.borrow) { + #[cfg(feature = "debug_cell")] + { + self.borrowed_at.set(Some(std::panic::Location::caller())); + } + + // SAFETY: this only adjusts a counter + bql_block_unlock(true); + + // SAFETY: `BorrowRefMut` guarantees unique access. + let value = unsafe { NonNull::new_unchecked(self.value.get()) }; + BqlRefMut { + value, + _borrow: b, + marker: PhantomData, + } + } else { + self.panic_already_borrowed() + } + } + + /// Returns a raw pointer to the underlying data in this cell. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlRefCell; + /// + /// let c = BqlRefCell::new(5); + /// + /// let ptr = c.as_ptr(); + /// ``` + #[inline] + pub const fn as_ptr(&self) -> *mut T { + self.value.get() + } +} + +// SAFETY: Same as for std::sync::Mutex. In the end this is a Mutex that is +// stored out-of-line. Even though BqlRefCell includes Cells, they are +// themselves protected by the Big QEMU Lock. Furtheremore, the Big QEMU +// Lock cannot be released while any borrows is active. +unsafe impl Send for BqlRefCell where T: Send {} +unsafe impl Sync for BqlRefCell {} + +impl Clone for BqlRefCell { + /// # Panics + /// + /// Panics if the value is currently mutably borrowed. + #[inline] + #[track_caller] + fn clone(&self) -> BqlRefCell { + BqlRefCell::new(self.borrow().clone()) + } + + /// # Panics + /// + /// Panics if `source` is currently mutably borrowed. + #[inline] + #[track_caller] + fn clone_from(&mut self, source: &Self) { + self.value.get_mut().clone_from(&source.borrow()) + } +} + +impl Default for BqlRefCell { + /// Creates a `BqlRefCell`, with the `Default` value for T. + #[inline] + fn default() -> BqlRefCell { + BqlRefCell::new(Default::default()) + } +} + +impl PartialEq for BqlRefCell { + /// # Panics + /// + /// Panics if the value in either `BqlRefCell` is currently mutably + /// borrowed. + #[inline] + fn eq(&self, other: &BqlRefCell) -> bool { + *self.borrow() == *other.borrow() + } +} + +impl Eq for BqlRefCell {} + +impl PartialOrd for BqlRefCell { + /// # Panics + /// + /// Panics if the value in either `BqlRefCell` is currently mutably + /// borrowed. + #[inline] + fn partial_cmp(&self, other: &BqlRefCell) -> Option { + self.borrow().partial_cmp(&*other.borrow()) + } +} + +impl Ord for BqlRefCell { + /// # Panics + /// + /// Panics if the value in either `BqlRefCell` is currently mutably + /// borrowed. + #[inline] + fn cmp(&self, other: &BqlRefCell) -> Ordering { + self.borrow().cmp(&*other.borrow()) + } +} + +impl From for BqlRefCell { + /// Creates a new `BqlRefCell` containing the given value. + fn from(t: T) -> BqlRefCell { + BqlRefCell::new(t) + } +} + +struct BorrowRef<'b> { + borrow: &'b Cell, +} + +impl<'b> BorrowRef<'b> { + #[inline] + fn new(borrow: &'b Cell) -> Option> { + let b = borrow.get().wrapping_add(1); + if !is_reading(b) { + // Incrementing borrow can result in a non-reading value (<= 0) in these cases: + // 1. It was < 0, i.e. there are writing borrows, so we can't allow a read + // borrow due to Rust's reference aliasing rules + // 2. It was isize::MAX (the max amount of reading borrows) and it overflowed + // into isize::MIN (the max amount of writing borrows) so we can't allow an + // additional read borrow because isize can't represent so many read borrows + // (this can only happen if you mem::forget more than a small constant amount + // of `BqlRef`s, which is not good practice) + None + } else { + // Incrementing borrow can result in a reading value (> 0) in these cases: + // 1. It was = 0, i.e. it wasn't borrowed, and we are taking the first read + // borrow + // 2. It was > 0 and < isize::MAX, i.e. there were read borrows, and isize is + // large enough to represent having one more read borrow + borrow.set(b); + Some(BorrowRef { borrow }) + } + } +} + +impl Drop for BorrowRef<'_> { + #[inline] + fn drop(&mut self) { + let borrow = self.borrow.get(); + debug_assert!(is_reading(borrow)); + self.borrow.set(borrow - 1); + bql_block_unlock(false) + } +} + +impl Clone for BorrowRef<'_> { + #[inline] + fn clone(&self) -> Self { + BorrowRef::new(self.borrow).unwrap() + } +} + +/// Wraps a borrowed reference to a value in a `BqlRefCell` box. +/// A wrapper type for an immutably borrowed value from a `BqlRefCell`. +/// +/// See the [module-level documentation](self) for more. +pub struct BqlRef<'b, T: 'b> { + // NB: we use a pointer instead of `&'b T` to avoid `noalias` violations, because a + // `BqlRef` argument doesn't hold immutability for its whole scope, only until it drops. + // `NonNull` is also covariant over `T`, just like we would have with `&T`. + value: NonNull, + borrow: BorrowRef<'b>, +} + +impl Deref for BqlRef<'_, T> { + type Target = T; + + #[inline] + fn deref(&self) -> &T { + // SAFETY: the value is accessible as long as we hold our borrow. + unsafe { self.value.as_ref() } + } +} + +impl<'b, T> BqlRef<'b, T> { + /// Copies a `BqlRef`. + /// + /// The `BqlRefCell` is already immutably borrowed, so this cannot fail. + /// + /// This is an associated function that needs to be used as + /// `BqlRef::clone(...)`. A `Clone` implementation or a method would + /// interfere with the widespread use of `r.borrow().clone()` to clone + /// the contents of a `BqlRefCell`. + #[must_use] + #[inline] + #[allow(clippy::should_implement_trait)] + pub fn clone(orig: &BqlRef<'b, T>) -> BqlRef<'b, T> { + BqlRef { + value: orig.value, + borrow: orig.borrow.clone(), + } + } +} + +impl fmt::Debug for BqlRef<'_, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + (**self).fmt(f) + } +} + +impl fmt::Display for BqlRef<'_, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + (**self).fmt(f) + } +} + +struct BorrowRefMut<'b> { + borrow: &'b Cell, +} + +impl<'b> BorrowRefMut<'b> { + #[inline] + fn new(borrow: &'b Cell) -> Option> { + // There must currently be no existing references when borrow_mut() is + // called, so we explicitly only allow going from UNUSED to UNUSED - 1. + match borrow.get() { + UNUSED => { + borrow.set(UNUSED - 1); + Some(BorrowRefMut { borrow }) + } + _ => None, + } + } +} + +impl Drop for BorrowRefMut<'_> { + #[inline] + fn drop(&mut self) { + let borrow = self.borrow.get(); + debug_assert!(is_writing(borrow)); + self.borrow.set(borrow + 1); + bql_block_unlock(false) + } +} + +/// A wrapper type for a mutably borrowed value from a `BqlRefCell`. +/// +/// See the [module-level documentation](self) for more. +pub struct BqlRefMut<'b, T: 'b> { + // NB: we use a pointer instead of `&'b mut T` to avoid `noalias` violations, because a + // `BqlRefMut` argument doesn't hold exclusivity for its whole scope, only until it drops. + value: NonNull, + _borrow: BorrowRefMut<'b>, + // `NonNull` is covariant over `T`, so we need to reintroduce invariance. + marker: PhantomData<&'b mut T>, +} + +impl Deref for BqlRefMut<'_, T> { + type Target = T; + + #[inline] + fn deref(&self) -> &T { + // SAFETY: the value is accessible as long as we hold our borrow. + unsafe { self.value.as_ref() } + } +} + +impl DerefMut for BqlRefMut<'_, T> { + #[inline] + fn deref_mut(&mut self) -> &mut T { + // SAFETY: the value is accessible as long as we hold our borrow. + unsafe { self.value.as_mut() } + } +} + +impl fmt::Debug for BqlRefMut<'_, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + (**self).fmt(f) + } +} + +impl fmt::Display for BqlRefMut<'_, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + (**self).fmt(f) + } +} -- cgit v1.1 From 28d0ad3d425a956a1257256c46ef44581f6678c5 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 5 Dec 2024 13:42:33 +0100 Subject: rust: define prelude Add a module that will contain frequently used traits and occasionally structs. They can be included quickly with "use qemu_api::prelude::*". Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/lib.rs | 5 +++++ rust/qemu-api/src/prelude.rs | 6 ++++++ 2 files changed, 11 insertions(+) create mode 100644 rust/qemu-api/src/prelude.rs (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index b04d110..e5956cd 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -7,6 +7,11 @@ #[rustfmt::skip] pub mod bindings; +// preserve one-item-per-"use" syntax, it is clearer +// for prelude-like modules +#[rustfmt::skip] +pub mod prelude; + pub mod c_str; pub mod cell; pub mod definitions; diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs new file mode 100644 index 0000000..dfaddbd --- /dev/null +++ b/rust/qemu-api/src/prelude.rs @@ -0,0 +1,6 @@ +// Copyright 2024 Red Hat, Inc. +// Author(s): Paolo Bonzini +// SPDX-License-Identifier: GPL-2.0-or-later + +pub use crate::cell::BqlCell; +pub use crate::cell::BqlRefCell; -- cgit v1.1 From 4ed4da164c957a4475b9d075206f33113a69abda Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 31 Oct 2024 11:29:42 +0100 Subject: rust: add bindings for interrupt sources The InterruptSource bindings let us call qemu_set_irq() and sysbus_init_irq() as safe code. Interrupt sources, qemu_irq in C code, are pointers to IRQState objects. They are QOM link properties and can be written to outside the control of the device (i.e. from a shared reference); therefore they must be interior-mutable in Rust. Since thread-safety is provided by the BQL, what we want here is the newly-introduced BqlCell. A pointer to the contents of the BqlCell (an IRQState**, or equivalently qemu_irq*) is then passed to the C sysbus_init_irq function. Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/irq.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++ rust/qemu-api/src/lib.rs | 2 + rust/qemu-api/src/sysbus.rs | 27 ++++++++++++++ 3 files changed, 120 insertions(+) create mode 100644 rust/qemu-api/src/irq.rs create mode 100644 rust/qemu-api/src/sysbus.rs (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/irq.rs b/rust/qemu-api/src/irq.rs new file mode 100644 index 0000000..6258141 --- /dev/null +++ b/rust/qemu-api/src/irq.rs @@ -0,0 +1,91 @@ +// Copyright 2024 Red Hat, Inc. +// Author(s): Paolo Bonzini +// SPDX-License-Identifier: GPL-2.0-or-later + +//! Bindings for interrupt sources + +use core::ptr; +use std::{marker::PhantomData, os::raw::c_int}; + +use crate::{ + bindings::{qemu_set_irq, IRQState}, + prelude::*, +}; + +/// Interrupt sources are used by devices to pass changes to a value (typically +/// a boolean). The interrupt sink is usually an interrupt controller or +/// GPIO controller. +/// +/// As far as devices are concerned, interrupt sources are always active-high: +/// for example, `InterruptSource`'s [`raise`](InterruptSource::raise) +/// 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. +/// +/// 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 +/// [`SysBusDevice::init_irq`](crate::sysbus::SysBusDevice::init_irq), and +/// initially leaves the pointer to a NULL value, representing an unconnected +/// interrupt. To connect it, whoever creates the device fills the pointer with +/// the sink's `IRQState *`, for example using `sysbus_connect_irq`. Because +/// devices are generally shared objects, interrupt sources are an example of +/// the interior mutability pattern. +/// +/// Interrupt sources can only be triggered under the Big QEMU Lock; `BqlCell` +/// allows access from whatever thread has it. +#[derive(Debug)] +#[repr(transparent)] +pub struct InterruptSource +where + c_int: From, +{ + cell: BqlCell<*mut IRQState>, + _marker: PhantomData, +} + +impl InterruptSource { + /// Send a low (`false`) value to the interrupt sink. + pub fn lower(&self) { + self.set(false); + } + + /// Send a high-low pulse to the interrupt sink. + pub fn pulse(&self) { + self.set(true); + self.set(false); + } + + /// Send a high (`true`) value to the interrupt sink. + pub fn raise(&self) { + self.set(true); + } +} + +impl InterruptSource +where + c_int: From, +{ + /// Send `level` to the interrupt sink. + pub fn set(&self, level: T) { + let ptr = self.cell.get(); + // SAFETY: the pointer is retrieved under the BQL and remains valid + // until the BQL is released, which is after qemu_set_irq() is entered. + unsafe { + qemu_set_irq(ptr, level.into()); + } + } + + pub(crate) const fn as_ptr(&self) -> *mut *mut IRQState { + self.cell.as_ptr() + } +} + +impl Default for InterruptSource { + fn default() -> Self { + InterruptSource { + cell: BqlCell::new(ptr::null_mut()), + _marker: PhantomData, + } + } +} diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index e5956cd..0efbef4 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -16,7 +16,9 @@ pub mod c_str; pub mod cell; pub mod definitions; pub mod device_class; +pub mod irq; pub mod offset_of; +pub mod sysbus; pub mod vmstate; pub mod zeroable; diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs new file mode 100644 index 0000000..4e192c7 --- /dev/null +++ b/rust/qemu-api/src/sysbus.rs @@ -0,0 +1,27 @@ +// Copyright 2024 Red Hat, Inc. +// Author(s): Paolo Bonzini +// SPDX-License-Identifier: GPL-2.0-or-later + +use std::ptr::addr_of; + +pub use bindings::{SysBusDevice, SysBusDeviceClass}; + +use crate::{bindings, cell::bql_locked, irq::InterruptSource}; + +impl SysBusDevice { + /// Return `self` cast to a mutable pointer, for use in calls to C code. + const fn as_mut_ptr(&self) -> *mut SysBusDevice { + addr_of!(*self) as *mut _ + } + + /// Expose an interrupt source outside the device as a qdev GPIO output. + /// Note that the ordering of calls to `init_irq` is important, since + /// whoever creates the sysbus device will refer to the interrupts with + /// a number that corresponds to the order of calls to `init_irq`. + pub fn init_irq(&self, irq: &InterruptSource) { + assert!(bql_locked()); + unsafe { + bindings::sysbus_init_irq(self.as_mut_ptr(), irq.as_ptr()); + } + } +} -- cgit v1.1 From ab870fa106e0e3f48db2c5ef0507d107b1b41a21 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 5 Dec 2024 14:29:13 +0100 Subject: rust: add a bit operation module The bindgen supports `static inline` function binding since v0.64.0 as an experimental feature (`--wrap-static-fns`), and stabilizes it after v0.70.0. But the oldest version of bindgen supported by QEMU is v0.60.1, so there's no way to generate the binding for deposit64() which is `static inline` (in include/qemu/bitops.h). Instead, implement it by hand in Rust and make it available for all unsigned types through an IntegerExt trait. Since it only involves bit operations, the Rust version of the code is almost identical to the original C version, but it applies to more types than just u64. Signed-off-by: Zhao Liu Co-authored-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/bitops.rs | 119 +++++++++++++++++++++++++++++++++++++++++++ rust/qemu-api/src/lib.rs | 1 + rust/qemu-api/src/prelude.rs | 2 + 3 files changed, 122 insertions(+) create mode 100644 rust/qemu-api/src/bitops.rs (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/bitops.rs b/rust/qemu-api/src/bitops.rs new file mode 100644 index 0000000..023ec1a --- /dev/null +++ b/rust/qemu-api/src/bitops.rs @@ -0,0 +1,119 @@ +// Copyright (C) 2024 Intel Corporation. +// Author(s): Zhao Liu +// SPDX-License-Identifier: GPL-2.0-or-later + +//! This module provides bit operation extensions to integer types. +//! It is usually included via the `qemu_api` prelude. + +use std::ops::{ + Add, AddAssign, BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Div, DivAssign, + Mul, MulAssign, Not, Rem, RemAssign, Shl, ShlAssign, Shr, ShrAssign, +}; + +/// Trait for extensions to integer types +pub trait IntegerExt: + Add + AddAssign + + BitAnd + BitAndAssign + + BitOr + BitOrAssign + + BitXor + BitXorAssign + + Copy + + Div + DivAssign + + Eq + + Mul + MulAssign + + Not + Ord + PartialOrd + + Rem + RemAssign + + Shl + ShlAssign + + Shl + ShlAssign + // add more as needed + Shr + ShrAssign + + Shr + ShrAssign // add more as needed +{ + const BITS: u32; + const MAX: Self; + const MIN: Self; + const ONE: Self; + const ZERO: Self; + + #[inline] + #[must_use] + fn bit(start: u32) -> Self + { + debug_assert!(start < Self::BITS); + + Self::ONE << start + } + + #[inline] + #[must_use] + fn mask(start: u32, length: u32) -> Self + { + /* FIXME: Implement a more elegant check with error handling support? */ + debug_assert!(start < Self::BITS && length > 0 && length <= Self::BITS - start); + + (Self::MAX >> (Self::BITS - length)) << start + } + + #[inline] + #[must_use] + fn deposit(self, start: u32, length: u32, + fieldval: U) -> Self + where Self: From + { + debug_assert!(length <= U::BITS); + + let mask = Self::mask(start, length); + (self & !mask) | ((Self::from(fieldval) << start) & mask) + } + + #[inline] + #[must_use] + fn extract(self, start: u32, length: u32) -> Self + { + let mask = Self::mask(start, length); + (self & mask) >> start + } +} + +macro_rules! impl_num_ext { + ($type:ty) => { + impl IntegerExt for $type { + const BITS: u32 = <$type>::BITS; + const MAX: Self = <$type>::MAX; + const MIN: Self = <$type>::MIN; + const ONE: Self = 1; + const ZERO: Self = 0; + } + }; +} + +impl_num_ext!(u8); +impl_num_ext!(u16); +impl_num_ext!(u32); +impl_num_ext!(u64); + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_deposit() { + assert_eq!(15u32.deposit(8, 8, 1u32), 256 + 15); + assert_eq!(15u32.deposit(8, 1, 255u8), 256 + 15); + } + + #[test] + fn test_extract() { + assert_eq!(15u32.extract(2, 4), 3); + } + + #[test] + fn test_bit() { + assert_eq!(u8::bit(7), 128); + assert_eq!(u32::bit(16), 0x10000); + } + + #[test] + fn test_mask() { + assert_eq!(u8::mask(7, 1), 128); + assert_eq!(u32::mask(8, 8), 0xff00); + } +} diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index 0efbef4..9e007e1 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -12,6 +12,7 @@ pub mod bindings; #[rustfmt::skip] pub mod prelude; +pub mod bitops; pub mod c_str; pub mod cell; pub mod definitions; diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs index dfaddbd..a39e228 100644 --- a/rust/qemu-api/src/prelude.rs +++ b/rust/qemu-api/src/prelude.rs @@ -2,5 +2,7 @@ // Author(s): Paolo Bonzini // SPDX-License-Identifier: GPL-2.0-or-later +pub use crate::bitops::IntegerExt; + pub use crate::cell::BqlCell; pub use crate::cell::BqlRefCell; -- cgit v1.1 From b2a4854508a02fc8a585890e0272c8ae5fbad5c1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 28 Oct 2024 11:28:23 +0100 Subject: rust: qom: add default definitions for ObjectImpl Remove a bunch of duplicate const definitions. Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/definitions.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 2659793..92b3c6f 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -14,10 +14,10 @@ pub trait ObjectImpl { const TYPE_INFO: TypeInfo; const TYPE_NAME: &'static CStr; const PARENT_TYPE_NAME: Option<&'static CStr>; - const ABSTRACT: bool; - const INSTANCE_INIT: Option; - const INSTANCE_POST_INIT: Option; - const INSTANCE_FINALIZE: Option; + const ABSTRACT: bool = false; + const INSTANCE_INIT: Option = None; + const INSTANCE_POST_INIT: Option = None; + const INSTANCE_FINALIZE: Option = None; } pub trait Class { -- cgit v1.1 From 93ea0896eaa97adfcc664fa65b5b70e555a652ff Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 28 Oct 2024 13:05:43 +0100 Subject: rust: qom: rename Class trait to ClassInitImpl While at it, document it. Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/definitions.rs | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 92b3c6f..3291f42 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -20,8 +20,27 @@ pub trait ObjectImpl { const INSTANCE_FINALIZE: Option = None; } -pub trait Class { +/// Trait used to fill in a class struct. +/// +/// Each QOM class that has virtual methods describes them in a +/// _class struct_. Class structs include a parent field corresponding +/// to the vtable of the parent class, all the way up to [`ObjectClass`]. +/// Each QOM type has one such class struct. +/// +/// The Rust implementation of methods will usually come from a trait +/// like [`ObjectImpl`]. +pub trait ClassInitImpl { + /// Function that is called after all parent class initialization + /// has occurred. On entry, the virtual method pointers are set to + /// the default values coming from the parent classes; the function + /// can change them to override virtual methods of a parent class. const CLASS_INIT: Option; + + /// Called on descendent classes after all parent class initialization + /// has occurred, but before the class itself is initialized. This + /// is only useful if a class is not a leaf, and can be used to undo + /// the effects of copying the contents of the parent's class struct + /// to the descendants. const CLASS_BASE_INIT: Option< unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void), >; @@ -82,8 +101,8 @@ macro_rules! type_info { instance_finalize: <$t as $crate::definitions::ObjectImpl>::INSTANCE_FINALIZE, abstract_: <$t as $crate::definitions::ObjectImpl>::ABSTRACT, class_size: ::core::mem::size_of::<<$t as $crate::definitions::ObjectImpl>::Class>(), - class_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::Class>::CLASS_INIT, - class_base_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::Class>::CLASS_BASE_INIT, + class_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_INIT, + class_base_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_BASE_INIT, class_data: ::core::ptr::null_mut(), interfaces: ::core::ptr::null_mut(), }; -- cgit v1.1 From 3701fb22dfd438993e76e158beb97683359d1dd9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 28 Oct 2024 11:47:12 +0100 Subject: rust: qom: convert type_info! macro to an associated const type_info! is only used in the definition of ObjectImpl::TYPE_INFO, and in fact in all of them. Pull type_info!'s definition into the ObjectImpl trait, thus simplifying the external interface of qemu_api::definitions. Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/definitions.rs | 50 ++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 28 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 3291f42..6ecfaf5 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -9,15 +9,34 @@ use std::{ffi::CStr, os::raw::c_void}; use crate::bindings::{Object, ObjectClass, TypeInfo}; /// Trait a type must implement to be registered with QEMU. -pub trait ObjectImpl { - type Class; - const TYPE_INFO: TypeInfo; +pub trait ObjectImpl: Sized { + type Class: ClassInitImpl; const TYPE_NAME: &'static CStr; const PARENT_TYPE_NAME: Option<&'static CStr>; const ABSTRACT: bool = false; const INSTANCE_INIT: Option = None; const INSTANCE_POST_INIT: Option = None; const INSTANCE_FINALIZE: Option = None; + + const TYPE_INFO: TypeInfo = TypeInfo { + name: Self::TYPE_NAME.as_ptr(), + parent: if let Some(pname) = Self::PARENT_TYPE_NAME { + pname.as_ptr() + } else { + core::ptr::null_mut() + }, + instance_size: core::mem::size_of::(), + instance_align: core::mem::align_of::(), + instance_init: Self::INSTANCE_INIT, + instance_post_init: Self::INSTANCE_POST_INIT, + instance_finalize: Self::INSTANCE_FINALIZE, + abstract_: Self::ABSTRACT, + class_size: core::mem::size_of::(), + class_init: ::CLASS_INIT, + class_base_init: ::CLASS_BASE_INIT, + class_data: core::ptr::null_mut(), + interfaces: core::ptr::null_mut(), + }; } /// Trait used to fill in a class struct. @@ -83,28 +102,3 @@ macro_rules! module_init { } }; } - -#[macro_export] -macro_rules! type_info { - ($t:ty) => { - $crate::bindings::TypeInfo { - name: <$t as $crate::definitions::ObjectImpl>::TYPE_NAME.as_ptr(), - parent: if let Some(pname) = <$t as $crate::definitions::ObjectImpl>::PARENT_TYPE_NAME { - pname.as_ptr() - } else { - ::core::ptr::null_mut() - }, - instance_size: ::core::mem::size_of::<$t>(), - instance_align: ::core::mem::align_of::<$t>(), - instance_init: <$t as $crate::definitions::ObjectImpl>::INSTANCE_INIT, - instance_post_init: <$t as $crate::definitions::ObjectImpl>::INSTANCE_POST_INIT, - instance_finalize: <$t as $crate::definitions::ObjectImpl>::INSTANCE_FINALIZE, - abstract_: <$t as $crate::definitions::ObjectImpl>::ABSTRACT, - class_size: ::core::mem::size_of::<<$t as $crate::definitions::ObjectImpl>::Class>(), - class_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_INIT, - class_base_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_BASE_INIT, - class_data: ::core::ptr::null_mut(), - interfaces: ::core::ptr::null_mut(), - }; - } -} -- cgit v1.1 From c6c4f3e0d90990c642523c087482eac3f42566c1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 28 Oct 2024 14:42:23 +0100 Subject: rust: qom: move ClassInitImpl to the instance side Put all traits on the instance struct, which makes it possible to reuse class structs if no new virtual methods or class fields are added. This is almost always the case for devices (because they are leaf classes), which is the primary use case for Rust. This is also simpler: soon we will find the implemented methods without macros, and this removes the need to go from the class struct to the instance struct to find the implementation of the *Impl traits. Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/definitions.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 6ecfaf5..4877126 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -9,8 +9,8 @@ use std::{ffi::CStr, os::raw::c_void}; use crate::bindings::{Object, ObjectClass, TypeInfo}; /// Trait a type must implement to be registered with QEMU. -pub trait ObjectImpl: Sized { - type Class: ClassInitImpl; +pub trait ObjectImpl: ClassInitImpl + Sized { + type Class; const TYPE_NAME: &'static CStr; const PARENT_TYPE_NAME: Option<&'static CStr>; const ABSTRACT: bool = false; @@ -32,8 +32,8 @@ pub trait ObjectImpl: Sized { instance_finalize: Self::INSTANCE_FINALIZE, abstract_: Self::ABSTRACT, class_size: core::mem::size_of::(), - class_init: ::CLASS_INIT, - class_base_init: ::CLASS_BASE_INIT, + class_init: ::CLASS_INIT, + class_base_init: ::CLASS_BASE_INIT, class_data: core::ptr::null_mut(), interfaces: core::ptr::null_mut(), }; -- cgit v1.1 From 8c80c472da6342c5924bc4ea7e87c77ca61477b8 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 28 Oct 2024 10:29:27 +0100 Subject: rust: qdev: move device_class_init! body to generic function, ClassInitImpl implementation to macro Use a trait to access the former parameters to device_class_init!. This allows hiding the details of the class_init implementation behind a generic function and makes higher-level functionality available from qemu_api. The implementation of ClassInitImpl is then the same for all devices and is easily macroized. Later on, we can remove the need to implement ClassInitImpl by hand for all device types, and stop making rust_device_class_init<>() public. While at it, document the members of DeviceImpl. Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/device_class.rs | 87 ++++++++++++++++++++++++++++++++------- 1 file changed, 71 insertions(+), 16 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index 922bbce..f683f94 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -2,25 +2,80 @@ // Author(s): Manos Pitsidianakis // SPDX-License-Identifier: GPL-2.0-or-later -use std::ffi::CStr; +use std::{ffi::CStr, os::raw::c_void}; -use crate::bindings; +use crate::{ + bindings::{self, DeviceClass, DeviceState, Error, ObjectClass, Property, VMStateDescription}, + zeroable::Zeroable, +}; + +/// Trait providing the contents of [`DeviceClass`]. +pub trait DeviceImpl { + /// _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). + /// + /// If not `None`, the parent class's `realize` method is overridden + /// with the function pointed to by `REALIZE`. + const REALIZE: Option = 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 = 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. + fn properties() -> &'static [Property] { + &[Zeroable::ZERO; 1] + } + + /// A `VMStateDescription` providing the migration format for the device + /// Not a `const` because referencing statics in constants is unstable + /// until Rust 1.83.0. + fn vmsd() -> Option<&'static VMStateDescription> { + None + } +} + +/// # Safety +/// +/// We expect the FFI user of this function to pass a valid pointer that +/// can be downcasted to type `DeviceClass`, because `T` implements +/// `DeviceImpl`. +pub unsafe extern "C" fn rust_device_class_init( + klass: *mut ObjectClass, + _: *mut c_void, +) { + let mut dc = ::core::ptr::NonNull::new(klass.cast::()).unwrap(); + unsafe { + let dc = dc.as_mut(); + if let Some(realize_fn) = ::REALIZE { + dc.realize = Some(realize_fn); + } + if let Some(reset_fn) = ::RESET { + bindings::device_class_set_legacy_reset(dc, Some(reset_fn)); + } + if let Some(vmsd) = ::vmsd() { + dc.vmsd = vmsd; + } + bindings::device_class_set_props(dc, ::properties().as_ptr()); + } +} #[macro_export] -macro_rules! device_class_init { - ($func:ident, props => $props:ident, realize_fn => $realize_fn:expr, legacy_reset_fn => $legacy_reset_fn:expr, vmsd => $vmsd:ident$(,)*) => { - pub unsafe extern "C" fn $func( - klass: *mut $crate::bindings::ObjectClass, - _: *mut ::std::os::raw::c_void, - ) { - let mut dc = - ::core::ptr::NonNull::new(klass.cast::<$crate::bindings::DeviceClass>()).unwrap(); - unsafe { - dc.as_mut().realize = $realize_fn; - dc.as_mut().vmsd = &$vmsd; - $crate::bindings::device_class_set_legacy_reset(dc.as_mut(), $legacy_reset_fn); - $crate::bindings::device_class_set_props(dc.as_mut(), $props.as_ptr()); - } +macro_rules! impl_device_class { + ($type:ty) => { + impl $crate::definitions::ClassInitImpl for $type { + const CLASS_INIT: Option< + unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut ::std::os::raw::c_void), + > = Some($crate::device_class::rust_device_class_init::<$type>); + const CLASS_BASE_INIT: Option< + unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut ::std::os::raw::c_void), + > = None; } }; } -- cgit v1.1 From f75fb90ff2af75cd4405fe4c6ba0c0c38a120590 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 12 Nov 2024 17:08:07 +0100 Subject: rust: qdev: move bridge for realize and reset functions out of pl011 Allow the DeviceImpl trait to expose safe Rust functions. rust_device_class_init<> adds thunks around the functions in DeviceImpl. Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/definitions.rs | 2 +- rust/qemu-api/src/device_class.rs | 36 ++++++++++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 7 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 4877126..0467e62 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -47,7 +47,7 @@ pub trait ObjectImpl: ClassInitImpl + Sized { /// Each QOM type has one such class struct. /// /// The Rust implementation of methods will usually come from a trait -/// like [`ObjectImpl`]. +/// like [`ObjectImpl`] or [`DeviceImpl`](crate::device_class::DeviceImpl). pub trait ClassInitImpl { /// Function that is called after all parent class initialization /// has occurred. On entry, the virtual method pointers are set to diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index f683f94..f25904b 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -17,14 +17,14 @@ pub trait DeviceImpl { /// /// If not `None`, the parent class's `realize` method is overridden /// with the function pointed to by `REALIZE`. - const REALIZE: Option = None; + const REALIZE: Option = 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 = None; + const RESET: Option = None; /// An array providing the properties that the user can set on the /// device. Not a `const` because referencing statics in constants @@ -43,6 +43,30 @@ pub trait DeviceImpl { /// # Safety /// +/// This function is only called through the QOM machinery and +/// the `impl_device_class!` macro. +/// 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_realize_fn(dev: *mut DeviceState, _errp: *mut *mut Error) { + assert!(!dev.is_null()); + let state = dev.cast::(); + T::REALIZE.unwrap()(unsafe { &mut *state }); +} + +/// # 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(dev: *mut DeviceState) { + assert!(!dev.is_null()); + let state = dev.cast::(); + T::RESET.unwrap()(unsafe { &mut *state }); +} + +/// # Safety +/// /// We expect the FFI user of this function to pass a valid pointer that /// can be downcasted to type `DeviceClass`, because `T` implements /// `DeviceImpl`. @@ -53,11 +77,11 @@ pub unsafe extern "C" fn rust_device_class_init( let mut dc = ::core::ptr::NonNull::new(klass.cast::()).unwrap(); unsafe { let dc = dc.as_mut(); - if let Some(realize_fn) = ::REALIZE { - dc.realize = Some(realize_fn); + if ::REALIZE.is_some() { + dc.realize = Some(rust_realize_fn::); } - if let Some(reset_fn) = ::RESET { - bindings::device_class_set_legacy_reset(dc, Some(reset_fn)); + if ::RESET.is_some() { + bindings::device_class_set_legacy_reset(dc, Some(rust_reset_fn::)); } if let Some(vmsd) = ::vmsd() { dc.vmsd = vmsd; -- cgit v1.1 From 1f9d52c9388d14c3f5a605543a8ef53dceaad5bb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 28 Oct 2024 10:45:55 +0100 Subject: rust: qom: move bridge for TypeInfo functions out of pl011 Allow the ObjectImpl trait to expose Rust functions that avoid raw pointers (though INSTANCE_INIT for example is still unsafe). ObjectImpl::TYPE_INFO adds thunks around the functions in ObjectImpl. While at it, document `TypeInfo`. Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/definitions.rs | 61 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 4 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 0467e62..f297075 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -8,16 +8,63 @@ use std::{ffi::CStr, os::raw::c_void}; use crate::bindings::{Object, ObjectClass, TypeInfo}; +unsafe extern "C" fn rust_instance_init(obj: *mut Object) { + // SAFETY: obj is an instance of T, since rust_instance_init + // is called from QOM core as the instance_init function + // for class T + unsafe { T::INSTANCE_INIT.unwrap()(&mut *obj.cast::()) } +} + +unsafe extern "C" fn rust_instance_post_init(obj: *mut Object) { + // SAFETY: obj is an instance of T, since rust_instance_post_init + // is called from QOM core as the instance_post_init function + // for class T + // + // FIXME: it's not really guaranteed that there are no backpointers to + // obj; it's quite possible that they have been created by instance_init(). + // The receiver should be &self, not &mut self. + T::INSTANCE_POST_INIT.unwrap()(unsafe { &mut *obj.cast::() }) +} + /// Trait a type must implement to be registered with QEMU. +/// +/// # Safety +/// +/// - the struct must be `#[repr(C)]` +/// +/// - `Class` and `TYPE` must match the data in the `TypeInfo` (this is +/// automatic if the class is defined via `ObjectImpl`). +/// +/// - the first field of the struct must be of the instance struct corresponding +/// to the superclass declared as `PARENT_TYPE_NAME` pub trait ObjectImpl: ClassInitImpl + Sized { + /// The QOM class object corresponding to this struct. Not used yet. type Class; + + /// The name of the type, which can be passed to `object_new()` to + /// generate an instance of this type. const TYPE_NAME: &'static CStr; + + /// The parent of the type. This should match the first field of + /// the struct that implements `ObjectImpl`: const PARENT_TYPE_NAME: Option<&'static CStr>; + + /// Whether the object can be instantiated const ABSTRACT: bool = false; - const INSTANCE_INIT: Option = None; - const INSTANCE_POST_INIT: Option = None; const INSTANCE_FINALIZE: Option = None; + /// Function that is called to initialize an object. The parent class will + /// have already been initialized so the type is only responsible for + /// initializing its own members. + /// + /// FIXME: The argument is not really a valid reference. `&mut + /// MaybeUninit` would be a better description. + const INSTANCE_INIT: Option = None; + + /// Function that is called to finish initialization of an object, once + /// `INSTANCE_INIT` functions have been called. + const INSTANCE_POST_INIT: Option = None; + const TYPE_INFO: TypeInfo = TypeInfo { name: Self::TYPE_NAME.as_ptr(), parent: if let Some(pname) = Self::PARENT_TYPE_NAME { @@ -27,8 +74,14 @@ pub trait ObjectImpl: ClassInitImpl + Sized { }, instance_size: core::mem::size_of::(), instance_align: core::mem::align_of::(), - instance_init: Self::INSTANCE_INIT, - instance_post_init: Self::INSTANCE_POST_INIT, + instance_init: match Self::INSTANCE_INIT { + None => None, + Some(_) => Some(rust_instance_init::), + }, + instance_post_init: match Self::INSTANCE_POST_INIT { + None => None, + Some(_) => Some(rust_instance_post_init::), + }, instance_finalize: Self::INSTANCE_FINALIZE, abstract_: Self::ABSTRACT, class_size: core::mem::size_of::(), -- cgit v1.1 From 7bd8e3ef63330e870cf4644d21c285cce35c703d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 31 Oct 2024 09:56:15 +0100 Subject: rust: qom: split ObjectType from ObjectImpl trait Define a separate trait for fields that also applies to classes that are defined by C code. This makes it possible to add metadata to core classes, which has multiple uses: - it makes it possible to access the parent struct's TYPE_* for types that are defined in Rust code, and to avoid repeating it in every subclass - implementors of ObjectType will be allowed to implement the IsA<> trait and therefore to perform typesafe casts from one class to another. - in the future, an ObjectType could be created with Foo::new() in a type-safe manner, without having to pass a TYPE_* constant. Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/definitions.rs | 27 +++++++++++++++++++++------ rust/qemu-api/src/device_class.rs | 11 ++++++----- rust/qemu-api/src/prelude.rs | 2 ++ rust/qemu-api/src/sysbus.rs | 10 ++++++++-- 4 files changed, 37 insertions(+), 13 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index f297075..b98a692 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -26,25 +26,40 @@ unsafe extern "C" fn rust_instance_post_init(obj: *mut Object) { T::INSTANCE_POST_INIT.unwrap()(unsafe { &mut *obj.cast::() }) } -/// Trait a type must implement to be registered with QEMU. +/// Trait exposed by all structs corresponding to QOM objects. /// /// # Safety /// -/// - the struct must be `#[repr(C)]` +/// For classes declared in C: +/// +/// - `Class` and `TYPE` must match the data in the `TypeInfo`; +/// +/// - the first field of the struct must be of the instance type corresponding +/// to the superclass, as declared in the `TypeInfo` +/// +/// - likewise, the first field of the `Class` struct must be of the class type +/// corresponding to the superclass /// -/// - `Class` and `TYPE` must match the data in the `TypeInfo` (this is -/// automatic if the class is defined via `ObjectImpl`). +/// For classes declared in Rust and implementing [`ObjectImpl`]: +/// +/// - the struct must be `#[repr(C)]`; /// /// - the first field of the struct must be of the instance struct corresponding -/// to the superclass declared as `PARENT_TYPE_NAME` -pub trait ObjectImpl: ClassInitImpl + Sized { +/// to the superclass, as declared in `ObjectImpl::PARENT_TYPE_NAME` +/// +/// - likewise, the first field of the `Class` must be of the class struct +/// corresponding to the superclass +pub unsafe trait ObjectType: Sized { /// The QOM class object corresponding to this struct. Not used yet. type Class; /// The name of the type, which can be passed to `object_new()` to /// generate an instance of this type. const TYPE_NAME: &'static CStr; +} +/// Trait a type must implement to be registered with QEMU. +pub trait ObjectImpl: ObjectType + ClassInitImpl { /// The parent of the type. This should match the first field of /// the struct that implements `ObjectImpl`: const PARENT_TYPE_NAME: Option<&'static CStr>; diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index f25904b..03d03fe 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -6,6 +6,7 @@ use std::{ffi::CStr, os::raw::c_void}; use crate::{ bindings::{self, DeviceClass, DeviceState, Error, ObjectClass, Property, VMStateDescription}, + prelude::*, zeroable::Zeroable, }; @@ -146,8 +147,8 @@ macro_rules! declare_properties { }; } -// workaround until we can use --generate-cstr in bindgen. -pub const TYPE_DEVICE: &CStr = - unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_DEVICE) }; -pub const TYPE_SYS_BUS_DEVICE: &CStr = - unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_SYS_BUS_DEVICE) }; +unsafe impl ObjectType for bindings::DeviceState { + type Class = bindings::DeviceClass; + const TYPE_NAME: &'static CStr = + unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_DEVICE) }; +} diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs index a39e228..1b8677b 100644 --- a/rust/qemu-api/src/prelude.rs +++ b/rust/qemu-api/src/prelude.rs @@ -6,3 +6,5 @@ pub use crate::bitops::IntegerExt; pub use crate::cell::BqlCell; pub use crate::cell::BqlRefCell; + +pub use crate::definitions::ObjectType; diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs index 4e192c7..5ee0685 100644 --- a/rust/qemu-api/src/sysbus.rs +++ b/rust/qemu-api/src/sysbus.rs @@ -2,11 +2,17 @@ // Author(s): Paolo Bonzini // SPDX-License-Identifier: GPL-2.0-or-later -use std::ptr::addr_of; +use std::{ffi::CStr, ptr::addr_of}; pub use bindings::{SysBusDevice, SysBusDeviceClass}; -use crate::{bindings, cell::bql_locked, irq::InterruptSource}; +use crate::{bindings, cell::bql_locked, irq::InterruptSource, prelude::*}; + +unsafe impl ObjectType for SysBusDevice { + type Class = SysBusDeviceClass; + const TYPE_NAME: &'static CStr = + unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_SYS_BUS_DEVICE) }; +} impl SysBusDevice { /// Return `self` cast to a mutable pointer, for use in calls to C code. -- cgit v1.1 From 166e8a1fd15bfa527b25fc15ca315e572c0556d2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Sun, 24 Nov 2024 18:51:34 +0100 Subject: rust: qom: change the parent type to an associated type Avoid duplicated code to retrieve the QOM type strings from the Rust type. Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/definitions.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'rust/qemu-api/src') diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index b98a692..df91a2e 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -45,10 +45,10 @@ unsafe extern "C" fn rust_instance_post_init(obj: *mut Object) { /// - the struct must be `#[repr(C)]`; /// /// - the first field of the struct must be of the instance struct corresponding -/// to the superclass, as declared in `ObjectImpl::PARENT_TYPE_NAME` +/// to the superclass, which is `ObjectImpl::ParentType` /// /// - likewise, the first field of the `Class` must be of the class struct -/// corresponding to the superclass +/// corresponding to the superclass, which is `ObjectImpl::ParentType::Class`. pub unsafe trait ObjectType: Sized { /// The QOM class object corresponding to this struct. Not used yet. type Class; @@ -62,7 +62,7 @@ pub unsafe trait ObjectType: Sized { pub trait ObjectImpl: ObjectType + ClassInitImpl { /// The parent of the type. This should match the first field of /// the struct that implements `ObjectImpl`: - const PARENT_TYPE_NAME: Option<&'static CStr>; + type ParentType: ObjectType; /// Whether the object can be instantiated const ABSTRACT: bool = false; @@ -82,11 +82,7 @@ pub trait ObjectImpl: ObjectType + ClassInitImpl { const TYPE_INFO: TypeInfo = TypeInfo { name: Self::TYPE_NAME.as_ptr(), - parent: if let Some(pname) = Self::PARENT_TYPE_NAME { - pname.as_ptr() - } else { - core::ptr::null_mut() - }, + parent: Self::ParentType::TYPE_NAME.as_ptr(), instance_size: core::mem::size_of::(), instance_align: core::mem::align_of::(), instance_init: match Self::INSTANCE_INIT { -- cgit v1.1