Re: [Qemu-devel] [PATCH 2/4] target-i386: Convert kvm_default_*features to property/value pairs

[Paolo Bonzini]

On 11/09/2015 21:25, Eduardo Habkost wrote:
> Convert the kvm_default_features and kvm_default_unset_features arrays
> into a simple list of property/value pairs that will be applied to
> X86CPU objects when using KVM.
> 
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
>  hw/i386/pc_piix.c |  8 ++---
>  hw/i386/pc_q35.c  |  4 +--
>  target-i386/cpu.c | 87 
> ++++++++++++++++++++++++++++++++-----------------------
>  target-i386/cpu.h | 11 +++++--
>  4 files changed, 65 insertions(+), 45 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 3f925b2..e79ef0d 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -325,7 +325,7 @@ static void pc_compat_2_1(MachineState *machine)
>  
>      pc_compat_2_2(machine);
>      smbios_uuid_encoded = false;
> -    x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
> +    x86_cpu_change_kvm_default("svm", NULL);
>      pcms->enforce_aligned_dimm = false;
>  }
>  
> @@ -361,7 +361,7 @@ static void pc_compat_1_7(MachineState *machine)
>      gigabyte_align = false;
>      option_rom_has_mr = true;
>      legacy_acpi_table_size = 6414;
> -    x86_cpu_compat_kvm_no_autoenable(FEAT_1_ECX, CPUID_EXT_X2APIC);
> +    x86_cpu_change_kvm_default("x2apic", NULL);
>  }
>  
>  static void pc_compat_1_6(MachineState *machine)
> @@ -391,7 +391,7 @@ static void pc_compat_1_3(MachineState *machine)
>  static void pc_compat_1_2(MachineState *machine)
>  {
>      pc_compat_1_3(machine);
> -    x86_cpu_compat_kvm_no_autoenable(FEAT_KVM, 1 << KVM_FEATURE_PV_EOI);
> +    x86_cpu_change_kvm_default("kvm-pv-eoi", NULL);
>  }
>  
>  /* PC compat function for pc-0.10 to pc-0.13 */
> @@ -414,7 +414,7 @@ static void pc_init_isa(MachineState *machine)
>      if (!machine->cpu_model) {
>          machine->cpu_model = "486";
>      }
> -    x86_cpu_compat_kvm_no_autoenable(FEAT_KVM, 1 << KVM_FEATURE_PV_EOI);
> +    x86_cpu_change_kvm_default("kvm-pv-eoi", NULL);
>      enable_compat_apic_id_mode();
>      pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE);
>  }
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 11601ab..c17d9df 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -309,7 +309,7 @@ static void pc_compat_2_1(MachineState *machine)
>      pc_compat_2_2(machine);
>      pcms->enforce_aligned_dimm = false;
>      smbios_uuid_encoded = false;
> -    x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
> +    x86_cpu_change_kvm_default("svm", NULL);
>  }
>  
>  static void pc_compat_2_0(MachineState *machine)
> @@ -326,7 +326,7 @@ static void pc_compat_1_7(MachineState *machine)
>      smbios_defaults = false;
>      gigabyte_align = false;
>      option_rom_has_mr = true;
> -    x86_cpu_compat_kvm_no_autoenable(FEAT_1_ECX, CPUID_EXT_X2APIC);
> +    x86_cpu_change_kvm_default("x2apic", NULL);
>  }
>  
>  static void pc_compat_1_6(MachineState *machine)
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index d505743..9a4bfa3 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -478,38 +478,6 @@ const char *get_register_name_32(unsigned int reg)
>      return x86_reg_info_32[reg].name;
>  }
>  
> -/* KVM-specific features that are automatically added to all CPU models
> - * when KVM is enabled.
> - */
> -static uint32_t kvm_default_features[FEATURE_WORDS] = {
> -    [FEAT_KVM] = (1 << KVM_FEATURE_CLOCKSOURCE) |
> -        (1 << KVM_FEATURE_NOP_IO_DELAY) |
> -        (1 << KVM_FEATURE_CLOCKSOURCE2) |
> -        (1 << KVM_FEATURE_ASYNC_PF) |
> -        (1 << KVM_FEATURE_STEAL_TIME) |
> -        (1 << KVM_FEATURE_PV_EOI) |
> -        (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT),
> -    [FEAT_1_ECX] = CPUID_EXT_X2APIC,
> -};
> -
> -/* Features that are not added by default to any CPU model when KVM is 
> enabled.
> - */
> -static uint32_t kvm_default_unset_features[FEATURE_WORDS] = {
> -    [FEAT_1_EDX] = CPUID_ACPI,
> -    [FEAT_1_ECX] = CPUID_EXT_MONITOR,
> -    [FEAT_8000_0001_ECX] = CPUID_EXT3_SVM,
> -};
> -
> -void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features)
> -{
> -    kvm_default_features[w] &= ~features;
> -}
> -
> -void x86_cpu_compat_kvm_no_autodisable(FeatureWord w, uint32_t features)
> -{
> -    kvm_default_unset_features[w] &= ~features;
> -}
> -
>  /*
>   * Returns the set of feature flags that are supported and migratable by
>   * QEMU, for a given FeatureWord.
> @@ -1398,6 +1366,43 @@ static X86CPUDefinition builtin_x86_defs[] = {
>      },
>  };
>  
> +typedef struct PropValue {
> +    const char *prop, *value;
> +} PropValue;
> +
> +/* KVM-specific features that are automatically added/removed
> + * from all CPU models when KVM is enabled.
> + */
> +static PropValue kvm_default_props[] = {
> +    { "kvmclock", "on" },
> +    { "kvm-nopiodelay", "on" },
> +    { "kvm-asyncpf", "on" },
> +    { "kvm-steal-time", "on" },
> +    { "kvm-pv-eoi", "on" },
> +    { "kvmclock-stable-bit", "on" },
> +    { "x2apic", "on" },
> +    { "acpi", "off" },
> +    { "monitor", "off" },
> +    { "svm", "off" },
> +    { NULL, NULL },
> +};
> +
> +void x86_cpu_change_kvm_default(const char *prop, const char *value)
> +{
> +    PropValue *pv;
> +    for (pv = kvm_default_props; pv->prop; pv++) {
> +        if (!strcmp(pv->prop, prop)) {
> +            pv->value = value;
> +            break;
> +        }
> +    }
> +
> +    /* It is valid to call this function only for properties that
> +     * are already present in the kvm_default_props table.
> +     */
> +    assert(pv->prop);
> +}
> +
>  static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
>                                                     bool migratable_only);
>  
> @@ -2067,6 +2072,18 @@ static int x86_cpu_filter_features(X86CPU *cpu)
>      return rv;
>  }
>  
> +static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
> +{
> +    PropValue *pv;
> +    for (pv = props; pv->prop; pv++) {
> +        if (!pv->value) {
> +            continue;
> +        }
> +        object_property_parse(OBJECT(cpu), pv->value, pv->prop,
> +                              &error_abort);
> +    }
> +}
> +
>  /* Load data from X86CPUDefinition
>   */
>  static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error 
> **errp)
> @@ -2090,11 +2107,7 @@ static void x86_cpu_load_def(X86CPU *cpu, 
> X86CPUDefinition *def, Error **errp)
>  
>      /* Special cases not set in the X86CPUDefinition structs: */
>      if (kvm_enabled()) {
> -        FeatureWord w;
> -        for (w = 0; w < FEATURE_WORDS; w++) {
> -            env->features[w] |= kvm_default_features[w];
> -            env->features[w] &= ~kvm_default_unset_features[w];
> -        }
> +        x86_cpu_apply_props(cpu, kvm_default_props);
>      }
>  
>      env->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 74b674d..f7c733b 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -1332,8 +1332,15 @@ void cpu_smm_update(X86CPU *cpu);
>  
>  void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
>  
> -void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features);
> -void x86_cpu_compat_kvm_no_autodisable(FeatureWord w, uint32_t features);
> +/* Change the value of a KVM-specific default
> + *
> + * If value is NULL, no default will be set and the original
> + * value from the CPU model table will be kept.
> + *
> + * It is valid to call this funciton only for properties that
> + * are already present in the kvm_default_props table.
> + */
> +void x86_cpu_change_kvm_default(const char *prop, const char *value);
>  
>  
>  /* Return name of 32-bit register, from a R_* constant */
> 

Acked-by: Paolo Bonzini <address@hidden>