rust: pl011: switch from bilge to bitfield-struct
The bilge crate is heavily reliant on traits and, because trait functions are never const, bilge and const mix about as well as water and oil. In addition, it has support for the zerocopy crate that only works for an older version, and is hard to update because the implementation doesn't like that zerocopy::FromBits and bilge::FromBits are the same name. zerocopy is definitely something that QEMU could use in the future. The bitfield-struct crate, instead, is built from the ground up to support const. Its use is pretty much the same (device code does not change at all, only register declarations do), with some things being more verbose and others being simpler. The code for the crate itself is much smaller, too. It does have two disadvantages: it does not let you annotate enums as bitfields, and it does not integrate with arbitrary-int. Thus, it requires manual size annotations for anything that is not a bool, iNN or uNN. Lack of support for arbitrary-int is a very small deal, while enums are a bit more annoying because they require some repetition and an implementation of two functions from_bits() and into_bits(). However, the latter is already provided by the "bits!" and "#[derive(common::TryInto)]" utilities, and thus is not manual in QEMU's case. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini committed
May 13, 2025 at 10:23 UTC
64574f1c517bc5a6eee56d94d7db46fd602044d8
8 files changed
+75
-60
Cargo.lock
+12
-2
@@ -24,6 +24,17 @@ dependencies = [
24
"syn",
25
]
26
27
+[[package]]
28
+name = "bitfield-struct"
29
+version = "0.13.0"
30
+source = "registry+https://github.com/rust-lang/crates.io-index"
31
+checksum = "3ca6739863c590881f038d033a146c51ddae239186a4327014839fd864f44ed5"
32
+dependencies = [
33
+ "proc-macro2",
34
+ "quote",
35
+ "syn",
36
+]
37
+
38
[[package]]
39
name = "bilge"
40
version = "0.2.0"
@@ -243,8 +254,7 @@ checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c"
254
name = "pl011"
255
version = "0.1.0"
256
dependencies = [
246
- "bilge",
247
- "bilge-impl",
257
+ "bitfield-struct",
258
"bits",
259
"bql",
260
"chardev",
Cargo.toml
+1
@@ -101,5 +101,6 @@ used_underscore_binding = "deny"
101
#wildcard_imports = "deny" # still have many bindings::* imports
102
103
# these may have false positives
104
+enum_variant_names = "allow"
105
#option_if_let_else = "deny"
106
cognitive_complexity = "deny"
rust/hw/char/pl011/Cargo.toml
+1
-2
@@ -14,8 +14,7 @@ rust-version.workspace = true
14
15
[dependencies]
16
glib-sys.workspace = true
17
-bilge = { version = "0.2.0" }
18
-bilge-impl = { version = "0.2.0" }
17
+bitfield-struct = { version = "0.13" }
18
bits = { path = "../../../bits" }
19
common = { path = "../../../common" }
20
util = { path = "../../../util" }
rust/hw/char/pl011/src/registers.rs
+52
-56
@@ -5,12 +5,16 @@
5
//! Device registers exposed as typed structs which are backed by arbitrary
6
//! integer bitmaps. [`Data`], [`Control`], [`LineControl`], etc.
7
8
+// rustc prefers "constant-like" enums to use upper case names, but that
9
+// is inconsistent in its own way.
10
+#![allow(non_upper_case_globals)]
11
+
12
// For more detail see the PL011 Technical Reference Manual DDI0183:
13
// https://developer.arm.com/documentation/ddi0183/latest/
14
11
-use bilge::prelude::*;
15
+use bitfield_struct::bitfield;
16
use bits::bits;
13
-use migration::{impl_vmstate_bitsized, impl_vmstate_forward};
17
+use migration::impl_vmstate_forward;
18
19
/// Offset of each register from the base memory address of the device.
20
#[doc(alias = "offset")]
@@ -78,14 +82,18 @@ pub enum RegisterOffset {
82
/// The `UARTRSR` register is updated only when a read occurs
83
/// from the `UARTDR` register with the same status information
84
/// that can also be obtained by reading the `UARTDR` register
81
-#[bitsize(8)]
82
-#[derive(Clone, Copy, Default, DebugBits, FromBits)]
85
+#[bitfield(u8)]
86
pub struct Errors {
87
pub framing_error: bool,
88
pub parity_error: bool,
89
pub break_error: bool,
90
pub overrun_error: bool,
88
- _reserved_unpredictable: u4,
91
+ #[bits(4)]
92
+ _reserved_unpredictable: u8,
93
+}
94
+
95
+impl Errors {
96
+ pub const BREAK: Self = Errors::new().with_break_error(true);
97
}
98
99
/// Data Register, `UARTDR`
@@ -93,19 +101,18 @@ pub struct Errors {
101
/// The `UARTDR` register is the data register; write for TX and
102
/// read for RX. It is a 12-bit register, where bits 7..0 are the
103
/// character and bits 11..8 are error bits.
96
-#[bitsize(32)]
97
-#[derive(Clone, Copy, Default, DebugBits, FromBits)]
104
+#[bitfield(u32)]
105
#[doc(alias = "UARTDR")]
106
pub struct Data {
107
pub data: u8,
108
+ #[bits(8)]
109
pub errors: Errors,
110
_reserved: u16,
111
}
104
-impl_vmstate_bitsized!(Data);
112
+impl_vmstate_forward!(Data);
113
114
impl Data {
107
- // bilge is not very const-friendly, unfortunately
108
- pub const BREAK: Self = Self { value: 1 << 10 };
115
+ pub const BREAK: Self = Self::new().with_errors(Errors::BREAK);
116
}
117
118
/// Receive Status Register / Error Clear Register, `UARTRSR/UARTECR`
@@ -119,13 +126,14 @@ impl Data {
126
/// and UARTECR for writes, but really it's a single error status
127
/// register where writing anything to the register clears the error
128
/// bits.
122
-#[bitsize(32)]
123
-#[derive(Clone, Copy, DebugBits, FromBits)]
129
+#[bitfield(u32)]
130
pub struct ReceiveStatusErrorClear {
131
+ #[bits(8)]
132
pub errors: Errors,
126
- _reserved_unpredictable: u24,
133
+ #[bits(24)]
134
+ _reserved_unpredictable: u32,
135
}
128
-impl_vmstate_bitsized!(ReceiveStatusErrorClear);
136
+impl_vmstate_forward!(ReceiveStatusErrorClear);
137
138
impl ReceiveStatusErrorClear {
139
pub fn set_from_data(&mut self, data: Data) {
@@ -138,14 +146,7 @@ impl ReceiveStatusErrorClear {
146
}
147
}
148
141
-impl Default for ReceiveStatusErrorClear {
142
- fn default() -> Self {
143
- 0.into()
144
- }
145
-}
146
-
147
-#[bitsize(32)]
148
-#[derive(Clone, Copy, DebugBits, FromBits)]
149
+#[bitfield(u32, default = false)]
150
/// Flag Register, `UARTFR`
151
///
152
/// This has the usual inbound RS232 modem-control signals, plus flags
@@ -171,9 +172,10 @@ pub struct Flags {
172
pub transmit_fifo_empty: bool,
173
/// RI: Ring indicator
174
pub ring_indicator: bool,
174
- _reserved_zero_no_modify: u23,
175
+ #[bits(23)]
176
+ _reserved_zero_no_modify: u32,
177
}
176
-impl_vmstate_bitsized!(Flags);
178
+impl_vmstate_forward!(Flags);
179
180
impl Flags {
181
pub fn reset(&mut self) {
@@ -183,16 +185,14 @@ impl Flags {
185
186
impl Default for Flags {
187
fn default() -> Self {
186
- let mut ret: Self = 0.into();
188
// After reset TXFF, RXFF, and BUSY are 0, and TXFE and RXFE are 1
188
- ret.set_receive_fifo_empty(true);
189
- ret.set_transmit_fifo_empty(true);
190
- ret
189
+ Self::from(0)
190
+ .with_receive_fifo_empty(true)
191
+ .with_transmit_fifo_empty(true)
192
}
193
}
194
194
-#[bitsize(32)]
195
-#[derive(Clone, Copy, DebugBits, FromBits)]
195
+#[bitfield(u32)]
196
/// Line Control Register, `UARTLCR_H`
197
#[doc(alias = "UARTLCR_H")]
198
pub struct LineControl {
@@ -201,48 +201,46 @@ pub struct LineControl {
201
/// PEN: Parity enable
202
pub parity_enabled: bool,
203
/// EPS: Even parity select
204
+ #[bits(1)]
205
pub parity: Parity,
206
/// STP2: Two stop bits select
207
pub two_stops_bits: bool,
208
/// FEN: Enable FIFOs
209
+ #[bits(1)]
210
pub fifos_enabled: Mode,
211
/// WLEN: Word length in bits
212
/// b11 = 8 bits
213
/// b10 = 7 bits
214
/// b01 = 6 bits
215
/// b00 = 5 bits.
216
+ #[bits(2)]
217
pub word_length: WordLength,
218
/// SPS Stick parity select
219
pub sticky_parity: bool,
220
/// 31:8 - Reserved, do not modify, read as zero.
218
- _reserved_zero_no_modify: u24,
221
+ #[bits(24)]
222
+ _reserved_zero_no_modify: u32,
223
}
220
-impl_vmstate_bitsized!(LineControl);
224
+impl_vmstate_forward!(LineControl);
225
226
impl LineControl {
227
pub fn reset(&mut self) {
228
// All the bits are cleared to 0 when reset.
225
- *self = 0.into();
226
- }
227
-}
228
-
229
-impl Default for LineControl {
230
- fn default() -> Self {
231
- 0.into()
229
+ *self = Self::default();
230
}
231
}
232
235
-#[bitsize(1)]
236
-#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
233
/// `EPS` "Even parity select", field of [Line Control
234
/// register](LineControl).
235
+#[repr(u8)]
236
+#[derive(Clone, Copy, Debug, Eq, PartialEq, common::TryInto)]
237
pub enum Parity {
238
Odd = 0,
239
Even = 1,
240
}
241
244
-#[bitsize(1)]
245
-#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
242
+#[repr(u8)]
243
+#[derive(Clone, Copy, Debug, Eq, PartialEq, common::TryInto)]
244
/// `FEN` "Enable FIFOs" or Device mode, field of [Line Control
245
/// register](LineControl).
246
pub enum Mode {
@@ -253,8 +251,8 @@ pub enum Mode {
251
FIFO = 1,
252
}
253
256
-#[bitsize(2)]
257
-#[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
254
+#[repr(u8)]
255
+#[derive(Clone, Copy, Debug, Eq, PartialEq, common::TryInto)]
256
#[allow(clippy::enum_variant_names)]
257
/// `WLEN` Word length, field of [Line Control register](LineControl).
258
///
@@ -276,9 +274,8 @@ pub enum WordLength {
274
/// The `UARTCR` register is the control register. It contains various
275
/// enable bits, and the bits to write to set the usual outbound RS232
276
/// modem control signals. All bits reset to 0 except TXE and RXE.
279
-#[bitsize(32)]
277
+#[bitfield(u32, default = false)]
278
#[doc(alias = "UARTCR")]
281
-#[derive(Clone, Copy, DebugBits, FromBits)]
279
pub struct Control {
280
/// `UARTEN` UART enable: 0 = UART is disabled.
281
pub enable_uart: bool,
@@ -286,9 +283,10 @@ pub struct Control {
283
/// QEMU does not model this.
284
pub enable_sir: bool,
285
/// `SIRLP` SIR low-power IrDA mode. QEMU does not model this.
289
- pub sir_lowpower_irda_mode: u1,
286
+ pub sir_lowpower_irda_mode: bool,
287
/// Reserved, do not modify, read as zero.
291
- _reserved_zero_no_modify: u4,
288
+ #[bits(4)]
289
+ _reserved_zero_no_modify: u8,
290
/// `LBE` Loopback enable: feed UART output back to the input
291
pub enable_loopback: bool,
292
/// `TXE` Transmit enable
@@ -310,21 +308,19 @@ pub struct Control {
308
/// 31:16 - Reserved, do not modify, read as zero.
309
_reserved_zero_no_modify2: u16,
310
}
313
-impl_vmstate_bitsized!(Control);
311
+impl_vmstate_forward!(Control);
312
313
impl Control {
314
pub fn reset(&mut self) {
317
- *self = 0.into();
318
- self.set_enable_receive(true);
319
- self.set_enable_transmit(true);
315
+ *self = Self::default();
316
}
317
}
318
319
impl Default for Control {
320
fn default() -> Self {
325
- let mut ret: Self = 0.into();
326
- ret.reset();
327
- ret
321
+ Self::from(0)
322
+ .with_enable_receive(true)
323
+ .with_enable_transmit(true)
324
}
325
}
326
scripts/archive-source.sh
+1
@@ -34,6 +34,7 @@ subprojects=(
34
berkeley-testfloat-3
35
bilge-0.2-rs
36
bilge-impl-0.2-rs
37
+ bitfield-0.9-rs
38
either-1-rs
39
foreign-0.3-rs
40
glib-sys-0.21-rs
scripts/make-release
+1
@@ -42,6 +42,7 @@ fi
42
SUBPROJECTS="libvfio-user keycodemapdb berkeley-softfloat-3
43
berkeley-testfloat-3 anyhow-1-rs arbitrary-int-1-rs attrs-0.2-rs bilge-0.2-rs
44
bilge-impl-0.2-rs either-1-rs foreign-0.3-rs itertools-0.11-rs
45
+ bilge-impl-0.2-rs bitfield-0.9-rs either-1-rs foreign-0.3-rs itertools-0.11-rs
46
libc-0.2-rs probe-0.5-rs proc-macro2-1-rs
47
proc-macro-error-1-rs proc-macro-error-attr-1-rs quote-1-rs
48
syn-2-rs unicode-ident-1-rs"
subprojects/.gitignore
+1
@@ -11,6 +11,7 @@
11
/attrs-*
12
/bilge-*
13
/bilge-impl-*
14
+/bitfield-struct-*
15
/either-*
16
/foreign-*
17
/glib-sys-*
subprojects/bitfield-struct-0.13-rs.wrap
new
+6
@@ -0,0 +1,6 @@
1
+[wrap-file]
2
+directory = bitfield-struct-0.13.0
3
+source_url = https://crates.io/api/v1/crates/bitfield-struct/0.13.0/download
4
+source_filename = bitfield-struct-0.13.0.tar.gz
5
+source_hash = 3ca6739863c590881f038d033a146c51ddae239186a4327014839fd864f44ed5
6
+method = cargo