2:46 a.m.
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(a)linux.alibaba.com>
> ---
> 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