kernel: Add an "identifier" to AppId

capsules/src/ble_advertising_driver.rs

@@ -222,10 +222,10 @@ impl App {
     fn generate_random_address(&mut self, appid: kernel::AppId) -> ReturnCode {
         self.address = [
             0xf0,
-            (appid.idx() & 0xff) as u8,
-            ((appid.idx() << 8) & 0xff) as u8,
-            ((appid.idx() << 16) & 0xff) as u8,
-            ((appid.idx() << 24) & 0xff) as u8,
+            (appid.id() & 0xff) as u8,
+            ((appid.id() << 8) & 0xff) as u8,
+            ((appid.id() << 16) & 0xff) as u8,
+            ((appid.id() << 24) & 0xff) as u8,
             0xf0,
         ];
         ReturnCode::SUCCESS

capsules/src/low_level_debug/mod.rs

@@ -72,7 +72,7 @@ impl<'u, U: Transmit<'u>> TransmitClient for LowLevelDebug<'u, U> {
             let (app_num, first_entry) = applied_grant.enter(|owned_app_data, _| {
                 owned_app_data.queue.rotate_left(1);
                 (
-                    owned_app_data.appid().idx(),
+                    owned_app_data.appid().id(),
                     owned_app_data.queue[QUEUE_SIZE - 1].take(),
                 )
             });
@@ -98,7 +98,7 @@ impl<'u, U: Transmit<'u>> LowLevelDebug<'u, U> {
         use DebugEntry::Dropped;
 
         if let Some(buffer) = self.buffer.take() {
-            self.transmit_entry(buffer, appid.idx(), entry);
+            self.transmit_entry(buffer, appid.id(), entry);
             return;
         }
 

kernel/src/callback.rs

@@ -9,44 +9,122 @@ use crate::process;
 use crate::sched::Kernel;
 
 /// Userspace app identifier.
+///
+/// This should be treated as an opaque type that can be used to represent an
+/// application on the board without requiring an actual reference to a
+/// `ProcessType` object. Having this `AppId` reference type is useful for
+/// managing ownership and type issues in Rust, but more importantly `AppId`
+/// serves as a tool for capsules to hold pointers to applications.
+///
+/// Since `AppId` implements `Copy`, having an `AppId` does _not_ ensure that
+/// the process the `AppId` refers to is still valid. The process may have been
+/// removed, terminated, or restarted as a new process. Therefore, all uses of
+/// `AppId` in the kernel must check that the `AppId` is still valid. This check
+/// happens automatically when `.index()` is called, as noted by the return
+/// type: `Option<usize>`. `.index()` will return the index of the process in
+/// the processes array, but if the process no longer exists then `None` is
+/// returned.
+///
+/// Outside of the kernel crate, holders of an `AppId` may want to use `.id()`
+/// to retrieve a simple identifier for the process that can be communicated
+/// over a UART bus or syscall interface. This call is guaranteed to return a
+/// suitable identifier for the `AppId`, but does not check that the
+/// corresponding application still exists.
+///
+/// This type also provides capsules an interface for interacting with processes
+/// since they otherwise would have no reference to a `ProcessType`. Very limited
+/// operations are available through this interface since capsules should not
+/// need to know the details of any given process. However, certain information
+/// makes certain capsules possible to implement. For example, capsules can use
+/// the `get_editable_flash_range()` function so they can safely allow an app
+/// to modify its own flash.
 #[derive(Clone, Copy)]
 pub struct AppId {
+    /// Reference to the main kernel struct. This is needed for checking on
+    /// certain properties of the referred app (like its editable bounds), but
+    /// also for checking that the index is valid.
     crate kernel: &'static Kernel,
-    idx: usize,
+
+    /// The index in the kernel.PROCESSES[] array where this app's state is
+    /// stored. This makes for fast lookup of the process and helps with
+    /// implementing IPC.
+    ///
+    /// This value is crate visible to enable optimizations in sched.rs. Other
+    /// users should call `.index()` instead.
+    crate index: usize,
+
+    /// The unique identifier for this process. This can be used to refer to the
+    /// process in situations where a single number is required, for instance
+    /// when referring to specific applications across the syscall interface.
+    ///
+    /// The combination of (index, identifier) is used to check if the app this
+    /// `AppId` refers to is still valid. If the stored identifier in the
+    /// process at the given index does not match the value saved here, then the
+    /// process moved or otherwise ended, and this `AppId` is no longer valid.
+    identifier: usize,
 }
 
 impl PartialEq for AppId {
     fn eq(&self, other: &AppId) -> bool {
-        self.idx == other.idx
+        self.identifier == other.identifier
     }
 }
 
 impl Eq for AppId {}
 
 impl fmt::Debug for AppId {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", self.idx)
+        write!(f, "{}", self.identifier)
     }
 }
 
 impl AppId {
-    crate fn new(kernel: &'static Kernel, idx: usize) -> AppId {
+    crate fn new(kernel: &'static Kernel, identifier: usize, index: usize) -> AppId {
         AppId {
             kernel: kernel,
-            idx: idx,
+            identifier: identifier,
+            index: index,
         }
     }
 
-    pub fn idx(&self) -> usize {
-        self.idx
+    /// Get the location of this app in the processes array.
+    ///
+    /// This will return `Some(index)` if the identifier stored in this `AppId`
+    /// matches the app saved at the known index. If the identifier does not
+    /// match then `None` will be returned.
+    crate fn index(&self) -> Option<usize> {
+        // Do a lookup to make sure that the index we have is correct.
+        if self.kernel.appid_is_valid(self) {
+            Some(self.index)
+        } else {
+            None
+        }
+    }
+
+    /// Get a `usize` unique identifier for the app this `AppId` refers to.
+    ///
+    /// This function should not generally be used, instead code should just use
+    /// the `AppId` object itself to refer to various apps on the system.
+    /// However, getting just a `usize` identifier is particularly useful when
+    /// referring to a specific app with things outside of the kernel, say for
+    /// userspace (e.g. IPC) or tockloader (e.g. for debugging) where a concrete
+    /// number is required.
+    ///
+    /// Note, this will always return the saved unique identifier for the app
+    /// originally referred to, even if that app no longer exists. For example,
+    /// the app may have restarted, or may have been ended or removed by the
+    /// kernel. Therefore, calling `id()` is _not_ a valid way to check
+    /// that an application still exists.
+    pub fn id(&self) -> usize {
+        self.identifier
     }
 
     /// Returns the full address of the start and end of the flash region that
     /// the app owns and can write to. This includes the app's code and data and
     /// any padding at the end of the app. It does not include the TBF header,
     /// or any space that the kernel is using for any potential bookkeeping.
     pub fn get_editable_flash_range(&self) -> (usize, usize) {
-        self.kernel.process_map_or((0, 0), self.idx, |process| {
+        self.kernel.process_map_or((0, 0), *self, |process| {
             let start = process.flash_non_protected_start() as usize;
             let end = process.flash_end() as usize;
             (start, end)
@@ -101,7 +179,7 @@ impl Callback {
         let res = self
             .app_id
             .kernel
-            .process_map_or(false, self.app_id.idx(), |process| {
+            .process_map_or(false, self.app_id, |process| {
                 process.enqueue_task(process::Task::FunctionCall(process::FunctionCall {
                     source: process::FunctionCallSource::Driver(self.callback_id),
                     argument0: r0,

kernel/src/grant.rs

@@ -6,7 +6,7 @@ use core::ops::{Deref, DerefMut};
 use core::ptr::{write, write_volatile, Unique};
 
 use crate::callback::AppId;
-use crate::process::Error;
+use crate::process::{Error, ProcessType};
 use crate::sched::Kernel;
 
 /// Region of process memory reserved for the kernel.
@@ -60,11 +60,9 @@ impl<T: ?Sized> Drop for Owned<T> {
     fn drop(&mut self) {
         unsafe {
             let data = self.data.as_ptr() as *mut u8;
-            self.appid
-                .kernel
-                .process_map_or((), self.appid.idx(), |process| {
-                    process.free(data);
-                });
+            self.appid.kernel.process_map_or((), self.appid, |process| {
+                process.free(data);
+            });
         }
     }
 }
@@ -87,7 +85,7 @@ impl Allocator {
         unsafe {
             self.appid
                 .kernel
-                .process_map_or(Err(Error::NoSuchApp), self.appid.idx(), |process| {
+                .process_map_or(Err(Error::NoSuchApp), self.appid, |process| {
                     process.alloc(size_of::<T>(), align_of::<T>()).map_or(
                         Err(Error::OutOfMemory),
                         |arr| {
@@ -145,16 +143,20 @@ impl<T: Default> Grant<T> {
 
     pub fn grant(&self, appid: AppId) -> Option<AppliedGrant<T>> {
         unsafe {
-            appid.kernel.process_map_or(None, appid.idx(), |process| {
-                let cntr = *(process.grant_ptr(self.grant_num) as *mut *mut T);
-                if cntr.is_null() {
-                    None
+            appid.kernel.process_map_or(None, appid, |process| {
+                if let Some(grant_ptr_ref) = process.grant_ptr(self.grant_num) {
+                    let cntr = *(grant_ptr_ref as *mut *mut T);
+                    if cntr.is_null() {
+                        None
+                    } else {
+                        Some(AppliedGrant {
+                            appid: appid,
+                            grant: cntr,
+                            _phantom: PhantomData,
+                        })
+                    }
                 } else {
-                    Some(AppliedGrant {
-                        appid: appid,
-                        grant: cntr,
-                        _phantom: PhantomData,
-                    })
+                    None
                 }
             })
         }
@@ -168,7 +170,7 @@ impl<T: Default> Grant<T> {
         unsafe {
             appid
                 .kernel
-                .process_map_or(Err(Error::NoSuchApp), appid.idx(), |process| {
+                .process_map_or(Err(Error::NoSuchApp), appid, |process| {
                     // Here is an example of how the grants are laid out in a
                     // process's memory:
                     //
@@ -199,36 +201,40 @@ impl<T: Default> Grant<T> {
                     //
                     // Get a pointer to where the grant pointer is stored in the
                     // process memory.
-                    let ctr_ptr = process.grant_ptr(self.grant_num) as *mut *mut T;
-                    // If the pointer at that location is NULL then the grant
-                    // memory needs to be allocated.
-                    let new_grant = if (*ctr_ptr).is_null() {
-                        process
-                            .alloc(size_of::<T>(), align_of::<T>())
-                            .map(|root_arr| {
-                                let root_ptr = root_arr.as_mut_ptr() as *mut T;
-                                // Initialize the grant contents using ptr::write, to
-                                // ensure that we don't try to drop the contents of
-                                // uninitialized memory when T implements Drop.
-                                write(root_ptr, Default::default());
-                                // Record the location in the grant pointer.
-                                write_volatile(ctr_ptr, root_ptr);
-                                root_ptr
-                            })
-                    } else {
-                        Some(*ctr_ptr)
-                    };
+                    if let Some(grant_ptr_ref) = process.grant_ptr(self.grant_num) {
+                        let ctr_ptr = grant_ptr_ref as *mut *mut T;
+                        // If the pointer at that location is NULL then the grant
+                        // memory needs to be allocated.
+                        let new_grant = if (*ctr_ptr).is_null() {
+                            process
+                                .alloc(size_of::<T>(), align_of::<T>())
+                                .map(|root_arr| {
+                                    let root_ptr = root_arr.as_mut_ptr() as *mut T;
+                                    // Initialize the grant contents using ptr::write, to
+                                    // ensure that we don't try to drop the contents of
+                                    // uninitialized memory when T implements Drop.
+                                    write(root_ptr, Default::default());
+                                    // Record the location in the grant pointer.
+                                    write_volatile(ctr_ptr, root_ptr);
+                                    root_ptr
+                                })
+                        } else {
+                            Some(*ctr_ptr)
+                        };
 
-                    // If the grant region already exists or there was enough
-                    // memory to allocate it, call the passed in closure with
-                    // the borrowed grant region.
-                    new_grant.map_or(Err(Error::OutOfMemory), move |root_ptr| {
-                        let root_ptr = root_ptr as *mut T;
-                        let mut root = Borrowed::new(&mut *root_ptr, appid);
-                        let mut allocator = Allocator { appid: appid };
-                        let res = fun(&mut root, &mut allocator);
-                        Ok(res)
-                    })
+                        // If the grant region already exists or there was enough
+                        // memory to allocate it, call the passed in closure with
+                        // the borrowed grant region.
+                        new_grant.map_or(Err(Error::OutOfMemory), move |root_ptr| {
+                            let root_ptr = root_ptr as *mut T;
+                            let mut root = Borrowed::new(&mut *root_ptr, appid);
+                            let mut allocator = Allocator { appid: appid };
+                            let res = fun(&mut root, &mut allocator);
+                            Ok(res)
+                        })
+                    } else {
+                        Err(Error::InactiveApp)
+                    }
                 })
         }
     }
@@ -238,41 +244,67 @@ impl<T: Default> Grant<T> {
         F: Fn(&mut Owned<T>),
     {
         self.kernel.process_each(|process| unsafe {
-            let root_ptr = *(process.grant_ptr(self.grant_num) as *mut *mut T);
-            if !root_ptr.is_null() {
-                let mut root = Owned::new(root_ptr, process.appid());
-                fun(&mut root);
+            if let Some(grant_ptr_ref) = process.grant_ptr(self.grant_num) {
+                let root_ptr = *(grant_ptr_ref as *mut *mut T);
+                if !root_ptr.is_null() {
+                    let mut root = Owned::new(root_ptr, process.appid());
+                    fun(&mut root);
+                }
             }
         });
     }
 
+    /// Get an iterator over all processes and their active grant regions for
+    /// this particular grant.
     pub fn iter(&self) -> Iter<T> {
         Iter {
             grant: self,
-            index: 0,
-            len: self.kernel.number_of_process_slots(),
+            subiter: self.kernel.get_process_iter(),
         }
     }
 }
 
 pub struct Iter<'a, T: 'a + Default> {
     grant: &'a Grant<T>,
-    index: usize,
-    len: usize,
+    subiter: core::slice::Iter<'a, Option<&'a dyn ProcessType>>,
 }
 
 impl<T: Default> Iterator for Iter<'a, T> {
     type Item = AppliedGrant<T>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        while self.index < self.len {
-            let idx = self.index;
-            self.index += 1;
-            let res = self.grant.grant(AppId::new(self.grant.kernel, idx));
-            if res.is_some() {
-                return res;
-            }
-        }
-        None
+        // Save a local copy of grant_num so we don't have to access `self`
+        // in the closure below.
+        let grant_num = self.grant.grant_num;
+
+        // Get the next `AppId` from the kernel processes array. There can be
+        // empty slots in the processes array, so we use `find_map()` to skip
+        // over those. Since the iterator itself is saved calling this function
+        // again will start where we left off.
+        let res = self.subiter.find_map(|pi| {
+            pi.map_or(None, |p| {
+                // We have found a candidate process that exists in the
+                // processes array. Now we have to check if this grant is setup
+                // for this process. If not, we have to skip it and keep
+                // looking.
+                unsafe {
+                    if let Some(grant_ptr_ref) = p.grant_ptr(grant_num) {
+                        let cntr = *(grant_ptr_ref as *mut *mut T);
+                        if cntr.is_null() {
+                            None
+                        } else {
+                            Some(p.appid())
+                        }
+                    } else {
+                        None
+                    }
+                }
+            })
+        });
+
+        // Check if our find above returned another `AppId`, or if we hit the
+        // end of the iterator. If we found another app, try to access its grant
+        // region.
+        res.map_or(None, |app| self.grant.grant(app))
     }
 }