[PATCH 0/4] KVM/ARM fixes for 4.13-rc4

[PATCH 0/4] KVM/ARM fixes for 4.13-rc4

[Marc Zyngier]

Paolo, Radim,

Here's a pull request for a few KVM/ARM updates. Nothing major except
for a VM destruction race (Suzuki has a knack for finding those...).

Please pull.

	M.

The following changes since commit 5771a8c08880cdca3bfb4a3fc6d309d6bba20877:

  Linux v4.13-rc1 (2017-07-15 15:22:10 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git tags/kvm-arm-for-v4.13-rc4

for you to fetch changes up to 3af4e414af5ce014d1f88cd816d997048298b310:

  KVM: arm/arm64: vgic: Use READ_ONCE fo cmpxchg (2017-08-03 15:47:36 +0100)

----------------------------------------------------------------
KVM/ARM Fixes for v4.13-rc4

- Yet another race with VM destruction plugged
- A set of small vgic fixes

----------------------------------------------------------------
Andrew Jones (1):
      KVM: arm/arm64: PMU: Fix overflow interrupt injection

Christoffer Dall (1):
      KVM: arm/arm64: vgic: Use READ_ONCE fo cmpxchg

Shanker Donthineni (1):
      KVM: arm/arm64: Fix bug in advertising KVM_CAP_MSI_DEVID capability

Suzuki K Poulose (1):
      KVM: arm/arm64: Handle hva aging while destroying the vm

 arch/arm64/kvm/sys_regs.c        |  2 +-
 include/kvm/arm_pmu.h            |  2 --
 virt/kvm/arm/mmu.c               |  4 ++++
 virt/kvm/arm/pmu.c               | 43 ++++++++++++++--------------------------
 virt/kvm/arm/vgic/vgic-init.c    |  3 ---
 virt/kvm/arm/vgic/vgic-its.c     |  1 +
 virt/kvm/arm/vgic/vgic-mmio-v3.c |  4 ++--
 7 files changed, 23 insertions(+), 36 deletions(-)

[PATCH 1/4] KVM: arm/arm64: Fix bug in advertising KVM_CAP_MSI_DEVID capability

[Marc Zyngier]

From: Shanker Donthineni <shankerd@codeaurora.org>

Commit 0e4e82f154e3 ("KVM: arm64: vgic-its: Enable ITS emulation as
a virtual MSI controller") tried to advertise KVM_CAP_MSI_DEVID, but
the code logic was not updating the dist->msis_require_devid field
correctly. If hypervisor tool creates the ITS device after VGIC
initialization then we don't advertise KVM_CAP_MSI_DEVID capability.

Update the field msis_require_devid to true inside vgic_its_create()
to fix the issue.

Fixes: 0e4e82f154e3 ("vgic-its: Enable ITS emulation as a virtual MSI controller")
Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 virt/kvm/arm/vgic/vgic-init.c | 3 ---
 virt/kvm/arm/vgic/vgic-its.c  | 1 +
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 3a0b8999f011..5801261f3add 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -285,9 +285,6 @@ int vgic_init(struct kvm *kvm)
 	if (ret)
 		goto out;
 
-	if (vgic_has_its(kvm))
-		dist->msis_require_devid = true;
-
 	kvm_for_each_vcpu(i, vcpu, kvm)
 		kvm_vgic_vcpu_enable(vcpu);
 
diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index 2dff288b3a66..aa6b68db80b4 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -1598,6 +1598,7 @@ static int vgic_its_create(struct kvm_device *dev, u32 type)
 	INIT_LIST_HEAD(&its->device_list);
 	INIT_LIST_HEAD(&its->collection_list);
 
+	dev->kvm->arch.vgic.msis_require_devid = true;
 	dev->kvm->arch.vgic.has_its = true;
 	its->enabled = false;
 	its->dev = dev;
-- 
2.11.0

[PATCH 2/4] KVM: arm/arm64: PMU: Fix overflow interrupt injection

[Marc Zyngier]

From: Andrew Jones <drjones@redhat.com>

kvm_pmu_overflow_set() is called from perf's interrupt handler,
making the call of kvm_vgic_inject_irq() from it introduced with
"KVM: arm/arm64: PMU: remove request-less vcpu kick" a really bad
idea, as it's quite easy to try and retake a lock that the
interrupted context is already holding. The fix is to use a vcpu
kick, leaving the interrupt injection to kvm_pmu_sync_hwstate(),
like it was doing before the refactoring. We don't just revert,
though, because before the kick was request-less, leaving the vcpu
exposed to the request-less vcpu kick race, and also because the
kick was used unnecessarily from register access handlers.

Reviewed-by: Christoffer Dall <cdall@linaro.org>
Signed-off-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/sys_regs.c |  2 +-
 include/kvm/arm_pmu.h     |  2 --
 virt/kvm/arm/pmu.c        | 43 +++++++++++++++----------------------------
 3 files changed, 16 insertions(+), 31 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 77862881ae86..2e070d3baf9f 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -764,7 +764,7 @@ static bool access_pmovs(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 	if (p->is_write) {
 		if (r->CRm & 0x2)
 			/* accessing PMOVSSET_EL0 */
-			kvm_pmu_overflow_set(vcpu, p->regval & mask);
+			vcpu_sys_reg(vcpu, PMOVSSET_EL0) |= (p->regval & mask);
 		else
 			/* accessing PMOVSCLR_EL0 */
 			vcpu_sys_reg(vcpu, PMOVSSET_EL0) &= ~(p->regval & mask);
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index f6e030617467..f87fe20fcb05 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -48,7 +48,6 @@ void kvm_pmu_vcpu_reset(struct kvm_vcpu *vcpu);
 void kvm_pmu_vcpu_destroy(struct kvm_vcpu *vcpu);
 void kvm_pmu_disable_counter(struct kvm_vcpu *vcpu, u64 val);
 void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64 val);
-void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val);
 void kvm_pmu_flush_hwstate(struct kvm_vcpu *vcpu);
 void kvm_pmu_sync_hwstate(struct kvm_vcpu *vcpu);
 bool kvm_pmu_should_notify_user(struct kvm_vcpu *vcpu);
@@ -86,7 +85,6 @@ static inline void kvm_pmu_vcpu_reset(struct kvm_vcpu *vcpu) {}
 static inline void kvm_pmu_vcpu_destroy(struct kvm_vcpu *vcpu) {}
 static inline void kvm_pmu_disable_counter(struct kvm_vcpu *vcpu, u64 val) {}
 static inline void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64 val) {}
-static inline void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val) {}
 static inline void kvm_pmu_flush_hwstate(struct kvm_vcpu *vcpu) {}
 static inline void kvm_pmu_sync_hwstate(struct kvm_vcpu *vcpu) {}
 static inline bool kvm_pmu_should_notify_user(struct kvm_vcpu *vcpu)
diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index fc8a723ff387..8a9c42366db7 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c
@@ -203,11 +203,15 @@ static u64 kvm_pmu_overflow_status(struct kvm_vcpu *vcpu)
 	return reg;
 }
 
-static void kvm_pmu_check_overflow(struct kvm_vcpu *vcpu)
+static void kvm_pmu_update_state(struct kvm_vcpu *vcpu)
 {
 	struct kvm_pmu *pmu = &vcpu->arch.pmu;
-	bool overflow = !!kvm_pmu_overflow_status(vcpu);
+	bool overflow;
+
+	if (!kvm_arm_pmu_v3_ready(vcpu))
+		return;
 
+	overflow = !!kvm_pmu_overflow_status(vcpu);
 	if (pmu->irq_level == overflow)
 		return;
 
@@ -215,33 +219,11 @@ static void kvm_pmu_check_overflow(struct kvm_vcpu *vcpu)
 
 	if (likely(irqchip_in_kernel(vcpu->kvm))) {
 		int ret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id,
-					      pmu->irq_num, overflow,
-					      &vcpu->arch.pmu);
+					      pmu->irq_num, overflow, pmu);
 		WARN_ON(ret);
 	}
 }
 
-/**
- * kvm_pmu_overflow_set - set PMU overflow interrupt
- * @vcpu: The vcpu pointer
- * @val: the value guest writes to PMOVSSET register
- */
-void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val)
-{
-	if (val == 0)
-		return;
-
-	vcpu_sys_reg(vcpu, PMOVSSET_EL0) |= val;
-	kvm_pmu_check_overflow(vcpu);
-}
-
-static void kvm_pmu_update_state(struct kvm_vcpu *vcpu)
-{
-	if (!kvm_arm_pmu_v3_ready(vcpu))
-		return;
-	kvm_pmu_check_overflow(vcpu);
-}
-
 bool kvm_pmu_should_notify_user(struct kvm_vcpu *vcpu)
 {
 	struct kvm_pmu *pmu = &vcpu->arch.pmu;
@@ -303,7 +285,7 @@ static inline struct kvm_vcpu *kvm_pmc_to_vcpu(struct kvm_pmc *pmc)
 }
 
 /**
- * When perf event overflows, call kvm_pmu_overflow_set to set overflow status.
+ * When the perf event overflows, set the overflow status and inform the vcpu.
  */
 static void kvm_pmu_perf_overflow(struct perf_event *perf_event,
 				  struct perf_sample_data *data,
@@ -313,7 +295,12 @@ static void kvm_pmu_perf_overflow(struct perf_event *perf_event,
 	struct kvm_vcpu *vcpu = kvm_pmc_to_vcpu(pmc);
 	int idx = pmc->idx;
 
-	kvm_pmu_overflow_set(vcpu, BIT(idx));
+	vcpu_sys_reg(vcpu, PMOVSSET_EL0) |= BIT(idx);
+
+	if (kvm_pmu_overflow_status(vcpu)) {
+		kvm_make_request(KVM_REQ_IRQ_PENDING, vcpu);
+		kvm_vcpu_kick(vcpu);
+	}
 }
 
 /**
@@ -341,7 +328,7 @@ void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val)
 			reg = lower_32_bits(reg);
 			vcpu_sys_reg(vcpu, PMEVCNTR0_EL0 + i) = reg;
 			if (!reg)
-				kvm_pmu_overflow_set(vcpu, BIT(i));
+				vcpu_sys_reg(vcpu, PMOVSSET_EL0) |= BIT(i);
 		}
 	}
 }
-- 
2.11.0

[PATCH 3/4] KVM: arm/arm64: Handle hva aging while destroying the vm

[Marc Zyngier]

From: Suzuki K Poulose <Suzuki.Poulose@arm.com>

The mmu_notifier_release() callback of KVM triggers cleaning up
the stage2 page table on kvm-arm. However there could be other
notifier callbacks in parallel with the mmu_notifier_release(),
which could cause the call backs ending up in an empty stage2
page table. Make sure we check it for all the notifier callbacks.

Cc: stable@vger.kernel.org
Fixes: commit 293f29363 ("kvm-arm: Unmap shadow pagetables properly")
Reported-by: Alex Graf <agraf@suse.de>
Reviewed-by: Christoffer Dall <cdall@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 virt/kvm/arm/mmu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 0e1fc75f3585..2ea21dac0b44 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1718,12 +1718,16 @@ static int kvm_test_age_hva_handler(struct kvm *kvm, gpa_t gpa, u64 size, void *
 
 int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end)
 {
+	if (!kvm->arch.pgd)
+		return 0;
 	trace_kvm_age_hva(start, end);
 	return handle_hva_to_gpa(kvm, start, end, kvm_age_hva_handler, NULL);
 }
 
 int kvm_test_age_hva(struct kvm *kvm, unsigned long hva)
 {
+	if (!kvm->arch.pgd)
+		return 0;
 	trace_kvm_test_age_hva(hva);
 	return handle_hva_to_gpa(kvm, hva, hva, kvm_test_age_hva_handler, NULL);
 }
-- 
2.11.0

[PATCH 4/4] KVM: arm/arm64: vgic: Use READ_ONCE fo cmpxchg

[Marc Zyngier]

From: Christoffer Dall <cdall@linaro.org>

There is a small chance that the compiler could generate separate loads
for the dist->propbaser which could be modified from another CPU.  As we
want to make sure we atomically update the entire value, and don't race
with other updates, guarantee that the cmpxchg operation compares
against the original value.

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Christoffer Dall <cdall@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 virt/kvm/arm/vgic/vgic-mmio-v3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index 714fa3933546..408ef06638fc 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -369,7 +369,7 @@ static void vgic_mmio_write_propbase(struct kvm_vcpu *vcpu,
 		return;
 
 	do {
-		old_propbaser = dist->propbaser;
+		old_propbaser = READ_ONCE(dist->propbaser);
 		propbaser = old_propbaser;
 		propbaser = update_64bit_reg(propbaser, addr & 4, len, val);
 		propbaser = vgic_sanitise_propbaser(propbaser);
@@ -397,7 +397,7 @@ static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
 		return;
 
 	do {
-		old_pendbaser = vgic_cpu->pendbaser;
+		old_pendbaser = READ_ONCE(vgic_cpu->pendbaser);
 		pendbaser = old_pendbaser;
 		pendbaser = update_64bit_reg(pendbaser, addr & 4, len, val);
 		pendbaser = vgic_sanitise_pendbaser(pendbaser);
-- 
2.11.0

Re: [PATCH 0/4] KVM/ARM fixes for 4.13-rc4

[Radim Krčmář]

2017-08-03 16:28+0100, Marc Zyngier:
> Paolo, Radim,
> 
> Here's a pull request for a few KVM/ARM updates. Nothing major except
> for a VM destruction race (Suzuki has a knack for finding those...).
> 
> Please pull.

Pulled, thanks.