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 installer to work with the OnLogic FR201 device #4543

Merged
merged 2 commits into from
Jan 25, 2025

Conversation

rene
Copy link
Contributor

@rene rene commented Jan 23, 2025

Overview

This PR provides two changes in the installer script: one small fix and an additional functional to make installer work with the OnLogic FR201 device.

Commits

installer: Copy bootloader files from the installer device EFI partition

The installer script (within the installer container) look for EFI files under /parts/boot. Currently this directory it's not being populated at all, so the installer script will just create a link for the files from u-boot container (mounted at /uboot/boot). For default installations there is no issues on this behavior. However, for the OnLogic FR201 device, the workflow to produce an installer image is the following:

  1. Create the installer raw image
  2. Flash the installer image to an USB Stick
  3. Change the config.txt file of the EFI partition in the USB Stick to enable the options for the FR201 device
  4. Boot the USB Stick into the device

For this particular case, the config.txt from the EFI partition of the USB Stick (installer device) must be copied to the destination EFI partition of the target storage device (eMMC or SSD). However, the default config.txt (from the u-boot container) is copied and the device is not able to boot.

This commits adds a function to search for the EFI partition and copy the files to /parts/boot, so the installer script will install these files instead of the default ones from u-boot container. In this way, changes in the config.txt of the installer USB Stick will reflect in the target device.

installer: Fix directory checking

Fix the test command argument to check for directories -d instead of regular file (-e).

Fix the test command argument to check for directories '-d' instead of
regular file ('-e').

Signed-off-by: Renê de Souza Pinto <rene@renesp.com.br>
@rene rene requested review from deitch and OhmSpectator January 23, 2025 19:47
@rene rene requested a review from eriknordmark as a code owner January 23, 2025 19:47
The installer script (within the installer container) look for EFI files
under /parts/boot. Currently this directory it's not being populated at
all, so the installer script will just create a link for the files from
u-boot container (mounted at /uboot/boot). For default installations there
is no issues on this behavior. However, for the OnLogic FR201 device, the
workflow to produce an installer image is the following:

1. Create the installer raw image
2. Flash the installer image to an USB Stick
3. Change the config.txt file of the EFI partition in the USB Stick to enable
   the options for the FR201 device
4. Boot the USB Stick into the device

For this particular case, the config.txt from the EFI partition of the USB
Stick (installer device) must be copied to the destination EFI partition of
the target storage device (eMMC or SSD). However, the default config.txt
(from the u-boot container) is copied and the device is not able to boot.

This commits adds a function to search for the EFI partition and copy the
files to /parts/boot, so the installer script will install these files
instead of the default ones from u-boot container. In this way, changes in
the config.txt of the installer USB Stick will reflect in the target
device.

Signed-off-by: Renê de Souza Pinto <rene@renesp.com.br>
@deitch
Copy link
Contributor

deitch commented Jan 23, 2025

I keep trying to understand this and failing. Probably because the installer has some confusing logic. We cleaned some of it during the major installer rewrite, but not all of it.

This isn't specifically about FR201, but a generic process that seems to say, when running an installer, if I already have an EFI partition, mount it, and copy everything in it to my target BOOT_DIR (/parts/boot). If not, just link it to /uboot/boot.

I don't understand what that does, and how it changes the flow. Doesn't that mean that we now are using the EFI files already on there? Or was the purpose of /uboot just in case nothing is there?

@rene
Copy link
Contributor Author

rene commented Jan 24, 2025

I keep trying to understand this and failing. Probably because the installer has some confusing logic. We cleaned some of it during the major installer rewrite, but not all of it.

This isn't specifically about FR201, but a generic process that seems to say, when running an installer, if I already have an EFI partition, mount it, and copy everything in it to my target BOOT_DIR (/parts/boot). If not, just link it to /uboot/boot.

I don't understand what that does, and how it changes the flow. Doesn't that mean that we now are using the EFI files already on there? Or was the purpose of /uboot just in case nothing is there?

@deitch , what I recall (please, notice that maybe I can't be 100% correct) is that for the former installer, all the files were populated into /parts directory. When you rewrote the installer, the container didn't have access to these files anymore, so that's why the files were taken from u-boot container, see this PR #4293.

This wasn't a problem so far, because for generic arm64 the EFI partition of the final image would just contain the same files provided by the u-boot container, so it didn't matter from where we were taking these files. However, with the introduction of the FR201 device support, now we use the same EFI files, but with a an additional step:

EFI files will be exact the same for RPi 4B and for the FR201. However, we need additional configuration that must be enabled in the config.txt for the FR201. For live images that's not a big deal, users flashes the image into an SD-Card, USB, etc, just mount the EFI partition and edit the config.txt. Unfortunately, this cannot be easily done when you install EVE into the internal storage, like eMMC or SSD device. So the installer must take the files from the EFI partition of the installer device, in this way it can get the "correct" config.txt for the FR201 device. For any other device, it doesn't change the behavior, because the files will be exact the same from the u-boot container.

TBH, for me "the /parts stuff" is a legacy from the old installer, we can cleanup when convenient, for now, I would like to unblock the installation image for this device....

Copy link
Member

@OhmSpectator OhmSpectator left a comment

Choose a reason for hiding this comment

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

Looks fine...

@@ -336,8 +336,8 @@ ln -s /bits/* ${ARTIFACTS_DIR} 2>/dev/null
# and now a few measures of last resort - make sure the ${ARTIFACTS_DIR}/* we need exist, or else
# symlink to the root filesystem
[ -e ${ROOTFS_IMG} ] || { echo "ERROR: no rootfs.img found"; exit 1; }
[ -e ${EFI_DIR} ] || ln -s /root/EFI ${EFI_DIR}
[ -e ${BOOT_DIR} ] || ln -s /uboot/boot ${BOOT_DIR}
[ -d ${EFI_DIR} ] || ln -s /root/EFI ${EFI_DIR}
Copy link
Member

Choose a reason for hiding this comment

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

So, before the fix, were the links being created regardless of whether the directory already existed? How did that even work then? oO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly @OhmSpectator , in this case it was working because the directories didn't exist...

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

Run tests

@eriknordmark eriknordmark merged commit 7b88861 into lf-edge:master Jan 25, 2025
51 of 63 checks passed
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.

4 participants