Re: pkvm boot failures

[Marc Zyngier <maz@kernel.org>]

Hi Mostafa,

Thanks for looking into this.

On Mon, 09 Jun 2025 18:25:15 +0100,
Mostafa Saleh <smostafa@google.com> wrote:
> 
> On Mon, Jun 09, 2025 at 06:53:40PM +0530, Aneesh Kumar K.V wrote:
> > 
> > I am hitting the below failure with v6.15 (I tried other kernel versions
> > with similar results). I disabled CONFIG_PROTECTED_NVHE_STACKTRACE
> > because with CONFIG_NVHE_EL2_DEBUG, the stack was pointing at
> > hyp_assert_lock_held() .
> > 
> > [    0.664457] kvm [1]: nVHE hyp panic at: [<ffff80008050b10c>] __kvm_nvhe_handle_trap+0x34/0x10c!
> > [    0.664538] kvm [1]: Cannot dump pKVM nVHE stacktrace: !CONFIG_PROTECTED_NVHE_STACKTRACE
> > [    0.664566] kvm [1]: Hyp Offset: 0xffff000007c00000
> > [    0.664631] Kernel panic - not syncing: HYP panic:
> > [    0.664631] PS:614023c9 PC:000080007890b10c ESR:0000000096000007
> > [    0.664631] FAR:0000800078c252f0 HPFAR:0000000000000000 PAR:0000000000000000
> > [    0.664631] VCPU:0000000000000000
> > [    0.664938] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.15.0-rc1 #594 NONE 
> > [    0.665068] Hardware name: FVP Base RevC (DT)
> > [    0.665140] Call trace:
> > [    0.665196]  show_stack+0x18/0x24 (C)
> > [    0.665346]  dump_stack_lvl+0x3c/0x80
> > [    0.665468]  dump_stack+0x18/0x24
> > [    0.665588]  panic+0x124/0x2d8
> > [    0.665699]  nvhe_hyp_panic_handler+0x108/0x180
> > [    0.665825]  do_pkvm_init+0xb0/0x124
> > [    0.665957]  do_pkvm_init+0xb0/0x124
> > [    0.666089]  kvm_hyp_init_protection+0x5c/0x6c
> > [    0.666226]  init_hyp_mode+0x760/0x790
> > [    0.666362]  kvm_arm_init+0xac/0x23c
> > [    0.666492]  do_one_initcall+0xa0/0x1f0
> > [    0.666617]  do_initcall_level+0x8c/0xac
> > [    0.666753]  do_initcalls+0x54/0x94
> > [    0.666885]  do_basic_setup+0x18/0x24
> > [    0.667019]  kernel_init_freeable+0xc0/0x10c
> > [    0.667157]  kernel_init+0x20/0x118
> > [    0.667271]  ret_from_fork+0x10/0x20
> > [    0.667400] SMP: stopping secondary CPUs
> > [    0.667475] Kernel Offset: disabled
> > [    0.667534] CPU features: 0x0000,00000140,064dc298,cb7a552f
> > [    0.667619] Memory Limit: none
> > [    0.667681] ---[ end Kernel panic - not syncing: HYP panic:
> > [    0.667681] PS:614023c9 PC:000080007890b10c ESR:0000000096000007
> > [    0.667681] FAR:0000800078c252f0 HPFAR:0000000000000000 PAR:0000000000000000
> > [    0.667681] VCPU:0000000000000000 ]
> > 
> > I was able to locate a .config that make the pkvm work, But i am not
> > able to identify which config dependency is making the difference. I am
> > attaching below the working and non working kernel configs. I am using
> > FVP to test this.
> > 
> 
> I had a look at this and tracked the issue to "CONFIG_JUMP_LABEL=n"
> It seems that it panics at
> 	if (static_branch_unlikely(&kvm_protected_mode_initialized))
> Where "kvm_protected_mode_initialized" is mapped in the initial PGD for the
> hypervisor, but not mapped in the hypervisor created one.
> As the variable is defined outside the hypervisor namespace, it doesn’t exist
> in the hyp bss section.
> And in case of "CONFIG_JUMP_LABEL=n" it  won't be patched in this case, causing
> next access to read the variable and panicking.

It really begs the question: why do we even support JUMP_LABEL=n? It
really feels like a backward configuration, and I'd be very glad to
either mark it as "always on", or make KVM depend on it.

> I guess moving this key to hyp would cause problems with kernel access after
> de-privilege as cases from kvm_share_hyp(), So I can only think of having a
> different key for the hypervisor as
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index 8e8848de4d47..8945b335bcea 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -21,6 +21,7 @@
>  #include <nvhe/trap_handler.h>
>  
>  DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
> +DEFINE_STATIC_KEY_FALSE(kvm_protected_mode_initialized_hyp);
>  
>  void __kvm_hyp_host_forward_smc(struct kvm_cpu_context *host_ctxt);
>  
> @@ -626,7 +627,7 @@ static void handle_host_hcall(struct kvm_cpu_context *host_ctxt)
>  	 * basis. This is all fine, however, since __pkvm_prot_finalize
>  	 * returns -EPERM after the first call for a given CPU.
>  	 */
> -	if (static_branch_unlikely(&kvm_protected_mode_initialized))
> +	if (static_branch_unlikely(&kvm_protected_mode_initialized_hyp))
>  		hcall_min = __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize;
>  
>  	id &= ~ARM_SMCCC_CALL_HINTS;
> diff --git a/arch/arm64/kvm/pkvm.c b/arch/arm64/kvm/pkvm.c
> index fcd70bfe44fb..af0854e98902 100644
> --- a/arch/arm64/kvm/pkvm.c
> +++ b/arch/arm64/kvm/pkvm.c
> @@ -17,6 +17,7 @@
>  #include "hyp_constants.h"
>  
>  DEFINE_STATIC_KEY_FALSE(kvm_protected_mode_initialized);
> +DECLARE_STATIC_KEY_FALSE(kvm_nvhe_sym(kvm_protected_mode_initialized_hyp));
>  
>  static struct memblock_region *hyp_memory = kvm_nvhe_sym(hyp_memory);
>  static unsigned int *hyp_memblock_nr_ptr = &kvm_nvhe_sym(hyp_memblock_nr);
> @@ -229,6 +230,7 @@ static int __init pkvm_drop_host_privileges(void)
>  	 * once the host stage 2 is installed.
>  	 */
>  	static_branch_enable(&kvm_protected_mode_initialized);
> +	static_branch_enable(&kvm_nvhe_sym(kvm_protected_mode_initialized_hyp));
>  	on_each_cpu(_kvm_host_prot_finalize, &ret, 1);
>  	return ret;
>  }
>

I don't really enjoy this duplication, and unless we have a good
reason not too, I'd rather have something like:

diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 713248f240e0..66d232e7c894 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -37,6 +37,7 @@ menuconfig KVM
 	select HAVE_KVM_VCPU_RUN_PID_CHANGE
 	select SCHED_INFO
 	select GUEST_PERF_EVENTS if PERF_EVENTS
+	select JUMP_LABEL
 	help
 	  Support hosting virtualized guest machines.
 
It should be OK now that all the supported compilers have asm goto
support.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.