lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKv+Gu_6zkq0cfp715sn-boGD=ZVDJ-Xk7venkQb1Gry_hxoeg@mail.gmail.com>
Date:   Fri, 13 Sep 2019 10:05:30 +0100
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Vishal L Verma <vishal.l.verma@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-efi <linux-efi@...r.kernel.org>
Subject: Re: [PATCH v5 03/10] x86, efi: Push EFI_MEMMAP check into leaf routines

On Fri, 30 Aug 2019 at 03:06, Dan Williams <dan.j.williams@...el.com> wrote:
>
> In preparation for adding another EFI_MEMMAP dependent call that needs
> to occur before e820__memblock_setup() fixup the existing efi calls to
> check for EFI_MEMMAP internally. This ends up being cleaner than the
> alternative of checking EFI_MEMMAP multiple times in setup_arch().
>
> Cc: <x86@...nel.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: Andy Lutomirski <luto@...nel.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Reviewed-by: Dave Hansen <dave.hansen@...ux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>

I'd prefer it if the spurious whitespace changes could be dropped, but
otherwise, this looks fine to me, so I am not going to obsess about
it.

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>


> ---
>  arch/x86/include/asm/efi.h      |    9 ++++++++-
>  arch/x86/kernel/setup.c         |   19 +++++++++----------
>  arch/x86/platform/efi/efi.c     |    3 +++
>  arch/x86/platform/efi/quirks.c  |    3 +++
>  drivers/firmware/efi/esrt.c     |    3 +++
>  drivers/firmware/efi/fake_mem.c |    2 +-
>  include/linux/efi.h             |    2 --
>  7 files changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index 43a82e59c59d..45f853bce869 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -140,7 +140,6 @@ extern void efi_delete_dummy_variable(void);
>  extern void efi_switch_mm(struct mm_struct *mm);
>  extern void efi_recover_from_page_fault(unsigned long phys_addr);
>  extern void efi_free_boot_services(void);
> -extern void efi_reserve_boot_services(void);
>
>  struct efi_setup_data {
>         u64 fw_vendor;
> @@ -244,6 +243,8 @@ static inline bool efi_is_64bit(void)
>  extern bool efi_reboot_required(void);
>  extern bool efi_is_table_address(unsigned long phys_addr);
>
> +extern void efi_find_mirror(void);
> +extern void efi_reserve_boot_services(void);
>  #else
>  static inline void parse_efi_setup(u64 phys_addr, u32 data_len) {}
>  static inline bool efi_reboot_required(void)
> @@ -254,6 +255,12 @@ static inline  bool efi_is_table_address(unsigned long phys_addr)
>  {
>         return false;
>  }
> +static inline void efi_find_mirror(void)
> +{
> +}
> +static inline void efi_reserve_boot_services(void)
> +{
> +}
>  #endif /* CONFIG_EFI */
>
>  #endif /* _ASM_X86_EFI_H */
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index bbe35bf879f5..9bfecb542440 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1118,21 +1118,20 @@ void __init setup_arch(char **cmdline_p)
>         cleanup_highmap();
>
>         memblock_set_current_limit(ISA_END_ADDRESS);
> +
>         e820__memblock_setup();
>
>         reserve_bios_regions();
>
> -       if (efi_enabled(EFI_MEMMAP)) {
> -               efi_fake_memmap();
> -               efi_find_mirror();
> -               efi_esrt_init();
> +       efi_fake_memmap();
> +       efi_find_mirror();
> +       efi_esrt_init();
>
> -               /*
> -                * The EFI specification says that boot service code won't be
> -                * called after ExitBootServices(). This is, in fact, a lie.
> -                */
> -               efi_reserve_boot_services();
> -       }
> +       /*
> +        * The EFI specification says that boot service code won't be
> +        * called after ExitBootServices(). This is, in fact, a lie.
> +        */
> +       efi_reserve_boot_services();
>
>         /* preallocate 4k for mptable mpc */
>         e820__memblock_alloc_reserved_mpc_new();
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index c202e1b07e29..0bb58eb33ca0 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -128,6 +128,9 @@ void __init efi_find_mirror(void)
>         efi_memory_desc_t *md;
>         u64 mirror_size = 0, total_size = 0;
>
> +       if (!efi_enabled(EFI_MEMMAP))
> +               return;
> +
>         for_each_efi_memory_desc(md) {
>                 unsigned long long start = md->phys_addr;
>                 unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index 3b9fd679cea9..7675cf754d90 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -320,6 +320,9 @@ void __init efi_reserve_boot_services(void)
>  {
>         efi_memory_desc_t *md;
>
> +       if (!efi_enabled(EFI_MEMMAP))
> +               return;
> +
>         for_each_efi_memory_desc(md) {
>                 u64 start = md->phys_addr;
>                 u64 size = md->num_pages << EFI_PAGE_SHIFT;
> diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
> index d6dd5f503fa2..2762e0662bf4 100644
> --- a/drivers/firmware/efi/esrt.c
> +++ b/drivers/firmware/efi/esrt.c
> @@ -246,6 +246,9 @@ void __init efi_esrt_init(void)
>         int rc;
>         phys_addr_t end;
>
> +       if (!efi_enabled(EFI_MEMMAP))
> +               return;
> +
>         pr_debug("esrt-init: loading.\n");
>         if (!esrt_table_exists())
>                 return;
> diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
> index 9501edc0fcfb..526b45331d96 100644
> --- a/drivers/firmware/efi/fake_mem.c
> +++ b/drivers/firmware/efi/fake_mem.c
> @@ -44,7 +44,7 @@ void __init efi_fake_memmap(void)
>         void *new_memmap;
>         int i;
>
> -       if (!nr_fake_mem)
> +       if (!efi_enabled(EFI_MEMMAP) || !nr_fake_mem)
>                 return;
>
>         /* count up the number of EFI memory descriptor */
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 5c1dd0221384..acc2b8982ed2 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1045,9 +1045,7 @@ extern void efi_enter_virtual_mode (void);        /* switch EFI to virtual mode, if pos
>  extern efi_status_t efi_query_variable_store(u32 attributes,
>                                              unsigned long size,
>                                              bool nonblocking);
> -extern void efi_find_mirror(void);
>  #else
> -
>  static inline efi_status_t efi_query_variable_store(u32 attributes,
>                                                     unsigned long size,
>                                                     bool nonblocking)
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ