From e2e0828e0f25042a09b1cbada41a436d1258fdb8 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 10 Dec 2024 12:12:47 +0100 Subject: [PATCH] rust: pl011: extend registers to 32 bits MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The PL011 Technical Reference Manual lists the "real" size of the registers in table 3-1, and only rounds up to the next byte when describing the registers; for example, UARTDR is listed as having width 12/8 (12 bits read, 8 written) and only bits 15:0 are listed in "Table 3-2 UARTDR Register". However, in practice these are 32-bit registers, accessible only through 32-bit MMIO accesses; preserving the fiction that they're smaller introduces multiple casts (to go from the bilge bitfield type to e.g u16 to u64) and more importantly it breaks the migration stream because the Rust vmstate macros are not yet type safe. So, just make everything 32-bits wide. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device.rs | 36 ++++++++++++++------------------ rust/hw/char/pl011/src/lib.rs | 23 +++++++++----------- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index 5e3a9c6f58..090e5d6450 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -186,9 +186,9 @@ impl PL011State { pub fn read(&mut self, offset: hwaddr, _size: c_uint) -> std::ops::ControlFlow { use RegisterOffset::*; - std::ops::ControlFlow::Break(match RegisterOffset::try_from(offset) { + let value = match RegisterOffset::try_from(offset) { Err(v) if (0x3f8..0x400).contains(&(v >> 2)) => { - u64::from(self.device_id[(offset - 0xfe0) >> 2]) + u32::from(self.device_id[(offset - 0xfe0) >> 2]) } Err(_) => { // qemu_log_mask(LOG_GUEST_ERROR, "pl011_read: Bad offset 0x%x\n", (int)offset); @@ -214,27 +214,25 @@ impl PL011State { let c = u32::from(c); return std::ops::ControlFlow::Continue(u64::from(c)); } - Ok(RSR) => u8::from(self.receive_status_error_clear).into(), - Ok(FR) => u16::from(self.flags).into(), - Ok(FBRD) => self.fbrd.into(), - Ok(ILPR) => self.ilpr.into(), - Ok(IBRD) => self.ibrd.into(), - Ok(LCR_H) => u16::from(self.line_control).into(), - Ok(CR) => { - // We exercise our self-control. - u16::from(self.control).into() - } - Ok(FLS) => self.ifl.into(), - Ok(IMSC) => self.int_enabled.into(), - Ok(RIS) => self.int_level.into(), - Ok(MIS) => u64::from(self.int_level & self.int_enabled), + Ok(RSR) => u32::from(self.receive_status_error_clear), + Ok(FR) => u32::from(self.flags), + Ok(FBRD) => self.fbrd, + Ok(ILPR) => self.ilpr, + Ok(IBRD) => self.ibrd, + Ok(LCR_H) => u32::from(self.line_control), + Ok(CR) => u32::from(self.control), + Ok(FLS) => self.ifl, + Ok(IMSC) => self.int_enabled, + Ok(RIS) => self.int_level, + Ok(MIS) => self.int_level & self.int_enabled, Ok(ICR) => { // "The UARTICR Register is the interrupt clear register and is write-only" // Source: ARM DDI 0183G 3.3.13 Interrupt Clear Register, UARTICR 0 } - Ok(DMACR) => self.dmacr.into(), - }) + Ok(DMACR) => self.dmacr, + }; + std::ops::ControlFlow::Break(value.into()) } pub fn write(&mut self, offset: hwaddr, value: u64) { @@ -276,7 +274,6 @@ impl PL011State { self.fbrd = value; } Ok(LCR_H) => { - let value = value as u16; let new_val: registers::LineControl = value.into(); // Reset the FIFO state on FIFO enable or disable if bool::from(self.line_control.fifos_enabled()) @@ -303,7 +300,6 @@ impl PL011State { } Ok(CR) => { // ??? Need to implement the enable bit. - let value = value as u16; self.control = value.into(); self.loopback_mdmctrl(); } diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs index 463ae60543..0747e130ca 100644 --- a/rust/hw/char/pl011/src/lib.rs +++ b/rust/hw/char/pl011/src/lib.rs @@ -131,12 +131,6 @@ impl core::convert::TryFrom for RegisterOffset { pub mod registers { //! Device registers exposed as typed structs which are backed by arbitrary //! integer bitmaps. [`Data`], [`Control`], [`LineControl`], etc. - //! - //! All PL011 registers are essentially 32-bit wide, but are typed here as - //! bitmaps with only the necessary width. That is, if a struct bitmap - //! in this module is for example 16 bits long, it should be conceived - //! as a 32-bit register where the unmentioned higher bits are always - //! unused thus treated as zero when read or written. use bilge::prelude::*; /// Receive Status Register / Data Register common error bits @@ -234,10 +228,11 @@ pub mod registers { /// # Source /// ARM DDI 0183G 3.3.2 Receive Status Register/Error Clear Register, /// UARTRSR/UARTECR - #[bitsize(8)] + #[bitsize(32)] #[derive(Clone, Copy, DebugBits, FromBits)] pub struct ReceiveStatusErrorClear { pub errors: Errors, + _reserved_unpredictable: u24, } impl ReceiveStatusErrorClear { @@ -257,7 +252,7 @@ pub mod registers { } } - #[bitsize(16)] + #[bitsize(32)] #[derive(Clone, Copy, DebugBits, FromBits)] /// Flag Register, `UARTFR` #[doc(alias = "UARTFR")] @@ -309,7 +304,7 @@ pub mod registers { pub transmit_fifo_empty: bool, /// `RI`, is `true` when `nUARTRI` is `LOW`. pub ring_indicator: bool, - _reserved_zero_no_modify: u7, + _reserved_zero_no_modify: u23, } impl Flags { @@ -328,7 +323,7 @@ pub mod registers { } } - #[bitsize(16)] + #[bitsize(32)] #[derive(Clone, Copy, DebugBits, FromBits)] /// Line Control Register, `UARTLCR_H` #[doc(alias = "UARTLCR_H")] @@ -382,8 +377,8 @@ pub mod registers { /// the PEN bit disables parity checking and generation. See Table 3-11 /// on page 3-14 for the parity truth table. pub sticky_parity: bool, - /// 15:8 - Reserved, do not modify, read as zero. - _reserved_zero_no_modify: u8, + /// 31:8 - Reserved, do not modify, read as zero. + _reserved_zero_no_modify: u24, } impl LineControl { @@ -454,7 +449,7 @@ pub mod registers { /// /// # Source /// ARM DDI 0183G, 3.3.8 Control Register, `UARTCR`, Table 3-12 - #[bitsize(16)] + #[bitsize(32)] #[doc(alias = "UARTCR")] #[derive(Clone, Copy, DebugBits, FromBits)] pub struct Control { @@ -532,6 +527,8 @@ pub mod registers { /// CTS hardware flow control is enabled. Data is only transmitted when /// the `nUARTCTS` signal is asserted. pub cts_hardware_flow_control_enable: bool, + /// 31:16 - Reserved, do not modify, read as zero. + _reserved_zero_no_modify2: u16, } impl Control { -- 2.30.2