On Tue, Sep 27, 2022 at 4:27 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio...@nec.com>
wrote:
> cc Alexey, reverting this affects VMware folks.
>
> On 2022/09/27 15:27, Lianbo Jiang wrote:
> > This reverts commit 2f967fb5ebd737ce5eadba462df35935122e8865. John
> > Pittman reports that it causes a regression issue on the old Vmware
> > vmcore as below:
> >
> > crash> set scope dm_table_create
> > set: attempting to find/load "dm_mod" module debuginfo
> > MODULE NAME BASE SIZE
> OBJECT FILE
> > ffffffffa0012dc0 dm_mod ffffffffa0000000 102823
> /home/2.6.32-754.35.1.el6.x86_64/kernel/drivers/md/dm-mod.ko.debug
> > scope: ffffffffa0006cf0 (dm_table_create)
> > crash> whatis struct dm_table
> > struct dm_table {
> > uint64_t features;
> > struct mapped_device *md;
> > unsigned int type;
> > unsigned int depth;
> > unsigned int counts[16];
> > sector_t *index[16];
> > unsigned int num_targets;
> > unsigned int num_allocated;
> > sector_t *highs;
> > struct dm_target *targets;
> > struct target_type *immutable_target_type;
> > unsigned int integrity_supported : 1;
> > unsigned int singleton : 1;
> > fmode_t mode;
> > struct list_head devices;
> > void (*event_fn)(void *);
> > void *event_context;
> > struct dm_md_mempools *mempools;
> > struct list_head target_callbacks;
> > }
> > SIZE: 312
> > crash> whatis _name_buckets --->| After using the whatis
> _name_buckets command,
> > struct list_head _name_buckets[64]; |
> > crash> whatis struct dm_table <---| it won't show the contents
> of the dm_table struct.
> > struct dm_table {
> > int undefined__;
> > }
> > SIZE: 312
> > crash>
> >
> > With the patch, this issue disappeared.
> At first glance, I did not find any part of the patch that can cause
> the issue. Do you have any detailed information?
>
>
This issue is observed on an old Vmware vmcore, and looks like a regression
issue.
I tried to dig into the detail, the raw_supply() operation may affect the
gdb behavior, it
will store the register values to a cache in the gdb.
So far I haven't got a good solution, let's see if Alex has a better way to
fix it, that would be welcome. Any thoughts?
Thanks.
Lianbo
> Thanks,
> Kazu
>
> >
> > Signed-off-by: Lianbo Jiang <liji...@redhat.com>
> > ---
> > crash_target.c | 33 +-------------------------------
> > defs.h | 51 -------------------------------------------------
> > gdb_interface.c | 19 +++++-------------
> > vmware_vmss.c | 51 +------------------------------------------------
> > x86_64.c | 16 ----------------
> > 5 files changed, 7 insertions(+), 163 deletions(-)
> >
> > diff --git a/crash_target.c b/crash_target.c
> > index 455480679741..a123329019f5 100644
> > --- a/crash_target.c
> > +++ b/crash_target.c
> > @@ -27,8 +27,6 @@ void crash_target_init (void);
> >
> > extern "C" int gdb_readmem_callback(unsigned long, void *, int, int);
> > extern "C" int crash_get_nr_cpus(void);
> > -extern "C" int crash_get_cpu_reg (int cpu, int regno, const char
> *regname,
> > - int regsize, void *val);
> >
> >
> > /* The crash target. */
> > @@ -46,7 +44,6 @@ public:
> > const target_info &info () const override
> > { return crash_target_info; }
> >
> > - void fetch_registers (struct regcache *, int) override;
> > enum target_xfer_status xfer_partial (enum target_object object,
> > const char *annex,
> > gdb_byte *readbuf,
> > @@ -57,35 +54,13 @@ public:
> > bool has_all_memory () override { return true; }
> > bool has_memory () override { return true; }
> > bool has_stack () override { return true; }
> > - bool has_registers () override { return true; }
> > + bool has_registers () override { return false; }
> > bool thread_alive (ptid_t ptid) override { return true; }
> > std::string pid_to_str (ptid_t ptid) override
> > { return string_printf ("CPU %ld", ptid.tid ()); }
> >
> > };
> >
> > -/* We just get all the registers, so we don't use regno. */
> > -void
> > -crash_target::fetch_registers (struct regcache *regcache, int regno)
> > -{
> > - gdb_byte regval[16];
> > - int cpu = inferior_ptid.tid();
> > - struct gdbarch *arch = regcache->arch ();
> > -
> > - for (int r = 0; r < gdbarch_num_regs (arch); r++)
> > - {
> > - const char *regname = gdbarch_register_name(arch, r);
> > - int regsize = register_size (arch, r);
> > - if (regsize > sizeof (regval))
> > - error (_("fatal error: buffer size is not enough to fit
> register value"));
> > -
> > - if (crash_get_cpu_reg (cpu, r, regname, regsize, (void *)®val))
> > - regcache->raw_supply (r, regval);
> > - else
> > - regcache->raw_supply (r, NULL);
> > - }
> > -}
> > -
> >
> > enum target_xfer_status
> > crash_target::xfer_partial (enum target_object object, const char
> *annex,
> > @@ -126,10 +101,4 @@ crash_target_init (void)
> > if (!i)
> > switch_to_thread (thread);
> > }
> > -
> > - /* Fetch all registers from core file. */
> > - target_fetch_registers (get_current_regcache (), -1);
> > -
> > - /* Now, set up the frame cache. */
> > - reinit_frame_cache ();
> > }
> > diff --git a/defs.h b/defs.h
> > index e0ccf2ddc9c2..e8acf49894b8 100644
> > --- a/defs.h
> > +++ b/defs.h
> > @@ -1015,7 +1015,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 *);
> > @@ -6977,7 +6976,6 @@ int vmware_vmss_get_nr_cpus(void);
> > int vmware_vmss_get_cr3_cr4_idtr(int, ulong *, ulong *, ulong *);
> > int vmware_vmss_phys_base(ulong *phys_base);
> > int vmware_vmss_set_phys_base(ulong);
> > -int vmware_vmss_get_cpu_reg(int, int, const char *, int, void *);
> >
> > /*
> > * vmware_guestdump.c
> > @@ -7403,53 +7401,4 @@ extern int have_full_symbols(void);
> > #define XEN_HYPERVISOR_ARCH
> > #endif
> >
> > -/*
> > - * Register numbers must be in sync with gdb/features/i386/64bit-core.c
> > - * to make crash_target->fetch_registers() ---> machdep->get_cpu_reg()
> > - * working properly.
> > - */
> > -enum x86_64_regnum {
> > - RAX_REGNUM,
> > - RBX_REGNUM,
> > - RCX_REGNUM,
> > - RDX_REGNUM,
> > - RSI_REGNUM,
> > - RDI_REGNUM,
> > - RBP_REGNUM,
> > - RSP_REGNUM,
> > - R8_REGNUM,
> > - R9_REGNUM,
> > - R10_REGNUM,
> > - R11_REGNUM,
> > - R12_REGNUM,
> > - R13_REGNUM,
> > - R14_REGNUM,
> > - R15_REGNUM,
> > - RIP_REGNUM,
> > - EFLAGS_REGNUM,
> > - CS_REGNUM,
> > - SS_REGNUM,
> > - DS_REGNUM,
> > - ES_REGNUM,
> > - FS_REGNUM,
> > - GS_REGNUM,
> > - ST0_REGNUM,
> > - ST1_REGNUM,
> > - ST2_REGNUM,
> > - ST3_REGNUM,
> > - ST4_REGNUM,
> > - ST5_REGNUM,
> > - ST6_REGNUM,
> > - ST7_REGNUM,
> > - FCTRL_REGNUM,
> > - FSTAT_REGNUM,
> > - FTAG_REGNUM,
> > - FISEG_REGNUM,
> > - FIOFF_REGNUM,
> > - FOSEG_REGNUM,
> > - FOOFF_REGNUM,
> > - FOP_REGNUM,
> > - LAST_REGNUM
> > -};
> > -
> > #endif /* !GDB_COMMON */
> > diff --git a/gdb_interface.c b/gdb_interface.c
> > index b14319c66147..574447fd82b7 100644
> > --- a/gdb_interface.c
> > +++ b/gdb_interface.c
> > @@ -710,10 +710,11 @@ static char *prohibited_list[] = {
> > "run", "r", "break", "b", "tbreak", "hbreak", "thbreak", "rbreak",
> > "watch", "rwatch", "awatch", "attach", "continue", "c", "fg",
> "detach",
> > "finish", "handle", "interrupt", "jump", "kill", "next", "nexti",
> > - "signal", "step", "s", "stepi", "target", "until", "delete",
> > - "clear", "disable", "enable", "condition", "ignore", "frame",
> "catch",
> > - "tcatch", "return", "file", "exec-file", "core-file",
> "symbol-file",
> > - "load", "si", "ni", "shell", "sy",
> > + "signal", "step", "s", "stepi", "target", "thread", "until",
> "delete",
> > + "clear", "disable", "enable", "condition", "ignore", "frame",
> > + "select-frame", "f", "up", "down", "catch", "tcatch", "return",
> > + "file", "exec-file", "core-file", "symbol-file", "load", "si",
> "ni",
> > + "shell", "sy",
> > NULL /* must be last */
> > };
> >
> > @@ -1068,8 +1069,6 @@ unsigned long crash_get_kaslr_offset(void)
> >
> > /* Callbacks for crash_target */
> > int crash_get_nr_cpus(void);
> > -int crash_get_cpu_reg (int cpu, int regno, const char *regname,
> > - int regsize, void *val);
> >
> > int crash_get_nr_cpus(void)
> > {
> > @@ -1086,11 +1085,3 @@ int crash_get_nr_cpus(void)
> > return 1;
> > }
> >
> > -int crash_get_cpu_reg (int cpu, int regno, const char *regname,
> > - int regsize, void *value)
> > -{
> > - if (!machdep->get_cpu_reg)
> > - return FALSE;
> > - return machdep->get_cpu_reg(cpu, regno, regname, regsize,
> value);
> > -}
> > -
> > diff --git a/vmware_vmss.c b/vmware_vmss.c
> > index f6c5f32ea4c0..151cfdbca5e5 100644
> > --- a/vmware_vmss.c
> > +++ b/vmware_vmss.c
> > @@ -1,7 +1,7 @@
> > /*
> > * vmware_vmss.c
> > *
> > - * Copyright (c) 2015, 2020 VMware, Inc.
> > + * Copyright (c) 2015 VMware, Inc.
> > * Copyright (c) 2018 Red Hat Inc.
> > *
> > * This program is free software; you can redistribute it and/or modify
> > @@ -16,7 +16,6 @@
> > *
> > * Authors: Dyno Hongjun Fu <h...@vmware.com>
> > * Sergio Lopez <s...@redhat.com>
> > - * Alexey Makhalov <amakha...@vmware.com>
> > */
> >
> > #include "defs.h"
> > @@ -895,54 +894,6 @@ vmware_vmss_get_cr3_cr4_idtr(int cpu, ulong *cr3,
> ulong *cr4, ulong *idtr)
> > return TRUE;
> > }
> >
> > -int
> > -vmware_vmss_get_cpu_reg(int cpu, int regno, const char *name, int size,
> > - void *value)
> > -{
> > - if (cpu >= vmss.num_vcpus)
> > - return FALSE;
> > -
> > - /* All supported registers are 8 bytes long. */
> > - if (size != 8)
> > - return FALSE;
> > -
> > -#define CASE(R,r) \
> > - case R##_REGNUM: \
> > - if (!(vmss.vcpu_regs[cpu] & REGS_PRESENT_##R)) \
> > - return FALSE; \
> > - memcpy(value, &vmss.regs64[cpu]->r, size); \
> > - break
> > -
> > -
> > - switch (regno) {
> > - CASE (RAX, rax);
> > - CASE (RBX, rbx);
> > - CASE (RCX, rcx);
> > - CASE (RDX, rdx);
> > - CASE (RSI, rsi);
> > - CASE (RDI, rdi);
> > - CASE (RBP, rbp);
> > - CASE (RSP, rsp);
> > - CASE (R8, r8);
> > - CASE (R9, r9);
> > - CASE (R10, r10);
> > - CASE (R11, r11);
> > - CASE (R12, r12);
> > - CASE (R13, r13);
> > - CASE (R14, r14);
> > - CASE (R15, r15);
> > - CASE (RIP, rip);
> > - case EFLAGS_REGNUM:
> > - if (!(vmss.vcpu_regs[cpu] &
> REGS_PRESENT_RFLAGS))
> > - return FALSE;
> > - memcpy(value, &vmss.regs64[cpu]->rflags, size);
> > - break;
> > - default:
> > - return FALSE;
> > - }
> > - return TRUE;
> > -}
> > -
> > int
> > vmware_vmss_phys_base(ulong *phys_base)
> > {
> > diff --git a/x86_64.c b/x86_64.c
> > index 74bd1bbde41c..da94b1c0e917 100644
> > --- a/x86_64.c
> > +++ b/x86_64.c
> > @@ -126,7 +126,6 @@ static int x86_64_get_framesize(struct bt_info *,
> ulong, ulong);
> > static void x86_64_framesize_debug(struct bt_info *);
> > static void x86_64_get_active_set(void);
> > static int x86_64_get_kvaddr_ranges(struct vaddr_range *);
> > -static int x86_64_get_cpu_reg(int, int, const char *, int, void *);
> > static int x86_64_verify_paddr(uint64_t);
> > static void GART_init(void);
> > static void x86_64_exception_stacks_init(void);
> > @@ -195,7 +194,6 @@ x86_64_init(int when)
> > machdep->machspec->irq_eframe_link = UNINITIALIZED;
> > machdep->machspec->irq_stack_gap = UNINITIALIZED;
> > machdep->get_kvaddr_ranges = x86_64_get_kvaddr_ranges;
> > - machdep->get_cpu_reg = x86_64_get_cpu_reg;
> > if (machdep->cmdline_args[0])
> > parse_cmdline_args();
> > if ((string = pc->read_vmcoreinfo("relocate"))) {
> > @@ -889,7 +887,6 @@ x86_64_dump_machdep_table(ulong arg)
> > fprintf(fp, " is_page_ptr: x86_64_is_page_ptr()\n");
> > fprintf(fp, " verify_paddr: x86_64_verify_paddr()\n");
> > fprintf(fp, " get_kvaddr_ranges:
> x86_64_get_kvaddr_ranges()\n");
> > - fprintf(fp, " get_cpu_reg: x86_64_get_cpu_reg()\n");
> > fprintf(fp, " init_kernel_pgd: x86_64_init_kernel_pgd()\n");
> > fprintf(fp, "clear_machdep_cache:
> x86_64_clear_machdep_cache()\n");
> > fprintf(fp, " xendump_p2m_create: %s\n", PVOPS_XEN() ?
> > @@ -8957,19 +8954,6 @@ x86_64_get_kvaddr_ranges(struct vaddr_range *vrp)
> > return cnt;
> > }
> >
> > -static int
> > -x86_64_get_cpu_reg(int cpu, int regno, const char *name,
> > - int size, void *value)
> > -{
> > - if (regno >= LAST_REGNUM)
> > - return FALSE;
> > -
> > - if (VMSS_DUMPFILE())
> > - return vmware_vmss_get_cpu_reg(cpu, regno, name, size,
> value);
> > -
> > - return FALSE;
> > -}
> > -
> > /*
> > * Determine the physical memory range reserved for GART.
> > */
--
Crash-utility mailing list
Crash-utility@redhat.com
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
