Message ID | 20230216182043.1946553-19-sunilvl@ventanamicro.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Add basic ACPI support for RISC-V | expand |
Context | Check | Description |
---|---|---|
conchuod/tree_selection | fail | Failed to apply to next/pending-fixes or riscv/for-next |
On Thu, Feb 16, 2023 at 11:50:40PM +0530, Sunil V L wrote: > Initialize the ACPI core for RISC-V during boot. > > ACPI tables and interpreter are initialized based on > the information passed from the firmware and the value of > the kernel parameter 'acpi'. > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > arch/riscv/kernel/acpi.c | 113 ++++++++++++++++++++++++++++++++++++++ > arch/riscv/kernel/setup.c | 25 ++++++--- > 2 files changed, 130 insertions(+), 8 deletions(-) > > diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c > index 13b26c87c136..35e7b24a30c8 100644 > --- a/arch/riscv/kernel/acpi.c > +++ b/arch/riscv/kernel/acpi.c > @@ -16,6 +16,7 @@ > #include <linux/acpi.h> > #include <linux/io.h> > #include <linux/pci.h> > +#include <linux/efi.h> > > int acpi_noirq = 1; /* skip ACPI IRQ initialization */ > int acpi_disabled = 1; > @@ -26,6 +27,118 @@ EXPORT_SYMBOL(acpi_pci_disabled); > > static unsigned int intc_count; > static struct acpi_madt_rintc cpu_madt_rintc[NR_CPUS]; > +static bool param_acpi_off __initdata; > +static bool param_acpi_on __initdata; > +static bool param_acpi_force __initdata; > + > +static int __init parse_acpi(char *arg) > +{ > + if (!arg) > + return -EINVAL; > + > + /* "acpi=off" disables both ACPI table parsing and interpreter */ > + if (strcmp(arg, "off") == 0) > + param_acpi_off = true; > + else if (strcmp(arg, "on") == 0) /* prefer ACPI over DT */ > + param_acpi_on = true; > + else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */ > + param_acpi_force = true; > + else > + return -EINVAL; /* Core will print when we return error */ > + > + return 0; > +} > +early_param("acpi", parse_acpi); > + > +/* > + * acpi_fadt_sanity_check() - Check FADT presence and carry out sanity > + * checks on it > + * > + * Return 0 on success, <0 on failure > + */ > +static int __init acpi_fadt_sanity_check(void) > +{ > + struct acpi_table_header *table; > + struct acpi_table_fadt *fadt; > + acpi_status status; > + int ret = 0; > + > + /* > + * FADT is required on riscv; retrieve it to check its presence > + * and carry out revision and ACPI HW reduced compliancy tests > + */ > + status = acpi_get_table(ACPI_SIG_FADT, 0, &table); > + if (ACPI_FAILURE(status)) { > + const char *msg = acpi_format_exception(status); > + > + pr_err("Failed to get FADT table, %s\n", msg); > + return -ENODEV; > + } > + > + fadt = (struct acpi_table_fadt *)table; > + > + if (!(fadt->flags & ACPI_FADT_HW_REDUCED)) { Do we also need to check for ACPI version 5.0+ when checking for HW reduced? > + pr_err("FADT not ACPI hardware reduced compliant\n"); > + ret = -EINVAL; > + } > + > + /* > + * acpi_get_table() creates FADT table mapping that > + * should be released after parsing and before resuming boot > + */ > + acpi_put_table(table); > + return ret; > +} > + > +/* > + * acpi_boot_table_init() called from setup_arch(), always. > + * 1. find RSDP and get its address, and then find XSDT > + * 2. extract all tables and checksums them all > + * 3. check ACPI FADT HW reduced flag > + * > + * We can parse ACPI boot-time tables such as MADT after > + * this function is called. > + * > + * On return ACPI is enabled if either: > + * > + * - ACPI tables are initialized and sanity checks passed > + * - acpi=force was passed in the command line and ACPI was not disabled > + * explicitly through acpi=off command line parameter > + * > + * ACPI is disabled on function return otherwise > + */ > +void __init acpi_boot_table_init(void) > +{ > + /* > + * Enable ACPI instead of device tree unless > + * - ACPI has been disabled explicitly (acpi=off), or > + * - firmware has not populated ACPI ptr in EFI system table > + * and ACPI has not been [force] enabled (acpi=on|force) > + */ > + if (param_acpi_off || > + (!param_acpi_on && !param_acpi_force && > + efi.acpi20 == EFI_INVALID_TABLE_ADDR)) > + return; > + > + /* > + * ACPI is disabled at this point. Enable it in order to parse > + * the ACPI tables and carry out sanity checks > + */ > + enable_acpi(); > + > + /* > + * If ACPI tables are initialized and FADT sanity checks passed, > + * leave ACPI enabled and carry on booting; otherwise disable ACPI > + * on initialization error. > + * If acpi=force was passed on the command line it forces ACPI > + * to be enabled even if its initialization failed. > + */ > + if (acpi_table_init() || acpi_fadt_sanity_check()) { > + pr_err("Failed to init ACPI tables\n"); > + if (!param_acpi_force) > + disable_acpi(); > + } > +} A lot of the above code is common with arm64. It'd be nice to share that, but maybe refactoring can be done on top of this. > > static int acpi_parse_madt_rintc(union acpi_subtable_headers *header, const unsigned long end) > { > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index 4335f08ffaf2..c2ee7f4427a1 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -8,6 +8,7 @@ > * Nick Kossifidis <mick@ics.forth.gr> > */ > > +#include <linux/acpi.h> > #include <linux/init.h> > #include <linux/mm.h> > #include <linux/memblock.h> > @@ -276,14 +277,22 @@ void __init setup_arch(char **cmdline_p) > > efi_init(); > paging_init(); > -#if IS_ENABLED(CONFIG_BUILTIN_DTB) > - unflatten_and_copy_device_tree(); > -#else > - if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa)))) > - unflatten_device_tree(); > - else > - pr_err("No DTB found in kernel mappings\n"); > -#endif > + > + /* Parse the ACPI tables for possible boot-time configuration */ > + acpi_boot_table_init(); > + if (acpi_disabled) { > + if (IS_ENABLED(CONFIG_BUILTIN_DTB)) { > + unflatten_and_copy_device_tree(); > + } else { > + if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa)))) > + unflatten_device_tree(); > + else > + pr_err("No DTB found in kernel mappings\n"); > + } > + } else { > + early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa))); > + } > + > early_init_fdt_scan_reserved_mem(); > misc_mem_init(); > > -- > 2.34.1 > Otherwise, Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
On Mon, Feb 20, 2023 at 09:07:43PM +0100, Andrew Jones wrote: > On Thu, Feb 16, 2023 at 11:50:40PM +0530, Sunil V L wrote: > > Initialize the ACPI core for RISC-V during boot. > > > > ACPI tables and interpreter are initialized based on > > the information passed from the firmware and the value of > > the kernel parameter 'acpi'. > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > arch/riscv/kernel/acpi.c | 113 ++++++++++++++++++++++++++++++++++++++ > > arch/riscv/kernel/setup.c | 25 ++++++--- > > 2 files changed, 130 insertions(+), 8 deletions(-) > > > > diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c > > index 13b26c87c136..35e7b24a30c8 100644 > > --- a/arch/riscv/kernel/acpi.c > > +++ b/arch/riscv/kernel/acpi.c > > @@ -16,6 +16,7 @@ > > #include <linux/acpi.h> > > #include <linux/io.h> > > #include <linux/pci.h> > > +#include <linux/efi.h> > > > > int acpi_noirq = 1; /* skip ACPI IRQ initialization */ > > int acpi_disabled = 1; > > @@ -26,6 +27,118 @@ EXPORT_SYMBOL(acpi_pci_disabled); > > > > static unsigned int intc_count; > > static struct acpi_madt_rintc cpu_madt_rintc[NR_CPUS]; > > +static bool param_acpi_off __initdata; > > +static bool param_acpi_on __initdata; > > +static bool param_acpi_force __initdata; > > + > > +static int __init parse_acpi(char *arg) > > +{ > > + if (!arg) > > + return -EINVAL; > > + > > + /* "acpi=off" disables both ACPI table parsing and interpreter */ > > + if (strcmp(arg, "off") == 0) > > + param_acpi_off = true; > > + else if (strcmp(arg, "on") == 0) /* prefer ACPI over DT */ > > + param_acpi_on = true; > > + else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */ > > + param_acpi_force = true; > > + else > > + return -EINVAL; /* Core will print when we return error */ > > + > > + return 0; > > +} > > +early_param("acpi", parse_acpi); > > + > > +/* > > + * acpi_fadt_sanity_check() - Check FADT presence and carry out sanity > > + * checks on it > > + * > > + * Return 0 on success, <0 on failure > > + */ > > +static int __init acpi_fadt_sanity_check(void) > > +{ > > + struct acpi_table_header *table; > > + struct acpi_table_fadt *fadt; > > + acpi_status status; > > + int ret = 0; > > + > > + /* > > + * FADT is required on riscv; retrieve it to check its presence > > + * and carry out revision and ACPI HW reduced compliancy tests > > + */ > > + status = acpi_get_table(ACPI_SIG_FADT, 0, &table); > > + if (ACPI_FAILURE(status)) { > > + const char *msg = acpi_format_exception(status); > > + > > + pr_err("Failed to get FADT table, %s\n", msg); > > + return -ENODEV; > > + } > > + > > + fadt = (struct acpi_table_fadt *)table; > > + > > + if (!(fadt->flags & ACPI_FADT_HW_REDUCED)) { > > Do we also need to check for ACPI version 5.0+ when checking for HW > reduced? > We need to add version check of FADT once spec is released. Will update it at that time. > > + pr_err("FADT not ACPI hardware reduced compliant\n"); > > + ret = -EINVAL; > > + } > > + > > + /* > > + * acpi_get_table() creates FADT table mapping that > > + * should be released after parsing and before resuming boot > > + */ > > + acpi_put_table(table); > > + return ret; > > +} > > + > > +/* > > + * acpi_boot_table_init() called from setup_arch(), always. > > + * 1. find RSDP and get its address, and then find XSDT > > + * 2. extract all tables and checksums them all > > + * 3. check ACPI FADT HW reduced flag > > + * > > + * We can parse ACPI boot-time tables such as MADT after > > + * this function is called. > > + * > > + * On return ACPI is enabled if either: > > + * > > + * - ACPI tables are initialized and sanity checks passed > > + * - acpi=force was passed in the command line and ACPI was not disabled > > + * explicitly through acpi=off command line parameter > > + * > > + * ACPI is disabled on function return otherwise > > + */ > > +void __init acpi_boot_table_init(void) > > +{ > > + /* > > + * Enable ACPI instead of device tree unless > > + * - ACPI has been disabled explicitly (acpi=off), or > > + * - firmware has not populated ACPI ptr in EFI system table > > + * and ACPI has not been [force] enabled (acpi=on|force) > > + */ > > + if (param_acpi_off || > > + (!param_acpi_on && !param_acpi_force && > > + efi.acpi20 == EFI_INVALID_TABLE_ADDR)) > > + return; > > + > > + /* > > + * ACPI is disabled at this point. Enable it in order to parse > > + * the ACPI tables and carry out sanity checks > > + */ > > + enable_acpi(); > > + > > + /* > > + * If ACPI tables are initialized and FADT sanity checks passed, > > + * leave ACPI enabled and carry on booting; otherwise disable ACPI > > + * on initialization error. > > + * If acpi=force was passed on the command line it forces ACPI > > + * to be enabled even if its initialization failed. > > + */ > > + if (acpi_table_init() || acpi_fadt_sanity_check()) { > > + pr_err("Failed to init ACPI tables\n"); > > + if (!param_acpi_force) > > + disable_acpi(); > > + } > > +} > > A lot of the above code is common with arm64. It'd be nice to share that, > but maybe refactoring can be done on top of this. > Okay. > > > > static int acpi_parse_madt_rintc(union acpi_subtable_headers *header, const unsigned long end) > > { > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > > index 4335f08ffaf2..c2ee7f4427a1 100644 > > --- a/arch/riscv/kernel/setup.c > > +++ b/arch/riscv/kernel/setup.c > > @@ -8,6 +8,7 @@ > > * Nick Kossifidis <mick@ics.forth.gr> > > */ > > > > +#include <linux/acpi.h> > > #include <linux/init.h> > > #include <linux/mm.h> > > #include <linux/memblock.h> > > @@ -276,14 +277,22 @@ void __init setup_arch(char **cmdline_p) > > > > efi_init(); > > paging_init(); > > -#if IS_ENABLED(CONFIG_BUILTIN_DTB) > > - unflatten_and_copy_device_tree(); > > -#else > > - if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa)))) > > - unflatten_device_tree(); > > - else > > - pr_err("No DTB found in kernel mappings\n"); > > -#endif > > + > > + /* Parse the ACPI tables for possible boot-time configuration */ > > + acpi_boot_table_init(); > > + if (acpi_disabled) { > > + if (IS_ENABLED(CONFIG_BUILTIN_DTB)) { > > + unflatten_and_copy_device_tree(); > > + } else { > > + if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa)))) > > + unflatten_device_tree(); > > + else > > + pr_err("No DTB found in kernel mappings\n"); > > + } > > + } else { > > + early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa))); > > + } > > + > > early_init_fdt_scan_reserved_mem(); > > misc_mem_init(); > > > > -- > > 2.34.1 > > > > Otherwise, > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Thanks, Sunil
On Fri, Feb 24, 2023 at 06:06:22PM +0530, Sunil V L wrote: > On Mon, Feb 20, 2023 at 09:07:43PM +0100, Andrew Jones wrote: > > On Thu, Feb 16, 2023 at 11:50:40PM +0530, Sunil V L wrote: ... > > > + fadt = (struct acpi_table_fadt *)table; > > > + > > > + if (!(fadt->flags & ACPI_FADT_HW_REDUCED)) { > > > > Do we also need to check for ACPI version 5.0+ when checking for HW > > reduced? > > > We need to add version check of FADT once spec is released. Will > update it at that time. > I was thinking we need the version check already just for the flag. The spec has a footnote that says "The description of HW_REDUCED_ACPI provided here applies to ACPI specifications 5.0 and later" It doesn't really matter in practice since no RISC-V machines can boot with ACPI less than a version that support the new RISC-V tables... But I'd rather we either document that fact, or just do the check. Thanks, drew
On Fri, Feb 24, 2023 at 02:07:12PM +0100, Andrew Jones wrote: > On Fri, Feb 24, 2023 at 06:06:22PM +0530, Sunil V L wrote: > > On Mon, Feb 20, 2023 at 09:07:43PM +0100, Andrew Jones wrote: > > > On Thu, Feb 16, 2023 at 11:50:40PM +0530, Sunil V L wrote: > ... > > > > + fadt = (struct acpi_table_fadt *)table; > > > > + > > > > + if (!(fadt->flags & ACPI_FADT_HW_REDUCED)) { > > > > > > Do we also need to check for ACPI version 5.0+ when checking for HW > > > reduced? > > > > > We need to add version check of FADT once spec is released. Will > > update it at that time. > > > > I was thinking we need the version check already just for the flag. > The spec has a footnote that says > > "The description of HW_REDUCED_ACPI provided here applies to ACPI > specifications 5.0 and later" > > It doesn't really matter in practice since no RISC-V machines can > boot with ACPI less than a version that support the new RISC-V > tables... But I'd rather we either document that fact, or just do > the check. > Okay Let me add the current FADT version of 6.5 which for sure supports HW_REDUCED flag. When the spec gets released, we can update with actual revision we need for RISC-V. Thanks, Sunil
diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c index 13b26c87c136..35e7b24a30c8 100644 --- a/arch/riscv/kernel/acpi.c +++ b/arch/riscv/kernel/acpi.c @@ -16,6 +16,7 @@ #include <linux/acpi.h> #include <linux/io.h> #include <linux/pci.h> +#include <linux/efi.h> int acpi_noirq = 1; /* skip ACPI IRQ initialization */ int acpi_disabled = 1; @@ -26,6 +27,118 @@ EXPORT_SYMBOL(acpi_pci_disabled); static unsigned int intc_count; static struct acpi_madt_rintc cpu_madt_rintc[NR_CPUS]; +static bool param_acpi_off __initdata; +static bool param_acpi_on __initdata; +static bool param_acpi_force __initdata; + +static int __init parse_acpi(char *arg) +{ + if (!arg) + return -EINVAL; + + /* "acpi=off" disables both ACPI table parsing and interpreter */ + if (strcmp(arg, "off") == 0) + param_acpi_off = true; + else if (strcmp(arg, "on") == 0) /* prefer ACPI over DT */ + param_acpi_on = true; + else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */ + param_acpi_force = true; + else + return -EINVAL; /* Core will print when we return error */ + + return 0; +} +early_param("acpi", parse_acpi); + +/* + * acpi_fadt_sanity_check() - Check FADT presence and carry out sanity + * checks on it + * + * Return 0 on success, <0 on failure + */ +static int __init acpi_fadt_sanity_check(void) +{ + struct acpi_table_header *table; + struct acpi_table_fadt *fadt; + acpi_status status; + int ret = 0; + + /* + * FADT is required on riscv; retrieve it to check its presence + * and carry out revision and ACPI HW reduced compliancy tests + */ + status = acpi_get_table(ACPI_SIG_FADT, 0, &table); + if (ACPI_FAILURE(status)) { + const char *msg = acpi_format_exception(status); + + pr_err("Failed to get FADT table, %s\n", msg); + return -ENODEV; + } + + fadt = (struct acpi_table_fadt *)table; + + if (!(fadt->flags & ACPI_FADT_HW_REDUCED)) { + pr_err("FADT not ACPI hardware reduced compliant\n"); + ret = -EINVAL; + } + + /* + * acpi_get_table() creates FADT table mapping that + * should be released after parsing and before resuming boot + */ + acpi_put_table(table); + return ret; +} + +/* + * acpi_boot_table_init() called from setup_arch(), always. + * 1. find RSDP and get its address, and then find XSDT + * 2. extract all tables and checksums them all + * 3. check ACPI FADT HW reduced flag + * + * We can parse ACPI boot-time tables such as MADT after + * this function is called. + * + * On return ACPI is enabled if either: + * + * - ACPI tables are initialized and sanity checks passed + * - acpi=force was passed in the command line and ACPI was not disabled + * explicitly through acpi=off command line parameter + * + * ACPI is disabled on function return otherwise + */ +void __init acpi_boot_table_init(void) +{ + /* + * Enable ACPI instead of device tree unless + * - ACPI has been disabled explicitly (acpi=off), or + * - firmware has not populated ACPI ptr in EFI system table + * and ACPI has not been [force] enabled (acpi=on|force) + */ + if (param_acpi_off || + (!param_acpi_on && !param_acpi_force && + efi.acpi20 == EFI_INVALID_TABLE_ADDR)) + return; + + /* + * ACPI is disabled at this point. Enable it in order to parse + * the ACPI tables and carry out sanity checks + */ + enable_acpi(); + + /* + * If ACPI tables are initialized and FADT sanity checks passed, + * leave ACPI enabled and carry on booting; otherwise disable ACPI + * on initialization error. + * If acpi=force was passed on the command line it forces ACPI + * to be enabled even if its initialization failed. + */ + if (acpi_table_init() || acpi_fadt_sanity_check()) { + pr_err("Failed to init ACPI tables\n"); + if (!param_acpi_force) + disable_acpi(); + } +} static int acpi_parse_madt_rintc(union acpi_subtable_headers *header, const unsigned long end) { diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 4335f08ffaf2..c2ee7f4427a1 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -8,6 +8,7 @@ * Nick Kossifidis <mick@ics.forth.gr> */ +#include <linux/acpi.h> #include <linux/init.h> #include <linux/mm.h> #include <linux/memblock.h> @@ -276,14 +277,22 @@ void __init setup_arch(char **cmdline_p) efi_init(); paging_init(); -#if IS_ENABLED(CONFIG_BUILTIN_DTB) - unflatten_and_copy_device_tree(); -#else - if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa)))) - unflatten_device_tree(); - else - pr_err("No DTB found in kernel mappings\n"); -#endif + + /* Parse the ACPI tables for possible boot-time configuration */ + acpi_boot_table_init(); + if (acpi_disabled) { + if (IS_ENABLED(CONFIG_BUILTIN_DTB)) { + unflatten_and_copy_device_tree(); + } else { + if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa)))) + unflatten_device_tree(); + else + pr_err("No DTB found in kernel mappings\n"); + } + } else { + early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa))); + } + early_init_fdt_scan_reserved_mem(); misc_mem_init();