diff mbox

[1/2] arm64: mm: make create_mapping_late() non-allocating

Message ID 1469208745-6693-2-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit 258a1605c78878ada00417de4840c4c427673ffa
Headers show

Commit Message

Ard Biesheuvel July 22, 2016, 5:32 p.m. UTC
The only purpose served by create_mapping_late() is to remap the already
mapped .text and .rodata kernel segments with read-only permissions. Since
we no longer allow block mappings to be split or merged,
create_mapping_late() should not pass an allocation function pointer into
__create_pgd_mapping(). So pass NULL instead.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 arch/arm64/mm/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.7.4


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

Comments

Mark Rutland July 25, 2016, 1:46 p.m. UTC | #1
Hi Ard,

On Fri, Jul 22, 2016 at 07:32:24PM +0200, Ard Biesheuvel wrote:
> The only purpose served by create_mapping_late() is to remap the already

> mapped .text and .rodata kernel segments with read-only permissions. Since

> we no longer allow block mappings to be split or merged,

> create_mapping_late() should not pass an allocation function pointer into

> __create_pgd_mapping(). So pass NULL instead.

> 

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  arch/arm64/mm/mmu.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

> index a96a2413fa18..33f36cede02d 100644

> --- a/arch/arm64/mm/mmu.c

> +++ b/arch/arm64/mm/mmu.c

> @@ -312,7 +312,7 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,

>  	}

>  

>  	__create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,

> -			     late_pgtable_alloc, !debug_pagealloc_enabled());

> +			     NULL, !debug_pagealloc_enabled());

>  }


How about we drop the __init marker from create_mapping_noalloc(), and
update the callers in mark_rodata_ro() to use that, so that we can drop
create_mapping_late() entirely.

Other than the __init marker and name, create_mapping_late() and
create_mapping_noalloc() would be identical after this change, and the
naming of create_mapping_noalloc() does better describe what we're
trying to achieve in mark_rodata_ro().

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ard Biesheuvel July 25, 2016, 2:10 p.m. UTC | #2
> On 25 jul. 2016, at 15:46, Mark Rutland <mark.rutland@arm.com> wrote:

> 

> Hi Ard,

> 

>> On Fri, Jul 22, 2016 at 07:32:24PM +0200, Ard Biesheuvel wrote:

>> The only purpose served by create_mapping_late() is to remap the already

>> mapped .text and .rodata kernel segments with read-only permissions. Since

>> we no longer allow block mappings to be split or merged,

>> create_mapping_late() should not pass an allocation function pointer into

>> __create_pgd_mapping(). So pass NULL instead.

>> 

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> ---

>> arch/arm64/mm/mmu.c | 2 +-

>> 1 file changed, 1 insertion(+), 1 deletion(-)

>> 

>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c

>> index a96a2413fa18..33f36cede02d 100644

>> --- a/arch/arm64/mm/mmu.c

>> +++ b/arch/arm64/mm/mmu.c

>> @@ -312,7 +312,7 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,

>>    }

>> 

>>    __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,

>> -                 late_pgtable_alloc, !debug_pagealloc_enabled());

>> +                 NULL, !debug_pagealloc_enabled());

>> }

> 

> How about we drop the __init marker from create_mapping_noalloc(), and

> update the callers in mark_rodata_ro() to use that, so that we can drop

> create_mapping_late() entirely.

> 

> Other than the __init marker and name, create_mapping_late() and

> create_mapping_noalloc() would be identical after this change, and the

> naming of create_mapping_noalloc() does better describe what we're

> trying to achieve in mark_rodata_ro().

> 


Not entirely. The _late() variant should map down to pages if the first mapping was mapped down to pages as well. The _noalloc() variant may end attempting to merge regions


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Mark Rutland July 25, 2016, 2:22 p.m. UTC | #3
On Mon, Jul 25, 2016 at 04:10:50PM +0200, Ard Biesheuvel wrote:
> > On 25 jul. 2016, at 15:46, Mark Rutland <mark.rutland@arm.com> wrote:

> > How about we drop the __init marker from create_mapping_noalloc(), and

> > update the callers in mark_rodata_ro() to use that, so that we can drop

> > create_mapping_late() entirely.

> > 

> > Other than the __init marker and name, create_mapping_late() and

> > create_mapping_noalloc() would be identical after this change, and the

> > naming of create_mapping_noalloc() does better describe what we're

> > trying to achieve in mark_rodata_ro().

> 

> Not entirely. The _late() variant should map down to pages if the

> first mapping was mapped down to pages as well. The _noalloc() variant

> may end attempting to merge regions


Ah, I was comparing against mainline rather than the arm64 for-next/core
branch with the recent rework. Sorry for the noise.

Given that, FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>


Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox

Patch

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index a96a2413fa18..33f36cede02d 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -312,7 +312,7 @@  static void create_mapping_late(phys_addr_t phys, unsigned long virt,
 	}
 
 	__create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,
-			     late_pgtable_alloc, !debug_pagealloc_enabled());
+			     NULL, !debug_pagealloc_enabled());
 }
 
 static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)