Re: [Crash-utility] arm64: Get CPU registers without crash_notes symbol

-----Original Message----- > Dear Kazu, > > Sorry for late reply > Thanks for your suggestion and I have prepared a V2 patch, please > help > to review. ok, I've modified your v2 patch to fix the following compilation warning and rewrite the commit log, and attached it. arm64.c: In function ‘arm64_init’: arm64.c:3728:35: warning: ‘note’ may be used uninitialized in this function [-Wmaybe-uninitialized] offset = roundup(offset + note->n_namesz, 4); ^~

Lianbo, could you review the attached patch? Thanks, Kazu > > BTW, I have switched to evolution mail system, if still look like > html > format, please let me know, thanks. > > Signed-off-by: James Hsu <james.hsu(a)mediatek.com&gt; > > diff --git a/arm64.c b/arm64.c > index 8934961..03c8167 100644 > --- a/arm64.c > +++ b/arm64.c > @@ -3667,8 +3667,42 @@ arm64_get_crash_notes(void) > ulong *notes_ptrs; > ulong i, found; > > - if (!symbol_exists("crash_notes")) > + if (!symbol_exists("crash_notes")) { > + if (DISKDUMP_DUMPFILE() || KDUMP_DUMPFILE()) { > + if (!(ms->panic_task_regs = calloc((size_t)kt- > > cpus, sizeof(struct arm64_pt_regs)))) > > + error(FATAL, "cannot calloc > panic_task_regs space\n"); > + > + for (i = found = 0; i < kt->cpus; i++) { > + if (DISKDUMP_DUMPFILE()) > + note = > diskdump_get_prstatus_percpu(i); > + else if (KDUMP_DUMPFILE()) > + note = > netdump_get_prstatus_percpu(i); > + > + if(!note) { > + error(WARNING, "cpu %d: cannot > find NT_PRSTATUS note\n", i); > + continue; > + } > + > + /* > + * Find correct location of note data. > This contains elf_prstatus > + * structure which has registers etc. > for the crashed task. > + */ > + offset = sizeof(Elf64_Nhdr); > + offset = roundup(offset + note- > > n_namesz, 4); > > + p = (char *)note + offset; /* start of > elf_prstatus */ > + > + BCOPY(p + OFFSET(elf_prstatus_pr_reg), > &ms->panic_task_regs[i], > + sizeof(struct arm64_pt_regs)); > + > + found++; > + } > + if (!found) { > + free(ms->panic_task_regs); > + ms->panic_task_regs = NULL; > + } > + } > return; > + } > > crash_notes = symbol_value("crash_notes"); > > Thanks > James Hsu > > On Wed, 2021-08-11 at 02:52 +0000, HAGIO KAZUHITO(萩尾　一仁) wrote: > > -----Original Message----- > > > > > > Dear Crash maintainers, > > > > > > We want to improve the crash_tool for the case that need > > > offline > > > cpu register info even Kdump without > > > crash_notes. > > > > > > For one thing, we will prepare a Kdump with all CPU register > > > info > > > in ELF note. > > > For another thing, we need to modify crash_tool and make it > > > support > > > adding offline cpu register info even > > > Kdump without crash_notes. > > > > > > We prepare a patch for the case with ARCH=arm64. > > > It is verified with our kdump without crash_note and can get > > > all > > > cpu register. > > > Please take your time to review our patch and look forward to > > > receiving your comments. > > > > > > Patch detail as below > > > > I've rewritten the commit log, is this ok? > > -- > > arm64: Get CPU registers from ELF notes even without crash_notes > > symbol > > > > Currently arm64 crash retrieves the CPU registers from > > crash_notes > > symbol > > or ELF notes only when the symbol exists, but there are dumpfiles > > which > > have the registers in ELF notes without the symbol. > > > > With the patch, crash can retrieve the registers from ELF notes > > without > > the crash_notes symbol. > > -- > > > > And please add Signed-off-by tag. > > > > > --- crash-7.3.0/arm64.c 2021-04-27 16:01:07.000000000 +0800 > > > +++ crash-7.3.0.mod/arm64.c 2021-06-15 17:13:54.037273227 > > > +0800 > > > @@ -3667,8 +3667,41 @@ arm64_get_crash_notes(void) > > > ulong *notes_ptrs; > > > ulong i, found; > > > > > > - if (!symbol_exists("crash_notes")) > > > + if (!symbol_exists("crash_notes")) { > > > + if (DISKDUMP_DUMPFILE() || KDUMP_DUMPFILE()) { > > > + if (!(ms->panic_task_regs = > > > calloc((size_t)kt- > > > > cpus, sizeof(struct arm64_pt_regs)))) > > > > > > + error(FATAL, "cannot calloc > > > panic_task_regs space\n"); > > > + > > > + for (i = found = 0; i < kt->cpus; i++) > > > { > > > + if (DISKDUMP_DUMPFILE()) > > > + note = > > > diskdump_get_prstatus_percpu(i); > > > + else if (KDUMP_DUMPFILE()) > > > + note = > > > netdump_get_prstatus_percpu(i); > > > + else { > > > + error(WARNING, "cpu %d: > > > cannot > > > find NT_PRSTATUS note\n", i); > > > + continue; > > > + } > > > > This else block should be separated from the if block like this? > > > > if (!note) { > > error(WARNING, "cpu %d: cannot find NT_PRSTATUS note\n", i); > > continue; > > } > > > > > + > > > + /* > > > + * Find correct location of > > > note data. > > > This contains elf_prstatus > > > + * structure which has > > > registers etc. > > > for the crashed task. > > > + */ > > > + offset = sizeof(Elf64_Nhdr); > > > + offset = roundup(offset + note- > > > > n_namesz, 4); > > > > > > + p = (char *)note + offset; /* > > > start of > > > elf_prstatus */ > > > + > > > + BCOPY(p + > > > OFFSET(elf_prstatus_pr_reg), > > > &ms->panic_task_regs[i], > > > + sizeof(struct > > > arm64_pt_regs)); > > > + > > > + found++; > > > + } > > > + } > > > + if (!found) { > > > + free(ms->panic_task_regs); > > > + ms->panic_task_regs = NULL; > > > + } > > > > This if block should be within the if (DISKDUMP_DUMPFILE()... > > block. > > > > > return; > > > + } > > > > > > crash_notes = symbol_value("crash_notes"); > > > > > > > (and this email still looks HTML one to me :-) > > > > Thanks, > > Kazu > >