No UEFI Shell #208
Open
No UEFI Shell #208
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
…RK_ENABLE When trying to build without EFI Shell, a build issue appears saying that PcdAllowHttpConnections is not declared in DEC files referenced in INF files in FDF. It occurred that NetworkPcds.dsc.inc initializes a that PCD, but modules that use this PCD are only built and included into the image if NETWORK_ENABLE is TRUE. As a result a PCD was defined, but not a single module actually existed in the image to use it, i.e. use the NetworkPkg.dec file. This bug lived silently until EFI Shell was removed, because one of the EFI Shell module INF files included NetworkPkg.dec. To solve this, guard the network defines with NETWORK_ENABLE, so that the PCD is only initialized in case where network is actually enabled. Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>
…oot options When updating firmware to a build without UEFI Shell, the settings may be preserved, including the boot options from previous firmware, which results in the non-existent UEFI Shell still being presented as an active boot option. To avoid confusion, check the existence of a file before registering a boot option. Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>
The UefiDevicePathLib from MdePkg is used everywhere, so no need to define the same library three times in different places. Removed redunant entries with UefiDevicePathLib. Also the FileHandleLib seems to be used by QEMU Q35 target and is not compiled when Shell is removed. Move it to common section with libraries. Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>
|
Tested on VP3230 without shell, then with shell to see if the Shell option is created back. Workflows that build all platforms with and without UEFI Shell: https://github.com/Dasharo/coreboot/actions/runs/13203150358 |
| @@ -141,6 +153,8 @@ PlatformRegisterFvBootOption ( | |||
| ASSERT_EFI_ERROR (Status); | |||
| FreePool (DevicePath); | |||
|
|
|||
| // No need to guard here, because an error would be displayed on the screen | |||
There was a problem hiding this comment.
No need to guard here in front of an if is confusing without knowing context of this commit. Would be clearer to mention file existence explicitly.
| Status = EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINTN); | ||
| ASSERT_EFI_ERROR (Status); | ||
| } else if (OptionIndex != -1 && !FileExists) { | ||
| // Option exists but no longer in the image. Remove it from boot option list. | ||
| Status = EfiBootManagerDeleteLoadOptionVariable (BootOptions[OptionIndex].OptionNumber, |
There was a problem hiding this comment.
This makes function for adding an option to remove an already existing one.
I don't really have a better way of implementing it (need some place which can drop all outdated entries and there might be none), but an update to a comment on PlatformRegisterFvBootOption() to warn about this unexpected behaviour seems warranted.
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.
Necessary fixes for no-Shell builds. Details in commit messages.