On Sun, 16 Feb 2020 at 20:12, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx>
> > ---
> > 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(-)
> >
...
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
...
> > @@ -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
>
Noted, thanks for spotting that.
