Skip to content

Commit c43120a

Browse files
ouptonMarc Zyngier
authored andcommitted
KVM: arm64: Avoid lock inversion when setting the VM register width
kvm->lock must be taken outside of the vcpu->mutex. Of course, the locking documentation for KVM makes this abundantly clear. Nonetheless, the locking order in KVM/arm64 has been wrong for quite a while; we acquire the kvm->lock while holding the vcpu->mutex all over the shop. All was seemingly fine until commit 42a9000 ("KVM: Ensure lockdep knows about kvm->lock vs. vcpu->mutex ordering rule") caught us with our pants down, leading to lockdep barfing: ====================================================== WARNING: possible circular locking dependency detected 6.2.0-rc7+ #19 Not tainted ------------------------------------------------------ qemu-system-aar/859 is trying to acquire lock: ffff5aa69269eba0 (&host_kvm->lock){+.+.}-{3:3}, at: kvm_reset_vcpu+0x34/0x274 but task is already holding lock: ffff5aa68768c0b8 (&vcpu->mutex){+.+.}-{3:3}, at: kvm_vcpu_ioctl+0x8c/0xba0 which lock already depends on the new lock. Add a dedicated lock to serialize writes to VM-scoped configuration from the context of a vCPU. Protect the register width flags with the new lock, thus avoiding the need to grab the kvm->lock while holding vcpu->mutex in kvm_reset_vcpu(). Cc: stable@vger.kernel.org Reported-by: Jeremy Linton <jeremy.linton@arm.com> Link: https://lore.kernel.org/kvmarm/f6452cdd-65ff-34b8-bab0-5c06416da5f6@arm.com/ Tested-by: Jeremy Linton <jeremy.linton@arm.com> Signed-off-by: Oliver Upton <oliver.upton@linux.dev> Signed-off-by: Marc Zyngier <maz@kernel.org> Link: https://lore.kernel.org/r/20230327164747.2466958-3-oliver.upton@linux.dev
1 parent 0acc723 commit c43120a

3 files changed

Lines changed: 24 additions & 3 deletions

File tree

arch/arm64/include/asm/kvm_host.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ struct kvm_arch {
199199
/* Mandated version of PSCI */
200200
u32 psci_version;
201201

202+
/* Protects VM-scoped configuration data */
203+
struct mutex config_lock;
204+
202205
/*
203206
* If we encounter a data abort without valid instruction syndrome
204207
* information, report this to user space. User space can (and

arch/arm64/kvm/arm.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,16 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
128128
{
129129
int ret;
130130

131+
mutex_init(&kvm->arch.config_lock);
132+
133+
#ifdef CONFIG_LOCKDEP
134+
/* Clue in lockdep that the config_lock must be taken inside kvm->lock */
135+
mutex_lock(&kvm->lock);
136+
mutex_lock(&kvm->arch.config_lock);
137+
mutex_unlock(&kvm->arch.config_lock);
138+
mutex_unlock(&kvm->lock);
139+
#endif
140+
131141
ret = kvm_share_hyp(kvm, kvm + 1);
132142
if (ret)
133143
return ret;
@@ -328,6 +338,14 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
328338

329339
spin_lock_init(&vcpu->arch.mp_state_lock);
330340

341+
#ifdef CONFIG_LOCKDEP
342+
/* Inform lockdep that the config_lock is acquired after vcpu->mutex */
343+
mutex_lock(&vcpu->mutex);
344+
mutex_lock(&vcpu->kvm->arch.config_lock);
345+
mutex_unlock(&vcpu->kvm->arch.config_lock);
346+
mutex_unlock(&vcpu->mutex);
347+
#endif
348+
331349
/* Force users to call KVM_ARM_VCPU_INIT */
332350
vcpu->arch.target = -1;
333351
bitmap_zero(vcpu->arch.features, KVM_VCPU_MAX_FEATURES);

arch/arm64/kvm/reset.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ static int kvm_set_vm_width(struct kvm_vcpu *vcpu)
205205

206206
is32bit = vcpu_has_feature(vcpu, KVM_ARM_VCPU_EL1_32BIT);
207207

208-
lockdep_assert_held(&kvm->lock);
208+
lockdep_assert_held(&kvm->arch.config_lock);
209209

210210
if (test_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED, &kvm->arch.flags)) {
211211
/*
@@ -262,9 +262,9 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
262262
bool loaded;
263263
u32 pstate;
264264

265-
mutex_lock(&vcpu->kvm->lock);
265+
mutex_lock(&vcpu->kvm->arch.config_lock);
266266
ret = kvm_set_vm_width(vcpu);
267-
mutex_unlock(&vcpu->kvm->lock);
267+
mutex_unlock(&vcpu->kvm->arch.config_lock);
268268

269269
if (ret)
270270
return ret;

0 commit comments

Comments
 (0)