diff mbox series

[3/7] riscv: mm: Refactor create_linear_mapping_range() for hot add

Message ID 20230512145737.985671-4-bjorn@kernel.org (mailing list archive)
State Changes Requested
Headers show
Series riscv: Memory Hot(Un)Plug support | expand

Checks

Context Check Description
conchuod/cover_letter success Series has a cover letter
conchuod/tree_selection success Guessed tree name to be for-next at HEAD ac9a78681b92
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/maintainers_pattern success MAINTAINERS pattern errors before the patch: 6 and now 6
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/build_rv64_clang_allmodconfig fail Failed to build the tree with this patch.
conchuod/module_param success Was 0 now: 0
conchuod/build_rv64_gcc_allmodconfig fail Failed to build the tree with this patch.
conchuod/build_rv32_defconfig fail Build failed
conchuod/dtb_warn_rv64 success Errors and warnings before: 3 this patch: 3
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch success total: 0 errors, 0 warnings, 0 checks, 40 lines checked
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success No Fixes tag
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Björn Töpel May 12, 2023, 2:57 p.m. UTC 
From: Björn Töpel <bjorn@rivosinc.com>

Add a parameter to the direct map setup function, so it can be used in
arch_add_memory() later.

Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
---
 arch/riscv/mm/init.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Palmer Dabbelt June 21, 2023, 11:56 p.m. UTC | #1 
On Fri, 12 May 2023 07:57:33 PDT (-0700), bjorn@kernel.org wrote:
> From: Björn Töpel <bjorn@rivosinc.com>
>
> Add a parameter to the direct map setup function, so it can be used in
> arch_add_memory() later.
>
> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
> ---
>  arch/riscv/mm/init.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index e974ff6ef036..aea8ccb3f4ae 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -1247,18 +1247,19 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>  	pt_ops_set_fixmap();
>  }
>
> -static void __init create_linear_mapping_range(phys_addr_t start,
> -					       phys_addr_t end)
> +static void __meminit create_linear_mapping_range(phys_addr_t start, phys_addr_t end,
> +						  struct mhp_params *params)

Sorry if I missed a v2, but it looks like this fails to build under 
CONFIG_MEMORY_HOTPLUG=n (as struct mhp_params isn't defined) -- unless I 
screwed up some merge conflict, but doesn't look like it here.

I'm getting

      CC      arch/riscv/mm/init.o
    arch/riscv/mm/init.c:1252:58: warning: ‘struct mhp_params’ declared inside parameter list will not be visible outside of this definition or declaration
     1252 |                                                   struct mhp_params *params)
          |                                                          ^~~~~~~~~~
    arch/riscv/mm/init.c: In function ‘create_linear_mapping_range’:
    arch/riscv/mm/init.c:1261:42: error: invalid use of undefined type ‘struct mhp_params’
     1261 |                 pgprot =  params ? params->pgprot : pgprot_from_va(va);
          |                                          ^~
    make[3]: *** [scripts/Makefile.build:252: arch/riscv/mm/init.o] Error 1
    make[2]: *** [scripts/Makefile.build:494: arch/riscv/mm] Error 2
    make[1]: *** [scripts/Makefile.build:494: arch/riscv] Error 2
    make: *** [Makefile:2026: .] Error 2

patchwork is saying something similar
<https://gist.github.com/conor-pwbot/9ed9a564e63d824aed1786050ee06558>.

>  {
>  	phys_addr_t pa;
>  	uintptr_t va, map_size;
>
>  	for (pa = start; pa < end; pa += map_size) {
> +		pgprot_t pgprot;
> +
>  		va = (uintptr_t)__va(pa);
> +		pgprot =  params ? params->pgprot : pgprot_from_va(va);
>  		map_size = best_map_size(pa, end - pa);
> -
> -		create_pgd_mapping(swapper_pg_dir, va, pa, map_size,
> -				   pgprot_from_va(va));
> +		create_pgd_mapping(swapper_pg_dir, va, pa, map_size, pgprot);
>  	}
>  }
>
> @@ -1288,13 +1289,12 @@ static void __init create_linear_mapping_page_table(void)
>  		if (end >= __pa(PAGE_OFFSET) + memory_limit)
>  			end = __pa(PAGE_OFFSET) + memory_limit;
>
> -		create_linear_mapping_range(start, end);
> +		create_linear_mapping_range(start, end, NULL);
>  	}
>
>  #ifdef CONFIG_STRICT_KERNEL_RWX
> -	create_linear_mapping_range(ktext_start, ktext_start + ktext_size);
> -	create_linear_mapping_range(krodata_start,
> -				    krodata_start + krodata_size);
> +	create_linear_mapping_range(ktext_start, ktext_start + ktext_size, NULL);
> +	create_linear_mapping_range(krodata_start, krodata_start + krodata_size, NULL);
>
>  	memblock_clear_nomap(ktext_start,  ktext_size);
>  	memblock_clear_nomap(krodata_start, krodata_size);
Björn Töpel June 22, 2023, 4:56 a.m. UTC | #2 
Palmer Dabbelt <palmer@dabbelt.com> writes:

> On Fri, 12 May 2023 07:57:33 PDT (-0700), bjorn@kernel.org wrote:
>> From: Björn Töpel <bjorn@rivosinc.com>
>>
>> Add a parameter to the direct map setup function, so it can be used in
>> arch_add_memory() later.
>>
>> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
>> ---
>>  arch/riscv/mm/init.c | 18 +++++++++---------
>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index e974ff6ef036..aea8ccb3f4ae 100644
>> --- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -1247,18 +1247,19 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>>  	pt_ops_set_fixmap();
>>  }
>>
>> -static void __init create_linear_mapping_range(phys_addr_t start,
>> -					       phys_addr_t end)
>> +static void __meminit create_linear_mapping_range(phys_addr_t start, phys_addr_t end,
>> +						  struct mhp_params *params)
>
> Sorry if I missed a v2, but it looks like this fails to build under 
> CONFIG_MEMORY_HOTPLUG=n (as struct mhp_params isn't defined) -- unless I 
> screwed up some merge conflict, but doesn't look like it here.
>
> I'm getting
>
>       CC      arch/riscv/mm/init.o
>     arch/riscv/mm/init.c:1252:58: warning: ‘struct mhp_params’ declared inside parameter list will not be visible outside of this definition or declaration
>      1252 |                                                   struct mhp_params *params)
>           |                                                          ^~~~~~~~~~
>     arch/riscv/mm/init.c: In function ‘create_linear_mapping_range’:
>     arch/riscv/mm/init.c:1261:42: error: invalid use of undefined type ‘struct mhp_params’
>      1261 |                 pgprot =  params ? params->pgprot : pgprot_from_va(va);
>           |                                          ^~
>     make[3]: *** [scripts/Makefile.build:252: arch/riscv/mm/init.o] Error 1
>     make[2]: *** [scripts/Makefile.build:494: arch/riscv/mm] Error 2
>     make[1]: *** [scripts/Makefile.build:494: arch/riscv] Error 2
>     make: *** [Makefile:2026: .] Error 2
>
> patchwork is saying something similar
> <https://gist.github.com/conor-pwbot/9ed9a564e63d824aed1786050ee06558>.

Yup! Thanks for pointing that out. This series has a bunch of more
issues, that need to be resolved in a v2.


Björn
diff mbox series

Patch

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index e974ff6ef036..aea8ccb3f4ae 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -1247,18 +1247,19 @@  asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 	pt_ops_set_fixmap();
 }
 
-static void __init create_linear_mapping_range(phys_addr_t start,
-					       phys_addr_t end)
+static void __meminit create_linear_mapping_range(phys_addr_t start, phys_addr_t end,
+						  struct mhp_params *params)
 {
 	phys_addr_t pa;
 	uintptr_t va, map_size;
 
 	for (pa = start; pa < end; pa += map_size) {
+		pgprot_t pgprot;
+
 		va = (uintptr_t)__va(pa);
+		pgprot =  params ? params->pgprot : pgprot_from_va(va);
 		map_size = best_map_size(pa, end - pa);
-
-		create_pgd_mapping(swapper_pg_dir, va, pa, map_size,
-				   pgprot_from_va(va));
+		create_pgd_mapping(swapper_pg_dir, va, pa, map_size, pgprot);
 	}
 }
 
@@ -1288,13 +1289,12 @@  static void __init create_linear_mapping_page_table(void)
 		if (end >= __pa(PAGE_OFFSET) + memory_limit)
 			end = __pa(PAGE_OFFSET) + memory_limit;
 
-		create_linear_mapping_range(start, end);
+		create_linear_mapping_range(start, end, NULL);
 	}
 
 #ifdef CONFIG_STRICT_KERNEL_RWX
-	create_linear_mapping_range(ktext_start, ktext_start + ktext_size);
-	create_linear_mapping_range(krodata_start,
-				    krodata_start + krodata_size);
+	create_linear_mapping_range(ktext_start, ktext_start + ktext_size, NULL);
+	create_linear_mapping_range(krodata_start, krodata_start + krodata_size, NULL);
 
 	memblock_clear_nomap(ktext_start,  ktext_size);
 	memblock_clear_nomap(krodata_start, krodata_size);