Fix #15: Implement a proper CSR file

Dummy implementation of all known but unsupported CSRs

Author: tommythorn

TODO.md

@@ -4,15 +4,14 @@
 
 - Fix boot with the newest kernels (it hangs, unknown why)
 
-- CSR handling is still a bit suspect
-  The access checks looks broken and we shouldn't have a CSR array but instead
-  individually implement CSR registers we support and trap access to everything else.
-  need to cross check the read/write CSR behavior against Dromajo
+- CSR handling is still a bit suspect; the access checks looks
+  broken. Need to cross check the read/write CSR behavior against
+  Dromajo
 
 - Go though all explicit usage of cpu.f[] and look for mistakes
 
 - Pass all of riscv-test (status: debug, svnapot, and lots of FP.
-  Might punt on debug and svnapot)
+  Might punt on debug and svnapot for now)
 
 - Pass all of riscof
 

src/cpu.rs

@@ -173,7 +173,7 @@ pub struct Cpu {
 
     // Supervisor and CSR
     pub cycle: u64,
-    csr: Box<[u64]>, // XXX this should be replaced with individual registers
+    csr: csr::CsrFile,
     reservation: Option<u64>,
 
     // Wait-For-Interrupt; relax and await further instruction
@@ -237,15 +237,11 @@ impl Cpu {
             cycle: 0,
             wfi: false,
             pc: 0,
-            csr: vec![0; 4096].into_boxed_slice(), // XXX MUST GO AWAY SOON
+            csr: CsrFile::new(),
             mmu,
             reservation: None,
             flush_icache: false,
         };
-        cpu.csr[Csr::Misa as usize] = 1 << 63; // RV64
-        for c in "SUIMAFDC".bytes() {
-            cpu.csr[Csr::Misa as usize] |= 1 << (c as usize - 65);
-        }
         cpu.mmu.mstatus = 2 << MSTATUS_UXL_SHIFT | 2 << MSTATUS_SXL_SHIFT | 3 << MSTATUS_MPP_SHIFT;
         cpu.write_x(x(11), 0x1020); // start of DTB (XXX could put that elsewhere);
         cpu
@@ -334,7 +330,7 @@ impl Cpu {
     fn step_cpu(&mut self, uop_cache: &mut IntMap<u64, Uop>) -> Result<(), Exception> {
         self.cycle = self.cycle.wrapping_add(1);
         if self.wfi {
-            if self.mmu.mip & self.read_csr_raw(Csr::Mie) != 0 {
+            if self.mmu.mip & self.csr.mie != 0 {
                 self.wfi = false;
             }
             return Ok(());
@@ -400,7 +396,7 @@ impl Cpu {
         use self::Trap::SupervisorExternalInterrupt;
         use self::Trap::SupervisorSoftwareInterrupt;
         use self::Trap::SupervisorTimerInterrupt;
-        let minterrupt = self.mmu.mip & self.read_csr_raw(Csr::Mie);
+        let minterrupt = self.mmu.mip & self.csr.mie;
         if minterrupt == 0 {
             return;
         }
@@ -442,16 +438,16 @@ impl Cpu {
         // First, determine which privilege mode should handle the trap.
         // @TODO: Check if this logic is correct
         let mdeleg = if is_interrupt {
-            self.read_csr_raw(Csr::Mideleg)
+            self.csr.mideleg
         } else {
-            self.read_csr_raw(Csr::Medeleg)
+            self.csr.medeleg
         };
         let sdeleg = if is_interrupt {
-            self.read_csr_raw(Csr::Sideleg)
+            self.csr.sideleg
         } else {
-            self.read_csr_raw(Csr::Sedeleg)
+            self.csr.sedeleg
         };
-        let pos = cause & 0xffff;
+        let pos = cause & 63;
 
         let new_priv_mode = if (mdeleg >> pos) & 1 == 0 {
             PrivMode::M
@@ -468,11 +464,11 @@ impl Cpu {
             let current_status = match self.mmu.prv {
                 PrivMode::M => self.read_csr_raw(Csr::Mstatus),
                 PrivMode::S => self.read_csr_raw(Csr::Sstatus),
-                PrivMode::U => self.read_csr_raw(Csr::Ustatus),
+                PrivMode::U => self.csr.ustatus,
             };
 
             let ie = match new_priv_mode {
-                PrivMode::M => self.read_csr_raw(Csr::Mie),
+                PrivMode::M => self.csr.mie,
                 PrivMode::S => self.read_csr_raw(Csr::Sie),
                 PrivMode::U => self.read_csr_raw(Csr::Uie),
             };
@@ -582,16 +578,15 @@ impl Cpu {
             PrivMode::S => Csr::Stval,
             PrivMode::U => Csr::Utval,
         };
-        let csr_tvec_address = match self.mmu.prv {
-            PrivMode::M => Csr::Mtvec,
-            PrivMode::S => Csr::Stvec,
-            PrivMode::U => Csr::Utvec,
+        self.pc = match self.mmu.prv {
+            PrivMode::M => self.read_csr_raw(Csr::Mtvec),
+            PrivMode::S => self.read_csr_raw(Csr::Stvec),
+            PrivMode::U => self.read_csr_raw(Csr::Utvec),
         };
 
         self.write_csr_raw(csr_epc_address, insn_addr);
         self.write_csr_raw(csr_cause_address, cause);
         self.write_csr_raw(csr_tval_address, exc.tval);
-        self.pc = self.read_csr_raw(csr_tvec_address);
 
         // Add 4 * cause if tvec has vector type address
         if self.pc & 3 != 0 {
@@ -663,6 +658,7 @@ impl Cpu {
                 if self.mmu.prv == S && self.mmu.mstatus & MSTATUS_TVM != 0 {
                     return illegal;
                 }
+                return Ok(self.mmu.satp);
             }
             _ => {}
         }
@@ -671,7 +667,6 @@ impl Cpu {
 
     #[allow(clippy::cast_sign_loss)]
     fn write_csr(&mut self, csrno: u16, value: u64) -> Result<(), Exception> {
-        let mut value = value;
         let illegal = Err(Exception {
             trap: Trap::IllegalInstruction,
             tval: 0,
@@ -687,10 +682,6 @@ impl Cpu {
         }
 
         match csr {
-            Csr::Mstatus => {
-                let mask = MSTATUS_MASK & !(MSTATUS_VS | MSTATUS_UXL_MASK | MSTATUS_SXL_MASK);
-                value = value & mask | self.mmu.mstatus & !mask;
-            }
             Csr::Fflags | Csr::Frm | Csr::Fcsr => self.check_float_access_and_dirty(0)?,
             Csr::Cycle => {
                 log::info!("** deny cycle writing");
@@ -708,6 +699,10 @@ impl Cpu {
                     log::warn!("wrote illegal value {value:x} to satp");
                     return illegal;
                 }
+
+                self.mmu.satp = value;
+                self.mmu.clear_page_cache();
+                return Ok(());
             }
             _ => {}
         }
@@ -720,82 +715,96 @@ impl Cpu {
     #[allow(clippy::cast_sign_loss)]
     fn read_csr_raw(&self, csr: Csr) -> u64 {
         match csr {
+            Csr::Cycle | Csr::Mcycle | Csr::Minstret => self.cycle,
+            Csr::Fcsr => self.read_fcsr(),
             Csr::Fflags => u64::from(self.read_fflags()),
             Csr::Frm => self.read_frm() as u64,
-            Csr::Fcsr => self.read_fcsr(),
-            Csr::Sstatus => {
-                let mut sstatus = self.mmu.mstatus;
-                sstatus &= !MSTATUS_FS;
-                sstatus |= u64::from(self.fs) << MSTATUS_FS_SHIFT;
-                sstatus &= 0x8000_0003_000d_e162;
+            Csr::Mcause => self.csr.mcause,
+            Csr::Medeleg => self.csr.medeleg,
+            Csr::Mepc => self.csr.mepc,
+            Csr::Mhartid => self.csr.mhartid,
+            Csr::Mideleg => self.csr.mideleg,
+            Csr::Mie => self.csr.mie,
+            Csr::Mip => self.mmu.mip,
+            Csr::Misa => self.csr.misa,
+            Csr::Mscratch => self.csr.mscratch,
+            Csr::Mstatus => {
+                let mut mstatus = self.mmu.mstatus;
+                mstatus &= !MSTATUS_FS;
+                mstatus |= u64::from(self.fs) << MSTATUS_FS_SHIFT;
                 if self.fs == 3 {
-                    sstatus |= 1 << 63;
+                    mstatus |= 1 << 63;
                 }
-                sstatus
+                mstatus
             }
-            Csr::Mstatus => {
+            Csr::Mtval => self.csr.mtval,
+            Csr::Mtvec => self.csr.mtvec,
+            Csr::Satp => self.mmu.satp,
+            Csr::Scause => self.csr.scause,
+            Csr::Sedeleg => self.csr.sedeleg,
+            Csr::Sepc => self.csr.sepc,
+            Csr::Sideleg => self.csr.sideleg,
+            Csr::Sie => self.csr.mie & self.csr.mideleg,
+            Csr::Sip => self.mmu.mip & self.csr.mideleg,
+            Csr::Sscratch => self.csr.sscratch,
+            Csr::Sstatus => {
                 let mut mstatus = self.mmu.mstatus;
                 mstatus &= !MSTATUS_FS;
                 mstatus |= u64::from(self.fs) << MSTATUS_FS_SHIFT;
+                mstatus &= 0x8000_0003_000d_e162;
                 if self.fs == 3 {
                     mstatus |= 1 << 63;
                 }
                 mstatus
             }
-            Csr::Sie => self.csr[Csr::Mie as usize] & self.csr[Csr::Mideleg as usize],
-            Csr::Sip => self.mmu.mip & self.csr[Csr::Mideleg as usize],
-            Csr::Mip => self.mmu.mip,
+            Csr::Stval => self.csr.stval,
+            Csr::Stvec => self.csr.stvec,
             Csr::Time => self.mmu.get_clint().read_mtime(),
-            Csr::Cycle | Csr::Mcycle | Csr::Minstret => self.cycle,
-            Csr::Satp => self.mmu.satp,
-            _ => self.csr[csr as usize],
+            Csr::Ustatus => self.csr.ustatus,
+            _ => 0,
         }
     }
 
     fn write_csr_raw(&mut self, csr: Csr, value: u64) {
         match csr {
-            Csr::Misa => {} // Not writable
+            Csr::Mcycle => self.cycle = value,
+            Csr::Fcsr => self.write_fcsr(value),
             Csr::Fflags => self.write_fflags((value & 31) as u8),
             Csr::Frm => self.write_frm(
                 FromPrimitive::from_u64(value & 7).unwrap_or(RoundingMode::RoundNearestEven),
             ),
-            Csr::Fcsr => self.write_fcsr(value),
+            Csr::Mcause => self.csr.mcause = value,
+            Csr::Medeleg => self.csr.medeleg = value,
+            Csr::Mepc => self.csr.mepc = value,
+            Csr::Mhartid => self.csr.mhartid = value,
+            Csr::Mideleg => self.csr.mideleg = value & 0x222,
+            Csr::Mie => self.csr.mie = value,
+            Csr::Mip => self.mmu.mip = value,
+            Csr::Mscratch => self.csr.mscratch = value,
+            Csr::Mstatus => {
+                let mask = MSTATUS_MASK & !(MSTATUS_VS | MSTATUS_UXL_MASK | MSTATUS_SXL_MASK);
+                self.mmu.mstatus = value & mask | self.mmu.mstatus & !mask;
+                self.fs = ((value >> MSTATUS_FS_SHIFT) & 3) as u8;
+            }
+            Csr::Mtval => self.csr.mtval = value,
+            Csr::Mtvec => self.csr.mtvec = value,
+            Csr::Scause => self.csr.scause = value,
+            Csr::Sedeleg => self.csr.sedeleg = value,
+            Csr::Sepc => self.csr.sepc = value,
+            Csr::Sideleg => self.csr.sideleg = value,
+            Csr::Sie => self.csr.mie = self.csr.mie & !0x222 | value & 0x222,
+            Csr::Sip => self.mmu.mip = value & 0x222 | self.mmu.mip & !0x222,
+            Csr::Sscratch => self.csr.sscratch = value,
             Csr::Sstatus => {
                 self.mmu.mstatus &= !0x8000_0003_000d_e162;
                 self.mmu.mstatus |= value & 0x8000_0003_000d_e162;
                 self.fs = ((value >> MSTATUS_FS_SHIFT) & 3) as u8;
             }
-            Csr::Sie => {
-                self.csr[Csr::Mie as usize] &= !0x222;
-                self.csr[Csr::Mie as usize] |= value & 0x222;
-            }
-            Csr::Sip => {
-                let mask = 0x222;
-                self.mmu.mip = value & mask | self.mmu.mip & !mask;
-            }
-            Csr::Mip => {
-                let mask = !0; // XXX 0x555 was too restrictive?? Stopped Ubuntu booting
-                self.mmu.mip = value & mask | self.mmu.mip & !mask;
-            }
-            Csr::Mideleg => {
-                self.csr[Csr::Mideleg as usize] = value & 0x222;
-            }
-            Csr::Mstatus => {
-                self.mmu.mstatus = value;
-                self.fs = ((value >> MSTATUS_FS_SHIFT) & 3) as u8;
-            }
-            Csr::Time => {
-                // XXX This should trap actually
-                self.mmu.get_mut_clint().write_mtime(value);
-            }
-            Csr::Satp => {
-                self.mmu.satp = value;
-                self.mmu.clear_page_cache();
-            }
-            /* Csr::Cycle | */ Csr::Mcycle => self.cycle = value,
-            _ => {
-                self.csr[csr as usize] = value;
-            }
+            Csr::Stval => self.csr.stval = value,
+            Csr::Stvec => self.csr.stvec = value,
+            Csr::Time => self.mmu.get_mut_clint().write_mtime(value), // XXX SHOULD trap
+            Csr::Ustatus => self.csr.ustatus = value,
+            _ => log::warn!("We are ignoring writes to {csr:?}"),
         }
     }
 
@@ -1974,7 +1983,7 @@ mod test_cpu {
         cpu.update_pc(MEMORY_BASE);
 
         // Machine timer interrupt but mie in mstatus is not enabled yet
-        cpu.write_csr_raw(Csr::Mie, MIP_MTIP);
+        cpu.csr.mie = MIP_MTIP;
         cpu.mmu.mip |= MIP_MTIP;
         cpu.write_csr_raw(Csr::Mtvec, handler_vector);
 

src/csr.rs

@@ -34,7 +34,7 @@ pub enum Csr {
     Mideleg = 0x303,
     Mie = 0x304,
     Mtvec = 0x305,
-    Csr306 = 0x306,
+    Mcounteren = 0x306,
     Menvcfg = 0x30a, // Unsupported Mostly Just Configure S Access To Timecmp
     Mscratch = 0x340,
     Mepc = 0x341,
@@ -148,6 +148,7 @@ pub const fn legal(csr: Csr) -> bool {
             | Csr::Instret
             | Csr::Marchid
             | Csr::Mcause
+            | Csr::Mcounteren
             | Csr::Mcycle
             | Csr::Minstret
             | Csr::Medeleg
@@ -184,6 +185,61 @@ pub const fn legal(csr: Csr) -> bool {
             | Csr::Ustatus
             | Csr::Utval
             | Csr::Utvec
-            | Csr::Csr306
     )
 }
+
+pub struct CsrFile {
+    pub mcause: u64,
+    pub medeleg: u64,
+    pub mepc: u64,
+    pub mhartid: u64,
+    pub mideleg: u64,
+    pub mie: u64,
+    pub misa: u64,
+    pub mscratch: u64,
+    pub mtval: u64,
+    pub mtvec: u64,
+    pub scause: u64,
+    pub sedeleg: u64,
+    pub sepc: u64,
+    pub sideleg: u64,
+    pub sscratch: u64,
+    pub stval: u64,
+    pub stvec: u64,
+    pub ustatus: u64,
+}
+
+impl Default for CsrFile {
+    fn default() -> Self { Self::new() }
+}
+
+impl CsrFile {
+    #[must_use]
+    pub fn new() -> Self {
+        let mut misa = 1 << 63; // RV64
+        for c in "SUIMAFDC".bytes() {
+            misa |= 1 << (c as usize - 65);
+        }
+
+        Self {
+            mcause: 0,
+            medeleg: 0,
+            mepc: 0,
+            mhartid: 0,
+            mideleg: 0,
+            mie: 0,
+            misa,
+            mscratch: 0,
+            mtval: 0,
+            mtvec: 0,
+            scause: 0,
+            sedeleg: 0,
+            sepc: 0,
+            sideleg: 0,
+            sscratch: 0,
+            stval: 0,
+            stvec: 0,
+            ustatus: 0,
+        }
+    }
+}