address pr comments

Signed-off-by: Krishnan Winter <krishnan.winter@unsw.edu.au>

Author: Kswin01

libmicrokit/include/microkit.h

@@ -34,6 +34,7 @@ typedef seL4_MessageInfo_t microkit_msginfo;
 // @kwinter: Bounding user caps to 128. Is this restriction ok for now?
 #define BASE_USER_CAPS 522
 
+#define MICROKIT_MAX_USER_CAPS 128
 #define MICROKIT_MAX_CHANNELS 62
 #define MICROKIT_MAX_CHANNEL_ID (MICROKIT_MAX_CHANNELS - 1)
 #define MICROKIT_MAX_IOPORT_ID MICROKIT_MAX_CHANNELS

tool/microkit/src/capdl/builder.rs

@@ -24,7 +24,7 @@ use crate::{
     },
     elf::ElfFile,
     sdf::{
-        CapMapType, CpuCore, SysMap, SysMapPerms, SystemDescription, BUDGET_DEFAULT,
+        CapMapType, CpuCore, SysMap, SysMapPerms, SystemDescription, BUDGET_DEFAULT, CAP_MAP_TYPES,
         MONITOR_PD_NAME, MONITOR_PRIORITY,
     },
     sel4::{Arch, Config, PageSize},
@@ -584,7 +584,7 @@ pub fn build_capdl_spec(
 
         pd_shadow_cspace
             .entry(pd_global_idx)
-            .or_insert_with(|| vec![None; CapMapType::__Len as usize])[CapMapType::Tcb as usize] =
+            .or_insert_with(|| vec![None; CAP_MAP_TYPES])[CapMapType::Tcb as usize] =
             Some(pd_tcb_obj.clone());
         pd_shadow_cspace.get_mut(&pd_global_idx).unwrap()[CapMapType::Vspace as usize] =
             Some(pd_vspace_obj.clone());
@@ -597,10 +597,8 @@ pub fn build_capdl_spec(
         }
 
         // Allow PD to access their own VSpace for ops such as cache cleaning on ARM.
-        caps_to_insert_to_pd_cspace.push(capdl_util_make_cte(
-            PD_VSPACE_CAP_IDX as u32,
-            pd_vspace_obj,
-        ));
+        caps_to_insert_to_pd_cspace
+            .push(capdl_util_make_cte(PD_VSPACE_CAP_IDX as u32, pd_vspace_obj));
 
         // Step 3-2: Map in all Memory Regions
         for map in pd.maps.iter() {
@@ -1120,15 +1118,15 @@ pub fn build_capdl_spec(
     // *********************************
 
     for (pd_dest_idx, pd) in system.protection_domains.iter().enumerate() {
-        let pd_dest_cspace_id = *pd_id_to_cspace_id.get(&pd_dest_idx).unwrap();
+        let pd_dest_cspace_id = pd_id_to_cspace_id[&pd_dest_idx];
 
         for cap_map in pd.cap_maps.iter() {
             let pd_src_idx = pd_name_to_id.get(&cap_map.pd_name).ok_or(format!(
                 "PD: '{}', does not exist when trying to map extra TCB cap into PD: '{}'",
                 cap_map.pd_name, pd.name
             ))?;
 
-            let pd_obj = pd_shadow_cspace.get(pd_src_idx).unwrap()[cap_map.cap_type as usize]
+            let pd_obj = pd_shadow_cspace[pd_src_idx][cap_map.cap_type as usize]
                 .as_ref()
                 .unwrap();
             // Map this into the destination pd's cspace and the specified slot.

tool/microkit/src/sdf.rs

@@ -37,6 +37,10 @@ use std::path::{Path, PathBuf};
 const PD_MAX_ID: u64 = 61;
 const VCPU_MAX_ID: u64 = PD_MAX_ID;
 
+/// This is the maximum slot allowed for cap maps. This can change if you wish,
+/// but also update the MICROKIT_MAX_USER_CAPS define in `microkit.h`.
+const CAP_MAP_MAX_SLOT: u64 = 128;
+
 pub const MONITOR_PRIORITY: u8 = 255;
 const PD_MAX_PRIORITY: u8 = 254;
 /// In microseconds
@@ -276,13 +280,15 @@ pub struct ProtectionDomain {
     text_pos: Option<roxmltree::TextPos>,
 }
 
+/// Update CAP_MAP_TYPES whenever making changes to the CapMapType enum
+pub const CAP_MAP_TYPES: usize = 4;
+
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub enum CapMapType {
     Tcb = 0,
     Sc,
     Vspace,
     Cnode,
-    __Len,
 }
 
 #[derive(Debug, PartialEq, Eq)]
@@ -1240,29 +1246,24 @@ impl CapMap {
     fn from_xml(xml_sdf: &XmlSystemDescription, node: &roxmltree::Node) -> Result<CapMap, String> {
         check_attributes(xml_sdf, node, &["type", "pd", "dest_cspace_slot"])?;
 
-        let cap_type = match checked_lookup(xml_sdf, node, "type")? {
+        let xml_cap_type = checked_lookup(xml_sdf, node, "type")?;
+        let cap_type = match xml_cap_type {
             "tcb" => CapMapType::Tcb,
             "sc" => CapMapType::Sc,
             "vspace" => CapMapType::Vspace,
             "cnode" => CapMapType::Cnode,
-            _ => {
-                return Err(value_error(
-                    xml_sdf,
-                    node,
-                    "type must be 'tcb' or 'sc' ".to_string(),
-                ))
-            }
+            _ => return Err(format!("Cap type: '{}' is not supported.", xml_cap_type,)),
         };
 
         let pd_name = checked_lookup(xml_sdf, node, "pd")?.to_string();
         let dest_cspace_slot =
             sdf_parse_number(checked_lookup(xml_sdf, node, "dest_cspace_slot")?, node)?;
 
-        if dest_cspace_slot >= 128 {
+        if dest_cspace_slot >= CAP_MAP_MAX_SLOT {
             return Err(value_error(
                 xml_sdf,
                 node,
-                "There are only 128 destination cspace slots available. Max slot allowed is 63"
+                format!("There are only {CAP_MAP_MAX_SLOT} destination cspace slots available.")
                     .to_string(),
             ));
         }
@@ -1956,7 +1957,7 @@ pub fn parse(filename: &str, xml: &str, config: &Config) -> Result<SystemDescrip
         let mut user_cap_slots = Vec::new();
         let mut seen_pd_cap_maps: Vec<(CapMapType, String)> = Vec::new();
 
-        for cap_map in pd.cap_maps.iter() {
+        for cap_map in &pd.cap_maps {
             if user_cap_slots.contains(&cap_map.dest_cspace_slot) {
                 return Err(format!(
                     "Error: Overlapping cap slot: {} in protection domain: '{}'",
@@ -1968,9 +1969,9 @@ pub fn parse(filename: &str, xml: &str, config: &Config) -> Result<SystemDescrip
 
             if seen_pd_cap_maps.contains(&(cap_map.cap_type, cap_map.pd_name.clone())) {
                 return Err(format!(
-                        "Error: Duplicate cap mapping of type '{:?}'. Src PD: '{}', dest PD: '{}'.",
-                        cap_map.cap_type, cap_map.pd_name, pd.name
-                    ));
+                    "Error: Duplicate cap mapping of type '{:?}'. Src PD: '{}', dest PD: '{}'.",
+                    cap_map.cap_type, cap_map.pd_name, pd.name
+                ));
             } else {
                 seen_pd_cap_maps.push((cap_map.cap_type.clone(), cap_map.pd_name.clone()))
             }