diff mbox series

[1/7] riscv: cleanup XIP_FIXUP macro

Message ID 19e63324d7a099f561c4a2e55f7df051bd5b8a6f.1715286093.git.namcao@linutronix.de (mailing list archive)
State New
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
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(-)

Comments

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

Patch

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)