* [PATCHv3 1/3] arm64: Drop alloc function from create_mapping [not found] <1454615017-24672-1-git-send-email-labbott@fedoraproject.org> @ 2016-02-04 19:43 ` Laura Abbott 2016-02-04 19:43 ` [PATCHv2 2/3] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC Laura Abbott 2016-02-04 19:43 ` [PATCHv3 3/3] arm64: ptdump: Indicate whether memory should be faulting Laura Abbott 2 siblings, 0 replies; 8+ messages in thread From: Laura Abbott @ 2016-02-04 19:43 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel Cc: Laura Abbott, linux-arm-kernel, linux-kernel 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 its use. Also change the name to create_mapping_noalloc to better capture what exactly is going on. Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Laura Abbott <labbott@fedoraproject.org> --- arch/arm64/mm/mmu.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index 7711554..ef0d66c 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_set_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_set_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)); @@ -312,7 +318,12 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys, init_pgd(pgd_offset_raw(pgdir, virt), phys, virt, size, prot, alloc); } -static void __init create_mapping(phys_addr_t phys, unsigned long virt, +/* + * This function can only be used to modify existing table entries, + * without allocating new levels of table. Note that this permits the + * creation of new section or page entries. + */ +static void __init create_mapping_noalloc(phys_addr_t phys, unsigned long virt, phys_addr_t size, pgprot_t prot) { if (virt < VMALLOC_START) { @@ -321,7 +332,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, @@ -680,7 +691,7 @@ void *__init fixmap_remap_fdt(phys_addr_t dt_phys) /* * Make sure that the FDT region can be mapped without the need to * allocate additional translation table pages, so that it is safe - * to call create_mapping() this early. + * to call create_mapping_noalloc() this early. * * On 64k pages, the FDT will be mapped using PTEs, so we need to * be in the same PMD as the rest of the fixmap. @@ -696,8 +707,8 @@ void *__init fixmap_remap_fdt(phys_addr_t dt_phys) dt_virt = (void *)dt_virt_base + offset; /* map the first chunk so we can read the size from the header */ - create_mapping(round_down(dt_phys, SWAPPER_BLOCK_SIZE), dt_virt_base, - SWAPPER_BLOCK_SIZE, prot); + create_mapping_noalloc(round_down(dt_phys, SWAPPER_BLOCK_SIZE), + dt_virt_base, SWAPPER_BLOCK_SIZE, prot); if (fdt_check_header(dt_virt) != 0) return NULL; @@ -707,7 +718,7 @@ void *__init fixmap_remap_fdt(phys_addr_t dt_phys) return NULL; if (offset + size > SWAPPER_BLOCK_SIZE) - create_mapping(round_down(dt_phys, SWAPPER_BLOCK_SIZE), dt_virt_base, + create_mapping_noalloc(round_down(dt_phys, SWAPPER_BLOCK_SIZE), dt_virt_base, round_up(offset + size, SWAPPER_BLOCK_SIZE), prot); memblock_reserve(dt_phys, size); -- 2.5.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCHv2 2/3] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC [not found] <1454615017-24672-1-git-send-email-labbott@fedoraproject.org> 2016-02-04 19:43 ` [PATCHv3 1/3] arm64: Drop alloc function from create_mapping Laura Abbott @ 2016-02-04 19:43 ` Laura Abbott 2016-02-05 12:05 ` Ard Biesheuvel 2016-02-05 14:20 ` Mark Rutland 2016-02-04 19:43 ` [PATCHv3 3/3] arm64: ptdump: Indicate whether memory should be faulting Laura Abbott 2 siblings, 2 replies; 8+ messages in thread From: Laura Abbott @ 2016-02-04 19:43 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel Cc: Laura Abbott, linux-arm-kernel, linux-kernel ARCH_SUPPORTS_DEBUG_PAGEALLOC provides a hook to map and unmap pages for debugging purposes. This requires memory be mapped with PAGE_SIZE mappings since breaking down larger mappings at runtime will lead to TLB conflicts. Check if debug_pagealloc is enabled at runtime and if so, map everyting with PAGE_SIZE pages. Implement the functions to actually map/unmap the pages at runtime. Signed-off-by: Laura Abbott <labbott@fedoraproject.org> --- arch/arm64/Kconfig | 3 +++ arch/arm64/mm/mmu.c | 25 +++++++++++++++++++++---- arch/arm64/mm/pageattr.c | 46 ++++++++++++++++++++++++++++++++++++---------- 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 8cc6228..0f33218 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -537,6 +537,9 @@ config HOTPLUG_CPU source kernel/Kconfig.preempt source kernel/Kconfig.hz +config ARCH_SUPPORTS_DEBUG_PAGEALLOC + def_bool y + config ARCH_HAS_HOLES_MEMORYMODEL def_bool y if SPARSEMEM diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index ef0d66c..be81a59 100644 --- a/arch/arm64/mm/mmu.c +++ b/arch/arm64/mm/mmu.c @@ -180,8 +180,14 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end, pmd = pmd_set_fixmap_offset(pud, addr); do { next = pmd_addr_end(addr, end); - /* try section mapping first */ - if (((addr | next | phys) & ~SECTION_MASK) == 0) { + /* + * try a section mapping first + * + * See comment in use_1G_block for why we need the check + * for !pgtable_alloc with !debug_pagealloc + */ + if (((addr | next | phys) & ~SECTION_MASK) == 0 && + (!debug_pagealloc_enabled() || !pgtable_alloc)) { pmd_t old_pmd =*pmd; set_pmd(pmd, __pmd(phys | pgprot_val(mk_sect_prot(prot)))); @@ -208,8 +214,19 @@ 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 debug_page_alloc is enabled we don't want to be using sections + * since everything needs to be mapped with pages. The catch is + * that we only want to force pages if we can allocate the next + * layer of page tables. If there is no pgtable_alloc function, + * it's too early to allocate another layer and we should use + * section mappings. + */ + if (pgtable_alloc && debug_pagealloc_enabled()) + return false; + if (PAGE_SHIFT != 12) return false; @@ -241,7 +258,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)))); diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c index 1360a02..57877af 100644 --- a/arch/arm64/mm/pageattr.c +++ b/arch/arm64/mm/pageattr.c @@ -37,14 +37,31 @@ static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr, return 0; } +/* + * This function assumes that the range is mapped with PAGE_SIZE pages. + */ +static int __change_memory_common(unsigned long start, unsigned long size, + pgprot_t set_mask, pgprot_t clear_mask) +{ + struct page_change_data data; + int ret; + + data.set_mask = set_mask; + data.clear_mask = clear_mask; + + ret = apply_to_page_range(&init_mm, start, size, change_page_range, + &data); + + flush_tlb_kernel_range(start, start+size); + return ret; +} + static int change_memory_common(unsigned long addr, int numpages, pgprot_t set_mask, pgprot_t clear_mask) { unsigned long start = addr; unsigned long size = PAGE_SIZE*numpages; unsigned long end = start + size; - int ret; - struct page_change_data data; struct vm_struct *area; if (!PAGE_ALIGNED(addr)) { @@ -72,14 +89,7 @@ static int change_memory_common(unsigned long addr, int numpages, !(area->flags & VM_ALLOC)) return -EINVAL; - data.set_mask = set_mask; - data.clear_mask = clear_mask; - - ret = apply_to_page_range(&init_mm, start, size, change_page_range, - &data); - - flush_tlb_kernel_range(start, end); - return ret; + return __change_memory_common(start, size, set_mask, clear_mask); } int set_memory_ro(unsigned long addr, int numpages) @@ -111,3 +121,19 @@ int set_memory_x(unsigned long addr, int numpages) __pgprot(PTE_PXN)); } EXPORT_SYMBOL_GPL(set_memory_x); + +#ifdef CONFIG_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, PAGE_SIZE*numpages, + __pgprot(PTE_VALID), + __pgprot(0)); + else + __change_memory_common(addr, PAGE_SIZE*numpages, + __pgprot(0), + __pgprot(PTE_VALID)); +} +#endif -- 2.5.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCHv2 2/3] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC 2016-02-04 19:43 ` [PATCHv2 2/3] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC Laura Abbott @ 2016-02-05 12:05 ` Ard Biesheuvel 2016-02-05 14:20 ` Mark Rutland 1 sibling, 0 replies; 8+ messages in thread From: Ard Biesheuvel @ 2016-02-05 12:05 UTC (permalink / raw) To: Laura Abbott Cc: Catalin Marinas, Will Deacon, Mark Rutland, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Hi Laura, On 4 February 2016 at 20:43, Laura Abbott <labbott@fedoraproject.org> wrote: > > ARCH_SUPPORTS_DEBUG_PAGEALLOC provides a hook to map and unmap > pages for debugging purposes. This requires memory be mapped > with PAGE_SIZE mappings since breaking down larger mappings > at runtime will lead to TLB conflicts. Check if debug_pagealloc > is enabled at runtime and if so, map everyting with PAGE_SIZE > pages. Implement the functions to actually map/unmap the > pages at runtime. > > Signed-off-by: Laura Abbott <labbott@fedoraproject.org> This looks correct to me, but I still have a concern below. > --- > arch/arm64/Kconfig | 3 +++ > arch/arm64/mm/mmu.c | 25 +++++++++++++++++++++---- > arch/arm64/mm/pageattr.c | 46 ++++++++++++++++++++++++++++++++++++---------- > 3 files changed, 60 insertions(+), 14 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 8cc6228..0f33218 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -537,6 +537,9 @@ config HOTPLUG_CPU > source kernel/Kconfig.preempt > source kernel/Kconfig.hz > > +config ARCH_SUPPORTS_DEBUG_PAGEALLOC > + def_bool y > + > config ARCH_HAS_HOLES_MEMORYMODEL > def_bool y if SPARSEMEM > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index ef0d66c..be81a59 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -180,8 +180,14 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end, > pmd = pmd_set_fixmap_offset(pud, addr); > do { > next = pmd_addr_end(addr, end); > - /* try section mapping first */ > - if (((addr | next | phys) & ~SECTION_MASK) == 0) { > + /* > + * try a section mapping first > + * > + * See comment in use_1G_block for why we need the check > + * for !pgtable_alloc with !debug_pagealloc > + */ > + if (((addr | next | phys) & ~SECTION_MASK) == 0 && > + (!debug_pagealloc_enabled() || !pgtable_alloc)) { > pmd_t old_pmd =*pmd; > set_pmd(pmd, __pmd(phys | > pgprot_val(mk_sect_prot(prot)))); > @@ -208,8 +214,19 @@ 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 debug_page_alloc is enabled we don't want to be using sections > + * since everything needs to be mapped with pages. The catch is > + * that we only want to force pages if we can allocate the next > + * layer of page tables. If there is no pgtable_alloc function, > + * it's too early to allocate another layer and we should use > + * section mappings. > + */ > + if (pgtable_alloc && debug_pagealloc_enabled()) > + return false; > + I would suggest you stick this comment and test in a separate function 'bool block_mappings_allowed(phys_addr_t (*pgtable_alloc)(void))' (or a better name if you can think of one, by all means) and call it from both sites where you need to perform the check. This keeps the symmetry, and removes the need to change the prototype of use_1G_block() to pass pgtable_alloc only to test it for NULL-ness With that change: Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro,.org> > if (PAGE_SHIFT != 12) > return false; > > @@ -241,7 +258,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)))); > diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c > index 1360a02..57877af 100644 > --- a/arch/arm64/mm/pageattr.c > +++ b/arch/arm64/mm/pageattr.c > @@ -37,14 +37,31 @@ static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr, > return 0; > } > > +/* > + * This function assumes that the range is mapped with PAGE_SIZE pages. > + */ > +static int __change_memory_common(unsigned long start, unsigned long size, > + pgprot_t set_mask, pgprot_t clear_mask) > +{ > + struct page_change_data data; > + int ret; > + > + data.set_mask = set_mask; > + data.clear_mask = clear_mask; > + > + ret = apply_to_page_range(&init_mm, start, size, change_page_range, > + &data); > + > + flush_tlb_kernel_range(start, start+size); > + return ret; > +} > + > static int change_memory_common(unsigned long addr, int numpages, > pgprot_t set_mask, pgprot_t clear_mask) > { > unsigned long start = addr; > unsigned long size = PAGE_SIZE*numpages; > unsigned long end = start + size; > - int ret; > - struct page_change_data data; > struct vm_struct *area; > > if (!PAGE_ALIGNED(addr)) { > @@ -72,14 +89,7 @@ static int change_memory_common(unsigned long addr, int numpages, > !(area->flags & VM_ALLOC)) > return -EINVAL; > > - data.set_mask = set_mask; > - data.clear_mask = clear_mask; > - > - ret = apply_to_page_range(&init_mm, start, size, change_page_range, > - &data); > - > - flush_tlb_kernel_range(start, end); > - return ret; > + return __change_memory_common(start, size, set_mask, clear_mask); > } > > int set_memory_ro(unsigned long addr, int numpages) > @@ -111,3 +121,19 @@ int set_memory_x(unsigned long addr, int numpages) > __pgprot(PTE_PXN)); > } > EXPORT_SYMBOL_GPL(set_memory_x); > + > +#ifdef CONFIG_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, PAGE_SIZE*numpages, > + __pgprot(PTE_VALID), > + __pgprot(0)); > + else > + __change_memory_common(addr, PAGE_SIZE*numpages, > + __pgprot(0), > + __pgprot(PTE_VALID)); > +} > +#endif > -- > 2.5.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCHv2 2/3] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC 2016-02-04 19:43 ` [PATCHv2 2/3] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC Laura Abbott 2016-02-05 12:05 ` Ard Biesheuvel @ 2016-02-05 14:20 ` Mark Rutland 2016-02-06 0:08 ` Laura Abbott 1 sibling, 1 reply; 8+ messages in thread From: Mark Rutland @ 2016-02-05 14:20 UTC (permalink / raw) To: Laura Abbott Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, linux-arm-kernel, linux-kernel On Thu, Feb 04, 2016 at 11:43:36AM -0800, Laura Abbott wrote: > > ARCH_SUPPORTS_DEBUG_PAGEALLOC provides a hook to map and unmap > pages for debugging purposes. This requires memory be mapped > with PAGE_SIZE mappings since breaking down larger mappings > at runtime will lead to TLB conflicts. Check if debug_pagealloc > is enabled at runtime and if so, map everyting with PAGE_SIZE > pages. Implement the functions to actually map/unmap the > pages at runtime. > > Signed-off-by: Laura Abbott <labbott@fedoraproject.org> I've given this a spin on Juno, with and without the config option selected, and with and without the command line option. I've also given it a spin on Seattle with inline KASAN also enabled. I wasn't sure how to deliberately trigger a failure, but those all booted fine, and the dumepd page tables looks right, so FWIW: Tested-by: Mark Rutland <mark.rutland@arm.com> I have a few minor comments below, and with those fixed up: Reviewed-by: Mark Rutland <mark.rutland@arm.com> > --- > arch/arm64/Kconfig | 3 +++ > arch/arm64/mm/mmu.c | 25 +++++++++++++++++++++---- > arch/arm64/mm/pageattr.c | 46 ++++++++++++++++++++++++++++++++++++---------- > 3 files changed, 60 insertions(+), 14 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 8cc6228..0f33218 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -537,6 +537,9 @@ config HOTPLUG_CPU > source kernel/Kconfig.preempt > source kernel/Kconfig.hz > > +config ARCH_SUPPORTS_DEBUG_PAGEALLOC > + def_bool y > + > config ARCH_HAS_HOLES_MEMORYMODEL > def_bool y if SPARSEMEM > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index ef0d66c..be81a59 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -180,8 +180,14 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end, > pmd = pmd_set_fixmap_offset(pud, addr); > do { > next = pmd_addr_end(addr, end); > - /* try section mapping first */ > - if (((addr | next | phys) & ~SECTION_MASK) == 0) { > + /* > + * try a section mapping first > + * > + * See comment in use_1G_block for why we need the check > + * for !pgtable_alloc with !debug_pagealloc > + */ > + if (((addr | next | phys) & ~SECTION_MASK) == 0 && > + (!debug_pagealloc_enabled() || !pgtable_alloc)) { > pmd_t old_pmd =*pmd; > set_pmd(pmd, __pmd(phys | > pgprot_val(mk_sect_prot(prot)))); > @@ -208,8 +214,19 @@ 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 debug_page_alloc is enabled we don't want to be using sections > + * since everything needs to be mapped with pages. The catch is > + * that we only want to force pages if we can allocate the next > + * layer of page tables. If there is no pgtable_alloc function, > + * it's too early to allocate another layer and we should use > + * section mappings. > + */ I'm not sure this quite captures the rationale, as we only care about the linear map using pages (AFAIK), and the earliness only matters w.r.t. the DTB mapping. How about: /* * If debug_page_alloc is enabled we must map the linear map * using pages. However, other mappings created by * create_mapping_noalloc must use sections in some cases. Allow * sections to be used in those cases, where no pgtable_alloc * function is provided. */ Does that sound ok to you? As a future optimisation, I think we can allow sections when mapping permanent kernel chunks (.e.g .rodata and .text), as these shouldn't contain pages available for dynamic allocation. That would require using something other than the presence of pgtable_alloc to determine when we should force page usage. > + if (pgtable_alloc && debug_pagealloc_enabled()) > + return false; > + > if (PAGE_SHIFT != 12) > return false; > > @@ -241,7 +258,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)))); > diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c > index 1360a02..57877af 100644 > --- a/arch/arm64/mm/pageattr.c > +++ b/arch/arm64/mm/pageattr.c > @@ -37,14 +37,31 @@ static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr, > return 0; > } > > +/* > + * This function assumes that the range is mapped with PAGE_SIZE pages. > + */ > +static int __change_memory_common(unsigned long start, unsigned long size, > + pgprot_t set_mask, pgprot_t clear_mask) > +{ > + struct page_change_data data; > + int ret; > + > + data.set_mask = set_mask; > + data.clear_mask = clear_mask; > + > + ret = apply_to_page_range(&init_mm, start, size, change_page_range, > + &data); > + > + flush_tlb_kernel_range(start, start+size); Nit: spaces around '+' please. > + return ret; > +} > + > static int change_memory_common(unsigned long addr, int numpages, > pgprot_t set_mask, pgprot_t clear_mask) > { > unsigned long start = addr; > unsigned long size = PAGE_SIZE*numpages; > unsigned long end = start + size; > - int ret; > - struct page_change_data data; > struct vm_struct *area; > > if (!PAGE_ALIGNED(addr)) { > @@ -72,14 +89,7 @@ static int change_memory_common(unsigned long addr, int numpages, > !(area->flags & VM_ALLOC)) > return -EINVAL; > > - data.set_mask = set_mask; > - data.clear_mask = clear_mask; > - > - ret = apply_to_page_range(&init_mm, start, size, change_page_range, > - &data); > - > - flush_tlb_kernel_range(start, end); > - return ret; > + return __change_memory_common(start, size, set_mask, clear_mask); > } > > int set_memory_ro(unsigned long addr, int numpages) > @@ -111,3 +121,19 @@ int set_memory_x(unsigned long addr, int numpages) > __pgprot(PTE_PXN)); > } > EXPORT_SYMBOL_GPL(set_memory_x); > + > +#ifdef CONFIG_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, PAGE_SIZE*numpages, > + __pgprot(PTE_VALID), > + __pgprot(0)); > + else > + __change_memory_common(addr, PAGE_SIZE*numpages, > + __pgprot(0), > + __pgprot(PTE_VALID)); Nit: spaces around each '*' here, please. Thanks, Mark. > +} > +#endif > -- > 2.5.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCHv2 2/3] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC 2016-02-05 14:20 ` Mark Rutland @ 2016-02-06 0:08 ` Laura Abbott 0 siblings, 0 replies; 8+ messages in thread From: Laura Abbott @ 2016-02-06 0:08 UTC (permalink / raw) To: Mark Rutland, Laura Abbott Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, linux-arm-kernel, linux-kernel On 02/05/2016 06:20 AM, Mark Rutland wrote: > On Thu, Feb 04, 2016 at 11:43:36AM -0800, Laura Abbott wrote: >> >> ARCH_SUPPORTS_DEBUG_PAGEALLOC provides a hook to map and unmap >> pages for debugging purposes. This requires memory be mapped >> with PAGE_SIZE mappings since breaking down larger mappings >> at runtime will lead to TLB conflicts. Check if debug_pagealloc >> is enabled at runtime and if so, map everyting with PAGE_SIZE >> pages. Implement the functions to actually map/unmap the >> pages at runtime. >> >> Signed-off-by: Laura Abbott <labbott@fedoraproject.org> > > I've given this a spin on Juno, with and without the config option > selected, and with and without the command line option. I've also given > it a spin on Seattle with inline KASAN also enabled. > > I wasn't sure how to deliberately trigger a failure, but those all > booted fine, and the dumepd page tables looks right, so FWIW: > I wrote a test that does a write after free on an allocated page for my testing. Might be worth it to look into adding a test to the lkdtm module. I also did testing with 64K pages but couldn't test on 16K due to lack of hardware and QEMU running off into the weeds. > Tested-by: Mark Rutland <mark.rutland@arm.com> > > I have a few minor comments below, and with those fixed up: > > Reviewed-by: Mark Rutland <mark.rutland@arm.com> > >> --- >> arch/arm64/Kconfig | 3 +++ >> arch/arm64/mm/mmu.c | 25 +++++++++++++++++++++---- >> arch/arm64/mm/pageattr.c | 46 ++++++++++++++++++++++++++++++++++++---------- >> 3 files changed, 60 insertions(+), 14 deletions(-) >> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index 8cc6228..0f33218 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -537,6 +537,9 @@ config HOTPLUG_CPU >> source kernel/Kconfig.preempt >> source kernel/Kconfig.hz >> >> +config ARCH_SUPPORTS_DEBUG_PAGEALLOC >> + def_bool y >> + >> config ARCH_HAS_HOLES_MEMORYMODEL >> def_bool y if SPARSEMEM >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> index ef0d66c..be81a59 100644 >> --- a/arch/arm64/mm/mmu.c >> +++ b/arch/arm64/mm/mmu.c >> @@ -180,8 +180,14 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end, >> pmd = pmd_set_fixmap_offset(pud, addr); >> do { >> next = pmd_addr_end(addr, end); >> - /* try section mapping first */ >> - if (((addr | next | phys) & ~SECTION_MASK) == 0) { >> + /* >> + * try a section mapping first >> + * >> + * See comment in use_1G_block for why we need the check >> + * for !pgtable_alloc with !debug_pagealloc >> + */ >> + if (((addr | next | phys) & ~SECTION_MASK) == 0 && >> + (!debug_pagealloc_enabled() || !pgtable_alloc)) { >> pmd_t old_pmd =*pmd; >> set_pmd(pmd, __pmd(phys | >> pgprot_val(mk_sect_prot(prot)))); >> @@ -208,8 +214,19 @@ 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 debug_page_alloc is enabled we don't want to be using sections >> + * since everything needs to be mapped with pages. The catch is >> + * that we only want to force pages if we can allocate the next >> + * layer of page tables. If there is no pgtable_alloc function, >> + * it's too early to allocate another layer and we should use >> + * section mappings. >> + */ > > I'm not sure this quite captures the rationale, as we only care about > the linear map using pages (AFAIK), and the earliness only matters > w.r.t. the DTB mapping. How about: > > /* > * If debug_page_alloc is enabled we must map the linear map > * using pages. However, other mappings created by > * create_mapping_noalloc must use sections in some cases. Allow > * sections to be used in those cases, where no pgtable_alloc > * function is provided. > */ > > Does that sound ok to you? > > As a future optimisation, I think we can allow sections when mapping > permanent kernel chunks (.e.g .rodata and .text), as these shouldn't > contain pages available for dynamic allocation. That would require using > something other than the presence of pgtable_alloc to determine when we > should force page usage. > >> + if (pgtable_alloc && debug_pagealloc_enabled()) >> + return false; >> + >> if (PAGE_SHIFT != 12) >> return false; >> >> @@ -241,7 +258,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)))); >> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c >> index 1360a02..57877af 100644 >> --- a/arch/arm64/mm/pageattr.c >> +++ b/arch/arm64/mm/pageattr.c >> @@ -37,14 +37,31 @@ static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr, >> return 0; >> } >> >> +/* >> + * This function assumes that the range is mapped with PAGE_SIZE pages. >> + */ >> +static int __change_memory_common(unsigned long start, unsigned long size, >> + pgprot_t set_mask, pgprot_t clear_mask) >> +{ >> + struct page_change_data data; >> + int ret; >> + >> + data.set_mask = set_mask; >> + data.clear_mask = clear_mask; >> + >> + ret = apply_to_page_range(&init_mm, start, size, change_page_range, >> + &data); >> + >> + flush_tlb_kernel_range(start, start+size); > > Nit: spaces around '+' please. > >> + return ret; >> +} >> + >> static int change_memory_common(unsigned long addr, int numpages, >> pgprot_t set_mask, pgprot_t clear_mask) >> { >> unsigned long start = addr; >> unsigned long size = PAGE_SIZE*numpages; >> unsigned long end = start + size; >> - int ret; >> - struct page_change_data data; >> struct vm_struct *area; >> >> if (!PAGE_ALIGNED(addr)) { >> @@ -72,14 +89,7 @@ static int change_memory_common(unsigned long addr, int numpages, >> !(area->flags & VM_ALLOC)) >> return -EINVAL; >> >> - data.set_mask = set_mask; >> - data.clear_mask = clear_mask; >> - >> - ret = apply_to_page_range(&init_mm, start, size, change_page_range, >> - &data); >> - >> - flush_tlb_kernel_range(start, end); >> - return ret; >> + return __change_memory_common(start, size, set_mask, clear_mask); >> } >> >> int set_memory_ro(unsigned long addr, int numpages) >> @@ -111,3 +121,19 @@ int set_memory_x(unsigned long addr, int numpages) >> __pgprot(PTE_PXN)); >> } >> EXPORT_SYMBOL_GPL(set_memory_x); >> + >> +#ifdef CONFIG_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, PAGE_SIZE*numpages, >> + __pgprot(PTE_VALID), >> + __pgprot(0)); >> + else >> + __change_memory_common(addr, PAGE_SIZE*numpages, >> + __pgprot(0), >> + __pgprot(PTE_VALID)); > > Nit: spaces around each '*' here, please. > > Thanks, > Mark. > >> +} >> +#endif >> -- >> 2.5.0 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCHv3 3/3] arm64: ptdump: Indicate whether memory should be faulting [not found] <1454615017-24672-1-git-send-email-labbott@fedoraproject.org> 2016-02-04 19:43 ` [PATCHv3 1/3] arm64: Drop alloc function from create_mapping Laura Abbott 2016-02-04 19:43 ` [PATCHv2 2/3] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC Laura Abbott @ 2016-02-04 19:43 ` Laura Abbott 2016-02-05 12:05 ` Ard Biesheuvel 2016-02-05 14:34 ` Mark Rutland 2 siblings, 2 replies; 8+ messages in thread From: Laura Abbott @ 2016-02-04 19:43 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel Cc: Laura Abbott, linux-arm-kernel, linux-kernel 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> --- 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCHv3 3/3] arm64: ptdump: Indicate whether memory should be faulting 2016-02-04 19:43 ` [PATCHv3 3/3] arm64: ptdump: Indicate whether memory should be faulting Laura Abbott @ 2016-02-05 12:05 ` Ard Biesheuvel 2016-02-05 14:34 ` Mark Rutland 1 sibling, 0 replies; 8+ messages in thread From: Ard Biesheuvel @ 2016-02-05 12:05 UTC (permalink / raw) To: Laura Abbott Cc: Catalin Marinas, Will Deacon, Mark Rutland, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org On 4 February 2016 at 20:43, Laura Abbott <labbott@fedoraproject.org> wrote: > > 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> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCHv3 3/3] arm64: ptdump: Indicate whether memory should be faulting 2016-02-04 19:43 ` [PATCHv3 3/3] arm64: ptdump: Indicate whether memory should be faulting Laura Abbott 2016-02-05 12:05 ` Ard Biesheuvel @ 2016-02-05 14:34 ` Mark Rutland 1 sibling, 0 replies; 8+ messages in thread From: Mark Rutland @ 2016-02-05 14:34 UTC (permalink / raw) To: Laura Abbott Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, linux-arm-kernel, linux-kernel On Thu, Feb 04, 2016 at 11:43:37AM -0800, Laura Abbott wrote: > > 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> This looks sensible to me, and worked in testing. Reviewed-by: Mark Rutland <mark.rutland@arm.com> Tested-by: Mark Rutland <mark.rutland@arm.com> Mark. > --- > 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-02-06 0:08 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1454615017-24672-1-git-send-email-labbott@fedoraproject.org>
2016-02-04 19:43 ` [PATCHv3 1/3] arm64: Drop alloc function from create_mapping Laura Abbott
2016-02-04 19:43 ` [PATCHv2 2/3] arm64: Add support for ARCH_SUPPORTS_DEBUG_PAGEALLOC Laura Abbott
2016-02-05 12:05 ` Ard Biesheuvel
2016-02-05 14:20 ` Mark Rutland
2016-02-06 0:08 ` Laura Abbott
2016-02-04 19:43 ` [PATCHv3 3/3] arm64: ptdump: Indicate whether memory should be faulting Laura Abbott
2016-02-05 12:05 ` Ard Biesheuvel
2016-02-05 14:34 ` Mark Rutland
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).