diff --git a/tool/microkit/src/sdf.rs b/tool/microkit/src/sdf.rs index 7314f89ed..dda5d22d2 100644 --- a/tool/microkit/src/sdf.rs +++ b/tool/microkit/src/sdf.rs @@ -49,7 +49,7 @@ pub(crate) use consts::*; pub(crate) use cspace::CapMapType; pub(crate) use iommu::IommuDeviceIdentifier; pub(crate) use irq::{SysIrq, SysIrqKind}; -pub(crate) use memory_region::{Map, SysMapPerms}; +pub(crate) use memory_region::Map; pub(crate) use pd_vm::{CpuCore, SysSetVarKind}; // Public re-exports diff --git a/tool/microkit/src/sdf/memory_region.rs b/tool/microkit/src/sdf/memory_region.rs index 2c1d8c82b..1ac5a1658 100644 --- a/tool/microkit/src/sdf/memory_region.rs +++ b/tool/microkit/src/sdf/memory_region.rs @@ -21,26 +21,72 @@ use crate::util::round_up; use crate::util::str_to_bool; use crate::{Config, PageSize}; -#[repr(u8)] -pub enum SysMapPerms { - Read = 1, - Write = 2, - Execute = 4, +// We do not include attributes (execute/cached) here since seL4 does not treat them like vm_rights. +// Note on seL4 you can't mint a write only frame. However you can request a write only mapping, for the IOMMU on x86. +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum FrameRights { + Read, + Write, + ReadWrite, + None, +} + +impl FrameRights { + fn from_bools(read: bool, write: bool) -> Self { + match (read, write) { + (true, false) => Self::Read, + (false, true) => Self::Write, + (true, true) => Self::ReadWrite, + (false, false) => Self::None, + } + } + pub fn read(self) -> bool { + matches!(self, Self::Read | Self::ReadWrite) + } + pub fn write(self) -> bool { + matches!(self, Self::Write | Self::ReadWrite) + } +} + +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub struct SysMapPerms { + rights: FrameRights, + execute: bool, +} + +pub enum SysMapPermsParseError { + InvalidChar, + WriteOnly, } impl SysMapPerms { - fn from_str(s: &str) -> Result { - let mut perms = 0; + fn from_str(s: &str) -> Result { + let mut read = false; + let mut write = false; + let mut execute = false; for c in s.chars() { match c { - 'r' => perms |= SysMapPerms::Read as u8, - 'w' => perms |= SysMapPerms::Write as u8, - 'x' => perms |= SysMapPerms::Execute as u8, - _ => return Err(()), + 'r' => read = true, + 'w' => write = true, + 'x' => execute = true, + _ => return Err(SysMapPermsParseError::InvalidChar), } } + let rights = match FrameRights::from_bools(read, write) { + FrameRights::Write => return Err(SysMapPermsParseError::WriteOnly), + frame_rights => frame_rights, + }; - Ok(perms) + Ok(Self { rights, execute }) + } + pub fn read(self) -> bool { + self.rights.read() + } + pub fn write(self) -> bool { + self.rights.write() + } + pub fn execute(self) -> bool { + self.execute } } @@ -48,7 +94,7 @@ impl SysMapPerms { pub struct SysMap { pub mr: String, pub vaddr: u64, - pub perms: u8, + pub perms: SysMapPerms, pub cached: bool, /// Location in the parsed SDF file. Because this struct is /// used in a non-XML context, we make the position optional. @@ -94,15 +140,15 @@ impl Map for SysMap { } fn read(&self) -> bool { - self.perms & SysMapPerms::Read as u8 != 0 + self.perms.read() } fn write(&self) -> bool { - self.perms & SysMapPerms::Write as u8 != 0 + self.perms.write() } fn execute(&self) -> bool { - self.perms & SysMapPerms::Execute as u8 != 0 + self.perms.execute() } fn cached(&self) -> bool { @@ -136,15 +182,15 @@ impl Map for SysIOMap { } fn read(&self) -> bool { - matches!(self.perms, SysIOMapPerms::Read | SysIOMapPerms::ReadWrite) + self.perms.read() } fn write(&self) -> bool { - matches!(self.perms, SysIOMapPerms::Write | SysIOMapPerms::ReadWrite) + self.perms.write() } fn execute(&self) -> bool { - false + self.perms.execute() } fn cached(&self) -> bool { @@ -214,14 +260,10 @@ impl SysMemoryRegion { } #[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub enum SysIOMapPerms { - Read, - Write, - ReadWrite, -} +pub struct SysIOMapPerms(FrameRights); impl SysIOMapPerms { - fn from_str(s: &str) -> Result { + fn from_str(s: &str) -> Result { let mut read = false; let mut write = false; @@ -229,16 +271,23 @@ impl SysIOMapPerms { match c { 'r' => read = true, 'w' => write = true, - _ => return Err(()), + _ => return Err(format!("Invalid character in string {s}")), } } - - match (read, write) { - (true, true) => Ok(SysIOMapPerms::ReadWrite), - (true, false) => Ok(SysIOMapPerms::Read), - (false, true) => Ok(SysIOMapPerms::Write), - (false, false) => Err(()), - } + let frame_rights = match FrameRights::from_bools(read, write) { + FrameRights::None => return Err("Invalid frame right for IOMap".into()), + frame_rights => frame_rights, + }; + Ok(SysIOMapPerms(frame_rights)) + } + pub fn read(self) -> bool { + self.0.read() + } + pub fn write(self) -> bool { + self.0.write() + } + pub fn execute(self) -> bool { + false } } @@ -282,7 +331,15 @@ impl SysMap { let perms = if let Some(xml_perms) = node.attribute("perms") { match SysMapPerms::from_str(xml_perms) { Ok(parsed_perms) => parsed_perms, - Err(()) => { + // On all architectures, the kernel does not allow write-only mappings + Err(SysMapPermsParseError::WriteOnly) => { + return Err(value_error( + xml_sdf, + node, + "perms must not be 'w', write-only mappings are not allowed".to_string(), + )); + } + Err(_) => { return Err(value_error( xml_sdf, node, @@ -292,18 +349,12 @@ impl SysMap { } } else { // Default to read-write - SysMapPerms::Read as u8 | SysMapPerms::Write as u8 + SysMapPerms { + rights: FrameRights::ReadWrite, + execute: false, + } }; - // On all architectures, the kernel does not allow write-only mappings - if perms == SysMapPerms::Write as u8 { - return Err(value_error( - xml_sdf, - node, - "perms must not be 'w', write-only mappings are not allowed".to_string(), - )); - } - let cached = if let Some(xml_cached) = node.attribute("cached") { match str_to_bool(xml_cached) { Some(val) => val, @@ -360,7 +411,7 @@ impl SysIOMap { let perms = if let Some(xml_perms) = node.attribute("perms") { match SysIOMapPerms::from_str(xml_perms) { Ok(parsed_perms) => parsed_perms, - Err(()) => { + Err(_) => { return Err(value_error( xml_sdf, node, @@ -371,7 +422,7 @@ impl SysIOMap { } } else { // Default to read-write - SysIOMapPerms::ReadWrite + SysIOMapPerms(FrameRights::ReadWrite) }; Ok(SysIOMap { diff --git a/tool/microkit/src/viper.rs b/tool/microkit/src/viper.rs index 7a034aa23..78ce13ea6 100644 --- a/tool/microkit/src/viper.rs +++ b/tool/microkit/src/viper.rs @@ -6,7 +6,6 @@ use sel4_capdl_initializer_types::{Cap, Object}; use crate::capdl::CapDLSpecContainer; -use crate::sdf::SysMapPerms; use crate::sdf::SystemDescription; fn export_define_set(name: &'static str, vector: &[u64], target: &mut String) { @@ -312,7 +311,7 @@ pub fn get_mem_view(system: &SystemDescription, current_pd: usize) -> Option