On Fri, Dec 10, 2021 at 12:44 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@xxxxxxx> wrote:
>
> (sorry, I mistook the email to reply.. this is the correct one.)
>
> -----Original Message-----
> > Commit 2f967fb5ebd737ce5eadba462df35935122e8865 (crash_taget:
> > fetch_registers support) added new member get_cpu_reg in the middle of
> > struct machdep_table as:
> >
> > @@ -1013,6 +1013,7 @@ struct machdep_table {
> > ulong (*processor_speed)(void);
> > int (*uvtop)(struct task_context *, ulong, physaddr_t *, int);
> > int (*kvtop)(struct task_context *, ulong, physaddr_t *, int);
> > + int (*get_cpu_reg)(int, int, const char *, int, void *);
> > ulong (*get_task_pgd)(ulong);
> > void (*dump_irq)(int);
> > void (*get_stack_frame)(struct bt_info *, ulong *, ulong *);
> >
> > which breaks compatibility of struct machdep_table for extension
> > modules. In general, new member variables must be added at the end of
> > structures to maintain compatibility of data structures shared among
> > extension modules.
> >
> > For example, as the result, crash gcore command results in unexpected
> > behavior:
> >
> > crash> gcore -v 7
> > GETBUF(168 -> 0)
> > <readmem: ffff8b267dd28020, KVADDR, "task_struct.stack", 8, (FOE), 7fffaef8afa8>
> > <read_kdump: addr: ffff8b267dd28020 paddr: 7dd28020 cnt: 8>
> > read_netdump: addr: ffff8b267dd28020 paddr: 7dd28020 cnt: 8 offset: 767ec020
> > <readmem: ffffa3f740abff58, KVADDR, "genregs_get: pt_regs", 168, (FOE), e71f80>
> > gcore: invalid kernel virtual address: ffffa3f740abff58 type: "genregs_get: pt_regs"
> > vma hit rate: 0% (0 of 274308)
> >
> > crash gcore uses machdep->get_stacktop() to retrieve registers saved
> > at the bottom of a given user-space task's kernel stack:
> >
> > static int genregs_get(struct task_context *target,
> > const struct user_regset *regset,
> > unsigned int size, void *buf)
> > {
> > ...snip...
> > pt_regs_buf = GETBUF(SIZE(pt_regs));
> >
> > readmem(machdep->get_stacktop(target->task) - SIZE(pt_regs), KVADDR,
> > pt_regs_buf, SIZE(pt_regs), "genregs_get: pt_regs",
> > gcore_verbose_error_handle());
> >
> > As seen above, the position of get_stacktop in struct machdep_table is
> > behind the position where get_cpu_reg was added:
> >
> > int (*uvtop)(struct task_context *, ulong, physaddr_t *, int);
> > int (*kvtop)(struct task_context *, ulong, physaddr_t *, int);
> > int (*get_cpu_reg)(int, int, const char *, int, void *); ulong (*get_task_pgd)(ulong);
> > void (*dump_irq)(int);
> > void (*get_stack_frame)(struct bt_info *, ulong *, ulong *);
> > ulong (*get_stackbase)(ulong);
> > ulong (*get_stacktop)(ulong);
> >
> > Signed-off-by: HATAYAMA Daisuke <d.hatayama@xxxxxxxxxxx>
>
> Acked-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx>
>
> Lianbo, this is also needed for RHEL9 crash before GA.
>
OK, thank you, Kazu.
> Thanks,
> Kazu
>
> > ---
> > defs.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/defs.h b/defs.h
> > index e9e8143..b63741c 100644
> > --- a/defs.h
> > +++ b/defs.h
> > @@ -1013,7 +1013,6 @@ struct machdep_table {
> > ulong (*processor_speed)(void);
> > int (*uvtop)(struct task_context *, ulong, physaddr_t *, int);
> > int (*kvtop)(struct task_context *, ulong, physaddr_t *, int);
> > - int (*get_cpu_reg)(int, int, const char *, int, void *);
> > ulong (*get_task_pgd)(ulong);
> > void (*dump_irq)(int);
> > void (*get_stack_frame)(struct bt_info *, ulong *, ulong *);
> > @@ -1063,6 +1062,7 @@ struct machdep_table {
> > void (*get_irq_affinity)(int);
> > void (*show_interrupts)(int, ulong *);
> > int (*is_page_ptr)(ulong, physaddr_t *);
> > + int (*get_cpu_reg)(int, int, const char *, int, void *);
> > };
> >
> > /*
> > --
> > 2.31.1
> >
> >
> >
> >
> > --
> > Crash-utility mailing list
> > Crash-utility@xxxxxxxxxx
> > https://listman.redhat.com/mailman/listinfo/crash-utility
>
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
