Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable booting of Slaunch-enabled EFI binaries via chainloader command #28

Merged
merged 84 commits into from
Feb 19, 2025
Merged

Enable booting of Slaunch-enabled EFI binaries via chainloader command #28

merged 84 commits into from
Feb 19, 2025

Conversation

SergiiDmytruk
Copy link
Member

Relevant changes are in the top 2 commits, the rest are more fixes noticed in the process (to be squashed later).

@rossphilipson @SergiiDmytruk
slaunch: Debug command line arg to allow switch between legacy and EF…
235648b 
…I boot

GRUB can either the linux legacy boot command but the default is to use the EFI
linux boot command on EFI capable systems. This debug parameter allows forcing
the use of legacy linux boot.

NOTE: This is not part of the final patch set, just for debugging.

Signed-off-by: Ross Philipson <[email protected]>
miczyg1
miczyg1 requested changes Jan 2, 2025
View reviewed changes
@SergiiDmytruk SergiiDmytruk force-pushed the xen-uefi branch 2 times, most recently from 8b8a8b8 to 35c725e Compare January 3, 2025 18:26
@miczyg1
Copy link

miczyg1 commented Jan 14, 2025

@SergiiDmytruk can we get packages built by CI here too?

@SergiiDmytruk
Copy link
Member Author

@SergiiDmytruk can we get packages built by CI here too?

I'm trying to make CI work at https://github.com/TrenchBoot/grub/commits/xen-efi-ci/.

@SergiiDmytruk SergiiDmytruk force-pushed the xen-uefi branch 2 times, most recently from 54debcb to 6965858 Compare January 15, 2025 14:29
@SergiiDmytruk SergiiDmytruk force-pushed the xen-uefi branch 5 times, most recently from 45b4004 to 8cff0a3 Compare January 25, 2025 23:10
@SergiiDmytruk SergiiDmytruk force-pushed the xen-uefi branch 2 times, most recently from d2bcffb to eea3c7c Compare February 5, 2025 18:49
@macpijan macpijan requested a review from miczyg1 February 18, 2025 17:18
@macpijan
Copy link
Member

@miczyg1 can you please resolve your threads ?

miczyg1
miczyg1 approved these changes Feb 19, 2025
View reviewed changes
Copy link

@miczyg1 miczyg1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my side only the field names are to be hanged. As it is just a minor thing, I already approve the rest.

krystian-hebel
krystian-hebel reviewed Feb 19, 2025
View reviewed changes
Copy link
Member

@krystian-hebel krystian-hebel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from one comment, the changes look good. I'm assuming that most of them will eventually be squashed into earlier commits, and because of that I didn't paid too much attention to commit descriptions and their relative order.

@SergiiDmytruk SergiiDmytruk mentioned this pull request Feb 19, 2025
Merged
@SergiiDmytruk
Copy link
Member Author

I'm assuming that most of them will eventually be squashed into earlier commits, and because of that I didn't paid too much attention to commit descriptions and their relative order.

Right. There are several branches on top of each other and I thought it will be easier to apply fixes and update all of them at some later point to not have unrelated branches now. A couple commits at the top can be squashed/moved right now, I just left them above RC1 tag so far.

@SergiiDmytruk
loader: change mle_mem to mle_entry in grub_slaunch_params
c58ba03 
This avoids the need to keep memory pointed to by mle_mem around.

Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
grub-core/loader/slaunch/x86_efi_linux.c: more pointer conversion fixes
9a763f1 
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
grub-core/loader/slaunch/x86_efi_linux.c: fix grub_efi_free_pages() c…
e5e9073 
…alls

Size wasn't converted to pages.

Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
grub-core/loader/slaunch/skl.c: fix one more format specifier
14c9b9d 
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
loader/slaunch: add missing articles
1d810cc 
Signed-off-by: Sergii Dmytruk <[email protected]>
LorbusChris and others added 22 commits February 19, 2025 21:29
@LorbusChris @SergiiDmytruk
grub.d: Split out boot success reset from menu auto hide script
48deda4 
Also rename fallback and menu auto hide script to be executed
before and after boot success reset script.
In menu auto hide script, rename last_boot_ok var to menu_hide_ok
@jwrdegoede @SergiiDmytruk
grub-set-bootflag: Update comment about running as root through pkexec
caee4fa 
We have stopped using pkexec for grub-set-bootflag, instead it is now
installed suid root, update the comment accordingly.

Signed-off-by: Hans de Goede <[email protected]>
@jwrdegoede @SergiiDmytruk
grub-set-bootflag: Write new env to tmpfile and then rename
8bfa40b 
Make the grubenv writing code in grub-set-bootflag more robust by
writing the modified grubenv to a tmpfile first and then renaming the
tmpfile over the old grubenv (following symlinks).

Signed-off-by: Hans de Goede <[email protected]>
@jwrdegoede @SergiiDmytruk
grub.d: Fix boot_indeterminate getting set on boot_success=0 boot
d4d150b 
The "grub.d: Split out boot success reset from menu auto hide script"
not only moved the code to clear boot_success and boot_indeterminate
but for some reason also mixed in some broken changes to the
boot_indeterminate handling.

The boot_indeterminate var is meant to suppress the boot menu after
a reboot from either a selinux-relabel or offline-updates. These
2 special boot scenarios do not set boot_success since there is no
successfull interaction with the user. Instead they increment
boot_indeterminate, and if it is 1 and only when it is 1, so the
first reboot after a "special" boot we suppress the menu.

To ensure that we do show the menu if we somehow get stuck in a
"special" boot loop where we do special-boots without them
incrementing boot_indeterminate, the code before the
"grub.d: Split out boot success reset from menu auto hide script"
commit would increment boot_indeterminate once when it is 1, so that
even if the "special" boot reboot-loop immediately we would show the
menu on the next boot.

That commit broke this however, because it not only moves the code,
it also changes it from only "incrementing" boot_indeterminate once to
always incrementing it, except when boot_success == 1 (and we reset it).

This broken behavior causes the following problem:

1. Boot a broken kernel, system hangs, power-cycle
2. boot_success now != 1, so we increment boot_indeterminate from 0
   (unset!) to 1. User either simply tries again, or makes some changes
   but the end-result still is a system hang, power-cycle
3. Now boot_indeterminate==1 so we do not show the menu even though the
   previous boot failed -> BAD

This commit fixes this by restoring the behavior of setting
boot_indeterminate to 2 when it was 1 before.

Fixes: "grub.d: Split out boot success reset from menu auto hide script"
Signed-off-by: Hans de Goede <[email protected]>
@martinezjavier @SergiiDmytruk
bootstrap.conf: Force autogen.sh to use python3
7fcf4f0 
The python-unversioned-command package is not installed in the buildroot,
but the bootstrap script expects the python command to be present if one
is not defined. So building the package leads to the following error:

./autogen.sh: line 20: python: command not found

This is harmless since gnulib is included as a source anyways, because the
builders can't download. But still the issue should be fixed by forcing to
use python3 that's the default in Fedora now.

Signed-off-by: Javier Martinez Canillas <[email protected]>
@vathpela @SergiiDmytruk
Fix a missing return in efi-export-env and efi-load-env commands
fe735b4 
Somewhere along the way this got mis-merged to include a return without
a value.  Fix it up.

Signed-off-by: Peter Jones <[email protected]>
@jwrdegoede @SergiiDmytruk
Add systemd integration scripts to make "systemctl reboot --boot-load…
d8e1f3c 
…er-menu=xxx" work with grub

This commit adds a number of scripts / config files to make
"systemctl reboot --boot-loader-menu=xxx" work with grub:

1. /lib/systemd/system/systemd-logind.service.d/10-grub.conf
This sets SYSTEMD_REBOOT_TO_BOOT_LOADER_MENU in the env. for logind,
indicating that the boot-loader which is used supports this feature, see:
https://github.com/systemd/systemd/blob/master/docs/ENVIRONMENT.md

2. /lib/systemd/system/grub-systemd-integration.service
   /lib/systemd/system/reboot.target.wants/grub-systemd-integration.service ->
     ../grub-systemd-integration.service
   /usr/libexec/grub/grub-systemd-integration.sh

The symlink in the .wants dir causes the added service file to be started
by systemd just before rebooting the system.
If /run/systemd/reboot-to-boot-loader-menu exist then the service will run
the grub-systemd-integration.sh script.
This script sets the new menu_show_once_timeout grubenv variable to the
requested timeout in seconds.

3. /etc/grub.d/14_menu_show_once

This new grub-mkconfig snippet adds the necessary code to the generated
grub.conf to honor the new menu_show_once_timeout variable, and to
automatically clear it after consuming it.

Note the service and libexec script use grub-systemd-integration as name
because in the future they may be used to add further integration with
systemctl reboot --foo options, e.g. support for --boot-loader-entry=NAME.

A few notes about upstreaming this patch from the rhboot grub2 fork:
1. I have deliberately put the grub.conf bits for this in a new / separate
   grub-mkconfig snippet generator for easy upstreaming
2. Even though the commit message mentions the .wants symlink for the .service
   I have been unable to come up with a clean way to do this at "make install"
   time, this should be fixed before upstreaming.

Downstream notes:
1. Since make install does not add the .wants symlink, this needs to be done
   in grub2.spec %install
2. This is keeping support for the "old" Fedora specific menu_show_once env
   variable, which has a hardcoded timeout of 60 sec in 12_menu_auto_hide in
   place for now. This can be dropped (eventually) in a follow-up patch once
   GNOME has been converted to use the systemd dbus API equivalent of
   "systemctl reboot --boot-loader-menu=xxx".

Signed-off-by: Hans de Goede <[email protected]>
@jwrdegoede @SergiiDmytruk
systemd-integration.sh: Also set old menu_show_once grubenv var
eaa00df 
Downstream RH / Fedora patch for compatibility with old, not (yet)
regenerated grub.cfg files which miss the menu_show_once_timeout check.
This older grubenv variable leads to a fixed timeout of 60 seconds.

Note that the new menu_show_once_timeout will overrule these 60 seconds
if both are set and the grub.cfg does have the menu_show_once_timeout
check.

Signed-off-by: Hans de Goede <[email protected]>
@rmetrich @SergiiDmytruk
at_keyboard: use set 1 when keyboard is in Translate mode
f9acbb8 
When keyboard controller acts in Translate mode (0x40 mask), then use
set 1 since translation is done.
Otherwise use the mode queried from the controller (usually set 2).

Added "atkeyb" debugging messages in at_keyboard module as well.

Resolves: rhbz#1897587

Tested on:
- Asus N53SN (set 1 used)
- Dell Precision (set 1 used)
- HP Elitebook (set 2 used)
- HP G5430 (set 1 used, keyboard in XT mode!)
- Lenovo P71 & Lenovo T460s (set 2 used)
- QEMU/KVM (set 1 used)

Signed-off-by: Renaud Métrich <[email protected]>
@jhlavac @SergiiDmytruk
grub-install: disable support for EFI platforms
ac7d360 
For each platform, GRUB is shipped as a kernel image and a set of
modules. These files are then used by the grub-install utility to
install GRUB on a specific device. However, in order to support UEFI
Secure Boot, the resulting EFI binary must be signed by a recognized
private key. For this reason, for EFI platforms, most distributions also
ship prebuilt EFI binaries signed by a distribution-specific private
key. In this case, however, the grub-install utility should not be used
because it would overwrite the signed EFI binary.

The current fix is suboptimal because it preserves all EFI-related code.
A better solution could be to modularize the code and provide a
build-time option.

Resolves: rhbz#1737444

Signed-off-by: Jan Hlavac <[email protected]>
[rharwood: drop man page]
@rmetrich @SergiiDmytruk
Introduce function grub_debug_is_enabled(void) returning 1 if 'debug'…
b7a2972 
… is in the environment and not empty

Signed-off-by: Renaud Métrich <[email protected]>
@rmetrich @SergiiDmytruk
Don't clear screen when debugging is enabled
519d175 
Signed-off-by: Renaud Métrich <[email protected]>
[[email protected]: rebase fuzz]
Signed-off-by: Robbie Harwood <[email protected]>
@rmetrich @SergiiDmytruk
Add 'at_keyboard_fallback_set' var to force the set manually
2dd6f7e 
This seems required with HP DL380p Gen 8 systems.
Indeed, with this system, we can see the following sequence:

1. controller is queried to get current configuration (returns 0x30 which is quite standard)
2. controller is queried to get the current keyboard set in used, using code 0xf0 (first part)
3. controller answers with 0xfa which means "ACK" (== ok)
4. then we send "0" to tell "we want to know which set your are supporting"
5. controller answers with 0xfa ("ACK")
6. controller should then give us 1, 2, 3 or 0x43, 0x41, 0x3f, but here it gives us 0xfe which means "NACK"

Since there seems no way to determine the current set, and in fact the
controller expects set2 to be used, we need to rely on an environment
variable.
Everything has been tested on this system: using 0xFE (resend command),
making sure we wait for ACK in the 2 steps "write_mode", etc.

Below is litterature I used to come up with "there is no other
solution":
- https://wiki.osdev.org/%228042%22_PS/2_Controller
- http://www-ug.eecg.toronto.edu/msl/nios_devices/datasheets/PS2%20Keyboard%20Protocol.htm
- http://www.s100computers.com/My%20System%20Pages/MSDOS%20Board/PC%20Keyboard.pdf
@pfsmorigo @SergiiDmytruk
Suppress gettext error message
5ca3398 
Colin Watson's patch from comment #11 on the upstream bug:
https://savannah.gnu.org/bugs/?35880#comment11

Resolves: rhbz#1592124

Signed-off-by: Paulo Flabiano Smorigo <[email protected]>
@martinezjavier @SergiiDmytruk
templates: Check for EFI at runtime instead of config generation time
8aba845 
The 30_uefi-firmware template checks if an OsIndicationsSupported UEFI var
exists and EFI_OS_INDICATIONS_BOOT_TO_FW_UI bit is set, to decide whether
a "fwsetup" menu entry would be added or not to the GRUB menu.

But this has the problem that it will only work if the configuration file
was created on an UEFI machine that supports booting to a firmware UI.

This for example doesn't support creating GRUB config files when executing
on systems that support both UEFI and legacy BIOS booting. Since creating
the config file from legacy BIOS wouldn't allow to access the firmware UI.

To prevent this, make the template to unconditionally create the grub.cfg
snippet but check at runtime if was booted through UEFI to decide if this
entry should be added. That way it won't be added when booting with BIOS.

There's no need to check if EFI_OS_INDICATIONS_BOOT_TO_FW_UI bit is set,
since that's already done by the "fwsetup" command when is executed.

Resolves: rhbz#1823864

Signed-off-by: Javier Martinez Canillas <[email protected]>
@frozencemetery @SergiiDmytruk
grub_fs_probe(): dprint errors from filesystems
a921270 
When filesystem detection fails, all that's currently debug-logged is a
series of messages like:

    grub-core/kern/fs.c:56:fs: Detecting ntfs...
    grub-core/kern/fs.c:76:fs: ntfs detection failed.

repeated for each filesystem.  Any messages provided to grub_error() by
the filesystem are lost, and one has to break out gdb to figure out what
went wrong.

With this change, one instead sees:

    grub-core/kern/fs.c:56:fs: Detecting fat...
    grub-core/osdep/hostdisk.c:357:hostdisk: reusing open device
    `/path/to/device'
    grub-core/kern/fs.c:77:fs: error: invalid modification timestamp for /.
    grub-core/kern/fs.c:79:fs: fat detection failed.

in the debug prints.

Signed-off-by: Robbie Harwood <[email protected]>
(cherry picked from commit 838c79d658797d0662ee7f9e033e38ee88059e02)
@submachine @SergiiDmytruk
core: Fix several implicit function declarations
c93cdfe 
These #include lines ensure that grub2 continues to build with C99
where implicit function declarations are removed.

Related to:

  <https://fedoraproject.org/wiki/Changes/PortingToModernC>
  <https://fedoraproject.org/wiki/Toolchain/PortingToModernC>
@vathpela @SergiiDmytruk
Fix missing #include in ofdisk.c
ca364a3 
Recently we started building with -Werror=implicit-function-declaration,
and discovered that ofdisk.c is missing an include to declare
grub_env_get().

This patch adds that #include.

Signed-off-by: Peter Jones <[email protected]>
@marta-lewandowska @SergiiDmytruk
add flag to only search root dev
d535d80 
fixes bz#2223437

Signed-off-by: Marta Lewandowska <[email protected]>
@nfrayer @SergiiDmytruk
cmd/search: Rework of CVE-2023-4001 fix
12256ab 
The initial fix implemented a new flag that forces the grub cfg
stub to be located on the same disk as grub. This created several
issues such as RAID machines not being able to boot as their
partition names under grub were different from the partition where
grub is located. It also simply means that any machines with the
/boot partition located on a disk other than the one containing grub
won't boot.
This commit denies booting if the grub cfg stub is located on a USB
drive with a duplicated UUID (UUID being the same as the partition
containing the actual grub cfg stub)

Signed-off-by: Nicolas Frayer <[email protected]>
@marmarek @SergiiDmytruk
Hide "os-prober disabled" warning
6fe7b54 
It is disabled very much intentional, to not parse arbitrary partition
on the disk. Do not suggest enabling it.

Signed-off-by: Marek Marczykowski-Górecki <[email protected]>
@SergiiDmytruk
QUBESOS: build packages using GitHub Actions
2f477ee 
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk SergiiDmytruk mentioned this pull request Feb 19, 2025
Merged
krystian-hebel
krystian-hebel approved these changes Feb 19, 2025
View reviewed changes
Copy link
Member

@krystian-hebel krystian-hebel left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last commit adds few binary files (archived source code of dependencies), but AFAIU those are just for CI/qubesbuilder and won't be a part of final, hopefully upstreamed, code.

@krystian-hebel krystian-hebel merged commit 2f477ee into tb-dev Feb 19, 2025
1 check passed
@krystian-hebel krystian-hebel deleted the xen-uefi branch February 19, 2025 20:35
@macpijan macpijan mentioned this pull request Feb 19, 2025
Closed
@SergiiDmytruk
Copy link
Member Author

Last commit adds few binary files (archived source code of dependencies), but AFAIU those are just for CI/qubesbuilder and won't be a part of final, hopefully upstreamed, code.

Yes, maybe there is a prettier way of doing it (e.g., downloading files during CI build), but that would need investigation and after finally making CI work for the latest GRUB I didn't return to this.

@SergiiDmytruk SergiiDmytruk mentioned this pull request Feb 20, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miczyg1 miczyg1 miczyg1 approved these changes

@krystian-hebel krystian-hebel krystian-hebel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.