Generalize timer_and_addr() to decode all registers into a single enum
HPETRegister, and use the TryInto derive to separate valid and
invalid values.
The main advantage lies in checking that all registers are enumerated
in the "match" statements.
Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
---
rust/Cargo.toml | 2 +
rust/hw/char/pl011/src/lib.rs | 2 -
rust/hw/timer/hpet/src/hpet.rs | 204 +++++++++++++++++----------------
3 files changed, 110 insertions(+), 98 deletions(-)
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 5041d6291fd..ab1185a8143 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -37,6 +37,8 @@ result_unit_err = "allow"
should_implement_trait = "deny"
# can be for a reason, e.g. in callbacks
unused_self = "allow"
+# common in device crates
+upper_case_acronyms = "allow"
# default-allow lints
as_ptr_cast_mut = "deny"
diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
index 45c13ba899e..dbae76991c9 100644
--- a/rust/hw/char/pl011/src/lib.rs
+++ b/rust/hw/char/pl011/src/lib.rs
@@ -12,8 +12,6 @@
//! See [`PL011State`](crate::device::PL011State) for the device model type and
//! the [`registers`] module for register types.
-#![allow(clippy::upper_case_acronyms)]
-
use qemu_api::c_str;
mod device;
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index a440c9f4cb9..af273f02c54 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -48,8 +48,6 @@
const HPET_CLK_PERIOD: u64 = 10; // 10 ns
const FS_PER_NS: u64 = 1000000; // 1000000 femtoseconds == 1 ns
-/// General Capabilities and ID Register
-const HPET_CAP_REG: u64 = 0x000;
/// Revision ID (bits 0:7). Revision 1 is implemented (refer to v1.0a spec).
const HPET_CAP_REV_ID_VALUE: u64 = 0x1;
const HPET_CAP_REV_ID_SHIFT: usize = 0;
@@ -65,8 +63,6 @@
/// Main Counter Tick Period (bits 32:63)
const HPET_CAP_CNT_CLK_PERIOD_SHIFT: usize = 32;
-/// General Configuration Register
-const HPET_CFG_REG: u64 = 0x010;
/// Overall Enable (bit 0)
const HPET_CFG_ENABLE_SHIFT: usize = 0;
/// Legacy Replacement Route (bit 1)
@@ -74,14 +70,6 @@
/// Other bits are reserved.
const HPET_CFG_WRITE_MASK: u64 = 0x003;
-/// General Interrupt Status Register
-const HPET_INT_STATUS_REG: u64 = 0x020;
-
-/// Main Counter Value Register
-const HPET_COUNTER_REG: u64 = 0x0f0;
-
-/// Timer N Configuration and Capability Register (masked by 0x18)
-const HPET_TN_CFG_REG: u64 = 0x000;
/// bit 0, 7, and bits 16:31 are reserved.
/// bit 4, 5, 15, and bits 32:64 are read-only.
const HPET_TN_CFG_WRITE_MASK: u64 = 0x7f4e;
@@ -109,11 +97,51 @@
/// Timer N Interrupt Routing Capability (bits 32:63)
const HPET_TN_CFG_INT_ROUTE_CAP_SHIFT: usize = 32;
-/// Timer N Comparator Value Register (masked by 0x18)
-const HPET_TN_CMP_REG: u64 = 0x008;
+#[derive(qemu_api_macros::TryInto)]
+#[repr(u64)]
+#[allow(non_camel_case_types)]
+/// Timer registers, masked by 0x18
+enum TimerRegister {
+ /// Timer N Configuration and Capability Register
+ CFG = 0,
+ /// Timer N Comparator Value Register
+ CMP = 8,
+ /// Timer N FSB Interrupt Route Register
+ ROUTE = 16,
+}
-/// Timer N FSB Interrupt Route Register (masked by 0x18)
-const HPET_TN_FSB_ROUTE_REG: u64 = 0x010;
+#[derive(qemu_api_macros::TryInto)]
+#[repr(u64)]
+#[allow(non_camel_case_types)]
+/// Global registers
+enum GlobalRegister {
+ /// General Capabilities and ID Register
+ CAP = 0,
+ /// General Configuration Register
+ CFG = 0x10,
+ /// General Interrupt Status Register
+ INT_STATUS = 0x20,
+ /// Main Counter Value Register
+ COUNTER = 0xF0,
+}
+
+enum HPETRegister<'a> {
+ /// Global register in the range from `0` to `0xff`
+ Global(GlobalRegister),
+
+ /// Register in the timer block `0x100`...`0x3ff`
+ Timer(&'a BqlRefCell<HPETTimer>, TimerRegister),
+
+ /// Invalid address
+ #[allow(dead_code)]
+ Unknown(hwaddr),
+}
+
+struct HPETAddrDecode<'a> {
+ shift: u32,
+ len: u32,
+ reg: HPETRegister<'a>,
+}
const fn hpet_next_wrap(cur_tick: u64) -> u64 {
(cur_tick | 0xffffffff) + 1
@@ -460,33 +488,21 @@ fn callback(&mut self) {
self.update_irq(true);
}
- const fn read(&self, addr: hwaddr, _size: u32) -> u64 {
- let shift: u64 = (addr & 4) * 8;
-
- match addr & !4 {
- HPET_TN_CFG_REG => self.config >> shift, // including interrupt
capabilities
- HPET_TN_CMP_REG => self.cmp >> shift, // comparator register
- HPET_TN_FSB_ROUTE_REG => self.fsb >> shift,
- _ => {
- // TODO: Add trace point - trace_hpet_ram_read_invalid()
- // Reserved.
- 0
- }
+ const fn read(&self, reg: TimerRegister) -> u64 {
+ use TimerRegister::*;
+ match reg {
+ CFG => self.config, // including interrupt capabilities
+ CMP => self.cmp, // comparator register
+ ROUTE => self.fsb,
}
}
- fn write(&mut self, addr: hwaddr, value: u64, size: u32) {
- let shift = ((addr & 4) * 8) as u32;
- let len = std::cmp::min(size * 8, 64 - shift);
-
- match addr & !4 {
- HPET_TN_CFG_REG => self.set_tn_cfg_reg(shift, len, value),
- HPET_TN_CMP_REG => self.set_tn_cmp_reg(shift, len, value),
- HPET_TN_FSB_ROUTE_REG => self.set_tn_fsb_route_reg(shift, len,
value),
- _ => {
- // TODO: Add trace point - trace_hpet_ram_write_invalid()
- // Reserved.
- }
+ fn write(&mut self, reg: TimerRegister, value: u64, shift: u32, len: u32) {
+ use TimerRegister::*;
+ match reg {
+ CFG => self.set_tn_cfg_reg(shift, len, value),
+ CMP => self.set_tn_cmp_reg(shift, len, value),
+ ROUTE => self.set_tn_fsb_route_reg(shift, len, value),
}
}
}
@@ -743,76 +759,72 @@ fn reset_hold(&self, _type: ResetType) {
self.rtc_irq_level.set(0);
}
- fn timer_and_addr(&self, addr: hwaddr) -> Option<(&BqlRefCell<HPETTimer>,
hwaddr)> {
- let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
+ fn decode(&self, mut addr: hwaddr, size: u32) -> HPETAddrDecode {
+ let shift = ((addr & 4) * 8) as u32;
+ let len = std::cmp::min(size * 8, 64 - shift);
- // TODO: Add trace point -
trace_hpet_ram_[read|write]_timer_id(timer_id)
- if timer_id > self.num_timers.get() {
- // TODO: Add trace point -
trace_hpet_timer_id_out_of_range(timer_id)
- None
+ addr &= !4;
+ let reg = if (0..=0xff).contains(&addr) {
+ GlobalRegister::try_from(addr).map(HPETRegister::Global)
} else {
- // Keep the complete address so that HPETTimer's read and write
could
- // detect the invalid access.
- Some((&self.timers[timer_id], addr & 0x1F))
- }
+ let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
+ if timer_id <= self.num_timers.get() {
+ // TODO: Add trace point -
trace_hpet_ram_[read|write]_timer_id(timer_id)
+ TimerRegister::try_from(addr)
+ .map(|reg| HPETRegister::Timer(&self.timers[timer_id],
reg))
+ } else {
+ // TODO: Add trace point -
trace_hpet_timer_id_out_of_range(timer_id)
+ Err(addr)
+ }
+ };
+
+ // reg is now a Result<HPETRegister, hwaddr>
+ // convert the Err case into HPETRegister as well
+ let reg = reg.unwrap_or_else(HPETRegister::Unknown);
+ HPETAddrDecode { shift, len, reg }
}
fn read(&self, addr: hwaddr, size: u32) -> u64 {
- let shift: u64 = (addr & 4) * 8;
-
- // address range of all TN regs
// TODO: Add trace point - trace_hpet_ram_read(addr)
- if (0x100..=0x3ff).contains(&addr) {
- match self.timer_and_addr(addr) {
- None => 0, // Reserved,
- Some((timer, tn_addr)) => timer.borrow_mut().read(tn_addr,
size),
- }
- } else {
- match addr & !4 {
- HPET_CAP_REG => self.capability.get() >> shift, /* including
HPET_PERIOD 0x004 */
- // (CNT_CLK_PERIOD field)
- HPET_CFG_REG => self.config.get() >> shift,
- HPET_COUNTER_REG => {
- let cur_tick: u64 = if self.is_hpet_enabled() {
- self.get_ticks()
- } else {
- self.counter.get()
- };
+ let HPETAddrDecode { shift, reg, .. } = self.decode(addr, size);
- // TODO: Add trace point -
trace_hpet_ram_read_reading_counter(addr & 4,
- // cur_tick)
- cur_tick >> shift
- }
- HPET_INT_STATUS_REG => self.int_status.get() >> shift,
- _ => {
- // TODO: Add trace point- trace_hpet_ram_read_invalid()
- // Reserved.
- 0
+ use GlobalRegister::*;
+ use HPETRegister::*;
+ (match reg {
+ Timer(timer, tn_reg) => timer.borrow_mut().read(tn_reg),
+ Global(CAP) => self.capability.get(), /* including HPET_PERIOD
0x004 */
+ Global(CFG) => self.config.get(),
+ Global(INT_STATUS) => self.int_status.get(),
+ Global(COUNTER) => {
+ // TODO: Add trace point
+ // trace_hpet_ram_read_reading_counter(addr & 4, cur_tick)
+ if self.is_hpet_enabled() {
+ self.get_ticks()
+ } else {
+ self.counter.get()
}
}
- }
+ Unknown(_) => {
+ // TODO: Add trace point- trace_hpet_ram_read_invalid()
+ 0
+ }
+ }) >> shift
}
fn write(&self, addr: hwaddr, value: u64, size: u32) {
- let shift = ((addr & 4) * 8) as u32;
- let len = std::cmp::min(size * 8, 64 - shift);
+ let HPETAddrDecode { shift, len, reg } = self.decode(addr, size);
// TODO: Add trace point - trace_hpet_ram_write(addr, value)
- if (0x100..=0x3ff).contains(&addr) {
- match self.timer_and_addr(addr) {
- None => (), // Reserved.
- Some((timer, tn_addr)) => timer.borrow_mut().write(tn_addr,
value, size),
- }
- } else {
- match addr & !0x4 {
- HPET_CAP_REG => {} // General Capabilities and ID Register:
Read Only
- HPET_CFG_REG => self.set_cfg_reg(shift, len, value),
- HPET_INT_STATUS_REG => self.set_int_status_reg(shift, len,
value),
- HPET_COUNTER_REG => self.set_counter_reg(shift, len, value),
- _ => {
- // TODO: Add trace point - trace_hpet_ram_write_invalid()
- // Reserved.
- }
+ use GlobalRegister::*;
+ use HPETRegister::*;
+ match reg {
+ Timer(timer, tn_reg) => timer.borrow_mut().write(tn_reg, value,
shift, len),
+ Global(CAP) => {} // General Capabilities and ID Register: Read
Only
+ Global(CFG) => self.set_cfg_reg(shift, len, value),
+ Global(INT_STATUS) => self.set_int_status_reg(shift, len, value),
+ Global(COUNTER) => self.set_counter_reg(shift, len, value),
+ Unknown(_) => {
+ // TODO: Add trace point - trace_hpet_ram_write_invalid()
}
}
}
--
2.48.1
