plat/kvm: Use AllocatePages() to allocate memory regions in UEFI
#1061
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ Checkpatch passed Beep boop! I ran Unikraft's
|
unikraft-bot
added
area/plat
razvand
requested review from
mschlumpp and
eduardvintila
and removed request for
a team and
craciunoiuc
mschlumpp
reviewed
Oct 6, 2023
mogasergiu
force-pushed
the
smoga/s/allocate_pool/allocate_pages
branch
2 times, most recently
from
d088656
to
32411b4
Compare
mschlumpp
approved these changes
Oct 7, 2023
There was a problem hiding this comment.
I have no idea about UEFI, and wasn't able to get QEMU working with it. I assume that somebody else will test it.
Okay seems to work, but we should really increase the default memory region count. With the default memory region count the image does not boot on QEMU+OVMF.
Reviewed-by: Marco Schlumpp marco@unikraft.io
According to the `UEFI` specifications, `AllocatePool()` is the safest allocation method, however there are two pitfalls: - we don't know the BIOS's memory allocator or the system's memory map, so the resulted allocation start address may be out of the range that would be compatible with a Unikraft configuration that does not have `CONFIG_HAVE_PAGING` - it does not guarantee page-aligned allocations and, even though we may be able to align the length or the end address, the allocation's start address may therefore not be page-aligned Thus, replace such calls to `AllocatePool()` with calls to `AllocatePages()` to ensure page-aligned start addresses. Furthermore, preserve backwards compatibility with Unikraft configurations that do not have `CONFIG_HAVE_PAGING` by limiting the allocation's start address to be below the highest address of the static boot page tables through the help of `bpt_unmap_mrd`. Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
mogasergiu
force-pushed
the
smoga/s/allocate_pool/allocate_pages
branch
from
32411b4
to
6b81bd5
Compare
eduardvintila
approved these changes
Oct 11, 2023
There was a problem hiding this comment.
Looks very good to me. Thanks, @mogasergiu!
Reviewed-by: Eduard Vintilă eduard.vintila47@gmail.com
michpappas
approved these changes
Oct 14, 2023
razvand
pushed a commit
that referenced
this pull request
Oct 14, 2023
According to the `UEFI` specifications, `AllocatePool()` is the safest allocation method, however there are two pitfalls: - we don't know the BIOS's memory allocator or the system's memory map, so the resulted allocation start address may be out of the range that would be compatible with a Unikraft configuration that does not have `CONFIG_HAVE_PAGING` - it does not guarantee page-aligned allocations and, even though we may be able to align the length or the end address, the allocation's start address may therefore not be page-aligned Thus, replace such calls to `AllocatePool()` with calls to `AllocatePages()` to ensure page-aligned start addresses. Furthermore, preserve backwards compatibility with Unikraft configurations that do not have `CONFIG_HAVE_PAGING` by limiting the allocation's start address to be below the highest address of the static boot page tables through the help of `bpt_unmap_mrd`. Signed-off-by: Sergiu Moga <sergiu@unikraft.io> Reviewed-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com> Approved-by: Michalis Pappas <michalis@unikraft.io> GitHub-Closes: #1061
razvand
pushed a commit
that referenced
this pull request
Oct 20, 2023
According to the `UEFI` specifications, `AllocatePool()` is the safest allocation method, however there are two pitfalls: - we don't know the BIOS's memory allocator or the system's memory map, so the resulted allocation start address may be out of the range that would be compatible with a Unikraft configuration that does not have `CONFIG_HAVE_PAGING` - it does not guarantee page-aligned allocations and, even though we may be able to align the length or the end address, the allocation's start address may therefore not be page-aligned Thus, replace such calls to `AllocatePool()` with calls to `AllocatePages()` to ensure page-aligned start addresses. Furthermore, preserve backwards compatibility with Unikraft configurations that do not have `CONFIG_HAVE_PAGING` by limiting the allocation's start address to be below the highest address of the static boot page tables through the help of `bpt_unmap_mrd`. Signed-off-by: Sergiu Moga <sergiu@unikraft.io> Reviewed-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com> Approved-by: Michalis Pappas <michalis@unikraft.io> GitHub-Closes: #1061
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to the
UEFIspecifications,AllocatePool()is the safest allocation method, however there are two pitfalls:CONFIG_HAVE_PAGINGThus, replace such calls to
AllocatePool()with calls toAllocatePages()to ensure page-aligned start addresses. Furthermore, preserve backwards compatibility with Unikraft configurations that do not haveCONFIG_HAVE_PAGINGby limiting the allocation's start address to be below the highest address of the static boot page tables through the help ofbpt_unmap_mrd.Prerequisite checklist
checkpatch.ukon your commit series before opening this PR;Base target
kvm]Additional configuration
Description of changes