* [PATCH v2 1/6] hw/arm/virt: Document 'max' value in gic-version property description
2020-03-01 10:40 [PATCH v2 0/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work Eric Auger
@ 2020-03-01 10:40 ` Eric Auger
2020-03-01 17:39 ` Richard Henderson
2020-03-01 10:40 ` [PATCH v2 2/6] hw/arm/virt: Use VIRT_GIC_VERSION defines Eric Auger
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Eric Auger @ 2020-03-01 10:40 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell
Cc: maz, drjones
Mention 'max' value in the gic-version property description.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
hw/arm/virt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 856808599d..c093f0ab85 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2144,7 +2144,8 @@ static void virt_instance_init(Object *obj)
virt_set_gic_version, NULL);
object_property_set_description(obj, "gic-version",
"Set GIC version. "
- "Valid values are 2, 3 and host", NULL);
+ "Valid values are 2, 3, host and max",
+ NULL);
vms->highmem_ecam = !vmc->no_highmem_ecam;
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 2/6] hw/arm/virt: Use VIRT_GIC_VERSION defines
2020-03-01 10:40 [PATCH v2 0/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work Eric Auger
2020-03-01 10:40 ` [PATCH v2 1/6] hw/arm/virt: Document 'max' value in gic-version property description Eric Auger
@ 2020-03-01 10:40 ` Eric Auger
2020-03-01 17:44 ` Richard Henderson
2020-03-01 10:40 ` [PATCH v2 3/6] hw/arm/virt: Introduce finalize_gic_version() Eric Auger
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Eric Auger @ 2020-03-01 10:40 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell
Cc: maz, drjones
We plan to introduce yet another value for the gic version (nosel).
As we already use exotic values such as 0 and -1, let's introduce
some defines.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
hw/arm/virt.c | 30 +++++++++++++++---------------
include/hw/arm/virt.h | 7 ++++++-
2 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index c093f0ab85..b449a445de 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -298,7 +298,7 @@ static void fdt_add_timer_nodes(const VirtMachineState *vms)
irqflags = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI;
}
- if (vms->gic_version == 2) {
+ if (vms->gic_version == VIRT_GIC_VERSION_2) {
irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
GIC_FDT_IRQ_PPI_CPU_WIDTH,
(1 << vms->smp_cpus) - 1);
@@ -439,7 +439,7 @@ static void fdt_add_gic_node(VirtMachineState *vms)
qemu_fdt_setprop_cell(vms->fdt, nodename, "#address-cells", 0x2);
qemu_fdt_setprop_cell(vms->fdt, nodename, "#size-cells", 0x2);
qemu_fdt_setprop(vms->fdt, nodename, "ranges", NULL, 0);
- if (vms->gic_version == 3) {
+ if (vms->gic_version == VIRT_GIC_VERSION_3) {
int nb_redist_regions = virt_gicv3_redist_region_count(vms);
qemu_fdt_setprop_string(vms->fdt, nodename, "compatible",
@@ -518,7 +518,7 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms)
}
}
- if (vms->gic_version == 2) {
+ if (vms->gic_version == VIRT_GIC_VERSION_2) {
irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
GIC_FDT_IRQ_PPI_CPU_WIDTH,
(1 << vms->smp_cpus) - 1);
@@ -1469,7 +1469,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
* purposes are to make TCG consistent (with 64-bit KVM hosts)
* and to improve SGI efficiency.
*/
- if (vms->gic_version == 3) {
+ if (vms->gic_version == VIRT_GIC_VERSION_3) {
clustersz = GICV3_TARGETLIST_BITS;
} else {
clustersz = GIC_TARGETLIST_BITS;
@@ -1560,15 +1560,15 @@ static void machvirt_init(MachineState *machine)
/* We can probe only here because during property set
* KVM is not available yet
*/
- if (vms->gic_version <= 0) {
- /* "host" or "max" */
+ if (vms->gic_version == VIRT_GIC_VERSION_HOST ||
+ vms->gic_version == VIRT_GIC_VERSION_MAX) {
if (!kvm_enabled()) {
- if (vms->gic_version == 0) {
+ if (vms->gic_version == VIRT_GIC_VERSION_HOST) {
error_report("gic-version=host requires KVM");
exit(1);
} else {
/* "max": currently means 3 for TCG */
- vms->gic_version = 3;
+ vms->gic_version = VIRT_GIC_VERSION_3;
}
} else {
vms->gic_version = kvm_arm_vgic_probe();
@@ -1627,7 +1627,7 @@ static void machvirt_init(MachineState *machine)
/* The maximum number of CPUs depends on the GIC version, or on how
* many redistributors we can fit into the memory map.
*/
- if (vms->gic_version == 3) {
+ if (vms->gic_version == VIRT_GIC_VERSION_3) {
virt_max_cpus =
vms->memmap[VIRT_GIC_REDIST].size / GICV3_REDIST_SIZE;
virt_max_cpus +=
@@ -1855,7 +1855,7 @@ static void virt_set_its(Object *obj, bool value, Error **errp)
static char *virt_get_gic_version(Object *obj, Error **errp)
{
VirtMachineState *vms = VIRT_MACHINE(obj);
- const char *val = vms->gic_version == 3 ? "3" : "2";
+ const char *val = vms->gic_version == VIRT_GIC_VERSION_3 ? "3" : "2";
return g_strdup(val);
}
@@ -1865,13 +1865,13 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
VirtMachineState *vms = VIRT_MACHINE(obj);
if (!strcmp(value, "3")) {
- vms->gic_version = 3;
+ vms->gic_version = VIRT_GIC_VERSION_3;
} else if (!strcmp(value, "2")) {
- vms->gic_version = 2;
+ vms->gic_version = VIRT_GIC_VERSION_2;
} else if (!strcmp(value, "host")) {
- vms->gic_version = 0; /* Will probe later */
+ vms->gic_version = VIRT_GIC_VERSION_HOST; /* Will probe later */
} else if (!strcmp(value, "max")) {
- vms->gic_version = -1; /* Will probe later */
+ vms->gic_version = VIRT_GIC_VERSION_MAX; /* Will probe later */
} else {
error_setg(errp, "Invalid gic-version value");
error_append_hint(errp, "Valid values are 3, 2, host, max.\n");
@@ -2139,7 +2139,7 @@ static void virt_instance_init(Object *obj)
"physical address space above 32 bits",
NULL);
/* Default GIC type is v2 */
- vms->gic_version = 2;
+ vms->gic_version = VIRT_GIC_VERSION_2;
object_property_add_str(obj, "gic-version", virt_get_gic_version,
virt_set_gic_version, NULL);
object_property_set_description(obj, "gic-version",
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 02f500cb8e..6325b98269 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -95,6 +95,11 @@ typedef enum VirtIOMMUType {
VIRT_IOMMU_VIRTIO,
} VirtIOMMUType;
+#define VIRT_GIC_VERSION_MAX (-1)
+#define VIRT_GIC_VERSION_HOST 0
+#define VIRT_GIC_VERSION_2 2
+#define VIRT_GIC_VERSION_3 3
+
typedef struct MemMapEntry {
hwaddr base;
hwaddr size;
@@ -162,7 +167,7 @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
uint32_t redist0_capacity =
vms->memmap[VIRT_GIC_REDIST].size / GICV3_REDIST_SIZE;
- assert(vms->gic_version == 3);
+ assert(vms->gic_version == VIRT_GIC_VERSION_3);
return vms->smp_cpus > redist0_capacity ? 2 : 1;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 3/6] hw/arm/virt: Introduce finalize_gic_version()
2020-03-01 10:40 [PATCH v2 0/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work Eric Auger
2020-03-01 10:40 ` [PATCH v2 1/6] hw/arm/virt: Document 'max' value in gic-version property description Eric Auger
2020-03-01 10:40 ` [PATCH v2 2/6] hw/arm/virt: Use VIRT_GIC_VERSION defines Eric Auger
@ 2020-03-01 10:40 ` Eric Auger
2020-03-01 17:48 ` Richard Henderson
2020-03-01 10:40 ` [PATCH v2 4/6] target/arm/kvm: Let kvm_arm_vgic_probe() return a bitmap Eric Auger
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Eric Auger @ 2020-03-01 10:40 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell
Cc: maz, drjones
Let's move the code which freezes which gic-version to
be applied in a dedicated function. We also now set by
default the VIRT_GIC_VERSION_NO_SET. This eventually
turns into the legacy v2 choice in the finalize() function.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
hw/arm/virt.c | 54 ++++++++++++++++++++++++++-----------------
include/hw/arm/virt.h | 1 +
2 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b449a445de..338d56999f 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1534,6 +1534,37 @@ static void virt_set_memmap(VirtMachineState *vms)
}
}
+/*
+ * finalize_gic_version - Determines the final gic_version
+ * according to the gic-version property
+ *
+ * Default GIC type is v2
+ */
+static void finalize_gic_version(VirtMachineState *vms)
+{
+ if (vms->gic_version == VIRT_GIC_VERSION_HOST ||
+ vms->gic_version == VIRT_GIC_VERSION_MAX) {
+ if (!kvm_enabled()) {
+ if (vms->gic_version == VIRT_GIC_VERSION_HOST) {
+ error_report("gic-version=host requires KVM");
+ exit(1);
+ } else {
+ /* "max": currently means 3 for TCG */
+ vms->gic_version = VIRT_GIC_VERSION_3;
+ }
+ } else {
+ vms->gic_version = kvm_arm_vgic_probe();
+ if (!vms->gic_version) {
+ error_report(
+ "Unable to determine GIC version supported by host");
+ exit(1);
+ }
+ }
+ } else if (vms->gic_version == VIRT_GIC_VERSION_NOSEL) {
+ vms->gic_version = VIRT_GIC_VERSION_2;
+ }
+}
+
static void machvirt_init(MachineState *machine)
{
VirtMachineState *vms = VIRT_MACHINE(machine);
@@ -1560,25 +1591,7 @@ static void machvirt_init(MachineState *machine)
/* We can probe only here because during property set
* KVM is not available yet
*/
- if (vms->gic_version == VIRT_GIC_VERSION_HOST ||
- vms->gic_version == VIRT_GIC_VERSION_MAX) {
- if (!kvm_enabled()) {
- if (vms->gic_version == VIRT_GIC_VERSION_HOST) {
- error_report("gic-version=host requires KVM");
- exit(1);
- } else {
- /* "max": currently means 3 for TCG */
- vms->gic_version = VIRT_GIC_VERSION_3;
- }
- } else {
- vms->gic_version = kvm_arm_vgic_probe();
- if (!vms->gic_version) {
- error_report(
- "Unable to determine GIC version supported by host");
- exit(1);
- }
- }
- }
+ finalize_gic_version(vms);
if (!cpu_type_valid(machine->cpu_type)) {
error_report("mach-virt: CPU type %s not supported", machine->cpu_type);
@@ -2138,8 +2151,7 @@ static void virt_instance_init(Object *obj)
"Set on/off to enable/disable using "
"physical address space above 32 bits",
NULL);
- /* Default GIC type is v2 */
- vms->gic_version = VIRT_GIC_VERSION_2;
+ vms->gic_version = VIRT_GIC_VERSION_NOSEL;
object_property_add_str(obj, "gic-version", virt_get_gic_version,
virt_set_gic_version, NULL);
object_property_set_description(obj, "gic-version",
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 6325b98269..5785416480 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -95,6 +95,7 @@ typedef enum VirtIOMMUType {
VIRT_IOMMU_VIRTIO,
} VirtIOMMUType;
+#define VIRT_GIC_VERSION_NOSEL (-2)
#define VIRT_GIC_VERSION_MAX (-1)
#define VIRT_GIC_VERSION_HOST 0
#define VIRT_GIC_VERSION_2 2
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 3/6] hw/arm/virt: Introduce finalize_gic_version()
2020-03-01 10:40 ` [PATCH v2 3/6] hw/arm/virt: Introduce finalize_gic_version() Eric Auger
@ 2020-03-01 17:48 ` Richard Henderson
0 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2020-03-01 17:48 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell
Cc: maz, drjones
On 3/1/20 2:40 AM, Eric Auger wrote:
> Let's move the code which freezes which gic-version to
> be applied in a dedicated function. We also now set by
> default the VIRT_GIC_VERSION_NO_SET. This eventually
> turns into the legacy v2 choice in the finalize() function.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> hw/arm/virt.c | 54 ++++++++++++++++++++++++++-----------------
> include/hw/arm/virt.h | 1 +
> 2 files changed, 34 insertions(+), 21 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 4/6] target/arm/kvm: Let kvm_arm_vgic_probe() return a bitmap
2020-03-01 10:40 [PATCH v2 0/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work Eric Auger
` (2 preceding siblings ...)
2020-03-01 10:40 ` [PATCH v2 3/6] hw/arm/virt: Introduce finalize_gic_version() Eric Auger
@ 2020-03-01 10:40 ` Eric Auger
2020-03-01 17:50 ` Richard Henderson
2020-03-01 10:40 ` [PATCH v2 5/6] hw/arm/virt: kvm: Check the chosen gic version is supported by the host Eric Auger
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Eric Auger @ 2020-03-01 10:40 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell
Cc: maz, drjones
Convert kvm_arm_vgic_probe() so that it returns a
bitmap of supported in-kernel emulation VGIC versions instead
of the max version: at the moment values can be v2 and v3.
This allows to expose the case where the host GICv3 also
supports GICv2 emulation. This will be useful to choose the
default version in KVM accelerated mode.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
hw/arm/virt.c | 11 +++++++++--
target/arm/kvm.c | 14 ++++++++------
target/arm/kvm_arm.h | 3 +++
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 338d56999f..eb8c57c85e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1553,11 +1553,18 @@ static void finalize_gic_version(VirtMachineState *vms)
vms->gic_version = VIRT_GIC_VERSION_3;
}
} else {
- vms->gic_version = kvm_arm_vgic_probe();
- if (!vms->gic_version) {
+ int probe_bitmap = kvm_arm_vgic_probe();
+
+ if (!probe_bitmap) {
error_report(
"Unable to determine GIC version supported by host");
exit(1);
+ } else {
+ if (probe_bitmap & KVM_ARM_VGIC_V3) {
+ vms->gic_version = VIRT_GIC_VERSION_3;
+ } else {
+ vms->gic_version = VIRT_GIC_VERSION_2;
+ }
}
}
} else if (vms->gic_version == VIRT_GIC_VERSION_NOSEL) {
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 85860e6f95..390077c518 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -874,15 +874,17 @@ int kvm_arch_irqchip_create(KVMState *s)
int kvm_arm_vgic_probe(void)
{
+ int val = 0;
+
if (kvm_create_device(kvm_state,
KVM_DEV_TYPE_ARM_VGIC_V3, true) == 0) {
- return 3;
- } else if (kvm_create_device(kvm_state,
- KVM_DEV_TYPE_ARM_VGIC_V2, true) == 0) {
- return 2;
- } else {
- return 0;
+ val |= KVM_ARM_VGIC_V3;
}
+ if (kvm_create_device(kvm_state,
+ KVM_DEV_TYPE_ARM_VGIC_V2, true) == 0) {
+ val |= KVM_ARM_VGIC_V2;
+ }
+ return val;
}
int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index ae9e075d75..48bf5e16d5 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -15,6 +15,9 @@
#include "exec/memory.h"
#include "qemu/error-report.h"
+#define KVM_ARM_VGIC_V2 (1 << 0)
+#define KVM_ARM_VGIC_V3 (1 << 1)
+
/**
* kvm_arm_vcpu_init:
* @cs: CPUState
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 4/6] target/arm/kvm: Let kvm_arm_vgic_probe() return a bitmap
2020-03-01 10:40 ` [PATCH v2 4/6] target/arm/kvm: Let kvm_arm_vgic_probe() return a bitmap Eric Auger
@ 2020-03-01 17:50 ` Richard Henderson
0 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2020-03-01 17:50 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell
Cc: maz, drjones
On 3/1/20 2:40 AM, Eric Auger wrote:
> Convert kvm_arm_vgic_probe() so that it returns a
> bitmap of supported in-kernel emulation VGIC versions instead
> of the max version: at the moment values can be v2 and v3.
> This allows to expose the case where the host GICv3 also
> supports GICv2 emulation. This will be useful to choose the
> default version in KVM accelerated mode.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> hw/arm/virt.c | 11 +++++++++--
> target/arm/kvm.c | 14 ++++++++------
> target/arm/kvm_arm.h | 3 +++
> 3 files changed, 20 insertions(+), 8 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 5/6] hw/arm/virt: kvm: Check the chosen gic version is supported by the host
2020-03-01 10:40 [PATCH v2 0/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work Eric Auger
` (3 preceding siblings ...)
2020-03-01 10:40 ` [PATCH v2 4/6] target/arm/kvm: Let kvm_arm_vgic_probe() return a bitmap Eric Auger
@ 2020-03-01 10:40 ` Eric Auger
2020-03-01 17:56 ` Richard Henderson
2020-03-01 10:40 ` [PATCH v2 6/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work Eric Auger
2020-03-02 7:55 ` [PATCH v2 0/6] " Andrew Jones
6 siblings, 1 reply; 16+ messages in thread
From: Eric Auger @ 2020-03-01 10:40 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell
Cc: maz, drjones
Restructure the finalize_gic_version with switch cases and, in
KVM mode, explictly check whether the chosen version is supported
by the host.
if the end-user explicitly sets v2/v3 and this is not supported by
the host, then the user gets an explicit error message.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
hw/arm/virt.c | 76 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 52 insertions(+), 24 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index eb8c57c85e..610bfc9ee9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1542,33 +1542,61 @@ static void virt_set_memmap(VirtMachineState *vms)
*/
static void finalize_gic_version(VirtMachineState *vms)
{
- if (vms->gic_version == VIRT_GIC_VERSION_HOST ||
- vms->gic_version == VIRT_GIC_VERSION_MAX) {
- if (!kvm_enabled()) {
- if (vms->gic_version == VIRT_GIC_VERSION_HOST) {
- error_report("gic-version=host requires KVM");
- exit(1);
- } else {
- /* "max": currently means 3 for TCG */
- vms->gic_version = VIRT_GIC_VERSION_3;
- }
- } else {
- int probe_bitmap = kvm_arm_vgic_probe();
+ if (kvm_enabled()) {
+ int probe_bitmap = kvm_arm_vgic_probe();
- if (!probe_bitmap) {
- error_report(
- "Unable to determine GIC version supported by host");
- exit(1);
- } else {
- if (probe_bitmap & KVM_ARM_VGIC_V3) {
- vms->gic_version = VIRT_GIC_VERSION_3;
- } else {
- vms->gic_version = VIRT_GIC_VERSION_2;
- }
- }
+ if (!probe_bitmap) {
+ error_report("Unable to determine GIC version supported by host");
+ exit(1);
}
- } else if (vms->gic_version == VIRT_GIC_VERSION_NOSEL) {
+
+ switch (vms->gic_version) {
+ case VIRT_GIC_VERSION_NOSEL:
+ vms->gic_version = VIRT_GIC_VERSION_2;
+ break;
+ case VIRT_GIC_VERSION_HOST:
+ case VIRT_GIC_VERSION_MAX:
+ if (probe_bitmap & KVM_ARM_VGIC_V3) {
+ vms->gic_version = VIRT_GIC_VERSION_3;
+ } else {
+ vms->gic_version = VIRT_GIC_VERSION_2;
+ }
+ return;
+ case VIRT_GIC_VERSION_2:
+ case VIRT_GIC_VERSION_3:
+ break;
+ }
+
+ if (!kvm_irqchip_in_kernel()) {
+ return;
+ }
+
+ /* Check chosen version is effectively supported by the host */
+ if (vms->gic_version == VIRT_GIC_VERSION_2 &&
+ !(probe_bitmap & KVM_ARM_VGIC_V2)) {
+ error_report("host does not support in-kernel GICv2 emulation");
+ exit(1);
+ } else if (vms->gic_version == VIRT_GIC_VERSION_3 &&
+ !(probe_bitmap & KVM_ARM_VGIC_V3)) {
+ error_report("host does not support in-kernel GICv3 emulation");
+ exit(1);
+ }
+ return;
+ }
+
+ /* TCG mode */
+ switch (vms->gic_version) {
+ case VIRT_GIC_VERSION_NOSEL:
vms->gic_version = VIRT_GIC_VERSION_2;
+ break;
+ case VIRT_GIC_VERSION_MAX:
+ vms->gic_version = VIRT_GIC_VERSION_3;
+ break;
+ case VIRT_GIC_VERSION_HOST:
+ error_report("gic-version=host requires KVM");
+ exit(1);
+ default: /* explicit V2/V3 are left untouched */
+ break;
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/6] hw/arm/virt: kvm: Check the chosen gic version is supported by the host
2020-03-01 10:40 ` [PATCH v2 5/6] hw/arm/virt: kvm: Check the chosen gic version is supported by the host Eric Auger
@ 2020-03-01 17:56 ` Richard Henderson
2020-03-01 18:02 ` Auger Eric
0 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2020-03-01 17:56 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell
Cc: maz, drjones
On 3/1/20 2:40 AM, Eric Auger wrote:
> + /* Check chosen version is effectively supported by the host */
> + if (vms->gic_version == VIRT_GIC_VERSION_2 &&
> + !(probe_bitmap & KVM_ARM_VGIC_V2)) {
> + error_report("host does not support in-kernel GICv2 emulation");
> + exit(1);
> + } else if (vms->gic_version == VIRT_GIC_VERSION_3 &&
> + !(probe_bitmap & KVM_ARM_VGIC_V3)) {
> + error_report("host does not support in-kernel GICv3 emulation");
> + exit(1);
> + }
Indentation is wrong here.
> + case VIRT_GIC_VERSION_HOST:
> + error_report("gic-version=host requires KVM");
> + exit(1);
> + default: /* explicit V2/V3 are left untouched */
> + break;
> }
I'd prefer to just list V2 and V3 here explicitly, instead of the default.
It'll be nicer with gic_version changed to an enum.
With those changes,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/6] hw/arm/virt: kvm: Check the chosen gic version is supported by the host
2020-03-01 17:56 ` Richard Henderson
@ 2020-03-01 18:02 ` Auger Eric
0 siblings, 0 replies; 16+ messages in thread
From: Auger Eric @ 2020-03-01 18:02 UTC (permalink / raw)
To: Richard Henderson, eric.auger.pro, qemu-devel, qemu-arm,
peter.maydell
Cc: maz, drjones
Hi Richard,
On 3/1/20 6:56 PM, Richard Henderson wrote:
> On 3/1/20 2:40 AM, Eric Auger wrote:
>> + /* Check chosen version is effectively supported by the host */
>> + if (vms->gic_version == VIRT_GIC_VERSION_2 &&
>> + !(probe_bitmap & KVM_ARM_VGIC_V2)) {
>> + error_report("host does not support in-kernel GICv2 emulation");
>> + exit(1);
>> + } else if (vms->gic_version == VIRT_GIC_VERSION_3 &&
>> + !(probe_bitmap & KVM_ARM_VGIC_V3)) {
>> + error_report("host does not support in-kernel GICv3 emulation");
>> + exit(1);
>> + }
>
> Indentation is wrong here.
OK
>
>> + case VIRT_GIC_VERSION_HOST:
>> + error_report("gic-version=host requires KVM");
>> + exit(1);
>> + default: /* explicit V2/V3 are left untouched */
>> + break;
>> }
>
> I'd prefer to just list V2 and V3 here explicitly, instead of the default.
> It'll be nicer with gic_version changed to an enum.
OK I will respin with those changes.
Thank you for the review!
Best Regards
Eric
>
> With those changes,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>
> r~
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 6/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work
2020-03-01 10:40 [PATCH v2 0/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work Eric Auger
` (4 preceding siblings ...)
2020-03-01 10:40 ` [PATCH v2 5/6] hw/arm/virt: kvm: Check the chosen gic version is supported by the host Eric Auger
@ 2020-03-01 10:40 ` Eric Auger
2020-03-01 17:58 ` Richard Henderson
2020-03-02 7:54 ` Andrew Jones
2020-03-02 7:55 ` [PATCH v2 0/6] " Andrew Jones
6 siblings, 2 replies; 16+ messages in thread
From: Eric Auger @ 2020-03-01 10:40 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell
Cc: maz, drjones
At the moment if the end-user does not specify the gic-version along
with KVM acceleration, v2 is set by default. However most of the
systems now have GICv3 and sometimes they do not support GICv2
compatibility.
This patch keeps the default v2 selection in all cases except
in the KVM accelerated mode when either
- the host does not support GICv2 in-kernel emulation or
- number of VCPUS exceeds 8.
Those cases did not work anyway so we do not break any compatibility.
Now we get v3 selected in such a case.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hw/arm/virt.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 610bfc9ee9..2d12a7b0b8 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1542,6 +1542,8 @@ static void virt_set_memmap(VirtMachineState *vms)
*/
static void finalize_gic_version(VirtMachineState *vms)
{
+ unsigned int max_cpus = MACHINE(vms)->smp.max_cpus;
+
if (kvm_enabled()) {
int probe_bitmap = kvm_arm_vgic_probe();
@@ -1552,7 +1554,17 @@ static void finalize_gic_version(VirtMachineState *vms)
switch (vms->gic_version) {
case VIRT_GIC_VERSION_NOSEL:
- vms->gic_version = VIRT_GIC_VERSION_2;
+ if ((probe_bitmap & KVM_ARM_VGIC_V2 && max_cpus <= GIC_NCPU) ||
+ !kvm_irqchip_in_kernel()) {
+ vms->gic_version = VIRT_GIC_VERSION_2;
+ } else {
+ /*
+ * in case the host does not support v2 in-kernel emulation or
+ * the end-user requested more than 8 VCPUs we now default
+ * to v3. In any case defaulting to v2 would be broken.
+ */
+ vms->gic_version = VIRT_GIC_VERSION_3;
+ }
break;
case VIRT_GIC_VERSION_HOST:
case VIRT_GIC_VERSION_MAX:
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 6/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work
2020-03-01 10:40 ` [PATCH v2 6/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work Eric Auger
@ 2020-03-01 17:58 ` Richard Henderson
2020-03-02 7:54 ` Andrew Jones
1 sibling, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2020-03-01 17:58 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, qemu-arm, peter.maydell
Cc: maz, drjones
On 3/1/20 2:40 AM, Eric Auger wrote:
> At the moment if the end-user does not specify the gic-version along
> with KVM acceleration, v2 is set by default. However most of the
> systems now have GICv3 and sometimes they do not support GICv2
> compatibility.
>
> This patch keeps the default v2 selection in all cases except
> in the KVM accelerated mode when either
> - the host does not support GICv2 in-kernel emulation or
> - number of VCPUS exceeds 8.
>
> Those cases did not work anyway so we do not break any compatibility.
> Now we get v3 selected in such a case.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hw/arm/virt.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 6/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work
2020-03-01 10:40 ` [PATCH v2 6/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work Eric Auger
2020-03-01 17:58 ` Richard Henderson
@ 2020-03-02 7:54 ` Andrew Jones
1 sibling, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2020-03-02 7:54 UTC (permalink / raw)
To: Eric Auger; +Cc: peter.maydell, qemu-arm, maz, qemu-devel, eric.auger.pro
On Sun, Mar 01, 2020 at 11:40:40AM +0100, Eric Auger wrote:
> At the moment if the end-user does not specify the gic-version along
> with KVM acceleration, v2 is set by default. However most of the
> systems now have GICv3 and sometimes they do not support GICv2
> compatibility.
>
> This patch keeps the default v2 selection in all cases except
> in the KVM accelerated mode when either
> - the host does not support GICv2 in-kernel emulation or
> - number of VCPUS exceeds 8.
>
> Those cases did not work anyway so we do not break any compatibility.
> Now we get v3 selected in such a case.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hw/arm/virt.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 610bfc9ee9..2d12a7b0b8 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1542,6 +1542,8 @@ static void virt_set_memmap(VirtMachineState *vms)
> */
> static void finalize_gic_version(VirtMachineState *vms)
> {
> + unsigned int max_cpus = MACHINE(vms)->smp.max_cpus;
> +
> if (kvm_enabled()) {
> int probe_bitmap = kvm_arm_vgic_probe();
>
> @@ -1552,7 +1554,17 @@ static void finalize_gic_version(VirtMachineState *vms)
>
> switch (vms->gic_version) {
> case VIRT_GIC_VERSION_NOSEL:
> - vms->gic_version = VIRT_GIC_VERSION_2;
> + if ((probe_bitmap & KVM_ARM_VGIC_V2 && max_cpus <= GIC_NCPU) ||
> + !kvm_irqchip_in_kernel()) {
nit: () around the bitmap & would be nice
> + vms->gic_version = VIRT_GIC_VERSION_2;
> + } else {
> + /*
> + * in case the host does not support v2 in-kernel emulation or
> + * the end-user requested more than 8 VCPUs we now default
> + * to v3. In any case defaulting to v2 would be broken.
> + */
> + vms->gic_version = VIRT_GIC_VERSION_3;
> + }
> break;
> case VIRT_GIC_VERSION_HOST:
> case VIRT_GIC_VERSION_MAX:
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work
2020-03-01 10:40 [PATCH v2 0/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work Eric Auger
` (5 preceding siblings ...)
2020-03-01 10:40 ` [PATCH v2 6/6] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work Eric Auger
@ 2020-03-02 7:55 ` Andrew Jones
6 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2020-03-02 7:55 UTC (permalink / raw)
To: Eric Auger; +Cc: peter.maydell, qemu-arm, maz, qemu-devel, eric.auger.pro
On Sun, Mar 01, 2020 at 11:40:34AM +0100, Eric Auger wrote:
> At the moment if the end-user does not specify the gic-version along
> with KVM acceleration, v2 is set by default. However most of the
> systems now have GICv3 and sometimes they do not support GICv2
> compatibility. In that case we now end up with the following error:
>
> "qemu-system-aarch64: Initialization of device kvm-arm-gic failed:
> error creating in-kernel VGIC: No such device
> Perhaps the host CPU does not support GICv2?"
>
> since "1904f9b5f1 hw/intc/arm_gic_kvm: Don't assume kernel can
> provide a GICv2", which already allowed to output an explicit error
> message.
>
> This series keeps the default v2 selection in all cases except
> in the KVM accelerated mode when v2 cannot work:
> - either because the host does not support v2 in-kernel emulation or
> - because more than 8 vcpus were requested.
>
> Those cases did not work anyway so we do not break any compatibility.
> Now we get v3 selected in such a case.
>
> Best Regards
>
> Eric
>
> This series can be found at:
> https://github.com/eauger/qemu/tree/v4.2.0-gic-version-v2
>
> History:
> RFC -> v2:
> - 1904f9b5f1 hw/intc/arm_gic_kvm: Don't assume kernel can
> provide a GICv2" now has landed upstream
> - Fix gic-version description
> - Introduce finalize_gic_version and use switch/cases
> - take into account smp value
>
> Eric Auger (6):
> hw/arm/virt: Document 'max' value in gic-version property description
> hw/arm/virt: Use VIRT_GIC_VERSION defines
> hw/arm/virt: Introduce finalize_gic_version()
> target/arm/kvm: Let kvm_arm_vgic_probe() return a bitmap
> hw/arm/virt: kvm: Check the chosen gic version is supported by the
> host
> hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work
>
> hw/arm/virt.c | 124 +++++++++++++++++++++++++++++++-----------
> include/hw/arm/virt.h | 8 ++-
> target/arm/kvm.c | 14 +++--
> target/arm/kvm_arm.h | 3 +
> 4 files changed, 110 insertions(+), 39 deletions(-)
>
> --
> 2.20.1
>
>
With Richard's enum suggestions
For the series
Reviewed-by: Andrew Jones <drjones@redhat.com>
^ permalink raw reply [flat|nested] 16+ messages in thread