@@ -5,7 +5,7 @@
use core::ptr::{addr_of, addr_of_mut, NonNull};
use std::{
ffi::CStr,
- os::raw::{c_int, c_uchar, c_uint, c_void},
+ os::raw::{c_int, c_uint, c_void},
};
use qemu_api::{
@@ -32,6 +32,7 @@
/// QEMU sourced constant.
pub const PL011_FIFO_DEPTH: usize = 16_usize;
+/// State enum that represents the values of the peripheral and PCell registers of a PL011 device.
#[derive(Clone, Copy, Debug)]
enum DeviceId {
#[allow(dead_code)]
@@ -39,20 +40,51 @@ enum DeviceId {
Luminary,
}
-impl std::ops::Index<hwaddr> for DeviceId {
- type Output = c_uchar;
+macro_rules! pcell_reg_getter {
+ ($($(#[$attrs:meta])* fn $getter_fn:ident -> $value:literal),*$(,)?) => {
+ $($(#[$attrs])* const fn $getter_fn(self) -> u64 { $value })*
+ };
+}
- fn index(&self, idx: hwaddr) -> &Self::Output {
- match self {
- Self::Arm => &Self::PL011_ID_ARM[idx as usize],
- Self::Luminary => &Self::PL011_ID_LUMINARY[idx as usize],
- }
- }
+macro_rules! periph_reg_getter {
+ ($($(#[$attrs:meta])* fn $getter_fn:ident -> { Arm => $arm:literal, Luminary => $lum:literal$(,)?}),*$(,)?) => {
+ $(
+ $(#[$attrs])*
+ const fn $getter_fn(self) -> u64 {
+ (match self {
+ Self::Arm => $arm,
+ Self::Luminary => $lum,
+ }) as u64
+ }
+ )*
+ };
}
impl DeviceId {
- const PL011_ID_ARM: [c_uchar; 8] = [0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1];
- const PL011_ID_LUMINARY: [c_uchar; 8] = [0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1];
+ /// Value of `UARTPeriphID0` register, which contains the `PartNumber0` value.
+ const fn uart_periph_id0(self) -> u64 {
+ 0x11
+ }
+
+ periph_reg_getter! {
+ /// Value of `UARTPeriphID1` register, which contains the `Designer0` and `PartNumber1` values.
+ fn uart_periph_id1 -> { Arm => 0x10, Luminary => 0x00 },
+ /// Value of `UARTPeriphID2` register, which contains the `Revision` and `Designer1` values.
+ fn uart_periph_id2 -> { Arm => 0x14, Luminary => 0x18 },
+ /// Value of `UARTPeriphID3` register, which contains the `Configuration` value.
+ fn uart_periph_id3 -> { Arm => 0x0, Luminary => 0x1 }
+ }
+
+ pcell_reg_getter! {
+ /// Value of `UARTPCellID0` register.
+ fn uart_pcell_id0 -> 0x0d,
+ /// Value of `UARTPCellID1` register.
+ fn uart_pcell_id1 -> 0xf0,
+ /// Value of `UARTPCellID2` register.
+ fn uart_pcell_id2 -> 0x05,
+ /// Value of `UARTPCellID3` register.
+ fn uart_pcell_id3 -> 0xb1,
+ }
}
#[repr(C)]
@@ -182,9 +214,14 @@ pub fn read(&mut self, offset: hwaddr, _size: c_uint) -> std::ops::ControlFlow<u
use RegisterOffset::*;
std::ops::ControlFlow::Break(match RegisterOffset::try_from(offset) {
- Err(v) if (0x3f8..0x400).contains(&v) => {
- u64::from(self.device_id[(offset - 0xfe0) >> 2])
- }
+ Ok(PeriphID0) => self.device_id.uart_periph_id0(),
+ Ok(PeriphID1) => self.device_id.uart_periph_id1(),
+ Ok(PeriphID2) => self.device_id.uart_periph_id2(),
+ Ok(PeriphID3) => self.device_id.uart_periph_id3(),
+ Ok(PCellID0) => self.device_id.uart_pcell_id0(),
+ Ok(PCellID1) => self.device_id.uart_pcell_id1(),
+ Ok(PCellID2) => self.device_id.uart_pcell_id2(),
+ Ok(PCellID3) => self.device_id.uart_pcell_id3(),
Err(_) => {
// qemu_log_mask(LOG_GUEST_ERROR, "pl011_read: Bad offset 0x%x\n", (int)offset);
0
@@ -236,9 +273,15 @@ pub fn write(&mut self, offset: hwaddr, value: u64) {
use RegisterOffset::*;
let value: u32 = value as u32;
match RegisterOffset::try_from(offset) {
- Err(_bad_offset) => {
+ Err(_) => {
eprintln!("write bad offset {offset} value {value}");
}
+ Ok(
+ dev_id @ (PeriphID0 | PeriphID1 | PeriphID2 | PeriphID3 | PCellID0 | PCellID1
+ | PCellID2 | PCellID3 | FR | RIS | MIS),
+ ) => {
+ eprintln!("write bad offset {offset} at RO register {dev_id:?} value {value}");
+ }
Ok(DR) => {
// ??? Check if transmitter is enabled.
let ch: u8 = value as u8;
@@ -257,9 +300,6 @@ pub fn write(&mut self, offset: hwaddr, value: u64) {
Ok(RSR) => {
self.receive_status_error_clear = 0.into();
}
- Ok(FR) => {
- // flag writes are ignored
- }
Ok(ILPR) => {
self.ilpr = value;
}
@@ -308,8 +348,6 @@ pub fn write(&mut self, offset: hwaddr, value: u64) {
self.int_enabled = value;
self.update();
}
- Ok(RIS) => {}
- Ok(MIS) => {}
Ok(ICR) => {
self.int_level &= !value;
self.update();
@@ -111,6 +111,22 @@ pub enum RegisterOffset {
/// DMA control Register
#[doc(alias = "UARTDMACR")]
DMACR = 0x048,
+ #[doc(alias = "UARTPeriphID0")]
+ PeriphID0 = 0xFE0,
+ #[doc(alias = "UARTPeriphID1")]
+ PeriphID1 = 0xFE4,
+ #[doc(alias = "UARTPeriphID2")]
+ PeriphID2 = 0xFE8,
+ #[doc(alias = "UARTPeriphID3")]
+ PeriphID3 = 0xFEC,
+ #[doc(alias = "UARTPCellID0")]
+ PCellID0 = 0xFF0,
+ #[doc(alias = "UARTPCellID1")]
+ PCellID1 = 0xFF4,
+ #[doc(alias = "UARTPCellID2")]
+ PCellID2 = 0xFF8,
+ #[doc(alias = "UARTPCellID3")]
+ PCellID3 = 0xFFC,
///// Reserved, offsets `0x04C` to `0x07C`.
//Reserved = 0x04C,
}
@@ -137,7 +153,11 @@ const fn _assert_exhaustive(val: RegisterOffset) {
}
}
}
- case! { DR, RSR, FR, FBRD, ILPR, IBRD, LCR_H, CR, FLS, IMSC, RIS, MIS, ICR, DMACR }
+ case! {
+ DR, RSR, FR, FBRD, ILPR, IBRD, LCR_H, CR, FLS, IMSC, RIS, MIS, ICR, DMACR,
+ PeriphID0, PeriphID1, PeriphID2, PeriphID3,
+ PCellID0, PCellID1, PCellID2, PCellID3,
+ }
}
}
DeviceId, which maps the peripheral and PCell registers of a PL011 device, was not treating each register value as a 32 bit value. Change DeviceId enum to return register values via constified getter functions instead of leveraging the std::ops::Index<_> trait. While at it, print errors when guest attempts to write to other RO registers as well. Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> --- Notes: Changes from v2: - Group invalid write case matches (Paolo) - Reduce register getter line count to aid review (Peter Maydell) v1 -> v2: - Print error when guest attempts to write to RO registers (Peter Maydell) Version 1: <20241116181549.3430225-1-manos.pitsidianakis@linaro.org> Version 2: <20241116221757.3501603-1-manos.pitsidianakis@linaro.org> Interdiff against v2: diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index fc6f3f394d..f1d959ca28 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -40,54 +40,50 @@ enum DeviceId { Luminary, } +macro_rules! pcell_reg_getter { + ($($(#[$attrs:meta])* fn $getter_fn:ident -> $value:literal),*$(,)?) => { + $($(#[$attrs])* const fn $getter_fn(self) -> u64 { $value })* + }; +} + +macro_rules! periph_reg_getter { + ($($(#[$attrs:meta])* fn $getter_fn:ident -> { Arm => $arm:literal, Luminary => $lum:literal$(,)?}),*$(,)?) => { + $( + $(#[$attrs])* + const fn $getter_fn(self) -> u64 { + (match self { + Self::Arm => $arm, + Self::Luminary => $lum, + }) as u64 + } + )* + }; +} + impl DeviceId { /// Value of `UARTPeriphID0` register, which contains the `PartNumber0` value. const fn uart_periph_id0(self) -> u64 { 0x11 } - /// Value of `UARTPeriphID1` register, which contains the `Designer0` and `PartNumber1` values. - const fn uart_periph_id1(self) -> u64 { - (match self { - Self::Arm => 0x10, - Self::Luminary => 0x00, - }) as u64 + periph_reg_getter! { + /// Value of `UARTPeriphID1` register, which contains the `Designer0` and `PartNumber1` values. + fn uart_periph_id1 -> { Arm => 0x10, Luminary => 0x00 }, + /// Value of `UARTPeriphID2` register, which contains the `Revision` and `Designer1` values. + fn uart_periph_id2 -> { Arm => 0x14, Luminary => 0x18 }, + /// Value of `UARTPeriphID3` register, which contains the `Configuration` value. + fn uart_periph_id3 -> { Arm => 0x0, Luminary => 0x1 } } - /// Value of `UARTPeriphID2` register, which contains the `Revision` and `Designer1` values. - const fn uart_periph_id2(self) -> u64 { - (match self { - Self::Arm => 0x14, - Self::Luminary => 0x18, - }) as u64 - } - - /// Value of `UARTPeriphID3` register, which contains the `Configuration` value. - const fn uart_periph_id3(self) -> u64 { - (match self { - Self::Arm => 0x0, - Self::Luminary => 0x1, - }) as u64 - } - - /// Value of `UARTPCellID0` register. - const fn uart_pcell_id0(self) -> u64 { - 0x0d - } - - /// Value of `UARTPCellID1` register. - const fn uart_pcell_id1(self) -> u64 { - 0xf0 - } - - /// Value of `UARTPCellID2` register. - const fn uart_pcell_id2(self) -> u64 { - 0x05 - } - - /// Value of `UARTPCellID3` register. - const fn uart_pcell_id3(self) -> u64 { - 0xb1 + pcell_reg_getter! { + /// Value of `UARTPCellID0` register. + fn uart_pcell_id0 -> 0x0d, + /// Value of `UARTPCellID1` register. + fn uart_pcell_id1 -> 0xf0, + /// Value of `UARTPCellID2` register. + fn uart_pcell_id2 -> 0x05, + /// Value of `UARTPCellID3` register. + fn uart_pcell_id3 -> 0xb1, } } @@ -282,7 +278,7 @@ pub fn write(&mut self, offset: hwaddr, value: u64) { } Ok( dev_id @ (PeriphID0 | PeriphID1 | PeriphID2 | PeriphID3 | PCellID0 | PCellID1 - | PCellID2 | PCellID3), + | PCellID2 | PCellID3 | FR | RIS | MIS), ) => { eprintln!("write bad offset {offset} at RO register {dev_id:?} value {value}"); } @@ -304,9 +300,6 @@ pub fn write(&mut self, offset: hwaddr, value: u64) { Ok(RSR) => { self.receive_status_error_clear = 0.into(); } - Ok(FR) => { - eprintln!("write bad offset {offset} at RO register UARTFR value {value}"); - } Ok(ILPR) => { self.ilpr = value; } @@ -355,12 +348,6 @@ pub fn write(&mut self, offset: hwaddr, value: u64) { self.int_enabled = value; self.update(); } - Ok(RIS) => { - eprintln!("write bad offset {offset} at RO register UARTRIS value {value}"); - } - Ok(MIS) => { - eprintln!("write bad offset {offset} at RO register UARTMIS value {value}"); - } Ok(ICR) => { self.int_level &= !value; self.update(); rust/hw/char/pl011/src/device.rs | 78 ++++++++++++++++++++++++-------- rust/hw/char/pl011/src/lib.rs | 22 ++++++++- 2 files changed, 79 insertions(+), 21 deletions(-) base-commit: 43f2def68476697deb0d119cbae51b20019c6c86