-----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@xxxxxxxxxxxx>
>
> 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
> >
Attachment:
0001-arm64-Get-CPU-registers-from-ELF-notes-even-without-.patch
Description: 0001-arm64-Get-CPU-registers-from-ELF-notes-even-without-.patch
-- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility
