Re: [PATCH] riscv: Drop setup_initrd

[Palmer Dabbelt]

On Tue, 28 Aug 2018 14:59:59 PDT (-0700), linux@xxxxxxxxxxxx wrote:
> On Tue, Aug 28, 2018 at 11:46:09PM +0200, Andreas Schwab wrote:
> > On Aug 28 2018, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> >
> > > On Tue, Aug 28, 2018 at 01:10:20PM -0700, Palmer Dabbelt wrote:
> > >> On Thu, 09 Aug 2018 21:11:40 PDT (-0700), linux@xxxxxxxxxxxx wrote:
> > >> >setup_initrd() does not appear to serve a practical purpose other than
> > >> >preventing qemu boots with "-initrd" parameter, so let's drop it.
> > >> >
> > >> >Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> > >> >---
> > >> > arch/riscv/kernel/setup.c | 39 ---------------------------------------
> > >> > 1 file changed, 39 deletions(-)
> > >> >
> > >> >diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > >> >index 2e56af3281f8..579f58a42974 100644
> > >> >--- a/arch/riscv/kernel/setup.c
> > >> >+++ b/arch/riscv/kernel/setup.c
> > >> >@@ -82,41 +82,6 @@ EXPORT_SYMBOL(empty_zero_page);
> > >> > /* The lucky hart to first increment this variable will boot the other cores */
> > >> > atomic_t hart_lottery;
> > >> >
> > >> >-#ifdef CONFIG_BLK_DEV_INITRD
> > >> >-static void __init setup_initrd(void)
> > >> >-{
> > >> >-	extern char __initramfs_start[];
> > >> >-	extern unsigned long __initramfs_size;
> > >> >-	unsigned long size;
> > >> >-
> > >> >-	if (__initramfs_size > 0) {
> > >> >-		initrd_start = (unsigned long)(&__initramfs_start);
> > >> >-		initrd_end = initrd_start + __initramfs_size;
> > >> >-	}
> > >
> > > The underlying problem is probably that __initramfs_size == 512 even
> > > if there is no embedded initrd. Result is that initrd_start and initrd_end
> > > are always overwritten, even if provided and even if there is no embedded
> > > initrd. Result is that initrd_start and initrd_end are always overwritten,
> > > and -initrd from the qemu command line is always ignored.
> > >
> > > A less invasive fix than mine would be
> > >
> > > -	if (__initramfs_size > 0) {
> > > +	if (__initramfs_size > 0 && !initrd_start) {
> > >
> > > Any chance you can test that with your setup ?
> >
> > You should just delete the last four lines above.  They serve no purpose.
>
> You mean the entire if() statement plus the variable declarations ?
>
> That works for me, for both embedded initrd and initrd specified with
> -initrd option, but we still need someone to test if it works for
> Palmer's use case, ie with vmlinux (and possibly initrd) embedded in
> bbl.

This still boots my Fedora images

   diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
   index db20dc630e7e..aee603123030 100644
   --- a/arch/riscv/kernel/setup.c
   +++ b/arch/riscv/kernel/setup.c
   @@ -85,15 +85,8 @@ atomic_t hart_lottery;
    #ifdef CONFIG_BLK_DEV_INITRD
    static void __init setup_initrd(void)
    {
   -	extern char __initramfs_start[];
   -	extern unsigned long __initramfs_size;
    	unsigned long size;
    
   -	if (__initramfs_size > 0) {
   -		initrd_start = (unsigned long)(&__initramfs_start);
   -		initrd_end = initrd_start + __initramfs_size;
   -	}
   -
    	if (initrd_start >= initrd_end) {
    		printk(KERN_INFO "initrd not found or empty");
    		goto disable;

but I have not tried an integrated initramfs.