2021-02-23 20:13:28

by Andrew Scull

[permalink] [raw]
Subject: [PATCH v2 0/4] Debug info for nVHE hyp panics

Panics from arm64's nVHE hyp mode are hard to interpret. This series
adds some more debug info to help with diagnosis.

Using BUG() in nVHE hyp gives a meaningful address to locate invariants
that fail to hold. The host can also look up the bug to provide the file
and line, if the debug configs are enabled. Otherwise a kimg address is
much more useful than a hyp VA since it can be looked up in vmlinux.

The lib/bug.c changes apply on 5.11.

This arm64 KVM changes apply on top of the previous panic fix at
https://lore.kernel.org/r/[email protected]/

From v1 (https://lore.kernel.org/r/[email protected]/):
- keeping struct bug details in bug.c
- using SPSR to distinguish hyp from host
- inlined __hyp_panic_string

Andrew Scull (4):
bug: Remove redundant condition check in report_bug
bug: Factor out a getter for a bug's file line
KVM: arm64: Use BUG and BUG_ON in nVHE hyp
KVM: arm64: Log source when panicking from nVHE hyp

arch/arm64/include/asm/kvm_hyp.h | 1 -
arch/arm64/include/asm/kvm_mmu.h | 2 +
arch/arm64/kernel/image-vars.h | 3 +-
arch/arm64/kvm/handle_exit.c | 31 ++++++++++++++
arch/arm64/kvm/hyp/include/hyp/switch.h | 2 -
arch/arm64/kvm/hyp/nvhe/host.S | 17 ++++----
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-smp.c | 6 +--
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 2 -
arch/arm64/kvm/hyp/vhe/switch.c | 4 +-
include/linux/bug.h | 3 ++
lib/bug.c | 54 +++++++++++++------------
12 files changed, 77 insertions(+), 50 deletions(-)

--
2.30.0.617.g56c4b15f3c-goog


2021-02-23 20:13:29

by Andrew Scull

[permalink] [raw]
Subject: [PATCH v2 1/4] bug: Remove redundant condition check in report_bug

report_bug() will return early if it cannot find a bug corresponding to
the provided address. The subsequent test for the bug will always be
true so remove it.

Signed-off-by: Andrew Scull <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: "Steven Rostedt (VMware)" <[email protected]>
---
lib/bug.c | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/lib/bug.c b/lib/bug.c
index 7103440c0ee1..4ab398a2de93 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -158,30 +158,27 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)

file = NULL;
line = 0;
- warning = 0;

- if (bug) {
#ifdef CONFIG_DEBUG_BUGVERBOSE
#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
- file = bug->file;
+ file = bug->file;
#else
- file = (const char *)bug + bug->file_disp;
+ file = (const char *)bug + bug->file_disp;
#endif
- line = bug->line;
+ line = bug->line;
#endif
- warning = (bug->flags & BUGFLAG_WARNING) != 0;
- once = (bug->flags & BUGFLAG_ONCE) != 0;
- done = (bug->flags & BUGFLAG_DONE) != 0;
-
- if (warning && once) {
- if (done)
- return BUG_TRAP_TYPE_WARN;
-
- /*
- * Since this is the only store, concurrency is not an issue.
- */
- bug->flags |= BUGFLAG_DONE;
- }
+ warning = (bug->flags & BUGFLAG_WARNING) != 0;
+ once = (bug->flags & BUGFLAG_ONCE) != 0;
+ done = (bug->flags & BUGFLAG_DONE) != 0;
+
+ if (warning && once) {
+ if (done)
+ return BUG_TRAP_TYPE_WARN;
+
+ /*
+ * Since this is the only store, concurrency is not an issue.
+ */
+ bug->flags |= BUGFLAG_DONE;
}

/*
--
2.30.0.617.g56c4b15f3c-goog

2021-02-23 20:15:49

by Andrew Scull

[permalink] [raw]
Subject: [PATCH v2 4/4] KVM: arm64: Log source when panicking from nVHE hyp

To aid with debugging, add details of the source of a panic. This is
done by having nVHE hyp exit to nvhe_hyp_panic_handler() rather than
directly to panic(). The handler will then add the extra details for
debugging before panicking the kernel.

If the panic was due to a BUG(), look up the metadata to log the file
and line, if available, otherwise log the kimg address that can be
looked up in vmlinux.

__hyp_panic_string is now inlined since it no longer needs to be
references as a symbol and message is free to diverge between VHE and
nVHE.

Signed-off-by: Andrew Scull <[email protected]>
---
arch/arm64/include/asm/kvm_mmu.h | 2 ++
arch/arm64/kernel/image-vars.h | 3 +--
arch/arm64/kvm/handle_exit.c | 31 +++++++++++++++++++++++++
arch/arm64/kvm/hyp/include/hyp/switch.h | 2 --
arch/arm64/kvm/hyp/nvhe/host.S | 17 ++++++--------
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 2 --
arch/arm64/kvm/hyp/vhe/switch.c | 4 +---
7 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index e52d82aeadca..f07c55f9dd1e 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -130,6 +130,8 @@ void kvm_update_va_mask(struct alt_instr *alt,
__le32 *origptr, __le32 *updptr, int nr_inst);
void kvm_compute_layout(void);

+#define __hyp_pa(x) ((phys_addr_t)((x)) + hyp_physvirt_offset)
+
static __always_inline unsigned long __kern_hyp_va(unsigned long v)
{
asm volatile(ALTERNATIVE_CB("and %0, %0, #1\n"
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index f676243abac6..cf12b0d6441e 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -71,8 +71,7 @@ KVM_NVHE_ALIAS(kvm_get_kimage_voffset);
KVM_NVHE_ALIAS(kvm_vgic_global_state);

/* Kernel symbols used to call panic() from nVHE hyp code (via ERET). */
-KVM_NVHE_ALIAS(__hyp_panic_string);
-KVM_NVHE_ALIAS(panic);
+KVM_NVHE_ALIAS(nvhe_hyp_panic_handler);

/* Vectors installed by hyp-init on reset HVC. */
KVM_NVHE_ALIAS(__hyp_stub_vectors);
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index cebe39f3b1b6..b25b88d8c150 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -291,3 +291,34 @@ void handle_exit_early(struct kvm_vcpu *vcpu, int exception_index)
if (exception_index == ARM_EXCEPTION_EL1_SERROR)
kvm_handle_guest_serror(vcpu, kvm_vcpu_get_esr(vcpu));
}
+
+void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr, u64 elr,
+ u64 par, uintptr_t vcpu,
+ u64 far, u64 hpfar) {
+ u64 elr_in_kimg = __phys_to_kimg(__hyp_pa(elr));
+ u64 mode = spsr & PSR_MODE_MASK;
+
+ if (mode != PSR_MODE_EL2t && mode != PSR_MODE_EL2h) {
+ kvm_err("Invalid host exception to nVHE hyp!\n");
+ } else if (ESR_ELx_EC(esr) == ESR_ELx_EC_BRK64 &&
+ (esr & ESR_ELx_BRK64_ISS_COMMENT_MASK) == BUG_BRK_IMM) {
+ struct bug_entry *bug = find_bug(elr_in_kimg);
+ const char *file = NULL;
+ unsigned int line = 0;
+
+ /* All hyp bugs, including warnings, are treated as fatal. */
+ if (bug)
+ bug_get_file_line(bug, &file, &line);
+
+ if (file) {
+ kvm_err("nVHE hyp BUG at: %s:%u!\n", file, line);
+ } else {
+ kvm_err("nVHE hyp BUG at: %016llx!\n", elr_in_kimg);
+ }
+ } else {
+ kvm_err("nVHE hyp panic at: %016llx!\n", elr_in_kimg);
+ }
+
+ panic("HYP panic:\nPS:%08llx PC:%016llx ESR:%08llx\nFAR:%016llx HPFAR:%016llx PAR:%016llx\nVCPU:%016lx\n",
+ spsr, elr, esr, far, hpfar, par, vcpu);
+}
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 84473574c2e7..f9e8bb97d199 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -30,8 +30,6 @@
#include <asm/processor.h>
#include <asm/thread_info.h>

-extern const char __hyp_panic_string[];
-
extern struct exception_table_entry __start___kvm_ex_table;
extern struct exception_table_entry __stop___kvm_ex_table;

diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S
index 3dc5a9f3e575..04d661614b0f 100644
--- a/arch/arm64/kvm/hyp/nvhe/host.S
+++ b/arch/arm64/kvm/hyp/nvhe/host.S
@@ -77,21 +77,18 @@ SYM_FUNC_END(__host_enter)
SYM_FUNC_START(__hyp_do_panic)
mov x29, x0

- /* Load the format string into x0 and arguments into x1-7 */
- ldr x0, =__hyp_panic_string
-
- mov x6, x3
- get_vcpu_ptr x7, x3
-
- mrs x3, esr_el2
- mrs x4, far_el2
- mrs x5, hpfar_el2
+ /* Load the panic arguments into x0-7 */
+ mrs x0, esr_el2
+ get_vcpu_ptr x4, x5
+ mrs x5, far_el2
+ mrs x6, hpfar_el2
+ mov x7, x0 // Unused argument

/* Prepare and exit to the host's panic funciton. */
mov lr, #(PSR_F_BIT | PSR_I_BIT | PSR_A_BIT | PSR_D_BIT |\
PSR_MODE_EL1h)
msr spsr_el2, lr
- ldr lr, =panic
+ ldr lr, =nvhe_hyp_panic_handler
msr elr_el2, lr

/* Enter the host, restoring the host context if it was provided. */
diff --git a/arch/arm64/kvm/hyp/nvhe/psci-relay.c b/arch/arm64/kvm/hyp/nvhe/psci-relay.c
index 8e7128cb7667..54b70189229b 100644
--- a/arch/arm64/kvm/hyp/nvhe/psci-relay.c
+++ b/arch/arm64/kvm/hyp/nvhe/psci-relay.c
@@ -22,8 +22,6 @@ void __noreturn __host_enter(struct kvm_cpu_context *host_ctxt);
struct kvm_host_psci_config __ro_after_init kvm_host_psci_config;
s64 __ro_after_init hyp_physvirt_offset;

-#define __hyp_pa(x) ((phys_addr_t)((x)) + hyp_physvirt_offset)
-
#define INVALID_CPU_ID UINT_MAX

struct psci_boot_args {
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index af8e940d0f03..7b8f7db5c1ed 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -27,8 +27,6 @@
#include <asm/processor.h>
#include <asm/thread_info.h>

-const char __hyp_panic_string[] = "HYP panic:\nPS:%08llx PC:%016llx ESR:%08llx\nFAR:%016llx HPFAR:%016llx PAR:%016llx\nVCPU:%p\n";
-
/* VHE specific context */
DEFINE_PER_CPU(struct kvm_host_data, kvm_host_data);
DEFINE_PER_CPU(struct kvm_cpu_context, kvm_hyp_ctxt);
@@ -207,7 +205,7 @@ static void __hyp_call_panic(u64 spsr, u64 elr, u64 par)
__deactivate_traps(vcpu);
sysreg_restore_host_state_vhe(host_ctxt);

- panic(__hyp_panic_string,
+ panic("HYP panic:\nPS:%08llx PC:%016llx ESR:%08llx\nFAR:%016llx HPFAR:%016llx PAR:%016llx\nVCPU:%p\n",
spsr, elr,
read_sysreg_el2(SYS_ESR), read_sysreg_el2(SYS_FAR),
read_sysreg(hpfar_el2), par, vcpu);
--
2.30.0.617.g56c4b15f3c-goog