2021-09-20 20:09:21

by Juergen Gross

[permalink] [raw]
Subject: [PATCH v2] x86/setup: call early_reserve_memory() earlier

Commit a799c2bd29d19c565 ("x86/setup: Consolidate early memory
reservations") introduced early_reserve_memory() to do all needed
initial memblock_reserve() calls in one function. Unfortunately the
call of early_reserve_memory() is done too late for Xen dom0, as in
some cases a Xen hook called by e820__memory_setup() will need those
memory reservations to have happened already.

Move the call of early_reserve_memory() before the call of
e820__memory_setup() in order to avoid such problems.

Cc: [email protected]
Fixes: a799c2bd29d19c565 ("x86/setup: Consolidate early memory reservations")
Reported-by: Marek Marczykowski-Górecki <[email protected]>
Signed-off-by: Juergen Gross <[email protected]>
---
V2:
- update comment (Jan Beulich, Boris Petkov)
- move call down in setup_arch() (Mike Galbraith)
---
arch/x86/kernel/setup.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 79f164141116..40ed44ead063 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -830,6 +830,20 @@ void __init setup_arch(char **cmdline_p)

x86_init.oem.arch_setup();

+ /*
+ * Do some memory reservations *before* memory is added to memblock, so
+ * memblock allocations won't overwrite it.
+ *
+ * After this point, everything still needed from the boot loader or
+ * firmware or kernel text should be early reserved or marked not RAM in
+ * e820. All other memory is free game.
+ *
+ * This call needs to happen before e820__memory_setup() which calls the
+ * xen_memory_setup() on Xen dom0 which relies on the fact that those
+ * early reservations have happened already.
+ */
+ early_reserve_memory();
+
iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
e820__memory_setup();
parse_setup_data();
@@ -876,18 +890,6 @@ void __init setup_arch(char **cmdline_p)

parse_early_param();

- /*
- * Do some memory reservations *before* memory is added to
- * memblock, so memblock allocations won't overwrite it.
- * Do it after early param, so we could get (unlikely) panic from
- * serial.
- *
- * After this point everything still needed from the boot loader or
- * firmware or kernel text should be early reserved or marked not
- * RAM in e820. All other memory is free game.
- */
- early_reserve_memory();
-
#ifdef CONFIG_MEMORY_HOTPLUG
/*
* Memory used by the kernel cannot be hot-removed because Linux
--
2.26.2


Subject: Re: [PATCH v2] x86/setup: call early_reserve_memory() earlier

On Mon, Sep 20, 2021 at 02:04:21PM +0200, Juergen Gross wrote:
> Commit a799c2bd29d19c565 ("x86/setup: Consolidate early memory
> reservations") introduced early_reserve_memory() to do all needed
> initial memblock_reserve() calls in one function. Unfortunately the
> call of early_reserve_memory() is done too late for Xen dom0, as in
> some cases a Xen hook called by e820__memory_setup() will need those
> memory reservations to have happened already.
>
> Move the call of early_reserve_memory() before the call of
> e820__memory_setup() in order to avoid such problems.
>
> Cc: [email protected]
> Fixes: a799c2bd29d19c565 ("x86/setup: Consolidate early memory reservations")
> Reported-by: Marek Marczykowski-Górecki <[email protected]>
> Signed-off-by: Juergen Gross <[email protected]>

I confirm this fixes my boot issue too.

Tested-by: Marek Marczykowski-Górecki <[email protected]>

> ---
> V2:
> - update comment (Jan Beulich, Boris Petkov)
> - move call down in setup_arch() (Mike Galbraith)
> ---
> arch/x86/kernel/setup.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 79f164141116..40ed44ead063 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -830,6 +830,20 @@ void __init setup_arch(char **cmdline_p)
>
> x86_init.oem.arch_setup();
>
> + /*
> + * Do some memory reservations *before* memory is added to memblock, so
> + * memblock allocations won't overwrite it.
> + *
> + * After this point, everything still needed from the boot loader or
> + * firmware or kernel text should be early reserved or marked not RAM in
> + * e820. All other memory is free game.
> + *
> + * This call needs to happen before e820__memory_setup() which calls the
> + * xen_memory_setup() on Xen dom0 which relies on the fact that those
> + * early reservations have happened already.
> + */
> + early_reserve_memory();
> +
> iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
> e820__memory_setup();
> parse_setup_data();
> @@ -876,18 +890,6 @@ void __init setup_arch(char **cmdline_p)
>
> parse_early_param();
>
> - /*
> - * Do some memory reservations *before* memory is added to
> - * memblock, so memblock allocations won't overwrite it.
> - * Do it after early param, so we could get (unlikely) panic from
> - * serial.
> - *
> - * After this point everything still needed from the boot loader or
> - * firmware or kernel text should be early reserved or marked not
> - * RAM in e820. All other memory is free game.
> - */
> - early_reserve_memory();
> -
> #ifdef CONFIG_MEMORY_HOTPLUG
> /*
> * Memory used by the kernel cannot be hot-removed because Linux
> --
> 2.26.2
>

--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab


Attachments:
(No filename) (2.89 kB)
signature.asc (499.00 B)
Download all attachments

2021-09-21 04:10:52

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2] x86/setup: call early_reserve_memory() earlier

On Mon, Sep 20, 2021 at 02:04:21PM +0200, Juergen Gross wrote:
> Commit a799c2bd29d19c565 ("x86/setup: Consolidate early memory
> reservations") introduced early_reserve_memory() to do all needed
> initial memblock_reserve() calls in one function. Unfortunately the
> call of early_reserve_memory() is done too late for Xen dom0, as in
> some cases a Xen hook called by e820__memory_setup() will need those
> memory reservations to have happened already.
>
> Move the call of early_reserve_memory() before the call of
> e820__memory_setup() in order to avoid such problems.
>
> Cc: [email protected]
> Fixes: a799c2bd29d19c565 ("x86/setup: Consolidate early memory reservations")
> Reported-by: Marek Marczykowski-G?recki <[email protected]>
> Signed-off-by: Juergen Gross <[email protected]>

I had issues on an AMD Ryzen 3 4300G based system with v1. v2 does not
trigger any boot issues on that same machine or an Intel i5-4210U based
system that I also test with.

Tested-by: Nathan Chancellor <[email protected]>

> ---
> V2:
> - update comment (Jan Beulich, Boris Petkov)
> - move call down in setup_arch() (Mike Galbraith)
> ---
> arch/x86/kernel/setup.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 79f164141116..40ed44ead063 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -830,6 +830,20 @@ void __init setup_arch(char **cmdline_p)
>
> x86_init.oem.arch_setup();
>
> + /*
> + * Do some memory reservations *before* memory is added to memblock, so
> + * memblock allocations won't overwrite it.
> + *
> + * After this point, everything still needed from the boot loader or
> + * firmware or kernel text should be early reserved or marked not RAM in
> + * e820. All other memory is free game.
> + *
> + * This call needs to happen before e820__memory_setup() which calls the
> + * xen_memory_setup() on Xen dom0 which relies on the fact that those
> + * early reservations have happened already.
> + */
> + early_reserve_memory();
> +
> iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
> e820__memory_setup();
> parse_setup_data();
> @@ -876,18 +890,6 @@ void __init setup_arch(char **cmdline_p)
>
> parse_early_param();
>
> - /*
> - * Do some memory reservations *before* memory is added to
> - * memblock, so memblock allocations won't overwrite it.
> - * Do it after early param, so we could get (unlikely) panic from
> - * serial.
> - *
> - * After this point everything still needed from the boot loader or
> - * firmware or kernel text should be early reserved or marked not
> - * RAM in e820. All other memory is free game.
> - */
> - early_reserve_memory();
> -
> #ifdef CONFIG_MEMORY_HOTPLUG
> /*
> * Memory used by the kernel cannot be hot-removed because Linux
> --
> 2.26.2
>
>

2021-09-22 17:21:33

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: x86/urgent] x86/setup: Call early_reserve_memory() earlier

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 8aa83e6395ce047a506f0b16edca45f36c1ae7f8
Gitweb: https://git.kernel.org/tip/8aa83e6395ce047a506f0b16edca45f36c1ae7f8
Author: Juergen Gross <[email protected]>
AuthorDate: Mon, 20 Sep 2021 14:04:21 +02:00
Committer: Borislav Petkov <[email protected]>
CommitterDate: Tue, 21 Sep 2021 09:52:08 +02:00

x86/setup: Call early_reserve_memory() earlier

Commit in Fixes introduced early_reserve_memory() to do all needed
initial memblock_reserve() calls in one function. Unfortunately, the call
of early_reserve_memory() is done too late for Xen dom0, as in some
cases a Xen hook called by e820__memory_setup() will need those memory
reservations to have happened already.

Move the call of early_reserve_memory() before the call of
e820__memory_setup() in order to avoid such problems.

Fixes: a799c2bd29d1 ("x86/setup: Consolidate early memory reservations")
Reported-by: Marek Marczykowski-Górecki <[email protected]>
Signed-off-by: Juergen Gross <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Tested-by: Marek Marczykowski-Górecki <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/kernel/setup.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 79f1641..40ed44e 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -830,6 +830,20 @@ void __init setup_arch(char **cmdline_p)

x86_init.oem.arch_setup();

+ /*
+ * Do some memory reservations *before* memory is added to memblock, so
+ * memblock allocations won't overwrite it.
+ *
+ * After this point, everything still needed from the boot loader or
+ * firmware or kernel text should be early reserved or marked not RAM in
+ * e820. All other memory is free game.
+ *
+ * This call needs to happen before e820__memory_setup() which calls the
+ * xen_memory_setup() on Xen dom0 which relies on the fact that those
+ * early reservations have happened already.
+ */
+ early_reserve_memory();
+
iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
e820__memory_setup();
parse_setup_data();
@@ -876,18 +890,6 @@ void __init setup_arch(char **cmdline_p)

parse_early_param();

- /*
- * Do some memory reservations *before* memory is added to
- * memblock, so memblock allocations won't overwrite it.
- * Do it after early param, so we could get (unlikely) panic from
- * serial.
- *
- * After this point everything still needed from the boot loader or
- * firmware or kernel text should be early reserved or marked not
- * RAM in e820. All other memory is free game.
- */
- early_reserve_memory();
-
#ifdef CONFIG_MEMORY_HOTPLUG
/*
* Memory used by the kernel cannot be hot-removed because Linux

2021-11-04 05:41:15

by Dan Williams

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/setup: Call early_reserve_memory() earlier

On Wed, 2021-09-22 at 17:18 +0000, tip-bot2 for Juergen Gross wrote:
> The following commit has been merged into the x86/urgent branch of
> tip:
>
> Commit-ID:     8aa83e6395ce047a506f0b16edca45f36c1ae7f8
> Gitweb:       
> https://git.kernel.org/tip/8aa83e6395ce047a506f0b16edca45f36c1ae7f8
> Author:        Juergen Gross <[email protected]>
> AuthorDate:    Mon, 20 Sep 2021 14:04:21 +02:00
> Committer:     Borislav Petkov <[email protected]>
> CommitterDate: Tue, 21 Sep 2021 09:52:08 +02:00
>
> x86/setup: Call early_reserve_memory() earlier

Hi,

I got a report from Anjaneya that starting with the v5.15 kernel he can
no longer use the "efi=nosoftreserve" kernel command line parameter to
suppress "soft reservation" behavior. Recall that "soft reserved" is
the Linux designation for memory that is marked with the EFI "Special
Purpose" attribute.

By inspection, this commit looks like the source of the problem because
early_reserve_memory() now runs before parse_early_param(). I suspect
that this also affects memmap= since it is also an early_param(). I'll
verify that tomorrow when I'm more awake, but wanted to give a heads up
in the meantime.


>
> Commit in Fixes introduced early_reserve_memory() to do all needed
> initial memblock_reserve() calls in one function. Unfortunately, the call
> of early_reserve_memory() is done too late for Xen dom0, as in some
> cases a Xen hook called by e820__memory_setup() will need those memory
> reservations to have happened already.
>
> Move the call of early_reserve_memory() before the call of
> e820__memory_setup() in order to avoid such problems.
>
> Fixes: a799c2bd29d1 ("x86/setup: Consolidate early memory reservations")
> Reported-by: Marek Marczykowski-Górecki <[email protected]>
> Signed-off-by: Juergen Gross <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>
> Tested-by: Marek Marczykowski-Górecki <[email protected]>
> Tested-by: Nathan Chancellor <[email protected]>
> Cc: [email protected]
> Link: https://lkml.kernel.org/r/[email protected]
> ---
>  arch/x86/kernel/setup.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 79f1641..40ed44e 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -830,6 +830,20 @@ void __init setup_arch(char **cmdline_p)
>  
>         x86_init.oem.arch_setup();
>  
> +       /*
> +        * Do some memory reservations *before* memory is added to memblock, so
> +        * memblock allocations won't overwrite it.
> +        *
> +        * After this point, everything still needed from the boot loader or
> +        * firmware or kernel text should be early reserved or marked not RAM in
> +        * e820. All other memory is free game.
> +        *
> +        * This call needs to happen before e820__memory_setup() which calls the
> +        * xen_memory_setup() on Xen dom0 which relies on the fact that those
> +        * early reservations have happened already.
> +        */
> +       early_reserve_memory();
> +
>         iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
>         e820__memory_setup();
>         parse_setup_data();
> @@ -876,18 +890,6 @@ void __init setup_arch(char **cmdline_p)
>  
>         parse_early_param();
>  
> -       /*
> -        * Do some memory reservations *before* memory is added to
> -        * memblock, so memblock allocations won't overwrite it.
> -        * Do it after early param, so we could get (unlikely) panic from
> -        * serial.
> -        *
> -        * After this point everything still needed from the boot loader or
> -        * firmware or kernel text should be early reserved or marked not
> -        * RAM in e820. All other memory is free game.
> -        */
> -       early_reserve_memory();
> -
>  #ifdef CONFIG_MEMORY_HOTPLUG
>         /*
>          * Memory used by the kernel cannot be hot-removed because Linux

2021-11-04 11:43:23

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/setup: Call early_reserve_memory() earlier

+ Mike.

On Thu, Nov 04, 2021 at 05:38:54AM +0000, Williams, Dan J wrote:
> By inspection, this commit looks like the source of the problem because
> early_reserve_memory() now runs before parse_early_param().

Yah, I see it too:

early_reserve_memory
|-> efi_memblock_x86_reserve_range
|-> efi_fake_memmap_early

which does

if (!efi_soft_reserve_enabled())
return;

and that would have set EFI_MEM_NO_SOFT_RESERVE after having parsed
"nosoftreserve".

And parse_early_param() happens later.

Uuuf, that early memory reservation dance is not going to be over,
ever...

Well, I guess we can do something like this below. The intent being to
carve out the command line preparation into a separate function which
does the early param parsing too. So that it all goes together.

And then call that function before early_reserve_memory() so that the
params have been parsed by then.

Looking at the changed flow, I think we should be ok but I've said that
a bunch of times already regarding this memory reservation early and
something in our house of cards called early boot order always broke.

Damn.

---
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 40ed44ead063..05f69e7d84dc 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -742,6 +742,28 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
return 0;
}

+static char *prepare_command_line(void)
+{
+#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);
+
+ parse_early_param();
+
+ return command_line;
+}
+
/*
* Determine if we were loaded by an EFI loader. If so, then we have also been
* passed the efi memmap, systab, etc., so we should use these data structures
@@ -830,6 +852,23 @@ void __init setup_arch(char **cmdline_p)

x86_init.oem.arch_setup();

+ /*
+ * x86_configure_nx() is called before parse_early_param() (called by
+ * prepare_command_line()) 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();
+
+ /*
+ * This parses early params and it needs to run before
+ * early_reserve_memory() because latter relies on such settings
+ * supplies as early params.
+ */
+ *cmdline_p = prepare_command_line();
+
/*
* Do some memory reservations *before* memory is added to memblock, so
* memblock allocations won't overwrite it.
@@ -863,33 +902,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


--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-11-04 17:40:58

by Dan Williams

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/setup: Call early_reserve_memory() earlier

On Thu, Nov 4, 2021 at 4:40 AM Borislav Petkov <[email protected]> wrote:
>
> + Mike.
>
> On Thu, Nov 04, 2021 at 05:38:54AM +0000, Williams, Dan J wrote:
> > By inspection, this commit looks like the source of the problem because
> > early_reserve_memory() now runs before parse_early_param().
>
> Yah, I see it too:
>
> early_reserve_memory
> |-> efi_memblock_x86_reserve_range
> |-> efi_fake_memmap_early
>
> which does
>
> if (!efi_soft_reserve_enabled())
> return;
>
> and that would have set EFI_MEM_NO_SOFT_RESERVE after having parsed
> "nosoftreserve".
>
> And parse_early_param() happens later.
>
> Uuuf, that early memory reservation dance is not going to be over,
> ever...
>
> Well, I guess we can do something like this below. The intent being to
> carve out the command line preparation into a separate function which
> does the early param parsing too. So that it all goes together.
>
> And then call that function before early_reserve_memory() so that the
> params have been parsed by then.
>
> Looking at the changed flow, I think we should be ok but I've said that
> a bunch of times already regarding this memory reservation early and
> something in our house of cards called early boot order always broke.
>
> Damn.

Thanks Boris!

You can add:

Tested-by: Anjaneya Chagam <[email protected]>
Reviewed-by: Dan Williams <[email protected]>

>
> ---
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 40ed44ead063..05f69e7d84dc 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -742,6 +742,28 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
> return 0;
> }
>
> +static char *prepare_command_line(void)
> +{
> +#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);
> +
> + parse_early_param();
> +
> + return command_line;
> +}
> +
> /*
> * Determine if we were loaded by an EFI loader. If so, then we have also been
> * passed the efi memmap, systab, etc., so we should use these data structures
> @@ -830,6 +852,23 @@ void __init setup_arch(char **cmdline_p)
>
> x86_init.oem.arch_setup();
>
> + /*
> + * x86_configure_nx() is called before parse_early_param() (called by
> + * prepare_command_line()) 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();
> +
> + /*
> + * This parses early params and it needs to run before
> + * early_reserve_memory() because latter relies on such settings
> + * supplies as early params.
> + */
> + *cmdline_p = prepare_command_line();
> +
> /*
> * Do some memory reservations *before* memory is added to memblock, so
> * memblock allocations won't overwrite it.
> @@ -863,33 +902,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
>
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

2021-11-15 11:36:47

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 8d48bf8206f77aa8687f0e241e901e5197e52423
Gitweb: https://git.kernel.org/tip/8d48bf8206f77aa8687f0e241e901e5197e52423
Author: Borislav Petkov <[email protected]>
AuthorDate: Fri, 05 Nov 2021 10:41:51 +01:00
Committer: Borislav Petkov <[email protected]>
CommitterDate: Mon, 15 Nov 2021 12:27:40 +01:00

x86/boot: Pull up cmdline preparation and early param parsing

Dan reports that Anjaneya Chagam can no longer use the efi=nosoftreserve
kernel command line parameter to suppress "soft reservation" behavior.

This is due to the fact that the following call-chain happens at boot:

early_reserve_memory
|-> efi_memblock_x86_reserve_range
|-> efi_fake_memmap_early

which does

if (!efi_soft_reserve_enabled())
return;

and that would have set EFI_MEM_NO_SOFT_RESERVE after having parsed
"nosoftreserve".

However, parse_early_param() gets called *after* it, leading to the boot
cmdline not being taken into account.

Therefore, carve out the command line preparation into a separate
function which does the early param parsing too. So that it all goes
together.

And then call that function before early_reserve_memory() so that the
params would have been parsed by then.

Fixes: 8aa83e6395ce ("x86/setup: Call early_reserve_memory() earlier")
Reported-by: Dan Williams <[email protected]>
Reviewed-by: Dan Williams <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Tested-by: Anjaneya Chagam <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/setup.c | 66 +++++++++++++++++++++++-----------------
1 file changed, 39 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 49b596d..c410be7 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -742,6 +742,28 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
return 0;
}

+static char *prepare_command_line(void)
+{
+#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);
+
+ parse_early_param();
+
+ return command_line;
+}
+
/*
* Determine if we were loaded by an EFI loader. If so, then we have also been
* passed the efi memmap, systab, etc., so we should use these data structures
@@ -831,6 +853,23 @@ void __init setup_arch(char **cmdline_p)
x86_init.oem.arch_setup();

/*
+ * x86_configure_nx() is called before parse_early_param() (called by
+ * prepare_command_line()) 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();
+
+ /*
+ * This parses early params and it needs to run before
+ * early_reserve_memory() because latter relies on such settings
+ * supplied as early params.
+ */
+ *cmdline_p = prepare_command_line();
+
+ /*
* Do some memory reservations *before* memory is added to memblock, so
* memblock allocations won't overwrite it.
*
@@ -863,33 +902,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

2021-12-09 14:38:43

by John Dorminy

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

Greetings;

It seems that this patch causes a mem= parameter to the kernel to have no effect, unfortunately...

As far as I understand, the x86 mem parameter handler parse_memopt() (called by parse_early_param()) relies on being called after e820__memory_setup(): it simply removes any memory above the specified limit at that moment, allowing memory to later be hotplugged without regard for the initial limit. However, the initial non-hotplugged memory must already have been set up, in e820__memory_setup(), so that it can be removed in parse_memopt(); if parse_early_param() is called before e820__memory_setup(), as this change does, the parameter ends up having no effect.

I apologize that I don't know how to fix this, but I'm happy to test patches.

Typical dmesg output showing the lack of effect, built from the prior change and this change:

With a git tree synced to 8d48bf8206f77aa8687f0e241e901e5197e52423^ (working):
[ 0.000000] Command line: BOOT_IMAGE=(hd0,msdos1)/boot/vmlinuz-5.16.0-rc1 root=UUID=a4f7bd84-4f29-40bc-8c98-f4a72d0856c4 ro net.ifnames=0 crashkernel=128M mem=4G
...
[ 0.000000] BIOS-provided physical RAM map:
[ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009abff] usable
[ 0.000000] BIOS-e820: [mem 0x000000000009ac00-0x000000000009ffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
[ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000007dd3afff] usable
[ 0.000000] BIOS-e820: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
[ 0.000000] BIOS-e820: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
[ 0.000000] BIOS-e820: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
[ 0.000000] BIOS-e820: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
[ 0.000000] BIOS-e820: [mem 0x0000000080000000-0x000000008fffffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
[ 0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000207fffffff] usable
[ 0.000000] e820: remove [mem 0x100000000-0xfffffffffffffffe] usable
[ 0.000000] NX (Execute Disable) protection: active
[ 0.000000] user-defined physical RAM map:
[ 0.000000] user: [mem 0x0000000000000000-0x000000000009abff] usable
[ 0.000000] user: [mem 0x000000000009ac00-0x000000000009ffff] reserved
[ 0.000000] user: [mem 0x00000000000e0000-0x00000000000fffff] reserved
[ 0.000000] user: [mem 0x0000000000100000-0x000000007dd3afff] usable
[ 0.000000] user: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
[ 0.000000] user: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
[ 0.000000] user: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
[ 0.000000] user: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
[ 0.000000] user: [mem 0x0000000080000000-0x000000008fffffff] reserved
[ 0.000000] user: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
[ 0.000000] user: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
...
[ 0.025617] Memory: 1762876K/2061136K available (16394K kernel code, 3568K rwdata, 10324K rodata, 2676K init, 4924K bss, 298000K reserved, 0K cma-reserved)

Synced 8d48bf8206f77aa8687f0e241e901e5197e52423 (not working):

[ 0.000000] Command line: BOOT_IMAGE=(hd0,msdos1)/boot/vmlinuz-5.16.0-rc4+ root=UUID=0e750e61-b92e-4708-a974-c50a3fb7e969 ro net.ifnames=0 crashkernel=128M mem=4G
[ 0.000000] e820: remove [mem 0x100000000-0xfffffffffffffffe] usable
[ 0.000000] BIOS-provided physical RAM map:
[ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009abff] usable
[ 0.000000] BIOS-e820: [mem 0x000000000009ac00-0x000000000009ffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
[ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000007dd3afff] usable
[ 0.000000] BIOS-e820: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
[ 0.000000] BIOS-e820: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
[ 0.000000] BIOS-e820: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
[ 0.000000] BIOS-e820: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
[ 0.000000] BIOS-e820: [mem 0x0000000080000000-0x000000008fffffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
[ 0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000207fffffff] usable
[ 0.000000] NX (Execute Disable) protection: active
[ 0.000000] user-defined physical RAM map:
[ 0.000000] user: [mem 0x0000000000000000-0x000000000009abff] usable
[ 0.000000] user: [mem 0x000000000009ac00-0x000000000009ffff] reserved
[ 0.000000] user: [mem 0x00000000000e0000-0x00000000000fffff] reserved
[ 0.000000] user: [mem 0x0000000000100000-0x000000007dd3afff] usable
[ 0.000000] user: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
[ 0.000000] user: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
[ 0.000000] user: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
[ 0.000000] user: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
[ 0.000000] user: [mem 0x0000000080000000-0x000000008fffffff] reserved
[ 0.000000] user: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
[ 0.000000] user: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
[ 0.000000] user: [mem 0x0000000100000000-0x000000207fffffff] usable
...
[ 0.695267] Memory: 131657608K/134181712K available (16394K kernel code, 3568K rwdata, 10328K rodata, 2676K init, 4924K bss, 2523844K reserved, 0K cma-reserved)


2021-12-09 15:19:14

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

+ Hugh and Patrick.

On Thu, Dec 09, 2021 at 09:38:10AM -0500, John Dorminy wrote:
> Greetings;
>
> It seems that this patch causes a mem= parameter to the kernel to have no effect, unfortunately...
>
> As far as I understand, the x86 mem parameter handler parse_memopt() (called by parse_early_param()) relies on being called after e820__memory_setup(): it simply removes any memory above the specified limit at that moment, allowing memory to later be hotplugged without regard for the initial limit. However, the initial non-hotplugged memory must already have been set up, in e820__memory_setup(), so that it can be removed in parse_memopt(); if parse_early_param() is called before e820__memory_setup(), as this change does, the parameter ends up having no effect.
>
> I apologize that I don't know how to fix this, but I'm happy to test patches.

Yeah, people have been reporting boot failures with mem= on the cmdline.

I think I see why, can you try this one:

---
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 6a190c7f4d71..6db971e61e4b 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -862,6 +862,8 @@ void __init setup_arch(char **cmdline_p)
*/
x86_configure_nx();

+ e820__memory_setup();
+
/*
* This parses early params and it needs to run before
* early_reserve_memory() because latter relies on such settings
@@ -884,7 +886,6 @@ void __init setup_arch(char **cmdline_p)
early_reserve_memory();

iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
- e820__memory_setup();
parse_setup_data();

copy_edd();
---

Leaving in the rest for the newly added folks.

> Typical dmesg output showing the lack of effect, built from the prior change and this change:
>
> With a git tree synced to 8d48bf8206f77aa8687f0e241e901e5197e52423^ (working):
> [ 0.000000] Command line: BOOT_IMAGE=(hd0,msdos1)/boot/vmlinuz-5.16.0-rc1 root=UUID=a4f7bd84-4f29-40bc-8c98-f4a72d0856c4 ro net.ifnames=0 crashkernel=128M mem=4G
> ...
> [ 0.000000] BIOS-provided physical RAM map:
> [ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009abff] usable
> [ 0.000000] BIOS-e820: [mem 0x000000000009ac00-0x000000000009ffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000007dd3afff] usable
> [ 0.000000] BIOS-e820: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
> [ 0.000000] BIOS-e820: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
> [ 0.000000] BIOS-e820: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
> [ 0.000000] BIOS-e820: [mem 0x0000000080000000-0x000000008fffffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000207fffffff] usable
> [ 0.000000] e820: remove [mem 0x100000000-0xfffffffffffffffe] usable
> [ 0.000000] NX (Execute Disable) protection: active
> [ 0.000000] user-defined physical RAM map:
> [ 0.000000] user: [mem 0x0000000000000000-0x000000000009abff] usable
> [ 0.000000] user: [mem 0x000000000009ac00-0x000000000009ffff] reserved
> [ 0.000000] user: [mem 0x00000000000e0000-0x00000000000fffff] reserved
> [ 0.000000] user: [mem 0x0000000000100000-0x000000007dd3afff] usable
> [ 0.000000] user: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
> [ 0.000000] user: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
> [ 0.000000] user: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
> [ 0.000000] user: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
> [ 0.000000] user: [mem 0x0000000080000000-0x000000008fffffff] reserved
> [ 0.000000] user: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
> [ 0.000000] user: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
> ...
> [ 0.025617] Memory: 1762876K/2061136K available (16394K kernel code, 3568K rwdata, 10324K rodata, 2676K init, 4924K bss, 298000K reserved, 0K cma-reserved)
>
> Synced 8d48bf8206f77aa8687f0e241e901e5197e52423 (not working):
>
> [ 0.000000] Command line: BOOT_IMAGE=(hd0,msdos1)/boot/vmlinuz-5.16.0-rc4+ root=UUID=0e750e61-b92e-4708-a974-c50a3fb7e969 ro net.ifnames=0 crashkernel=128M mem=4G
> [ 0.000000] e820: remove [mem 0x100000000-0xfffffffffffffffe] usable
> [ 0.000000] BIOS-provided physical RAM map:
> [ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009abff] usable
> [ 0.000000] BIOS-e820: [mem 0x000000000009ac00-0x000000000009ffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000007dd3afff] usable
> [ 0.000000] BIOS-e820: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
> [ 0.000000] BIOS-e820: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
> [ 0.000000] BIOS-e820: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
> [ 0.000000] BIOS-e820: [mem 0x0000000080000000-0x000000008fffffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000207fffffff] usable
> [ 0.000000] NX (Execute Disable) protection: active
> [ 0.000000] user-defined physical RAM map:
> [ 0.000000] user: [mem 0x0000000000000000-0x000000000009abff] usable
> [ 0.000000] user: [mem 0x000000000009ac00-0x000000000009ffff] reserved
> [ 0.000000] user: [mem 0x00000000000e0000-0x00000000000fffff] reserved
> [ 0.000000] user: [mem 0x0000000000100000-0x000000007dd3afff] usable
> [ 0.000000] user: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
> [ 0.000000] user: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
> [ 0.000000] user: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
> [ 0.000000] user: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
> [ 0.000000] user: [mem 0x0000000080000000-0x000000008fffffff] reserved
> [ 0.000000] user: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
> [ 0.000000] user: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
> [ 0.000000] user: [mem 0x0000000100000000-0x000000207fffffff] usable
> ...
> [ 0.695267] Memory: 131657608K/134181712K available (16394K kernel code, 3568K rwdata, 10328K rodata, 2676K init, 4924K bss, 2523844K reserved, 0K cma-reserved)
>

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-12-09 15:27:03

by Juergen Gross

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On 09.12.21 16:19, Borislav Petkov wrote:
> + Hugh and Patrick.
>
> On Thu, Dec 09, 2021 at 09:38:10AM -0500, John Dorminy wrote:
>> Greetings;
>>
>> It seems that this patch causes a mem= parameter to the kernel to have no effect, unfortunately...
>>
>> As far as I understand, the x86 mem parameter handler parse_memopt() (called by parse_early_param()) relies on being called after e820__memory_setup(): it simply removes any memory above the specified limit at that moment, allowing memory to later be hotplugged without regard for the initial limit. However, the initial non-hotplugged memory must already have been set up, in e820__memory_setup(), so that it can be removed in parse_memopt(); if parse_early_param() is called before e820__memory_setup(), as this change does, the parameter ends up having no effect.
>>
>> I apologize that I don't know how to fix this, but I'm happy to test patches.
>
> Yeah, people have been reporting boot failures with mem= on the cmdline.
>
> I think I see why, can you try this one:

Sigh. This will break Xen PV. Again. The comment above the call of
early_reserve_memory() tells you why.

Juergen

>
> ---
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 6a190c7f4d71..6db971e61e4b 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -862,6 +862,8 @@ void __init setup_arch(char **cmdline_p)
> */
> x86_configure_nx();
>
> + e820__memory_setup();
> +
> /*
> * This parses early params and it needs to run before
> * early_reserve_memory() because latter relies on such settings
> @@ -884,7 +886,6 @@ void __init setup_arch(char **cmdline_p)
> early_reserve_memory();
>
> iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
> - e820__memory_setup();
> parse_setup_data();
>
> copy_edd();
> ---
>
> Leaving in the rest for the newly added folks.
>
>> Typical dmesg output showing the lack of effect, built from the prior change and this change:
>>
>> With a git tree synced to 8d48bf8206f77aa8687f0e241e901e5197e52423^ (working):
>> [ 0.000000] Command line: BOOT_IMAGE=(hd0,msdos1)/boot/vmlinuz-5.16.0-rc1 root=UUID=a4f7bd84-4f29-40bc-8c98-f4a72d0856c4 ro net.ifnames=0 crashkernel=128M mem=4G
>> ...
>> [ 0.000000] BIOS-provided physical RAM map:
>> [ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009abff] usable
>> [ 0.000000] BIOS-e820: [mem 0x000000000009ac00-0x000000000009ffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000007dd3afff] usable
>> [ 0.000000] BIOS-e820: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
>> [ 0.000000] BIOS-e820: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
>> [ 0.000000] BIOS-e820: [mem 0x0000000080000000-0x000000008fffffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000207fffffff] usable
>> [ 0.000000] e820: remove [mem 0x100000000-0xfffffffffffffffe] usable
>> [ 0.000000] NX (Execute Disable) protection: active
>> [ 0.000000] user-defined physical RAM map:
>> [ 0.000000] user: [mem 0x0000000000000000-0x000000000009abff] usable
>> [ 0.000000] user: [mem 0x000000000009ac00-0x000000000009ffff] reserved
>> [ 0.000000] user: [mem 0x00000000000e0000-0x00000000000fffff] reserved
>> [ 0.000000] user: [mem 0x0000000000100000-0x000000007dd3afff] usable
>> [ 0.000000] user: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
>> [ 0.000000] user: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
>> [ 0.000000] user: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
>> [ 0.000000] user: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
>> [ 0.000000] user: [mem 0x0000000080000000-0x000000008fffffff] reserved
>> [ 0.000000] user: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
>> [ 0.000000] user: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
>> ...
>> [ 0.025617] Memory: 1762876K/2061136K available (16394K kernel code, 3568K rwdata, 10324K rodata, 2676K init, 4924K bss, 298000K reserved, 0K cma-reserved)
>>
>> Synced 8d48bf8206f77aa8687f0e241e901e5197e52423 (not working):
>>
>> [ 0.000000] Command line: BOOT_IMAGE=(hd0,msdos1)/boot/vmlinuz-5.16.0-rc4+ root=UUID=0e750e61-b92e-4708-a974-c50a3fb7e969 ro net.ifnames=0 crashkernel=128M mem=4G
>> [ 0.000000] e820: remove [mem 0x100000000-0xfffffffffffffffe] usable
>> [ 0.000000] BIOS-provided physical RAM map:
>> [ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009abff] usable
>> [ 0.000000] BIOS-e820: [mem 0x000000000009ac00-0x000000000009ffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000007dd3afff] usable
>> [ 0.000000] BIOS-e820: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
>> [ 0.000000] BIOS-e820: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
>> [ 0.000000] BIOS-e820: [mem 0x0000000080000000-0x000000008fffffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
>> [ 0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000207fffffff] usable
>> [ 0.000000] NX (Execute Disable) protection: active
>> [ 0.000000] user-defined physical RAM map:
>> [ 0.000000] user: [mem 0x0000000000000000-0x000000000009abff] usable
>> [ 0.000000] user: [mem 0x000000000009ac00-0x000000000009ffff] reserved
>> [ 0.000000] user: [mem 0x00000000000e0000-0x00000000000fffff] reserved
>> [ 0.000000] user: [mem 0x0000000000100000-0x000000007dd3afff] usable
>> [ 0.000000] user: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
>> [ 0.000000] user: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
>> [ 0.000000] user: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
>> [ 0.000000] user: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
>> [ 0.000000] user: [mem 0x0000000080000000-0x000000008fffffff] reserved
>> [ 0.000000] user: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
>> [ 0.000000] user: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
>> [ 0.000000] user: [mem 0x0000000100000000-0x000000207fffffff] usable
>> ...
>> [ 0.695267] Memory: 131657608K/134181712K available (16394K kernel code, 3568K rwdata, 10328K rodata, 2676K init, 4924K bss, 2523844K reserved, 0K cma-reserved)
>>
>


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.02 kB)
OpenPGP public key
OpenPGP_signature (495.00 B)
OpenPGP digital signature
Download all attachments

2021-12-09 15:28:51

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Thu, Dec 09, 2021 at 04:26:55PM +0100, Juergen Gross wrote:
> Sigh. This will break Xen PV. Again. The comment above the call of
> early_reserve_memory() tells you why.

I know. I was just looking at how to fix that particular thing and was
going to find you on IRC to talk to you about it...

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-12-09 15:49:21

by John Dorminy

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Thu, Dec 9, 2021 at 10:19 AM Borislav Petkov <[email protected]> wrote:
>
> + Hugh and Patrick.
>
> On Thu, Dec 09, 2021 at 09:38:10AM -0500, John Dorminy wrote:
> > Greetings;
> >
> > It seems that this patch causes a mem= parameter to the kernel to have no effect, unfortunately...
> >
> > As far as I understand, the x86 mem parameter handler parse_memopt() (called by parse_early_param()) relies on being called after e820__memory_setup(): it simply removes any memory above the specified limit at that moment, allowing memory to later be hotplugged without regard for the initial limit. However, the initial non-hotplugged memory must already have been set up, in e820__memory_setup(), so that it can be removed in parse_memopt(); if parse_early_param() is called before e820__memory_setup(), as this change does, the parameter ends up having no effect.
> >
> > I apologize that I don't know how to fix this, but I'm happy to test patches.
>
> Yeah, people have been reporting boot failures with mem= on the cmdline.
>
> I think I see why, can you try this one:
>
> ---
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 6a190c7f4d71..6db971e61e4b 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -862,6 +862,8 @@ void __init setup_arch(char **cmdline_p)
> */
> x86_configure_nx();
>
> + e820__memory_setup();
> +
> /*
> * This parses early params and it needs to run before
> * early_reserve_memory() because latter relies on such settings
> @@ -884,7 +886,6 @@ void __init setup_arch(char **cmdline_p)
> early_reserve_memory();
>
> iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
> - e820__memory_setup();
> parse_setup_data();
>
> copy_edd();
> ---
>
Confirmed that that patch makes mem= work again:

[ 0.000000] Command line:
BOOT_IMAGE=(hd0,msdos1)/boot/vmlinuz-5.16.0-rc4+ root=UUID=0e
750e61-b92e-4708-a974-c50a3fb7e969 ro net.ifnames=0 crashkernel=128M mem=4G
...
[ 0.000000] BIOS-provided physical RAM map:
[ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009abff] usable
[ 0.000000] BIOS-e820: [mem 0x000000000009ac00-0x000000000009ffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
[ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000007dd3afff] usable
[ 0.000000] BIOS-e820: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
[ 0.000000] BIOS-e820: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
[ 0.000000] BIOS-e820: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
[ 0.000000] BIOS-e820: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
[ 0.000000] BIOS-e820: [mem 0x0000000080000000-0x000000008fffffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
[ 0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000207fffffff] usable
[ 0.000000] e820: remove [mem 0x100000000-0xfffffffffffffffe] usable
[ 0.000000] NX (Execute Disable) protection: active
[ 0.000000] user-defined physical RAM map:
[ 0.000000] user: [mem 0x0000000000000000-0x000000000009abff] usable
[ 0.000000] user: [mem 0x000000000009ac00-0x000000000009ffff] reserved
[ 0.000000] user: [mem 0x00000000000e0000-0x00000000000fffff] reserved
[ 0.000000] user: [mem 0x0000000000100000-0x000000007dd3afff] usable
[ 0.000000] user: [mem 0x000000007dd3b000-0x000000007deeffff] reserved
[ 0.000000] user: [mem 0x000000007def0000-0x000000007e0d3fff] ACPI NVS
[ 0.000000] user: [mem 0x000000007e0d4000-0x000000007f367fff] reserved
[ 0.000000] user: [mem 0x000000007f368000-0x000000007f7fffff] ACPI NVS
[ 0.000000] user: [mem 0x0000000080000000-0x000000008fffffff] reserved
[ 0.000000] user: [mem 0x00000000fed1c000-0x00000000fed3ffff] reserved
[ 0.000000] user: [mem 0x00000000ff000000-0x00000000ffffffff] reserved
...
[ 0.025520] Memory: 1762976K/2061136K available (16394K kernel
code, 3568K rwdata, 10328K rodata, 2676K init, 4924K bss, 297900K
reserved, 0K cma-reserved)


2021-12-09 16:07:38

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Thu, Dec 09, 2021 at 10:49:06AM -0500, John Dorminy wrote:
> Confirmed that that patch makes mem= work again:

Thanks!

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-12-09 16:29:42

by Mike Rapoport

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Thu, Dec 09, 2021 at 04:28:48PM +0100, Borislav Petkov wrote:
> On Thu, Dec 09, 2021 at 04:26:55PM +0100, Juergen Gross wrote:
> > Sigh. This will break Xen PV. Again. The comment above the call of
> > early_reserve_memory() tells you why.
>
> I know. I was just looking at how to fix that particular thing and was
> going to find you on IRC to talk to you about it...

The memory reservation in arch/x86/platform/efi/efi.c depends on at least
two command line parameters, I think it's better put it back later in the
boot process and move efi_memblock_x86_reserve_range() out of
early_memory_reserve().

I.e. revert c0f2077baa41 ("x86/boot: Mark prepare_command_line() __init")
and 8d48bf8206f7 ("x86/boot: Pull up cmdline preparation and early param
parsing") and add the patch below on top.

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 49b596db5631..da36b8f8430b 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -713,9 +713,6 @@ static void __init early_reserve_memory(void)

early_reserve_initrd();

- if (efi_enabled(EFI_BOOT))
- efi_memblock_x86_reserve_range();
-
memblock_x86_reserve_range_setup_data();

reserve_ibft_region();
@@ -890,6 +887,9 @@ 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

--
Sincerely yours,
Mike.

2021-12-09 16:37:52

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Thu, Dec 09, 2021 at 06:29:27PM +0200, Mike Rapoport wrote:
> On Thu, Dec 09, 2021 at 04:28:48PM +0100, Borislav Petkov wrote:
> > On Thu, Dec 09, 2021 at 04:26:55PM +0100, Juergen Gross wrote:
> > > Sigh. This will break Xen PV. Again. The comment above the call of
> > > early_reserve_memory() tells you why.
> >
> > I know. I was just looking at how to fix that particular thing and was
> > going to find you on IRC to talk to you about it...
>
> The memory reservation in arch/x86/platform/efi/efi.c depends on at least
> two command line parameters, I think it's better put it back later in the
> boot process and move efi_memblock_x86_reserve_range() out of
> early_memory_reserve().
>
> I.e. revert c0f2077baa41 ("x86/boot: Mark prepare_command_line() __init")
> and 8d48bf8206f7 ("x86/boot: Pull up cmdline preparation and early param
> parsing") and add the patch below on top.
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 49b596db5631..da36b8f8430b 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -713,9 +713,6 @@ static void __init early_reserve_memory(void)
>
> early_reserve_initrd();
>
> - if (efi_enabled(EFI_BOOT))
> - efi_memblock_x86_reserve_range();
> -
> memblock_x86_reserve_range_setup_data();
>
> reserve_ibft_region();
> @@ -890,6 +887,9 @@ 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
>
> --

Jürgen and I were thinking about a different fix but that's probably
ok too. But I've said that already about this mess and there's always
something we haven't thought about.

Whatever we do, it needs to be tested by all folks on Cc who already
reported regressions, i.e., Anjaneya, Hugh, John and Patrick.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-12-10 11:28:10

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Thu, Dec 09, 2021 at 05:37:42PM +0100, Borislav Petkov wrote:
> Whatever we do, it needs to be tested by all folks on Cc who already
> reported regressions, i.e., Anjaneya, Hugh, John and Patrick.

Ok, Mike is busy so here are some patches for testing:

https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=rc4-boot

I'd appreciate it if folks who reported an issue, verify those.

The first two are reverts which should address the issues with mem=
folks have reported. And the last one should address Anjaneya's issue.

I guess doing it the way as Mike suggested is cleaner/better.

Thx!

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-12-10 20:11:17

by Hugh Dickins

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Fri, 10 Dec 2021, Borislav Petkov wrote:
> On Thu, Dec 09, 2021 at 05:37:42PM +0100, Borislav Petkov wrote:
> > Whatever we do, it needs to be tested by all folks on Cc who already
> > reported regressions, i.e., Anjaneya, Hugh, John and Patrick.
>
> Ok, Mike is busy so here are some patches for testing:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=rc4-boot
>
> I'd appreciate it if folks who reported an issue, verify those.
>
> The first two are reverts which should address the issues with mem=
> folks have reported. And the last one should address Anjaneya's issue.
>
> I guess doing it the way as Mike suggested is cleaner/better.

Yes, mem= works fine for me, on both machines, 64-bit and 32-bit,
thanks; but I'm not exercising the troublesome EFI case at all.

Hugh

2021-12-10 20:32:18

by Patrick J. Volkerding

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Fri, Dec 10, 2021 at 5:28 AM Borislav Petkov <[email protected]> wrote:
>
> On Thu, Dec 09, 2021 at 05:37:42PM +0100, Borislav Petkov wrote:
> > Whatever we do, it needs to be tested by all folks on Cc who already
> > reported regressions, i.e., Anjaneya, Hugh, John and Patrick.
>
> Ok, Mike is busy so here are some patches for testing:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=rc4-boot
>
> I'd appreciate it if folks who reported an issue, verify those.
>
> The first two are reverts which should address the issues with mem=
> folks have reported. And the last one should address Anjaneya's issue.

I applied the two revert patches to 5.15.7 (the last one won't apply
so I skipped it) and the resulting x86 32-bit kernel boots fine here
on the Thinkpad X1E that was having issues previously.

Then I tested an unpatched 5.16-rc4, which (as expected) got the boot
hang on the affected machine. Applied the three patches, and the
resulting kernel boots fine.

Take care,

Pat

2021-12-11 05:24:44

by John Dorminy

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

Apologies for delay; my dev machine was broken much of today. But I
have tested this patch under the same conditions as previously, and
agree with Hugh that the patches make mem= work correctly.

Thanks!

John Dorminy


2021-12-11 10:14:55

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Fri, Dec 10, 2021 at 12:11:02PM -0800, Hugh Dickins wrote:
> Yes, mem= works fine for me, on both machines, 64-bit and 32-bit,
> thanks;

Thanks!

> but I'm not exercising the troublesome EFI case at all.

Yeah, I added some debug printks in a VM yesterday to confirm the
ordering. But will give Anjaneya some more time to verify, before I
queue them next week.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-12-11 10:29:38

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Fri, Dec 10, 2021 at 02:32:38PM -0600, Patrick J. Volkerding wrote:
> I applied the two revert patches to 5.15.7 (the last one won't apply
> so I skipped it)

Looks like it is only a contextual conflict.

> and the resulting x86 32-bit kernel boots fine here on the Thinkpad
> X1E that was having issues previously.

Good.

> Then I tested an unpatched 5.16-rc4, which (as expected) got the boot
> hang on the affected machine. Applied the three patches, and the
> resulting kernel boots fine.

Thanks for testing!

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-12-11 10:30:12

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Sat, Dec 11, 2021 at 12:24:25AM -0500, John Dorminy wrote:
> Apologies for delay; my dev machine was broken much of today. But I
> have tested this patch under the same conditions as previously, and
> agree with Hugh that the patches make mem= work correctly.
>
> Thanks!

Thanks for testing!

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-12-13 08:20:37

by Mike Rapoport

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Fri, Dec 10, 2021 at 12:28:09PM +0100, Borislav Petkov wrote:
> On Thu, Dec 09, 2021 at 05:37:42PM +0100, Borislav Petkov wrote:
> > Whatever we do, it needs to be tested by all folks on Cc who already
> > reported regressions, i.e., Anjaneya, Hugh, John and Patrick.
>
> Ok, Mike is busy so here are some patches for testing:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=rc4-boot

Thanks for taking care of this!

--
Sincerely yours,
Mike.

2021-12-13 09:34:40

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Mon, Dec 13, 2021 at 10:20:03AM +0200, Mike Rapoport wrote:
> Thanks for taking care of this!

Sure, no probs.

Lemme send them out officially so they're on the list. Will queue them
this week.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-12-13 11:28:07

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 0/3] x86: Fix boot ordering issues yet again

From: Borislav Petkov <[email protected]>

Hi,

here are the three I'm going to queue this week. Thanks again to
everyone for the quick testing.

Borislav Petkov (2):
Revert "x86/boot: Mark prepare_command_line() __init"
Revert "x86/boot: Pull up cmdline preparation and early param parsing"

Mike Rapoport (1):
x86/boot: Move EFI range reservation after cmdline parsing

arch/x86/kernel/setup.c | 72 +++++++++++++++++------------------------
1 file changed, 30 insertions(+), 42 deletions(-)

--
2.29.2


2021-12-13 11:28:08

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 2/3] Revert "x86/boot: Pull up cmdline preparation and early param parsing"

From: Borislav Petkov <[email protected]>

This reverts commit 8d48bf8206f77aa8687f0e241e901e5197e52423.

It turned out to be a bad idea as it broke supplying mem= cmdline
parameters due to parse_memopt() requiring preparatory work like setting
up the e820 table in e820__memory_setup() in order to be able to exclude
the range specified by mem=.

Pulling that up would've broken Xen PV again, see threads at

https://lkml.kernel.org/r/[email protected]

due to xen_memory_setup() needing the first reservations in
early_reserve_memory() - kernel and initrd - to have happened already.

This could be fixed again by having Xen do those reservations itself...

Long story short, revert this and do a simpler fix in a later patch.

Signed-off-by: Borislav Petkov <[email protected]>
---
arch/x86/kernel/setup.c | 66 +++++++++++++++++------------------------
1 file changed, 27 insertions(+), 39 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index c410be738ae7..49b596db5631 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -742,28 +742,6 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
return 0;
}

-static char *prepare_command_line(void)
-{
-#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);
-
- parse_early_param();
-
- return command_line;
-}
-
/*
* Determine if we were loaded by an EFI loader. If so, then we have also been
* passed the efi memmap, systab, etc., so we should use these data structures
@@ -852,23 +830,6 @@ void __init setup_arch(char **cmdline_p)

x86_init.oem.arch_setup();

- /*
- * x86_configure_nx() is called before parse_early_param() (called by
- * prepare_command_line()) 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();
-
- /*
- * This parses early params and it needs to run before
- * early_reserve_memory() because latter relies on such settings
- * supplied as early params.
- */
- *cmdline_p = prepare_command_line();
-
/*
* Do some memory reservations *before* memory is added to memblock, so
* memblock allocations won't overwrite it.
@@ -902,6 +863,33 @@ 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
--
2.29.2


2021-12-13 11:28:10

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 3/3] x86/boot: Move EFI range reservation after cmdline parsing

From: Mike Rapoport <[email protected]>

The memory reservation in arch/x86/platform/efi/efi.c depends on at
least two command line parameters. Put it back later in the boot process
and move efi_memblock_x86_reserve_range() out of early_memory_reserve().

An attempt to fix this was done in

8d48bf8206f7 ("x86/boot: Pull up cmdline preparation and early param parsing")

but that caused other troubles so it got reverted.

The bug this is addressing is:

Dan reports that Anjaneya Chagam can no longer use the efi=nosoftreserve
kernel command line parameter to suppress "soft reservation" behavior.

This is due to the fact that the following call-chain happens at boot:

early_reserve_memory
|-> efi_memblock_x86_reserve_range
|-> efi_fake_memmap_early

which does

if (!efi_soft_reserve_enabled())
return;

and that would have set EFI_MEM_NO_SOFT_RESERVE after having parsed
"nosoftreserve".

However, parse_early_param() gets called *after* it, leading to the boot
cmdline not being taken into account.

[ bp: Productize into a proper patch. ]

Signed-off-by: Mike Rapoport <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/setup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 49b596db5631..e04f5e6eb33f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -713,9 +713,6 @@ static void __init early_reserve_memory(void)

early_reserve_initrd();

- if (efi_enabled(EFI_BOOT))
- efi_memblock_x86_reserve_range();
-
memblock_x86_reserve_range_setup_data();

reserve_ibft_region();
@@ -890,6 +887,9 @@ 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
--
2.29.2


2021-12-15 13:05:53

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: x86/urgent] x86/boot: Move EFI range reservation after cmdline parsing

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 83757bbb9fe029b704fb28e80c3f2b92f23a1994
Gitweb: https://git.kernel.org/tip/83757bbb9fe029b704fb28e80c3f2b92f23a1994
Author: Mike Rapoport <[email protected]>
AuthorDate: Mon, 13 Dec 2021 12:27:57 +01:00
Committer: Borislav Petkov <[email protected]>
CommitterDate: Wed, 15 Dec 2021 12:36:47 +01:00

x86/boot: Move EFI range reservation after cmdline parsing

The memory reservation in arch/x86/platform/efi/efi.c depends on at
least two command line parameters. Put it back later in the boot process
and move efi_memblock_x86_reserve_range() out of early_memory_reserve().

An attempt to fix this was done in

8d48bf8206f7 ("x86/boot: Pull up cmdline preparation and early param parsing")

but that caused other troubles so it got reverted.

The bug this is addressing is:

Dan reports that Anjaneya Chagam can no longer use the efi=nosoftreserve
kernel command line parameter to suppress "soft reservation" behavior.

This is due to the fact that the following call-chain happens at boot:

early_reserve_memory
|-> efi_memblock_x86_reserve_range
|-> efi_fake_memmap_early

which does

if (!efi_soft_reserve_enabled())
return;

and that would have set EFI_MEM_NO_SOFT_RESERVE after having parsed
"nosoftreserve".

However, parse_early_param() gets called *after* it, leading to the boot
cmdline not being taken into account.

[ bp: Produce into a proper patch. ]

Signed-off-by: Borislav Petkov <[email protected]>
Signed-off-by: Mike Rapoport <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/setup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 49b596d..e04f5e6 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -713,9 +713,6 @@ static void __init early_reserve_memory(void)

early_reserve_initrd();

- if (efi_enabled(EFI_BOOT))
- efi_memblock_x86_reserve_range();
-
memblock_x86_reserve_range_setup_data();

reserve_ibft_region();
@@ -890,6 +887,9 @@ 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

2021-12-15 13:05:55

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: x86/urgent] Revert "x86/boot: Pull up cmdline preparation and early param parsing"

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: fbe6183998546f8896ee0b620ece86deff5a2fd1
Gitweb: https://git.kernel.org/tip/fbe6183998546f8896ee0b620ece86deff5a2fd1
Author: Borislav Petkov <[email protected]>
AuthorDate: Mon, 13 Dec 2021 12:27:56 +01:00
Committer: Borislav Petkov <[email protected]>
CommitterDate: Wed, 15 Dec 2021 11:38:57 +01:00

Revert "x86/boot: Pull up cmdline preparation and early param parsing"

This reverts commit 8d48bf8206f77aa8687f0e241e901e5197e52423.

It turned out to be a bad idea as it broke supplying mem= cmdline
parameters due to parse_memopt() requiring preparatory work like setting
up the e820 table in e820__memory_setup() in order to be able to exclude
the range specified by mem=.

Pulling that up would've broken Xen PV again, see threads at

https://lkml.kernel.org/r/[email protected]

due to xen_memory_setup() needing the first reservations in
early_reserve_memory() - kernel and initrd - to have happened already.

This could be fixed again by having Xen do those reservations itself...

Long story short, revert this and do a simpler fix in a later patch.

Signed-off-by: Borislav Petkov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/setup.c | 66 ++++++++++++++++------------------------
1 file changed, 27 insertions(+), 39 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index c410be7..49b596d 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -742,28 +742,6 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
return 0;
}

-static char *prepare_command_line(void)
-{
-#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);
-
- parse_early_param();
-
- return command_line;
-}
-
/*
* Determine if we were loaded by an EFI loader. If so, then we have also been
* passed the efi memmap, systab, etc., so we should use these data structures
@@ -853,23 +831,6 @@ void __init setup_arch(char **cmdline_p)
x86_init.oem.arch_setup();

/*
- * x86_configure_nx() is called before parse_early_param() (called by
- * prepare_command_line()) 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();
-
- /*
- * This parses early params and it needs to run before
- * early_reserve_memory() because latter relies on such settings
- * supplied as early params.
- */
- *cmdline_p = prepare_command_line();
-
- /*
* Do some memory reservations *before* memory is added to memblock, so
* memblock allocations won't overwrite it.
*
@@ -902,6 +863,33 @@ 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

2021-12-15 13:12:32

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: x86/urgent] x86/boot: Move EFI range reservation after cmdline parsing

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 2f5b3514c33fecad4003ce0f22ca9691492d310b
Gitweb: https://git.kernel.org/tip/2f5b3514c33fecad4003ce0f22ca9691492d310b
Author: Mike Rapoport <[email protected]>
AuthorDate: Mon, 13 Dec 2021 12:27:57 +01:00
Committer: Borislav Petkov <[email protected]>
CommitterDate: Wed, 15 Dec 2021 14:07:54 +01:00

x86/boot: Move EFI range reservation after cmdline parsing

The memory reservation in arch/x86/platform/efi/efi.c depends on at
least two command line parameters. Put it back later in the boot process
and move efi_memblock_x86_reserve_range() out of early_memory_reserve().

An attempt to fix this was done in

8d48bf8206f7 ("x86/boot: Pull up cmdline preparation and early param parsing")

but that caused other troubles so it got reverted.

The bug this is addressing is:

Dan reports that Anjaneya Chagam can no longer use the efi=nosoftreserve
kernel command line parameter to suppress "soft reservation" behavior.

This is due to the fact that the following call-chain happens at boot:

early_reserve_memory
|-> efi_memblock_x86_reserve_range
|-> efi_fake_memmap_early

which does

if (!efi_soft_reserve_enabled())
return;

and that would have set EFI_MEM_NO_SOFT_RESERVE after having parsed
"nosoftreserve".

However, parse_early_param() gets called *after* it, leading to the boot
cmdline not being taken into account.

See also https://lore.kernel.org/r/[email protected]

[ bp: Turn into a proper patch. ]

Signed-off-by: Mike Rapoport <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/setup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 49b596d..e04f5e6 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -713,9 +713,6 @@ static void __init early_reserve_memory(void)

early_reserve_initrd();

- if (efi_enabled(EFI_BOOT))
- efi_memblock_x86_reserve_range();
-
memblock_x86_reserve_range_setup_data();

reserve_ibft_region();
@@ -890,6 +887,9 @@ 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

2021-12-20 18:49:20

by Patrick J. Volkerding

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

Trying again since gmail didn't use plain text and the message got rejected.

We're waiting for these patches to appear in a 5.15 kernel so that we
can ship with an unpatched kernel. Will they be queued for the stable
kernels sometime soon?

Thanks,

Pat


On Mon, Dec 20, 2021 at 12:43 PM Patrick J. Volkerding
<[email protected]> wrote:
>
> Will these patches be queued for the stable kernels soon?
>
> Thanks,
>
> Pat
>
> On Mon, Dec 13, 2021, 03:33 Borislav Petkov <[email protected]> wrote:
>>
>> On Mon, Dec 13, 2021 at 10:20:03AM +0200, Mike Rapoport wrote:
>> > Thanks for taking care of this!
>>
>> Sure, no probs.
>>
>> Lemme send them out officially so they're on the list. Will queue them
>> this week.
>>
>> Thx.
>>
>> --
>> Regards/Gruss,
>> Boris.
>>
>> https://people.kernel.org/tglx/notes-about-netiquette

2021-12-20 18:59:50

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Mon, Dec 20, 2021 at 12:49:52PM -0600, Patrick J. Volkerding wrote:
> Trying again since gmail didn't use plain text and the message got rejected.
>
> We're waiting for these patches to appear in a 5.15 kernel so that we
> can ship with an unpatched kernel. Will they be queued for the stable
> kernels sometime soon?

Yes, they're currently queued here:

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=x86/urgent

and will go to Linus on the weekend.

Which reminds me - they need stable tags. Lemme add those.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-12-20 19:08:48

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/boot: Pull up cmdline preparation and early param parsing

On Mon, Dec 20, 2021 at 07:59:49PM +0100, Borislav Petkov wrote:
> Which reminds me - they need stable tags. Lemme add those.

Actually, it'll be a lot easier if I send backporting instructions to
the stable@ folks next week. Yap, I'll do that.

/me adds a TODO.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette