linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] remove size limit on XIP kernel
@ 2024-05-10  6:28 Nam Cao
  2024-05-10  6:28 ` [PATCH 1/7] riscv: cleanup XIP_FIXUP macro Nam Cao
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Nam Cao @ 2024-05-10  6:28 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel
  Cc: Nam Cao

Hi,

For XIP kernel, the writable data section is always at offset specified in
XIP_OFFSET, which is hard-coded to 32MB.

Unfortunately, this means the read-only section (placed before the
writable section) is restricted in size. This causes build failure if the
kernel gets too large.

This series remove the use of XIP_OFFSET one by one, then remove this
macro entirely at the end, with the goal of lifting this size restriction.

Also some cleanup and documentation along the way.

This series depends on
    https://lore.kernel.org/linux-riscv/20240508191917.2892064-1-namcao@linutronix.de/
to apply cleanly, and also depends on
    https://lore.kernel.org/linux-riscv/20240508173116.2866192-1-namcao@linutronix.de/
which fixes a boot issue.

Best regards,
Nam

Nam Cao (7):
  riscv: cleanup XIP_FIXUP macro
  riscv: replace va_kernel_pa_offset with va_kernel_data_pa_offset on
    XIP
  riscv: drop the use of XIP_OFFSET in XIP_FIXUP_OFFSET
  riscv: drop the use of XIP_OFFSET in XIP_FIXUP_FLASH_OFFSET
  riscv: drop the use of XIP_OFFSET in kernel_mapping_va_to_pa()
  riscv: drop the use of XIP_OFFSET in create_kernel_page_table()
  riscv: remove limit on the size of read-only section for XIP kernel

 arch/riscv/include/asm/page.h       | 25 ++++++++++++++++++------
 arch/riscv/include/asm/pgtable.h    | 18 +++++++----------
 arch/riscv/include/asm/xip_fixup.h  | 30 +++++++++++++++++++++++------
 arch/riscv/kernel/vmlinux-xip.lds.S |  4 ++--
 arch/riscv/mm/init.c                | 11 +++++++----
 5 files changed, 59 insertions(+), 29 deletions(-)

-- 
2.39.2


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

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

* [PATCH 1/7] riscv: cleanup XIP_FIXUP macro
  2024-05-10  6:28 [PATCH 0/7] remove size limit on XIP kernel Nam Cao
@ 2024-05-10  6:28 ` Nam Cao
  2024-05-27 12:16   ` Alexandre Ghiti
  2024-05-10  6:28 ` [PATCH 2/7] riscv: replace va_kernel_pa_offset with va_kernel_data_pa_offset on XIP Nam Cao
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Nam Cao @ 2024-05-10  6:28 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel
  Cc: Nam Cao

The XIP_FIXUP macro is used to fix addresses early during boot before MMU:
generated code "thinks" the data section is in ROM while it is actually in
RAM. So this macro correct the addresses in the data section.

This macro determines if the address needs to be fixed by checking if it is
within the range startting of ROM address up to the size of 2 * XIP_OFFSET

This means addresses within the .text section would incorrectly get fixed.
Also if the kernel size if bigger than (2 * XIP_OFFSET), some addresses
would not be fixed up.

XIP kernel can still work if the above 2 cases do not happen. But this
macro is obviously incorrect.

Rewrite this macro to only fix up addresses within the data section.

Signed-off-by: Nam Cao <namcao@linutronix.de>
---
 arch/riscv/include/asm/pgtable.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 58fd7b70b903..fbf342f4afee 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -139,11 +139,14 @@
 
 #ifdef CONFIG_XIP_KERNEL
 #define XIP_FIXUP(addr) ({							\
+	extern char _sdata[], _start[], _end[];					\
+	uintptr_t __rom_start_data = CONFIG_XIP_PHYS_ADDR			\
+				+ (uintptr_t)&_sdata - (uintptr_t)&_start;	\
+	uintptr_t __rom_end_data = CONFIG_XIP_PHYS_ADDR				\
+				+ (uintptr_t)&_end - (uintptr_t)&_start;	\
 	uintptr_t __a = (uintptr_t)(addr);					\
-	(__a >= CONFIG_XIP_PHYS_ADDR && \
-	 __a < CONFIG_XIP_PHYS_ADDR + XIP_OFFSET * 2) ?	\
-		__a - CONFIG_XIP_PHYS_ADDR + CONFIG_PHYS_RAM_BASE - XIP_OFFSET :\
-		__a;								\
+	(__a >= __rom_start_data && __a < __rom_end_data) ?			\
+		__a - __rom_start_data + CONFIG_PHYS_RAM_BASE :	__a;		\
 	})
 #else
 #define XIP_FIXUP(addr)		(addr)
-- 
2.39.2


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

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

* [PATCH 2/7] riscv: replace va_kernel_pa_offset with va_kernel_data_pa_offset on XIP
  2024-05-10  6:28 [PATCH 0/7] remove size limit on XIP kernel Nam Cao
  2024-05-10  6:28 ` [PATCH 1/7] riscv: cleanup XIP_FIXUP macro Nam Cao
@ 2024-05-10  6:28 ` Nam Cao
  2024-05-27 12:32   ` Alexandre Ghiti
  2024-05-10  6:28 ` [PATCH 3/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_OFFSET Nam Cao
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Nam Cao @ 2024-05-10  6:28 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel
  Cc: Nam Cao

On XIP kernel, the name "va_kernel_pa_offset" is misleading: unlike
"normal" kernel, it is not the virtual-physical address offset of kernel
mapping, it is the offset of kernel mapping's first virtual address to
first physical address in DRAM, which is not meaningful because the
kernel's first physical address is not in DRAM.

For XIP kernel, there are 2 different offsets because the read-only part of
the kernel resides in ROM while the rest is in RAM. The offset to ROM is in
kernel_map.va_kernel_xip_pa_offset, while the offset to RAM is not stored
anywhere: it is calculated on-the-fly.

Remove this confusing "va_kernel_pa_offset" and add
"va_kernel_data_pa_offset" as its replacement. This new variable is the
offset of virtual mapping of the kernel's data portion to the corresponding
physical addresses.

Signed-off-by: Nam Cao <namcao@linutronix.de>
---
 arch/riscv/include/asm/page.h | 25 +++++++++++++++++++------
 arch/riscv/mm/init.c          |  4 +++-
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index 115ac98b8d72..14d0de928f9b 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -112,11 +112,13 @@ struct kernel_mapping {
 	/* Offset between linear mapping virtual address and kernel load address */
 	unsigned long va_pa_offset;
 	/* Offset between kernel mapping virtual address and kernel load address */
-	unsigned long va_kernel_pa_offset;
-	unsigned long va_kernel_xip_pa_offset;
 #ifdef CONFIG_XIP_KERNEL
+	unsigned long va_kernel_xip_pa_offset;
+	unsigned long va_kernel_data_pa_offset;
 	uintptr_t xiprom;
 	uintptr_t xiprom_sz;
+#else
+	unsigned long va_kernel_pa_offset;
 #endif
 };
 
@@ -134,12 +136,18 @@ extern phys_addr_t phys_ram_base;
 #else
 void *linear_mapping_pa_to_va(unsigned long x);
 #endif
+
+#ifdef CONFIG_XIP_KERNEL
 #define kernel_mapping_pa_to_va(y)	({					\
 	unsigned long _y = (unsigned long)(y);					\
-	(IS_ENABLED(CONFIG_XIP_KERNEL) && _y < phys_ram_base) ?			\
+	(_y < phys_ram_base) ?							\
 		(void *)(_y + kernel_map.va_kernel_xip_pa_offset) :		\
-		(void *)(_y + kernel_map.va_kernel_pa_offset + XIP_OFFSET);	\
+		(void *)(_y + kernel_map.va_kernel_data_pa_offset);		\
 	})
+#else
+#define kernel_mapping_pa_to_va(y) (void *)((unsigned long)(y) + kernel_map.va_kernel_pa_offset)
+#endif
+
 #define __pa_to_va_nodebug(x)		linear_mapping_pa_to_va(x)
 
 #ifndef CONFIG_DEBUG_VIRTUAL
@@ -147,12 +155,17 @@ void *linear_mapping_pa_to_va(unsigned long x);
 #else
 phys_addr_t linear_mapping_va_to_pa(unsigned long x);
 #endif
+
+#ifdef CONFIG_XIP_KERNEL
 #define kernel_mapping_va_to_pa(y) ({						\
 	unsigned long _y = (unsigned long)(y);					\
-	(IS_ENABLED(CONFIG_XIP_KERNEL) && _y < kernel_map.virt_addr + XIP_OFFSET) ? \
+	(_y < kernel_map.virt_addr + XIP_OFFSET) ?				\
 		(_y - kernel_map.va_kernel_xip_pa_offset) :			\
-		(_y - kernel_map.va_kernel_pa_offset - XIP_OFFSET);		\
+		(_y - kernel_map.va_kernel_data_pa_offset);			\
 	})
+#else
+#define kernel_mapping_va_to_pa(y) ((unsigned long)(y) - kernel_map.va_kernel_pa_offset)
+#endif
 
 #define __va_to_pa_nodebug(x)	({						\
 	unsigned long _x = x;							\
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 5e3ec076ab95..9846c6924509 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -1089,10 +1089,13 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 	kernel_map.size = (uintptr_t)(&_end) - (uintptr_t)(&_start);
 
 	kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
+	kernel_map.va_kernel_data_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr
+						+ (uintptr_t)&_sdata - (uintptr_t)&_start;
 #else
 	kernel_map.page_offset = _AC(CONFIG_PAGE_OFFSET, UL);
 	kernel_map.phys_addr = (uintptr_t)(&_start);
 	kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
+	kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;
 #endif
 
 #if defined(CONFIG_64BIT) && !defined(CONFIG_XIP_KERNEL)
@@ -1114,7 +1117,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 	 */
 	kernel_map.va_pa_offset = IS_ENABLED(CONFIG_64BIT) ?
 				0UL : PAGE_OFFSET - kernel_map.phys_addr;
-	kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;
 
 	/*
 	 * The default maximal physical memory size is KERN_VIRT_SIZE for 32-bit
-- 
2.39.2


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

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

* [PATCH 3/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_OFFSET
  2024-05-10  6:28 [PATCH 0/7] remove size limit on XIP kernel Nam Cao
  2024-05-10  6:28 ` [PATCH 1/7] riscv: cleanup XIP_FIXUP macro Nam Cao
  2024-05-10  6:28 ` [PATCH 2/7] riscv: replace va_kernel_pa_offset with va_kernel_data_pa_offset on XIP Nam Cao
@ 2024-05-10  6:28 ` Nam Cao
  2024-05-27 12:37   ` Alexandre Ghiti
  2024-05-10  6:28 ` [PATCH 4/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_FLASH_OFFSET Nam Cao
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Nam Cao @ 2024-05-10  6:28 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel
  Cc: Nam Cao

XIP_OFFSET is the hard-coded offset of writable data section within the
kernel.

By hard-coding this value, the read-only section of the kernel (which is
placed before the writable data section) is restricted in size.

As a preparation to remove this hard-coded macro XIP_OFFSET entirely, stop
using XIP_OFFSET in XIP_FIXUP_OFFSET. Instead, use CONFIG_PHYS_RAM_BASE and
_sdata to do the same thing.

While at it, also add a description for XIP_FIXUP_OFFSET.

Signed-off-by: Nam Cao <namcao@linutronix.de>
---
 arch/riscv/include/asm/xip_fixup.h | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/include/asm/xip_fixup.h b/arch/riscv/include/asm/xip_fixup.h
index b65bf6306f69..9ed2cfae09e0 100644
--- a/arch/riscv/include/asm/xip_fixup.h
+++ b/arch/riscv/include/asm/xip_fixup.h
@@ -9,8 +9,19 @@
 
 #ifdef CONFIG_XIP_KERNEL
 .macro XIP_FIXUP_OFFSET reg
-        REG_L t0, _xip_fixup
+	/* Fix-up address in Flash into address in RAM early during boot before
+	 * MMU is up. Because generated code "thinks" data is in Flash, but it
+	 * is actually in RAM (actually data is also in Flash, but Flash is
+	 * read-only, thus we need to use the data residing in RAM).
+	 *
+	 * The start of data in Flash is _sdata and the start of data in RAM is
+	 * CONFIG_PHYS_RAM_BASE. So this fix-up essentially does this:
+	 * reg += CONFIG_PHYS_RAM_BASE - _start
+	 */
+	li t0, CONFIG_PHYS_RAM_BASE
         add \reg, \reg, t0
+	la t0, _sdata
+	sub \reg, \reg, t0
 .endm
 .macro XIP_FIXUP_FLASH_OFFSET reg
 	la t0, __data_loc
@@ -19,7 +30,6 @@
 	add \reg, \reg, t0
 .endm
 
-_xip_fixup: .dword CONFIG_PHYS_RAM_BASE - CONFIG_XIP_PHYS_ADDR - XIP_OFFSET
 _xip_phys_offset: .dword CONFIG_XIP_PHYS_ADDR + XIP_OFFSET
 #else
 .macro XIP_FIXUP_OFFSET reg
-- 
2.39.2


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

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

* [PATCH 4/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_FLASH_OFFSET
  2024-05-10  6:28 [PATCH 0/7] remove size limit on XIP kernel Nam Cao
                   ` (2 preceding siblings ...)
  2024-05-10  6:28 ` [PATCH 3/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_OFFSET Nam Cao
@ 2024-05-10  6:28 ` Nam Cao
  2024-05-27 12:47   ` Alexandre Ghiti
  2024-05-10  6:28 ` [PATCH 5/7] riscv: drop the use of XIP_OFFSET in kernel_mapping_va_to_pa() Nam Cao
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Nam Cao @ 2024-05-10  6:28 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel
  Cc: Nam Cao

XIP_OFFSET is the hard-coded offset of writable data section within the
kernel.

By hard-coding this value, the read-only section of the kernel (which is
placed before the writable data section) is restricted in size.

As a preparation to remove this hard-coded macro XIP_OFFSET entirely, stop
using XIP_OFFSET in XIP_FIXUP_FLASH_OFFSET. Instead, use __data_loc and
_sdata to do the same thing.

While at it, also add a description for XIP_FIXUP_FLASH_OFFSET.

Signed-off-by: Nam Cao <namcao@linutronix.de>
---
 arch/riscv/include/asm/xip_fixup.h | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/include/asm/xip_fixup.h b/arch/riscv/include/asm/xip_fixup.h
index 9ed2cfae09e0..f3d56299bc22 100644
--- a/arch/riscv/include/asm/xip_fixup.h
+++ b/arch/riscv/include/asm/xip_fixup.h
@@ -24,13 +24,21 @@
 	sub \reg, \reg, t0
 .endm
 .macro XIP_FIXUP_FLASH_OFFSET reg
+	/* In linker script, at the transition from read-only section to
+	 * writable section, the VMA is increased while LMA remains the same.
+	 * (See in linker script how _sdata, __data_loc and LOAD_OFFSET is
+	 * changed)
+	 *
+	 * Consequently, early during boot before MMU is up, the generated code
+	 * reads the "writable" section at wrong addresses, because VMA is used
+	 * by compiler to generate code, but the data is located in Flash using
+	 * LMA.
+	 */
+	la t0, _sdata
+	sub \reg, \reg, t0
 	la t0, __data_loc
-	REG_L t1, _xip_phys_offset
-	sub \reg, \reg, t1
 	add \reg, \reg, t0
 .endm
-
-_xip_phys_offset: .dword CONFIG_XIP_PHYS_ADDR + XIP_OFFSET
 #else
 .macro XIP_FIXUP_OFFSET reg
 .endm
-- 
2.39.2


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

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

* [PATCH 5/7] riscv: drop the use of XIP_OFFSET in kernel_mapping_va_to_pa()
  2024-05-10  6:28 [PATCH 0/7] remove size limit on XIP kernel Nam Cao
                   ` (3 preceding siblings ...)
  2024-05-10  6:28 ` [PATCH 4/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_FLASH_OFFSET Nam Cao
@ 2024-05-10  6:28 ` Nam Cao
  2024-05-27 12:49   ` Alexandre Ghiti
  2024-05-10  6:28 ` [PATCH 6/7] riscv: drop the use of XIP_OFFSET in create_kernel_page_table() Nam Cao
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Nam Cao @ 2024-05-10  6:28 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel
  Cc: Nam Cao

XIP_OFFSET is the hard-coded offset of writable data section within the
kernel.

By hard-coding this value, the read-only section of the kernel (which is
placed before the writable data section) is restricted in size.

As a preparation to remove this hard-coded macro XIP_OFFSET entirely,
remove the use of XIP_OFFSET in kernel_mapping_va_to_pa(). The macro
XIP_OFFSET is used in this case to check if the virtual address is mapped
to Flash or to RAM. The same check can be done with kernel_map.xiprom_sz.

Signed-off-by: Nam Cao <namcao@linutronix.de>
---
 arch/riscv/include/asm/page.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index 14d0de928f9b..bcd77df15835 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -159,7 +159,7 @@ phys_addr_t linear_mapping_va_to_pa(unsigned long x);
 #ifdef CONFIG_XIP_KERNEL
 #define kernel_mapping_va_to_pa(y) ({						\
 	unsigned long _y = (unsigned long)(y);					\
-	(_y < kernel_map.virt_addr + XIP_OFFSET) ?				\
+	(_y < kernel_map.virt_addr + kernel_map.xiprom_sz) ?			\
 		(_y - kernel_map.va_kernel_xip_pa_offset) :			\
 		(_y - kernel_map.va_kernel_data_pa_offset);			\
 	})
-- 
2.39.2


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

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

* [PATCH 6/7] riscv: drop the use of XIP_OFFSET in create_kernel_page_table()
  2024-05-10  6:28 [PATCH 0/7] remove size limit on XIP kernel Nam Cao
                   ` (4 preceding siblings ...)
  2024-05-10  6:28 ` [PATCH 5/7] riscv: drop the use of XIP_OFFSET in kernel_mapping_va_to_pa() Nam Cao
@ 2024-05-10  6:28 ` Nam Cao
  2024-05-27 12:53   ` Alexandre Ghiti
  2024-05-10  6:28 ` [PATCH 7/7] riscv: remove limit on the size of read-only section for XIP kernel Nam Cao
  2024-05-12 15:47 ` [PATCH 0/7] remove size limit on " Alexandre Ghiti
  7 siblings, 1 reply; 17+ messages in thread
From: Nam Cao @ 2024-05-10  6:28 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel
  Cc: Nam Cao

XIP_OFFSET is the hard-coded offset of writable data section within the
kernel.

By hard-coding this value, the read-only section of the kernel (which is
placed before the writable data section) is restricted in size.

As a preparation to remove this hard-coded value entirely, stop using
XIP_OFFSET in create_kernel_page_table(). Instead use _sdata and _start to
do the same thing.

Signed-off-by: Nam Cao <namcao@linutronix.de>
---
 arch/riscv/mm/init.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 9846c6924509..62ff4aa2be96 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -907,7 +907,7 @@ static void __init relocate_kernel(void)
 static void __init create_kernel_page_table(pgd_t *pgdir,
 					    __always_unused bool early)
 {
-	uintptr_t va, end_va;
+	uintptr_t va, start_va, end_va;
 
 	/* Map the flash resident part */
 	end_va = kernel_map.virt_addr + kernel_map.xiprom_sz;
@@ -917,10 +917,11 @@ static void __init create_kernel_page_table(pgd_t *pgdir,
 				   PMD_SIZE, PAGE_KERNEL_EXEC);
 
 	/* Map the data in RAM */
+	start_va = kernel_map.virt_addr + (uintptr_t)&_sdata - (uintptr_t)&_start;
 	end_va = kernel_map.virt_addr + kernel_map.size;
-	for (va = kernel_map.virt_addr + XIP_OFFSET; va < end_va; va += PMD_SIZE)
+	for (va = start_va; va < end_va; va += PMD_SIZE)
 		create_pgd_mapping(pgdir, va,
-				   kernel_map.phys_addr + (va - (kernel_map.virt_addr + XIP_OFFSET)),
+				   kernel_map.phys_addr + (va - start_va),
 				   PMD_SIZE, PAGE_KERNEL);
 }
 #else
-- 
2.39.2


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

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

* [PATCH 7/7] riscv: remove limit on the size of read-only section for XIP kernel
  2024-05-10  6:28 [PATCH 0/7] remove size limit on XIP kernel Nam Cao
                   ` (5 preceding siblings ...)
  2024-05-10  6:28 ` [PATCH 6/7] riscv: drop the use of XIP_OFFSET in create_kernel_page_table() Nam Cao
@ 2024-05-10  6:28 ` Nam Cao
  2024-05-27 12:58   ` Alexandre Ghiti
  2024-05-12 15:47 ` [PATCH 0/7] remove size limit on " Alexandre Ghiti
  7 siblings, 1 reply; 17+ messages in thread
From: Nam Cao @ 2024-05-10  6:28 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel
  Cc: Nam Cao, kernel test robot

XIP_OFFSET is the hard-coded offset of writable data section within the
kernel.

By hard-coding this value, the read-only section of the kernel (which is
placed before the writable data section) is restricted in size. This causes
build failures if the kernel get too big (an example is in Closes:).

Remove this limit.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202404211031.J6l2AfJk-lkp@intel.com/
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
 arch/riscv/include/asm/pgtable.h    | 7 -------
 arch/riscv/kernel/vmlinux-xip.lds.S | 4 ++--
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index fbf342f4afee..75f4a92ea5bb 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -104,13 +104,6 @@
 
 #endif
 
-#ifdef CONFIG_XIP_KERNEL
-#define XIP_OFFSET		SZ_32M
-#define XIP_OFFSET_MASK		(SZ_32M - 1)
-#else
-#define XIP_OFFSET		0
-#endif
-
 #ifndef __ASSEMBLY__
 
 #include <asm/page.h>
diff --git a/arch/riscv/kernel/vmlinux-xip.lds.S b/arch/riscv/kernel/vmlinux-xip.lds.S
index 8c3daa1b0531..01f73f2ffecc 100644
--- a/arch/riscv/kernel/vmlinux-xip.lds.S
+++ b/arch/riscv/kernel/vmlinux-xip.lds.S
@@ -65,10 +65,10 @@ SECTIONS
  * From this point, stuff is considered writable and will be copied to RAM
  */
 	__data_loc = ALIGN(PAGE_SIZE);		/* location in file */
-	. = KERNEL_LINK_ADDR + XIP_OFFSET;	/* location in memory */
+	. = ALIGN(SZ_2M);			/* location in memory */
 
 #undef LOAD_OFFSET
-#define LOAD_OFFSET (KERNEL_LINK_ADDR + XIP_OFFSET - (__data_loc & XIP_OFFSET_MASK))
+#define LOAD_OFFSET (KERNEL_LINK_ADDR + _sdata - __data_loc)
 
 	_sdata = .;			/* Start of data section */
 	_data = .;
-- 
2.39.2


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

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

* Re: [PATCH 0/7] remove size limit on XIP kernel
  2024-05-10  6:28 [PATCH 0/7] remove size limit on XIP kernel Nam Cao
                   ` (6 preceding siblings ...)
  2024-05-10  6:28 ` [PATCH 7/7] riscv: remove limit on the size of read-only section for XIP kernel Nam Cao
@ 2024-05-12 15:47 ` Alexandre Ghiti
  2024-05-13  8:19   ` Nam Cao
  7 siblings, 1 reply; 17+ messages in thread
From: Alexandre Ghiti @ 2024-05-12 15:47 UTC (permalink / raw)
  To: Nam Cao, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
	linux-kernel

Hi Nam,

On 10/05/2024 08:28, Nam Cao wrote:
> Hi,
>
> For XIP kernel, the writable data section is always at offset specified in
> XIP_OFFSET, which is hard-coded to 32MB.
>
> Unfortunately, this means the read-only section (placed before the
> writable section) is restricted in size. This causes build failure if the
> kernel gets too large.
>
> This series remove the use of XIP_OFFSET one by one, then remove this
> macro entirely at the end, with the goal of lifting this size restriction.
>
> Also some cleanup and documentation along the way.
>
> This series depends on
>      https://lore.kernel.org/linux-riscv/20240508191917.2892064-1-namcao@linutronix.de/
> to apply cleanly, and also depends on
>      https://lore.kernel.org/linux-riscv/20240508173116.2866192-1-namcao@linutronix.de/
> which fixes a boot issue.
>
> Best regards,
> Nam
>
> Nam Cao (7):
>    riscv: cleanup XIP_FIXUP macro
>    riscv: replace va_kernel_pa_offset with va_kernel_data_pa_offset on
>      XIP
>    riscv: drop the use of XIP_OFFSET in XIP_FIXUP_OFFSET
>    riscv: drop the use of XIP_OFFSET in XIP_FIXUP_FLASH_OFFSET
>    riscv: drop the use of XIP_OFFSET in kernel_mapping_va_to_pa()
>    riscv: drop the use of XIP_OFFSET in create_kernel_page_table()
>    riscv: remove limit on the size of read-only section for XIP kernel
>
>   arch/riscv/include/asm/page.h       | 25 ++++++++++++++++++------
>   arch/riscv/include/asm/pgtable.h    | 18 +++++++----------
>   arch/riscv/include/asm/xip_fixup.h  | 30 +++++++++++++++++++++++------
>   arch/riscv/kernel/vmlinux-xip.lds.S |  4 ++--
>   arch/riscv/mm/init.c                | 11 +++++++----
>   5 files changed, 59 insertions(+), 29 deletions(-)
>

XIP kernels are intended for use with flash devices so the XIP_OFFSET 
restriction actually represents the size of the flash device: IIRC this 
32MB was chosen because it would fit "most devices". I think it would be 
good to come up with a mechanism that allows to restrict the size at 
build time: a config? XIP kernels are custom kernels so the user could 
enter its flash size so that if kernel ends up being too large, it 
fails. And by default, we could a very large size to avoid kernel test 
robot build failures.

Thanks,

Alex


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

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

* Re: [PATCH 0/7] remove size limit on XIP kernel
  2024-05-12 15:47 ` [PATCH 0/7] remove size limit on " Alexandre Ghiti
@ 2024-05-13  8:19   ` Nam Cao
  0 siblings, 0 replies; 17+ messages in thread
From: Nam Cao @ 2024-05-13  8:19 UTC (permalink / raw)
  To: Alexandre Ghiti
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel

On Sun, May 12, 2024 at 05:47:24PM +0200, Alexandre Ghiti wrote:
> XIP kernels are intended for use with flash devices so the XIP_OFFSET
> restriction actually represents the size of the flash device: IIRC this 32MB
> was chosen because it would fit "most devices". I think it would be good to
> come up with a mechanism that allows to restrict the size at build time: a
> config? XIP kernels are custom kernels so the user could enter its flash
> size so that if kernel ends up being too large, it fails. And by default, we
> could a very large size to avoid kernel test robot build failures.

I'm not sure if it is beneficial to do that. Users' flash tool would
complain about the kernel not fitting in flash anyway. I think this would
only make it (a bit more) complicated to build the kernel.

Furthermore XIP_OFFSET at the moment is not the flash size, it is size of
.text (and some other read-only sections). Kernel's data sections are in
flash too, which is copied to RAM during boot.

With the current linker setting, the first 32MB of virtual memory is
occupied by .text. Then at 32MB offset, .data section starts. So if
XIP_OFFSET limit is exceeded, the kernel's size is already way more than
32MB.

Instead, this series moves .data and .text right next to each other (well,
almost, because we need PMD_SIZE alignment). So that if .text size exceed
32MB, the offset that .data resides would increase as well.

If we really want to set flash size during build time (which, again, I
doubt the benefit of), this series would still be relevant: it is not just
removing the size limit, it is also removing the fixed position of the
.data section in virtual address space (in other words, removing the
useless "hole" between .text section and .data section).

Best regards,
Nam


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

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

* Re: [PATCH 1/7] riscv: cleanup XIP_FIXUP macro
  2024-05-10  6:28 ` [PATCH 1/7] riscv: cleanup XIP_FIXUP macro Nam Cao
@ 2024-05-27 12:16   ` Alexandre Ghiti
  0 siblings, 0 replies; 17+ messages in thread
From: Alexandre Ghiti @ 2024-05-27 12:16 UTC (permalink / raw)
  To: Nam Cao, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
	linux-kernel

Hi Nam,

On 10/05/2024 08:28, Nam Cao wrote:
> The XIP_FIXUP macro is used to fix addresses early during boot before MMU:
> generated code "thinks" the data section is in ROM while it is actually in
> RAM. So this macro correct the addresses in the data section.
>
> This macro determines if the address needs to be fixed by checking if it is
> within the range startting of ROM address up to the size of 2 * XIP_OFFSET


s/startting/starting

And the sentence lacks the final dot.


>
> This means addresses within the .text section would incorrectly get fixed.


Yes, but XIP_FIXUP() should only be applied to data symbols so I believe 
this ^ is not relevant.


> Also if the kernel size if bigger than (2 * XIP_OFFSET), some addresses
> would not be fixed up.

s/the kernel size if/the kernel size is

>
> XIP kernel can still work if the above 2 cases do not happen. But this
> macro is obviously incorrect.
>
> Rewrite this macro to only fix up addresses within the data section.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
>   arch/riscv/include/asm/pgtable.h | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 58fd7b70b903..fbf342f4afee 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -139,11 +139,14 @@
>   
>   #ifdef CONFIG_XIP_KERNEL
>   #define XIP_FIXUP(addr) ({							\
> +	extern char _sdata[], _start[], _end[];					\
> +	uintptr_t __rom_start_data = CONFIG_XIP_PHYS_ADDR			\
> +				+ (uintptr_t)&_sdata - (uintptr_t)&_start;	\
> +	uintptr_t __rom_end_data = CONFIG_XIP_PHYS_ADDR				\
> +				+ (uintptr_t)&_end - (uintptr_t)&_start;	\
>   	uintptr_t __a = (uintptr_t)(addr);					\
> -	(__a >= CONFIG_XIP_PHYS_ADDR && \
> -	 __a < CONFIG_XIP_PHYS_ADDR + XIP_OFFSET * 2) ?	\
> -		__a - CONFIG_XIP_PHYS_ADDR + CONFIG_PHYS_RAM_BASE - XIP_OFFSET :\
> -		__a;								\
> +	(__a >= __rom_start_data && __a < __rom_end_data) ?			\
> +		__a - __rom_start_data + CONFIG_PHYS_RAM_BASE :	__a;		\
>   	})
>   #else
>   #define XIP_FIXUP(addr)		(addr)


Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex


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

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

* Re: [PATCH 2/7] riscv: replace va_kernel_pa_offset with va_kernel_data_pa_offset on XIP
  2024-05-10  6:28 ` [PATCH 2/7] riscv: replace va_kernel_pa_offset with va_kernel_data_pa_offset on XIP Nam Cao
@ 2024-05-27 12:32   ` Alexandre Ghiti
  0 siblings, 0 replies; 17+ messages in thread
From: Alexandre Ghiti @ 2024-05-27 12:32 UTC (permalink / raw)
  To: Nam Cao, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
	linux-kernel


On 10/05/2024 08:28, Nam Cao wrote:
> On XIP kernel, the name "va_kernel_pa_offset" is misleading: unlike
> "normal" kernel, it is not the virtual-physical address offset of kernel
> mapping, it is the offset of kernel mapping's first virtual address to
> first physical address in DRAM, which is not meaningful because the
> kernel's first physical address is not in DRAM.
>
> For XIP kernel, there are 2 different offsets because the read-only part of
> the kernel resides in ROM while the rest is in RAM. The offset to ROM is in
> kernel_map.va_kernel_xip_pa_offset, while the offset to RAM is not stored
> anywhere: it is calculated on-the-fly.
>
> Remove this confusing "va_kernel_pa_offset" and add
> "va_kernel_data_pa_offset" as its replacement. This new variable is the
> offset of virtual mapping of the kernel's data portion to the corresponding
> physical addresses.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
>   arch/riscv/include/asm/page.h | 25 +++++++++++++++++++------
>   arch/riscv/mm/init.c          |  4 +++-
>   2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> index 115ac98b8d72..14d0de928f9b 100644
> --- a/arch/riscv/include/asm/page.h
> +++ b/arch/riscv/include/asm/page.h
> @@ -112,11 +112,13 @@ struct kernel_mapping {
>   	/* Offset between linear mapping virtual address and kernel load address */
>   	unsigned long va_pa_offset;
>   	/* Offset between kernel mapping virtual address and kernel load address */
> -	unsigned long va_kernel_pa_offset;
> -	unsigned long va_kernel_xip_pa_offset;
>   #ifdef CONFIG_XIP_KERNEL
> +	unsigned long va_kernel_xip_pa_offset;
> +	unsigned long va_kernel_data_pa_offset;


I would call that new field va_kernel_xip_data_pa_offset so that we know 
at first sight it only applies to XIP_KERNEL (and maybe rename 
va_kernel_xip_pa_offset into va_kernel_xip_text_pa_offset?).


>   	uintptr_t xiprom;
>   	uintptr_t xiprom_sz;
> +#else
> +	unsigned long va_kernel_pa_offset;
>   #endif
>   };
>   
> @@ -134,12 +136,18 @@ extern phys_addr_t phys_ram_base;
>   #else
>   void *linear_mapping_pa_to_va(unsigned long x);
>   #endif
> +
> +#ifdef CONFIG_XIP_KERNEL
>   #define kernel_mapping_pa_to_va(y)	({					\
>   	unsigned long _y = (unsigned long)(y);					\
> -	(IS_ENABLED(CONFIG_XIP_KERNEL) && _y < phys_ram_base) ?			\
> +	(_y < phys_ram_base) ?							\
>   		(void *)(_y + kernel_map.va_kernel_xip_pa_offset) :		\
> -		(void *)(_y + kernel_map.va_kernel_pa_offset + XIP_OFFSET);	\
> +		(void *)(_y + kernel_map.va_kernel_data_pa_offset);		\
>   	})
> +#else
> +#define kernel_mapping_pa_to_va(y) (void *)((unsigned long)(y) + kernel_map.va_kernel_pa_offset)
> +#endif
> +
>   #define __pa_to_va_nodebug(x)		linear_mapping_pa_to_va(x)
>   
>   #ifndef CONFIG_DEBUG_VIRTUAL
> @@ -147,12 +155,17 @@ void *linear_mapping_pa_to_va(unsigned long x);
>   #else
>   phys_addr_t linear_mapping_va_to_pa(unsigned long x);
>   #endif
> +
> +#ifdef CONFIG_XIP_KERNEL
>   #define kernel_mapping_va_to_pa(y) ({						\
>   	unsigned long _y = (unsigned long)(y);					\
> -	(IS_ENABLED(CONFIG_XIP_KERNEL) && _y < kernel_map.virt_addr + XIP_OFFSET) ? \
> +	(_y < kernel_map.virt_addr + XIP_OFFSET) ?				\
>   		(_y - kernel_map.va_kernel_xip_pa_offset) :			\
> -		(_y - kernel_map.va_kernel_pa_offset - XIP_OFFSET);		\
> +		(_y - kernel_map.va_kernel_data_pa_offset);			\
>   	})
> +#else
> +#define kernel_mapping_va_to_pa(y) ((unsigned long)(y) - kernel_map.va_kernel_pa_offset)
> +#endif
>   
>   #define __va_to_pa_nodebug(x)	({						\
>   	unsigned long _x = x;							\
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 5e3ec076ab95..9846c6924509 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -1089,10 +1089,13 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>   	kernel_map.size = (uintptr_t)(&_end) - (uintptr_t)(&_start);
>   
>   	kernel_map.va_kernel_xip_pa_offset = kernel_map.virt_addr - kernel_map.xiprom;
> +	kernel_map.va_kernel_data_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr
> +						+ (uintptr_t)&_sdata - (uintptr_t)&_start;
>   #else
>   	kernel_map.page_offset = _AC(CONFIG_PAGE_OFFSET, UL);
>   	kernel_map.phys_addr = (uintptr_t)(&_start);
>   	kernel_map.size = (uintptr_t)(&_end) - kernel_map.phys_addr;
> +	kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;
>   #endif
>   
>   #if defined(CONFIG_64BIT) && !defined(CONFIG_XIP_KERNEL)
> @@ -1114,7 +1117,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>   	 */
>   	kernel_map.va_pa_offset = IS_ENABLED(CONFIG_64BIT) ?
>   				0UL : PAGE_OFFSET - kernel_map.phys_addr;
> -	kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;
>   
>   	/*
>   	 * The default maximal physical memory size is KERN_VIRT_SIZE for 32-bit


I think you missed the occurence of va_kernel_pa_offset() in 
arch/riscv/kernel/vmcore_info.c.

Although some nits above, you can add:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex


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

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

* Re: [PATCH 3/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_OFFSET
  2024-05-10  6:28 ` [PATCH 3/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_OFFSET Nam Cao
@ 2024-05-27 12:37   ` Alexandre Ghiti
  0 siblings, 0 replies; 17+ messages in thread
From: Alexandre Ghiti @ 2024-05-27 12:37 UTC (permalink / raw)
  To: Nam Cao, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
	linux-kernel


On 10/05/2024 08:28, Nam Cao wrote:
> XIP_OFFSET is the hard-coded offset of writable data section within the
> kernel.
>
> By hard-coding this value, the read-only section of the kernel (which is
> placed before the writable data section) is restricted in size.
>
> As a preparation to remove this hard-coded macro XIP_OFFSET entirely, stop
> using XIP_OFFSET in XIP_FIXUP_OFFSET. Instead, use CONFIG_PHYS_RAM_BASE and
> _sdata to do the same thing.
>
> While at it, also add a description for XIP_FIXUP_OFFSET.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
>   arch/riscv/include/asm/xip_fixup.h | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/xip_fixup.h b/arch/riscv/include/asm/xip_fixup.h
> index b65bf6306f69..9ed2cfae09e0 100644
> --- a/arch/riscv/include/asm/xip_fixup.h
> +++ b/arch/riscv/include/asm/xip_fixup.h
> @@ -9,8 +9,19 @@
>   
>   #ifdef CONFIG_XIP_KERNEL
>   .macro XIP_FIXUP_OFFSET reg
> -        REG_L t0, _xip_fixup
> +	/* Fix-up address in Flash into address in RAM early during boot before
> +	 * MMU is up. Because generated code "thinks" data is in Flash, but it
> +	 * is actually in RAM (actually data is also in Flash, but Flash is
> +	 * read-only, thus we need to use the data residing in RAM).
> +	 *
> +	 * The start of data in Flash is _sdata and the start of data in RAM is
> +	 * CONFIG_PHYS_RAM_BASE. So this fix-up essentially does this:
> +	 * reg += CONFIG_PHYS_RAM_BASE - _start
> +	 */
> +	li t0, CONFIG_PHYS_RAM_BASE
>           add \reg, \reg, t0
> +	la t0, _sdata
> +	sub \reg, \reg, t0
>   .endm
>   .macro XIP_FIXUP_FLASH_OFFSET reg
>   	la t0, __data_loc
> @@ -19,7 +30,6 @@
>   	add \reg, \reg, t0
>   .endm
>   
> -_xip_fixup: .dword CONFIG_PHYS_RAM_BASE - CONFIG_XIP_PHYS_ADDR - XIP_OFFSET
>   _xip_phys_offset: .dword CONFIG_XIP_PHYS_ADDR + XIP_OFFSET
>   #else
>   .macro XIP_FIXUP_OFFSET reg


Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex


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

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

* Re: [PATCH 4/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_FLASH_OFFSET
  2024-05-10  6:28 ` [PATCH 4/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_FLASH_OFFSET Nam Cao
@ 2024-05-27 12:47   ` Alexandre Ghiti
  0 siblings, 0 replies; 17+ messages in thread
From: Alexandre Ghiti @ 2024-05-27 12:47 UTC (permalink / raw)
  To: Nam Cao, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
	linux-kernel


On 10/05/2024 08:28, Nam Cao wrote:
> XIP_OFFSET is the hard-coded offset of writable data section within the
> kernel.
>
> By hard-coding this value, the read-only section of the kernel (which is
> placed before the writable data section) is restricted in size.
>
> As a preparation to remove this hard-coded macro XIP_OFFSET entirely, stop
> using XIP_OFFSET in XIP_FIXUP_FLASH_OFFSET. Instead, use __data_loc and
> _sdata to do the same thing.
>
> While at it, also add a description for XIP_FIXUP_FLASH_OFFSET.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
>   arch/riscv/include/asm/xip_fixup.h | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/include/asm/xip_fixup.h b/arch/riscv/include/asm/xip_fixup.h
> index 9ed2cfae09e0..f3d56299bc22 100644
> --- a/arch/riscv/include/asm/xip_fixup.h
> +++ b/arch/riscv/include/asm/xip_fixup.h
> @@ -24,13 +24,21 @@
>   	sub \reg, \reg, t0
>   .endm
>   .macro XIP_FIXUP_FLASH_OFFSET reg
> +	/* In linker script, at the transition from read-only section to
> +	 * writable section, the VMA is increased while LMA remains the same.
> +	 * (See in linker script how _sdata, __data_loc and LOAD_OFFSET is
> +	 * changed)
> +	 *
> +	 * Consequently, early during boot before MMU is up, the generated code
> +	 * reads the "writable" section at wrong addresses, because VMA is used
> +	 * by compiler to generate code, but the data is located in Flash using
> +	 * LMA.
> +	 */
> +	la t0, _sdata
> +	sub \reg, \reg, t0
>   	la t0, __data_loc
> -	REG_L t1, _xip_phys_offset
> -	sub \reg, \reg, t1
>   	add \reg, \reg, t0
>   .endm
> -
> -_xip_phys_offset: .dword CONFIG_XIP_PHYS_ADDR + XIP_OFFSET
>   #else
>   .macro XIP_FIXUP_OFFSET reg
>   .endm


Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex


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

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

* Re: [PATCH 5/7] riscv: drop the use of XIP_OFFSET in kernel_mapping_va_to_pa()
  2024-05-10  6:28 ` [PATCH 5/7] riscv: drop the use of XIP_OFFSET in kernel_mapping_va_to_pa() Nam Cao
@ 2024-05-27 12:49   ` Alexandre Ghiti
  0 siblings, 0 replies; 17+ messages in thread
From: Alexandre Ghiti @ 2024-05-27 12:49 UTC (permalink / raw)
  To: Nam Cao, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
	linux-kernel


On 10/05/2024 08:28, Nam Cao wrote:
> XIP_OFFSET is the hard-coded offset of writable data section within the
> kernel.
>
> By hard-coding this value, the read-only section of the kernel (which is
> placed before the writable data section) is restricted in size.
>
> As a preparation to remove this hard-coded macro XIP_OFFSET entirely,
> remove the use of XIP_OFFSET in kernel_mapping_va_to_pa(). The macro
> XIP_OFFSET is used in this case to check if the virtual address is mapped
> to Flash or to RAM. The same check can be done with kernel_map.xiprom_sz.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
>   arch/riscv/include/asm/page.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> index 14d0de928f9b..bcd77df15835 100644
> --- a/arch/riscv/include/asm/page.h
> +++ b/arch/riscv/include/asm/page.h
> @@ -159,7 +159,7 @@ phys_addr_t linear_mapping_va_to_pa(unsigned long x);
>   #ifdef CONFIG_XIP_KERNEL
>   #define kernel_mapping_va_to_pa(y) ({						\
>   	unsigned long _y = (unsigned long)(y);					\
> -	(_y < kernel_map.virt_addr + XIP_OFFSET) ?				\
> +	(_y < kernel_map.virt_addr + kernel_map.xiprom_sz) ?			\
>   		(_y - kernel_map.va_kernel_xip_pa_offset) :			\
>   		(_y - kernel_map.va_kernel_data_pa_offset);			\
>   	})


Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex


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

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

* Re: [PATCH 6/7] riscv: drop the use of XIP_OFFSET in create_kernel_page_table()
  2024-05-10  6:28 ` [PATCH 6/7] riscv: drop the use of XIP_OFFSET in create_kernel_page_table() Nam Cao
@ 2024-05-27 12:53   ` Alexandre Ghiti
  0 siblings, 0 replies; 17+ messages in thread
From: Alexandre Ghiti @ 2024-05-27 12:53 UTC (permalink / raw)
  To: Nam Cao, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
	linux-kernel


On 10/05/2024 08:28, Nam Cao wrote:
> XIP_OFFSET is the hard-coded offset of writable data section within the
> kernel.
>
> By hard-coding this value, the read-only section of the kernel (which is
> placed before the writable data section) is restricted in size.
>
> As a preparation to remove this hard-coded value entirely, stop using
> XIP_OFFSET in create_kernel_page_table(). Instead use _sdata and _start to
> do the same thing.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
>   arch/riscv/mm/init.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 9846c6924509..62ff4aa2be96 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -907,7 +907,7 @@ static void __init relocate_kernel(void)
>   static void __init create_kernel_page_table(pgd_t *pgdir,
>   					    __always_unused bool early)
>   {
> -	uintptr_t va, end_va;
> +	uintptr_t va, start_va, end_va;
>   
>   	/* Map the flash resident part */
>   	end_va = kernel_map.virt_addr + kernel_map.xiprom_sz;
> @@ -917,10 +917,11 @@ static void __init create_kernel_page_table(pgd_t *pgdir,
>   				   PMD_SIZE, PAGE_KERNEL_EXEC);
>   
>   	/* Map the data in RAM */
> +	start_va = kernel_map.virt_addr + (uintptr_t)&_sdata - (uintptr_t)&_start;
>   	end_va = kernel_map.virt_addr + kernel_map.size;
> -	for (va = kernel_map.virt_addr + XIP_OFFSET; va < end_va; va += PMD_SIZE)
> +	for (va = start_va; va < end_va; va += PMD_SIZE)
>   		create_pgd_mapping(pgdir, va,
> -				   kernel_map.phys_addr + (va - (kernel_map.virt_addr + XIP_OFFSET)),
> +				   kernel_map.phys_addr + (va - start_va),
>   				   PMD_SIZE, PAGE_KERNEL);
>   }
>   #else


Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex


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

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

* Re: [PATCH 7/7] riscv: remove limit on the size of read-only section for XIP kernel
  2024-05-10  6:28 ` [PATCH 7/7] riscv: remove limit on the size of read-only section for XIP kernel Nam Cao
@ 2024-05-27 12:58   ` Alexandre Ghiti
  0 siblings, 0 replies; 17+ messages in thread
From: Alexandre Ghiti @ 2024-05-27 12:58 UTC (permalink / raw)
  To: Nam Cao, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
	linux-kernel
  Cc: kernel test robot


On 10/05/2024 08:28, Nam Cao wrote:
> XIP_OFFSET is the hard-coded offset of writable data section within the
> kernel.
>
> By hard-coding this value, the read-only section of the kernel (which is
> placed before the writable data section) is restricted in size. This causes
> build failures if the kernel get too big (an example is in Closes:).

s/get/gets

I think you can use:

Closes: https://lore.kernel.org/oe-kbuild-all/202404211031.J6l2AfJk-lkp@intel.com/ [1]

And instead use:

s/an example is in Closes:/[1]


>
> Remove this limit.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202404211031.J6l2AfJk-lkp@intel.com/
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
>   arch/riscv/include/asm/pgtable.h    | 7 -------
>   arch/riscv/kernel/vmlinux-xip.lds.S | 4 ++--
>   2 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index fbf342f4afee..75f4a92ea5bb 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -104,13 +104,6 @@
>   
>   #endif
>   
> -#ifdef CONFIG_XIP_KERNEL
> -#define XIP_OFFSET		SZ_32M
> -#define XIP_OFFSET_MASK		(SZ_32M - 1)
> -#else
> -#define XIP_OFFSET		0
> -#endif
> -
>   #ifndef __ASSEMBLY__
>   
>   #include <asm/page.h>
> diff --git a/arch/riscv/kernel/vmlinux-xip.lds.S b/arch/riscv/kernel/vmlinux-xip.lds.S
> index 8c3daa1b0531..01f73f2ffecc 100644
> --- a/arch/riscv/kernel/vmlinux-xip.lds.S
> +++ b/arch/riscv/kernel/vmlinux-xip.lds.S
> @@ -65,10 +65,10 @@ SECTIONS
>    * From this point, stuff is considered writable and will be copied to RAM
>    */
>   	__data_loc = ALIGN(PAGE_SIZE);		/* location in file */
> -	. = KERNEL_LINK_ADDR + XIP_OFFSET;	/* location in memory */
> +	. = ALIGN(SZ_2M);			/* location in memory */


You can't use SZ_2M here since it corresponds to PMD_SIZE for rv64 but 
on rv32 (which is allowed to use xip kernels), it's 4MB. Use 
SECTION_ALIGN instead.


>   
>   #undef LOAD_OFFSET
> -#define LOAD_OFFSET (KERNEL_LINK_ADDR + XIP_OFFSET - (__data_loc & XIP_OFFSET_MASK))
> +#define LOAD_OFFSET (KERNEL_LINK_ADDR + _sdata - __data_loc)
>   
>   	_sdata = .;			/* Start of data section */
>   	_data = .;

When the above comment is fixed, you can add:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

And many thanks for this big cleanup.

Alex


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

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

end of thread, other threads:[~2024-05-27 12:58 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-10  6:28 [PATCH 0/7] remove size limit on XIP kernel Nam Cao
2024-05-10  6:28 ` [PATCH 1/7] riscv: cleanup XIP_FIXUP macro Nam Cao
2024-05-27 12:16   ` Alexandre Ghiti
2024-05-10  6:28 ` [PATCH 2/7] riscv: replace va_kernel_pa_offset with va_kernel_data_pa_offset on XIP Nam Cao
2024-05-27 12:32   ` Alexandre Ghiti
2024-05-10  6:28 ` [PATCH 3/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_OFFSET Nam Cao
2024-05-27 12:37   ` Alexandre Ghiti
2024-05-10  6:28 ` [PATCH 4/7] riscv: drop the use of XIP_OFFSET in XIP_FIXUP_FLASH_OFFSET Nam Cao
2024-05-27 12:47   ` Alexandre Ghiti
2024-05-10  6:28 ` [PATCH 5/7] riscv: drop the use of XIP_OFFSET in kernel_mapping_va_to_pa() Nam Cao
2024-05-27 12:49   ` Alexandre Ghiti
2024-05-10  6:28 ` [PATCH 6/7] riscv: drop the use of XIP_OFFSET in create_kernel_page_table() Nam Cao
2024-05-27 12:53   ` Alexandre Ghiti
2024-05-10  6:28 ` [PATCH 7/7] riscv: remove limit on the size of read-only section for XIP kernel Nam Cao
2024-05-27 12:58   ` Alexandre Ghiti
2024-05-12 15:47 ` [PATCH 0/7] remove size limit on " Alexandre Ghiti
2024-05-13  8:19   ` Nam Cao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).