[RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
4MB(32bit) in Linux. It's very wasteful to small memory footprint
soc chip such as Allwinner D1s/F133. The kernel parameter gives a
chance to users to set the proper size of the firmware and get
more than 1.5MB of memory.

Guo Ren (3):
  riscv: Remove 2MB offset in the mm layout
  riscv: Add early_param to decrease firmware region
  riscv: Add riscv.fwsz kernel parameter

 .../admin-guide/kernel-parameters.txt         |  3 +++
 arch/riscv/include/asm/page.h                 |  8 +++++++
 arch/riscv/kernel/head.S                      | 10 +++-----
 arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
 arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
 5 files changed, 36 insertions(+), 13 deletions(-)

-- 
2.25.1

[RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
4MB(32bit) in Linux. It's very wasteful to small memory footprint
soc chip such as Allwinner D1s/F133. The kernel parameter gives a
chance to users to set the proper size of the firmware and get
more than 1.5MB of memory.

Guo Ren (3):
  riscv: Remove 2MB offset in the mm layout
  riscv: Add early_param to decrease firmware region
  riscv: Add riscv.fwsz kernel parameter

 .../admin-guide/kernel-parameters.txt         |  3 +++
 arch/riscv/include/asm/page.h                 |  8 +++++++
 arch/riscv/kernel/head.S                      | 10 +++-----
 arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
 arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
 5 files changed, 36 insertions(+), 13 deletions(-)

-- 
2.25.1


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

[RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

The current RISC-V's mm layout is based on a 2MB offset and wasting
memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
Then we could reduce the memory reserved for opensbi in the next
patch.

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Anup Patel <anup.patel@wdc.com>
Cc: Atish Patra <atishp@rivosinc.com>
---
 arch/riscv/include/asm/page.h   |  8 ++++++++
 arch/riscv/kernel/head.S        | 10 +++-------
 arch/riscv/kernel/vmlinux.lds.S |  5 ++---
 arch/riscv/mm/init.c            | 11 ++++++++---
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index b3e5ff0125fe..299147c78b4a 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -16,6 +16,14 @@
 #define PAGE_SIZE	(_AC(1, UL) << PAGE_SHIFT)
 #define PAGE_MASK	(~(PAGE_SIZE - 1))
 
+#if __riscv_xlen == 64
+/* Image load offset(2MB) from start of RAM */
+#define LOAD_OFFSET	0x200000
+#else
+/* Image load offset(4MB) from start of RAM */
+#define LOAD_OFFSET	0x400000
+#endif
+
 #ifdef CONFIG_64BIT
 #define HUGE_MAX_HSTATE		2
 #else
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index f52f01ecbeea..a6ac892d2ccf 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -61,13 +61,7 @@ ENTRY(_start)
 	/* Image load offset (0MB) from start of RAM for M-mode */
 	.dword 0
 #else
-#if __riscv_xlen == 64
-	/* Image load offset(2MB) from start of RAM */
-	.dword 0x200000
-#else
-	/* Image load offset(4MB) from start of RAM */
-	.dword 0x400000
-#endif
+	.dword LOAD_OFFSET
 #endif
 	/* Effective size of kernel image */
 	.dword _end - _start
@@ -94,6 +88,8 @@ relocate:
 	la a1, kernel_map
 	XIP_FIXUP_OFFSET a1
 	REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
+	li a2, LOAD_OFFSET
+	add a1, a1, a2
 	la a2, _start
 	sub a1, a1, a2
 	add ra, ra, a1
diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
index 5104f3a871e3..75b7c72cd4bd 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -11,10 +11,9 @@
 #else
 
 #include <asm/pgtable.h>
-#define LOAD_OFFSET KERNEL_LINK_ADDR
 
-#include <asm/vmlinux.lds.h>
 #include <asm/page.h>
+#include <asm/vmlinux.lds.h>
 #include <asm/cache.h>
 #include <asm/thread_info.h>
 #include <asm/set_memory.h>
@@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
 SECTIONS
 {
 	/* Beginning of code and text segment */
-	. = LOAD_OFFSET;
+	. = LOAD_OFFSET + KERNEL_LINK_ADDR;
 	_start = .;
 	HEAD_TEXT_SECTION
 	. = ALIGN(PAGE_SIZE);
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 24b2b8044602..920e78f8c3e4 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
 	if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
 		memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
 
+	/*
+	 * Reserve OpenSBI region and depends on PMP to deny accesses.
+	 */
+	memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
+
 	early_init_fdt_scan_reserved_mem();
 	dma_contiguous_reserve(dma32_phys_limit);
 	if (IS_ENABLED(CONFIG_64BIT))
@@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 
 	kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
 #else
-	kernel_map.phys_addr = (uintptr_t)(&_start);
+	kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
 	kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
 #endif
 	kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
@@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 	create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
 			   kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
 #else
-	create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
-			   kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
+	create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
+			   kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
 #endif
 #else
 	/* Setup trampoline PGD */
-- 
2.25.1

[RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

The current RISC-V's mm layout is based on a 2MB offset and wasting
memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
Then we could reduce the memory reserved for opensbi in the next
patch.

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Anup Patel <anup.patel@wdc.com>
Cc: Atish Patra <atishp@rivosinc.com>
---
 arch/riscv/include/asm/page.h   |  8 ++++++++
 arch/riscv/kernel/head.S        | 10 +++-------
 arch/riscv/kernel/vmlinux.lds.S |  5 ++---
 arch/riscv/mm/init.c            | 11 ++++++++---
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index b3e5ff0125fe..299147c78b4a 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -16,6 +16,14 @@
 #define PAGE_SIZE	(_AC(1, UL) << PAGE_SHIFT)
 #define PAGE_MASK	(~(PAGE_SIZE - 1))
 
+#if __riscv_xlen == 64
+/* Image load offset(2MB) from start of RAM */
+#define LOAD_OFFSET	0x200000
+#else
+/* Image load offset(4MB) from start of RAM */
+#define LOAD_OFFSET	0x400000
+#endif
+
 #ifdef CONFIG_64BIT
 #define HUGE_MAX_HSTATE		2
 #else
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index f52f01ecbeea..a6ac892d2ccf 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -61,13 +61,7 @@ ENTRY(_start)
 	/* Image load offset (0MB) from start of RAM for M-mode */
 	.dword 0
 #else
-#if __riscv_xlen == 64
-	/* Image load offset(2MB) from start of RAM */
-	.dword 0x200000
-#else
-	/* Image load offset(4MB) from start of RAM */
-	.dword 0x400000
-#endif
+	.dword LOAD_OFFSET
 #endif
 	/* Effective size of kernel image */
 	.dword _end - _start
@@ -94,6 +88,8 @@ relocate:
 	la a1, kernel_map
 	XIP_FIXUP_OFFSET a1
 	REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
+	li a2, LOAD_OFFSET
+	add a1, a1, a2
 	la a2, _start
 	sub a1, a1, a2
 	add ra, ra, a1
diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
index 5104f3a871e3..75b7c72cd4bd 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -11,10 +11,9 @@
 #else
 
 #include <asm/pgtable.h>
-#define LOAD_OFFSET KERNEL_LINK_ADDR
 
-#include <asm/vmlinux.lds.h>
 #include <asm/page.h>
+#include <asm/vmlinux.lds.h>
 #include <asm/cache.h>
 #include <asm/thread_info.h>
 #include <asm/set_memory.h>
@@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
 SECTIONS
 {
 	/* Beginning of code and text segment */
-	. = LOAD_OFFSET;
+	. = LOAD_OFFSET + KERNEL_LINK_ADDR;
 	_start = .;
 	HEAD_TEXT_SECTION
 	. = ALIGN(PAGE_SIZE);
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 24b2b8044602..920e78f8c3e4 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
 	if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
 		memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
 
+	/*
+	 * Reserve OpenSBI region and depends on PMP to deny accesses.
+	 */
+	memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
+
 	early_init_fdt_scan_reserved_mem();
 	dma_contiguous_reserve(dma32_phys_limit);
 	if (IS_ENABLED(CONFIG_64BIT))
@@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 
 	kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
 #else
-	kernel_map.phys_addr = (uintptr_t)(&_start);
+	kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
 	kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
 #endif
 	kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
@@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 	create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
 			   kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
 #else
-	create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
-			   kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
+	create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
+			   kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
 #endif
 #else
 	/* Setup trampoline PGD */
-- 
2.25.1


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

[RFC PATCH 2/3] riscv: Add early_param to decrease firmware region

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

Using riscv.fw_size in cmdline to tell the kernel what the
firmware (opensbi) size is. Then reserve the proper size of
firmware to save memory instead of the whole 2MB. It's helpful
to satisfy a small memory system (D1s/F133 from Allwinner).

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Anup Patel <anup.patel@wdc.com>
Cc: Atish Patra <atishp@rivosinc.com>
---
 arch/riscv/mm/init.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 920e78f8c3e4..f7db6d40213d 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -159,6 +159,15 @@ static int __init early_mem(char *p)
 }
 early_param("mem", early_mem);
 
+static phys_addr_t firmware_size __initdata;
+static int __init early_get_firmware_size(char *arg)
+{
+	firmware_size = memparse(arg, &arg);
+
+	return 0;
+}
+early_param("riscv.fwsz", early_get_firmware_size);
+
 static void __init setup_bootmem(void)
 {
 	phys_addr_t vmlinux_end = __pa_symbol(&_end);
@@ -224,7 +233,10 @@ static void __init setup_bootmem(void)
 	/*
 	 * Reserve OpenSBI region and depends on PMP to deny accesses.
 	 */
-	memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
+	if (firmware_size > PAGE_SIZE)
+		memblock_reserve(__pa(PAGE_OFFSET), firmware_size);
+	else
+		memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
 
 	early_init_fdt_scan_reserved_mem();
 	dma_contiguous_reserve(dma32_phys_limit);
-- 
2.25.1

[RFC PATCH 2/3] riscv: Add early_param to decrease firmware region

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

Using riscv.fw_size in cmdline to tell the kernel what the
firmware (opensbi) size is. Then reserve the proper size of
firmware to save memory instead of the whole 2MB. It's helpful
to satisfy a small memory system (D1s/F133 from Allwinner).

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Anup Patel <anup.patel@wdc.com>
Cc: Atish Patra <atishp@rivosinc.com>
---
 arch/riscv/mm/init.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 920e78f8c3e4..f7db6d40213d 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -159,6 +159,15 @@ static int __init early_mem(char *p)
 }
 early_param("mem", early_mem);
 
+static phys_addr_t firmware_size __initdata;
+static int __init early_get_firmware_size(char *arg)
+{
+	firmware_size = memparse(arg, &arg);
+
+	return 0;
+}
+early_param("riscv.fwsz", early_get_firmware_size);
+
 static void __init setup_bootmem(void)
 {
 	phys_addr_t vmlinux_end = __pa_symbol(&_end);
@@ -224,7 +233,10 @@ static void __init setup_bootmem(void)
 	/*
 	 * Reserve OpenSBI region and depends on PMP to deny accesses.
 	 */
-	memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
+	if (firmware_size > PAGE_SIZE)
+		memblock_reserve(__pa(PAGE_OFFSET), firmware_size);
+	else
+		memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
 
 	early_init_fdt_scan_reserved_mem();
 	dma_contiguous_reserve(dma32_phys_limit);
-- 
2.25.1


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

[RFC PATCH 3/3] riscv: Add riscv.fwsz kernel parameter

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
4MB(32bit) in Linux. It's very wasteful to small memory footprint
soc chip such as Allwinner D1s/F133. The kernel parameter gives a
chance to users to set the proper size of the firmware and get
more than 1.5MB of memory.

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Anup Patel <anup.patel@wdc.com>
Cc: Atish Patra <atishp@rivosinc.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 9725c546a0d4..ee505743c8f4 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4964,6 +4964,9 @@
 			[KNL] Disable ring 3 MONITOR/MWAIT feature on supported
 			CPUs.
 
+	riscv.fwsz=nn[KMG]
+			[RISC-V] Determine firmware size to save memory
+
 	ro		[KNL] Mount root device read-only on boot
 
 	rodata=		[KNL]
-- 
2.25.1

[RFC PATCH 3/3] riscv: Add riscv.fwsz kernel parameter

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
4MB(32bit) in Linux. It's very wasteful to small memory footprint
soc chip such as Allwinner D1s/F133. The kernel parameter gives a
chance to users to set the proper size of the firmware and get
more than 1.5MB of memory.

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Anup Patel <anup.patel@wdc.com>
Cc: Atish Patra <atishp@rivosinc.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 9725c546a0d4..ee505743c8f4 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4964,6 +4964,9 @@
 			[KNL] Disable ring 3 MONITOR/MWAIT feature on supported
 			CPUs.
 
+	riscv.fwsz=nn[KMG]
+			[RISC-V] Determine firmware size to save memory
+
 	ro		[KNL] Mount root device read-only on boot
 
 	rodata=		[KNL]
-- 
2.25.1


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

Re: [RFC PATCH 3/3] riscv: Add riscv.fwsz kernel parameter

[Randy Dunlap]

On 11/22/21 5:57 PM, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> chance to users to set the proper size of the firmware and get
> more than 1.5MB of memory.
> 
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: Atish Patra <atishp@rivosinc.com>
> ---
>   Documentation/admin-guide/kernel-parameters.txt | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9725c546a0d4..ee505743c8f4 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4964,6 +4964,9 @@
>   			[KNL] Disable ring 3 MONITOR/MWAIT feature on supported
>   			CPUs.
>   
> +	riscv.fwsz=nn[KMG]
> +			[RISC-V] Determine firmware size to save memory

Is "Determine" like "Set"?  The user is setting (telling the software)
the firmware size?

"Determine" makes it sound to me like the Linux software is somehow
helping to determine the firmware size.

> +
>   	ro		[KNL] Mount root device read-only on boot
>   
>   	rodata=		[KNL]
> 


-- 
~Randy

Re: [RFC PATCH 3/3] riscv: Add riscv.fwsz kernel parameter

[Randy Dunlap]

On 11/22/21 5:57 PM, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> chance to users to set the proper size of the firmware and get
> more than 1.5MB of memory.
> 
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: Atish Patra <atishp@rivosinc.com>
> ---
>   Documentation/admin-guide/kernel-parameters.txt | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9725c546a0d4..ee505743c8f4 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4964,6 +4964,9 @@
>   			[KNL] Disable ring 3 MONITOR/MWAIT feature on supported
>   			CPUs.
>   
> +	riscv.fwsz=nn[KMG]
> +			[RISC-V] Determine firmware size to save memory

Is "Determine" like "Set"?  The user is setting (telling the software)
the firmware size?

"Determine" makes it sound to me like the Linux software is somehow
helping to determine the firmware size.

> +
>   	ro		[KNL] Mount root device read-only on boot
>   
>   	rodata=		[KNL]
> 


-- 
~Randy

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

Re: [RFC PATCH 3/3] riscv: Add riscv.fwsz kernel parameter

[Guo Ren]

On Tue, Nov 23, 2021 at 10:34 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 11/22/21 5:57 PM, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > chance to users to set the proper size of the firmware and get
> > more than 1.5MB of memory.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Anup Patel <anup.patel@wdc.com>
> > Cc: Atish Patra <atishp@rivosinc.com>
> > ---
> >   Documentation/admin-guide/kernel-parameters.txt | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 9725c546a0d4..ee505743c8f4 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -4964,6 +4964,9 @@
> >                       [KNL] Disable ring 3 MONITOR/MWAIT feature on supported
> >                       CPUs.
> >
> > +     riscv.fwsz=nn[KMG]
> > +                     [RISC-V] Determine firmware size to save memory
>
> Is "Determine" like "Set"?  The user is setting (telling the software)
> the firmware size?
I mean "Set" here, thx for pointing it out.

>
> "Determine" makes it sound to me like the Linux software is somehow
> helping to determine the firmware size.
>
> > +
> >       ro              [KNL] Mount root device read-only on boot
> >
> >       rodata=         [KNL]
> >
>
>
> --
> ~Randy



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Re: [RFC PATCH 3/3] riscv: Add riscv.fwsz kernel parameter

[Guo Ren]

On Tue, Nov 23, 2021 at 10:34 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 11/22/21 5:57 PM, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > chance to users to set the proper size of the firmware and get
> > more than 1.5MB of memory.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Anup Patel <anup.patel@wdc.com>
> > Cc: Atish Patra <atishp@rivosinc.com>
> > ---
> >   Documentation/admin-guide/kernel-parameters.txt | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 9725c546a0d4..ee505743c8f4 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -4964,6 +4964,9 @@
> >                       [KNL] Disable ring 3 MONITOR/MWAIT feature on supported
> >                       CPUs.
> >
> > +     riscv.fwsz=nn[KMG]
> > +                     [RISC-V] Determine firmware size to save memory
>
> Is "Determine" like "Set"?  The user is setting (telling the software)
> the firmware size?
I mean "Set" here, thx for pointing it out.

>
> "Determine" makes it sound to me like the Linux software is somehow
> helping to determine the firmware size.
>
> > +
> >       ro              [KNL] Mount root device read-only on boot
> >
> >       rodata=         [KNL]
> >
>
>
> --
> ~Randy



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

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

Re: [RFC PATCH 2/3] riscv: Add early_param to decrease firmware region

[Anup Patel]

+Alex

On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Using riscv.fw_size in cmdline to tell the kernel what the
> firmware (opensbi) size is. Then reserve the proper size of
> firmware to save memory instead of the whole 2MB. It's helpful
> to satisfy a small memory system (D1s/F133 from Allwinner).
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: Atish Patra <atishp@rivosinc.com>
> ---
>  arch/riscv/mm/init.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 920e78f8c3e4..f7db6d40213d 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -159,6 +159,15 @@ static int __init early_mem(char *p)
>  }
>  early_param("mem", early_mem);
>
> +static phys_addr_t firmware_size __initdata;
> +static int __init early_get_firmware_size(char *arg)
> +{
> +       firmware_size = memparse(arg, &arg);
> +
> +       return 0;
> +}
> +early_param("riscv.fwsz", early_get_firmware_size);
> +

We have avoided any RISC-V specific kernel parameter till now
and I don't think adding "riscv.fwsz" is the right approach.

OpenSBI adds a reserved memory node (mmode_resv@8000000)
to mark the memory where it is running as reserved. In fact, all
M-mode runtime firmware should be adding a reserved memory
node just like OpenSBI.

Regards,
Anup

>  static void __init setup_bootmem(void)
>  {
>         phys_addr_t vmlinux_end = __pa_symbol(&_end);
> @@ -224,7 +233,10 @@ static void __init setup_bootmem(void)
>         /*
>          * Reserve OpenSBI region and depends on PMP to deny accesses.
>          */
> -       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
> +       if (firmware_size > PAGE_SIZE)
> +               memblock_reserve(__pa(PAGE_OFFSET), firmware_size);
> +       else
> +               memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
>
>         early_init_fdt_scan_reserved_mem();
>         dma_contiguous_reserve(dma32_phys_limit);
> --
> 2.25.1
>

Re: [RFC PATCH 2/3] riscv: Add early_param to decrease firmware region

[Anup Patel]

+Alex

On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Using riscv.fw_size in cmdline to tell the kernel what the
> firmware (opensbi) size is. Then reserve the proper size of
> firmware to save memory instead of the whole 2MB. It's helpful
> to satisfy a small memory system (D1s/F133 from Allwinner).
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: Atish Patra <atishp@rivosinc.com>
> ---
>  arch/riscv/mm/init.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 920e78f8c3e4..f7db6d40213d 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -159,6 +159,15 @@ static int __init early_mem(char *p)
>  }
>  early_param("mem", early_mem);
>
> +static phys_addr_t firmware_size __initdata;
> +static int __init early_get_firmware_size(char *arg)
> +{
> +       firmware_size = memparse(arg, &arg);
> +
> +       return 0;
> +}
> +early_param("riscv.fwsz", early_get_firmware_size);
> +

We have avoided any RISC-V specific kernel parameter till now
and I don't think adding "riscv.fwsz" is the right approach.

OpenSBI adds a reserved memory node (mmode_resv@8000000)
to mark the memory where it is running as reserved. In fact, all
M-mode runtime firmware should be adding a reserved memory
node just like OpenSBI.

Regards,
Anup

>  static void __init setup_bootmem(void)
>  {
>         phys_addr_t vmlinux_end = __pa_symbol(&_end);
> @@ -224,7 +233,10 @@ static void __init setup_bootmem(void)
>         /*
>          * Reserve OpenSBI region and depends on PMP to deny accesses.
>          */
> -       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
> +       if (firmware_size > PAGE_SIZE)
> +               memblock_reserve(__pa(PAGE_OFFSET), firmware_size);
> +       else
> +               memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
>
>         early_init_fdt_scan_reserved_mem();
>         dma_contiguous_reserve(dma32_phys_limit);
> --
> 2.25.1
>

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

Re: [RFC PATCH 3/3] riscv: Add riscv.fwsz kernel parameter

[Anup Patel]

On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> chance to users to set the proper size of the firmware and get
> more than 1.5MB of memory.

This kernel parameter is redundant see my comment on other patch.

regards,
Anup

>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: Atish Patra <atishp@rivosinc.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9725c546a0d4..ee505743c8f4 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4964,6 +4964,9 @@
>                         [KNL] Disable ring 3 MONITOR/MWAIT feature on supported
>                         CPUs.
>
> +       riscv.fwsz=nn[KMG]
> +                       [RISC-V] Determine firmware size to save memory
> +
>         ro              [KNL] Mount root device read-only on boot
>
>         rodata=         [KNL]
> --
> 2.25.1
>

Re: [RFC PATCH 3/3] riscv: Add riscv.fwsz kernel parameter

[Anup Patel]

On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> chance to users to set the proper size of the firmware and get
> more than 1.5MB of memory.

This kernel parameter is redundant see my comment on other patch.

regards,
Anup

>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: Atish Patra <atishp@rivosinc.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9725c546a0d4..ee505743c8f4 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4964,6 +4964,9 @@
>                         [KNL] Disable ring 3 MONITOR/MWAIT feature on supported
>                         CPUs.
>
> +       riscv.fwsz=nn[KMG]
> +                       [RISC-V] Determine firmware size to save memory
> +
>         ro              [KNL] Mount root device read-only on boot
>
>         rodata=         [KNL]
> --
> 2.25.1
>

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

Re: [RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[Anup Patel]

+Alex

On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> The current RISC-V's mm layout is based on a 2MB offset and wasting
> memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
> Then we could reduce the memory reserved for opensbi in the next
> patch.

The real problem is that the generic kernel marks memory before
__pa(PAGE_OFFSET) as reserved which is evident from the boot
print "OF: fdt: Ignoring memory range 0x80000000 - 0x80200000".

One simple way to re-claim the first 2MB of memory is by:
1) Not placing OpenSBI firmware at start of RAM and rather
place it towards end/middle or RAM away from kernel and initrd
2) Load kernel at start of the RAM

The point#1 is already supported by OpenSBI firmwares using
position independent compilation. In fact, U-Boot SPL does
not load OpenSBI firmware at the start of RAM.

I would suggest Allwinner D1 to follow U-Boot SPL and have
the booting stage before OpenSBI to load OpenSBI firmware
somewhere else.

Regards,
Anup

>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: Atish Patra <atishp@rivosinc.com>
> ---
>  arch/riscv/include/asm/page.h   |  8 ++++++++
>  arch/riscv/kernel/head.S        | 10 +++-------
>  arch/riscv/kernel/vmlinux.lds.S |  5 ++---
>  arch/riscv/mm/init.c            | 11 ++++++++---
>  4 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> index b3e5ff0125fe..299147c78b4a 100644
> --- a/arch/riscv/include/asm/page.h
> +++ b/arch/riscv/include/asm/page.h
> @@ -16,6 +16,14 @@
>  #define PAGE_SIZE      (_AC(1, UL) << PAGE_SHIFT)
>  #define PAGE_MASK      (~(PAGE_SIZE - 1))
>
> +#if __riscv_xlen == 64
> +/* Image load offset(2MB) from start of RAM */
> +#define LOAD_OFFSET    0x200000
> +#else
> +/* Image load offset(4MB) from start of RAM */
> +#define LOAD_OFFSET    0x400000
> +#endif
> +
>  #ifdef CONFIG_64BIT
>  #define HUGE_MAX_HSTATE                2
>  #else
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index f52f01ecbeea..a6ac892d2ccf 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -61,13 +61,7 @@ ENTRY(_start)
>         /* Image load offset (0MB) from start of RAM for M-mode */
>         .dword 0
>  #else
> -#if __riscv_xlen == 64
> -       /* Image load offset(2MB) from start of RAM */
> -       .dword 0x200000
> -#else
> -       /* Image load offset(4MB) from start of RAM */
> -       .dword 0x400000
> -#endif
> +       .dword LOAD_OFFSET
>  #endif
>         /* Effective size of kernel image */
>         .dword _end - _start
> @@ -94,6 +88,8 @@ relocate:
>         la a1, kernel_map
>         XIP_FIXUP_OFFSET a1
>         REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
> +       li a2, LOAD_OFFSET
> +       add a1, a1, a2
>         la a2, _start
>         sub a1, a1, a2
>         add ra, ra, a1
> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> index 5104f3a871e3..75b7c72cd4bd 100644
> --- a/arch/riscv/kernel/vmlinux.lds.S
> +++ b/arch/riscv/kernel/vmlinux.lds.S
> @@ -11,10 +11,9 @@
>  #else
>
>  #include <asm/pgtable.h>
> -#define LOAD_OFFSET KERNEL_LINK_ADDR
>
> -#include <asm/vmlinux.lds.h>
>  #include <asm/page.h>
> +#include <asm/vmlinux.lds.h>
>  #include <asm/cache.h>
>  #include <asm/thread_info.h>
>  #include <asm/set_memory.h>
> @@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
>  SECTIONS
>  {
>         /* Beginning of code and text segment */
> -       . = LOAD_OFFSET;
> +       . = LOAD_OFFSET + KERNEL_LINK_ADDR;
>         _start = .;
>         HEAD_TEXT_SECTION
>         . = ALIGN(PAGE_SIZE);
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 24b2b8044602..920e78f8c3e4 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
>         if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
>                 memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
>
> +       /*
> +        * Reserve OpenSBI region and depends on PMP to deny accesses.
> +        */
> +       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
> +
>         early_init_fdt_scan_reserved_mem();
>         dma_contiguous_reserve(dma32_phys_limit);
>         if (IS_ENABLED(CONFIG_64BIT))
> @@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>
>         kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
>  #else
> -       kernel_map.phys_addr = (uintptr_t)(&_start);
> +       kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
>         kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
>  #endif
>         kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
> @@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>         create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
>                            kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
>  #else
> -       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> -                          kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
> +       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
> +                          kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
>  #endif
>  #else
>         /* Setup trampoline PGD */
> --
> 2.25.1
>

Re: [RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[Anup Patel]

+Alex

On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> The current RISC-V's mm layout is based on a 2MB offset and wasting
> memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
> Then we could reduce the memory reserved for opensbi in the next
> patch.

The real problem is that the generic kernel marks memory before
__pa(PAGE_OFFSET) as reserved which is evident from the boot
print "OF: fdt: Ignoring memory range 0x80000000 - 0x80200000".

One simple way to re-claim the first 2MB of memory is by:
1) Not placing OpenSBI firmware at start of RAM and rather
place it towards end/middle or RAM away from kernel and initrd
2) Load kernel at start of the RAM

The point#1 is already supported by OpenSBI firmwares using
position independent compilation. In fact, U-Boot SPL does
not load OpenSBI firmware at the start of RAM.

I would suggest Allwinner D1 to follow U-Boot SPL and have
the booting stage before OpenSBI to load OpenSBI firmware
somewhere else.

Regards,
Anup

>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: Atish Patra <atishp@rivosinc.com>
> ---
>  arch/riscv/include/asm/page.h   |  8 ++++++++
>  arch/riscv/kernel/head.S        | 10 +++-------
>  arch/riscv/kernel/vmlinux.lds.S |  5 ++---
>  arch/riscv/mm/init.c            | 11 ++++++++---
>  4 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> index b3e5ff0125fe..299147c78b4a 100644
> --- a/arch/riscv/include/asm/page.h
> +++ b/arch/riscv/include/asm/page.h
> @@ -16,6 +16,14 @@
>  #define PAGE_SIZE      (_AC(1, UL) << PAGE_SHIFT)
>  #define PAGE_MASK      (~(PAGE_SIZE - 1))
>
> +#if __riscv_xlen == 64
> +/* Image load offset(2MB) from start of RAM */
> +#define LOAD_OFFSET    0x200000
> +#else
> +/* Image load offset(4MB) from start of RAM */
> +#define LOAD_OFFSET    0x400000
> +#endif
> +
>  #ifdef CONFIG_64BIT
>  #define HUGE_MAX_HSTATE                2
>  #else
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index f52f01ecbeea..a6ac892d2ccf 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -61,13 +61,7 @@ ENTRY(_start)
>         /* Image load offset (0MB) from start of RAM for M-mode */
>         .dword 0
>  #else
> -#if __riscv_xlen == 64
> -       /* Image load offset(2MB) from start of RAM */
> -       .dword 0x200000
> -#else
> -       /* Image load offset(4MB) from start of RAM */
> -       .dword 0x400000
> -#endif
> +       .dword LOAD_OFFSET
>  #endif
>         /* Effective size of kernel image */
>         .dword _end - _start
> @@ -94,6 +88,8 @@ relocate:
>         la a1, kernel_map
>         XIP_FIXUP_OFFSET a1
>         REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
> +       li a2, LOAD_OFFSET
> +       add a1, a1, a2
>         la a2, _start
>         sub a1, a1, a2
>         add ra, ra, a1
> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> index 5104f3a871e3..75b7c72cd4bd 100644
> --- a/arch/riscv/kernel/vmlinux.lds.S
> +++ b/arch/riscv/kernel/vmlinux.lds.S
> @@ -11,10 +11,9 @@
>  #else
>
>  #include <asm/pgtable.h>
> -#define LOAD_OFFSET KERNEL_LINK_ADDR
>
> -#include <asm/vmlinux.lds.h>
>  #include <asm/page.h>
> +#include <asm/vmlinux.lds.h>
>  #include <asm/cache.h>
>  #include <asm/thread_info.h>
>  #include <asm/set_memory.h>
> @@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
>  SECTIONS
>  {
>         /* Beginning of code and text segment */
> -       . = LOAD_OFFSET;
> +       . = LOAD_OFFSET + KERNEL_LINK_ADDR;
>         _start = .;
>         HEAD_TEXT_SECTION
>         . = ALIGN(PAGE_SIZE);
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 24b2b8044602..920e78f8c3e4 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
>         if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
>                 memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
>
> +       /*
> +        * Reserve OpenSBI region and depends on PMP to deny accesses.
> +        */
> +       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
> +
>         early_init_fdt_scan_reserved_mem();
>         dma_contiguous_reserve(dma32_phys_limit);
>         if (IS_ENABLED(CONFIG_64BIT))
> @@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>
>         kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
>  #else
> -       kernel_map.phys_addr = (uintptr_t)(&_start);
> +       kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
>         kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
>  #endif
>         kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
> @@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>         create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
>                            kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
>  #else
> -       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> -                          kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
> +       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
> +                          kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
>  #endif
>  #else
>         /* Setup trampoline PGD */
> --
> 2.25.1
>

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

Re: [RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[Guo Ren]

On Tue, Nov 23, 2021 at 11:56 AM Anup Patel <anup@brainfault.org> wrote:
>
> +Alex
>
> On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
> >
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > The current RISC-V's mm layout is based on a 2MB offset and wasting
> > memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
> > Then we could reduce the memory reserved for opensbi in the next
> > patch.
>
> The real problem is that the generic kernel marks memory before
> __pa(PAGE_OFFSET) as reserved which is evident from the boot
> print "OF: fdt: Ignoring memory range 0x80000000 - 0x80200000".
>
> One simple way to re-claim the first 2MB of memory is by:
> 1) Not placing OpenSBI firmware at start of RAM and rather
> place it towards end/middle or RAM away from kernel and initrd
> 2) Load kernel at start of the RAM
>
> The point#1 is already supported by OpenSBI firmwares using
> position independent compilation. In fact, U-Boot SPL does
> not load OpenSBI firmware at the start of RAM.
This deviates from the original intention of this patch. Some users
have been used to 2MB/4MB LOAD_OFFSET and we also should save the
memory of opensbi for them.

>
> I would suggest Allwinner D1 to follow U-Boot SPL and have
> the booting stage before OpenSBI to load OpenSBI firmware
>
> Regards,
> Anup
>
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Anup Patel <anup.patel@wdc.com>
> > Cc: Atish Patra <atishp@rivosinc.com>
> > ---
> >  arch/riscv/include/asm/page.h   |  8 ++++++++
> >  arch/riscv/kernel/head.S        | 10 +++-------
> >  arch/riscv/kernel/vmlinux.lds.S |  5 ++---
> >  arch/riscv/mm/init.c            | 11 ++++++++---
> >  4 files changed, 21 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> > index b3e5ff0125fe..299147c78b4a 100644
> > --- a/arch/riscv/include/asm/page.h
> > +++ b/arch/riscv/include/asm/page.h
> > @@ -16,6 +16,14 @@
> >  #define PAGE_SIZE      (_AC(1, UL) << PAGE_SHIFT)
> >  #define PAGE_MASK      (~(PAGE_SIZE - 1))
> >
> > +#if __riscv_xlen == 64
> > +/* Image load offset(2MB) from start of RAM */
> > +#define LOAD_OFFSET    0x200000
> > +#else
> > +/* Image load offset(4MB) from start of RAM */
> > +#define LOAD_OFFSET    0x400000
> > +#endif
> > +
> >  #ifdef CONFIG_64BIT
> >  #define HUGE_MAX_HSTATE                2
> >  #else
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index f52f01ecbeea..a6ac892d2ccf 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -61,13 +61,7 @@ ENTRY(_start)
> >         /* Image load offset (0MB) from start of RAM for M-mode */
> >         .dword 0
> >  #else
> > -#if __riscv_xlen == 64
> > -       /* Image load offset(2MB) from start of RAM */
> > -       .dword 0x200000
> > -#else
> > -       /* Image load offset(4MB) from start of RAM */
> > -       .dword 0x400000
> > -#endif
> > +       .dword LOAD_OFFSET
> >  #endif
> >         /* Effective size of kernel image */
> >         .dword _end - _start
> > @@ -94,6 +88,8 @@ relocate:
> >         la a1, kernel_map
> >         XIP_FIXUP_OFFSET a1
> >         REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
> > +       li a2, LOAD_OFFSET
> > +       add a1, a1, a2
> >         la a2, _start
> >         sub a1, a1, a2
> >         add ra, ra, a1
> > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > index 5104f3a871e3..75b7c72cd4bd 100644
> > --- a/arch/riscv/kernel/vmlinux.lds.S
> > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > @@ -11,10 +11,9 @@
> >  #else
> >
> >  #include <asm/pgtable.h>
> > -#define LOAD_OFFSET KERNEL_LINK_ADDR
> >
> > -#include <asm/vmlinux.lds.h>
> >  #include <asm/page.h>
> > +#include <asm/vmlinux.lds.h>
> >  #include <asm/cache.h>
> >  #include <asm/thread_info.h>
> >  #include <asm/set_memory.h>
> > @@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
> >  SECTIONS
> >  {
> >         /* Beginning of code and text segment */
> > -       . = LOAD_OFFSET;
> > +       . = LOAD_OFFSET + KERNEL_LINK_ADDR;
> >         _start = .;
> >         HEAD_TEXT_SECTION
> >         . = ALIGN(PAGE_SIZE);
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 24b2b8044602..920e78f8c3e4 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
> >         if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
> >                 memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> >
> > +       /*
> > +        * Reserve OpenSBI region and depends on PMP to deny accesses.
> > +        */
> > +       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
> > +
> >         early_init_fdt_scan_reserved_mem();
> >         dma_contiguous_reserve(dma32_phys_limit);
> >         if (IS_ENABLED(CONFIG_64BIT))
> > @@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >
> >         kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
> >  #else
> > -       kernel_map.phys_addr = (uintptr_t)(&_start);
> > +       kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
> >         kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
> >  #endif
> >         kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
> > @@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >         create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> >                            kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
> >  #else
> > -       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> > -                          kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
> > +       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
> > +                          kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
> >  #endif
> >  #else
> >         /* Setup trampoline PGD */
> > --
> > 2.25.1
> >



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Re: [RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[Guo Ren]

On Tue, Nov 23, 2021 at 11:56 AM Anup Patel <anup@brainfault.org> wrote:
>
> +Alex
>
> On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
> >
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > The current RISC-V's mm layout is based on a 2MB offset and wasting
> > memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
> > Then we could reduce the memory reserved for opensbi in the next
> > patch.
>
> The real problem is that the generic kernel marks memory before
> __pa(PAGE_OFFSET) as reserved which is evident from the boot
> print "OF: fdt: Ignoring memory range 0x80000000 - 0x80200000".
>
> One simple way to re-claim the first 2MB of memory is by:
> 1) Not placing OpenSBI firmware at start of RAM and rather
> place it towards end/middle or RAM away from kernel and initrd
> 2) Load kernel at start of the RAM
>
> The point#1 is already supported by OpenSBI firmwares using
> position independent compilation. In fact, U-Boot SPL does
> not load OpenSBI firmware at the start of RAM.
This deviates from the original intention of this patch. Some users
have been used to 2MB/4MB LOAD_OFFSET and we also should save the
memory of opensbi for them.

>
> I would suggest Allwinner D1 to follow U-Boot SPL and have
> the booting stage before OpenSBI to load OpenSBI firmware
>
> Regards,
> Anup
>
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Anup Patel <anup.patel@wdc.com>
> > Cc: Atish Patra <atishp@rivosinc.com>
> > ---
> >  arch/riscv/include/asm/page.h   |  8 ++++++++
> >  arch/riscv/kernel/head.S        | 10 +++-------
> >  arch/riscv/kernel/vmlinux.lds.S |  5 ++---
> >  arch/riscv/mm/init.c            | 11 ++++++++---
> >  4 files changed, 21 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> > index b3e5ff0125fe..299147c78b4a 100644
> > --- a/arch/riscv/include/asm/page.h
> > +++ b/arch/riscv/include/asm/page.h
> > @@ -16,6 +16,14 @@
> >  #define PAGE_SIZE      (_AC(1, UL) << PAGE_SHIFT)
> >  #define PAGE_MASK      (~(PAGE_SIZE - 1))
> >
> > +#if __riscv_xlen == 64
> > +/* Image load offset(2MB) from start of RAM */
> > +#define LOAD_OFFSET    0x200000
> > +#else
> > +/* Image load offset(4MB) from start of RAM */
> > +#define LOAD_OFFSET    0x400000
> > +#endif
> > +
> >  #ifdef CONFIG_64BIT
> >  #define HUGE_MAX_HSTATE                2
> >  #else
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index f52f01ecbeea..a6ac892d2ccf 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -61,13 +61,7 @@ ENTRY(_start)
> >         /* Image load offset (0MB) from start of RAM for M-mode */
> >         .dword 0
> >  #else
> > -#if __riscv_xlen == 64
> > -       /* Image load offset(2MB) from start of RAM */
> > -       .dword 0x200000
> > -#else
> > -       /* Image load offset(4MB) from start of RAM */
> > -       .dword 0x400000
> > -#endif
> > +       .dword LOAD_OFFSET
> >  #endif
> >         /* Effective size of kernel image */
> >         .dword _end - _start
> > @@ -94,6 +88,8 @@ relocate:
> >         la a1, kernel_map
> >         XIP_FIXUP_OFFSET a1
> >         REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
> > +       li a2, LOAD_OFFSET
> > +       add a1, a1, a2
> >         la a2, _start
> >         sub a1, a1, a2
> >         add ra, ra, a1
> > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > index 5104f3a871e3..75b7c72cd4bd 100644
> > --- a/arch/riscv/kernel/vmlinux.lds.S
> > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > @@ -11,10 +11,9 @@
> >  #else
> >
> >  #include <asm/pgtable.h>
> > -#define LOAD_OFFSET KERNEL_LINK_ADDR
> >
> > -#include <asm/vmlinux.lds.h>
> >  #include <asm/page.h>
> > +#include <asm/vmlinux.lds.h>
> >  #include <asm/cache.h>
> >  #include <asm/thread_info.h>
> >  #include <asm/set_memory.h>
> > @@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
> >  SECTIONS
> >  {
> >         /* Beginning of code and text segment */
> > -       . = LOAD_OFFSET;
> > +       . = LOAD_OFFSET + KERNEL_LINK_ADDR;
> >         _start = .;
> >         HEAD_TEXT_SECTION
> >         . = ALIGN(PAGE_SIZE);
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 24b2b8044602..920e78f8c3e4 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
> >         if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
> >                 memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> >
> > +       /*
> > +        * Reserve OpenSBI region and depends on PMP to deny accesses.
> > +        */
> > +       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
> > +
> >         early_init_fdt_scan_reserved_mem();
> >         dma_contiguous_reserve(dma32_phys_limit);
> >         if (IS_ENABLED(CONFIG_64BIT))
> > @@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >
> >         kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
> >  #else
> > -       kernel_map.phys_addr = (uintptr_t)(&_start);
> > +       kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
> >         kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
> >  #endif
> >         kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
> > @@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >         create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> >                            kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
> >  #else
> > -       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> > -                          kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
> > +       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
> > +                          kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
> >  #endif
> >  #else
> >         /* Setup trampoline PGD */
> > --
> > 2.25.1
> >



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

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

Re: [RFC PATCH 2/3] riscv: Add early_param to decrease firmware region

[Jessica Clarke]

On 23 Nov 2021, at 03:44, Anup Patel <anup@brainfault.org> wrote:
> 
> +Alex
> 
> On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
>> 
>> From: Guo Ren <guoren@linux.alibaba.com>
>> 
>> Using riscv.fw_size in cmdline to tell the kernel what the
>> firmware (opensbi) size is. Then reserve the proper size of
>> firmware to save memory instead of the whole 2MB. It's helpful
>> to satisfy a small memory system (D1s/F133 from Allwinner).
>> 
>> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
>> Cc: Anup Patel <anup.patel@wdc.com>
>> Cc: Atish Patra <atishp@rivosinc.com>
>> ---
>> arch/riscv/mm/init.c | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index 920e78f8c3e4..f7db6d40213d 100644
>> --- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -159,6 +159,15 @@ static int __init early_mem(char *p)
>> }
>> early_param("mem", early_mem);
>> 
>> +static phys_addr_t firmware_size __initdata;
>> +static int __init early_get_firmware_size(char *arg)
>> +{
>> +       firmware_size = memparse(arg, &arg);
>> +
>> +       return 0;
>> +}
>> +early_param("riscv.fwsz", early_get_firmware_size);
>> +
> 
> We have avoided any RISC-V specific kernel parameter till now
> and I don't think adding "riscv.fwsz" is the right approach.
> 
> OpenSBI adds a reserved memory node (mmode_resv@8000000)
> to mark the memory where it is running as reserved. In fact, all
> M-mode runtime firmware should be adding a reserved memory
> node just like OpenSBI.

BBL does not do this and, even if it’s modified today, older versions
will still need to be supported for quite a while longer.

In FreeBSD[1] we only reserve the first 2 MiB of DRAM (we don’t care
about RV32) if there is no reserved memory node covering the DRAM base
address, which avoids this issue. The only downside with that approach
is that if firmware occupies a different region than the beginning of
DRAM (or there is no firmware resident in the supervisor’s physical
address space, as is the case for a virtualised guest) then it
unnecessarily reserves that first 2 MiB, but that’s not a huge deal,
and can’t be avoided so long as BBL continues to exist (well, I guess
you could probe the SBI implementation ID if you really cared about
that, but I’ve yet to hear of a platform where the SBI implementation,
if it exists, isn’t at the start of DRAM, and if you’re virtualising
then you probably have enough DRAM that you don’t notice 2 MiB going
missing).

Jess

[1] https://github.com/freebsd/freebsd-src/blob/main/sys/riscv/riscv/machdep.c#L554-L568

Re: [RFC PATCH 2/3] riscv: Add early_param to decrease firmware region

[Jessica Clarke]

On 23 Nov 2021, at 03:44, Anup Patel <anup@brainfault.org> wrote:
> 
> +Alex
> 
> On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
>> 
>> From: Guo Ren <guoren@linux.alibaba.com>
>> 
>> Using riscv.fw_size in cmdline to tell the kernel what the
>> firmware (opensbi) size is. Then reserve the proper size of
>> firmware to save memory instead of the whole 2MB. It's helpful
>> to satisfy a small memory system (D1s/F133 from Allwinner).
>> 
>> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
>> Cc: Anup Patel <anup.patel@wdc.com>
>> Cc: Atish Patra <atishp@rivosinc.com>
>> ---
>> arch/riscv/mm/init.c | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index 920e78f8c3e4..f7db6d40213d 100644
>> --- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -159,6 +159,15 @@ static int __init early_mem(char *p)
>> }
>> early_param("mem", early_mem);
>> 
>> +static phys_addr_t firmware_size __initdata;
>> +static int __init early_get_firmware_size(char *arg)
>> +{
>> +       firmware_size = memparse(arg, &arg);
>> +
>> +       return 0;
>> +}
>> +early_param("riscv.fwsz", early_get_firmware_size);
>> +
> 
> We have avoided any RISC-V specific kernel parameter till now
> and I don't think adding "riscv.fwsz" is the right approach.
> 
> OpenSBI adds a reserved memory node (mmode_resv@8000000)
> to mark the memory where it is running as reserved. In fact, all
> M-mode runtime firmware should be adding a reserved memory
> node just like OpenSBI.

BBL does not do this and, even if it’s modified today, older versions
will still need to be supported for quite a while longer.

In FreeBSD[1] we only reserve the first 2 MiB of DRAM (we don’t care
about RV32) if there is no reserved memory node covering the DRAM base
address, which avoids this issue. The only downside with that approach
is that if firmware occupies a different region than the beginning of
DRAM (or there is no firmware resident in the supervisor’s physical
address space, as is the case for a virtualised guest) then it
unnecessarily reserves that first 2 MiB, but that’s not a huge deal,
and can’t be avoided so long as BBL continues to exist (well, I guess
you could probe the SBI implementation ID if you really cared about
that, but I’ve yet to hear of a platform where the SBI implementation,
if it exists, isn’t at the start of DRAM, and if you’re virtualising
then you probably have enough DRAM that you don’t notice 2 MiB going
missing).

Jess

[1] https://github.com/freebsd/freebsd-src/blob/main/sys/riscv/riscv/machdep.c#L554-L568


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

Re: [RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[Alexandre Ghiti]

On Tue, Nov 23, 2021 at 4:56 AM Anup Patel <anup@brainfault.org> wrote:
>
> +Alex
>
> On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
> >
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > The current RISC-V's mm layout is based on a 2MB offset and wasting
> > memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
> > Then we could reduce the memory reserved for opensbi in the next
> > patch.
>
> The real problem is that the generic kernel marks memory before
> __pa(PAGE_OFFSET) as reserved which is evident from the boot
> print "OF: fdt: Ignoring memory range 0x80000000 - 0x80200000".

Agreed, memblock 'rejects' this region because MIN_MEMBLOCK_ADDR is
defined as __pa(PAGE_OFFSET) which points to 0x80200000. I have a
patch to enable the use of hugepages for the linear mapping [1] that
does just that, things are not that easy since then it breaks initrd
initialization which is an early caller of __va, I updated this
patchset a few months ago, I can push that soon @Guo Ren.

[1] https://lkml.org/lkml/2020/6/3/696



>
> One simple way to re-claim the first 2MB of memory is by:
> 1) Not placing OpenSBI firmware at start of RAM and rather
> place it towards end/middle or RAM away from kernel and initrd
> 2) Load kernel at start of the RAM
>
> The point#1 is already supported by OpenSBI firmwares using
> position independent compilation. In fact, U-Boot SPL does
> not load OpenSBI firmware at the start of RAM.
>
> I would suggest Allwinner D1 to follow U-Boot SPL and have
> the booting stage before OpenSBI to load OpenSBI firmware
> somewhere else.
>
> Regards,
> Anup
>
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Anup Patel <anup.patel@wdc.com>
> > Cc: Atish Patra <atishp@rivosinc.com>
> > ---
> >  arch/riscv/include/asm/page.h   |  8 ++++++++
> >  arch/riscv/kernel/head.S        | 10 +++-------
> >  arch/riscv/kernel/vmlinux.lds.S |  5 ++---
> >  arch/riscv/mm/init.c            | 11 ++++++++---
> >  4 files changed, 21 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> > index b3e5ff0125fe..299147c78b4a 100644
> > --- a/arch/riscv/include/asm/page.h
> > +++ b/arch/riscv/include/asm/page.h
> > @@ -16,6 +16,14 @@
> >  #define PAGE_SIZE      (_AC(1, UL) << PAGE_SHIFT)
> >  #define PAGE_MASK      (~(PAGE_SIZE - 1))
> >
> > +#if __riscv_xlen == 64
> > +/* Image load offset(2MB) from start of RAM */
> > +#define LOAD_OFFSET    0x200000
> > +#else
> > +/* Image load offset(4MB) from start of RAM */
> > +#define LOAD_OFFSET    0x400000
> > +#endif
> > +
> >  #ifdef CONFIG_64BIT
> >  #define HUGE_MAX_HSTATE                2
> >  #else
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index f52f01ecbeea..a6ac892d2ccf 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -61,13 +61,7 @@ ENTRY(_start)
> >         /* Image load offset (0MB) from start of RAM for M-mode */
> >         .dword 0
> >  #else
> > -#if __riscv_xlen == 64
> > -       /* Image load offset(2MB) from start of RAM */
> > -       .dword 0x200000
> > -#else
> > -       /* Image load offset(4MB) from start of RAM */
> > -       .dword 0x400000
> > -#endif
> > +       .dword LOAD_OFFSET
> >  #endif
> >         /* Effective size of kernel image */
> >         .dword _end - _start
> > @@ -94,6 +88,8 @@ relocate:
> >         la a1, kernel_map
> >         XIP_FIXUP_OFFSET a1
> >         REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
> > +       li a2, LOAD_OFFSET
> > +       add a1, a1, a2
> >         la a2, _start
> >         sub a1, a1, a2
> >         add ra, ra, a1
> > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > index 5104f3a871e3..75b7c72cd4bd 100644
> > --- a/arch/riscv/kernel/vmlinux.lds.S
> > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > @@ -11,10 +11,9 @@
> >  #else
> >
> >  #include <asm/pgtable.h>
> > -#define LOAD_OFFSET KERNEL_LINK_ADDR
> >
> > -#include <asm/vmlinux.lds.h>
> >  #include <asm/page.h>
> > +#include <asm/vmlinux.lds.h>
> >  #include <asm/cache.h>
> >  #include <asm/thread_info.h>
> >  #include <asm/set_memory.h>
> > @@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
> >  SECTIONS
> >  {
> >         /* Beginning of code and text segment */
> > -       . = LOAD_OFFSET;
> > +       . = LOAD_OFFSET + KERNEL_LINK_ADDR;
> >         _start = .;
> >         HEAD_TEXT_SECTION
> >         . = ALIGN(PAGE_SIZE);
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 24b2b8044602..920e78f8c3e4 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
> >         if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
> >                 memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> >
> > +       /*
> > +        * Reserve OpenSBI region and depends on PMP to deny accesses.
> > +        */
> > +       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
> > +
> >         early_init_fdt_scan_reserved_mem();
> >         dma_contiguous_reserve(dma32_phys_limit);
> >         if (IS_ENABLED(CONFIG_64BIT))
> > @@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >
> >         kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
> >  #else
> > -       kernel_map.phys_addr = (uintptr_t)(&_start);
> > +       kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
> >         kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
> >  #endif
> >         kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
> > @@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >         create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> >                            kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
> >  #else
> > -       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> > -                          kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
> > +       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
> > +                          kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
> >  #endif
> >  #else
> >         /* Setup trampoline PGD */
> > --
> > 2.25.1
> >

Re: [RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[Alexandre Ghiti]

On Tue, Nov 23, 2021 at 4:56 AM Anup Patel <anup@brainfault.org> wrote:
>
> +Alex
>
> On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
> >
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > The current RISC-V's mm layout is based on a 2MB offset and wasting
> > memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
> > Then we could reduce the memory reserved for opensbi in the next
> > patch.
>
> The real problem is that the generic kernel marks memory before
> __pa(PAGE_OFFSET) as reserved which is evident from the boot
> print "OF: fdt: Ignoring memory range 0x80000000 - 0x80200000".

Agreed, memblock 'rejects' this region because MIN_MEMBLOCK_ADDR is
defined as __pa(PAGE_OFFSET) which points to 0x80200000. I have a
patch to enable the use of hugepages for the linear mapping [1] that
does just that, things are not that easy since then it breaks initrd
initialization which is an early caller of __va, I updated this
patchset a few months ago, I can push that soon @Guo Ren.

[1] https://lkml.org/lkml/2020/6/3/696



>
> One simple way to re-claim the first 2MB of memory is by:
> 1) Not placing OpenSBI firmware at start of RAM and rather
> place it towards end/middle or RAM away from kernel and initrd
> 2) Load kernel at start of the RAM
>
> The point#1 is already supported by OpenSBI firmwares using
> position independent compilation. In fact, U-Boot SPL does
> not load OpenSBI firmware at the start of RAM.
>
> I would suggest Allwinner D1 to follow U-Boot SPL and have
> the booting stage before OpenSBI to load OpenSBI firmware
> somewhere else.
>
> Regards,
> Anup
>
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Anup Patel <anup.patel@wdc.com>
> > Cc: Atish Patra <atishp@rivosinc.com>
> > ---
> >  arch/riscv/include/asm/page.h   |  8 ++++++++
> >  arch/riscv/kernel/head.S        | 10 +++-------
> >  arch/riscv/kernel/vmlinux.lds.S |  5 ++---
> >  arch/riscv/mm/init.c            | 11 ++++++++---
> >  4 files changed, 21 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> > index b3e5ff0125fe..299147c78b4a 100644
> > --- a/arch/riscv/include/asm/page.h
> > +++ b/arch/riscv/include/asm/page.h
> > @@ -16,6 +16,14 @@
> >  #define PAGE_SIZE      (_AC(1, UL) << PAGE_SHIFT)
> >  #define PAGE_MASK      (~(PAGE_SIZE - 1))
> >
> > +#if __riscv_xlen == 64
> > +/* Image load offset(2MB) from start of RAM */
> > +#define LOAD_OFFSET    0x200000
> > +#else
> > +/* Image load offset(4MB) from start of RAM */
> > +#define LOAD_OFFSET    0x400000
> > +#endif
> > +
> >  #ifdef CONFIG_64BIT
> >  #define HUGE_MAX_HSTATE                2
> >  #else
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index f52f01ecbeea..a6ac892d2ccf 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -61,13 +61,7 @@ ENTRY(_start)
> >         /* Image load offset (0MB) from start of RAM for M-mode */
> >         .dword 0
> >  #else
> > -#if __riscv_xlen == 64
> > -       /* Image load offset(2MB) from start of RAM */
> > -       .dword 0x200000
> > -#else
> > -       /* Image load offset(4MB) from start of RAM */
> > -       .dword 0x400000
> > -#endif
> > +       .dword LOAD_OFFSET
> >  #endif
> >         /* Effective size of kernel image */
> >         .dword _end - _start
> > @@ -94,6 +88,8 @@ relocate:
> >         la a1, kernel_map
> >         XIP_FIXUP_OFFSET a1
> >         REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
> > +       li a2, LOAD_OFFSET
> > +       add a1, a1, a2
> >         la a2, _start
> >         sub a1, a1, a2
> >         add ra, ra, a1
> > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > index 5104f3a871e3..75b7c72cd4bd 100644
> > --- a/arch/riscv/kernel/vmlinux.lds.S
> > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > @@ -11,10 +11,9 @@
> >  #else
> >
> >  #include <asm/pgtable.h>
> > -#define LOAD_OFFSET KERNEL_LINK_ADDR
> >
> > -#include <asm/vmlinux.lds.h>
> >  #include <asm/page.h>
> > +#include <asm/vmlinux.lds.h>
> >  #include <asm/cache.h>
> >  #include <asm/thread_info.h>
> >  #include <asm/set_memory.h>
> > @@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
> >  SECTIONS
> >  {
> >         /* Beginning of code and text segment */
> > -       . = LOAD_OFFSET;
> > +       . = LOAD_OFFSET + KERNEL_LINK_ADDR;
> >         _start = .;
> >         HEAD_TEXT_SECTION
> >         . = ALIGN(PAGE_SIZE);
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 24b2b8044602..920e78f8c3e4 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
> >         if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
> >                 memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> >
> > +       /*
> > +        * Reserve OpenSBI region and depends on PMP to deny accesses.
> > +        */
> > +       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
> > +
> >         early_init_fdt_scan_reserved_mem();
> >         dma_contiguous_reserve(dma32_phys_limit);
> >         if (IS_ENABLED(CONFIG_64BIT))
> > @@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >
> >         kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
> >  #else
> > -       kernel_map.phys_addr = (uintptr_t)(&_start);
> > +       kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
> >         kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
> >  #endif
> >         kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
> > @@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >         create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> >                            kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
> >  #else
> > -       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> > -                          kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
> > +       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
> > +                          kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
> >  #endif
> >  #else
> >         /* Setup trampoline PGD */
> > --
> > 2.25.1
> >

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

Re: [RFC PATCH 2/3] riscv: Add early_param to decrease firmware region

[Alexandre Ghiti]

On Tue, Nov 23, 2021 at 12:53 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 23 Nov 2021, at 03:44, Anup Patel <anup@brainfault.org> wrote:
> >
> > +Alex
> >
> > On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
> >>
> >> From: Guo Ren <guoren@linux.alibaba.com>
> >>
> >> Using riscv.fw_size in cmdline to tell the kernel what the
> >> firmware (opensbi) size is. Then reserve the proper size of
> >> firmware to save memory instead of the whole 2MB. It's helpful
> >> to satisfy a small memory system (D1s/F133 from Allwinner).
> >>
> >> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> >> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> >> Cc: Anup Patel <anup.patel@wdc.com>
> >> Cc: Atish Patra <atishp@rivosinc.com>
> >> ---
> >> arch/riscv/mm/init.c | 14 +++++++++++++-
> >> 1 file changed, 13 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> >> index 920e78f8c3e4..f7db6d40213d 100644
> >> --- a/arch/riscv/mm/init.c
> >> +++ b/arch/riscv/mm/init.c
> >> @@ -159,6 +159,15 @@ static int __init early_mem(char *p)
> >> }
> >> early_param("mem", early_mem);
> >>
> >> +static phys_addr_t firmware_size __initdata;
> >> +static int __init early_get_firmware_size(char *arg)
> >> +{
> >> +       firmware_size = memparse(arg, &arg);
> >> +
> >> +       return 0;
> >> +}
> >> +early_param("riscv.fwsz", early_get_firmware_size);
> >> +
> >
> > We have avoided any RISC-V specific kernel parameter till now
> > and I don't think adding "riscv.fwsz" is the right approach.
> >
> > OpenSBI adds a reserved memory node (mmode_resv@8000000)
> > to mark the memory where it is running as reserved. In fact, all
> > M-mode runtime firmware should be adding a reserved memory
> > node just like OpenSBI.

Yes I agree that this should be in the device tree, IMO there is no
need to introduce a new kernel parameter.

>
> BBL does not do this and, even if it’s modified today, older versions
> will still need to be supported for quite a while longer.

It's fair to expect the firmware to advertise its existence: we
briefly discussed that last year with Atish [1] and he proposed to
introduce a document that describes what the kernel expects from the
'platform' when it boots, that would be a way to drop those old legacy
bootloaders.

[1] https://lkml.org/lkml/2020/6/3/696



>
> In FreeBSD[1] we only reserve the first 2 MiB of DRAM (we don’t care
> about RV32) if there is no reserved memory node covering the DRAM base
> address, which avoids this issue. The only downside with that approach
> is that if firmware occupies a different region than the beginning of
> DRAM (or there is no firmware resident in the supervisor’s physical
> address space, as is the case for a virtualised guest) then it
> unnecessarily reserves that first 2 MiB, but that’s not a huge deal,
> and can’t be avoided so long as BBL continues to exist (well, I guess
> you could probe the SBI implementation ID if you really cared about
> that, but I’ve yet to hear of a platform where the SBI implementation,
> if it exists, isn’t at the start of DRAM, and if you’re virtualising
> then you probably have enough DRAM that you don’t notice 2 MiB going
> missing).
>
> Jess
>
> [1] https://github.com/freebsd/freebsd-src/blob/main/sys/riscv/riscv/machdep.c#L554-L568
>

Re: [RFC PATCH 2/3] riscv: Add early_param to decrease firmware region

[Alexandre Ghiti]

On Tue, Nov 23, 2021 at 12:53 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 23 Nov 2021, at 03:44, Anup Patel <anup@brainfault.org> wrote:
> >
> > +Alex
> >
> > On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
> >>
> >> From: Guo Ren <guoren@linux.alibaba.com>
> >>
> >> Using riscv.fw_size in cmdline to tell the kernel what the
> >> firmware (opensbi) size is. Then reserve the proper size of
> >> firmware to save memory instead of the whole 2MB. It's helpful
> >> to satisfy a small memory system (D1s/F133 from Allwinner).
> >>
> >> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> >> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> >> Cc: Anup Patel <anup.patel@wdc.com>
> >> Cc: Atish Patra <atishp@rivosinc.com>
> >> ---
> >> arch/riscv/mm/init.c | 14 +++++++++++++-
> >> 1 file changed, 13 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> >> index 920e78f8c3e4..f7db6d40213d 100644
> >> --- a/arch/riscv/mm/init.c
> >> +++ b/arch/riscv/mm/init.c
> >> @@ -159,6 +159,15 @@ static int __init early_mem(char *p)
> >> }
> >> early_param("mem", early_mem);
> >>
> >> +static phys_addr_t firmware_size __initdata;
> >> +static int __init early_get_firmware_size(char *arg)
> >> +{
> >> +       firmware_size = memparse(arg, &arg);
> >> +
> >> +       return 0;
> >> +}
> >> +early_param("riscv.fwsz", early_get_firmware_size);
> >> +
> >
> > We have avoided any RISC-V specific kernel parameter till now
> > and I don't think adding "riscv.fwsz" is the right approach.
> >
> > OpenSBI adds a reserved memory node (mmode_resv@8000000)
> > to mark the memory where it is running as reserved. In fact, all
> > M-mode runtime firmware should be adding a reserved memory
> > node just like OpenSBI.

Yes I agree that this should be in the device tree, IMO there is no
need to introduce a new kernel parameter.

>
> BBL does not do this and, even if it’s modified today, older versions
> will still need to be supported for quite a while longer.

It's fair to expect the firmware to advertise its existence: we
briefly discussed that last year with Atish [1] and he proposed to
introduce a document that describes what the kernel expects from the
'platform' when it boots, that would be a way to drop those old legacy
bootloaders.

[1] https://lkml.org/lkml/2020/6/3/696



>
> In FreeBSD[1] we only reserve the first 2 MiB of DRAM (we don’t care
> about RV32) if there is no reserved memory node covering the DRAM base
> address, which avoids this issue. The only downside with that approach
> is that if firmware occupies a different region than the beginning of
> DRAM (or there is no firmware resident in the supervisor’s physical
> address space, as is the case for a virtualised guest) then it
> unnecessarily reserves that first 2 MiB, but that’s not a huge deal,
> and can’t be avoided so long as BBL continues to exist (well, I guess
> you could probe the SBI implementation ID if you really cared about
> that, but I’ve yet to hear of a platform where the SBI implementation,
> if it exists, isn’t at the start of DRAM, and if you’re virtualising
> then you probably have enough DRAM that you don’t notice 2 MiB going
> missing).
>
> Jess
>
> [1] https://github.com/freebsd/freebsd-src/blob/main/sys/riscv/riscv/machdep.c#L554-L568
>

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

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Heiko Stübner]

Hi Guo,

Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> chance to users to set the proper size of the firmware and get
> more than 1.5MB of memory.

is this kernel parameter approach a result of the T-Head Ice-SoC
currently loading its openSBI from inside the main u-boot via extfs-load,
directly before the kernel itself [0] ?

Because that approach in general looks not ideal.

Normally you want the main u-boot already running with less privileges
so firmware like openSBI should've been already loaded before that.
Even more true when you're employing methods to protect memory regions
from less privileged access.

A lot of socs set u-boot as opensbi payload, but for the example the D1
mainline approach uses the Allwinner TOC1 image format to load both
opensbi and the main uboot into memory from its 1st stage loader.


Of course the best way would be to just mimic what a number of
arm64 and also riscv socs do and use already existing u-boot utilities.

U-Boot can create a FIT image containing both main u-boot, dtb and
firmware images that all get loaded from SPL and placed at the correct
addresses before having the SPL jump into opensbi and from there
into u-boot [1] .

And as Anup was writing, reserved-memory should then be the way
to go to tell the kernel what regions to omit.

And mainline u-boot has already the means to even take the reserved-memory
from the devicetree used by opensbi and copy it to a new devicetree,
if the second one is different.


Heiko


[0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
[1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
[2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c

> 
> Guo Ren (3):
>   riscv: Remove 2MB offset in the mm layout
>   riscv: Add early_param to decrease firmware region
>   riscv: Add riscv.fwsz kernel parameter
> 
>  .../admin-guide/kernel-parameters.txt         |  3 +++
>  arch/riscv/include/asm/page.h                 |  8 +++++++
>  arch/riscv/kernel/head.S                      | 10 +++-----
>  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
>  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
>  5 files changed, 36 insertions(+), 13 deletions(-)
> 
> 

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Heiko Stübner]

Hi Guo,

Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> chance to users to set the proper size of the firmware and get
> more than 1.5MB of memory.

is this kernel parameter approach a result of the T-Head Ice-SoC
currently loading its openSBI from inside the main u-boot via extfs-load,
directly before the kernel itself [0] ?

Because that approach in general looks not ideal.

Normally you want the main u-boot already running with less privileges
so firmware like openSBI should've been already loaded before that.
Even more true when you're employing methods to protect memory regions
from less privileged access.

A lot of socs set u-boot as opensbi payload, but for the example the D1
mainline approach uses the Allwinner TOC1 image format to load both
opensbi and the main uboot into memory from its 1st stage loader.


Of course the best way would be to just mimic what a number of
arm64 and also riscv socs do and use already existing u-boot utilities.

U-Boot can create a FIT image containing both main u-boot, dtb and
firmware images that all get loaded from SPL and placed at the correct
addresses before having the SPL jump into opensbi and from there
into u-boot [1] .

And as Anup was writing, reserved-memory should then be the way
to go to tell the kernel what regions to omit.

And mainline u-boot has already the means to even take the reserved-memory
from the devicetree used by opensbi and copy it to a new devicetree,
if the second one is different.


Heiko


[0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
[1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
[2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c

> 
> Guo Ren (3):
>   riscv: Remove 2MB offset in the mm layout
>   riscv: Add early_param to decrease firmware region
>   riscv: Add riscv.fwsz kernel parameter
> 
>  .../admin-guide/kernel-parameters.txt         |  3 +++
>  arch/riscv/include/asm/page.h                 |  8 +++++++
>  arch/riscv/kernel/head.S                      | 10 +++-----
>  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
>  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
>  5 files changed, 36 insertions(+), 13 deletions(-)
> 
> 





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

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Atish Patra]

On Tue, Nov 23, 2021 at 11:33 AM Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi Guo,
>
> Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > chance to users to set the proper size of the firmware and get
> > more than 1.5MB of memory.
>
> is this kernel parameter approach a result of the T-Head Ice-SoC
> currently loading its openSBI from inside the main u-boot via extfs-load,
> directly before the kernel itself [0] ?

Looking at the defconfig[1], it may be U-Boot SPL not U-Boot proper. I
may be looking at the wrong config though.
If U-Boot SPL is actually used, you don't even need to manually load
OpenSBI "fw_jump" binary.

As Heiko pointed, you should just follow how U-Boot SPL works on
hifive unmatched (creating the FIT image)
The standard U-Boot SPL uses with fw_dynamic which provides all the
flexibility you want.

[1] https://github.com/T-head-Semi/u-boot/blob/main/configs/ice_evb_c910_defconfig
>
> Because that approach in general looks not ideal.
>
> Normally you want the main u-boot already running with less privileges
> so firmware like openSBI should've been already loaded before that.
> Even more true when you're employing methods to protect memory regions
> from less privileged access.
>
> A lot of socs set u-boot as opensbi payload, but for the example the D1
> mainline approach uses the Allwinner TOC1 image format to load both
> opensbi and the main uboot into memory from its 1st stage loader.
>
>
> Of course the best way would be to just mimic what a number of
> arm64 and also riscv socs do and use already existing u-boot utilities.
>
> U-Boot can create a FIT image containing both main u-boot, dtb and
> firmware images that all get loaded from SPL and placed at the correct
> addresses before having the SPL jump into opensbi and from there
> into u-boot [1] .
>
> And as Anup was writing, reserved-memory should then be the way
> to go to tell the kernel what regions to omit.
>
> And mainline u-boot has already the means to even take the reserved-memory
> from the devicetree used by opensbi and copy it to a new devicetree,
> if the second one is different.
>
>
> Heiko
>
>
> [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
>
> >
> > Guo Ren (3):
> >   riscv: Remove 2MB offset in the mm layout
> >   riscv: Add early_param to decrease firmware region
> >   riscv: Add riscv.fwsz kernel parameter
> >
> >  .../admin-guide/kernel-parameters.txt         |  3 +++
> >  arch/riscv/include/asm/page.h                 |  8 +++++++
> >  arch/riscv/kernel/head.S                      | 10 +++-----
> >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> >  5 files changed, 36 insertions(+), 13 deletions(-)
> >
> >
>
>
>
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Atish Patra]

On Tue, Nov 23, 2021 at 11:33 AM Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi Guo,
>
> Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > chance to users to set the proper size of the firmware and get
> > more than 1.5MB of memory.
>
> is this kernel parameter approach a result of the T-Head Ice-SoC
> currently loading its openSBI from inside the main u-boot via extfs-load,
> directly before the kernel itself [0] ?

Looking at the defconfig[1], it may be U-Boot SPL not U-Boot proper. I
may be looking at the wrong config though.
If U-Boot SPL is actually used, you don't even need to manually load
OpenSBI "fw_jump" binary.

As Heiko pointed, you should just follow how U-Boot SPL works on
hifive unmatched (creating the FIT image)
The standard U-Boot SPL uses with fw_dynamic which provides all the
flexibility you want.

[1] https://github.com/T-head-Semi/u-boot/blob/main/configs/ice_evb_c910_defconfig
>
> Because that approach in general looks not ideal.
>
> Normally you want the main u-boot already running with less privileges
> so firmware like openSBI should've been already loaded before that.
> Even more true when you're employing methods to protect memory regions
> from less privileged access.
>
> A lot of socs set u-boot as opensbi payload, but for the example the D1
> mainline approach uses the Allwinner TOC1 image format to load both
> opensbi and the main uboot into memory from its 1st stage loader.
>
>
> Of course the best way would be to just mimic what a number of
> arm64 and also riscv socs do and use already existing u-boot utilities.
>
> U-Boot can create a FIT image containing both main u-boot, dtb and
> firmware images that all get loaded from SPL and placed at the correct
> addresses before having the SPL jump into opensbi and from there
> into u-boot [1] .
>
> And as Anup was writing, reserved-memory should then be the way
> to go to tell the kernel what regions to omit.
>
> And mainline u-boot has already the means to even take the reserved-memory
> from the devicetree used by opensbi and copy it to a new devicetree,
> if the second one is different.
>
>
> Heiko
>
>
> [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
>
> >
> > Guo Ren (3):
> >   riscv: Remove 2MB offset in the mm layout
> >   riscv: Add early_param to decrease firmware region
> >   riscv: Add riscv.fwsz kernel parameter
> >
> >  .../admin-guide/kernel-parameters.txt         |  3 +++
> >  arch/riscv/include/asm/page.h                 |  8 +++++++
> >  arch/riscv/kernel/head.S                      | 10 +++-----
> >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> >  5 files changed, 36 insertions(+), 13 deletions(-)
> >
> >
>
>
>
>
>
> _______________________________________________
> 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

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Heiko Stübner]

Am Dienstag, 23. November 2021, 21:01:29 CET schrieb Atish Patra:
> On Tue, Nov 23, 2021 at 11:33 AM Heiko Stübner <heiko@sntech.de> wrote:
> >
> > Hi Guo,
> >
> > Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > > chance to users to set the proper size of the firmware and get
> > > more than 1.5MB of memory.
> >
> > is this kernel parameter approach a result of the T-Head Ice-SoC
> > currently loading its openSBI from inside the main u-boot via extfs-load,
> > directly before the kernel itself [0] ?
> 
> Looking at the defconfig[1], it may be U-Boot SPL not U-Boot proper. I
> may be looking at the wrong config though.

It is actually uboot-spl + uboot proper (aka the standard spl loads u-boot
way) which is the reason I pointed to using the existing u-boot facilities :-)



> If U-Boot SPL is actually used, you don't even need to manually load
> OpenSBI "fw_jump" binary.
> 
> As Heiko pointed, you should just follow how U-Boot SPL works on
> hifive unmatched (creating the FIT image)
> The standard U-Boot SPL uses with fw_dynamic which provides all the
> flexibility you want.
> 
> [1] https://github.com/T-head-Semi/u-boot/blob/main/configs/ice_evb_c910_defconfig
> >
> > Because that approach in general looks not ideal.
> >
> > Normally you want the main u-boot already running with less privileges
> > so firmware like openSBI should've been already loaded before that.
> > Even more true when you're employing methods to protect memory regions
> > from less privileged access.
> >
> > A lot of socs set u-boot as opensbi payload, but for the example the D1
> > mainline approach uses the Allwinner TOC1 image format to load both
> > opensbi and the main uboot into memory from its 1st stage loader.
> >
> >
> > Of course the best way would be to just mimic what a number of
> > arm64 and also riscv socs do and use already existing u-boot utilities.
> >
> > U-Boot can create a FIT image containing both main u-boot, dtb and
> > firmware images that all get loaded from SPL and placed at the correct
> > addresses before having the SPL jump into opensbi and from there
> > into u-boot [1] .
> >
> > And as Anup was writing, reserved-memory should then be the way
> > to go to tell the kernel what regions to omit.
> >
> > And mainline u-boot has already the means to even take the reserved-memory
> > from the devicetree used by opensbi and copy it to a new devicetree,
> > if the second one is different.
> >
> >
> > Heiko
> >
> >
> > [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> > [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> > [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
> >
> > >
> > > Guo Ren (3):
> > >   riscv: Remove 2MB offset in the mm layout
> > >   riscv: Add early_param to decrease firmware region
> > >   riscv: Add riscv.fwsz kernel parameter
> > >
> > >  .../admin-guide/kernel-parameters.txt         |  3 +++
> > >  arch/riscv/include/asm/page.h                 |  8 +++++++
> > >  arch/riscv/kernel/head.S                      | 10 +++-----
> > >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> > >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> > >  5 files changed, 36 insertions(+), 13 deletions(-)
> > >
> > >
> >
> >
> >
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> 
> 
> 
> 

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Heiko Stübner]

Am Dienstag, 23. November 2021, 21:01:29 CET schrieb Atish Patra:
> On Tue, Nov 23, 2021 at 11:33 AM Heiko Stübner <heiko@sntech.de> wrote:
> >
> > Hi Guo,
> >
> > Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > > chance to users to set the proper size of the firmware and get
> > > more than 1.5MB of memory.
> >
> > is this kernel parameter approach a result of the T-Head Ice-SoC
> > currently loading its openSBI from inside the main u-boot via extfs-load,
> > directly before the kernel itself [0] ?
> 
> Looking at the defconfig[1], it may be U-Boot SPL not U-Boot proper. I
> may be looking at the wrong config though.

It is actually uboot-spl + uboot proper (aka the standard spl loads u-boot
way) which is the reason I pointed to using the existing u-boot facilities :-)



> If U-Boot SPL is actually used, you don't even need to manually load
> OpenSBI "fw_jump" binary.
> 
> As Heiko pointed, you should just follow how U-Boot SPL works on
> hifive unmatched (creating the FIT image)
> The standard U-Boot SPL uses with fw_dynamic which provides all the
> flexibility you want.
> 
> [1] https://github.com/T-head-Semi/u-boot/blob/main/configs/ice_evb_c910_defconfig
> >
> > Because that approach in general looks not ideal.
> >
> > Normally you want the main u-boot already running with less privileges
> > so firmware like openSBI should've been already loaded before that.
> > Even more true when you're employing methods to protect memory regions
> > from less privileged access.
> >
> > A lot of socs set u-boot as opensbi payload, but for the example the D1
> > mainline approach uses the Allwinner TOC1 image format to load both
> > opensbi and the main uboot into memory from its 1st stage loader.
> >
> >
> > Of course the best way would be to just mimic what a number of
> > arm64 and also riscv socs do and use already existing u-boot utilities.
> >
> > U-Boot can create a FIT image containing both main u-boot, dtb and
> > firmware images that all get loaded from SPL and placed at the correct
> > addresses before having the SPL jump into opensbi and from there
> > into u-boot [1] .
> >
> > And as Anup was writing, reserved-memory should then be the way
> > to go to tell the kernel what regions to omit.
> >
> > And mainline u-boot has already the means to even take the reserved-memory
> > from the devicetree used by opensbi and copy it to a new devicetree,
> > if the second one is different.
> >
> >
> > Heiko
> >
> >
> > [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> > [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> > [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
> >
> > >
> > > Guo Ren (3):
> > >   riscv: Remove 2MB offset in the mm layout
> > >   riscv: Add early_param to decrease firmware region
> > >   riscv: Add riscv.fwsz kernel parameter
> > >
> > >  .../admin-guide/kernel-parameters.txt         |  3 +++
> > >  arch/riscv/include/asm/page.h                 |  8 +++++++
> > >  arch/riscv/kernel/head.S                      | 10 +++-----
> > >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> > >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> > >  5 files changed, 36 insertions(+), 13 deletions(-)
> > >
> > >
> >
> >
> >
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> 
> 
> 
> 





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

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Guo Ren]

On Wed, Nov 24, 2021 at 3:33 AM Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi Guo,
>
> Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > chance to users to set the proper size of the firmware and get
> > more than 1.5MB of memory.
>
> is this kernel parameter approach a result of the T-Head Ice-SoC
> currently loading its openSBI from inside the main u-boot via extfs-load,
> directly before the kernel itself [0] ?
The patch is not related to that issue. The patch just helps users who
put opensbi at 0~2MB paddr to save memory.

>
> Because that approach in general looks not ideal.
>
> Normally you want the main u-boot already running with less privileges
> so firmware like openSBI should've been already loaded before that.
> Even more true when you're employing methods to protect memory regions
> from less privileged access.
>
> A lot of socs set u-boot as opensbi payload, but for the example the D1
> mainline approach uses the Allwinner TOC1 image format to load both
> opensbi and the main uboot into memory from its 1st stage loader.
>
>
> Of course the best way would be to just mimic what a number of
> arm64 and also riscv socs do and use already existing u-boot utilities.
>
> U-Boot can create a FIT image containing both main u-boot, dtb and
> firmware images that all get loaded from SPL and placed at the correct
> addresses before having the SPL jump into opensbi and from there
> into u-boot [1] .
>
> And as Anup was writing, reserved-memory should then be the way
> to go to tell the kernel what regions to omit.
>
> And mainline u-boot has already the means to even take the reserved-memory
> from the devicetree used by opensbi and copy it to a new devicetree,
> if the second one is different.
>
>
> Heiko
>
>
> [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
>
> >
> > Guo Ren (3):
> >   riscv: Remove 2MB offset in the mm layout
> >   riscv: Add early_param to decrease firmware region
> >   riscv: Add riscv.fwsz kernel parameter
> >
> >  .../admin-guide/kernel-parameters.txt         |  3 +++
> >  arch/riscv/include/asm/page.h                 |  8 +++++++
> >  arch/riscv/kernel/head.S                      | 10 +++-----
> >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> >  5 files changed, 36 insertions(+), 13 deletions(-)
> >
> >
>
>
>
>


-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Guo Ren]

On Wed, Nov 24, 2021 at 3:33 AM Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi Guo,
>
> Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > chance to users to set the proper size of the firmware and get
> > more than 1.5MB of memory.
>
> is this kernel parameter approach a result of the T-Head Ice-SoC
> currently loading its openSBI from inside the main u-boot via extfs-load,
> directly before the kernel itself [0] ?
The patch is not related to that issue. The patch just helps users who
put opensbi at 0~2MB paddr to save memory.

>
> Because that approach in general looks not ideal.
>
> Normally you want the main u-boot already running with less privileges
> so firmware like openSBI should've been already loaded before that.
> Even more true when you're employing methods to protect memory regions
> from less privileged access.
>
> A lot of socs set u-boot as opensbi payload, but for the example the D1
> mainline approach uses the Allwinner TOC1 image format to load both
> opensbi and the main uboot into memory from its 1st stage loader.
>
>
> Of course the best way would be to just mimic what a number of
> arm64 and also riscv socs do and use already existing u-boot utilities.
>
> U-Boot can create a FIT image containing both main u-boot, dtb and
> firmware images that all get loaded from SPL and placed at the correct
> addresses before having the SPL jump into opensbi and from there
> into u-boot [1] .
>
> And as Anup was writing, reserved-memory should then be the way
> to go to tell the kernel what regions to omit.
>
> And mainline u-boot has already the means to even take the reserved-memory
> from the devicetree used by opensbi and copy it to a new devicetree,
> if the second one is different.
>
>
> Heiko
>
>
> [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
>
> >
> > Guo Ren (3):
> >   riscv: Remove 2MB offset in the mm layout
> >   riscv: Add early_param to decrease firmware region
> >   riscv: Add riscv.fwsz kernel parameter
> >
> >  .../admin-guide/kernel-parameters.txt         |  3 +++
> >  arch/riscv/include/asm/page.h                 |  8 +++++++
> >  arch/riscv/kernel/head.S                      | 10 +++-----
> >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> >  5 files changed, 36 insertions(+), 13 deletions(-)
> >
> >
>
>
>
>


-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

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

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Guo Ren]

On Wed, Nov 24, 2021 at 4:01 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Tue, Nov 23, 2021 at 11:33 AM Heiko Stübner <heiko@sntech.de> wrote:
> >
> > Hi Guo,
> >
> > Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > > chance to users to set the proper size of the firmware and get
> > > more than 1.5MB of memory.
> >
> > is this kernel parameter approach a result of the T-Head Ice-SoC
> > currently loading its openSBI from inside the main u-boot via extfs-load,
> > directly before the kernel itself [0] ?
>
> Looking at the defconfig[1], it may be U-Boot SPL not U-Boot proper. I
> may be looking at the wrong config though.
> If U-Boot SPL is actually used, you don't even need to manually load
> OpenSBI "fw_jump" binary.
>
> As Heiko pointed, you should just follow how U-Boot SPL works on
> hifive unmatched (creating the FIT image)
> The standard U-Boot SPL uses with fw_dynamic which provides all the
> flexibility you want.
I've no right to force users' flavor of boot flow.

1) SPL -> opensbi M-mode -> u-boot S-mode -> Linux
2) SPL -> u-boot M-mode -> opensbi M-mode -> Linux

All are okay for me. I think the most straightforward reason for
people choosing 2) is that they want to try the newest OpenSBI & Linux
and 2) is more convenient for replacing.

>
> [1] https://github.com/T-head-Semi/u-boot/blob/main/configs/ice_evb_c910_defconfig
> >
> > Because that approach in general looks not ideal.
> >
> > Normally you want the main u-boot already running with less privileges
> > so firmware like openSBI should've been already loaded before that.
> > Even more true when you're employing methods to protect memory regions
> > from less privileged access.
> >
> > A lot of socs set u-boot as opensbi payload, but for the example the D1
> > mainline approach uses the Allwinner TOC1 image format to load both
> > opensbi and the main uboot into memory from its 1st stage loader.
> >
> >
> > Of course the best way would be to just mimic what a number of
> > arm64 and also riscv socs do and use already existing u-boot utilities.
> >
> > U-Boot can create a FIT image containing both main u-boot, dtb and
> > firmware images that all get loaded from SPL and placed at the correct
> > addresses before having the SPL jump into opensbi and from there
> > into u-boot [1] .
> >
> > And as Anup was writing, reserved-memory should then be the way
> > to go to tell the kernel what regions to omit.
> >
> > And mainline u-boot has already the means to even take the reserved-memory
> > from the devicetree used by opensbi and copy it to a new devicetree,
> > if the second one is different.
> >
> >
> > Heiko
> >
> >
> > [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> > [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> > [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
> >
> > >
> > > Guo Ren (3):
> > >   riscv: Remove 2MB offset in the mm layout
> > >   riscv: Add early_param to decrease firmware region
> > >   riscv: Add riscv.fwsz kernel parameter
> > >
> > >  .../admin-guide/kernel-parameters.txt         |  3 +++
> > >  arch/riscv/include/asm/page.h                 |  8 +++++++
> > >  arch/riscv/kernel/head.S                      | 10 +++-----
> > >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> > >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> > >  5 files changed, 36 insertions(+), 13 deletions(-)
> > >
> > >
> >
> >
> >
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
>
> --
> Regards,
> Atish



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Guo Ren]

On Wed, Nov 24, 2021 at 4:01 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Tue, Nov 23, 2021 at 11:33 AM Heiko Stübner <heiko@sntech.de> wrote:
> >
> > Hi Guo,
> >
> > Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > > chance to users to set the proper size of the firmware and get
> > > more than 1.5MB of memory.
> >
> > is this kernel parameter approach a result of the T-Head Ice-SoC
> > currently loading its openSBI from inside the main u-boot via extfs-load,
> > directly before the kernel itself [0] ?
>
> Looking at the defconfig[1], it may be U-Boot SPL not U-Boot proper. I
> may be looking at the wrong config though.
> If U-Boot SPL is actually used, you don't even need to manually load
> OpenSBI "fw_jump" binary.
>
> As Heiko pointed, you should just follow how U-Boot SPL works on
> hifive unmatched (creating the FIT image)
> The standard U-Boot SPL uses with fw_dynamic which provides all the
> flexibility you want.
I've no right to force users' flavor of boot flow.

1) SPL -> opensbi M-mode -> u-boot S-mode -> Linux
2) SPL -> u-boot M-mode -> opensbi M-mode -> Linux

All are okay for me. I think the most straightforward reason for
people choosing 2) is that they want to try the newest OpenSBI & Linux
and 2) is more convenient for replacing.

>
> [1] https://github.com/T-head-Semi/u-boot/blob/main/configs/ice_evb_c910_defconfig
> >
> > Because that approach in general looks not ideal.
> >
> > Normally you want the main u-boot already running with less privileges
> > so firmware like openSBI should've been already loaded before that.
> > Even more true when you're employing methods to protect memory regions
> > from less privileged access.
> >
> > A lot of socs set u-boot as opensbi payload, but for the example the D1
> > mainline approach uses the Allwinner TOC1 image format to load both
> > opensbi and the main uboot into memory from its 1st stage loader.
> >
> >
> > Of course the best way would be to just mimic what a number of
> > arm64 and also riscv socs do and use already existing u-boot utilities.
> >
> > U-Boot can create a FIT image containing both main u-boot, dtb and
> > firmware images that all get loaded from SPL and placed at the correct
> > addresses before having the SPL jump into opensbi and from there
> > into u-boot [1] .
> >
> > And as Anup was writing, reserved-memory should then be the way
> > to go to tell the kernel what regions to omit.
> >
> > And mainline u-boot has already the means to even take the reserved-memory
> > from the devicetree used by opensbi and copy it to a new devicetree,
> > if the second one is different.
> >
> >
> > Heiko
> >
> >
> > [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> > [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> > [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
> >
> > >
> > > Guo Ren (3):
> > >   riscv: Remove 2MB offset in the mm layout
> > >   riscv: Add early_param to decrease firmware region
> > >   riscv: Add riscv.fwsz kernel parameter
> > >
> > >  .../admin-guide/kernel-parameters.txt         |  3 +++
> > >  arch/riscv/include/asm/page.h                 |  8 +++++++
> > >  arch/riscv/kernel/head.S                      | 10 +++-----
> > >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> > >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> > >  5 files changed, 36 insertions(+), 13 deletions(-)
> > >
> > >
> >
> >
> >
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
>
> --
> Regards,
> Atish



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

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

Re: [RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[Guo Ren]

On Tue, Nov 23, 2021 at 9:37 PM Alexandre Ghiti
<alexandre.ghiti@canonical.com> wrote:
>
> On Tue, Nov 23, 2021 at 4:56 AM Anup Patel <anup@brainfault.org> wrote:
> >
> > +Alex
> >
> > On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
> > >
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > The current RISC-V's mm layout is based on a 2MB offset and wasting
> > > memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
> > > Then we could reduce the memory reserved for opensbi in the next
> > > patch.
> >
> > The real problem is that the generic kernel marks memory before
> > __pa(PAGE_OFFSET) as reserved which is evident from the boot
> > print "OF: fdt: Ignoring memory range 0x80000000 - 0x80200000".
>
> Agreed, memblock 'rejects' this region because MIN_MEMBLOCK_ADDR is
> defined as __pa(PAGE_OFFSET) which points to 0x80200000. I have a
> patch to enable the use of hugepages for the linear mapping [1] that
> does just that, things are not that easy since then it breaks initrd
> initialization which is an early caller of __va, I updated this
> patchset a few months ago, I can push that soon @Guo Ren.
Seems your patch makes the mapping of early_pg_dir & swapper_pg_dir different.

early_pg_dir: 0x80200000 -> 0xffffffe000000000
swapper_pg_dir: 0x80000000 -> 0xffffffe000000000

It breaks the vmlinux.ld.S, doesn't it?

>
> [1] https://lkml.org/lkml/2020/6/3/696
>
>
>
> >
> > One simple way to re-claim the first 2MB of memory is by:
> > 1) Not placing OpenSBI firmware at start of RAM and rather
> > place it towards end/middle or RAM away from kernel and initrd
> > 2) Load kernel at start of the RAM
> >
> > The point#1 is already supported by OpenSBI firmwares using
> > position independent compilation. In fact, U-Boot SPL does
> > not load OpenSBI firmware at the start of RAM.
> >
> > I would suggest Allwinner D1 to follow U-Boot SPL and have
> > the booting stage before OpenSBI to load OpenSBI firmware
> > somewhere else.
> >
> > Regards,
> > Anup
> >
> > >
> > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > Cc: Anup Patel <anup.patel@wdc.com>
> > > Cc: Atish Patra <atishp@rivosinc.com>
> > > ---
> > >  arch/riscv/include/asm/page.h   |  8 ++++++++
> > >  arch/riscv/kernel/head.S        | 10 +++-------
> > >  arch/riscv/kernel/vmlinux.lds.S |  5 ++---
> > >  arch/riscv/mm/init.c            | 11 ++++++++---
> > >  4 files changed, 21 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> > > index b3e5ff0125fe..299147c78b4a 100644
> > > --- a/arch/riscv/include/asm/page.h
> > > +++ b/arch/riscv/include/asm/page.h
> > > @@ -16,6 +16,14 @@
> > >  #define PAGE_SIZE      (_AC(1, UL) << PAGE_SHIFT)
> > >  #define PAGE_MASK      (~(PAGE_SIZE - 1))
> > >
> > > +#if __riscv_xlen == 64
> > > +/* Image load offset(2MB) from start of RAM */
> > > +#define LOAD_OFFSET    0x200000
> > > +#else
> > > +/* Image load offset(4MB) from start of RAM */
> > > +#define LOAD_OFFSET    0x400000
> > > +#endif
> > > +
> > >  #ifdef CONFIG_64BIT
> > >  #define HUGE_MAX_HSTATE                2
> > >  #else
> > > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > > index f52f01ecbeea..a6ac892d2ccf 100644
> > > --- a/arch/riscv/kernel/head.S
> > > +++ b/arch/riscv/kernel/head.S
> > > @@ -61,13 +61,7 @@ ENTRY(_start)
> > >         /* Image load offset (0MB) from start of RAM for M-mode */
> > >         .dword 0
> > >  #else
> > > -#if __riscv_xlen == 64
> > > -       /* Image load offset(2MB) from start of RAM */
> > > -       .dword 0x200000
> > > -#else
> > > -       /* Image load offset(4MB) from start of RAM */
> > > -       .dword 0x400000
> > > -#endif
> > > +       .dword LOAD_OFFSET
> > >  #endif
> > >         /* Effective size of kernel image */
> > >         .dword _end - _start
> > > @@ -94,6 +88,8 @@ relocate:
> > >         la a1, kernel_map
> > >         XIP_FIXUP_OFFSET a1
> > >         REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
> > > +       li a2, LOAD_OFFSET
> > > +       add a1, a1, a2
> > >         la a2, _start
> > >         sub a1, a1, a2
> > >         add ra, ra, a1
> > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > > index 5104f3a871e3..75b7c72cd4bd 100644
> > > --- a/arch/riscv/kernel/vmlinux.lds.S
> > > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > > @@ -11,10 +11,9 @@
> > >  #else
> > >
> > >  #include <asm/pgtable.h>
> > > -#define LOAD_OFFSET KERNEL_LINK_ADDR
> > >
> > > -#include <asm/vmlinux.lds.h>
> > >  #include <asm/page.h>
> > > +#include <asm/vmlinux.lds.h>
> > >  #include <asm/cache.h>
> > >  #include <asm/thread_info.h>
> > >  #include <asm/set_memory.h>
> > > @@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
> > >  SECTIONS
> > >  {
> > >         /* Beginning of code and text segment */
> > > -       . = LOAD_OFFSET;
> > > +       . = LOAD_OFFSET + KERNEL_LINK_ADDR;
> > >         _start = .;
> > >         HEAD_TEXT_SECTION
> > >         . = ALIGN(PAGE_SIZE);
> > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > index 24b2b8044602..920e78f8c3e4 100644
> > > --- a/arch/riscv/mm/init.c
> > > +++ b/arch/riscv/mm/init.c
> > > @@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
> > >         if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
> > >                 memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > >
> > > +       /*
> > > +        * Reserve OpenSBI region and depends on PMP to deny accesses.
> > > +        */
> > > +       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
> > > +
> > >         early_init_fdt_scan_reserved_mem();
> > >         dma_contiguous_reserve(dma32_phys_limit);
> > >         if (IS_ENABLED(CONFIG_64BIT))
> > > @@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> > >
> > >         kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
> > >  #else
> > > -       kernel_map.phys_addr = (uintptr_t)(&_start);
> > > +       kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
> > >         kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
> > >  #endif
> > >         kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
> > > @@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> > >         create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> > >                            kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
> > >  #else
> > > -       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> > > -                          kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
> > > +       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
> > > +                          kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
> > >  #endif
> > >  #else
> > >         /* Setup trampoline PGD */
> > > --
> > > 2.25.1
> > >



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Re: [RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[Guo Ren]

On Tue, Nov 23, 2021 at 9:37 PM Alexandre Ghiti
<alexandre.ghiti@canonical.com> wrote:
>
> On Tue, Nov 23, 2021 at 4:56 AM Anup Patel <anup@brainfault.org> wrote:
> >
> > +Alex
> >
> > On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
> > >
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > The current RISC-V's mm layout is based on a 2MB offset and wasting
> > > memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
> > > Then we could reduce the memory reserved for opensbi in the next
> > > patch.
> >
> > The real problem is that the generic kernel marks memory before
> > __pa(PAGE_OFFSET) as reserved which is evident from the boot
> > print "OF: fdt: Ignoring memory range 0x80000000 - 0x80200000".
>
> Agreed, memblock 'rejects' this region because MIN_MEMBLOCK_ADDR is
> defined as __pa(PAGE_OFFSET) which points to 0x80200000. I have a
> patch to enable the use of hugepages for the linear mapping [1] that
> does just that, things are not that easy since then it breaks initrd
> initialization which is an early caller of __va, I updated this
> patchset a few months ago, I can push that soon @Guo Ren.
Seems your patch makes the mapping of early_pg_dir & swapper_pg_dir different.

early_pg_dir: 0x80200000 -> 0xffffffe000000000
swapper_pg_dir: 0x80000000 -> 0xffffffe000000000

It breaks the vmlinux.ld.S, doesn't it?

>
> [1] https://lkml.org/lkml/2020/6/3/696
>
>
>
> >
> > One simple way to re-claim the first 2MB of memory is by:
> > 1) Not placing OpenSBI firmware at start of RAM and rather
> > place it towards end/middle or RAM away from kernel and initrd
> > 2) Load kernel at start of the RAM
> >
> > The point#1 is already supported by OpenSBI firmwares using
> > position independent compilation. In fact, U-Boot SPL does
> > not load OpenSBI firmware at the start of RAM.
> >
> > I would suggest Allwinner D1 to follow U-Boot SPL and have
> > the booting stage before OpenSBI to load OpenSBI firmware
> > somewhere else.
> >
> > Regards,
> > Anup
> >
> > >
> > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > Cc: Anup Patel <anup.patel@wdc.com>
> > > Cc: Atish Patra <atishp@rivosinc.com>
> > > ---
> > >  arch/riscv/include/asm/page.h   |  8 ++++++++
> > >  arch/riscv/kernel/head.S        | 10 +++-------
> > >  arch/riscv/kernel/vmlinux.lds.S |  5 ++---
> > >  arch/riscv/mm/init.c            | 11 ++++++++---
> > >  4 files changed, 21 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> > > index b3e5ff0125fe..299147c78b4a 100644
> > > --- a/arch/riscv/include/asm/page.h
> > > +++ b/arch/riscv/include/asm/page.h
> > > @@ -16,6 +16,14 @@
> > >  #define PAGE_SIZE      (_AC(1, UL) << PAGE_SHIFT)
> > >  #define PAGE_MASK      (~(PAGE_SIZE - 1))
> > >
> > > +#if __riscv_xlen == 64
> > > +/* Image load offset(2MB) from start of RAM */
> > > +#define LOAD_OFFSET    0x200000
> > > +#else
> > > +/* Image load offset(4MB) from start of RAM */
> > > +#define LOAD_OFFSET    0x400000
> > > +#endif
> > > +
> > >  #ifdef CONFIG_64BIT
> > >  #define HUGE_MAX_HSTATE                2
> > >  #else
> > > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > > index f52f01ecbeea..a6ac892d2ccf 100644
> > > --- a/arch/riscv/kernel/head.S
> > > +++ b/arch/riscv/kernel/head.S
> > > @@ -61,13 +61,7 @@ ENTRY(_start)
> > >         /* Image load offset (0MB) from start of RAM for M-mode */
> > >         .dword 0
> > >  #else
> > > -#if __riscv_xlen == 64
> > > -       /* Image load offset(2MB) from start of RAM */
> > > -       .dword 0x200000
> > > -#else
> > > -       /* Image load offset(4MB) from start of RAM */
> > > -       .dword 0x400000
> > > -#endif
> > > +       .dword LOAD_OFFSET
> > >  #endif
> > >         /* Effective size of kernel image */
> > >         .dword _end - _start
> > > @@ -94,6 +88,8 @@ relocate:
> > >         la a1, kernel_map
> > >         XIP_FIXUP_OFFSET a1
> > >         REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
> > > +       li a2, LOAD_OFFSET
> > > +       add a1, a1, a2
> > >         la a2, _start
> > >         sub a1, a1, a2
> > >         add ra, ra, a1
> > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > > index 5104f3a871e3..75b7c72cd4bd 100644
> > > --- a/arch/riscv/kernel/vmlinux.lds.S
> > > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > > @@ -11,10 +11,9 @@
> > >  #else
> > >
> > >  #include <asm/pgtable.h>
> > > -#define LOAD_OFFSET KERNEL_LINK_ADDR
> > >
> > > -#include <asm/vmlinux.lds.h>
> > >  #include <asm/page.h>
> > > +#include <asm/vmlinux.lds.h>
> > >  #include <asm/cache.h>
> > >  #include <asm/thread_info.h>
> > >  #include <asm/set_memory.h>
> > > @@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
> > >  SECTIONS
> > >  {
> > >         /* Beginning of code and text segment */
> > > -       . = LOAD_OFFSET;
> > > +       . = LOAD_OFFSET + KERNEL_LINK_ADDR;
> > >         _start = .;
> > >         HEAD_TEXT_SECTION
> > >         . = ALIGN(PAGE_SIZE);
> > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > index 24b2b8044602..920e78f8c3e4 100644
> > > --- a/arch/riscv/mm/init.c
> > > +++ b/arch/riscv/mm/init.c
> > > @@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
> > >         if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
> > >                 memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > >
> > > +       /*
> > > +        * Reserve OpenSBI region and depends on PMP to deny accesses.
> > > +        */
> > > +       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
> > > +
> > >         early_init_fdt_scan_reserved_mem();
> > >         dma_contiguous_reserve(dma32_phys_limit);
> > >         if (IS_ENABLED(CONFIG_64BIT))
> > > @@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> > >
> > >         kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
> > >  #else
> > > -       kernel_map.phys_addr = (uintptr_t)(&_start);
> > > +       kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
> > >         kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
> > >  #endif
> > >         kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
> > > @@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> > >         create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> > >                            kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
> > >  #else
> > > -       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> > > -                          kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
> > > +       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
> > > +                          kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
> > >  #endif
> > >  #else
> > >         /* Setup trampoline PGD */
> > > --
> > > 2.25.1
> > >



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

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

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Heiko Stübner]

Am Mittwoch, 24. November 2021, 07:49:26 CET schrieb Guo Ren:
> On Wed, Nov 24, 2021 at 4:01 AM Atish Patra <atishp@atishpatra.org> wrote:
> >
> > On Tue, Nov 23, 2021 at 11:33 AM Heiko Stübner <heiko@sntech.de> wrote:
> > >
> > > Hi Guo,
> > >
> > > Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > >
> > > > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > > > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > > > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > > > chance to users to set the proper size of the firmware and get
> > > > more than 1.5MB of memory.
> > >
> > > is this kernel parameter approach a result of the T-Head Ice-SoC
> > > currently loading its openSBI from inside the main u-boot via extfs-load,
> > > directly before the kernel itself [0] ?
> >
> > Looking at the defconfig[1], it may be U-Boot SPL not U-Boot proper. I
> > may be looking at the wrong config though.
> > If U-Boot SPL is actually used, you don't even need to manually load
> > OpenSBI "fw_jump" binary.
> >
> > As Heiko pointed, you should just follow how U-Boot SPL works on
> > hifive unmatched (creating the FIT image)
> > The standard U-Boot SPL uses with fw_dynamic which provides all the
> > flexibility you want.
> I've no right to force users' flavor of boot flow.
> 
> 1) SPL -> opensbi M-mode -> u-boot S-mode -> Linux
> 2) SPL -> u-boot M-mode -> opensbi M-mode -> Linux
>
> All are okay for me. I think the most straightforward reason for
> people choosing 2) is that they want to try the newest OpenSBI & Linux
> and 2) is more convenient for replacing.

Though that second option is merely a hack during development.

Having u-boot run in M-mode creates an attack surface that is a lot
bigger (with it running usb, ethernet and whatnot) compared to shedding
privileges before that.

I'd consider openSBI as part of the device firmware, so that shouldn't be
a component you replace daily. Also U-Boot for example already provides
established ways to sign and verify the parts loaded by SPL, by signing
the created FIT image this would also include the openSBI image.

So in case (1) you can add more security by simply adding the necessary
key references during u-boot build, where on the other hand if you _want_
security in case (2) you're back to hand-rolling any verification
[with less review and thus more prone to have issues]

Having the _ability_ to verify the loaded firmware components can be a
requirement in projects, so I think the default should always case (1),
to not encourage insecure implementations any more than necessary ;-) .


Heiko


> >
> > [1] https://github.com/T-head-Semi/u-boot/blob/main/configs/ice_evb_c910_defconfig
> > >
> > > Because that approach in general looks not ideal.
> > >
> > > Normally you want the main u-boot already running with less privileges
> > > so firmware like openSBI should've been already loaded before that.
> > > Even more true when you're employing methods to protect memory regions
> > > from less privileged access.
> > >
> > > A lot of socs set u-boot as opensbi payload, but for the example the D1
> > > mainline approach uses the Allwinner TOC1 image format to load both
> > > opensbi and the main uboot into memory from its 1st stage loader.
> > >
> > >
> > > Of course the best way would be to just mimic what a number of
> > > arm64 and also riscv socs do and use already existing u-boot utilities.
> > >
> > > U-Boot can create a FIT image containing both main u-boot, dtb and
> > > firmware images that all get loaded from SPL and placed at the correct
> > > addresses before having the SPL jump into opensbi and from there
> > > into u-boot [1] .
> > >
> > > And as Anup was writing, reserved-memory should then be the way
> > > to go to tell the kernel what regions to omit.
> > >
> > > And mainline u-boot has already the means to even take the reserved-memory
> > > from the devicetree used by opensbi and copy it to a new devicetree,
> > > if the second one is different.
> > >
> > >
> > > Heiko
> > >
> > >
> > > [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> > > [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> > > [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
> > >
> > > >
> > > > Guo Ren (3):
> > > >   riscv: Remove 2MB offset in the mm layout
> > > >   riscv: Add early_param to decrease firmware region
> > > >   riscv: Add riscv.fwsz kernel parameter
> > > >
> > > >  .../admin-guide/kernel-parameters.txt         |  3 +++
> > > >  arch/riscv/include/asm/page.h                 |  8 +++++++
> > > >  arch/riscv/kernel/head.S                      | 10 +++-----
> > > >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> > > >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> > > >  5 files changed, 36 insertions(+), 13 deletions(-)
> > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > 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

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Heiko Stübner]

Am Mittwoch, 24. November 2021, 07:49:26 CET schrieb Guo Ren:
> On Wed, Nov 24, 2021 at 4:01 AM Atish Patra <atishp@atishpatra.org> wrote:
> >
> > On Tue, Nov 23, 2021 at 11:33 AM Heiko Stübner <heiko@sntech.de> wrote:
> > >
> > > Hi Guo,
> > >
> > > Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > >
> > > > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > > > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > > > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > > > chance to users to set the proper size of the firmware and get
> > > > more than 1.5MB of memory.
> > >
> > > is this kernel parameter approach a result of the T-Head Ice-SoC
> > > currently loading its openSBI from inside the main u-boot via extfs-load,
> > > directly before the kernel itself [0] ?
> >
> > Looking at the defconfig[1], it may be U-Boot SPL not U-Boot proper. I
> > may be looking at the wrong config though.
> > If U-Boot SPL is actually used, you don't even need to manually load
> > OpenSBI "fw_jump" binary.
> >
> > As Heiko pointed, you should just follow how U-Boot SPL works on
> > hifive unmatched (creating the FIT image)
> > The standard U-Boot SPL uses with fw_dynamic which provides all the
> > flexibility you want.
> I've no right to force users' flavor of boot flow.
> 
> 1) SPL -> opensbi M-mode -> u-boot S-mode -> Linux
> 2) SPL -> u-boot M-mode -> opensbi M-mode -> Linux
>
> All are okay for me. I think the most straightforward reason for
> people choosing 2) is that they want to try the newest OpenSBI & Linux
> and 2) is more convenient for replacing.

Though that second option is merely a hack during development.

Having u-boot run in M-mode creates an attack surface that is a lot
bigger (with it running usb, ethernet and whatnot) compared to shedding
privileges before that.

I'd consider openSBI as part of the device firmware, so that shouldn't be
a component you replace daily. Also U-Boot for example already provides
established ways to sign and verify the parts loaded by SPL, by signing
the created FIT image this would also include the openSBI image.

So in case (1) you can add more security by simply adding the necessary
key references during u-boot build, where on the other hand if you _want_
security in case (2) you're back to hand-rolling any verification
[with less review and thus more prone to have issues]

Having the _ability_ to verify the loaded firmware components can be a
requirement in projects, so I think the default should always case (1),
to not encourage insecure implementations any more than necessary ;-) .


Heiko


> >
> > [1] https://github.com/T-head-Semi/u-boot/blob/main/configs/ice_evb_c910_defconfig
> > >
> > > Because that approach in general looks not ideal.
> > >
> > > Normally you want the main u-boot already running with less privileges
> > > so firmware like openSBI should've been already loaded before that.
> > > Even more true when you're employing methods to protect memory regions
> > > from less privileged access.
> > >
> > > A lot of socs set u-boot as opensbi payload, but for the example the D1
> > > mainline approach uses the Allwinner TOC1 image format to load both
> > > opensbi and the main uboot into memory from its 1st stage loader.
> > >
> > >
> > > Of course the best way would be to just mimic what a number of
> > > arm64 and also riscv socs do and use already existing u-boot utilities.
> > >
> > > U-Boot can create a FIT image containing both main u-boot, dtb and
> > > firmware images that all get loaded from SPL and placed at the correct
> > > addresses before having the SPL jump into opensbi and from there
> > > into u-boot [1] .
> > >
> > > And as Anup was writing, reserved-memory should then be the way
> > > to go to tell the kernel what regions to omit.
> > >
> > > And mainline u-boot has already the means to even take the reserved-memory
> > > from the devicetree used by opensbi and copy it to a new devicetree,
> > > if the second one is different.
> > >
> > >
> > > Heiko
> > >
> > >
> > > [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> > > [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> > > [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
> > >
> > > >
> > > > Guo Ren (3):
> > > >   riscv: Remove 2MB offset in the mm layout
> > > >   riscv: Add early_param to decrease firmware region
> > > >   riscv: Add riscv.fwsz kernel parameter
> > > >
> > > >  .../admin-guide/kernel-parameters.txt         |  3 +++
> > > >  arch/riscv/include/asm/page.h                 |  8 +++++++
> > > >  arch/riscv/kernel/head.S                      | 10 +++-----
> > > >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> > > >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> > > >  5 files changed, 36 insertions(+), 13 deletions(-)
> > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> >
> >
> >
> > --
> > Regards,
> > Atish
> 
> 
> 
> 

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Heiko Stübner]

Am Mittwoch, 24. November 2021, 07:42:17 CET schrieb Guo Ren:
> On Wed, Nov 24, 2021 at 3:33 AM Heiko Stübner <heiko@sntech.de> wrote:
> >
> > Hi Guo,
> >
> > Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > > chance to users to set the proper size of the firmware and get
> > > more than 1.5MB of memory.
> >
> > is this kernel parameter approach a result of the T-Head Ice-SoC
> > currently loading its openSBI from inside the main u-boot via extfs-load,
> > directly before the kernel itself [0] ?
> The patch is not related to that issue. The patch just helps users who
> put opensbi at 0~2MB paddr to save memory.

So as Anup wrote, this should just be solved by using a correct reserved memory
node in the devicetree passed to the kernel. And the firmware will know what
memory region it actually occupies ;-)


> 
> >
> > Because that approach in general looks not ideal.
> >
> > Normally you want the main u-boot already running with less privileges
> > so firmware like openSBI should've been already loaded before that.
> > Even more true when you're employing methods to protect memory regions
> > from less privileged access.
> >
> > A lot of socs set u-boot as opensbi payload, but for the example the D1
> > mainline approach uses the Allwinner TOC1 image format to load both
> > opensbi and the main uboot into memory from its 1st stage loader.
> >
> >
> > Of course the best way would be to just mimic what a number of
> > arm64 and also riscv socs do and use already existing u-boot utilities.
> >
> > U-Boot can create a FIT image containing both main u-boot, dtb and
> > firmware images that all get loaded from SPL and placed at the correct
> > addresses before having the SPL jump into opensbi and from there
> > into u-boot [1] .
> >
> > And as Anup was writing, reserved-memory should then be the way
> > to go to tell the kernel what regions to omit.
> >
> > And mainline u-boot has already the means to even take the reserved-memory
> > from the devicetree used by opensbi and copy it to a new devicetree,
> > if the second one is different.
> >
> >
> > Heiko
> >
> >
> > [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> > [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> > [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
> >
> > >
> > > Guo Ren (3):
> > >   riscv: Remove 2MB offset in the mm layout
> > >   riscv: Add early_param to decrease firmware region
> > >   riscv: Add riscv.fwsz kernel parameter
> > >
> > >  .../admin-guide/kernel-parameters.txt         |  3 +++
> > >  arch/riscv/include/asm/page.h                 |  8 +++++++
> > >  arch/riscv/kernel/head.S                      | 10 +++-----
> > >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> > >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> > >  5 files changed, 36 insertions(+), 13 deletions(-)
> > >
> > >
> >
> >
> >
> >
> 
> 
> 





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

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Heiko Stübner]

Am Mittwoch, 24. November 2021, 07:42:17 CET schrieb Guo Ren:
> On Wed, Nov 24, 2021 at 3:33 AM Heiko Stübner <heiko@sntech.de> wrote:
> >
> > Hi Guo,
> >
> > Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > > chance to users to set the proper size of the firmware and get
> > > more than 1.5MB of memory.
> >
> > is this kernel parameter approach a result of the T-Head Ice-SoC
> > currently loading its openSBI from inside the main u-boot via extfs-load,
> > directly before the kernel itself [0] ?
> The patch is not related to that issue. The patch just helps users who
> put opensbi at 0~2MB paddr to save memory.

So as Anup wrote, this should just be solved by using a correct reserved memory
node in the devicetree passed to the kernel. And the firmware will know what
memory region it actually occupies ;-)


> 
> >
> > Because that approach in general looks not ideal.
> >
> > Normally you want the main u-boot already running with less privileges
> > so firmware like openSBI should've been already loaded before that.
> > Even more true when you're employing methods to protect memory regions
> > from less privileged access.
> >
> > A lot of socs set u-boot as opensbi payload, but for the example the D1
> > mainline approach uses the Allwinner TOC1 image format to load both
> > opensbi and the main uboot into memory from its 1st stage loader.
> >
> >
> > Of course the best way would be to just mimic what a number of
> > arm64 and also riscv socs do and use already existing u-boot utilities.
> >
> > U-Boot can create a FIT image containing both main u-boot, dtb and
> > firmware images that all get loaded from SPL and placed at the correct
> > addresses before having the SPL jump into opensbi and from there
> > into u-boot [1] .
> >
> > And as Anup was writing, reserved-memory should then be the way
> > to go to tell the kernel what regions to omit.
> >
> > And mainline u-boot has already the means to even take the reserved-memory
> > from the devicetree used by opensbi and copy it to a new devicetree,
> > if the second one is different.
> >
> >
> > Heiko
> >
> >
> > [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> > [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> > [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
> >
> > >
> > > Guo Ren (3):
> > >   riscv: Remove 2MB offset in the mm layout
> > >   riscv: Add early_param to decrease firmware region
> > >   riscv: Add riscv.fwsz kernel parameter
> > >
> > >  .../admin-guide/kernel-parameters.txt         |  3 +++
> > >  arch/riscv/include/asm/page.h                 |  8 +++++++
> > >  arch/riscv/kernel/head.S                      | 10 +++-----
> > >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> > >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> > >  5 files changed, 36 insertions(+), 13 deletions(-)
> > >
> > >
> >
> >
> >
> >
> 
> 
> 

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Guo Ren]

On Wed, Nov 24, 2021 at 8:15 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Am Mittwoch, 24. November 2021, 07:49:26 CET schrieb Guo Ren:
> > On Wed, Nov 24, 2021 at 4:01 AM Atish Patra <atishp@atishpatra.org> wrote:
> > >
> > > On Tue, Nov 23, 2021 at 11:33 AM Heiko Stübner <heiko@sntech.de> wrote:
> > > >
> > > > Hi Guo,
> > > >
> > > > Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > > >
> > > > > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > > > > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > > > > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > > > > chance to users to set the proper size of the firmware and get
> > > > > more than 1.5MB of memory.
> > > >
> > > > is this kernel parameter approach a result of the T-Head Ice-SoC
> > > > currently loading its openSBI from inside the main u-boot via extfs-load,
> > > > directly before the kernel itself [0] ?
> > >
> > > Looking at the defconfig[1], it may be U-Boot SPL not U-Boot proper. I
> > > may be looking at the wrong config though.
> > > If U-Boot SPL is actually used, you don't even need to manually load
> > > OpenSBI "fw_jump" binary.
> > >
> > > As Heiko pointed, you should just follow how U-Boot SPL works on
> > > hifive unmatched (creating the FIT image)
> > > The standard U-Boot SPL uses with fw_dynamic which provides all the
> > > flexibility you want.
> > I've no right to force users' flavor of boot flow.
> >
> > 1) SPL -> opensbi M-mode -> u-boot S-mode -> Linux
> > 2) SPL -> u-boot M-mode -> opensbi M-mode -> Linux
> >
> > All are okay for me. I think the most straightforward reason for
> > people choosing 2) is that they want to try the newest OpenSBI & Linux
> > and 2) is more convenient for replacing.
>
> Though that second option is merely a hack during development.
>
> Having u-boot run in M-mode creates an attack surface that is a lot
> bigger (with it running usb, ethernet and whatnot) compared to shedding
> privileges before that.
>
> I'd consider openSBI as part of the device firmware, so that shouldn't be
> a component you replace daily. Also U-Boot for example already provides
> established ways to sign and verify the parts loaded by SPL, by signing
> the created FIT image this would also include the openSBI image.
>
> So in case (1) you can add more security by simply adding the necessary
> key references during u-boot build, where on the other hand if you _want_
> security in case (2) you're back to hand-rolling any verification
> [with less review and thus more prone to have issues]
>
> Having the _ability_ to verify the loaded firmware components can be a
> requirement in projects, so I think the default should always case (1),
> to not encourage insecure implementations any more than necessary ;-) .
I think nobody would use case (2) in production.

>
>
> Heiko
>
>
> > >
> > > [1] https://github.com/T-head-Semi/u-boot/blob/main/configs/ice_evb_c910_defconfig
> > > >
> > > > Because that approach in general looks not ideal.
> > > >
> > > > Normally you want the main u-boot already running with less privileges
> > > > so firmware like openSBI should've been already loaded before that.
> > > > Even more true when you're employing methods to protect memory regions
> > > > from less privileged access.
> > > >
> > > > A lot of socs set u-boot as opensbi payload, but for the example the D1
> > > > mainline approach uses the Allwinner TOC1 image format to load both
> > > > opensbi and the main uboot into memory from its 1st stage loader.
> > > >
> > > >
> > > > Of course the best way would be to just mimic what a number of
> > > > arm64 and also riscv socs do and use already existing u-boot utilities.
> > > >
> > > > U-Boot can create a FIT image containing both main u-boot, dtb and
> > > > firmware images that all get loaded from SPL and placed at the correct
> > > > addresses before having the SPL jump into opensbi and from there
> > > > into u-boot [1] .
> > > >
> > > > And as Anup was writing, reserved-memory should then be the way
> > > > to go to tell the kernel what regions to omit.
> > > >
> > > > And mainline u-boot has already the means to even take the reserved-memory
> > > > from the devicetree used by opensbi and copy it to a new devicetree,
> > > > if the second one is different.
> > > >
> > > >
> > > > Heiko
> > > >
> > > >
> > > > [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> > > > [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> > > > [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
> > > >
> > > > >
> > > > > Guo Ren (3):
> > > > >   riscv: Remove 2MB offset in the mm layout
> > > > >   riscv: Add early_param to decrease firmware region
> > > > >   riscv: Add riscv.fwsz kernel parameter
> > > > >
> > > > >  .../admin-guide/kernel-parameters.txt         |  3 +++
> > > > >  arch/riscv/include/asm/page.h                 |  8 +++++++
> > > > >  arch/riscv/kernel/head.S                      | 10 +++-----
> > > > >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> > > > >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> > > > >  5 files changed, 36 insertions(+), 13 deletions(-)
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > linux-riscv@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > >
> > >
> > >
> > > --
> > > Regards,
> > > Atish
> >
> >
> >
> >
>
>
>
>


--
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Re: [RFC PATCH 0/3] riscv: Add riscv.fwsz kernel parameter to save memory

[Guo Ren]

On Wed, Nov 24, 2021 at 8:15 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Am Mittwoch, 24. November 2021, 07:49:26 CET schrieb Guo Ren:
> > On Wed, Nov 24, 2021 at 4:01 AM Atish Patra <atishp@atishpatra.org> wrote:
> > >
> > > On Tue, Nov 23, 2021 at 11:33 AM Heiko Stübner <heiko@sntech.de> wrote:
> > > >
> > > > Hi Guo,
> > > >
> > > > Am Dienstag, 23. November 2021, 02:57:14 CET schrieb guoren@kernel.org:
> > > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > > >
> > > > > The firmware of riscv (such as opensbi) occupy 2MB(64bit) /
> > > > > 4MB(32bit) in Linux. It's very wasteful to small memory footprint
> > > > > soc chip such as Allwinner D1s/F133. The kernel parameter gives a
> > > > > chance to users to set the proper size of the firmware and get
> > > > > more than 1.5MB of memory.
> > > >
> > > > is this kernel parameter approach a result of the T-Head Ice-SoC
> > > > currently loading its openSBI from inside the main u-boot via extfs-load,
> > > > directly before the kernel itself [0] ?
> > >
> > > Looking at the defconfig[1], it may be U-Boot SPL not U-Boot proper. I
> > > may be looking at the wrong config though.
> > > If U-Boot SPL is actually used, you don't even need to manually load
> > > OpenSBI "fw_jump" binary.
> > >
> > > As Heiko pointed, you should just follow how U-Boot SPL works on
> > > hifive unmatched (creating the FIT image)
> > > The standard U-Boot SPL uses with fw_dynamic which provides all the
> > > flexibility you want.
> > I've no right to force users' flavor of boot flow.
> >
> > 1) SPL -> opensbi M-mode -> u-boot S-mode -> Linux
> > 2) SPL -> u-boot M-mode -> opensbi M-mode -> Linux
> >
> > All are okay for me. I think the most straightforward reason for
> > people choosing 2) is that they want to try the newest OpenSBI & Linux
> > and 2) is more convenient for replacing.
>
> Though that second option is merely a hack during development.
>
> Having u-boot run in M-mode creates an attack surface that is a lot
> bigger (with it running usb, ethernet and whatnot) compared to shedding
> privileges before that.
>
> I'd consider openSBI as part of the device firmware, so that shouldn't be
> a component you replace daily. Also U-Boot for example already provides
> established ways to sign and verify the parts loaded by SPL, by signing
> the created FIT image this would also include the openSBI image.
>
> So in case (1) you can add more security by simply adding the necessary
> key references during u-boot build, where on the other hand if you _want_
> security in case (2) you're back to hand-rolling any verification
> [with less review and thus more prone to have issues]
>
> Having the _ability_ to verify the loaded firmware components can be a
> requirement in projects, so I think the default should always case (1),
> to not encourage insecure implementations any more than necessary ;-) .
I think nobody would use case (2) in production.

>
>
> Heiko
>
>
> > >
> > > [1] https://github.com/T-head-Semi/u-boot/blob/main/configs/ice_evb_c910_defconfig
> > > >
> > > > Because that approach in general looks not ideal.
> > > >
> > > > Normally you want the main u-boot already running with less privileges
> > > > so firmware like openSBI should've been already loaded before that.
> > > > Even more true when you're employing methods to protect memory regions
> > > > from less privileged access.
> > > >
> > > > A lot of socs set u-boot as opensbi payload, but for the example the D1
> > > > mainline approach uses the Allwinner TOC1 image format to load both
> > > > opensbi and the main uboot into memory from its 1st stage loader.
> > > >
> > > >
> > > > Of course the best way would be to just mimic what a number of
> > > > arm64 and also riscv socs do and use already existing u-boot utilities.
> > > >
> > > > U-Boot can create a FIT image containing both main u-boot, dtb and
> > > > firmware images that all get loaded from SPL and placed at the correct
> > > > addresses before having the SPL jump into opensbi and from there
> > > > into u-boot [1] .
> > > >
> > > > And as Anup was writing, reserved-memory should then be the way
> > > > to go to tell the kernel what regions to omit.
> > > >
> > > > And mainline u-boot has already the means to even take the reserved-memory
> > > > from the devicetree used by opensbi and copy it to a new devicetree,
> > > > if the second one is different.
> > > >
> > > >
> > > > Heiko
> > > >
> > > >
> > > > [0] https://github.com/T-head-Semi/u-boot/blob/main/include/configs/ice-c910.h#L46
> > > > [1] see spl_invoke_opensbi() in common/spl/spl_opensbi.c
> > > > [2] see riscv_board_reserved_mem_fixup() in arch/riscv/lib/fdt_fixup.c
> > > >
> > > > >
> > > > > Guo Ren (3):
> > > > >   riscv: Remove 2MB offset in the mm layout
> > > > >   riscv: Add early_param to decrease firmware region
> > > > >   riscv: Add riscv.fwsz kernel parameter
> > > > >
> > > > >  .../admin-guide/kernel-parameters.txt         |  3 +++
> > > > >  arch/riscv/include/asm/page.h                 |  8 +++++++
> > > > >  arch/riscv/kernel/head.S                      | 10 +++-----
> > > > >  arch/riscv/kernel/vmlinux.lds.S               |  5 ++--
> > > > >  arch/riscv/mm/init.c                          | 23 ++++++++++++++++---
> > > > >  5 files changed, 36 insertions(+), 13 deletions(-)
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > linux-riscv@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > >
> > >
> > >
> > > --
> > > Regards,
> > > Atish
> >
> >
> >
> >
>
>
>
>


--
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

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

Re: [RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[Alexandre ghiti]

On 11/24/21 12:58, Guo Ren wrote:
> On Tue, Nov 23, 2021 at 9:37 PM Alexandre Ghiti
> <alexandre.ghiti@canonical.com> wrote:
>> On Tue, Nov 23, 2021 at 4:56 AM Anup Patel <anup@brainfault.org> wrote:
>>> +Alex
>>>
>>> On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
>>>> From: Guo Ren <guoren@linux.alibaba.com>
>>>>
>>>> The current RISC-V's mm layout is based on a 2MB offset and wasting
>>>> memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
>>>> Then we could reduce the memory reserved for opensbi in the next
>>>> patch.
>>> The real problem is that the generic kernel marks memory before
>>> __pa(PAGE_OFFSET) as reserved which is evident from the boot
>>> print "OF: fdt: Ignoring memory range 0x80000000 - 0x80200000".
>> Agreed, memblock 'rejects' this region because MIN_MEMBLOCK_ADDR is
>> defined as __pa(PAGE_OFFSET) which points to 0x80200000. I have a
>> patch to enable the use of hugepages for the linear mapping [1] that
>> does just that, things are not that easy since then it breaks initrd
>> initialization which is an early caller of __va, I updated this
>> patchset a few months ago, I can push that soon @Guo Ren.
> Seems your patch makes the mapping of early_pg_dir & swapper_pg_dir different.
>
> early_pg_dir: 0x80200000 -> 0xffffffe000000000
> swapper_pg_dir: 0x80000000 -> 0xffffffe000000000
>
> It breaks the vmlinux.ld.S, doesn't it?


Indeed, early_pg_dir and swapper_pg_dir have different mappings, but 
that's because when establishing the early_pg_dir mapping, the only 
piece of information we have is the load address of the kernel, which is 
0x8020_0000 (or whatever). And that breaks initrd because 
__early_init_dt_declare_initrd calls __va in between and then when 
swapper_pg_dir is used, it breaks because the mappings differ. I did not 
find any better way to do that, and IIRC arm64 has a similar issue.


>
>> [1] https://lkml.org/lkml/2020/6/3/696
>>
>>
>>
>>> One simple way to re-claim the first 2MB of memory is by:
>>> 1) Not placing OpenSBI firmware at start of RAM and rather
>>> place it towards end/middle or RAM away from kernel and initrd
>>> 2) Load kernel at start of the RAM
>>>
>>> The point#1 is already supported by OpenSBI firmwares using
>>> position independent compilation. In fact, U-Boot SPL does
>>> not load OpenSBI firmware at the start of RAM.
>>>
>>> I would suggest Allwinner D1 to follow U-Boot SPL and have
>>> the booting stage before OpenSBI to load OpenSBI firmware
>>> somewhere else.
>>>
>>> Regards,
>>> Anup
>>>
>>>> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
>>>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
>>>> Cc: Anup Patel <anup.patel@wdc.com>
>>>> Cc: Atish Patra <atishp@rivosinc.com>
>>>> ---
>>>>   arch/riscv/include/asm/page.h   |  8 ++++++++
>>>>   arch/riscv/kernel/head.S        | 10 +++-------
>>>>   arch/riscv/kernel/vmlinux.lds.S |  5 ++---
>>>>   arch/riscv/mm/init.c            | 11 ++++++++---
>>>>   4 files changed, 21 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
>>>> index b3e5ff0125fe..299147c78b4a 100644
>>>> --- a/arch/riscv/include/asm/page.h
>>>> +++ b/arch/riscv/include/asm/page.h
>>>> @@ -16,6 +16,14 @@
>>>>   #define PAGE_SIZE      (_AC(1, UL) << PAGE_SHIFT)
>>>>   #define PAGE_MASK      (~(PAGE_SIZE - 1))
>>>>
>>>> +#if __riscv_xlen == 64
>>>> +/* Image load offset(2MB) from start of RAM */
>>>> +#define LOAD_OFFSET    0x200000
>>>> +#else
>>>> +/* Image load offset(4MB) from start of RAM */
>>>> +#define LOAD_OFFSET    0x400000
>>>> +#endif
>>>> +
>>>>   #ifdef CONFIG_64BIT
>>>>   #define HUGE_MAX_HSTATE                2
>>>>   #else
>>>> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>>>> index f52f01ecbeea..a6ac892d2ccf 100644
>>>> --- a/arch/riscv/kernel/head.S
>>>> +++ b/arch/riscv/kernel/head.S
>>>> @@ -61,13 +61,7 @@ ENTRY(_start)
>>>>          /* Image load offset (0MB) from start of RAM for M-mode */
>>>>          .dword 0
>>>>   #else
>>>> -#if __riscv_xlen == 64
>>>> -       /* Image load offset(2MB) from start of RAM */
>>>> -       .dword 0x200000
>>>> -#else
>>>> -       /* Image load offset(4MB) from start of RAM */
>>>> -       .dword 0x400000
>>>> -#endif
>>>> +       .dword LOAD_OFFSET
>>>>   #endif
>>>>          /* Effective size of kernel image */
>>>>          .dword _end - _start
>>>> @@ -94,6 +88,8 @@ relocate:
>>>>          la a1, kernel_map
>>>>          XIP_FIXUP_OFFSET a1
>>>>          REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
>>>> +       li a2, LOAD_OFFSET
>>>> +       add a1, a1, a2
>>>>          la a2, _start
>>>>          sub a1, a1, a2
>>>>          add ra, ra, a1
>>>> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
>>>> index 5104f3a871e3..75b7c72cd4bd 100644
>>>> --- a/arch/riscv/kernel/vmlinux.lds.S
>>>> +++ b/arch/riscv/kernel/vmlinux.lds.S
>>>> @@ -11,10 +11,9 @@
>>>>   #else
>>>>
>>>>   #include <asm/pgtable.h>
>>>> -#define LOAD_OFFSET KERNEL_LINK_ADDR
>>>>
>>>> -#include <asm/vmlinux.lds.h>
>>>>   #include <asm/page.h>
>>>> +#include <asm/vmlinux.lds.h>
>>>>   #include <asm/cache.h>
>>>>   #include <asm/thread_info.h>
>>>>   #include <asm/set_memory.h>
>>>> @@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
>>>>   SECTIONS
>>>>   {
>>>>          /* Beginning of code and text segment */
>>>> -       . = LOAD_OFFSET;
>>>> +       . = LOAD_OFFSET + KERNEL_LINK_ADDR;
>>>>          _start = .;
>>>>          HEAD_TEXT_SECTION
>>>>          . = ALIGN(PAGE_SIZE);
>>>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>>>> index 24b2b8044602..920e78f8c3e4 100644
>>>> --- a/arch/riscv/mm/init.c
>>>> +++ b/arch/riscv/mm/init.c
>>>> @@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
>>>>          if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
>>>>                  memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
>>>>
>>>> +       /*
>>>> +        * Reserve OpenSBI region and depends on PMP to deny accesses.
>>>> +        */
>>>> +       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
>>>> +
>>>>          early_init_fdt_scan_reserved_mem();
>>>>          dma_contiguous_reserve(dma32_phys_limit);
>>>>          if (IS_ENABLED(CONFIG_64BIT))
>>>> @@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>>>>
>>>>          kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
>>>>   #else
>>>> -       kernel_map.phys_addr = (uintptr_t)(&_start);
>>>> +       kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
>>>>          kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
>>>>   #endif
>>>>          kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
>>>> @@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>>>>          create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
>>>>                             kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
>>>>   #else
>>>> -       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
>>>> -                          kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
>>>> +       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
>>>> +                          kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
>>>>   #endif
>>>>   #else
>>>>          /* Setup trampoline PGD */
>>>> --
>>>> 2.25.1
>>>>
>
>

Re: [RFC PATCH 1/3] riscv: Remove 2MB offset in the mm layout

[Alexandre ghiti]

On 11/24/21 12:58, Guo Ren wrote:
> On Tue, Nov 23, 2021 at 9:37 PM Alexandre Ghiti
> <alexandre.ghiti@canonical.com> wrote:
>> On Tue, Nov 23, 2021 at 4:56 AM Anup Patel <anup@brainfault.org> wrote:
>>> +Alex
>>>
>>> On Tue, Nov 23, 2021 at 7:27 AM <guoren@kernel.org> wrote:
>>>> From: Guo Ren <guoren@linux.alibaba.com>
>>>>
>>>> The current RISC-V's mm layout is based on a 2MB offset and wasting
>>>> memory. Remove 2MB offset and map PAGE_OFFSET at start_of_DRAM.
>>>> Then we could reduce the memory reserved for opensbi in the next
>>>> patch.
>>> The real problem is that the generic kernel marks memory before
>>> __pa(PAGE_OFFSET) as reserved which is evident from the boot
>>> print "OF: fdt: Ignoring memory range 0x80000000 - 0x80200000".
>> Agreed, memblock 'rejects' this region because MIN_MEMBLOCK_ADDR is
>> defined as __pa(PAGE_OFFSET) which points to 0x80200000. I have a
>> patch to enable the use of hugepages for the linear mapping [1] that
>> does just that, things are not that easy since then it breaks initrd
>> initialization which is an early caller of __va, I updated this
>> patchset a few months ago, I can push that soon @Guo Ren.
> Seems your patch makes the mapping of early_pg_dir & swapper_pg_dir different.
>
> early_pg_dir: 0x80200000 -> 0xffffffe000000000
> swapper_pg_dir: 0x80000000 -> 0xffffffe000000000
>
> It breaks the vmlinux.ld.S, doesn't it?


Indeed, early_pg_dir and swapper_pg_dir have different mappings, but 
that's because when establishing the early_pg_dir mapping, the only 
piece of information we have is the load address of the kernel, which is 
0x8020_0000 (or whatever). And that breaks initrd because 
__early_init_dt_declare_initrd calls __va in between and then when 
swapper_pg_dir is used, it breaks because the mappings differ. I did not 
find any better way to do that, and IIRC arm64 has a similar issue.


>
>> [1] https://lkml.org/lkml/2020/6/3/696
>>
>>
>>
>>> One simple way to re-claim the first 2MB of memory is by:
>>> 1) Not placing OpenSBI firmware at start of RAM and rather
>>> place it towards end/middle or RAM away from kernel and initrd
>>> 2) Load kernel at start of the RAM
>>>
>>> The point#1 is already supported by OpenSBI firmwares using
>>> position independent compilation. In fact, U-Boot SPL does
>>> not load OpenSBI firmware at the start of RAM.
>>>
>>> I would suggest Allwinner D1 to follow U-Boot SPL and have
>>> the booting stage before OpenSBI to load OpenSBI firmware
>>> somewhere else.
>>>
>>> Regards,
>>> Anup
>>>
>>>> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
>>>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
>>>> Cc: Anup Patel <anup.patel@wdc.com>
>>>> Cc: Atish Patra <atishp@rivosinc.com>
>>>> ---
>>>>   arch/riscv/include/asm/page.h   |  8 ++++++++
>>>>   arch/riscv/kernel/head.S        | 10 +++-------
>>>>   arch/riscv/kernel/vmlinux.lds.S |  5 ++---
>>>>   arch/riscv/mm/init.c            | 11 ++++++++---
>>>>   4 files changed, 21 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
>>>> index b3e5ff0125fe..299147c78b4a 100644
>>>> --- a/arch/riscv/include/asm/page.h
>>>> +++ b/arch/riscv/include/asm/page.h
>>>> @@ -16,6 +16,14 @@
>>>>   #define PAGE_SIZE      (_AC(1, UL) << PAGE_SHIFT)
>>>>   #define PAGE_MASK      (~(PAGE_SIZE - 1))
>>>>
>>>> +#if __riscv_xlen == 64
>>>> +/* Image load offset(2MB) from start of RAM */
>>>> +#define LOAD_OFFSET    0x200000
>>>> +#else
>>>> +/* Image load offset(4MB) from start of RAM */
>>>> +#define LOAD_OFFSET    0x400000
>>>> +#endif
>>>> +
>>>>   #ifdef CONFIG_64BIT
>>>>   #define HUGE_MAX_HSTATE                2
>>>>   #else
>>>> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>>>> index f52f01ecbeea..a6ac892d2ccf 100644
>>>> --- a/arch/riscv/kernel/head.S
>>>> +++ b/arch/riscv/kernel/head.S
>>>> @@ -61,13 +61,7 @@ ENTRY(_start)
>>>>          /* Image load offset (0MB) from start of RAM for M-mode */
>>>>          .dword 0
>>>>   #else
>>>> -#if __riscv_xlen == 64
>>>> -       /* Image load offset(2MB) from start of RAM */
>>>> -       .dword 0x200000
>>>> -#else
>>>> -       /* Image load offset(4MB) from start of RAM */
>>>> -       .dword 0x400000
>>>> -#endif
>>>> +       .dword LOAD_OFFSET
>>>>   #endif
>>>>          /* Effective size of kernel image */
>>>>          .dword _end - _start
>>>> @@ -94,6 +88,8 @@ relocate:
>>>>          la a1, kernel_map
>>>>          XIP_FIXUP_OFFSET a1
>>>>          REG_L a1, KERNEL_MAP_VIRT_ADDR(a1)
>>>> +       li a2, LOAD_OFFSET
>>>> +       add a1, a1, a2
>>>>          la a2, _start
>>>>          sub a1, a1, a2
>>>>          add ra, ra, a1
>>>> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
>>>> index 5104f3a871e3..75b7c72cd4bd 100644
>>>> --- a/arch/riscv/kernel/vmlinux.lds.S
>>>> +++ b/arch/riscv/kernel/vmlinux.lds.S
>>>> @@ -11,10 +11,9 @@
>>>>   #else
>>>>
>>>>   #include <asm/pgtable.h>
>>>> -#define LOAD_OFFSET KERNEL_LINK_ADDR
>>>>
>>>> -#include <asm/vmlinux.lds.h>
>>>>   #include <asm/page.h>
>>>> +#include <asm/vmlinux.lds.h>
>>>>   #include <asm/cache.h>
>>>>   #include <asm/thread_info.h>
>>>>   #include <asm/set_memory.h>
>>>> @@ -32,7 +31,7 @@ PECOFF_FILE_ALIGNMENT = 0x200;
>>>>   SECTIONS
>>>>   {
>>>>          /* Beginning of code and text segment */
>>>> -       . = LOAD_OFFSET;
>>>> +       . = LOAD_OFFSET + KERNEL_LINK_ADDR;
>>>>          _start = .;
>>>>          HEAD_TEXT_SECTION
>>>>          . = ALIGN(PAGE_SIZE);
>>>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>>>> index 24b2b8044602..920e78f8c3e4 100644
>>>> --- a/arch/riscv/mm/init.c
>>>> +++ b/arch/riscv/mm/init.c
>>>> @@ -221,6 +221,11 @@ static void __init setup_bootmem(void)
>>>>          if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
>>>>                  memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
>>>>
>>>> +       /*
>>>> +        * Reserve OpenSBI region and depends on PMP to deny accesses.
>>>> +        */
>>>> +       memblock_reserve(__pa(PAGE_OFFSET), LOAD_OFFSET);
>>>> +
>>>>          early_init_fdt_scan_reserved_mem();
>>>>          dma_contiguous_reserve(dma32_phys_limit);
>>>>          if (IS_ENABLED(CONFIG_64BIT))
>>>> @@ -604,7 +609,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>>>>
>>>>          kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
>>>>   #else
>>>> -       kernel_map.phys_addr = (uintptr_t)(&_start);
>>>> +       kernel_map.phys_addr = (uintptr_t)(&_start) - LOAD_OFFSET;
>>>>          kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
>>>>   #endif
>>>>          kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
>>>> @@ -645,8 +650,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>>>>          create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
>>>>                             kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
>>>>   #else
>>>> -       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
>>>> -                          kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
>>>> +       create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr + LOAD_OFFSET,
>>>> +                          kernel_map.phys_addr + LOAD_OFFSET, PMD_SIZE, PAGE_KERNEL_EXEC);
>>>>   #endif
>>>>   #else
>>>>          /* Setup trampoline PGD */
>>>> --
>>>> 2.25.1
>>>>
>
>

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