linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] initrd: Remove erroneous comment
@ 2020-06-19 14:30 Tom Rini
  2020-06-19 17:49 ` [tip: x86/cleanups] " tip-bot2 for Tom Rini
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Tom Rini @ 2020-06-19 14:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Borislav Petkov, Dominik Brodowski,
	H . Peter Anvin, Ronald G . Minnich

Most architectures have been passing the location of an initrd via the
initrd= option since their inception.  Remove the comment as it's both
wrong and unrelated to the commit that introduced it.

Fixes: 694cfd87b0c8 ("x86/setup: Add an initrdmem= option to specify initrd physical address")
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: H. Peter Anvin (Intel) <hpa@zytor.com>
Cc: Ronald G. Minnich <rminnich@gmail.com>
Signed-off-by: Tom Rini <trini@konsulko.com>
---
For a bit more context, I assume there's been some confusion between
"initrd" being a keyword in things like extlinux.conf and also that for
quite a long time now initrd information is passed via device tree and
not the command line on relevant architectures.  But it's still true
that it's been a valid command line option to the kernel since the 90s.
It's just the case that in 2018 the code was consolidated from under
arch/ and in to this file.
---
 init/do_mounts_initrd.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c
index d72beda824aa..53314d7da4be 100644
--- a/init/do_mounts_initrd.c
+++ b/init/do_mounts_initrd.c
@@ -45,11 +45,6 @@ static int __init early_initrdmem(char *p)
 }
 early_param("initrdmem", early_initrdmem);
 
-/*
- * This is here as the initrd keyword has been in use since 11/2018
- * on ARM, PowerPC, and MIPS.
- * It should not be; it is reserved for bootloaders.
- */
 static int __init early_initrd(char *p)
 {
 	return early_initrdmem(p);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip: x86/cleanups] initrd: Remove erroneous comment
  2020-06-19 14:30 [PATCH] initrd: Remove erroneous comment Tom Rini
@ 2020-06-19 17:49 ` tip-bot2 for Tom Rini
  2020-06-20  0:03 ` [PATCH] " ron minnich
  2020-06-22 20:02 ` ron minnich
  2 siblings, 0 replies; 12+ messages in thread
From: tip-bot2 for Tom Rini @ 2020-06-19 17:49 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Tom Rini, Borislav Petkov, x86, LKML

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

Commit-ID:     eacb0c101a0bdf14de77cc9d107493e2d8d6389c
Gitweb:        https://git.kernel.org/tip/eacb0c101a0bdf14de77cc9d107493e2d8d6389c
Author:        Tom Rini <trini@konsulko.com>
AuthorDate:    Fri, 19 Jun 2020 10:30:56 -04:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 19 Jun 2020 19:23:54 +02:00

initrd: Remove erroneous comment

Most architectures have been passing the location of an initrd via the
initrd= option since their inception.  Remove the comment as it's both
wrong and unrelated to the commit that introduced it.

For a bit more context, I assume there's been some confusion between
"initrd" being a keyword in things like extlinux.conf and also that for
quite a long time now initrd information is passed via device tree and
not the command line on relevant architectures. But it's still true that
it's been a valid command line option to the kernel since the 90s. It's
just the case that in 2018 the code was consolidated from under arch/
and in to this file.

 [ bp: Move the context clarification up into the commit message proper. ]

Fixes: 694cfd87b0c8 ("x86/setup: Add an initrdmem= option to specify initrd physical address")
Signed-off-by: Tom Rini <trini@konsulko.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: https://lkml.kernel.org/r/20200619143056.24538-1-trini@konsulko.com
---
 init/do_mounts_initrd.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c
index d72beda..53314d7 100644
--- a/init/do_mounts_initrd.c
+++ b/init/do_mounts_initrd.c
@@ -45,11 +45,6 @@ static int __init early_initrdmem(char *p)
 }
 early_param("initrdmem", early_initrdmem);
 
-/*
- * This is here as the initrd keyword has been in use since 11/2018
- * on ARM, PowerPC, and MIPS.
- * It should not be; it is reserved for bootloaders.
- */
 static int __init early_initrd(char *p)
 {
 	return early_initrdmem(p);

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] initrd: Remove erroneous comment
  2020-06-19 14:30 [PATCH] initrd: Remove erroneous comment Tom Rini
  2020-06-19 17:49 ` [tip: x86/cleanups] " tip-bot2 for Tom Rini
@ 2020-06-20  0:03 ` ron minnich
  2020-06-22 20:23   ` hpa
  2020-06-22 20:02 ` ron minnich
  2 siblings, 1 reply; 12+ messages in thread
From: ron minnich @ 2020-06-20  0:03 UTC (permalink / raw)
  To: Tom Rini
  Cc: lkml - Kernel Mailing List, Andrew Morton, Borislav Petkov,
	Dominik Brodowski, H . Peter Anvin

It seems fine to me, but I did not initially object to the use of that
name anyway. hpa, what do you think?

On Fri, Jun 19, 2020 at 7:31 AM Tom Rini <trini@konsulko.com> wrote:
>
> Most architectures have been passing the location of an initrd via the
> initrd= option since their inception.  Remove the comment as it's both
> wrong and unrelated to the commit that introduced it.
>
> Fixes: 694cfd87b0c8 ("x86/setup: Add an initrdmem= option to specify initrd physical address")
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Dominik Brodowski <linux@dominikbrodowski.net>
> Cc: H. Peter Anvin (Intel) <hpa@zytor.com>
> Cc: Ronald G. Minnich <rminnich@gmail.com>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> For a bit more context, I assume there's been some confusion between
> "initrd" being a keyword in things like extlinux.conf and also that for
> quite a long time now initrd information is passed via device tree and
> not the command line on relevant architectures.  But it's still true
> that it's been a valid command line option to the kernel since the 90s.
> It's just the case that in 2018 the code was consolidated from under
> arch/ and in to this file.
> ---
>  init/do_mounts_initrd.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c
> index d72beda824aa..53314d7da4be 100644
> --- a/init/do_mounts_initrd.c
> +++ b/init/do_mounts_initrd.c
> @@ -45,11 +45,6 @@ static int __init early_initrdmem(char *p)
>  }
>  early_param("initrdmem", early_initrdmem);
>
> -/*
> - * This is here as the initrd keyword has been in use since 11/2018
> - * on ARM, PowerPC, and MIPS.
> - * It should not be; it is reserved for bootloaders.
> - */
>  static int __init early_initrd(char *p)
>  {
>         return early_initrdmem(p);
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] initrd: Remove erroneous comment
  2020-06-19 14:30 [PATCH] initrd: Remove erroneous comment Tom Rini
  2020-06-19 17:49 ` [tip: x86/cleanups] " tip-bot2 for Tom Rini
  2020-06-20  0:03 ` [PATCH] " ron minnich
@ 2020-06-22 20:02 ` ron minnich
  2020-06-22 20:40   ` Tom Rini
  2 siblings, 1 reply; 12+ messages in thread
From: ron minnich @ 2020-06-22 20:02 UTC (permalink / raw)
  To: Tom Rini
  Cc: lkml - Kernel Mailing List, Andrew Morton, Borislav Petkov,
	Dominik Brodowski, H . Peter Anvin

The other thing you ought to consider fixing:
initrd is documented as follows:

        initrd=         [BOOT] Specify the location of the initial ramdisk

for bootloaders only.

UEFI consumes initrd from the command line as well. As ARM servers
increasingly use UEFI, there may be situations in which the initrd
option doesn't make its way to the kernel? I don't know, UEFI is such
a black box to me. But I've seen this "initrd consumption" happen.

Based on docs, and the growing use of bootloaders that are happy to
consume initrd= and not pass it to the kernel, you might be better off
trying to move to the new command line option anyway.

IOW, this comment may not be what people want to see, but ... it might
also be right. Or possibly changed to:

/*
 * The initrd keyword is in use today on ARM, PowerPC, and MIPS.
 * It is also reserved for use by bootloaders such as UEFI and may
 * be consumed by them and not passed on to the kernel.
 * The documentation also shows it as reserved for bootloaders.
 * It is advised to move to the initrdmem= option whereever possible.
 */

On Fri, Jun 19, 2020 at 7:31 AM Tom Rini <trini@konsulko.com> wrote:
>
> Most architectures have been passing the location of an initrd via the
> initrd= option since their inception.  Remove the comment as it's both
> wrong and unrelated to the commit that introduced it.
>
> Fixes: 694cfd87b0c8 ("x86/setup: Add an initrdmem= option to specify initrd physical address")
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Dominik Brodowski <linux@dominikbrodowski.net>
> Cc: H. Peter Anvin (Intel) <hpa@zytor.com>
> Cc: Ronald G. Minnich <rminnich@gmail.com>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> For a bit more context, I assume there's been some confusion between
> "initrd" being a keyword in things like extlinux.conf and also that for
> quite a long time now initrd information is passed via device tree and
> not the command line on relevant architectures.  But it's still true
> that it's been a valid command line option to the kernel since the 90s.
> It's just the case that in 2018 the code was consolidated from under
> arch/ and in to this file.
> ---
>  init/do_mounts_initrd.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c
> index d72beda824aa..53314d7da4be 100644
> --- a/init/do_mounts_initrd.c
> +++ b/init/do_mounts_initrd.c
> @@ -45,11 +45,6 @@ static int __init early_initrdmem(char *p)
>  }
>  early_param("initrdmem", early_initrdmem);
>
> -/*
> - * This is here as the initrd keyword has been in use since 11/2018
> - * on ARM, PowerPC, and MIPS.
> - * It should not be; it is reserved for bootloaders.
> - */
>  static int __init early_initrd(char *p)
>  {
>         return early_initrdmem(p);
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] initrd: Remove erroneous comment
  2020-06-20  0:03 ` [PATCH] " ron minnich
@ 2020-06-22 20:23   ` hpa
  0 siblings, 0 replies; 12+ messages in thread
From: hpa @ 2020-06-22 20:23 UTC (permalink / raw)
  To: ron minnich, Tom Rini
  Cc: lkml - Kernel Mailing List, Andrew Morton, Borislav Petkov,
	Dominik Brodowski

On June 19, 2020 5:03:33 PM PDT, ron minnich <rminnich@gmail.com> wrote:
>It seems fine to me, but I did not initially object to the use of that
>name anyway. hpa, what do you think?
>
>On Fri, Jun 19, 2020 at 7:31 AM Tom Rini <trini@konsulko.com> wrote:
>>
>> Most architectures have been passing the location of an initrd via
>the
>> initrd= option since their inception.  Remove the comment as it's
>both
>> wrong and unrelated to the commit that introduced it.
>>
>> Fixes: 694cfd87b0c8 ("x86/setup: Add an initrdmem= option to specify
>initrd physical address")
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: Dominik Brodowski <linux@dominikbrodowski.net>
>> Cc: H. Peter Anvin (Intel) <hpa@zytor.com>
>> Cc: Ronald G. Minnich <rminnich@gmail.com>
>> Signed-off-by: Tom Rini <trini@konsulko.com>
>> ---
>> For a bit more context, I assume there's been some confusion between
>> "initrd" being a keyword in things like extlinux.conf and also that
>for
>> quite a long time now initrd information is passed via device tree
>and
>> not the command line on relevant architectures.  But it's still true
>> that it's been a valid command line option to the kernel since the
>90s.
>> It's just the case that in 2018 the code was consolidated from under
>> arch/ and in to this file.
>> ---
>>  init/do_mounts_initrd.c | 5 -----
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c
>> index d72beda824aa..53314d7da4be 100644
>> --- a/init/do_mounts_initrd.c
>> +++ b/init/do_mounts_initrd.c
>> @@ -45,11 +45,6 @@ static int __init early_initrdmem(char *p)
>>  }
>>  early_param("initrdmem", early_initrdmem);
>>
>> -/*
>> - * This is here as the initrd keyword has been in use since 11/2018
>> - * on ARM, PowerPC, and MIPS.
>> - * It should not be; it is reserved for bootloaders.
>> - */
>>  static int __init early_initrd(char *p)
>>  {
>>         return early_initrdmem(p);
>> --
>> 2.17.1
>>

Well, I observe that it was documented as reserved for bootloaders since the mid-90s at least.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] initrd: Remove erroneous comment
  2020-06-22 20:02 ` ron minnich
@ 2020-06-22 20:40   ` Tom Rini
  2020-06-22 20:48     ` H. Peter Anvin
  2020-06-22 20:56     ` ron minnich
  0 siblings, 2 replies; 12+ messages in thread
From: Tom Rini @ 2020-06-22 20:40 UTC (permalink / raw)
  To: ron minnich
  Cc: lkml - Kernel Mailing List, Andrew Morton, Borislav Petkov,
	Dominik Brodowski, H . Peter Anvin

[-- Attachment #1: Type: text/plain, Size: 2839 bytes --]

On Mon, Jun 22, 2020 at 01:02:16PM -0700, ron minnich wrote:

> The other thing you ought to consider fixing:
> initrd is documented as follows:
> 
>         initrd=         [BOOT] Specify the location of the initial ramdisk
> 
> for bootloaders only.
> 
> UEFI consumes initrd from the command line as well. As ARM servers
> increasingly use UEFI, there may be situations in which the initrd
> option doesn't make its way to the kernel? I don't know, UEFI is such
> a black box to me. But I've seen this "initrd consumption" happen.
> 
> Based on docs, and the growing use of bootloaders that are happy to
> consume initrd= and not pass it to the kernel, you might be better off
> trying to move to the new command line option anyway.
> 
> IOW, this comment may not be what people want to see, but ... it might
> also be right. Or possibly changed to:
> 
> /*
>  * The initrd keyword is in use today on ARM, PowerPC, and MIPS.
>  * It is also reserved for use by bootloaders such as UEFI and may
>  * be consumed by them and not passed on to the kernel.
>  * The documentation also shows it as reserved for bootloaders.
>  * It is advised to move to the initrdmem= option whereever possible.
>  */

Fair warning, one of the other hats I wear is the chief custodian of the
U-Boot project.

Note that on most architectures in modern times the device tree is used
to pass in initrd type information and "initrd=" on the command line is
quite legacy.

But what do you mean UEFI "consumes" initrd= ?  It's quite expected that
when you configure grub/syslinux/systemd-boot/whatever via extlinux.conf
or similar with "initrd /some/file" something reasonable happens to
read that in to memory and pass along the location to Linux (which can
vary from arch to arch, when not using device tree).  I guess looking at 
Documentation/x86/boot.rst is where treating initrd= as a file that
should be handled and ramdisk_image / ramdisk_size set came from.  I do
wonder what happens in the case of ARM/ARM64 + UEFI without device tree.

That said, no the comment is wrong.  It's not "since 11/2018" but "since
the 1990s".  And it doesn't provide any sort of link / context to the
boot loader specification project or similar that explains the cases
when a non-filename "initrd=" would reasonably (or unreasonably but
happens in reality) be removed.

I would go so far as to suggest that adding special handling for some
x86 setups is the wrong to place to start / further deprecate how other
architectures and firmwares handle a given situation.  I'm only chiming
in here as I saw this commit go by on LWN and wanted to see how this was
different from the traditional usage of initrd= in the rest of the
kernel (it's not) and then saw the otherwise unrelated new comment being
added.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] initrd: Remove erroneous comment
  2020-06-22 20:40   ` Tom Rini
@ 2020-06-22 20:48     ` H. Peter Anvin
  2020-06-22 21:06       ` Tom Rini
  2020-06-22 20:56     ` ron minnich
  1 sibling, 1 reply; 12+ messages in thread
From: H. Peter Anvin @ 2020-06-22 20:48 UTC (permalink / raw)
  To: Tom Rini, ron minnich
  Cc: lkml - Kernel Mailing List, Andrew Morton, Borislav Petkov,
	Dominik Brodowski

On 2020-06-22 13:40, Tom Rini wrote:
> On Mon, Jun 22, 2020 at 01:02:16PM -0700, ron minnich wrote:
> 
>> The other thing you ought to consider fixing:
>> initrd is documented as follows:
>>
>>         initrd=         [BOOT] Specify the location of the initial ramdisk
>>
>> for bootloaders only.
>>
>> UEFI consumes initrd from the command line as well. As ARM servers
>> increasingly use UEFI, there may be situations in which the initrd
>> option doesn't make its way to the kernel? I don't know, UEFI is such
>> a black box to me. But I've seen this "initrd consumption" happen.
>>
>> Based on docs, and the growing use of bootloaders that are happy to
>> consume initrd= and not pass it to the kernel, you might be better off
>> trying to move to the new command line option anyway.
>>
>> IOW, this comment may not be what people want to see, but ... it might
>> also be right. Or possibly changed to:
>>
>> /*
>>  * The initrd keyword is in use today on ARM, PowerPC, and MIPS.
>>  * It is also reserved for use by bootloaders such as UEFI and may
>>  * be consumed by them and not passed on to the kernel.
>>  * The documentation also shows it as reserved for bootloaders.
>>  * It is advised to move to the initrdmem= option whereever possible.
>>  */
> 
> Fair warning, one of the other hats I wear is the chief custodian of the
> U-Boot project.
> 
> Note that on most architectures in modern times the device tree is used
> to pass in initrd type information and "initrd=" on the command line is
> quite legacy.
> 
> But what do you mean UEFI "consumes" initrd= ?  It's quite expected that
> when you configure grub/syslinux/systemd-boot/whatever via extlinux.conf
> or similar with "initrd /some/file" something reasonable happens to
> read that in to memory and pass along the location to Linux (which can
> vary from arch to arch, when not using device tree).  I guess looking at 
> Documentation/x86/boot.rst is where treating initrd= as a file that
> should be handled and ramdisk_image / ramdisk_size set came from.  I do
> wonder what happens in the case of ARM/ARM64 + UEFI without device tree.
> 

UEFI plus the in-kernel UEFI stub is, in some ways, a "bootloader" in
the traditional sense. It is totally fair that we should update the
documentation with this as a different case, though, because it is part
of the kernel tree and so the kernel now has partial ownership of the
namespace.

I suggest "STUB" for "in-kernel firmware stub" for this purpose; no need
to restrict it to a specific firmware for the purpose of namespace
reservation.

	-hpa

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] initrd: Remove erroneous comment
  2020-06-22 20:40   ` Tom Rini
  2020-06-22 20:48     ` H. Peter Anvin
@ 2020-06-22 20:56     ` ron minnich
  2020-06-22 21:01       ` Tom Rini
  1 sibling, 1 reply; 12+ messages in thread
From: ron minnich @ 2020-06-22 20:56 UTC (permalink / raw)
  To: Tom Rini
  Cc: lkml - Kernel Mailing List, Andrew Morton, Borislav Petkov,
	Dominik Brodowski, H . Peter Anvin

So, let me first add,  the comment can be removed as needed. Comments
offered only for clarification.

On Mon, Jun 22, 2020 at 1:40 PM Tom Rini <trini@konsulko.com> wrote:

> But what do you mean UEFI "consumes" initrd= ?

What I mean is, there are bootloaders that will, if they see initrd=
in the command line, remove it: the kernel will never see it.

>  I guess looking at
> Documentation/x86/boot.rst is where treating initrd= as a file that
> should be handled and ramdisk_image / ramdisk_size set came from.  I do
> wonder what happens in the case of ARM/ARM64 + UEFI without device tree.

it is possible that the initrd= argument will not be seen by the
kernel. That's my understanding. Will this be a problem if so? It
would be for me :-)

>  And it doesn't provide any sort of link / context to the
> boot loader specification project or similar that explains the cases
> when a non-filename "initrd=" would reasonably (or unreasonably but
> happens in reality) be removed.

But it unreasonably happens as I learned the hard way :-)

Anyway, thanks Tom, I have no objections to whatever you all feel is
best to do with that comment. It was a failed attempt on my part to
explain the state of things :-)

ron

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] initrd: Remove erroneous comment
  2020-06-22 20:56     ` ron minnich
@ 2020-06-22 21:01       ` Tom Rini
  2020-06-22 21:03         ` H. Peter Anvin
  0 siblings, 1 reply; 12+ messages in thread
From: Tom Rini @ 2020-06-22 21:01 UTC (permalink / raw)
  To: ron minnich
  Cc: lkml - Kernel Mailing List, Andrew Morton, Borislav Petkov,
	Dominik Brodowski, H . Peter Anvin

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

On Mon, Jun 22, 2020 at 01:56:24PM -0700, ron minnich wrote:

> So, let me first add,  the comment can be removed as needed. Comments
> offered only for clarification.

Noted, thanks.

> On Mon, Jun 22, 2020 at 1:40 PM Tom Rini <trini@konsulko.com> wrote:
> 
> > But what do you mean UEFI "consumes" initrd= ?
> 
> What I mean is, there are bootloaders that will, if they see initrd=
> in the command line, remove it: the kernel will never see it.

I'm picky here because, well, there's a whole lot of moving parts in the
pre-kernel world.  In a strict sense, "UEFI" doesn't do anything with
the kernel but based on hpa's comments I assume that at least the
in-kernel UEFI stub does what Documentation/x86/booting.rst suggests to
do and consumes initrd=/file just like "initrd /file" in extlinux.conf,
etc do.  And since the EFI stub is cross-platform, it's worth noting
this too.

> >  I guess looking at
> > Documentation/x86/boot.rst is where treating initrd= as a file that
> > should be handled and ramdisk_image / ramdisk_size set came from.  I do
> > wonder what happens in the case of ARM/ARM64 + UEFI without device tree.
> 
> it is possible that the initrd= argument will not be seen by the
> kernel. That's my understanding. Will this be a problem if so? It
> would be for me :-)
> 
> >  And it doesn't provide any sort of link / context to the
> > boot loader specification project or similar that explains the cases
> > when a non-filename "initrd=" would reasonably (or unreasonably but
> > happens in reality) be removed.
> 
> But it unreasonably happens as I learned the hard way :-)
> 
> Anyway, thanks Tom, I have no objections to whatever you all feel is
> best to do with that comment. It was a failed attempt on my part to
> explain the state of things :-)

Booting up the kernel is quite the "fun" area indeed.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] initrd: Remove erroneous comment
  2020-06-22 21:01       ` Tom Rini
@ 2020-06-22 21:03         ` H. Peter Anvin
  2020-06-22 21:07           ` Tom Rini
  0 siblings, 1 reply; 12+ messages in thread
From: H. Peter Anvin @ 2020-06-22 21:03 UTC (permalink / raw)
  To: Tom Rini, ron minnich
  Cc: lkml - Kernel Mailing List, Andrew Morton, Borislav Petkov,
	Dominik Brodowski

On 2020-06-22 14:01, Tom Rini wrote:
> 
> I'm picky here because, well, there's a whole lot of moving parts in the
> pre-kernel world.  In a strict sense, "UEFI" doesn't do anything with
> the kernel but based on hpa's comments I assume that at least the
> in-kernel UEFI stub does what Documentation/x86/booting.rst suggests to
> do and consumes initrd=/file just like "initrd /file" in extlinux.conf,
> etc do.  And since the EFI stub is cross-platform, it's worth noting
> this too.
> 

For what it's worth, normally boot loaders don't strip this from the
kernel command line passed to the kernel, although there might be ones
which do so. In general this is bad practice; it is better to let the
initrd show in /proc/cmdline.

	-hpa


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] initrd: Remove erroneous comment
  2020-06-22 20:48     ` H. Peter Anvin
@ 2020-06-22 21:06       ` Tom Rini
  0 siblings, 0 replies; 12+ messages in thread
From: Tom Rini @ 2020-06-22 21:06 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: ron minnich, lkml - Kernel Mailing List, Andrew Morton,
	Borislav Petkov, Dominik Brodowski

[-- Attachment #1: Type: text/plain, Size: 3251 bytes --]

On Mon, Jun 22, 2020 at 01:48:45PM -0700, H. Peter Anvin wrote:
> On 2020-06-22 13:40, Tom Rini wrote:
> > On Mon, Jun 22, 2020 at 01:02:16PM -0700, ron minnich wrote:
> > 
> >> The other thing you ought to consider fixing:
> >> initrd is documented as follows:
> >>
> >>         initrd=         [BOOT] Specify the location of the initial ramdisk
> >>
> >> for bootloaders only.
> >>
> >> UEFI consumes initrd from the command line as well. As ARM servers
> >> increasingly use UEFI, there may be situations in which the initrd
> >> option doesn't make its way to the kernel? I don't know, UEFI is such
> >> a black box to me. But I've seen this "initrd consumption" happen.
> >>
> >> Based on docs, and the growing use of bootloaders that are happy to
> >> consume initrd= and not pass it to the kernel, you might be better off
> >> trying to move to the new command line option anyway.
> >>
> >> IOW, this comment may not be what people want to see, but ... it might
> >> also be right. Or possibly changed to:
> >>
> >> /*
> >>  * The initrd keyword is in use today on ARM, PowerPC, and MIPS.
> >>  * It is also reserved for use by bootloaders such as UEFI and may
> >>  * be consumed by them and not passed on to the kernel.
> >>  * The documentation also shows it as reserved for bootloaders.
> >>  * It is advised to move to the initrdmem= option whereever possible.
> >>  */
> > 
> > Fair warning, one of the other hats I wear is the chief custodian of the
> > U-Boot project.
> > 
> > Note that on most architectures in modern times the device tree is used
> > to pass in initrd type information and "initrd=" on the command line is
> > quite legacy.
> > 
> > But what do you mean UEFI "consumes" initrd= ?  It's quite expected that
> > when you configure grub/syslinux/systemd-boot/whatever via extlinux.conf
> > or similar with "initrd /some/file" something reasonable happens to
> > read that in to memory and pass along the location to Linux (which can
> > vary from arch to arch, when not using device tree).  I guess looking at 
> > Documentation/x86/boot.rst is where treating initrd= as a file that
> > should be handled and ramdisk_image / ramdisk_size set came from.  I do
> > wonder what happens in the case of ARM/ARM64 + UEFI without device tree.
> > 
> 
> UEFI plus the in-kernel UEFI stub is, in some ways, a "bootloader" in
> the traditional sense. It is totally fair that we should update the
> documentation with this as a different case, though, because it is part
> of the kernel tree and so the kernel now has partial ownership of the
> namespace.
> 
> I suggest "STUB" for "in-kernel firmware stub" for this purpose; no need
> to restrict it to a specific firmware for the purpose of namespace
> reservation.

With a little bit of quick digging, yes, it would be good to document
and be very clear which things are reserved for (and how are treated by)
the in-kernel firmware stub or "kernel EFI stub" or whatever name is
best for drivers/firmware/efi/libstub/.  I forget the last time we tried
booting a linux kernel EFI stub rather than grub/etc over in U-Boot
under our EFI loader support but it's reasonable to expect that it work.
Thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] initrd: Remove erroneous comment
  2020-06-22 21:03         ` H. Peter Anvin
@ 2020-06-22 21:07           ` Tom Rini
  0 siblings, 0 replies; 12+ messages in thread
From: Tom Rini @ 2020-06-22 21:07 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: ron minnich, lkml - Kernel Mailing List, Andrew Morton,
	Borislav Petkov, Dominik Brodowski

[-- Attachment #1: Type: text/plain, Size: 865 bytes --]

On Mon, Jun 22, 2020 at 02:03:28PM -0700, H. Peter Anvin wrote:
> On 2020-06-22 14:01, Tom Rini wrote:
> > 
> > I'm picky here because, well, there's a whole lot of moving parts in the
> > pre-kernel world.  In a strict sense, "UEFI" doesn't do anything with
> > the kernel but based on hpa's comments I assume that at least the
> > in-kernel UEFI stub does what Documentation/x86/booting.rst suggests to
> > do and consumes initrd=/file just like "initrd /file" in extlinux.conf,
> > etc do.  And since the EFI stub is cross-platform, it's worth noting
> > this too.
> 
> For what it's worth, normally boot loaders don't strip this from the
> kernel command line passed to the kernel, although there might be ones
> which do so. In general this is bad practice; it is better to let the
> initrd show in /proc/cmdline.

Strongly agree.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-06-22 21:07 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-19 14:30 [PATCH] initrd: Remove erroneous comment Tom Rini
2020-06-19 17:49 ` [tip: x86/cleanups] " tip-bot2 for Tom Rini
2020-06-20  0:03 ` [PATCH] " ron minnich
2020-06-22 20:23   ` hpa
2020-06-22 20:02 ` ron minnich
2020-06-22 20:40   ` Tom Rini
2020-06-22 20:48     ` H. Peter Anvin
2020-06-22 21:06       ` Tom Rini
2020-06-22 20:56     ` ron minnich
2020-06-22 21:01       ` Tom Rini
2020-06-22 21:03         ` H. Peter Anvin
2020-06-22 21:07           ` Tom Rini

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).