diff --git a/src/riscv/lib/src/machine_state/address_translation.rs b/src/riscv/lib/src/machine_state/address_translation.rs index d0c423d2bac684d80d18b1e62976707df1c08ec6..3cbeffd1f261dd98a5c099289864e9267a945132 100644 --- a/src/riscv/lib/src/machine_state/address_translation.rs +++ b/src/riscv/lib/src/machine_state/address_translation.rs @@ -214,16 +214,16 @@ impl MachineState mode: Mode, satp: CSRRepr, access_type: AccessType, - ) -> Option { + ) -> TranslationAlgorithm { // 1. Let a be satp.ppn × PAGESIZE, and let i = LEVELS − 1. // The satp register must be active, i.e., // the effective privilege mode must be S-mode or U-mode. if let Mode::Machine = self.effective_translation_hart_mode(mode, access_type) { - return None; + return TranslationAlgorithm::Bare; } - Satp::from_bits(satp).mode().ok() + Satp::from_bits(satp).mode() } /// Like [`Self::translate`] but allows the caller to prevent extraneous reads from CSRs. @@ -237,15 +237,17 @@ impl MachineState ) -> Result { let mode = self.effective_translation_alg(mode, satp, access_type); - match mode { - // An invalid mode should not be writable, but it is treated as BARE - None | Some(TranslationAlgorithm::Bare) => Ok(virt_addr), - Some(TranslationAlgorithm::Sv(length)) => { - let satp = self.hart.csregisters.read(CSRegister::satp); - sv_translate_impl(&self.bus, virt_addr, satp, length, access_type) - .map_err(|_e| access_type.exception(virt_addr)) - } - } + use TranslationAlgorithm::*; + let sv_length = match mode { + Bare => return Ok(virt_addr), + Sv39 => SvLength::Sv39, + Sv48 => SvLength::Sv48, + Sv57 => SvLength::Sv57, + }; + + let satp = self.hart.csregisters.read(CSRegister::satp); + sv_translate_impl(&self.bus, virt_addr, satp, sv_length, access_type) + .map_err(|_e| access_type.exception(virt_addr)) } /// Translate a virtual address to a physical address as described in section 5.3.2 diff --git a/src/riscv/lib/src/machine_state/csregisters.rs b/src/riscv/lib/src/machine_state/csregisters.rs index 8adca79bcc9cdd2cb2a6365ed537898a47aca789..5e0d48f7c79cdb6a7108eb0d9cc8973ba592a027 100644 --- a/src/riscv/lib/src/machine_state/csregisters.rs +++ b/src/riscv/lib/src/machine_state/csregisters.rs @@ -607,7 +607,7 @@ impl CSRegister { CSRegister::mepc | CSRegister::sepc | CSRegister::mnepc => { new_value & CSRegister::WARL_MASK_XEPC } - CSRegister::satp => Satp::from_bits(new_value).normalise()?.to_bits(), + CSRegister::satp => Satp::from_bits(new_value).normalise().to_bits(), CSRegister::mstatus => MStatus::from_bits(new_value).normalise().to_bits(), CSRegister::sstatus => SStatus::from_bits(new_value).normalise().to_bits(), CSRegister::mnstatus => MNStatus::from_bits(new_value).normalise().to_bits(), @@ -1500,7 +1500,8 @@ mod tests { // satp assert_eq!(check_wrapped(csreg::satp, 0x0), Some(0x0)); assert_eq!(check_wrapped(csreg::satp, 0x0000_FFFF_0000_FFFF), Some(0x0)); - assert_eq!(check_wrapped(csreg::satp, 0x4200_FFFF_FFFF_FFFF), None); + // Invalid mode is read as 0 + assert_eq!(check_wrapped(csreg::satp, 0x4200_FFFF_FFFF_FFFF), Some(0x0)); assert_eq!( check_wrapped(csreg::satp, 0x90F0_0000_FFFF_0000), Some(0x90F0_0000_FFFF_0000) diff --git a/src/riscv/lib/src/machine_state/csregisters/satp.rs b/src/riscv/lib/src/machine_state/csregisters/satp.rs index ad86acd8c8a65a392a788c488642fcffdff714ea..fb531754f574ca1385aa038fe0f17554fd85b644 100644 --- a/src/riscv/lib/src/machine_state/csregisters/satp.rs +++ b/src/riscv/lib/src/machine_state/csregisters/satp.rs @@ -36,51 +36,46 @@ pub enum SvLength { } /// `MODE` field of the `satp` register. See table 5.4 -#[derive(PartialEq, Debug)] +#[derive(PartialEq, Debug, Copy, Clone)] +#[repr(u8)] pub enum TranslationAlgorithm { - Bare, - Sv(SvLength), + Bare = 0, + Sv39 = 8, + Sv48 = 9, + Sv57 = 10, } impl TranslationAlgorithm { pub const fn enc(&self) -> CSRRepr { - match self { - Self::Bare => MODE_BARE, - Self::Sv(SvLength::Sv39) => MODE_SV39, - Self::Sv(SvLength::Sv48) => MODE_SV48, - Self::Sv(SvLength::Sv57) => MODE_SV57, - } + *self as u8 as u64 } } /// `Err` represents that the value of SATP.mode is reserved or /// that we do not care about it / is irrelevant. -impl Bits64 for Result { +impl Bits64 for TranslationAlgorithm { const WIDTH: usize = 4; fn from_bits(value: u64) -> Self { - use SvLength::*; use TranslationAlgorithm::*; match value & 0b1111 { - MODE_BARE => Ok(Bare), - MODE_SV39 => Ok(Sv(Sv39)), - MODE_SV48 => Ok(Sv(Sv48)), - MODE_SV57 => Ok(Sv(Sv57)), - value @ (1..=7 | 11..=15) => { - // We need to retain invalid/unknown values so we can re-encode - // them correctly later. In other words we want to be loss-less. - Err(value) - } - 16.. => unreachable!(), + MODE_BARE => Bare, + MODE_SV39 => Sv39, + MODE_SV48 => Sv48, + MODE_SV57 => Sv57, + // The satp.mode is a WARL field. + // This allows us to treat any illegal value as + // a legal one of our choice or to throw an exception. + // We are choosing to treat illegal values as `Bare` mode. + // Note: Reading an illegal value is only be possible if the raw memory is + // tampered with as the `reset` method and any write also leaves a legal value. + _ => Bare, } } fn to_bits(&self) -> u64 { - match self { - Err(code) => *code, - Ok(algorithm) => algorithm.enc(), - } + self.enc() } } @@ -88,7 +83,7 @@ csr! { pub struct Satp { PPN: FixedWidthBits<44>, ASID: FixedWidthBits<16>, - MODE: Result, + MODE: TranslationAlgorithm, } } @@ -100,18 +95,16 @@ impl Default for Satp { impl Satp { /// Normalise the SATP value. - pub fn normalise(self) -> Option { + pub fn normalise(self) -> Self { + use TranslationAlgorithm::*; match self.mode() { - Err(_) => None, - Ok(TranslationAlgorithm::Bare) => { + Bare => { // When no address translation algo is selected, the other fields // have no meaning and shall therefore be reset. - Some( - self.with_ppn(FixedWidthBits::from_bits(0)) - .with_asid(FixedWidthBits::from_bits(0)), - ) + self.with_ppn(FixedWidthBits::from_bits(0)) + .with_asid(FixedWidthBits::from_bits(0)) } - Ok(TranslationAlgorithm::Sv(_)) => Some(self), + Sv39 | Sv48 | Sv57 => self, } } } @@ -120,7 +113,7 @@ impl Satp { mod tests { use crate::{ bits::Bits64, - machine_state::csregisters::satp::{Satp, SvLength, TranslationAlgorithm}, + machine_state::csregisters::satp::{Satp, TranslationAlgorithm}, }; #[test] @@ -128,29 +121,29 @@ mod tests { let field = u64::from_bits(0xF0F0_0BC0_AAAA_DEAD); assert_eq!(field.to_bits(), 0xF0F0_0BC0_AAAA_DEAD); - type AlgoField = Result; + type AlgoField = TranslationAlgorithm; let field = ::from_bits(0x0000); - assert_eq!(field, Ok(TranslationAlgorithm::Bare)); + assert_eq!(field, TranslationAlgorithm::Bare); // This `FieldValue` looks at only at the 4 least significant bits let field = ::from_bits(0xFFFF_FFF0); - assert_eq!(field, Ok(TranslationAlgorithm::Bare)); + assert_eq!(field, TranslationAlgorithm::Bare); let field = ::from_bits(0x0002); - assert_eq!(field, Err(0x2)); + assert_eq!(field, TranslationAlgorithm::Bare); let field = ::from_bits(0x0008); - assert_eq!(field, Ok(TranslationAlgorithm::Sv(SvLength::Sv39))); + assert_eq!(field, TranslationAlgorithm::Sv39); let field = ::from_bits(0x0009); - assert_eq!(field, Ok(TranslationAlgorithm::Sv(SvLength::Sv48))); + assert_eq!(field, TranslationAlgorithm::Sv48); let field = ::from_bits(0x000A); - assert_eq!(field, Ok(TranslationAlgorithm::Sv(SvLength::Sv57))); + assert_eq!(field, TranslationAlgorithm::Sv57); let field = ::from_bits(0x000B); - assert_eq!(field, Err(0xB)); + assert_eq!(field, TranslationAlgorithm::Bare); } #[test] @@ -159,15 +152,15 @@ mod tests { let mode = satp.mode(); let asid = satp.asid(); let ppn = satp.ppn(); - assert_eq!(mode, Ok(TranslationAlgorithm::Sv(SvLength::Sv39))); + assert_eq!(mode, TranslationAlgorithm::Sv39); assert_eq!(asid.to_bits(), 0xD07); assert_eq!(ppn.to_bits(), 0xABC_DEAD_0BAD); - let satp = satp.with_mode(Ok(TranslationAlgorithm::Bare)); + let satp = satp.with_mode(TranslationAlgorithm::Bare); let mode = satp.mode(); let asid = satp.asid(); let ppn = satp.ppn(); - assert_eq!(mode, Ok(TranslationAlgorithm::Bare)); + assert_eq!(mode, TranslationAlgorithm::Bare); assert_eq!(asid.to_bits(), 0xD07); assert_eq!(ppn.to_bits(), 0xABC_DEAD_0BAD); } diff --git a/src/riscv/sandbox/src/commands/debug/debugger_app.rs b/src/riscv/sandbox/src/commands/debug/debugger_app.rs index f6d8e1a0eae0dba2fa04294c54c9760fd1866ac5..fef25cf1b9ec43ce8af4d2810731707269d6caea 100644 --- a/src/riscv/sandbox/src/commands/debug/debugger_app.rs +++ b/src/riscv/sandbox/src/commands/debug/debugger_app.rs @@ -93,8 +93,6 @@ impl EffectiveTranslationState { } enum SATPModeState { - // Reserved / Unsupported - Invalid, // Translation is BARE mode Bare, // Translation is SvXY mode @@ -111,26 +109,26 @@ impl TranslationState { pub(super) fn update( &mut self, faulting: bool, - effective_mode: Option, + effective_mode: TranslationAlgorithm, satp_val: Satp, ) { self.effective = if faulting { EffectiveTranslationState::Faulting } else { match effective_mode { - None => EffectiveTranslationState::Off, - Some(_alg) => EffectiveTranslationState::On, + Bare => EffectiveTranslationState::Off, + Sv39 | Sv48 | Sv57 => EffectiveTranslationState::On, } }; self.base = satp_val.ppn().to_bits(); - self.mode = match satp_val.mode().ok() { - None => SATPModeState::Invalid, - Some(alg) => match alg { - TranslationAlgorithm::Bare => SATPModeState::Bare, - TranslationAlgorithm::Sv(length) => SATPModeState::Sv(length), - }, + use TranslationAlgorithm::*; + self.mode = match satp_val.mode() { + Bare => SATPModeState::Bare, + Sv39 => SATPModeState::Sv(SvLength::Sv39), + Sv48 => SATPModeState::Sv(SvLength::Sv48), + Sv57 => SATPModeState::Sv(SvLength::Sv57), } } } diff --git a/src/riscv/sandbox/src/commands/debug/debugger_app/render.rs b/src/riscv/sandbox/src/commands/debug/debugger_app/render.rs index e85cd9783de593933adc28e4c4236cb894ce4491..1e825501db6db2d3a55998978a8d0da68c55b569 100644 --- a/src/riscv/sandbox/src/commands/debug/debugger_app/render.rs +++ b/src/riscv/sandbox/src/commands/debug/debugger_app/render.rs @@ -66,7 +66,6 @@ macro_rules! fregister_line { impl SATPModeState { pub fn text(&self) -> &'static str { match self { - Self::Invalid => "Invalid", Self::Bare => "Bare", Self::Sv(length) => match length { SvLength::Sv39 => "Sv39", @@ -78,7 +77,6 @@ impl SATPModeState { pub fn fg(&self) -> Color { match self { - Self::Invalid => GRAY, Self::Bare => BLUE, Self::Sv(_) => GREEN, }