Merge pull request #42 from ardbiesheuvel/bbm2

Bbm2
google · Oct 31, 2023 · 39e7e39 · 39e7e39
2 parents 1d4b4b5 + 32dbf60
commit 39e7e39
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 24 deletions.
diff --git a/src/idmap.rs b/src/idmap.rs
@@ -434,7 +434,7 @@ mod tests {
                 Attributes::NORMAL | Attributes::VALID | Attributes::READ_ONLY,
             ),
             Err(MapError::BreakBeforeMakeViolation(MemoryRegion::new(
-                0,
+                BLOCK_SIZE,
                 BLOCK_SIZE + PAGE_SIZE
             )))
         );
@@ -453,7 +453,7 @@ mod tests {
                 Attributes::DEVICE_NGNRE | Attributes::VALID | Attributes::NON_GLOBAL,
             ),
             Err(MapError::BreakBeforeMakeViolation(MemoryRegion::new(
-                0, BLOCK_SIZE
+                0, PAGE_SIZE
             )))
         );
 
@@ -464,7 +464,7 @@ mod tests {
                 Attributes::NORMAL,
             ),
             Err(MapError::BreakBeforeMakeViolation(MemoryRegion::new(
-                PAGE_SIZE,
+                BLOCK_SIZE,
                 BLOCK_SIZE + PAGE_SIZE
             )))
         );

diff --git a/src/lib.rs b/src/lib.rs
@@ -232,21 +232,23 @@ impl<T: Translation + Clone> Mapping<T> {
     where
         F: Fn(&MemoryRegion, &mut Descriptor, usize) -> Result<(), ()> + ?Sized,
     {
-        self.walk_range(
+        self.root.visit_range(
             range,
             &mut |mr: &MemoryRegion, d: &Descriptor, level: usize| {
                 if d.is_valid() {
+                    let err = MapError::BreakBeforeMakeViolation(mr.clone());
+
                     if !mr.is_block(level) {
                         // Cannot split a live block mapping
-                        return Err(());
+                        return Err(err);
                     }
 
                     // Get the new flags and output address for this descriptor by applying
                     // the updater function to a copy
                     let (flags, oa) = {
                         let mut dd = *d;
-                        updater(mr, &mut dd, level)?;
-                        (dd.flags().ok_or(())?, dd.output_address())
+                        updater(mr, &mut dd, level).or(Err(err.clone()))?;
+                        (dd.flags().ok_or(err.clone())?, dd.output_address())
                     };
 
                     if !flags.contains(Attributes::VALID) {
@@ -256,26 +258,24 @@ impl<T: Translation + Clone> Mapping<T> {
 
                     if oa != d.output_address() {
                         // Cannot change output address on a live mapping
-                        return Err(());
+                        return Err(err);
                     }
 
                     let desc_flags = d.flags().unwrap();
 
                     if (desc_flags ^ flags).intersects(Attributes::NORMAL) {
                         // Cannot change memory type
-                        return Err(());
+                        return Err(err);
                     }
 
                     if (desc_flags - flags).contains(Attributes::NON_GLOBAL) {
                         // Cannot convert from non-global to global
-                        return Err(());
+                        return Err(err);
                     }
                 }
                 Ok(())
             },
         )
-        .map_err(|_| MapError::BreakBeforeMakeViolation(range.clone()))?;
-        Ok(())
     }
 
     /// Maps the given range of virtual addresses to the corresponding range of physical addresses

diff --git a/src/paging.rs b/src/paging.rs
@@ -366,9 +366,19 @@ impl<T: Translation> RootTable<T> {
     pub fn walk_range<F>(&self, range: &MemoryRegion, f: &mut F) -> Result<(), MapError>
     where
         F: FnMut(&MemoryRegion, &Descriptor, usize) -> Result<(), ()>,
+    {
+        self.visit_range(range, &mut |mr, desc, level| {
+            f(mr, desc, level).map_err(|_| MapError::PteUpdateFault(*desc))
+        })
+    }
+
+    // Private version of `walk_range` using a closure that returns MapError on error
+    pub(crate) fn visit_range<F>(&self, range: &MemoryRegion, f: &mut F) -> Result<(), MapError>
+    where
+        F: FnMut(&MemoryRegion, &Descriptor, usize) -> Result<(), MapError>,
     {
         self.verify_region(range)?;
-        self.table.walk_range(&self.translation, range, f)
+        self.table.visit_range(&self.translation, range, f)
     }
 
     /// Returns the level of mapping used for the given virtual address:
@@ -699,24 +709,19 @@ impl<T: Translation> PageTableWithLevel<T> {
         Ok(())
     }
 
-    /// Walks a range of page table entries and passes each one to a caller provided function
-    /// If the range is not aligned to block boundaries, it will be expanded.
-    fn walk_range<F>(
-        &self,
-        translation: &T,
-        range: &MemoryRegion,
-        f: &mut F,
-    ) -> Result<(), MapError>
+    /// Walks a range of page table entries and passes each one to a caller provided function.
+    /// If the function returns an error, the walk is terminated and the error value is passed on
+    fn visit_range<F, E>(&self, translation: &T, range: &MemoryRegion, f: &mut F) -> Result<(), E>
     where
-        F: FnMut(&MemoryRegion, &Descriptor, usize) -> Result<(), ()>,
+        F: FnMut(&MemoryRegion, &Descriptor, usize) -> Result<(), E>,
     {
         let level = self.level;
         for chunk in range.split(level) {
             let entry = self.get_entry(chunk.0.start);
             if let Some(subtable) = entry.subtable(translation, level) {
-                subtable.walk_range(translation, &chunk, f)?;
+                subtable.visit_range(translation, &chunk, f)?;
             } else {
-                f(&chunk, entry, level).map_err(|_| MapError::PteUpdateFault(*entry))?;
+                f(&chunk, entry, level)?;
             }
         }
         Ok(())