| Message ID | 20240909124721.1672199-3-sebastianene@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: ptdump: View the second stage page-tables | expand |
On Mon, Sep 09, 2024 at 12:47:18PM +0000, Sebastian Ene wrote: > Reuse the descriptor parsing functionality to keep the same output format > as the original ptdump code. In order for this to happen, move the state > tracking objects into a common header. > > Signed-off-by: Sebastian Ene <sebastianene@google.com> > --- > arch/arm64/include/asm/ptdump.h | 41 +++++++++++++++++++++++- > arch/arm64/mm/ptdump.c | 55 +++++++-------------------------- > 2 files changed, 51 insertions(+), 45 deletions(-) > > diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h > index 5b1701c76d1c..bd5d3ee3e8dc 100644 > --- a/arch/arm64/include/asm/ptdump.h > +++ b/arch/arm64/include/asm/ptdump.h > @@ -9,6 +9,7 @@ > > #include <linux/mm_types.h> > #include <linux/seq_file.h> > +#include <linux/ptdump.h> > > struct addr_marker { > unsigned long start_address; > @@ -21,14 +22,52 @@ struct ptdump_info { > unsigned long base_addr; > }; > > +struct ptdump_prot_bits { > + u64 mask; > + u64 val; > + const char *set; > + const char *clear; > +}; > + > +struct ptdump_pg_level { > + const struct ptdump_prot_bits *bits; > + char name[4]; > + int num; > + u64 mask; > +}; > + > +/* > + * The page dumper groups page table entries of the same type into a single > + * description. It uses pg_state to track the range information while > + * iterating over the pte entries. When the continuity is broken it then > + * dumps out a description of the range. > + */ > +struct ptdump_pg_state { > + struct ptdump_state ptdump; > + struct seq_file *seq; > + const struct addr_marker *marker; > + const struct mm_struct *mm; > + unsigned long start_address; > + int level; > + u64 current_prot; > + bool check_wx; > + unsigned long wx_pages; > + unsigned long uxn_pages; > +}; > + > void ptdump_walk(struct seq_file *s, struct ptdump_info *info); > +void note_page(struct ptdump_state *pt_st, unsigned long addr, int level, > + u64 val); > #ifdef CONFIG_PTDUMP_DEBUGFS > #define EFI_RUNTIME_MAP_END DEFAULT_MAP_WINDOW_64 > void __init ptdump_debugfs_register(struct ptdump_info *info, const char *name); > #else > static inline void ptdump_debugfs_register(struct ptdump_info *info, > const char *name) { } > -#endif > +#endif /* CONFIG_PTDUMP_DEBUGFS */ > +#else > +static inline void note_page(void *pt_st, unsigned long addr, > + int level, u64 val) { } nit: but why isn't 'pt_st' a pointer to 'struct ptdump_state'? Perhaps you should #include <linux/ptdump.h> before the #ifdef CONFIG_PTDUMP_CORE ? In any case, the meat of the patch is fine: Acked-by: Will Deacon <will@kernel.org> Will
On Tue, 10 Sep 2024 10:57:18 +0100, Will Deacon <will@kernel.org> wrote: > > On Mon, Sep 09, 2024 at 12:47:18PM +0000, Sebastian Ene wrote: > > Reuse the descriptor parsing functionality to keep the same output format > > as the original ptdump code. In order for this to happen, move the state > > tracking objects into a common header. > > > > Signed-off-by: Sebastian Ene <sebastianene@google.com> > > --- > > arch/arm64/include/asm/ptdump.h | 41 +++++++++++++++++++++++- > > arch/arm64/mm/ptdump.c | 55 +++++++-------------------------- > > 2 files changed, 51 insertions(+), 45 deletions(-) > > > > diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h > > index 5b1701c76d1c..bd5d3ee3e8dc 100644 > > --- a/arch/arm64/include/asm/ptdump.h > > +++ b/arch/arm64/include/asm/ptdump.h > > @@ -9,6 +9,7 @@ > > > > #include <linux/mm_types.h> > > #include <linux/seq_file.h> > > +#include <linux/ptdump.h> > > > > struct addr_marker { > > unsigned long start_address; > > @@ -21,14 +22,52 @@ struct ptdump_info { > > unsigned long base_addr; > > }; > > > > +struct ptdump_prot_bits { > > + u64 mask; > > + u64 val; > > + const char *set; > > + const char *clear; > > +}; > > + > > +struct ptdump_pg_level { > > + const struct ptdump_prot_bits *bits; > > + char name[4]; > > + int num; > > + u64 mask; > > +}; > > + > > +/* > > + * The page dumper groups page table entries of the same type into a single > > + * description. It uses pg_state to track the range information while > > + * iterating over the pte entries. When the continuity is broken it then > > + * dumps out a description of the range. > > + */ > > +struct ptdump_pg_state { > > + struct ptdump_state ptdump; > > + struct seq_file *seq; > > + const struct addr_marker *marker; > > + const struct mm_struct *mm; > > + unsigned long start_address; > > + int level; > > + u64 current_prot; > > + bool check_wx; > > + unsigned long wx_pages; > > + unsigned long uxn_pages; > > +}; > > + > > void ptdump_walk(struct seq_file *s, struct ptdump_info *info); > > +void note_page(struct ptdump_state *pt_st, unsigned long addr, int level, > > + u64 val); > > #ifdef CONFIG_PTDUMP_DEBUGFS > > #define EFI_RUNTIME_MAP_END DEFAULT_MAP_WINDOW_64 > > void __init ptdump_debugfs_register(struct ptdump_info *info, const char *name); > > #else > > static inline void ptdump_debugfs_register(struct ptdump_info *info, > > const char *name) { } > > -#endif > > +#endif /* CONFIG_PTDUMP_DEBUGFS */ > > +#else > > +static inline void note_page(void *pt_st, unsigned long addr, > > + int level, u64 val) { } > > nit: but why isn't 'pt_st' a pointer to 'struct ptdump_state'? > > Perhaps you should #include <linux/ptdump.h> before the #ifdef > CONFIG_PTDUMP_CORE ? Yup, that seems to do the trick. I'm folding this into the patch: diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h index 71a7ed01153a4..6cf4aae052191 100644 --- a/arch/arm64/include/asm/ptdump.h +++ b/arch/arm64/include/asm/ptdump.h @@ -5,11 +5,12 @@ #ifndef __ASM_PTDUMP_H #define __ASM_PTDUMP_H +#include <linux/ptdump.h> + #ifdef CONFIG_PTDUMP_CORE #include <linux/mm_types.h> #include <linux/seq_file.h> -#include <linux/ptdump.h> struct addr_marker { unsigned long start_address; @@ -67,7 +68,7 @@ static inline void ptdump_debugfs_register(struct ptdump_info *info, const char *name) { } #endif /* CONFIG_PTDUMP_DEBUGFS */ #else -static inline void note_page(void *pt_st, unsigned long addr, +static inline void note_page(struct ptdump_state *pt_st, unsigned long addr, int level, u64 val) { } #endif /* CONFIG_PTDUMP_CORE */ > In any case, the meat of the patch is fine: > > Acked-by: Will Deacon <will@kernel.org> Thanks, M.
diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h index 5b1701c76d1c..bd5d3ee3e8dc 100644 --- a/arch/arm64/include/asm/ptdump.h +++ b/arch/arm64/include/asm/ptdump.h @@ -9,6 +9,7 @@ #include <linux/mm_types.h> #include <linux/seq_file.h> +#include <linux/ptdump.h> struct addr_marker { unsigned long start_address; @@ -21,14 +22,52 @@ struct ptdump_info { unsigned long base_addr; }; +struct ptdump_prot_bits { + u64 mask; + u64 val; + const char *set; + const char *clear; +}; + +struct ptdump_pg_level { + const struct ptdump_prot_bits *bits; + char name[4]; + int num; + u64 mask; +}; + +/* + * The page dumper groups page table entries of the same type into a single + * description. It uses pg_state to track the range information while + * iterating over the pte entries. When the continuity is broken it then + * dumps out a description of the range. + */ +struct ptdump_pg_state { + struct ptdump_state ptdump; + struct seq_file *seq; + const struct addr_marker *marker; + const struct mm_struct *mm; + unsigned long start_address; + int level; + u64 current_prot; + bool check_wx; + unsigned long wx_pages; + unsigned long uxn_pages; +}; + void ptdump_walk(struct seq_file *s, struct ptdump_info *info); +void note_page(struct ptdump_state *pt_st, unsigned long addr, int level, + u64 val); #ifdef CONFIG_PTDUMP_DEBUGFS #define EFI_RUNTIME_MAP_END DEFAULT_MAP_WINDOW_64 void __init ptdump_debugfs_register(struct ptdump_info *info, const char *name); #else static inline void ptdump_debugfs_register(struct ptdump_info *info, const char *name) { } -#endif +#endif /* CONFIG_PTDUMP_DEBUGFS */ +#else +static inline void note_page(void *pt_st, unsigned long addr, + int level, u64 val) { } #endif /* CONFIG_PTDUMP_CORE */ #endif /* __ASM_PTDUMP_H */ diff --git a/arch/arm64/mm/ptdump.c b/arch/arm64/mm/ptdump.c index 6986827e0d64..404751fd30fe 100644 --- a/arch/arm64/mm/ptdump.c +++ b/arch/arm64/mm/ptdump.c @@ -38,33 +38,7 @@ seq_printf(m, fmt); \ }) -/* - * The page dumper groups page table entries of the same type into a single - * description. It uses pg_state to track the range information while - * iterating over the pte entries. When the continuity is broken it then - * dumps out a description of the range. - */ -struct pg_state { - struct ptdump_state ptdump; - struct seq_file *seq; - const struct addr_marker *marker; - const struct mm_struct *mm; - unsigned long start_address; - int level; - u64 current_prot; - bool check_wx; - unsigned long wx_pages; - unsigned long uxn_pages; -}; - -struct prot_bits { - u64 mask; - u64 val; - const char *set; - const char *clear; -}; - -static const struct prot_bits pte_bits[] = { +static const struct ptdump_prot_bits pte_bits[] = { { .mask = PTE_VALID, .val = PTE_VALID, @@ -143,14 +117,7 @@ static const struct prot_bits pte_bits[] = { } }; -struct pg_level { - const struct prot_bits *bits; - char name[4]; - int num; - u64 mask; -}; - -static struct pg_level pg_level[] __ro_after_init = { +static struct ptdump_pg_level pg_level[] __ro_after_init = { { /* pgd */ .name = "PGD", .bits = pte_bits, @@ -174,7 +141,7 @@ static struct pg_level pg_level[] __ro_after_init = { }, }; -static void dump_prot(struct pg_state *st, const struct prot_bits *bits, +static void dump_prot(struct ptdump_pg_state *st, const struct ptdump_prot_bits *bits, size_t num) { unsigned i; @@ -192,7 +159,7 @@ static void dump_prot(struct pg_state *st, const struct prot_bits *bits, } } -static void note_prot_uxn(struct pg_state *st, unsigned long addr) +static void note_prot_uxn(struct ptdump_pg_state *st, unsigned long addr) { if (!st->check_wx) return; @@ -206,7 +173,7 @@ static void note_prot_uxn(struct pg_state *st, unsigned long addr) st->uxn_pages += (addr - st->start_address) / PAGE_SIZE; } -static void note_prot_wx(struct pg_state *st, unsigned long addr) +static void note_prot_wx(struct ptdump_pg_state *st, unsigned long addr) { if (!st->check_wx) return; @@ -221,10 +188,10 @@ static void note_prot_wx(struct pg_state *st, unsigned long addr) st->wx_pages += (addr - st->start_address) / PAGE_SIZE; } -static void note_page(struct ptdump_state *pt_st, unsigned long addr, int level, - u64 val) +void note_page(struct ptdump_state *pt_st, unsigned long addr, int level, + u64 val) { - struct pg_state *st = container_of(pt_st, struct pg_state, ptdump); + struct ptdump_pg_state *st = container_of(pt_st, struct ptdump_pg_state, ptdump); static const char units[] = "KMGTPE"; u64 prot = 0; @@ -286,12 +253,12 @@ static void note_page(struct ptdump_state *pt_st, unsigned long addr, int level, void ptdump_walk(struct seq_file *s, struct ptdump_info *info) { unsigned long end = ~0UL; - struct pg_state st; + struct ptdump_pg_state st; if (info->base_addr < TASK_SIZE_64) end = TASK_SIZE_64; - st = (struct pg_state){ + st = (struct ptdump_pg_state){ .seq = s, .marker = info->markers, .mm = info->mm, @@ -324,7 +291,7 @@ static struct ptdump_info kernel_ptdump_info __ro_after_init = { bool ptdump_check_wx(void) { - struct pg_state st = { + struct ptdump_pg_state st = { .seq = NULL, .marker = (struct addr_marker[]) { { 0, NULL},
Reuse the descriptor parsing functionality to keep the same output format as the original ptdump code. In order for this to happen, move the state tracking objects into a common header. Signed-off-by: Sebastian Ene <sebastianene@google.com> --- arch/arm64/include/asm/ptdump.h | 41 +++++++++++++++++++++++- arch/arm64/mm/ptdump.c | 55 +++++++-------------------------- 2 files changed, 51 insertions(+), 45 deletions(-)