powerpc/kvm: Fix spinlock member access for PREEMPT_RT

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.

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 <[email protected]>

---
  arch/powerpc/kvm/book3s_hv_rm_mmu.c | 14 +++++++-------
  1 file changed, 7 insertions(+), 7 deletions(-)

arch/powerpc/kvm/book3s_hv_rm_mmu.c +7 -8

index 17cb75a127b04..abf1e6de85644 100644
@@ -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

Quoted 6 lines · Vishal Chourasia

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.

Quoted 6 lines · Vishal Chourasia

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.

arch/powerpc/Kconfig +1 -0

index 8094a01974cca..568dc856f0dfa 100644
@@ -168,6 +168,7 @@ config PPC
        select ARCH_STACKWALK
        select ARCH_SUPPORTS_ATOMIC_RMW
        select ARCH_SUPPORTS_DEBUG_PAGEALLOC    if PPC_BOOK3S || PPC_8xx
+       select ARCH_SUPPORTS_RT                 if !PPC || !KVM_BOOK3S_64_HV
        select ARCH_USE_BUILTIN_BSWAP
        select ARCH_USE_CMPXCHG_LOCKREF         if PPC64
        select ARCH_USE_MEMTEST

I tried rebuilding with the above diff as per your suggestion though it works when KVM_BOOK3S_64_HV is set to N, but for pseries_le_defconfig, it's set to M, by default, which then requires setting it to N explicitly.

Will something like below be a better solution? This will set KVM_BOOK3S_64_HV to N if ARCH_SUPPORTS_RT is set.

arch/powerpc/kvm/Kconfig +0 -0

index dbfdc126bf144..33e0d50b08b14 100644