[U-Boot] [PATCH 0/1] distro_bootcmd: Switch bootefi to use loadaddr by default.

This patch fixes the issue that CONFIG_LOADADDR is not respected when booting via distro_bootcmd and bootefi, and follows from the thread "Thoughts on EFI, CONFIG_LOADADDR and kernel_addr_r". Since there was no reply, I opted for altering the behavior and providing a fallback.
Kristian Amlie (1): distro_bootcmd: Switch bootefi to use loadaddr by default.
include/config_distro_bootcmd.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)

loadaddr is configurable in Kconfig using CONFIG_LOADADDR, while kernel_addr_r is not. Hence, loadaddr is the future. Provide the existing kernel_addr_r as a fallback if loadaddr is not set.
Signed-off-by: Kristian Amlie kristian.amlie@northern.tech --- include/config_distro_bootcmd.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index d672e8e..839afcc 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -129,12 +129,15 @@ "else " \ "bootefi bootmgr ${fdtcontroladdr};" \ "fi;" \ + "if test -z "${loadaddr}"; then " \ + "setenv loadaddr ${kernel_addr_r};" \ + "fi;" \ "load ${devtype} ${devnum}:${distro_bootpart} " \ - "${kernel_addr_r} efi/boot/"BOOTEFI_NAME"; " \ + "${loadaddr} efi/boot/"BOOTEFI_NAME"; " \ "if fdt addr ${fdt_addr_r}; then " \ - "bootefi ${kernel_addr_r} ${fdt_addr_r};" \ + "bootefi ${loadaddr} ${fdt_addr_r};" \ "else " \ - "bootefi ${kernel_addr_r} ${fdtcontroladdr};" \ + "bootefi ${loadaddr} ${fdtcontroladdr};" \ "fi\0" \ \ "load_efi_dtb=" \ @@ -277,12 +280,15 @@ "setenv efi_old_arch ${bootp_arch};" \ "setenv bootp_vci " BOOTENV_EFI_PXE_VCI ";" \ "setenv bootp_arch " BOOTENV_EFI_PXE_ARCH ";" \ - "if dhcp ${kernel_addr_r}; then " \ + "if test -z "${loadaddr}"; then " \ + "setenv loadaddr ${kernel_addr_r};" \ + "fi;" \ + "if dhcp ${loadaddr}; then " \ "tftpboot ${fdt_addr_r} dtb/${efi_fdtfile};" \ "if fdt addr ${fdt_addr_r}; then " \ - "bootefi ${kernel_addr_r} ${fdt_addr_r}; " \ + "bootefi ${loadaddr} ${fdt_addr_r}; " \ "else " \ - "bootefi ${kernel_addr_r} ${fdtcontroladdr};" \ + "bootefi ${loadaddr} ${fdtcontroladdr};" \ "fi;" \ "fi;" \ "setenv bootp_vci ${efi_old_vci};" \

Ping. Any objections to this change?

On 06.08.18 13:00, Kristian Amlie wrote:
Ping. Any objections to this change?
I definitely don't want to have the bootefi path behave any different from the other distro boot targets. That would just cause confusion down the road.
Do they (pxe boot, extlinux, etc) make use of loadaddr?
Thanks,
Alex
-- Kristian On 10/07/18 15:29, Kristian Amlie wrote:
loadaddr is configurable in Kconfig using CONFIG_LOADADDR, while kernel_addr_r is not. Hence, loadaddr is the future. Provide the existing kernel_addr_r as a fallback if loadaddr is not set.
Signed-off-by: Kristian Amlie kristian.amlie@northern.tech
include/config_distro_bootcmd.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index d672e8e..839afcc 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -129,12 +129,15 @@ "else " \ "bootefi bootmgr ${fdtcontroladdr};" \ "fi;" \
"if test -z \"${loadaddr}\"; then " \
"setenv loadaddr ${kernel_addr_r};" \
"load ${devtype} ${devnum}:${distro_bootpart} " \"fi;" \
"${kernel_addr_r} efi/boot/"BOOTEFI_NAME"; " \
"if fdt addr ${fdt_addr_r}; then " \"${loadaddr} efi/boot/"BOOTEFI_NAME"; " \
"bootefi ${kernel_addr_r} ${fdt_addr_r};" \
"else " \"bootefi ${loadaddr} ${fdt_addr_r};" \
"bootefi ${kernel_addr_r} ${fdtcontroladdr};" \
"fi\0" \ \ "load_efi_dtb=" \"bootefi ${loadaddr} ${fdtcontroladdr};" \
@@ -277,12 +280,15 @@ "setenv efi_old_arch ${bootp_arch};" \ "setenv bootp_vci " BOOTENV_EFI_PXE_VCI ";" \ "setenv bootp_arch " BOOTENV_EFI_PXE_ARCH ";" \
- "if dhcp ${kernel_addr_r}; then " \
- "if test -z "${loadaddr}"; then " \
"setenv loadaddr ${kernel_addr_r};" \
- "fi;" \
- "if dhcp ${loadaddr}; then " \ "tftpboot ${fdt_addr_r} dtb/${efi_fdtfile};" \ "if fdt addr ${fdt_addr_r}; then " \
"bootefi ${kernel_addr_r} ${fdt_addr_r}; " \
"else " \"bootefi ${loadaddr} ${fdt_addr_r}; " \
"bootefi ${kernel_addr_r} ${fdtcontroladdr};" \
"fi;" \ "fi;" \ "setenv bootp_vci ${efi_old_vci};" \"bootefi ${loadaddr} ${fdtcontroladdr};" \

On 07/08/18 00:06, Alexander Graf wrote:
On 06.08.18 13:00, Kristian Amlie wrote:
Ping. Any objections to this change?
I definitely don't want to have the bootefi path behave any different from the other distro boot targets. That would just cause confusion down the road.
Do they (pxe boot, extlinux, etc) make use of loadaddr?
No, you're right, apparently they use kernel_addr_r AFAICS [1].
So maybe it is better to stay with kernel_addr_r. The problem is that not all boards follow this, and there are various "bootm ${loadaddr}" and "bootz ${loadaddr}" sprinkled across the various board headers, indicating that they use this.
What I want out of this exercise is to make sure that the EFI path in distro_bootcmd will be able to boot an EFI app on any board, regardless of whether it uses kernel_addr_r or loadaddr. Sounds reasonable, right?
Following that it seems natural to standardize on one of the two, but it is not entirely clear from the source code which one it should be. I picked loadaddr because it has a CONFIG_LOADADDR define, unlike kernel_addr_r which is just hardcoded in a lot of places, and thus seemed less "proper". But I'm fine with staying with kernel_addr_r as well.
There are several things I could do from here on:
1. Keep the current patch, but add "loadaddr" to pxe and extlinux variants as well to get consistent behavior.
2. Reverse order of attempts, and try to use kernel_addr_r first, then loadaddr. Possibly this could also include a patch to pxe and extlinux to make them behave the same.
3. Do nothing, and keep kernel_addr_r as the one and only. IMHO this implicitly means that loadaddr should be deprecated and removed, given their overlapping meaning, and boards converted to using kernel_addr_r.
4. Opposite of 3, make loadaddr into the one and only, and convert all boards to that.
Personally I think either option 1 or option 2 is the best, both quite smooth transition paths. Option 3 or 4 are perhaps cleaner solutions in the long term, but are *a lot* more work, and much more likely to cause breakage as well.
What do you think?
[1] http://git.denx.de/?p=u-boot.git;a=blob;f=cmd/pxe.c;h=5609545de575d090b27f48...

On 07.08.18 09:26, Kristian Amlie wrote:
On 07/08/18 00:06, Alexander Graf wrote:
On 06.08.18 13:00, Kristian Amlie wrote:
Ping. Any objections to this change?
I definitely don't want to have the bootefi path behave any different from the other distro boot targets. That would just cause confusion down the road.
Do they (pxe boot, extlinux, etc) make use of loadaddr?
No, you're right, apparently they use kernel_addr_r AFAICS [1].
So maybe it is better to stay with kernel_addr_r. The problem is that not all boards follow this, and there are various "bootm ${loadaddr}" and "bootz ${loadaddr}" sprinkled across the various board headers, indicating that they use this.
Yes, they haven't been converted to distro boot then :). So the path forward for those would be to convert them and slowly deprecate loadaddr.
What I want out of this exercise is to make sure that the EFI path in distro_bootcmd will be able to boot an EFI app on any board, regardless of whether it uses kernel_addr_r or loadaddr. Sounds reasonable, right?
If you enable distro boot for a board, one of the requirement is that kernel_addr_r is set to a reasonable value:
http://git.denx.de/?p=u-boot.git;a=blob;f=doc/README.distro;h=f8e9752a0fa424...
Following that it seems natural to standardize on one of the two, but it is not entirely clear from the source code which one it should be. I picked loadaddr because it has a CONFIG_LOADADDR define, unlike kernel_addr_r which is just hardcoded in a lot of places, and thus seemed less "proper". But I'm fine with staying with kernel_addr_r as well.
I think kernel_addr_r is the defined one for distro boot, yes.
There are several things I could do from here on:
- Keep the current patch, but add "loadaddr" to pxe and extlinux
variants as well to get consistent behavior.
- Reverse order of attempts, and try to use kernel_addr_r first, then
loadaddr. Possibly this could also include a patch to pxe and extlinux to make them behave the same.
- Do nothing, and keep kernel_addr_r as the one and only. IMHO this
implicitly means that loadaddr should be deprecated and removed, given their overlapping meaning, and boards converted to using kernel_addr_r.
I think this is the best option. Just convert boards you care about slowly over to distro boot (which implicitly means they also use kernel_addr_r). For other boards you can not rely on any EFI boot path enumeration anyway, because the boot command doesn't search for EFI binaries ;).
Thanks,
Alex
- Opposite of 3, make loadaddr into the one and only, and convert all
boards to that.
Personally I think either option 1 or option 2 is the best, both quite smooth transition paths. Option 3 or 4 are perhaps cleaner solutions in the long term, but are *a lot* more work, and much more likely to cause breakage as well.
What do you think?
[1] http://git.denx.de/?p=u-boot.git;a=blob;f=cmd/pxe.c;h=5609545de575d090b27f48...

On 08/08/18 01:10, Alexander Graf wrote:
On 07.08.18 09:26, Kristian Amlie wrote:
What I want out of this exercise is to make sure that the EFI path in distro_bootcmd will be able to boot an EFI app on any board, regardless of whether it uses kernel_addr_r or loadaddr. Sounds reasonable, right?
If you enable distro boot for a board, one of the requirement is that kernel_addr_r is set to a reasonable value:
http://git.denx.de/?p=u-boot.git;a=blob;f=doc/README.distro;h=f8e9752a0fa424...
Aha! This link clears up the confusion.
There are several things I could do from here on:
- Keep the current patch, but add "loadaddr" to pxe and extlinux
variants as well to get consistent behavior.
- Reverse order of attempts, and try to use kernel_addr_r first, then
loadaddr. Possibly this could also include a patch to pxe and extlinux to make them behave the same.
- Do nothing, and keep kernel_addr_r as the one and only. IMHO this
implicitly means that loadaddr should be deprecated and removed, given their overlapping meaning, and boards converted to using kernel_addr_r.
I think this is the best option. Just convert boards you care about slowly over to distro boot (which implicitly means they also use kernel_addr_r). For other boards you can not rely on any EFI boot path enumeration anyway, because the boot command doesn't search for EFI binaries ;).
You're right, I had not thought of that. It makes a lot of sense, we will make this part of our recommendation for board porting then.
Thanks!
participants (2)
-
Alexander Graf
-
Kristian Amlie