Re: [PATCH v5] KVM: PPC: Book3S HV: Validate arch_compat against host compatibility mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Destructive API failure corrupts shared vcore state (`arch_compat`, `pcr`) but only updates the per-vCPU `sane` flag, desynchronizing sibling vCPUs.
- [Medium] Lockless read of the shared `vcore->arch_compat` in `kvmppc_sanity_check()` causes a data race.
--

--- Patch [1]: [PATCH v5] KVM: PPC: Book3S HV: Validate arch_compat against host compatibility mode ---
commit 3439effb1356334e2a4fa14fb4f8e03f83d0556d
Author: Amit Machhiwal <amachhiw@linux.ibm.com>
Date:   Tue Jul 14 17:54:32 2026 +0000

KVM: PPC: Book3S HV: Validate arch_compat against host compatibility mode

This patch introduces early validation of arch_compat during guest
initialization. When an unsupported arch_compat is requested, it detects the
mismatch, sets arch_compat to PVR_ARCH_INVALID, and prevents the vCPU from
running.

> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 61dbeea317f3b..f9380ef657500 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
[ ... ]
> @@ -469,6 +481,7 @@ static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, u32 arch_compat)
>  			return -EINVAL;
>  	}
>  
> +out:
>  	spin_lock(&vc->lock);
>  	vc->arch_compat = arch_compat;
>  	kvmhv_nestedv2_mark_dirty(vcpu, KVMPPC_GSID_LOGICAL_PVR);
> @@ -479,7 +492,7 @@ static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, u32 arch_compat)
>  	vc->pcr = (host_pcr_bit - guest_pcr_bit) | PCR_MASK;
>  	spin_unlock(&vc->lock);
>  
> -	return 0;
> +	return kvmppc_sanity_check(vcpu);
>  }

[Severity: High]
Does this error path corrupt the shared vcore state for sibling vCPUs?

If userspace requests an unsupported arch_compat on one vCPU, the shared
vcore state (arch_compat and pcr) is poisoned. However, kvmppc_sanity_check()
only updates the per-vCPU sane flag for the calling vCPU:

arch/powerpc/kvm/powerpc.c:kvmppc_sanity_check() {
    ...
out:
    vcpu->arch.sane = r;
    return r ? 0 : -EINVAL;
}

Because the sanity flag is cached per-vCPU, sibling vCPUs would not get
updated. If userspace ignores the error on the first vCPU and attempts to run
a sibling vCPU, could that sibling bypass the validation in
kvmppc_vcpu_run_hv() since its individual flag is still true? This would
allow it to enter the guest with the corrupted vcore state (raw mode).

Conversely, if userspace later fixes the vcore via a valid request on a
sibling vCPU, does the first vCPU remain permanently dead because its cached
sane flag is not re-evaluated?

> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 00302399fc37b..b6b83fe3233f3 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -258,6 +258,12 @@ int kvmppc_sanity_check(struct kvm_vcpu *vcpu)
>  	if (!vcpu->arch.pvr)
>  		goto out;
>  
> +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE)
> +	if (vcpu->arch.vcore &&
> +	    vcpu->arch.vcore->arch_compat == PVR_ARCH_INVALID)
> +		goto out;
> +#endif
> +

[Severity: Medium]
Is there a data race when accessing the shared vcore->arch_compat here?

In kvmppc_set_arch_compat(), the modification to vcore->arch_compat is done
while holding vc->lock:

arch/powerpc/kvm/book3s_hv.c:kvmppc_set_arch_compat() {
    ...
    spin_lock(&vc->lock);
    vc->arch_compat = arch_compat;
    ...
}

However, the read in kvmppc_sanity_check() occurs locklessly.

If a concurrent operation like KVM_SET_SREGS on a sibling vCPU calls
kvmppc_sanity_check() under its own per-vCPU mutex, could it race with
a KVM_SET_ONE_REG updating the compatibility mode on the first vCPU?

Since lockless access to shared variables usually requires at least READ_ONCE()
and WRITE_ONCE() to prevent compiler optimizations like tearing or stale
caching, should this read be protected or use READ_ONCE()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714175432.86388-1-amachhiw@linux.ibm.com?part=1