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.

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

Fix different behaviour between usb and cd in efi #135

Merged
merged 5 commits into from
Dec 11, 2024
Merged

Conversation

Itxaka
Copy link
Member

@Itxaka Itxaka commented Dec 9, 2024

Seems that the actual issue was not that the USB is not recognized, but the usb boots and it gets a grub shell because it cannot load the default grub.cfg for efi.

This seems to be a side issue of how efi booting works differently from livecd or usb in grub.
Looks like when booting from livecd, the efi artifacts are loaded from the efi image file BUT the livecd is mounted as root for grub.
When booting from usb it seems like the efi artifacts are loaded from the efi image AND the efi image is mounted as root for grub.

In both cases, grub is hardcoded to search for the grub config in /EFI/boot/grub.cfg (so alongside the grubx64.efi file)
When we build the ISO we only copy those artifacts on the ISO root, so you got the /EFI/boot/grub.cfg directly accessible when booting from livecd.
But when booting from usb the livecd is ignored and only the contents of the uefi.img are in the root, so /EFI/boot/grub.cfg does not exists.

This is fixed by copying it to both places so it works on both boot methods.

this supposedly increases compatibility when booting from usb

Mark the current El Torito boot image (see options -b and -e) in an actually invalid GPT as partition of type Basic Data. This works only with -isohybrid-mbr and has the same impact on the system area as -efi-boot-part. It cannot be combined with -efi-boot-part or -hfsplus.
The first three boot images which are marked by GPT will also show up as partition entries in MBR. The MBR partition of type 0xEF is what actually is used by EFI firmware for booting from USB stick. The MBR partition for PC-BIOS gets type 0x00 rather than 0x17 in this case. Often the further MBR entries are the ones which actually get used by EFI.

Signed-off-by: Itxaka <itxaka@kairos.io>
@Itxaka Itxaka requested review from a team and Copilot December 9, 2024 11:37

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no suggestions.

Copy link

codecov bot commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.48%. Comparing base (ea77e08) to head (7c17ecb).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #135   +/-   ##
=======================================
  Coverage   15.48%   15.48%           
=======================================
  Files          10       10           
  Lines        1614     1614           
=======================================
  Hits          250      250           
  Misses       1348     1348           
  Partials       16       16           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Itxaka <itxaka@kairos.io>
@Itxaka Itxaka force-pushed the efi_compatibility branch from 9d35aa2 to 2b40dfe Compare December 9, 2024 12:25
@Itxaka Itxaka changed the title Use the efi image directly and set efi_boot_part Fix different behaviour between usb and cd in efi Dec 9, 2024
"-boot_image", "any", "platform_id=0xef",
"-boot_image", "any", "emul_type=no_emulation",
"-boot_image", "any", "efi_path=" + IsoEFIPath, // Tell where the efi path is
Copy link
Member Author

Choose a reason for hiding this comment

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

this is just an improvement over what we had.
Before:

  • uefi.img in the disc
  • second partition in the disc with uefi contents

Now:

  • just the uefi.img in the disc

pkg/ops/iso.go Outdated Show resolved Hide resolved
pkg/ops/iso.go Outdated Show resolved Hide resolved
Signed-off-by: Itxaka <itxaka@kairos.io>
Copy link
Member

@mauromorales mauromorales left a comment

Choose a reason for hiding this comment

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

I mean more like with the current comments I added, because it encapsulates the logic of the "hard coded" ubuntu logic and also allows commenting why it is needed in a single place.

pkg/ops/iso.go Outdated Show resolved Hide resolved
pkg/ops/iso.go Outdated Show resolved Hide resolved
pkg/ops/iso.go Outdated Show resolved Hide resolved
pkg/ops/iso.go Outdated Show resolved Hide resolved
pkg/ops/iso.go Outdated Show resolved Hide resolved
Co-authored-by: Mauro Morales <mauro.morales@spectrocloud.com>
pkg/ops/iso.go Outdated Show resolved Hide resolved
pkg/ops/iso.go Outdated Show resolved Hide resolved
Signed-off-by: Itxaka <itxaka@kairos.io>
@Itxaka Itxaka enabled auto-merge (squash) December 11, 2024 13:40
@Itxaka Itxaka merged commit c0d7450 into main Dec 11, 2024
10 checks passed
@Itxaka Itxaka deleted the efi_compatibility branch December 11, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[no-UKI] Make ISO images hybrid and with GPT to improve compatability
2 participants