Skip to content

Add warning to custom memory types #1526

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

Merged
merged 3 commits into from
Jan 24, 2025
Merged

Conversation

hannahfluch
Copy link
Contributor

@hannahfluch hannahfluch commented Jan 24, 2025

When using custom memory types in an uefi application there is a bug that causes the system to freeze on some machines. I had the same issue and struggled for quite sometime. #1375

Thus, I think it would be helpful to add a warning to the documentation regarding this issue:

For the memory management functions in EFI, an OS is meant to be able to use "memory type" values above 0x80000000 >for its own purposes. In the OVMF EFI firmware release "r11337" (for Qemu, etc) there is a bug where the firmware >assumes the memory type is within the range of values defined for EFI's own use, and uses the memory type as an array >index. The end result is an "array index out of bounds" bug; where the higher memory type values (e.g. perfectly legal >values above 0x80000000) cause the 64-bit version of the firmware to crash (page fault), and cause incorrect "attribute" >values to be reported by the 32-bit version of the firmware. This same bug is also present in whatever version of the EFI >firmware VirtualBox uses (which looks like an older version of OVMF); and I suspect (but don't know) that the bug may be >present in a wide variety of firmware that was derived from the tianocore project (not just OVMF).
Source

It is a small change, but can still save new devs a lot of time & frustration.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

Co-authored-by: Philipp Schuster <[email protected]>
@nicholasbishop
Copy link
Member

We're in the process of relicensing the project and need signoff on the licensing change from all new contributions. Please take a look at #1470 and add your approval if you're OK with it. Thanks!

@nicholasbishop nicholasbishop added this pull request to the merge queue Jan 24, 2025
Merged via the queue into rust-osdev:main with commit f10465a Jan 24, 2025
15 checks passed
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.

3 participants