add options that allow hiding entries depending of firmware/architecture#560
add options that allow hiding entries depending of firmware/architecture#560iczelia merged 10 commits intoLimine-Bootloader:trunkfrom
Conversation
IF_FW_TYPE allows to hide menu entries according to the
firmware type (bios/uefi)
if IF_FW_TYPE is present and does not match the firmware
the entry will be hidden
IF_ARCH allows to hide menu entries according to the
processor architecture (x86-64/ia-32/aarch64/...)
if IF_ARCH is present it must contain a space seperated
list of allowed architectures
(ie. IF_ARCH: x86-64 ia-32)
There was a problem hiding this comment.
Thanks for the patch. Unfortunately, by my understanding, this is completely wrong. Three main issues:
- The
IF_FW_TYPEblock inshould_skip_entrycomparescur_entry_protocolinstead ofcur_entry_if_fw_type; the new option is never actually read, and entries without aPROTOCOLkey will crash on the dereference of aNULLpointer. - I don't understand the purpose or even the origin of the wait macro and the val declaration above it.
- You have not documented your changes in CONFIG.md. The architecture detection block also seems to be a verbatim copy-paste of common/lib/config.c:415-434, perhaps a shared helper is in order.
|
thanks for the quick response
sorry for that, should be fixed now
sorry for that, the wait macro was just a breakpoint for debugging
I have added documentation to CONFIG.md now. |
iczelia
left a comment
iczelia
left a comment
There was a problem hiding this comment.
There are other outstanding issues.
| * `if_fw_type` - Hide the entry if the firmware does not match. Valid values | ||
| are: `bios`, `efi` (or `uefi`) | ||
| * `if_arch` - Hide the entry if the current CPU architecture is not in the | ||
| space seperated list of permitted architecutres |
| if (cur_entry_if_fw_type) { | ||
| #if defined (UEFI) | ||
| if (strcmp(cur_entry_if_fw_type, "efi") != 0 | ||
| && strcmp(cur_entry_if_fw_type, "uefi") != 0) { |
There was a problem hiding this comment.
The new option accepts bios/efi/uefi but the existing FW_TYPE produces BIOS/UEFI. Please either accept both (case-insensitive compare) or switch to the canonical spellings.
There was a problem hiding this comment.
using the same value as FW_TYPE makes sense, I just used the bios/efi protocol values (except for the _chainloading values).
should this then too be extracted into a function? (for now I have placed the function current_firmware in misc.h)
| } | ||
| char *cur_entry_if_arch = config_get_value(entry->body, 0, "IF_ARCH"); | ||
| if (cur_entry_if_arch) { | ||
| const char *arch; |
There was a problem hiding this comment.
the deduplicated function can likely take the prototype of static inline const char *current_arch(void) in common/sys/cpu.h.
|
thanks for the help |
2 participants
the new option
if_fw_typecan be set to eitherbiosorefi/uefi.bios: hide the entry, unless booted from biosefi/uefi: hide the entry, unless booted from uefithe new option
if_archis interpreted as a space seperated list of permitted architectures.the entry will be hidden if
${ARCH}is not inside the list.note that for 32-bit executables the 64-bit architecutre and the 32-bit architecture needs to be in the list (i.e.
ia-32 x86-64)this is requested by #541 and #537