Patchwork [kvmtool,6/6] arm: Auto-detect guest GIC type

login
register
mail settings
Submitter Andre Przywara
Date Jan. 25, 2019, 6:08 p.m.
Message ID <20190125180801.209910-7-andre.przywara@arm.com>
Download mbox | patch
Permalink /patch/710053/
State New
Headers show

Comments

Andre Przywara - Jan. 25, 2019, 6:08 p.m.
At the moment kvmtool always tries to instantiate a virtual GICv2 for
the guest, and fails with some scary error message if that doesn't work.
The user has then to manually specify "--irqchip=gicv3", which is not
really obvious.
With the advent of more GICv3-only machines, let's try to be more
clever and implement some auto-detection of the GIC type needed:
- We try GICv3 first. On GICv3-only hosts this will be the only working
option, so we don't loose anything. On GICv2-backwards compatible GICv3
machines GICv3 is probably the better choice these days.
- If that fails, we try GICv2.
- If that fails, we ran out out options and bail out.

We deduce the choice between "ITS vs. pure GICv3" and "GICv2m vs. GICv2" by
the presence of PCI devices, they would be the only MSI users anyway.

This algorithm is in effect is there is no explicit --irqchip parameter
on the command line. We still allow the GIC type to be set explicitly.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arm/gic.c                    | 25 +++++++++++++++++++++++++
 arm/include/arm-common/gic.h |  1 +
 2 files changed, 26 insertions(+)
Will Deacon - Jan. 30, 2019, 6:20 p.m.
On Fri, Jan 25, 2019 at 06:08:01PM +0000, Andre Przywara wrote:
> At the moment kvmtool always tries to instantiate a virtual GICv2 for
> the guest, and fails with some scary error message if that doesn't work.
> The user has then to manually specify "--irqchip=gicv3", which is not
> really obvious.
> With the advent of more GICv3-only machines, let's try to be more
> clever and implement some auto-detection of the GIC type needed:
> - We try GICv3 first. On GICv3-only hosts this will be the only working
> option, so we don't loose anything. On GICv2-backwards compatible GICv3
> machines GICv3 is probably the better choice these days.

Could you elaborate on "probably the better choice" please?

> - If that fails, we try GICv2.
> - If that fails, we ran out out options and bail out.
> 
> We deduce the choice between "ITS vs. pure GICv3" and "GICv2m vs. GICv2" by
> the presence of PCI devices, they would be the only MSI users anyway.

This feels really flakey. Why don't we just try, in order:

  v3 + ITS
  v3
  v2m
  v2

regardless of the VM configuration?

Will
Andre Przywara - Jan. 31, 2019, 6:46 p.m.
On Wed, 30 Jan 2019 18:20:46 +0000
Will Deacon <will.deacon@arm.com> wrote:

> On Fri, Jan 25, 2019 at 06:08:01PM +0000, Andre Przywara wrote:
> > At the moment kvmtool always tries to instantiate a virtual GICv2
> > for the guest, and fails with some scary error message if that
> > doesn't work. The user has then to manually specify
> > "--irqchip=gicv3", which is not really obvious.
> > With the advent of more GICv3-only machines, let's try to be more
> > clever and implement some auto-detection of the GIC type needed:
> > - We try GICv3 first. On GICv3-only hosts this will be the only
> > working option, so we don't loose anything. On GICv2-backwards
> > compatible GICv3 machines GICv3 is probably the better choice these
> > days.  
> 
> Could you elaborate on "probably the better choice" please?

When we introduced the GICv3 emulation and the kvmtool support,
it was deemed less mature than the GICv2 support. This was one of the
reasons we were happy with GICv2-on-v3 as the default. Now with GICv3
support being stable, we should use the advantages like the system
register interface.

> > - If that fails, we try GICv2.
> > - If that fails, we ran out out options and bail out.
> > 
> > We deduce the choice between "ITS vs. pure GICv3" and "GICv2m vs.
> > GICv2" by the presence of PCI devices, they would be the only MSI
> > users anyway.  
> 
> This feels really flakey. Why don't we just try, in order:
> 
>   v3 + ITS
>   v3
>   v2m
>   v2

I think v2m will fail only rarely (doesn't rely on any kernel
support), so in practise we will likely never get a "pure" GICv2. But
since we always have the --irqchip option to override this, that's
probably OK.

Will change it to that effect.

Cheers,
Andre.

Patch

diff --git a/arm/gic.c b/arm/gic.c
index abcbcc09..e6b66047 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -182,6 +182,8 @@  static int gic__create_device(struct kvm *kvm, enum irqchip_type type)
 		gic_device.type = KVM_DEV_TYPE_ARM_VGIC_V3;
 		dist_attr.attr  = KVM_VGIC_V3_ADDR_TYPE_DIST;
 		break;
+	case IRQCHIP_AUTO:
+		return -ENODEV;
 	}
 
 	err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &gic_device);
@@ -199,6 +201,8 @@  static int gic__create_device(struct kvm *kvm, enum irqchip_type type)
 	case IRQCHIP_GICV3:
 		err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &redist_attr);
 		break;
+	case IRQCHIP_AUTO:
+		return -ENODEV;
 	}
 	if (err)
 		goto out_err;
@@ -249,9 +253,30 @@  static int gic__create_irqchip(struct kvm *kvm)
 
 int gic__create(struct kvm *kvm, enum irqchip_type type)
 {
+	enum irqchip_type try;
+	bool needs_msis;
 	int err;
 
 	switch (type) {
+	case IRQCHIP_AUTO:
+		needs_msis = kvm->cfg.arch.virtio_trans_pci;
+		if (needs_msis)
+			try = IRQCHIP_GICV3_ITS;
+		else
+			try = IRQCHIP_GICV3;
+		err = gic__create(kvm, try);
+		if (!err) {
+			kvm->cfg.arch.irqchip = try;
+			return 0;
+		}
+		if (needs_msis)
+			try = IRQCHIP_GICV2M;
+		else
+			try = IRQCHIP_GICV2;
+		err = gic__create(kvm, try);
+		if (!err)
+			kvm->cfg.arch.irqchip = try;
+		return err;
 	case IRQCHIP_GICV2M:
 		gic_msi_size = KVM_VGIC_V2M_SIZE;
 		gic_msi_base = ARM_GIC_CPUI_BASE - gic_msi_size;
diff --git a/arm/include/arm-common/gic.h b/arm/include/arm-common/gic.h
index 1125d601..ec9cf31a 100644
--- a/arm/include/arm-common/gic.h
+++ b/arm/include/arm-common/gic.h
@@ -24,6 +24,7 @@ 
 #define KVM_VGIC_V2M_SIZE		0x1000
 
 enum irqchip_type {
+	IRQCHIP_AUTO,
 	IRQCHIP_GICV2,
 	IRQCHIP_GICV2M,
 	IRQCHIP_GICV3,