From e4446b24fde7d26d6f9f75ff9f919e95e9734b2d Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sat, 10 Jun 2023 08:25:51 +0000 Subject: [PATCH] Revert "KVM: arm64: Avoid vcpu->mutex v. kvm->lock inversion in CPU_ON" This reverts commit 36e0c405b86ed5300263fbbe1c70690ffb5664a1. It breaks the Android kernel ABI at this point in time, so needs to be dropped. If it is needed, it can come back in an ABI-safe way in the future. Bug: 161946584 Cc: Will Deacon Change-Id: Ia133048be41bab446ff3e8643816dd0c1f337798 Signed-off-by: Greg Kroah-Hartman --- arch/arm64/include/asm/kvm_host.h | 1 - arch/arm64/kvm/arm.c | 31 +++++++++---------------------- arch/arm64/kvm/psci.c | 28 ++++++++++++---------------- arch/arm64/kvm/reset.c | 9 ++++----- 4 files changed, 25 insertions(+), 44 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 43015791f9c5..52a1901b38f7 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -507,7 +507,6 @@ struct kvm_vcpu_arch { /* vcpu power state */ struct kvm_mp_state mp_state; - spinlock_t mp_state_lock; union { /* Cache some mmu pages needed inside spinlock regions */ diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index e4965f6038b2..a219e6ddf4f5 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -423,8 +423,6 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) { int err; - spin_lock_init(&vcpu->arch.mp_state_lock); - /* Force users to call KVM_ARM_VCPU_INIT */ vcpu->arch.target = -1; bitmap_zero(vcpu->arch.features, KVM_VCPU_MAX_FEATURES); @@ -568,41 +566,34 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) vcpu->cpu = -1; } -static void __kvm_arm_vcpu_power_off(struct kvm_vcpu *vcpu) +void kvm_arm_vcpu_power_off(struct kvm_vcpu *vcpu) { - WRITE_ONCE(vcpu->arch.mp_state.mp_state, KVM_MP_STATE_STOPPED); + vcpu->arch.mp_state.mp_state = KVM_MP_STATE_STOPPED; kvm_make_request(KVM_REQ_SLEEP, vcpu); kvm_vcpu_kick(vcpu); } -void kvm_arm_vcpu_power_off(struct kvm_vcpu *vcpu) -{ - spin_lock(&vcpu->arch.mp_state_lock); - __kvm_arm_vcpu_power_off(vcpu); - spin_unlock(&vcpu->arch.mp_state_lock); -} - bool kvm_arm_vcpu_stopped(struct kvm_vcpu *vcpu) { - return READ_ONCE(vcpu->arch.mp_state.mp_state) == KVM_MP_STATE_STOPPED; + return vcpu->arch.mp_state.mp_state == KVM_MP_STATE_STOPPED; } static void kvm_arm_vcpu_suspend(struct kvm_vcpu *vcpu) { - WRITE_ONCE(vcpu->arch.mp_state.mp_state, KVM_MP_STATE_SUSPENDED); + vcpu->arch.mp_state.mp_state = KVM_MP_STATE_SUSPENDED; kvm_make_request(KVM_REQ_SUSPEND, vcpu); kvm_vcpu_kick(vcpu); } static bool kvm_arm_vcpu_suspended(struct kvm_vcpu *vcpu) { - return READ_ONCE(vcpu->arch.mp_state.mp_state) == KVM_MP_STATE_SUSPENDED; + return vcpu->arch.mp_state.mp_state == KVM_MP_STATE_SUSPENDED; } int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu, struct kvm_mp_state *mp_state) { - *mp_state = READ_ONCE(vcpu->arch.mp_state); + *mp_state = vcpu->arch.mp_state; return 0; } @@ -612,14 +603,12 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu, { int ret = 0; - spin_lock(&vcpu->arch.mp_state_lock); - switch (mp_state->mp_state) { case KVM_MP_STATE_RUNNABLE: - WRITE_ONCE(vcpu->arch.mp_state, *mp_state); + vcpu->arch.mp_state = *mp_state; break; case KVM_MP_STATE_STOPPED: - __kvm_arm_vcpu_power_off(vcpu); + kvm_arm_vcpu_power_off(vcpu); break; case KVM_MP_STATE_SUSPENDED: kvm_arm_vcpu_suspend(vcpu); @@ -628,8 +617,6 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu, ret = -EINVAL; } - spin_unlock(&vcpu->arch.mp_state_lock); - return ret; } @@ -1340,7 +1327,7 @@ 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 - WRITE_ONCE(vcpu->arch.mp_state.mp_state, KVM_MP_STATE_RUNNABLE); + vcpu->arch.mp_state.mp_state = KVM_MP_STATE_RUNNABLE; return 0; } diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c index 70cabcea8a4f..0f7001d726a2 100644 --- a/arch/arm64/kvm/psci.c +++ b/arch/arm64/kvm/psci.c @@ -46,7 +46,6 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) struct vcpu_reset_state *reset_state; struct kvm *kvm = source_vcpu->kvm; struct kvm_vcpu *vcpu = NULL; - int ret = PSCI_RET_SUCCESS; unsigned long cpu_id; cpu_id = smccc_get_arg1(source_vcpu); @@ -61,15 +60,11 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) */ if (!vcpu) return PSCI_RET_INVALID_PARAMS; - - spin_lock(&vcpu->arch.mp_state_lock); if (!kvm_arm_vcpu_stopped(vcpu)) { if (kvm_psci_version(source_vcpu) != KVM_ARM_PSCI_0_1) - ret = PSCI_RET_ALREADY_ON; + return PSCI_RET_ALREADY_ON; else - ret = PSCI_RET_INVALID_PARAMS; - - goto out_unlock; + return PSCI_RET_INVALID_PARAMS; } reset_state = &vcpu->arch.reset_state; @@ -85,7 +80,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) */ reset_state->r0 = smccc_get_arg3(source_vcpu); - reset_state->reset = true; + WRITE_ONCE(reset_state->reset, true); kvm_make_request(KVM_REQ_VCPU_RESET, vcpu); /* @@ -97,9 +92,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) vcpu->arch.mp_state.mp_state = KVM_MP_STATE_RUNNABLE; kvm_vcpu_wake_up(vcpu); -out_unlock: - spin_unlock(&vcpu->arch.mp_state_lock); - return ret; + return PSCI_RET_SUCCESS; } static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu) @@ -159,11 +152,8 @@ static void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type, u64 flags) * after this call is handled and before the VCPUs have been * re-initialized. */ - kvm_for_each_vcpu(i, tmp, vcpu->kvm) { - spin_lock(&tmp->arch.mp_state_lock); - WRITE_ONCE(tmp->arch.mp_state.mp_state, KVM_MP_STATE_STOPPED); - spin_unlock(&tmp->arch.mp_state_lock); - } + kvm_for_each_vcpu(i, tmp, vcpu->kvm) + tmp->arch.mp_state.mp_state = KVM_MP_STATE_STOPPED; kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_SLEEP); memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event)); @@ -211,6 +201,7 @@ static unsigned long kvm_psci_check_allowed_function(struct kvm_vcpu *vcpu, u32 static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu) { + struct kvm *kvm = vcpu->kvm; u32 psci_fn = smccc_get_function(vcpu); unsigned long val; int ret = 1; @@ -235,7 +226,9 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu) kvm_psci_narrow_to_32bit(vcpu); fallthrough; case PSCI_0_2_FN64_CPU_ON: + mutex_lock(&kvm->lock); val = kvm_psci_vcpu_on(vcpu); + mutex_unlock(&kvm->lock); break; case PSCI_0_2_FN_AFFINITY_INFO: kvm_psci_narrow_to_32bit(vcpu); @@ -374,6 +367,7 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor) static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu) { + struct kvm *kvm = vcpu->kvm; u32 psci_fn = smccc_get_function(vcpu); unsigned long val; @@ -383,7 +377,9 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu) val = PSCI_RET_SUCCESS; break; case KVM_PSCI_FN_CPU_ON: + mutex_lock(&kvm->lock); val = kvm_psci_vcpu_on(vcpu); + mutex_unlock(&kvm->lock); break; default: val = PSCI_RET_NOT_SUPPORTED; diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c index 50349553bee9..2ede29175bf0 100644 --- a/arch/arm64/kvm/reset.c +++ b/arch/arm64/kvm/reset.c @@ -230,16 +230,15 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu) mutex_lock(&vcpu->kvm->lock); ret = kvm_set_vm_width(vcpu); + if (!ret) { + reset_state = vcpu->arch.reset_state; + WRITE_ONCE(vcpu->arch.reset_state.reset, false); + } mutex_unlock(&vcpu->kvm->lock); if (ret) return ret; - spin_lock(&vcpu->arch.mp_state_lock); - reset_state = vcpu->arch.reset_state; - vcpu->arch.reset_state.reset = false; - spin_unlock(&vcpu->arch.mp_state_lock); - /* Reset PMU outside of the non-preemptible section */ kvm_pmu_vcpu_reset(vcpu);