Re: [PATCH v20 04/14] KVM: arm64: Ensure GCS memory effects are visible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing explicit GCS barriers on the vCPU load/entry paths, failing to order host acquires against incoming guest GCS memory effects.
--

commit 171e0d39ad041f789da5126a2b828fc123587e80
Author: Mark Brown <broonie@kernel.org>

KVM: arm64: Ensure GCS memory effects are visible

This patch adds explicit GCS barriers to vCPU context switch operations.
Since the hypervisor and host kernel do not use GCS themselves, they cannot
rely on implicit barriers and must manually insert them to ensure GCS memory
effects are visible.

>     In order to allow efficient implementation of GCS implementations are
>     permitted to do GCS specific caching, with barriers implicit in stack
>     switch operations and GCSB DSYNC as an explicit barrier.  Since we do
>     not use GCS in the hypervisor or host kernel the hypervisor cannot rely
>     on any barriers being present in vCPU context switches other than those
>     it explicitly inserts.  Add explicit barriers when loading and saving
>     vCPU state.

[Severity: High]
The commit message states that explicit barriers are added when "loading and
saving vCPU state", but the barriers appear to be missing from the load paths.

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 8b080804bc90b..52f53f682e6af 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -755,6 +755,13 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>  			vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY);
>  	}
>  
> +	/*
> +	 * Ensure any GCS memory effects from the outgoing vCPU are
> +	 * visible elsewhere.
> +	 */
> +	if (kvm_has_gcs(vcpu->kvm))
> +		gcsb_dsync();
> +

[Severity: High]
While gcsb_dsync() is added here in the saving/outgoing path
(kvm_arch_vcpu_put), does the corresponding loading/incoming path in
kvm_arch_vcpu_load() also require this barrier?

Without an incoming barrier on the load/entry paths, could a guest's
subsequent GCS memory stores be speculatively reordered before the host's lock
acquisitions are complete?

Could this regression potentially violate memory ordering guarantees if another
CPU observes the guest's execution before the host's context switch locks are
fully resolved?

[ ... ]

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index 9a3b92e626adb..534db037c5f69 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -261,6 +261,13 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
>  	fpsimd_sve_sync(&hyp_vcpu->vcpu);
>  	sync_debug_state(hyp_vcpu);
>  
> +	/*
> +	 * Ensure any GCS memory effects from the outgoing vCPU are
> +	 * visible elsewhere even if the host skips syncing.
> +	 */
> +	if (kvm_has_gcs(hyp_vcpu->vcpu.kvm))
> +		gcsb_dsync();
> +

[Severity: High]
Similarly, a barrier is added here to sync_hyp_vcpu(), but the entry seam
flush_hyp_vcpu() does not receive one. Should flush_hyp_vcpu() also include
an incoming barrier to ensure prior lock acquires are properly ordered against
the incoming thread's GCS effects?

>  	if (pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
>  		host_vcpu->arch.ctxt = hyp_vcpu->vcpu.arch.ctxt;
>  	} else {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-arm64-gcs-v20-0-f31750bdfadb@kernel.org?part=4