[Crash-utility] [PATCH RFC] arm64: show zero pfn information when using vtop
Rongwei Wang
rongwei.wang at linux.alibaba.com
Tue May 9 08:32:23 UTC 2023
On 2023/5/9 15:58, HAGIO KAZUHITO(萩尾 一仁) wrote:
> On 2023/05/08 21:07, Rongwei Wang wrote:
>> On 2023/5/8 15:46, HAGIO KAZUHITO(萩尾 一仁) wrote:
>>> 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 reply.
>>
>> I will continue to finish this patch for other arches. ;)
>>
>>>> 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 at 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?
>> Good idea, it looks better than arm64_init(). Next version will fix here.
>>> And zero_paddr might be better than zero_pfn to reduce the calculation
>>> on printing, e.g. vt->zero_paddr = zero_pfn << PAGESHIFT().
>> OK, next version will fix here.
>>>>> +
>>>>> 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.
>> Actually, I'm not sure this 'zero_paddr' has possibility of equal to 0
>> (seems unlikely).
>>
>> Anyway, I will try to make sure that.
> Thanks!
>
> If 0 cannot be used, ~0 might be good. And you can also use macros if
> possible. For example,
>
> #define IS_ZEROPAGE(paddr) (vt->zero_paddr != ~0UL && ...
Cool!
>
> or something. Just an idea, not tried.
>
> One more thing, it's hard for us to test all of arches that crash
> supports, so it would be ok to implement it first for arches that you
> can test. For the other arches, each arch user can reuse the patch if
> they want it.
OK, I plan to finish this in arm and x86 at first.
Thanks for your time.
-wrw
>
> Thanks,
> Kazu
>
>
>>> 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)" : "");
>> Nice, I'm also think my above style is verbose, but have no idea at that
>> time.
>>
>> I will update here at next version.
>>
>>> Thanks,
>>> Kazu
>> Thanks for your time.
>>
>> -wrw
>>
>>>>> 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
More information about the Crash-utility
mailing list