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

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

> -----Original Message-----
> From: Tom Rini <trini@konsulko.com>
> Sent: 2021年7月27日 21:08
> To: Z.Q. Hou <zhiqiang.hou@nxp.com>
> Cc: Michael Walle <michael@walle.cc>; 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
> 
> On Tue, Jul 27, 2021 at 05:42:51AM +0000, Z.Q. Hou wrote:
> > Hi Tom,
> >
> > > -----Original Message-----
> > > From: Tom Rini <trini@konsulko.com>
> > > Sent: 2021年7月26日 20:29
> > > To: Z.Q. Hou <zhiqiang.hou@nxp.com>
> > > Cc: Michael Walle <michael@walle.cc>; 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
> > >
> > > On Mon, Jul 26, 2021 at 07:37:53AM +0000, Z.Q. Hou wrote:
> > > > 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?
> > >
> > > I'll let Heinrich comment on this part.
> > >
> > > > > > 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.
> > >
> > > OK, so that shows a problem to fix.  If there's not a valid device
> > > tree found, that error needs to be handled and not ignored.
> >
> > Drop this patch if the problem can be fix.
> 
> Yes, it certainly seems like this should be fixed?  Are you going to
> investigate?
 
No plan to dig deep.

Thanks,
Zhiqiang

> 
> --
> Tom