Re: [PATCH 19/20] KVM: arm64: Raise PKVM_HYP_REQ_SPLIT on guest to host sharing

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent MEM_SHARE or MEM_UNSHARE on the same huge block causes the VM to be terminated with -EINVAL.
--

--- Patch [19]: [PATCH 19/20] KVM: arm64: Raise PKVM_HYP_REQ_SPLIT on guest to host sharing ---
commit 267c394d1b60753f90e3da486356d60c47b19643
Author: Vincent Donnefort <[email protected]>

KVM: arm64: Raise PKVM_HYP_REQ_SPLIT on guest to host sharing

Pages shared by the guest with the host are annotated into the guest
stage-2. If the shared page is backed by a huge mapping, we need to
split it first.

> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> index 3b8e95b83bf45..57f4303aede10 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c

[ ... ]

> -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: High]
Does this code handle concurrent MEM_UNSHARE or MEM_SHARE requests on the
same huge block correctly?

If two guest vCPUs concurrently execute HVC MEM_SHARE (or MEM_UNSHARE) on
different pages within the same PMD_SIZE stage-2 block, both vCPUs would
receive -E2BIG from get_valid_guest_pte() in EL2 and exit to the host with
PKVM_HYP_REQ_SPLIT.

Host thread A successfully handles the split request, splitting the block in
EL2 and the host's pkvm_mappings tree.

When host thread B handles the split request for the same block, it looks up
the mapping in __pkvm_pgtable_stage2_split():

arch/arm64/kvm/pkvm.c:__pkvm_pgtable_stage2_split() {
    ...
	mapping = pkvm_mapping_iter_first(&pgt->pkvm_mappings, ipa,
					  ipa + size - 1);
	if (!pkvm_mapping_can_split(mapping)) {
		ret = -EINVAL;
		goto unlock_mmu;
	}
    ...
}

Because it finds a PAGE_SIZE mapping (since it was just split),
pkvm_mapping_can_split() returns false, causing __pkvm_pgtable_stage2_split()
to return -EINVAL instead of returning 0 (success) or -EAGAIN to retry.

This -EINVAL error propagates up through pkvm_hyp_req_handle_split() and
handle_exit(), returning -EINVAL to userspace from KVM_RUN, which causes the
VMM to terminate the VM.

> +	}
> +
> +out_guest:
> +	return true;
> +out_host:
> +	return false;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=19