From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 54402C433DB for ; Tue, 23 Feb 2021 11:30:05 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 93D8164E58 for ; Tue, 23 Feb 2021 11:30:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 93D8164E58 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id DFCBD4B1B6; Tue, 23 Feb 2021 06:30:03 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Va8jInPdQssd; Tue, 23 Feb 2021 06:30:02 -0500 (EST) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 90BC54B1BC; Tue, 23 Feb 2021 06:30:02 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 953B74B1D3 for ; Tue, 23 Feb 2021 06:30:00 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id o1G64uR0Hm5p for ; Tue, 23 Feb 2021 06:29:59 -0500 (EST) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 2E4724B1BC for ; Tue, 23 Feb 2021 06:29:59 -0500 (EST) Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id D8CB76186A; Tue, 23 Feb 2021 11:29:57 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94) (envelope-from ) id 1lEVt5-00FUFf-8w; Tue, 23 Feb 2021 11:29:55 +0000 Date: Tue, 23 Feb 2021 11:29:54 +0000 Message-ID: <87czwr10p9.wl-maz@kernel.org> From: Marc Zyngier To: Andrew Scull Subject: Re: [PATCH 2/2] KVM: arm64: Log source when panicking from nVHE hyp In-Reply-To: <20210223094927.766572-3-ascull@google.com> References: <20210223094927.766572-1-ascull@google.com> <20210223094927.766572-3-ascull@google.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: ascull@google.com, kvmarm@lists.cs.columbia.edu, james.morse@arm.com, suzuki.poulose@arm.com, julien.thierry.kdev@gmail.com, will@kernel.org, catalin.marinas@arm.com, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Cc: kernel-team@android.com, catalin.marinas@arm.com, will@kernel.org, kvmarm@lists.cs.columbia.edu X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu Hi Andrew, On Tue, 23 Feb 2021 09:49:27 +0000, Andrew Scull wrote: > > 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. > > Signed-off-by: Andrew Scull > --- > arch/arm64/include/asm/kvm_mmu.h | 2 ++ > arch/arm64/kernel/image-vars.h | 3 +- > arch/arm64/kvm/handle_exit.c | 38 +++++++++++++++++++++++++ > arch/arm64/kvm/hyp/include/hyp/switch.h | 2 -- > arch/arm64/kvm/hyp/nvhe/host.S | 18 ++++++------ > arch/arm64/kvm/hyp/nvhe/psci-relay.c | 2 -- > 6 files changed, 49 insertions(+), 16 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..5e0d9a5152e5 100644 > --- a/arch/arm64/kvm/handle_exit.c > +++ b/arch/arm64/kvm/handle_exit.c > @@ -291,3 +291,41 @@ 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(bool hyp, u64 spsr, u64 elr, > + u64 par, uintptr_t vcpu, > + u64 far, u64 hpfar, u64 esr) { > + extern const char __hyp_panic_string[]; Is there any reason left to not to make this a symbol at all, but instead to feed the string to the panic() call below? > + u64 elr_in_kimg = __phys_to_kimg(__hyp_pa(elr)); > + > + if (!hyp) { > + kvm_err("Invalid host exception to nVHE hyp!\n"); Do we need to have hyp as a parameter? Can't we work that out from the SPSR passed as a parameter? > + } 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 line = 0; > + > + /* All hyp bugs, including warnings, are treated as fatal. */ > + if (bug) { > +#ifdef CONFIG_DEBUG_BUGVERBOSE > +#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS > + file = bug->file; > +#else > + file = (const char *)bug + bug->file_disp; > +#endif > + line = bug->line; > +#endif It looks like you have lifted this from lib/bugs.c. Would it be worth making it a helper that lives there? Something like struct bug_entry *bug_get_entry(unsigned long pc, char **file, unsigned int *line); that hides the #ifdefery and the fund_bug() call away? The disable_trace_on_warning() call isn't a problem, as we're about to die anyway. > + } > + > + 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_string, 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 > #include > > -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..d9a9dd66b1a2 100644 > --- a/arch/arm64/kvm/hyp/nvhe/host.S > +++ b/arch/arm64/kvm/hyp/nvhe/host.S > @@ -77,21 +77,19 @@ 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 */ > + cmp x0, xzr > + cset x0, ne > + get_vcpu_ptr x4, x5 > + mrs x5, far_el2 > + mrs x6, hpfar_el2 > + mrs x7, esr_el2 > > /* 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 { > -- > 2.30.0.617.g56c4b15f3c-goog > > Otherwise, I like the idea! Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm