Re: [PATCH 16/20] KVM: arm64: Add __pkvm_host_split_guest HVC

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Failed guest stage-2 page table splits leave the host stage-2 ownership metadata permanently split, causing subsequent split attempts to fail with -EINVAL.
- [High] Incorrect WARN_ON() semantics in host_stage2_split_gfn_meta() turns a recoverable memory allocation failure into a fatal EL2 panic.
--

--- Patch [16]: [PATCH 16/20] KVM: arm64: Add __pkvm_host_split_guest HVC ---
commit 76334f00eacfa593f4125057892101e13067d2e0
Author: Vincent Donnefort <[email protected]>

KVM: arm64: Add __pkvm_host_split_guest HVC

This HVC allows the host to break an existing block in a protected VM. This
will later allow the guest to share a sub-region of an existing
huge-mapping with the host.

> diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> index e16dde4c5f4b9..71ebd1856cb0c 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
[ ... ]
> @@ -660,14 +658,49 @@ static int host_stage2_decode_gfn_meta(kvm_pte_t pte, struct pkvm_hyp_vm **vm,
[ ... ]
> +	end = phys + size;
> +	while (phys < end) {
> +		u64 meta = host_stage2_encode_gfn_meta(vm, gfn);
> +		kvm_pte_t annotation = FIELD_PREP(KVM_HOST_DONATION_PTE_OWNER_MASK, PKVM_ID_GUEST) |
> +				       FIELD_PREP(KVM_HOST_DONATION_PTE_EXTRA_MASK, meta);
> +
> +		ret = host_stage2_try(kvm_pgtable_stage2_annotate, &host_mmu.pgt,
> +				      phys, PAGE_SIZE, &host_s2_pool,
> +				      KVM_HOST_INVALID_PTE_TYPE_DONATION, annotation);
> +		if (WARN_ON(ret))

[Severity: High]
Can a memory allocation failure here trigger a fatal EL2 panic?

Since WARN_ON() at EL2 expands to a fatal BRK instruction with no "warn and
continue" semantics, an -ENOMEM from host_stage2_try() will cause a hypervisor
panic rather than taking the error path.

> +			return ret;
> +
> +		phys += PAGE_SIZE;
> +		gfn++;
>  	}
>  
> -	*gfn = FIELD_GET(KVM_HOST_PTE_OWNER_GUEST_GFN_MASK, meta);
>  	return 0;
>  }
[ ... ]
> @@ -1748,6 +1788,39 @@ int __pkvm_host_mkyoung_guest(u64 gfn, struct pkvm_hyp_vcpu *vcpu)
[ ... ]
> +	ret = host_stage2_split_gfn_meta(phys, ipa, size, vm);
> +	if (ret)
> +		goto unlock;
> +
> +	ret = kvm_pgtable_stage2_split(&vm->pgt, ipa, size, mc);

[Severity: High]
Does this leave the host stage-2 ownership metadata permanently split if
kvm_pgtable_stage2_split() fails?

If the guest split fails (e.g., due to memory exhaustion returning -ENOMEM
or -EAGAIN), the error is returned immediately without reverting the host
stage-2 modifications made by host_stage2_split_gfn_meta(). This could cause
subsequent split attempts for this block to fail with -EINVAL due to the
desynchronized state.

> +
> +unlock:
> +	guest_unlock_component(vm);
> +	host_unlock_component();
> +
> +	return ret;
> +}

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