Re: [PATCH RFC] KVM: arm64: Document that overlapping MEM_SHARE and MEM_UNSHARE are unordered

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: arm64: hypercall doc gives insufficient condition for private region
--

commit 39c34714b27ef8c9e3ad559a667906cddd707b8b
Author: Xuheng Li <xuheng@cs.columbia.edu>

KVM: arm64: Document that overlapping MEM_SHARE and MEM_UNSHARE are unordered

This commit documents in Documentation/virt/kvm/arm/hypercalls.rst that
overlapping calls to ARM_SMCCC_KVM_FUNC_MEM_SHARE and
ARM_SMCCC_KVM_FUNC_MEM_UNSHARE for the same memory region are unordered with
respect to each other.

> diff --git a/Documentation/virt/kvm/arm/hypercalls.rst b/Documentation/virt/kvm/arm/hypercalls.rst
> index 7400a89aabf88..ad7082da1b9fc 100644
> --- a/Documentation/virt/kvm/arm/hypercalls.rst
> +++ b/Documentation/virt/kvm/arm/hypercalls.rst
> @@ -99,6 +99,16 @@ Revoke access permission from the KVM host to a memory region previously shared
>  with ``ARM_SMCCC_KVM_FUNC_MEM_SHARE``. The size of the region is equal to the
>  memory protection granule advertised by ``ARM_SMCCC_KVM_FUNC_HYP_MEMINFO``.
>  
> +Calls to ``ARM_SMCCC_KVM_FUNC_MEM_SHARE`` and ``ARM_SMCCC_KVM_FUNC_MEM_UNSHARE``
> +for the same region that overlap in time, for example on different vCPUs, are
> +not ordered with respect to each other: a share that has not yet returned may
> +take effect after an unshare of the region has returned, leaving the region
> +shared. A share of a region that is not yet mapped is restarted after the host
> +has mapped it, so such a share can remain pending for as long as the host
> +delays the vCPU. A guest must therefore not treat a region as private again
> +until every ``ARM_SMCCC_KVM_FUNC_MEM_SHARE`` it issued for the region has
> +returned.

[Severity: Medium]
Is waiting until every issued ARM_SMCCC_KVM_FUNC_MEM_SHARE has returned
sufficient for a guest to treat the region as private again?

In the race described by the commit message, if vCPU A issues
ARM_SMCCC_KVM_FUNC_MEM_SHARE while the page is unmapped at stage-2, and vCPU B
issues ARM_SMCCC_KVM_FUNC_MEM_SHARE followed by
ARM_SMCCC_KVM_FUNC_MEM_UNSHARE, vCPU B's __pkvm_guest_unshare_host() returns
the page state to PKVM_PAGE_OWNED in the guest stage-2 and PKVM_NOPAGE in the
host stage-2.

When vCPU A's pending ARM_SMCCC_KVM_FUNC_MEM_SHARE subsequently runs,
arch/arm64/kvm/hyp/nvhe/mem_protect.c:__pkvm_guest_share_host() sees the page
back in PKVM_PAGE_OWNED state and shares it again:

	if (pkvm_getstate(kvm_pgtable_stage2_pte_prot(pte)) != PKVM_PAGE_OWNED)
		goto unlock;
	if (__host_check_page_state_range(phys, PAGE_SIZE, PKVM_NOPAGE))
		goto unlock;

	ret = 0;
	WARN_ON(kvm_pgtable_stage2_map(&vm->pgt, ipa, PAGE_SIZE, phys,
				       pkvm_mkstate(KVM_PGTABLE_PROT_RWX, PKVM_PAGE_SHARED_OWNED),
				       &vcpu->vcpu.arch.stage2_mc, 0));
	WARN_ON(__host_set_page_state_range(phys, PAGE_SIZE, PKVM_PAGE_SHARED_BORROWED));

Once vCPU A's call returns, every ARM_SMCCC_KVM_FUNC_MEM_SHARE issued for the
region has returned, satisfying the documented condition, yet the region is
still shared with the host rather than private.

Should the documentation instead state that every prior
ARM_SMCCC_KVM_FUNC_MEM_SHARE for a region must have returned before issuing
ARM_SMCCC_KVM_FUNC_MEM_UNSHARE for the guest to treat the region as private
once the unshare succeeds?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-mem-share-unshare-v1-1-e4c7d12d5d1d@cs.columbia.edu?part=1