On Mon, Mar 13, 2023 at 9:07 PM <crash-utility-request@redhat.com> wrote:
Date: Mon, 13 Mar 2023 14:01:11 +0100
From: Juergen Gross <jgross@suse.com>
To: 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@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@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.

Thanks.
Lianbo

+               }
                cpu_info = XEN_HYPER_GET_CPU_INFO(sp);
                if (CRASHDEBUG(1)) {
                        fprintf(fp, "sp=%lx, cpu_info=%lx\n", sp, cpu_info);
--
2.35.3