Unwind the stack in EL1, when CONFIG_NVHE_EL2_DEBUG is enabled. This is
possible because CONFIG_NVHE_EL2_DEBUG disables the host stage 2 protection
which allows host to access the hypervisor stack pages in EL1.
Unwinding and dumping hyp call traces is gated on CONFIG_NVHE_EL2_DEBUG
to avoid the potential leaking of information to the host.
A simple stack overflow test produces the following output:
[ 580.376051][ T412] kvm: nVHE hyp panic at: ffffffc0116145c4!
[ 580.378034][ T412] kvm [412]: nVHE HYP call trace:
[ 580.378591][ T412] kvm [412]: [<ffffffc011614934>]
[ 580.378993][ T412] kvm [412]: [<ffffffc01160fa48>]
[ 580.379386][ T412] kvm [412]: [<ffffffc0116145dc>] // Non-terminating recursive call
[ 580.379772][ T412] kvm [412]: [<ffffffc0116145dc>]
[ 580.380158][ T412] kvm [412]: [<ffffffc0116145dc>]
[ 580.380544][ T412] kvm [412]: [<ffffffc0116145dc>]
[ 580.380928][ T412] kvm [412]: [<ffffffc0116145dc>]
. . .
Since nVHE hyp symbols are not included by kallsyms to avoid issues
with aliasing, we fallback to the vmlinux addresses. Symbolizing the
addresses is handled in the next patch in this series.
Signed-off-by: Kalesh Singh <[email protected]>
---
Changes in v3:
- The nvhe hyp stack unwinder now makes use of the core logic from the
regular kernel unwinder to avoid duplication, per Mark
Changes in v2:
- Add cpu_prepare_nvhe_panic_info()
- Move updating the panic info to hyp_panic(), so that unwinding also
works for conventional nVHE Hyp-mode.
arch/arm64/include/asm/kvm_asm.h | 19 +++
arch/arm64/include/asm/stacktrace.h | 12 ++
arch/arm64/kernel/stacktrace.c | 210 +++++++++++++++++++++++++---
arch/arm64/kvm/Kconfig | 5 +-
arch/arm64/kvm/arm.c | 2 +-
arch/arm64/kvm/handle_exit.c | 3 +
arch/arm64/kvm/hyp/nvhe/switch.c | 18 +++
7 files changed, 243 insertions(+), 26 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 2e277f2ed671..16efdf150a37 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -176,6 +176,25 @@ struct kvm_nvhe_init_params {
unsigned long vtcr;
};
+#ifdef CONFIG_NVHE_EL2_DEBUG
+/*
+ * Used by the host in EL1 to dump the nVHE hypervisor backtrace on
+ * hyp_panic. This is possible because CONFIG_NVHE_EL2_DEBUG disables
+ * the host stage 2 protection. See: __hyp_do_panic()
+ *
+ * @hyp_stack_base: hyp VA of the hyp_stack base.
+ * @hyp_overflow_stack_base: hyp VA of the hyp_overflow_stack base.
+ * @fp: hyp FP where the backtrace begins.
+ * @pc: hyp PC where the backtrace begins.
+ */
+struct kvm_nvhe_panic_info {
+ unsigned long hyp_stack_base;
+ unsigned long hyp_overflow_stack_base;
+ unsigned long fp;
+ unsigned long pc;
+};
+#endif /* CONFIG_NVHE_EL2_DEBUG */
+
/* Translate a kernel address @ptr into its equivalent linear mapping */
#define kvm_ksym_ref(ptr) \
({ \
diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
index e77cdef9ca29..18611a51cf14 100644
--- a/arch/arm64/include/asm/stacktrace.h
+++ b/arch/arm64/include/asm/stacktrace.h
@@ -22,6 +22,10 @@ enum stack_type {
STACK_TYPE_OVERFLOW,
STACK_TYPE_SDEI_NORMAL,
STACK_TYPE_SDEI_CRITICAL,
+#ifdef CONFIG_NVHE_EL2_DEBUG
+ STACK_TYPE_KVM_NVHE_HYP,
+ STACK_TYPE_KVM_NVHE_OVERFLOW,
+#endif /* CONFIG_NVHE_EL2_DEBUG */
__NR_STACK_TYPES
};
@@ -147,4 +151,12 @@ static inline bool on_accessible_stack(const struct task_struct *tsk,
return false;
}
+#ifdef CONFIG_NVHE_EL2_DEBUG
+void kvm_nvhe_dump_backtrace(unsigned long hyp_offset);
+#else
+static inline void kvm_nvhe_dump_backtrace(unsigned long hyp_offset)
+{
+}
+#endif /* CONFIG_NVHE_EL2_DEBUG */
+
#endif /* __ASM_STACKTRACE_H */
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index e4103e085681..6ec85cb69b1f 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -15,6 +15,8 @@
#include <asm/irq.h>
#include <asm/pointer_auth.h>
+#include <asm/kvm_asm.h>
+#include <asm/kvm_hyp.h>
#include <asm/stack_pointer.h>
#include <asm/stacktrace.h>
@@ -64,26 +66,15 @@ NOKPROBE_SYMBOL(start_backtrace);
* records (e.g. a cycle), determined based on the location and fp value of A
* and the location (but not the fp value) of B.
*/
-static int notrace unwind_frame(struct task_struct *tsk,
- struct stackframe *frame)
+static int notrace __unwind_frame(struct stackframe *frame, struct stack_info *info,
+ unsigned long (*translate_fp)(unsigned long, enum stack_type))
{
unsigned long fp = frame->fp;
- struct stack_info info;
-
- if (!tsk)
- tsk = current;
-
- /* Final frame; nothing to unwind */
- if (fp == (unsigned long)task_pt_regs(tsk)->stackframe)
- return -ENOENT;
if (fp & 0x7)
return -EINVAL;
- if (!on_accessible_stack(tsk, fp, 16, &info))
- return -EINVAL;
-
- if (test_bit(info.type, frame->stacks_done))
+ if (test_bit(info->type, frame->stacks_done))
return -EINVAL;
/*
@@ -94,28 +85,62 @@ static int notrace unwind_frame(struct task_struct *tsk,
*
* TASK -> IRQ -> OVERFLOW -> SDEI_NORMAL
* TASK -> SDEI_NORMAL -> SDEI_CRITICAL -> OVERFLOW
+ * KVM_NVHE_HYP -> KVM_NVHE_OVERFLOW
*
* ... but the nesting itself is strict. Once we transition from one
* stack to another, it's never valid to unwind back to that first
* stack.
*/
- if (info.type == frame->prev_type) {
+ if (info->type == frame->prev_type) {
if (fp <= frame->prev_fp)
return -EINVAL;
} else {
set_bit(frame->prev_type, frame->stacks_done);
}
+ /* Record fp as prev_fp before attempting to get the next fp */
+ frame->prev_fp = fp;
+
+ /*
+ * If fp is not from the current address space perform the
+ * necessary translation before dereferencing it to get next fp.
+ */
+ if (translate_fp)
+ fp = translate_fp(fp, info->type);
+ if (!fp)
+ return -EINVAL;
+
/*
* Record this frame record's values and location. The prev_fp and
- * prev_type are only meaningful to the next unwind_frame() invocation.
+ * prev_type are only meaningful to the next __unwind_frame() invocation.
*/
frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
- frame->prev_fp = fp;
- frame->prev_type = info.type;
-
frame->pc = ptrauth_strip_insn_pac(frame->pc);
+ frame->prev_type = info->type;
+
+ return 0;
+}
+
+static int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
+{
+ unsigned long fp = frame->fp;
+ struct stack_info info;
+ int err;
+
+ if (!tsk)
+ tsk = current;
+
+ /* Final frame; nothing to unwind */
+ if (fp == (unsigned long)task_pt_regs(tsk)->stackframe)
+ return -ENOENT;
+
+ if (!on_accessible_stack(tsk, fp, 16, &info))
+ return -EINVAL;
+
+ err = __unwind_frame(frame, &info, NULL);
+ if (err)
+ return err;
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
if (tsk->ret_stack &&
@@ -143,20 +168,27 @@ static int notrace unwind_frame(struct task_struct *tsk,
}
NOKPROBE_SYMBOL(unwind_frame);
-static void notrace walk_stackframe(struct task_struct *tsk,
- struct stackframe *frame,
- bool (*fn)(void *, unsigned long), void *data)
+static void notrace __walk_stackframe(struct task_struct *tsk, struct stackframe *frame,
+ bool (*fn)(void *, unsigned long), void *data,
+ int (*unwind_frame_fn)(struct task_struct *tsk, struct stackframe *frame))
{
while (1) {
int ret;
if (!fn(data, frame->pc))
break;
- ret = unwind_frame(tsk, frame);
+ ret = unwind_frame_fn(tsk, frame);
if (ret < 0)
break;
}
}
+
+static void notrace walk_stackframe(struct task_struct *tsk,
+ struct stackframe *frame,
+ bool (*fn)(void *, unsigned long), void *data)
+{
+ __walk_stackframe(tsk, frame, fn, data, unwind_frame);
+}
NOKPROBE_SYMBOL(walk_stackframe);
static bool dump_backtrace_entry(void *arg, unsigned long where)
@@ -210,3 +242,135 @@ noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry,
walk_stackframe(task, &frame, consume_entry, cookie);
}
+
+#ifdef CONFIG_NVHE_EL2_DEBUG
+DECLARE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
+DECLARE_KVM_NVHE_PER_CPU(unsigned long [PAGE_SIZE/sizeof(long)], hyp_overflow_stack);
+DECLARE_KVM_NVHE_PER_CPU(struct kvm_nvhe_panic_info, kvm_panic_info);
+
+static inline bool kvm_nvhe_on_overflow_stack(unsigned long sp, unsigned long size,
+ struct stack_info *info)
+{
+ struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
+ unsigned long low = (unsigned long)panic_info->hyp_overflow_stack_base;
+ unsigned long high = low + PAGE_SIZE;
+
+ return on_stack(sp, size, low, high, STACK_TYPE_KVM_NVHE_OVERFLOW, info);
+}
+
+static inline bool kvm_nvhe_on_hyp_stack(unsigned long sp, unsigned long size,
+ struct stack_info *info)
+{
+ struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
+ unsigned long low = (unsigned long)panic_info->hyp_stack_base;
+ unsigned long high = low + PAGE_SIZE;
+
+ return on_stack(sp, size, low, high, STACK_TYPE_KVM_NVHE_HYP, info);
+}
+
+static inline bool kvm_nvhe_on_accessible_stack(unsigned long sp, unsigned long size,
+ struct stack_info *info)
+{
+ if (info)
+ info->type = STACK_TYPE_UNKNOWN;
+
+ if (kvm_nvhe_on_hyp_stack(sp, size, info))
+ return true;
+ if (kvm_nvhe_on_overflow_stack(sp, size, info))
+ return true;
+
+ return false;
+}
+
+static unsigned long kvm_nvhe_hyp_stack_kern_va(unsigned long addr)
+{
+ struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
+ unsigned long hyp_base, kern_base, hyp_offset;
+
+ hyp_base = (unsigned long)panic_info->hyp_stack_base;
+ hyp_offset = addr - hyp_base;
+
+ kern_base = (unsigned long)*this_cpu_ptr(&kvm_arm_hyp_stack_page);
+
+ return kern_base + hyp_offset;
+}
+
+static unsigned long kvm_nvhe_overflow_stack_kern_va(unsigned long addr)
+{
+ struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
+ unsigned long hyp_base, kern_base, hyp_offset;
+
+ hyp_base = (unsigned long)panic_info->hyp_overflow_stack_base;
+ hyp_offset = addr - hyp_base;
+
+ kern_base = (unsigned long)this_cpu_ptr_nvhe_sym(hyp_overflow_stack);
+
+ return kern_base + hyp_offset;
+}
+
+/*
+ * Convert KVM nVHE hypervisor stack VA to a kernel VA.
+ *
+ * The nVHE hypervisor stack is mapped in the flexible 'private' VA range, to allow
+ * for guard pages below the stack. Consequently, the fixed offset address
+ * translation macros won't work here.
+ *
+ * The kernel VA is calculated as an offset from the kernel VA of the hypervisor
+ * stack base. See: kvm_nvhe_hyp_stack_kern_va(), kvm_nvhe_overflow_stack_kern_va()
+ */
+static unsigned long kvm_nvhe_stack_kern_va(unsigned long addr,
+ enum stack_type type)
+{
+ switch (type) {
+ case STACK_TYPE_KVM_NVHE_HYP:
+ return kvm_nvhe_hyp_stack_kern_va(addr);
+ case STACK_TYPE_KVM_NVHE_OVERFLOW:
+ return kvm_nvhe_overflow_stack_kern_va(addr);
+ default:
+ return 0UL;
+ }
+}
+
+static int notrace kvm_nvhe_unwind_frame(struct task_struct *tsk,
+ struct stackframe *frame)
+{
+ struct stack_info info;
+
+ if (!kvm_nvhe_on_accessible_stack(frame->fp, 16, &info))
+ return -EINVAL;
+
+ return __unwind_frame(frame, &info, kvm_nvhe_stack_kern_va);
+}
+
+static bool kvm_nvhe_dump_backtrace_entry(void *arg, unsigned long where)
+{
+ unsigned long va_mask = GENMASK_ULL(vabits_actual - 1, 0);
+ unsigned long hyp_offset = (unsigned long)arg;
+
+ where &= va_mask; /* Mask tags */
+ where += hyp_offset; /* Convert to kern addr */
+
+ kvm_err("[<%016lx>] %pB\n", where, (void *)where);
+
+ return true;
+}
+
+static void notrace kvm_nvhe_walk_stackframe(struct task_struct *tsk,
+ struct stackframe *frame,
+ bool (*fn)(void *, unsigned long), void *data)
+{
+ __walk_stackframe(tsk, frame, fn, data, kvm_nvhe_unwind_frame);
+}
+
+void kvm_nvhe_dump_backtrace(unsigned long hyp_offset)
+{
+ struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
+ struct stackframe frame;
+
+ start_backtrace(&frame, panic_info->fp, panic_info->pc);
+ pr_err("nVHE HYP call trace:\n");
+ kvm_nvhe_walk_stackframe(NULL, &frame, kvm_nvhe_dump_backtrace_entry,
+ (void *)hyp_offset);
+ pr_err("---- end of nVHE HYP call trace ----\n");
+}
+#endif /* CONFIG_NVHE_EL2_DEBUG */
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 8a5fbbf084df..75f2c8255ff0 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -51,8 +51,9 @@ config NVHE_EL2_DEBUG
depends on KVM
help
Say Y here to enable the debug mode for the non-VHE KVM EL2 object.
- Failure reports will BUG() in the hypervisor. This is intended for
- local EL2 hypervisor development.
+ Failure reports will BUG() in the hypervisor; and panics will print
+ the hypervisor call stack. This is intended for local EL2 hypervisor
+ development.
If unsure, say N.
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 7a23630c4a7f..66c07c04eb52 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -49,7 +49,7 @@ DEFINE_STATIC_KEY_FALSE(kvm_protected_mode_initialized);
DECLARE_KVM_HYP_PER_CPU(unsigned long, kvm_hyp_vector);
-static DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
+DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
unsigned long kvm_arm_hyp_percpu_base[NR_CPUS];
DECLARE_KVM_NVHE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index e3140abd2e2e..ff69dff33700 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -17,6 +17,7 @@
#include <asm/kvm_emulate.h>
#include <asm/kvm_mmu.h>
#include <asm/debug-monitors.h>
+#include <asm/stacktrace.h>
#include <asm/traps.h>
#include <kvm/arm_hypercalls.h>
@@ -326,6 +327,8 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr,
kvm_err("nVHE hyp panic at: %016llx!\n", elr_virt + hyp_offset);
}
+ kvm_nvhe_dump_backtrace(hyp_offset);
+
/*
* Hyp has panicked and we're going to handle that by panicking the
* kernel. The kernel offset will be revealed in the panic so we're
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index efc20273a352..b8ecffc47424 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -37,6 +37,22 @@ DEFINE_PER_CPU(unsigned long, kvm_hyp_vector);
#ifdef CONFIG_NVHE_EL2_DEBUG
DEFINE_PER_CPU(unsigned long [PAGE_SIZE/sizeof(long)], hyp_overflow_stack)
__aligned(16);
+DEFINE_PER_CPU(struct kvm_nvhe_panic_info, kvm_panic_info);
+
+static inline void cpu_prepare_nvhe_panic_info(void)
+{
+ struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr(&kvm_panic_info);
+ struct kvm_nvhe_init_params *params = this_cpu_ptr(&kvm_init_params);
+
+ panic_info->hyp_stack_base = (unsigned long)(params->stack_hyp_va - PAGE_SIZE);
+ panic_info->hyp_overflow_stack_base = (unsigned long)this_cpu_ptr(hyp_overflow_stack);
+ panic_info->fp = (unsigned long)__builtin_frame_address(0);
+ panic_info->pc = _THIS_IP_;
+}
+ #else
+static inline void cpu_prepare_nvhe_panic_info(void)
+{
+}
#endif
static void __activate_traps(struct kvm_vcpu *vcpu)
@@ -360,6 +376,8 @@ asmlinkage void __noreturn hyp_panic(void)
struct kvm_cpu_context *host_ctxt;
struct kvm_vcpu *vcpu;
+ cpu_prepare_nvhe_panic_info();
+
host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
vcpu = host_ctxt->__hyp_running_vcpu;
--
2.35.1.473.g83b2b277ed-goog
Hi Kalesh,
On Thu, Feb 24, 2022 at 5:22 AM Kalesh Singh <[email protected]> wrote:
>
> Unwind the stack in EL1, when CONFIG_NVHE_EL2_DEBUG is enabled. This is
> possible because CONFIG_NVHE_EL2_DEBUG disables the host stage 2 protection
> which allows host to access the hypervisor stack pages in EL1.
For this comment to be clearer, and if my understanding is correct, I
think that it should say that CONFIG_NVHE_EL2_DEBUG allows host stage
2 protection to be disabled on a hyp_panic. Otherwise, on reading the
comment one might think that CONFIG_NVHE_EL2_DEBUG runs without host
stage 2 protection at all.
>
> Unwinding and dumping hyp call traces is gated on CONFIG_NVHE_EL2_DEBUG
> to avoid the potential leaking of information to the host.
>
> A simple stack overflow test produces the following output:
>
> [ 580.376051][ T412] kvm: nVHE hyp panic at: ffffffc0116145c4!
> [ 580.378034][ T412] kvm [412]: nVHE HYP call trace:
> [ 580.378591][ T412] kvm [412]: [<ffffffc011614934>]
> [ 580.378993][ T412] kvm [412]: [<ffffffc01160fa48>]
> [ 580.379386][ T412] kvm [412]: [<ffffffc0116145dc>] // Non-terminating recursive call
> [ 580.379772][ T412] kvm [412]: [<ffffffc0116145dc>]
> [ 580.380158][ T412] kvm [412]: [<ffffffc0116145dc>]
> [ 580.380544][ T412] kvm [412]: [<ffffffc0116145dc>]
> [ 580.380928][ T412] kvm [412]: [<ffffffc0116145dc>]
> . . .
>
> Since nVHE hyp symbols are not included by kallsyms to avoid issues
> with aliasing, we fallback to the vmlinux addresses. Symbolizing the
> addresses is handled in the next patch in this series.
>
> Signed-off-by: Kalesh Singh <[email protected]>
> ---
>
> Changes in v3:
> - The nvhe hyp stack unwinder now makes use of the core logic from the
> regular kernel unwinder to avoid duplication, per Mark
>
> Changes in v2:
> - Add cpu_prepare_nvhe_panic_info()
> - Move updating the panic info to hyp_panic(), so that unwinding also
> works for conventional nVHE Hyp-mode.
>
> arch/arm64/include/asm/kvm_asm.h | 19 +++
> arch/arm64/include/asm/stacktrace.h | 12 ++
> arch/arm64/kernel/stacktrace.c | 210 +++++++++++++++++++++++++---
> arch/arm64/kvm/Kconfig | 5 +-
> arch/arm64/kvm/arm.c | 2 +-
> arch/arm64/kvm/handle_exit.c | 3 +
> arch/arm64/kvm/hyp/nvhe/switch.c | 18 +++
> 7 files changed, 243 insertions(+), 26 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index 2e277f2ed671..16efdf150a37 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -176,6 +176,25 @@ struct kvm_nvhe_init_params {
> unsigned long vtcr;
> };
>
> +#ifdef CONFIG_NVHE_EL2_DEBUG
> +/*
> + * Used by the host in EL1 to dump the nVHE hypervisor backtrace on
> + * hyp_panic. This is possible because CONFIG_NVHE_EL2_DEBUG disables
> + * the host stage 2 protection. See: __hyp_do_panic()
Same as my comment above.
> + *
> + * @hyp_stack_base: hyp VA of the hyp_stack base.
> + * @hyp_overflow_stack_base: hyp VA of the hyp_overflow_stack base.
> + * @fp: hyp FP where the backtrace begins.
> + * @pc: hyp PC where the backtrace begins.
> + */
> +struct kvm_nvhe_panic_info {
> + unsigned long hyp_stack_base;
> + unsigned long hyp_overflow_stack_base;
> + unsigned long fp;
> + unsigned long pc;
> +};
> +#endif /* CONFIG_NVHE_EL2_DEBUG */
> +
> /* Translate a kernel address @ptr into its equivalent linear mapping */
> #define kvm_ksym_ref(ptr) \
> ({ \
> diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
> index e77cdef9ca29..18611a51cf14 100644
> --- a/arch/arm64/include/asm/stacktrace.h
> +++ b/arch/arm64/include/asm/stacktrace.h
> @@ -22,6 +22,10 @@ enum stack_type {
> STACK_TYPE_OVERFLOW,
> STACK_TYPE_SDEI_NORMAL,
> STACK_TYPE_SDEI_CRITICAL,
> +#ifdef CONFIG_NVHE_EL2_DEBUG
> + STACK_TYPE_KVM_NVHE_HYP,
> + STACK_TYPE_KVM_NVHE_OVERFLOW,
> +#endif /* CONFIG_NVHE_EL2_DEBUG */
> __NR_STACK_TYPES
> };
>
> @@ -147,4 +151,12 @@ static inline bool on_accessible_stack(const struct task_struct *tsk,
> return false;
> }
>
> +#ifdef CONFIG_NVHE_EL2_DEBUG
> +void kvm_nvhe_dump_backtrace(unsigned long hyp_offset);
> +#else
> +static inline void kvm_nvhe_dump_backtrace(unsigned long hyp_offset)
> +{
> +}
> +#endif /* CONFIG_NVHE_EL2_DEBUG */
> +
> #endif /* __ASM_STACKTRACE_H */
> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> index e4103e085681..6ec85cb69b1f 100644
> --- a/arch/arm64/kernel/stacktrace.c
> +++ b/arch/arm64/kernel/stacktrace.c
> @@ -15,6 +15,8 @@
>
> #include <asm/irq.h>
> #include <asm/pointer_auth.h>
> +#include <asm/kvm_asm.h>
> +#include <asm/kvm_hyp.h>
> #include <asm/stack_pointer.h>
> #include <asm/stacktrace.h>
>
> @@ -64,26 +66,15 @@ NOKPROBE_SYMBOL(start_backtrace);
> * records (e.g. a cycle), determined based on the location and fp value of A
> * and the location (but not the fp value) of B.
> */
> -static int notrace unwind_frame(struct task_struct *tsk,
> - struct stackframe *frame)
> +static int notrace __unwind_frame(struct stackframe *frame, struct stack_info *info,
> + unsigned long (*translate_fp)(unsigned long, enum stack_type))
> {
> unsigned long fp = frame->fp;
> - struct stack_info info;
> -
> - if (!tsk)
> - tsk = current;
> -
> - /* Final frame; nothing to unwind */
> - if (fp == (unsigned long)task_pt_regs(tsk)->stackframe)
> - return -ENOENT;
>
> if (fp & 0x7)
> return -EINVAL;
>
> - if (!on_accessible_stack(tsk, fp, 16, &info))
> - return -EINVAL;
> -
> - if (test_bit(info.type, frame->stacks_done))
> + if (test_bit(info->type, frame->stacks_done))
> return -EINVAL;
>
> /*
> @@ -94,28 +85,62 @@ static int notrace unwind_frame(struct task_struct *tsk,
> *
> * TASK -> IRQ -> OVERFLOW -> SDEI_NORMAL
> * TASK -> SDEI_NORMAL -> SDEI_CRITICAL -> OVERFLOW
> + * KVM_NVHE_HYP -> KVM_NVHE_OVERFLOW
> *
> * ... but the nesting itself is strict. Once we transition from one
> * stack to another, it's never valid to unwind back to that first
> * stack.
> */
> - if (info.type == frame->prev_type) {
> + if (info->type == frame->prev_type) {
> if (fp <= frame->prev_fp)
> return -EINVAL;
> } else {
> set_bit(frame->prev_type, frame->stacks_done);
> }
>
> + /* Record fp as prev_fp before attempting to get the next fp */
> + frame->prev_fp = fp;
> +
> + /*
> + * If fp is not from the current address space perform the
> + * necessary translation before dereferencing it to get next fp.
> + */
> + if (translate_fp)
> + fp = translate_fp(fp, info->type);
> + if (!fp)
> + return -EINVAL;
> +
> /*
> * Record this frame record's values and location. The prev_fp and
> - * prev_type are only meaningful to the next unwind_frame() invocation.
> + * prev_type are only meaningful to the next __unwind_frame() invocation.
> */
> frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
> frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
> - frame->prev_fp = fp;
> - frame->prev_type = info.type;
> -
> frame->pc = ptrauth_strip_insn_pac(frame->pc);
> + frame->prev_type = info->type;
> +
> + return 0;
> +}
> +
> +static int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
> +{
> + unsigned long fp = frame->fp;
> + struct stack_info info;
> + int err;
> +
> + if (!tsk)
> + tsk = current;
> +
> + /* Final frame; nothing to unwind */
> + if (fp == (unsigned long)task_pt_regs(tsk)->stackframe)
> + return -ENOENT;
> +
> + if (!on_accessible_stack(tsk, fp, 16, &info))
> + return -EINVAL;
> +
> + err = __unwind_frame(frame, &info, NULL);
> + if (err)
> + return err;
>
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> if (tsk->ret_stack &&
> @@ -143,20 +168,27 @@ static int notrace unwind_frame(struct task_struct *tsk,
> }
> NOKPROBE_SYMBOL(unwind_frame);
>
> -static void notrace walk_stackframe(struct task_struct *tsk,
> - struct stackframe *frame,
> - bool (*fn)(void *, unsigned long), void *data)
> +static void notrace __walk_stackframe(struct task_struct *tsk, struct stackframe *frame,
> + bool (*fn)(void *, unsigned long), void *data,
> + int (*unwind_frame_fn)(struct task_struct *tsk, struct stackframe *frame))
> {
> while (1) {
> int ret;
>
> if (!fn(data, frame->pc))
> break;
> - ret = unwind_frame(tsk, frame);
> + ret = unwind_frame_fn(tsk, frame);
> if (ret < 0)
> break;
> }
> }
> +
> +static void notrace walk_stackframe(struct task_struct *tsk,
> + struct stackframe *frame,
> + bool (*fn)(void *, unsigned long), void *data)
> +{
> + __walk_stackframe(tsk, frame, fn, data, unwind_frame);
> +}
> NOKPROBE_SYMBOL(walk_stackframe);
>
> static bool dump_backtrace_entry(void *arg, unsigned long where)
> @@ -210,3 +242,135 @@ noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry,
>
> walk_stackframe(task, &frame, consume_entry, cookie);
> }
> +
> +#ifdef CONFIG_NVHE_EL2_DEBUG
> +DECLARE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
> +DECLARE_KVM_NVHE_PER_CPU(unsigned long [PAGE_SIZE/sizeof(long)], hyp_overflow_stack);
> +DECLARE_KVM_NVHE_PER_CPU(struct kvm_nvhe_panic_info, kvm_panic_info);
> +
> +static inline bool kvm_nvhe_on_overflow_stack(unsigned long sp, unsigned long size,
> + struct stack_info *info)
> +{
> + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
> + unsigned long low = (unsigned long)panic_info->hyp_overflow_stack_base;
> + unsigned long high = low + PAGE_SIZE;
> +
> + return on_stack(sp, size, low, high, STACK_TYPE_KVM_NVHE_OVERFLOW, info);
> +}
> +
> +static inline bool kvm_nvhe_on_hyp_stack(unsigned long sp, unsigned long size,
> + struct stack_info *info)
> +{
> + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
> + unsigned long low = (unsigned long)panic_info->hyp_stack_base;
> + unsigned long high = low + PAGE_SIZE;
> +
> + return on_stack(sp, size, low, high, STACK_TYPE_KVM_NVHE_HYP, info);
> +}
> +
> +static inline bool kvm_nvhe_on_accessible_stack(unsigned long sp, unsigned long size,
> + struct stack_info *info)
> +{
> + if (info)
> + info->type = STACK_TYPE_UNKNOWN;
> +
> + if (kvm_nvhe_on_hyp_stack(sp, size, info))
> + return true;
> + if (kvm_nvhe_on_overflow_stack(sp, size, info))
> + return true;
> +
> + return false;
> +}
> +
> +static unsigned long kvm_nvhe_hyp_stack_kern_va(unsigned long addr)
> +{
> + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
> + unsigned long hyp_base, kern_base, hyp_offset;
> +
> + hyp_base = (unsigned long)panic_info->hyp_stack_base;
> + hyp_offset = addr - hyp_base;
> +
> + kern_base = (unsigned long)*this_cpu_ptr(&kvm_arm_hyp_stack_page);
> +
> + return kern_base + hyp_offset;
> +}
> +
> +static unsigned long kvm_nvhe_overflow_stack_kern_va(unsigned long addr)
> +{
> + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
> + unsigned long hyp_base, kern_base, hyp_offset;
> +
> + hyp_base = (unsigned long)panic_info->hyp_overflow_stack_base;
> + hyp_offset = addr - hyp_base;
> +
> + kern_base = (unsigned long)this_cpu_ptr_nvhe_sym(hyp_overflow_stack);
> +
> + return kern_base + hyp_offset;
> +}
> +
> +/*
> + * Convert KVM nVHE hypervisor stack VA to a kernel VA.
> + *
> + * The nVHE hypervisor stack is mapped in the flexible 'private' VA range, to allow
> + * for guard pages below the stack. Consequently, the fixed offset address
> + * translation macros won't work here.
> + *
> + * The kernel VA is calculated as an offset from the kernel VA of the hypervisor
> + * stack base. See: kvm_nvhe_hyp_stack_kern_va(), kvm_nvhe_overflow_stack_kern_va()
> + */
> +static unsigned long kvm_nvhe_stack_kern_va(unsigned long addr,
> + enum stack_type type)
> +{
> + switch (type) {
> + case STACK_TYPE_KVM_NVHE_HYP:
> + return kvm_nvhe_hyp_stack_kern_va(addr);
> + case STACK_TYPE_KVM_NVHE_OVERFLOW:
> + return kvm_nvhe_overflow_stack_kern_va(addr);
> + default:
> + return 0UL;
> + }
> +}
> +
> +static int notrace kvm_nvhe_unwind_frame(struct task_struct *tsk,
> + struct stackframe *frame)
> +{
> + struct stack_info info;
> +
> + if (!kvm_nvhe_on_accessible_stack(frame->fp, 16, &info))
> + return -EINVAL;
> +
> + return __unwind_frame(frame, &info, kvm_nvhe_stack_kern_va);
> +}
> +
> +static bool kvm_nvhe_dump_backtrace_entry(void *arg, unsigned long where)
> +{
> + unsigned long va_mask = GENMASK_ULL(vabits_actual - 1, 0);
> + unsigned long hyp_offset = (unsigned long)arg;
> +
> + where &= va_mask; /* Mask tags */
> + where += hyp_offset; /* Convert to kern addr */
> +
> + kvm_err("[<%016lx>] %pB\n", where, (void *)where);
> +
> + return true;
> +}
> +
> +static void notrace kvm_nvhe_walk_stackframe(struct task_struct *tsk,
> + struct stackframe *frame,
> + bool (*fn)(void *, unsigned long), void *data)
> +{
> + __walk_stackframe(tsk, frame, fn, data, kvm_nvhe_unwind_frame);
> +}
> +
> +void kvm_nvhe_dump_backtrace(unsigned long hyp_offset)
> +{
> + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
> + struct stackframe frame;
> +
> + start_backtrace(&frame, panic_info->fp, panic_info->pc);
> + pr_err("nVHE HYP call trace:\n");
> + kvm_nvhe_walk_stackframe(NULL, &frame, kvm_nvhe_dump_backtrace_entry,
> + (void *)hyp_offset);
> + pr_err("---- end of nVHE HYP call trace ----\n");
> +}
> +#endif /* CONFIG_NVHE_EL2_DEBUG */
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index 8a5fbbf084df..75f2c8255ff0 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -51,8 +51,9 @@ config NVHE_EL2_DEBUG
> depends on KVM
> help
> Say Y here to enable the debug mode for the non-VHE KVM EL2 object.
> - Failure reports will BUG() in the hypervisor. This is intended for
> - local EL2 hypervisor development.
> + Failure reports will BUG() in the hypervisor; and panics will print
> + the hypervisor call stack. This is intended for local EL2 hypervisor
> + development.
Nit: maybe for clarity you could rephrase as "calls to hyp_panic()
will result in printing the hypervisor call stack".
Thanks,
/fuad
>
> If unsure, say N.
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 7a23630c4a7f..66c07c04eb52 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -49,7 +49,7 @@ DEFINE_STATIC_KEY_FALSE(kvm_protected_mode_initialized);
>
> DECLARE_KVM_HYP_PER_CPU(unsigned long, kvm_hyp_vector);
>
> -static DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
> +DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
> unsigned long kvm_arm_hyp_percpu_base[NR_CPUS];
> DECLARE_KVM_NVHE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
>
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index e3140abd2e2e..ff69dff33700 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -17,6 +17,7 @@
> #include <asm/kvm_emulate.h>
> #include <asm/kvm_mmu.h>
> #include <asm/debug-monitors.h>
> +#include <asm/stacktrace.h>
> #include <asm/traps.h>
>
> #include <kvm/arm_hypercalls.h>
> @@ -326,6 +327,8 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr,
> kvm_err("nVHE hyp panic at: %016llx!\n", elr_virt + hyp_offset);
> }
>
> + kvm_nvhe_dump_backtrace(hyp_offset);
> +
> /*
> * Hyp has panicked and we're going to handle that by panicking the
> * kernel. The kernel offset will be revealed in the panic so we're
> diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> index efc20273a352..b8ecffc47424 100644
> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> @@ -37,6 +37,22 @@ DEFINE_PER_CPU(unsigned long, kvm_hyp_vector);
> #ifdef CONFIG_NVHE_EL2_DEBUG
> DEFINE_PER_CPU(unsigned long [PAGE_SIZE/sizeof(long)], hyp_overflow_stack)
> __aligned(16);
> +DEFINE_PER_CPU(struct kvm_nvhe_panic_info, kvm_panic_info);
> +
> +static inline void cpu_prepare_nvhe_panic_info(void)
> +{
> + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr(&kvm_panic_info);
> + struct kvm_nvhe_init_params *params = this_cpu_ptr(&kvm_init_params);
> +
> + panic_info->hyp_stack_base = (unsigned long)(params->stack_hyp_va - PAGE_SIZE);
> + panic_info->hyp_overflow_stack_base = (unsigned long)this_cpu_ptr(hyp_overflow_stack);
> + panic_info->fp = (unsigned long)__builtin_frame_address(0);
> + panic_info->pc = _THIS_IP_;
> +}
> + #else
> +static inline void cpu_prepare_nvhe_panic_info(void)
> +{
> +}
> #endif
>
> static void __activate_traps(struct kvm_vcpu *vcpu)
> @@ -360,6 +376,8 @@ asmlinkage void __noreturn hyp_panic(void)
> struct kvm_cpu_context *host_ctxt;
> struct kvm_vcpu *vcpu;
>
> + cpu_prepare_nvhe_panic_info();
> +
> host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
> vcpu = host_ctxt->__hyp_running_vcpu;
>
> --
> 2.35.1.473.g83b2b277ed-goog
>
On Thu, Feb 24, 2022 at 4:28 AM Fuad Tabba <[email protected]> wrote:
>
> Hi Kalesh,
>
> On Thu, Feb 24, 2022 at 5:22 AM Kalesh Singh <[email protected]> wrote:
> >
> > Unwind the stack in EL1, when CONFIG_NVHE_EL2_DEBUG is enabled. This is
> > possible because CONFIG_NVHE_EL2_DEBUG disables the host stage 2 protection
> > which allows host to access the hypervisor stack pages in EL1.
>
> For this comment to be clearer, and if my understanding is correct, I
> think that it should say that CONFIG_NVHE_EL2_DEBUG allows host stage
> 2 protection to be disabled on a hyp_panic. Otherwise, on reading the
> comment one might think that CONFIG_NVHE_EL2_DEBUG runs without host
> stage 2 protection at all.
Your understanding is correct: the host stage 2 protection is only
disabled on a hyp_panic(). I'll rephrase to make it clearer.
>
> >
> > Unwinding and dumping hyp call traces is gated on CONFIG_NVHE_EL2_DEBUG
> > to avoid the potential leaking of information to the host.
> >
> > A simple stack overflow test produces the following output:
> >
> > [ 580.376051][ T412] kvm: nVHE hyp panic at: ffffffc0116145c4!
> > [ 580.378034][ T412] kvm [412]: nVHE HYP call trace:
> > [ 580.378591][ T412] kvm [412]: [<ffffffc011614934>]
> > [ 580.378993][ T412] kvm [412]: [<ffffffc01160fa48>]
> > [ 580.379386][ T412] kvm [412]: [<ffffffc0116145dc>] // Non-terminating recursive call
> > [ 580.379772][ T412] kvm [412]: [<ffffffc0116145dc>]
> > [ 580.380158][ T412] kvm [412]: [<ffffffc0116145dc>]
> > [ 580.380544][ T412] kvm [412]: [<ffffffc0116145dc>]
> > [ 580.380928][ T412] kvm [412]: [<ffffffc0116145dc>]
> > . . .
> >
> > Since nVHE hyp symbols are not included by kallsyms to avoid issues
> > with aliasing, we fallback to the vmlinux addresses. Symbolizing the
> > addresses is handled in the next patch in this series.
> >
> > Signed-off-by: Kalesh Singh <[email protected]>
> > ---
> >
> > Changes in v3:
> > - The nvhe hyp stack unwinder now makes use of the core logic from the
> > regular kernel unwinder to avoid duplication, per Mark
> >
> > Changes in v2:
> > - Add cpu_prepare_nvhe_panic_info()
> > - Move updating the panic info to hyp_panic(), so that unwinding also
> > works for conventional nVHE Hyp-mode.
> >
> > arch/arm64/include/asm/kvm_asm.h | 19 +++
> > arch/arm64/include/asm/stacktrace.h | 12 ++
> > arch/arm64/kernel/stacktrace.c | 210 +++++++++++++++++++++++++---
> > arch/arm64/kvm/Kconfig | 5 +-
> > arch/arm64/kvm/arm.c | 2 +-
> > arch/arm64/kvm/handle_exit.c | 3 +
> > arch/arm64/kvm/hyp/nvhe/switch.c | 18 +++
> > 7 files changed, 243 insertions(+), 26 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> > index 2e277f2ed671..16efdf150a37 100644
> > --- a/arch/arm64/include/asm/kvm_asm.h
> > +++ b/arch/arm64/include/asm/kvm_asm.h
> > @@ -176,6 +176,25 @@ struct kvm_nvhe_init_params {
> > unsigned long vtcr;
> > };
> >
> > +#ifdef CONFIG_NVHE_EL2_DEBUG
> > +/*
> > + * Used by the host in EL1 to dump the nVHE hypervisor backtrace on
> > + * hyp_panic. This is possible because CONFIG_NVHE_EL2_DEBUG disables
> > + * the host stage 2 protection. See: __hyp_do_panic()
>
> Same as my comment above.
Ack
>
> > + *
> > + * @hyp_stack_base: hyp VA of the hyp_stack base.
> > + * @hyp_overflow_stack_base: hyp VA of the hyp_overflow_stack base.
> > + * @fp: hyp FP where the backtrace begins.
> > + * @pc: hyp PC where the backtrace begins.
> > + */
> > +struct kvm_nvhe_panic_info {
> > + unsigned long hyp_stack_base;
> > + unsigned long hyp_overflow_stack_base;
> > + unsigned long fp;
> > + unsigned long pc;
> > +};
> > +#endif /* CONFIG_NVHE_EL2_DEBUG */
> > +
> > /* Translate a kernel address @ptr into its equivalent linear mapping */
> > #define kvm_ksym_ref(ptr) \
> > ({ \
> > diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
> > index e77cdef9ca29..18611a51cf14 100644
> > --- a/arch/arm64/include/asm/stacktrace.h
> > +++ b/arch/arm64/include/asm/stacktrace.h
> > @@ -22,6 +22,10 @@ enum stack_type {
> > STACK_TYPE_OVERFLOW,
> > STACK_TYPE_SDEI_NORMAL,
> > STACK_TYPE_SDEI_CRITICAL,
> > +#ifdef CONFIG_NVHE_EL2_DEBUG
> > + STACK_TYPE_KVM_NVHE_HYP,
> > + STACK_TYPE_KVM_NVHE_OVERFLOW,
> > +#endif /* CONFIG_NVHE_EL2_DEBUG */
> > __NR_STACK_TYPES
> > };
> >
> > @@ -147,4 +151,12 @@ static inline bool on_accessible_stack(const struct task_struct *tsk,
> > return false;
> > }
> >
> > +#ifdef CONFIG_NVHE_EL2_DEBUG
> > +void kvm_nvhe_dump_backtrace(unsigned long hyp_offset);
> > +#else
> > +static inline void kvm_nvhe_dump_backtrace(unsigned long hyp_offset)
> > +{
> > +}
> > +#endif /* CONFIG_NVHE_EL2_DEBUG */
> > +
> > #endif /* __ASM_STACKTRACE_H */
> > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> > index e4103e085681..6ec85cb69b1f 100644
> > --- a/arch/arm64/kernel/stacktrace.c
> > +++ b/arch/arm64/kernel/stacktrace.c
> > @@ -15,6 +15,8 @@
> >
> > #include <asm/irq.h>
> > #include <asm/pointer_auth.h>
> > +#include <asm/kvm_asm.h>
> > +#include <asm/kvm_hyp.h>
> > #include <asm/stack_pointer.h>
> > #include <asm/stacktrace.h>
> >
> > @@ -64,26 +66,15 @@ NOKPROBE_SYMBOL(start_backtrace);
> > * records (e.g. a cycle), determined based on the location and fp value of A
> > * and the location (but not the fp value) of B.
> > */
> > -static int notrace unwind_frame(struct task_struct *tsk,
> > - struct stackframe *frame)
> > +static int notrace __unwind_frame(struct stackframe *frame, struct stack_info *info,
> > + unsigned long (*translate_fp)(unsigned long, enum stack_type))
> > {
> > unsigned long fp = frame->fp;
> > - struct stack_info info;
> > -
> > - if (!tsk)
> > - tsk = current;
> > -
> > - /* Final frame; nothing to unwind */
> > - if (fp == (unsigned long)task_pt_regs(tsk)->stackframe)
> > - return -ENOENT;
> >
> > if (fp & 0x7)
> > return -EINVAL;
> >
> > - if (!on_accessible_stack(tsk, fp, 16, &info))
> > - return -EINVAL;
> > -
> > - if (test_bit(info.type, frame->stacks_done))
> > + if (test_bit(info->type, frame->stacks_done))
> > return -EINVAL;
> >
> > /*
> > @@ -94,28 +85,62 @@ static int notrace unwind_frame(struct task_struct *tsk,
> > *
> > * TASK -> IRQ -> OVERFLOW -> SDEI_NORMAL
> > * TASK -> SDEI_NORMAL -> SDEI_CRITICAL -> OVERFLOW
> > + * KVM_NVHE_HYP -> KVM_NVHE_OVERFLOW
> > *
> > * ... but the nesting itself is strict. Once we transition from one
> > * stack to another, it's never valid to unwind back to that first
> > * stack.
> > */
> > - if (info.type == frame->prev_type) {
> > + if (info->type == frame->prev_type) {
> > if (fp <= frame->prev_fp)
> > return -EINVAL;
> > } else {
> > set_bit(frame->prev_type, frame->stacks_done);
> > }
> >
> > + /* Record fp as prev_fp before attempting to get the next fp */
> > + frame->prev_fp = fp;
> > +
> > + /*
> > + * If fp is not from the current address space perform the
> > + * necessary translation before dereferencing it to get next fp.
> > + */
> > + if (translate_fp)
> > + fp = translate_fp(fp, info->type);
> > + if (!fp)
> > + return -EINVAL;
> > +
> > /*
> > * Record this frame record's values and location. The prev_fp and
> > - * prev_type are only meaningful to the next unwind_frame() invocation.
> > + * prev_type are only meaningful to the next __unwind_frame() invocation.
> > */
> > frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
> > frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));
> > - frame->prev_fp = fp;
> > - frame->prev_type = info.type;
> > -
> > frame->pc = ptrauth_strip_insn_pac(frame->pc);
> > + frame->prev_type = info->type;
> > +
> > + return 0;
> > +}
> > +
> > +static int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
> > +{
> > + unsigned long fp = frame->fp;
> > + struct stack_info info;
> > + int err;
> > +
> > + if (!tsk)
> > + tsk = current;
> > +
> > + /* Final frame; nothing to unwind */
> > + if (fp == (unsigned long)task_pt_regs(tsk)->stackframe)
> > + return -ENOENT;
> > +
> > + if (!on_accessible_stack(tsk, fp, 16, &info))
> > + return -EINVAL;
> > +
> > + err = __unwind_frame(frame, &info, NULL);
> > + if (err)
> > + return err;
> >
> > #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > if (tsk->ret_stack &&
> > @@ -143,20 +168,27 @@ static int notrace unwind_frame(struct task_struct *tsk,
> > }
> > NOKPROBE_SYMBOL(unwind_frame);
> >
> > -static void notrace walk_stackframe(struct task_struct *tsk,
> > - struct stackframe *frame,
> > - bool (*fn)(void *, unsigned long), void *data)
> > +static void notrace __walk_stackframe(struct task_struct *tsk, struct stackframe *frame,
> > + bool (*fn)(void *, unsigned long), void *data,
> > + int (*unwind_frame_fn)(struct task_struct *tsk, struct stackframe *frame))
> > {
> > while (1) {
> > int ret;
> >
> > if (!fn(data, frame->pc))
> > break;
> > - ret = unwind_frame(tsk, frame);
> > + ret = unwind_frame_fn(tsk, frame);
> > if (ret < 0)
> > break;
> > }
> > }
> > +
> > +static void notrace walk_stackframe(struct task_struct *tsk,
> > + struct stackframe *frame,
> > + bool (*fn)(void *, unsigned long), void *data)
> > +{
> > + __walk_stackframe(tsk, frame, fn, data, unwind_frame);
> > +}
> > NOKPROBE_SYMBOL(walk_stackframe);
> >
> > static bool dump_backtrace_entry(void *arg, unsigned long where)
> > @@ -210,3 +242,135 @@ noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry,
> >
> > walk_stackframe(task, &frame, consume_entry, cookie);
> > }
> > +
> > +#ifdef CONFIG_NVHE_EL2_DEBUG
> > +DECLARE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
> > +DECLARE_KVM_NVHE_PER_CPU(unsigned long [PAGE_SIZE/sizeof(long)], hyp_overflow_stack);
> > +DECLARE_KVM_NVHE_PER_CPU(struct kvm_nvhe_panic_info, kvm_panic_info);
> > +
> > +static inline bool kvm_nvhe_on_overflow_stack(unsigned long sp, unsigned long size,
> > + struct stack_info *info)
> > +{
> > + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
> > + unsigned long low = (unsigned long)panic_info->hyp_overflow_stack_base;
> > + unsigned long high = low + PAGE_SIZE;
> > +
> > + return on_stack(sp, size, low, high, STACK_TYPE_KVM_NVHE_OVERFLOW, info);
> > +}
> > +
> > +static inline bool kvm_nvhe_on_hyp_stack(unsigned long sp, unsigned long size,
> > + struct stack_info *info)
> > +{
> > + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
> > + unsigned long low = (unsigned long)panic_info->hyp_stack_base;
> > + unsigned long high = low + PAGE_SIZE;
> > +
> > + return on_stack(sp, size, low, high, STACK_TYPE_KVM_NVHE_HYP, info);
> > +}
> > +
> > +static inline bool kvm_nvhe_on_accessible_stack(unsigned long sp, unsigned long size,
> > + struct stack_info *info)
> > +{
> > + if (info)
> > + info->type = STACK_TYPE_UNKNOWN;
> > +
> > + if (kvm_nvhe_on_hyp_stack(sp, size, info))
> > + return true;
> > + if (kvm_nvhe_on_overflow_stack(sp, size, info))
> > + return true;
> > +
> > + return false;
> > +}
> > +
> > +static unsigned long kvm_nvhe_hyp_stack_kern_va(unsigned long addr)
> > +{
> > + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
> > + unsigned long hyp_base, kern_base, hyp_offset;
> > +
> > + hyp_base = (unsigned long)panic_info->hyp_stack_base;
> > + hyp_offset = addr - hyp_base;
> > +
> > + kern_base = (unsigned long)*this_cpu_ptr(&kvm_arm_hyp_stack_page);
> > +
> > + return kern_base + hyp_offset;
> > +}
> > +
> > +static unsigned long kvm_nvhe_overflow_stack_kern_va(unsigned long addr)
> > +{
> > + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
> > + unsigned long hyp_base, kern_base, hyp_offset;
> > +
> > + hyp_base = (unsigned long)panic_info->hyp_overflow_stack_base;
> > + hyp_offset = addr - hyp_base;
> > +
> > + kern_base = (unsigned long)this_cpu_ptr_nvhe_sym(hyp_overflow_stack);
> > +
> > + return kern_base + hyp_offset;
> > +}
> > +
> > +/*
> > + * Convert KVM nVHE hypervisor stack VA to a kernel VA.
> > + *
> > + * The nVHE hypervisor stack is mapped in the flexible 'private' VA range, to allow
> > + * for guard pages below the stack. Consequently, the fixed offset address
> > + * translation macros won't work here.
> > + *
> > + * The kernel VA is calculated as an offset from the kernel VA of the hypervisor
> > + * stack base. See: kvm_nvhe_hyp_stack_kern_va(), kvm_nvhe_overflow_stack_kern_va()
> > + */
> > +static unsigned long kvm_nvhe_stack_kern_va(unsigned long addr,
> > + enum stack_type type)
> > +{
> > + switch (type) {
> > + case STACK_TYPE_KVM_NVHE_HYP:
> > + return kvm_nvhe_hyp_stack_kern_va(addr);
> > + case STACK_TYPE_KVM_NVHE_OVERFLOW:
> > + return kvm_nvhe_overflow_stack_kern_va(addr);
> > + default:
> > + return 0UL;
> > + }
> > +}
> > +
> > +static int notrace kvm_nvhe_unwind_frame(struct task_struct *tsk,
> > + struct stackframe *frame)
> > +{
> > + struct stack_info info;
> > +
> > + if (!kvm_nvhe_on_accessible_stack(frame->fp, 16, &info))
> > + return -EINVAL;
> > +
> > + return __unwind_frame(frame, &info, kvm_nvhe_stack_kern_va);
> > +}
> > +
> > +static bool kvm_nvhe_dump_backtrace_entry(void *arg, unsigned long where)
> > +{
> > + unsigned long va_mask = GENMASK_ULL(vabits_actual - 1, 0);
> > + unsigned long hyp_offset = (unsigned long)arg;
> > +
> > + where &= va_mask; /* Mask tags */
> > + where += hyp_offset; /* Convert to kern addr */
> > +
> > + kvm_err("[<%016lx>] %pB\n", where, (void *)where);
> > +
> > + return true;
> > +}
> > +
> > +static void notrace kvm_nvhe_walk_stackframe(struct task_struct *tsk,
> > + struct stackframe *frame,
> > + bool (*fn)(void *, unsigned long), void *data)
> > +{
> > + __walk_stackframe(tsk, frame, fn, data, kvm_nvhe_unwind_frame);
> > +}
> > +
> > +void kvm_nvhe_dump_backtrace(unsigned long hyp_offset)
> > +{
> > + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr_nvhe_sym(kvm_panic_info);
> > + struct stackframe frame;
> > +
> > + start_backtrace(&frame, panic_info->fp, panic_info->pc);
> > + pr_err("nVHE HYP call trace:\n");
> > + kvm_nvhe_walk_stackframe(NULL, &frame, kvm_nvhe_dump_backtrace_entry,
> > + (void *)hyp_offset);
> > + pr_err("---- end of nVHE HYP call trace ----\n");
> > +}
> > +#endif /* CONFIG_NVHE_EL2_DEBUG */
> > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> > index 8a5fbbf084df..75f2c8255ff0 100644
> > --- a/arch/arm64/kvm/Kconfig
> > +++ b/arch/arm64/kvm/Kconfig
> > @@ -51,8 +51,9 @@ config NVHE_EL2_DEBUG
> > depends on KVM
> > help
> > Say Y here to enable the debug mode for the non-VHE KVM EL2 object.
> > - Failure reports will BUG() in the hypervisor. This is intended for
> > - local EL2 hypervisor development.
> > + Failure reports will BUG() in the hypervisor; and panics will print
> > + the hypervisor call stack. This is intended for local EL2 hypervisor
> > + development.
>
> Nit: maybe for clarity you could rephrase as "calls to hyp_panic()
> will result in printing the hypervisor call stack".
Ack. I'll update in the next version.
Thanks,
Kalesh
>
> Thanks,
> /fuad
>
>
> >
> > If unsure, say N.
> >
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 7a23630c4a7f..66c07c04eb52 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -49,7 +49,7 @@ DEFINE_STATIC_KEY_FALSE(kvm_protected_mode_initialized);
> >
> > DECLARE_KVM_HYP_PER_CPU(unsigned long, kvm_hyp_vector);
> >
> > -static DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
> > +DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
> > unsigned long kvm_arm_hyp_percpu_base[NR_CPUS];
> > DECLARE_KVM_NVHE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
> >
> > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> > index e3140abd2e2e..ff69dff33700 100644
> > --- a/arch/arm64/kvm/handle_exit.c
> > +++ b/arch/arm64/kvm/handle_exit.c
> > @@ -17,6 +17,7 @@
> > #include <asm/kvm_emulate.h>
> > #include <asm/kvm_mmu.h>
> > #include <asm/debug-monitors.h>
> > +#include <asm/stacktrace.h>
> > #include <asm/traps.h>
> >
> > #include <kvm/arm_hypercalls.h>
> > @@ -326,6 +327,8 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr,
> > kvm_err("nVHE hyp panic at: %016llx!\n", elr_virt + hyp_offset);
> > }
> >
> > + kvm_nvhe_dump_backtrace(hyp_offset);
> > +
> > /*
> > * Hyp has panicked and we're going to handle that by panicking the
> > * kernel. The kernel offset will be revealed in the panic so we're
> > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> > index efc20273a352..b8ecffc47424 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> > @@ -37,6 +37,22 @@ DEFINE_PER_CPU(unsigned long, kvm_hyp_vector);
> > #ifdef CONFIG_NVHE_EL2_DEBUG
> > DEFINE_PER_CPU(unsigned long [PAGE_SIZE/sizeof(long)], hyp_overflow_stack)
> > __aligned(16);
> > +DEFINE_PER_CPU(struct kvm_nvhe_panic_info, kvm_panic_info);
> > +
> > +static inline void cpu_prepare_nvhe_panic_info(void)
> > +{
> > + struct kvm_nvhe_panic_info *panic_info = this_cpu_ptr(&kvm_panic_info);
> > + struct kvm_nvhe_init_params *params = this_cpu_ptr(&kvm_init_params);
> > +
> > + panic_info->hyp_stack_base = (unsigned long)(params->stack_hyp_va - PAGE_SIZE);
> > + panic_info->hyp_overflow_stack_base = (unsigned long)this_cpu_ptr(hyp_overflow_stack);
> > + panic_info->fp = (unsigned long)__builtin_frame_address(0);
> > + panic_info->pc = _THIS_IP_;
> > +}
> > + #else
> > +static inline void cpu_prepare_nvhe_panic_info(void)
> > +{
> > +}
> > #endif
> >
> > static void __activate_traps(struct kvm_vcpu *vcpu)
> > @@ -360,6 +376,8 @@ asmlinkage void __noreturn hyp_panic(void)
> > struct kvm_cpu_context *host_ctxt;
> > struct kvm_vcpu *vcpu;
> >
> > + cpu_prepare_nvhe_panic_info();
> > +
> > host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
> > vcpu = host_ctxt->__hyp_running_vcpu;
> >
> > --
> > 2.35.1.473.g83b2b277ed-goog
> >