On 23/07/2019 10:57, Mark Rutland wrote: > On Mon, Jul 22, 2019 at 04:42:08PM +0100, Steven Price wrote: >> Add a generic version of page table dumping that architectures can >> opt-in to >> >> Signed-off-by: Steven Price <steven.pr...@arm.com> > > [...] > >> +#ifdef CONFIG_KASAN >> +/* >> + * This is an optimization for KASAN=y case. Since all kasan page tables >> + * eventually point to the kasan_early_shadow_page we could call note_page() >> + * right away without walking through lower level page tables. This saves >> + * us dozens of seconds (minutes for 5-level config) while checking for >> + * W+X mapping or reading kernel_page_tables debugfs file. >> + */ >> +static inline bool kasan_page_table(struct ptdump_state *st, void *pt, >> + unsigned long addr) >> +{ >> + if (__pa(pt) == __pa(kasan_early_shadow_pmd) || >> +#ifdef CONFIG_X86 >> + (pgtable_l5_enabled() && >> + __pa(pt) == __pa(kasan_early_shadow_p4d)) || >> +#endif >> + __pa(pt) == __pa(kasan_early_shadow_pud)) { >> + st->note_page(st, addr, 5, pte_val(kasan_early_shadow_pte[0])); >> + return true; >> + } >> + return false; > > Having you tried this with CONFIG_DEBUG_VIRTUAL? > > The kasan_early_shadow_pmd is a kernel object rather than a linear map > object, so you should use __pa_symbol for that.
Thanks for pointing that out - it is indeed broken on arm64. This was moved from x86 where CONFIG_DEBUG_VIRTUAL doesn't seem to pick this up. There is actually a problem here that 'pt' might not be in the linear map (so __pa(pt) barfs on arm64 as well as kasan_early_shadow_p?d). It looks like having the comparisons of the form "pt == lm_alias(kasan_early_shadow_p?d)" is probably best. > It's a bit horrid to have to test multiple levels in one function; can't > we check the relevant level inline in each of the test_p?d funcs? > > They're optional anyway, so they only need to be defined for > CONFIG_KASAN. Good point - removing the test_p?d callbacks when !CONFIG_KASAN simplifies the code. Thanks, Steve