On Mon, 19 Sept 2022 at 13:15, Huacai Chen <chenhuacai@xxxxxxxxxx> wrote: > > On Mon, Sep 19, 2022 at 6:49 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > On Mon, 19 Sept 2022 at 12:47, Huacai Chen <chenhuacai@xxxxxxxxxx> wrote: > > > > > > On Mon, Sep 19, 2022 at 6:37 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > > > > > On Mon, 19 Sept 2022 at 12:33, Huacai Chen <chenhuacai@xxxxxxxxxx> wrote: > > > > > > > > > > Hi, Ard, > > > > > > > > > > On Mon, Sep 19, 2022 at 2:22 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > > > > > > > > > On Mon, 19 Sept 2022 at 08:06, Huacai Chen <chenhuacai@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > Hi, Ard, > > > > > > > > > > > > > > On Mon, Sep 19, 2022 at 1:15 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > > > On Mon, 19 Sept 2022 at 03:58, Huacai Chen <chenhuacai@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > > > > > Hi, Ard, > > > > > > > > > > > > > > > > > > I think the parameters passed to the core kernel need to be discussed. > > > > > > > > > The old way (so-called old world): > > > > > > > > > a0=argc, a1=argv, a1=bpi > > > > > > > > > > > > > > > > > > The current way (so-called new world): > > > > > > > > > a0=efi boot flag, a1=fdt pointer > > > > > > > > > > > > > > > > > > The new way (in this patchset): > > > > > > > > > a0=efi boot flag, a1=systemtable, a2=cmdline > > > > > > > > > > > > > > > > > > I prefer to use the current way, there are 3 reasons: > > > > > > > > > 1, both acpi system and dt system can use the same parameters passing method; > > > > > > > > > > > > > > > > DT systems will use this too. The distinction is between EFI boot and > > > > > > > > non-EFI boot. We *really* should keep these separate, given the > > > > > > > > experience on ARM, where other projects invent ways to pass those > > > > > > > > values to the kernel without going through the stub. > > > > > > > In the last patch I see: > > > > > > > + void *fdt_ptr = early_memremap_ro(fw_arg1, SZ_64K); > > > > > > > + > > > > > > > + early_init_dt_scan(fdt_ptr); > > > > > > > + early_init_fdt_reserve_self(); > > > > > > > + > > > > > > > clear_bit(EFI_BOOT, &efi.flags); > > > > > > > So I suppose for the DT system that means a0=efi boot flag, a1=fdt > > > > > > > pointer, a2=cmdline? Then it is not exactly the same as the ACPI > > > > > > > system, but similar. > > > > > > > > > > > > > > > > > > > No, for non-EFI DT boot, the command line is passed via the DT, so > > > > > > a0=0x0 (non-efi), a1=DT, a2=0x0 > > > > > > > > > > > > Do you intend to support non-EFI DT boot by the way? > > > > > I think we needn't support non-EFI DT boot, so a0=efi boot flag, > > > > > a1=systemtable, a2=cmdline is just OK (or maybe we can exchange a1/a2, > > > > > which looks similar to the old-world). > > > > > > > > Excellent. If non-EFI boot is not supported, we can drop the code that > > > > deals with that. > > > > > > > > > But I have another question: is > > > > > it early enough to get DT from systemtable for DT boot (in the current > > > > > way DT is the earliest thing)? > > > > > > > > > > > > > If you rely on DT only for the hardware description, then loading it > > > > from efi_init() should be fine. > > > OK, then please drop patch #12 at this time. It can be added when we > > > add Loongson-2K support. > > > > > > > OK > > > > > > > > I don't think it makes sense to allow efi=novamap on LoongArch, given > > > > > > that we cannot make use of the runtime services that way. So in my > > > > > > code, efi_novamap is set to false unconditionally. > > > > > Emm, I prefer to support "efi=novamap", the core kernel has already > > > > > supported "noefi" to disable runtime, I don't want to hack > > > > > efi_novamap. So please keep the efi boot flag, thanks. > > > > > > > > > > > > > Fair enough. Do you have any uses for efi_novamap in mind? > > > I usually use "efi=novamap" in EFI shell to see whether it can work > > > well without runtime. And I think I will modify this patch these days > > > because in another thread you said that this series cannot boot. > > > > It works fine now. > > > > However, clearing the EFI_BOOT flag is not the correct way to disable > > runtime services only. And note that we also have efi=noruntime - does > > that currently work on LoongArch? > OK, but we don't need to add a new "efi=noruntime" parameter, I can > use "noefi" instead. OK > But I think support "efi=novamap" is not a bad > idea (maybe I misunderstood its usage). > It basically exists to deal with broken EFI firmware on ARM laptops that were only intended to run Windows. Windows calls SetVirtualAddressMap() *after* creating the new mappings, and some broken firmware drivers will access those regions too early. On Linux, we install the mapping first, and then much later, we actually create the mappings and only activate them on a single CPU while the EFI runtime service call is in progress. In any case, I will reinstate the efi_novamap logic for now - we can always revisit it later.
