From: Arvind Sankar <nivedita@alum.mit.edu>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: x86@kernel.org, nivedita@alum.mit.edu, linux-efi@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 12/18] efi: clean up config_parse_tables()
Date: Sun, 16 Feb 2020 14:12:19 -0500 [thread overview]
Message-ID: <20200216191219.GA589207@rani.riverdale.lan> (raw)
In-Reply-To: <20200216182334.8121-13-ardb@kernel.org>
On Sun, Feb 16, 2020 at 07:23:28PM +0100, Ard Biesheuvel wrote:
> config_parse_tables() is a jumble of pointer arithmetic, due to the
> fact that on x86, we may be dealing with firmware whose native word
> size differs from the kernel's.
>
> This is not a concern on other architectures, and doesn't quite
> justify the state of the code, so let's clean it up by adding a
> non-x86 code path, constifying statically allocated tables and
> replacing preprocessor conditionals with IS_ENABLED() checks.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/ia64/kernel/efi.c | 3 +-
> arch/x86/platform/efi/efi.c | 6 +--
> drivers/firmware/efi/arm-init.c | 5 +--
> drivers/firmware/efi/efi.c | 47 ++++++++++----------
> include/linux/efi.h | 5 ++-
> 5 files changed, 32 insertions(+), 34 deletions(-)
>
> diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
> index 3b5cf551489c..f69f3fe0532e 100644
> --- a/arch/ia64/kernel/efi.c
> +++ b/arch/ia64/kernel/efi.c
> @@ -56,7 +56,7 @@ unsigned long __initdata esi_phys = EFI_INVALID_TABLE_ADDR;
> unsigned long hcdp_phys = EFI_INVALID_TABLE_ADDR;
> unsigned long sal_systab_phys = EFI_INVALID_TABLE_ADDR;
>
> -static __initdata efi_config_table_type_t arch_tables[] = {
> +static const efi_config_table_type_t arch_tables[] __initconst = {
> {ESI_TABLE_GUID, "ESI", &esi_phys},
> {HCDP_TABLE_GUID, "HCDP", &hcdp_phys},
> {MPS_TABLE_GUID, "MPS", &mps_phys},
> @@ -533,7 +533,6 @@ efi_init (void)
>
> if (efi_config_parse_tables(__va(efi_systab->tables),
> efi_systab->nr_tables,
> - sizeof(efi_config_table_t),
> arch_tables) != 0)
> return;
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 26d905e6b579..f7025b9075b4 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -60,7 +60,7 @@ static u64 efi_systab_phys __initdata;
> static unsigned long prop_phys = EFI_INVALID_TABLE_ADDR;
> static unsigned long uga_phys = EFI_INVALID_TABLE_ADDR;
>
> -static efi_config_table_type_t arch_tables[] __initdata = {
> +static const efi_config_table_type_t arch_tables[] __initconst = {
> {EFI_PROPERTIES_TABLE_GUID, "PROP", &prop_phys},
> {UGA_IO_PROTOCOL_GUID, "UGA", &uga_phys},
> #ifdef CONFIG_X86_UV
> @@ -434,7 +434,7 @@ static int __init efi_systab_init(u64 phys)
> return 0;
> }
>
> -static int __init efi_config_init(efi_config_table_type_t *arch_tables)
> +static int __init efi_config_init(const efi_config_table_type_t *arch_tables)
> {
> void *config_tables;
> int sz, ret;
> @@ -457,7 +457,7 @@ static int __init efi_config_init(efi_config_table_type_t *arch_tables)
> return -ENOMEM;
> }
>
> - ret = efi_config_parse_tables(config_tables, efi.systab->nr_tables, sz,
> + ret = efi_config_parse_tables(config_tables, efi.systab->nr_tables,
> arch_tables);
>
> early_memunmap(config_tables, efi.systab->nr_tables * sz);
> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index a656bfcd7e27..d1f44c847841 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -55,7 +55,7 @@ static phys_addr_t efi_to_phys(unsigned long addr)
>
> static __initdata unsigned long screen_info_table = EFI_INVALID_TABLE_ADDR;
>
> -static __initdata efi_config_table_type_t arch_tables[] = {
> +static const efi_config_table_type_t arch_tables[] __initconst = {
> {LINUX_EFI_ARM_SCREEN_INFO_TABLE_GUID, NULL, &screen_info_table},
> {NULL_GUID, NULL, NULL}
> };
> @@ -85,7 +85,7 @@ static void __init init_screen_info(void)
>
> static int __init uefi_init(void)
> {
> - void *config_tables;
> + efi_config_table_t *config_tables;
> size_t table_size;
> int retval;
>
> @@ -118,7 +118,6 @@ static int __init uefi_init(void)
> goto out;
> }
> retval = efi_config_parse_tables(config_tables, efi.systab->nr_tables,
> - sizeof(efi_config_table_t),
> arch_tables);
>
> if (!retval)
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 2bfd6c0806ce..db1fe765380f 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -460,7 +460,7 @@ void __init efi_mem_reserve(phys_addr_t addr, u64 size)
> efi_arch_mem_reserve(addr, size);
> }
>
> -static __initdata efi_config_table_type_t common_tables[] = {
> +static const efi_config_table_type_t common_tables[] __initconst = {
> {ACPI_20_TABLE_GUID, "ACPI 2.0", &efi.acpi20},
> {ACPI_TABLE_GUID, "ACPI", &efi.acpi},
> {SMBIOS_TABLE_GUID, "SMBIOS", &efi.smbios},
> @@ -477,9 +477,9 @@ static __initdata efi_config_table_type_t common_tables[] = {
> {NULL_GUID, NULL, NULL},
> };
>
> -static __init int match_config_table(efi_guid_t *guid,
> +static __init int match_config_table(const efi_guid_t *guid,
> unsigned long table,
> - efi_config_table_type_t *table_types)
> + const efi_config_table_type_t *table_types)
> {
> int i;
>
> @@ -498,39 +498,38 @@ static __init int match_config_table(efi_guid_t *guid,
> return 0;
> }
>
> -int __init efi_config_parse_tables(void *config_tables, int count, int sz,
> - efi_config_table_type_t *arch_tables)
> +int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
> + int count,
> + const efi_config_table_type_t *arch_tables)
> {
> - void *tablep;
> + const efi_config_table_64_t *tbl64 = (void *)config_tables;
> + const efi_config_table_32_t *tbl32 = (void *)config_tables;
> + const efi_guid_t *guid;
> + unsigned long table;
> int i;
>
> - tablep = config_tables;
> pr_info("");
> for (i = 0; i < count; i++) {
> - efi_guid_t guid;
> - unsigned long table;
> -
> - if (efi_enabled(EFI_64BIT)) {
> - u64 table64;
> - guid = ((efi_config_table_64_t *)tablep)->guid;
> - table64 = ((efi_config_table_64_t *)tablep)->table;
> - table = table64;
> -#ifndef CONFIG_64BIT
> - if (table64 >> 32) {
> + if (!IS_ENABLED(CONFIG_X86)) {
> + guid = &config_tables[i].guid;
> + table = (unsigned long)config_tables[i].table;
> + } else if (efi_enabled(EFI_64BIT)) {
> + guid = &tbl64[i].guid;
> + table = tbl64[i].table;
> +
> + if (IS_ENABLED(CONFIG_X64_32) &&
^^^ typo, should be X86
> + tbl64[i].table > U32_MAX) {
> pr_cont("\n");
> pr_err("Table located above 4GB, disabling EFI.\n");
> return -EINVAL;
> }
> -#endif
> } else {
> - guid = ((efi_config_table_32_t *)tablep)->guid;
> - table = ((efi_config_table_32_t *)tablep)->table;
> + guid = &tbl32[i].guid;
> + table = tbl32[i].table;
> }
>
> - if (!match_config_table(&guid, table, common_tables))
> - match_config_table(&guid, table, arch_tables);
> -
> - tablep += sz;
> + if (!match_config_table(guid, table, common_tables))
> + match_config_table(guid, table, arch_tables);
> }
> pr_cont("\n");
> set_bit(EFI_CONFIG_TABLES, &efi.flags);
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index d61c25fd5824..99a7fcbe5e9b 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -613,8 +613,9 @@ extern void __init efi_esrt_init(void);
> #else
> static inline void efi_esrt_init(void) { }
> #endif
> -extern int efi_config_parse_tables(void *config_tables, int count, int sz,
> - efi_config_table_type_t *arch_tables);
> +extern int efi_config_parse_tables(const efi_config_table_t *config_tables,
> + int count,
> + const efi_config_table_type_t *arch_tables);
> extern int efi_systab_check_header(const efi_table_hdr_t *systab_hdr,
> int min_major_version);
> extern void efi_systab_report_header(const efi_table_hdr_t *systab_hdr,
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-02-16 19:12 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-16 18:23 [PATCH 00/18] efi: clean up contents of struct efi Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 01/18] efi: drop handling of 'boot_info' configuration table Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 02/18] efi/ia64: move HCDP and MPS table handling into IA64 arch code Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 03/18] efi: move UGA and PROP table handling to x86 code Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 04/18] efi: make rng_seed table handling local to efi.c Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 05/18] efi: move mem_attr_table out of struct efi Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 06/18] efi: make memreserve table handling local to efi.c Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 07/18] efi: merge EFI system table revision and vendor checks Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 08/18] efi/ia64: use existing helpers to locate ESI table Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 09/18] efi/ia64: use local variable for EFI system table address Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 10/18] efi/ia64: switch to efi_config_parse_tables() Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 11/18] efi: make efi_config_init() x86 only Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 12/18] efi: clean up config_parse_tables() Ard Biesheuvel
2020-02-16 19:12 ` Arvind Sankar [this message]
2020-02-17 8:32 ` Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 13/18] efi/x86: remove runtime table address from kexec EFI setup data Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 14/18] efi/x86: make fw_vendor, config_table and runtime sysfs nodes x86 specific Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 15/18] efi/x86: merge assignments of efi.runtime_version Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 16/18] efi: add 'runtime' pointer to struct efi Ard Biesheuvel
2020-03-03 16:03 ` Guenter Roeck
2020-03-03 16:39 ` Ard Biesheuvel
2020-03-03 17:53 ` Guenter Roeck
2020-03-03 18:01 ` Ard Biesheuvel
2020-03-03 18:14 ` Ard Biesheuvel
2020-03-03 20:30 ` Guenter Roeck
2020-03-03 21:40 ` Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 17/18] efi/arm: drop unnecessary references to efi.systab Ard Biesheuvel
2020-02-16 18:23 ` [PATCH 18/18] efi/x86: drop 'systab' member from struct efi Ard Biesheuvel
2020-02-16 18:31 ` [PATCH 00/18] efi: clean up contents of " Ard Biesheuvel
2020-02-18 19:46 ` Luck, Tony
2020-02-18 22:08 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200216191219.GA589207@rani.riverdale.lan \
--to=nivedita@alum.mit.edu \
--cc=ardb@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).