From 803bb65f7f73d71dad9801eb47d1de90d6653044 Mon Sep 17 00:00:00 2001 From: Callum Date: Thu, 23 Jul 2026 14:31:58 +1000 Subject: [PATCH] MapPerms: Consistency between map and iomap This commit unifies the way VM rights are encoded in the sdf. Previously normal map perms were represented as a raw u8 while the iomap perms were represented as a proper type. Preserves existing behaviour. Signed-off-by: Callum --- Cargo.lock | 4 +- tool/microkit/src/sdf.rs | 133 +++++++++++++++++++++++++------------ tool/microkit/src/viper.rs | 3 +- 3 files changed, 94 insertions(+), 46 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 05b6a61e9..67bd2d7ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -135,7 +135,7 @@ checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" [[package]] name = "initialiser" -version = "2.3.0" +version = "2.3.0-dev" dependencies = [ "sel4-capdl-initializer", ] @@ -194,7 +194,7 @@ checksum = "88904434abc2901f197fe8cc55f0445e7ded921dba5911dad2e2b39b48e663c4" [[package]] name = "microkit-tool" -version = "2.3.0" +version = "2.3.0-dev" dependencies = [ "rkyv", "roxmltree", diff --git a/tool/microkit/src/sdf.rs b/tool/microkit/src/sdf.rs index ca0e2dd1a..639bf8ea8 100644 --- a/tool/microkit/src/sdf.rs +++ b/tool/microkit/src/sdf.rs @@ -266,26 +266,45 @@ impl fmt::Display for IommuDeviceIdentifierParseError { } } -#[repr(u8)] -pub enum SysMapPerms { - Read = 1, - Write = 2, - Execute = 4, +#[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 } } @@ -293,22 +312,44 @@ 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. pub text_pos: Option, } +// We do not include attributes (execute/cached) here since seL4 does not treat them like vm_rights. #[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub enum SysIOMapPerms { +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 SysIOMapPerms(FrameRights); + impl SysIOMapPerms { - fn from_str(s: &str) -> Result { + fn from_str(s: &str) -> Result { let mut read = false; let mut write = false; @@ -316,16 +357,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 } } @@ -379,15 +427,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 { @@ -421,15 +469,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 { @@ -704,7 +752,14 @@ impl SysMap { let perms = if let Some(xml_perms) = node.attribute("perms") { match SysMapPerms::from_str(xml_perms) { Ok(parsed_perms) => parsed_perms, - Err(()) => { + 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, @@ -714,18 +769,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, @@ -782,7 +831,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, @@ -793,7 +842,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