Skip to content

Commit

Permalink
page-tracking: Propagate errors from hypervisor page allocator
Browse files Browse the repository at this point in the history
Propagate the errors, rather than use .expect() for the creation of the
page allocator for the hypervisor's memory.

See: #100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
  • Loading branch information
rbradford authored and abrestic-rivos committed Apr 4, 2023
1 parent 69c0a52 commit edc0d96
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 14 deletions.
2 changes: 1 addition & 1 deletion drivers/src/iommu/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ mod tests {
.unwrap()
.build()
};
let hyp_mem = HypPageAlloc::new(&mut hw_map);
let hyp_mem = HypPageAlloc::new(&mut hw_map).unwrap();
let (page_tracker, host_pages) = PageTracker::from(hyp_mem, PageSize::Size4k as u64);
// Leak the backing ram so it doesn't get freed
std::mem::forget(backing_mem);
Expand Down
10 changes: 5 additions & 5 deletions page-tracking/src/page_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ pub struct PageMap {
impl PageMap {
/// Builds a new `PageMap` from a populated `HwMemMap`. It will track ownership information
/// for each page in the system.
pub fn build_from(mem_map: &mut HwMemMap) -> Self {
pub fn build_from(mem_map: &mut HwMemMap) -> PageTrackingResult<Self> {
// Determine how many pages we'll need for the page map.
let total_pages = mem_map
.regions()
Expand All @@ -536,7 +536,8 @@ impl PageMap {
let page_map_region = mem_map
.regions()
.find(|r| r.region_type() == HwMemRegionType::Available && r.size() >= page_map_size)
.expect("No free space for PageMap");
.ok_or(PageTrackingError::PageMapNoSpace)?;

let page_map_base = page_map_region.base();

// Safe to create pages from this memory as `HwMemMap` guarantees that this range is
Expand All @@ -554,11 +555,10 @@ impl PageMap {
RawAddr::from(page_map_base),
page_map_size,
)
.expect("Failed to reserve page map");

.map_err(PageTrackingError::PageMapReserveRegion)?;
let mut page_map = Self::new(struct_pages);
page_map.populate_from(mem_map);
page_map
Ok(page_map)
}

/// Constructs an empty `PageMap` from an existing vector of `PageInfo` structs.
Expand Down
16 changes: 10 additions & 6 deletions page-tracking/src/page_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use sync::Mutex;

use crate::collections::{RawPageVec, StaticPageRef};
use crate::page_info::{PageInfo, PageMap, PageState};
use crate::{HwMemMap, PageList, TlbVersion};
use crate::{hw_mem_map, HwMemMap, PageList, TlbVersion};

/// Errors related to managing physical page information.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -63,6 +63,10 @@ pub enum Error {
PageNotBlockable,
/// Attempt to unblock, promote, demote or remove a page that is not blocked.
PageNotBlocked,
/// Failed to reserve region for page map
PageMapReserveRegion(hw_mem_map::Error),
/// No memory available for page map
PageMapNoSpace,
}

/// Holds the result of page tracking operations.
Expand Down Expand Up @@ -163,7 +167,7 @@ impl PageTracker {
.unwrap()
.build()
};
let hyp_mem = HypPageAlloc::new(&mut hw_map);
let hyp_mem = HypPageAlloc::new(&mut hw_map).unwrap();
let (page_tracker, host_pages) = PageTracker::from(hyp_mem, PageSize::Size4k as u64);
// Leak the backing ram so it doesn't get freed
std::mem::forget(backing_mem);
Expand Down Expand Up @@ -655,14 +659,14 @@ pub struct HypPageAlloc {
impl HypPageAlloc {
/// Creates a new `HypPageAlloc`. The memory map passed in contains information about what
/// physical memory can be used by the machine.
pub fn new(mem_map: &mut HwMemMap) -> Self {
pub fn new(mem_map: &mut HwMemMap) -> Result<Self> {
let first_page = mem_map.regions().next().unwrap().base();
let mut hyp_pages = Self {
next_page: None,
pages: PageMap::build_from(mem_map),
pages: PageMap::build_from(mem_map)?,
};
hyp_pages.next_page = hyp_pages.next_free_page(first_page);
hyp_pages
Ok(hyp_pages)
}

/// Takes ownership of the remaining free pages, cleaning them and linking them together. Returns
Expand Down Expand Up @@ -848,7 +852,7 @@ mod tests {
.unwrap()
.build()
};
let hyp_mem = HypPageAlloc::new(&mut hw_map);
let hyp_mem = HypPageAlloc::new(&mut hw_map).unwrap();
// Leak the backing ram so it doesn't get freed
std::mem::forget(backing_mem);
hyp_mem
Expand Down
2 changes: 1 addition & 1 deletion riscv-page-tables/src/test_stubs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub fn stub_sys_memory() -> StubState {
.unwrap()
.build()
};
let mut hyp_mem = HypPageAlloc::new(&mut hw_map);
let mut hyp_mem = HypPageAlloc::new(&mut hw_map).unwrap();
let root_pages = hyp_mem.take_pages_for_host_state_with_alignment(4, Sv48x4::TOP_LEVEL_ALIGN);
let pte_pages = hyp_mem.take_pages_for_host_state(3);
let (page_tracker, host_pages) = PageTracker::from(hyp_mem, MEM_ALIGN as u64);
Expand Down
7 changes: 6 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,8 @@ enum Error {
CpuMissingFeature(RequiredCpuFeature),
/// Problem generating CPU topology
CpuTopologyGeneration(drivers::cpu::Error),
// Error creating hypervisor allocator
CreateHypervisorAllocator(PageTrackingError),
/// Creating hypervisor map failed
CreateHypervisorMap(hyp_map::Error),
/// Creating (per CPU) SMP state
Expand Down Expand Up @@ -398,6 +400,9 @@ impl Display for Error {
BuildMemoryMap(e) => write!(f, "Failed to build memory map: {:?}", e),
CpuMissingFeature(feature) => write!(f, "Missing required CPU feature: {:?}", feature),
CpuTopologyGeneration(e) => write!(f, "Failed to generate CPU topology: {}", e),
CreateHypervisorAllocator(e) => {
write!(f, "Failed to create hypervisor page allocator: {:?}", e)
}
CreateHypervisorMap(e) => write!(f, "Cannot create Hypervisor map: {:?}", e),
CreateSmpState(e) => write!(f, "Error during (per CPU) SMP setup: {}", e),
FdtCreation(e) => write!(f, "Failed to construct device-tree: {}", e),
Expand Down Expand Up @@ -537,7 +542,7 @@ fn primary_init(hart_id: u64, fdt_addr: u64) -> Result<CpuParams, Error> {

// Create an allocator for the remaining pages. Anything that's left over will be mapped
// into the host VM.
let mut hyp_mem = HypPageAlloc::new(&mut mem_map);
let mut hyp_mem = HypPageAlloc::new(&mut mem_map).map_err(Error::CreateHypervisorAllocator)?;
// NOTE: Do not modify the hardware memory map from here on.
let mem_map = mem_map; // Remove mutability.

Expand Down

0 comments on commit edc0d96

Please sign in to comment.