Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork175
Add uefi::boot module#1255
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
d986905 to172b40dCompareuefi/src/boot.rs Outdated
| ty: AllocateType, | ||
| mem_ty: MemoryType, | ||
| count: usize, | ||
| ) -> Result<PhysicalAddress> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seems like an API inconsistency withallocate_pool, which returnsResult<NonNull<u8>>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That's true. This inconsistency is present in ourBootServices methods and in the UEFI spec;AllocatePool produces avoid* pointer, whileAllocatePages produces aEFI_PHYSICAL_ADDRESS. Notably,EFI_PHYSICAL_ADDRESS is always aUINT64 rather than being pointer sized, so there is a real difference in behavior. The docstring notes this in the context of ia32 PAE (I copied it from theBootServices method, the text was originally added inthis commit).
That said, I think in almost all cases the first thing a caller is going to do with theEFI_PHYSICAL_ADDRESS is cast it to a pointer, and I can't currently think of any real-world counter example. So, while I don't feel very strongly in either direction, I don't mind converting this to returnNonNull<u8> instead ofPhysicalAddress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Updated. For full consistency, I also changed both free functions to useNonNull<u8> as well.
172b40d tof6a2dbdCompareThe initial version contains allocate_pages/allocate_pool and the corresponding`free` functions.Unlike the `BootServices` methods, these functions all use `NonNull<u8>` forconsistency.
64c26a4Uh oh!
There was an error while loading.Please reload this page.
The initial version contains
allocate_pages/allocate_pooland the correspondingfreefunctions.Checklist