Re: [PATCH v7 2/6] arm64: ptdump: Expose the attribute parsing functionality
From: Will Deacon <will@kernel.org>
Date: 2024-07-05 11:07:55
Also in:
kvmarm, lkml
On Fri, Jun 21, 2024 at 12:32:26PM +0000, Sebastian Ene wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- arch/arm64/include/asm/ptdump.h | 41 ++++++++++++++++++++++++++++++++- arch/arm64/mm/ptdump.c | 37 ++--------------------------- 2 files changed, 42 insertions(+), 36 deletions(-)diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h index 5b1701c76d1c..c550b2afcab7 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 prot_bits { + u64 mask; + u64 val; + const char *set; + const char *clear; +}; + +struct pg_level { + const struct 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 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; +};
Minor nit, but if we're moving these structure definitions into the header then I'd be inclined to give them some more specific names (e.g. prefix them with 'ptdump_'). Granted, this header isn't used widely, but it's included by arch/arm64/mm/mmu.c and claiming 'struct prot_bits' is a bit over-reaching imo! Will