[PATCH v1 1/2] KVM: arm64: Acquire mp_state_lock in kvm_arch_vcpu_ioctl_vcpu_init()

[Marc Zyngier]

On Wed, 19 Apr 2023 03:18:51 +0100,
Reiji Watanabe <reijiw at google.com> wrote:
> 
> kvm_arch_vcpu_ioctl_vcpu_init() doesn't acquire mp_state_lock
> when setting the mp_state to KVM_MP_STATE_RUNNABLE. Fix the
> code to acquire the lock.
> 
> Signed-off-by: Reiji Watanabe <reijiw at google.com>
> ---
>  arch/arm64/kvm/arm.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index fbafcbbcc463..388aa4f18f21 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1244,8 +1244,11 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
>  	 */
>  	if (test_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features))
>  		kvm_arm_vcpu_power_off(vcpu);
> -	else
> +	else {
> +		spin_lock(&vcpu->arch.mp_state_lock);
>  		WRITE_ONCE(vcpu->arch.mp_state.mp_state, KVM_MP_STATE_RUNNABLE);
> +		spin_unlock(&vcpu->arch.mp_state_lock);
> +	}
>  
>  	return 0;
>  }

I'm not entirely convinced that this fixes anything. What does the
lock hazard against given that the write is atomic? But maybe a
slightly more readable of this would be to expand the critical section
this way:

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 4ec888fdd4f7..bb21d0c25de7 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1246,11 +1246,15 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
 	/*
 	 * Handle the "start in power-off" case.
 	 */
+	spin_lock(&vcpu->arch.mp_state_lock);
+
 	if (test_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features))
-		kvm_arm_vcpu_power_off(vcpu);
+		__kvm_arm_vcpu_power_off(vcpu);
 	else
 		WRITE_ONCE(vcpu->arch.mp_state.mp_state, KVM_MP_STATE_RUNNABLE);
 
+	spin_unlock(&vcpu->arch.mp_state_lock);
+
 	return 0;
 }

Thoughts?

	M.

-- 
Without deviation from the norm, progress is not possible.