[Crash-utility] [PATCH] Make the __per_cpu_offset symbol available

[Dave Anderson]

----- Original Message -----
> Hi,
> 
> the code in xen_hyper_init() tries to determine the per-cpu shift from the
> __per_cpu_shift symbol if available. This doesn't work because the symbol's
> value is outside the kernel text address range. This patch makes a special
> case for this symbol on Xen targets, so it can be used later.

Hi Petr,

The patch looks OK -- but it seems to be a bit of overkill adding a bunch
of new functions?  Couldn't each of the original functions simply have
a check like this right at the top?:

     if (XEN_HYPER_MODE() && STREQ(symbol, "__per_cpu_shift"))
         return TRUE;

> Without this patch, crash initialization fails on SLES11 Xen hypervisor dumps,
> because on incorrect per-cpu offset is assumed. The failure can't be fixed
> simply by adding a version check, because SLES11 Xen is patched to increase
> the per-cpu shift in order to support more CPUs.
> 
> It is also important not to try to relocate ABSOLUTE symbol (such as this
> one), so add this condition in store_symbols().

The reloc stuff is only applicable to live x86/x86_64 vmlinux sessions.  To absolutely
ensure backwards-compatibility, I'd rather just put this at the top of relocate():

     if (XEN_HYPER_MODE())
             return symval;

Would that work for you?

Dave

> Signed-off-by: Petr Tesarik <ptesarik at suse.cz>
> ---
> ia64.c | 22 ++++++++++++++++++++--
> symbols.c | 3 ++-
> x86.c | 22 ++++++++++++++++++++--
> x86_64.c | 22 ++++++++++++++++++++--
> 4 files changed, 62 insertions(+), 7 deletions(-)
> 
> --- a/ia64.c
> +++ b/ia64.c
> @@ -20,6 +20,7 @@
> #include <sys/prctl.h>
> 
> static int ia64_verify_symbol(const char *, ulong, char);
> +static int ia64_hyper_verify_symbol(const char *, ulong, char);
> static int ia64_eframe_search(struct bt_info *);
> static void ia64_back_trace_cmd(struct bt_info *);
> static void ia64_old_unwind(struct bt_info *);
> @@ -571,7 +572,12 @@ ia64_dump_machdep_table(ulong arg)
> fprintf(fp, " memory_size: generic_memory_size()\n");
> fprintf(fp, " vmalloc_start: ia64_vmalloc_start()\n");
> fprintf(fp, " is_task_addr: ia64_is_task_addr()\n");
> - fprintf(fp, " verify_symbol: ia64_verify_symbol()\n");
> + if (machdep->verify_symbol == ia64_verify_symbol)
> + fprintf(fp, " verify_symbol: ia64_verify_symbol()\n");
> + else if (machdep->verify_symbol == ia64_hyper_verify_symbol)
> + fprintf(fp, " verify_symbol: ia64_hyper_verify_symbol()\n");
> + else
> + fprintf(fp, " verify_symbol: %lx\n", (ulong)machdep->verify_symbol);
> fprintf(fp, " dis_filter: ia64_dis_filter()\n");
> fprintf(fp, " cmd_mach: ia64_cmd_mach()\n");
> fprintf(fp, " get_smp_cpus: ia64_get_smp_cpus()\n");
> @@ -724,6 +730,18 @@ ia64_verify_symbol(const char *name, ulo
> (region == KERNEL_VMALLOC_REGION)));
> }
> 
> +static int
> +ia64_hyper_verify_symbol(const char *name, ulong value, char type)
> +{
> + if (ia64_verify_symbol(name, value, type))
> + return TRUE;
> +
> + if (STREQ(name, "__per_cpu_shift"))
> + return TRUE;
> +
> + return FALSE;
> +}
> +
> 
> /*
> * Look for likely exception frames in a stack.
> @@ -4225,7 +4243,7 @@ ia64_init_hyper(int when)
> break;
> 
> case PRE_SYMTAB:
> - machdep->verify_symbol = ia64_verify_symbol;
> + machdep->verify_symbol = ia64_hyper_verify_symbol;
> machdep->machspec = &ia64_machine_specific;
> if (pc->flags & KERNEL_DEBUG_QUERY)
> return;
> --- a/symbols.c
> +++ b/symbols.c
> @@ -568,7 +568,8 @@ store_symbols(bfd *abfd, int dynamic, vo
> bfd_get_symbol_info(abfd, sym, &syminfo);
> if (machdep->verify_symbol(syminfo.name, syminfo.value,
> syminfo.type)) {
> - if (kt->flags & (RELOC_SET|RELOC_FORCE))
> + if ((kt->flags & (RELOC_SET|RELOC_FORCE)) &&
> + sym->section != &bfd_abs_section)
> sp->value = relocate(syminfo.value,
> (char *)syminfo.name, !(first++));
> else
> --- a/x86.c
> +++ b/x86.c
> @@ -986,6 +986,7 @@ static void eframe_init(void);
> static char *extract_idt_function(ulong *, char *, ulong *);
> static int x86_is_task_addr(ulong);
> static int x86_verify_symbol(const char *, ulong, char);
> +static int x86_hyper_verify_symbol(const char *, ulong, char);
> static int x86_eframe_search(struct bt_info *);
> static ulong x86_in_irqstack(ulong);
> static int x86_dis_filter(ulong, char *);
> @@ -3254,7 +3255,12 @@ x86_dump_machdep_table(ulong arg)
> fprintf(fp, " memory_size: x86_memory_size()\n");
> fprintf(fp, " vmalloc_start: x86_vmalloc_start()\n");
> fprintf(fp, " is_task_addr: x86_is_task_addr()\n");
> - fprintf(fp, " verify_symbol: x86_verify_symbol()\n");
> + if (machdep->verify_symbol == x86_verify_symbol)
> + fprintf(fp, " verify_symbol: x86_verify_symbol()\n");
> + else if (machdep->verify_symbol == x86_hyper_verify_symbol)
> + fprintf(fp, " verify_symbol: x86_hyper_verify_symbol()\n");
> + else
> + fprintf(fp, " verify_symbol: %lx\n", (ulong)machdep->verify_symbol);
> fprintf(fp, " dis_filter: x86_dis_filter()\n");
> fprintf(fp, " cmd_mach: x86_cmd_mach()\n");
> fprintf(fp, " get_smp_cpus: x86_get_smp_cpus()\n");
> @@ -3789,6 +3795,18 @@ x86_verify_symbol(const char *name, ulon
> return TRUE;
> }
> 
> +static int
> +x86_hyper_verify_symbol(const char *name, ulong value, char type)
> +{
> + if (x86_verify_symbol(name, value, type))
> + return TRUE;
> +
> + if (STREQ(name, "__per_cpu_shift"))
> + return TRUE;
> +
> + return FALSE;
> +}
> +
> /*
> * Filter disassembly output if the output radix is not gdb's default
> 10
> */
> @@ -4967,7 +4985,7 @@ x86_init_hyper(int when)
> switch (when)
> {
> case PRE_SYMTAB:
> - machdep->verify_symbol = x86_verify_symbol;
> + machdep->verify_symbol = x86_hyper_verify_symbol;
> if (pc->flags & KERNEL_DEBUG_QUERY)
> return;
> machdep->pagesize = memory_page_size();
> --- a/x86_64.c
> +++ b/x86_64.c
> @@ -27,6 +27,7 @@ static int x86_64_uvtop_level4_rhel4_xen
> static ulong x86_64_vmalloc_start(void);
> static int x86_64_is_task_addr(ulong);
> static int x86_64_verify_symbol(const char *, ulong, char);
> +static int x86_64_hyper_verify_symbol(const char *, ulong, char);
> static ulong x86_64_get_task_pgd(ulong);
> static int x86_64_translate_pte(ulong, void *, ulonglong);
> static ulong x86_64_processor_speed(void);
> @@ -491,7 +492,12 @@ x86_64_dump_machdep_table(ulong arg)
> fprintf(fp, " memory_size: generic_memory_size()\n");
> fprintf(fp, " vmalloc_start: x86_64_vmalloc_start()\n");
> fprintf(fp, " is_task_addr: x86_64_is_task_addr()\n");
> - fprintf(fp, " verify_symbol: x86_64_verify_symbol()\n");
> + if (machdep->verify_symbol == x86_64_verify_symbol)
> + fprintf(fp, " verify_symbol: x86_64_verify_symbol()\n");
> + else if (machdep->verify_symbol == x86_64_hyper_verify_symbol)
> + fprintf(fp, " verify_symbol: x86_64_hyper_verify_symbol()\n");
> + else
> + fprintf(fp, " verify_symbol: %lx\n", (ulong)machdep->verify_symbol);
> fprintf(fp, " dis_filter: x86_64_dis_filter()\n");
> fprintf(fp, " cmd_mach: x86_64_cmd_mach()\n");
> fprintf(fp, " get_smp_cpus: x86_64_get_smp_cpus()\n");
> @@ -1978,6 +1984,18 @@ x86_64_verify_symbol(const char *name, u
> return TRUE;
> }
> 
> +static int
> +x86_64_hyper_verify_symbol(const char *name, ulong value, char type)
> +{
> + if (x86_64_verify_symbol(name, value, type))
> + return TRUE;
> +
> + if (STREQ(name, "__per_cpu_shift"))
> + return TRUE;
> +
> + return FALSE;
> +}
> +
> 
> /*
> * Get the relevant page directory pointer from a task structure.
> @@ -5949,7 +5967,7 @@ x86_64_init_hyper(int when)
> switch (when)
> {
> case PRE_SYMTAB:
> - machdep->verify_symbol = x86_64_verify_symbol;
> + machdep->verify_symbol = x86_64_hyper_verify_symbol;
> machdep->machspec = &x86_64_machine_specific;
> if (pc->flags & KERNEL_DEBUG_QUERY)
> return;
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility