Re: [PATCH V6 09/22] LoongArch: Add boot and setup routines

[Ard Biesheuvel]

On Thu, 3 Mar 2022 at 07:26, Huacai Chen <chenhuacai@xxxxxxxxx> wrote:
>
> Hi, Ard & Arnd,
>
> On Wed, Mar 2, 2022 at 5:20 PM Huacai Chen <chenhuacai@xxxxxxxxx> wrote:
> >
> > Hi, Ard,
> >
> > On Wed, Mar 2, 2022 at 4:58 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> > >
> > > On Wed, 2 Mar 2022 at 09:56, Huacai Chen <chenhuacai@xxxxxxxxx> wrote:
> > > >
> > > > Hi, Arnd & Ard,
> > > >
> > > > On Tue, Mar 1, 2022 at 6:19 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
> > > > >
> > > > > On Tue, Mar 1, 2022 at 5:17 AM Huacai Chen <chenhuacai@xxxxxxxxx> wrote:
> > > > > > On Mon, Feb 28, 2022 at 7:35 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> > > > > > > On Mon, 28 Feb 2022 at 12:24, Arnd Bergmann <arnd@xxxxxxxx> wrote:
> > > > > > > > On Mon, Feb 28, 2022 at 11:42 AM Huacai Chen <chenhuacai@xxxxxxxxx> wrote:
> > > > > > > > Can't you just use the UEFI protocol for kernel entry regardless
> > > > > > > > of the bootloader? It seems odd to use a different protocol for loading
> > > > > > > > grub and the kernel, especially if that means you end up having to
> > > > > > > > support both protocols inside of u-boot and grub, in order to chain-load
> > > > > > > > a uefi application like grub.
> > > > > > > >
> > > > > > >
> > > > > > > I think this would make sense. Now that the EFI stub has generic
> > > > > > > support for loading the initrd via a UEFI specific protocol (of which
> > > > > > > u-boot already carries an implementation), booting via UEFI only would
> > > > > > > mean that no Linux boot protocol would need to be defined outside of
> > > > > > > the kernel at all (i.e., where to load the kernel, where to put the
> > > > > > > command line, where to put the initrd, other arch specific rules etc
> > > > > > > etc) UEFI already supports both ACPI and DT boot
> > > > > >
> > > > > > After one night thinking, I agree with Ard that we can use RISCV-style
> > > > > > fdt to support the raw elf kernel at present, and add efistub support
> > > > > > after new UEFI SPEC released.
> > > > >
> > > > > I think that is the opposite of what Ard and I discussed above.
> > > > Hmm, I thought that new UEFI SPEC is a requirement of efistub, maybe I'm wrong?
> > > >
> > > > >
> > > > > > If I'm right, it seems that RISC-V passes a0 (hartid) and a1 (fdt
> > > > > > pointer, which contains cmdline, initrd, etc.) to the raw elf kernel.
> > > > > > And in my opinion, the main drawback of current LoongArch method
> > > > > > (a0=argc a1=argv a2=bootparamsinterface pointer) is it uses a
> > > > > > non-standard method to pass kernel args and initrd. So, can the below
> > > > > > new solution be acceptable?
> > > > > >
> > > > > > a0=bootparamsinterface pointer (the same as a2 in current method)
> > > > > > a1=fdt pointer (contains cmdline, initrd, etc., like RISC-V, I think
> > > > > > this is the standard method)
> > > > >
> > > > > It would seem more logical to me to keep those details as part of the
> > > > > interface between the EFI stub and the kernel, rather than the
> > > > > documented boot interface.
> > > > >
> > > > > You said that there is already grub support using the UEFI
> > > > > loader, so I assume you have a working draft of the boot
> > > > > protocol. Are there still open questions about the interface
> > > > > definition for that preventing you from using it as the only
> > > > > way to enter the kernel from a bootloader?
> > > > Things become simple if we only consider efistub rather than raw elf.
> > > > But there are still some problems:
> > > > 1, We want the first patch series as minimal as possible, efistub
> > > > support will add a lot of code.
> > > > 2, EFISTUB hides the interface between bootloader and raw kernel, but
> > > > the interface does actually exist (efistub itself is also a
> > > > bootloader, though it binds with the raw kernel). In the current
> > > > implementation (a0=argc a1=argv a2=bootparaminterface), we should
> > > > select EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER which is marked as
> > > > deprecated. Is this acceptable? If not, we still need to change the
> > > > bootloader-kernel interface, maybe use the method in my previous
> > > > email?
> > >
> > > Why do you need this?
> > Because in the current implementation (a0=argc a1=argv
> > a2=bootparaminterface), initrd should be passed by cmdline
> > (initrd=xxxx). If without that option, efi_load_initrd_cmdline() will
> > not call handle_cmdline_files().
> It seems I'm wrong. EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER controls
> "initrd=xxxx" from BIOS to EFISTUB, but has nothing to do with
> a0/a1/a2 usage (which controls the "initrd=xxxx" from efistub to raw
> kernel). The real reason is our UEFI BIOS has an old codebase without
> LoadFile2 support.
>

The problem with initrd= is that it can only load the initrd from the
same EFI block device that the kernel was loaded from, which is highly
restrictive, and doesn't work with bootloaders that call LoadImage()
on a kernel image loaded into memory. This is why x86 supports passing
the initrd in memory, and provide the base/size via struct bootparams,
and arm64 supports the same using DT.

The LoadImage2 protocol based method intends to provide a generic
alternative to this, as it uses a pure EFI abstraction, and therefore
does not rely on struct bootparams or DT at all.

So the LoadImage2() based method is preferred, but if your
architecture implements DT support already, there is nothing
preventing you from passing initrd information directly to the kernel
via the /chosen node.

> Then, my new questions are:
> 1, Is EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER an unacceptable option
> for a new Arch? If yes, we should backport LoadFile2 support to our
> BIOS.

See above.

> 2, We now all agree that EFISTUB is the standard and maybe the only
> way in future. But, can we do the efistub work in the second series,
> in order to make the first series as minimal as possible? (I will
> update the commit message to make it clear that a0/a1/a2 usage is only
> an internal interface between efistub and raw kernel).
>

I think it would be better to drop the UEFI and ACPI pieces for now,
and resubmit it once the dust has settled around this.