* [RFC PATCH 0/6] rust: make usage of bitmasks and bitfields nicer
@ 2025-05-21 8:18 Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 1/6] rust: add "bits", a custom bitflags implementation Paolo Bonzini
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: Paolo Bonzini @ 2025-05-21 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-rust, manos.pitsidianakis
This adds or modifies some of the early choices that were done when
writing the pl011 crate. Now that the implementation is safe and
readable, it's possible to look at the code as a whole, see what
looks ugly and fix it.
To see the effect on the pl011 code, jump at patches 2 and 4. But
overall the focus is on "ugly" constant declarations of two kinds.
First, bitmasks. These are currently written like this:
const IRQMASK: [u32; 6] = [
Interrupt::E.0 | Interrupt::MS.0 | Interrupt::RT.0 | Interrupt::TX.0 | Interrupt::RX.0,
...
}
Here the issue is threefold: 1) the repeated "Interrupt::"; 2) the
ugly ".0" and 3) the type of IRQMASK is not "Interrupt" to avoid further
".0"s elsewhere.
All this can be fixed by abstracting this kind of bitmasks, and the go-to
solution for Rust is called "bitflags". However, it does not solve the
constant declaration part so I'm rolling my own here, while keeping it
similar to bitflags so it's not completely foreign to Rust developers.
It's a bit simpler and it has the extra expression parsing functionality,
so that the above becomes:
const IRQMASK: [Interrupt; 6] = [
bits!(Interrupt: E | MS | RT | TX | RX)
...
}
(this specific case can also be written simply as "Interrupt::all()",
because all bits are set, but still).
Second, while the "bilge" crate is very nice it was written with the
expectation that const traits would be stable Real Soon. They didn't
and we're left with stuff like
pub const BREAK: Self = Self { value: 1 << 10 };
So this series replaces "bilge" with an alternative implementation of
bitfields called (less cryptically) "bitfield-struct". Now, there is a
trade-off here. It is described more in detail in patch 4, but roughly
speaking:
++ bitfield-struct supports "const" well
+ bitfield-struct is much smaller than bilge, so that it is possible
to remove a bunch of subprojects
- bitfield-struct requires manual size annotations for anything that is
not a primitive type (bool, iNN or uNN); this is especially annoying
for enums
It's important that the disadvantages affect only the definition of
the type. Code that *uses* the type is the same or better, for example
the above const declaration becomes:
pub const BREAK: Self = Self::new().with_errors(Errors::BREAK);
This "with_*()" convention is in fact the same that it's used for
VMState declarations, such as in ".with_version_id(N)", so there is some
consistency too.
Again, I do like "bilge" and I think it's usage of arbitrary-width
types like "u4" is very nice and readable. If it ever grows "const"
capabilities it's certainly possible to come back to it, but right
now I feel that the trade-off leans towards the switch.
Paolo
Paolo Bonzini (6):
rust: add "bits", a custom bitflags implementation
rust: pl011: use the bits macro
rust: qemu-api-macros: add from_bits and into_bits to
#[derive(TryInto)]
rust: subprojects: add bitfield-struct
rust: pl011: switch from bilge to bitfield-struct
rust: remove bilge crate
rust/Cargo.lock | 75 +--
rust/Cargo.toml | 2 +
rust/bits/Cargo.toml | 19 +
rust/bits/meson.build | 12 +
rust/bits/src/lib.rs | 441 ++++++++++++++++++
rust/hw/char/pl011/Cargo.toml | 4 +-
rust/hw/char/pl011/meson.build | 12 +-
rust/hw/char/pl011/src/device.rs | 51 +-
rust/hw/char/pl011/src/registers.rs | 145 +++---
rust/meson.build | 4 +
rust/qemu-api-macros/src/bits.rs | 227 +++++++++
rust/qemu-api-macros/src/lib.rs | 60 ++-
rust/qemu-api/src/vmstate.rs | 34 +-
subprojects/.gitignore | 8 +-
subprojects/arbitrary-int-1-rs.wrap | 10 -
subprojects/bilge-0.2-rs.wrap | 10 -
subprojects/bilge-impl-0.2-rs.wrap | 10 -
subprojects/bitfield-struct-0.9-rs.wrap | 7 +
subprojects/either-1-rs.wrap | 10 -
subprojects/itertools-0.11-rs.wrap | 10 -
.../bitfield-struct-0.9-rs/meson.build | 36 ++
subprojects/proc-macro-error-1-rs.wrap | 10 -
subprojects/proc-macro-error-attr-1-rs.wrap | 10 -
23 files changed, 917 insertions(+), 290 deletions(-)
create mode 100644 rust/bits/Cargo.toml
create mode 100644 rust/bits/meson.build
create mode 100644 rust/bits/src/lib.rs
create mode 100644 rust/qemu-api-macros/src/bits.rs
delete mode 100644 subprojects/arbitrary-int-1-rs.wrap
delete mode 100644 subprojects/bilge-0.2-rs.wrap
delete mode 100644 subprojects/bilge-impl-0.2-rs.wrap
create mode 100644 subprojects/bitfield-struct-0.9-rs.wrap
delete mode 100644 subprojects/either-1-rs.wrap
delete mode 100644 subprojects/itertools-0.11-rs.wrap
create mode 100644 subprojects/packagefiles/bitfield-struct-0.9-rs/meson.build
delete mode 100644 subprojects/proc-macro-error-1-rs.wrap
delete mode 100644 subprojects/proc-macro-error-attr-1-rs.wrap
--
2.49.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH 1/6] rust: add "bits", a custom bitflags implementation
2025-05-21 8:18 [RFC PATCH 0/6] rust: make usage of bitmasks and bitfields nicer Paolo Bonzini
@ 2025-05-21 8:18 ` Paolo Bonzini
2025-05-21 8:29 ` Daniel P. Berrangé
2025-05-21 8:18 ` [RFC PATCH 2/6] rust: pl011: use the bits macro Paolo Bonzini
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2025-05-21 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-rust, manos.pitsidianakis
One common thing that device emulation does is manipulate bitmasks, for example
to check whether two bitmaps have common bits. One example in the pl011 crate
is the checks for pending interrupts, where an interrupt cause corresponds to
at least one interrupt source from a fixed set.
Unfortunately, this is one case where Rust *can* provide some kind of
abstraction but it does so with a rather Perl-ish There Is More Way To
Do It. It is not something where a crate like "bilge" helps, because
it only covers the packing of bits in a structure; operations like "are
all bits of Y set in X" almost never make sense for bit-packed structs;
you need something else, there are several crates that do it and of course
we're going to roll our own.
In particular I examined three:
- bitmask (https://docs.rs/bitmask/0.5.0/bitmask/) does not support const
at all. This is a showstopper because one of the ugly things in the
current pl011 code is the ugliness of code that defines interrupt masks
at compile time:
pub const E: Self = Self(Self::OE.0 | Self::BE.0 | Self::PE.0 | Self::FE.0);
or even worse:
const IRQMASK: [u32; 6] = [
Interrupt::E.0 | Interrupt::MS.0 | Interrupt::RT.0 | Interrupt::TX.0 | Interrupt::RX.0,
...
}
You would have to use roughly the same code---"bitmask" only helps with
defining the struct.
- bitmask_enum (https://docs.rs/bitmask-enum/2.2.5/bitmask_enum/) does not
have a good separation of "valid" and "invalid" bits, so for example "!x"
will invert all 16 bits if you choose u16 as the representation -- even if
you only defined 10 bits. This makes it easier to introduce subtle bugs
in comparisons.
- bitflags (https://docs.rs/bitflags/2.6.0/bitflags/) is generally the most
used such crate and is the one that I took most inspiration from with
respect to the syntax. It's a pretty sophisticated implementation,
with a lot of bells and whistles such as an implementation of "Iter"
that returns the bits one at a time.
The main thing that all of them lack, however, is a way to simplify the
ugly definitions like the above. "bitflags" includes const methods that
perform AND/OR/XOR of masks (these are necessary because Rust operator
overloading does not support const yet, and therefore overloaded operators
cannot be used in the definition of a "static" variable), but they become
even more verbose and unmanageable, like
Interrupt::E.union(Interrupt::MS).union(Interrupt::RT).union(Interrupt::TX).union(Interrupt::RX)
This was the main reason to create "bits", which allows something like
bits!(Interrupt: E | MS | RT | TX | RX)
and expands it 1) add "Interrupt::" in front of all identifiers 2) convert
operators to the wordy const functions like "union". It supports boolean
operators "&", "|", "^", "!" and parentheses, with a relatively simple
recursive descent parser that's implemented in qemu_api_macros.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/Cargo.lock | 7 +
rust/Cargo.toml | 1 +
rust/bits/Cargo.toml | 19 ++
rust/bits/meson.build | 12 +
rust/bits/src/lib.rs | 441 +++++++++++++++++++++++++++++++
rust/meson.build | 1 +
rust/qemu-api-macros/src/bits.rs | 227 ++++++++++++++++
rust/qemu-api-macros/src/lib.rs | 12 +
8 files changed, 720 insertions(+)
create mode 100644 rust/bits/Cargo.toml
create mode 100644 rust/bits/meson.build
create mode 100644 rust/bits/src/lib.rs
create mode 100644 rust/qemu-api-macros/src/bits.rs
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index 13d580c693b..0dfe0fb6ced 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -31,6 +31,13 @@ dependencies = [
"syn",
]
+[[package]]
+name = "bits"
+version = "0.1.0"
+dependencies = [
+ "qemu_api_macros",
+]
+
[[package]]
name = "either"
version = "1.12.0"
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index d9faeecb10b..165328b6d01 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -1,6 +1,7 @@
[workspace]
resolver = "2"
members = [
+ "bits",
"qemu-api-macros",
"qemu-api",
"hw/char/pl011",
diff --git a/rust/bits/Cargo.toml b/rust/bits/Cargo.toml
new file mode 100644
index 00000000000..1ff38a41175
--- /dev/null
+++ b/rust/bits/Cargo.toml
@@ -0,0 +1,19 @@
+[package]
+name = "bits"
+version = "0.1.0"
+authors = ["Paolo Bonzini <pbonzini@redhat.com>"]
+description = "const-friendly bit flags"
+resolver = "2"
+publish = false
+
+edition.workspace = true
+homepage.workspace = true
+license.workspace = true
+repository.workspace = true
+rust-version.workspace = true
+
+[dependencies]
+qemu_api_macros = { path = "../qemu-api-macros" }
+
+[lints]
+workspace = true
diff --git a/rust/bits/meson.build b/rust/bits/meson.build
new file mode 100644
index 00000000000..2cd4121a546
--- /dev/null
+++ b/rust/bits/meson.build
@@ -0,0 +1,12 @@
+_bits_rs = static_library(
+ 'bits',
+ 'src/lib.rs',
+ override_options: ['rust_std=2021', 'build.rust_std=2021'],
+ rust_abi: 'rust',
+ dependencies: [qemu_api_macros],
+)
+
+bits_rs = declare_dependency(link_with: _bits_rs)
+
+rust.test('rust-bits-tests', _bits_rs,
+ suite: ['unit', 'rust'])
diff --git a/rust/bits/src/lib.rs b/rust/bits/src/lib.rs
new file mode 100644
index 00000000000..d80a6263f1e
--- /dev/null
+++ b/rust/bits/src/lib.rs
@@ -0,0 +1,441 @@
+/// # Definition entry point
+///
+/// Define a struct with a single field of type $type. Include public constants
+/// for each element listed in braces.
+///
+/// The unnamed element at the end, if present, can be used to enlarge the set
+/// of valid bits. Bits that are valid but not listed are treated normally for
+/// the purpose of arithmetic operations, and are printed with their hexadecimal
+/// value.
+///
+/// The struct implements the following traits: [`BitAnd`](std::ops::BitAnd),
+/// [`BitOr`](std::ops::BitOr), [`BitXor`](std::ops::BitXor),
+/// [`Not`](std::ops::Not), [`Sub`](std::ops::Sub); [`Debug`](std::fmt::Debug),
+/// [`Display`](std::fmt::Display), [`Binary`](std::fmt::Binary),
+/// [`Octal`](std::fmt::Octal), [`LowerHex`](std::fmt::LowerHex),
+/// [`UpperHex`](std::fmt::UpperHex); [`From`]`<type>`/[`Into`]`<type>` where
+/// type is the type specified in the definition.
+///
+/// ## Example
+///
+/// ```
+/// # use bits::bits;
+/// bits! {
+/// pub struct Colors(u8) {
+/// BLACK = 0,
+/// RED = 1,
+/// GREEN = 1 << 1,
+/// BLUE = 1 << 2,
+/// WHITE = (1 << 0) | (1 << 1) | (1 << 2),
+/// }
+/// }
+/// ```
+///
+/// ```
+/// # use bits::bits;
+/// # bits! { pub struct Colors(u8) { BLACK = 0, RED = 1, GREEN = 1 << 1, BLUE = 1 << 2, } }
+///
+/// bits! {
+/// pub struct Colors8(u8) {
+/// BLACK = 0,
+/// RED = 1,
+/// GREEN = 1 << 1,
+/// BLUE = 1 << 2,
+/// WHITE = (1 << 0) | (1 << 1) | (1 << 2),
+///
+/// _ = 255,
+/// }
+/// }
+///
+/// // The previously defined struct ignores bits not explicitly defined.
+/// assert_eq!(
+/// Colors::from(255).into_bits(),
+/// (Colors::RED | Colors::GREEN | Colors::BLUE).into_bits()
+/// );
+///
+/// // Adding "_ = 255" makes it retain other bits as well.
+/// assert_eq!(Colors8::from(255).into_bits(), 255);
+///
+/// // all() does not include the additional bits, valid_bits() does
+/// assert_eq!(Colors8::all().into_bits(), Colors::all().into_bits());
+/// assert_eq!(Colors8::valid_bits().into_bits(), 255);
+/// ```
+///
+/// # Evaluation entry point
+///
+/// Return a constant corresponding to the boolean expression `$expr`.
+/// Identifiers in the expression correspond to values defined for the
+/// type `$type`. Supported operators are `!` (unary), `-`, `&`, `^`, `|`.
+///
+/// ## Examples
+///
+/// ```
+/// # use bits::bits;
+/// bits! {
+/// pub struct Colors(u8) {
+/// BLACK = 0,
+/// RED = 1,
+/// GREEN = 1 << 1,
+/// BLUE = 1 << 2,
+/// // same as "WHITE = 7",
+/// WHITE = bits!(Self as u8: RED | GREEN | BLUE),
+/// }
+/// }
+///
+/// let rgb = bits! { Colors: RED | GREEN | BLUE };
+/// assert_eq!(rgb, Colors::WHITE);
+/// ```
+#[macro_export]
+macro_rules! bits {
+ {
+ $(#[$struct_meta:meta])*
+ $struct_vis:vis struct $struct_name:ident($field_vis:vis $type:ty) {
+ $($(#[$const_meta:meta])* $const:ident = $val:expr),+
+ $(,_ = $mask:expr)?
+ $(,)?
+ }
+ } => {
+ $(#[$struct_meta])*
+ #[derive(Clone, Copy, PartialEq, Eq)]
+ #[repr(transparent)]
+ $struct_vis struct $struct_name($field_vis $type);
+
+ impl $struct_name {
+ $( #[allow(dead_code)] $(#[$const_meta])*
+ pub const $const: $struct_name = $struct_name($val); )+
+
+ #[doc(hidden)]
+ const VALID__: $type = $( Self::$const.0 )|+ $(|$mask)?;
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn empty() -> Self {
+ Self(0)
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn all() -> Self {
+ Self($( Self::$const.0 )|+)
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn valid_bits() -> Self {
+ Self(Self::VALID__)
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn valid(val: $type) -> bool {
+ (val & !Self::VALID__) == 0
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn any_set(self, mask: Self) -> bool {
+ (self.0 & mask.0) != 0
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn all_set(self, mask: Self) -> bool {
+ (self.0 & mask.0) == mask.0
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn none_set(self, mask: Self) -> bool {
+ (self.0 & mask.0) == 0
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn from_bits(value: $type) -> Self {
+ $struct_name(value)
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn into_bits(self) -> $type {
+ self.0
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub fn set(&mut self, rhs: Self) {
+ self.0 |= rhs.0;
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub fn clear(&mut self, rhs: Self) {
+ self.0 &= !rhs.0;
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub fn toggle(&mut self, rhs: Self) {
+ self.0 ^= rhs.0;
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn intersection(self, rhs: Self) -> Self {
+ $struct_name(self.0 & rhs.0)
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn difference(self, rhs: Self) -> Self {
+ $struct_name(self.0 & !rhs.0)
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn symmetric_difference(self, rhs: Self) -> Self {
+ $struct_name(self.0 ^ rhs.0)
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn union(self, rhs: Self) -> Self {
+ $struct_name(self.0 | rhs.0)
+ }
+
+ #[allow(dead_code)]
+ #[inline(always)]
+ pub const fn invert(self) -> Self {
+ $struct_name(self.0 ^ Self::VALID__)
+ }
+ }
+
+ impl ::std::fmt::Binary for $struct_name {
+ fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+ // If no width, use the highest valid bit
+ let width = f.width().unwrap_or((Self::VALID__.ilog2() + 1) as usize);
+ write!(f, "{:0>width$.precision$b}", self.0,
+ width = width,
+ precision = f.precision().unwrap_or(width))
+ }
+ }
+
+ impl ::std::fmt::LowerHex for $struct_name {
+ fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+ <$type as ::std::fmt::LowerHex>::fmt(&self.0, f)
+ }
+ }
+
+ impl ::std::fmt::Octal for $struct_name {
+ fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+ <$type as ::std::fmt::Octal>::fmt(&self.0, f)
+ }
+ }
+
+ impl ::std::fmt::UpperHex for $struct_name {
+ fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+ <$type as ::std::fmt::UpperHex>::fmt(&self.0, f)
+ }
+ }
+
+ impl ::std::fmt::Debug for $struct_name {
+ fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+ write!(f, "{}({})", stringify!($struct_name), self)
+ }
+ }
+
+ impl ::std::fmt::Display for $struct_name {
+ fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+ use ::std::fmt::Display;
+ let mut first = true;
+ let mut left = self.0;
+ $(if Self::$const.0.is_power_of_two() && (self & Self::$const).0 != 0 {
+ if first { first = false } else { Display::fmt(&'|', f)?; }
+ Display::fmt(stringify!($const), f)?;
+ left -= Self::$const.0;
+ })+
+ if first {
+ Display::fmt(&'0', f)
+ } else if left != 0 {
+ write!(f, "|{left:#x}")
+ } else {
+ Ok(())
+ }
+ }
+ }
+
+ impl ::std::cmp::PartialEq<$type> for $struct_name {
+ fn eq(&self, rhs: &$type) -> bool {
+ self.0 == *rhs
+ }
+ }
+
+ impl ::std::ops::BitAnd<$struct_name> for &$struct_name {
+ type Output = $struct_name;
+ fn bitand(self, rhs: $struct_name) -> Self::Output {
+ $struct_name(self.0 & rhs.0)
+ }
+ }
+
+ impl ::std::ops::BitAndAssign<$struct_name> for $struct_name {
+ fn bitand_assign(&mut self, rhs: $struct_name) {
+ self.0 = self.0 & rhs.0
+ }
+ }
+
+ impl ::std::ops::BitXor<$struct_name> for &$struct_name {
+ type Output = $struct_name;
+ fn bitxor(self, rhs: $struct_name) -> Self::Output {
+ $struct_name(self.0 ^ rhs.0)
+ }
+ }
+
+ impl ::std::ops::BitXorAssign<$struct_name> for $struct_name {
+ fn bitxor_assign(&mut self, rhs: $struct_name) {
+ self.0 = self.0 ^ rhs.0
+ }
+ }
+
+ impl ::std::ops::BitOr<$struct_name> for &$struct_name {
+ type Output = $struct_name;
+ fn bitor(self, rhs: $struct_name) -> Self::Output {
+ $struct_name(self.0 | rhs.0)
+ }
+ }
+
+ impl ::std::ops::BitOrAssign<$struct_name> for $struct_name {
+ fn bitor_assign(&mut self, rhs: $struct_name) {
+ self.0 = self.0 | rhs.0
+ }
+ }
+
+ impl ::std::ops::Sub<$struct_name> for &$struct_name {
+ type Output = $struct_name;
+ fn sub(self, rhs: $struct_name) -> Self::Output {
+ $struct_name(self.0 & !rhs.0)
+ }
+ }
+
+ impl ::std::ops::SubAssign<$struct_name> for $struct_name {
+ fn sub_assign(&mut self, rhs: $struct_name) {
+ self.0 = self.0 - rhs.0
+ }
+ }
+
+ impl ::std::ops::Not for &$struct_name {
+ type Output = $struct_name;
+ fn not(self) -> Self::Output {
+ $struct_name(self.0 ^ $struct_name::VALID__)
+ }
+ }
+
+ impl ::std::ops::BitAnd<$struct_name> for $struct_name {
+ type Output = Self;
+ fn bitand(self, rhs: Self) -> Self::Output {
+ $struct_name(self.0 & rhs.0)
+ }
+ }
+
+ impl ::std::ops::BitXor<$struct_name> for $struct_name {
+ type Output = Self;
+ fn bitxor(self, rhs: Self) -> Self::Output {
+ $struct_name(self.0 ^ rhs.0)
+ }
+ }
+
+ impl ::std::ops::BitOr<$struct_name> for $struct_name {
+ type Output = Self;
+ fn bitor(self, rhs: Self) -> Self::Output {
+ $struct_name(self.0 | rhs.0)
+ }
+ }
+
+ impl ::std::ops::Sub<$struct_name> for $struct_name {
+ type Output = Self;
+ fn sub(self, rhs: Self) -> Self::Output {
+ $struct_name(self.0 & !rhs.0)
+ }
+ }
+
+ impl ::std::ops::Not for $struct_name {
+ type Output = Self;
+ fn not(self) -> Self::Output {
+ $struct_name(self.0 ^ Self::VALID__)
+ }
+ }
+
+ impl From<$struct_name> for $type {
+ fn from(x: $struct_name) -> $type {
+ x.0
+ }
+ }
+
+ impl From<$type> for $struct_name {
+ fn from(x: $type) -> Self {
+ $struct_name(x & Self::VALID__)
+ }
+ }
+ };
+
+ { $type:ty: $expr:expr } => {
+ ::qemu_api_macros::bits_const_internal! { $type @ ($expr) }
+ };
+
+ { $type:ty as $int_type:ty: $expr:expr } => {
+ (::qemu_api_macros::bits_const_internal! { $type @ ($expr) }.into_bits()) as $int_type
+ };
+}
+
+#[cfg(test)]
+mod test {
+ bits! {
+ pub(crate) struct InterruptMask(u32) {
+ OE = 1 << 10,
+ BE = 1 << 9,
+ PE = 1 << 8,
+ FE = 1 << 7,
+ RT = 1 << 6,
+ TX = 1 << 5,
+ RX = 1 << 4,
+ DSR = 1 << 3,
+ DCD = 1 << 2,
+ CTS = 1 << 1,
+ RI = 1 << 0,
+
+ E = bits!(Self as u32: OE | BE | PE | FE),
+ MS = bits!(Self as u32: RI | DSR | DCD | CTS),
+ }
+ }
+
+ #[test]
+ pub fn test_not() {
+ assert_eq!(
+ !InterruptMask::from(InterruptMask::RT),
+ InterruptMask::E | InterruptMask::MS | InterruptMask::TX | InterruptMask::RX
+ );
+ }
+
+ #[test]
+ pub fn test_and() {
+ assert_eq!(
+ InterruptMask::from(0),
+ InterruptMask::MS & InterruptMask::OE
+ )
+ }
+
+ #[test]
+ pub fn test_or() {
+ assert_eq!(
+ InterruptMask::E,
+ InterruptMask::OE | InterruptMask::BE | InterruptMask::PE | InterruptMask::FE
+ );
+ }
+
+ #[test]
+ pub fn test_xor() {
+ assert_eq!(
+ InterruptMask::E ^ InterruptMask::BE,
+ InterruptMask::OE | InterruptMask::PE | InterruptMask::FE
+ );
+ }
+}
diff --git a/rust/meson.build b/rust/meson.build
index 91e52b8fb8e..f370b0ec939 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -1,4 +1,5 @@
subdir('qemu-api-macros')
+subdir('bits')
subdir('qemu-api')
subdir('hw')
diff --git a/rust/qemu-api-macros/src/bits.rs b/rust/qemu-api-macros/src/bits.rs
new file mode 100644
index 00000000000..b6e07d4e890
--- /dev/null
+++ b/rust/qemu-api-macros/src/bits.rs
@@ -0,0 +1,227 @@
+// shadowing is useful together with "if let"
+#![allow(clippy::shadow_unrelated)]
+
+use proc_macro2::{
+ Delimiter, Group, Ident, Punct, Spacing, Span, TokenStream, TokenTree, TokenTree as TT,
+};
+
+use crate::utils::MacroError;
+
+pub struct BitsConstInternal {
+ typ: TokenTree,
+}
+
+fn paren(ts: TokenStream) -> TokenTree {
+ TT::Group(Group::new(Delimiter::Parenthesis, ts))
+}
+
+fn ident(s: &'static str) -> TokenTree {
+ TT::Ident(Ident::new(s, Span::call_site()))
+}
+
+fn punct(ch: char) -> TokenTree {
+ TT::Punct(Punct::new(ch, Spacing::Alone))
+}
+
+/// Implements a recursive-descent parser that translates Boolean expressions on
+/// bitmasks to invocations of `const` functions defined by the `bits!` macro.
+impl BitsConstInternal {
+ // primary ::= '(' or ')'
+ // | ident
+ // | '!' ident
+ fn parse_primary(
+ &self,
+ tok: TokenTree,
+ it: &mut dyn Iterator<Item = TokenTree>,
+ out: &mut TokenStream,
+ ) -> Result<Option<TokenTree>, MacroError> {
+ let next = match tok {
+ TT::Group(ref g) => {
+ if g.delimiter() != Delimiter::Parenthesis && g.delimiter() != Delimiter::None {
+ return Err(MacroError::Message("expected parenthesis".into(), g.span()));
+ }
+ let mut stream = g.stream().into_iter();
+ let Some(first_tok) = stream.next() else {
+ return Err(MacroError::Message(
+ "expected operand, found ')'".into(),
+ g.span(),
+ ));
+ };
+ let mut output = TokenStream::new();
+ // start from the lowest precedence
+ let next = self.parse_or(first_tok, &mut stream, &mut output)?;
+ if let Some(tok) = next {
+ return Err(MacroError::Message(
+ format!("unexpected token {tok}"),
+ tok.span(),
+ ));
+ }
+ out.extend(Some(paren(output)));
+ it.next()
+ }
+ TT::Ident(_) => {
+ let mut output = TokenStream::new();
+ output.extend([
+ self.typ.clone(),
+ TT::Punct(Punct::new(':', Spacing::Joint)),
+ TT::Punct(Punct::new(':', Spacing::Joint)),
+ tok,
+ ]);
+ out.extend(Some(paren(output)));
+ it.next()
+ }
+ TT::Punct(ref p) => {
+ if p.as_char() != '!' {
+ return Err(MacroError::Message("expected operand".into(), p.span()));
+ }
+ let Some(rhs_tok) = it.next() else {
+ return Err(MacroError::Message(
+ "expected operand at end of input".into(),
+ p.span(),
+ ));
+ };
+ let next = self.parse_primary(rhs_tok, it, out)?;
+ out.extend([punct('.'), ident("invert"), paren(TokenStream::new())]);
+ next
+ }
+ _ => {
+ return Err(MacroError::Message("unexpected literal".into(), tok.span()));
+ }
+ };
+ Ok(next)
+ }
+
+ fn parse_binop<
+ F: Fn(
+ &Self,
+ TokenTree,
+ &mut dyn Iterator<Item = TokenTree>,
+ &mut TokenStream,
+ ) -> Result<Option<TokenTree>, MacroError>,
+ >(
+ &self,
+ tok: TokenTree,
+ it: &mut dyn Iterator<Item = TokenTree>,
+ out: &mut TokenStream,
+ ch: char,
+ f: F,
+ method: &'static str,
+ ) -> Result<Option<TokenTree>, MacroError> {
+ let mut next = f(self, tok, it, out)?;
+ while next.is_some() {
+ let op = next.as_ref().unwrap();
+ let TT::Punct(ref p) = op else { break };
+ if p.as_char() != ch {
+ break;
+ }
+
+ let Some(rhs_tok) = it.next() else {
+ return Err(MacroError::Message(
+ "expected operand at end of input".into(),
+ p.span(),
+ ));
+ };
+ let mut rhs = TokenStream::new();
+ next = f(self, rhs_tok, it, &mut rhs)?;
+ out.extend([punct('.'), ident(method), paren(rhs)]);
+ }
+ Ok(next)
+ }
+
+ // sub ::= primary ('-' primary)*
+ pub fn parse_sub(
+ &self,
+ tok: TokenTree,
+ it: &mut dyn Iterator<Item = TokenTree>,
+ out: &mut TokenStream,
+ ) -> Result<Option<TokenTree>, MacroError> {
+ self.parse_binop(tok, it, out, '-', Self::parse_primary, "difference")
+ }
+
+ // and ::= sub ('&' sub)*
+ fn parse_and(
+ &self,
+ tok: TokenTree,
+ it: &mut dyn Iterator<Item = TokenTree>,
+ out: &mut TokenStream,
+ ) -> Result<Option<TokenTree>, MacroError> {
+ self.parse_binop(tok, it, out, '&', Self::parse_sub, "intersection")
+ }
+
+ // xor ::= and ('&' and)*
+ fn parse_xor(
+ &self,
+ tok: TokenTree,
+ it: &mut dyn Iterator<Item = TokenTree>,
+ out: &mut TokenStream,
+ ) -> Result<Option<TokenTree>, MacroError> {
+ self.parse_binop(tok, it, out, '^', Self::parse_and, "symmetric_difference")
+ }
+
+ // or ::= xor ('|' xor)*
+ pub fn parse_or(
+ &self,
+ tok: TokenTree,
+ it: &mut dyn Iterator<Item = TokenTree>,
+ out: &mut TokenStream,
+ ) -> Result<Option<TokenTree>, MacroError> {
+ self.parse_binop(tok, it, out, '|', Self::parse_xor, "union")
+ }
+
+ pub fn parse(
+ it: &mut dyn Iterator<Item = TokenTree>,
+ ) -> Result<proc_macro2::TokenStream, MacroError> {
+ let mut pos = Span::call_site();
+ let mut typ = proc_macro2::TokenStream::new();
+
+ // Gobble everything up to an `@` sign, which is followed by a
+ // parenthesized expression; that is, all token trees except the
+ // last two form the type.
+ let next = loop {
+ let tok = it.next();
+ if let Some(ref t) = tok {
+ pos = t.span();
+ }
+ match tok {
+ None => break None,
+ Some(TT::Punct(ref p)) if p.as_char() == '@' => {
+ let tok = it.next();
+ if let Some(ref t) = tok {
+ pos = t.span();
+ }
+ break tok;
+ }
+ Some(x) => typ.extend(Some(x)),
+ }
+ };
+
+ let Some(tok) = next else {
+ return Err(MacroError::Message(
+ "expected expression, do not call this macro directly".into(),
+ pos,
+ ));
+ };
+ let TT::Group(ref _group) = tok else {
+ return Err(MacroError::Message(
+ "expected parenthesis, do not call this macro directly".into(),
+ tok.span(),
+ ));
+ };
+ let mut out = TokenStream::new();
+ let state = Self {
+ typ: TT::Group(Group::new(Delimiter::None, typ)),
+ };
+
+ let next = state.parse_primary(tok, it, &mut out)?;
+
+ // A parenthesized expression is a single production of the grammar,
+ // so the input must have reached the last token.
+ if let Some(tok) = next {
+ return Err(MacroError::Message(
+ format!("unexpected token {tok}"),
+ tok.span(),
+ ));
+ }
+ Ok(out)
+ }
+}
diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
index f97449bb304..ceb79f09f97 100644
--- a/rust/qemu-api-macros/src/lib.rs
+++ b/rust/qemu-api-macros/src/lib.rs
@@ -12,6 +12,9 @@
mod utils;
use utils::MacroError;
+mod bits;
+use bits::BitsConstInternal;
+
fn get_fields<'a>(
input: &'a DeriveInput,
msg: &str,
@@ -219,3 +222,12 @@ pub fn derive_tryinto(input: TokenStream) -> TokenStream {
TokenStream::from(expanded)
}
+
+#[proc_macro]
+pub fn bits_const_internal(ts: TokenStream) -> TokenStream {
+ let ts = proc_macro2::TokenStream::from(ts);
+ let mut it = ts.into_iter();
+
+ let expanded = BitsConstInternal::parse(&mut it).unwrap_or_else(Into::into);
+ TokenStream::from(expanded)
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 2/6] rust: pl011: use the bits macro
2025-05-21 8:18 [RFC PATCH 0/6] rust: make usage of bitmasks and bitfields nicer Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 1/6] rust: add "bits", a custom bitflags implementation Paolo Bonzini
@ 2025-05-21 8:18 ` Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 3/6] rust: qemu-api-macros: add from_bits and into_bits to #[derive(TryInto)] Paolo Bonzini
` (3 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2025-05-21 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-rust, manos.pitsidianakis
This avoids the repeated ".0" when using the Interrupt struct.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/Cargo.lock | 1 +
rust/hw/char/pl011/Cargo.toml | 1 +
rust/hw/char/pl011/meson.build | 1 +
rust/hw/char/pl011/src/device.rs | 51 ++++++++++++++---------------
rust/hw/char/pl011/src/registers.rs | 39 ++++++++++++----------
5 files changed, 49 insertions(+), 44 deletions(-)
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index 0dfe0fb6ced..bccfe855a70 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -73,6 +73,7 @@ version = "0.1.0"
dependencies = [
"bilge",
"bilge-impl",
+ "bits",
"qemu_api",
"qemu_api_macros",
]
diff --git a/rust/hw/char/pl011/Cargo.toml b/rust/hw/char/pl011/Cargo.toml
index a1f431ab4a3..003ef9613d4 100644
--- a/rust/hw/char/pl011/Cargo.toml
+++ b/rust/hw/char/pl011/Cargo.toml
@@ -18,6 +18,7 @@ crate-type = ["staticlib"]
[dependencies]
bilge = { version = "0.2.0" }
bilge-impl = { version = "0.2.0" }
+bits = { path = "../../../bits" }
qemu_api = { path = "../../../qemu-api" }
qemu_api_macros = { path = "../../../qemu-api-macros" }
diff --git a/rust/hw/char/pl011/meson.build b/rust/hw/char/pl011/meson.build
index 547cca5a96f..f134a6cdc6b 100644
--- a/rust/hw/char/pl011/meson.build
+++ b/rust/hw/char/pl011/meson.build
@@ -12,6 +12,7 @@ _libpl011_rs = static_library(
dependencies: [
bilge_dep,
bilge_impl_dep,
+ bits_rs,
qemu_api,
qemu_api_macros,
],
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index bde3be65c5b..8d89c2448dc 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -85,8 +85,8 @@ pub struct PL011Registers {
#[doc(alias = "cr")]
pub control: registers::Control,
pub dmacr: u32,
- pub int_enabled: u32,
- pub int_level: u32,
+ pub int_enabled: Interrupt,
+ pub int_level: Interrupt,
pub read_fifo: Fifo,
pub ilpr: u32,
pub ibrd: u32,
@@ -199,9 +199,9 @@ pub(self) fn read(&mut self, offset: RegisterOffset) -> (bool, u32) {
LCR_H => u32::from(self.line_control),
CR => u32::from(self.control),
FLS => self.ifl,
- IMSC => self.int_enabled,
- RIS => self.int_level,
- MIS => self.int_level & self.int_enabled,
+ IMSC => u32::from(self.int_enabled),
+ RIS => u32::from(self.int_level),
+ MIS => u32::from(self.int_level & self.int_enabled),
ICR => {
// "The UARTICR Register is the interrupt clear register and is write-only"
// Source: ARM DDI 0183G 3.3.13 Interrupt Clear Register, UARTICR
@@ -263,13 +263,13 @@ pub(self) fn write(
self.set_read_trigger();
}
IMSC => {
- self.int_enabled = value;
+ self.int_enabled = Interrupt::from(value);
return true;
}
RIS => {}
MIS => {}
ICR => {
- self.int_level &= !value;
+ self.int_level &= !Interrupt::from(value);
return true;
}
DMACR => {
@@ -295,7 +295,7 @@ fn read_data_register(&mut self, update: &mut bool) -> u32 {
self.flags.set_receive_fifo_empty(true);
}
if self.read_count + 1 == self.read_trigger {
- self.int_level &= !Interrupt::RX.0;
+ self.int_level &= !Interrupt::RX;
}
self.receive_status_error_clear.set_from_data(c);
*update = true;
@@ -305,7 +305,7 @@ fn read_data_register(&mut self, update: &mut bool) -> u32 {
fn write_data_register(&mut self, value: u32) -> bool {
// interrupts always checked
let _ = self.loopback_tx(value.into());
- self.int_level |= Interrupt::TX.0;
+ self.int_level |= Interrupt::TX;
true
}
@@ -361,19 +361,19 @@ fn loopback_mdmctrl(&mut self) -> bool {
// Change interrupts based on updated FR
let mut il = self.int_level;
- il &= !Interrupt::MS.0;
+ il &= !Interrupt::MS;
if self.flags.data_set_ready() {
- il |= Interrupt::DSR.0;
+ il |= Interrupt::DSR;
}
if self.flags.data_carrier_detect() {
- il |= Interrupt::DCD.0;
+ il |= Interrupt::DCD;
}
if self.flags.clear_to_send() {
- il |= Interrupt::CTS.0;
+ il |= Interrupt::CTS;
}
if self.flags.ring_indicator() {
- il |= Interrupt::RI.0;
+ il |= Interrupt::RI;
}
self.int_level = il;
true
@@ -391,8 +391,8 @@ pub fn reset(&mut self) {
self.line_control.reset();
self.receive_status_error_clear.reset();
self.dmacr = 0;
- self.int_enabled = 0;
- self.int_level = 0;
+ self.int_enabled = 0.into();
+ self.int_level = 0.into();
self.ilpr = 0;
self.ibrd = 0;
self.fbrd = 0;
@@ -451,7 +451,7 @@ pub fn fifo_rx_put(&mut self, value: registers::Data) -> bool {
}
if self.read_count == self.read_trigger {
- self.int_level |= Interrupt::RX.0;
+ self.int_level |= Interrupt::RX;
return true;
}
false
@@ -632,7 +632,7 @@ fn update(&self) {
let regs = self.regs.borrow();
let flags = regs.int_level & regs.int_enabled;
for (irq, i) in self.interrupts.iter().zip(IRQMASK) {
- irq.set(flags & i != 0);
+ irq.set(flags.any_set(i));
}
}
@@ -642,14 +642,13 @@ pub fn post_load(&self, _version_id: u32) -> Result<(), ()> {
}
/// Which bits in the interrupt status matter for each outbound IRQ line ?
-const IRQMASK: [u32; 6] = [
- /* combined IRQ */
- Interrupt::E.0 | Interrupt::MS.0 | Interrupt::RT.0 | Interrupt::TX.0 | Interrupt::RX.0,
- Interrupt::RX.0,
- Interrupt::TX.0,
- Interrupt::RT.0,
- Interrupt::MS.0,
- Interrupt::E.0,
+const IRQMASK: [Interrupt; 6] = [
+ Interrupt::all(),
+ Interrupt::RX,
+ Interrupt::TX,
+ Interrupt::RT,
+ Interrupt::MS,
+ Interrupt::E,
];
/// # Safety
diff --git a/rust/hw/char/pl011/src/registers.rs b/rust/hw/char/pl011/src/registers.rs
index 690feb63785..7ececd39f86 100644
--- a/rust/hw/char/pl011/src/registers.rs
+++ b/rust/hw/char/pl011/src/registers.rs
@@ -9,7 +9,8 @@
// https://developer.arm.com/documentation/ddi0183/latest/
use bilge::prelude::*;
-use qemu_api::impl_vmstate_bitsized;
+use bits::bits;
+use qemu_api::{impl_vmstate_bitsized, impl_vmstate_forward};
/// Offset of each register from the base memory address of the device.
#[doc(alias = "offset")]
@@ -326,22 +327,24 @@ fn default() -> Self {
}
}
-/// Interrupt status bits in UARTRIS, UARTMIS, UARTIMSC
-pub struct Interrupt(pub u32);
+bits! {
+ /// Interrupt status bits in UARTRIS, UARTMIS, UARTIMSC
+ #[derive(Default)]
+ pub struct Interrupt(u32) {
+ OE = 1 << 10,
+ BE = 1 << 9,
+ PE = 1 << 8,
+ FE = 1 << 7,
+ RT = 1 << 6,
+ TX = 1 << 5,
+ RX = 1 << 4,
+ DSR = 1 << 3,
+ DCD = 1 << 2,
+ CTS = 1 << 1,
+ RI = 1 << 0,
-impl Interrupt {
- pub const OE: Self = Self(1 << 10);
- pub const BE: Self = Self(1 << 9);
- pub const PE: Self = Self(1 << 8);
- pub const FE: Self = Self(1 << 7);
- pub const RT: Self = Self(1 << 6);
- pub const TX: Self = Self(1 << 5);
- pub const RX: Self = Self(1 << 4);
- pub const DSR: Self = Self(1 << 3);
- pub const DCD: Self = Self(1 << 2);
- pub const CTS: Self = Self(1 << 1);
- pub const RI: Self = Self(1 << 0);
-
- pub const E: Self = Self(Self::OE.0 | Self::BE.0 | Self::PE.0 | Self::FE.0);
- pub const MS: Self = Self(Self::RI.0 | Self::DSR.0 | Self::DCD.0 | Self::CTS.0);
+ E = bits!(Self as u32: OE | BE | PE | FE),
+ MS = bits!(Self as u32: RI | DSR | DCD | CTS),
+ }
}
+impl_vmstate_forward!(Interrupt);
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 3/6] rust: qemu-api-macros: add from_bits and into_bits to #[derive(TryInto)]
2025-05-21 8:18 [RFC PATCH 0/6] rust: make usage of bitmasks and bitfields nicer Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 1/6] rust: add "bits", a custom bitflags implementation Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 2/6] rust: pl011: use the bits macro Paolo Bonzini
@ 2025-05-21 8:18 ` Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 4/6] rust: subprojects: add bitfield-struct Paolo Bonzini
` (2 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2025-05-21 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-rust, manos.pitsidianakis
These const functions make it possible to use enums easily together
with the bitfield-struct crate.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api-macros/src/lib.rs | 48 ++++++++++++++++++++++++++-------
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
index ceb79f09f97..103470785e3 100644
--- a/rust/qemu-api-macros/src/lib.rs
+++ b/rust/qemu-api-macros/src/lib.rs
@@ -193,23 +193,51 @@ fn get_variants(input: &DeriveInput) -> Result<&Punctuated<Variant, Comma>, Macr
}
#[rustfmt::skip::macros(quote)]
-fn derive_tryinto_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream, MacroError> {
- let repr = get_repr_uN(&input, "#[derive(TryInto)]")?;
-
- let name = &input.ident;
- let variants = get_variants(&input)?;
+fn derive_tryinto_body(
+ name: &Ident,
+ variants: &Punctuated<Variant, Comma>,
+ repr: &Path,
+) -> Result<proc_macro2::TokenStream, MacroError> {
let discriminants: Vec<&Ident> = variants.iter().map(|f| &f.ident).collect();
Ok(quote! {
+ #(const #discriminants: #repr = #name::#discriminants as #repr;)*;
+ match value {
+ #(#discriminants => Ok(#name::#discriminants),)*
+ _ => Err(value),
+ }
+ })
+}
+
+#[rustfmt::skip::macros(quote)]
+fn derive_tryinto_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream, MacroError> {
+ let repr = get_repr_uN(&input, "#[derive(TryInto)]")?;
+ let name = &input.ident;
+ let body = derive_tryinto_body(name, get_variants(&input)?, &repr)?;
+ let errmsg = format!("invalid value for {name}");
+
+ Ok(quote! {
+ impl #name {
+ #[allow(dead_code)]
+ pub const fn into_bits(self) -> #repr {
+ self as #repr
+ }
+
+ #[allow(dead_code)]
+ pub const fn from_bits(value: #repr) -> Self {
+ match ({
+ #body
+ }) {
+ Ok(x) => x,
+ Err(_) => panic!(#errmsg)
+ }
+ }
+ }
impl core::convert::TryFrom<#repr> for #name {
type Error = #repr;
fn try_from(value: #repr) -> Result<Self, Self::Error> {
- #(const #discriminants: #repr = #name::#discriminants as #repr;)*;
- match value {
- #(#discriminants => Ok(Self::#discriminants),)*
- _ => Err(value),
- }
+ #body
}
}
})
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 4/6] rust: subprojects: add bitfield-struct
2025-05-21 8:18 [RFC PATCH 0/6] rust: make usage of bitmasks and bitfields nicer Paolo Bonzini
` (2 preceding siblings ...)
2025-05-21 8:18 ` [RFC PATCH 3/6] rust: qemu-api-macros: add from_bits and into_bits to #[derive(TryInto)] Paolo Bonzini
@ 2025-05-21 8:18 ` Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 6/6] rust: remove bilge crate Paolo Bonzini
5 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2025-05-21 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-rust, manos.pitsidianakis
The bitfield-struct is much smaller than "bilge" and supports "const"
expressions better. The main disadvantage is that it does not let you
annotate enums as bitfields and does not integrate with arbitrary-int,
thus requiring manual size annotations for anything that is not a bool,
iNN or uNN.
Lack of support for arbitrary-int is a very minor change; enums are a
bit more annoying because they require manual implementation of two
functions from_bits() and into_bits(); however, that is already handled
by QEMU's bits! and #[derive(qemu_api_macros::TryInto)] utilities.
Together, these basically make the manual work go away.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/meson.build | 3 ++
subprojects/.gitignore | 1 +
subprojects/bitfield-struct-0.9-rs.wrap | 7 ++++
.../bitfield-struct-0.9-rs/meson.build | 36 +++++++++++++++++++
4 files changed, 47 insertions(+)
create mode 100644 subprojects/bitfield-struct-0.9-rs.wrap
create mode 100644 subprojects/packagefiles/bitfield-struct-0.9-rs/meson.build
diff --git a/rust/meson.build b/rust/meson.build
index f370b0ec939..58f14a70d6d 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -1,3 +1,6 @@
+subproject('bitfield-struct-0.9-rs', required: true)
+bitfield_struct_dep = dependency('bitfield-struct-0.9-rs')
+
subdir('qemu-api-macros')
subdir('bits')
subdir('qemu-api')
diff --git a/subprojects/.gitignore b/subprojects/.gitignore
index d12d34618cc..180c3134864 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -9,6 +9,7 @@
/arbitrary-int-1.2.7
/bilge-0.2.0
/bilge-impl-0.2.0
+/bitfield-struct-0.9.5
/either-1.12.0
/itertools-0.11.0
/libc-0.2.162
diff --git a/subprojects/bitfield-struct-0.9-rs.wrap b/subprojects/bitfield-struct-0.9-rs.wrap
new file mode 100644
index 00000000000..dfdbbc853af
--- /dev/null
+++ b/subprojects/bitfield-struct-0.9-rs.wrap
@@ -0,0 +1,7 @@
+[wrap-file]
+directory = bitfield-struct-0.9.5
+source_url = https://crates.io/api/v1/crates/bitfield-struct/0.9.5/download
+source_filename = bitfield-struct-0.9.5.tar.gz
+source_hash = b2869c63ccf4f8bf0d485070b880e60e097fb7aeea80ee82a0a94a957e372a0b
+#method = cargo
+patch_directory = bitfield-struct-0.9-rs
diff --git a/subprojects/packagefiles/bitfield-struct-0.9-rs/meson.build b/subprojects/packagefiles/bitfield-struct-0.9-rs/meson.build
new file mode 100644
index 00000000000..e8badb7da18
--- /dev/null
+++ b/subprojects/packagefiles/bitfield-struct-0.9-rs/meson.build
@@ -0,0 +1,36 @@
+project('bitfield-struct-0.9-rs', 'rust',
+ meson_version: '>=1.5.0',
+ version: '0.9.5',
+ license: 'MIT',
+ default_options: [])
+
+subproject('quote-1-rs', required: true)
+subproject('syn-2-rs', required: true)
+subproject('proc-macro2-1-rs', required: true)
+
+quote_dep = dependency('quote-1-rs', native: true)
+syn_dep = dependency('syn-2-rs', native: true)
+proc_macro2_dep = dependency('proc-macro2-1-rs', native: true)
+
+rust = import('rust')
+
+_bitfield_struct_rs = rust.proc_macro(
+ 'bitfield_struct',
+ files('src/lib.rs'),
+ override_options: ['rust_std=2021', 'build.rust_std=2021'],
+ rust_args: [
+ '--cap-lints', 'allow',
+ '--cfg', 'use_fallback',
+ ],
+ dependencies: [
+ quote_dep,
+ syn_dep,
+ proc_macro2_dep,
+ ],
+)
+
+bitfield_struct_dep = declare_dependency(
+ link_with: _bitfield_struct_rs,
+)
+
+meson.override_dependency('bitfield-struct-0.9-rs', bitfield_struct_dep)
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct
2025-05-21 8:18 [RFC PATCH 0/6] rust: make usage of bitmasks and bitfields nicer Paolo Bonzini
` (3 preceding siblings ...)
2025-05-21 8:18 ` [RFC PATCH 4/6] rust: subprojects: add bitfield-struct Paolo Bonzini
@ 2025-05-21 8:18 ` Paolo Bonzini
2025-05-21 9:21 ` Manos Pitsidianakis
2025-05-21 9:50 ` Alex Bennée
2025-05-21 8:18 ` [RFC PATCH 6/6] rust: remove bilge crate Paolo Bonzini
5 siblings, 2 replies; 16+ messages in thread
From: Paolo Bonzini @ 2025-05-21 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-rust, manos.pitsidianakis
The bilge crate, while very nice and espressive, is heavily reliant on
traits; because trait functions are never const, bilge and const mix
about as well as water and oil.
Try using the bitfield-struct crate instead. It is built to support
const very well and the only downside is that more manual annotations
are needed (for enums and non-full-byte members). Otherwise, the use
is pretty much the same and in fact device code does not change at all,
only register declarations.
Recent versions want to use Rust 1.83, so this uses a slightly older
version with basically no lost functionality; but anyway, I want to switch
to 1.83 for QEMU as well due to improved "const" support in the compiler.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/Cargo.toml | 1 +
rust/hw/char/pl011/Cargo.toml | 3 +-
rust/hw/char/pl011/meson.build | 11 +--
rust/hw/char/pl011/src/registers.rs | 108 ++++++++++++++--------------
4 files changed, 56 insertions(+), 67 deletions(-)
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 165328b6d01..3345858b5b4 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -97,5 +97,6 @@ used_underscore_binding = "deny"
#wildcard_imports = "deny" # still have many bindings::* imports
# these may have false positives
+enum_variant_names = "allow"
#option_if_let_else = "deny"
cognitive_complexity = "deny"
diff --git a/rust/hw/char/pl011/Cargo.toml b/rust/hw/char/pl011/Cargo.toml
index 003ef9613d4..97e3dd00c35 100644
--- a/rust/hw/char/pl011/Cargo.toml
+++ b/rust/hw/char/pl011/Cargo.toml
@@ -16,8 +16,7 @@ rust-version.workspace = true
crate-type = ["staticlib"]
[dependencies]
-bilge = { version = "0.2.0" }
-bilge-impl = { version = "0.2.0" }
+bitfield-struct = { version = "0.9" }
bits = { path = "../../../bits" }
qemu_api = { path = "../../../qemu-api" }
qemu_api_macros = { path = "../../../qemu-api-macros" }
diff --git a/rust/hw/char/pl011/meson.build b/rust/hw/char/pl011/meson.build
index f134a6cdc6b..1bae5a03310 100644
--- a/rust/hw/char/pl011/meson.build
+++ b/rust/hw/char/pl011/meson.build
@@ -1,17 +1,10 @@
-subproject('bilge-0.2-rs', required: true)
-subproject('bilge-impl-0.2-rs', required: true)
-
-bilge_dep = dependency('bilge-0.2-rs')
-bilge_impl_dep = dependency('bilge-impl-0.2-rs')
-
_libpl011_rs = static_library(
'pl011',
files('src/lib.rs'),
override_options: ['rust_std=2021', 'build.rust_std=2021'],
rust_abi: 'rust',
dependencies: [
- bilge_dep,
- bilge_impl_dep,
+ bitfield_struct_dep,
bits_rs,
qemu_api,
qemu_api_macros,
@@ -22,6 +15,6 @@ rust_devices_ss.add(when: 'CONFIG_X_PL011_RUST', if_true: [declare_dependency(
link_whole: [_libpl011_rs],
# Putting proc macro crates in `dependencies` is necessary for Meson to find
# them when compiling the root per-target static rust lib.
- dependencies: [bilge_impl_dep, qemu_api_macros],
+ dependencies: [bitfield_struct_dep, qemu_api_macros],
variables: {'crate': 'pl011'},
)])
diff --git a/rust/hw/char/pl011/src/registers.rs b/rust/hw/char/pl011/src/registers.rs
index 7ececd39f86..f2138c637c5 100644
--- a/rust/hw/char/pl011/src/registers.rs
+++ b/rust/hw/char/pl011/src/registers.rs
@@ -5,12 +5,16 @@
//! Device registers exposed as typed structs which are backed by arbitrary
//! integer bitmaps. [`Data`], [`Control`], [`LineControl`], etc.
+// rustc prefers "constant-like" enums to use upper case names, but that
+// is inconsistent in its own way.
+#![allow(non_upper_case_globals)]
+
// For more detail see the PL011 Technical Reference Manual DDI0183:
// https://developer.arm.com/documentation/ddi0183/latest/
-use bilge::prelude::*;
+use bitfield_struct::bitfield;
use bits::bits;
-use qemu_api::{impl_vmstate_bitsized, impl_vmstate_forward};
+use qemu_api::impl_vmstate_forward;
/// Offset of each register from the base memory address of the device.
#[doc(alias = "offset")]
@@ -78,14 +82,18 @@ pub enum RegisterOffset {
/// The `UARTRSR` register is updated only when a read occurs
/// from the `UARTDR` register with the same status information
/// that can also be obtained by reading the `UARTDR` register
-#[bitsize(8)]
-#[derive(Clone, Copy, Default, DebugBits, FromBits)]
+#[bitfield(u8)]
pub struct Errors {
pub framing_error: bool,
pub parity_error: bool,
pub break_error: bool,
pub overrun_error: bool,
- _reserved_unpredictable: u4,
+ #[bits(4)]
+ _reserved_unpredictable: u8,
+}
+
+impl Errors {
+ pub const BREAK: Self = Errors::new().with_break_error(true);
}
/// Data Register, `UARTDR`
@@ -93,19 +101,18 @@ pub struct Errors {
/// The `UARTDR` register is the data register; write for TX and
/// read for RX. It is a 12-bit register, where bits 7..0 are the
/// character and bits 11..8 are error bits.
-#[bitsize(32)]
-#[derive(Clone, Copy, Default, DebugBits, FromBits)]
+#[bitfield(u32)]
#[doc(alias = "UARTDR")]
pub struct Data {
pub data: u8,
+ #[bits(8)]
pub errors: Errors,
_reserved: u16,
}
-impl_vmstate_bitsized!(Data);
+impl_vmstate_forward!(Data);
impl Data {
- // bilge is not very const-friendly, unfortunately
- pub const BREAK: Self = Self { value: 1 << 10 };
+ pub const BREAK: Self = Self::new().with_errors(Errors::BREAK);
}
/// Receive Status Register / Error Clear Register, `UARTRSR/UARTECR`
@@ -119,13 +126,14 @@ impl Data {
/// and UARTECR for writes, but really it's a single error status
/// register where writing anything to the register clears the error
/// bits.
-#[bitsize(32)]
-#[derive(Clone, Copy, DebugBits, FromBits)]
+#[bitfield(u32)]
pub struct ReceiveStatusErrorClear {
+ #[bits(8)]
pub errors: Errors,
- _reserved_unpredictable: u24,
+ #[bits(24)]
+ _reserved_unpredictable: u32,
}
-impl_vmstate_bitsized!(ReceiveStatusErrorClear);
+impl_vmstate_forward!(ReceiveStatusErrorClear);
impl ReceiveStatusErrorClear {
pub fn set_from_data(&mut self, data: Data) {
@@ -138,14 +146,7 @@ pub fn reset(&mut self) {
}
}
-impl Default for ReceiveStatusErrorClear {
- fn default() -> Self {
- 0.into()
- }
-}
-
-#[bitsize(32)]
-#[derive(Clone, Copy, DebugBits, FromBits)]
+#[bitfield(u32, default = false)]
/// Flag Register, `UARTFR`
///
/// This has the usual inbound RS232 modem-control signals, plus flags
@@ -171,9 +172,10 @@ pub struct Flags {
pub transmit_fifo_empty: bool,
/// RI: Ring indicator
pub ring_indicator: bool,
- _reserved_zero_no_modify: u23,
+ #[bits(23)]
+ _reserved_zero_no_modify: u32,
}
-impl_vmstate_bitsized!(Flags);
+impl_vmstate_forward!(Flags);
impl Flags {
pub fn reset(&mut self) {
@@ -183,16 +185,14 @@ pub fn reset(&mut self) {
impl Default for Flags {
fn default() -> Self {
- let mut ret: Self = 0.into();
// After reset TXFF, RXFF, and BUSY are 0, and TXFE and RXFE are 1
- ret.set_receive_fifo_empty(true);
- ret.set_transmit_fifo_empty(true);
- ret
+ Self::from(0)
+ .with_receive_fifo_empty(true)
+ .with_transmit_fifo_empty(true)
}
}
-#[bitsize(32)]
-#[derive(Clone, Copy, DebugBits, FromBits)]
+#[bitfield(u32)]
/// Line Control Register, `UARTLCR_H`
#[doc(alias = "UARTLCR_H")]
pub struct LineControl {
@@ -201,48 +201,46 @@ pub struct LineControl {
/// PEN: Parity enable
pub parity_enabled: bool,
/// EPS: Even parity select
+ #[bits(1)]
pub parity: Parity,
/// STP2: Two stop bits select
pub two_stops_bits: bool,
/// FEN: Enable FIFOs
+ #[bits(1)]
pub fifos_enabled: Mode,
/// WLEN: Word length in bits
/// b11 = 8 bits
/// b10 = 7 bits
/// b01 = 6 bits
/// b00 = 5 bits.
+ #[bits(2)]
pub word_length: WordLength,
/// SPS Stick parity select
pub sticky_parity: bool,
/// 31:8 - Reserved, do not modify, read as zero.
- _reserved_zero_no_modify: u24,
+ #[bits(24)]
+ _reserved_zero_no_modify: u32,
}
-impl_vmstate_bitsized!(LineControl);
+impl_vmstate_forward!(LineControl);
impl LineControl {
pub fn reset(&mut self) {
// All the bits are cleared to 0 when reset.
- *self = 0.into();
+ *self = Self::default();
}
}
-impl Default for LineControl {
- fn default() -> Self {
- 0.into()
- }
-}
-
-#[bitsize(1)]
-#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
/// `EPS` "Even parity select", field of [Line Control
/// register](LineControl).
+#[repr(u8)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
pub enum Parity {
Odd = 0,
Even = 1,
}
-#[bitsize(1)]
-#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
+#[repr(u8)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
/// `FEN` "Enable FIFOs" or Device mode, field of [Line Control
/// register](LineControl).
pub enum Mode {
@@ -253,8 +251,8 @@ pub enum Mode {
FIFO = 1,
}
-#[bitsize(2)]
-#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
+#[repr(u8)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
/// `WLEN` Word length, field of [Line Control register](LineControl).
///
/// These bits indicate the number of data bits transmitted or received in a
@@ -275,9 +273,8 @@ pub enum WordLength {
/// The `UARTCR` register is the control register. It contains various
/// enable bits, and the bits to write to set the usual outbound RS232
/// modem control signals. All bits reset to 0 except TXE and RXE.
-#[bitsize(32)]
+#[bitfield(u32, default = false)]
#[doc(alias = "UARTCR")]
-#[derive(Clone, Copy, DebugBits, FromBits)]
pub struct Control {
/// `UARTEN` UART enable: 0 = UART is disabled.
pub enable_uart: bool,
@@ -285,9 +282,10 @@ pub struct Control {
/// QEMU does not model this.
pub enable_sir: bool,
/// `SIRLP` SIR low-power IrDA mode. QEMU does not model this.
- pub sir_lowpower_irda_mode: u1,
+ pub sir_lowpower_irda_mode: bool,
/// Reserved, do not modify, read as zero.
- _reserved_zero_no_modify: u4,
+ #[bits(4)]
+ _reserved_zero_no_modify: u8,
/// `LBE` Loopback enable: feed UART output back to the input
pub enable_loopback: bool,
/// `TXE` Transmit enable
@@ -309,21 +307,19 @@ pub struct Control {
/// 31:16 - Reserved, do not modify, read as zero.
_reserved_zero_no_modify2: u16,
}
-impl_vmstate_bitsized!(Control);
+impl_vmstate_forward!(Control);
impl Control {
pub fn reset(&mut self) {
- *self = 0.into();
- self.set_enable_receive(true);
- self.set_enable_transmit(true);
+ *self = Self::default();
}
}
impl Default for Control {
fn default() -> Self {
- let mut ret: Self = 0.into();
- ret.reset();
- ret
+ Self::from(0)
+ .with_enable_receive(true)
+ .with_enable_transmit(true)
}
}
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 6/6] rust: remove bilge crate
2025-05-21 8:18 [RFC PATCH 0/6] rust: make usage of bitmasks and bitfields nicer Paolo Bonzini
` (4 preceding siblings ...)
2025-05-21 8:18 ` [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct Paolo Bonzini
@ 2025-05-21 8:18 ` Paolo Bonzini
5 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2025-05-21 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-rust, manos.pitsidianakis
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/Cargo.lock | 71 ++-------------------
rust/qemu-api/src/vmstate.rs | 34 ++--------
subprojects/.gitignore | 7 --
subprojects/arbitrary-int-1-rs.wrap | 10 ---
subprojects/bilge-0.2-rs.wrap | 10 ---
subprojects/bilge-impl-0.2-rs.wrap | 10 ---
subprojects/either-1-rs.wrap | 10 ---
subprojects/itertools-0.11-rs.wrap | 10 ---
subprojects/proc-macro-error-1-rs.wrap | 10 ---
subprojects/proc-macro-error-attr-1-rs.wrap | 10 ---
10 files changed, 10 insertions(+), 172 deletions(-)
delete mode 100644 subprojects/arbitrary-int-1-rs.wrap
delete mode 100644 subprojects/bilge-0.2-rs.wrap
delete mode 100644 subprojects/bilge-impl-0.2-rs.wrap
delete mode 100644 subprojects/either-1-rs.wrap
delete mode 100644 subprojects/itertools-0.11-rs.wrap
delete mode 100644 subprojects/proc-macro-error-1-rs.wrap
delete mode 100644 subprojects/proc-macro-error-attr-1-rs.wrap
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index bccfe855a70..29ac523bfdb 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -3,29 +3,11 @@
version = 3
[[package]]
-name = "arbitrary-int"
-version = "1.2.7"
+name = "bitfield-struct"
+version = "0.9.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c84fc003e338a6f69fbd4f7fe9f92b535ff13e9af8997f3b14b6ddff8b1df46d"
-
-[[package]]
-name = "bilge"
-version = "0.2.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "dc707ed8ebf81de5cd6c7f48f54b4c8621760926cdf35a57000747c512e67b57"
+checksum = "b2869c63ccf4f8bf0d485070b880e60e097fb7aeea80ee82a0a94a957e372a0b"
dependencies = [
- "arbitrary-int",
- "bilge-impl",
-]
-
-[[package]]
-name = "bilge-impl"
-version = "0.2.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "feb11e002038ad243af39c2068c8a72bcf147acf05025dcdb916fcc000adb2d8"
-dependencies = [
- "itertools",
- "proc-macro-error",
"proc-macro2",
"quote",
"syn",
@@ -38,12 +20,6 @@ dependencies = [
"qemu_api_macros",
]
-[[package]]
-name = "either"
-version = "1.12.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b"
-
[[package]]
name = "hpet"
version = "0.1.0"
@@ -52,15 +28,6 @@ dependencies = [
"qemu_api_macros",
]
-[[package]]
-name = "itertools"
-version = "0.11.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b1c173a5686ce8bfa551b3563d0c2170bf24ca44da99c7ca4bfdab5418c3fe57"
-dependencies = [
- "either",
-]
-
[[package]]
name = "libc"
version = "0.2.162"
@@ -71,36 +38,12 @@ checksum = "18d287de67fe55fd7e1581fe933d965a5a9477b38e949cfa9f8574ef01506398"
name = "pl011"
version = "0.1.0"
dependencies = [
- "bilge",
- "bilge-impl",
+ "bitfield-struct",
"bits",
"qemu_api",
"qemu_api_macros",
]
-[[package]]
-name = "proc-macro-error"
-version = "1.0.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c"
-dependencies = [
- "proc-macro-error-attr",
- "proc-macro2",
- "quote",
- "version_check",
-]
-
-[[package]]
-name = "proc-macro-error-attr"
-version = "1.0.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869"
-dependencies = [
- "proc-macro2",
- "quote",
- "version_check",
-]
-
[[package]]
name = "proc-macro2"
version = "1.0.84"
@@ -152,9 +95,3 @@ name = "unicode-ident"
version = "1.0.12"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b"
-
-[[package]]
-name = "version_check"
-version = "0.9.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f"
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 9c8b2398e9d..38faf6ce0dc 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -11,10 +11,9 @@
//! migration format for a struct. This is based on the [`VMState`] trait,
//! which is defined by all migrateable types.
//!
-//! * [`impl_vmstate_forward`](crate::impl_vmstate_forward) and
-//! [`impl_vmstate_bitsized`](crate::impl_vmstate_bitsized), which help with
-//! the definition of the [`VMState`] trait (respectively for transparent
-//! structs and for `bilge`-defined types)
+//! * [`impl_vmstate_forward`](crate::impl_vmstate_forward), which help with the
+//! definition of the [`VMState`] trait (respectively for transparent structs
+//! and for `bilge`-defined types)
//!
//! * helper macros to declare a device model state struct, in particular
//! [`vmstate_subsections`](crate::vmstate_subsections) and
@@ -141,7 +140,7 @@ macro_rules! info_enum_to_ref {
/// The contents of this trait go straight into structs that are parsed by C
/// code and used to introspect into other structs. Generally, you don't need
/// to implement it except via macros that do it for you, such as
-/// `impl_vmstate_bitsized!`.
+/// `impl_vmstate_forward!`.
pub unsafe trait VMState {
/// The `info` member of a `VMStateField` is a pointer and as such cannot
/// yet be included in the [`BASE`](VMState::BASE) associated constant;
@@ -195,9 +194,8 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateFlags
/// * an array of any of the above
///
/// In order to support other types, the trait `VMState` must be implemented
-/// for them. The macros
-/// [`impl_vmstate_bitsized!`](crate::impl_vmstate_bitsized)
-/// and [`impl_vmstate_forward!`](crate::impl_vmstate_forward) help with this.
+/// for them. The macro
+/// [`impl_vmstate_forward!`](crate::impl_vmstate_forward) helps with this.
#[macro_export]
macro_rules! vmstate_of {
($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])? $(, $test_fn:expr)? $(,)?) => {
@@ -345,26 +343,6 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
impl_vmstate_transparent!(crate::cell::BqlRefCell<T> where T: VMState);
impl_vmstate_transparent!(crate::cell::Opaque<T> where T: VMState);
-#[macro_export]
-macro_rules! impl_vmstate_bitsized {
- ($type:ty) => {
- unsafe impl $crate::vmstate::VMState for $type {
- const SCALAR_TYPE: $crate::vmstate::VMStateFieldType =
- <<<$type as ::bilge::prelude::Bitsized>::ArbitraryInt
- as ::bilge::prelude::Number>::UnderlyingType
- as $crate::vmstate::VMState>::SCALAR_TYPE;
- const BASE: $crate::bindings::VMStateField =
- <<<$type as ::bilge::prelude::Bitsized>::ArbitraryInt
- as ::bilge::prelude::Number>::UnderlyingType
- as $crate::vmstate::VMState>::BASE;
- const VARRAY_FLAG: $crate::bindings::VMStateFlags =
- <<<$type as ::bilge::prelude::Bitsized>::ArbitraryInt
- as ::bilge::prelude::Number>::UnderlyingType
- as $crate::vmstate::VMState>::VARRAY_FLAG;
- }
- };
-}
-
// Scalar types using predefined VMStateInfos
macro_rules! impl_vmstate_scalar {
diff --git a/subprojects/.gitignore b/subprojects/.gitignore
index 180c3134864..183700d986e 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -6,15 +6,8 @@
/keycodemapdb
/libvfio-user
/slirp
-/arbitrary-int-1.2.7
-/bilge-0.2.0
-/bilge-impl-0.2.0
/bitfield-struct-0.9.5
-/either-1.12.0
-/itertools-0.11.0
/libc-0.2.162
-/proc-macro-error-1.0.4
-/proc-macro-error-attr-1.0.4
/proc-macro2-1.0.84
/quote-1.0.36
/syn-2.0.66
diff --git a/subprojects/arbitrary-int-1-rs.wrap b/subprojects/arbitrary-int-1-rs.wrap
deleted file mode 100644
index a1838b20b0f..00000000000
--- a/subprojects/arbitrary-int-1-rs.wrap
+++ /dev/null
@@ -1,10 +0,0 @@
-[wrap-file]
-directory = arbitrary-int-1.2.7
-source_url = https://crates.io/api/v1/crates/arbitrary-int/1.2.7/download
-source_filename = arbitrary-int-1.2.7.tar.gz
-source_hash = c84fc003e338a6f69fbd4f7fe9f92b535ff13e9af8997f3b14b6ddff8b1df46d
-#method = cargo
-patch_directory = arbitrary-int-1-rs
-
-# bump this version number on every change to meson.build or the patches:
-# v2
diff --git a/subprojects/bilge-0.2-rs.wrap b/subprojects/bilge-0.2-rs.wrap
deleted file mode 100644
index 900bb1497b9..00000000000
--- a/subprojects/bilge-0.2-rs.wrap
+++ /dev/null
@@ -1,10 +0,0 @@
-[wrap-file]
-directory = bilge-0.2.0
-source_url = https://crates.io/api/v1/crates/bilge/0.2.0/download
-source_filename = bilge-0.2.0.tar.gz
-source_hash = dc707ed8ebf81de5cd6c7f48f54b4c8621760926cdf35a57000747c512e67b57
-#method = cargo
-patch_directory = bilge-0.2-rs
-
-# bump this version number on every change to meson.build or the patches:
-# v2
diff --git a/subprojects/bilge-impl-0.2-rs.wrap b/subprojects/bilge-impl-0.2-rs.wrap
deleted file mode 100644
index 4f84eca1ccd..00000000000
--- a/subprojects/bilge-impl-0.2-rs.wrap
+++ /dev/null
@@ -1,10 +0,0 @@
-[wrap-file]
-directory = bilge-impl-0.2.0
-source_url = https://crates.io/api/v1/crates/bilge-impl/0.2.0/download
-source_filename = bilge-impl-0.2.0.tar.gz
-source_hash = feb11e002038ad243af39c2068c8a72bcf147acf05025dcdb916fcc000adb2d8
-#method = cargo
-patch_directory = bilge-impl-0.2-rs
-
-# bump this version number on every change to meson.build or the patches:
-# v2
diff --git a/subprojects/either-1-rs.wrap b/subprojects/either-1-rs.wrap
deleted file mode 100644
index 352e11cfee6..00000000000
--- a/subprojects/either-1-rs.wrap
+++ /dev/null
@@ -1,10 +0,0 @@
-[wrap-file]
-directory = either-1.12.0
-source_url = https://crates.io/api/v1/crates/either/1.12.0/download
-source_filename = either-1.12.0.tar.gz
-source_hash = 3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b
-#method = cargo
-patch_directory = either-1-rs
-
-# bump this version number on every change to meson.build or the patches:
-# v2
diff --git a/subprojects/itertools-0.11-rs.wrap b/subprojects/itertools-0.11-rs.wrap
deleted file mode 100644
index ee12d0053bc..00000000000
--- a/subprojects/itertools-0.11-rs.wrap
+++ /dev/null
@@ -1,10 +0,0 @@
-[wrap-file]
-directory = itertools-0.11.0
-source_url = https://crates.io/api/v1/crates/itertools/0.11.0/download
-source_filename = itertools-0.11.0.tar.gz
-source_hash = b1c173a5686ce8bfa551b3563d0c2170bf24ca44da99c7ca4bfdab5418c3fe57
-#method = cargo
-patch_directory = itertools-0.11-rs
-
-# bump this version number on every change to meson.build or the patches:
-# v2
diff --git a/subprojects/proc-macro-error-1-rs.wrap b/subprojects/proc-macro-error-1-rs.wrap
deleted file mode 100644
index 59f892f7825..00000000000
--- a/subprojects/proc-macro-error-1-rs.wrap
+++ /dev/null
@@ -1,10 +0,0 @@
-[wrap-file]
-directory = proc-macro-error-1.0.4
-source_url = https://crates.io/api/v1/crates/proc-macro-error/1.0.4/download
-source_filename = proc-macro-error-1.0.4.tar.gz
-source_hash = da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c
-#method = cargo
-patch_directory = proc-macro-error-1-rs
-
-# bump this version number on every change to meson.build or the patches:
-# v2
diff --git a/subprojects/proc-macro-error-attr-1-rs.wrap b/subprojects/proc-macro-error-attr-1-rs.wrap
deleted file mode 100644
index 5aeb224a103..00000000000
--- a/subprojects/proc-macro-error-attr-1-rs.wrap
+++ /dev/null
@@ -1,10 +0,0 @@
-[wrap-file]
-directory = proc-macro-error-attr-1.0.4
-source_url = https://crates.io/api/v1/crates/proc-macro-error-attr/1.0.4/download
-source_filename = proc-macro-error-attr-1.0.4.tar.gz
-source_hash = a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869
-#method = cargo
-patch_directory = proc-macro-error-attr-1-rs
-
-# bump this version number on every change to meson.build or the patches:
-# v2
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/6] rust: add "bits", a custom bitflags implementation
2025-05-21 8:18 ` [RFC PATCH 1/6] rust: add "bits", a custom bitflags implementation Paolo Bonzini
@ 2025-05-21 8:29 ` Daniel P. Berrangé
2025-05-21 9:23 ` Manos Pitsidianakis
0 siblings, 1 reply; 16+ messages in thread
From: Daniel P. Berrangé @ 2025-05-21 8:29 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, peter.maydell, qemu-rust, manos.pitsidianakis
On Wed, May 21, 2025 at 10:18:40AM +0200, Paolo Bonzini wrote:
> One common thing that device emulation does is manipulate bitmasks, for example
> to check whether two bitmaps have common bits. One example in the pl011 crate
> is the checks for pending interrupts, where an interrupt cause corresponds to
> at least one interrupt source from a fixed set.
>
> Unfortunately, this is one case where Rust *can* provide some kind of
> abstraction but it does so with a rather Perl-ish There Is More Way To
> Do It. It is not something where a crate like "bilge" helps, because
> it only covers the packing of bits in a structure; operations like "are
> all bits of Y set in X" almost never make sense for bit-packed structs;
> you need something else, there are several crates that do it and of course
> we're going to roll our own.
>
> In particular I examined three:
>
> - bitmask (https://docs.rs/bitmask/0.5.0/bitmask/) does not support const
> at all. This is a showstopper because one of the ugly things in the
> current pl011 code is the ugliness of code that defines interrupt masks
> at compile time:
>
> pub const E: Self = Self(Self::OE.0 | Self::BE.0 | Self::PE.0 | Self::FE.0);
>
> or even worse:
>
> const IRQMASK: [u32; 6] = [
> Interrupt::E.0 | Interrupt::MS.0 | Interrupt::RT.0 | Interrupt::TX.0 | Interrupt::RX.0,
> ...
> }
>
> You would have to use roughly the same code---"bitmask" only helps with
> defining the struct.
>
> - bitmask_enum (https://docs.rs/bitmask-enum/2.2.5/bitmask_enum/) does not
> have a good separation of "valid" and "invalid" bits, so for example "!x"
> will invert all 16 bits if you choose u16 as the representation -- even if
> you only defined 10 bits. This makes it easier to introduce subtle bugs
> in comparisons.
>
> - bitflags (https://docs.rs/bitflags/2.6.0/bitflags/) is generally the most
> used such crate and is the one that I took most inspiration from with
> respect to the syntax. It's a pretty sophisticated implementation,
> with a lot of bells and whistles such as an implementation of "Iter"
> that returns the bits one at a time.
>
> The main thing that all of them lack, however, is a way to simplify the
> ugly definitions like the above. "bitflags" includes const methods that
> perform AND/OR/XOR of masks (these are necessary because Rust operator
> overloading does not support const yet, and therefore overloaded operators
> cannot be used in the definition of a "static" variable), but they become
> even more verbose and unmanageable, like
>
> Interrupt::E.union(Interrupt::MS).union(Interrupt::RT).union(Interrupt::TX).union(Interrupt::RX)
>
> This was the main reason to create "bits", which allows something like
>
> bits!(Interrupt: E | MS | RT | TX | RX)
>
> and expands it 1) add "Interrupt::" in front of all identifiers 2) convert
> operators to the wordy const functions like "union". It supports boolean
> operators "&", "|", "^", "!" and parentheses, with a relatively simple
> recursive descent parser that's implemented in qemu_api_macros.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/Cargo.lock | 7 +
> rust/Cargo.toml | 1 +
> rust/bits/Cargo.toml | 19 ++
> rust/bits/meson.build | 12 +
> rust/bits/src/lib.rs | 441 +++++++++++++++++++++++++++++++
> rust/meson.build | 1 +
> rust/qemu-api-macros/src/bits.rs | 227 ++++++++++++++++
> rust/qemu-api-macros/src/lib.rs | 12 +
> 8 files changed, 720 insertions(+)
> create mode 100644 rust/bits/Cargo.toml
> create mode 100644 rust/bits/meson.build
> create mode 100644 rust/bits/src/lib.rs
> create mode 100644 rust/qemu-api-macros/src/bits.rs
> diff --git a/rust/bits/src/lib.rs b/rust/bits/src/lib.rs
> new file mode 100644
> index 00000000000..d80a6263f1e
> --- /dev/null
> +++ b/rust/bits/src/lib.rs
> @@ -0,0 +1,441 @@
This (and other new .rs files) needs SPDX-License-Identifier
> +/// # Definition entry point
> +///
> +/// Define a struct with a single field of type $type. Include public constants
> +/// for each element listed in braces.
> +///
> +/// The unnamed element at the end, if present, can be used to enlarge the set
> +/// of valid bits. Bits that are valid but not listed are treated normally for
> +/// the purpose of arithmetic operations, and are printed with their hexadecimal
> +/// value.
> +///
> +/// The struct implements the following traits: [`BitAnd`](std::ops::BitAnd),
> +/// [`BitOr`](std::ops::BitOr), [`BitXor`](std::ops::BitXor),
> +/// [`Not`](std::ops::Not), [`Sub`](std::ops::Sub); [`Debug`](std::fmt::Debug),
> +/// [`Display`](std::fmt::Display), [`Binary`](std::fmt::Binary),
> +/// [`Octal`](std::fmt::Octal), [`LowerHex`](std::fmt::LowerHex),
> +/// [`UpperHex`](std::fmt::UpperHex); [`From`]`<type>`/[`Into`]`<type>` where
> +/// type is the type specified in the definition.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct
2025-05-21 8:18 ` [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct Paolo Bonzini
@ 2025-05-21 9:21 ` Manos Pitsidianakis
2025-05-21 9:40 ` Manos Pitsidianakis
2025-05-21 13:51 ` Paolo Bonzini
2025-05-21 9:50 ` Alex Bennée
1 sibling, 2 replies; 16+ messages in thread
From: Manos Pitsidianakis @ 2025-05-21 9:21 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, peter.maydell, qemu-rust
On Wed, May 21, 2025 at 11:19 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The bilge crate, while very nice and espressive, is heavily reliant on
> traits; because trait functions are never const, bilge and const mix
> about as well as water and oil.
>
> Try using the bitfield-struct crate instead. It is built to support
> const very well and the only downside is that more manual annotations
> are needed (for enums and non-full-byte members). Otherwise, the use
> is pretty much the same and in fact device code does not change at all,
> only register declarations.
>
> Recent versions want to use Rust 1.83, so this uses a slightly older
> version with basically no lost functionality; but anyway, I want to switch
> to 1.83 for QEMU as well due to improved "const" support in the compiler.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/Cargo.toml | 1 +
> rust/hw/char/pl011/Cargo.toml | 3 +-
> rust/hw/char/pl011/meson.build | 11 +--
> rust/hw/char/pl011/src/registers.rs | 108 ++++++++++++++--------------
> 4 files changed, 56 insertions(+), 67 deletions(-)
>
> diff --git a/rust/Cargo.toml b/rust/Cargo.toml
> index 165328b6d01..3345858b5b4 100644
> --- a/rust/Cargo.toml
> +++ b/rust/Cargo.toml
> @@ -97,5 +97,6 @@ used_underscore_binding = "deny"
> #wildcard_imports = "deny" # still have many bindings::* imports
>
> # these may have false positives
> +enum_variant_names = "allow"
> #option_if_let_else = "deny"
> cognitive_complexity = "deny"
> diff --git a/rust/hw/char/pl011/Cargo.toml b/rust/hw/char/pl011/Cargo.toml
> index 003ef9613d4..97e3dd00c35 100644
> --- a/rust/hw/char/pl011/Cargo.toml
> +++ b/rust/hw/char/pl011/Cargo.toml
> @@ -16,8 +16,7 @@ rust-version.workspace = true
> crate-type = ["staticlib"]
>
> [dependencies]
> -bilge = { version = "0.2.0" }
> -bilge-impl = { version = "0.2.0" }
> +bitfield-struct = { version = "0.9" }
> bits = { path = "../../../bits" }
> qemu_api = { path = "../../../qemu-api" }
> qemu_api_macros = { path = "../../../qemu-api-macros" }
> diff --git a/rust/hw/char/pl011/meson.build b/rust/hw/char/pl011/meson.build
> index f134a6cdc6b..1bae5a03310 100644
> --- a/rust/hw/char/pl011/meson.build
> +++ b/rust/hw/char/pl011/meson.build
> @@ -1,17 +1,10 @@
> -subproject('bilge-0.2-rs', required: true)
> -subproject('bilge-impl-0.2-rs', required: true)
> -
> -bilge_dep = dependency('bilge-0.2-rs')
> -bilge_impl_dep = dependency('bilge-impl-0.2-rs')
> -
> _libpl011_rs = static_library(
> 'pl011',
> files('src/lib.rs'),
> override_options: ['rust_std=2021', 'build.rust_std=2021'],
> rust_abi: 'rust',
> dependencies: [
> - bilge_dep,
> - bilge_impl_dep,
> + bitfield_struct_dep,
> bits_rs,
> qemu_api,
> qemu_api_macros,
> @@ -22,6 +15,6 @@ rust_devices_ss.add(when: 'CONFIG_X_PL011_RUST', if_true: [declare_dependency(
> link_whole: [_libpl011_rs],
> # Putting proc macro crates in `dependencies` is necessary for Meson to find
> # them when compiling the root per-target static rust lib.
> - dependencies: [bilge_impl_dep, qemu_api_macros],
> + dependencies: [bitfield_struct_dep, qemu_api_macros],
> variables: {'crate': 'pl011'},
> )])
> diff --git a/rust/hw/char/pl011/src/registers.rs b/rust/hw/char/pl011/src/registers.rs
> index 7ececd39f86..f2138c637c5 100644
> --- a/rust/hw/char/pl011/src/registers.rs
> +++ b/rust/hw/char/pl011/src/registers.rs
> @@ -5,12 +5,16 @@
> //! Device registers exposed as typed structs which are backed by arbitrary
> //! integer bitmaps. [`Data`], [`Control`], [`LineControl`], etc.
>
> +// rustc prefers "constant-like" enums to use upper case names, but that
> +// is inconsistent in its own way.
> +#![allow(non_upper_case_globals)]
> +
> // For more detail see the PL011 Technical Reference Manual DDI0183:
> // https://developer.arm.com/documentation/ddi0183/latest/
>
> -use bilge::prelude::*;
> +use bitfield_struct::bitfield;
> use bits::bits;
> -use qemu_api::{impl_vmstate_bitsized, impl_vmstate_forward};
> +use qemu_api::impl_vmstate_forward;
>
> /// Offset of each register from the base memory address of the device.
> #[doc(alias = "offset")]
> @@ -78,14 +82,18 @@ pub enum RegisterOffset {
> /// The `UARTRSR` register is updated only when a read occurs
> /// from the `UARTDR` register with the same status information
> /// that can also be obtained by reading the `UARTDR` register
> -#[bitsize(8)]
> -#[derive(Clone, Copy, Default, DebugBits, FromBits)]
> +#[bitfield(u8)]
> pub struct Errors {
> pub framing_error: bool,
> pub parity_error: bool,
> pub break_error: bool,
> pub overrun_error: bool,
> - _reserved_unpredictable: u4,
> + #[bits(4)]
> + _reserved_unpredictable: u8,
> +}
> +
> +impl Errors {
> + pub const BREAK: Self = Errors::new().with_break_error(true);
> }
>
> /// Data Register, `UARTDR`
> @@ -93,19 +101,18 @@ pub struct Errors {
> /// The `UARTDR` register is the data register; write for TX and
> /// read for RX. It is a 12-bit register, where bits 7..0 are the
> /// character and bits 11..8 are error bits.
> -#[bitsize(32)]
> -#[derive(Clone, Copy, Default, DebugBits, FromBits)]
> +#[bitfield(u32)]
> #[doc(alias = "UARTDR")]
> pub struct Data {
> pub data: u8,
> + #[bits(8)]
> pub errors: Errors,
> _reserved: u16,
> }
> -impl_vmstate_bitsized!(Data);
> +impl_vmstate_forward!(Data);
>
> impl Data {
> - // bilge is not very const-friendly, unfortunately
> - pub const BREAK: Self = Self { value: 1 << 10 };
> + pub const BREAK: Self = Self::new().with_errors(Errors::BREAK);
> }
>
> /// Receive Status Register / Error Clear Register, `UARTRSR/UARTECR`
> @@ -119,13 +126,14 @@ impl Data {
> /// and UARTECR for writes, but really it's a single error status
> /// register where writing anything to the register clears the error
> /// bits.
> -#[bitsize(32)]
> -#[derive(Clone, Copy, DebugBits, FromBits)]
> +#[bitfield(u32)]
> pub struct ReceiveStatusErrorClear {
> + #[bits(8)]
> pub errors: Errors,
> - _reserved_unpredictable: u24,
> + #[bits(24)]
> + _reserved_unpredictable: u32,
> }
> -impl_vmstate_bitsized!(ReceiveStatusErrorClear);
> +impl_vmstate_forward!(ReceiveStatusErrorClear);
>
> impl ReceiveStatusErrorClear {
> pub fn set_from_data(&mut self, data: Data) {
> @@ -138,14 +146,7 @@ pub fn reset(&mut self) {
> }
> }
>
> -impl Default for ReceiveStatusErrorClear {
> - fn default() -> Self {
> - 0.into()
> - }
> -}
> -
> -#[bitsize(32)]
> -#[derive(Clone, Copy, DebugBits, FromBits)]
> +#[bitfield(u32, default = false)]
> /// Flag Register, `UARTFR`
> ///
> /// This has the usual inbound RS232 modem-control signals, plus flags
> @@ -171,9 +172,10 @@ pub struct Flags {
> pub transmit_fifo_empty: bool,
> /// RI: Ring indicator
> pub ring_indicator: bool,
> - _reserved_zero_no_modify: u23,
> + #[bits(23)]
> + _reserved_zero_no_modify: u32,
> }
> -impl_vmstate_bitsized!(Flags);
> +impl_vmstate_forward!(Flags);
>
> impl Flags {
> pub fn reset(&mut self) {
> @@ -183,16 +185,14 @@ pub fn reset(&mut self) {
>
> impl Default for Flags {
> fn default() -> Self {
> - let mut ret: Self = 0.into();
> // After reset TXFF, RXFF, and BUSY are 0, and TXFE and RXFE are 1
> - ret.set_receive_fifo_empty(true);
> - ret.set_transmit_fifo_empty(true);
> - ret
> + Self::from(0)
> + .with_receive_fifo_empty(true)
> + .with_transmit_fifo_empty(true)
> }
> }
>
> -#[bitsize(32)]
> -#[derive(Clone, Copy, DebugBits, FromBits)]
> +#[bitfield(u32)]
> /// Line Control Register, `UARTLCR_H`
> #[doc(alias = "UARTLCR_H")]
> pub struct LineControl {
> @@ -201,48 +201,46 @@ pub struct LineControl {
> /// PEN: Parity enable
> pub parity_enabled: bool,
> /// EPS: Even parity select
> + #[bits(1)]
> pub parity: Parity,
> /// STP2: Two stop bits select
> pub two_stops_bits: bool,
> /// FEN: Enable FIFOs
> + #[bits(1)]
> pub fifos_enabled: Mode,
> /// WLEN: Word length in bits
> /// b11 = 8 bits
> /// b10 = 7 bits
> /// b01 = 6 bits
> /// b00 = 5 bits.
> + #[bits(2)]
> pub word_length: WordLength,
> /// SPS Stick parity select
> pub sticky_parity: bool,
> /// 31:8 - Reserved, do not modify, read as zero.
> - _reserved_zero_no_modify: u24,
> + #[bits(24)]
> + _reserved_zero_no_modify: u32,
> }
> -impl_vmstate_bitsized!(LineControl);
> +impl_vmstate_forward!(LineControl);
>
> impl LineControl {
> pub fn reset(&mut self) {
> // All the bits are cleared to 0 when reset.
> - *self = 0.into();
> + *self = Self::default();
> }
> }
>
> -impl Default for LineControl {
> - fn default() -> Self {
> - 0.into()
> - }
> -}
> -
> -#[bitsize(1)]
> -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> /// `EPS` "Even parity select", field of [Line Control
> /// register](LineControl).
> +#[repr(u8)]
> +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> pub enum Parity {
> Odd = 0,
> Even = 1,
> }
>
> -#[bitsize(1)]
> -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> +#[repr(u8)]
> +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> /// `FEN` "Enable FIFOs" or Device mode, field of [Line Control
> /// register](LineControl).
> pub enum Mode {
> @@ -253,8 +251,8 @@ pub enum Mode {
> FIFO = 1,
> }
>
> -#[bitsize(2)]
> -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> +#[repr(u8)]
> +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> /// `WLEN` Word length, field of [Line Control register](LineControl).
> ///
> /// These bits indicate the number of data bits transmitted or received in a
> @@ -275,9 +273,8 @@ pub enum WordLength {
> /// The `UARTCR` register is the control register. It contains various
> /// enable bits, and the bits to write to set the usual outbound RS232
> /// modem control signals. All bits reset to 0 except TXE and RXE.
> -#[bitsize(32)]
> +#[bitfield(u32, default = false)]
> #[doc(alias = "UARTCR")]
> -#[derive(Clone, Copy, DebugBits, FromBits)]
> pub struct Control {
> /// `UARTEN` UART enable: 0 = UART is disabled.
> pub enable_uart: bool,
> @@ -285,9 +282,10 @@ pub struct Control {
> /// QEMU does not model this.
> pub enable_sir: bool,
> /// `SIRLP` SIR low-power IrDA mode. QEMU does not model this.
> - pub sir_lowpower_irda_mode: u1,
> + pub sir_lowpower_irda_mode: bool,
> /// Reserved, do not modify, read as zero.
> - _reserved_zero_no_modify: u4,
> + #[bits(4)]
> + _reserved_zero_no_modify: u8,
> /// `LBE` Loopback enable: feed UART output back to the input
> pub enable_loopback: bool,
> /// `TXE` Transmit enable
> @@ -309,21 +307,19 @@ pub struct Control {
> /// 31:16 - Reserved, do not modify, read as zero.
> _reserved_zero_no_modify2: u16,
> }
> -impl_vmstate_bitsized!(Control);
> +impl_vmstate_forward!(Control);
>
> impl Control {
> pub fn reset(&mut self) {
> - *self = 0.into();
> - self.set_enable_receive(true);
> - self.set_enable_transmit(true);
> + *self = Self::default();
> }
> }
>
> impl Default for Control {
> fn default() -> Self {
> - let mut ret: Self = 0.into();
> - ret.reset();
> - ret
> + Self::from(0)
> + .with_enable_receive(true)
> + .with_enable_transmit(true)
> }
> }
>
> --
> 2.49.0
>
Perhaps it'd be simpler to contribute const-ability to upstream bilge?
Is From/Into the only problem trait? I was thinking we can generate
from/into associated methods for each type that are const. It'd not
even be a big change and we can carry it as a patch until we can
catchup with upstream crates.io version in subprojects/. WDYT?
--
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/6] rust: add "bits", a custom bitflags implementation
2025-05-21 8:29 ` Daniel P. Berrangé
@ 2025-05-21 9:23 ` Manos Pitsidianakis
2025-05-21 15:05 ` Daniel P. Berrangé
0 siblings, 1 reply; 16+ messages in thread
From: Manos Pitsidianakis @ 2025-05-21 9:23 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Paolo Bonzini, qemu-devel, peter.maydell, qemu-rust
On Wed, May 21, 2025 at 11:29 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, May 21, 2025 at 10:18:40AM +0200, Paolo Bonzini wrote:
> > One common thing that device emulation does is manipulate bitmasks, for example
> > to check whether two bitmaps have common bits. One example in the pl011 crate
> > is the checks for pending interrupts, where an interrupt cause corresponds to
> > at least one interrupt source from a fixed set.
> >
> > Unfortunately, this is one case where Rust *can* provide some kind of
> > abstraction but it does so with a rather Perl-ish There Is More Way To
> > Do It. It is not something where a crate like "bilge" helps, because
> > it only covers the packing of bits in a structure; operations like "are
> > all bits of Y set in X" almost never make sense for bit-packed structs;
> > you need something else, there are several crates that do it and of course
> > we're going to roll our own.
> >
> > In particular I examined three:
> >
> > - bitmask (https://docs.rs/bitmask/0.5.0/bitmask/) does not support const
> > at all. This is a showstopper because one of the ugly things in the
> > current pl011 code is the ugliness of code that defines interrupt masks
> > at compile time:
> >
> > pub const E: Self = Self(Self::OE.0 | Self::BE.0 | Self::PE.0 | Self::FE.0);
> >
> > or even worse:
> >
> > const IRQMASK: [u32; 6] = [
> > Interrupt::E.0 | Interrupt::MS.0 | Interrupt::RT.0 | Interrupt::TX.0 | Interrupt::RX.0,
> > ...
> > }
> >
> > You would have to use roughly the same code---"bitmask" only helps with
> > defining the struct.
> >
> > - bitmask_enum (https://docs.rs/bitmask-enum/2.2.5/bitmask_enum/) does not
> > have a good separation of "valid" and "invalid" bits, so for example "!x"
> > will invert all 16 bits if you choose u16 as the representation -- even if
> > you only defined 10 bits. This makes it easier to introduce subtle bugs
> > in comparisons.
> >
> > - bitflags (https://docs.rs/bitflags/2.6.0/bitflags/) is generally the most
> > used such crate and is the one that I took most inspiration from with
> > respect to the syntax. It's a pretty sophisticated implementation,
> > with a lot of bells and whistles such as an implementation of "Iter"
> > that returns the bits one at a time.
> >
> > The main thing that all of them lack, however, is a way to simplify the
> > ugly definitions like the above. "bitflags" includes const methods that
> > perform AND/OR/XOR of masks (these are necessary because Rust operator
> > overloading does not support const yet, and therefore overloaded operators
> > cannot be used in the definition of a "static" variable), but they become
> > even more verbose and unmanageable, like
> >
> > Interrupt::E.union(Interrupt::MS).union(Interrupt::RT).union(Interrupt::TX).union(Interrupt::RX)
> >
> > This was the main reason to create "bits", which allows something like
> >
> > bits!(Interrupt: E | MS | RT | TX | RX)
> >
> > and expands it 1) add "Interrupt::" in front of all identifiers 2) convert
> > operators to the wordy const functions like "union". It supports boolean
> > operators "&", "|", "^", "!" and parentheses, with a relatively simple
> > recursive descent parser that's implemented in qemu_api_macros.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > rust/Cargo.lock | 7 +
> > rust/Cargo.toml | 1 +
> > rust/bits/Cargo.toml | 19 ++
> > rust/bits/meson.build | 12 +
> > rust/bits/src/lib.rs | 441 +++++++++++++++++++++++++++++++
> > rust/meson.build | 1 +
> > rust/qemu-api-macros/src/bits.rs | 227 ++++++++++++++++
> > rust/qemu-api-macros/src/lib.rs | 12 +
> > 8 files changed, 720 insertions(+)
> > create mode 100644 rust/bits/Cargo.toml
> > create mode 100644 rust/bits/meson.build
> > create mode 100644 rust/bits/src/lib.rs
> > create mode 100644 rust/qemu-api-macros/src/bits.rs
>
> > diff --git a/rust/bits/src/lib.rs b/rust/bits/src/lib.rs
> > new file mode 100644
> > index 00000000000..d80a6263f1e
> > --- /dev/null
> > +++ b/rust/bits/src/lib.rs
> > @@ -0,0 +1,441 @@
>
> This (and other new .rs files) needs SPDX-License-Identifier
We should probably lint for this in .rs files.
--
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct
2025-05-21 9:21 ` Manos Pitsidianakis
@ 2025-05-21 9:40 ` Manos Pitsidianakis
2025-05-21 13:51 ` Paolo Bonzini
1 sibling, 0 replies; 16+ messages in thread
From: Manos Pitsidianakis @ 2025-05-21 9:40 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, peter.maydell, qemu-rust
On Wed, May 21, 2025 at 12:21 PM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> On Wed, May 21, 2025 at 11:19 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > The bilge crate, while very nice and espressive, is heavily reliant on
> > traits; because trait functions are never const, bilge and const mix
> > about as well as water and oil.
> >
> > Try using the bitfield-struct crate instead. It is built to support
> > const very well and the only downside is that more manual annotations
> > are needed (for enums and non-full-byte members). Otherwise, the use
> > is pretty much the same and in fact device code does not change at all,
> > only register declarations.
> >
> > Recent versions want to use Rust 1.83, so this uses a slightly older
> > version with basically no lost functionality; but anyway, I want to switch
> > to 1.83 for QEMU as well due to improved "const" support in the compiler.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > rust/Cargo.toml | 1 +
> > rust/hw/char/pl011/Cargo.toml | 3 +-
> > rust/hw/char/pl011/meson.build | 11 +--
> > rust/hw/char/pl011/src/registers.rs | 108 ++++++++++++++--------------
> > 4 files changed, 56 insertions(+), 67 deletions(-)
> >
> > diff --git a/rust/Cargo.toml b/rust/Cargo.toml
> > index 165328b6d01..3345858b5b4 100644
> > --- a/rust/Cargo.toml
> > +++ b/rust/Cargo.toml
> > @@ -97,5 +97,6 @@ used_underscore_binding = "deny"
> > #wildcard_imports = "deny" # still have many bindings::* imports
> >
> > # these may have false positives
> > +enum_variant_names = "allow"
> > #option_if_let_else = "deny"
> > cognitive_complexity = "deny"
> > diff --git a/rust/hw/char/pl011/Cargo.toml b/rust/hw/char/pl011/Cargo.toml
> > index 003ef9613d4..97e3dd00c35 100644
> > --- a/rust/hw/char/pl011/Cargo.toml
> > +++ b/rust/hw/char/pl011/Cargo.toml
> > @@ -16,8 +16,7 @@ rust-version.workspace = true
> > crate-type = ["staticlib"]
> >
> > [dependencies]
> > -bilge = { version = "0.2.0" }
> > -bilge-impl = { version = "0.2.0" }
> > +bitfield-struct = { version = "0.9" }
> > bits = { path = "../../../bits" }
> > qemu_api = { path = "../../../qemu-api" }
> > qemu_api_macros = { path = "../../../qemu-api-macros" }
> > diff --git a/rust/hw/char/pl011/meson.build b/rust/hw/char/pl011/meson.build
> > index f134a6cdc6b..1bae5a03310 100644
> > --- a/rust/hw/char/pl011/meson.build
> > +++ b/rust/hw/char/pl011/meson.build
> > @@ -1,17 +1,10 @@
> > -subproject('bilge-0.2-rs', required: true)
> > -subproject('bilge-impl-0.2-rs', required: true)
> > -
> > -bilge_dep = dependency('bilge-0.2-rs')
> > -bilge_impl_dep = dependency('bilge-impl-0.2-rs')
> > -
> > _libpl011_rs = static_library(
> > 'pl011',
> > files('src/lib.rs'),
> > override_options: ['rust_std=2021', 'build.rust_std=2021'],
> > rust_abi: 'rust',
> > dependencies: [
> > - bilge_dep,
> > - bilge_impl_dep,
> > + bitfield_struct_dep,
> > bits_rs,
> > qemu_api,
> > qemu_api_macros,
> > @@ -22,6 +15,6 @@ rust_devices_ss.add(when: 'CONFIG_X_PL011_RUST', if_true: [declare_dependency(
> > link_whole: [_libpl011_rs],
> > # Putting proc macro crates in `dependencies` is necessary for Meson to find
> > # them when compiling the root per-target static rust lib.
> > - dependencies: [bilge_impl_dep, qemu_api_macros],
> > + dependencies: [bitfield_struct_dep, qemu_api_macros],
> > variables: {'crate': 'pl011'},
> > )])
> > diff --git a/rust/hw/char/pl011/src/registers.rs b/rust/hw/char/pl011/src/registers.rs
> > index 7ececd39f86..f2138c637c5 100644
> > --- a/rust/hw/char/pl011/src/registers.rs
> > +++ b/rust/hw/char/pl011/src/registers.rs
> > @@ -5,12 +5,16 @@
> > //! Device registers exposed as typed structs which are backed by arbitrary
> > //! integer bitmaps. [`Data`], [`Control`], [`LineControl`], etc.
> >
> > +// rustc prefers "constant-like" enums to use upper case names, but that
> > +// is inconsistent in its own way.
> > +#![allow(non_upper_case_globals)]
> > +
> > // For more detail see the PL011 Technical Reference Manual DDI0183:
> > // https://developer.arm.com/documentation/ddi0183/latest/
> >
> > -use bilge::prelude::*;
> > +use bitfield_struct::bitfield;
> > use bits::bits;
> > -use qemu_api::{impl_vmstate_bitsized, impl_vmstate_forward};
> > +use qemu_api::impl_vmstate_forward;
> >
> > /// Offset of each register from the base memory address of the device.
> > #[doc(alias = "offset")]
> > @@ -78,14 +82,18 @@ pub enum RegisterOffset {
> > /// The `UARTRSR` register is updated only when a read occurs
> > /// from the `UARTDR` register with the same status information
> > /// that can also be obtained by reading the `UARTDR` register
> > -#[bitsize(8)]
> > -#[derive(Clone, Copy, Default, DebugBits, FromBits)]
> > +#[bitfield(u8)]
> > pub struct Errors {
> > pub framing_error: bool,
> > pub parity_error: bool,
> > pub break_error: bool,
> > pub overrun_error: bool,
> > - _reserved_unpredictable: u4,
> > + #[bits(4)]
> > + _reserved_unpredictable: u8,
> > +}
> > +
> > +impl Errors {
> > + pub const BREAK: Self = Errors::new().with_break_error(true);
> > }
> >
> > /// Data Register, `UARTDR`
> > @@ -93,19 +101,18 @@ pub struct Errors {
> > /// The `UARTDR` register is the data register; write for TX and
> > /// read for RX. It is a 12-bit register, where bits 7..0 are the
> > /// character and bits 11..8 are error bits.
> > -#[bitsize(32)]
> > -#[derive(Clone, Copy, Default, DebugBits, FromBits)]
> > +#[bitfield(u32)]
> > #[doc(alias = "UARTDR")]
> > pub struct Data {
> > pub data: u8,
> > + #[bits(8)]
> > pub errors: Errors,
> > _reserved: u16,
> > }
> > -impl_vmstate_bitsized!(Data);
> > +impl_vmstate_forward!(Data);
> >
> > impl Data {
> > - // bilge is not very const-friendly, unfortunately
> > - pub const BREAK: Self = Self { value: 1 << 10 };
> > + pub const BREAK: Self = Self::new().with_errors(Errors::BREAK);
> > }
> >
> > /// Receive Status Register / Error Clear Register, `UARTRSR/UARTECR`
> > @@ -119,13 +126,14 @@ impl Data {
> > /// and UARTECR for writes, but really it's a single error status
> > /// register where writing anything to the register clears the error
> > /// bits.
> > -#[bitsize(32)]
> > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > +#[bitfield(u32)]
> > pub struct ReceiveStatusErrorClear {
> > + #[bits(8)]
> > pub errors: Errors,
> > - _reserved_unpredictable: u24,
> > + #[bits(24)]
> > + _reserved_unpredictable: u32,
> > }
> > -impl_vmstate_bitsized!(ReceiveStatusErrorClear);
> > +impl_vmstate_forward!(ReceiveStatusErrorClear);
> >
> > impl ReceiveStatusErrorClear {
> > pub fn set_from_data(&mut self, data: Data) {
> > @@ -138,14 +146,7 @@ pub fn reset(&mut self) {
> > }
> > }
> >
> > -impl Default for ReceiveStatusErrorClear {
> > - fn default() -> Self {
> > - 0.into()
> > - }
> > -}
> > -
> > -#[bitsize(32)]
> > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > +#[bitfield(u32, default = false)]
> > /// Flag Register, `UARTFR`
> > ///
> > /// This has the usual inbound RS232 modem-control signals, plus flags
> > @@ -171,9 +172,10 @@ pub struct Flags {
> > pub transmit_fifo_empty: bool,
> > /// RI: Ring indicator
> > pub ring_indicator: bool,
> > - _reserved_zero_no_modify: u23,
> > + #[bits(23)]
> > + _reserved_zero_no_modify: u32,
> > }
> > -impl_vmstate_bitsized!(Flags);
> > +impl_vmstate_forward!(Flags);
> >
> > impl Flags {
> > pub fn reset(&mut self) {
> > @@ -183,16 +185,14 @@ pub fn reset(&mut self) {
> >
> > impl Default for Flags {
> > fn default() -> Self {
> > - let mut ret: Self = 0.into();
> > // After reset TXFF, RXFF, and BUSY are 0, and TXFE and RXFE are 1
> > - ret.set_receive_fifo_empty(true);
> > - ret.set_transmit_fifo_empty(true);
> > - ret
> > + Self::from(0)
> > + .with_receive_fifo_empty(true)
> > + .with_transmit_fifo_empty(true)
> > }
> > }
> >
> > -#[bitsize(32)]
> > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > +#[bitfield(u32)]
> > /// Line Control Register, `UARTLCR_H`
> > #[doc(alias = "UARTLCR_H")]
> > pub struct LineControl {
> > @@ -201,48 +201,46 @@ pub struct LineControl {
> > /// PEN: Parity enable
> > pub parity_enabled: bool,
> > /// EPS: Even parity select
> > + #[bits(1)]
> > pub parity: Parity,
> > /// STP2: Two stop bits select
> > pub two_stops_bits: bool,
> > /// FEN: Enable FIFOs
> > + #[bits(1)]
> > pub fifos_enabled: Mode,
> > /// WLEN: Word length in bits
> > /// b11 = 8 bits
> > /// b10 = 7 bits
> > /// b01 = 6 bits
> > /// b00 = 5 bits.
> > + #[bits(2)]
> > pub word_length: WordLength,
> > /// SPS Stick parity select
> > pub sticky_parity: bool,
> > /// 31:8 - Reserved, do not modify, read as zero.
> > - _reserved_zero_no_modify: u24,
> > + #[bits(24)]
> > + _reserved_zero_no_modify: u32,
> > }
> > -impl_vmstate_bitsized!(LineControl);
> > +impl_vmstate_forward!(LineControl);
> >
> > impl LineControl {
> > pub fn reset(&mut self) {
> > // All the bits are cleared to 0 when reset.
> > - *self = 0.into();
> > + *self = Self::default();
> > }
> > }
> >
> > -impl Default for LineControl {
> > - fn default() -> Self {
> > - 0.into()
> > - }
> > -}
> > -
> > -#[bitsize(1)]
> > -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> > /// `EPS` "Even parity select", field of [Line Control
> > /// register](LineControl).
> > +#[repr(u8)]
> > +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> > pub enum Parity {
> > Odd = 0,
> > Even = 1,
> > }
> >
> > -#[bitsize(1)]
> > -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> > +#[repr(u8)]
> > +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> > /// `FEN` "Enable FIFOs" or Device mode, field of [Line Control
> > /// register](LineControl).
> > pub enum Mode {
> > @@ -253,8 +251,8 @@ pub enum Mode {
> > FIFO = 1,
> > }
> >
> > -#[bitsize(2)]
> > -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> > +#[repr(u8)]
> > +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> > /// `WLEN` Word length, field of [Line Control register](LineControl).
> > ///
> > /// These bits indicate the number of data bits transmitted or received in a
> > @@ -275,9 +273,8 @@ pub enum WordLength {
> > /// The `UARTCR` register is the control register. It contains various
> > /// enable bits, and the bits to write to set the usual outbound RS232
> > /// modem control signals. All bits reset to 0 except TXE and RXE.
> > -#[bitsize(32)]
> > +#[bitfield(u32, default = false)]
> > #[doc(alias = "UARTCR")]
> > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > pub struct Control {
> > /// `UARTEN` UART enable: 0 = UART is disabled.
> > pub enable_uart: bool,
> > @@ -285,9 +282,10 @@ pub struct Control {
> > /// QEMU does not model this.
> > pub enable_sir: bool,
> > /// `SIRLP` SIR low-power IrDA mode. QEMU does not model this.
> > - pub sir_lowpower_irda_mode: u1,
> > + pub sir_lowpower_irda_mode: bool,
> > /// Reserved, do not modify, read as zero.
> > - _reserved_zero_no_modify: u4,
> > + #[bits(4)]
> > + _reserved_zero_no_modify: u8,
> > /// `LBE` Loopback enable: feed UART output back to the input
> > pub enable_loopback: bool,
> > /// `TXE` Transmit enable
> > @@ -309,21 +307,19 @@ pub struct Control {
> > /// 31:16 - Reserved, do not modify, read as zero.
> > _reserved_zero_no_modify2: u16,
> > }
> > -impl_vmstate_bitsized!(Control);
> > +impl_vmstate_forward!(Control);
> >
> > impl Control {
> > pub fn reset(&mut self) {
> > - *self = 0.into();
> > - self.set_enable_receive(true);
> > - self.set_enable_transmit(true);
> > + *self = Self::default();
> > }
> > }
> >
> > impl Default for Control {
> > fn default() -> Self {
> > - let mut ret: Self = 0.into();
> > - ret.reset();
> > - ret
> > + Self::from(0)
> > + .with_enable_receive(true)
> > + .with_enable_transmit(true)
> > }
> > }
> >
> > --
> > 2.49.0
> >
>
> Perhaps it'd be simpler to contribute const-ability to upstream bilge?
> Is From/Into the only problem trait? I was thinking we can generate
> from/into associated methods for each type that are const. It'd not
> even be a big change and we can carry it as a patch until we can
> catchup with upstream crates.io version in subprojects/. WDYT?
I see that https://github.com/danlehmann/arbitrary-int for example
says almost everything can be used in const context. If we just skip
using traits for generated register types and use associated const
methods (and even implement From/Into that call these internally)
maybe it's not really a problem anymore.
--
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct
2025-05-21 8:18 ` [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct Paolo Bonzini
2025-05-21 9:21 ` Manos Pitsidianakis
@ 2025-05-21 9:50 ` Alex Bennée
2025-05-21 11:12 ` Manos Pitsidianakis
1 sibling, 1 reply; 16+ messages in thread
From: Alex Bennée @ 2025-05-21 9:50 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, peter.maydell, qemu-rust, manos.pitsidianakis
Paolo Bonzini <pbonzini@redhat.com> writes:
> The bilge crate, while very nice and espressive, is heavily reliant on
> traits; because trait functions are never const, bilge and const mix
> about as well as water and oil.
>
> Try using the bitfield-struct crate instead. It is built to support
> const very well and the only downside is that more manual annotations
> are needed (for enums and non-full-byte members). Otherwise, the use
> is pretty much the same and in fact device code does not change at all,
> only register declarations.
>
> Recent versions want to use Rust 1.83, so this uses a slightly older
> version with basically no lost functionality; but anyway, I want to switch
> to 1.83 for QEMU as well due to improved "const" support in the compiler.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/Cargo.toml | 1 +
> rust/hw/char/pl011/Cargo.toml | 3 +-
> rust/hw/char/pl011/meson.build | 11 +--
> rust/hw/char/pl011/src/registers.rs | 108 ++++++++++++++--------------
> 4 files changed, 56 insertions(+), 67 deletions(-)
>
> diff --git a/rust/Cargo.toml b/rust/Cargo.toml
> index 165328b6d01..3345858b5b4 100644
> --- a/rust/Cargo.toml
> +++ b/rust/Cargo.toml
> @@ -97,5 +97,6 @@ used_underscore_binding = "deny"
> #wildcard_imports = "deny" # still have many bindings::* imports
>
> # these may have false positives
> +enum_variant_names = "allow"
> #option_if_let_else = "deny"
> cognitive_complexity = "deny"
> diff --git a/rust/hw/char/pl011/Cargo.toml b/rust/hw/char/pl011/Cargo.toml
> index 003ef9613d4..97e3dd00c35 100644
> --- a/rust/hw/char/pl011/Cargo.toml
> +++ b/rust/hw/char/pl011/Cargo.toml
> @@ -16,8 +16,7 @@ rust-version.workspace = true
> crate-type = ["staticlib"]
>
> [dependencies]
> -bilge = { version = "0.2.0" }
> -bilge-impl = { version = "0.2.0" }
> +bitfield-struct = { version = "0.9" }
> bits = { path = "../../../bits" }
> qemu_api = { path = "../../../qemu-api" }
> qemu_api_macros = { path = "../../../qemu-api-macros" }
> diff --git a/rust/hw/char/pl011/meson.build b/rust/hw/char/pl011/meson.build
> index f134a6cdc6b..1bae5a03310 100644
> --- a/rust/hw/char/pl011/meson.build
> +++ b/rust/hw/char/pl011/meson.build
> @@ -1,17 +1,10 @@
> -subproject('bilge-0.2-rs', required: true)
> -subproject('bilge-impl-0.2-rs', required: true)
> -
> -bilge_dep = dependency('bilge-0.2-rs')
> -bilge_impl_dep = dependency('bilge-impl-0.2-rs')
> -
> _libpl011_rs = static_library(
> 'pl011',
> files('src/lib.rs'),
> override_options: ['rust_std=2021', 'build.rust_std=2021'],
> rust_abi: 'rust',
> dependencies: [
> - bilge_dep,
> - bilge_impl_dep,
> + bitfield_struct_dep,
> bits_rs,
> qemu_api,
> qemu_api_macros,
> @@ -22,6 +15,6 @@ rust_devices_ss.add(when: 'CONFIG_X_PL011_RUST', if_true: [declare_dependency(
> link_whole: [_libpl011_rs],
> # Putting proc macro crates in `dependencies` is necessary for Meson to find
> # them when compiling the root per-target static rust lib.
> - dependencies: [bilge_impl_dep, qemu_api_macros],
> + dependencies: [bitfield_struct_dep, qemu_api_macros],
> variables: {'crate': 'pl011'},
> )])
> diff --git a/rust/hw/char/pl011/src/registers.rs b/rust/hw/char/pl011/src/registers.rs
> index 7ececd39f86..f2138c637c5 100644
> --- a/rust/hw/char/pl011/src/registers.rs
> +++ b/rust/hw/char/pl011/src/registers.rs
> @@ -5,12 +5,16 @@
> //! Device registers exposed as typed structs which are backed by arbitrary
> //! integer bitmaps. [`Data`], [`Control`], [`LineControl`], etc.
>
> +// rustc prefers "constant-like" enums to use upper case names, but that
> +// is inconsistent in its own way.
> +#![allow(non_upper_case_globals)]
> +
> // For more detail see the PL011 Technical Reference Manual DDI0183:
> // https://developer.arm.com/documentation/ddi0183/latest/
>
> -use bilge::prelude::*;
> +use bitfield_struct::bitfield;
> use bits::bits;
> -use qemu_api::{impl_vmstate_bitsized, impl_vmstate_forward};
> +use qemu_api::impl_vmstate_forward;
>
> /// Offset of each register from the base memory address of the device.
> #[doc(alias = "offset")]
> @@ -78,14 +82,18 @@ pub enum RegisterOffset {
> /// The `UARTRSR` register is updated only when a read occurs
> /// from the `UARTDR` register with the same status information
> /// that can also be obtained by reading the `UARTDR` register
> -#[bitsize(8)]
> -#[derive(Clone, Copy, Default, DebugBits, FromBits)]
> +#[bitfield(u8)]
> pub struct Errors {
> pub framing_error: bool,
> pub parity_error: bool,
> pub break_error: bool,
> pub overrun_error: bool,
> - _reserved_unpredictable: u4,
> + #[bits(4)]
> + _reserved_unpredictable: u8,
This does come off as a little janky - effectively casting the u8 to
only cover 4 bits. Is this not something we can derive from the type? I
see lower down...
> +}
> +
> +impl Errors {
> + pub const BREAK: Self = Errors::new().with_break_error(true);
> }
>
> /// Data Register, `UARTDR`
> @@ -93,19 +101,18 @@ pub struct Errors {
> /// The `UARTDR` register is the data register; write for TX and
> /// read for RX. It is a 12-bit register, where bits 7..0 are the
> /// character and bits 11..8 are error bits.
> -#[bitsize(32)]
> -#[derive(Clone, Copy, Default, DebugBits, FromBits)]
> +#[bitfield(u32)]
> #[doc(alias = "UARTDR")]
> pub struct Data {
> pub data: u8,
> + #[bits(8)]
> pub errors: Errors,
We should be able to derive that Errors fits into 8 bits as defined above.
> _reserved: u16,
> }
> -impl_vmstate_bitsized!(Data);
> +impl_vmstate_forward!(Data);
>
> impl Data {
> - // bilge is not very const-friendly, unfortunately
> - pub const BREAK: Self = Self { value: 1 << 10 };
> + pub const BREAK: Self = Self::new().with_errors(Errors::BREAK);
> }
I guess this flys a little over my head, is the effect only seen in the
generated code?
>
> /// Receive Status Register / Error Clear Register, `UARTRSR/UARTECR`
> @@ -119,13 +126,14 @@ impl Data {
> /// and UARTECR for writes, but really it's a single error status
> /// register where writing anything to the register clears the error
> /// bits.
> -#[bitsize(32)]
> -#[derive(Clone, Copy, DebugBits, FromBits)]
> +#[bitfield(u32)]
> pub struct ReceiveStatusErrorClear {
> + #[bits(8)]
> pub errors: Errors,
> - _reserved_unpredictable: u24,
> + #[bits(24)]
> + _reserved_unpredictable: u32,
> }
> -impl_vmstate_bitsized!(ReceiveStatusErrorClear);
> +impl_vmstate_forward!(ReceiveStatusErrorClear);
>
> impl ReceiveStatusErrorClear {
> pub fn set_from_data(&mut self, data: Data) {
> @@ -138,14 +146,7 @@ pub fn reset(&mut self) {
> }
> }
>
> -impl Default for ReceiveStatusErrorClear {
> - fn default() -> Self {
> - 0.into()
> - }
> -}
> -
> -#[bitsize(32)]
> -#[derive(Clone, Copy, DebugBits, FromBits)]
> +#[bitfield(u32, default = false)]
> /// Flag Register, `UARTFR`
> ///
> /// This has the usual inbound RS232 modem-control signals, plus flags
> @@ -171,9 +172,10 @@ pub struct Flags {
> pub transmit_fifo_empty: bool,
> /// RI: Ring indicator
> pub ring_indicator: bool,
> - _reserved_zero_no_modify: u23,
> + #[bits(23)]
> + _reserved_zero_no_modify: u32,
> }
> -impl_vmstate_bitsized!(Flags);
> +impl_vmstate_forward!(Flags);
>
> impl Flags {
> pub fn reset(&mut self) {
> @@ -183,16 +185,14 @@ pub fn reset(&mut self) {
>
> impl Default for Flags {
> fn default() -> Self {
> - let mut ret: Self = 0.into();
> // After reset TXFF, RXFF, and BUSY are 0, and TXFE and RXFE are 1
> - ret.set_receive_fifo_empty(true);
> - ret.set_transmit_fifo_empty(true);
> - ret
> + Self::from(0)
> + .with_receive_fifo_empty(true)
> + .with_transmit_fifo_empty(true)
I guess skipping the mut is the advantage of being able to const eval.
> }
> }
>
> -#[bitsize(32)]
> -#[derive(Clone, Copy, DebugBits, FromBits)]
> +#[bitfield(u32)]
> /// Line Control Register, `UARTLCR_H`
> #[doc(alias = "UARTLCR_H")]
> pub struct LineControl {
> @@ -201,48 +201,46 @@ pub struct LineControl {
> /// PEN: Parity enable
> pub parity_enabled: bool,
> /// EPS: Even parity select
> + #[bits(1)]
> pub parity: Parity,
> /// STP2: Two stop bits select
> pub two_stops_bits: bool,
> /// FEN: Enable FIFOs
> + #[bits(1)]
> pub fifos_enabled: Mode,
> /// WLEN: Word length in bits
> /// b11 = 8 bits
> /// b10 = 7 bits
> /// b01 = 6 bits
> /// b00 = 5 bits.
> + #[bits(2)]
> pub word_length: WordLength,
> /// SPS Stick parity select
> pub sticky_parity: bool,
> /// 31:8 - Reserved, do not modify, read as zero.
> - _reserved_zero_no_modify: u24,
> + #[bits(24)]
> + _reserved_zero_no_modify: u32,
> }
> -impl_vmstate_bitsized!(LineControl);
> +impl_vmstate_forward!(LineControl);
>
> impl LineControl {
> pub fn reset(&mut self) {
> // All the bits are cleared to 0 when reset.
> - *self = 0.into();
> + *self = Self::default();
> }
> }
>
> -impl Default for LineControl {
> - fn default() -> Self {
> - 0.into()
> - }
> -}
> -
> -#[bitsize(1)]
> -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> /// `EPS` "Even parity select", field of [Line Control
> /// register](LineControl).
> +#[repr(u8)]
> +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> pub enum Parity {
> Odd = 0,
> Even = 1,
> }
>
> -#[bitsize(1)]
> -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> +#[repr(u8)]
> +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> /// `FEN` "Enable FIFOs" or Device mode, field of [Line Control
> /// register](LineControl).
> pub enum Mode {
> @@ -253,8 +251,8 @@ pub enum Mode {
> FIFO = 1,
> }
>
> -#[bitsize(2)]
> -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> +#[repr(u8)]
> +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> /// `WLEN` Word length, field of [Line Control register](LineControl).
> ///
> /// These bits indicate the number of data bits transmitted or received in a
> @@ -275,9 +273,8 @@ pub enum WordLength {
> /// The `UARTCR` register is the control register. It contains various
> /// enable bits, and the bits to write to set the usual outbound RS232
> /// modem control signals. All bits reset to 0 except TXE and RXE.
> -#[bitsize(32)]
> +#[bitfield(u32, default = false)]
> #[doc(alias = "UARTCR")]
> -#[derive(Clone, Copy, DebugBits, FromBits)]
> pub struct Control {
> /// `UARTEN` UART enable: 0 = UART is disabled.
> pub enable_uart: bool,
> @@ -285,9 +282,10 @@ pub struct Control {
> /// QEMU does not model this.
> pub enable_sir: bool,
> /// `SIRLP` SIR low-power IrDA mode. QEMU does not model this.
> - pub sir_lowpower_irda_mode: u1,
> + pub sir_lowpower_irda_mode: bool,
> /// Reserved, do not modify, read as zero.
> - _reserved_zero_no_modify: u4,
> + #[bits(4)]
> + _reserved_zero_no_modify: u8,
> /// `LBE` Loopback enable: feed UART output back to the input
> pub enable_loopback: bool,
> /// `TXE` Transmit enable
<snip>
I guess I'm not seeing a massive difference here. I guess the const eval
is nice but there is cognitive dissonance having annotations not match
types. It would be nice to have the best of both worlds.
For now I don't see a compelling reason to change from a standard crate
(which I guess is the reason this is an RFC ;-)
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct
2025-05-21 9:50 ` Alex Bennée
@ 2025-05-21 11:12 ` Manos Pitsidianakis
2025-05-21 13:54 ` Paolo Bonzini
0 siblings, 1 reply; 16+ messages in thread
From: Manos Pitsidianakis @ 2025-05-21 11:12 UTC (permalink / raw)
To: Alex Bennée; +Cc: Paolo Bonzini, qemu-devel, peter.maydell, qemu-rust
On Wed, May 21, 2025 at 12:50 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > The bilge crate, while very nice and espressive, is heavily reliant on
> > traits; because trait functions are never const, bilge and const mix
> > about as well as water and oil.
> >
> > Try using the bitfield-struct crate instead. It is built to support
> > const very well and the only downside is that more manual annotations
> > are needed (for enums and non-full-byte members). Otherwise, the use
> > is pretty much the same and in fact device code does not change at all,
> > only register declarations.
> >
> > Recent versions want to use Rust 1.83, so this uses a slightly older
> > version with basically no lost functionality; but anyway, I want to switch
> > to 1.83 for QEMU as well due to improved "const" support in the compiler.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > rust/Cargo.toml | 1 +
> > rust/hw/char/pl011/Cargo.toml | 3 +-
> > rust/hw/char/pl011/meson.build | 11 +--
> > rust/hw/char/pl011/src/registers.rs | 108 ++++++++++++++--------------
> > 4 files changed, 56 insertions(+), 67 deletions(-)
> >
> > diff --git a/rust/Cargo.toml b/rust/Cargo.toml
> > index 165328b6d01..3345858b5b4 100644
> > --- a/rust/Cargo.toml
> > +++ b/rust/Cargo.toml
> > @@ -97,5 +97,6 @@ used_underscore_binding = "deny"
> > #wildcard_imports = "deny" # still have many bindings::* imports
> >
> > # these may have false positives
> > +enum_variant_names = "allow"
> > #option_if_let_else = "deny"
> > cognitive_complexity = "deny"
> > diff --git a/rust/hw/char/pl011/Cargo.toml b/rust/hw/char/pl011/Cargo.toml
> > index 003ef9613d4..97e3dd00c35 100644
> > --- a/rust/hw/char/pl011/Cargo.toml
> > +++ b/rust/hw/char/pl011/Cargo.toml
> > @@ -16,8 +16,7 @@ rust-version.workspace = true
> > crate-type = ["staticlib"]
> >
> > [dependencies]
> > -bilge = { version = "0.2.0" }
> > -bilge-impl = { version = "0.2.0" }
> > +bitfield-struct = { version = "0.9" }
> > bits = { path = "../../../bits" }
> > qemu_api = { path = "../../../qemu-api" }
> > qemu_api_macros = { path = "../../../qemu-api-macros" }
> > diff --git a/rust/hw/char/pl011/meson.build b/rust/hw/char/pl011/meson.build
> > index f134a6cdc6b..1bae5a03310 100644
> > --- a/rust/hw/char/pl011/meson.build
> > +++ b/rust/hw/char/pl011/meson.build
> > @@ -1,17 +1,10 @@
> > -subproject('bilge-0.2-rs', required: true)
> > -subproject('bilge-impl-0.2-rs', required: true)
> > -
> > -bilge_dep = dependency('bilge-0.2-rs')
> > -bilge_impl_dep = dependency('bilge-impl-0.2-rs')
> > -
> > _libpl011_rs = static_library(
> > 'pl011',
> > files('src/lib.rs'),
> > override_options: ['rust_std=2021', 'build.rust_std=2021'],
> > rust_abi: 'rust',
> > dependencies: [
> > - bilge_dep,
> > - bilge_impl_dep,
> > + bitfield_struct_dep,
> > bits_rs,
> > qemu_api,
> > qemu_api_macros,
> > @@ -22,6 +15,6 @@ rust_devices_ss.add(when: 'CONFIG_X_PL011_RUST', if_true: [declare_dependency(
> > link_whole: [_libpl011_rs],
> > # Putting proc macro crates in `dependencies` is necessary for Meson to find
> > # them when compiling the root per-target static rust lib.
> > - dependencies: [bilge_impl_dep, qemu_api_macros],
> > + dependencies: [bitfield_struct_dep, qemu_api_macros],
> > variables: {'crate': 'pl011'},
> > )])
> > diff --git a/rust/hw/char/pl011/src/registers.rs b/rust/hw/char/pl011/src/registers.rs
> > index 7ececd39f86..f2138c637c5 100644
> > --- a/rust/hw/char/pl011/src/registers.rs
> > +++ b/rust/hw/char/pl011/src/registers.rs
> > @@ -5,12 +5,16 @@
> > //! Device registers exposed as typed structs which are backed by arbitrary
> > //! integer bitmaps. [`Data`], [`Control`], [`LineControl`], etc.
> >
> > +// rustc prefers "constant-like" enums to use upper case names, but that
> > +// is inconsistent in its own way.
> > +#![allow(non_upper_case_globals)]
> > +
> > // For more detail see the PL011 Technical Reference Manual DDI0183:
> > // https://developer.arm.com/documentation/ddi0183/latest/
> >
> > -use bilge::prelude::*;
> > +use bitfield_struct::bitfield;
> > use bits::bits;
> > -use qemu_api::{impl_vmstate_bitsized, impl_vmstate_forward};
> > +use qemu_api::impl_vmstate_forward;
> >
> > /// Offset of each register from the base memory address of the device.
> > #[doc(alias = "offset")]
> > @@ -78,14 +82,18 @@ pub enum RegisterOffset {
> > /// The `UARTRSR` register is updated only when a read occurs
> > /// from the `UARTDR` register with the same status information
> > /// that can also be obtained by reading the `UARTDR` register
> > -#[bitsize(8)]
> > -#[derive(Clone, Copy, Default, DebugBits, FromBits)]
> > +#[bitfield(u8)]
> > pub struct Errors {
> > pub framing_error: bool,
> > pub parity_error: bool,
> > pub break_error: bool,
> > pub overrun_error: bool,
> > - _reserved_unpredictable: u4,
> > + #[bits(4)]
> > + _reserved_unpredictable: u8,
>
> This does come off as a little janky - effectively casting the u8 to
> only cover 4 bits. Is this not something we can derive from the type? I
> see lower down...
Also, I wonder, does bitfield_struct also use 1 bit to represent bool?
>
> > +}
> > +
> > +impl Errors {
> > + pub const BREAK: Self = Errors::new().with_break_error(true);
> > }
> >
> > /// Data Register, `UARTDR`
> > @@ -93,19 +101,18 @@ pub struct Errors {
> > /// The `UARTDR` register is the data register; write for TX and
> > /// read for RX. It is a 12-bit register, where bits 7..0 are the
> > /// character and bits 11..8 are error bits.
> > -#[bitsize(32)]
> > -#[derive(Clone, Copy, Default, DebugBits, FromBits)]
> > +#[bitfield(u32)]
> > #[doc(alias = "UARTDR")]
> > pub struct Data {
> > pub data: u8,
> > + #[bits(8)]
> > pub errors: Errors,
>
> We should be able to derive that Errors fits into 8 bits as defined above.
>
> > _reserved: u16,
> > }
> > -impl_vmstate_bitsized!(Data);
> > +impl_vmstate_forward!(Data);
> >
> > impl Data {
> > - // bilge is not very const-friendly, unfortunately
> > - pub const BREAK: Self = Self { value: 1 << 10 };
> > + pub const BREAK: Self = Self::new().with_errors(Errors::BREAK);
> > }
>
> I guess this flys a little over my head, is the effect only seen in the
> generated code?
Because these functions are const, they can be evaluated at compile
time, so this would be replaced with a constant value when compiled.
>
> >
> > /// Receive Status Register / Error Clear Register, `UARTRSR/UARTECR`
> > @@ -119,13 +126,14 @@ impl Data {
> > /// and UARTECR for writes, but really it's a single error status
> > /// register where writing anything to the register clears the error
> > /// bits.
> > -#[bitsize(32)]
> > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > +#[bitfield(u32)]
> > pub struct ReceiveStatusErrorClear {
> > + #[bits(8)]
> > pub errors: Errors,
> > - _reserved_unpredictable: u24,
> > + #[bits(24)]
> > + _reserved_unpredictable: u32,
> > }
> > -impl_vmstate_bitsized!(ReceiveStatusErrorClear);
> > +impl_vmstate_forward!(ReceiveStatusErrorClear);
> >
> > impl ReceiveStatusErrorClear {
> > pub fn set_from_data(&mut self, data: Data) {
> > @@ -138,14 +146,7 @@ pub fn reset(&mut self) {
> > }
> > }
> >
> > -impl Default for ReceiveStatusErrorClear {
> > - fn default() -> Self {
> > - 0.into()
> > - }
> > -}
> > -
> > -#[bitsize(32)]
> > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > +#[bitfield(u32, default = false)]
> > /// Flag Register, `UARTFR`
> > ///
> > /// This has the usual inbound RS232 modem-control signals, plus flags
> > @@ -171,9 +172,10 @@ pub struct Flags {
> > pub transmit_fifo_empty: bool,
> > /// RI: Ring indicator
> > pub ring_indicator: bool,
> > - _reserved_zero_no_modify: u23,
> > + #[bits(23)]
> > + _reserved_zero_no_modify: u32,
> > }
> > -impl_vmstate_bitsized!(Flags);
> > +impl_vmstate_forward!(Flags);
> >
> > impl Flags {
> > pub fn reset(&mut self) {
> > @@ -183,16 +185,14 @@ pub fn reset(&mut self) {
> >
> > impl Default for Flags {
> > fn default() -> Self {
> > - let mut ret: Self = 0.into();
> > // After reset TXFF, RXFF, and BUSY are 0, and TXFE and RXFE are 1
> > - ret.set_receive_fifo_empty(true);
> > - ret.set_transmit_fifo_empty(true);
> > - ret
> > + Self::from(0)
> > + .with_receive_fifo_empty(true)
> > + .with_transmit_fifo_empty(true)
>
> I guess skipping the mut is the advantage of being able to const eval.
No you can actually have mut in const-eval. What you can't have is
heap allocations and calling non-const functions, and some other
things. But in this case it doesn't matter, because this is the
Default trait and it's not const, because it's a trait method.
>
> > }
> > }
> >
> > -#[bitsize(32)]
> > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > +#[bitfield(u32)]
> > /// Line Control Register, `UARTLCR_H`
> > #[doc(alias = "UARTLCR_H")]
> > pub struct LineControl {
> > @@ -201,48 +201,46 @@ pub struct LineControl {
> > /// PEN: Parity enable
> > pub parity_enabled: bool,
> > /// EPS: Even parity select
> > + #[bits(1)]
> > pub parity: Parity,
> > /// STP2: Two stop bits select
> > pub two_stops_bits: bool,
> > /// FEN: Enable FIFOs
> > + #[bits(1)]
> > pub fifos_enabled: Mode,
> > /// WLEN: Word length in bits
> > /// b11 = 8 bits
> > /// b10 = 7 bits
> > /// b01 = 6 bits
> > /// b00 = 5 bits.
> > + #[bits(2)]
> > pub word_length: WordLength,
> > /// SPS Stick parity select
> > pub sticky_parity: bool,
> > /// 31:8 - Reserved, do not modify, read as zero.
> > - _reserved_zero_no_modify: u24,
> > + #[bits(24)]
> > + _reserved_zero_no_modify: u32,
> > }
> > -impl_vmstate_bitsized!(LineControl);
> > +impl_vmstate_forward!(LineControl);
> >
> > impl LineControl {
> > pub fn reset(&mut self) {
> > // All the bits are cleared to 0 when reset.
> > - *self = 0.into();
> > + *self = Self::default();
> > }
> > }
> >
> > -impl Default for LineControl {
> > - fn default() -> Self {
> > - 0.into()
> > - }
> > -}
> > -
> > -#[bitsize(1)]
> > -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> > /// `EPS` "Even parity select", field of [Line Control
> > /// register](LineControl).
> > +#[repr(u8)]
> > +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> > pub enum Parity {
> > Odd = 0,
> > Even = 1,
> > }
> >
> > -#[bitsize(1)]
> > -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> > +#[repr(u8)]
> > +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> > /// `FEN` "Enable FIFOs" or Device mode, field of [Line Control
> > /// register](LineControl).
> > pub enum Mode {
> > @@ -253,8 +251,8 @@ pub enum Mode {
> > FIFO = 1,
> > }
> >
> > -#[bitsize(2)]
> > -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> > +#[repr(u8)]
> > +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> > /// `WLEN` Word length, field of [Line Control register](LineControl).
> > ///
> > /// These bits indicate the number of data bits transmitted or received in a
> > @@ -275,9 +273,8 @@ pub enum WordLength {
> > /// The `UARTCR` register is the control register. It contains various
> > /// enable bits, and the bits to write to set the usual outbound RS232
> > /// modem control signals. All bits reset to 0 except TXE and RXE.
> > -#[bitsize(32)]
> > +#[bitfield(u32, default = false)]
> > #[doc(alias = "UARTCR")]
> > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > pub struct Control {
> > /// `UARTEN` UART enable: 0 = UART is disabled.
> > pub enable_uart: bool,
> > @@ -285,9 +282,10 @@ pub struct Control {
> > /// QEMU does not model this.
> > pub enable_sir: bool,
> > /// `SIRLP` SIR low-power IrDA mode. QEMU does not model this.
> > - pub sir_lowpower_irda_mode: u1,
> > + pub sir_lowpower_irda_mode: bool,
> > /// Reserved, do not modify, read as zero.
> > - _reserved_zero_no_modify: u4,
> > + #[bits(4)]
> > + _reserved_zero_no_modify: u8,
> > /// `LBE` Loopback enable: feed UART output back to the input
> > pub enable_loopback: bool,
> > /// `TXE` Transmit enable
> <snip>
>
> I guess I'm not seeing a massive difference here. I guess the const eval
> is nice but there is cognitive dissonance having annotations not match
> types. It would be nice to have the best of both worlds.
>
> For now I don't see a compelling reason to change from a standard crate
> (which I guess is the reason this is an RFC ;-)
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
--
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct
2025-05-21 9:21 ` Manos Pitsidianakis
2025-05-21 9:40 ` Manos Pitsidianakis
@ 2025-05-21 13:51 ` Paolo Bonzini
1 sibling, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2025-05-21 13:51 UTC (permalink / raw)
To: Manos Pitsidianakis; +Cc: qemu-devel, Maydell, Peter, qemu-rust
[-- Attachment #1: Type: text/plain, Size: 876 bytes --]
Il mer 21 mag 2025, 11:22 Manos Pitsidianakis <
manos.pitsidianakis@linaro.org> ha scritto:
> Perhaps it'd be simpler to contribute const-ability to upstream bilge?
> Is From/Into the only problem trait?
Probably.
I was thinking we can generate
> from/into associated methods for each type that are const
Yes, that's what bitfield-struct does too.
. It'd not
> even be a big change and we can carry it as a patch until we can
> catchup with upstream crates.io version in subprojects/. WDYT?
>
Sure, I have no idea how hard it is but I think they'd accept it? The
bitflags/bits part is more important; I put the two together just for
convenience, as a "let's define our standard toolbox" kind of series,
because they conflict. But it's possible to merge just the first part.
Paolo
> --
> Manos Pitsidianakis
> Emulation and Virtualization Engineer at Linaro Ltd
>
>
[-- Attachment #2: Type: text/html, Size: 2181 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct
2025-05-21 11:12 ` Manos Pitsidianakis
@ 2025-05-21 13:54 ` Paolo Bonzini
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2025-05-21 13:54 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: Alex Bennée, qemu-devel, Maydell, Peter, qemu-rust
[-- Attachment #1: Type: text/plain, Size: 9016 bytes --]
Il mer 21 mag 2025, 13:12 Manos Pitsidianakis <
manos.pitsidianakis@linaro.org> ha scritto:
> On Wed, May 21, 2025 at 12:50 PM Alex
> > > - _reserved_unpredictable: u4,
> > > + #[bits(4)]
> > > + _reserved_unpredictable: u8,
> >
> > This does come off as a little janky - effectively casting the u8 to
> > only cover 4 bits. Is this not something we can derive from the type? I
> > see lower down...
>
> Also, I wonder, does bitfield_struct also use 1 bit to represent bool?
>
Yes.
Also, to answer Alex, note that u4 is not a standard Rust type, it comes
from the arbitrary-int crate.
Paolo
>
> >
> > > +}
> > > +
> > > +impl Errors {
> > > + pub const BREAK: Self = Errors::new().with_break_error(true);
> > > }
> > >
> > > /// Data Register, `UARTDR`
> > > @@ -93,19 +101,18 @@ pub struct Errors {
> > > /// The `UARTDR` register is the data register; write for TX and
> > > /// read for RX. It is a 12-bit register, where bits 7..0 are the
> > > /// character and bits 11..8 are error bits.
> > > -#[bitsize(32)]
> > > -#[derive(Clone, Copy, Default, DebugBits, FromBits)]
> > > +#[bitfield(u32)]
> > > #[doc(alias = "UARTDR")]
> > > pub struct Data {
> > > pub data: u8,
> > > + #[bits(8)]
> > > pub errors: Errors,
> >
> > We should be able to derive that Errors fits into 8 bits as defined
> above.
> >
> > > _reserved: u16,
> > > }
> > > -impl_vmstate_bitsized!(Data);
> > > +impl_vmstate_forward!(Data);
> > >
> > > impl Data {
> > > - // bilge is not very const-friendly, unfortunately
> > > - pub const BREAK: Self = Self { value: 1 << 10 };
> > > + pub const BREAK: Self = Self::new().with_errors(Errors::BREAK);
> > > }
> >
> > I guess this flys a little over my head, is the effect only seen in the
> > generated code?
>
> Because these functions are const, they can be evaluated at compile
> time, so this would be replaced with a constant value when compiled.
>
> >
> > >
> > > /// Receive Status Register / Error Clear Register, `UARTRSR/UARTECR`
> > > @@ -119,13 +126,14 @@ impl Data {
> > > /// and UARTECR for writes, but really it's a single error status
> > > /// register where writing anything to the register clears the error
> > > /// bits.
> > > -#[bitsize(32)]
> > > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > > +#[bitfield(u32)]
> > > pub struct ReceiveStatusErrorClear {
> > > + #[bits(8)]
> > > pub errors: Errors,
> > > - _reserved_unpredictable: u24,
> > > + #[bits(24)]
> > > + _reserved_unpredictable: u32,
> > > }
> > > -impl_vmstate_bitsized!(ReceiveStatusErrorClear);
> > > +impl_vmstate_forward!(ReceiveStatusErrorClear);
> > >
> > > impl ReceiveStatusErrorClear {
> > > pub fn set_from_data(&mut self, data: Data) {
> > > @@ -138,14 +146,7 @@ pub fn reset(&mut self) {
> > > }
> > > }
> > >
> > > -impl Default for ReceiveStatusErrorClear {
> > > - fn default() -> Self {
> > > - 0.into()
> > > - }
> > > -}
> > > -
> > > -#[bitsize(32)]
> > > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > > +#[bitfield(u32, default = false)]
> > > /// Flag Register, `UARTFR`
> > > ///
> > > /// This has the usual inbound RS232 modem-control signals, plus flags
> > > @@ -171,9 +172,10 @@ pub struct Flags {
> > > pub transmit_fifo_empty: bool,
> > > /// RI: Ring indicator
> > > pub ring_indicator: bool,
> > > - _reserved_zero_no_modify: u23,
> > > + #[bits(23)]
> > > + _reserved_zero_no_modify: u32,
> > > }
> > > -impl_vmstate_bitsized!(Flags);
> > > +impl_vmstate_forward!(Flags);
> > >
> > > impl Flags {
> > > pub fn reset(&mut self) {
> > > @@ -183,16 +185,14 @@ pub fn reset(&mut self) {
> > >
> > > impl Default for Flags {
> > > fn default() -> Self {
> > > - let mut ret: Self = 0.into();
> > > // After reset TXFF, RXFF, and BUSY are 0, and TXFE and RXFE
> are 1
> > > - ret.set_receive_fifo_empty(true);
> > > - ret.set_transmit_fifo_empty(true);
> > > - ret
> > > + Self::from(0)
> > > + .with_receive_fifo_empty(true)
> > > + .with_transmit_fifo_empty(true)
> >
> > I guess skipping the mut is the advantage of being able to const eval.
>
> No you can actually have mut in const-eval. What you can't have is
> heap allocations and calling non-const functions, and some other
> things. But in this case it doesn't matter, because this is the
> Default trait and it's not const, because it's a trait method.
>
> >
> > > }
> > > }
> > >
> > > -#[bitsize(32)]
> > > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > > +#[bitfield(u32)]
> > > /// Line Control Register, `UARTLCR_H`
> > > #[doc(alias = "UARTLCR_H")]
> > > pub struct LineControl {
> > > @@ -201,48 +201,46 @@ pub struct LineControl {
> > > /// PEN: Parity enable
> > > pub parity_enabled: bool,
> > > /// EPS: Even parity select
> > > + #[bits(1)]
> > > pub parity: Parity,
> > > /// STP2: Two stop bits select
> > > pub two_stops_bits: bool,
> > > /// FEN: Enable FIFOs
> > > + #[bits(1)]
> > > pub fifos_enabled: Mode,
> > > /// WLEN: Word length in bits
> > > /// b11 = 8 bits
> > > /// b10 = 7 bits
> > > /// b01 = 6 bits
> > > /// b00 = 5 bits.
> > > + #[bits(2)]
> > > pub word_length: WordLength,
> > > /// SPS Stick parity select
> > > pub sticky_parity: bool,
> > > /// 31:8 - Reserved, do not modify, read as zero.
> > > - _reserved_zero_no_modify: u24,
> > > + #[bits(24)]
> > > + _reserved_zero_no_modify: u32,
> > > }
> > > -impl_vmstate_bitsized!(LineControl);
> > > +impl_vmstate_forward!(LineControl);
> > >
> > > impl LineControl {
> > > pub fn reset(&mut self) {
> > > // All the bits are cleared to 0 when reset.
> > > - *self = 0.into();
> > > + *self = Self::default();
> > > }
> > > }
> > >
> > > -impl Default for LineControl {
> > > - fn default() -> Self {
> > > - 0.into()
> > > - }
> > > -}
> > > -
> > > -#[bitsize(1)]
> > > -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> > > /// `EPS` "Even parity select", field of [Line Control
> > > /// register](LineControl).
> > > +#[repr(u8)]
> > > +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> > > pub enum Parity {
> > > Odd = 0,
> > > Even = 1,
> > > }
> > >
> > > -#[bitsize(1)]
> > > -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> > > +#[repr(u8)]
> > > +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> > > /// `FEN` "Enable FIFOs" or Device mode, field of [Line Control
> > > /// register](LineControl).
> > > pub enum Mode {
> > > @@ -253,8 +251,8 @@ pub enum Mode {
> > > FIFO = 1,
> > > }
> > >
> > > -#[bitsize(2)]
> > > -#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
> > > +#[repr(u8)]
> > > +#[derive(Clone, Copy, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
> > > /// `WLEN` Word length, field of [Line Control register](LineControl).
> > > ///
> > > /// These bits indicate the number of data bits transmitted or
> received in a
> > > @@ -275,9 +273,8 @@ pub enum WordLength {
> > > /// The `UARTCR` register is the control register. It contains various
> > > /// enable bits, and the bits to write to set the usual outbound RS232
> > > /// modem control signals. All bits reset to 0 except TXE and RXE.
> > > -#[bitsize(32)]
> > > +#[bitfield(u32, default = false)]
> > > #[doc(alias = "UARTCR")]
> > > -#[derive(Clone, Copy, DebugBits, FromBits)]
> > > pub struct Control {
> > > /// `UARTEN` UART enable: 0 = UART is disabled.
> > > pub enable_uart: bool,
> > > @@ -285,9 +282,10 @@ pub struct Control {
> > > /// QEMU does not model this.
> > > pub enable_sir: bool,
> > > /// `SIRLP` SIR low-power IrDA mode. QEMU does not model this.
> > > - pub sir_lowpower_irda_mode: u1,
> > > + pub sir_lowpower_irda_mode: bool,
> > > /// Reserved, do not modify, read as zero.
> > > - _reserved_zero_no_modify: u4,
> > > + #[bits(4)]
> > > + _reserved_zero_no_modify: u8,
> > > /// `LBE` Loopback enable: feed UART output back to the input
> > > pub enable_loopback: bool,
> > > /// `TXE` Transmit enable
> > <snip>
> >
> > I guess I'm not seeing a massive difference here. I guess the const eval
> > is nice but there is cognitive dissonance having annotations not match
> > types. It would be nice to have the best of both worlds.
> >
> > For now I don't see a compelling reason to change from a standard crate
> > (which I guess is the reason this is an RFC ;-)
> >
> > --
> > Alex Bennée
> > Virtualisation Tech Lead @ Linaro
>
> --
> Manos Pitsidianakis
> Emulation and Virtualization Engineer at Linaro Ltd
>
>
[-- Attachment #2: Type: text/html, Size: 11846 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/6] rust: add "bits", a custom bitflags implementation
2025-05-21 9:23 ` Manos Pitsidianakis
@ 2025-05-21 15:05 ` Daniel P. Berrangé
0 siblings, 0 replies; 16+ messages in thread
From: Daniel P. Berrangé @ 2025-05-21 15:05 UTC (permalink / raw)
To: Manos Pitsidianakis; +Cc: Paolo Bonzini, qemu-devel, peter.maydell, qemu-rust
On Wed, May 21, 2025 at 12:23:02PM +0300, Manos Pitsidianakis wrote:
> On Wed, May 21, 2025 at 11:29 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Wed, May 21, 2025 at 10:18:40AM +0200, Paolo Bonzini wrote:
> > > One common thing that device emulation does is manipulate bitmasks, for example
> > > to check whether two bitmaps have common bits. One example in the pl011 crate
> > > is the checks for pending interrupts, where an interrupt cause corresponds to
> > > at least one interrupt source from a fixed set.
> > >
> > > Unfortunately, this is one case where Rust *can* provide some kind of
> > > abstraction but it does so with a rather Perl-ish There Is More Way To
> > > Do It. It is not something where a crate like "bilge" helps, because
> > > it only covers the packing of bits in a structure; operations like "are
> > > all bits of Y set in X" almost never make sense for bit-packed structs;
> > > you need something else, there are several crates that do it and of course
> > > we're going to roll our own.
> > >
> > > In particular I examined three:
> > >
> > > - bitmask (https://docs.rs/bitmask/0.5.0/bitmask/) does not support const
> > > at all. This is a showstopper because one of the ugly things in the
> > > current pl011 code is the ugliness of code that defines interrupt masks
> > > at compile time:
> > >
> > > pub const E: Self = Self(Self::OE.0 | Self::BE.0 | Self::PE.0 | Self::FE.0);
> > >
> > > or even worse:
> > >
> > > const IRQMASK: [u32; 6] = [
> > > Interrupt::E.0 | Interrupt::MS.0 | Interrupt::RT.0 | Interrupt::TX.0 | Interrupt::RX.0,
> > > ...
> > > }
> > >
> > > You would have to use roughly the same code---"bitmask" only helps with
> > > defining the struct.
> > >
> > > - bitmask_enum (https://docs.rs/bitmask-enum/2.2.5/bitmask_enum/) does not
> > > have a good separation of "valid" and "invalid" bits, so for example "!x"
> > > will invert all 16 bits if you choose u16 as the representation -- even if
> > > you only defined 10 bits. This makes it easier to introduce subtle bugs
> > > in comparisons.
> > >
> > > - bitflags (https://docs.rs/bitflags/2.6.0/bitflags/) is generally the most
> > > used such crate and is the one that I took most inspiration from with
> > > respect to the syntax. It's a pretty sophisticated implementation,
> > > with a lot of bells and whistles such as an implementation of "Iter"
> > > that returns the bits one at a time.
> > >
> > > The main thing that all of them lack, however, is a way to simplify the
> > > ugly definitions like the above. "bitflags" includes const methods that
> > > perform AND/OR/XOR of masks (these are necessary because Rust operator
> > > overloading does not support const yet, and therefore overloaded operators
> > > cannot be used in the definition of a "static" variable), but they become
> > > even more verbose and unmanageable, like
> > >
> > > Interrupt::E.union(Interrupt::MS).union(Interrupt::RT).union(Interrupt::TX).union(Interrupt::RX)
> > >
> > > This was the main reason to create "bits", which allows something like
> > >
> > > bits!(Interrupt: E | MS | RT | TX | RX)
> > >
> > > and expands it 1) add "Interrupt::" in front of all identifiers 2) convert
> > > operators to the wordy const functions like "union". It supports boolean
> > > operators "&", "|", "^", "!" and parentheses, with a relatively simple
> > > recursive descent parser that's implemented in qemu_api_macros.
> > >
> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > > ---
> > > rust/Cargo.lock | 7 +
> > > rust/Cargo.toml | 1 +
> > > rust/bits/Cargo.toml | 19 ++
> > > rust/bits/meson.build | 12 +
> > > rust/bits/src/lib.rs | 441 +++++++++++++++++++++++++++++++
> > > rust/meson.build | 1 +
> > > rust/qemu-api-macros/src/bits.rs | 227 ++++++++++++++++
> > > rust/qemu-api-macros/src/lib.rs | 12 +
> > > 8 files changed, 720 insertions(+)
> > > create mode 100644 rust/bits/Cargo.toml
> > > create mode 100644 rust/bits/meson.build
> > > create mode 100644 rust/bits/src/lib.rs
> > > create mode 100644 rust/qemu-api-macros/src/bits.rs
> >
> > > diff --git a/rust/bits/src/lib.rs b/rust/bits/src/lib.rs
> > > new file mode 100644
> > > index 00000000000..d80a6263f1e
> > > --- /dev/null
> > > +++ b/rust/bits/src/lib.rs
> > > @@ -0,0 +1,441 @@
> >
> > This (and other new .rs files) needs SPDX-License-Identifier
>
> We should probably lint for this in .rs files.
Strengthened from a recommendation to a mandate in:
https://lists.nongnu.org/archive/html/qemu-devel/2025-05/msg04968.html
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-05-21 15:05 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-21 8:18 [RFC PATCH 0/6] rust: make usage of bitmasks and bitfields nicer Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 1/6] rust: add "bits", a custom bitflags implementation Paolo Bonzini
2025-05-21 8:29 ` Daniel P. Berrangé
2025-05-21 9:23 ` Manos Pitsidianakis
2025-05-21 15:05 ` Daniel P. Berrangé
2025-05-21 8:18 ` [RFC PATCH 2/6] rust: pl011: use the bits macro Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 3/6] rust: qemu-api-macros: add from_bits and into_bits to #[derive(TryInto)] Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 4/6] rust: subprojects: add bitfield-struct Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 5/6] rust: pl011: switch from bilge to bitfield-struct Paolo Bonzini
2025-05-21 9:21 ` Manos Pitsidianakis
2025-05-21 9:40 ` Manos Pitsidianakis
2025-05-21 13:51 ` Paolo Bonzini
2025-05-21 9:50 ` Alex Bennée
2025-05-21 11:12 ` Manos Pitsidianakis
2025-05-21 13:54 ` Paolo Bonzini
2025-05-21 8:18 ` [RFC PATCH 6/6] rust: remove bilge crate Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).