Re: [RFC] powerpc/kvm: Fix spinlock member access for PREEMPT_RT

From: Paolo Bonzini
Date: Thu Oct 10 2024 - 17:24:22 EST


On 10/10/24 20:09, Vishal Chourasia wrote:
Hi,

While building the kernel with CONFIG_PREEMPT_RT, I encountered several
compilation errors in the PowerPC KVM code. The issues appear in
book3s_hv_rm_mmu.c where it tries to access the 'rlock' member of struct
spinlock, which doesn't exist in the RT configuration.

How was this tested? I suspect that putting to sleep a task that is running in real mode is a huge no-no. The actual solution would have to be to split mmu_lock into a spin_lock and a raw_spin_lock, but that's a huge amount of work probably. I'd just add a "depends on !PPC || !KVM_BOOK3S_64_HV" or something like that, to prevent enabling KVM-HV on PREEMPT_RT kernels.

Thanks,

Paolo

arch/powerpc/kvm/book3s_hv_rm_mmu.c:248:32: error: no member named 'rlock' in 'struct spinlock'
248 | arch_spin_lock(&kvm->mmu_lock.rlock.raw_lock);
| ~~~~~~~~~~~~~ ^
./arch/powerpc/include/asm/qspinlock.h:164:45: note: expanded from macro 'arch_spin_lock'
164 | #define arch_spin_lock(l) queued_spin_lock(l)
| ^
arch/powerpc/kvm/book3s_hv_rm_mmu.c:263:36: error: no member named 'rlock' in 'struct spinlock'
263 | arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
| ~~~~~~~~~~~~~ ^
./arch/powerpc/include/asm/qspinlock.h:166:49: note: expanded from macro 'arch_spin_unlock'
166 | #define arch_spin_unlock(l) queued_spin_unlock(l)
| ^
arch/powerpc/kvm/book3s_hv_rm_mmu.c:277:34: error: no member named 'rlock' in 'struct spinlock'
277 | arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
| ~~~~~~~~~~~~~ ^
./arch/powerpc/include/asm/qspinlock.h:166:49: note: expanded from macro 'arch_spin_unlock'
166 | #define arch_spin_unlock(l) queued_spin_unlock(l)
| ^
arch/powerpc/kvm/book3s_hv_rm_mmu.c:938:32: error: no member named 'rlock' in 'struct spinlock'
938 | arch_spin_lock(&kvm->mmu_lock.rlock.raw_lock);
| ~~~~~~~~~~~~~ ^
./arch/powerpc/include/asm/qspinlock.h:164:45: note: expanded from macro 'arch_spin_lock'
164 | #define arch_spin_lock(l) queued_spin_lock(l)
| ^
arch/powerpc/kvm/book3s_hv_rm_mmu.c:950:34: error: no member named 'rlock' in 'struct spinlock'
950 | arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
| ~~~~~~~~~~~~~ ^
./arch/powerpc/include/asm/qspinlock.h:166:49: note: expanded from macro 'arch_spin_unlock'
166 | #define arch_spin_unlock(l) queued_spin_unlock(l)
| ^
arch/powerpc/kvm/book3s_hv_rm_mmu.c:966:32: error: no member named 'rlock' in 'struct spinlock'
966 | arch_spin_lock(&kvm->mmu_lock.rlock.raw_lock);
| ~~~~~~~~~~~~~ ^
./arch/powerpc/include/asm/qspinlock.h:164:45: note: expanded from macro 'arch_spin_lock'
164 | #define arch_spin_lock(l) queued_spin_lock(l)
| ^
arch/powerpc/kvm/book3s_hv_rm_mmu.c:981:34: error: no member named 'rlock' in 'struct spinlock'
981 | arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
| ~~~~~~~~~~~~~ ^
./arch/powerpc/include/asm/qspinlock.h:166:49: note: expanded from macro 'arch_spin_unlock'
166 | #define arch_spin_unlock(l) queued_spin_unlock(l)
| ^
7 errors generated.
make[4]: *** [scripts/Makefile.build:229: arch/powerpc/kvm/book3s_hv_rm_mmu.o] Error 1
make[3]: *** [scripts/Makefile.build:478: arch/powerpc/kvm] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:478: arch/powerpc] Error 2
make[2]: *** Waiting for unfinished jobs....

Replace arch_spin_lock/unlock on kvm->mmu_lock.rlock.raw_lock with
simple spin_lock/unlock on kvm->mmu_lock to fix build errors with
CONFIG_PREEMPT_RT. The RT configuration changes the spinlock structure,
removing the rlock member.

Signed-off-by: Vishal Chourasia <vishalc@xxxxxxxxxxxxx>
---
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index 17cb75a127b04..abf1e6de85644 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
@@ -245,7 +245,7 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long flags,
/* Translate to host virtual address */
hva = __gfn_to_hva_memslot(memslot, gfn);

- arch_spin_lock(&kvm->mmu_lock.rlock.raw_lock);
+ spin_lock(&kvm->mmu_lock);
ptep = find_kvm_host_pte(kvm, mmu_seq, hva, &hpage_shift);
if (ptep) {
pte_t pte;
@@ -260,7 +260,7 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long flags,
* to <= host page size, if host is using hugepage
*/
if (host_pte_size < psize) {
- arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
+ spin_unlock(&kvm->mmu_lock);
return H_PARAMETER;
}
pte = kvmppc_read_update_linux_pte(ptep, writing);
@@ -274,7 +274,7 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long flags,
pa |= gpa & ~PAGE_MASK;
}
}
- arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
+ spin_unlock(&kvm->mmu_lock);

ptel &= HPTE_R_KEY | HPTE_R_PP0 | (psize-1);
ptel |= pa;
@@ -935,7 +935,7 @@ static long kvmppc_do_h_page_init_zero(struct kvm_vcpu *vcpu,
mmu_seq = kvm->mmu_invalidate_seq;
smp_rmb();

- arch_spin_lock(&kvm->mmu_lock.rlock.raw_lock);
+ spin_lock(&kvm->mmu_lock);

ret = kvmppc_get_hpa(vcpu, mmu_seq, dest, 1, &pa, &memslot);
if (ret != H_SUCCESS)
@@ -947,7 +947,7 @@ static long kvmppc_do_h_page_init_zero(struct kvm_vcpu *vcpu,
kvmppc_update_dirty_map(memslot, dest >> PAGE_SHIFT, PAGE_SIZE);

out_unlock:
- arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
+ spin_unlock(&kvm->mmu_lock);
return ret;
}

@@ -963,7 +963,7 @@ static long kvmppc_do_h_page_init_copy(struct kvm_vcpu *vcpu,
mmu_seq = kvm->mmu_invalidate_seq;
smp_rmb();

- arch_spin_lock(&kvm->mmu_lock.rlock.raw_lock);
+ spin_lock(&kvm->mmu_lock);
ret = kvmppc_get_hpa(vcpu, mmu_seq, dest, 1, &dest_pa, &dest_memslot);
if (ret != H_SUCCESS)
goto out_unlock;
@@ -978,7 +978,7 @@ static long kvmppc_do_h_page_init_copy(struct kvm_vcpu *vcpu,
kvmppc_update_dirty_map(dest_memslot, dest >> PAGE_SHIFT, PAGE_SIZE);

out_unlock:
- arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
+ spin_unlock(&kvm->mmu_lock);
return ret;
}

--
2.46.2