On Thu, Aug 14, 2014 at 04:26:00PM -0300, Eduardo Habkost wrote:
> With the new fields, the x86_cpu_compat_disable_kvm_features() calls on
> pc_compat_*() functions can be replace
be replaced
> by simple field initialization on
> class_init functions. This gets us one step closer to eliminating all
> pc_compat_*() functions.
>
> Those new fields may eventually become simple compat_props properties,
> if we QOMify the accelerator code, and move the KVM-specific CPUID code
> to a x86-kvm-accel class.
>
> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> ---
> hw/i386/pc.c | 15 +++++++++++++++
> hw/i386/pc_piix.c | 7 ++++---
> hw/i386/pc_q35.c | 2 +-
> include/hw/i386/pc.h | 3 +++
> target-i386/cpu.c | 25 ++++++++++++-------------
> target-i386/cpu.h | 2 +-
> 6 files changed, 36 insertions(+), 18 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index dba7f62..a0eaaf6 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -24,6 +24,7 @@
> #include "hw/hw.h"
> #include "hw/i386/pc.h"
> #include "hw/char/serial.h"
> +#include "hw/i386/cpu.h"
> #include "hw/i386/apic.h"
> #include "hw/block/fdc.h"
> #include "hw/ide.h"
> @@ -1020,6 +1021,10 @@ void pc_cpus_init(PCMachineState *pcms, DeviceState
> *icc_bridge)
> Error *error = NULL;
> unsigned long apic_id_limit;
> MachineState *machine = MACHINE(pcms);
> + PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> +
> + x86_cpu_set_kvm_defaults(pcmc->kvm_default_features,
> + pcmc->kvm_default_unset_features);
>
> /* init CPUs */
> current_cpu_model = machine->cpu_model;
> @@ -1687,6 +1692,16 @@ static void pc_machine_class_init(ObjectClass *oc,
> void *data)
> pcmc->gigabyte_align = true;
> pcmc->has_reserved_memory = true;
> pcmc->kvmclock_enabled = true;
> + pcmc->kvm_default_features[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),
> + pcmc->kvm_default_features[FEAT_1_ECX] = CPUID_EXT_X2APIC,
> + pcmc->kvm_default_unset_features[FEAT_1_ECX] = CPUID_EXT_MONITOR,
>
> mc->get_hotplug_handler = pc_get_hotpug_handler;
> mc->default_boot_order = "cad";
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 988583c..9ec63d5 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -298,7 +298,6 @@ static void pc_compat_2_0(MachineState *machine)
> static void pc_compat_1_7(MachineState *machine)
> {
> pc_compat_2_0(machine);
> - x86_cpu_compat_disable_kvm_features(FEAT_1_ECX, CPUID_EXT_X2APIC);
> }
>
> static void pc_compat_1_6(MachineState *machine)
> @@ -325,7 +324,6 @@ static void pc_compat_1_3(MachineState *machine)
> static void pc_compat_1_2(MachineState *machine)
> {
> pc_compat_1_3(machine);
> - x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
> }
>
> /* PC compat function for pc-0.10 to pc-0.13 */
> @@ -386,7 +384,6 @@ static void pc_init_pci_no_kvmclock(MachineState *machine)
>
> static void pc_init_isa(MachineState *machine)
> {
> - x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
> pc_init1(machine);
> }
>
> @@ -494,6 +491,7 @@ static void pc_i440fx_machine_v1_7_class_init(ObjectClass
> *oc, void *data)
> pcmc->smbios_defaults = false;
> pcmc->gigabyte_align = false;
> pcmc->legacy_acpi_table_size = 6414;
> + pcmc->kvm_default_features[FEAT_1_ECX] &= ~CPUID_EXT_X2APIC;
> }
>
> static const TypeInfo pc_i440fx_machine_v1_7_type_info = {
> @@ -633,6 +631,7 @@ static const TypeInfo pc_machine_v1_3_type_info = {
> static void pc_machine_v1_2_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> + PCMachineClass *pcmc = PC_MACHINE_CLASS(oc);
> static GlobalProperty compat_props[] = {
> PC_COMPAT_1_2,
> { /* end of list */ }
> @@ -641,6 +640,7 @@ static void pc_machine_v1_2_class_init(ObjectClass *oc,
> void *data)
> mc->init = pc_init_pci_1_2;
> mc->name = "pc-1.2";
> machine_class_add_compat_props(mc, compat_props);
> + pcmc->kvm_default_features[FEAT_KVM] &= ~KVM_FEATURE_PV_EOI;
> }
>
> static const TypeInfo pc_machine_v1_2_type_info = {
> @@ -984,6 +984,7 @@ static void isapc_machine_class_init(ObjectClass *oc,
> void *data)
> pcmc->smbios_legacy_mode = true;
> pcmc->has_reserved_memory = false;
> pcmc->compat_apic_id_mode = true;
> + pcmc->kvm_default_features[FEAT_KVM] &= ~KVM_FEATURE_PV_EOI;
> }
>
> static const TypeInfo isapc_machine_type_info = {
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 2ebd598..5878964 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -271,7 +271,6 @@ static void pc_compat_2_0(MachineState *machine)
> static void pc_compat_1_7(MachineState *machine)
> {
> pc_compat_2_0(machine);
> - x86_cpu_compat_disable_kvm_features(FEAT_1_ECX, CPUID_EXT_X2APIC);
> }
>
> static void pc_compat_1_6(MachineState *machine)
> @@ -389,6 +388,7 @@ static void pc_q35_machine_v1_7_class_init(ObjectClass
> *oc, void *data)
> mc->name = "pc-q35-1.7";
> pcmc->smbios_defaults = false;
> pcmc->gigabyte_align = false;
> + pcmc->kvm_default_features[FEAT_1_ECX] &= ~CPUID_EXT_X2APIC;
> }
>
> static TypeInfo pc_q35_machine_v1_7_type_info = {
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 460ff21..b7f36d6 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -8,6 +8,7 @@
> #include "hw/block/fdc.h"
> #include "net/net.h"
> #include "hw/i386/ioapic.h"
> +#include "hw/i386/cpu.h"
> #include "hw/boards.h"
>
> #include "qemu/range.h"
> @@ -66,6 +67,8 @@ struct PCMachineClass {
> bool has_reserved_memory;
> bool kvmclock_enabled;
> bool compat_apic_id_mode;
> + uint32_t kvm_default_features[FEATURE_WORDS];
> + uint32_t kvm_default_unset_features[FEATURE_WORDS];
> };
>
> typedef struct PCMachineState PCMachineState;
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index e86f99e..e02b0d9 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -446,27 +446,26 @@ typedef struct model_features_t {
>
> /* KVM-specific features that are automatically added to all CPU models
> * when KVM is enabled.
> + *
> + * The default is to enable all KVM features, but machine-types may override
> it
> + * to keep a stable ABI. See x86_cpu_set_kvm_defaults().
> */
> 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,
> + [FEAT_KVM] = ~0,
> };
>
> /* 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_ECX] = CPUID_EXT_MONITOR,
> -};
> +static uint32_t kvm_default_unset_features[FEATURE_WORDS];
>
> -void x86_cpu_compat_disable_kvm_features(FeatureWord w, uint32_t features)
> +/* Override default-enabled and default-disabled KVM features. Used by
> + * machine-type code to ensure a stable ABI.
> + */
> +void x86_cpu_set_kvm_defaults(uint32_t *default_set, uint32_t *default_unset)
> {
> - kvm_default_features[w] &= ~features;
> + memcpy(kvm_default_features, default_set, sizeof(kvm_default_features));
> + memcpy(kvm_default_unset_features, default_unset,
> + sizeof(kvm_default_unset_features));
> }
>
> /*
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 3f299d7..2138996 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -741,7 +741,7 @@ void do_smm_enter(X86CPU *cpu);
>
> void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
>
> -void x86_cpu_compat_disable_kvm_features(FeatureWord w, uint32_t features);
> +void x86_cpu_set_kvm_defaults(uint32_t *default_set, uint32_t
> *default_unset);
>
>
> /* Return name of 32-bit register, from a R_* constant */
> --
> 1.9.3
