Skip to content

Commit

Permalink
openhcl: cleanup private page pool changes (#461)
Browse files Browse the repository at this point in the history 
Some additional cleanup of the private pool and fixes with keep alive on
openvmm, and enable using the private pool via cmdline.

Do not save the pool state if we are not doing keep alive, as no device
should have allocations that need persisting. Fix advertising keep alive
support on openvmm because it doesn't work yet.
  • Loading branch information
@chris-oo
chris-oo authored Dec 12, 2024
1 parent 61c34e9 commit 457cea8
Show file tree
Hide file tree
Showing 11 changed files with 232 additions and 52 deletions.
44 changes: 34 additions & 10 deletions openhcl/host_fdt_parser/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,11 @@ pub struct ParsedDeviceTree<
/// Entropy from the host to be used by the OpenHCL kernel
#[cfg_attr(feature = "inspect", inspect(with = "Option::is_some"))]
pub entropy: Option<ArrayVec<u8, MAX_ENTROPY_SIZE>>,
/// Preserved DMA memory size in pages.
pub preserve_dma_4k_pages: Option<u64>,
/// The number of pages the host has provided as a hint for device dma.
///
/// This is used to allocate a persistent VTL2 pool on non-isolated guests,
/// to allow devices to stay alive during a servicing operation.
pub device_dma_page_count: Option<u64>,
}

/// The memory allocation mode provided by the host. This determines how OpenHCL
Expand Down Expand Up @@ -309,7 +312,7 @@ impl<
gic: None,
memory_allocation_mode: MemoryAllocationMode::Host,
entropy: None,
preserve_dma_4k_pages: None,
device_dma_page_count: None,
}
}

Expand Down Expand Up @@ -511,7 +514,7 @@ impl<
}
// These parameters may not be present so it is not an error if they are missing.
"servicing" => {
storage.preserve_dma_4k_pages = openhcl_child
storage.device_dma_page_count = openhcl_child
.find_property("dma-preserve-pages")
.ok()
.flatten()
Expand Down Expand Up @@ -709,7 +712,7 @@ impl<
gic: _,
memory_allocation_mode: _,
entropy: _,
preserve_dma_4k_pages: _,
device_dma_page_count: _,
} = storage;

*device_tree_size = parser.total_size;
Expand Down Expand Up @@ -1351,10 +1354,11 @@ mod tests {
.end_node()
.unwrap();

// openhcl node - contains memory allocation mode.
// openhcl node - contains openhcl specific information.
let p_memory_allocation_mode = root.add_string("memory-allocation-mode").unwrap();
let p_memory_allocation_size = root.add_string("memory-size").unwrap();
let p_mmio_allocation_size = root.add_string("mmio-size").unwrap();
let p_device_dma_page_count = root.add_string("dma-preserve-pages").unwrap();
let mut openhcl = root.start_node("openhcl").unwrap();

let memory_alloc_str = match context.memory_allocation_mode {
Expand All @@ -1376,12 +1380,23 @@ mod tests {
}
};

root = openhcl
openhcl = openhcl
.add_str(p_memory_allocation_mode, memory_alloc_str)
.unwrap()
.end_node()
.unwrap();

// add device_dma_page_count
if let Some(device_dma_page_count) = context.device_dma_page_count {
openhcl = openhcl
.start_node("servicing")
.unwrap()
.add_u64(p_device_dma_page_count, device_dma_page_count)
.unwrap()
.end_node()
.unwrap();
}

root = openhcl.end_node().unwrap();

let bytes_used = root
.end_node()
.unwrap()
Expand All @@ -1404,6 +1419,7 @@ mod tests {
com3_serial: bool,
gic: Option<GicInfo>,
memory_allocation_mode: MemoryAllocationMode,
device_dma_page_count: Option<u64>,
) -> TestParsedDeviceTree {
let mut context = TestParsedDeviceTree::new();
context.device_tree_size = dt_size;
Expand All @@ -1416,13 +1432,14 @@ mod tests {
context.cpus.try_extend_from_slice(cpus).unwrap();
context.gic = gic;
context.memory_allocation_mode = memory_allocation_mode;
context.device_dma_page_count = device_dma_page_count;
context
}

#[test]
fn test_basic_dt() {
let orig = create_parsed(
2568,
2608,
&[
MemoryEntry {
range: MemoryRange::try_new(0..(1024 * HV_PAGE_SIZE)).unwrap(),
Expand Down Expand Up @@ -1473,6 +1490,7 @@ mod tests {
gic_redistributor_stride: 0x20000,
}),
MemoryAllocationMode::Host,
Some(1234),
);

let dt = build_dt(&orig);
Expand Down Expand Up @@ -1538,6 +1556,7 @@ mod tests {
memory_size: Some(1000 * 1024 * 1024), // 1000 MB
mmio_size: Some(128 * 1024 * 1024), // 128 MB
},
None,
);

let dt = build_dt(&orig);
Expand Down Expand Up @@ -1584,6 +1603,7 @@ mod tests {
false,
None,
MemoryAllocationMode::Host,
None,
);

let dt = build_dt(&bad);
Expand Down Expand Up @@ -1621,6 +1641,7 @@ mod tests {
false,
None,
MemoryAllocationMode::Host,
None,
);

let dt = build_dt(&bad);
Expand Down Expand Up @@ -1663,6 +1684,7 @@ mod tests {
false,
None,
MemoryAllocationMode::Host,
None,
);

let dt = build_dt(&bad);
Expand Down Expand Up @@ -1705,6 +1727,7 @@ mod tests {
false,
None,
MemoryAllocationMode::Host,
None,
);

let dt = build_dt(&bad);
Expand Down Expand Up @@ -1764,6 +1787,7 @@ mod tests {
true,
None,
MemoryAllocationMode::Host,
None,
);

let dt = build_dt(&orig);
Expand Down
79 changes: 73 additions & 6 deletions openhcl/openhcl_boot/src/cmdline.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,31 @@ use underhill_confidentiality::OPENHCL_CONFIDENTIAL_DEBUG_ENV_VAR_NAME;
const BOOT_LOG: &str = "OPENHCL_BOOT_LOG=";
const SERIAL_LOGGER: &str = "com3";

/// Enable the private VTL2 GPA pool for page allocations. This is only enabled
/// via the command line, because in order to support the VTL2 GPA pool
/// generically, the boot shim must read serialized data from the previous
/// OpenHCL instance on a servicing boot in order to guarantee the same memory
/// layout is presented.
///
/// The value specified is the number of 4K pages to reserve for the pool.
///
/// TODO: Remove this commandline once support for reading saved state is
/// supported in openhcl_boot.
const ENABLE_VTL2_GPA_POOL: &str = "OPENHCL_ENABLE_VTL2_GPA_POOL=";

#[derive(Debug, PartialEq)]
pub struct BootCommandLineOptions {
pub logger: Option<LoggerType>,
pub confidential_debug: bool,
pub enable_vtl2_gpa_pool: Option<u64>,
}

/// Parse arguments from a command line.
pub fn parse_boot_command_line(cmdline: &str) -> BootCommandLineOptions {
let mut result = BootCommandLineOptions {
logger: None,
confidential_debug: false,
enable_vtl2_gpa_pool: None,
};

for arg in cmdline.split_whitespace() {
Expand All @@ -41,6 +55,17 @@ pub fn parse_boot_command_line(cmdline: &str) -> BootCommandLineOptions {
result.logger = Some(LoggerType::Serial);
}
}
} else if arg.starts_with(ENABLE_VTL2_GPA_POOL) {
result.enable_vtl2_gpa_pool = arg.split_once('=').and_then(|(_, arg)| {
let num = arg.parse::<u64>().unwrap_or(0);
// A size of 0 or failure to parse is treated as disabling
// the pool.
if num == 0 {
None
} else {
Some(num)
}
});
}
}

Expand All @@ -57,39 +82,44 @@ mod tests {
parse_boot_command_line("OPENHCL_BOOT_LOG=com3"),
BootCommandLineOptions {
logger: Some(LoggerType::Serial),
confidential_debug: false
confidential_debug: false,
enable_vtl2_gpa_pool: None
}
);

assert_eq!(
parse_boot_command_line("OPENHCL_BOOT_LOG=1"),
BootCommandLineOptions {
logger: None,
confidential_debug: false
confidential_debug: false,
enable_vtl2_gpa_pool: None
}
);

assert_eq!(
parse_boot_command_line("OPENHCL_BOOT_LOG=random"),
BootCommandLineOptions {
logger: None,
confidential_debug: false
confidential_debug: false,
enable_vtl2_gpa_pool: None
}
);

assert_eq!(
parse_boot_command_line("OPENHCL_BOOT_LOG==com3"),
BootCommandLineOptions {
logger: None,
confidential_debug: false
confidential_debug: false,
enable_vtl2_gpa_pool: None
}
);

assert_eq!(
parse_boot_command_line("OPENHCL_BOOT_LOGserial"),
BootCommandLineOptions {
logger: None,
confidential_debug: false
confidential_debug: false,
enable_vtl2_gpa_pool: None
}
);

Expand All @@ -98,7 +128,44 @@ mod tests {
parse_boot_command_line(&cmdline),
BootCommandLineOptions {
logger: Some(LoggerType::Serial),
confidential_debug: true
confidential_debug: true,
enable_vtl2_gpa_pool: None
}
);
}

#[test]
fn test_vtl2_gpa_pool_parsing() {
assert_eq!(
parse_boot_command_line("OPENHCL_ENABLE_VTL2_GPA_POOL=1"),
BootCommandLineOptions {
logger: None,
confidential_debug: false,
enable_vtl2_gpa_pool: Some(1),
}
);
assert_eq!(
parse_boot_command_line("OPENHCL_ENABLE_VTL2_GPA_POOL=0"),
BootCommandLineOptions {
logger: None,
confidential_debug: false,
enable_vtl2_gpa_pool: None,
}
);
assert_eq!(
parse_boot_command_line("OPENHCL_ENABLE_VTL2_GPA_POOL=asdf"),
BootCommandLineOptions {
logger: None,
confidential_debug: false,
enable_vtl2_gpa_pool: None,
}
);
assert_eq!(
parse_boot_command_line("OPENHCL_ENABLE_VTL2_GPA_POOL=1024"),
BootCommandLineOptions {
logger: None,
confidential_debug: false,
enable_vtl2_gpa_pool: Some(1024),
}
);
}
Expand Down
2 changes: 1 addition & 1 deletion openhcl/openhcl_boot/src/dt.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ pub fn write_dt(
ReservedMemoryType::SidecarImage => MemoryVtlType::VTL2_SIDECAR_IMAGE,
ReservedMemoryType::SidecarNode => MemoryVtlType::VTL2_SIDECAR_NODE,
ReservedMemoryType::Vtl2Reserved => MemoryVtlType::VTL2_RESERVED,
ReservedMemoryType::PersistentGpaPool => MemoryVtlType::VTL2_GPA_POOL,
ReservedMemoryType::Vtl2GpaPool => MemoryVtlType::VTL2_GPA_POOL,
},
)
}),
Expand Down
Loading

0 comments on commit 457cea8

Please sign in to comment.