KVM Archive on lore.kernel.org
 help / color / Atom feed
* [GIT PULL] KVM/arm updates for 5.3-rc6
@ 2019-08-23 16:35 Marc Zyngier
  2019-08-23 16:35 ` [PATCH 1/2] KVM: arm/arm64: Only skip MMIO insn once Marc Zyngier
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Marc Zyngier @ 2019-08-23 16:35 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Andre Przywara, Andrew Jones, Dave Martin, Julien Grall,
	Mark Rutland, Julien Thierry, James Morse, Suzuki K Poulose,
	linux-arm-kernel, kvm, kvmarm

Paolo, Radim,

One (hopefully last) set of fixes for KVM/arm for 5.3: an embarassing
MMIO emulation regression, and a UBSAN splat. Oh well...

Please pull,

       M.

The following changes since commit 16e604a437c89751dc626c9e90cf88ba93c5be64:

  KVM: arm/arm64: vgic: Reevaluate level sensitive interrupts on enable (2019-08-09 08:07:26 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git tags/kvmarm-fixes-for-5.3-3

for you to fetch changes up to 2e16f3e926ed48373c98edea85c6ad0ef69425d1:

  KVM: arm/arm64: VGIC: Properly initialise private IRQ affinity (2019-08-23 17:23:01 +0100)

----------------------------------------------------------------
KVM/arm fixes for 5.3, take #3

- Don't overskip instructions on MMIO emulation
- Fix UBSAN splat when initializing PPI priorities

----------------------------------------------------------------
Andre Przywara (1):
      KVM: arm/arm64: VGIC: Properly initialise private IRQ affinity

Andrew Jones (1):
      KVM: arm/arm64: Only skip MMIO insn once

 virt/kvm/arm/mmio.c           |  7 +++++++
 virt/kvm/arm/vgic/vgic-init.c | 30 ++++++++++++++++++++----------
 2 files changed, 27 insertions(+), 10 deletions(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] KVM: arm/arm64: Only skip MMIO insn once
  2019-08-23 16:35 [GIT PULL] KVM/arm updates for 5.3-rc6 Marc Zyngier
@ 2019-08-23 16:35 ` Marc Zyngier
  2019-08-23 16:35 ` [PATCH 2/2] KVM: arm/arm64: VGIC: Properly initialise private IRQ affinity Marc Zyngier
  2019-08-23 17:07 ` [GIT PULL] KVM/arm updates for 5.3-rc6 Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2019-08-23 16:35 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Andre Przywara, Andrew Jones, Dave Martin, Julien Grall,
	Mark Rutland, Julien Thierry, James Morse, Suzuki K Poulose,
	linux-arm-kernel, kvm, kvmarm

From: Andrew Jones <drjones@redhat.com>

If after an MMIO exit to userspace a VCPU is immediately run with an
immediate_exit request, such as when a signal is delivered or an MMIO
emulation completion is needed, then the VCPU completes the MMIO
emulation and immediately returns to userspace. As the exit_reason
does not get changed from KVM_EXIT_MMIO in these cases we have to
be careful not to complete the MMIO emulation again, when the VCPU is
eventually run again, because the emulation does an instruction skip
(and doing too many skips would be a waste of guest code :-) We need
to use additional VCPU state to track if the emulation is complete.
As luck would have it, we already have 'mmio_needed', which even
appears to be used in this way by other architectures already.

Fixes: 0d640732dbeb ("arm64: KVM: Skip MMIO insn after emulation")
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 virt/kvm/arm/mmio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/virt/kvm/arm/mmio.c b/virt/kvm/arm/mmio.c
index a8a6a0c883f1..6af5c91337f2 100644
--- a/virt/kvm/arm/mmio.c
+++ b/virt/kvm/arm/mmio.c
@@ -86,6 +86,12 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	unsigned int len;
 	int mask;
 
+	/* Detect an already handled MMIO return */
+	if (unlikely(!vcpu->mmio_needed))
+		return 0;
+
+	vcpu->mmio_needed = 0;
+
 	if (!run->mmio.is_write) {
 		len = run->mmio.len;
 		if (len > sizeof(unsigned long))
@@ -188,6 +194,7 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	run->mmio.is_write	= is_write;
 	run->mmio.phys_addr	= fault_ipa;
 	run->mmio.len		= len;
+	vcpu->mmio_needed	= 1;
 
 	if (!ret) {
 		/* We handled the access successfully in the kernel. */
-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] KVM: arm/arm64: VGIC: Properly initialise private IRQ affinity
  2019-08-23 16:35 [GIT PULL] KVM/arm updates for 5.3-rc6 Marc Zyngier
  2019-08-23 16:35 ` [PATCH 1/2] KVM: arm/arm64: Only skip MMIO insn once Marc Zyngier
@ 2019-08-23 16:35 ` Marc Zyngier
  2019-08-23 17:07 ` [GIT PULL] KVM/arm updates for 5.3-rc6 Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2019-08-23 16:35 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Andre Przywara, Andrew Jones, Dave Martin, Julien Grall,
	Mark Rutland, Julien Thierry, James Morse, Suzuki K Poulose,
	linux-arm-kernel, kvm, kvmarm

From: Andre Przywara <andre.przywara@arm.com>

At the moment we initialise the target *mask* of a virtual IRQ to the
VCPU it belongs to, even though this mask is only defined for GICv2 and
quickly runs out of bits for many GICv3 guests.
This behaviour triggers an UBSAN complaint for more than 32 VCPUs:
------
[ 5659.462377] UBSAN: Undefined behaviour in virt/kvm/arm/vgic/vgic-init.c:223:21
[ 5659.471689] shift exponent 32 is too large for 32-bit type 'unsigned int'
------
Also for GICv3 guests the reporting of TARGET in the "vgic-state" debugfs
dump is wrong, due to this very same problem.

Because there is no requirement to create the VGIC device before the
VCPUs (and QEMU actually does it the other way round), we can't safely
initialise mpidr or targets in kvm_vgic_vcpu_init(). But since we touch
every private IRQ for each VCPU anyway later (in vgic_init()), we can
just move the initialisation of those fields into there, where we
definitely know the VGIC type.

On the way make sure we really have either a VGICv2 or a VGICv3 device,
since the existing code is just checking for "VGICv3 or not", silently
ignoring the uninitialised case.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reported-by: Dave Martin <dave.martin@arm.com>
Tested-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 virt/kvm/arm/vgic/vgic-init.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index bdbc297d06fb..e621b5d45b27 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -8,6 +8,7 @@
 #include <linux/cpu.h>
 #include <linux/kvm_host.h>
 #include <kvm/arm_vgic.h>
+#include <asm/kvm_emulate.h>
 #include <asm/kvm_mmu.h>
 #include "vgic.h"
 
@@ -164,12 +165,18 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
 		irq->vcpu = NULL;
 		irq->target_vcpu = vcpu0;
 		kref_init(&irq->refcount);
-		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) {
+		switch (dist->vgic_model) {
+		case KVM_DEV_TYPE_ARM_VGIC_V2:
 			irq->targets = 0;
 			irq->group = 0;
-		} else {
+			break;
+		case KVM_DEV_TYPE_ARM_VGIC_V3:
 			irq->mpidr = 0;
 			irq->group = 1;
+			break;
+		default:
+			kfree(dist->spis);
+			return -EINVAL;
 		}
 	}
 	return 0;
@@ -209,7 +216,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 		irq->intid = i;
 		irq->vcpu = NULL;
 		irq->target_vcpu = vcpu;
-		irq->targets = 1U << vcpu->vcpu_id;
 		kref_init(&irq->refcount);
 		if (vgic_irq_is_sgi(i)) {
 			/* SGIs */
@@ -219,11 +225,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 			/* PPIs */
 			irq->config = VGIC_CONFIG_LEVEL;
 		}
-
-		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
-			irq->group = 1;
-		else
-			irq->group = 0;
 	}
 
 	if (!irqchip_in_kernel(vcpu->kvm))
@@ -286,10 +287,19 @@ int vgic_init(struct kvm *kvm)
 
 		for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
 			struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
-			if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+			switch (dist->vgic_model) {
+			case KVM_DEV_TYPE_ARM_VGIC_V3:
 				irq->group = 1;
-			else
+				irq->mpidr = kvm_vcpu_get_mpidr_aff(vcpu);
+				break;
+			case KVM_DEV_TYPE_ARM_VGIC_V2:
 				irq->group = 0;
+				irq->targets = 1U << idx;
+				break;
+			default:
+				ret = -EINVAL;
+				goto out;
+			}
 		}
 	}
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [GIT PULL] KVM/arm updates for 5.3-rc6
  2019-08-23 16:35 [GIT PULL] KVM/arm updates for 5.3-rc6 Marc Zyngier
  2019-08-23 16:35 ` [PATCH 1/2] KVM: arm/arm64: Only skip MMIO insn once Marc Zyngier
  2019-08-23 16:35 ` [PATCH 2/2] KVM: arm/arm64: VGIC: Properly initialise private IRQ affinity Marc Zyngier
@ 2019-08-23 17:07 ` Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2019-08-23 17:07 UTC (permalink / raw)
  To: Marc Zyngier, Radim Krčmář
  Cc: Andre Przywara, Andrew Jones, Dave Martin, Julien Grall,
	Mark Rutland, Julien Thierry, James Morse, Suzuki K Poulose,
	linux-arm-kernel, kvm, kvmarm, Christian Borntraeger

On 23/08/19 18:35, Marc Zyngier wrote:
> Paolo, Radim,
> 
> One (hopefully last) set of fixes for KVM/arm for 5.3: an embarassing
> MMIO emulation regression, and a UBSAN splat. Oh well...
> 
> Please pull,

Please send this (and any other changes until the release) through the
ARM tree---same for s390 if need be.

This way Radim can concentrate on pending 5.4 patches while I am away.

Paolo

>        M.
> 
> The following changes since commit 16e604a437c89751dc626c9e90cf88ba93c5be64:
> 
>   KVM: arm/arm64: vgic: Reevaluate level sensitive interrupts on enable (2019-08-09 08:07:26 +0100)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git tags/kvmarm-fixes-for-5.3-3
> 
> for you to fetch changes up to 2e16f3e926ed48373c98edea85c6ad0ef69425d1:
> 
>   KVM: arm/arm64: VGIC: Properly initialise private IRQ affinity (2019-08-23 17:23:01 +0100)
> 
> ----------------------------------------------------------------
> KVM/arm fixes for 5.3, take #3
> 
> - Don't overskip instructions on MMIO emulation
> - Fix UBSAN splat when initializing PPI priorities
> 
> ----------------------------------------------------------------
> Andre Przywara (1):
>       KVM: arm/arm64: VGIC: Properly initialise private IRQ affinity
> 
> Andrew Jones (1):
>       KVM: arm/arm64: Only skip MMIO insn once
> 
>  virt/kvm/arm/mmio.c           |  7 +++++++
>  virt/kvm/arm/vgic/vgic-init.c | 30 ++++++++++++++++++++----------
>  2 files changed, 27 insertions(+), 10 deletions(-)
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-23 16:35 [GIT PULL] KVM/arm updates for 5.3-rc6 Marc Zyngier
2019-08-23 16:35 ` [PATCH 1/2] KVM: arm/arm64: Only skip MMIO insn once Marc Zyngier
2019-08-23 16:35 ` [PATCH 2/2] KVM: arm/arm64: VGIC: Properly initialise private IRQ affinity Marc Zyngier
2019-08-23 17:07 ` [GIT PULL] KVM/arm updates for 5.3-rc6 Paolo Bonzini

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org kvm@archiver.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox