Re: [PATCH v4 3/6] hw/arm/virt: Introduce finalize_gic_version()

[Andrew Jones]

On Wed, Mar 11, 2020 at 12:16:23PM +0100, 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 <address@hidden>
> Reviewed-by: Richard Henderson <address@hidden>
> Reviewed-by: Andrew Jones <address@hidden>
> 
> ---
> 
> v2 -> v3:
> - add NOTSEL value at the end of the new enum type
> ---
>  include/hw/arm/virt.h |  1 +
>  hw/arm/virt.c         | 54 ++++++++++++++++++++++++++-----------------
>  2 files changed, 34 insertions(+), 21 deletions(-)
> 
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index c0827cacdf..893796d3b0 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -100,6 +100,7 @@ typedef enum VirtGICType {
>      VIRT_GIC_VERSION_HOST,
>      VIRT_GIC_VERSION_2,
>      VIRT_GIC_VERSION_3,
> +    VIRT_GIC_VERSION_NOSEL,
>  } VirtGICType;
>  
>  typedef struct MemMapEntry {
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 4ed1f0cb2e..22da0a7ad7 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1535,6 +1535,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);
> @@ -1561,25 +1592,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);
       ^ extra space
>  
>      if (!cpu_type_valid(machine->cpu_type)) {
>          error_report("mach-virt: CPU type %s not supported", 
> machine->cpu_type);
> @@ -2140,8 +2153,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",
> -- 
> 2.20.1
>