Linux-RISC-V Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/4] Few mm & exception handling fixes
@ 2020-07-15 23:30 Atish Patra
  2020-07-15 23:30 ` [PATCH 1/4] RISC-V: Setup exception vector early Atish Patra
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Atish Patra @ 2020-07-15 23:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Vincent Chen, Anup Patel, Sudeep Holla, Atish Patra,
	Palmer Dabbelt, Zong Li, Paul Walmsley, Greentime Hu,
	linux-riscv

This series addresses some generic fixes that I found while working
on UEFI series. These patches are completely independent from UEFI
implementation and can be merged sooner that UEFI.

Some of the patches were already part of the UEFI series [1]. I will
remove them from next UEFI series (v3).

Patch1 moves set up the exception vector earlier while all other patches
are mm related fixes.

[1] http://lists.infradead.org/pipermail/linux-riscv/2020-July/000942.html

Atish Patra (4):
RISC-V: Setup exception vector early
RISC-V: Set maximum number of mapped pages correctly
RISC-V: Do not rely on initrd_start/end computed during early dt
parsing
riscv: Parse all memory blocks to remove unusable memory

arch/riscv/kernel/head.S    | 10 ++++--
arch/riscv/kernel/smpboot.c |  1 -
arch/riscv/kernel/traps.c   |  8 +----
arch/riscv/mm/init.c        | 66 +++++++++++++++++++++++++------------
4 files changed, 54 insertions(+), 31 deletions(-)

--
2.24.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH 1/4] RISC-V: Setup exception vector early
  2020-07-15 23:30 [PATCH 0/4] Few mm & exception handling fixes Atish Patra
@ 2020-07-15 23:30 ` Atish Patra
  2020-07-25  5:12   ` Palmer Dabbelt
  2020-07-15 23:30 ` [PATCH 2/4] RISC-V: Set maximum number of mapped pages correctly Atish Patra
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Atish Patra @ 2020-07-15 23:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Vincent Chen, Anup Patel, Sudeep Holla, Atish Patra,
	Palmer Dabbelt, Zong Li, Paul Walmsley, Greentime Hu,
	linux-riscv

The trap vector is set only in trap_init which may be too late in some
cases. Early ioremap/efi spits many warning messages which may be useful.

Setup the trap vector early so that any warning/bug can be handled before
generic code invokes trap_init.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/kernel/head.S    | 10 ++++++++--
 arch/riscv/kernel/smpboot.c |  1 -
 arch/riscv/kernel/traps.c   |  8 +-------
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 7ed1b22950fd..d0c5c316e9bb 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -77,10 +77,16 @@ relocate:
 	csrw CSR_SATP, a0
 .align 2
 1:
-	/* Set trap vector to spin forever to help debug */
-	la a0, .Lsecondary_park
+	/* Set trap vector to exception handler */
+	la a0, handle_exception
 	csrw CSR_TVEC, a0
 
+	/*
+	 * Set sup0 scratch register to 0, indicating to exception vector that
+	 * we are presently executing in kernel.
+	 */
+	csrw CSR_SCRATCH, zero
+
 	/* Reload the global pointer */
 .option push
 .option norelax
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 4e9922790f6e..5a9c127a380e 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -154,7 +154,6 @@ asmlinkage __visible void smp_callin(void)
 	mmgrab(mm);
 	current->active_mm = mm;
 
-	trap_init();
 	notify_cpu_starting(smp_processor_id());
 	update_siblings_masks(smp_processor_id());
 	set_cpu_online(smp_processor_id(), 1);
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 7d95cce5e47c..ad14f4466d92 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -174,13 +174,7 @@ int is_valid_bugaddr(unsigned long pc)
 }
 #endif /* CONFIG_GENERIC_BUG */
 
+/* stvec & scratch is already set from head.S */
 void trap_init(void)
 {
-	/*
-	 * Set sup0 scratch register to 0, indicating to exception vector
-	 * that we are presently executing in the kernel
-	 */
-	csr_write(CSR_SCRATCH, 0);
-	/* Set the exception vector address */
-	csr_write(CSR_TVEC, &handle_exception);
 }
-- 
2.24.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH 2/4] RISC-V: Set maximum number of mapped pages correctly
  2020-07-15 23:30 [PATCH 0/4] Few mm & exception handling fixes Atish Patra
  2020-07-15 23:30 ` [PATCH 1/4] RISC-V: Setup exception vector early Atish Patra
@ 2020-07-15 23:30 ` Atish Patra
  2020-07-25  5:12   ` Palmer Dabbelt
  2020-07-15 23:30 ` [PATCH 3/4] RISC-V: Do not rely on initrd_start/end computed during early dt parsing Atish Patra
  2020-07-15 23:30 ` [PATCH 4/4] riscv: Parse all memory blocks to remove unusable memory Atish Patra
  3 siblings, 1 reply; 11+ messages in thread
From: Atish Patra @ 2020-07-15 23:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Vincent Chen, Anup Patel, Sudeep Holla, Atish Patra,
	Palmer Dabbelt, Zong Li, Paul Walmsley, Greentime Hu,
	linux-riscv

Currently, maximum number of mapper pages are set to the pfn calculated
from the memblock size of the memblock containing kernel. This will work
until that memblock spans the entire memory. However, it will be set to
a wrong value if there are multiple memblocks defined in kernel
(e.g. with efi runtime services).

Set the the maximum value to the pfn calculated from dram size.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/mm/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index f4adb3684f3d..8d22973bde40 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -150,9 +150,9 @@ void __init setup_bootmem(void)
 	/* Reserve from the start of the kernel to the end of the kernel */
 	memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
 
-	set_max_mapnr(PFN_DOWN(mem_size));
 	max_pfn = PFN_DOWN(memblock_end_of_DRAM());
 	max_low_pfn = max_pfn;
+	set_max_mapnr(max_low_pfn);
 
 #ifdef CONFIG_BLK_DEV_INITRD
 	setup_initrd();
-- 
2.24.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH 3/4] RISC-V: Do not rely on initrd_start/end computed during early dt parsing
  2020-07-15 23:30 [PATCH 0/4] Few mm & exception handling fixes Atish Patra
  2020-07-15 23:30 ` [PATCH 1/4] RISC-V: Setup exception vector early Atish Patra
  2020-07-15 23:30 ` [PATCH 2/4] RISC-V: Set maximum number of mapped pages correctly Atish Patra
@ 2020-07-15 23:30 ` Atish Patra
  2020-07-25  5:12   ` Palmer Dabbelt
  2020-07-15 23:30 ` [PATCH 4/4] riscv: Parse all memory blocks to remove unusable memory Atish Patra
  3 siblings, 1 reply; 11+ messages in thread
From: Atish Patra @ 2020-07-15 23:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Vincent Chen, Anup Patel, Sudeep Holla, Atish Patra,
	Palmer Dabbelt, Zong Li, Paul Walmsley, Greentime Hu,
	linux-riscv

Currently, initrd_start/end are computed during early_init_dt_scan
but used during arch_setup. We will get the following panic if initrd is used
and CONFIG_DEBUG_VIRTUAL is turned on.

[    0.000000] ------------[ cut here ]------------
[    0.000000] kernel BUG at arch/riscv/mm/physaddr.c:33!
[    0.000000] Kernel BUG [#1]
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.8.0-rc4-00015-ged0b226fed02 #886
[    0.000000] epc: ffffffe0002058d2 ra : ffffffe0000053f0 sp : ffffffe001001f40
[    0.000000]  gp : ffffffe00106e250 tp : ffffffe001009d40 t0 : ffffffe00107ee28
[    0.000000]  t1 : 0000000000000000 t2 : ffffffe000a2e880 s0 : ffffffe001001f50
[    0.000000]  s1 : ffffffe0001383e8 a0 : ffffffe00c087e00 a1 : 0000000080200000
[    0.000000]  a2 : 00000000010bf000 a3 : ffffffe00106f3c8 a4 : ffffffe0010bf000
[    0.000000]  a5 : ffffffe000000000 a6 : 0000000000000006 a7 : 0000000000000001
[    0.000000]  s2 : ffffffe00106f068 s3 : ffffffe00106f070 s4 : 0000000080200000
[    0.000000]  s5 : 0000000082200000 s6 : 0000000000000000 s7 : 0000000000000000
[    0.000000]  s8 : 0000000080011010 s9 : 0000000080012700 s10: 0000000000000000
[    0.000000]  s11: 0000000000000000 t3 : 000000000001fe30 t4 : 000000000001fe30
[    0.000000]  t5 : 0000000000000000 t6 : ffffffe00107c471
[    0.000000] status: 0000000000000100 badaddr: 0000000000000000 cause: 0000000000000003
[    0.000000] random: get_random_bytes called from print_oops_end_marker+0x22/0x46 with crng_init=0

To avoid the error, initrd_start/end can be computed from phys_initrd_start/size
in setup itself. It also improves the initrd placement by aligning the start
and size with the page size.

Fixes: 6435f773d81f (riscv: mm: add support for CONFIG_DEBUG_VIRTUAL)
Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/mm/init.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 8d22973bde40..f818a47a72d1 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -95,19 +95,40 @@ void __init mem_init(void)
 #ifdef CONFIG_BLK_DEV_INITRD
 static void __init setup_initrd(void)
 {
+	phys_addr_t start;
 	unsigned long size;
 
-	if (initrd_start >= initrd_end) {
-		pr_info("initrd not found or empty");
+	/* Ignore the virtul address computed during device tree parsing */
+	initrd_start = initrd_end = 0;
+
+	if (!phys_initrd_size)
+		return;
+	/*
+	 * Round the memory region to page boundaries as per free_initrd_mem()
+	 * This allows us to detect whether the pages overlapping the initrd
+	 * are in use, but more importantly, reserves the entire set of pages
+	 * as we don't want these pages allocated for other purposes.
+	 */
+	start = round_down(phys_initrd_start, PAGE_SIZE);
+	size = phys_initrd_size + (phys_initrd_start - start);
+	size = round_up(size, PAGE_SIZE);
+
+	if (!memblock_is_region_memory(start, size)) {
+		pr_err("INITRD: 0x%08llx+0x%08lx is not a memory region",
+		       (u64)start, size);
 		goto disable;
 	}
-	if (__pa_symbol(initrd_end) > PFN_PHYS(max_low_pfn)) {
-		pr_err("initrd extends beyond end of memory");
+
+	if (memblock_is_region_reserved(start, size)) {
+		pr_err("INITRD: 0x%08llx+0x%08lx overlaps in-use memory region\n",
+		       (u64)start, size);
 		goto disable;
 	}
 
-	size = initrd_end - initrd_start;
-	memblock_reserve(__pa_symbol(initrd_start), size);
+	memblock_reserve(start, size);
+	/* Now convert initrd to virtual addresses */
+	initrd_start = (unsigned long)__va(phys_initrd_start);
+	initrd_end = initrd_start + phys_initrd_size;
 	initrd_below_start_ok = 1;
 
 	pr_info("Initial ramdisk at: 0x%p (%lu bytes)\n",
-- 
2.24.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH 4/4] riscv: Parse all memory blocks to remove unusable memory
  2020-07-15 23:30 [PATCH 0/4] Few mm & exception handling fixes Atish Patra
                   ` (2 preceding siblings ...)
  2020-07-15 23:30 ` [PATCH 3/4] RISC-V: Do not rely on initrd_start/end computed during early dt parsing Atish Patra
@ 2020-07-15 23:30 ` Atish Patra
  2020-07-25  5:12   ` Palmer Dabbelt
  3 siblings, 1 reply; 11+ messages in thread
From: Atish Patra @ 2020-07-15 23:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Vincent Chen, Anup Patel, Sudeep Holla, Atish Patra,
	Palmer Dabbelt, Zong Li, Paul Walmsley, Greentime Hu,
	linux-riscv

Currently, maximum physical memory allowed is equal to -PAGE_OFFSET.
That's why we remove any memory blocks spanning beyond that size. However,
it is done only for memblock containing linux kernel which will not work
if there are multiple memblocks.

Process all memory blocks to figure out how much memory needs to be removed
and remove at the end instead of updating the memblock list in place.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/mm/init.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index f818a47a72d1..79e9d55bdf1a 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -147,26 +147,29 @@ void __init setup_bootmem(void)
 {
 	struct memblock_region *reg;
 	phys_addr_t mem_size = 0;
+	phys_addr_t total_mem = 0;
+	phys_addr_t mem_start, end = 0;
 	phys_addr_t vmlinux_end = __pa_symbol(&_end);
 	phys_addr_t vmlinux_start = __pa_symbol(&_start);
 
 	/* Find the memory region containing the kernel */
 	for_each_memblock(memory, reg) {
-		phys_addr_t end = reg->base + reg->size;
-
-		if (reg->base <= vmlinux_start && vmlinux_end <= end) {
-			mem_size = min(reg->size, (phys_addr_t)-PAGE_OFFSET);
-
-			/*
-			 * Remove memblock from the end of usable area to the
-			 * end of region
-			 */
-			if (reg->base + mem_size < end)
-				memblock_remove(reg->base + mem_size,
-						end - reg->base - mem_size);
-		}
+		end = reg->base + reg->size;
+		if (!total_mem)
+			mem_start = reg->base;
+		if (reg->base <= vmlinux_start && vmlinux_end <= end)
+			BUG_ON(reg->size == 0);
+		total_mem = total_mem + reg->size;
 	}
-	BUG_ON(mem_size == 0);
+
+	/*
+	 * Remove memblock from the end of usable area to the
+	 * end of region
+	 */
+	mem_size = min(total_mem, (phys_addr_t)-PAGE_OFFSET);
+	if (mem_start + mem_size < end)
+		memblock_remove(mem_start + mem_size,
+				end - mem_start - mem_size);
 
 	/* Reserve from the start of the kernel to the end of the kernel */
 	memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
-- 
2.24.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH 1/4] RISC-V: Setup exception vector early
  2020-07-15 23:30 ` [PATCH 1/4] RISC-V: Setup exception vector early Atish Patra
@ 2020-07-25  5:12   ` Palmer Dabbelt
  2020-07-25  5:26     ` Atish Patra
  0 siblings, 1 reply; 11+ messages in thread
From: Palmer Dabbelt @ 2020-07-25  5:12 UTC (permalink / raw)
  To: Atish Patra
  Cc: aou, vincent.chen, Anup Patel, linux-kernel, sudeep.holla,
	Atish Patra, zong.li, Paul Walmsley, greentime.hu, linux-riscv

On Wed, 15 Jul 2020 16:30:06 PDT (-0700), Atish Patra wrote:
> The trap vector is set only in trap_init which may be too late in some
> cases. Early ioremap/efi spits many warning messages which may be useful.
>
> Setup the trap vector early so that any warning/bug can be handled before
> generic code invokes trap_init.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/kernel/head.S    | 10 ++++++++--
>  arch/riscv/kernel/smpboot.c |  1 -
>  arch/riscv/kernel/traps.c   |  8 +-------
>  3 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index 7ed1b22950fd..d0c5c316e9bb 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -77,10 +77,16 @@ relocate:
>  	csrw CSR_SATP, a0
>  .align 2
>  1:
> -	/* Set trap vector to spin forever to help debug */
> -	la a0, .Lsecondary_park
> +	/* Set trap vector to exception handler */
> +	la a0, handle_exception
>  	csrw CSR_TVEC, a0
>
> +	/*
> +	 * Set sup0 scratch register to 0, indicating to exception vector that
> +	 * we are presently executing in kernel.
> +	 */
> +	csrw CSR_SCRATCH, zero
> +
>  	/* Reload the global pointer */
>  .option push
>  .option norelax
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 4e9922790f6e..5a9c127a380e 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -154,7 +154,6 @@ asmlinkage __visible void smp_callin(void)
>  	mmgrab(mm);
>  	current->active_mm = mm;
>
> -	trap_init();
>  	notify_cpu_starting(smp_processor_id());
>  	update_siblings_masks(smp_processor_id());
>  	set_cpu_online(smp_processor_id(), 1);
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index 7d95cce5e47c..ad14f4466d92 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -174,13 +174,7 @@ int is_valid_bugaddr(unsigned long pc)
>  }
>  #endif /* CONFIG_GENERIC_BUG */
>
> +/* stvec & scratch is already set from head.S */
>  void trap_init(void)
>  {
> -	/*
> -	 * Set sup0 scratch register to 0, indicating to exception vector
> -	 * that we are presently executing in the kernel
> -	 */
> -	csr_write(CSR_SCRATCH, 0);
> -	/* Set the exception vector address */
> -	csr_write(CSR_TVEC, &handle_exception);
>  }

While I think these are all actual fixes, it's pretty late in the cycle here so
I'm going to a bit on the careful side and only take the patches that actually
manifest as bugs in the current port.  Assuming this doesn't manifest until
early_ioremap is enabled (and we don't do that yet), I've put it on for-next.

LMK if I'm wrong about this, or the following ones.

Thanks!

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH 2/4] RISC-V: Set maximum number of mapped pages correctly
  2020-07-15 23:30 ` [PATCH 2/4] RISC-V: Set maximum number of mapped pages correctly Atish Patra
@ 2020-07-25  5:12   ` Palmer Dabbelt
  0 siblings, 0 replies; 11+ messages in thread
From: Palmer Dabbelt @ 2020-07-25  5:12 UTC (permalink / raw)
  To: Atish Patra
  Cc: aou, vincent.chen, Anup Patel, linux-kernel, sudeep.holla,
	Atish Patra, zong.li, Paul Walmsley, greentime.hu, linux-riscv

On Wed, 15 Jul 2020 16:30:07 PDT (-0700), Atish Patra wrote:
> Currently, maximum number of mapper pages are set to the pfn calculated
> from the memblock size of the memblock containing kernel. This will work
> until that memblock spans the entire memory. However, it will be set to
> a wrong value if there are multiple memblocks defined in kernel
> (e.g. with efi runtime services).
>
> Set the the maximum value to the pfn calculated from dram size.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/mm/init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index f4adb3684f3d..8d22973bde40 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -150,9 +150,9 @@ void __init setup_bootmem(void)
>  	/* Reserve from the start of the kernel to the end of the kernel */
>  	memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
>
> -	set_max_mapnr(PFN_DOWN(mem_size));
>  	max_pfn = PFN_DOWN(memblock_end_of_DRAM());
>  	max_low_pfn = max_pfn;
> +	set_max_mapnr(max_low_pfn);
>
>  #ifdef CONFIG_BLK_DEV_INITRD
>  	setup_initrd();

This one I'm putting on fixes, as there's nothing preventing us from having
multiple memory regions in a current boot and this seems very safe.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH 3/4] RISC-V: Do not rely on initrd_start/end computed during early dt parsing
  2020-07-15 23:30 ` [PATCH 3/4] RISC-V: Do not rely on initrd_start/end computed during early dt parsing Atish Patra
@ 2020-07-25  5:12   ` Palmer Dabbelt
  2020-07-25  5:44     ` Atish Patra
  0 siblings, 1 reply; 11+ messages in thread
From: Palmer Dabbelt @ 2020-07-25  5:12 UTC (permalink / raw)
  To: Atish Patra
  Cc: aou, vincent.chen, Anup Patel, linux-kernel, sudeep.holla,
	Atish Patra, zong.li, Paul Walmsley, greentime.hu, linux-riscv

On Wed, 15 Jul 2020 16:30:08 PDT (-0700), Atish Patra wrote:
> Currently, initrd_start/end are computed during early_init_dt_scan
> but used during arch_setup. We will get the following panic if initrd is used
> and CONFIG_DEBUG_VIRTUAL is turned on.
>
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] kernel BUG at arch/riscv/mm/physaddr.c:33!
> [    0.000000] Kernel BUG [#1]
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.8.0-rc4-00015-ged0b226fed02 #886
> [    0.000000] epc: ffffffe0002058d2 ra : ffffffe0000053f0 sp : ffffffe001001f40
> [    0.000000]  gp : ffffffe00106e250 tp : ffffffe001009d40 t0 : ffffffe00107ee28
> [    0.000000]  t1 : 0000000000000000 t2 : ffffffe000a2e880 s0 : ffffffe001001f50
> [    0.000000]  s1 : ffffffe0001383e8 a0 : ffffffe00c087e00 a1 : 0000000080200000
> [    0.000000]  a2 : 00000000010bf000 a3 : ffffffe00106f3c8 a4 : ffffffe0010bf000
> [    0.000000]  a5 : ffffffe000000000 a6 : 0000000000000006 a7 : 0000000000000001
> [    0.000000]  s2 : ffffffe00106f068 s3 : ffffffe00106f070 s4 : 0000000080200000
> [    0.000000]  s5 : 0000000082200000 s6 : 0000000000000000 s7 : 0000000000000000
> [    0.000000]  s8 : 0000000080011010 s9 : 0000000080012700 s10: 0000000000000000
> [    0.000000]  s11: 0000000000000000 t3 : 000000000001fe30 t4 : 000000000001fe30
> [    0.000000]  t5 : 0000000000000000 t6 : ffffffe00107c471
> [    0.000000] status: 0000000000000100 badaddr: 0000000000000000 cause: 0000000000000003
> [    0.000000] random: get_random_bytes called from print_oops_end_marker+0x22/0x46 with crng_init=0
>
> To avoid the error, initrd_start/end can be computed from phys_initrd_start/size
> in setup itself. It also improves the initrd placement by aligning the start
> and size with the page size.
>
> Fixes: 6435f773d81f (riscv: mm: add support for CONFIG_DEBUG_VIRTUAL)
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/mm/init.c | 33 +++++++++++++++++++++++++++------
>  1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 8d22973bde40..f818a47a72d1 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -95,19 +95,40 @@ void __init mem_init(void)
>  #ifdef CONFIG_BLK_DEV_INITRD
>  static void __init setup_initrd(void)
>  {
> +	phys_addr_t start;
>  	unsigned long size;
>
> -	if (initrd_start >= initrd_end) {
> -		pr_info("initrd not found or empty");
> +	/* Ignore the virtul address computed during device tree parsing */
> +	initrd_start = initrd_end = 0;
> +
> +	if (!phys_initrd_size)
> +		return;
> +	/*
> +	 * Round the memory region to page boundaries as per free_initrd_mem()
> +	 * This allows us to detect whether the pages overlapping the initrd
> +	 * are in use, but more importantly, reserves the entire set of pages
> +	 * as we don't want these pages allocated for other purposes.
> +	 */
> +	start = round_down(phys_initrd_start, PAGE_SIZE);
> +	size = phys_initrd_size + (phys_initrd_start - start);
> +	size = round_up(size, PAGE_SIZE);
> +
> +	if (!memblock_is_region_memory(start, size)) {
> +		pr_err("INITRD: 0x%08llx+0x%08lx is not a memory region",
> +		       (u64)start, size);
>  		goto disable;
>  	}
> -	if (__pa_symbol(initrd_end) > PFN_PHYS(max_low_pfn)) {
> -		pr_err("initrd extends beyond end of memory");
> +
> +	if (memblock_is_region_reserved(start, size)) {
> +		pr_err("INITRD: 0x%08llx+0x%08lx overlaps in-use memory region\n",
> +		       (u64)start, size);
>  		goto disable;
>  	}
>
> -	size = initrd_end - initrd_start;
> -	memblock_reserve(__pa_symbol(initrd_start), size);
> +	memblock_reserve(start, size);
> +	/* Now convert initrd to virtual addresses */
> +	initrd_start = (unsigned long)__va(phys_initrd_start);
> +	initrd_end = initrd_start + phys_initrd_size;
>  	initrd_below_start_ok = 1;
>
>  	pr_info("Initial ramdisk at: 0x%p (%lu bytes)\n",

I'm going to put this one on fixes, but I don't think that's the right:
DEBUG_VIRTUAL just catches the bug, but as far as I can tell it's been there
since the beginning.  I'm going to replace this with

Fixes: 76d2a0493a17 ("RISC-V: Init and Halt Code")

It's not going to apply back that far, but we can always backport it where it
fails.

Thanks!

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH 4/4] riscv: Parse all memory blocks to remove unusable memory
  2020-07-15 23:30 ` [PATCH 4/4] riscv: Parse all memory blocks to remove unusable memory Atish Patra
@ 2020-07-25  5:12   ` Palmer Dabbelt
  0 siblings, 0 replies; 11+ messages in thread
From: Palmer Dabbelt @ 2020-07-25  5:12 UTC (permalink / raw)
  To: Atish Patra
  Cc: aou, vincent.chen, Anup Patel, linux-kernel, sudeep.holla,
	Atish Patra, zong.li, Paul Walmsley, greentime.hu, linux-riscv

On Wed, 15 Jul 2020 16:30:09 PDT (-0700), Atish Patra wrote:
> Currently, maximum physical memory allowed is equal to -PAGE_OFFSET.
> That's why we remove any memory blocks spanning beyond that size. However,
> it is done only for memblock containing linux kernel which will not work
> if there are multiple memblocks.
>
> Process all memory blocks to figure out how much memory needs to be removed
> and remove at the end instead of updating the memblock list in place.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/mm/init.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index f818a47a72d1..79e9d55bdf1a 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -147,26 +147,29 @@ void __init setup_bootmem(void)
>  {
>  	struct memblock_region *reg;
>  	phys_addr_t mem_size = 0;
> +	phys_addr_t total_mem = 0;
> +	phys_addr_t mem_start, end = 0;
>  	phys_addr_t vmlinux_end = __pa_symbol(&_end);
>  	phys_addr_t vmlinux_start = __pa_symbol(&_start);
>
>  	/* Find the memory region containing the kernel */
>  	for_each_memblock(memory, reg) {
> -		phys_addr_t end = reg->base + reg->size;
> -
> -		if (reg->base <= vmlinux_start && vmlinux_end <= end) {
> -			mem_size = min(reg->size, (phys_addr_t)-PAGE_OFFSET);
> -
> -			/*
> -			 * Remove memblock from the end of usable area to the
> -			 * end of region
> -			 */
> -			if (reg->base + mem_size < end)
> -				memblock_remove(reg->base + mem_size,
> -						end - reg->base - mem_size);
> -		}
> +		end = reg->base + reg->size;
> +		if (!total_mem)
> +			mem_start = reg->base;
> +		if (reg->base <= vmlinux_start && vmlinux_end <= end)
> +			BUG_ON(reg->size == 0);
> +		total_mem = total_mem + reg->size;
>  	}
> -	BUG_ON(mem_size == 0);
> +
> +	/*
> +	 * Remove memblock from the end of usable area to the
> +	 * end of region
> +	 */
> +	mem_size = min(total_mem, (phys_addr_t)-PAGE_OFFSET);
> +	if (mem_start + mem_size < end)
> +		memblock_remove(mem_start + mem_size,
> +				end - mem_start - mem_size);
>
>  	/* Reserve from the start of the kernel to the end of the kernel */
>  	memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);

Thanks, this one is also on fixes.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH 1/4] RISC-V: Setup exception vector early
  2020-07-25  5:12   ` Palmer Dabbelt
@ 2020-07-25  5:26     ` Atish Patra
  0 siblings, 0 replies; 11+ messages in thread
From: Atish Patra @ 2020-07-25  5:26 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Albert Ou, Vincent Chen, Anup Patel,
	linux-kernel@vger.kernel.org List, Paul Walmsley, Atish Patra,
	Zong Li, sudeep.holla, Greentime Hu, linux-riscv

On Fri, Jul 24, 2020 at 10:12 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 15 Jul 2020 16:30:06 PDT (-0700), Atish Patra wrote:
> > The trap vector is set only in trap_init which may be too late in some
> > cases. Early ioremap/efi spits many warning messages which may be useful.
> >
> > Setup the trap vector early so that any warning/bug can be handled before
> > generic code invokes trap_init.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> >  arch/riscv/kernel/head.S    | 10 ++++++++--
> >  arch/riscv/kernel/smpboot.c |  1 -
> >  arch/riscv/kernel/traps.c   |  8 +-------
> >  3 files changed, 9 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index 7ed1b22950fd..d0c5c316e9bb 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -77,10 +77,16 @@ relocate:
> >       csrw CSR_SATP, a0
> >  .align 2
> >  1:
> > -     /* Set trap vector to spin forever to help debug */
> > -     la a0, .Lsecondary_park
> > +     /* Set trap vector to exception handler */
> > +     la a0, handle_exception
> >       csrw CSR_TVEC, a0
> >
> > +     /*
> > +      * Set sup0 scratch register to 0, indicating to exception vector that
> > +      * we are presently executing in kernel.
> > +      */
> > +     csrw CSR_SCRATCH, zero
> > +
> >       /* Reload the global pointer */
> >  .option push
> >  .option norelax
> > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> > index 4e9922790f6e..5a9c127a380e 100644
> > --- a/arch/riscv/kernel/smpboot.c
> > +++ b/arch/riscv/kernel/smpboot.c
> > @@ -154,7 +154,6 @@ asmlinkage __visible void smp_callin(void)
> >       mmgrab(mm);
> >       current->active_mm = mm;
> >
> > -     trap_init();
> >       notify_cpu_starting(smp_processor_id());
> >       update_siblings_masks(smp_processor_id());
> >       set_cpu_online(smp_processor_id(), 1);
> > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> > index 7d95cce5e47c..ad14f4466d92 100644
> > --- a/arch/riscv/kernel/traps.c
> > +++ b/arch/riscv/kernel/traps.c
> > @@ -174,13 +174,7 @@ int is_valid_bugaddr(unsigned long pc)
> >  }
> >  #endif /* CONFIG_GENERIC_BUG */
> >
> > +/* stvec & scratch is already set from head.S */
> >  void trap_init(void)
> >  {
> > -     /*
> > -      * Set sup0 scratch register to 0, indicating to exception vector
> > -      * that we are presently executing in the kernel
> > -      */
> > -     csr_write(CSR_SCRATCH, 0);
> > -     /* Set the exception vector address */
> > -     csr_write(CSR_TVEC, &handle_exception);
> >  }
>
> While I think these are all actual fixes, it's pretty late in the cycle here so
> I'm going to a bit on the careful side and only take the patches that actually
> manifest as bugs in the current port.  Assuming this doesn't manifest until
> early_ioremap is enabled (and we don't do that yet), I've put it on for-next.
>

Yeah. early_ioremap is part of the UEFI series. So this can go into for-next.
Thanks.

> LMK if I'm wrong about this, or the following ones.
>
> Thanks!
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH 3/4] RISC-V: Do not rely on initrd_start/end computed during early dt parsing
  2020-07-25  5:12   ` Palmer Dabbelt
@ 2020-07-25  5:44     ` Atish Patra
  0 siblings, 0 replies; 11+ messages in thread
From: Atish Patra @ 2020-07-25  5:44 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Albert Ou, Vincent Chen, Anup Patel,
	linux-kernel@vger.kernel.org List, Paul Walmsley, Atish Patra,
	Zong Li, sudeep.holla, Greentime Hu, linux-riscv

On Fri, Jul 24, 2020 at 10:12 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 15 Jul 2020 16:30:08 PDT (-0700), Atish Patra wrote:
> > Currently, initrd_start/end are computed during early_init_dt_scan
> > but used during arch_setup. We will get the following panic if initrd is used
> > and CONFIG_DEBUG_VIRTUAL is turned on.
> >
> > [    0.000000] ------------[ cut here ]------------
> > [    0.000000] kernel BUG at arch/riscv/mm/physaddr.c:33!
> > [    0.000000] Kernel BUG [#1]
> > [    0.000000] Modules linked in:
> > [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.8.0-rc4-00015-ged0b226fed02 #886
> > [    0.000000] epc: ffffffe0002058d2 ra : ffffffe0000053f0 sp : ffffffe001001f40
> > [    0.000000]  gp : ffffffe00106e250 tp : ffffffe001009d40 t0 : ffffffe00107ee28
> > [    0.000000]  t1 : 0000000000000000 t2 : ffffffe000a2e880 s0 : ffffffe001001f50
> > [    0.000000]  s1 : ffffffe0001383e8 a0 : ffffffe00c087e00 a1 : 0000000080200000
> > [    0.000000]  a2 : 00000000010bf000 a3 : ffffffe00106f3c8 a4 : ffffffe0010bf000
> > [    0.000000]  a5 : ffffffe000000000 a6 : 0000000000000006 a7 : 0000000000000001
> > [    0.000000]  s2 : ffffffe00106f068 s3 : ffffffe00106f070 s4 : 0000000080200000
> > [    0.000000]  s5 : 0000000082200000 s6 : 0000000000000000 s7 : 0000000000000000
> > [    0.000000]  s8 : 0000000080011010 s9 : 0000000080012700 s10: 0000000000000000
> > [    0.000000]  s11: 0000000000000000 t3 : 000000000001fe30 t4 : 000000000001fe30
> > [    0.000000]  t5 : 0000000000000000 t6 : ffffffe00107c471
> > [    0.000000] status: 0000000000000100 badaddr: 0000000000000000 cause: 0000000000000003
> > [    0.000000] random: get_random_bytes called from print_oops_end_marker+0x22/0x46 with crng_init=0
> >
> > To avoid the error, initrd_start/end can be computed from phys_initrd_start/size
> > in setup itself. It also improves the initrd placement by aligning the start
> > and size with the page size.
> >
> > Fixes: 6435f773d81f (riscv: mm: add support for CONFIG_DEBUG_VIRTUAL)
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> >  arch/riscv/mm/init.c | 33 +++++++++++++++++++++++++++------
> >  1 file changed, 27 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 8d22973bde40..f818a47a72d1 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -95,19 +95,40 @@ void __init mem_init(void)
> >  #ifdef CONFIG_BLK_DEV_INITRD
> >  static void __init setup_initrd(void)
> >  {
> > +     phys_addr_t start;
> >       unsigned long size;
> >
> > -     if (initrd_start >= initrd_end) {
> > -             pr_info("initrd not found or empty");
> > +     /* Ignore the virtul address computed during device tree parsing */
> > +     initrd_start = initrd_end = 0;
> > +
> > +     if (!phys_initrd_size)
> > +             return;
> > +     /*
> > +      * Round the memory region to page boundaries as per free_initrd_mem()
> > +      * This allows us to detect whether the pages overlapping the initrd
> > +      * are in use, but more importantly, reserves the entire set of pages
> > +      * as we don't want these pages allocated for other purposes.
> > +      */
> > +     start = round_down(phys_initrd_start, PAGE_SIZE);
> > +     size = phys_initrd_size + (phys_initrd_start - start);
> > +     size = round_up(size, PAGE_SIZE);
> > +
> > +     if (!memblock_is_region_memory(start, size)) {
> > +             pr_err("INITRD: 0x%08llx+0x%08lx is not a memory region",
> > +                    (u64)start, size);
> >               goto disable;
> >       }
> > -     if (__pa_symbol(initrd_end) > PFN_PHYS(max_low_pfn)) {
> > -             pr_err("initrd extends beyond end of memory");
> > +
> > +     if (memblock_is_region_reserved(start, size)) {
> > +             pr_err("INITRD: 0x%08llx+0x%08lx overlaps in-use memory region\n",
> > +                    (u64)start, size);
> >               goto disable;
> >       }
> >
> > -     size = initrd_end - initrd_start;
> > -     memblock_reserve(__pa_symbol(initrd_start), size);
> > +     memblock_reserve(start, size);
> > +     /* Now convert initrd to virtual addresses */
> > +     initrd_start = (unsigned long)__va(phys_initrd_start);
> > +     initrd_end = initrd_start + phys_initrd_size;
> >       initrd_below_start_ok = 1;
> >
> >       pr_info("Initial ramdisk at: 0x%p (%lu bytes)\n",
>
> I'm going to put this one on fixes, but I don't think that's the right:
> DEBUG_VIRTUAL just catches the bug, but as far as I can tell it's been there
> since the beginning.  I'm going to replace this with
>
> Fixes: 76d2a0493a17 ("RISC-V: Init and Halt Code")
>
> It's not going to apply back that far, but we can always backport it where it
> fails.
>

Yeah. That's that reason I didn't want to go that far.
I am afraid Greg may be not too happy with the stable tree compilation
failures (if there are any) :)

> Thanks!
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-15 23:30 [PATCH 0/4] Few mm & exception handling fixes Atish Patra
2020-07-15 23:30 ` [PATCH 1/4] RISC-V: Setup exception vector early Atish Patra
2020-07-25  5:12   ` Palmer Dabbelt
2020-07-25  5:26     ` Atish Patra
2020-07-15 23:30 ` [PATCH 2/4] RISC-V: Set maximum number of mapped pages correctly Atish Patra
2020-07-25  5:12   ` Palmer Dabbelt
2020-07-15 23:30 ` [PATCH 3/4] RISC-V: Do not rely on initrd_start/end computed during early dt parsing Atish Patra
2020-07-25  5:12   ` Palmer Dabbelt
2020-07-25  5:44     ` Atish Patra
2020-07-15 23:30 ` [PATCH 4/4] riscv: Parse all memory blocks to remove unusable memory Atish Patra
2020-07-25  5:12   ` Palmer Dabbelt

Linux-RISC-V Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-riscv/0 linux-riscv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-riscv linux-riscv/ https://lore.kernel.org/linux-riscv \
		linux-riscv@lists.infradead.org
	public-inbox-index linux-riscv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-riscv


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git