Small changes from review

Change-Id: I5d7b7d9d6e7655fa888d2776d9aef988d5a96318

Author: Lawrence Esswood

arch/riscv/src/csr/mod.rs

@@ -5,17 +5,16 @@
 //! Tock Register interface for using CSR registers.
 
 use riscv_csr::csr::{
-    ReadWriteRiscvCsr, HMPCOUNTER_BASE, MCAUSE, MCYCLE, MEPC, MIE, MINSTRET, MIP, MSCRATCH,
-    MSECCFG, MSTATUS, MTVAL, MTVEC, PMPADDR0, PMPADDR1, PMPADDR10, PMPADDR11, PMPADDR12, PMPADDR13,
-    PMPADDR14, PMPADDR15, PMPADDR16, PMPADDR17, PMPADDR18, PMPADDR19, PMPADDR2, PMPADDR20,
-    PMPADDR21, PMPADDR22, PMPADDR23, PMPADDR24, PMPADDR25, PMPADDR26, PMPADDR27, PMPADDR28,
-    PMPADDR29, PMPADDR3, PMPADDR30, PMPADDR31, PMPADDR32, PMPADDR33, PMPADDR34, PMPADDR35,
-    PMPADDR36, PMPADDR37, PMPADDR38, PMPADDR39, PMPADDR4, PMPADDR40, PMPADDR41, PMPADDR42,
-    PMPADDR43, PMPADDR44, PMPADDR45, PMPADDR46, PMPADDR47, PMPADDR48, PMPADDR49, PMPADDR5,
-    PMPADDR50, PMPADDR51, PMPADDR52, PMPADDR53, PMPADDR54, PMPADDR55, PMPADDR56, PMPADDR57,
-    PMPADDR58, PMPADDR59, PMPADDR6, PMPADDR60, PMPADDR61, PMPADDR62, PMPADDR63, PMPADDR7, PMPADDR8,
-    PMPADDR9, PMPCFG0, PMPCFG10, PMPCFG12, PMPCFG14, PMPCFG2, PMPCFG4, PMPCFG6, PMPCFG8, SATP,
-    STVEC, UTVEC,
+    ReadWriteRiscvCsr, MCAUSE, MCYCLE, MEPC, MIE, MINSTRET, MIP, MSCRATCH, MSECCFG, MSTATUS, MTVAL,
+    MTVEC, PMPADDR0, PMPADDR1, PMPADDR10, PMPADDR11, PMPADDR12, PMPADDR13, PMPADDR14, PMPADDR15,
+    PMPADDR16, PMPADDR17, PMPADDR18, PMPADDR19, PMPADDR2, PMPADDR20, PMPADDR21, PMPADDR22,
+    PMPADDR23, PMPADDR24, PMPADDR25, PMPADDR26, PMPADDR27, PMPADDR28, PMPADDR29, PMPADDR3,
+    PMPADDR30, PMPADDR31, PMPADDR32, PMPADDR33, PMPADDR34, PMPADDR35, PMPADDR36, PMPADDR37,
+    PMPADDR38, PMPADDR39, PMPADDR4, PMPADDR40, PMPADDR41, PMPADDR42, PMPADDR43, PMPADDR44,
+    PMPADDR45, PMPADDR46, PMPADDR47, PMPADDR48, PMPADDR49, PMPADDR5, PMPADDR50, PMPADDR51,
+    PMPADDR52, PMPADDR53, PMPADDR54, PMPADDR55, PMPADDR56, PMPADDR57, PMPADDR58, PMPADDR59,
+    PMPADDR6, PMPADDR60, PMPADDR61, PMPADDR62, PMPADDR63, PMPADDR7, PMPADDR8, PMPADDR9, PMPCFG0,
+    PMPCFG10, PMPCFG12, PMPCFG14, PMPCFG2, PMPCFG4, PMPCFG6, PMPCFG8, SATP, STVEC, UTVEC,
 };
 
 #[cfg(any(target_arch = "riscv32", not(target_os = "none")))]
@@ -149,20 +148,6 @@ pub struct CSR {
     pub pmpaddr62: ReadWriteRiscvCsr<usize, pmpaddr::pmpaddr::Register, PMPADDR62>,
     pub pmpaddr63: ReadWriteRiscvCsr<usize, pmpaddr::pmpaddr::Register, PMPADDR63>,
 
-    pub hpmcounter3: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 3 }>,
-    pub hpmcounter4: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 4 }>,
-    pub hpmcounter5: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 5 }>,
-    pub hpmcounter6: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 6 }>,
-    pub hpmcounter7: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 7 }>,
-    pub hpmcounter8: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 8 }>,
-    pub hpmcounter9: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 9 }>,
-    pub hpmcounter10: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 10 }>,
-    pub hpmcounter11: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 11 }>,
-    pub hpmcounter12: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 12 }>,
-    pub hpmcounter13: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 13 }>,
-    pub hpmcounter14: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 14 }>,
-    pub hpmcounter15: ReadWriteRiscvCsr<usize, (), { HMPCOUNTER_BASE + 15 }>,
-
     pub mie: ReadWriteRiscvCsr<usize, mie::mie::Register, MIE>,
     pub mscratch: ReadWriteRiscvCsr<usize, mscratch::mscratch::Register, MSCRATCH>,
     pub mepc: ReadWriteRiscvCsr<usize, mepc::mepc::Register, MEPC>,
@@ -282,20 +267,6 @@ pub const CSR: &CSR = &CSR {
     pmpaddr62: ReadWriteRiscvCsr::new(),
     pmpaddr63: ReadWriteRiscvCsr::new(),
 
-    hpmcounter3: ReadWriteRiscvCsr::new(),
-    hpmcounter4: ReadWriteRiscvCsr::new(),
-    hpmcounter5: ReadWriteRiscvCsr::new(),
-    hpmcounter6: ReadWriteRiscvCsr::new(),
-    hpmcounter7: ReadWriteRiscvCsr::new(),
-    hpmcounter8: ReadWriteRiscvCsr::new(),
-    hpmcounter9: ReadWriteRiscvCsr::new(),
-    hpmcounter10: ReadWriteRiscvCsr::new(),
-    hpmcounter11: ReadWriteRiscvCsr::new(),
-    hpmcounter12: ReadWriteRiscvCsr::new(),
-    hpmcounter13: ReadWriteRiscvCsr::new(),
-    hpmcounter14: ReadWriteRiscvCsr::new(),
-    hpmcounter15: ReadWriteRiscvCsr::new(),
-
     mie: ReadWriteRiscvCsr::new(),
     mscratch: ReadWriteRiscvCsr::new(),
     mepc: ReadWriteRiscvCsr::new(),

arch/riscv/src/syscall.rs

@@ -86,8 +86,6 @@ const TAG: [u8; 4] = [b'r', b'v', b'5', b'i'];
 const TAG: [u8; 8] = [b'r', b'v', b'5', b'i', b'r', b'v', b'5', b'i'];
 const METADATA_LEN: usize = 3;
 
-// TODO: CHERI. This seems to be for swap or some such. Needs thinking about.
-
 const VERSION_IDX: usize = 0;
 const SIZE_IDX: usize = 1;
 const TAG_IDX: usize = 2;

kernel/src/utilities/arch_helpers.rs

@@ -196,10 +196,9 @@ impl<T: From<SyscallReturnVariant> + Into<usize>> Variant for T {}
 
 /// An extension of TRD104 that works for 32-bit and 64-bit platforms, and can remap variants.
 ///
-/// On 32-bit platforms using.
 /// Using TRD104SyscallReturnVariant on a 32-bit platform, this is exactly TRD104.
 /// Using TRD105SyscallReturnVariant on any platform should be TRD1105.
-/// Archtiectures not following either of these are free to provide their own mappings.
+/// Architectures not following either of these are free to provide their own mappings.
 /// On 64-bit platforms, both 64-bit and usize values are passed as a single register,
 /// shifting down register number if that means fewer registers are needed.
 /// For usize, there is no change in number of registers between platforms.
@@ -234,11 +233,11 @@ pub fn encode_syscall_return_with_variant<SyscallVariant: Variant>(
     fn variant_to_reg<SyscallVariant: From<SyscallReturnVariant> + Into<usize>>(
         v: SyscallReturnVariant,
     ) -> MachineRegister {
-        // First map from
+        // First map from `SyscallReturnVariant` to platform specific `SyscallVariant`
         let lowered_to_abi: SyscallVariant = v.into();
-        // Then cast to usize
+        // Then cast to usize to pass across user/kernel boundary
         let as_usize: usize = lowered_to_abi.into();
-        // and pack that into a register
+        // and pack that usize into a register
         as_usize.into()
     }
 

kernel/src/utilities/capability_ptr.rs

@@ -155,10 +155,12 @@ impl CapabilityPtr {
                 // CHERI can distinguish between a valid and invalid capability with zero length.
                 // Tock, on the other hand, allows length zero allocations at any address.
                 // This is especially important as NULL and zero are often used with allow
-                // syscalls, which will reject invalid capabilities.
-                // We special case length 0 here. This is important for users as they cannot rely
-                // on the sanctity of zero length allocations to use as tokens. They are likely
-                // already using length 1 anyway so as not to be confused between adjacent objects.
+                // syscalls, which will reject invalid capabilities but must accept those values.
+                // As we special case length 0 here (making it always valid) this can result in the
+                // kernel accepting an invalid CHERI capability and returning it back to
+                // userspace as a valid one.
+                // This means that CHERI Tock users should not rely on tags of zero length
+                // capabilities for securing any of their own mechanisms.
                 (length == 0) || cheri_ptr.is_valid_for_operation(length, cheri_perms_for(perms))
             }
             CfgMatch::False(_) => true,

libraries/riscv-csr/src/csr.rs

@@ -107,8 +107,6 @@ pub const PMPADDR61: usize = 0x3ED;
 pub const PMPADDR62: usize = 0x3EE;
 pub const PMPADDR63: usize = 0x3EF;
 
-pub const HMPCOUNTER_BASE: usize = 0xC00;
-
 pub const SATP: usize = 0x180;
 
 /// Read/Write registers.