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

panic on large memory regions when map-physical-memory is enabled #262

Closed
hawkw opened this issue Sep 22, 2022 · 7 comments · Fixed by #263
Closed

panic on large memory regions when map-physical-memory is enabled #262

hawkw opened this issue Sep 22, 2022 · 7 comments · Fixed by #263

Comments

@hawkw
Copy link
Contributor

hawkw commented Sep 22, 2022

Trying to test out a higher half kernel using PR #229, the bootloader panics when the map-physical-memory, map-framebuffer, and dynamic-range-start configurations are all present.

I have the following configurations in my Cargo.toml:

[package.metadata.bootloader]
map-physical-memory = true
# the kernel is mapped into the higher half of the virtual address space.
dynamic-range-start = "0xFFFF_8000_0000_0000"

[patch.crates-io]
# patch `bootloader` to use https://github.com/rust-osdev/bootloader/pull/229
# for dynamic range address configuration support.
#
# remove this patch once rust-osdev/bootloader#229 makes it into a release
# version.
bootloader = { git = "https://github.com/rust-osdev/bootloader", rev = "ac46d0455b41c11e5d316348d068df1c495ce0af" }

I get the following panic:

Err(err) => panic!("failed to map page {:?}: {:?}", page, err),

which fails on a ParentEntryHugeFrame error:
image

@hawkw
Copy link
Contributor Author

hawkw commented Sep 22, 2022

When the map-page-table-recursively setting is also true, there's a different panic while setting up the recursive mapping:

panic!(
"Could not set up recursive mapping: index {} already in use",
u16::from(index)
);

image

[package.metadata.bootloader]
map-physical-memory = true
map-page-table-recursively = true
dynamic-range-start = "0xFFFF_8000_0000_0000"

It appears the recursive index has already been used while mapping the dynamic range in this case?

hawkw added a commit to hawkw/mycelium that referenced this issue Sep 22, 2022
@Freax13
Copy link
Member

Freax13 commented Sep 22, 2022

I feel like this is also related to #259. I can reproduce this on qemu 7.1, but not on 7.0.

@Freax13
Copy link
Member

Freax13 commented Sep 22, 2022

let base =
Page::from_page_table_indices_1gib(self.get_free_entry(), PageTableIndex::new(0))
.start_address();

This allocates a level 4 entry which spans 2^39 bytes irregardless of the requested size. QEMU's reported memory regions at 1TB are larger than that (2^40 bytes) which results the physical memory mapping overlapping into other regions.

@Freax13
Copy link
Member

Freax13 commented Sep 22, 2022

dynamic-range-start is not actually required for the crash, only map-physical-memory.

@hawkw
Copy link
Contributor Author

hawkw commented Sep 22, 2022

dynamic-range-start is not actually required for the crash, only map-physical-memory.

Ah, yeah, you're right — it turns out I can reproduce this without the dynamic range configuration, and it's having map-physical-memory enabled that's the culprit. My bad!

This is definitely related to #259, then.

@hawkw hawkw changed the title panic when both map-physical-memory and dynamic-range-start are enabled panic on large memory regions when map-physical-memory is enabled Sep 22, 2022
@hawkw
Copy link
Contributor Author

hawkw commented Sep 22, 2022

I can close this issue as a duplicate of #259 if you like?

@Freax13
Copy link
Member

Freax13 commented Sep 22, 2022

Sure

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 a pull request may close this issue.

2 participants