From e1586b91c89b554be6839f71c559ae792227559c Mon Sep 17 00:00:00 2001 From: Julia Vassiliki Date: Tue, 11 Aug 2026 15:53:37 +1000 Subject: [PATCH 1/3] tool(cleanup): use names not IDs for PDs As part of this, use Rc instead of String/&str in places so that we can cheaply have owned-strings in HashMap keys without needing to allocate more and more string data. This will be necessary for the Multikernel version of the Microkit as it will be implemented by building a system/spec for each core, which makes the indices invalid when embedded in e.g. channels or otherwise. This also means that you can declare channels before PDs, instead they were forced to be placed in the SDF after the PDs due to the way the parsing worked. Signed-off-by: Julia Vassiliki --- tool/microkit/src/build.rs | 2 +- tool/microkit/src/capdl/builder.rs | 24 ++--- tool/microkit/src/sdf.rs | 147 +++++++++++++++-------------- tool/microkit/src/sdf/channels.rs | 13 ++- tool/microkit/src/sdf/cspace.rs | 18 +--- tool/microkit/src/sdf/pd_vm.rs | 32 +++---- tool/microkit/src/symbols.rs | 18 ++-- tool/microkit/src/util.rs | 3 +- tool/microkit/src/viper.rs | 31 +++--- 9 files changed, 143 insertions(+), 145 deletions(-) diff --git a/tool/microkit/src/build.rs b/tool/microkit/src/build.rs index f2e6e3112..3ee853661 100644 --- a/tool/microkit/src/build.rs +++ b/tool/microkit/src/build.rs @@ -137,7 +137,7 @@ pub fn build_system( // a list like this. let mut system_elfs = Vec::with_capacity(system.protection_domains.len()); // Get the elf files for each pd: - for pd in &system.protection_domains { + for pd in system.protection_domains.values() { match get_full_path(&pd.program_image, &args.search_paths) { Some(path) => { let path_for_symbols = pd diff --git a/tool/microkit/src/capdl/builder.rs b/tool/microkit/src/capdl/builder.rs index 5266ca168..7a116dbf0 100644 --- a/tool/microkit/src/capdl/builder.rs +++ b/tool/microkit/src/capdl/builder.rs @@ -8,6 +8,7 @@ use core::ops::Range; use std::{ cmp::{min, Ordering}, collections::HashMap, + rc::Rc, }; use sel4_capdl_initializer_types::{ @@ -606,7 +607,7 @@ pub fn build_capdl_spec( // On ARM, check if we need to create the SMC object let arm_smc_obj_id = if kernel_config.arch == Arch::Aarch64 && kernel_config.arm_smc.unwrap_or(false) - && system.protection_domains.iter().any(|pd| pd.smc) + && system.protection_domains.values().any(|pd| pd.smc) { Some(spec_container.add_root_object(NamedObject { name: "arm_smc".to_owned().into(), @@ -618,7 +619,7 @@ pub fn build_capdl_spec( // This object keeps track of object IDs for various 'important' / nameable kernel objects for // each PD so that we can make various references to them at later steps. - let mut pd_shadow_cspaces: HashMap = HashMap::new(); + let mut pd_shadow_cspaces: HashMap, PDShadowCspace> = HashMap::new(); // Keep track of the global count of vCPU objects so we can bind them to the monitor for setting TCB name in debug config. // Only used on ARM and RISC-V as on x86-64 VMs share the same TCB as PD's which will have their TCB name set separately. @@ -627,7 +628,7 @@ pub fn build_capdl_spec( // Keep tabs on each PD's stack bottom so we can write it out to the monitor for stack overflow detection. let mut pd_stack_bottoms: Vec = Vec::new(); - for (pd_global_idx, pd) in system.protection_domains.iter().enumerate() { + for (pd_global_idx, pd) in system.protection_domains.values().enumerate() { let elf_obj = &elfs[pd_global_idx]; let mut caps_to_bind_to_tcb: Vec = Vec::new(); @@ -761,10 +762,9 @@ pub fn build_capdl_spec( )); // Step 3-5 Create fault Endpoint cap to parent/monitor - let pd_fault_ep_cap = if let Some(pd_parent_id) = pd.parent { - assert!(pd_global_idx > pd_parent_id); + let pd_fault_ep_cap = if let Some(pd_parent) = &pd.parent { let badge: u64 = FAULT_BADGE | pd.id.unwrap(); - let parent_shadow_cspace = &pd_shadow_cspaces[&pd_parent_id]; + let parent_shadow_cspace = &pd_shadow_cspaces[pd_parent]; let parent_ep_obj_id = parent_shadow_cspace .endpoint .expect("parent should have EP due to needs_ep()"); @@ -812,7 +812,7 @@ pub fn build_capdl_spec( let pd_ntfn_obj_id = capdl_util_make_ntfn_obj(&mut spec_container, &pd.name); let pd_ntfn_cap = capdl_util_make_ntfn_cap(pd_ntfn_obj_id, true, true, 0); let mut pd_ep_obj_id: Option = None; - if pd.needs_ep(pd_global_idx, &system.channels) { + if pd.needs_ep(&system.channels) { pd_ep_obj_id = Some(capdl_util_make_endpoint_obj( &mut spec_container, &pd.name, @@ -1071,7 +1071,7 @@ pub fn build_capdl_spec( let pd_root_cnode_obj_id = capdl_util_make_cnode_obj( &mut spec_container, - &(pd.name.clone() + "_root"), + &format!("{}_root", pd.name), PD_ROOT_CAP_BITS, Vec::new(), ); @@ -1139,7 +1139,7 @@ pub fn build_capdl_spec( } pd_shadow_cspaces.insert( - pd_global_idx, + pd.name.clone(), PDShadowCspace { cspace: pd_root_cnode_obj_id, microkit_cnode: pd_cnode_obj_id, @@ -1255,11 +1255,11 @@ pub fn build_capdl_spec( // ********************************* // Step 6. Handle extra cap mappings // ********************************* - for (pd_dest_idx, pd) in system.protection_domains.iter().enumerate() { + for pd in system.protection_domains.values() { for cap_map in pd.cap_maps.iter() { // TODO: Once we add more CapMap options, they might not all have // the pd_name. But for now, they do. - let pd_src_shadow_cspace = &pd_shadow_cspaces[&cap_map.pd.unwrap()]; + let pd_src_shadow_cspace = &pd_shadow_cspaces[&cap_map.pd]; let cap_map_obj = match cap_map.cap_type { CapMapType::Tcb => capdl_util_make_tcb_cap(pd_src_shadow_cspace.tcb), @@ -1268,7 +1268,7 @@ pub fn build_capdl_spec( }; // Map this into the destination pd's cspace and the specified slot. - pd_shadow_cspaces[&pd_dest_idx].insert_cap_into_root_cnode( + pd_shadow_cspaces[&pd.name].insert_cap_into_root_cnode( &mut spec_container, cap_map.slot as u32, cap_map_obj, diff --git a/tool/microkit/src/sdf.rs b/tool/microkit/src/sdf.rs index 7efd1bad3..8c3ce5027 100644 --- a/tool/microkit/src/sdf.rs +++ b/tool/microkit/src/sdf.rs @@ -31,6 +31,7 @@ mod util; use std::collections::{HashMap, HashSet}; use std::ops::Range; use std::path::{Path, PathBuf}; +use std::rc::Rc; use crate::sel4::{Arch, Config}; use crate::util::ranges_overlap; @@ -132,7 +133,7 @@ pub(crate) struct SystemDescriptionFile<'a> { #[derive(Debug)] pub struct SystemDescription { - pub protection_domains: Vec, + pub protection_domains: HashMap, ProtectionDomain>, pub memory_regions: Vec, pub iomaps: Vec, pub channels: Vec, @@ -242,7 +243,38 @@ pub fn parse( } } - let mut pds = pd_flatten(&xml_sdf, root_pds)?; + let pds = pd_flatten(&xml_sdf, root_pds)?; + + // Now that we have parsed everything in the system description we can validate any + // global properties (e.g no duplicate PD names etc). + + if pds.is_empty() { + return Err("Error: at least one protection domain must be defined".to_string()); + } + + if pds.len() > MAX_PDS { + return Err(format!( + "Error: too many protection domains ({}) defined. Maximum is {}.", + pds.len(), + MAX_PDS + )); + } + + for pd in pds.iter() { + if pds.iter().filter(|x| pd.name == x.name).count() > 1 { + return Err(format!( + "Error: duplicate protection domain name '{}'.", + pd.name + )); + } + if &*pd.name == MONITOR_PD_NAME { + return Err( + "Error: the PD name 'monitor' is reserved for the Microkit Monitor.".to_string(), + ); + } + } + + let mut pds: HashMap, _> = pds.into_iter().map(|pd| (pd.name.clone(), pd)).collect(); for node in channel_nodes { let ch = Channel::from_xml(&xml_sdf, &*node, &pds)?; @@ -252,7 +284,12 @@ pub fn parse( symbol: setvar_id.to_string(), kind: SysSetVarKind::Id { id: ch.end_a.id }, }; - checked_add_setvar(&mut pds[ch.end_a.pd].setvars, setvar, &xml_sdf, &*node)?; + checked_add_setvar( + &mut pds.get_mut(&ch.end_a.pd).unwrap().setvars, + setvar, + &xml_sdf, + &*node, + )?; } if let Some(setvar_id) = &ch.end_b.setvar_id { @@ -260,59 +297,26 @@ pub fn parse( symbol: setvar_id.to_string(), kind: SysSetVarKind::Id { id: ch.end_b.id }, }; - checked_add_setvar(&mut pds[ch.end_b.pd].setvars, setvar, &xml_sdf, &*node)?; + checked_add_setvar( + &mut pds.get_mut(&ch.end_b.pd).unwrap().setvars, + setvar, + &xml_sdf, + &*node, + )?; } channels.push(ch); } - // FIXME: Now we post-fill the PD ids in the capmap elements, which is - // ugly, and we should rework this to be less so. - let pd_names_to_id: HashMap<_, _> = pds - .iter() - .enumerate() - .map(|(idx, pd)| (pd.name.clone(), idx)) - .collect(); - for pd in pds.iter_mut() { - for cap_map in pd.cap_maps.iter_mut() { - let Some(&pd) = pd_names_to_id.get(&cap_map.pd_name) else { + for pd in pds.values() { + for cap_map in pd.cap_maps.iter() { + if !pds.contains_key(&cap_map.pd) { return Err(format!( "Error: unknown PD name '{}': {}", - cap_map.pd_name, + cap_map.pd, loc_string(&xml_sdf, cap_map.text_pos) )); }; - - cap_map.pd = Some(pd); - } - } - - // Now that we have parsed everything in the system description we can validate any - // global properties (e.g no duplicate PD names etc). - - if pds.is_empty() { - return Err("Error: at least one protection domain must be defined".to_string()); - } - - if pds.len() > MAX_PDS { - return Err(format!( - "Error: too many protection domains ({}) defined. Maximum is {}.", - pds.len(), - MAX_PDS - )); - } - - for pd in &pds { - if pds.iter().filter(|x| pd.name == x.name).count() > 1 { - return Err(format!( - "Error: duplicate protection domain name '{}'.", - pd.name - )); - } - if pd.name == MONITOR_PD_NAME { - return Err( - "Error: the PD name 'monitor' is reserved for the Microkit Monitor.".to_string(), - ); } } @@ -325,10 +329,10 @@ pub fn parse( } } - let mut vms: Vec<&String> = vec![]; - for pd in &pds { + let mut vms: Vec<&str> = vec![]; + for pd in pds.values() { if let Some(vm) = &pd.virtual_machine { - if vms.contains(&&vm.name) { + if vms.contains(&vm.name.as_ref()) { return Err(format!( "Error: duplicate virtual machine name '{}'.", vm.name @@ -352,7 +356,7 @@ pub fn parse( // Ensure no duplicate IRQs let mut all_irqs = Vec::new(); - for pd in &pds { + for pd in pds.values() { for sysirq in &pd.irqs { if all_irqs.contains(&sysirq.irq_num()) { return Err(format!( @@ -370,10 +374,12 @@ pub fn parse( // Ensure no duplicate channel identifiers. // This means checking that no interrupt IDs clash with any channel IDs - let mut ch_ids = vec![vec![]; pds.len()]; - for (pd_idx, pd) in pds.iter().enumerate() { + let mut ch_ids = HashMap::with_capacity(pds.len()); + for pd in pds.values() { + let mut pd_ch_ids = vec![]; + for sysirq in &pd.irqs { - if ch_ids[pd_idx].contains(&sysirq.id) { + if pd_ch_ids.contains(&sysirq.id) { return Err(format!( "Error: duplicate channel id: {} in protection domain: '{}' @ {}:{}:{}", sysirq.id, @@ -383,13 +389,16 @@ pub fn parse( pd.text_pos.unwrap().col )); } - ch_ids[pd_idx].push(sysirq.id); + + pd_ch_ids.push(sysirq.id); } + + ch_ids.insert(&pd.name, pd_ch_ids); } for ch in &channels { - if ch_ids[ch.end_a.pd].contains(&ch.end_a.id) { - let pd = &pds[ch.end_a.pd]; + if ch_ids[&ch.end_a.pd].contains(&ch.end_a.id) { + let pd = &pds[&ch.end_a.pd]; return Err(format!( "Error: duplicate channel id: {} in protection domain: '{}' @ {}:{}:{}", ch.end_a.id, @@ -400,8 +409,8 @@ pub fn parse( )); } - if ch_ids[ch.end_b.pd].contains(&ch.end_b.id) { - let pd = &pds[ch.end_b.pd]; + if ch_ids[&ch.end_b.pd].contains(&ch.end_b.id) { + let pd = &pds[&ch.end_b.pd]; return Err(format!( "Error: duplicate channel id: {} in protection domain: '{}' @ {}:{}:{}", ch.end_b.id, @@ -412,8 +421,8 @@ pub fn parse( )); } - let pd_a = &pds[ch.end_a.pd]; - let pd_b = &pds[ch.end_b.pd]; + let pd_a = &pds[&ch.end_a.pd]; + let pd_b = &pds[&ch.end_b.pd]; if ch.end_a.pp && pd_a.priority() >= pd_b.priority() { return Err(format!( "Error: PPCs must be to protection domains of strictly higher priorities; \ @@ -449,12 +458,12 @@ pub fn parse( )); } - ch_ids[ch.end_a.pd].push(ch.end_a.id); - ch_ids[ch.end_b.pd].push(ch.end_b.id); + ch_ids.get_mut(&ch.end_a.pd).unwrap().push(ch.end_a.id); + ch_ids.get_mut(&ch.end_b.pd).unwrap().push(ch.end_b.id); } // Ensure no duplicate I/O Ports - for pd in &pds { + for pd in pds.values() { let mut seen_ioport_ids: Vec = Vec::new(); for ioport in &pd.ioports { if seen_ioport_ids.contains(&ioport.id) { @@ -474,7 +483,7 @@ pub fn parse( // Ensure I/O Ports' size are valid and they don't overlap. let mut seen_ioports: Vec<(&str, &IOPort)> = Vec::new(); - for pd in &pds { + for pd in pds.values() { for this_ioport in &pd.ioports { for (seen_pd_name, seen_ioport) in &seen_ioports { let left_range = this_ioport.addr..this_ioport.addr + this_ioport.size; @@ -504,7 +513,7 @@ pub fn parse( } // Ensure that all maps are correct - for pd in &pds { + for pd in pds.values() { check_maps( &xml_sdf, &mrs, @@ -527,7 +536,7 @@ pub fn parse( // Ensure that there are no overlapping extra cap maps in the user caps region // and we are not mapping in the same cap from the same source more than once - for pd in &pds { + for pd in pds.values() { let mut user_cap_slots = HashMap::>::new(); for cap_map in &pd.cap_maps { @@ -544,7 +553,7 @@ pub fn parse( lines.push_str(&format!( "\n type {:?} from '{}' at '{}'", mapping.cap_type, - mapping.pd_name, + mapping.pd, loc_string(&xml_sdf, mapping.text_pos) )); } @@ -587,7 +596,7 @@ pub fn parse( // Check that all MRs are used let mut all_maps = vec![]; - for pd in &pds { + for pd in pds.values() { all_maps.extend(&pd.maps); if let Some(vm) = &pd.virtual_machine { all_maps.extend(&vm.maps); @@ -661,7 +670,7 @@ pub fn parse( // If any MRs are subject of a setvar region_paddr, update its phys_addr field to indicate tool allocated. let mut mr_names_with_setvar_paddr = HashSet::new(); - for pd in pds.iter() { + for pd in pds.values() { for setvar in pd.setvars.iter() { if let SysSetVarKind::Paddr { region } = &setvar.kind { mr_names_with_setvar_paddr.insert(region); diff --git a/tool/microkit/src/sdf/channels.rs b/tool/microkit/src/sdf/channels.rs index e400cf0aa..c9d924d7d 100644 --- a/tool/microkit/src/sdf/channels.rs +++ b/tool/microkit/src/sdf/channels.rs @@ -4,6 +4,9 @@ // SPDX-License-Identifier: BSD-2-Clause // +use std::collections::HashMap; +use std::rc::Rc; + use super::consts::*; use super::pd_vm::ProtectionDomain; use super::util::{check_attributes, checked_lookup, loc_string, value_error}; @@ -13,7 +16,7 @@ use crate::util::str_to_bool; #[derive(Debug, Clone)] pub struct ChannelEnd { - pub pd: usize, + pub pd: Rc, pub id: u64, pub notify: bool, pub pp: bool, @@ -30,7 +33,7 @@ impl ChannelEnd { fn from_xml<'a>( xml_sdf: &'a SystemDescriptionFile, node: &'a dyn SdfNode, - pds: &[ProtectionDomain], + pds: &HashMap, ProtectionDomain>, ) -> Result { let node_name = node.tag_name(); if node_name != "end" { @@ -78,10 +81,10 @@ impl ChannelEnd { value_error(xml_sdf, node, "pp must be 'true' or 'false'".to_string()) })?; - if let Some(pd_idx) = pds.iter().position(|pd| pd.name == end_pd) { + if let Some(pd) = pds.get(end_pd) { let setvar_id = node.attribute("setvar_id").map(ToOwned::to_owned); Ok(ChannelEnd { - pd: pd_idx, + pd: pd.name.clone(), id: end_id.try_into().unwrap(), notify, pp, @@ -104,7 +107,7 @@ impl Channel { pub(super) fn from_xml<'a>( xml_sdf: &'a SystemDescriptionFile, node: &'a dyn SdfNode, - pds: &[ProtectionDomain], + pds: &HashMap, ProtectionDomain>, ) -> Result { check_attributes(xml_sdf, node, &[])?; diff --git a/tool/microkit/src/sdf/cspace.rs b/tool/microkit/src/sdf/cspace.rs index 71f5f9b0c..c3edc60ef 100644 --- a/tool/microkit/src/sdf/cspace.rs +++ b/tool/microkit/src/sdf/cspace.rs @@ -4,6 +4,8 @@ // SPDX-License-Identifier: BSD-2-Clause // +use std::rc::Rc; + use super::consts::*; use super::util::{check_attributes, checked_lookup, loc_string, sdf_parse_number, value_error}; use super::{SdfLocation, SdfNode, SystemDescriptionFile}; @@ -18,15 +20,7 @@ pub enum CapMapType { #[derive(Debug, PartialEq, Eq)] pub struct CapMap { pub cap_type: CapMapType, - // FIXME: This is quite a hack. Basically, we need to be able to reference - // arbitrary PDs, but to gather the index, we need to know all the PDs. - // However, at the time of parsing the cap maps, we are in the process - // of parsing all the PDs. In lieu of something better (in my - @midnightveil's - // opinion, making everything think in terms of PD names, and something - // that was necessary to do for the multikernel changes); the pd idx will - // be filled out later during SDF parse process. - pub pd_name: String, - pub pd: Option, + pub pd: Rc, // The destination "slot" in the CSpace: note that this is "opaque" and // can be shifted depending on the location in the CSpace to work as the CPtr, // but here it is given as the index into the CNode. @@ -51,7 +45,7 @@ impl CapMap { // have to rework this a bit. check_attributes(xml_sdf, node, &["slot", "pd"])?; - let pd_name = checked_lookup(xml_sdf, node, "pd")?.to_string(); + let pd = Rc::from(checked_lookup(xml_sdf, node, "pd")?); let slot = sdf_parse_number(checked_lookup(xml_sdf, node, "slot")?, node)?; @@ -74,9 +68,7 @@ impl CapMap { Ok(CapMap { cap_type, - pd_name, - // FIXME: Hack, filled out later. - pd: None, + pd, slot, text_pos: node.range().start, }) diff --git a/tool/microkit/src/sdf/pd_vm.rs b/tool/microkit/src/sdf/pd_vm.rs index 7accbe8e0..91fb588ec 100644 --- a/tool/microkit/src/sdf/pd_vm.rs +++ b/tool/microkit/src/sdf/pd_vm.rs @@ -6,6 +6,7 @@ use std::fmt; use std::path::{Path, PathBuf}; +use std::rc::Rc; use std::str::FromStr; use super::channels::Channel; @@ -71,7 +72,7 @@ pub struct SysSetVar { pub struct ProtectionDomain { /// Only populated for child protection domains pub id: Option, - pub name: String, + pub name: Rc, pub sched_params: SchedulingParams, pub passive: bool, pub stack_size: u64, @@ -94,7 +95,7 @@ pub struct ProtectionDomain { pub has_children: bool, /// Index into the total list of protection domains if a parent /// protection domain exists - pub parent: Option, + pub parent: Option>, /// Value of the setvar_id attribute, if a parent protection domain exists pub setvar_id: Option, /// Location in the parsed SDF file @@ -102,12 +103,12 @@ pub struct ProtectionDomain { } impl ProtectionDomain { - pub fn needs_ep(&self, self_id: usize, channels: &[Channel]) -> bool { + pub fn needs_ep(&self, channels: &[Channel]) -> bool { self.has_children || self.virtual_machine.is_some() || channels.iter().any(|channel| { - (channel.end_a.pp && channel.end_b.pd == self_id) - || (channel.end_b.pp && channel.end_a.pd == self_id) + (channel.end_a.pp && channel.end_b.pd == self.name) + || (channel.end_b.pp && channel.end_a.pd == self.name) }) } @@ -160,7 +161,7 @@ impl ProtectionDomain { } check_attributes(xml_sdf, node, &attrs)?; - let name = checked_lookup(xml_sdf, node, "name")?.to_string(); + let name = Rc::from(checked_lookup(xml_sdf, node, "name")?); let (id, setvar_id) = if is_child { let id = sdf_parse_number(checked_lookup(xml_sdf, node, "id")?, node)?; @@ -819,7 +820,7 @@ pub fn pd_flatten( // These are all root PDs, so should not have parents. assert!(pd.parent.is_none()); // We provide the index of the PD in the entire PD list - all_pds.extend(pd_tree_to_list(xml_sdf, pd, all_pds.len())?); + all_pds.extend(pd_tree_to_list(xml_sdf, pd)?); } Ok(all_pds) @@ -832,7 +833,6 @@ pub fn pd_flatten( fn pd_tree_to_list( xml_sdf: &SystemDescriptionFile, mut pd: ProtectionDomain, - idx: usize, ) -> Result, String> { let mut child_ids = vec![]; for child_pd in &pd.child_pds { @@ -862,16 +862,8 @@ fn pd_tree_to_list( for mut child_pd in child_pds { // The parent PD's index is set for each child. We then pass the index relative to the *total* // list to any nested children so their parent index can be set to the position of this child. - child_pd.parent = Some(idx); - new_child_pds.extend(pd_tree_to_list( - xml_sdf, - child_pd, - // We need to pass the position of this current child PD in the global list. - // `idx` is this child's parent index in the global list, so we need to add - // the position of this child to `idx` which will be the number of extra child - // PDs we've just processed, plus one for the actual entry of this child. - idx + new_child_pds.len() + 1, - )?); + child_pd.parent = Some(pd.name.clone()); + new_child_pds.extend(pd_tree_to_list(xml_sdf, child_pd)?); } let mut all = vec![pd]; @@ -883,7 +875,7 @@ fn pd_tree_to_list( #[derive(Debug, PartialEq, Eq)] pub struct VirtualMachine { pub vcpus: Vec, - pub name: String, + pub name: Rc, pub maps: Vec, pub sched_params: Option, } @@ -907,7 +899,7 @@ impl VirtualMachine { check_attributes(xml_sdf, node, &["name"])?; } - let name = checked_lookup(xml_sdf, node, "name")?.to_string(); + let name = Rc::from(checked_lookup(xml_sdf, node, "name")?); let sched_params = if config.arch == Arch::Aarch64 { // If we do not have an explicit budget the period is equal to the default budget. diff --git a/tool/microkit/src/symbols.rs b/tool/microkit/src/symbols.rs index 477ef5c7b..cd17089e5 100644 --- a/tool/microkit/src/symbols.rs +++ b/tool/microkit/src/symbols.rs @@ -4,7 +4,7 @@ // SPDX-License-Identifier: BSD-2-Clause // -use std::{cmp::min, collections::HashMap}; +use std::{cmp::min, collections::HashMap, rc::Rc}; use crate::{ elf::ElfFile, @@ -26,9 +26,9 @@ pub fn patch_symbols( // ********************************* let monitor_elf = pd_elf_files.last_mut().unwrap(); - let pd_names: Vec = system + let pd_names: Vec> = system .protection_domains - .iter() + .values() .map(|pd| pd.name.clone()) .collect(); monitor_elf @@ -44,9 +44,9 @@ pub fn patch_symbols( ) .unwrap(); - let vm_names: Vec = system + let vm_names: Vec> = system .protection_domains - .iter() + .values() .filter(|pd| pd.virtual_machine.is_some()) .flat_map(|pd_with_vm| { let vm = pd_with_vm.virtual_machine.as_ref().unwrap(); @@ -71,7 +71,7 @@ pub fn patch_symbols( .unwrap(); let mut pd_stack_bottoms: Vec = Vec::new(); - for pd in system.protection_domains.iter() { + for pd in system.protection_domains.values() { let cur_stack_vaddr = kernel_config.pd_stack_bottom(pd.stack_size); pd_stack_bottoms.push(cur_stack_vaddr); } @@ -90,7 +90,7 @@ pub fn patch_symbols( mr_name_to_desc.insert(&mr.name, mr); } - for (pd_global_idx, pd) in system.protection_domains.iter().enumerate() { + for (pd_global_idx, pd) in system.protection_domains.values().enumerate() { let elf_obj = &mut pd_elf_files[pd_global_idx]; let name = pd.name.as_bytes(); @@ -105,7 +105,7 @@ pub fn patch_symbols( let mut notification_bits: u64 = 0; let mut pp_bits: u64 = 0; for channel in system.channels.iter() { - if channel.end_a.pd == pd_global_idx { + if channel.end_a.pd == pd.name { if channel.end_a.notify { notification_bits |= 1 << channel.end_a.id; } @@ -113,7 +113,7 @@ pub fn patch_symbols( pp_bits |= 1 << channel.end_a.id; } } - if channel.end_b.pd == pd_global_idx { + if channel.end_b.pd == pd.name { if channel.end_b.notify { notification_bits |= 1 << channel.end_b.id; } diff --git a/tool/microkit/src/util.rs b/tool/microkit/src/util.rs index dc832e166..f5672b25c 100644 --- a/tool/microkit/src/util.rs +++ b/tool/microkit/src/util.rs @@ -7,6 +7,7 @@ use serde_json; use std::ops::Range; use std::path::{Path, PathBuf}; +use std::rc::Rc; pub fn msb(x: u64) -> u64 { 64 - x.leading_zeros() as u64 - 1 @@ -184,7 +185,7 @@ pub fn monitor_serialise_u64_vec(vec: &[u64]) -> Vec { } /// For serialising an array of PD or VM names -pub fn monitor_serialise_names(names: &[String], max_len: usize, max_name_len: usize) -> Vec { +pub fn monitor_serialise_names(names: &[Rc], max_len: usize, max_name_len: usize) -> Vec { let mut names_bytes = vec![0; (max_len + 1) * max_name_len]; for (i, name) in names.iter().enumerate() { let name_bytes = name.as_bytes(); diff --git a/tool/microkit/src/viper.rs b/tool/microkit/src/viper.rs index 7a034aa23..3badc9f45 100644 --- a/tool/microkit/src/viper.rs +++ b/tool/microkit/src/viper.rs @@ -3,6 +3,8 @@ // // SPDX-License-Identifier: BSD-2-Clause +use std::rc::Rc; + use sel4_capdl_initializer_types::{Cap, Object}; use crate::capdl::CapDLSpecContainer; @@ -84,7 +86,7 @@ impl CapView { pub fn get_cap_view( capdl_spec: &CapDLSpecContainer, system: &SystemDescription, - current_pd: usize, + current_pd: &str, ) -> Option { let pd = system.protection_domains.get(current_pd)?; let cnode_name = format!("cnode_{}", pd.name); @@ -186,7 +188,7 @@ impl SdfView { } } -pub fn get_sdf_view(system: &SystemDescription, current_pd: usize) -> Option { +pub fn get_sdf_view(system: &SystemDescription, current_pd: &str) -> Option { let current = system.protection_domains.get(current_pd)?; let mut view = SdfView { @@ -199,9 +201,9 @@ pub fn get_sdf_view(system: &SystemDescription, current_pd: usize) -> Option Option Option Option { +pub fn get_mem_view(system: &SystemDescription, current_pd: &str) -> Option { let current = system.protection_domains.get(current_pd)?; let mut view = MemView { @@ -344,15 +346,14 @@ pub fn get_combined_views( ) -> Vec { system .protection_domains - .iter() - .enumerate() - .filter_map(|(current_pd, pd)| { - let sdf = get_sdf_view(system, current_pd)?; - let cap = get_cap_view(capdl_spec, system, current_pd)?; - let mem = get_mem_view(system, current_pd)?; + .values() + .filter_map(|pd| { + let sdf = get_sdf_view(system, &pd.name)?; + let cap = get_cap_view(capdl_spec, system, &pd.name)?; + let mem = get_mem_view(system, &pd.name)?; Some(CombinedView { - pd_name: pd.name.clone(), + pd_name: pd.name.to_string(), sdf, cap, mem, From 4e1e9af03fbe879ff549bdfc01004cd360e23aa8 Mon Sep 17 00:00:00 2001 From: Julia Vassiliki Date: Tue, 11 Aug 2026 16:22:36 +1000 Subject: [PATCH 2/3] tool(cleanup): use BTreeMap not HashMap HashMaps have inconsistent ordering across platforms so this can cause slight behaviour differences; this is enough to map tests pass/fail based on order of names. We'd be able to remove all uses of HashMaps if the rust-seL4 ObjectId implemented Ord, but let's not for now. Signed-off-by: Julia Vassiliki --- tool/microkit/src/capdl/builder.rs | 10 +++++----- tool/microkit/src/elf.rs | 12 ++++++------ tool/microkit/src/sdf.rs | 16 ++++++++-------- tool/microkit/src/sdf/channels.rs | 6 +++--- tool/microkit/src/sdf/cspace.rs | 2 +- tool/microkit/src/sdf/domains.rs | 14 +++++++------- tool/microkit/src/sdf/iommu.rs | 6 +++--- tool/microkit/src/sdf/memory_region.rs | 4 ++-- tool/microkit/src/sel4.rs | 2 +- tool/microkit/src/symbols.rs | 4 ++-- 10 files changed, 38 insertions(+), 38 deletions(-) diff --git a/tool/microkit/src/capdl/builder.rs b/tool/microkit/src/capdl/builder.rs index 7a116dbf0..2a451e5b6 100644 --- a/tool/microkit/src/capdl/builder.rs +++ b/tool/microkit/src/capdl/builder.rs @@ -7,7 +7,7 @@ use core::ops::Range; use std::{ cmp::{min, Ordering}, - collections::HashMap, + collections::{BTreeMap, HashMap}, rc::Rc, }; @@ -533,7 +533,7 @@ pub fn build_capdl_spec( // ********************************* // Step 2. Create the memory regions' spec. Result is a hashmap keyed on MR name, value is (parsed XML obj, Vec of frame object IDs) // ********************************* - let mut mr_name_to_frames: HashMap<&String, Vec> = HashMap::new(); + let mut mr_name_to_frames: BTreeMap<&String, Vec> = BTreeMap::new(); for mr in system.memory_regions.iter() { let mut frame_ids = Vec::new(); let frame_size_bits = mr.page_size.fixed_size_bits(kernel_config); @@ -619,7 +619,7 @@ pub fn build_capdl_spec( // This object keeps track of object IDs for various 'important' / nameable kernel objects for // each PD so that we can make various references to them at later steps. - let mut pd_shadow_cspaces: HashMap, PDShadowCspace> = HashMap::new(); + let mut pd_shadow_cspaces: BTreeMap, PDShadowCspace> = BTreeMap::new(); // Keep track of the global count of vCPU objects so we can bind them to the monitor for setting TCB name in debug config. // Only used on ARM and RISC-V as on x86-64 VMs share the same TCB as PD's which will have their TCB name set separately. @@ -1218,7 +1218,7 @@ pub fn build_capdl_spec( // ********************************* // Step 5. Create IOMMU Address Spaces // ********************************* - let mut iospace_by_device: HashMap<&str, AddressSpace> = HashMap::new(); + let mut iospace_by_device: BTreeMap<&str, AddressSpace> = BTreeMap::new(); for iomap in system.iomaps.iter() { let address_space = iospace_by_device.entry(&iomap.name).or_insert_with(|| { create_iospace( @@ -1298,7 +1298,7 @@ pub fn build_capdl_spec( // 4. Recurse through every cap, for any cap bearing the original object ID, write the new object ID. // Step 8-1 - let mut obj_name_to_old_id: HashMap = HashMap::new(); + let mut obj_name_to_old_id: BTreeMap = BTreeMap::new(); for (id, obj) in spec_container.spec.objects.iter().enumerate() { obj_name_to_old_id.insert(obj.name.as_ref().unwrap().clone(), id.into()); } diff --git a/tool/microkit/src/elf.rs b/tool/microkit/src/elf.rs index 11caa8db6..f03d6bcc3 100644 --- a/tool/microkit/src/elf.rs +++ b/tool/microkit/src/elf.rs @@ -6,7 +6,7 @@ use crate::sel4::PageSize; use crate::util::{bytes_to_struct, round_down, struct_to_bytes}; -use std::collections::HashMap; +use std::collections::BTreeMap; use std::fs::{self, metadata, File}; use std::io::Write; use std::path::{Path, PathBuf}; @@ -212,7 +212,7 @@ pub struct ElfFile { pub machine: u16, pub segments: Vec, pub program_headers: Vec, - symbols: HashMap, + symbols: BTreeMap, } impl ElfFile { @@ -224,7 +224,7 @@ impl ElfFile { machine, segments: vec![], program_headers: vec![], - symbols: HashMap::new(), + symbols: BTreeMap::new(), } } @@ -373,7 +373,7 @@ impl ElfFileReader { Ok(segments) } - fn symbols(&self) -> Result, String> { + fn symbols(&self) -> Result, String> { let hdr = &self.hdr; // Read all the section headers @@ -406,7 +406,7 @@ impl ElfFileReader { let symtab_str = &self.bytes[symtab_str_start..symtab_str_end]; // Read all the symbols - let mut symbols: HashMap = HashMap::new(); + let mut symbols: BTreeMap = BTreeMap::new(); let mut offset = 0; let symbol_size = std::mem::size_of::(); while offset < symtab.len() { @@ -649,7 +649,7 @@ impl ElfFile { + (shnum as u64) * (shentsize as u64); // First thing to do is work out where to place all the data segments - let mut seg_idx_to_data_off: HashMap = Default::default(); + let mut seg_idx_to_data_off: BTreeMap = Default::default(); for (i, seg) in self.loadable_segments().iter().enumerate() { seg_idx_to_data_off.insert(i, data_off_watermark); data_off_watermark += seg.file_size(); diff --git a/tool/microkit/src/sdf.rs b/tool/microkit/src/sdf.rs index 8c3ce5027..bb5c5b189 100644 --- a/tool/microkit/src/sdf.rs +++ b/tool/microkit/src/sdf.rs @@ -28,7 +28,7 @@ mod pci; mod pd_vm; mod util; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet}; use std::ops::Range; use std::path::{Path, PathBuf}; use std::rc::Rc; @@ -133,7 +133,7 @@ pub(crate) struct SystemDescriptionFile<'a> { #[derive(Debug)] pub struct SystemDescription { - pub protection_domains: HashMap, ProtectionDomain>, + pub protection_domains: BTreeMap, ProtectionDomain>, pub memory_regions: Vec, pub iomaps: Vec, pub channels: Vec, @@ -177,8 +177,8 @@ pub fn parse( let mut root_pds = vec![]; let mut mrs = vec![]; let mut iomaps = vec![]; - let mut io_address_space_names = HashSet::new(); - let mut iommu_domain_ids = HashSet::new(); + let mut io_address_space_names = BTreeSet::new(); + let mut iommu_domain_ids = BTreeSet::new(); let mut iommu_device_identifiers = Vec::new(); let mut channels = vec![]; let mut domains = Domains::default(); @@ -274,7 +274,7 @@ pub fn parse( } } - let mut pds: HashMap, _> = pds.into_iter().map(|pd| (pd.name.clone(), pd)).collect(); + let mut pds: BTreeMap, _> = pds.into_iter().map(|pd| (pd.name.clone(), pd)).collect(); for node in channel_nodes { let ch = Channel::from_xml(&xml_sdf, &*node, &pds)?; @@ -374,7 +374,7 @@ pub fn parse( // Ensure no duplicate channel identifiers. // This means checking that no interrupt IDs clash with any channel IDs - let mut ch_ids = HashMap::with_capacity(pds.len()); + let mut ch_ids = BTreeMap::new(); for pd in pds.values() { let mut pd_ch_ids = vec![]; @@ -537,7 +537,7 @@ pub fn parse( // Ensure that there are no overlapping extra cap maps in the user caps region // and we are not mapping in the same cap from the same source more than once for pd in pds.values() { - let mut user_cap_slots = HashMap::>::new(); + let mut user_cap_slots = BTreeMap::>::new(); for cap_map in &pd.cap_maps { user_cap_slots @@ -669,7 +669,7 @@ pub fn parse( } // If any MRs are subject of a setvar region_paddr, update its phys_addr field to indicate tool allocated. - let mut mr_names_with_setvar_paddr = HashSet::new(); + let mut mr_names_with_setvar_paddr = BTreeSet::new(); for pd in pds.values() { for setvar in pd.setvars.iter() { if let SysSetVarKind::Paddr { region } = &setvar.kind { diff --git a/tool/microkit/src/sdf/channels.rs b/tool/microkit/src/sdf/channels.rs index c9d924d7d..2eabfa02e 100644 --- a/tool/microkit/src/sdf/channels.rs +++ b/tool/microkit/src/sdf/channels.rs @@ -4,7 +4,7 @@ // SPDX-License-Identifier: BSD-2-Clause // -use std::collections::HashMap; +use std::collections::BTreeMap; use std::rc::Rc; use super::consts::*; @@ -33,7 +33,7 @@ impl ChannelEnd { fn from_xml<'a>( xml_sdf: &'a SystemDescriptionFile, node: &'a dyn SdfNode, - pds: &HashMap, ProtectionDomain>, + pds: &BTreeMap, ProtectionDomain>, ) -> Result { let node_name = node.tag_name(); if node_name != "end" { @@ -107,7 +107,7 @@ impl Channel { pub(super) fn from_xml<'a>( xml_sdf: &'a SystemDescriptionFile, node: &'a dyn SdfNode, - pds: &HashMap, ProtectionDomain>, + pds: &BTreeMap, ProtectionDomain>, ) -> Result { check_attributes(xml_sdf, node, &[])?; diff --git a/tool/microkit/src/sdf/cspace.rs b/tool/microkit/src/sdf/cspace.rs index c3edc60ef..61021f8be 100644 --- a/tool/microkit/src/sdf/cspace.rs +++ b/tool/microkit/src/sdf/cspace.rs @@ -10,7 +10,7 @@ use super::consts::*; use super::util::{check_attributes, checked_lookup, loc_string, sdf_parse_number, value_error}; use super::{SdfLocation, SdfNode, SystemDescriptionFile}; -#[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] +#[derive(Debug, PartialEq, Eq, Copy, Clone)] pub enum CapMapType { Tcb, Sc, diff --git a/tool/microkit/src/sdf/domains.rs b/tool/microkit/src/sdf/domains.rs index 3bf96a8b6..2c062b180 100644 --- a/tool/microkit/src/sdf/domains.rs +++ b/tool/microkit/src/sdf/domains.rs @@ -4,7 +4,7 @@ // SPDX-License-Identifier: BSD-2-Clause // -use std::collections::{hash_map, HashMap}; +use std::collections::{btree_map, BTreeMap}; use std::num::NonZero; use sel4_capdl_initializer_types::{DomainSchedDuration, DomainSchedEntry}; @@ -16,7 +16,7 @@ use crate::Config; #[derive(Debug, Default)] pub struct Domains { - pub name_to_id_map: HashMap, + pub name_to_id_map: BTreeMap, pub schedule_set_start: Option, pub schedule_index_shift: Option, pub schedule: Vec, @@ -37,8 +37,8 @@ impl Domains { ); } - let mut name_to_id_map = HashMap::>::new(); - let mut id_to_name_map = HashMap::::new(); + let mut name_to_id_map = BTreeMap::>::new(); + let mut id_to_name_map = BTreeMap::::new(); let mut domain_schedule_element = None; for child in node.children() { @@ -112,7 +112,7 @@ impl Domains { let mut dom = None; for i in 0..=config.num_domains { - if let hash_map::Entry::Vacant(e) = id_to_name_map.entry(i) { + if let btree_map::Entry::Vacant(e) = id_to_name_map.entry(i) { e.insert(name.clone()); dom = Some(i); break; @@ -173,7 +173,7 @@ impl Domains { config: &Config, xml_sdf: &SystemDescriptionFile, node: &dyn SdfNode, - name_to_id_map: HashMap, + name_to_id_map: BTreeMap, ) -> Result { check_attributes(xml_sdf, node, &["index_shift", "start_index"])?; @@ -266,7 +266,7 @@ impl Domains { fn schedule_entry_from_xml( xml_sdf: &SystemDescriptionFile, node: &dyn SdfNode, - name_to_id_map: &HashMap, + name_to_id_map: &BTreeMap, ) -> Result { check_attributes(xml_sdf, node, &["domain", "duration"])?; diff --git a/tool/microkit/src/sdf/iommu.rs b/tool/microkit/src/sdf/iommu.rs index e1ca9cf65..78514fa12 100644 --- a/tool/microkit/src/sdf/iommu.rs +++ b/tool/microkit/src/sdf/iommu.rs @@ -4,7 +4,7 @@ // SPDX-License-Identifier: BSD-2-Clause // -use std::collections::HashSet; +use std::collections::BTreeSet; use std::fmt; use std::str::FromStr; @@ -75,8 +75,8 @@ impl IOAddressSpace { config: &Config, xml_sdf: &SystemDescriptionFile, node: &dyn SdfNode, - names: &mut HashSet, - domain_ids: &mut HashSet, + names: &mut BTreeSet, + domain_ids: &mut BTreeSet, iommu_device_identifiers: &mut Vec, ) -> Result { if !config.iommu { diff --git a/tool/microkit/src/sdf/memory_region.rs b/tool/microkit/src/sdf/memory_region.rs index 88ac061fd..e20077119 100644 --- a/tool/microkit/src/sdf/memory_region.rs +++ b/tool/microkit/src/sdf/memory_region.rs @@ -4,7 +4,7 @@ // SPDX-License-Identifier: BSD-2-Clause // -use std::collections::HashMap; +use std::collections::BTreeMap; use std::fs; use std::path::PathBuf; @@ -676,7 +676,7 @@ pub fn check_io_maps( mrs: &[SysMemoryRegion], iomaps: &[SysIOMap], ) -> Result<(), String> { - let mut by_device: HashMap<&str, Vec<&SysIOMap>> = HashMap::new(); + let mut by_device: BTreeMap<&str, Vec<&SysIOMap>> = BTreeMap::new(); for iomap in iomaps { by_device diff --git a/tool/microkit/src/sel4.rs b/tool/microkit/src/sel4.rs index aba618d19..bf9b3d03a 100644 --- a/tool/microkit/src/sel4.rs +++ b/tool/microkit/src/sel4.rs @@ -681,7 +681,7 @@ impl RiscvVirtualMemory { } } -#[derive(Debug, Hash, Eq, PartialEq, Clone)] +#[derive(Debug, Eq, PartialEq, Clone)] pub enum ObjectType { Untyped, Tcb, diff --git a/tool/microkit/src/symbols.rs b/tool/microkit/src/symbols.rs index cd17089e5..baf3c723e 100644 --- a/tool/microkit/src/symbols.rs +++ b/tool/microkit/src/symbols.rs @@ -4,7 +4,7 @@ // SPDX-License-Identifier: BSD-2-Clause // -use std::{cmp::min, collections::HashMap, rc::Rc}; +use std::{cmp::min, collections::BTreeMap, rc::Rc}; use crate::{ elf::ElfFile, @@ -85,7 +85,7 @@ pub fn patch_symbols( // ********************************* // Step 2. Write ELF symbols for each PD // ********************************* - let mut mr_name_to_desc: HashMap<&String, &SysMemoryRegion> = HashMap::new(); + let mut mr_name_to_desc: BTreeMap<&String, &SysMemoryRegion> = BTreeMap::new(); for mr in system.memory_regions.iter() { mr_name_to_desc.insert(&mr.name, mr); } From 2a43066d50079ee3d32bc30ea06ee18772c6f556 Mon Sep 17 00:00:00 2001 From: Julia Vassiliki Date: Tue, 11 Aug 2026 16:36:04 +1000 Subject: [PATCH 3/3] tool(cleanup): prefer &str over &String This is a little more flexible than &String. Signed-off-by: Julia Vassiliki --- tool/microkit/src/capdl/builder.rs | 10 +++++----- tool/microkit/src/capdl/util.rs | 5 +---- tool/microkit/src/symbols.rs | 17 ++++++++++------- tool/microkit/src/viper.rs | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tool/microkit/src/capdl/builder.rs b/tool/microkit/src/capdl/builder.rs index 2a451e5b6..1520022dc 100644 --- a/tool/microkit/src/capdl/builder.rs +++ b/tool/microkit/src/capdl/builder.rs @@ -533,7 +533,7 @@ pub fn build_capdl_spec( // ********************************* // Step 2. Create the memory regions' spec. Result is a hashmap keyed on MR name, value is (parsed XML obj, Vec of frame object IDs) // ********************************* - let mut mr_name_to_frames: BTreeMap<&String, Vec> = BTreeMap::new(); + let mut mr_name_to_frames: BTreeMap<&str, Vec> = BTreeMap::new(); for mr in system.memory_regions.iter() { let mut frame_ids = Vec::new(); let frame_size_bits = mr.page_size.fixed_size_bits(kernel_config); @@ -659,7 +659,7 @@ pub fn build_capdl_spec( // Step 3-2: Map in all Memory Regions for map in pd.maps.iter() { - let frames = &mr_name_to_frames[&map.mr]; + let frames = &mr_name_to_frames[map.mr.as_str()]; // MRs have frames of equal size so just use the first frame's page size. let page_size_bytes = 1 << capdl_util_get_frame_size_bits(&spec_container, *frames.first().unwrap()); @@ -883,7 +883,7 @@ pub fn build_capdl_spec( let vm_vspace_obj_id = vm_address_space.root(); let vm_vspace_cap = capdl_util_make_page_table_cap(vm_vspace_obj_id); for map in virtual_machine.maps.iter() { - let frames = &mr_name_to_frames[&map.mr]; + let frames = &mr_name_to_frames[map.mr.as_str()]; let page_size_bytes = 1 << capdl_util_get_frame_size_bits(&spec_container, *frames.first().unwrap()); map_memory_region( @@ -1230,7 +1230,7 @@ pub fn build_capdl_spec( ) }); let page_size_bytes = mr_name_to_frames - .get(&iomap.mr) + .get(iomap.mr.as_str()) .ok_or(format!( "Error: Memory region {} referenced by iomap not found.", iomap.mr @@ -1248,7 +1248,7 @@ pub fn build_capdl_spec( iomap, page_size_bytes, address_space, - &mr_name_to_frames[&iomap.mr], + &mr_name_to_frames[iomap.mr.as_str()], )?; } diff --git a/tool/microkit/src/capdl/util.rs b/tool/microkit/src/capdl/util.rs index a1c20e3ca..46f283107 100644 --- a/tool/microkit/src/capdl/util.rs +++ b/tool/microkit/src/capdl/util.rs @@ -275,10 +275,7 @@ pub fn capdl_util_insert_cap_into_cspace( } } -pub fn capdl_util_make_vcpu_obj( - spec_container: &mut CapDLSpecContainer, - name: &String, -) -> ObjectId { +pub fn capdl_util_make_vcpu_obj(spec_container: &mut CapDLSpecContainer, name: &str) -> ObjectId { let vcpu_inner_obj = Object::VCpu; let vcpu_obj = CapDLNamedObject { name: format!("vcpu_{name}").into(), diff --git a/tool/microkit/src/symbols.rs b/tool/microkit/src/symbols.rs index baf3c723e..43a17d4b7 100644 --- a/tool/microkit/src/symbols.rs +++ b/tool/microkit/src/symbols.rs @@ -85,7 +85,7 @@ pub fn patch_symbols( // ********************************* // Step 2. Write ELF symbols for each PD // ********************************* - let mut mr_name_to_desc: BTreeMap<&String, &SysMemoryRegion> = BTreeMap::new(); + let mut mr_name_to_desc: BTreeMap<&str, &SysMemoryRegion> = BTreeMap::new(); for mr in system.memory_regions.iter() { mr_name_to_desc.insert(&mr.name, mr); } @@ -135,7 +135,7 @@ pub fn patch_symbols( .write_symbol("microkit_ioports", &pd.ioport_bits().to_le_bytes()) .unwrap(); - let mut symbols_to_write: Vec<(&String, u64)> = Vec::new(); + let mut symbols_to_write: Vec<(&str, u64)> = Vec::new(); for setvar in pd.setvars.iter() { // Check that the symbol exists in the ELF match elf_obj.find_symbol(&setvar.symbol) { @@ -149,16 +149,19 @@ pub fn patch_symbols( )); } let data = match &setvar.kind { - sdf::SysSetVarKind::Size { mr } => mr_name_to_desc[mr].size, + sdf::SysSetVarKind::Size { mr } => mr_name_to_desc[mr.as_str()].size, sdf::SysSetVarKind::Vaddr { address } => *address, sdf::SysSetVarKind::Paddr { region } => { - mr_name_to_desc[region].paddr().unwrap_or_default() + mr_name_to_desc[region.as_str()].paddr().unwrap_or_default() } sdf::SysSetVarKind::Id { id } => *id, sdf::SysSetVarKind::X86IoPortAddr { address } => *address, - sdf::SysSetVarKind::PrefillSize { mr } => { - mr_name_to_desc[mr].prefill_bytes.as_ref().unwrap().len() as u64 - } + sdf::SysSetVarKind::PrefillSize { mr } => mr_name_to_desc[mr.as_str()] + .prefill_bytes + .as_ref() + .unwrap() + .len() + as u64, }; symbols_to_write.push((&setvar.symbol, data)); } diff --git a/tool/microkit/src/viper.rs b/tool/microkit/src/viper.rs index 3badc9f45..0cdd32c56 100644 --- a/tool/microkit/src/viper.rs +++ b/tool/microkit/src/viper.rs @@ -251,7 +251,7 @@ pub struct Mem { impl Mem { pub fn export(&self, target: &mut String) { - let name: &String = &self.name; + let name: &str = &self.name; let start: u64 = self.start; let end: u64 = self.end; target.push_str(&format!(