Re: [PATCH v2] KVM: arm64: Drop %pB on nVHE panic when stage-2 is active

On Mon, Aug 03, 2026 at 09:50:16AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incomplete fix for host stage-2 deadlock in nVHE hyp panic handler.
> --
> 
> --- Patch [1]: [PATCH v2] KVM: arm64: Drop %pB on nVHE panic when stage-2 is active ---
> commit 6bd2e7dd3552f4e44146bf3e1065ca1511de7411
> Author: Vincent Donnefort <[email protected]>
> 
> KVM: arm64: Drop %pB on nVHE panic when stage-2 is active
> 
> This patch introduces nvhe_hyp_panic_host_s2_disabled() to skip %pB
> formatting during a hyp panic when host stage-2 is active. It aims to prevent
> deadlocks that occur if the host stage-2 lock is held and a fault happens
> when accessing lazily mapped kallsyms data.
> 
> > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> > index 54aedf93c78b6..88dcb8cb36f3c 100644
> > --- a/arch/arm64/kvm/handle_exit.c
> > +++ b/arch/arm64/kvm/handle_exit.c
> [ ... ]
> > @@ -538,8 +548,7 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr,
> >  		unsigned int line = 0;
> >  
> >  		/* All hyp bugs, including warnings, are treated as fatal. */
> > -		if (!is_protected_kvm_enabled() ||
> > -		    IS_ENABLED(CONFIG_PKVM_DISABLE_STAGE2_ON_PANIC)) {
> > +		if (nvhe_hyp_panic_host_s2_disabled()) {
> >  			struct bug_entry *bug = find_bug(elr_in_kimg);
> >  
> >  			if (bug)
> 
> [Severity: High]
> This is a pre-existing issue, but does this fully resolve the host stage-2
> deadlock?
> 
> Further down in this same function, the backtrace and instruction dumpers
> are called unconditionally:
> 
> 	/* Dump the nVHE hypervisor backtrace */
> 	kvm_nvhe_dump_backtrace(hyp_offset);
> 
> 	/* Dump the faulting instruction */
> 	dump_kernel_instr(panic_addr + kaslr_offset());
> 
> kvm_nvhe_dump_backtrace() iterates over stack frames and calls
> kvm_nvhe_dump_backtrace_entry(), which uses the exact same %pB formatter
> to print the frames:
> 
> arch/arm64/kvm/stacktrace.c:kvm_nvhe_dump_backtrace_entry() {
>     ...
> 	kvm_err(" [<%016lx>] %pB\n", where, (void *)(where + kaslr_offset()));
>     ...
> }

stacktrace depends on CONFIG_PKVM_DISABLE_STAGE2_ON_PANIC already...

> 
> Similarly, dump_kernel_instr() reads the faulting instruction directly from
> kernel memory, which could trigger a stage-2 data abort if the page is not
> present in the lazy host stage-2 page tables:
> 
> arch/arm64/kernel/traps.c:dump_kernel_instr() {
>     ...
> 		bad = aarch64_insn_read(&((u32 *)kaddr)[i], &val);
>     ...
> }

We actively maps .hyp.text in the host stage-2 at boot.

> 
> If a panic occurs while the host stage-2 lock is held, won't these
> unconditional calls still trigger the stage-2 fault and deadlock that
> this patch is trying to avoid?
> 
> Should these operations also be gated by nvhe_hyp_panic_host_s2_disabled()?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

-- 
Vincent