Re: pkvm boot failures

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Mostafa Saleh <smostafa@google.com> writes:

> 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.
>
> 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;
>

There are other references to kvm_protected_mode_initialized within
nvhe. Shouldn't all of those be updated as well?

Also, do we need to drop this line?

KVM_NVHE_ALIAS(kvm_protected_mode_initialized);

If I understand correctly, nvhe will now have a separate static key,
distinct from the one used by the EL1 host. So we'll need to update
functions like is_pkvm_initialized() and host_data_ptr() accordingly,
right?

>  
>  	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;
>  }
>
> --
>
> Can you please check if that helps in your case?
>
> Thanks,
> Mostafa
>

-aneesh