-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
OpenNetworkBoot: Add PXE and HTTP(S) Boot support #549
Conversation
[LibraryClasses] | ||
NULL|OpenCorePkg/Library/OcCompilerIntrinsicsLib/OcCompilerIntrinsicsLib.inf | ||
NULL|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf |
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.
cc @savvamitrofanov @vit9696 - I believe it is important to support HTTPS boot as well as HTTP boot, therefore we need to also ship TlsDxe. However this depends on CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
. We cannot just specify that library in the !if $(NETWORK_TLS_ENABLE) == TRUE
code block further up in this file, because it conflicts with our OcCompilerIntrinsicsLib (they define some of the same symbols, not surprisingly), and our intrinsics library was set to be added to everything by this line.
I have currently changed to using the CryptoPkg intrinsics library for everything (currently, including OpenDuet), instead. All seems to work fine, AFAICT, but might possibly be running slower than necessary for the rest of the code (e.g. see here) (though it makes no visible change that I've noticed). Or might be other negatives I am not aware of.
We could try to update our intrinsics library to add the couple of functions which are missing for it to link with CryptoPkg (namely __bzero
and __udivdi3
, which show as missing for TlsDxe on the 32-bit build), but suspect this may be a bad idea (for similar reasons, i.e. is CryptoPkg intrinsics lib addressing e.g. any constant time security concerns which we do not otherwise need to be concerned with? (I don't think so?) Or is at anyway addressing compilation concerns which are important (as above), and which our intrinsics library doesn't otherwise need to address? I think so.).
We could instead change to adding our intrinsics library manually wherever it is actually needed, rather than globally; so that CryptoPkg intrinsics library can be used just in CryptoPkg without conflicting with ours.
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.
As discussed, will try modifying so that everything uses more or less our intrinsics lib, however updated with missing required functions, and moved as core library to MdePkg in audk.
@vit9696 - I have not (yet) implemented what we discussed, about loading a DMG file directly into an OC-format DMG ramdisk. I think this could be done by adding a protocol of ours, which would be understood by a new custom OpenHttpBootDxe. Currently a DMG is loaded into contiguous memory by HttpBootDxe.efi, then transferred from there to a not-necessarily-contiguous OC DMG ramdisk. However, and as you suggested, reading directly to a not-necessarily-contiguous ramdisk of the right size will work faster, and on machines with considerably less RAM. This would be something to be added later. |
75ca794
to
61f26b9
Compare
Will need to update to take account of acidanthera/bugtracker#2421 |
751bea5
to
3f66bb7
Compare
Done and committed here. See also acidanthera/audk#66 . |
@@ -160,7 +160,7 @@ | |||
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf | |||
UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf | |||
UefiDriverEntryPoint|OpenCorePkg/Library/OcDriverEntryPoint/UefiDriverEntryPoint.inf | |||
UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf | |||
UefiHiiServicesLib|OpenCorePkg/Library/OcHiiServicesLib/OcHiiServicesLib.inf |
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.
Suspect this is another change which makes sense anyway. Resolves same issue as in 13a1c62 - namely that the EDK 2 HiiServicesLib asserts when officially optional protocols are not present.
bb608d9
to
f73dfc7
Compare
Resolved various dependencies (HiiDatabase; plus another new edk2-stable202405 requirement, on Hash2 protocol) which were stopping HTTP Boot working on MacPro5,1. So now gets much further, and visibly starts HTTP Boot, but still won't actually load anything. Returns eventually, so not sure why not. Not sure if this will be easy to resolve or not. Also not quite sure whether to make OpenNetworkBoot Staging or Platform? Feel like it's useful (if you want network boot) and stable on non-Apple hardware, but a shame it won't currently work on Apple. |
666f952
to
d699561
Compare
72cbd76
to
1eefd37
Compare
23521af
to
0df44d0
Compare
0df44d0
to
a447667
Compare
Also as discussed, I will convert the various separable supporting changes here to separate PRs. |
Addresses acidanthera/bugtracker#2386.
Supports launching PXE and HTTP(S) boot options from the OpenCore menu if these are supported by the underlying firmware, or by additional drivers loaded by OpenCore. Additionally supports loading
.dmg
files and their associated.chunklist
files over HTTP(S) Boot, allowing macOS Recovery to be started over HTTP(S) Boot.This includes some changes which could be separate commits:
UEFI/Unload
config option to unload existing firmware drivers - because it was necessary in a few cases to unload an existing firmware driver before loading the standard EDK 2 one provided with OC (since e.g. vendor HttpBootDxe makes ours think it is already started, but vendor driver is trying to use vendor UI which we do not have, therefore causes only non-working (blank screen) HTTP Boot options; and/or can be that vendor HttpDxe is locked to https:// but user wants to use http://)