Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200619143056.24538-1-trini@konsulko.com>
In-Reply-To: <20200619143056.24538-1-trini@konsulko.com>
From:   ron minnich <rminnich@gmail.com>
Date:   Mon, 22 Jun 2020 13:02:16 -0700
Message-ID: <CAP6exYJ64Hy9y3Dzh9Asrq8Y0oDWYk+tf4UAcasEc-ZxTY8DAw@mail.gmail.com>
Subject: Re: [PATCH] initrd: Remove erroneous comment
To:     Tom Rini <trini@konsulko.com>
Cc:     lkml - Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Borislav Petkov <bp@suse.de>,
        Dominik Brodowski <linux@dominikbrodowski.net>,
        "H . Peter Anvin" <hpa@zytor.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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
>