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

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~

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
> 
>