Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1317015 > unrolled thread

[RFC][PATCH 0/4] ARCH_SUPPORTS_DEBUG_PAGEALLOC for arm64

Started byLaura Abbott <labbott@fedoraproject.org>
First post2016-01-25 18:00 +0100
Last post2016-01-26 22:30 +0100
Articles 9 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [RFC][PATCH 0/4] ARCH_SUPPORTS_DEBUG_PAGEALLOC for arm64 Laura Abbott <labbott@fedoraproject.org> - 2016-01-25 18:00 +0100
    [RFC][PATCH 3/4] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC Laura Abbott <labbott@fedoraproject.org> - 2016-01-25 18:00 +0100
    [RFC][PATCH 4/4] arm64: ptdump: Indicate whether memory should be faulting Laura Abbott <labbott@fedoraproject.org> - 2016-01-25 18:00 +0100
    [RFC][PATCH 2/4] arm64: Add option to force mapping with PAGE_SIZE pages Laura Abbott <labbott@fedoraproject.org> - 2016-01-25 18:00 +0100
      Re: [RFC][PATCH 2/4] arm64: Add option to force mapping with  PAGE_SIZE pages Mark Rutland <mark.rutland@arm.com> - 2016-01-26 12:20 +0100
        Re: [RFC][PATCH 2/4] arm64: Add option to force mapping with  PAGE_SIZE pages Laura Abbott <labbott@redhat.com> - 2016-01-26 23:00 +0100
    [RFC][PATCH 1/4] arm64: Drop alloc function from create_mapping Laura Abbott <labbott@fedoraproject.org> - 2016-01-25 18:00 +0100
      Re: [RFC][PATCH 1/4] arm64: Drop alloc function from create_mapping Mark Rutland <mark.rutland@arm.com> - 2016-01-26 12:30 +0100
        Re: [RFC][PATCH 1/4] arm64: Drop alloc function from create_mapping Laura Abbott <labbott@redhat.com> - 2016-01-26 22:30 +0100

#1317015 — [RFC][PATCH 0/4] ARCH_SUPPORTS_DEBUG_PAGEALLOC for arm64

FromLaura Abbott <labbott@fedoraproject.org>
Date2016-01-25 18:00 +0100
Subject[RFC][PATCH 0/4] ARCH_SUPPORTS_DEBUG_PAGEALLOC for arm64
Message-ID<qUSEG-6QD-19@gated-at.bofh.it>
Hi,

This series implements proper debug page alloc support for arm64.
Pages are unmapped on free. This series is based on top of Mark
Rutland's work to TLB conflicts in early MMU mapping
(lkml.kernel.org/g/<1451930211-22460-1-git-send-email-mark.rutland@arm.com>)
and Ard Biesheuvel's patch to allow set_memory_* to work on vmalloc
regions (<1453125665-26627-1-git-send-email-ard.biesheuvel@linaro.org>)

The most important part is to force all memory to be mapped with page
mappings. Once that happens it's trivial to use the existing page_attr
infrastructure to clear the valid bit and cause faults. I know there was
an attempt at this a while back (can't find it in my archives) but the
changes by Mark make this significantly easier and probably with fewer
TLB conflicts.

As always, comments are appreciated.

Laura Abbott (4):
  arm64: Drop alloc function from create_mapping
  arm64: Add option to force mapping with PAGE_SIZE pages
  arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC
  arm64: ptdump: Indicate whether memory should be faulting

 arch/arm64/Kconfig       | 14 ++++++++++++++
 arch/arm64/mm/dump.c     |  5 +++++
 arch/arm64/mm/mmu.c      | 24 +++++++++++++++++-------
 arch/arm64/mm/pageattr.c | 40 +++++++++++++++++++++++++++++++++-------
 4 files changed, 69 insertions(+), 14 deletions(-)

-- 
2.5.0

[toc] | [next] | [standalone]


#1317017 — [RFC][PATCH 3/4] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC

FromLaura Abbott <labbott@fedoraproject.org>
Date2016-01-25 18:00 +0100
Subject[RFC][PATCH 3/4] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC
Message-ID<qUSEG-6QD-27@gated-at.bofh.it>
In reply to#1317015
With support for forcing all memory, we can now easily map and unmap
individual pages for debugging purposes. Add support for
ARCH_SUPPORTS_DEBUG_PAGEALLOC to map and unmap pages when this
debugging feature is enabled.

Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
---
 arch/arm64/Kconfig       |  3 +++
 arch/arm64/mm/pageattr.c | 40 +++++++++++++++++++++++++++++++++-------
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index faf7eac..0d757eb 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -518,6 +518,9 @@ config FORCE_PAGES
 source kernel/Kconfig.preempt
 source kernel/Kconfig.hz
 
+config ARCH_SUPPORTS_DEBUG_PAGEALLOC
+	def_bool y if FORCE_PAGES
+
 config ARCH_HAS_HOLES_MEMORYMODEL
 	def_bool y if SPARSEMEM
 
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 1360a02..16efc7c 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -38,7 +38,8 @@ static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
 }
 
 static int change_memory_common(unsigned long addr, int numpages,
-				pgprot_t set_mask, pgprot_t clear_mask)
+				pgprot_t set_mask, pgprot_t clear_mask,
+				bool ignore_vma_check)
 {
 	unsigned long start = addr;
 	unsigned long size = PAGE_SIZE*numpages;
@@ -65,11 +66,14 @@ static int change_memory_common(unsigned long addr, int numpages,
 	 *
 	 * So check whether the [addr, addr + size) interval is entirely
 	 * covered by precisely one VM area that has the VM_ALLOC flag set.
+	 *
+	 * The one exception to this is is we're forcing everything to be
+	 * page mapped
 	 */
 	area = find_vm_area((void *)addr);
-	if (!area ||
+	if (!ignore_vma_check && (!area ||
 	    end > (unsigned long)area->addr + area->size ||
-	    !(area->flags & VM_ALLOC))
+	    !(area->flags & VM_ALLOC)))
 		return -EINVAL;
 
 	data.set_mask = set_mask;
@@ -86,21 +90,24 @@ int set_memory_ro(unsigned long addr, int numpages)
 {
 	return change_memory_common(addr, numpages,
 					__pgprot(PTE_RDONLY),
-					__pgprot(PTE_WRITE));
+					__pgprot(PTE_WRITE),
+					false);
 }
 
 int set_memory_rw(unsigned long addr, int numpages)
 {
 	return change_memory_common(addr, numpages,
 					__pgprot(PTE_WRITE),
-					__pgprot(PTE_RDONLY));
+					__pgprot(PTE_RDONLY),
+					false);
 }
 
 int set_memory_nx(unsigned long addr, int numpages)
 {
 	return change_memory_common(addr, numpages,
 					__pgprot(PTE_PXN),
-					__pgprot(0));
+					__pgprot(0),
+					false);
 }
 EXPORT_SYMBOL_GPL(set_memory_nx);
 
@@ -108,6 +115,25 @@ int set_memory_x(unsigned long addr, int numpages)
 {
 	return change_memory_common(addr, numpages,
 					__pgprot(0),
-					__pgprot(PTE_PXN));
+					__pgprot(PTE_PXN),
+					false);
 }
 EXPORT_SYMBOL_GPL(set_memory_x);
+
+#ifdef CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC
+void __kernel_map_pages(struct page *page, int numpages, int enable)
+{
+	unsigned long addr = (unsigned long) page_address(page);
+
+	if (enable)
+		change_memory_common(addr, numpages,
+					__pgprot(PTE_VALID),
+					__pgprot(0),
+					true);
+	else
+		change_memory_common(addr, numpages,
+					__pgprot(0),
+					__pgprot(PTE_VALID),
+					true);
+}
+#endif
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1317020 — [RFC][PATCH 4/4] arm64: ptdump: Indicate whether memory should be faulting

FromLaura Abbott <labbott@fedoraproject.org>
Date2016-01-25 18:00 +0100
Subject[RFC][PATCH 4/4] arm64: ptdump: Indicate whether memory should be faulting
Message-ID<qUSEH-6QD-35@gated-at.bofh.it>
In reply to#1317015
With CONFIG_DEBUG_PAGEALLOC, pages do not have the valid bit
set when free in the buddy allocator. Add an indiciation to
the page table dumping code that the valid bit is not set,
'F' for fault, to make this easier to understand.

Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
---
Might be too verbose but I found this useful when I was testing
---
 arch/arm64/mm/dump.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index 5a22a11..f381ac9 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -90,6 +90,11 @@ struct prot_bits {
 
 static const struct prot_bits pte_bits[] = {
 	{
+		.mask	= PTE_VALID,
+		.val	= PTE_VALID,
+		.set	= " ",
+		.clear	= "F",
+	}, {
 		.mask	= PTE_USER,
 		.val	= PTE_USER,
 		.set	= "USR",
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1317021 — [RFC][PATCH 2/4] arm64: Add option to force mapping with PAGE_SIZE pages

FromLaura Abbott <labbott@fedoraproject.org>
Date2016-01-25 18:00 +0100
Subject[RFC][PATCH 2/4] arm64: Add option to force mapping with PAGE_SIZE pages
Message-ID<qUSEH-6QD-39@gated-at.bofh.it>
In reply to#1317015
Under some circumstances (e.g. debugging) it may be useful to have all
kernel memory mapped using PAGE_SIZE pages. Add an option for this.

Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
---
 arch/arm64/Kconfig  | 11 +++++++++++
 arch/arm64/mm/mmu.c | 10 +++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index ffa3c54..faf7eac 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -504,6 +504,17 @@ config HOTPLUG_CPU
 	  Say Y here to experiment with turning CPUs off and on.  CPUs
 	  can be controlled through /sys/devices/system/cpu.
 
+config FORCE_PAGES
+	bool "Force all memory mappings to be PAGE_SIZE"
+	help
+	  For performance reasons, kernel memory may be mapped with
+	  page table entries larger than the expected PAGE_SIZE. This results
+	  in better TLB performance but prevents adjustment of page table
+	  attributes at runtime. Say Y here to have all kernel memory mapped
+	  with PAGE_SIZE entries.
+
+	  If unsure, say N.
+
 source kernel/Kconfig.preempt
 source kernel/Kconfig.hz
 
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 2d6e7cf..450d38a 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -181,7 +181,8 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
 	do {
 		next = pmd_addr_end(addr, end);
 		/* try section mapping first */
-		if (((addr | next | phys) & ~SECTION_MASK) == 0) {
+		if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
+		      (!IS_ENABLED(CONFIG_FORCE_PAGES) || !pgtable_alloc)) {
 			pmd_t old_pmd =*pmd;
 			set_pmd(pmd, __pmd(phys |
 					   pgprot_val(mk_sect_prot(prot))));
@@ -208,8 +209,11 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
 }
 
 static inline bool use_1G_block(unsigned long addr, unsigned long next,
-			unsigned long phys)
+			unsigned long phys, phys_addr_t (*pgtable_alloc)(void))
 {
+	if (pgtable_alloc && IS_ENABLED(CONFIG_FORCE_PAGES))
+		return false;
+
 	if (PAGE_SHIFT != 12)
 		return false;
 
@@ -241,7 +245,7 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
 		/*
 		 * For 4K granule only, attempt to put down a 1GB block
 		 */
-		if (use_1G_block(addr, next, phys)) {
+		if (use_1G_block(addr, next, phys, pgtable_alloc)) {
 			pud_t old_pud = *pud;
 			set_pud(pud, __pud(phys |
 					   pgprot_val(mk_sect_prot(prot))));
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1317749 — Re: [RFC][PATCH 2/4] arm64: Add option to force mapping with PAGE_SIZE pages

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-26 12:20 +0100
SubjectRe: [RFC][PATCH 2/4] arm64: Add option to force mapping with PAGE_SIZE pages
Message-ID<qV9Pb-3ib-5@gated-at.bofh.it>
In reply to#1317021
Hi,

On Mon, Jan 25, 2016 at 08:52:13AM -0800, Laura Abbott wrote:
> Under some circumstances (e.g. debugging) it may be useful to have all
> kernel memory mapped using PAGE_SIZE pages. Add an option for this.
> 
> Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
> ---
>  arch/arm64/Kconfig  | 11 +++++++++++
>  arch/arm64/mm/mmu.c | 10 +++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index ffa3c54..faf7eac 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -504,6 +504,17 @@ config HOTPLUG_CPU
>  	  Say Y here to experiment with turning CPUs off and on.  CPUs
>  	  can be controlled through /sys/devices/system/cpu.
>  
> +config FORCE_PAGES
> +	bool "Force all memory mappings to be PAGE_SIZE"
> +	help
> +	  For performance reasons, kernel memory may be mapped with
> +	  page table entries larger than the expected PAGE_SIZE. This results
> +	  in better TLB performance but prevents adjustment of page table
> +	  attributes at runtime. Say Y here to have all kernel memory mapped
> +	  with PAGE_SIZE entries.
> +
> +	  If unsure, say N.
> +

I think we can make this a boot-time option, given that we're simply
flipping a boolean that only directly affects a small amount of boot
code. I doubt there would be a noticeable overhead, and it makes it more
likely that we can use this to debug production kernels.

Is there a usecase other than DEBUG_PAGEALLOC that you have in mind? If
not, we could just hang this off of debug_pagealloc_enabled(), as we
parse the early params before we create the usual page tables.

Thanks,
Mark.

>  source kernel/Kconfig.preempt
>  source kernel/Kconfig.hz
>  
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 2d6e7cf..450d38a 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -181,7 +181,8 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
>  	do {
>  		next = pmd_addr_end(addr, end);
>  		/* try section mapping first */
> -		if (((addr | next | phys) & ~SECTION_MASK) == 0) {
> +		if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
> +		      (!IS_ENABLED(CONFIG_FORCE_PAGES) || !pgtable_alloc)) {
>  			pmd_t old_pmd =*pmd;
>  			set_pmd(pmd, __pmd(phys |
>  					   pgprot_val(mk_sect_prot(prot))));
> @@ -208,8 +209,11 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
>  }
>  
>  static inline bool use_1G_block(unsigned long addr, unsigned long next,
> -			unsigned long phys)
> +			unsigned long phys, phys_addr_t (*pgtable_alloc)(void))
>  {
> +	if (pgtable_alloc && IS_ENABLED(CONFIG_FORCE_PAGES))
> +		return false;
> +
>  	if (PAGE_SHIFT != 12)
>  		return false;
>  
> @@ -241,7 +245,7 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
>  		/*
>  		 * For 4K granule only, attempt to put down a 1GB block
>  		 */
> -		if (use_1G_block(addr, next, phys)) {
> +		if (use_1G_block(addr, next, phys, pgtable_alloc)) {
>  			pud_t old_pud = *pud;
>  			set_pud(pud, __pud(phys |
>  					   pgprot_val(mk_sect_prot(prot))));
> -- 
> 2.5.0
> 

[toc] | [prev] | [next] | [standalone]


#1318428 — Re: [RFC][PATCH 2/4] arm64: Add option to force mapping with PAGE_SIZE pages

FromLaura Abbott <labbott@redhat.com>
Date2016-01-26 23:00 +0100
SubjectRe: [RFC][PATCH 2/4] arm64: Add option to force mapping with PAGE_SIZE pages
Message-ID<qVjOy-1X6-23@gated-at.bofh.it>
In reply to#1317749
On 01/26/2016 03:14 AM, Mark Rutland wrote:
> Hi,
>
> On Mon, Jan 25, 2016 at 08:52:13AM -0800, Laura Abbott wrote:
>> Under some circumstances (e.g. debugging) it may be useful to have all
>> kernel memory mapped using PAGE_SIZE pages. Add an option for this.
>>
>> Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
>> ---
>>   arch/arm64/Kconfig  | 11 +++++++++++
>>   arch/arm64/mm/mmu.c | 10 +++++++---
>>   2 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index ffa3c54..faf7eac 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -504,6 +504,17 @@ config HOTPLUG_CPU
>>   	  Say Y here to experiment with turning CPUs off and on.  CPUs
>>   	  can be controlled through /sys/devices/system/cpu.
>>
>> +config FORCE_PAGES
>> +	bool "Force all memory mappings to be PAGE_SIZE"
>> +	help
>> +	  For performance reasons, kernel memory may be mapped with
>> +	  page table entries larger than the expected PAGE_SIZE. This results
>> +	  in better TLB performance but prevents adjustment of page table
>> +	  attributes at runtime. Say Y here to have all kernel memory mapped
>> +	  with PAGE_SIZE entries.
>> +
>> +	  If unsure, say N.
>> +
>
> I think we can make this a boot-time option, given that we're simply
> flipping a boolean that only directly affects a small amount of boot
> code. I doubt there would be a noticeable overhead, and it makes it more
> likely that we can use this to debug production kernels.
>
> Is there a usecase other than DEBUG_PAGEALLOC that you have in mind? If
> not, we could just hang this off of debug_pagealloc_enabled(), as we
> parse the early params before we create the usual page tables.
>

I only had DEBUG_PAGEALLOC use case in mind but I'm not sure about linking
the two completely. I'll see about adding a boot option instead of the
Kconfig option and then disabling debug_pagealloc if it isn't set.

Thanks,
Laura
  
> Thanks,
> Mark.
>
>>   source kernel/Kconfig.preempt
>>   source kernel/Kconfig.hz
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 2d6e7cf..450d38a 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -181,7 +181,8 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
>>   	do {
>>   		next = pmd_addr_end(addr, end);
>>   		/* try section mapping first */
>> -		if (((addr | next | phys) & ~SECTION_MASK) == 0) {
>> +		if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
>> +		      (!IS_ENABLED(CONFIG_FORCE_PAGES) || !pgtable_alloc)) {
>>   			pmd_t old_pmd =*pmd;
>>   			set_pmd(pmd, __pmd(phys |
>>   					   pgprot_val(mk_sect_prot(prot))));
>> @@ -208,8 +209,11 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
>>   }
>>
>>   static inline bool use_1G_block(unsigned long addr, unsigned long next,
>> -			unsigned long phys)
>> +			unsigned long phys, phys_addr_t (*pgtable_alloc)(void))
>>   {
>> +	if (pgtable_alloc && IS_ENABLED(CONFIG_FORCE_PAGES))
>> +		return false;
>> +
>>   	if (PAGE_SHIFT != 12)
>>   		return false;
>>
>> @@ -241,7 +245,7 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
>>   		/*
>>   		 * For 4K granule only, attempt to put down a 1GB block
>>   		 */
>> -		if (use_1G_block(addr, next, phys)) {
>> +		if (use_1G_block(addr, next, phys, pgtable_alloc)) {
>>   			pud_t old_pud = *pud;
>>   			set_pud(pud, __pud(phys |
>>   					   pgprot_val(mk_sect_prot(prot))));
>> --
>> 2.5.0
>>

[toc] | [prev] | [next] | [standalone]


#1317031 — [RFC][PATCH 1/4] arm64: Drop alloc function from create_mapping

FromLaura Abbott <labbott@fedoraproject.org>
Date2016-01-25 18:00 +0100
Subject[RFC][PATCH 1/4] arm64: Drop alloc function from create_mapping
Message-ID<qUSEI-6QD-67@gated-at.bofh.it>
In reply to#1317015
create_mapping is only used in fixmap_remap_fdt. All the create_mapping
calls need to happen on existing translation table pages without
additional allocations. Rather than have an alloc function be called
and fail, just set it to NULL and catch it's use.

Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
---
 arch/arm64/mm/mmu.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index e141762..2d6e7cf 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -116,7 +116,9 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
 	pte_t *pte;
 
 	if (pmd_none(*pmd) || pmd_sect(*pmd)) {
-		phys_addr_t pte_phys = pgtable_alloc();
+		phys_addr_t pte_phys;
+		BUG_ON(!pgtable_alloc);
+		pte_phys = pgtable_alloc();
 		pte = pte_fixmap(pte_phys);
 		if (pmd_sect(*pmd))
 			split_pmd(pmd, pte);
@@ -158,7 +160,9 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
 	 * Check for initial section mappings in the pgd/pud and remove them.
 	 */
 	if (pud_none(*pud) || pud_sect(*pud)) {
-		phys_addr_t pmd_phys = pgtable_alloc();
+		phys_addr_t pmd_phys;
+		BUG_ON(!pgtable_alloc);
+		pmd_phys = pgtable_alloc();
 		pmd = pmd_fixmap(pmd_phys);
 		if (pud_sect(*pud)) {
 			/*
@@ -223,7 +227,9 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
 	unsigned long next;
 
 	if (pgd_none(*pgd)) {
-		phys_addr_t pud_phys = pgtable_alloc();
+		phys_addr_t pud_phys;
+		BUG_ON(!pgtable_alloc);
+		pud_phys = pgtable_alloc();
 		__pgd_populate(pgd, pud_phys, PUD_TYPE_TABLE);
 	}
 	BUG_ON(pgd_bad(*pgd));
@@ -319,7 +325,7 @@ static void __init create_mapping(phys_addr_t phys, unsigned long virt,
 		return;
 	}
 	__create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,
-			     early_pgtable_alloc);
+			     NULL);
 }
 
 void __init create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1317766 — Re: [RFC][PATCH 1/4] arm64: Drop alloc function from create_mapping

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-26 12:30 +0100
SubjectRe: [RFC][PATCH 1/4] arm64: Drop alloc function from create_mapping
Message-ID<qV9YT-3ml-25@gated-at.bofh.it>
In reply to#1317031
On Mon, Jan 25, 2016 at 08:52:12AM -0800, Laura Abbott wrote:
> create_mapping is only used in fixmap_remap_fdt. All the create_mapping
> calls need to happen on existing translation table pages without
> additional allocations. Rather than have an alloc function be called
> and fail, just set it to NULL and catch it's use.

This looks sensible to me, though I think we should rename
create_mapping to something that makes it clear that not allocating
pages is a deliberate design decision, and thus it must be called with
care.

Perhaps create_mapping_noalloc?

Thanks,
Mark.

> Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
> ---
>  arch/arm64/mm/mmu.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index e141762..2d6e7cf 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -116,7 +116,9 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
>  	pte_t *pte;
>  
>  	if (pmd_none(*pmd) || pmd_sect(*pmd)) {
> -		phys_addr_t pte_phys = pgtable_alloc();
> +		phys_addr_t pte_phys;
> +		BUG_ON(!pgtable_alloc);
> +		pte_phys = pgtable_alloc();
>  		pte = pte_fixmap(pte_phys);
>  		if (pmd_sect(*pmd))
>  			split_pmd(pmd, pte);
> @@ -158,7 +160,9 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
>  	 * Check for initial section mappings in the pgd/pud and remove them.
>  	 */
>  	if (pud_none(*pud) || pud_sect(*pud)) {
> -		phys_addr_t pmd_phys = pgtable_alloc();
> +		phys_addr_t pmd_phys;
> +		BUG_ON(!pgtable_alloc);
> +		pmd_phys = pgtable_alloc();
>  		pmd = pmd_fixmap(pmd_phys);
>  		if (pud_sect(*pud)) {
>  			/*
> @@ -223,7 +227,9 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
>  	unsigned long next;
>  
>  	if (pgd_none(*pgd)) {
> -		phys_addr_t pud_phys = pgtable_alloc();
> +		phys_addr_t pud_phys;
> +		BUG_ON(!pgtable_alloc);
> +		pud_phys = pgtable_alloc();
>  		__pgd_populate(pgd, pud_phys, PUD_TYPE_TABLE);
>  	}
>  	BUG_ON(pgd_bad(*pgd));
> @@ -319,7 +325,7 @@ static void __init create_mapping(phys_addr_t phys, unsigned long virt,
>  		return;
>  	}
>  	__create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,
> -			     early_pgtable_alloc);
> +			     NULL);
>  }
>  
>  void __init create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
> -- 
> 2.5.0
> 

[toc] | [prev] | [next] | [standalone]


#1318405 — Re: [RFC][PATCH 1/4] arm64: Drop alloc function from create_mapping

FromLaura Abbott <labbott@redhat.com>
Date2016-01-26 22:30 +0100
SubjectRe: [RFC][PATCH 1/4] arm64: Drop alloc function from create_mapping
Message-ID<qVjlw-1L4-3@gated-at.bofh.it>
In reply to#1317766
On 01/26/2016 03:22 AM, Mark Rutland wrote:
> On Mon, Jan 25, 2016 at 08:52:12AM -0800, Laura Abbott wrote:
>> create_mapping is only used in fixmap_remap_fdt. All the create_mapping
>> calls need to happen on existing translation table pages without
>> additional allocations. Rather than have an alloc function be called
>> and fail, just set it to NULL and catch it's use.
>
> This looks sensible to me, though I think we should rename
> create_mapping to something that makes it clear that not allocating
> pages is a deliberate design decision, and thus it must be called with
> care.
>
> Perhaps create_mapping_noalloc?

I was wondering about that. That name sounds clearer.

>
> Thanks,
> Mark.
>
>> Signed-off-by: Laura Abbott <labbott@fedoraproject.org>
>> ---
>>   arch/arm64/mm/mmu.c | 14 ++++++++++----
>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index e141762..2d6e7cf 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -116,7 +116,9 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
>>   	pte_t *pte;
>>
>>   	if (pmd_none(*pmd) || pmd_sect(*pmd)) {
>> -		phys_addr_t pte_phys = pgtable_alloc();
>> +		phys_addr_t pte_phys;
>> +		BUG_ON(!pgtable_alloc);
>> +		pte_phys = pgtable_alloc();
>>   		pte = pte_fixmap(pte_phys);
>>   		if (pmd_sect(*pmd))
>>   			split_pmd(pmd, pte);
>> @@ -158,7 +160,9 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
>>   	 * Check for initial section mappings in the pgd/pud and remove them.
>>   	 */
>>   	if (pud_none(*pud) || pud_sect(*pud)) {
>> -		phys_addr_t pmd_phys = pgtable_alloc();
>> +		phys_addr_t pmd_phys;
>> +		BUG_ON(!pgtable_alloc);
>> +		pmd_phys = pgtable_alloc();
>>   		pmd = pmd_fixmap(pmd_phys);
>>   		if (pud_sect(*pud)) {
>>   			/*
>> @@ -223,7 +227,9 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
>>   	unsigned long next;
>>
>>   	if (pgd_none(*pgd)) {
>> -		phys_addr_t pud_phys = pgtable_alloc();
>> +		phys_addr_t pud_phys;
>> +		BUG_ON(!pgtable_alloc);
>> +		pud_phys = pgtable_alloc();
>>   		__pgd_populate(pgd, pud_phys, PUD_TYPE_TABLE);
>>   	}
>>   	BUG_ON(pgd_bad(*pgd));
>> @@ -319,7 +325,7 @@ static void __init create_mapping(phys_addr_t phys, unsigned long virt,
>>   		return;
>>   	}
>>   	__create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,
>> -			     early_pgtable_alloc);
>> +			     NULL);
>>   }
>>
>>   void __init create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
>> --
>> 2.5.0
>>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web