Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751462AbdLNKl2 (ORCPT ); Thu, 14 Dec 2017 05:41:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35710 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750844AbdLNKl0 (ORCPT ); Thu, 14 Dec 2017 05:41:26 -0500 Date: Thu, 14 Dec 2017 18:41:19 +0800 From: Dave Young To: linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org Cc: mingo@kernel.org, ard.biesheuvel@linaro.org, matt@codeblueprint.co.uk Subject: Re: [PATCH] x86: move parse_early_param to earlier code for add_efi_memmap Message-ID: <20171214104119.GA2437@dhcp-128-65.nay.redhat.com> References: <20171130052327.GA3500@dhcp-128-65.nay.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171130052327.GA3500@dhcp-128-65.nay.redhat.com> User-Agent: Mutt/1.9.1 (2017-09-22) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 14 Dec 2017 10:41:26 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4222 Lines: 123 On 11/30/17 at 01:23pm, Dave Young wrote: > 'add_efi_memmap' is an early param, but do_add_efi_memmap() has no > chance to run because the code path is before parse_early_param(). > I believe it worked when the param was introduced but probably later > some other changes caused the wrong order and nobody noticed it. > > Move parse_early_param before efi_memblock_x86_reserve_range to fix > this. > > Signed-off-by: Dave Young > --- > arch/x86/kernel/setup.c | 55 ++++++++++++++++++++++++------------------------ > 1 file changed, 28 insertions(+), 27 deletions(-) > > --- linux-x86.orig/arch/x86/kernel/setup.c > +++ linux-x86/arch/x86/kernel/setup.c > @@ -897,6 +897,34 @@ void __init setup_arch(char **cmdline_p) > rd_prompt = ((boot_params.hdr.ram_size & RAMDISK_PROMPT_FLAG) != 0); > rd_doload = ((boot_params.hdr.ram_size & RAMDISK_LOAD_FLAG) != 0); > #endif > + > +#ifdef CONFIG_CMDLINE_BOOL > +#ifdef CONFIG_CMDLINE_OVERRIDE > + strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE); > +#else > + if (builtin_cmdline[0]) { > + /* append boot loader cmdline to builtin */ > + strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE); > + strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE); > + strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE); > + } > +#endif > +#endif > + > + strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE); > + *cmdline_p = command_line; > + > + /* > + * x86_configure_nx() is called before parse_early_param() to detect > + * whether hardware doesn't support NX (so that the early EHCI debug > + * console setup can safely call set_fixmap()). It may then be called > + * again from within noexec_setup() during parsing early parameters > + * to honor the respective command line option. > + */ > + x86_configure_nx(); > + > + parse_early_param(); > + > #ifdef CONFIG_EFI > if (!strncmp((char *)&boot_params.efi_info.efi_loader_signature, > EFI32_LOADER_SIGNATURE, 4)) { > @@ -935,33 +963,6 @@ void __init setup_arch(char **cmdline_p) > bss_resource.start = __pa_symbol(__bss_start); > bss_resource.end = __pa_symbol(__bss_stop)-1; > > -#ifdef CONFIG_CMDLINE_BOOL > -#ifdef CONFIG_CMDLINE_OVERRIDE > - strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE); > -#else > - if (builtin_cmdline[0]) { > - /* append boot loader cmdline to builtin */ > - strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE); > - strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE); > - strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE); > - } > -#endif > -#endif > - > - strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE); > - *cmdline_p = command_line; > - > - /* > - * x86_configure_nx() is called before parse_early_param() to detect > - * whether hardware doesn't support NX (so that the early EHCI debug > - * console setup can safely call set_fixmap()). It may then be called > - * again from within noexec_setup() during parsing early parameters > - * to honor the respective command line option. > - */ > - x86_configure_nx(); > - > - parse_early_param(); > - > #ifdef CONFIG_MEMORY_HOTPLUG > /* > * Memory used by the kernel cannot be hot-removed because Linux > -- > To unsubscribe from this list: send the line "unsubscribe linux-efi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Ping for review.. Another way is move "efi_memblock_x86_reserve_range" to later code Maybe below is better? --- arch/x86/kernel/setup.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) --- linux-x86.orig/arch/x86/kernel/setup.c +++ linux-x86/arch/x86/kernel/setup.c @@ -906,9 +906,6 @@ void __init setup_arch(char **cmdline_p) set_bit(EFI_BOOT, &efi.flags); set_bit(EFI_64BIT, &efi.flags); } - - if (efi_enabled(EFI_BOOT)) - efi_memblock_x86_reserve_range(); #endif x86_init.oem.arch_setup(); @@ -962,6 +959,8 @@ void __init setup_arch(char **cmdline_p) parse_early_param(); + if (efi_enabled(EFI_BOOT)) + efi_memblock_x86_reserve_range(); #ifdef CONFIG_MEMORY_HOTPLUG /* * Memory used by the kernel cannot be hot-removed because Linux Thanks Dave