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

Use Genimage for declarative image layout #3388

Merged
merged 19 commits into from
Jun 7, 2024
Merged

Use Genimage for declarative image layout #3388

merged 19 commits into from
Jun 7, 2024

Conversation

sairon
Copy link
Member

@sairon sairon commented May 22, 2024

No description provided.

@sairon sairon added the build Build and CI related issues label May 22, 2024
Copy link
Member

@agners agners left a comment

Choose a reason for hiding this comment

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

The whole PR looks very good 🤩

There are probably simplification opportunities, especially in the SPL/boot part of things.

But transitioning what we have to genimage first is a good approach. We can then still start to optimize/change things in future steps 👍

Copy link
Member

@agners agners left a comment

Choose a reason for hiding this comment

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

This LGTM, I guess we need to make sure that we check and test boot all boards.

@sairon
Copy link
Member Author

sairon commented May 28, 2024

With changes from pengutronix/genimage#248, we should be able to achieve the same MBR layout as we used previously.

sairon added 8 commits June 5, 2024 09:28
This is what it really is, so just make sure only one "fix" function is
called.
There is no reason to have separate efi and boot sys, since all boards
that use efi also use grub as the loader.
Current implementation is mostly working, the only current problem seems
to be the MBR layout which is not possible to be achieved with current
Genimage capabilities. So far there are three places where this can cause
problems:
 1. The partition resizing service/script can't resize the data
    partition, because the extended partition needs to be resized first.
 2. Partition numbers in mbr.rules do not match.
 3. U-Boot scripts use partition numbers instead of labels.
@sairon
Copy link
Member Author

sairon commented Jun 6, 2024

Okay, I think this is ready for the final review. I ran a test build with modified job definition to compare partition tables and layout of the generated image when an SPL is present. There are couple of differences but IMO none of them should break anything, or they even straighten some things. In summary:

  1. All images now have the "first usable sector" pointing to the first sector of the first partition. This shouldn't be a problem, since the previous one (LBA 34) wasn't 1M-aligned and in the cases where the image also contains an SPL, the sectors with the SPL shouldn't be usable. But in general it has no impact on function anyway.
  2. Images using the hybrid partition table and SPL (ODROID M1, M1S, Green and Tinker) now have the first partition moved by 2k sectors ahead - i.e. there's no 1M gap anymore. This gap was intentionally preserved here but I don't think we really need to do that, and it will just complicate the config, since an arbitrary and calculated offset would have to be added to the boot partition for those boards.
  3. Boards with MBR layout now use the 0x0F extended type instead of the 0x05. The type is hardcoded in Genimage (and on the other hand, sfdisk always creates extended partitions 0x05 even if you specify 0x0F). The difference should be the latter uses the CHS addressing, however any modern systems now use LBA, so this difference is just formal.
  4. MBR layout doesn't have gap of 1M unused space anymore between the end of the extended partition and the next primary one. There is no reason to have it, that space was just wasted, and doing that just for the sake of having the images identical would complicate things again.
  5. Boot partition on all hybrid table boards has changed from MSR to more appropriate ESP. I can't find proper reasoning for using this type (here), it even fails to boot on some historic RPi bootloader and it makes it harder to mount and edit the files in the boot partition.

tl;dr - we don't have binary-identical images but functionally there shouldn't be differences. We still should test all the boards (or at least those that have some specifics) - but I have tried on those I have available and things are looking good.

@sairon sairon marked this pull request as ready for review June 6, 2024 09:25
@sairon sairon requested a review from agners June 6, 2024 09:25
buildroot-external/scripts/rauc.sh Outdated Show resolved Hide resolved
buildroot-external/scripts/hdd-image.sh Outdated Show resolved Hide resolved
Copy link
Member

@agners agners left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks 👍

@sairon sairon merged commit 6135ecd into dev Jun 7, 2024
2 checks passed
@sairon sairon deleted the genimage branch June 7, 2024 11:44
@sairon sairon mentioned this pull request Jun 18, 2024
sairon added a commit that referenced this pull request Jul 29, 2024
…sues

Genimage sets the first usable LBA to the offset of the first partition. While
it shouldn't be an issue in theory, Windows may do some nasty things with the
GPT header afterwards which breaks the Raspberry Pi bootloader, manifesting as

Before purpose of this behavior is clarified in [1], add a downstream patch
that sets the first usable LBA back to 34, which was the value that was used
before migrating to Genimage in #3388. Since changing this value (hopefully)
doesn't have any other consequences, and the images now should be closer to
pre-genimage builds, no more side-effects are expected from this change.

[1] https://www.github.com/pengutronix/genimage/issues/262

Fixes #3437
sairon added a commit that referenced this pull request Jul 30, 2024
…sues (#3497)

Genimage sets the first usable LBA to the offset of the first partition. While
it shouldn't be an issue in theory, Windows may do some nasty things with the
GPT header afterwards which breaks the Raspberry Pi bootloader, manifesting as

Before purpose of this behavior is clarified in [1], add a downstream patch
that sets the first usable LBA back to 34, which was the value that was used
before migrating to Genimage in #3388. Since changing this value (hopefully)
doesn't have any other consequences, and the images now should be closer to
pre-genimage builds, no more side-effects are expected from this change.

[1] https://www.github.com/pengutronix/genimage/issues/262

Fixes #3437
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build and CI related issues cla-signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants