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 <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dominik Brodowski <[email protected]>
Cc: H. Peter Anvin (Intel) <[email protected]>
Cc: Ronald G. Minnich <[email protected]>
Signed-off-by: Tom Rini <[email protected]>
---
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
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 <[email protected]>
AuthorDate: Fri, 19 Jun 2020 10:30:56 -04:00
Committer: Borislav Petkov <[email protected]>
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 <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
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);
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 <[email protected]> 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 <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Dominik Brodowski <[email protected]>
> Cc: H. Peter Anvin (Intel) <[email protected]>
> Cc: Ronald G. Minnich <[email protected]>
> Signed-off-by: Tom Rini <[email protected]>
> ---
> 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
>
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 <[email protected]> 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 <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Dominik Brodowski <[email protected]>
> Cc: H. Peter Anvin (Intel) <[email protected]>
> Cc: Ronald G. Minnich <[email protected]>
> Signed-off-by: Tom Rini <[email protected]>
> ---
> 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
>
On June 19, 2020 5:03:33 PM PDT, ron minnich <[email protected]> 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 <[email protected]> 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 <[email protected]>
>> Cc: Borislav Petkov <[email protected]>
>> Cc: Dominik Brodowski <[email protected]>
>> Cc: H. Peter Anvin (Intel) <[email protected]>
>> Cc: Ronald G. Minnich <[email protected]>
>> Signed-off-by: Tom Rini <[email protected]>
>> ---
>> 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.
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
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
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 <[email protected]> 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
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 <[email protected]> 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
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
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
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