* [PATCH] x86: move parse_early_param to earlier code for add_efi_memmap
@ 2017-11-30 5:23 Dave Young
2017-12-14 10:41 ` Dave Young
0 siblings, 1 reply; 6+ messages in thread
From: Dave Young @ 2017-11-30 5:23 UTC (permalink / raw)
To: linux-kernel, linux-efi; +Cc: mingo, ard.biesheuvel, matt
'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 <dyoung@redhat.com>
---
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: move parse_early_param to earlier code for add_efi_memmap
2017-11-30 5:23 [PATCH] x86: move parse_early_param to earlier code for add_efi_memmap Dave Young
@ 2017-12-14 10:41 ` Dave Young
2017-12-15 15:18 ` Matt Fleming
2017-12-16 14:03 ` Ingo Molnar
0 siblings, 2 replies; 6+ messages in thread
From: Dave Young @ 2017-12-14 10:41 UTC (permalink / raw)
To: linux-kernel, linux-efi; +Cc: mingo, ard.biesheuvel, matt
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 <dyoung@redhat.com>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: move parse_early_param to earlier code for add_efi_memmap
2017-12-14 10:41 ` Dave Young
@ 2017-12-15 15:18 ` Matt Fleming
2017-12-16 14:06 ` Ingo Molnar
2017-12-16 14:03 ` Ingo Molnar
1 sibling, 1 reply; 6+ messages in thread
From: Matt Fleming @ 2017-12-15 15:18 UTC (permalink / raw)
To: Dave Young; +Cc: linux-kernel, linux-efi, mingo, ard.biesheuvel
On Thu, 14 Dec, at 06:41:19PM, Dave Young wrote:
> 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 <dyoung@redhat.com>
> > ---
> > 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
>
I prefer this version. Please re-send a full patch and update the
subject line to include the "fix" somewhere; it wasn't obvious to me
from the start that this is a bug fix.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: move parse_early_param to earlier code for add_efi_memmap
2017-12-14 10:41 ` Dave Young
2017-12-15 15:18 ` Matt Fleming
@ 2017-12-16 14:03 ` Ingo Molnar
1 sibling, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2017-12-16 14:03 UTC (permalink / raw)
To: Dave Young; +Cc: linux-kernel, linux-efi, ard.biesheuvel, matt
* Dave Young <dyoung@redhat.com> wrote:
> Another way is move "efi_memblock_x86_reserve_range" to later code
> Maybe below is better?
Yeah, that's much lower risk, if the affected EFI code does not mind being called
later. We had trouble from trying to move early param parsing wholesale.
I've applied your v2 patch tip:efi/core.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: move parse_early_param to earlier code for add_efi_memmap
2017-12-15 15:18 ` Matt Fleming
@ 2017-12-16 14:06 ` Ingo Molnar
2017-12-18 13:11 ` Matt Fleming
0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2017-12-16 14:06 UTC (permalink / raw)
To: Matt Fleming; +Cc: Dave Young, linux-kernel, linux-efi, ard.biesheuvel
* Matt Fleming <matt@codeblueprint.co.uk> wrote:
> > 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
> >
>
> I prefer this version. Please re-send a full patch and update the
> subject line to include the "fix" somewhere; it wasn't obvious to me
> from the start that this is a bug fix.
Agreed.
I dropped the commit I just applied to tip:efi/core, since you are handling this,
so this patch should come through the regular EFI channels, right?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: move parse_early_param to earlier code for add_efi_memmap
2017-12-16 14:06 ` Ingo Molnar
@ 2017-12-18 13:11 ` Matt Fleming
0 siblings, 0 replies; 6+ messages in thread
From: Matt Fleming @ 2017-12-18 13:11 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Dave Young, linux-kernel, linux-efi, ard.biesheuvel
On Sat, 16 Dec, at 03:06:32PM, Ingo Molnar wrote:
>
> * Matt Fleming <matt@codeblueprint.co.uk> wrote:
>
> > > 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
> > >
> >
> > I prefer this version. Please re-send a full patch and update the
> > subject line to include the "fix" somewhere; it wasn't obvious to me
> > from the start that this is a bug fix.
>
> Agreed.
>
> I dropped the commit I just applied to tip:efi/core, since you are handling this,
> so this patch should come through the regular EFI channels, right?
Yep, that's right. Me or Ard will take care of it.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-18 13:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-30 5:23 [PATCH] x86: move parse_early_param to earlier code for add_efi_memmap Dave Young
2017-12-14 10:41 ` Dave Young
2017-12-15 15:18 ` Matt Fleming
2017-12-16 14:06 ` Ingo Molnar
2017-12-18 13:11 ` Matt Fleming
2017-12-16 14:03 ` Ingo Molnar
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).