Skip to content

Conversation

@babayet2
Copy link
Collaborator

@babayet2 babayet2 commented Oct 28, 2025

This PR builds on the the loader refactor in #2062 to dynamically construct R+X page tables for AP startup in the boot shim.

The context for these changes can be found in issue #1563

@github-actions github-actions bot added the unsafe Related to unsafe code label Nov 11, 2025
@github-actions
Copy link

⚠️ 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

@babayet2
Copy link
Collaborator Author

Pulling this out of draft mode so we can see CI results - we should not merge this until after Chris has reviewed and merged #2062

@babayet2 babayet2 marked this pull request as ready for review November 11, 2025 19:06
@babayet2 babayet2 requested review from a team as code owners November 11, 2025 19:06
unsafe {
core::ptr::copy_nonoverlapping(
page_tables.as_ptr(),
page_table_region.range.start() as *mut u8,
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure it's guaranteed that page_table_region is valid to be accessed. In most cases, we are probably allocating from memory that is in the initial identity mapped region, but it's possible that the address space manager could choose to give you some other block of memory where we don't have a valid identity mapping for.

I think this leaves you with two options:
a) prereserve a region at load time that we may shrink to fit to return memory to the kernel (or just waste 20 pages and put a TODO there)
b) use the local_map to map this region on-the-fly, and then you don't need to use unsafe.

If possible, we could use b?

Copy link
Member

Choose a reason for hiding this comment

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

I think also using the local_map gives you a &mut [u8] which you can also then save another copy and allocation of the flattened page table, and you can pass that to the builder itself, right?

@chris-oo chris-oo self-assigned this Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants