Re: [PATCH v3 22/23] KVM: arm64: Raise PKVM_HYP_REQ_SPLIT on guest to host sharing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: arm64: unwanted block split and latent panic in pkvm_memunshare_call()
--

commit 8b44fbe0de868cf9681f0eef63a65d11368c1c9d
Author: Vincent Donnefort <vdonnefort@google.com>

KVM: arm64: Raise PKVM_HYP_REQ_SPLIT on guest to host sharing

This commit raises a PKVM_HYP_REQ_SPLIT hypervisor request and rewinds
ELR_EL2 when a guest MEM_SHARE or MEM_UNSHARE hypercall encounters a huge
stage-2 block mapping (-E2BIG) so that the host can split the block and
replay the hypercall.

> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> index d92f4da1c1807..6c126fcc3831b 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c

[ ... ]

> @@ -1197,17 +1218,29 @@ static bool pkvm_memshare_call(u64 *ret, struct kvm_vcpu *vcpu, u64 *exit_code)
>  	return false;
>  }
>  
> -static void pkvm_memunshare_call(u64 *ret, struct kvm_vcpu *vcpu)
> +static bool pkvm_memunshare_call(u64 *ret, struct kvm_vcpu *vcpu, u64 *exit_code)
>  {
>  	struct pkvm_hyp_vcpu *hyp_vcpu;
>  	u64 ipa = smccc_get_arg1(vcpu);
> +	u64 gfn = hyp_phys_to_pfn(ipa);
>  
>  	if (!PAGE_ALIGNED(ipa))
> -		return;
> +		goto out_guest;
>  
>  	hyp_vcpu = container_of(vcpu, struct pkvm_hyp_vcpu, vcpu);
> -	if (!__pkvm_guest_unshare_host(hyp_vcpu, hyp_phys_to_pfn(ipa), 1))
> +	switch (__pkvm_guest_unshare_host(hyp_vcpu, gfn, 1)) {
> +	case 0:
>  		ret[0] = SMCCC_RET_SUCCESS;
> +		goto out_guest;
> +	case -E2BIG:
> +		*exit_code = pkvm_request_split(hyp_vcpu, gfn, 1);
> +		goto out_host;
> +	}

[Severity: Medium]
Should pkvm_memunshare_call() request a block split when
__pkvm_guest_unshare_host() returns -E2BIG?

In arch/arm64/kvm/hyp/nvhe/mem_protect.c, __pkvm_guest_unshare_host() calls
get_valid_guest_pte() before checking whether the page state is
PKVM_PAGE_SHARED_OWNED:

arch/arm64/kvm/hyp/nvhe/mem_protect.c:__pkvm_guest_unshare_host() {
	...
	ret = get_valid_guest_pte(vm, ipa, size, &pte, &phys);
	if (ret)
		goto unlock;

	ret = -EPERM;
	if (pkvm_getstate(kvm_pgtable_stage2_pte_prot(pte)) != PKVM_PAGE_SHARED_OWNED)
		goto unlock;
	...
}

If a protected guest issues ARM_SMCCC_VENDOR_HYP_KVM_MEM_UNSHARE_FUNC_ID on
an IPA backed by an unshared (PKVM_PAGE_OWNED) PMD block mapping,
get_valid_guest_pte() sees size == PAGE_SIZE against the level-2 block leaf
and immediately returns -E2BIG.

With this change, pkvm_memunshare_call() catches -E2BIG and exits to the
host via pkvm_request_split() to shatter the unshared 2MB block in both
guest and host stage-2 page tables (and split its pkvm_mapping into 512
entries), only for the replayed MEM_UNSHARE hypercall to fail the
PKVM_PAGE_SHARED_OWNED check with -EPERM.

Furthermore, if a PMD block were ever shared with the host in the future as
mentioned in the commit message, __pkvm_guest_share_host() ->
__host_set_page_state_range() installs a valid PKVM_HOST_MEM_PROT entry in
host_mmu.pgt (replacing the invalid donation metadata PTE). If
pkvm_memunshare_call() then triggers PKVM_HYP_REQ_SPLIT on that shared
block, the host split path calls:

  __pkvm_host_split_guest()
    -> host_stage2_split_gfn_meta()
      -> host_stage2_decode_gfn_meta()

which hits WARN_ON(kvm_pte_valid(pte)) and panics EL2:

arch/arm64/kvm/hyp/nvhe/mem_protect.c:host_stage2_decode_gfn_meta() {
	...
	if (WARN_ON(kvm_pte_valid(pte)))
		return -EINVAL;
	...
}

Would it be better not to request PKVM_HYP_REQ_SPLIT in
pkvm_memunshare_call() while MEM_SHARE is single-page only (or check the
guest PTE state before returning -E2BIG)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102204628.401871-1-vdonnefort@google.com?part=22