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