RE: [PATCH] configs: layerscape: Disable the EFI_LOADER feature

["Z.Q. Hou" <zhiqiang.hou@nxp.com>]

Hi Micheal,

> -----Original Message-----
> From: Michael Walle <michael@walle.cc>
> Sent: 2021年7月26日 15:13
> To: Z.Q. Hou <zhiqiang.hou@nxp.com>
> Cc: Tom Rini <trini@konsulko.com>; Heinrich Schuchardt
> <xypron.glpk@gmx.de>; u-boot@lists.denx.de; Priyanka Jain
> <priyanka.jain@nxp.com>
> Subject: Re: [PATCH] configs: layerscape: Disable the EFI_LOADER feature
> 
> Am 2021-07-26 09:01, schrieb Z.Q. Hou:
> > Hi Michael,
> >
> >> -----Original Message-----
> >> From: Michael Walle <michael@walle.cc>
> >> Sent: 2021年7月23日 1:01
> >> To: Tom Rini <trini@konsulko.com>
> >> Cc: Z.Q. Hou <zhiqiang.hou@nxp.com>; Heinrich Schuchardt
> >> <xypron.glpk@gmx.de>; u-boot@lists.denx.de; Priyanka Jain
> >> <priyanka.jain@nxp.com>
> >> Subject: Re: [PATCH] configs: layerscape: Disable the EFI_LOADER
> >> feature
> >>
> >> Am 2021-07-22 17:26, schrieb Tom Rini:
> >> > On Thu, Jul 22, 2021 at 02:25:59PM +0800, Zhiqiang Hou wrote:
> >> >
> >> >> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >> >>
> >> >> The feature BOOTENV_SHARED_EFI is not supported on layerscape
> >> boards,
> >> >> it didn't result kernel boot crash previously since there isn't
> >> >> the efi/boot/"BOOTEFI_NAME" and it skip calling of 'boot_efi_binary'.
> >> >>
> >> >> But since the commit f3866909e350 ("distro_bootcmd: call EFI
> >> >> bootmgr even without having /EFI/boot"), it will cause kernel boot
> >> >> crash as there isn't a valid fdt_addr and it finially uses the
> >> >> device tree blob of U-Boot and further cause errors.
> >> >>
> >> >> As this feature is enabled by default for armv7 and armv8, so
> >> >> disable it explicitly to avoid calling the 'scan_dev_for_efi'.
> >> >
> >> > I'm not thrilled with this.  Why isn't the solution to get and keep
> >> > in sync the device trees, so that the tree U-Boot has is valid for
> >> > the kernel?  I'm also open to discussing f3866909e350 more.  But
> >> > I'm really opposed to disabling EFI_LOADER on modern platforms as
> >> > that will make adoption of U-Boot in device harder I feel.
> >>
> >> I don't know whats going on with the NXP boards, but the sl28 is a
> >> layerscape board it is working pretty well with EFI boot.
> >
> > Do you mean the EFI boot work well on sl28?
> This, for example, I can boot the debian installer out-of-the-box, given that
> the fdtfile variable is set correctly.

Oh, we are talking on different case.

> 
> > Or the EFI boot doesn't break other boot ways?
> >
> > In my case, there are 4 MMC partitions and a boot script with boot
> > images in the 2nd partition, while nothing in the 1st partition. So
> > the expected boot flow is the 'bootcmd_mmc0' scan the 1st partition
> > and find it's not bootable and then the 2nd partition and boot with
> > the script. But actually the 'scan_dev_for_efi' got problem when scan
> > the 1st partition, as the u-boot DTB is used in 'bootefi bootmgr' and
> > result in some error related to the DTB.
> 
> As mentioned in the other mail, I'm not sure why "bootefi bootmgr"
> does something at all, because AFAIK it needs the BootOrder/BootNext
> variables. Heinrich, please correct me if I'm wrong.

I'm not familiar with EFI boot, In this case, the 'scan_dev_for_efi' calls 'run boot_efi_bootmgr' then 'bootefi bootmgr', seems it doesn't check if the needed components exist.
Is the cmd 'scan_dev_for_efi' wrong?

> 
> > Actually, if give a readable but invalid 'fdt_addr' in env, the EFI
> > boot can also be skipped during the scan of the 1st partition.
> > Actually on some Layerscape boards the provided env 'fdt_addr' with a
> > non-readable address, and on other boards a readable 'fdt_addr'. Seems
> > the patch author copy them from somewhere but didn't cause issue that
> > time. But this is just a workaround, the EFI boot should not cause
> > problem during the scan phase when there isn't needed components in
> > one of these partitions.
> 
> What exactly is going wrong? Is linux booting at all? Or does the bootloader
> abort?

Pasted the log below, the direct cause seems the u-boot DTB doesn't have /cpus node.

=> run bootcmd_mmc0 
switch to partitions #0, OK
mmc0 is current device
Scanning mmc 0:1...
libfdt fdt_check_header(): FDT_ERR_BADMAGIC
Scanning disk esdhc@1560000.blk...
Found 5 disks 
No EFI system partition
couldn't find /cpus 
"Synchronous Abort" handler, esr 0x96000006
elr: 0000000082004a6c lr : 0000000082004a30 (reloc)
elr: 00000000fbd25a6c lr : 00000000fbd25a30
x0 : 0000000087f00a88 x1 : 000000001cfbfd5e
x2 : efbeaddeefbeadde x3 : 00000000efbeadde
x4 : 00000000fffffffc x5 : 0000000087f037d2
x6 : 0000000000000a58 x7 : 0000000000000003
x8 : 0000000087f00000 x9 : 0000000000000008
x10: 0000000000000a44 x11: 00000000fbc17c6c
x12: 00000000000009e4 x13: 0000000000000000
x14: 0000000087f00000 x15: 00000000fbc180d8
x16: 00000000fbd742d0 x17: 0000000000000000
x18: 00000000fbc1cdb0 x19: 00000000000009e4
x20: 0000000087f00000 x21: 00000000fbdb3404
x22: 00000000fbdb4a97 x23: 0000000000000018
x24: 00000000fbde5d44 x25: 0000000000000000
x26: 0000000000000000 x27: 0000000000000000
x28: 00000000fbc5ba60 x29: 00000000fbc17d30

Code: a94153f3 a9425bf5 a8c47bfd d65f03c0 (b8617803) 
Resetting CPU ...
 

> 
> And why don't you fix the fdt_addr then? Shouldn't it be unset if there is no
> actual device tree present in a ROM section? (I don't say there isn't another
> underlying problem when you use an invalid fdt_addr).
 
The problem shown in above log is triggered when unset the fdt_addr.
If it not unset, the SError is triggered when to check the magic of the fdt header.

Thanks,
Zhiqiang

> 
> -michael