diff mbox series

[2/7] riscv: replace va_kernel_pa_offset with va_kernel_data_pa_offset on XIP

Message ID 133e7cd43a67abfed96bc246ae5f8a870edde684.1715286093.git.namcao@linutronix.de (mailing list archive)
State Superseded
Headers show
Series remove size limit on XIP kernel | expand

Checks

Context Check Description
conchuod/vmtest-fixes-PR fail merge-conflict

Commit Message

Nam Cao May 10, 2024, 6:28 a.m. UTC
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(-)

Comments

Alexandre Ghiti May 27, 2024, 12:32 p.m. UTC | #1
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
diff mbox series

Patch

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