Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UEFI boot should avoid using the lower 1MB of physical RAM #311

Open
jasoncouture opened this issue Jan 4, 2023 · 7 comments · May be fixed by #317
Open

UEFI boot should avoid using the lower 1MB of physical RAM #311

jasoncouture opened this issue Jan 4, 2023 · 7 comments · May be fixed by #317
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@jasoncouture
Copy link
Contributor

jasoncouture commented Jan 4, 2023

When activating additional CPUs they will start in real mode.
Trampoline assembly is used to initialize them, after issuing two IPI (inter processor interrupts) (INIT and START) via the APIC.

The start IPI sends the starting code segment for real mode, which is limited to the lower 1MB of memory.

The maximum physical memory address for real mode is FFFFF, which equates to segment F0, offset FFFF

Currently, the bootloader, in UEFI mode, will allocate pages starting near page 0, leaving no space left to place the IPI trampoline.

We could achieve this by allocating all pages below 1MB using UEFI allocate page function in a loop, and freeing them immediately preceding the call to exit boot services. This would ensure the UEFI loader does not allocate and reserve physical pages below 1MB.

@jasoncouture jasoncouture changed the title Bootloader should avoid using the lower 1MB of RAM UEFI boot should avoid using the lower 1MB of physical RAM Jan 4, 2023
@phil-opp
Copy link
Member

phil-opp commented Jan 5, 2023

Is it guaranteed that the UEFI alloc_pages function will allocate the lowest memory first? @tsoutsman reported in #315 that the UEFI memory map is not ordered in all cases, so maybe alloc_pages might allocate from other memory regions first? If that's the case, it would make it much more difficult to force the UEFI allocation function to yield the desired address range.

Starting additional CPU cores seems like a common use case, so how do other UEFI bootloaders/kernels deal with this? Is there maybe a way to mark a specific physical memory region as used to the UEFI firmware?

@tsoutsman
Copy link
Contributor

If that's the case, it would make it much more difficult to force the UEFI allocation function to yield the desired address range.

UEFI has an API to allocate at a specific address so we could request the pages starting at 0.

But that's neither here nor there because we only allocate memory via UEFI using the LOADER_DATA memory type, which is released to the kernel after the bootloader exits.

However, LegacyFrameAllocator is able to allocate frames under 1MB that aren't released to the kernel. Changing this line to use LegacyFrameAllocator::new_starting_at should be enough.

@phil-opp
Copy link
Member

phil-opp commented Jan 5, 2023

UEFI has an API to allocate at a specific address so we could request the pages starting at 0.

Ok perfect!

But that's neither here nor there because we only allocate memory via UEFI using the LOADER_DATA memory type, which is released to the kernel after the bootloader exits.

That's true with one exception: The kernel memory stays mapped. It is manually marked as used in construct_memory_map. So if we're unlucky, this memory could contain the lower 1MB of the physical address space.

However, LegacyFrameAllocator is able to allocate frames under 1MB that aren't released to the kernel. Changing this line to use LegacyFrameAllocator::new_starting_at should be enough.

We're already skipping frame 0, so we could just increase this lower bound here:

// skip frame 0 because the rust core library does not see 0 as a valid address
let start_frame = PhysFrame::containing_address(PhysAddr::new(0x1000));

@phil-opp phil-opp added enhancement New feature or request help wanted Extra attention is needed labels Jan 5, 2023
@jasoncouture
Copy link
Contributor Author

Thanks guys! I'll make the changes in my pr in progress to address this.

This came about because I refuse to use page 0, and the next free page my os can find is address 0x900000, but for IPI I need an address under 0xFFFFF, (aka, segment 0xF0, offset 0000 for the first page.

On hardware, specifically my Ryzen 5950x, and my 7950x trying to use page 0 leads to instability despite being marked as free by the memory map.

Hardware quirks. Annoying as always.

@phil-opp
Copy link
Member

phil-opp commented Jan 5, 2023

On hardware, specifically my Ryzen 5950x, and my 7950x trying to use page 0 leads to instability despite being marked as free by the memory map.

This could also be compiler related, since most compilers treat address 0 as invalid (to avoid accidental misuse of null pointers). The Rust compiler even exploits this for reducing the memory size of enums such as Option. So for example Option<&T> is the size of one pointer in Rust because the compiler uses the zero value to represent the None case. So the perceived instability might be because of that.

@jasoncouture
Copy link
Contributor Author

Yeah, the code I'm referring to is pure assembly. I loads IPI trampoline from embedded code, copied it to page 0, and had additional CPUs jump to it.
The trampoline at the end, loads the page table, and jumps far away, with a different virtual address for the stack.

And this was c++ where the behavior was observed.

Moving the IPI trampoline to 0x1000 and setting CS to 0x01 during SIPI fixed the problem. So definitely not a compiler issue in this case.

@jasoncouture
Copy link
Contributor Author

Inspecting page 0 with a memory dump, without copying code, shows what appears to be valid assembly. Didn't re what it does.

But page 1 was filled with 0xFE (which is what I see on any empty page)

@jasoncouture jasoncouture linked a pull request Jan 6, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants