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

Limit BIOS bootloader's max_phys_addr to 4 GiB #260

Merged
merged 2 commits into from
Sep 25, 2022

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented Sep 22, 2022

When determining the maximum physical address for the BIOS bootloader's identity mapping, we currently use the highest address in the E380 memory map, no matter how high it is. However, the bootloader runs in protected mode, and therefore, it cannot address more than 4 GiB of memory. We can save some time by not identity mapping addresses over 4 GiB, as the bootloader cannot address them anyway.

This commit changes the BIOS bootloader to skip addresses over 4 GiB when determining the maximum physical address.

This is one of the changes described in issue #259.

When determining the maximum physical address for the BIOS bootloader's
identity mapping, we currently use the highest address in the E380
memory map, no matter how high it is. However, the bootloader runs in
protected mode, and therefore, it cannot address more than 4 GiB of
memory. We can save some time by not identity mapping addresses over 4
GiB, as the bootloader cannot address them anyway.

This commit changes the BIOS bootloader to skip addresses over 4 GiB
when determining the maximum physical address.

This is one of the changes described in issue rust-osdev#259.
Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Thank you!

src/bin/bios.rs Outdated Show resolved Hide resolved
@hawkw
Copy link
Contributor Author

hawkw commented Sep 22, 2022

Incidentally, it looks like just this change is sufficient to significantly improve boot times when running in QEMU v7.1.x. On my machine, running the whole test suite takes in 1m20s on this branch (including compiling everything), with the default_settings tests running in 12s. On the main branch, running the whole test suite takes 7m48s, with the default_settings tests take 116s to run.

I'm actually somewhat surprised by this, as I expected that the map_phys_mem tests, at least, would still be slow to boot after this change, since the memory mapped for the kernel when map-physical-memory is enabled isn't bounded by max_phys_addr as determined here. But, surprisingly, they run in 9s on this branch (on my machine):

    Finished release [optimized + debuginfo] target(s) in 1.56s
Created bootable disk image at /home/eliza/Code/bootloader/target/x86_64-map_phys_mem/debug/boot-bios-check_boot_info.img
Created bootable disk image at /home/eliza/Code/bootloader/target/x86_64-map_phys_mem/debug/boot-bios-access_phys_mem.img
test check_boot_info ... ok
test access_phys_mem ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 9.06s

@phil-opp
Copy link
Member

phil-opp commented Sep 22, 2022

Interesting, thanks for testing! I suspect that the performance difference is caused by the TLB flushes. For the identity mapping we flush each entry after creation. For the physical memory mapping, on the other hand, we don't do any flushing because the bootloader never accesses the memory. The TLB will be flushed completely when switching to the kernel address space anyway.

So I think we should be able to improve the performance further by doing a full TLB flush after creating the mappings instead of flushing each new entry separately.

For the full TLB flush we can use https://docs.rs/x86_64/0.14.10/x86_64/instructions/tlb/fn.flush_all.html

@hawkw
Copy link
Contributor Author

hawkw commented Sep 22, 2022

@phil-opp good idea --- should i make that change in this PR or on a separate branch?

@phil-opp
Copy link
Member

However you prefer :).

@hawkw
Copy link
Contributor Author

hawkw commented Sep 22, 2022

Sounds good, I'll make that change separately, then!

hawkw added a commit to hawkw/bootloader that referenced this pull request Sep 23, 2022
This commit changes the BIOS bootloader to only flush the TLB once,
after it has identity-mapped every physical memory frame. This should be
a bit more efficient, as we don't perform a separate `invlpg` for every
page table entry we create, and instead only flush the entire thing by
reloading `CR3` when we're actually ready to use it.

This is based on a suggestion by @phil-opp, here:
rust-osdev#260 (comment)

This change doesn't actually seem to make all that big an impact in boot
times on QEMU v7.1 on its own, relative to PR rust-osdev#260, but it might make an
additional improvement on top of that PR.
@phil-opp phil-opp merged commit 167eb99 into rust-osdev:main Sep 25, 2022
@phil-opp
Copy link
Member

Published together with #265 and #263 as v0.10.13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants