On 2023/05/01 1:16, Rongwei Wang wrote:
> Hello,
>
> Recently I handle some stuff about zero page with crash. I'm always use
> vtop to check
>
> whether an virtual address has been set the zero page. But, it can not
> directly show whether a page
>
> is zero page or not. So I try to add this patch to support this.
>
> And I'm not sure this idea is nice to be accepted, so just realize it on
> arm64 platform. I can finish others
>
> arch if it accepted.
Sorry for the delay, I had been on holidays last week.
I think this is a nice suggestion and would like to accept it also for
other architectures.
>
>
> Thanks for your time.
>
> On 2023/5/1 00:02, Rongwei Wang wrote:
>> Now vtop can not show us the page is zero pfn
>> when PTE or PMD has attached ZERO PAGE. This
>> patch supports show this information directly
>> when using vtop, likes:
>>
>> crash> vtop -c 13674 ffff8917e000
>> VIRTUAL PHYSICAL
>> ffff8917e000 836e71000
>>
>> PAGE DIRECTORY: ffff000802f8d000
>> PGD: ffff000802f8dff8 => 884e29003
>> PUD: ffff000844e29ff0 => 884e93003
>> PMD: ffff000844e93240 => 840413003
>> PTE: ffff000800413bf0 => 160000836e71fc3
>> PAGE: 836e71000 (ZERO PAGE)
>>
>> PTE PHYSICAL FLAGS
>> 160000836e71fc3 836e71000
>> (VALID|USER|RDONLY|SHARED|AF|NG|PXN|UXN|SPECIAL)
>>
>> VMA START END FLAGS FILE
>> ffff000844f51860 ffff8917c000 ffff8957d000 100073
>>
>> PAGE PHYSICAL MAPPING INDEX CNT FLAGS
>> fffffe001fbb9c40 836e71000 0 0 1
>> 2ffffc000001000 reserved
>>
>> If huge page found:
>>
>> crash> vtop -c 14538 ffff95800000
>> VIRTUAL PHYSICAL
>> ffff95800000 910c00000
>>
>> PAGE DIRECTORY: ffff000801fa0000
>> PGD: ffff000801fa0ff8 => 884f53003
>> PUD: ffff000844f53ff0 => 8426cb003
>> PMD: ffff0008026cb560 => 60000910c00fc1
>> PAGE: 910c00000 (2MB, ZERO PAGE)
>>
>> PTE PHYSICAL FLAGS
>> 60000910c00fc1 910c00000 (VALID|USER|RDONLY|SHARED|AF|NG|PXN|UXN)
>>
>> VMA START END FLAGS FILE
>> ffff0000caa711e0 ffff956a9000 ffff95aaa000 100073
>>
>> PAGE PHYSICAL MAPPING INDEX CNT FLAGS
>> fffffe0023230000 910c00000 0 0 1
>> 6ffffc000010000 head
>>
>> That seems be sensible with this patch.
>>
>> Signed-off-by: Rongwei Wang <rongwei.wang@xxxxxxxxxxxxxxxxx>
>> ---
>> arm64.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++------
>> defs.h | 5 ++++
>> 2 files changed, 88 insertions(+), 9 deletions(-)
>>
>> diff --git a/arm64.c b/arm64.c
>> index 56fb841..264572d 100644
>> --- a/arm64.c
>> +++ b/arm64.c
>> @@ -419,7 +419,25 @@ arm64_init(int when)
>> /* use machdep parameters */
>> arm64_calc_phys_offset();
>> arm64_calc_physvirt_offset();
>> -
>> +
>> + if (kernel_symbol_exists("zero_pfn")) {
>> + ulong zero_pfn = 0;
>> +
>> + if (readmem(symbol_value("zero_pfn"), KVADDR,
>> + &zero_pfn, sizeof(zero_pfn),
>> + "read zero_pfn", QUIET|RETURN_ON_ERROR))
>> + machdep->zero_pfn = zero_pfn;
>> + }
>> +
>> + if (kernel_symbol_exists("huge_zero_pfn")) {
>> + ulong huge_zero_pfn = 0;
>> +
>> + if (readmem(symbol_value("huge_zero_pfn"), KVADDR,
>> + &huge_zero_pfn, sizeof(huge_zero_pfn),
>> + "read huge_zero_pfn", QUIET|RETURN_ON_ERROR))
>> + machdep->huge_zero_pfn = huge_zero_pfn;
>> + }
so, for other architectures, are you going to move these to vm_init() or
somewhere?
And zero_paddr might be better than zero_pfn to reduce the calculation
on printing, e.g. vt->zero_paddr = zero_pfn << PAGESHIFT().
>> +
>> if (CRASHDEBUG(1)) {
>> if (machdep->flags & NEW_VMEMMAP)
>> fprintf(fp, "kimage_voffset: %lx\n",
>> @@ -1787,7 +1805,14 @@ arm64_vtop_2level_64k(ulong pgd, ulong vaddr,
>> physaddr_t *paddr, int verbose)
>> if ((pgd_val & PMD_TYPE_MASK) == PMD_TYPE_SECT) {
>> ulong sectionbase = (pgd_val & SECTION_PAGE_MASK_512MB) &
>> PHYS_MASK;
>> if (verbose) {
>> - fprintf(fp, " PAGE: %lx (512MB)\n\n", sectionbase);
>> + if (kernel_symbol_exists("huge_zero_pfn")) {
>> + if (sectionbase == (HUGE_ZEROPFN() << PAGESHIFT()))
>> + fprintf(fp, " PAGE: %lx (512MB, ZERO PAGE)\n\n",
>> + HUGE_ZEROPFN() << PAGESHIFT());
>> + else
>> + fprintf(fp, " PAGE: %lx (512MB)\n\n",
>> sectionbase);
>> + } else
>> + fprintf(fp, " PAGE: %lx (512MB)\n\n", sectionbase);
>> arm64_translate_pte(pgd_val, 0, 0);
>> }
>> *paddr = sectionbase + (vaddr & ~SECTION_PAGE_MASK_512MB);
>> @@ -1806,7 +1831,14 @@ arm64_vtop_2level_64k(ulong pgd, ulong vaddr,
>> physaddr_t *paddr, int verbose)
>> if (pte_val & PTE_VALID) {
>> *paddr = (PAGEBASE(pte_val) & PHYS_MASK) + PAGEOFFSET(vaddr);
>> if (verbose) {
>> - fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
>> + if (kernel_symbol_exists("zero_pfn")) {
>> + if (PAGEBASE(*paddr) == (ZEROPFN() << PAGESHIFT()))
>> + fprintf(fp, " PAGE: %lx (ZERO PAGE)\n\n",
>> + ZEROPFN() << PAGESHIFT());
>> + else
>> + fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
>> + } else
>> + fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
These blocks look a bit verbose and it would be better to not call
kernel_symbol_exists() each time, if possible.
Can it be changed to something like this?
if (vt->zero_paddr && PAGEBASE(*paddr) == vt->zero_paddr) {
or this.
fprintf(fp, " PAGE: %lx %s\n\n", PAGEBASE(*paddr),
vt->zero_paddr && PAGEBASE(*paddr) == vt->zero_paddr ?
"(ZERO PAGE)" : "");
Thanks,
Kazu
>> arm64_translate_pte(pte_val, 0, 0);
>> }
>> } else {
>> @@ -1859,7 +1891,14 @@ arm64_vtop_3level_64k(ulong pgd, ulong vaddr,
>> physaddr_t *paddr, int verbose)
>> if ((pmd_val & PMD_TYPE_MASK) == PMD_TYPE_SECT) {
>> ulong sectionbase = PTE_TO_PHYS(pmd_val) &
>> SECTION_PAGE_MASK_512MB;
>> if (verbose) {
>> - fprintf(fp, " PAGE: %lx (512MB)\n\n", sectionbase);
>> + if (kernel_symbol_exists("huge_zero_pfn")) {
>> + if (sectionbase == (HUGE_ZEROPFN() << PAGESHIFT()))
>> + fprintf(fp, " PAGE: %lx (512MB, ZERO PAGE)\n\n",
>> + HUGE_ZEROPFN() << PAGESHIFT());
>> + else
>> + fprintf(fp, " PAGE: %lx (512MB)\n\n",
>> sectionbase);
>> + } else
>> + fprintf(fp, " PAGE: %lx (512MB)\n\n", sectionbase);
>> arm64_translate_pte(pmd_val, 0, 0);
>> }
>> *paddr = sectionbase + (vaddr & ~SECTION_PAGE_MASK_512MB);
>> @@ -1878,7 +1917,14 @@ arm64_vtop_3level_64k(ulong pgd, ulong vaddr,
>> physaddr_t *paddr, int verbose)
>> if (pte_val & PTE_VALID) {
>> *paddr = PTE_TO_PHYS(pte_val) + PAGEOFFSET(vaddr);
>> if (verbose) {
>> - fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
>> + if (kernel_symbol_exists("zero_pfn")) {
>> + if (PAGEBASE(*paddr) == (ZEROPFN() << PAGESHIFT()))
>> + fprintf(fp, " PAGE: %lx (ZERO PAGE)\n\n",
>> + ZEROPFN() << PAGESHIFT());
>> + else
>> + fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
>> + } else
>> + fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
>> arm64_translate_pte(pte_val, 0, 0);
>> }
>> } else {
>> @@ -1940,7 +1986,14 @@ arm64_vtop_3level_4k(ulong pgd, ulong vaddr,
>> physaddr_t *paddr, int verbose)
>> if ((pmd_val & PMD_TYPE_MASK) == PMD_TYPE_SECT) {
>> ulong sectionbase = (pmd_val & SECTION_PAGE_MASK_2MB) &
>> PHYS_MASK;
>> if (verbose) {
>> - fprintf(fp, " PAGE: %lx (2MB)\n\n", sectionbase);
>> + if (kernel_symbol_exists("huge_zero_pfn")) {
>> + if (sectionbase == (HUGE_ZEROPFN() << PAGESHIFT()))
>> + fprintf(fp, " PAGE: %lx (2MB, ZERO PAGE)\n\n",
>> + HUGE_ZEROPFN() << PAGESHIFT());
>> + else
>> + fprintf(fp, " PAGE: %lx (2MB)\n\n", sectionbase);
>> + } else
>> + fprintf(fp, " PAGE: %lx (2MB)\n\n", sectionbase);
>> arm64_translate_pte(pmd_val, 0, 0);
>> }
>> *paddr = sectionbase + (vaddr & ~SECTION_PAGE_MASK_2MB);
>> @@ -1959,7 +2012,14 @@ arm64_vtop_3level_4k(ulong pgd, ulong vaddr,
>> physaddr_t *paddr, int verbose)
>> if (pte_val & PTE_VALID) {
>> *paddr = (PAGEBASE(pte_val) & PHYS_MASK) + PAGEOFFSET(vaddr);
>> if (verbose) {
>> - fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
>> + if (kernel_symbol_exists("zero_pfn")) {
>> + if (PAGEBASE(*paddr) == (ZEROPFN() << PAGESHIFT()))
>> + fprintf(fp, " PAGE: %lx (ZERO PAGE)\n\n",
>> + ZEROPFN() << PAGESHIFT());
>> + else
>> + fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
>> + } else
>> + fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
>> arm64_translate_pte(pte_val, 0, 0);
>> }
>> } else {
>> @@ -2029,7 +2089,14 @@ arm64_vtop_4level_4k(ulong pgd, ulong vaddr,
>> physaddr_t *paddr, int verbose)
>> if ((pmd_val & PMD_TYPE_MASK) == PMD_TYPE_SECT) {
>> ulong sectionbase = (pmd_val & SECTION_PAGE_MASK_2MB) &
>> PHYS_MASK;
>> if (verbose) {
>> - fprintf(fp, " PAGE: %lx (2MB)\n\n", sectionbase);
>> + if (kernel_symbol_exists("huge_zero_pfn")) {
>> + if (sectionbase == (HUGE_ZEROPFN() << PAGESHIFT()))
>> + fprintf(fp, " PAGE: %lx (2MB, ZERO PAGE)\n\n",
>> + HUGE_ZEROPFN() << PAGESHIFT());
>> + else
>> + fprintf(fp, " PAGE: %lx (2MB)\n\n", sectionbase);
>> + } else
>> + fprintf(fp, " PAGE: %lx (2MB)\n\n", sectionbase);
>> arm64_translate_pte(pmd_val, 0, 0);
>> }
>> *paddr = sectionbase + (vaddr & ~SECTION_PAGE_MASK_2MB);
>> @@ -2048,7 +2115,14 @@ arm64_vtop_4level_4k(ulong pgd, ulong vaddr,
>> physaddr_t *paddr, int verbose)
>> if (pte_val & PTE_VALID) {
>> *paddr = (PAGEBASE(pte_val) & PHYS_MASK) + PAGEOFFSET(vaddr);
>> if (verbose) {
>> - fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
>> + if (kernel_symbol_exists("zero_pfn")) {
>> + if (PAGEBASE(*paddr) == (ZEROPFN() << PAGESHIFT()))
>> + fprintf(fp, " PAGE: %lx (ZERO PAGE)\n\n",
>> + ZEROPFN() << PAGESHIFT());
>> + else
>> + fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
>> + } else
>> + fprintf(fp, " PAGE: %lx\n\n", PAGEBASE(*paddr));
>> arm64_translate_pte(pte_val, 0, 0);
>> }
>> } else {
>> diff --git a/defs.h b/defs.h
>> index 12ad6aa..4ed2d0a 100644
>> --- a/defs.h
>> +++ b/defs.h
>> @@ -1071,6 +1071,8 @@ struct machdep_table {
>> void (*show_interrupts)(int, ulong *);
>> int (*is_page_ptr)(ulong, physaddr_t *);
>> int (*get_cpu_reg)(int, int, const char *, int, void *);
>> + ulong zero_pfn;
>> + ulong huge_zero_pfn;
>> };
>> /*
>> @@ -2999,6 +3001,9 @@ struct load_module {
>> #define VIRTPAGEBASE(X) (((ulong)(X)) & (ulong)machdep->pagemask)
>> #define PHYSPAGEBASE(X) (((physaddr_t)(X)) &
>> (physaddr_t)machdep->pagemask)
>> +#define ZEROPFN() (machdep->zero_pfn)
>> +#define HUGE_ZEROPFN() (machdep->huge_zero_pfn)
>> +
>> /*
>> * Sparse memory stuff
>> * These must follow the definitions in the kernel mmzone.h
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
