The kvm_riscv_vcpu_aia_imsic_put() runs from the scheduler preemption notifier with the raw runqueue lock held, so it must not take vsfile_lock, an rwlock_t, which sleeps on PREEMPT_RT. All vsfile_lock critical sections are short, so a raw spinlock is fine. In addition, converting vsfile_lock to raw spinlock allows us to drop the vsfile_hgei_lock and also allows us to use writel_relaxed() in kvm_riscv_vcpu_aia_imsic_inject() since raw_spin_unlock() internally does mmiowb_spin_unlock() which in-turn drains the write-buffer. Co-developed-by: Andrew Jones Signed-off-by: Andrew Jones Signed-off-by: Anup Patel --- arch/riscv/kvm/aia_imsic.c | 79 ++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c index 302938972aa0..0b9d15721d52 100644 --- a/arch/riscv/kvm/aia_imsic.c +++ b/arch/riscv/kvm/aia_imsic.c @@ -47,9 +47,7 @@ struct imsic { */ /* IMSIC VS-file */ - rwlock_t vsfile_lock; - /* Writers hold both locks; scheduler-out only takes vsfile_hgei_lock. */ - raw_spinlock_t vsfile_hgei_lock; + raw_spinlock_t vsfile_lock; int vsfile_cpu; int vsfile_hgei; void __iomem *vsfile_va; @@ -490,17 +488,6 @@ static void imsic_vsfile_local_update(int vsfile_hgei, u32 nr_eix, csr_write(CSR_VSISELECT, old_vsiselect); } -static void imsic_vsfile_set_hgei(struct imsic *imsic, int cpu, int hgei) -{ - unsigned long flags; - - /* Serialize the CPU/HGEI pair with scheduler-out's wakeup setup. */ - raw_spin_lock_irqsave(&imsic->vsfile_hgei_lock, flags); - imsic->vsfile_cpu = cpu; - imsic->vsfile_hgei = hgei; - raw_spin_unlock_irqrestore(&imsic->vsfile_hgei_lock, flags); -} - static void imsic_vsfile_cleanup(struct imsic *imsic) { int old_vsfile_hgei, old_vsfile_cpu; @@ -512,13 +499,14 @@ static void imsic_vsfile_cleanup(struct imsic *imsic) * VCPU is being destroyed. */ - write_lock_irqsave(&imsic->vsfile_lock, flags); + raw_spin_lock_irqsave(&imsic->vsfile_lock, flags); old_vsfile_hgei = imsic->vsfile_hgei; old_vsfile_cpu = imsic->vsfile_cpu; - imsic_vsfile_set_hgei(imsic, -1, -1); + imsic->vsfile_cpu = -1; + imsic->vsfile_hgei = -1; imsic->vsfile_va = NULL; imsic->vsfile_pa = 0; - write_unlock_irqrestore(&imsic->vsfile_lock, flags); + raw_spin_unlock_irqrestore(&imsic->vsfile_lock, flags); memset(imsic->swfile, 0, sizeof(*imsic->swfile)); @@ -606,7 +594,7 @@ bool kvm_riscv_vcpu_aia_imsic_has_interrupt(struct kvm_vcpu *vcpu) * only check for interrupt when IMSIC VS-file is being used. */ - read_lock_irqsave(&imsic->vsfile_lock, flags); + raw_spin_lock_irqsave(&imsic->vsfile_lock, flags); if (imsic->vsfile_cpu > -1) { /* * This function is typically called from kvm_vcpu_block() via @@ -621,7 +609,7 @@ bool kvm_riscv_vcpu_aia_imsic_has_interrupt(struct kvm_vcpu *vcpu) else ret = !!(csr_read(CSR_HGEIP) & BIT(imsic->vsfile_hgei)); } - read_unlock_irqrestore(&imsic->vsfile_lock, flags); + raw_spin_unlock_irqrestore(&imsic->vsfile_lock, flags); return ret; } @@ -646,10 +634,10 @@ void kvm_riscv_vcpu_aia_imsic_put(struct kvm_vcpu *vcpu) if (!kvm_vcpu_is_blocking(vcpu)) return; - raw_spin_lock_irqsave(&imsic->vsfile_hgei_lock, flags); + raw_spin_lock_irqsave(&imsic->vsfile_lock, flags); if (imsic->vsfile_cpu > -1) csr_set(CSR_HGEIE, BIT(imsic->vsfile_hgei)); - raw_spin_unlock_irqrestore(&imsic->vsfile_hgei_lock, flags); + raw_spin_unlock_irqrestore(&imsic->vsfile_lock, flags); } void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu) @@ -663,13 +651,14 @@ void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu) return; /* Read and clear IMSIC VS-file details */ - write_lock_irqsave(&imsic->vsfile_lock, flags); + raw_spin_lock_irqsave(&imsic->vsfile_lock, flags); old_vsfile_hgei = imsic->vsfile_hgei; old_vsfile_cpu = imsic->vsfile_cpu; - imsic_vsfile_set_hgei(imsic, -1, -1); + imsic->vsfile_cpu = -1; + imsic->vsfile_hgei = -1; imsic->vsfile_va = NULL; imsic->vsfile_pa = 0; - write_unlock_irqrestore(&imsic->vsfile_lock, flags); + raw_spin_unlock_irqrestore(&imsic->vsfile_lock, flags); /* Do nothing, if no IMSIC VS-file to release */ if (old_vsfile_cpu < 0) @@ -726,10 +715,10 @@ int kvm_riscv_vcpu_aia_imsic_update(struct kvm_vcpu *vcpu) return 1; /* Read old IMSIC VS-file details */ - read_lock_irqsave(&imsic->vsfile_lock, flags); + raw_spin_lock_irqsave(&imsic->vsfile_lock, flags); old_vsfile_hgei = imsic->vsfile_hgei; old_vsfile_cpu = imsic->vsfile_cpu; - read_unlock_irqrestore(&imsic->vsfile_lock, flags); + raw_spin_unlock_irqrestore(&imsic->vsfile_lock, flags); /* Do nothing if we are continuing on same CPU */ if (old_vsfile_cpu == vcpu->cpu) @@ -779,11 +768,12 @@ int kvm_riscv_vcpu_aia_imsic_update(struct kvm_vcpu *vcpu) /* TODO: Update the IOMMU mapping ??? */ /* Update new IMSIC VS-file details in IMSIC context */ - write_lock_irqsave(&imsic->vsfile_lock, flags); - imsic_vsfile_set_hgei(imsic, vcpu->cpu, new_vsfile_hgei); + raw_spin_lock_irqsave(&imsic->vsfile_lock, flags); + imsic->vsfile_cpu = vcpu->cpu; + imsic->vsfile_hgei = new_vsfile_hgei; imsic->vsfile_va = new_vsfile_va; imsic->vsfile_pa = new_vsfile_pa; - write_unlock_irqrestore(&imsic->vsfile_lock, flags); + raw_spin_unlock_irqrestore(&imsic->vsfile_lock, flags); /* * At this point, all interrupt producers have been moved @@ -874,6 +864,7 @@ int kvm_riscv_aia_imsic_rw_attr(struct kvm *kvm, unsigned long type, struct imsic *imsic; struct kvm_vcpu *vcpu; int rc, vsfile_hgei, vsfile_cpu; + bool swfile_update = false; if (!kvm_riscv_aia_initialized(kvm)) return -ENODEV; @@ -893,7 +884,7 @@ int kvm_riscv_aia_imsic_rw_attr(struct kvm *kvm, unsigned long type, } isel = KVM_DEV_RISCV_AIA_IMSIC_GET_ISEL(type); - read_lock_irqsave(&imsic->vsfile_lock, flags); + raw_spin_lock_irqsave(&imsic->vsfile_lock, flags); rc = 0; vsfile_hgei = imsic->vsfile_hgei; @@ -902,13 +893,16 @@ int kvm_riscv_aia_imsic_rw_attr(struct kvm *kvm, unsigned long type, if (write) { rc = imsic_mrif_rmw(imsic->swfile, imsic->nr_eix, isel, NULL, *val, -1UL); - imsic_swfile_extirq_update(vcpu); + swfile_update = true; } else rc = imsic_mrif_rmw(imsic->swfile, imsic->nr_eix, isel, val, 0, 0); } - read_unlock_irqrestore(&imsic->vsfile_lock, flags); + raw_spin_unlock_irqrestore(&imsic->vsfile_lock, flags); + + if (swfile_update) + imsic_swfile_extirq_update(vcpu); if (!rc && vsfile_cpu >= 0) rc = imsic_vsfile_rw(vsfile_hgei, vsfile_cpu, imsic->nr_eix, @@ -959,6 +953,7 @@ int kvm_riscv_vcpu_aia_imsic_inject(struct kvm_vcpu *vcpu, unsigned long flags; struct imsic_mrif_eix *eix; struct imsic *imsic = vcpu->arch.aia_context.imsic_state; + bool swfile_update = false; /* We only emulate one IMSIC MMIO page for each Guest VCPU */ if (!imsic || !iid || guest_index || @@ -970,17 +965,26 @@ int kvm_riscv_vcpu_aia_imsic_inject(struct kvm_vcpu *vcpu, if (imsic->nr_msis <= iid) return -EINVAL; - read_lock_irqsave(&imsic->vsfile_lock, flags); + raw_spin_lock_irqsave(&imsic->vsfile_lock, flags); if (imsic->vsfile_cpu >= 0) { - writel(iid, imsic->vsfile_va + IMSIC_MMIO_SETIPNUM_LE); + /* + * We can use writel_relaxed() over here because the + * raw_spin_unlock() below will do mmiowb_spin_unlock() + * which in-turn drains the write-buffer ensuring the + * write is visible to the CPU owning the IMSIC VS-file. + */ + writel_relaxed(iid, imsic->vsfile_va + IMSIC_MMIO_SETIPNUM_LE); } else { eix = &imsic->swfile->eix[iid / BITS_PER_TYPE(u64)]; set_bit(iid & (BITS_PER_TYPE(u64) - 1), eix->eip); - imsic_swfile_extirq_update(vcpu); + swfile_update = true; } - read_unlock_irqrestore(&imsic->vsfile_lock, flags); + raw_spin_unlock_irqrestore(&imsic->vsfile_lock, flags); + + if (swfile_update) + imsic_swfile_extirq_update(vcpu); return 0; } @@ -1036,8 +1040,7 @@ int kvm_riscv_vcpu_aia_imsic_init(struct kvm_vcpu *vcpu) /* Setup IMSIC context */ imsic->nr_msis = kvm->arch.aia.nr_ids + 1; - rwlock_init(&imsic->vsfile_lock); - raw_spin_lock_init(&imsic->vsfile_hgei_lock); + raw_spin_lock_init(&imsic->vsfile_lock); imsic->nr_eix = BITS_TO_U64(imsic->nr_msis); imsic->nr_hw_eix = BITS_TO_U64(kvm_riscv_aia_max_ids); imsic->vsfile_hgei = imsic->vsfile_cpu = -1; -- 2.43.0