Skip to content

openhcl_boot: correctly divide ram per node to page aligned size - #4076

Open
chris-oo wants to merge 2 commits into
microsoft:mainfrom
chris-oo:ram-per-node-aligned
Open

openhcl_boot: correctly divide ram per node to page aligned size#4076
chris-oo wants to merge 2 commits into
microsoft:mainfrom
chris-oo:ram-per-node-aligned

Conversation

@chris-oo

Copy link
Copy Markdown
Member

Without doing this alignment, openhcl_boot will later panic when we try to split allocations based on free ranges for a MemoryRange, because the split point will not be page aligned.

@chris-oo
chris-oo requested a review from a team as a code owner July 28, 2026 23:48
Copilot AI review requested due to automatic review settings July 28, 2026 23:48
@github-actions github-actions Bot added the unsafe Related to unsafe code label Jul 28, 2026
@github-actions

Copy link
Copy Markdown

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

@chris-oo chris-oo added the backport_1.8.2607 Change should be backported to the release/1.8.2607 branch label Jul 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates openhcl_boot’s VTL2 RAM allocation logic to ensure the computed per-NUMA-node allocation size is page-aligned, preventing a later panic when splitting MemoryRanges at non-page-aligned offsets.

Changes:

  • Page-align the computed ram_per_node value used to drive per-node allocation sizing.

Comment on lines +248 to +252
// Next, calculate the amount of memory that needs to be allocated per numa
// node.
let ram_per_node = vtl2_size / numa_node_count as u64;
// node. Make sure ram_per_node is page aligned by aligning up to the next
// page.
let ram_per_node =
((vtl2_size / numa_node_count as u64) + (HV_PAGE_SIZE - 1)) & !(HV_PAGE_SIZE - 1);
@github-actions

Copy link
Copy Markdown

// node. Make sure ram_per_node is page aligned by aligning up to the next
// page.
let ram_per_node =
((vtl2_size / numa_node_count as u64) + (HV_PAGE_SIZE - 1)) & !(HV_PAGE_SIZE - 1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
((vtl2_size / numa_node_count as u64) + (HV_PAGE_SIZE - 1)) & !(HV_PAGE_SIZE - 1);
((vtl2_size / numa_node_count as u64).next_multiple_of(HV_PAGE_SIZE);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can still replace your align_up i think

Copilot AI review requested due to automatic review settings July 29, 2026 18:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

openhcl/openhcl_boot/src/host_params/dt/mod.rs:253

  • ram_per_node is being aligned up after dividing by numa_node_count. This can over-allocate VTL2 RAM by up to (numa_node_count * (ALIGNMENT_GRANULARITY-1)) bytes (worst case: +7 pages per node), which reduces VTL0 RAM more than necessary. Aligning the total once (max +7 pages overall) and then distributing the remainder to the last node (similar to the VTL2 pool split logic earlier in this file) keeps boundaries aligned without per-node over-allocation.
    const ALIGNMENT_GRANULARITY: u64 = HV_PAGE_SIZE * 8;
    let align_up = |value| (value + ALIGNMENT_GRANULARITY - 1) & !(ALIGNMENT_GRANULARITY - 1);
    let ram_per_node = align_up(vtl2_size / numa_node_count as u64);

@github-actions

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport_1.8.2607 Change should be backported to the release/1.8.2607 branch unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants