linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] riscv: mm: init clean up #ifdefs
@ 2021-12-06 15:03 Jisheng Zhang
  2021-12-06 15:03 ` [PATCH v2 1/5] riscv: mm: init: remove unnecessary "#ifdef CONFIG_CRASH_DUMP" Jisheng Zhang
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Jisheng Zhang @ 2021-12-06 15:03 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: linux-riscv, linux-kernel

To support NOMMU, XIP, the arch/riscv/mm/init.c becomes much complex
due to lots of #ifdefs, this not only impacts the code readability,
compile coverage, but may also bring bugs. For example, I believe one
recently fixed bug[1] is caused by this issue when merging.

This series tries to clean up unnecessary #ifdefs as much as possible.

Further cleanups may need to refactor the XIP code as Alexandre's patch
does.

[1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/010607.html


Since v1:
 - collect Reviewed-by tag.
 - remove the __maybe_unused used in max_mapped_addr declaration.
 - remove the BUG_ON check of mapping the last 4K bytes of the
   addressable memory since "this is true for every kernel actually"
   as pointed out by Alexandre.

Jisheng Zhang (5):
  riscv: mm: init: remove unnecessary "#ifdef CONFIG_CRASH_DUMP"
  riscv: mm: init: try best to use IS_ENABLED(CONFIG_64BIT) instead of
    #ifdef
  riscv: mm: init: remove _pt_ops and use pt_ops directly
  riscv: mm: init: try IS_ENABLED(CONFIG_XIP_KERNEL) instead of #ifdef
  riscv: mm: init: try best to remove #ifdef CONFIG_XIP_KERNEL usage

 arch/riscv/mm/init.c | 76 ++++++++++++++++----------------------------
 1 file changed, 27 insertions(+), 49 deletions(-)

-- 
2.34.1


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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v2 1/5] riscv: mm: init: remove unnecessary "#ifdef CONFIG_CRASH_DUMP"
  2021-12-06 15:03 [PATCH v2 0/5] riscv: mm: init clean up #ifdefs Jisheng Zhang
@ 2021-12-06 15:03 ` Jisheng Zhang
  2021-12-06 15:03 ` [PATCH v2 2/5] riscv: mm: init: try best to use IS_ENABLED(CONFIG_64BIT) instead of #ifdef Jisheng Zhang
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Jisheng Zhang @ 2021-12-06 15:03 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: linux-riscv, linux-kernel

The is_kdump_kernel() returns false for !CRASH_DUMP case, so we don't
need the #ifdef CONFIG_CRASH_DUMP for is_kdump_kernel() checking.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
---
 arch/riscv/mm/init.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 3c0649dba4ff..745f26a3b02e 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -790,12 +790,10 @@ static void __init reserve_crashkernel(void)
 	 * since it doesn't make much sense and we have limited memory
 	 * resources.
 	 */
-#ifdef CONFIG_CRASH_DUMP
 	if (is_kdump_kernel()) {
 		pr_info("crashkernel: ignoring reservation request\n");
 		return;
 	}
-#endif
 
 	ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
 				&crash_size, &crash_base);
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH v2 2/5] riscv: mm: init: try best to use IS_ENABLED(CONFIG_64BIT) instead of #ifdef
  2021-12-06 15:03 [PATCH v2 0/5] riscv: mm: init clean up #ifdefs Jisheng Zhang
  2021-12-06 15:03 ` [PATCH v2 1/5] riscv: mm: init: remove unnecessary "#ifdef CONFIG_CRASH_DUMP" Jisheng Zhang
@ 2021-12-06 15:03 ` Jisheng Zhang
  2021-12-16 10:41   ` Alexandre ghiti
  2021-12-06 15:03 ` [PATCH v2 3/5] riscv: mm: init: remove _pt_ops and use pt_ops directly Jisheng Zhang
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Jisheng Zhang @ 2021-12-06 15:03 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: linux-riscv, linux-kernel

Try our best to replace the conditional compilation using
"#ifdef CONFIG_64BIT" by a check for "IS_ENABLED(CONFIG_64BIT)", to
simplify the code and to increase compile coverage.

Now we can also remove the __maybe_unused used in max_mapped_addr
declaration.

We also remove the BUG_ON check of mapping the last 4K bytes of the
addressable memory since this is always true for every kernel actually.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 arch/riscv/mm/init.c | 43 ++++++++++++++++---------------------------
 1 file changed, 16 insertions(+), 27 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 745f26a3b02e..4edf5600bea9 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -102,10 +102,9 @@ static void __init print_vm_layout(void)
 		  (unsigned long)VMALLOC_END);
 	print_mlm("lowmem", (unsigned long)PAGE_OFFSET,
 		  (unsigned long)high_memory);
-#ifdef CONFIG_64BIT
-	print_mlm("kernel", (unsigned long)KERNEL_LINK_ADDR,
-		  (unsigned long)ADDRESS_SPACE_END);
-#endif
+	if (IS_ENABLED(CONFIG_64BIT))
+		print_mlm("kernel", (unsigned long)KERNEL_LINK_ADDR,
+			  (unsigned long)ADDRESS_SPACE_END);
 }
 #else
 static void print_vm_layout(void) { }
@@ -163,7 +162,7 @@ static void __init setup_bootmem(void)
 {
 	phys_addr_t vmlinux_end = __pa_symbol(&_end);
 	phys_addr_t vmlinux_start = __pa_symbol(&_start);
-	phys_addr_t __maybe_unused max_mapped_addr;
+	phys_addr_t max_mapped_addr;
 	phys_addr_t phys_ram_end;
 
 #ifdef CONFIG_XIP_KERNEL
@@ -172,17 +171,16 @@ static void __init setup_bootmem(void)
 
 	memblock_enforce_memory_limit(memory_limit);
 
-	/*
-	 * Reserve from the start of the kernel to the end of the kernel
-	 */
-#if defined(CONFIG_64BIT) && defined(CONFIG_STRICT_KERNEL_RWX)
 	/*
 	 * Make sure we align the reservation on PMD_SIZE since we will
 	 * map the kernel in the linear mapping as read-only: we do not want
 	 * any allocation to happen between _end and the next pmd aligned page.
 	 */
-	vmlinux_end = (vmlinux_end + PMD_SIZE - 1) & PMD_MASK;
-#endif
+	if (IS_ENABLED(CONFIG_64BIT) && IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
+		vmlinux_end = (vmlinux_end + PMD_SIZE - 1) & PMD_MASK;
+	/*
+	 * Reserve from the start of the kernel to the end of the kernel
+	 */
 	memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
 
 
@@ -190,7 +188,6 @@ static void __init setup_bootmem(void)
 #ifndef CONFIG_XIP_KERNEL
 	phys_ram_base = memblock_start_of_DRAM();
 #endif
-#ifndef CONFIG_64BIT
 	/*
 	 * memblock allocator is not aware of the fact that last 4K bytes of
 	 * the addressable memory can not be mapped because of IS_ERR_VALUE
@@ -200,10 +197,11 @@ static void __init setup_bootmem(void)
 	 * address space is occupied by the kernel mapping then this check must
 	 * be done as soon as the kernel mapping base address is determined.
 	 */
-	max_mapped_addr = __pa(~(ulong)0);
-	if (max_mapped_addr == (phys_ram_end - 1))
-		memblock_set_current_limit(max_mapped_addr - 4096);
-#endif
+	if (!IS_ENABLED(CONFIG_64BIT)) {
+		max_mapped_addr = __pa(~(ulong)0);
+		if (max_mapped_addr == (phys_ram_end - 1))
+			memblock_set_current_limit(max_mapped_addr - 4096);
+	}
 
 	min_low_pfn = PFN_UP(phys_ram_base);
 	max_low_pfn = max_pfn = PFN_DOWN(phys_ram_end);
@@ -616,14 +614,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 	BUG_ON((PAGE_OFFSET % PGDIR_SIZE) != 0);
 	BUG_ON((kernel_map.phys_addr % PMD_SIZE) != 0);
 
-#ifdef CONFIG_64BIT
-	/*
-	 * The last 4K bytes of the addressable memory can not be mapped because
-	 * of IS_ERR_VALUE macro.
-	 */
-	BUG_ON((kernel_map.virt_addr + kernel_map.size) > ADDRESS_SPACE_END - SZ_4K);
-#endif
-
 	pt_ops.alloc_pte = alloc_pte_early;
 	pt_ops.get_pte_virt = get_pte_virt_early;
 #ifndef __PAGETABLE_PMD_FOLDED
@@ -735,10 +725,9 @@ static void __init setup_vm_final(void)
 		}
 	}
 
-#ifdef CONFIG_64BIT
 	/* Map the kernel */
-	create_kernel_page_table(swapper_pg_dir, false);
-#endif
+	if (IS_ENABLED(CONFIG_64BIT))
+		create_kernel_page_table(swapper_pg_dir, false);
 
 	/* Clear fixmap PTE and PMD mappings */
 	clear_fixmap(FIX_PTE);
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH v2 3/5] riscv: mm: init: remove _pt_ops and use pt_ops directly
  2021-12-06 15:03 [PATCH v2 0/5] riscv: mm: init clean up #ifdefs Jisheng Zhang
  2021-12-06 15:03 ` [PATCH v2 1/5] riscv: mm: init: remove unnecessary "#ifdef CONFIG_CRASH_DUMP" Jisheng Zhang
  2021-12-06 15:03 ` [PATCH v2 2/5] riscv: mm: init: try best to use IS_ENABLED(CONFIG_64BIT) instead of #ifdef Jisheng Zhang
@ 2021-12-06 15:03 ` Jisheng Zhang
  2021-12-16 11:09   ` Alexandre ghiti
  2021-12-06 15:03 ` [PATCH v2 4/5] riscv: mm: init: try IS_ENABLED(CONFIG_XIP_KERNEL) instead of #ifdef Jisheng Zhang
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Jisheng Zhang @ 2021-12-06 15:03 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: linux-riscv, linux-kernel

Except "pt_ops", other global vars when CONFIG_XIP_KERNEL=y is defined
as below:

|foo_type foo;
|#ifdef CONFIG_XIP_KERNEL
|#define foo	(*(foo_type *)XIP_FIXUP(&foo))
|#endif

Follow the same way for pt_ops to unify the style and to simplify code.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 arch/riscv/mm/init.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 4edf5600bea9..9c5816971f40 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -227,12 +227,10 @@ static void __init setup_bootmem(void)
 }
 
 #ifdef CONFIG_MMU
-static struct pt_alloc_ops _pt_ops __initdata;
+static struct pt_alloc_ops pt_ops __initdata;
 
 #ifdef CONFIG_XIP_KERNEL
-#define pt_ops (*(struct pt_alloc_ops *)XIP_FIXUP(&_pt_ops))
-#else
-#define pt_ops _pt_ops
+#define pt_ops (*(struct pt_alloc_ops *)XIP_FIXUP(&pt_ops))
 #endif
 
 unsigned long riscv_pfn_base __ro_after_init;
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH v2 4/5] riscv: mm: init: try IS_ENABLED(CONFIG_XIP_KERNEL) instead of #ifdef
  2021-12-06 15:03 [PATCH v2 0/5] riscv: mm: init clean up #ifdefs Jisheng Zhang
                   ` (2 preceding siblings ...)
  2021-12-06 15:03 ` [PATCH v2 3/5] riscv: mm: init: remove _pt_ops and use pt_ops directly Jisheng Zhang
@ 2021-12-06 15:03 ` Jisheng Zhang
  2021-12-06 15:03 ` [PATCH v2 5/5] riscv: mm: init: try best to remove #ifdef CONFIG_XIP_KERNEL usage Jisheng Zhang
  2022-01-18 19:15 ` [PATCH v2 0/5] riscv: mm: init clean up #ifdefs Palmer Dabbelt
  5 siblings, 0 replies; 9+ messages in thread
From: Jisheng Zhang @ 2021-12-06 15:03 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: linux-riscv, linux-kernel

Try our best to replace the conditional compilation using
"#ifdef CONFIG_XIP_KERNEL" with "IS_ENABLED(CONFIG_XIP_KERNEL)", to
simplify the code and to increase compile coverage.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
---
 arch/riscv/mm/init.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 9c5816971f40..5e979fe06054 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -161,13 +161,13 @@ early_param("mem", early_mem);
 static void __init setup_bootmem(void)
 {
 	phys_addr_t vmlinux_end = __pa_symbol(&_end);
-	phys_addr_t vmlinux_start = __pa_symbol(&_start);
 	phys_addr_t max_mapped_addr;
-	phys_addr_t phys_ram_end;
+	phys_addr_t phys_ram_end, vmlinux_start;
 
-#ifdef CONFIG_XIP_KERNEL
-	vmlinux_start = __pa_symbol(&_sdata);
-#endif
+	if (IS_ENABLED(CONFIG_XIP_KERNEL))
+		vmlinux_start = __pa_symbol(&_sdata);
+	else
+		vmlinux_start = __pa_symbol(&_start);
 
 	memblock_enforce_memory_limit(memory_limit);
 
@@ -183,11 +183,9 @@ static void __init setup_bootmem(void)
 	 */
 	memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
 
-
 	phys_ram_end = memblock_end_of_DRAM();
-#ifndef CONFIG_XIP_KERNEL
-	phys_ram_base = memblock_start_of_DRAM();
-#endif
+	if (!IS_ENABLED(CONFIG_XIP_KERNEL))
+		phys_ram_base = memblock_start_of_DRAM();
 	/*
 	 * memblock allocator is not aware of the fact that last 4K bytes of
 	 * the addressable memory can not be mapped because of IS_ERR_VALUE
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH v2 5/5] riscv: mm: init: try best to remove #ifdef CONFIG_XIP_KERNEL usage
  2021-12-06 15:03 [PATCH v2 0/5] riscv: mm: init clean up #ifdefs Jisheng Zhang
                   ` (3 preceding siblings ...)
  2021-12-06 15:03 ` [PATCH v2 4/5] riscv: mm: init: try IS_ENABLED(CONFIG_XIP_KERNEL) instead of #ifdef Jisheng Zhang
@ 2021-12-06 15:03 ` Jisheng Zhang
  2022-01-18 19:15 ` [PATCH v2 0/5] riscv: mm: init clean up #ifdefs Palmer Dabbelt
  5 siblings, 0 replies; 9+ messages in thread
From: Jisheng Zhang @ 2021-12-06 15:03 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: linux-riscv, linux-kernel

Currently, the #ifdef CONFIG_XIP_KERNEL usage can be divided into the
following three types:

The first one is for functions/declarations only used in XIP case.

The second one is for XIP_FIXUP case. Something as below:
|foo_type foo;
|#ifdef CONFIG_XIP_KERNEL
|#define foo    (*(foo_type *)XIP_FIXUP(&foo))
|#endif

Usually, it's better to let the foo macro sit with the foo var
together. But if various foos are defined adjacently, we can
save some #ifdef CONFIG_XIP_KERNEL usage by grouping them together.

The third one is for different implementations for XIP, usually, this
is a #ifdef...#else...#endif case.

This patch moves the pt_ops macro to adjacent #ifdef CONFIG_XIP_KERNEL
and group first type usage cases into one.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
---
 arch/riscv/mm/init.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 5e979fe06054..a15640eeb334 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -40,10 +40,6 @@ EXPORT_SYMBOL(kernel_map);
 phys_addr_t phys_ram_base __ro_after_init;
 EXPORT_SYMBOL(phys_ram_base);
 
-#ifdef CONFIG_XIP_KERNEL
-extern char _xiprom[], _exiprom[], __data_loc;
-#endif
-
 unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)]
 							__page_aligned_bss;
 EXPORT_SYMBOL(empty_zero_page);
@@ -227,10 +223,6 @@ static void __init setup_bootmem(void)
 #ifdef CONFIG_MMU
 static struct pt_alloc_ops pt_ops __initdata;
 
-#ifdef CONFIG_XIP_KERNEL
-#define pt_ops (*(struct pt_alloc_ops *)XIP_FIXUP(&pt_ops))
-#endif
-
 unsigned long riscv_pfn_base __ro_after_init;
 EXPORT_SYMBOL(riscv_pfn_base);
 
@@ -242,6 +234,7 @@ pgd_t early_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
 static pmd_t __maybe_unused early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE);
 
 #ifdef CONFIG_XIP_KERNEL
+#define pt_ops			(*(struct pt_alloc_ops *)XIP_FIXUP(&pt_ops))
 #define trampoline_pg_dir      ((pgd_t *)XIP_FIXUP(trampoline_pg_dir))
 #define fixmap_pte             ((pte_t *)XIP_FIXUP(fixmap_pte))
 #define early_pg_dir           ((pgd_t *)XIP_FIXUP(early_pg_dir))
@@ -445,6 +438,8 @@ static uintptr_t __init best_map_size(phys_addr_t base, phys_addr_t size)
 }
 
 #ifdef CONFIG_XIP_KERNEL
+extern char _xiprom[], _exiprom[], __data_loc;
+
 /* called from head.S with MMU off */
 asmlinkage void __init __copy_data(void)
 {
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/5] riscv: mm: init: try best to use IS_ENABLED(CONFIG_64BIT) instead of #ifdef
  2021-12-06 15:03 ` [PATCH v2 2/5] riscv: mm: init: try best to use IS_ENABLED(CONFIG_64BIT) instead of #ifdef Jisheng Zhang
@ 2021-12-16 10:41   ` Alexandre ghiti
  0 siblings, 0 replies; 9+ messages in thread
From: Alexandre ghiti @ 2021-12-16 10:41 UTC (permalink / raw)
  To: Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: linux-riscv, linux-kernel

hi Jisheng,

On 12/6/21 16:03, Jisheng Zhang wrote:
> Try our best to replace the conditional compilation using
> "#ifdef CONFIG_64BIT" by a check for "IS_ENABLED(CONFIG_64BIT)", to
> simplify the code and to increase compile coverage.
>
> Now we can also remove the __maybe_unused used in max_mapped_addr
> declaration.
>
> We also remove the BUG_ON check of mapping the last 4K bytes of the
> addressable memory since this is always true for every kernel actually.


That could have been a different patch as this does not fit the patch 
subject, but anyway:

Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>

Thanks,

Alex


>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>   arch/riscv/mm/init.c | 43 ++++++++++++++++---------------------------
>   1 file changed, 16 insertions(+), 27 deletions(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 745f26a3b02e..4edf5600bea9 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -102,10 +102,9 @@ static void __init print_vm_layout(void)
>   		  (unsigned long)VMALLOC_END);
>   	print_mlm("lowmem", (unsigned long)PAGE_OFFSET,
>   		  (unsigned long)high_memory);
> -#ifdef CONFIG_64BIT
> -	print_mlm("kernel", (unsigned long)KERNEL_LINK_ADDR,
> -		  (unsigned long)ADDRESS_SPACE_END);
> -#endif
> +	if (IS_ENABLED(CONFIG_64BIT))
> +		print_mlm("kernel", (unsigned long)KERNEL_LINK_ADDR,
> +			  (unsigned long)ADDRESS_SPACE_END);
>   }
>   #else
>   static void print_vm_layout(void) { }
> @@ -163,7 +162,7 @@ static void __init setup_bootmem(void)
>   {
>   	phys_addr_t vmlinux_end = __pa_symbol(&_end);
>   	phys_addr_t vmlinux_start = __pa_symbol(&_start);
> -	phys_addr_t __maybe_unused max_mapped_addr;
> +	phys_addr_t max_mapped_addr;
>   	phys_addr_t phys_ram_end;
>   
>   #ifdef CONFIG_XIP_KERNEL
> @@ -172,17 +171,16 @@ static void __init setup_bootmem(void)
>   
>   	memblock_enforce_memory_limit(memory_limit);
>   
> -	/*
> -	 * Reserve from the start of the kernel to the end of the kernel
> -	 */
> -#if defined(CONFIG_64BIT) && defined(CONFIG_STRICT_KERNEL_RWX)
>   	/*
>   	 * Make sure we align the reservation on PMD_SIZE since we will
>   	 * map the kernel in the linear mapping as read-only: we do not want
>   	 * any allocation to happen between _end and the next pmd aligned page.
>   	 */
> -	vmlinux_end = (vmlinux_end + PMD_SIZE - 1) & PMD_MASK;
> -#endif
> +	if (IS_ENABLED(CONFIG_64BIT) && IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> +		vmlinux_end = (vmlinux_end + PMD_SIZE - 1) & PMD_MASK;
> +	/*
> +	 * Reserve from the start of the kernel to the end of the kernel
> +	 */
>   	memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
>   
>   
> @@ -190,7 +188,6 @@ static void __init setup_bootmem(void)
>   #ifndef CONFIG_XIP_KERNEL
>   	phys_ram_base = memblock_start_of_DRAM();
>   #endif
> -#ifndef CONFIG_64BIT
>   	/*
>   	 * memblock allocator is not aware of the fact that last 4K bytes of
>   	 * the addressable memory can not be mapped because of IS_ERR_VALUE
> @@ -200,10 +197,11 @@ static void __init setup_bootmem(void)
>   	 * address space is occupied by the kernel mapping then this check must
>   	 * be done as soon as the kernel mapping base address is determined.
>   	 */
> -	max_mapped_addr = __pa(~(ulong)0);
> -	if (max_mapped_addr == (phys_ram_end - 1))
> -		memblock_set_current_limit(max_mapped_addr - 4096);
> -#endif
> +	if (!IS_ENABLED(CONFIG_64BIT)) {
> +		max_mapped_addr = __pa(~(ulong)0);
> +		if (max_mapped_addr == (phys_ram_end - 1))
> +			memblock_set_current_limit(max_mapped_addr - 4096);
> +	}
>   
>   	min_low_pfn = PFN_UP(phys_ram_base);
>   	max_low_pfn = max_pfn = PFN_DOWN(phys_ram_end);
> @@ -616,14 +614,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>   	BUG_ON((PAGE_OFFSET % PGDIR_SIZE) != 0);
>   	BUG_ON((kernel_map.phys_addr % PMD_SIZE) != 0);
>   
> -#ifdef CONFIG_64BIT
> -	/*
> -	 * The last 4K bytes of the addressable memory can not be mapped because
> -	 * of IS_ERR_VALUE macro.
> -	 */
> -	BUG_ON((kernel_map.virt_addr + kernel_map.size) > ADDRESS_SPACE_END - SZ_4K);
> -#endif
> -
>   	pt_ops.alloc_pte = alloc_pte_early;
>   	pt_ops.get_pte_virt = get_pte_virt_early;
>   #ifndef __PAGETABLE_PMD_FOLDED
> @@ -735,10 +725,9 @@ static void __init setup_vm_final(void)
>   		}
>   	}
>   
> -#ifdef CONFIG_64BIT
>   	/* Map the kernel */
> -	create_kernel_page_table(swapper_pg_dir, false);
> -#endif
> +	if (IS_ENABLED(CONFIG_64BIT))
> +		create_kernel_page_table(swapper_pg_dir, false);
>   
>   	/* Clear fixmap PTE and PMD mappings */
>   	clear_fixmap(FIX_PTE);

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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/5] riscv: mm: init: remove _pt_ops and use pt_ops directly
  2021-12-06 15:03 ` [PATCH v2 3/5] riscv: mm: init: remove _pt_ops and use pt_ops directly Jisheng Zhang
@ 2021-12-16 11:09   ` Alexandre ghiti
  0 siblings, 0 replies; 9+ messages in thread
From: Alexandre ghiti @ 2021-12-16 11:09 UTC (permalink / raw)
  To: Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: linux-riscv, linux-kernel

On 12/6/21 16:03, Jisheng Zhang wrote:
> Except "pt_ops", other global vars when CONFIG_XIP_KERNEL=y is defined
> as below:
>
> |foo_type foo;
> |#ifdef CONFIG_XIP_KERNEL
> |#define foo	(*(foo_type *)XIP_FIXUP(&foo))
> |#endif
>
> Follow the same way for pt_ops to unify the style and to simplify code.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>   arch/riscv/mm/init.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 4edf5600bea9..9c5816971f40 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -227,12 +227,10 @@ static void __init setup_bootmem(void)
>   }
>   
>   #ifdef CONFIG_MMU
> -static struct pt_alloc_ops _pt_ops __initdata;
> +static struct pt_alloc_ops pt_ops __initdata;
>   
>   #ifdef CONFIG_XIP_KERNEL
> -#define pt_ops (*(struct pt_alloc_ops *)XIP_FIXUP(&_pt_ops))
> -#else
> -#define pt_ops _pt_ops
> +#define pt_ops (*(struct pt_alloc_ops *)XIP_FIXUP(&pt_ops))
>   #endif


I tried to compile a XIP kernel with success and I noticed we can go 
even further by removing the ifdef CONFIG_XIP_KERNEL since XIP_FIXUP is 
the identity for normal kernels.

Anyway, I'll do that later:

Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>

Thanks,

Alex


>   
>   unsigned long riscv_pfn_base __ro_after_init;

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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/5] riscv: mm: init clean up #ifdefs
  2021-12-06 15:03 [PATCH v2 0/5] riscv: mm: init clean up #ifdefs Jisheng Zhang
                   ` (4 preceding siblings ...)
  2021-12-06 15:03 ` [PATCH v2 5/5] riscv: mm: init: try best to remove #ifdef CONFIG_XIP_KERNEL usage Jisheng Zhang
@ 2022-01-18 19:15 ` Palmer Dabbelt
  5 siblings, 0 replies; 9+ messages in thread
From: Palmer Dabbelt @ 2022-01-18 19:15 UTC (permalink / raw)
  To: jszhang; +Cc: Paul Walmsley, aou, alex, linux-riscv, linux-kernel

On Mon, 06 Dec 2021 07:03:48 PST (-0800), jszhang@kernel.org wrote:
> To support NOMMU, XIP, the arch/riscv/mm/init.c becomes much complex
> due to lots of #ifdefs, this not only impacts the code readability,
> compile coverage, but may also bring bugs. For example, I believe one
> recently fixed bug[1] is caused by this issue when merging.
>
> This series tries to clean up unnecessary #ifdefs as much as possible.
>
> Further cleanups may need to refactor the XIP code as Alexandre's patch
> does.
>
> [1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/010607.html
>
>
> Since v1:
>  - collect Reviewed-by tag.
>  - remove the __maybe_unused used in max_mapped_addr declaration.
>  - remove the BUG_ON check of mapping the last 4K bytes of the
>    addressable memory since "this is true for every kernel actually"
>    as pointed out by Alexandre.
>
> Jisheng Zhang (5):
>   riscv: mm: init: remove unnecessary "#ifdef CONFIG_CRASH_DUMP"
>   riscv: mm: init: try best to use IS_ENABLED(CONFIG_64BIT) instead of
>     #ifdef
>   riscv: mm: init: remove _pt_ops and use pt_ops directly
>   riscv: mm: init: try IS_ENABLED(CONFIG_XIP_KERNEL) instead of #ifdef
>   riscv: mm: init: try best to remove #ifdef CONFIG_XIP_KERNEL usage
>
>  arch/riscv/mm/init.c | 76 ++++++++++++++++----------------------------
>  1 file changed, 27 insertions(+), 49 deletions(-)

Thanks, these look good.  I've put them on a staging branch that will 
soon be for-next, I'd like to start from after my recent PR's merge 
(assuming it's merged).

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

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-01-18 19:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-06 15:03 [PATCH v2 0/5] riscv: mm: init clean up #ifdefs Jisheng Zhang
2021-12-06 15:03 ` [PATCH v2 1/5] riscv: mm: init: remove unnecessary "#ifdef CONFIG_CRASH_DUMP" Jisheng Zhang
2021-12-06 15:03 ` [PATCH v2 2/5] riscv: mm: init: try best to use IS_ENABLED(CONFIG_64BIT) instead of #ifdef Jisheng Zhang
2021-12-16 10:41   ` Alexandre ghiti
2021-12-06 15:03 ` [PATCH v2 3/5] riscv: mm: init: remove _pt_ops and use pt_ops directly Jisheng Zhang
2021-12-16 11:09   ` Alexandre ghiti
2021-12-06 15:03 ` [PATCH v2 4/5] riscv: mm: init: try IS_ENABLED(CONFIG_XIP_KERNEL) instead of #ifdef Jisheng Zhang
2021-12-06 15:03 ` [PATCH v2 5/5] riscv: mm: init: try best to remove #ifdef CONFIG_XIP_KERNEL usage Jisheng Zhang
2022-01-18 19:15 ` [PATCH v2 0/5] riscv: mm: init clean up #ifdefs Palmer Dabbelt

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).