[PATCH 8/8] arm64: stacktrace: track hyp stacks in unwinder's address space
Kalesh Singh
kaleshsingh at google.com
Tue Aug 2 09:27:33 PDT 2022
On Mon, Aug 1, 2022 at 5:12 AM Mark Rutland <mark.rutland at arm.com> wrote:
>
> Currently unwind_next_frame_record() has an optional callback to convert
> the address space of the FP. This is necessary for the NVHE unwinder,
> which tracks the stacks in the hyp VA space, but accesses the frame
> records in the kernel VA space.
>
> This is a bit unfortunate since it clutters unwind_next_frame_record(),
> which will get in the way of future rework.
>
> Instead, this patch changes the NVHE unwinder to track the stacks in the
> kernel's VA space and translate to FP prior to calling
> unwind_next_frame_record(). This removes the need for the translate_fp()
> callback, as all unwinders consistently track stacks in the native
> address space of the unwinder.
>
> At the same time, this patch consolidates the generation of the stack
> addreses behind the stackinfo_get_*() helpers.
>
> Signed-off-by: Mark Rutland <mark.rutland at arm.com>
> Cc: Fuad Tabba <tabba at google.com>
> Cc: Kalesh Singh <kaleshsingh at google.com>
> Cc: Madhavan T. Venkataraman <madvenka at linux.microsoft.com>
> Cc: Marc Zyngier <maz at kernel.org>
> Cc: Mark Brown <broonie at kernel.org>
> ---
> arch/arm64/include/asm/stacktrace/common.h | 28 ++--------
> arch/arm64/kernel/stacktrace.c | 2 +-
> arch/arm64/kvm/hyp/nvhe/stacktrace.c | 2 +-
> arch/arm64/kvm/stacktrace.c | 62 ++++++++++++++--------
> 4 files changed, 46 insertions(+), 48 deletions(-)
>
> diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
> index c594f332bb946..0f634bb14ceb3 100644
> --- a/arch/arm64/include/asm/stacktrace/common.h
> +++ b/arch/arm64/include/asm/stacktrace/common.h
> @@ -78,17 +78,6 @@ static inline void unwind_init_common(struct unwind_state *state,
> state->stack = stackinfo_get_unknown();
> }
>
> -/*
> - * stack_trace_translate_fp_fn() - Translates a non-kernel frame pointer to
> - * a kernel address.
> - *
> - * @fp: the frame pointer to be updated to its kernel address.
> - *
> - * Returns true and success and @fp is updated to the corresponding
> - * kernel virtual address; otherwise returns false.
> - */
> -typedef bool (*stack_trace_translate_fp_fn)(unsigned long *fp);
> -
> static struct stack_info *unwind_find_next_stack(const struct unwind_state *state,
> unsigned long sp,
> unsigned long size)
> @@ -159,13 +148,11 @@ static inline int unwind_consume_stack(struct unwind_state *state,
> * @state->fp.
> *
> * @state: the current unwind state.
> - * @translate_fp: translates the fp prior to access (may be NULL)
> */
> static inline int
> -unwind_next_frame_record(struct unwind_state *state,
> - stack_trace_translate_fp_fn translate_fp)
> +unwind_next_frame_record(struct unwind_state *state)
> {
> - unsigned long fp = state->fp, kern_fp = fp;
> + unsigned long fp = state->fp;
> int err;
>
> if (fp & 0x7)
> @@ -175,18 +162,11 @@ unwind_next_frame_record(struct unwind_state *state,
> if (err)
> return err;
>
> - /*
> - * If fp is not from the current address space perform the necessary
> - * translation before dereferencing it to get the next fp.
> - */
> - if (translate_fp && !translate_fp(&kern_fp))
> - return -EINVAL;
> -
> /*
> * Record this frame record's values.
> */
> - state->fp = READ_ONCE(*(unsigned long *)(kern_fp));
> - state->pc = READ_ONCE(*(unsigned long *)(kern_fp + 8));
> + state->fp = READ_ONCE(*(unsigned long *)(fp));
> + state->pc = READ_ONCE(*(unsigned long *)(fp + 8));
>
> return 0;
> }
> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> index 1133be3ff774d..6b447319ce5b9 100644
> --- a/arch/arm64/kernel/stacktrace.c
> +++ b/arch/arm64/kernel/stacktrace.c
> @@ -84,7 +84,7 @@ static int notrace unwind_next(struct unwind_state *state)
> if (fp == (unsigned long)task_pt_regs(tsk)->stackframe)
> return -ENOENT;
>
> - err = unwind_next_frame_record(state, NULL);
> + err = unwind_next_frame_record(state);
> if (err)
> return err;
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/stacktrace.c b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
> index 08e1325ead73f..ed6b58b19cfa5 100644
> --- a/arch/arm64/kvm/hyp/nvhe/stacktrace.c
> +++ b/arch/arm64/kvm/hyp/nvhe/stacktrace.c
> @@ -64,7 +64,7 @@ static struct stack_info stackinfo_get_hyp(void)
>
> static int unwind_next(struct unwind_state *state)
> {
> - return unwind_next_frame_record(state, NULL);
> + return unwind_next_frame_record(state);
> }
>
> static void notrace unwind(struct unwind_state *state,
> diff --git a/arch/arm64/kvm/stacktrace.c b/arch/arm64/kvm/stacktrace.c
> index 8295e132da2f0..fde1ec757d03a 100644
> --- a/arch/arm64/kvm/stacktrace.c
> +++ b/arch/arm64/kvm/stacktrace.c
> @@ -34,6 +34,17 @@ static struct stack_info stackinfo_get_overflow(void)
> };
> }
>
> +static struct stack_info stackinfo_get_overflow_kern_va(void)
> +{
> + unsigned long low = (unsigned long)this_cpu_ptr_nvhe_sym(overflow_stack);
> + unsigned long high = low + OVERFLOW_STACK_SIZE;
> +
> + return (struct stack_info) {
> + .low = low,
> + .high = high,
> + };
> +}
> +
> static struct stack_info stackinfo_get_hyp(void)
> {
> struct kvm_nvhe_stacktrace_info *stacktrace_info
> @@ -47,6 +58,17 @@ static struct stack_info stackinfo_get_hyp(void)
> };
> }
>
> +static struct stack_info stackinfo_get_hyp_kern_va(void)
> +{
> + unsigned long low = (unsigned long)*this_cpu_ptr(&kvm_arm_hyp_stack_page);
> + unsigned long high = low + PAGE_SIZE;
> +
> + return (struct stack_info) {
> + .low = low,
> + .high = high,
> + };
> +}
> +
> /*
> * kvm_nvhe_stack_kern_va - Convert KVM nVHE HYP stack addresses to a kernel VAs
> *
> @@ -62,39 +84,35 @@ static struct stack_info stackinfo_get_hyp(void)
> */
> static bool kvm_nvhe_stack_kern_va(unsigned long *addr)
> {
> - struct kvm_nvhe_stacktrace_info *stacktrace_info;
> - unsigned long hyp_base, kern_base, hyp_offset;
> - struct stack_info stack;
> + struct stack_info stack_hyp, stack_kern;
>
> - stacktrace_info = this_cpu_ptr_nvhe_sym(kvm_stacktrace_info);
> -
> - stack = stackinfo_get_hyp();
> - if (stackinfo_on_stack(&stack, *addr, 1)) {
> - kern_base = (unsigned long)*this_cpu_ptr(&kvm_arm_hyp_stack_page);
> - hyp_base = (unsigned long)stacktrace_info->stack_base;
> + stack_hyp = stackinfo_get_hyp();
> + stack_kern = stackinfo_get_hyp_kern_va();
> + if (stackinfo_on_stack(&stack_hyp, *addr, 1))
> goto found;
> - }
>
> - stack = stackinfo_get_overflow();
> - if (stackinfo_on_stack(&stack, *addr, 1)) {
> - kern_base = (unsigned long)this_cpu_ptr_nvhe_sym(overflow_stack);
> - hyp_base = (unsigned long)stacktrace_info->overflow_stack_base;
> + stack_hyp = stackinfo_get_overflow();
> + stack_kern = stackinfo_get_overflow_kern_va();
> + if (stackinfo_on_stack(&stack_hyp, *addr, 1))
> goto found;
> - }
>
> return false;
>
> found:
> - hyp_offset = *addr - hyp_base;
> -
> - *addr = kern_base + hyp_offset;
> -
> + *addr = *addr - stack_hyp.low + stack_kern.low;
> return true;
> }
>
> static int unwind_next(struct unwind_state *state)
> {
> - return unwind_next_frame_record(state, kvm_nvhe_stack_kern_va);
> + /*
> + * The FP is in the hypervisor VA space. Convert it to the kernel VA
> + * space so it can be unwound be the regular unwind functions.
s/be the/by the
Reviewed-by: Kalesh Singh <kaleshsingh at google.com>
> + */
> + if (!kvm_nvhe_stack_kern_va(&state->fp))
> + return -EINVAL;
> +
> + return unwind_next_frame_record(state);
> }
>
> static void unwind(struct unwind_state *state,
> @@ -153,8 +171,8 @@ static void hyp_dump_backtrace(unsigned long hyp_offset)
> {
> struct kvm_nvhe_stacktrace_info *stacktrace_info;
> struct stack_info stacks[] = {
> - stackinfo_get_overflow(),
> - stackinfo_get_hyp(),
> + stackinfo_get_overflow_kern_va(),
> + stackinfo_get_hyp_kern_va(),
> };
> struct unwind_state state = {
> .stacks = stacks,
> --
> 2.30.2
>
More information about the linux-arm-kernel
mailing list