* [PATCH] LoongArch: Support booting with FDT from efi system table
@ 2022-11-06 6:56 Binbin Zhou
2022-11-06 7:55 ` Xi Ruoyao
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Binbin Zhou @ 2022-11-06 6:56 UTC (permalink / raw)
To: Huacai Chen, WANG Xuerui, Jiaxun Yang, Ard Biesheuvel, Jianmin Lv
Cc: loongarch, Binbin Zhou
Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
flattened DT"), we can parse the FDT from efi system table.
And now, the Loongson-2K series cpus based on LoongArch support booting
with FDT, so we adds the relevant booting support as well as parameter
parsing.
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
arch/loongarch/Kconfig | 2 +
arch/loongarch/include/asm/efi.h | 1 +
arch/loongarch/include/asm/setup.h | 1 +
arch/loongarch/kernel/efi.c | 27 ++++++++++-
arch/loongarch/kernel/env.c | 3 ++
arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
6 files changed, 106 insertions(+), 3 deletions(-)
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 903096bd87f8..ce53d1fa2b64 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -111,6 +111,8 @@ config LOONGARCH
select MODULES_USE_ELF_RELA if MODULES
select NEED_PER_CPU_EMBED_FIRST_CHUNK
select NEED_PER_CPU_PAGE_FIRST_CHUNK
+ select OF
+ select OF_EARLY_FLATTREE
select PCI
select PCI_DOMAINS_GENERIC
select PCI_ECAM if ACPI
diff --git a/arch/loongarch/include/asm/efi.h b/arch/loongarch/include/asm/efi.h
index 174567b00ddb..81e5d3371868 100644
--- a/arch/loongarch/include/asm/efi.h
+++ b/arch/loongarch/include/asm/efi.h
@@ -9,6 +9,7 @@
void __init efi_init(void);
void __init efi_runtime_init(void);
+void __init *efi_fdt_pointer(void);
void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
#define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
diff --git a/arch/loongarch/include/asm/setup.h b/arch/loongarch/include/asm/setup.h
index ca373f8e3c4d..b18baf878ed4 100644
--- a/arch/loongarch/include/asm/setup.h
+++ b/arch/loongarch/include/asm/setup.h
@@ -13,6 +13,7 @@
extern unsigned long eentry;
extern unsigned long tlbrentry;
+extern char original_cmdline[COMMAND_LINE_SIZE];
extern void tlb_init(int cpu);
extern void cpu_cache_init(void);
extern void cache_error_setup(void);
diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
index a31329971133..16dcd673e905 100644
--- a/arch/loongarch/kernel/efi.c
+++ b/arch/loongarch/kernel/efi.c
@@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[] __initdata = {
void __init efi_runtime_init(void)
{
- if (!efi_enabled(EFI_BOOT))
+ if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
return;
if (efi_runtime_disabled()) {
@@ -101,3 +101,28 @@ void __init efi_init(void)
early_memunmap(tbl, sizeof(*tbl));
}
}
+
+/* parse the fdt pointer from efi system table */
+void __init *efi_fdt_pointer(void)
+{
+ int i;
+ void *fdt = NULL;
+ efi_config_table_t *tables;
+
+ if (!efi_systab) {
+ pr_err("Can't find EFI system table.\n");
+ return NULL;
+ }
+
+ tables = (efi_config_table_t *) efi_systab->tables;
+
+ for (i = 0; i < efi_systab->nr_tables; i++)
+ if (efi_guidcmp(tables[i].guid, DEVICE_TREE_GUID) == 0) {
+ fdt = (void *)early_memremap_ro(
+ (resource_size_t)tables[i].table,
+ sizeof(efi_config_table_t));
+ break;
+ }
+
+ return fdt;
+}
diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
index 6d56a463b091..4fd6eb9398b4 100644
--- a/arch/loongarch/kernel/env.c
+++ b/arch/loongarch/kernel/env.c
@@ -11,6 +11,7 @@
#include <asm/early_ioremap.h>
#include <asm/bootinfo.h>
#include <asm/loongson.h>
+#include <asm/setup.h>
u64 efi_system_table;
struct loongson_system_configuration loongson_sysconf;
@@ -27,6 +28,8 @@ void __init init_environ(void)
clear_bit(EFI_BOOT, &efi.flags);
strscpy(boot_command_line, cmdline, COMMAND_LINE_SIZE);
+ /* Save unparsed command line copy for FDT */
+ strscpy(original_cmdline, boot_command_line, COMMAND_LINE_SIZE);
early_memunmap(cmdline, COMMAND_LINE_SIZE);
efi_system_table = fw_arg2;
diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
index 1eb63fa9bc81..2f578f809a54 100644
--- a/arch/loongarch/kernel/setup.c
+++ b/arch/loongarch/kernel/setup.c
@@ -29,6 +29,9 @@
#include <linux/device.h>
#include <linux/dma-map-ops.h>
#include <linux/swiotlb.h>
+#include <linux/of_fdt.h>
+#include <linux/of_address.h>
+#include <linux/libfdt.h>
#include <asm/addrspace.h>
#include <asm/bootinfo.h>
@@ -67,6 +70,7 @@ static const char dmi_empty_string[] = " ";
*
* These are initialized so they are in the .data section
*/
+char original_cmdline[COMMAND_LINE_SIZE] __initdata;
static int num_standard_resources;
static struct resource *standard_resources;
@@ -281,6 +285,47 @@ static void __init check_kernel_sections_mem(void)
}
}
+static void __init bootcmdline_init(char **cmdline_p)
+{
+ /*
+ * If CONFIG_CMDLINE_FORCE is enabled then initializing the command line
+ * is trivial - we simply use the built-in command line unconditionally &
+ * unmodified.
+ */
+ if (IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
+ strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+ return;
+ }
+
+#ifdef CONFIG_OF_FLATTREE
+ /*
+ * This is for CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER.
+ *
+ * If CONFIG_CMDLINE_BOOTLOADER is enabled, we need to determine if the
+ * system is ACPI-based or FDT-based.
+ *
+ * For ACPI-based system: nothing need to do.
+ *
+ * For FDT-based system: the boot_command_line will be overwritten by
+ * early_init_dt_scan_chosen(), so we need to append original_cmdline(the
+ * original copy of boot_command_line) to boot_command_line.
+ *
+ * As for the built-in command line, if CONFIG_CMDLINE_EXTEND is enabled,
+ * both ACPI-based and FDT-based system will append the built-in command
+ * line to the boot_command_line themselves, and no additional processing
+ * is required here.
+ */
+ if (initial_boot_params) {
+ if (boot_command_line[0])
+ strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
+
+ strlcat(boot_command_line, original_cmdline, COMMAND_LINE_SIZE);
+ }
+#endif
+
+ *cmdline_p = boot_command_line;
+}
+
/*
* arch_mem_init - initialize memory management subsystem
*/
@@ -291,6 +336,8 @@ static void __init arch_mem_init(char **cmdline_p)
check_kernel_sections_mem();
+ early_init_fdt_scan_reserved_mem();
+
/*
* In order to reduce the possibility of kernel panic when failed to
* get IO TLB memory under CONFIG_SWIOTLB, it is better to allocate
@@ -413,20 +460,44 @@ static void __init prefill_possible_map(void)
}
#endif
+static void __init fdt_setup(void)
+{
+#ifdef CONFIG_OF_EARLY_FLATTREE
+ void *fdt_pointer;
+
+ /* ACPI-based systems do not require parsing fdt */
+ if (acpi_os_get_root_pointer())
+ return;
+
+ /* Look for a device tree configuration table entry */
+ fdt_pointer = efi_fdt_pointer();
+ if (!fdt_pointer || fdt_check_header(fdt_pointer))
+ return;
+
+ early_init_dt_scan(fdt_pointer);
+ early_init_fdt_reserve_self();
+ early_init_dt_scan_chosen_stdout();
+
+ max_low_pfn = PFN_PHYS(memblock_end_of_DRAM());
+#endif
+}
+
void __init setup_arch(char **cmdline_p)
{
cpu_probe();
- *cmdline_p = boot_command_line;
+ pagetable_init();
init_environ();
efi_init();
+ fdt_setup();
memblock_init();
- pagetable_init();
+ bootcmdline_init(cmdline_p);
parse_early_param();
reserve_initrd_mem();
platform_init();
arch_mem_init(cmdline_p);
+ unflatten_and_copy_device_tree();
resource_init();
#ifdef CONFIG_SMP
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-06 6:56 [PATCH] LoongArch: Support booting with FDT from efi system table Binbin Zhou
@ 2022-11-06 7:55 ` Xi Ruoyao
2022-11-07 4:09 ` Huacai Chen
2022-11-06 9:15 ` WANG Xuerui
2022-11-06 15:06 ` Ard Biesheuvel
2 siblings, 1 reply; 17+ messages in thread
From: Xi Ruoyao @ 2022-11-06 7:55 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, WANG Xuerui, Jiaxun Yang,
Ard Biesheuvel, Jianmin Lv
Cc: loongarch
On Sun, 2022-11-06 at 14:56 +0800, Binbin Zhou wrote:
> Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
> flattened DT"), we can parse the FDT from efi system table.
>
> And now, the Loongson-2K series cpus based on LoongArch support booting
> with FDT, so we adds the relevant booting support as well as parameter
> parsing.
>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
> arch/loongarch/Kconfig | 2 +
> arch/loongarch/include/asm/efi.h | 1 +
> arch/loongarch/include/asm/setup.h | 1 +
> arch/loongarch/kernel/efi.c | 27 ++++++++++-
> arch/loongarch/kernel/env.c | 3 ++
> arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
> 6 files changed, 106 insertions(+), 3 deletions(-)
>
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index 903096bd87f8..ce53d1fa2b64 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -111,6 +111,8 @@ config LOONGARCH
> select MODULES_USE_ELF_RELA if MODULES
> select NEED_PER_CPU_EMBED_FIRST_CHUNK
> select NEED_PER_CPU_PAGE_FIRST_CHUNK
> + select OF
> + select OF_EARLY_FLATTREE
I don't like unconditionally enabling OF or OF_EARLY_FLATTREE because
they are not needed for Loongson-3 workstations or servers. And you are
already fusing the code with #ifdef CONFIG_OF_EARLY_FLATTREE anyway:
/* snip */
> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> index 1eb63fa9bc81..2f578f809a54 100644
> --- a/arch/loongarch/kernel/setup.c
> +++ b/arch/loongarch/kernel/setup.c
/* snip */
> +static void __init fdt_setup(void)
> +{
> +#ifdef CONFIG_OF_EARLY_FLATTREE
--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-06 6:56 [PATCH] LoongArch: Support booting with FDT from efi system table Binbin Zhou
2022-11-06 7:55 ` Xi Ruoyao
@ 2022-11-06 9:15 ` WANG Xuerui
2022-11-06 10:15 ` Xi Ruoyao
2022-11-06 15:06 ` Ard Biesheuvel
2 siblings, 1 reply; 17+ messages in thread
From: WANG Xuerui @ 2022-11-06 9:15 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Jiaxun Yang, Ard Biesheuvel, Jianmin Lv
Cc: loongarch
Hi,
On 2022/11/6 14:56, Binbin Zhou wrote:
> Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
> flattened DT"), we can parse the FDT from efi system table.
>
> And now, the Loongson-2K series cpus based on LoongArch support booting
> with FDT, so we adds the relevant booting support as well as parameter
> parsing.
CPU models or even indeed ISA subsets/levels have nothing to do with
boot protocol support. Theoretically, there's nothing stopping
Loongson-2 or even Loongson-1 models from enjoying full UEFI support,
even if Loongson the company decides not to do so. I can't imagine any
reason for mentioning Loongson-2K aside from company PR which generally
doesn't belong in commit messages...
>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
> arch/loongarch/Kconfig | 2 +
> arch/loongarch/include/asm/efi.h | 1 +
> arch/loongarch/include/asm/setup.h | 1 +
> arch/loongarch/kernel/efi.c | 27 ++++++++++-
> arch/loongarch/kernel/env.c | 3 ++
> arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
> 6 files changed, 106 insertions(+), 3 deletions(-)
>
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index 903096bd87f8..ce53d1fa2b64 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -111,6 +111,8 @@ config LOONGARCH
> select MODULES_USE_ELF_RELA if MODULES
> select NEED_PER_CPU_EMBED_FIRST_CHUNK
> select NEED_PER_CPU_PAGE_FIRST_CHUNK
> + select OF
> + select OF_EARLY_FLATTREE
I agree with Ruoyao that maybe this should not be unconditionally enabled.
You may as well allow the user to select the DT options themselves
without providing another MACH_xxx config, because the Loongson 2K1000LA
and 2K0500 are also capable of at least LA64 baseline, thus still
MACH_LOONGSON64.
> select PCI
> select PCI_DOMAINS_GENERIC
> select PCI_ECAM if ACPI
> diff --git a/arch/loongarch/include/asm/efi.h b/arch/loongarch/include/asm/efi.h
> index 174567b00ddb..81e5d3371868 100644
> --- a/arch/loongarch/include/asm/efi.h
> +++ b/arch/loongarch/include/asm/efi.h
> @@ -9,6 +9,7 @@
>
> void __init efi_init(void);
> void __init efi_runtime_init(void);
> +void __init *efi_fdt_pointer(void);
> void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
>
> #define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
> diff --git a/arch/loongarch/include/asm/setup.h b/arch/loongarch/include/asm/setup.h
> index ca373f8e3c4d..b18baf878ed4 100644
> --- a/arch/loongarch/include/asm/setup.h
> +++ b/arch/loongarch/include/asm/setup.h
> @@ -13,6 +13,7 @@
>
> extern unsigned long eentry;
> extern unsigned long tlbrentry;
> +extern char original_cmdline[COMMAND_LINE_SIZE];
> extern void tlb_init(int cpu);
> extern void cpu_cache_init(void);
> extern void cache_error_setup(void);
> diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
> index a31329971133..16dcd673e905 100644
> --- a/arch/loongarch/kernel/efi.c
> +++ b/arch/loongarch/kernel/efi.c
> @@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[] __initdata = {
>
> void __init efi_runtime_init(void)
> {
> - if (!efi_enabled(EFI_BOOT))
> + if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
> return;
>
> if (efi_runtime_disabled()) {
> @@ -101,3 +101,28 @@ void __init efi_init(void)
> early_memunmap(tbl, sizeof(*tbl));
> }
> }
> +
> +/* parse the fdt pointer from efi system table */
> +void __init *efi_fdt_pointer(void)
> +{
> + int i;
> + void *fdt = NULL;
> + efi_config_table_t *tables;
> +
> + if (!efi_systab) {
> + pr_err("Can't find EFI system table.\n");
> + return NULL;
> + }
> +
> + tables = (efi_config_table_t *) efi_systab->tables;
> +
> + for (i = 0; i < efi_systab->nr_tables; i++)
> + if (efi_guidcmp(tables[i].guid, DEVICE_TREE_GUID) == 0) {
> + fdt = (void *)early_memremap_ro(
> + (resource_size_t)tables[i].table,
> + sizeof(efi_config_table_t));
> + break;
> + }
> +
> + return fdt;
> +}
> diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
> index 6d56a463b091..4fd6eb9398b4 100644
> --- a/arch/loongarch/kernel/env.c
> +++ b/arch/loongarch/kernel/env.c
> @@ -11,6 +11,7 @@
> #include <asm/early_ioremap.h>
> #include <asm/bootinfo.h>
> #include <asm/loongson.h>
> +#include <asm/setup.h>
>
> u64 efi_system_table;
> struct loongson_system_configuration loongson_sysconf;
> @@ -27,6 +28,8 @@ void __init init_environ(void)
> clear_bit(EFI_BOOT, &efi.flags);
>
> strscpy(boot_command_line, cmdline, COMMAND_LINE_SIZE);
> + /* Save unparsed command line copy for FDT */
> + strscpy(original_cmdline, boot_command_line, COMMAND_LINE_SIZE);
> early_memunmap(cmdline, COMMAND_LINE_SIZE);
>
> efi_system_table = fw_arg2;
> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> index 1eb63fa9bc81..2f578f809a54 100644
> --- a/arch/loongarch/kernel/setup.c
> +++ b/arch/loongarch/kernel/setup.c
> @@ -29,6 +29,9 @@
> #include <linux/device.h>
> #include <linux/dma-map-ops.h>
> #include <linux/swiotlb.h>
> +#include <linux/of_fdt.h>
> +#include <linux/of_address.h>
> +#include <linux/libfdt.h>
>
> #include <asm/addrspace.h>
> #include <asm/bootinfo.h>
> @@ -67,6 +70,7 @@ static const char dmi_empty_string[] = " ";
> *
> * These are initialized so they are in the .data section
> */
> +char original_cmdline[COMMAND_LINE_SIZE] __initdata;
>
> static int num_standard_resources;
> static struct resource *standard_resources;
> @@ -281,6 +285,47 @@ static void __init check_kernel_sections_mem(void)
> }
> }
>
> +static void __init bootcmdline_init(char **cmdline_p)
> +{
> + /*
> + * If CONFIG_CMDLINE_FORCE is enabled then initializing the command line
> + * is trivial - we simply use the built-in command line unconditionally &
> + * unmodified.
> + */
> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
> + strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> + return;
> + }
> +
> +#ifdef CONFIG_OF_FLATTREE
> + /*
> + * This is for CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER.
Why mention CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER if
you're guarding the block with CONFIG_OF_FLATTREE?
> + *
> + * If CONFIG_CMDLINE_BOOTLOADER is enabled, we need to determine if the
> + * system is ACPI-based or FDT-based.
> + *
> + * For ACPI-based system: nothing need to do.
nit: "nothing to do".
> + *
> + * For FDT-based system: the boot_command_line will be overwritten by
> + * early_init_dt_scan_chosen(), so we need to append original_cmdline(the
> + * original copy of boot_command_line) to boot_command_line.
> + *
> + * As for the built-in command line, if CONFIG_CMDLINE_EXTEND is enabled,
> + * both ACPI-based and FDT-based system will append the built-in command
> + * line to the boot_command_line themselves, and no additional processing
> + * is required here.
> + */
> + if (initial_boot_params) {
> + if (boot_command_line[0])
> + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> +
> + strlcat(boot_command_line, original_cmdline, COMMAND_LINE_SIZE);
> + }
> +#endif
> +
> + *cmdline_p = boot_command_line;
> +}
> +
> /*
> * arch_mem_init - initialize memory management subsystem
> */
> @@ -291,6 +336,8 @@ static void __init arch_mem_init(char **cmdline_p)
>
> check_kernel_sections_mem();
>
> + early_init_fdt_scan_reserved_mem();
> +
> /*
> * In order to reduce the possibility of kernel panic when failed to
> * get IO TLB memory under CONFIG_SWIOTLB, it is better to allocate
> @@ -413,20 +460,44 @@ static void __init prefill_possible_map(void)
> }
> #endif
>
> +static void __init fdt_setup(void)
> +{
> +#ifdef CONFIG_OF_EARLY_FLATTREE
> + void *fdt_pointer;
> +
> + /* ACPI-based systems do not require parsing fdt */
> + if (acpi_os_get_root_pointer())
> + return;
> +
> + /* Look for a device tree configuration table entry */
> + fdt_pointer = efi_fdt_pointer();
> + if (!fdt_pointer || fdt_check_header(fdt_pointer))
> + return;
> +
> + early_init_dt_scan(fdt_pointer);
> + early_init_fdt_reserve_self();
> + early_init_dt_scan_chosen_stdout();
> +
> + max_low_pfn = PFN_PHYS(memblock_end_of_DRAM());
> +#endif
> +}
> +
> void __init setup_arch(char **cmdline_p)
> {
> cpu_probe();
> - *cmdline_p = boot_command_line;
>
> + pagetable_init();
> init_environ();
> efi_init();
> + fdt_setup();
> memblock_init();
> - pagetable_init();
> + bootcmdline_init(cmdline_p);
> parse_early_param();
> reserve_initrd_mem();
>
> platform_init();
> arch_mem_init(cmdline_p);
> + unflatten_and_copy_device_tree();
>
> resource_init();
> #ifdef CONFIG_SMP
I haven't reviewed as throughly as I'd like to though, and this is in
part because I can't test the changes at all without an open-source
LoongArch DT firmware available for QEMU. (Hint hint)
And, although we're already aware of the part-UEFI and part-FDT usage
pattern, and the arch UEFI code already has consideration for such, it
still makes me a bit wary because you risk losing standards compliance
and creating maintenance burden for admins/users/devs alike by relying
on this not-really-UEFI-but-advertising-as-such thing. I'd still suggest
you try throwing together something fully DT, to at least experiment a
little with. You already have to rewrite pretty much everything with all
the boot protocol changes in the past few months, so this would probably
not even hurt, and the burden on the LoongArch/generic UEFI infra could
be lessened even further if this plays out. And you can always continue
working on this change if it turns out infeasible to do so.
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-06 9:15 ` WANG Xuerui
@ 2022-11-06 10:15 ` Xi Ruoyao
2022-11-07 2:59 ` Binbin Zhou
0 siblings, 1 reply; 17+ messages in thread
From: Xi Ruoyao @ 2022-11-06 10:15 UTC (permalink / raw)
To: WANG Xuerui, Binbin Zhou, Huacai Chen, Jiaxun Yang,
Ard Biesheuvel, Jianmin Lv
Cc: loongarch
On Sun, 2022-11-06 at 17:15 +0800, WANG Xuerui wrote:
> And, although we're already aware of the part-UEFI and part-FDT usage
> pattern, and the arch UEFI code already has consideration for such, it
> still makes me a bit wary because you risk losing standards compliance
> and creating maintenance burden for admins/users/devs alike by relying
> on this not-really-UEFI-but-advertising-as-such thing. I'd still suggest
> you try throwing together something fully DT, to at least experiment a
> little with. You already have to rewrite pretty much everything with all
> the boot protocol changes in the past few months, so this would probably
> not even hurt, and the burden on the LoongArch/generic UEFI infra could
> be lessened even further if this plays out. And you can always continue
> working on this change if it turns out infeasible to do so.
AFAIK using UEFI with FDT (instead of ACPI) is fully allowed and
documented by the standard (section 4.6.1.3 of the UEFI spec). It may
be not possible or not worthy to implement ACPI for 2K systems, then FDT
will be the only choice.
In ARM64 ecosystem, SBBR (for server) demands UEFI+ACPI, EBBR (for
embedded) allows UEFI+ACPI or UEFI+FDT.
--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-06 6:56 [PATCH] LoongArch: Support booting with FDT from efi system table Binbin Zhou
2022-11-06 7:55 ` Xi Ruoyao
2022-11-06 9:15 ` WANG Xuerui
@ 2022-11-06 15:06 ` Ard Biesheuvel
2022-11-07 2:07 ` Jianmin Lv
` (2 more replies)
2 siblings, 3 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2022-11-06 15:06 UTC (permalink / raw)
To: Binbin Zhou; +Cc: Huacai Chen, WANG Xuerui, Jiaxun Yang, Jianmin Lv, loongarch
On Sun, 6 Nov 2022 at 07:58, Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>
> Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
> flattened DT"), we can parse the FDT from efi system table.
>
> And now, the Loongson-2K series cpus based on LoongArch support booting
> with FDT, so we adds the relevant booting support as well as parameter
> parsing.
>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
> arch/loongarch/Kconfig | 2 +
> arch/loongarch/include/asm/efi.h | 1 +
> arch/loongarch/include/asm/setup.h | 1 +
> arch/loongarch/kernel/efi.c | 27 ++++++++++-
> arch/loongarch/kernel/env.c | 3 ++
> arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
> 6 files changed, 106 insertions(+), 3 deletions(-)
>
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index 903096bd87f8..ce53d1fa2b64 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -111,6 +111,8 @@ config LOONGARCH
> select MODULES_USE_ELF_RELA if MODULES
> select NEED_PER_CPU_EMBED_FIRST_CHUNK
> select NEED_PER_CPU_PAGE_FIRST_CHUNK
> + select OF
> + select OF_EARLY_FLATTREE
> select PCI
> select PCI_DOMAINS_GENERIC
> select PCI_ECAM if ACPI
> diff --git a/arch/loongarch/include/asm/efi.h b/arch/loongarch/include/asm/efi.h
> index 174567b00ddb..81e5d3371868 100644
> --- a/arch/loongarch/include/asm/efi.h
> +++ b/arch/loongarch/include/asm/efi.h
> @@ -9,6 +9,7 @@
>
> void __init efi_init(void);
> void __init efi_runtime_init(void);
> +void __init *efi_fdt_pointer(void);
> void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
>
> #define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
> diff --git a/arch/loongarch/include/asm/setup.h b/arch/loongarch/include/asm/setup.h
> index ca373f8e3c4d..b18baf878ed4 100644
> --- a/arch/loongarch/include/asm/setup.h
> +++ b/arch/loongarch/include/asm/setup.h
> @@ -13,6 +13,7 @@
>
> extern unsigned long eentry;
> extern unsigned long tlbrentry;
> +extern char original_cmdline[COMMAND_LINE_SIZE];
> extern void tlb_init(int cpu);
> extern void cpu_cache_init(void);
> extern void cache_error_setup(void);
> diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
> index a31329971133..16dcd673e905 100644
> --- a/arch/loongarch/kernel/efi.c
> +++ b/arch/loongarch/kernel/efi.c
> @@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[] __initdata = {
>
> void __init efi_runtime_init(void)
> {
> - if (!efi_enabled(EFI_BOOT))
> + if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
How can this happen? Why would we ever set EFI_BOOT if
efi_systab->runtime == NULL?
> return;
>
> if (efi_runtime_disabled()) {
> @@ -101,3 +101,28 @@ void __init efi_init(void)
> early_memunmap(tbl, sizeof(*tbl));
> }
> }
> +
> +/* parse the fdt pointer from efi system table */
> +void __init *efi_fdt_pointer(void)
> +{
> + int i;
> + void *fdt = NULL;
> + efi_config_table_t *tables;
> +
> + if (!efi_systab) {
> + pr_err("Can't find EFI system table.\n");
> + return NULL;
> + }
> +
> + tables = (efi_config_table_t *) efi_systab->tables;
> +
> + for (i = 0; i < efi_systab->nr_tables; i++)
> + if (efi_guidcmp(tables[i].guid, DEVICE_TREE_GUID) == 0) {
> + fdt = (void *)early_memremap_ro(
> + (resource_size_t)tables[i].table,
> + sizeof(efi_config_table_t));
> + break;
> + }
> +
> + return fdt;
> +}
Please use the existing config table discovery mechanism for this.
> diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
> index 6d56a463b091..4fd6eb9398b4 100644
> --- a/arch/loongarch/kernel/env.c
> +++ b/arch/loongarch/kernel/env.c
> @@ -11,6 +11,7 @@
> #include <asm/early_ioremap.h>
> #include <asm/bootinfo.h>
> #include <asm/loongson.h>
> +#include <asm/setup.h>
>
> u64 efi_system_table;
> struct loongson_system_configuration loongson_sysconf;
> @@ -27,6 +28,8 @@ void __init init_environ(void)
> clear_bit(EFI_BOOT, &efi.flags);
>
> strscpy(boot_command_line, cmdline, COMMAND_LINE_SIZE);
> + /* Save unparsed command line copy for FDT */
Why is this necessary?
> + strscpy(original_cmdline, boot_command_line, COMMAND_LINE_SIZE);
> early_memunmap(cmdline, COMMAND_LINE_SIZE);
>
> efi_system_table = fw_arg2;
> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> index 1eb63fa9bc81..2f578f809a54 100644
> --- a/arch/loongarch/kernel/setup.c
> +++ b/arch/loongarch/kernel/setup.c
> @@ -29,6 +29,9 @@
> #include <linux/device.h>
> #include <linux/dma-map-ops.h>
> #include <linux/swiotlb.h>
> +#include <linux/of_fdt.h>
> +#include <linux/of_address.h>
> +#include <linux/libfdt.h>
>
> #include <asm/addrspace.h>
> #include <asm/bootinfo.h>
> @@ -67,6 +70,7 @@ static const char dmi_empty_string[] = " ";
> *
> * These are initialized so they are in the .data section
> */
> +char original_cmdline[COMMAND_LINE_SIZE] __initdata;
>
> static int num_standard_resources;
> static struct resource *standard_resources;
> @@ -281,6 +285,47 @@ static void __init check_kernel_sections_mem(void)
> }
> }
>
> +static void __init bootcmdline_init(char **cmdline_p)
> +{
> + /*
> + * If CONFIG_CMDLINE_FORCE is enabled then initializing the command line
> + * is trivial - we simply use the built-in command line unconditionally &
> + * unmodified.
> + */
> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
> + strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> + return;
> + }
> +
> +#ifdef CONFIG_OF_FLATTREE
> + /*
> + * This is for CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER.
> + *
> + * If CONFIG_CMDLINE_BOOTLOADER is enabled, we need to determine if the
> + * system is ACPI-based or FDT-based.
> + *
> + * For ACPI-based system: nothing need to do.
> + *
> + * For FDT-based system: the boot_command_line will be overwritten by
> + * early_init_dt_scan_chosen(), so we need to append original_cmdline(the
> + * original copy of boot_command_line) to boot_command_line.
> + *
> + * As for the built-in command line, if CONFIG_CMDLINE_EXTEND is enabled,
> + * both ACPI-based and FDT-based system will append the built-in command
> + * line to the boot_command_line themselves, and no additional processing
> + * is required here.
> + */
> + if (initial_boot_params) {
> + if (boot_command_line[0])
> + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> +
> + strlcat(boot_command_line, original_cmdline, COMMAND_LINE_SIZE);
> + }
> +#endif
> +
> + *cmdline_p = boot_command_line;
> +}
> +
> /*
> * arch_mem_init - initialize memory management subsystem
> */
> @@ -291,6 +336,8 @@ static void __init arch_mem_init(char **cmdline_p)
>
> check_kernel_sections_mem();
>
> + early_init_fdt_scan_reserved_mem();
> +
> /*
> * In order to reduce the possibility of kernel panic when failed to
> * get IO TLB memory under CONFIG_SWIOTLB, it is better to allocate
> @@ -413,20 +460,44 @@ static void __init prefill_possible_map(void)
> }
> #endif
>
> +static void __init fdt_setup(void)
> +{
> +#ifdef CONFIG_OF_EARLY_FLATTREE
> + void *fdt_pointer;
> +
> + /* ACPI-based systems do not require parsing fdt */
> + if (acpi_os_get_root_pointer())
> + return;
> +
> + /* Look for a device tree configuration table entry */
> + fdt_pointer = efi_fdt_pointer();
> + if (!fdt_pointer || fdt_check_header(fdt_pointer))
> + return;
> +
> + early_init_dt_scan(fdt_pointer);
> + early_init_fdt_reserve_self();
> + early_init_dt_scan_chosen_stdout();
> +
The FDT is only a hardware description, right? So why are we scanning
/chosen here?
> + max_low_pfn = PFN_PHYS(memblock_end_of_DRAM());
> +#endif
> +}
> +
> void __init setup_arch(char **cmdline_p)
> {
> cpu_probe();
> - *cmdline_p = boot_command_line;
>
> + pagetable_init();
> init_environ();
> efi_init();
> + fdt_setup();
> memblock_init();
> - pagetable_init();
> + bootcmdline_init(cmdline_p);
> parse_early_param();
> reserve_initrd_mem();
>
> platform_init();
> arch_mem_init(cmdline_p);
> + unflatten_and_copy_device_tree();
>
> resource_init();
> #ifdef CONFIG_SMP
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-06 15:06 ` Ard Biesheuvel
@ 2022-11-07 2:07 ` Jianmin Lv
2022-11-07 2:50 ` WANG Xuerui
2022-11-07 4:23 ` Huacai Chen
2022-11-07 13:06 ` Binbin Zhou
2 siblings, 1 reply; 17+ messages in thread
From: Jianmin Lv @ 2022-11-07 2:07 UTC (permalink / raw)
To: Ard Biesheuvel, Binbin Zhou
Cc: Huacai Chen, WANG Xuerui, Jiaxun Yang, loongarch
On 2022/11/6 下午11:06, Ard Biesheuvel wrote:
> On Sun, 6 Nov 2022 at 07:58, Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>>
>> Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
>> flattened DT"), we can parse the FDT from efi system table.
>>
>> And now, the Loongson-2K series cpus based on LoongArch support booting
>> with FDT, so we adds the relevant booting support as well as parameter
>> parsing.
>>
>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
>> ---
>> arch/loongarch/Kconfig | 2 +
>> arch/loongarch/include/asm/efi.h | 1 +
>> arch/loongarch/include/asm/setup.h | 1 +
>> arch/loongarch/kernel/efi.c | 27 ++++++++++-
>> arch/loongarch/kernel/env.c | 3 ++
>> arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
>> 6 files changed, 106 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>> index 903096bd87f8..ce53d1fa2b64 100644
>> --- a/arch/loongarch/Kconfig
>> +++ b/arch/loongarch/Kconfig
>> @@ -111,6 +111,8 @@ config LOONGARCH
>> select MODULES_USE_ELF_RELA if MODULES
>> select NEED_PER_CPU_EMBED_FIRST_CHUNK
>> select NEED_PER_CPU_PAGE_FIRST_CHUNK
>> + select OF
>> + select OF_EARLY_FLATTREE
>> select PCI
>> select PCI_DOMAINS_GENERIC
>> select PCI_ECAM if ACPI
>> diff --git a/arch/loongarch/include/asm/efi.h b/arch/loongarch/include/asm/efi.h
>> index 174567b00ddb..81e5d3371868 100644
>> --- a/arch/loongarch/include/asm/efi.h
>> +++ b/arch/loongarch/include/asm/efi.h
>> @@ -9,6 +9,7 @@
>>
>> void __init efi_init(void);
>> void __init efi_runtime_init(void);
>> +void __init *efi_fdt_pointer(void);
>> void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
>>
>> #define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
>> diff --git a/arch/loongarch/include/asm/setup.h b/arch/loongarch/include/asm/setup.h
>> index ca373f8e3c4d..b18baf878ed4 100644
>> --- a/arch/loongarch/include/asm/setup.h
>> +++ b/arch/loongarch/include/asm/setup.h
>> @@ -13,6 +13,7 @@
>>
>> extern unsigned long eentry;
>> extern unsigned long tlbrentry;
>> +extern char original_cmdline[COMMAND_LINE_SIZE];
>> extern void tlb_init(int cpu);
>> extern void cpu_cache_init(void);
>> extern void cache_error_setup(void);
>> diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
>> index a31329971133..16dcd673e905 100644
>> --- a/arch/loongarch/kernel/efi.c
>> +++ b/arch/loongarch/kernel/efi.c
>> @@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[] __initdata = {
>>
>> void __init efi_runtime_init(void)
>> {
>> - if (!efi_enabled(EFI_BOOT))
>> + if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
>
> How can this happen? Why would we ever set EFI_BOOT if
> efi_systab->runtime == NULL?
>
Because some firmware (such as PMON) employed UEFI compatible
implementaion without runtime (and set runtime null pointer).
Maybe *noefi* in command line is enough for this without
CONFIG_CMDLINE_FORCE simillar stuff configured?
>> return;
>>
>> if (efi_runtime_disabled()) {
>> @@ -101,3 +101,28 @@ void __init efi_init(void)
>> early_memunmap(tbl, sizeof(*tbl));
>> }
>> }
>> +
>> +/* parse the fdt pointer from efi system table */
>> +void __init *efi_fdt_pointer(void)
>> +{
>> + int i;
>> + void *fdt = NULL;
>> + efi_config_table_t *tables;
>> +
>> + if (!efi_systab) {
>> + pr_err("Can't find EFI system table.\n");
>> + return NULL;
>> + }
>> +
>> + tables = (efi_config_table_t *) efi_systab->tables;
>> +
>> + for (i = 0; i < efi_systab->nr_tables; i++)
>> + if (efi_guidcmp(tables[i].guid, DEVICE_TREE_GUID) == 0) {
>> + fdt = (void *)early_memremap_ro(
>> + (resource_size_t)tables[i].table,
>> + sizeof(efi_config_table_t));
>> + break;
>> + }
>> +
>> + return fdt;
>> +}
>
> Please use the existing config table discovery mechanism for this.
>
>> diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
>> index 6d56a463b091..4fd6eb9398b4 100644
>> --- a/arch/loongarch/kernel/env.c
>> +++ b/arch/loongarch/kernel/env.c
>> @@ -11,6 +11,7 @@
>> #include <asm/early_ioremap.h>
>> #include <asm/bootinfo.h>
>> #include <asm/loongson.h>
>> +#include <asm/setup.h>
>>
>> u64 efi_system_table;
>> struct loongson_system_configuration loongson_sysconf;
>> @@ -27,6 +28,8 @@ void __init init_environ(void)
>> clear_bit(EFI_BOOT, &efi.flags);
>>
>> strscpy(boot_command_line, cmdline, COMMAND_LINE_SIZE);
>> + /* Save unparsed command line copy for FDT */
>
> Why is this necessary?
>
>> + strscpy(original_cmdline, boot_command_line, COMMAND_LINE_SIZE);
>> early_memunmap(cmdline, COMMAND_LINE_SIZE);
>>
>> efi_system_table = fw_arg2;
>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
>> index 1eb63fa9bc81..2f578f809a54 100644
>> --- a/arch/loongarch/kernel/setup.c
>> +++ b/arch/loongarch/kernel/setup.c
>> @@ -29,6 +29,9 @@
>> #include <linux/device.h>
>> #include <linux/dma-map-ops.h>
>> #include <linux/swiotlb.h>
>> +#include <linux/of_fdt.h>
>> +#include <linux/of_address.h>
>> +#include <linux/libfdt.h>
>>
>> #include <asm/addrspace.h>
>> #include <asm/bootinfo.h>
>> @@ -67,6 +70,7 @@ static const char dmi_empty_string[] = " ";
>> *
>> * These are initialized so they are in the .data section
>> */
>> +char original_cmdline[COMMAND_LINE_SIZE] __initdata;
>>
>> static int num_standard_resources;
>> static struct resource *standard_resources;
>> @@ -281,6 +285,47 @@ static void __init check_kernel_sections_mem(void)
>> }
>> }
>>
>> +static void __init bootcmdline_init(char **cmdline_p)
>> +{
>> + /*
>> + * If CONFIG_CMDLINE_FORCE is enabled then initializing the command line
>> + * is trivial - we simply use the built-in command line unconditionally &
>> + * unmodified.
>> + */
>> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
>> + strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
>> + return;
>> + }
>> +
>> +#ifdef CONFIG_OF_FLATTREE
>> + /*
>> + * This is for CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER.
>> + *
>> + * If CONFIG_CMDLINE_BOOTLOADER is enabled, we need to determine if the
>> + * system is ACPI-based or FDT-based.
>> + *
>> + * For ACPI-based system: nothing need to do.
>> + *
>> + * For FDT-based system: the boot_command_line will be overwritten by
>> + * early_init_dt_scan_chosen(), so we need to append original_cmdline(the
>> + * original copy of boot_command_line) to boot_command_line.
>> + *
>> + * As for the built-in command line, if CONFIG_CMDLINE_EXTEND is enabled,
>> + * both ACPI-based and FDT-based system will append the built-in command
>> + * line to the boot_command_line themselves, and no additional processing
>> + * is required here.
>> + */
>> + if (initial_boot_params) {
>> + if (boot_command_line[0])
>> + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
>> +
>> + strlcat(boot_command_line, original_cmdline, COMMAND_LINE_SIZE);
>> + }
>> +#endif
>> +
>> + *cmdline_p = boot_command_line;
>> +}
>> +
>> /*
>> * arch_mem_init - initialize memory management subsystem
>> */
>> @@ -291,6 +336,8 @@ static void __init arch_mem_init(char **cmdline_p)
>>
>> check_kernel_sections_mem();
>>
>> + early_init_fdt_scan_reserved_mem();
>> +
>> /*
>> * In order to reduce the possibility of kernel panic when failed to
>> * get IO TLB memory under CONFIG_SWIOTLB, it is better to allocate
>> @@ -413,20 +460,44 @@ static void __init prefill_possible_map(void)
>> }
>> #endif
>>
>> +static void __init fdt_setup(void)
>> +{
>> +#ifdef CONFIG_OF_EARLY_FLATTREE
>> + void *fdt_pointer;
>> +
>> + /* ACPI-based systems do not require parsing fdt */
>> + if (acpi_os_get_root_pointer())
>> + return;
>> +
>> + /* Look for a device tree configuration table entry */
>> + fdt_pointer = efi_fdt_pointer();
>> + if (!fdt_pointer || fdt_check_header(fdt_pointer))
>> + return;
>> +
>> + early_init_dt_scan(fdt_pointer);
>> + early_init_fdt_reserve_self();
>> + early_init_dt_scan_chosen_stdout();
>> +
>
> The FDT is only a hardware description, right? So why are we scanning
> /chosen here?
>
>> + max_low_pfn = PFN_PHYS(memblock_end_of_DRAM());
>> +#endif
>> +}
>> +
>> void __init setup_arch(char **cmdline_p)
>> {
>> cpu_probe();
>> - *cmdline_p = boot_command_line;
>>
>> + pagetable_init();
>> init_environ();
>> efi_init();
>> + fdt_setup();
>> memblock_init();
>> - pagetable_init();
>> + bootcmdline_init(cmdline_p);
>> parse_early_param();
>> reserve_initrd_mem();
>>
>> platform_init();
>> arch_mem_init(cmdline_p);
>> + unflatten_and_copy_device_tree();
>>
>> resource_init();
>> #ifdef CONFIG_SMP
>> --
>> 2.31.1
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-07 2:07 ` Jianmin Lv
@ 2022-11-07 2:50 ` WANG Xuerui
2022-11-07 8:44 ` Jianmin Lv
0 siblings, 1 reply; 17+ messages in thread
From: WANG Xuerui @ 2022-11-07 2:50 UTC (permalink / raw)
To: Jianmin Lv, Ard Biesheuvel, Binbin Zhou
Cc: Huacai Chen, Jiaxun Yang, loongarch
On 2022/11/7 10:07, Jianmin Lv wrote:
>
>
> On 2022/11/6 下午11:06, Ard Biesheuvel wrote:
>> On Sun, 6 Nov 2022 at 07:58, Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>>>
>>> Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
>>> flattened DT"), we can parse the FDT from efi system table.
>>>
>>> And now, the Loongson-2K series cpus based on LoongArch support booting
>>> with FDT, so we adds the relevant booting support as well as parameter
>>> parsing.
>>>
>>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
>>> ---
>>> arch/loongarch/Kconfig | 2 +
>>> arch/loongarch/include/asm/efi.h | 1 +
>>> arch/loongarch/include/asm/setup.h | 1 +
>>> arch/loongarch/kernel/efi.c | 27 ++++++++++-
>>> arch/loongarch/kernel/env.c | 3 ++
>>> arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
>>> 6 files changed, 106 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>>> index 903096bd87f8..ce53d1fa2b64 100644
>>> --- a/arch/loongarch/Kconfig
>>> +++ b/arch/loongarch/Kconfig
>>> @@ -111,6 +111,8 @@ config LOONGARCH
>>> select MODULES_USE_ELF_RELA if MODULES
>>> select NEED_PER_CPU_EMBED_FIRST_CHUNK
>>> select NEED_PER_CPU_PAGE_FIRST_CHUNK
>>> + select OF
>>> + select OF_EARLY_FLATTREE
>>> select PCI
>>> select PCI_DOMAINS_GENERIC
>>> select PCI_ECAM if ACPI
>>> diff --git a/arch/loongarch/include/asm/efi.h
>>> b/arch/loongarch/include/asm/efi.h
>>> index 174567b00ddb..81e5d3371868 100644
>>> --- a/arch/loongarch/include/asm/efi.h
>>> +++ b/arch/loongarch/include/asm/efi.h
>>> @@ -9,6 +9,7 @@
>>>
>>> void __init efi_init(void);
>>> void __init efi_runtime_init(void);
>>> +void __init *efi_fdt_pointer(void);
>>> void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
>>>
>>> #define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
>>> diff --git a/arch/loongarch/include/asm/setup.h
>>> b/arch/loongarch/include/asm/setup.h
>>> index ca373f8e3c4d..b18baf878ed4 100644
>>> --- a/arch/loongarch/include/asm/setup.h
>>> +++ b/arch/loongarch/include/asm/setup.h
>>> @@ -13,6 +13,7 @@
>>>
>>> extern unsigned long eentry;
>>> extern unsigned long tlbrentry;
>>> +extern char original_cmdline[COMMAND_LINE_SIZE];
>>> extern void tlb_init(int cpu);
>>> extern void cpu_cache_init(void);
>>> extern void cache_error_setup(void);
>>> diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
>>> index a31329971133..16dcd673e905 100644
>>> --- a/arch/loongarch/kernel/efi.c
>>> +++ b/arch/loongarch/kernel/efi.c
>>> @@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[]
>>> __initdata = {
>>>
>>> void __init efi_runtime_init(void)
>>> {
>>> - if (!efi_enabled(EFI_BOOT))
>>> + if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
>>
>> How can this happen? Why would we ever set EFI_BOOT if
>> efi_systab->runtime == NULL?
>>
>
> Because some firmware (such as PMON) employed UEFI compatible
> implementaion without runtime (and set runtime null pointer).
Hmm, is it really "UEFI-compatible" when it's without a UEFI runtime?
Anyway it's becoming increasingly difficult for community reviewers to
check things without corresponding firmware sources available... (and I
think, given the Loongson fork of PMON even back in the MIPS era is
effectively GPL'd [1], the LoongArch port of PMON could probably be GPL
as well, in which case it's totally fair for us to request the sources.)
(I know the similar usage pattern i.e. making use of various UEFI
features but without a runtime was already widespread back in the MIPS
days of Loongson, but personally I don't think it's enough to justify
doing the same when it's 2022 and we're fully upstream.)
[1]: https://github.com/loongson-community/pmon/blob/master/LICENSE.md
> Maybe *noefi* in command line is enough for this without
> CONFIG_CMDLINE_FORCE simillar stuff configured?
I think having/forcing users to add "noefi" themselves wouldn't be good
ergonomics? Manipulating the cmdline in the firmware could be more
acceptable but I'm not sure.
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-06 10:15 ` Xi Ruoyao
@ 2022-11-07 2:59 ` Binbin Zhou
2022-11-07 3:11 ` WANG Xuerui
0 siblings, 1 reply; 17+ messages in thread
From: Binbin Zhou @ 2022-11-07 2:59 UTC (permalink / raw)
To: Xi Ruoyao, WANG Xuerui, Huacai Chen, Jiaxun Yang, Ard Biesheuvel,
Jianmin Lv
Cc: loongarch
Hi Ruoyao:
在 2022/11/6 18:15, Xi Ruoyao 写道:
> On Sun, 2022-11-06 at 17:15 +0800, WANG Xuerui wrote:
>> And, although we're already aware of the part-UEFI and part-FDT usage
>> pattern, and the arch UEFI code already has consideration for such, it
>> still makes me a bit wary because you risk losing standards compliance
>> and creating maintenance burden for admins/users/devs alike by relying
>> on this not-really-UEFI-but-advertising-as-such thing. I'd still suggest
>> you try throwing together something fully DT, to at least experiment a
>> little with. You already have to rewrite pretty much everything with all
>> the boot protocol changes in the past few months, so this would probably
>> not even hurt, and the burden on the LoongArch/generic UEFI infra could
>> be lessened even further if this plays out. And you can always continue
>> working on this change if it turns out infeasible to do so.
>
> AFAIK using UEFI with FDT (instead of ACPI) is fully allowed and
> documented by the standard (section 4.6.1.3 of the UEFI spec). It may
> be not possible or not worthy to implement ACPI for 2K systems, then FDT
> will be the only choice.
>
> In ARM64 ecosystem, SBBR (for server) demands UEFI+ACPI, EBBR (for
> embedded) allows UEFI+ACPI or UEFI+FDT.
>
Yes, I agree with you.
Theoretically, we have four combinations: EFI+ACPI, EFI+FDT,
non-EFI+ACPI,non-EFI+FDT.
Now, for LoongArch, EFI+ACPI has been used on Loongson-3, and
Loongson-2K being an embedded product, EFI+FDT is a more suitable way.
They are both relatively independent. So I don't see this as a violation
of standards compliance, nor does it add to the maintenance burden.
Binbin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-07 2:59 ` Binbin Zhou
@ 2022-11-07 3:11 ` WANG Xuerui
0 siblings, 0 replies; 17+ messages in thread
From: WANG Xuerui @ 2022-11-07 3:11 UTC (permalink / raw)
To: Binbin Zhou, Xi Ruoyao, Huacai Chen, Jiaxun Yang, Ard Biesheuvel,
Jianmin Lv
Cc: loongarch
On 2022/11/7 10:59, Binbin Zhou wrote:
> Hi Ruoyao:
>
> 在 2022/11/6 18:15, Xi Ruoyao 写道:
>> On Sun, 2022-11-06 at 17:15 +0800, WANG Xuerui wrote:
>>> And, although we're already aware of the part-UEFI and part-FDT usage
>>> pattern, and the arch UEFI code already has consideration for such, it
>>> still makes me a bit wary because you risk losing standards compliance
>>> and creating maintenance burden for admins/users/devs alike by relying
>>> on this not-really-UEFI-but-advertising-as-such thing. I'd still suggest
>>> you try throwing together something fully DT, to at least experiment a
>>> little with. You already have to rewrite pretty much everything with all
>>> the boot protocol changes in the past few months, so this would probably
>>> not even hurt, and the burden on the LoongArch/generic UEFI infra could
>>> be lessened even further if this plays out. And you can always continue
>>> working on this change if it turns out infeasible to do so.
>>
>> AFAIK using UEFI with FDT (instead of ACPI) is fully allowed and
>> documented by the standard (section 4.6.1.3 of the UEFI spec). It may
>> be not possible or not worthy to implement ACPI for 2K systems, then FDT
>> will be the only choice.
>>
>> In ARM64 ecosystem, SBBR (for server) demands UEFI+ACPI, EBBR (for
>> embedded) allows UEFI+ACPI or UEFI+FDT.
Ah I see. My bad for not reading the spec in enough detail...
>>
>
> Yes, I agree with you.
>
> Theoretically, we have four combinations: EFI+ACPI, EFI+FDT,
> non-EFI+ACPI,non-EFI+FDT.
>
> Now, for LoongArch, EFI+ACPI has been used on Loongson-3, and
> Loongson-2K being an embedded product, EFI+FDT is a more suitable way.
> They are both relatively independent. So I don't see this as a violation
> of standards compliance, nor does it add to the maintenance burden.
Yeah I now see the reason why it's acceptable. Then, I'm only suggesting
you to try formulating the changes and commit message, in a way that
doesn't leave people with the false impression that Loongson-3 is
somehow strongly bound with EFI+ACPI and Loongson-2 with EFI+FDT,
despite the typical capabilities of such systems.
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-06 7:55 ` Xi Ruoyao
@ 2022-11-07 4:09 ` Huacai Chen
0 siblings, 0 replies; 17+ messages in thread
From: Huacai Chen @ 2022-11-07 4:09 UTC (permalink / raw)
To: Xi Ruoyao
Cc: Binbin Zhou, WANG Xuerui, Jiaxun Yang, Ard Biesheuvel,
Jianmin Lv, loongarch
Hi, Ruoyao,
On Sun, Nov 6, 2022 at 3:55 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Sun, 2022-11-06 at 14:56 +0800, Binbin Zhou wrote:
> > Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
> > flattened DT"), we can parse the FDT from efi system table.
> >
> > And now, the Loongson-2K series cpus based on LoongArch support booting
> > with FDT, so we adds the relevant booting support as well as parameter
> > parsing.
> >
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> > arch/loongarch/Kconfig | 2 +
> > arch/loongarch/include/asm/efi.h | 1 +
> > arch/loongarch/include/asm/setup.h | 1 +
> > arch/loongarch/kernel/efi.c | 27 ++++++++++-
> > arch/loongarch/kernel/env.c | 3 ++
> > arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
> > 6 files changed, 106 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> > index 903096bd87f8..ce53d1fa2b64 100644
> > --- a/arch/loongarch/Kconfig
> > +++ b/arch/loongarch/Kconfig
> > @@ -111,6 +111,8 @@ config LOONGARCH
> > select MODULES_USE_ELF_RELA if MODULES
> > select NEED_PER_CPU_EMBED_FIRST_CHUNK
> > select NEED_PER_CPU_PAGE_FIRST_CHUNK
> > + select OF
> > + select OF_EARLY_FLATTREE
>
> I don't like unconditionally enabling OF or OF_EARLY_FLATTREE because
> they are not needed for Loongson-3 workstations or servers. And you are
> already fusing the code with #ifdef CONFIG_OF_EARLY_FLATTREE anyway:
I also don't like it. But the kernel test robot will use
allyesconfig/allnoconfig/allmodconfig/randconfig to test kernel
building. If we don't select OF unconditionally, there will be a ton
of build errors. :( And on the other hand, ARM64 & RISCV also selects
OF unconditionally.
Huacai
>
> /* snip */
>
> > diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> > index 1eb63fa9bc81..2f578f809a54 100644
> > --- a/arch/loongarch/kernel/setup.c
> > +++ b/arch/loongarch/kernel/setup.c
>
> /* snip */
>
> > +static void __init fdt_setup(void)
> > +{
> > +#ifdef CONFIG_OF_EARLY_FLATTREE
>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-06 15:06 ` Ard Biesheuvel
2022-11-07 2:07 ` Jianmin Lv
@ 2022-11-07 4:23 ` Huacai Chen
2022-11-07 13:33 ` Ard Biesheuvel
2022-11-07 13:06 ` Binbin Zhou
2 siblings, 1 reply; 17+ messages in thread
From: Huacai Chen @ 2022-11-07 4:23 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Binbin Zhou, WANG Xuerui, Jiaxun Yang, Jianmin Lv, loongarch
Hi, Ard,
On Sun, Nov 6, 2022 at 11:06 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sun, 6 Nov 2022 at 07:58, Binbin Zhou <zhoubinbin@loongson.cn> wrote:
> >
> > Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
> > flattened DT"), we can parse the FDT from efi system table.
> >
> > And now, the Loongson-2K series cpus based on LoongArch support booting
> > with FDT, so we adds the relevant booting support as well as parameter
> > parsing.
> >
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> > arch/loongarch/Kconfig | 2 +
> > arch/loongarch/include/asm/efi.h | 1 +
> > arch/loongarch/include/asm/setup.h | 1 +
> > arch/loongarch/kernel/efi.c | 27 ++++++++++-
> > arch/loongarch/kernel/env.c | 3 ++
> > arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
> > 6 files changed, 106 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> > index 903096bd87f8..ce53d1fa2b64 100644
> > --- a/arch/loongarch/Kconfig
> > +++ b/arch/loongarch/Kconfig
> > @@ -111,6 +111,8 @@ config LOONGARCH
> > select MODULES_USE_ELF_RELA if MODULES
> > select NEED_PER_CPU_EMBED_FIRST_CHUNK
> > select NEED_PER_CPU_PAGE_FIRST_CHUNK
> > + select OF
> > + select OF_EARLY_FLATTREE
> > select PCI
> > select PCI_DOMAINS_GENERIC
> > select PCI_ECAM if ACPI
> > diff --git a/arch/loongarch/include/asm/efi.h b/arch/loongarch/include/asm/efi.h
> > index 174567b00ddb..81e5d3371868 100644
> > --- a/arch/loongarch/include/asm/efi.h
> > +++ b/arch/loongarch/include/asm/efi.h
> > @@ -9,6 +9,7 @@
> >
> > void __init efi_init(void);
> > void __init efi_runtime_init(void);
> > +void __init *efi_fdt_pointer(void);
> > void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
> >
> > #define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
> > diff --git a/arch/loongarch/include/asm/setup.h b/arch/loongarch/include/asm/setup.h
> > index ca373f8e3c4d..b18baf878ed4 100644
> > --- a/arch/loongarch/include/asm/setup.h
> > +++ b/arch/loongarch/include/asm/setup.h
> > @@ -13,6 +13,7 @@
> >
> > extern unsigned long eentry;
> > extern unsigned long tlbrentry;
> > +extern char original_cmdline[COMMAND_LINE_SIZE];
> > extern void tlb_init(int cpu);
> > extern void cpu_cache_init(void);
> > extern void cache_error_setup(void);
> > diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
> > index a31329971133..16dcd673e905 100644
> > --- a/arch/loongarch/kernel/efi.c
> > +++ b/arch/loongarch/kernel/efi.c
> > @@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[] __initdata = {
> >
> > void __init efi_runtime_init(void)
> > {
> > - if (!efi_enabled(EFI_BOOT))
> > + if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
>
> How can this happen? Why would we ever set EFI_BOOT if
> efi_systab->runtime == NULL?
>
> > return;
> >
> > if (efi_runtime_disabled()) {
> > @@ -101,3 +101,28 @@ void __init efi_init(void)
> > early_memunmap(tbl, sizeof(*tbl));
> > }
> > }
> > +
> > +/* parse the fdt pointer from efi system table */
> > +void __init *efi_fdt_pointer(void)
> > +{
> > + int i;
> > + void *fdt = NULL;
> > + efi_config_table_t *tables;
> > +
> > + if (!efi_systab) {
> > + pr_err("Can't find EFI system table.\n");
> > + return NULL;
> > + }
> > +
> > + tables = (efi_config_table_t *) efi_systab->tables;
> > +
> > + for (i = 0; i < efi_systab->nr_tables; i++)
> > + if (efi_guidcmp(tables[i].guid, DEVICE_TREE_GUID) == 0) {
> > + fdt = (void *)early_memremap_ro(
> > + (resource_size_t)tables[i].table,
> > + sizeof(efi_config_table_t));
> > + break;
> > + }
> > +
> > + return fdt;
> > +}
>
> Please use the existing config table discovery mechanism for this.
>
> > diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
> > index 6d56a463b091..4fd6eb9398b4 100644
> > --- a/arch/loongarch/kernel/env.c
> > +++ b/arch/loongarch/kernel/env.c
> > @@ -11,6 +11,7 @@
> > #include <asm/early_ioremap.h>
> > #include <asm/bootinfo.h>
> > #include <asm/loongson.h>
> > +#include <asm/setup.h>
> >
> > u64 efi_system_table;
> > struct loongson_system_configuration loongson_sysconf;
> > @@ -27,6 +28,8 @@ void __init init_environ(void)
> > clear_bit(EFI_BOOT, &efi.flags);
> >
> > strscpy(boot_command_line, cmdline, COMMAND_LINE_SIZE);
> > + /* Save unparsed command line copy for FDT */
>
> Why is this necessary?
>
> > + strscpy(original_cmdline, boot_command_line, COMMAND_LINE_SIZE);
> > early_memunmap(cmdline, COMMAND_LINE_SIZE);
> >
> > efi_system_table = fw_arg2;
> > diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> > index 1eb63fa9bc81..2f578f809a54 100644
> > --- a/arch/loongarch/kernel/setup.c
> > +++ b/arch/loongarch/kernel/setup.c
> > @@ -29,6 +29,9 @@
> > #include <linux/device.h>
> > #include <linux/dma-map-ops.h>
> > #include <linux/swiotlb.h>
> > +#include <linux/of_fdt.h>
> > +#include <linux/of_address.h>
> > +#include <linux/libfdt.h>
> >
> > #include <asm/addrspace.h>
> > #include <asm/bootinfo.h>
> > @@ -67,6 +70,7 @@ static const char dmi_empty_string[] = " ";
> > *
> > * These are initialized so they are in the .data section
> > */
> > +char original_cmdline[COMMAND_LINE_SIZE] __initdata;
> >
> > static int num_standard_resources;
> > static struct resource *standard_resources;
> > @@ -281,6 +285,47 @@ static void __init check_kernel_sections_mem(void)
> > }
> > }
> >
> > +static void __init bootcmdline_init(char **cmdline_p)
> > +{
> > + /*
> > + * If CONFIG_CMDLINE_FORCE is enabled then initializing the command line
> > + * is trivial - we simply use the built-in command line unconditionally &
> > + * unmodified.
> > + */
> > + if (IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
> > + strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > + return;
> > + }
> > +
> > +#ifdef CONFIG_OF_FLATTREE
> > + /*
> > + * This is for CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER.
> > + *
> > + * If CONFIG_CMDLINE_BOOTLOADER is enabled, we need to determine if the
> > + * system is ACPI-based or FDT-based.
> > + *
> > + * For ACPI-based system: nothing need to do.
> > + *
> > + * For FDT-based system: the boot_command_line will be overwritten by
> > + * early_init_dt_scan_chosen(), so we need to append original_cmdline(the
> > + * original copy of boot_command_line) to boot_command_line.
> > + *
> > + * As for the built-in command line, if CONFIG_CMDLINE_EXTEND is enabled,
> > + * both ACPI-based and FDT-based system will append the built-in command
> > + * line to the boot_command_line themselves, and no additional processing
> > + * is required here.
> > + */
> > + if (initial_boot_params) {
> > + if (boot_command_line[0])
> > + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> > +
> > + strlcat(boot_command_line, original_cmdline, COMMAND_LINE_SIZE);
> > + }
> > +#endif
> > +
> > + *cmdline_p = boot_command_line;
> > +}
> > +
> > /*
> > * arch_mem_init - initialize memory management subsystem
> > */
> > @@ -291,6 +336,8 @@ static void __init arch_mem_init(char **cmdline_p)
> >
> > check_kernel_sections_mem();
> >
> > + early_init_fdt_scan_reserved_mem();
> > +
> > /*
> > * In order to reduce the possibility of kernel panic when failed to
> > * get IO TLB memory under CONFIG_SWIOTLB, it is better to allocate
> > @@ -413,20 +460,44 @@ static void __init prefill_possible_map(void)
> > }
> > #endif
> >
> > +static void __init fdt_setup(void)
> > +{
> > +#ifdef CONFIG_OF_EARLY_FLATTREE
> > + void *fdt_pointer;
> > +
> > + /* ACPI-based systems do not require parsing fdt */
> > + if (acpi_os_get_root_pointer())
> > + return;
> > +
> > + /* Look for a device tree configuration table entry */
> > + fdt_pointer = efi_fdt_pointer();
> > + if (!fdt_pointer || fdt_check_header(fdt_pointer))
> > + return;
> > +
> > + early_init_dt_scan(fdt_pointer);
> > + early_init_fdt_reserve_self();
> > + early_init_dt_scan_chosen_stdout();
> > +
>
> The FDT is only a hardware description, right? So why are we scanning
> /chosen here?
We scan the chosen node here in order to configure earlycon as early
as possible, this is very useful for debugging. ARM64 does similar
things in acpi_boot_table_init(), but I think fdt_setup() is more
suitable.
Huacai
>
> > + max_low_pfn = PFN_PHYS(memblock_end_of_DRAM());
> > +#endif
> > +}
> > +
> > void __init setup_arch(char **cmdline_p)
> > {
> > cpu_probe();
> > - *cmdline_p = boot_command_line;
> >
> > + pagetable_init();
> > init_environ();
> > efi_init();
> > + fdt_setup();
> > memblock_init();
> > - pagetable_init();
> > + bootcmdline_init(cmdline_p);
> > parse_early_param();
> > reserve_initrd_mem();
> >
> > platform_init();
> > arch_mem_init(cmdline_p);
> > + unflatten_and_copy_device_tree();
> >
> > resource_init();
> > #ifdef CONFIG_SMP
> > --
> > 2.31.1
> >
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-07 2:50 ` WANG Xuerui
@ 2022-11-07 8:44 ` Jianmin Lv
2022-11-21 6:44 ` Huacai Chen
0 siblings, 1 reply; 17+ messages in thread
From: Jianmin Lv @ 2022-11-07 8:44 UTC (permalink / raw)
To: WANG Xuerui, Ard Biesheuvel, Binbin Zhou
Cc: Huacai Chen, Jiaxun Yang, loongarch
On 2022/11/7 上午10:50, WANG Xuerui wrote:
> On 2022/11/7 10:07, Jianmin Lv wrote:
>>
>>
>> On 2022/11/6 下午11:06, Ard Biesheuvel wrote:
>>> On Sun, 6 Nov 2022 at 07:58, Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>>>>
>>>> Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
>>>> flattened DT"), we can parse the FDT from efi system table.
>>>>
>>>> And now, the Loongson-2K series cpus based on LoongArch support booting
>>>> with FDT, so we adds the relevant booting support as well as parameter
>>>> parsing.
>>>>
>>>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
>>>> ---
>>>> arch/loongarch/Kconfig | 2 +
>>>> arch/loongarch/include/asm/efi.h | 1 +
>>>> arch/loongarch/include/asm/setup.h | 1 +
>>>> arch/loongarch/kernel/efi.c | 27 ++++++++++-
>>>> arch/loongarch/kernel/env.c | 3 ++
>>>> arch/loongarch/kernel/setup.c | 75
>>>> +++++++++++++++++++++++++++++-
>>>> 6 files changed, 106 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>>>> index 903096bd87f8..ce53d1fa2b64 100644
>>>> --- a/arch/loongarch/Kconfig
>>>> +++ b/arch/loongarch/Kconfig
>>>> @@ -111,6 +111,8 @@ config LOONGARCH
>>>> select MODULES_USE_ELF_RELA if MODULES
>>>> select NEED_PER_CPU_EMBED_FIRST_CHUNK
>>>> select NEED_PER_CPU_PAGE_FIRST_CHUNK
>>>> + select OF
>>>> + select OF_EARLY_FLATTREE
>>>> select PCI
>>>> select PCI_DOMAINS_GENERIC
>>>> select PCI_ECAM if ACPI
>>>> diff --git a/arch/loongarch/include/asm/efi.h
>>>> b/arch/loongarch/include/asm/efi.h
>>>> index 174567b00ddb..81e5d3371868 100644
>>>> --- a/arch/loongarch/include/asm/efi.h
>>>> +++ b/arch/loongarch/include/asm/efi.h
>>>> @@ -9,6 +9,7 @@
>>>>
>>>> void __init efi_init(void);
>>>> void __init efi_runtime_init(void);
>>>> +void __init *efi_fdt_pointer(void);
>>>> void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
>>>>
>>>> #define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
>>>> diff --git a/arch/loongarch/include/asm/setup.h
>>>> b/arch/loongarch/include/asm/setup.h
>>>> index ca373f8e3c4d..b18baf878ed4 100644
>>>> --- a/arch/loongarch/include/asm/setup.h
>>>> +++ b/arch/loongarch/include/asm/setup.h
>>>> @@ -13,6 +13,7 @@
>>>>
>>>> extern unsigned long eentry;
>>>> extern unsigned long tlbrentry;
>>>> +extern char original_cmdline[COMMAND_LINE_SIZE];
>>>> extern void tlb_init(int cpu);
>>>> extern void cpu_cache_init(void);
>>>> extern void cache_error_setup(void);
>>>> diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
>>>> index a31329971133..16dcd673e905 100644
>>>> --- a/arch/loongarch/kernel/efi.c
>>>> +++ b/arch/loongarch/kernel/efi.c
>>>> @@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[]
>>>> __initdata = {
>>>>
>>>> void __init efi_runtime_init(void)
>>>> {
>>>> - if (!efi_enabled(EFI_BOOT))
>>>> + if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
>>>
>>> How can this happen? Why would we ever set EFI_BOOT if
>>> efi_systab->runtime == NULL?
>>>
>>
>> Because some firmware (such as PMON) employed UEFI compatible
>> implementaion without runtime (and set runtime null pointer).
>
> Hmm, is it really "UEFI-compatible" when it's without a UEFI runtime?
To be frank, it seems that runtime should be always valid in UEFI
complaint firmware (according to UEFI spec section 4.5, 8).
Unfortunately, some machines shipped with *partially-compatible"
(without runtime) firmware have been outside (maybe not many, I'm not
sure). So, thinking about compatibility with them, the case have to be
taken account, unless rather.
> Anyway it's becoming increasingly difficult for community reviewers to
> check things without corresponding firmware sources available... (and I
> think, given the Loongson fork of PMON even back in the MIPS era is
> effectively GPL'd [1], the LoongArch port of PMON could probably be GPL
> as well, in which case it's totally fair for us to request the sources.)
>
> (I know the similar usage pattern i.e. making use of various UEFI
> features but without a runtime was already widespread back in the MIPS
> days of Loongson, but personally I don't think it's enough to justify
> doing the same when it's 2022 and we're fully upstream.)
>
> [1]: https://github.com/loongson-community/pmon/blob/master/LICENSE.md
>
>> Maybe *noefi* in command line is enough for this without
>> CONFIG_CMDLINE_FORCE simillar stuff configured?
>
> I think having/forcing users to add "noefi" themselves wouldn't be good
> ergonomics? Manipulating the cmdline in the firmware could be more
> acceptable but I'm not sure.
>
I think so, adding it in firmware internally (if using "noefi") maybe
acceptable rather than adding it by users.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-06 15:06 ` Ard Biesheuvel
2022-11-07 2:07 ` Jianmin Lv
2022-11-07 4:23 ` Huacai Chen
@ 2022-11-07 13:06 ` Binbin Zhou
2022-11-07 13:24 ` Ard Biesheuvel
2 siblings, 1 reply; 17+ messages in thread
From: Binbin Zhou @ 2022-11-07 13:06 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Huacai Chen, WANG Xuerui, Jiaxun Yang, Jianmin Lv, loongarch
在 2022/11/6 23:06, Ard Biesheuvel 写道:
> On Sun, 6 Nov 2022 at 07:58, Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>>
>> Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
>> flattened DT"), we can parse the FDT from efi system table.
>>
>> And now, the Loongson-2K series cpus based on LoongArch support booting
>> with FDT, so we adds the relevant booting support as well as parameter
>> parsing.
>>
>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
>> ---
>> arch/loongarch/Kconfig | 2 +
>> arch/loongarch/include/asm/efi.h | 1 +
>> arch/loongarch/include/asm/setup.h | 1 +
>> arch/loongarch/kernel/efi.c | 27 ++++++++++-
>> arch/loongarch/kernel/env.c | 3 ++
>> arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
>> 6 files changed, 106 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>> index 903096bd87f8..ce53d1fa2b64 100644
>> --- a/arch/loongarch/Kconfig
>> +++ b/arch/loongarch/Kconfig
>> @@ -111,6 +111,8 @@ config LOONGARCH
>> select MODULES_USE_ELF_RELA if MODULES
>> select NEED_PER_CPU_EMBED_FIRST_CHUNK
>> select NEED_PER_CPU_PAGE_FIRST_CHUNK
>> + select OF
>> + select OF_EARLY_FLATTREE
>> select PCI
>> select PCI_DOMAINS_GENERIC
>> select PCI_ECAM if ACPI
>> diff --git a/arch/loongarch/include/asm/efi.h b/arch/loongarch/include/asm/efi.h
>> index 174567b00ddb..81e5d3371868 100644
>> --- a/arch/loongarch/include/asm/efi.h
>> +++ b/arch/loongarch/include/asm/efi.h
>> @@ -9,6 +9,7 @@
>>
>> void __init efi_init(void);
>> void __init efi_runtime_init(void);
>> +void __init *efi_fdt_pointer(void);
>> void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
>>
>> #define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
>> diff --git a/arch/loongarch/include/asm/setup.h b/arch/loongarch/include/asm/setup.h
>> index ca373f8e3c4d..b18baf878ed4 100644
>> --- a/arch/loongarch/include/asm/setup.h
>> +++ b/arch/loongarch/include/asm/setup.h
>> @@ -13,6 +13,7 @@
>>
>> extern unsigned long eentry;
>> extern unsigned long tlbrentry;
>> +extern char original_cmdline[COMMAND_LINE_SIZE];
>> extern void tlb_init(int cpu);
>> extern void cpu_cache_init(void);
>> extern void cache_error_setup(void);
>> diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
>> index a31329971133..16dcd673e905 100644
>> --- a/arch/loongarch/kernel/efi.c
>> +++ b/arch/loongarch/kernel/efi.c
>> @@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[] __initdata = {
>>
>> void __init efi_runtime_init(void)
>> {
>> - if (!efi_enabled(EFI_BOOT))
>> + if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
>
> How can this happen? Why would we ever set EFI_BOOT if
> efi_systab->runtime == NULL?
>
>> return;
>>
>> if (efi_runtime_disabled()) {
>> @@ -101,3 +101,28 @@ void __init efi_init(void)
>> early_memunmap(tbl, sizeof(*tbl));
>> }
>> }
>> +
>> +/* parse the fdt pointer from efi system table */
>> +void __init *efi_fdt_pointer(void)
>> +{
>> + int i;
>> + void *fdt = NULL;
>> + efi_config_table_t *tables;
>> +
>> + if (!efi_systab) {
>> + pr_err("Can't find EFI system table.\n");
>> + return NULL;
>> + }
>> +
>> + tables = (efi_config_table_t *) efi_systab->tables;
>> +
>> + for (i = 0; i < efi_systab->nr_tables; i++)
>> + if (efi_guidcmp(tables[i].guid, DEVICE_TREE_GUID) == 0) {
>> + fdt = (void *)early_memremap_ro(
>> + (resource_size_t)tables[i].table,
>> + sizeof(efi_config_table_t));
>> + break;
>> + }
>> +
>> + return fdt;
>> +}
>
Hi Ard:
> Please use the existing config table discovery mechanism for this.
Sorry, Regarding the mechanism related to config table discovery you
mentioned, I only found the get_fdt() or get_efi_config_table()
interfaces in driver/firmware/efi/libstub. Unfortunately, both of them
cannot be referenced in our case.
So which mechanism are you referring to specifically?
Thanks.
Binbin
>
>> diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
>> index 6d56a463b091..4fd6eb9398b4 100644
>> --- a/arch/loongarch/kernel/env.c
>> +++ b/arch/loongarch/kernel/env.c
>> @@ -11,6 +11,7 @@
>> #include <asm/early_ioremap.h>
>> #include <asm/bootinfo.h>
>> #include <asm/loongson.h>
>> +#include <asm/setup.h>
>>
>> u64 efi_system_table;
>> struct loongson_system_configuration loongson_sysconf;
>> @@ -27,6 +28,8 @@ void __init init_environ(void)
>> clear_bit(EFI_BOOT, &efi.flags);
>>
>> strscpy(boot_command_line, cmdline, COMMAND_LINE_SIZE);
>> + /* Save unparsed command line copy for FDT */
>
> Why is this necessary?
>
>> + strscpy(original_cmdline, boot_command_line, COMMAND_LINE_SIZE);
>> early_memunmap(cmdline, COMMAND_LINE_SIZE);
>>
>> efi_system_table = fw_arg2;
>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
>> index 1eb63fa9bc81..2f578f809a54 100644
>> --- a/arch/loongarch/kernel/setup.c
>> +++ b/arch/loongarch/kernel/setup.c
>> @@ -29,6 +29,9 @@
>> #include <linux/device.h>
>> #include <linux/dma-map-ops.h>
>> #include <linux/swiotlb.h>
>> +#include <linux/of_fdt.h>
>> +#include <linux/of_address.h>
>> +#include <linux/libfdt.h>
>>
>> #include <asm/addrspace.h>
>> #include <asm/bootinfo.h>
>> @@ -67,6 +70,7 @@ static const char dmi_empty_string[] = " ";
>> *
>> * These are initialized so they are in the .data section
>> */
>> +char original_cmdline[COMMAND_LINE_SIZE] __initdata;
>>
>> static int num_standard_resources;
>> static struct resource *standard_resources;
>> @@ -281,6 +285,47 @@ static void __init check_kernel_sections_mem(void)
>> }
>> }
>>
>> +static void __init bootcmdline_init(char **cmdline_p)
>> +{
>> + /*
>> + * If CONFIG_CMDLINE_FORCE is enabled then initializing the command line
>> + * is trivial - we simply use the built-in command line unconditionally &
>> + * unmodified.
>> + */
>> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
>> + strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
>> + return;
>> + }
>> +
>> +#ifdef CONFIG_OF_FLATTREE
>> + /*
>> + * This is for CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER.
>> + *
>> + * If CONFIG_CMDLINE_BOOTLOADER is enabled, we need to determine if the
>> + * system is ACPI-based or FDT-based.
>> + *
>> + * For ACPI-based system: nothing need to do.
>> + *
>> + * For FDT-based system: the boot_command_line will be overwritten by
>> + * early_init_dt_scan_chosen(), so we need to append original_cmdline(the
>> + * original copy of boot_command_line) to boot_command_line.
>> + *
>> + * As for the built-in command line, if CONFIG_CMDLINE_EXTEND is enabled,
>> + * both ACPI-based and FDT-based system will append the built-in command
>> + * line to the boot_command_line themselves, and no additional processing
>> + * is required here.
>> + */
>> + if (initial_boot_params) {
>> + if (boot_command_line[0])
>> + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
>> +
>> + strlcat(boot_command_line, original_cmdline, COMMAND_LINE_SIZE);
>> + }
>> +#endif
>> +
>> + *cmdline_p = boot_command_line;
>> +}
>> +
>> /*
>> * arch_mem_init - initialize memory management subsystem
>> */
>> @@ -291,6 +336,8 @@ static void __init arch_mem_init(char **cmdline_p)
>>
>> check_kernel_sections_mem();
>>
>> + early_init_fdt_scan_reserved_mem();
>> +
>> /*
>> * In order to reduce the possibility of kernel panic when failed to
>> * get IO TLB memory under CONFIG_SWIOTLB, it is better to allocate
>> @@ -413,20 +460,44 @@ static void __init prefill_possible_map(void)
>> }
>> #endif
>>
>> +static void __init fdt_setup(void)
>> +{
>> +#ifdef CONFIG_OF_EARLY_FLATTREE
>> + void *fdt_pointer;
>> +
>> + /* ACPI-based systems do not require parsing fdt */
>> + if (acpi_os_get_root_pointer())
>> + return;
>> +
>> + /* Look for a device tree configuration table entry */
>> + fdt_pointer = efi_fdt_pointer();
>> + if (!fdt_pointer || fdt_check_header(fdt_pointer))
>> + return;
>> +
>> + early_init_dt_scan(fdt_pointer);
>> + early_init_fdt_reserve_self();
>> + early_init_dt_scan_chosen_stdout();
>> +
>
> The FDT is only a hardware description, right? So why are we scanning
> /chosen here?
>
>> + max_low_pfn = PFN_PHYS(memblock_end_of_DRAM());
>> +#endif
>> +}
>> +
>> void __init setup_arch(char **cmdline_p)
>> {
>> cpu_probe();
>> - *cmdline_p = boot_command_line;
>>
>> + pagetable_init();
>> init_environ();
>> efi_init();
>> + fdt_setup();
>> memblock_init();
>> - pagetable_init();
>> + bootcmdline_init(cmdline_p);
>> parse_early_param();
>> reserve_initrd_mem();
>>
>> platform_init();
>> arch_mem_init(cmdline_p);
>> + unflatten_and_copy_device_tree();
>>
>> resource_init();
>> #ifdef CONFIG_SMP
>> --
>> 2.31.1
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-07 13:06 ` Binbin Zhou
@ 2022-11-07 13:24 ` Ard Biesheuvel
0 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2022-11-07 13:24 UTC (permalink / raw)
To: Binbin Zhou; +Cc: Huacai Chen, WANG Xuerui, Jiaxun Yang, Jianmin Lv, loongarch
On Mon, 7 Nov 2022 at 14:07, Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>
>
>
> 在 2022/11/6 23:06, Ard Biesheuvel 写道:
> > On Sun, 6 Nov 2022 at 07:58, Binbin Zhou <zhoubinbin@loongson.cn> wrote:
> >>
> >> Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
> >> flattened DT"), we can parse the FDT from efi system table.
> >>
> >> And now, the Loongson-2K series cpus based on LoongArch support booting
> >> with FDT, so we adds the relevant booting support as well as parameter
> >> parsing.
> >>
> >> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> >> ---
> >> arch/loongarch/Kconfig | 2 +
> >> arch/loongarch/include/asm/efi.h | 1 +
> >> arch/loongarch/include/asm/setup.h | 1 +
> >> arch/loongarch/kernel/efi.c | 27 ++++++++++-
> >> arch/loongarch/kernel/env.c | 3 ++
> >> arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
> >> 6 files changed, 106 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> >> index 903096bd87f8..ce53d1fa2b64 100644
> >> --- a/arch/loongarch/Kconfig
> >> +++ b/arch/loongarch/Kconfig
> >> @@ -111,6 +111,8 @@ config LOONGARCH
> >> select MODULES_USE_ELF_RELA if MODULES
> >> select NEED_PER_CPU_EMBED_FIRST_CHUNK
> >> select NEED_PER_CPU_PAGE_FIRST_CHUNK
> >> + select OF
> >> + select OF_EARLY_FLATTREE
> >> select PCI
> >> select PCI_DOMAINS_GENERIC
> >> select PCI_ECAM if ACPI
> >> diff --git a/arch/loongarch/include/asm/efi.h b/arch/loongarch/include/asm/efi.h
> >> index 174567b00ddb..81e5d3371868 100644
> >> --- a/arch/loongarch/include/asm/efi.h
> >> +++ b/arch/loongarch/include/asm/efi.h
> >> @@ -9,6 +9,7 @@
> >>
> >> void __init efi_init(void);
> >> void __init efi_runtime_init(void);
> >> +void __init *efi_fdt_pointer(void);
> >> void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
> >>
> >> #define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
> >> diff --git a/arch/loongarch/include/asm/setup.h b/arch/loongarch/include/asm/setup.h
> >> index ca373f8e3c4d..b18baf878ed4 100644
> >> --- a/arch/loongarch/include/asm/setup.h
> >> +++ b/arch/loongarch/include/asm/setup.h
> >> @@ -13,6 +13,7 @@
> >>
> >> extern unsigned long eentry;
> >> extern unsigned long tlbrentry;
> >> +extern char original_cmdline[COMMAND_LINE_SIZE];
> >> extern void tlb_init(int cpu);
> >> extern void cpu_cache_init(void);
> >> extern void cache_error_setup(void);
> >> diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
> >> index a31329971133..16dcd673e905 100644
> >> --- a/arch/loongarch/kernel/efi.c
> >> +++ b/arch/loongarch/kernel/efi.c
> >> @@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[] __initdata = {
> >>
> >> void __init efi_runtime_init(void)
> >> {
> >> - if (!efi_enabled(EFI_BOOT))
> >> + if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
> >
> > How can this happen? Why would we ever set EFI_BOOT if
> > efi_systab->runtime == NULL?
> >
> >> return;
> >>
> >> if (efi_runtime_disabled()) {
> >> @@ -101,3 +101,28 @@ void __init efi_init(void)
> >> early_memunmap(tbl, sizeof(*tbl));
> >> }
> >> }
> >> +
> >> +/* parse the fdt pointer from efi system table */
> >> +void __init *efi_fdt_pointer(void)
> >> +{
> >> + int i;
> >> + void *fdt = NULL;
> >> + efi_config_table_t *tables;
> >> +
> >> + if (!efi_systab) {
> >> + pr_err("Can't find EFI system table.\n");
> >> + return NULL;
> >> + }
> >> +
> >> + tables = (efi_config_table_t *) efi_systab->tables;
> >> +
> >> + for (i = 0; i < efi_systab->nr_tables; i++)
> >> + if (efi_guidcmp(tables[i].guid, DEVICE_TREE_GUID) == 0) {
> >> + fdt = (void *)early_memremap_ro(
> >> + (resource_size_t)tables[i].table,
> >> + sizeof(efi_config_table_t));
> >> + break;
> >> + }
> >> +
> >> + return fdt;
> >> +}
> >
> Hi Ard:
>
> > Please use the existing config table discovery mechanism for this.
>
> Sorry, Regarding the mechanism related to config table discovery you
> mentioned, I only found the get_fdt() or get_efi_config_table()
> interfaces in driver/firmware/efi/libstub. Unfortunately, both of them
> cannot be referenced in our case.
>
> So which mechanism are you referring to specifically?
>
Please look at how the existing LoongArch code uses efi_config_parse_tables()
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-07 4:23 ` Huacai Chen
@ 2022-11-07 13:33 ` Ard Biesheuvel
2022-11-08 1:16 ` Huacai Chen
0 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2022-11-07 13:33 UTC (permalink / raw)
To: Huacai Chen; +Cc: Binbin Zhou, WANG Xuerui, Jiaxun Yang, Jianmin Lv, loongarch
On Mon, 7 Nov 2022 at 05:24, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi, Ard,
>
> On Sun, Nov 6, 2022 at 11:06 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Sun, 6 Nov 2022 at 07:58, Binbin Zhou <zhoubinbin@loongson.cn> wrote:
> > >
> > > Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
> > > flattened DT"), we can parse the FDT from efi system table.
> > >
> > > And now, the Loongson-2K series cpus based on LoongArch support booting
> > > with FDT, so we adds the relevant booting support as well as parameter
> > > parsing.
> > >
> > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > ---
> > > arch/loongarch/Kconfig | 2 +
> > > arch/loongarch/include/asm/efi.h | 1 +
> > > arch/loongarch/include/asm/setup.h | 1 +
> > > arch/loongarch/kernel/efi.c | 27 ++++++++++-
> > > arch/loongarch/kernel/env.c | 3 ++
> > > arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
> > > 6 files changed, 106 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> > > index 903096bd87f8..ce53d1fa2b64 100644
> > > --- a/arch/loongarch/Kconfig
> > > +++ b/arch/loongarch/Kconfig
> > > @@ -111,6 +111,8 @@ config LOONGARCH
> > > select MODULES_USE_ELF_RELA if MODULES
> > > select NEED_PER_CPU_EMBED_FIRST_CHUNK
> > > select NEED_PER_CPU_PAGE_FIRST_CHUNK
> > > + select OF
> > > + select OF_EARLY_FLATTREE
> > > select PCI
> > > select PCI_DOMAINS_GENERIC
> > > select PCI_ECAM if ACPI
> > > diff --git a/arch/loongarch/include/asm/efi.h b/arch/loongarch/include/asm/efi.h
> > > index 174567b00ddb..81e5d3371868 100644
> > > --- a/arch/loongarch/include/asm/efi.h
> > > +++ b/arch/loongarch/include/asm/efi.h
> > > @@ -9,6 +9,7 @@
> > >
> > > void __init efi_init(void);
> > > void __init efi_runtime_init(void);
> > > +void __init *efi_fdt_pointer(void);
> > > void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
> > >
> > > #define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
> > > diff --git a/arch/loongarch/include/asm/setup.h b/arch/loongarch/include/asm/setup.h
> > > index ca373f8e3c4d..b18baf878ed4 100644
> > > --- a/arch/loongarch/include/asm/setup.h
> > > +++ b/arch/loongarch/include/asm/setup.h
> > > @@ -13,6 +13,7 @@
> > >
> > > extern unsigned long eentry;
> > > extern unsigned long tlbrentry;
> > > +extern char original_cmdline[COMMAND_LINE_SIZE];
> > > extern void tlb_init(int cpu);
> > > extern void cpu_cache_init(void);
> > > extern void cache_error_setup(void);
> > > diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
> > > index a31329971133..16dcd673e905 100644
> > > --- a/arch/loongarch/kernel/efi.c
> > > +++ b/arch/loongarch/kernel/efi.c
> > > @@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[] __initdata = {
> > >
> > > void __init efi_runtime_init(void)
> > > {
> > > - if (!efi_enabled(EFI_BOOT))
> > > + if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
> >
> > How can this happen? Why would we ever set EFI_BOOT if
> > efi_systab->runtime == NULL?
> >
> > > return;
> > >
> > > if (efi_runtime_disabled()) {
> > > @@ -101,3 +101,28 @@ void __init efi_init(void)
> > > early_memunmap(tbl, sizeof(*tbl));
> > > }
> > > }
> > > +
> > > +/* parse the fdt pointer from efi system table */
> > > +void __init *efi_fdt_pointer(void)
> > > +{
> > > + int i;
> > > + void *fdt = NULL;
> > > + efi_config_table_t *tables;
> > > +
> > > + if (!efi_systab) {
> > > + pr_err("Can't find EFI system table.\n");
> > > + return NULL;
> > > + }
> > > +
> > > + tables = (efi_config_table_t *) efi_systab->tables;
> > > +
> > > + for (i = 0; i < efi_systab->nr_tables; i++)
> > > + if (efi_guidcmp(tables[i].guid, DEVICE_TREE_GUID) == 0) {
> > > + fdt = (void *)early_memremap_ro(
> > > + (resource_size_t)tables[i].table,
> > > + sizeof(efi_config_table_t));
> > > + break;
> > > + }
> > > +
> > > + return fdt;
> > > +}
> >
> > Please use the existing config table discovery mechanism for this.
> >
> > > diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
> > > index 6d56a463b091..4fd6eb9398b4 100644
> > > --- a/arch/loongarch/kernel/env.c
> > > +++ b/arch/loongarch/kernel/env.c
> > > @@ -11,6 +11,7 @@
> > > #include <asm/early_ioremap.h>
> > > #include <asm/bootinfo.h>
> > > #include <asm/loongson.h>
> > > +#include <asm/setup.h>
> > >
> > > u64 efi_system_table;
> > > struct loongson_system_configuration loongson_sysconf;
> > > @@ -27,6 +28,8 @@ void __init init_environ(void)
> > > clear_bit(EFI_BOOT, &efi.flags);
> > >
> > > strscpy(boot_command_line, cmdline, COMMAND_LINE_SIZE);
> > > + /* Save unparsed command line copy for FDT */
> >
> > Why is this necessary?
> >
> > > + strscpy(original_cmdline, boot_command_line, COMMAND_LINE_SIZE);
> > > early_memunmap(cmdline, COMMAND_LINE_SIZE);
> > >
> > > efi_system_table = fw_arg2;
> > > diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> > > index 1eb63fa9bc81..2f578f809a54 100644
> > > --- a/arch/loongarch/kernel/setup.c
> > > +++ b/arch/loongarch/kernel/setup.c
> > > @@ -29,6 +29,9 @@
> > > #include <linux/device.h>
> > > #include <linux/dma-map-ops.h>
> > > #include <linux/swiotlb.h>
> > > +#include <linux/of_fdt.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/libfdt.h>
> > >
> > > #include <asm/addrspace.h>
> > > #include <asm/bootinfo.h>
> > > @@ -67,6 +70,7 @@ static const char dmi_empty_string[] = " ";
> > > *
> > > * These are initialized so they are in the .data section
> > > */
> > > +char original_cmdline[COMMAND_LINE_SIZE] __initdata;
> > >
> > > static int num_standard_resources;
> > > static struct resource *standard_resources;
> > > @@ -281,6 +285,47 @@ static void __init check_kernel_sections_mem(void)
> > > }
> > > }
> > >
> > > +static void __init bootcmdline_init(char **cmdline_p)
> > > +{
> > > + /*
> > > + * If CONFIG_CMDLINE_FORCE is enabled then initializing the command line
> > > + * is trivial - we simply use the built-in command line unconditionally &
> > > + * unmodified.
> > > + */
> > > + if (IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
> > > + strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > + return;
> > > + }
> > > +
> > > +#ifdef CONFIG_OF_FLATTREE
> > > + /*
> > > + * This is for CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER.
> > > + *
> > > + * If CONFIG_CMDLINE_BOOTLOADER is enabled, we need to determine if the
> > > + * system is ACPI-based or FDT-based.
> > > + *
> > > + * For ACPI-based system: nothing need to do.
> > > + *
> > > + * For FDT-based system: the boot_command_line will be overwritten by
> > > + * early_init_dt_scan_chosen(), so we need to append original_cmdline(the
> > > + * original copy of boot_command_line) to boot_command_line.
> > > + *
> > > + * As for the built-in command line, if CONFIG_CMDLINE_EXTEND is enabled,
> > > + * both ACPI-based and FDT-based system will append the built-in command
> > > + * line to the boot_command_line themselves, and no additional processing
> > > + * is required here.
> > > + */
> > > + if (initial_boot_params) {
> > > + if (boot_command_line[0])
> > > + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> > > +
> > > + strlcat(boot_command_line, original_cmdline, COMMAND_LINE_SIZE);
> > > + }
> > > +#endif
> > > +
> > > + *cmdline_p = boot_command_line;
> > > +}
> > > +
> > > /*
> > > * arch_mem_init - initialize memory management subsystem
> > > */
> > > @@ -291,6 +336,8 @@ static void __init arch_mem_init(char **cmdline_p)
> > >
> > > check_kernel_sections_mem();
> > >
> > > + early_init_fdt_scan_reserved_mem();
> > > +
> > > /*
> > > * In order to reduce the possibility of kernel panic when failed to
> > > * get IO TLB memory under CONFIG_SWIOTLB, it is better to allocate
> > > @@ -413,20 +460,44 @@ static void __init prefill_possible_map(void)
> > > }
> > > #endif
> > >
> > > +static void __init fdt_setup(void)
> > > +{
> > > +#ifdef CONFIG_OF_EARLY_FLATTREE
> > > + void *fdt_pointer;
> > > +
> > > + /* ACPI-based systems do not require parsing fdt */
> > > + if (acpi_os_get_root_pointer())
> > > + return;
> > > +
> > > + /* Look for a device tree configuration table entry */
> > > + fdt_pointer = efi_fdt_pointer();
> > > + if (!fdt_pointer || fdt_check_header(fdt_pointer))
> > > + return;
> > > +
> > > + early_init_dt_scan(fdt_pointer);
> > > + early_init_fdt_reserve_self();
> > > + early_init_dt_scan_chosen_stdout();
> > > +
> >
> > The FDT is only a hardware description, right? So why are we scanning
> > /chosen here?
> We scan the chosen node here in order to configure earlycon as early
> as possible, this is very useful for debugging. ARM64 does similar
> things in acpi_boot_table_init(), but I think fdt_setup() is more
> suitable.
>
Do you mean for the /chosen/stdout-path node?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-07 13:33 ` Ard Biesheuvel
@ 2022-11-08 1:16 ` Huacai Chen
0 siblings, 0 replies; 17+ messages in thread
From: Huacai Chen @ 2022-11-08 1:16 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Binbin Zhou, WANG Xuerui, Jiaxun Yang, Jianmin Lv, loongarch
On Mon, Nov 7, 2022 at 9:33 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 7 Nov 2022 at 05:24, Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > Hi, Ard,
> >
> > On Sun, Nov 6, 2022 at 11:06 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Sun, 6 Nov 2022 at 07:58, Binbin Zhou <zhoubinbin@loongson.cn> wrote:
> > > >
> > > > Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
> > > > flattened DT"), we can parse the FDT from efi system table.
> > > >
> > > > And now, the Loongson-2K series cpus based on LoongArch support booting
> > > > with FDT, so we adds the relevant booting support as well as parameter
> > > > parsing.
> > > >
> > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > > ---
> > > > arch/loongarch/Kconfig | 2 +
> > > > arch/loongarch/include/asm/efi.h | 1 +
> > > > arch/loongarch/include/asm/setup.h | 1 +
> > > > arch/loongarch/kernel/efi.c | 27 ++++++++++-
> > > > arch/loongarch/kernel/env.c | 3 ++
> > > > arch/loongarch/kernel/setup.c | 75 +++++++++++++++++++++++++++++-
> > > > 6 files changed, 106 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> > > > index 903096bd87f8..ce53d1fa2b64 100644
> > > > --- a/arch/loongarch/Kconfig
> > > > +++ b/arch/loongarch/Kconfig
> > > > @@ -111,6 +111,8 @@ config LOONGARCH
> > > > select MODULES_USE_ELF_RELA if MODULES
> > > > select NEED_PER_CPU_EMBED_FIRST_CHUNK
> > > > select NEED_PER_CPU_PAGE_FIRST_CHUNK
> > > > + select OF
> > > > + select OF_EARLY_FLATTREE
> > > > select PCI
> > > > select PCI_DOMAINS_GENERIC
> > > > select PCI_ECAM if ACPI
> > > > diff --git a/arch/loongarch/include/asm/efi.h b/arch/loongarch/include/asm/efi.h
> > > > index 174567b00ddb..81e5d3371868 100644
> > > > --- a/arch/loongarch/include/asm/efi.h
> > > > +++ b/arch/loongarch/include/asm/efi.h
> > > > @@ -9,6 +9,7 @@
> > > >
> > > > void __init efi_init(void);
> > > > void __init efi_runtime_init(void);
> > > > +void __init *efi_fdt_pointer(void);
> > > > void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
> > > >
> > > > #define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
> > > > diff --git a/arch/loongarch/include/asm/setup.h b/arch/loongarch/include/asm/setup.h
> > > > index ca373f8e3c4d..b18baf878ed4 100644
> > > > --- a/arch/loongarch/include/asm/setup.h
> > > > +++ b/arch/loongarch/include/asm/setup.h
> > > > @@ -13,6 +13,7 @@
> > > >
> > > > extern unsigned long eentry;
> > > > extern unsigned long tlbrentry;
> > > > +extern char original_cmdline[COMMAND_LINE_SIZE];
> > > > extern void tlb_init(int cpu);
> > > > extern void cpu_cache_init(void);
> > > > extern void cache_error_setup(void);
> > > > diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
> > > > index a31329971133..16dcd673e905 100644
> > > > --- a/arch/loongarch/kernel/efi.c
> > > > +++ b/arch/loongarch/kernel/efi.c
> > > > @@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[] __initdata = {
> > > >
> > > > void __init efi_runtime_init(void)
> > > > {
> > > > - if (!efi_enabled(EFI_BOOT))
> > > > + if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
> > >
> > > How can this happen? Why would we ever set EFI_BOOT if
> > > efi_systab->runtime == NULL?
> > >
> > > > return;
> > > >
> > > > if (efi_runtime_disabled()) {
> > > > @@ -101,3 +101,28 @@ void __init efi_init(void)
> > > > early_memunmap(tbl, sizeof(*tbl));
> > > > }
> > > > }
> > > > +
> > > > +/* parse the fdt pointer from efi system table */
> > > > +void __init *efi_fdt_pointer(void)
> > > > +{
> > > > + int i;
> > > > + void *fdt = NULL;
> > > > + efi_config_table_t *tables;
> > > > +
> > > > + if (!efi_systab) {
> > > > + pr_err("Can't find EFI system table.\n");
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + tables = (efi_config_table_t *) efi_systab->tables;
> > > > +
> > > > + for (i = 0; i < efi_systab->nr_tables; i++)
> > > > + if (efi_guidcmp(tables[i].guid, DEVICE_TREE_GUID) == 0) {
> > > > + fdt = (void *)early_memremap_ro(
> > > > + (resource_size_t)tables[i].table,
> > > > + sizeof(efi_config_table_t));
> > > > + break;
> > > > + }
> > > > +
> > > > + return fdt;
> > > > +}
> > >
> > > Please use the existing config table discovery mechanism for this.
> > >
> > > > diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
> > > > index 6d56a463b091..4fd6eb9398b4 100644
> > > > --- a/arch/loongarch/kernel/env.c
> > > > +++ b/arch/loongarch/kernel/env.c
> > > > @@ -11,6 +11,7 @@
> > > > #include <asm/early_ioremap.h>
> > > > #include <asm/bootinfo.h>
> > > > #include <asm/loongson.h>
> > > > +#include <asm/setup.h>
> > > >
> > > > u64 efi_system_table;
> > > > struct loongson_system_configuration loongson_sysconf;
> > > > @@ -27,6 +28,8 @@ void __init init_environ(void)
> > > > clear_bit(EFI_BOOT, &efi.flags);
> > > >
> > > > strscpy(boot_command_line, cmdline, COMMAND_LINE_SIZE);
> > > > + /* Save unparsed command line copy for FDT */
> > >
> > > Why is this necessary?
> > >
> > > > + strscpy(original_cmdline, boot_command_line, COMMAND_LINE_SIZE);
> > > > early_memunmap(cmdline, COMMAND_LINE_SIZE);
> > > >
> > > > efi_system_table = fw_arg2;
> > > > diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> > > > index 1eb63fa9bc81..2f578f809a54 100644
> > > > --- a/arch/loongarch/kernel/setup.c
> > > > +++ b/arch/loongarch/kernel/setup.c
> > > > @@ -29,6 +29,9 @@
> > > > #include <linux/device.h>
> > > > #include <linux/dma-map-ops.h>
> > > > #include <linux/swiotlb.h>
> > > > +#include <linux/of_fdt.h>
> > > > +#include <linux/of_address.h>
> > > > +#include <linux/libfdt.h>
> > > >
> > > > #include <asm/addrspace.h>
> > > > #include <asm/bootinfo.h>
> > > > @@ -67,6 +70,7 @@ static const char dmi_empty_string[] = " ";
> > > > *
> > > > * These are initialized so they are in the .data section
> > > > */
> > > > +char original_cmdline[COMMAND_LINE_SIZE] __initdata;
> > > >
> > > > static int num_standard_resources;
> > > > static struct resource *standard_resources;
> > > > @@ -281,6 +285,47 @@ static void __init check_kernel_sections_mem(void)
> > > > }
> > > > }
> > > >
> > > > +static void __init bootcmdline_init(char **cmdline_p)
> > > > +{
> > > > + /*
> > > > + * If CONFIG_CMDLINE_FORCE is enabled then initializing the command line
> > > > + * is trivial - we simply use the built-in command line unconditionally &
> > > > + * unmodified.
> > > > + */
> > > > + if (IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
> > > > + strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > > > + return;
> > > > + }
> > > > +
> > > > +#ifdef CONFIG_OF_FLATTREE
> > > > + /*
> > > > + * This is for CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER.
> > > > + *
> > > > + * If CONFIG_CMDLINE_BOOTLOADER is enabled, we need to determine if the
> > > > + * system is ACPI-based or FDT-based.
> > > > + *
> > > > + * For ACPI-based system: nothing need to do.
> > > > + *
> > > > + * For FDT-based system: the boot_command_line will be overwritten by
> > > > + * early_init_dt_scan_chosen(), so we need to append original_cmdline(the
> > > > + * original copy of boot_command_line) to boot_command_line.
> > > > + *
> > > > + * As for the built-in command line, if CONFIG_CMDLINE_EXTEND is enabled,
> > > > + * both ACPI-based and FDT-based system will append the built-in command
> > > > + * line to the boot_command_line themselves, and no additional processing
> > > > + * is required here.
> > > > + */
> > > > + if (initial_boot_params) {
> > > > + if (boot_command_line[0])
> > > > + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> > > > +
> > > > + strlcat(boot_command_line, original_cmdline, COMMAND_LINE_SIZE);
> > > > + }
> > > > +#endif
> > > > +
> > > > + *cmdline_p = boot_command_line;
> > > > +}
> > > > +
> > > > /*
> > > > * arch_mem_init - initialize memory management subsystem
> > > > */
> > > > @@ -291,6 +336,8 @@ static void __init arch_mem_init(char **cmdline_p)
> > > >
> > > > check_kernel_sections_mem();
> > > >
> > > > + early_init_fdt_scan_reserved_mem();
> > > > +
> > > > /*
> > > > * In order to reduce the possibility of kernel panic when failed to
> > > > * get IO TLB memory under CONFIG_SWIOTLB, it is better to allocate
> > > > @@ -413,20 +460,44 @@ static void __init prefill_possible_map(void)
> > > > }
> > > > #endif
> > > >
> > > > +static void __init fdt_setup(void)
> > > > +{
> > > > +#ifdef CONFIG_OF_EARLY_FLATTREE
> > > > + void *fdt_pointer;
> > > > +
> > > > + /* ACPI-based systems do not require parsing fdt */
> > > > + if (acpi_os_get_root_pointer())
> > > > + return;
> > > > +
> > > > + /* Look for a device tree configuration table entry */
> > > > + fdt_pointer = efi_fdt_pointer();
> > > > + if (!fdt_pointer || fdt_check_header(fdt_pointer))
> > > > + return;
> > > > +
> > > > + early_init_dt_scan(fdt_pointer);
> > > > + early_init_fdt_reserve_self();
> > > > + early_init_dt_scan_chosen_stdout();
> > > > +
> > >
> > > The FDT is only a hardware description, right? So why are we scanning
> > > /chosen here?
> > We scan the chosen node here in order to configure earlycon as early
> > as possible, this is very useful for debugging. ARM64 does similar
> > things in acpi_boot_table_init(), but I think fdt_setup() is more
> > suitable.
> >
>
> Do you mean for the /chosen/stdout-path node?
Yes.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] LoongArch: Support booting with FDT from efi system table
2022-11-07 8:44 ` Jianmin Lv
@ 2022-11-21 6:44 ` Huacai Chen
0 siblings, 0 replies; 17+ messages in thread
From: Huacai Chen @ 2022-11-21 6:44 UTC (permalink / raw)
To: Jianmin Lv, Ard Biesheuvel
Cc: WANG Xuerui, Binbin Zhou, Jiaxun Yang, loongarch
Hi, Ard,
On Mon, Nov 7, 2022 at 4:44 PM Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
>
>
> On 2022/11/7 上午10:50, WANG Xuerui wrote:
> > On 2022/11/7 10:07, Jianmin Lv wrote:
> >>
> >>
> >> On 2022/11/6 下午11:06, Ard Biesheuvel wrote:
> >>> On Sun, 6 Nov 2022 at 07:58, Binbin Zhou <zhoubinbin@loongson.cn> wrote:
> >>>>
> >>>> Since commit 40cd01a9c324("efi/loongarch: libstub: remove dependency on
> >>>> flattened DT"), we can parse the FDT from efi system table.
> >>>>
> >>>> And now, the Loongson-2K series cpus based on LoongArch support booting
> >>>> with FDT, so we adds the relevant booting support as well as parameter
> >>>> parsing.
> >>>>
> >>>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> >>>> ---
> >>>> arch/loongarch/Kconfig | 2 +
> >>>> arch/loongarch/include/asm/efi.h | 1 +
> >>>> arch/loongarch/include/asm/setup.h | 1 +
> >>>> arch/loongarch/kernel/efi.c | 27 ++++++++++-
> >>>> arch/loongarch/kernel/env.c | 3 ++
> >>>> arch/loongarch/kernel/setup.c | 75
> >>>> +++++++++++++++++++++++++++++-
> >>>> 6 files changed, 106 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> >>>> index 903096bd87f8..ce53d1fa2b64 100644
> >>>> --- a/arch/loongarch/Kconfig
> >>>> +++ b/arch/loongarch/Kconfig
> >>>> @@ -111,6 +111,8 @@ config LOONGARCH
> >>>> select MODULES_USE_ELF_RELA if MODULES
> >>>> select NEED_PER_CPU_EMBED_FIRST_CHUNK
> >>>> select NEED_PER_CPU_PAGE_FIRST_CHUNK
> >>>> + select OF
> >>>> + select OF_EARLY_FLATTREE
> >>>> select PCI
> >>>> select PCI_DOMAINS_GENERIC
> >>>> select PCI_ECAM if ACPI
> >>>> diff --git a/arch/loongarch/include/asm/efi.h
> >>>> b/arch/loongarch/include/asm/efi.h
> >>>> index 174567b00ddb..81e5d3371868 100644
> >>>> --- a/arch/loongarch/include/asm/efi.h
> >>>> +++ b/arch/loongarch/include/asm/efi.h
> >>>> @@ -9,6 +9,7 @@
> >>>>
> >>>> void __init efi_init(void);
> >>>> void __init efi_runtime_init(void);
> >>>> +void __init *efi_fdt_pointer(void);
> >>>> void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
> >>>>
> >>>> #define ARCH_EFI_IRQ_FLAGS_MASK 0x00000004 /* Bit 2: CSR.CRMD.IE */
> >>>> diff --git a/arch/loongarch/include/asm/setup.h
> >>>> b/arch/loongarch/include/asm/setup.h
> >>>> index ca373f8e3c4d..b18baf878ed4 100644
> >>>> --- a/arch/loongarch/include/asm/setup.h
> >>>> +++ b/arch/loongarch/include/asm/setup.h
> >>>> @@ -13,6 +13,7 @@
> >>>>
> >>>> extern unsigned long eentry;
> >>>> extern unsigned long tlbrentry;
> >>>> +extern char original_cmdline[COMMAND_LINE_SIZE];
> >>>> extern void tlb_init(int cpu);
> >>>> extern void cpu_cache_init(void);
> >>>> extern void cache_error_setup(void);
> >>>> diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
> >>>> index a31329971133..16dcd673e905 100644
> >>>> --- a/arch/loongarch/kernel/efi.c
> >>>> +++ b/arch/loongarch/kernel/efi.c
> >>>> @@ -37,7 +37,7 @@ static efi_config_table_type_t arch_tables[]
> >>>> __initdata = {
> >>>>
> >>>> void __init efi_runtime_init(void)
> >>>> {
> >>>> - if (!efi_enabled(EFI_BOOT))
> >>>> + if (!efi_enabled(EFI_BOOT) || !efi_systab->runtime)
> >>>
> >>> How can this happen? Why would we ever set EFI_BOOT if
> >>> efi_systab->runtime == NULL?
> >>>
> >>
> >> Because some firmware (such as PMON) employed UEFI compatible
> >> implementaion without runtime (and set runtime null pointer).
> >
> > Hmm, is it really "UEFI-compatible" when it's without a UEFI runtime?
>
> To be frank, it seems that runtime should be always valid in UEFI
> complaint firmware (according to UEFI spec section 4.5, 8).
> Unfortunately, some machines shipped with *partially-compatible"
> (without runtime) firmware have been outside (maybe not many, I'm not
> sure). So, thinking about compatibility with them, the case have to be
> taken account, unless rather.
>
> > Anyway it's becoming increasingly difficult for community reviewers to
> > check things without corresponding firmware sources available... (and I
> > think, given the Loongson fork of PMON even back in the MIPS era is
> > effectively GPL'd [1], the LoongArch port of PMON could probably be GPL
> > as well, in which case it's totally fair for us to request the sources.)
> >
> > (I know the similar usage pattern i.e. making use of various UEFI
> > features but without a runtime was already widespread back in the MIPS
> > days of Loongson, but personally I don't think it's enough to justify
> > doing the same when it's 2022 and we're fully upstream.)
> >
> > [1]: https://github.com/loongson-community/pmon/blob/master/LICENSE.md
> >
> >> Maybe *noefi* in command line is enough for this without
> >> CONFIG_CMDLINE_FORCE simillar stuff configured?
> >
> > I think having/forcing users to add "noefi" themselves wouldn't be good
> > ergonomics? Manipulating the cmdline in the firmware could be more
> > acceptable but I'm not sure.
> >
>
> I think so, adding it in firmware internally (if using "noefi") maybe
> acceptable rather than adding it by users.
With Jianmin's explanation, do you think it is acceptable for
PMON-like firmware (emulates a systemtable but doesn't provide
runtime) to set efi boot flag to 1? Or the flag should be 0 unless
firmware is fully efi-compatible (has systemtable and runtime service
is available)?
Huacai
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-11-21 6:44 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-06 6:56 [PATCH] LoongArch: Support booting with FDT from efi system table Binbin Zhou
2022-11-06 7:55 ` Xi Ruoyao
2022-11-07 4:09 ` Huacai Chen
2022-11-06 9:15 ` WANG Xuerui
2022-11-06 10:15 ` Xi Ruoyao
2022-11-07 2:59 ` Binbin Zhou
2022-11-07 3:11 ` WANG Xuerui
2022-11-06 15:06 ` Ard Biesheuvel
2022-11-07 2:07 ` Jianmin Lv
2022-11-07 2:50 ` WANG Xuerui
2022-11-07 8:44 ` Jianmin Lv
2022-11-21 6:44 ` Huacai Chen
2022-11-07 4:23 ` Huacai Chen
2022-11-07 13:33 ` Ard Biesheuvel
2022-11-08 1:16 ` Huacai Chen
2022-11-07 13:06 ` Binbin Zhou
2022-11-07 13:24 ` Ard Biesheuvel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.