All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Palmer Dabbelt <palmer@sifive.com>
Cc: aou@eecs.berkeley.edu, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] riscv: Drop setup_initrd
Date: Tue, 28 Aug 2018 14:25:05 -0700	[thread overview]
Message-ID: <20180828212505.GA24238@roeck-us.net> (raw)
In-Reply-To: <mhng-7faee2bb-ee3e-48d1-a016-782ebbbb903d@palmer-si-x1c4>

On Tue, Aug 28, 2018 at 01:10:20PM -0700, Palmer Dabbelt wrote:
> On Thu, 09 Aug 2018 21:11:40 PDT (-0700), linux@roeck-us.net 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@roeck-us.net>
> >---
> > 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 ?

Thanks,
Guenter

> >-
> >-	if (initrd_start >= initrd_end) {
> >-		printk(KERN_INFO "initrd not found or empty");
> >-		goto disable;
> >-	}
> >-	if (__pa(initrd_end) > PFN_PHYS(max_low_pfn)) {
> >-		printk(KERN_ERR "initrd extends beyond end of memory");
> >-		goto disable;
> >-	}
> >-
> >-	size =  initrd_end - initrd_start;
> >-	memblock_reserve(__pa(initrd_start), size);
> >-	initrd_below_start_ok = 1;
> >-
> >-	printk(KERN_INFO "Initial ramdisk at: 0x%p (%lu bytes)\n",
> >-		(void *)(initrd_start), size);
> >-	return;
> >-disable:
> >-	pr_cont(" - disabling initrd\n");
> >-	initrd_start = 0;
> >-	initrd_end = 0;
> >-}
> >-#endif /* CONFIG_BLK_DEV_INITRD */
> >-
> > pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> > pgd_t trampoline_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
> >
> >@@ -195,10 +160,6 @@ static void __init setup_bootmem(void)
> > 	set_max_mapnr(PFN_DOWN(mem_size));
> > 	max_low_pfn = pfn_base + PFN_DOWN(mem_size);
> >
> >-#ifdef CONFIG_BLK_DEV_INITRD
> >-	setup_initrd();
> >-#endif /* CONFIG_BLK_DEV_INITRD */
> >-
> > 	early_init_fdt_reserve_self();
> > 	early_init_fdt_scan_reserved_mem();
> > 	memblock_allow_resize();
> 
> With this patch I can't boot in QEMU.  It might be something with my setup,
> though.  I'm testing with a unified BBL+vmlinux, and using QEMU 3.0.0-rc3 (I
> should probably update that one, but I don't remember anything going by).
> Do you have time to take a look?  Here's my QEMU commandline
> 
> ./riscv64-softmmu/qemu-system-riscv64    -nographic    -machine virt    -smp 4    -m 2G    -kernel /home/palmer/work/riscv/riscv-pk/build/bbl -object rng-random,filename=/dev/urandom,id=rng0    -device virtio-rng-device,rng=rng0    -append "console=ttyS0 ro root=/dev/vda"    -device virtio-blk-device,drive=hd0    -drive file=stage4-disk.img,format=raw,id=hd0    -device virtio-net-device,netdev=usernet    -netdev user,id=usernet,hostfwd=tcp::10000-:22
> 
> I just tagged the PR I plan to submit for RC2 tomorrow, so at least it
> should be easy for everyone to get on the same page.

WARNING: multiple messages have this Message-ID (diff)
From: linux@roeck-us.net (Guenter Roeck)
To: linux-riscv@lists.infradead.org
Subject: [PATCH] riscv: Drop setup_initrd
Date: Tue, 28 Aug 2018 14:25:05 -0700	[thread overview]
Message-ID: <20180828212505.GA24238@roeck-us.net> (raw)
In-Reply-To: <mhng-7faee2bb-ee3e-48d1-a016-782ebbbb903d@palmer-si-x1c4>

On Tue, Aug 28, 2018 at 01:10:20PM -0700, Palmer Dabbelt wrote:
> On Thu, 09 Aug 2018 21:11:40 PDT (-0700), linux at roeck-us.net 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@roeck-us.net>
> >---
> > 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 ?

Thanks,
Guenter

> >-
> >-	if (initrd_start >= initrd_end) {
> >-		printk(KERN_INFO "initrd not found or empty");
> >-		goto disable;
> >-	}
> >-	if (__pa(initrd_end) > PFN_PHYS(max_low_pfn)) {
> >-		printk(KERN_ERR "initrd extends beyond end of memory");
> >-		goto disable;
> >-	}
> >-
> >-	size =  initrd_end - initrd_start;
> >-	memblock_reserve(__pa(initrd_start), size);
> >-	initrd_below_start_ok = 1;
> >-
> >-	printk(KERN_INFO "Initial ramdisk at: 0x%p (%lu bytes)\n",
> >-		(void *)(initrd_start), size);
> >-	return;
> >-disable:
> >-	pr_cont(" - disabling initrd\n");
> >-	initrd_start = 0;
> >-	initrd_end = 0;
> >-}
> >-#endif /* CONFIG_BLK_DEV_INITRD */
> >-
> > pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> > pgd_t trampoline_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
> >
> >@@ -195,10 +160,6 @@ static void __init setup_bootmem(void)
> > 	set_max_mapnr(PFN_DOWN(mem_size));
> > 	max_low_pfn = pfn_base + PFN_DOWN(mem_size);
> >
> >-#ifdef CONFIG_BLK_DEV_INITRD
> >-	setup_initrd();
> >-#endif /* CONFIG_BLK_DEV_INITRD */
> >-
> > 	early_init_fdt_reserve_self();
> > 	early_init_fdt_scan_reserved_mem();
> > 	memblock_allow_resize();
> 
> With this patch I can't boot in QEMU.  It might be something with my setup,
> though.  I'm testing with a unified BBL+vmlinux, and using QEMU 3.0.0-rc3 (I
> should probably update that one, but I don't remember anything going by).
> Do you have time to take a look?  Here's my QEMU commandline
> 
> ./riscv64-softmmu/qemu-system-riscv64    -nographic    -machine virt    -smp 4    -m 2G    -kernel /home/palmer/work/riscv/riscv-pk/build/bbl -object rng-random,filename=/dev/urandom,id=rng0    -device virtio-rng-device,rng=rng0    -append "console=ttyS0 ro root=/dev/vda"    -device virtio-blk-device,drive=hd0    -drive file=stage4-disk.img,format=raw,id=hd0    -device virtio-net-device,netdev=usernet    -netdev user,id=usernet,hostfwd=tcp::10000-:22
> 
> I just tagged the PR I plan to submit for RC2 tomorrow, so at least it
> should be easy for everyone to get on the same page.

  parent reply	other threads:[~2018-08-28 21:25 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-10  4:11 [PATCH] riscv: Drop setup_initrd Guenter Roeck
2018-08-10  4:11 ` Guenter Roeck
2018-08-10  8:38 ` Christoph Hellwig
2018-08-10  8:38   ` Christoph Hellwig
2018-08-20 22:33 ` Palmer Dabbelt
2018-08-20 22:33   ` Palmer Dabbelt
2018-08-28 20:10 ` Palmer Dabbelt
2018-08-28 20:10   ` Palmer Dabbelt
2018-08-28 20:59   ` Guenter Roeck
2018-08-28 20:59     ` Guenter Roeck
2018-08-28 21:25   ` Guenter Roeck [this message]
2018-08-28 21:25     ` Guenter Roeck
2018-08-28 21:46     ` Andreas Schwab
2018-08-28 21:46       ` Andreas Schwab
2018-08-28 21:59       ` Guenter Roeck
2018-08-28 21:59         ` Guenter Roeck
2018-08-28 22:03         ` Palmer Dabbelt
2018-08-28 22:03           ` Palmer Dabbelt
2018-08-28 22:12           ` Guenter Roeck
2018-08-28 22:12             ` Guenter Roeck
2018-08-29  0:09             ` Palmer Dabbelt
2018-08-29  0:09               ` Palmer Dabbelt
2018-08-29  0:36               ` Guenter Roeck
2018-08-29  0:36                 ` Guenter Roeck
2018-08-29  0:49                 ` Palmer Dabbelt
2018-08-29  0:49                   ` Palmer Dabbelt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180828212505.GA24238@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=aou@eecs.berkeley.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@sifive.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.