Re: [Crash-utility] [PATCH 2/3] xen: get stack address via stack_base array if available

On Mon, Mar 13, 2023 at 9:07 PM <crash-utility-request(a)redhat.com <mailto:crash-utility-request@redhat.com>> wrote: Date: Mon, 13 Mar 2023 14:01:11 +0100 From: Juergen Gross <jgross(a)suse.com <mailto:jgross@suse.com>> To: crash-utility(a)redhat.com <mailto:crash-utility@redhat.com> Subject: [Crash-utility] [PATCH 2/3] xen: get stack address via         stack_base array if available Message-ID: <20230313130112.15353-3-jgross(a)suse.com <mailto:20230313130112.15353-3-jgross@suse.com>> Content-Type: text/plain; charset="US-ASCII"; x-default=true Since many years now the stack address of each percpu stack is available via the stack_base[] array now. Use that instead of the indirect method via the percpu variables tss_init or tss_page, especially as the layout of tss_page has changed in Xen 4.16, resulting in the stack no longer to be found. It should be good to add the hypervisor commit(if any) here. Signed-off-by: Juergen Gross <jgross(a)suse.com <mailto:jgross@suse.com>> ---  xen_hyper.c | 50 ++++++++++++++++++++++++++++++--------------------  1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/xen_hyper.c b/xen_hyper.c index 1030c0a..72720e2 100644 --- a/xen_hyper.c +++ b/xen_hyper.c @@ -324,7 +324,7 @@ void  xen_hyper_x86_pcpu_init(void)  {         ulong cpu_info; -       ulong init_tss_base, init_tss; +       ulong init_tss_base, init_tss, stack_base;         ulong sp;         struct xen_hyper_pcpu_context *pcc;         char *buf, *bp; @@ -340,34 +340,44 @@ xen_hyper_x86_pcpu_init(void)         }         /* get physical cpu context */         xen_hyper_alloc_pcpu_context_space(XEN_HYPER_MAX_CPUS()); -       if (symbol_exists("per_cpu__init_tss")) { +       if (symbol_exists("stack_base")) { +               stack_base = symbol_value("stack_base"); +               flag = 0; +       } else if (symbol_exists("per_cpu__init_tss")) {                 init_tss_base = symbol_value("per_cpu__init_tss"); -               flag = TRUE; +               flag = 1;         } else if (symbol_exists("per_cpu__tss_page")) {                         init_tss_base = symbol_value("per_cpu__tss_page"); -                       flag = TRUE; +                       flag = 1;         } else {                 init_tss_base = symbol_value("init_tss"); -               flag = FALSE; +               flag = 2;         }         buf = GETBUF(XEN_HYPER_SIZE(tss)); The buf is not used in the stack_base code path, is it possible to not call the GETBUF() in the stack_base code path?

        for_cpu_indexes(i, cpuid)         { -               if (flag) -                       init_tss = xen_hyper_per_cpu(init_tss_base, cpuid); -               else -                       init_tss = init_tss_base + -                               XEN_HYPER_SIZE(tss) * cpuid; -               if (!readmem(init_tss, KVADDR, buf, -                       XEN_HYPER_SIZE(tss), "init_tss", RETURN_ON_ERROR)) { -                       error(FATAL, "cannot read init_tss.\n"); -               } -               if (machine_type("X86")) { -                       sp = ULONG(buf + XEN_HYPER_OFFSET(tss_esp0)); -               } else if (machine_type("X86_64")) { -                       sp = ULONG(buf + XEN_HYPER_OFFSET(tss_rsp0)); -               } else -                       sp = 0; +               if (flag) { +                       if (flag == 1) +                               init_tss = xen_hyper_per_cpu(init_tss_base, cpuid); +                       else +                               init_tss = init_tss_base + +                                       XEN_HYPER_SIZE(tss) * cpuid; +                       if (!readmem(init_tss, KVADDR, buf, +                               XEN_HYPER_SIZE(tss), "init_tss", RETURN_ON_ERROR)) { +                               error(FATAL, "cannot read init_tss.\n"); +                       } +                       if (machine_type("X86")) { +                               sp = ULONG(buf + XEN_HYPER_OFFSET(tss_esp0)); +                       } else if (machine_type("X86_64")) { +                               sp = ULONG(buf + XEN_HYPER_OFFSET(tss_rsp0)); +                       } else +                               sp = 0; +               } else { +                       if (!readmem(stack_base + sizeof(ulong) * cpuid, KVADDR, &sp, +                               sizeof(ulong), "stack_base", RETURN_ON_ERROR)) { +                               error(FATAL, "cannot read stack_base.\n"); +                       } The above if(!readmem()) code can be replaced with: readmem(stack_base + sizeof(ulong) * cpuid, KVADDR, &sp, sizeof(ulong), "stack_base", FAULT_ON_ERROR)); It looks more concise.