| Message ID | 1464702542-24394-2-git-send-email-mark.rutland@arm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 01, 2016 at 11:01:35AM +0800, Huang Shijie wrote: > On Tue, May 31, 2016 at 02:49:01PM +0100, Mark Rutland wrote: > > +struct ptdump_info { > > + struct mm_struct *mm; > > + const struct addr_marker *markers; > > + unsigned long base_addr; > > + unsigned long max_addr; > > +}; > > + > > +int ptdump_register(struct ptdump_info *info, const char *name); > Since we export this to other page tables, I guess the @base_addr in > the ptdump_info{} may not equal to the VA_START. Yes, that is the intent. The only requirement is that this is only VA_START or 0, as these are the only addresses aligned to VA_BITS which actually correspond to regions page tables can cover. > But the current dump.c does _NOT_ use the @start address been > passed in, it use the 0 as the start address for the walk_pgd/walk_pud/walk_pmd/walk_pte. Yes, this is deliberate. The trick is that start must be aligned to VA_BITS, and the page table accessors do the right thing, masking out bits which do not matter for their respective indices, e.g. #define pgd_index(addr) (((addr) >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1)) #define pgd_offset_raw(pgd, addr) ((pgd) + pgd_index(addr)) #define pgd_offset(mm, addr) (pgd_offset_raw((mm)->pgd, (addr))) This allows us to either provide a virtual address to the accessors (which can be in the low or high half), or an offset relative to the start of each pgd, pud, pmd, or pte. So for walk_pgd: static void walk_pgd(struct pg_state *st, struct mm_struct *mm, unsigned long start) { pgd_t *pgd = pgd_offset(mm, 0UL); unsigned i; unsigned long addr; for (i = 0; i < PTRS_PER_PGD; i++, pgd++) { addr = start + i * PGDIR_SIZE; if (pgd_none(*pgd)) { note_page(st, addr, 1, pgd_val(*pgd)); } else { BUG_ON(pgd_bad(*pgd)); walk_pud(st, pgd, addr); } } } Here, the 0UL we pass to pgd_offset is the offset from the start of the pgd, not the absolute virtual address. In the loop, we generate the virtual address each pgd_t corresponds to, and we pass this down to note_page or walk_pud as appropriate. We do likewise in walk_pud, walk_pmd, and walk_pte. So when we reach note_page, we should always have the right virtual address in the addr parameter. > It is wrong in logic, since the start address is VA_START, the code gets > the right result coincidentally. As above, I think that the logic is correct. Hopefully the explanation above allays your fears? Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, May 31, 2016 at 02:49:01PM +0100, Mark Rutland wrote: > For debugging purposes, it would be nice if we could export page tables > other than the swapper_pg_dir to userspace. To enable this, this patch > refactors the arm64 page table dumping code such that multiple tables > may be registered with the framework, and exported under debugfs. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Laura Abbott <labbott@fedoraproject.org> > Cc: Will Deacon <will.deacon@arm.com> Queued for 4.8. Thanks. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h new file mode 100644 index 0000000..07b8ed0 --- /dev/null +++ b/arch/arm64/include/asm/ptdump.h @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2014 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef __ASM_PTDUMP_H +#define __ASM_PTDUMP_H + +#ifdef CONFIG_ARM64_PTDUMP + +#include <linux/mm_types.h> + +struct addr_marker { + unsigned long start_address; + char *name; +}; + +struct ptdump_info { + struct mm_struct *mm; + const struct addr_marker *markers; + unsigned long base_addr; + unsigned long max_addr; +}; + +int ptdump_register(struct ptdump_info *info, const char *name); + +#else +static inline int ptdump_register(struct ptdump_info *info, const char *name) +{ + return 0; +} +#endif /* CONFIG_ARM64_PTDUMP */ + +#endif /* __ASM_PTDUMP_H */ diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c index 8404190..a56a7ad 100644 --- a/arch/arm64/mm/dump.c +++ b/arch/arm64/mm/dump.c @@ -27,11 +27,7 @@ #include <asm/memory.h> #include <asm/pgtable.h> #include <asm/pgtable-hwdef.h> - -struct addr_marker { - unsigned long start_address; - const char *name; -}; +#include <asm/ptdump.h> static const struct addr_marker address_markers[] = { #ifdef CONFIG_KASAN @@ -284,7 +280,8 @@ static void walk_pud(struct pg_state *st, pgd_t *pgd, unsigned long start) } } -static void walk_pgd(struct pg_state *st, struct mm_struct *mm, unsigned long start) +static void walk_pgd(struct pg_state *st, struct mm_struct *mm, + unsigned long start) { pgd_t *pgd = pgd_offset(mm, 0UL); unsigned i; @@ -303,12 +300,13 @@ static void walk_pgd(struct pg_state *st, struct mm_struct *mm, unsigned long st static int ptdump_show(struct seq_file *m, void *v) { + struct ptdump_info *info = m->private; struct pg_state st = { .seq = m, - .marker = address_markers, + .marker = info->markers, }; - walk_pgd(&st, &init_mm, VA_START); + walk_pgd(&st, info->mm, info->base_addr); note_page(&st, 0, 0, 0); return 0; @@ -316,7 +314,7 @@ static int ptdump_show(struct seq_file *m, void *v) static int ptdump_open(struct inode *inode, struct file *file) { - return single_open(file, ptdump_show, NULL); + return single_open(file, ptdump_show, inode->i_private); } static const struct file_operations ptdump_fops = { @@ -326,7 +324,7 @@ static const struct file_operations ptdump_fops = { .release = single_release, }; -static int ptdump_init(void) +int ptdump_register(struct ptdump_info *info, const char *name) { struct dentry *pe; unsigned i, j; @@ -336,8 +334,18 @@ static int ptdump_init(void) for (j = 0; j < pg_level[i].num; j++) pg_level[i].mask |= pg_level[i].bits[j].mask; - pe = debugfs_create_file("kernel_page_tables", 0400, NULL, NULL, - &ptdump_fops); + pe = debugfs_create_file(name, 0400, NULL, info, &ptdump_fops); return pe ? 0 : -ENOMEM; } + +static struct ptdump_info kernel_ptdump_info = { + .mm = &init_mm, + .markers = address_markers, + .base_addr = VA_START, +}; + +static int ptdump_init(void) +{ + return ptdump_register(&kernel_ptdump_info, "kernel_page_tables"); +} device_initcall(ptdump_init);