* [PATCH v2 1/6] arm64: ptdump: use static initializers for vmemmap region boundaries
2016-04-22 16:48 [PATCH v2 0/6] arm64/ARM pt dumper changes Ard Biesheuvel
@ 2016-04-22 16:48 ` Ard Biesheuvel
2016-04-22 17:07 ` Mark Rutland
2016-04-22 16:48 ` [PATCH v2 2/6] arm64: ptdump: add region marker for kasan shadow region Ard Biesheuvel
` (5 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2016-04-22 16:48 UTC (permalink / raw)
To: linux-arm-kernel
There is no need to initialize the vmemmap region boundaries dynamically,
since they are compile time constants. So just add these constants to the
global struct initializer, and drop the dynamic assignment and related code.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/mm/dump.c | 49 ++++++--------------
1 file changed, 13 insertions(+), 36 deletions(-)
diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index a21f47421b0c..493461159462 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -32,37 +32,21 @@ struct addr_marker {
const char *name;
};
-enum address_markers_idx {
- MODULES_START_NR = 0,
- MODULES_END_NR,
- VMALLOC_START_NR,
- VMALLOC_END_NR,
- FIXADDR_START_NR,
- FIXADDR_END_NR,
- PCI_START_NR,
- PCI_END_NR,
+static const struct addr_marker address_markers[] = {
+ { MODULES_VADDR, "Modules start" },
+ { MODULES_END, "Modules end" },
+ { VMALLOC_START, "vmalloc() Area" },
+ { VMALLOC_END, "vmalloc() End" },
+ { FIXADDR_START, "Fixmap start" },
+ { FIXADDR_TOP, "Fixmap end" },
+ { PCI_IO_START, "PCI I/O start" },
+ { PCI_IO_END, "PCI I/O end" },
#ifdef CONFIG_SPARSEMEM_VMEMMAP
- VMEMMAP_START_NR,
- VMEMMAP_END_NR,
+ { VMEMMAP_START, "vmemmap start" },
+ { VMEMMAP_START + VMEMMAP_SIZE, "vmemmap end" },
#endif
- KERNEL_SPACE_NR,
-};
-
-static struct addr_marker address_markers[] = {
- { MODULES_VADDR, "Modules start" },
- { MODULES_END, "Modules end" },
- { VMALLOC_START, "vmalloc() Area" },
- { VMALLOC_END, "vmalloc() End" },
- { FIXADDR_START, "Fixmap start" },
- { FIXADDR_TOP, "Fixmap end" },
- { PCI_IO_START, "PCI I/O start" },
- { PCI_IO_END, "PCI I/O end" },
-#ifdef CONFIG_SPARSEMEM_VMEMMAP
- { 0, "vmemmap start" },
- { 0, "vmemmap end" },
-#endif
- { PAGE_OFFSET, "Linear Mapping" },
- { -1, NULL },
+ { PAGE_OFFSET, "Linear Mapping" },
+ { -1, NULL },
};
/*
@@ -347,13 +331,6 @@ static int ptdump_init(void)
for (j = 0; j < pg_level[i].num; j++)
pg_level[i].mask |= pg_level[i].bits[j].mask;
-#ifdef CONFIG_SPARSEMEM_VMEMMAP
- address_markers[VMEMMAP_START_NR].start_address =
- (unsigned long)virt_to_page(PAGE_OFFSET);
- address_markers[VMEMMAP_END_NR].start_address =
- (unsigned long)virt_to_page(high_memory);
-#endif
-
pe = debugfs_create_file("kernel_page_tables", 0400, NULL, NULL,
&ptdump_fops);
return pe ? 0 : -ENOMEM;
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 1/6] arm64: ptdump: use static initializers for vmemmap region boundaries
2016-04-22 16:48 ` [PATCH v2 1/6] arm64: ptdump: use static initializers for vmemmap region boundaries Ard Biesheuvel
@ 2016-04-22 17:07 ` Mark Rutland
0 siblings, 0 replies; 19+ messages in thread
From: Mark Rutland @ 2016-04-22 17:07 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 22, 2016 at 06:48:03PM +0200, Ard Biesheuvel wrote:
> There is no need to initialize the vmemmap region boundaries dynamically,
> since they are compile time constants. So just add these constants to the
> global struct initializer, and drop the dynamic assignment and related code.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
There is a slight change in that we now print the VA boundaries for the
reserved vmemmap range rather than the populated vmemmap range, but that
makes more sense to me anyway. It's nice to see the sepaarate indices
go.
It may be worth defining VMEMMAP_END in the same header file as
VMEMMAP_{START,SIZE}, but either way:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> arch/arm64/mm/dump.c | 49 ++++++--------------
> 1 file changed, 13 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
> index a21f47421b0c..493461159462 100644
> --- a/arch/arm64/mm/dump.c
> +++ b/arch/arm64/mm/dump.c
> @@ -32,37 +32,21 @@ struct addr_marker {
> const char *name;
> };
>
> -enum address_markers_idx {
> - MODULES_START_NR = 0,
> - MODULES_END_NR,
> - VMALLOC_START_NR,
> - VMALLOC_END_NR,
> - FIXADDR_START_NR,
> - FIXADDR_END_NR,
> - PCI_START_NR,
> - PCI_END_NR,
> +static const struct addr_marker address_markers[] = {
> + { MODULES_VADDR, "Modules start" },
> + { MODULES_END, "Modules end" },
> + { VMALLOC_START, "vmalloc() Area" },
> + { VMALLOC_END, "vmalloc() End" },
> + { FIXADDR_START, "Fixmap start" },
> + { FIXADDR_TOP, "Fixmap end" },
> + { PCI_IO_START, "PCI I/O start" },
> + { PCI_IO_END, "PCI I/O end" },
> #ifdef CONFIG_SPARSEMEM_VMEMMAP
> - VMEMMAP_START_NR,
> - VMEMMAP_END_NR,
> + { VMEMMAP_START, "vmemmap start" },
> + { VMEMMAP_START + VMEMMAP_SIZE, "vmemmap end" },
> #endif
> - KERNEL_SPACE_NR,
> -};
> -
> -static struct addr_marker address_markers[] = {
> - { MODULES_VADDR, "Modules start" },
> - { MODULES_END, "Modules end" },
> - { VMALLOC_START, "vmalloc() Area" },
> - { VMALLOC_END, "vmalloc() End" },
> - { FIXADDR_START, "Fixmap start" },
> - { FIXADDR_TOP, "Fixmap end" },
> - { PCI_IO_START, "PCI I/O start" },
> - { PCI_IO_END, "PCI I/O end" },
> -#ifdef CONFIG_SPARSEMEM_VMEMMAP
> - { 0, "vmemmap start" },
> - { 0, "vmemmap end" },
> -#endif
> - { PAGE_OFFSET, "Linear Mapping" },
> - { -1, NULL },
> + { PAGE_OFFSET, "Linear Mapping" },
> + { -1, NULL },
> };
>
> /*
> @@ -347,13 +331,6 @@ static int ptdump_init(void)
> for (j = 0; j < pg_level[i].num; j++)
> pg_level[i].mask |= pg_level[i].bits[j].mask;
>
> -#ifdef CONFIG_SPARSEMEM_VMEMMAP
> - address_markers[VMEMMAP_START_NR].start_address =
> - (unsigned long)virt_to_page(PAGE_OFFSET);
> - address_markers[VMEMMAP_END_NR].start_address =
> - (unsigned long)virt_to_page(high_memory);
> -#endif
> -
> pe = debugfs_create_file("kernel_page_tables", 0400, NULL, NULL,
> &ptdump_fops);
> return pe ? 0 : -ENOMEM;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 2/6] arm64: ptdump: add region marker for kasan shadow region
2016-04-22 16:48 [PATCH v2 0/6] arm64/ARM pt dumper changes Ard Biesheuvel
2016-04-22 16:48 ` [PATCH v2 1/6] arm64: ptdump: use static initializers for vmemmap region boundaries Ard Biesheuvel
@ 2016-04-22 16:48 ` Ard Biesheuvel
2016-04-22 17:09 ` Mark Rutland
2016-04-22 16:48 ` [PATCH v2 3/6] arm64: ptdump: fold string literals into address_markers[] and pte_bits[] Ard Biesheuvel
` (4 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2016-04-22 16:48 UTC (permalink / raw)
To: linux-arm-kernel
Annotate the KASAN shadow region with boundary markers, so that its
mappings stand out in the page table dumper output.
Signed-off-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 493461159462..8404190fe2bd 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -23,6 +23,7 @@
#include <linux/seq_file.h>
#include <asm/fixmap.h>
+#include <asm/kasan.h>
#include <asm/memory.h>
#include <asm/pgtable.h>
#include <asm/pgtable-hwdef.h>
@@ -33,6 +34,10 @@ struct addr_marker {
};
static const struct addr_marker address_markers[] = {
+#ifdef CONFIG_KASAN
+ { KASAN_SHADOW_START, "Kasan shadow start" },
+ { KASAN_SHADOW_END, "Kasan shadow end" },
+#endif
{ MODULES_VADDR, "Modules start" },
{ MODULES_END, "Modules end" },
{ VMALLOC_START, "vmalloc() Area" },
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 2/6] arm64: ptdump: add region marker for kasan shadow region
2016-04-22 16:48 ` [PATCH v2 2/6] arm64: ptdump: add region marker for kasan shadow region Ard Biesheuvel
@ 2016-04-22 17:09 ` Mark Rutland
0 siblings, 0 replies; 19+ messages in thread
From: Mark Rutland @ 2016-04-22 17:09 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 22, 2016 at 06:48:04PM +0200, Ard Biesheuvel wrote:
> Annotate the KASAN shadow region with boundary markers, so that its
> mappings stand out in the page table dumper output.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-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 493461159462..8404190fe2bd 100644
> --- a/arch/arm64/mm/dump.c
> +++ b/arch/arm64/mm/dump.c
> @@ -23,6 +23,7 @@
> #include <linux/seq_file.h>
>
> #include <asm/fixmap.h>
> +#include <asm/kasan.h>
> #include <asm/memory.h>
> #include <asm/pgtable.h>
> #include <asm/pgtable-hwdef.h>
> @@ -33,6 +34,10 @@ struct addr_marker {
> };
>
> static const struct addr_marker address_markers[] = {
> +#ifdef CONFIG_KASAN
> + { KASAN_SHADOW_START, "Kasan shadow start" },
> + { KASAN_SHADOW_END, "Kasan shadow end" },
> +#endif
> { MODULES_VADDR, "Modules start" },
> { MODULES_END, "Modules end" },
> { VMALLOC_START, "vmalloc() Area" },
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 3/6] arm64: ptdump: fold string literals into address_markers[] and pte_bits[]
2016-04-22 16:48 [PATCH v2 0/6] arm64/ARM pt dumper changes Ard Biesheuvel
2016-04-22 16:48 ` [PATCH v2 1/6] arm64: ptdump: use static initializers for vmemmap region boundaries Ard Biesheuvel
2016-04-22 16:48 ` [PATCH v2 2/6] arm64: ptdump: add region marker for kasan shadow region Ard Biesheuvel
@ 2016-04-22 16:48 ` Ard Biesheuvel
2016-04-22 16:48 ` [PATCH v2 4/6] arm64: mm: dump: make page table dumping reusable Ard Biesheuvel
` (3 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2016-04-22 16:48 UTC (permalink / raw)
To: linux-arm-kernel
Replace the indirect string references in struct addr_marker and struct
prot_bits with char array fields. Under a relocatable kernel, this not
only saves 8 bytes in the absolute address that we no longer have to record,
it also saves 24 bytes of init data (the size of a RELA entry) since this
address no longer requires relocation processing at boot.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
I am aware that this is a drop in the ocean, but I thought I'd include it
anyway, since many people may not even realize that this issue exists in
the first place.
---
arch/arm64/mm/dump.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index 8404190fe2bd..024f803e2fb4 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -30,7 +30,7 @@
struct addr_marker {
unsigned long start_address;
- const char *name;
+ char name[32];
};
static const struct addr_marker address_markers[] = {
@@ -51,7 +51,7 @@ static const struct addr_marker address_markers[] = {
{ VMEMMAP_START + VMEMMAP_SIZE, "vmemmap end" },
#endif
{ PAGE_OFFSET, "Linear Mapping" },
- { -1, NULL },
+ { -1 },
};
/*
@@ -69,10 +69,10 @@ struct pg_state {
};
struct prot_bits {
- u64 mask;
- u64 val;
- const char *set;
- const char *clear;
+ u64 mask;
+ u64 val;
+ char set[16];
+ char clear[8];
};
static const struct prot_bits pte_bits[] = {
@@ -184,7 +184,7 @@ static void dump_prot(struct pg_state *st, const struct prot_bits *bits,
else
s = bits->clear;
- if (s)
+ if (*s)
seq_printf(st->seq, " %s", s);
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 4/6] arm64: mm: dump: make page table dumping reusable
2016-04-22 16:48 [PATCH v2 0/6] arm64/ARM pt dumper changes Ard Biesheuvel
` (2 preceding siblings ...)
2016-04-22 16:48 ` [PATCH v2 3/6] arm64: ptdump: fold string literals into address_markers[] and pte_bits[] Ard Biesheuvel
@ 2016-04-22 16:48 ` Ard Biesheuvel
2016-04-25 11:07 ` Will Deacon
2016-04-22 16:48 ` [PATCH v2 5/6] arm: " Ard Biesheuvel
` (2 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2016-04-22 16:48 UTC (permalink / raw)
To: linux-arm-kernel
From: Mark Rutland <mark.rutland@arm.com>
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>
---
arch/arm64/include/asm/ptdump.h | 44 ++++++++++++++++++++
arch/arm64/mm/dump.c | 32 ++++++++------
2 files changed, 64 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
new file mode 100644
index 000000000000..fe82d693638a
--- /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[32];
+};
+
+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 024f803e2fb4..73d7227d1403 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;
- char name[32];
-};
+#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);
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 4/6] arm64: mm: dump: make page table dumping reusable
2016-04-22 16:48 ` [PATCH v2 4/6] arm64: mm: dump: make page table dumping reusable Ard Biesheuvel
@ 2016-04-25 11:07 ` Will Deacon
2016-05-13 12:56 ` Mark Rutland
0 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2016-04-25 11:07 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard, Mark,
On Fri, Apr 22, 2016 at 06:48:06PM +0200, Ard Biesheuvel wrote:
> From: Mark Rutland <mark.rutland@arm.com>
>
> 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>
> ---
> arch/arm64/include/asm/ptdump.h | 44 ++++++++++++++++++++
> arch/arm64/mm/dump.c | 32 ++++++++------
> 2 files changed, 64 insertions(+), 12 deletions(-)
Could you reorder this with patch 3, please? I'm unsure about the benefit
of the very specific string literal changes, but the ptdump stuff here
certainly looks beneficial.
I can queue the first two patches as-is.
Thanks.
Will
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/6] arm64: mm: dump: make page table dumping reusable
2016-04-25 11:07 ` Will Deacon
@ 2016-05-13 12:56 ` Mark Rutland
2016-05-31 13:25 ` Will Deacon
0 siblings, 1 reply; 19+ messages in thread
From: Mark Rutland @ 2016-05-13 12:56 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Apr 25, 2016 at 12:07:57PM +0100, Will Deacon wrote:
> Hi Ard, Mark,
>
> On Fri, Apr 22, 2016 at 06:48:06PM +0200, Ard Biesheuvel wrote:
> > From: Mark Rutland <mark.rutland@arm.com>
> >
> > 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>
> > ---
> > arch/arm64/include/asm/ptdump.h | 44 ++++++++++++++++++++
> > arch/arm64/mm/dump.c | 32 ++++++++------
> > 2 files changed, 64 insertions(+), 12 deletions(-)
>
> Could you reorder this with patch 3, please? I'm unsure about the benefit
> of the very specific string literal changes, but the ptdump stuff here
> certainly looks beneficial.
I'm sure this is far too late, but for posterity I've rebased patch 4
below atop of pathes 1 & 2 as queued in the arm64 for-next/core branch,
without patch 3. It builds cleanly, and I've given it a spin on
Juno.
Will, I guess I should rebase and resend this after we hit v4.7-rc1?
Thanks,
Mark.
---->8----
>From f17b52c0f004614027c356c848fd21308f4177c1 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Fri, 22 Apr 2016 18:48:06 +0200
Subject: [PATCH] arm64: mm: dump: make page table dumping reusable
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>
---
arch/arm64/include/asm/ptdump.h | 44 +++++++++++++++++++++++++++++++++++++++++
arch/arm64/mm/dump.c | 32 +++++++++++++++++++-----------
2 files changed, 64 insertions(+), 12 deletions(-)
create mode 100644 arch/arm64/include/asm/ptdump.h
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);
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 4/6] arm64: mm: dump: make page table dumping reusable
2016-05-13 12:56 ` Mark Rutland
@ 2016-05-31 13:25 ` Will Deacon
0 siblings, 0 replies; 19+ messages in thread
From: Will Deacon @ 2016-05-31 13:25 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, May 13, 2016 at 01:56:07PM +0100, Mark Rutland wrote:
> On Mon, Apr 25, 2016 at 12:07:57PM +0100, Will Deacon wrote:
> > Hi Ard, Mark,
> >
> > On Fri, Apr 22, 2016 at 06:48:06PM +0200, Ard Biesheuvel wrote:
> > > From: Mark Rutland <mark.rutland@arm.com>
> > >
> > > 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>
> > > ---
> > > arch/arm64/include/asm/ptdump.h | 44 ++++++++++++++++++++
> > > arch/arm64/mm/dump.c | 32 ++++++++------
> > > 2 files changed, 64 insertions(+), 12 deletions(-)
> >
> > Could you reorder this with patch 3, please? I'm unsure about the benefit
> > of the very specific string literal changes, but the ptdump stuff here
> > certainly looks beneficial.
>
> I'm sure this is far too late, but for posterity I've rebased patch 4
> below atop of pathes 1 & 2 as queued in the arm64 for-next/core branch,
> without patch 3. It builds cleanly, and I've given it a spin on
> Juno.
>
> Will, I guess I should rebase and resend this after we hit v4.7-rc1?
Yes, please.
Will
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 5/6] arm: mm: dump: make page table dumping reusable
2016-04-22 16:48 [PATCH v2 0/6] arm64/ARM pt dumper changes Ard Biesheuvel
` (3 preceding siblings ...)
2016-04-22 16:48 ` [PATCH v2 4/6] arm64: mm: dump: make page table dumping reusable Ard Biesheuvel
@ 2016-04-22 16:48 ` Ard Biesheuvel
2016-04-22 16:48 ` [PATCH v2 6/6] efi/arm*: add support to dump the EFI page tables Ard Biesheuvel
2016-04-26 11:29 ` [PATCH v2 0/6] arm64/ARM pt dumper changes Will Deacon
6 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2016-04-22 16:48 UTC (permalink / raw)
To: linux-arm-kernel
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 ARM page table dumping code such that multiple tables
may be registered with the framework, and exported under debugfs.
(this is a port of Mark Rutland's arm64 patch that does the same)
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm/include/asm/ptdump.h | 44 ++++++++++++++
arch/arm/mm/dump.c | 60 ++++++++++++--------
2 files changed, 81 insertions(+), 23 deletions(-)
diff --git a/arch/arm/include/asm/ptdump.h b/arch/arm/include/asm/ptdump.h
new file mode 100644
index 000000000000..2df5737b2cc7
--- /dev/null
+++ b/arch/arm/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_ARM_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_ARM_PTDUMP */
+
+#endif /* __ASM_PTDUMP_H */
diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
index 9fe8e241335c..0c1ff61440aa 100644
--- a/arch/arm/mm/dump.c
+++ b/arch/arm/mm/dump.c
@@ -19,11 +19,7 @@
#include <asm/fixmap.h>
#include <asm/pgtable.h>
-
-struct addr_marker {
- unsigned long start_address;
- const char *name;
-};
+#include <asm/ptdump.h>
static struct addr_marker address_markers[] = {
{ MODULES_VADDR, "Modules" },
@@ -266,7 +262,8 @@ static void walk_pte(struct pg_state *st, pmd_t *pmd, unsigned long start)
}
}
-static void walk_pmd(struct pg_state *st, pud_t *pud, unsigned long start)
+static void walk_pmd(struct pg_state *st, pud_t *pud, unsigned long start,
+ unsigned long max)
{
pmd_t *pmd = pmd_offset(pud, 0);
unsigned long addr;
@@ -274,6 +271,8 @@ static void walk_pmd(struct pg_state *st, pud_t *pud, unsigned long start)
for (i = 0; i < PTRS_PER_PMD; i++, pmd++) {
addr = start + i * PMD_SIZE;
+ if (addr > max)
+ break;
if (pmd_none(*pmd) || pmd_large(*pmd) || !pmd_present(*pmd))
note_page(st, addr, 3, pmd_val(*pmd));
else
@@ -284,7 +283,8 @@ static void walk_pmd(struct pg_state *st, pud_t *pud, unsigned long start)
}
}
-static void walk_pud(struct pg_state *st, pgd_t *pgd, unsigned long start)
+static void walk_pud(struct pg_state *st, pgd_t *pgd, unsigned long start,
+ unsigned long max)
{
pud_t *pud = pud_offset(pgd, 0);
unsigned long addr;
@@ -292,46 +292,51 @@ static void walk_pud(struct pg_state *st, pgd_t *pgd, unsigned long start)
for (i = 0; i < PTRS_PER_PUD; i++, pud++) {
addr = start + i * PUD_SIZE;
+ if (addr > max)
+ break;
if (!pud_none(*pud)) {
- walk_pmd(st, pud, addr);
+ walk_pmd(st, pud, addr, max);
} else {
note_page(st, addr, 2, pud_val(*pud));
}
}
}
-static void walk_pgd(struct seq_file *m)
+static void walk_pgd(struct pg_state *st, struct mm_struct *mm, unsigned long max)
{
- pgd_t *pgd = swapper_pg_dir;
- struct pg_state st;
+ pgd_t *pgd = pgd_offset(mm, 0UL);
unsigned long addr;
unsigned i;
- memset(&st, 0, sizeof(st));
- st.seq = m;
- st.marker = address_markers;
-
for (i = 0; i < PTRS_PER_PGD; i++, pgd++) {
addr = i * PGDIR_SIZE;
+ if (addr > max)
+ break;
if (!pgd_none(*pgd)) {
- walk_pud(&st, pgd, addr);
+ walk_pud(st, pgd, addr, max);
} else {
- note_page(&st, addr, 1, pgd_val(*pgd));
+ note_page(st, addr, 1, pgd_val(*pgd));
}
}
- note_page(&st, 0, 0, 0);
+ note_page(st, 0, 0, 0);
}
static int ptdump_show(struct seq_file *m, void *v)
{
- walk_pgd(m);
+ struct ptdump_info *info = m->private;
+ struct pg_state st = {
+ .seq = m,
+ .marker = info->markers,
+ };
+
+ walk_pgd(&st, info->mm, info->max_addr);
return 0;
}
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 = {
@@ -341,7 +346,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;
@@ -353,8 +358,17 @@ static int ptdump_init(void)
address_markers[2].start_address = VMALLOC_START;
- 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,
+};
+
+static int ptdump_init(void)
+{
+ return ptdump_register(&kernel_ptdump_info, "kernel_page_tables");
+}
__initcall(ptdump_init);
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 6/6] efi/arm*: add support to dump the EFI page tables
2016-04-22 16:48 [PATCH v2 0/6] arm64/ARM pt dumper changes Ard Biesheuvel
` (4 preceding siblings ...)
2016-04-22 16:48 ` [PATCH v2 5/6] arm: " Ard Biesheuvel
@ 2016-04-22 16:48 ` Ard Biesheuvel
2016-04-22 17:01 ` Mark Rutland
2016-04-26 11:29 ` [PATCH v2 0/6] arm64/ARM pt dumper changes Will Deacon
6 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2016-04-22 16:48 UTC (permalink / raw)
To: linux-arm-kernel
From: Mark Rutland <mark.rutland@arm.com>
Occasionally it's useful to inspect the page tables used for EFI runtime
services, similarly to those cases where it's useful to be able to
inspect the kernel page tables.
Now that the kernel page table dump code has been made generic, use it
to export the EFI runtime services page tables under debugfs (as
"efi_page_tables").
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
[ ardb: mildly refactored for compatibility with ARM ]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
drivers/firmware/efi/arm-runtime.c | 21 ++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 6ae21e41a429..c48fc2f5413b 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -27,6 +27,7 @@
#include <asm/mmu.h>
#include <asm/pgalloc.h>
#include <asm/pgtable.h>
+#include <asm/ptdump.h>
extern u64 efi_system_table;
@@ -133,3 +134,23 @@ void efi_virtmap_unload(void)
efi_set_pgd(current->active_mm);
preempt_enable();
}
+
+static const struct addr_marker efi_addr_markers[] = {
+ { 0, "EFI runtime services" },
+ { -1 },
+};
+
+static struct ptdump_info efi_ptdump_info = {
+ .mm = &efi_mm,
+ .markers = efi_addr_markers,
+ .base_addr = 0,
+ .max_addr = SZ_1G,
+};
+
+static int efi_ptdump_init(void)
+{
+ if (!efi_enabled(EFI_RUNTIME_SERVICES))
+ return 0;
+ return ptdump_register(&efi_ptdump_info, "efi_page_tables");
+}
+device_initcall(efi_ptdump_init);
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 6/6] efi/arm*: add support to dump the EFI page tables
2016-04-22 16:48 ` [PATCH v2 6/6] efi/arm*: add support to dump the EFI page tables Ard Biesheuvel
@ 2016-04-22 17:01 ` Mark Rutland
2016-04-22 17:20 ` Ard Biesheuvel
0 siblings, 1 reply; 19+ messages in thread
From: Mark Rutland @ 2016-04-22 17:01 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 22, 2016 at 06:48:08PM +0200, Ard Biesheuvel wrote:
> From: Mark Rutland <mark.rutland@arm.com>
>
> Occasionally it's useful to inspect the page tables used for EFI runtime
> services, similarly to those cases where it's useful to be able to
> inspect the kernel page tables.
>
> Now that the kernel page table dump code has been made generic, use it
> to export the EFI runtime services page tables under debugfs (as
> "efi_page_tables").
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> [ ardb: mildly refactored for compatibility with ARM ]
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> drivers/firmware/efi/arm-runtime.c | 21 ++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
> index 6ae21e41a429..c48fc2f5413b 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -27,6 +27,7 @@
> #include <asm/mmu.h>
> #include <asm/pgalloc.h>
> #include <asm/pgtable.h>
> +#include <asm/ptdump.h>
>
> extern u64 efi_system_table;
>
> @@ -133,3 +134,23 @@ void efi_virtmap_unload(void)
> efi_set_pgd(current->active_mm);
> preempt_enable();
> }
> +
> +static const struct addr_marker efi_addr_markers[] = {
> + { 0, "EFI runtime services" },
> + { -1 },
> +};
> +
> +static struct ptdump_info efi_ptdump_info = {
> + .mm = &efi_mm,
> + .markers = efi_addr_markers,
> + .base_addr = 0,
> + .max_addr = SZ_1G,
> +};
I see that max_addr isn't used for arm64, and for ARM it's only used in
one place. It doesn't seem great to have that on arm64 given it's
unused.
Do we actually need max_addr? Is there any reason not to always dump
whole tables?
I guess you're trying to avoid dumping the kernel VA range on 32-bit?
Thanks,
Mark.
> +
> +static int efi_ptdump_init(void)
> +{
> + if (!efi_enabled(EFI_RUNTIME_SERVICES))
> + return 0;
> + return ptdump_register(&efi_ptdump_info, "efi_page_tables");
> +}
> +device_initcall(efi_ptdump_init);
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v2 6/6] efi/arm*: add support to dump the EFI page tables
2016-04-22 17:01 ` Mark Rutland
@ 2016-04-22 17:20 ` Ard Biesheuvel
2016-04-22 17:25 ` Mark Rutland
0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2016-04-22 17:20 UTC (permalink / raw)
To: linux-arm-kernel
On 22 April 2016 at 19:01, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Apr 22, 2016 at 06:48:08PM +0200, Ard Biesheuvel wrote:
>> From: Mark Rutland <mark.rutland@arm.com>
>>
>> Occasionally it's useful to inspect the page tables used for EFI runtime
>> services, similarly to those cases where it's useful to be able to
>> inspect the kernel page tables.
>>
>> Now that the kernel page table dump code has been made generic, use it
>> to export the EFI runtime services page tables under debugfs (as
>> "efi_page_tables").
>>
>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>> [ ardb: mildly refactored for compatibility with ARM ]
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> drivers/firmware/efi/arm-runtime.c | 21 ++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
>> index 6ae21e41a429..c48fc2f5413b 100644
>> --- a/drivers/firmware/efi/arm-runtime.c
>> +++ b/drivers/firmware/efi/arm-runtime.c
>> @@ -27,6 +27,7 @@
>> #include <asm/mmu.h>
>> #include <asm/pgalloc.h>
>> #include <asm/pgtable.h>
>> +#include <asm/ptdump.h>
>>
>> extern u64 efi_system_table;
>>
>> @@ -133,3 +134,23 @@ void efi_virtmap_unload(void)
>> efi_set_pgd(current->active_mm);
>> preempt_enable();
>> }
>> +
>> +static const struct addr_marker efi_addr_markers[] = {
>> + { 0, "EFI runtime services" },
>> + { -1 },
>> +};
>> +
>> +static struct ptdump_info efi_ptdump_info = {
>> + .mm = &efi_mm,
>> + .markers = efi_addr_markers,
>> + .base_addr = 0,
>> + .max_addr = SZ_1G,
>> +};
>
> I see that max_addr isn't used for arm64, and for ARM it's only used in
> one place. It doesn't seem great to have that on arm64 given it's
> unused.
>
> Do we actually need max_addr? Is there any reason not to always dump
> whole tables?
>
> I guess you're trying to avoid dumping the kernel VA range on 32-bit?
>
Indeed. On ARM, the efi_page_tables dumps its copy of
kernel_page_tables. If that is OK (the information could potentially
be useful, I suppose) we can drop the max
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v2 6/6] efi/arm*: add support to dump the EFI page tables
2016-04-22 17:20 ` Ard Biesheuvel
@ 2016-04-22 17:25 ` Mark Rutland
2016-04-25 11:26 ` Ard Biesheuvel
0 siblings, 1 reply; 19+ messages in thread
From: Mark Rutland @ 2016-04-22 17:25 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 22, 2016 at 07:20:38PM +0200, Ard Biesheuvel wrote:
> On 22 April 2016 at 19:01, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Fri, Apr 22, 2016 at 06:48:08PM +0200, Ard Biesheuvel wrote:
> >> +static struct ptdump_info efi_ptdump_info = {
> >> + .mm = &efi_mm,
> >> + .markers = efi_addr_markers,
> >> + .base_addr = 0,
> >> + .max_addr = SZ_1G,
> >> +};
> >
> > I see that max_addr isn't used for arm64, and for ARM it's only used in
> > one place. It doesn't seem great to have that on arm64 given it's
> > unused.
> >
> > Do we actually need max_addr? Is there any reason not to always dump
> > whole tables?
> >
> > I guess you're trying to avoid dumping the kernel VA range on 32-bit?
>
> Indeed. On ARM, the efi_page_tables dumps its copy of
> kernel_page_tables. If that is OK (the information could potentially
> be useful, I suppose) we can drop the max
I'd argue it's always worth dumping the full tables (dropipng the max).
For instance, we could have some bug that leaves the kernel VA range in
the EFI tables inconsistent with what we expect/require, and having that
exposed would make that clear.
Regardless, arbitrarily limiting the VA range of the arm64 dump doesn't
seem great.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v2 6/6] efi/arm*: add support to dump the EFI page tables
2016-04-22 17:25 ` Mark Rutland
@ 2016-04-25 11:26 ` Ard Biesheuvel
2016-04-27 20:12 ` Russell King - ARM Linux
0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2016-04-25 11:26 UTC (permalink / raw)
To: linux-arm-kernel
On 22 April 2016 at 19:25, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Apr 22, 2016 at 07:20:38PM +0200, Ard Biesheuvel wrote:
>> On 22 April 2016 at 19:01, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Fri, Apr 22, 2016 at 06:48:08PM +0200, Ard Biesheuvel wrote:
>
>> >> +static struct ptdump_info efi_ptdump_info = {
>> >> + .mm = &efi_mm,
>> >> + .markers = efi_addr_markers,
>> >> + .base_addr = 0,
>> >> + .max_addr = SZ_1G,
>> >> +};
>> >
>> > I see that max_addr isn't used for arm64, and for ARM it's only used in
>> > one place. It doesn't seem great to have that on arm64 given it's
>> > unused.
>> >
>> > Do we actually need max_addr? Is there any reason not to always dump
>> > whole tables?
>> >
>> > I guess you're trying to avoid dumping the kernel VA range on 32-bit?
>>
>> Indeed. On ARM, the efi_page_tables dumps its copy of
>> kernel_page_tables. If that is OK (the information could potentially
>> be useful, I suppose) we can drop the max
>
> I'd argue it's always worth dumping the full tables (dropipng the max).
>
> For instance, we could have some bug that leaves the kernel VA range in
> the EFI tables inconsistent with what we expect/require, and having that
> exposed would make that clear.
>
Well, the problem here is that we only sync the EFI page tables with
the kernel page tables in switch_mm() (if they turn out to be out of
sync). This means we should either sync them in this code path as
well, or remove the kernel VA range from the output, since we'll be
dumping known stale data otherwise.
> Regardless, arbitrarily limiting the VA range of the arm64 dump doesn't
> seem great.
>
True. [0 .. TASK_SIZE) looks like a useful and not arbitrary range to
limit the EFI page table output to. Unfortunately, TASK_SIZE resolves
to a value that depends on TIF_32BIT, so will still need some
preprocessor conditional to select between TASK_SIZE_64 (on arm64) and
TASK_SIZE (on ARM)
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v2 6/6] efi/arm*: add support to dump the EFI page tables
2016-04-25 11:26 ` Ard Biesheuvel
@ 2016-04-27 20:12 ` Russell King - ARM Linux
0 siblings, 0 replies; 19+ messages in thread
From: Russell King - ARM Linux @ 2016-04-27 20:12 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Apr 25, 2016 at 01:26:07PM +0200, Ard Biesheuvel wrote:
> Well, the problem here is that we only sync the EFI page tables with
> the kernel page tables in switch_mm() (if they turn out to be out of
> sync). This means we should either sync them in this code path as
> well, or remove the kernel VA range from the output, since we'll be
> dumping known stale data otherwise.
The kernel VA range can always be different - even at task switch time
we don't always update it. We only update when there's something that
needs to be removed from the page table, and we're running on UP systems.
--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 0/6] arm64/ARM pt dumper changes
2016-04-22 16:48 [PATCH v2 0/6] arm64/ARM pt dumper changes Ard Biesheuvel
` (5 preceding siblings ...)
2016-04-22 16:48 ` [PATCH v2 6/6] efi/arm*: add support to dump the EFI page tables Ard Biesheuvel
@ 2016-04-26 11:29 ` Will Deacon
2016-04-26 11:34 ` Ard Biesheuvel
6 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2016-04-26 11:29 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 22, 2016 at 06:48:02PM +0200, Ard Biesheuvel wrote:
> This series updates the kernel page table dumper to:
> - not initialize the vmemmap region markers dynamically
> - show region boundaries for the kasan shadow region
> - include the UEFI runtime services mappings
I've merged the first two patches of this series. I can take patch 4 too
if you can rebase onto for-next/core.
Cheers,
Will
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v2 0/6] arm64/ARM pt dumper changes
2016-04-26 11:29 ` [PATCH v2 0/6] arm64/ARM pt dumper changes Will Deacon
@ 2016-04-26 11:34 ` Ard Biesheuvel
0 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2016-04-26 11:34 UTC (permalink / raw)
To: linux-arm-kernel
On 26 April 2016 at 13:29, Will Deacon <will.deacon@arm.com> wrote:
> On Fri, Apr 22, 2016 at 06:48:02PM +0200, Ard Biesheuvel wrote:
>> This series updates the kernel page table dumper to:
>> - not initialize the vmemmap region markers dynamically
>> - show region boundaries for the kasan shadow region
>> - include the UEFI runtime services mappings
>
> I've merged the first two patches of this series. I can take patch 4 too
> if you can rebase onto for-next/core.
>
Thanks, but it is not very useful without the last patch. I am trying
to respin the code so it can be shared between ARM and arm64 in a way
that addresses Mark's concerns. But since this is debug code anyway, I
am not sure when I will get around to doing that.
Thanks,
Ard.
^ permalink raw reply [flat|nested] 19+ messages in thread