<div dir="ltr"><div>Hi, Aditya</div>Thank you for the update. <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Sep 25, 2023 at 5:28 PM Aditya Gupta <<a href="mailto:adityag@linux.ibm.com">adityag@linux.ibm.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Currently 'crash-tool' fails on vmcore collected on upstream kernel on PowerPC64 with the error: crash: invalid kernel virtual address: 0 type: "first list entry Presently the address translation for vmemmap addresses is done using the vmemmap_list. But with the below commit in Linux, vmemmap_list can be empty, in case of Radix MMU on PowerPC64 368a0590d954: (powerpc/book3s64/vmemmap: switch radix to use a different vmemmap handling function) In case vmemmap_list is empty, then it's head is NULL, which crash tries to access and fails due to accessing NULL. Instead of depending on 'vmemmap_list' for address translation for vmemmap addresses, do a kernel pagetable walk to get the physical address associated with given virtual address Tested-by: Sachin Sant <<a href="mailto:sachinp@linux.ibm.com" target="_blank">sachinp@linux.ibm.com</a>> Reviewed-by: Hari Bathini <<a href="mailto:hbathini@linux.ibm.com" target="_blank">hbathini@linux.ibm.com</a>> Signed-off-by: Aditya Gupta <<a href="mailto:adityag@linux.ibm.com" target="_blank">adityag@linux.ibm.com</a>> --- Testing ======= Git tree with patch applied: <a href="https://github.com/adi-g15-ibm/crash/tree/bugzilla-203296-list-v2" rel="noreferrer" target="_blank">https://github.com/adi-g15-ibm/crash/tree/bugzilla-203296-list-v2</a> This can be tested with '/proc/vmcore' as the vmcore, since makedumpfile also fails in absence of 'vmemmap_list' in upstream linux The fix for makedumpfile will also been sent to upstream Changelog ========= V2 + handle the case of 'vmemmap_list' symbol missing according to reviews </blockquote><div> </div><div>The v2 looks good to me, so: Ack.</div><div> </div><div>Thanks</div><div>Lianbo</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> --- --- ppc64.c | 51 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/ppc64.c b/ppc64.c index fc34006f4863..cd98a679c45e 100644 --- a/ppc64.c +++ b/ppc64.c @@ -1280,10 +1280,32 @@ void ppc64_vmemmap_init(void) long backing_size, virt_addr_offset, phys_offset, list_offset; ulong *vmemmap_list; char *vmemmap_buf; - struct machine_specific *ms; - - if (!(kernel_symbol_exists("vmemmap_list")) || - !(kernel_symbol_exists("mmu_psize_defs")) || + struct machine_specific *ms = machdep->machspec; + + ld = &list_data; + BZERO(ld, sizeof(struct list_data)); + + /* + * vmemmap_list is missing or set to 0 in the kernel would imply + * vmemmap region is mapped in the kernel pagetable. So, read vmemmap_list + * anyway and use the translation method accordingly. + */ + if (kernel_symbol_exists("vmemmap_list")) + readmem(symbol_value("vmemmap_list"), KVADDR, &ld->start, + sizeof(void *), "vmemmap_list", RETURN_ON_ERROR|QUIET); + if (!ld->start) { + /* + * vmemmap_list is set to 0 or missing. Do kernel pagetable walk + * for vmemmap address translation. + */ + ms->vmemmap_list = NULL; + ms->vmemmap_cnt = 0; + + machdep->flags |= VMEMMAP_AWARE; + return; + } + + if (!(kernel_symbol_exists("mmu_psize_defs")) || !(kernel_symbol_exists("mmu_vmemmap_psize")) || !STRUCT_EXISTS("vmemmap_backing") || !STRUCT_EXISTS("mmu_psize_def") || @@ -1293,8 +1315,6 @@ void ppc64_vmemmap_init(void) !MEMBER_EXISTS("vmemmap_backing", "list")) return; - ms = machdep->machspec; - backing_size = STRUCT_SIZE("vmemmap_backing"); virt_addr_offset = MEMBER_OFFSET("vmemmap_backing", "virt_addr"); phys_offset = MEMBER_OFFSET("vmemmap_backing", "phys"); @@ -1313,14 +1333,8 @@ void ppc64_vmemmap_init(void) ms->vmemmap_psize = 1 << shift; - ld = &list_data; - BZERO(ld, sizeof(struct list_data)); - if (!readmem(symbol_value("vmemmap_list"), - KVADDR, &ld->start, sizeof(void *), "vmemmap_list", - RETURN_ON_ERROR)) - return; - ld->end = symbol_value("vmemmap_list"); - ld->list_head_offset = list_offset; + ld->end = symbol_value("vmemmap_list"); + ld->list_head_offset = list_offset; hq_open(); cnt = do_list(ld); @@ -1366,7 +1380,7 @@ ppc64_vmemmap_to_phys(ulong kvaddr, physaddr_t *paddr, int verbose) { int i; ulong offset; - struct machine_specific *ms; + struct machine_specific *ms = machdep->machspec; if (!(machdep->flags & VMEMMAP_AWARE)) { /* @@ -1386,7 +1400,12 @@ ppc64_vmemmap_to_phys(ulong kvaddr, physaddr_t *paddr, int verbose) return FALSE; } - ms = machdep->machspec; + /** + * When vmemmap_list is not populated, kernel does the mapping in init_mm + * page table, so do a pagetable walk in kernel page table + */ + if (!ms->vmemmap_list) + return ppc64_vtop_level4(kvaddr, (ulong *)vt->kernel_pgd[0], paddr, verbose); for (i = 0; i < ms->vmemmap_cnt; i++) { if ((kvaddr >= ms->vmemmap_list[i].virt) && -- 2.41.0 </blockquote></div></div>