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.0Paolo 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
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
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
>
>
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
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 > >
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
© 2016 - 2025 Red Hat, Inc.