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

[Paolo Bonzini <pbonzini@redhat.com>]

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@linux.ibm.com>
> ---
>   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
> 
>