| Message ID | 1350918203-25198-17-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 10/22/12 11:03, Igor Mammedov wrote: > Replace setting default supported kvm features in cpu_x86_find_by_name() > by default values in corresponding static properties. > > - Compile in kvm CPUID features only if CONFIG_KVM is defined. > - Make "f-kvm_steal_tm" and "f-kvmclock_stable" CPUID features visible as properties. Not sure that KVM_FEATURE_STEAL_TIME should translate to kvm_steal_tm. I think that I have seen that new names should use "-". So I would think the new name should be "f-kvm-steal-time". And KVM_FEATURE_CLOCKSOURCE_STABLE_BIT should be "f-kvm-clock-stable". > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > target-i386/cpu.c | 30 +++++++++++------------------- > 1 file changed, 11 insertions(+), 19 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index dc4fcdf..407c5ce 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -200,12 +200,16 @@ static Property cpu_x86_properties[] = { > DEFINE_PROP_BIT("f-fma4", X86CPU, env.cpuid_ext3_features, 16, false), > DEFINE_PROP_BIT("f-cvt16", X86CPU, env.cpuid_ext3_features, 18, false), > DEFINE_PROP_BIT("f-nodeid_msr", X86CPU, env.cpuid_ext3_features, 19, false), > - DEFINE_PROP_BIT("f-kvmclock", X86CPU, env.cpuid_kvm_features, 0, false), > - DEFINE_PROP_BIT("f-kvm_nopiodelay", X86CPU, env.cpuid_kvm_features, 1, false), > - DEFINE_PROP_BIT("f-kvm_mmu", X86CPU, env.cpuid_kvm_features, 2, false), > - DEFINE_PROP_BIT("f-kvmclock2", X86CPU, env.cpuid_kvm_features, 3, false), > - DEFINE_PROP_BIT("f-kvm_asyncpf", X86CPU, env.cpuid_kvm_features, 4, false), > - DEFINE_PROP_BIT("f-kvm_pv_eoi", X86CPU, env.cpuid_kvm_features, 6, false), > +#if defined(CONFIG_KVM) > + DEFINE_PROP_BIT("f-kvmclock", X86CPU, env.cpuid_kvm_features, 0, true), > + DEFINE_PROP_BIT("f-kvm_nopiodelay", X86CPU, env.cpuid_kvm_features, 1, true), > + DEFINE_PROP_BIT("f-kvm_mmu", X86CPU, env.cpuid_kvm_features, 2, true), > + DEFINE_PROP_BIT("f-kvmclock2", X86CPU, env.cpuid_kvm_features, 3, true), > + DEFINE_PROP_BIT("f-kvm_asyncpf", X86CPU, env.cpuid_kvm_features, 4, true), > + DEFINE_PROP_BIT("f-kvm_steal_tm", X86CPU, env.cpuid_kvm_features, 5, true), > + DEFINE_PROP_BIT("f-kvm_pv_eoi", X86CPU, env.cpuid_kvm_features, 6, true), > + DEFINE_PROP_BIT("f-kvmclock_stable", X86CPU, env.cpuid_kvm_features, 24, true), > +#endif > DEFINE_PROP_BIT("f-npt", X86CPU, env.cpuid_svm_features, 0, false), > DEFINE_PROP_BIT("f-lbrv", X86CPU, env.cpuid_svm_features, 1, false), > DEFINE_PROP_BIT("f-svm_lock", X86CPU, env.cpuid_svm_features, 2, false), > @@ -1314,7 +1318,7 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > /* Features to be added*/ > uint32_t plus_features = 0, plus_ext_features = env->cpuid_ext_features; > uint32_t plus_ext2_features = 0, plus_ext3_features = 0; > - uint32_t plus_kvm_features = 0, plus_svm_features = 0; > + uint32_t plus_kvm_features = env->cpuid_kvm_features, plus_svm_features = 0; > uint32_t plus_7_0_ebx_features = 0; > /* Features to be removed */ > uint32_t minus_features = 0, minus_ext_features = 0; > @@ -1334,18 +1338,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > memcpy(x86_cpu_def, def, sizeof(*def)); > } > > -#if defined(CONFIG_KVM) > - plus_kvm_features = (1 << KVM_FEATURE_CLOCKSOURCE) | > - (1 << KVM_FEATURE_NOP_IO_DELAY) | > - (1 << KVM_FEATURE_MMU_OP) | > - (1 << KVM_FEATURE_CLOCKSOURCE2) | > - (1 << KVM_FEATURE_ASYNC_PF) | > - (1 << KVM_FEATURE_STEAL_TIME) | > - (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT); > -#else > - plus_kvm_features = 0; > -#endif > - > featurestr = strtok(NULL, ","); > > while (featurestr) { -Don Slutz
On Mon, 22 Oct 2012 19:20:10 -0400 Don Slutz <Don@CloudSwitch.Com> wrote: > On 10/22/12 11:03, Igor Mammedov wrote: > > Replace setting default supported kvm features in cpu_x86_find_by_name() > > by default values in corresponding static properties. > > > > - Compile in kvm CPUID features only if CONFIG_KVM is defined. > > - Make "f-kvm_steal_tm" and "f-kvmclock_stable" CPUID features visible as properties. > Not sure that KVM_FEATURE_STEAL_TIME should translate to kvm_steal_tm. > I think that I have seen that new names should use "-". So I would > think the new name should be "f-kvm-steal-time". And > KVM_FEATURE_CLOCKSOURCE_STABLE_BIT should be "f-kvm-clock-stable". That would be inconsistent with current naming scheme, I may be it should be f-kvm_steal_time and f-kvm_clock_stable for this series and than later all the features converted in a single batch, its a bit out of scope of this patch. Alternatively a separate patch could be send to convert current feature name arrays to '-' scheme. I'll happily rebase series on top of it. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > target-i386/cpu.c | 30 +++++++++++------------------- > > 1 file changed, 11 insertions(+), 19 deletions(-) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index dc4fcdf..407c5ce 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -200,12 +200,16 @@ static Property cpu_x86_properties[] = { > > DEFINE_PROP_BIT("f-fma4", X86CPU, env.cpuid_ext3_features, 16, false), > > DEFINE_PROP_BIT("f-cvt16", X86CPU, env.cpuid_ext3_features, 18, false), > > DEFINE_PROP_BIT("f-nodeid_msr", X86CPU, env.cpuid_ext3_features, 19, false), > > - DEFINE_PROP_BIT("f-kvmclock", X86CPU, env.cpuid_kvm_features, 0, false), > > - DEFINE_PROP_BIT("f-kvm_nopiodelay", X86CPU, env.cpuid_kvm_features, 1, false), > > - DEFINE_PROP_BIT("f-kvm_mmu", X86CPU, env.cpuid_kvm_features, 2, false), > > - DEFINE_PROP_BIT("f-kvmclock2", X86CPU, env.cpuid_kvm_features, 3, false), > > - DEFINE_PROP_BIT("f-kvm_asyncpf", X86CPU, env.cpuid_kvm_features, 4, false), > > - DEFINE_PROP_BIT("f-kvm_pv_eoi", X86CPU, env.cpuid_kvm_features, 6, false), > > +#if defined(CONFIG_KVM) > > + DEFINE_PROP_BIT("f-kvmclock", X86CPU, env.cpuid_kvm_features, 0, true), > > + DEFINE_PROP_BIT("f-kvm_nopiodelay", X86CPU, env.cpuid_kvm_features, 1, true), > > + DEFINE_PROP_BIT("f-kvm_mmu", X86CPU, env.cpuid_kvm_features, 2, true), > > + DEFINE_PROP_BIT("f-kvmclock2", X86CPU, env.cpuid_kvm_features, 3, true), > > + DEFINE_PROP_BIT("f-kvm_asyncpf", X86CPU, env.cpuid_kvm_features, 4, true), > > + DEFINE_PROP_BIT("f-kvm_steal_tm", X86CPU, env.cpuid_kvm_features, 5, true), > > + DEFINE_PROP_BIT("f-kvm_pv_eoi", X86CPU, env.cpuid_kvm_features, 6, true), > > + DEFINE_PROP_BIT("f-kvmclock_stable", X86CPU, env.cpuid_kvm_features, 24, true), > > +#endif > > DEFINE_PROP_BIT("f-npt", X86CPU, env.cpuid_svm_features, 0, false), > > DEFINE_PROP_BIT("f-lbrv", X86CPU, env.cpuid_svm_features, 1, false), > > DEFINE_PROP_BIT("f-svm_lock", X86CPU, env.cpuid_svm_features, 2, false), > > @@ -1314,7 +1318,7 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > > /* Features to be added*/ > > uint32_t plus_features = 0, plus_ext_features = env->cpuid_ext_features; > > uint32_t plus_ext2_features = 0, plus_ext3_features = 0; > > - uint32_t plus_kvm_features = 0, plus_svm_features = 0; > > + uint32_t plus_kvm_features = env->cpuid_kvm_features, plus_svm_features = 0; > > uint32_t plus_7_0_ebx_features = 0; > > /* Features to be removed */ > > uint32_t minus_features = 0, minus_ext_features = 0; > > @@ -1334,18 +1338,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > > memcpy(x86_cpu_def, def, sizeof(*def)); > > } > > > > -#if defined(CONFIG_KVM) > > - plus_kvm_features = (1 << KVM_FEATURE_CLOCKSOURCE) | > > - (1 << KVM_FEATURE_NOP_IO_DELAY) | > > - (1 << KVM_FEATURE_MMU_OP) | > > - (1 << KVM_FEATURE_CLOCKSOURCE2) | > > - (1 << KVM_FEATURE_ASYNC_PF) | > > - (1 << KVM_FEATURE_STEAL_TIME) | > > - (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT); > > -#else > > - plus_kvm_features = 0; > > -#endif > > - > > featurestr = strtok(NULL, ","); > > > > while (featurestr) { > -Don Slutz >
Am 23.10.2012 12:40, schrieb Igor Mammedov: > On Mon, 22 Oct 2012 19:20:10 -0400 > Don Slutz <Don@CloudSwitch.Com> wrote: > >> On 10/22/12 11:03, Igor Mammedov wrote: >>> Replace setting default supported kvm features in cpu_x86_find_by_name() >>> by default values in corresponding static properties. >>> >>> - Compile in kvm CPUID features only if CONFIG_KVM is defined. >>> - Make "f-kvm_steal_tm" and "f-kvmclock_stable" CPUID features visible as properties. >> Not sure that KVM_FEATURE_STEAL_TIME should translate to kvm_steal_tm. >> I think that I have seen that new names should use "-". So I would >> think the new name should be "f-kvm-steal-time". And >> KVM_FEATURE_CLOCKSOURCE_STABLE_BIT should be "f-kvm-clock-stable". > That would be inconsistent with current naming scheme, I may be it should be > f-kvm_steal_time and f-kvm_clock_stable for this series and than later all > the features converted in a single batch, its a bit out of scope of this > patch. Alternatively a separate patch could be send to convert current > feature name arrays to '-' scheme. I'll happily rebase series on top of it. I am pretty sure that someone (not me) proposed to have all new properties with dash and to translate the command line options from _ to -. Andreas
On Tue, 23 Oct 2012 12:55:00 +0200 Andreas Färber <afaerber@suse.de> wrote: > Am 23.10.2012 12:40, schrieb Igor Mammedov: > > On Mon, 22 Oct 2012 19:20:10 -0400 > > Don Slutz <Don@CloudSwitch.Com> wrote: > > > >> On 10/22/12 11:03, Igor Mammedov wrote: > >>> Replace setting default supported kvm features in cpu_x86_find_by_name() > >>> by default values in corresponding static properties. > >>> > >>> - Compile in kvm CPUID features only if CONFIG_KVM is defined. > >>> - Make "f-kvm_steal_tm" and "f-kvmclock_stable" CPUID features visible as properties. > >> Not sure that KVM_FEATURE_STEAL_TIME should translate to kvm_steal_tm. > >> I think that I have seen that new names should use "-". So I would > >> think the new name should be "f-kvm-steal-time". And > >> KVM_FEATURE_CLOCKSOURCE_STABLE_BIT should be "f-kvm-clock-stable". > > That would be inconsistent with current naming scheme, I may be it should be > > f-kvm_steal_time and f-kvm_clock_stable for this series and than later all > > the features converted in a single batch, its a bit out of scope of this > > patch. Alternatively a separate patch could be send to convert current > > feature name arrays to '-' scheme. I'll happily rebase series on top of it. > > I am pretty sure that someone (not me) proposed to have all new > properties with dash and to translate the command line options from _ to -. I remember it too except of how it ended. Any way since these are new features, I do not have any preference. Naming suggested by Don are now in WIP queue: https://github.com/imammedo/qemu/commit/fd4fc7b5abb37373d02e3f1861856846ce231316 Thanks, Igor > > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
On 22/10/2012, at 17:03, Igor Mammedov <imammedo@redhat.com> wrote: > Replace setting default supported kvm features in cpu_x86_find_by_name() > by default values in corresponding static properties. > > - Compile in kvm CPUID features only if CONFIG_KVM is defined. > - Make "f-kvm_steal_tm" and "f-kvmclock_stable" CPUID features visible as properties. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > target-i386/cpu.c | 30 +++++++++++------------------- > 1 file changed, 11 insertions(+), 19 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index dc4fcdf..407c5ce 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -200,12 +200,16 @@ static Property cpu_x86_properties[] = { > DEFINE_PROP_BIT("f-fma4", X86CPU, env.cpuid_ext3_features, 16, false), > DEFINE_PROP_BIT("f-cvt16", X86CPU, env.cpuid_ext3_features, 18, false), > DEFINE_PROP_BIT("f-nodeid_msr", X86CPU, env.cpuid_ext3_features, 19, false), > - DEFINE_PROP_BIT("f-kvmclock", X86CPU, env.cpuid_kvm_features, 0, false), > - DEFINE_PROP_BIT("f-kvm_nopiodelay", X86CPU, env.cpuid_kvm_features, 1, false), > - DEFINE_PROP_BIT("f-kvm_mmu", X86CPU, env.cpuid_kvm_features, 2, false), > - DEFINE_PROP_BIT("f-kvmclock2", X86CPU, env.cpuid_kvm_features, 3, false), > - DEFINE_PROP_BIT("f-kvm_asyncpf", X86CPU, env.cpuid_kvm_features, 4, false), > - DEFINE_PROP_BIT("f-kvm_pv_eoi", X86CPU, env.cpuid_kvm_features, 6, false), > +#if defined(CONFIG_KVM) > + DEFINE_PROP_BIT("f-kvmclock", X86CPU, env.cpuid_kvm_features, 0, true), > + DEFINE_PROP_BIT("f-kvm_nopiodelay", X86CPU, env.cpuid_kvm_features, 1, true), > + DEFINE_PROP_BIT("f-kvm_mmu", X86CPU, env.cpuid_kvm_features, 2, true), > + DEFINE_PROP_BIT("f-kvmclock2", X86CPU, env.cpuid_kvm_features, 3, true), > + DEFINE_PROP_BIT("f-kvm_asyncpf", X86CPU, env.cpuid_kvm_features, 4, true), > + DEFINE_PROP_BIT("f-kvm_steal_tm", X86CPU, env.cpuid_kvm_features, 5, true), > + DEFINE_PROP_BIT("f-kvm_pv_eoi", X86CPU, env.cpuid_kvm_features, 6, true), > + DEFINE_PROP_BIT("f-kvmclock_stable", X86CPU, env.cpuid_kvm_features, 24, true), > +#endif > DEFINE_PROP_BIT("f-npt", X86CPU, env.cpuid_svm_features, 0, false), > DEFINE_PROP_BIT("f-lbrv", X86CPU, env.cpuid_svm_features, 1, false), > DEFINE_PROP_BIT("f-svm_lock", X86CPU, env.cpuid_svm_features, 2, false), > @@ -1314,7 +1318,7 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > /* Features to be added*/ > uint32_t plus_features = 0, plus_ext_features = env->cpuid_ext_features; > uint32_t plus_ext2_features = 0, plus_ext3_features = 0; > - uint32_t plus_kvm_features = 0, plus_svm_features = 0; > + uint32_t plus_kvm_features = env->cpuid_kvm_features, plus_svm_features = 0; This is where things gets interesting: now the PV EOI feature is enabled by default, but only on the pc-1.3 machine-type. Maybe at this point in the series we are already able to use global properties for compatibility, so we can make the changes above, add "f-kvm_pv_eoi=false" to pc-1.2, and kill kvm_default_features and enable_kvm_pv_eoi(), all in the same patch? > uint32_t plus_7_0_ebx_features = 0; > /* Features to be removed */ > uint32_t minus_features = 0, minus_ext_features = 0; > @@ -1334,18 +1338,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > memcpy(x86_cpu_def, def, sizeof(*def)); > } > > -#if defined(CONFIG_KVM) > - plus_kvm_features = (1 << KVM_FEATURE_CLOCKSOURCE) | > - (1 << KVM_FEATURE_NOP_IO_DELAY) | > - (1 << KVM_FEATURE_MMU_OP) | > - (1 << KVM_FEATURE_CLOCKSOURCE2) | > - (1 << KVM_FEATURE_ASYNC_PF) | > - (1 << KVM_FEATURE_STEAL_TIME) | > - (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT); > -#else > - plus_kvm_features = 0; > -#endif > - > featurestr = strtok(NULL, ","); > > while (featurestr) { > -- > 1.7.11.7 > >
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index dc4fcdf..407c5ce 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -200,12 +200,16 @@ static Property cpu_x86_properties[] = { DEFINE_PROP_BIT("f-fma4", X86CPU, env.cpuid_ext3_features, 16, false), DEFINE_PROP_BIT("f-cvt16", X86CPU, env.cpuid_ext3_features, 18, false), DEFINE_PROP_BIT("f-nodeid_msr", X86CPU, env.cpuid_ext3_features, 19, false), - DEFINE_PROP_BIT("f-kvmclock", X86CPU, env.cpuid_kvm_features, 0, false), - DEFINE_PROP_BIT("f-kvm_nopiodelay", X86CPU, env.cpuid_kvm_features, 1, false), - DEFINE_PROP_BIT("f-kvm_mmu", X86CPU, env.cpuid_kvm_features, 2, false), - DEFINE_PROP_BIT("f-kvmclock2", X86CPU, env.cpuid_kvm_features, 3, false), - DEFINE_PROP_BIT("f-kvm_asyncpf", X86CPU, env.cpuid_kvm_features, 4, false), - DEFINE_PROP_BIT("f-kvm_pv_eoi", X86CPU, env.cpuid_kvm_features, 6, false), +#if defined(CONFIG_KVM) + DEFINE_PROP_BIT("f-kvmclock", X86CPU, env.cpuid_kvm_features, 0, true), + DEFINE_PROP_BIT("f-kvm_nopiodelay", X86CPU, env.cpuid_kvm_features, 1, true), + DEFINE_PROP_BIT("f-kvm_mmu", X86CPU, env.cpuid_kvm_features, 2, true), + DEFINE_PROP_BIT("f-kvmclock2", X86CPU, env.cpuid_kvm_features, 3, true), + DEFINE_PROP_BIT("f-kvm_asyncpf", X86CPU, env.cpuid_kvm_features, 4, true), + DEFINE_PROP_BIT("f-kvm_steal_tm", X86CPU, env.cpuid_kvm_features, 5, true), + DEFINE_PROP_BIT("f-kvm_pv_eoi", X86CPU, env.cpuid_kvm_features, 6, true), + DEFINE_PROP_BIT("f-kvmclock_stable", X86CPU, env.cpuid_kvm_features, 24, true), +#endif DEFINE_PROP_BIT("f-npt", X86CPU, env.cpuid_svm_features, 0, false), DEFINE_PROP_BIT("f-lbrv", X86CPU, env.cpuid_svm_features, 1, false), DEFINE_PROP_BIT("f-svm_lock", X86CPU, env.cpuid_svm_features, 2, false), @@ -1314,7 +1318,7 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, /* Features to be added*/ uint32_t plus_features = 0, plus_ext_features = env->cpuid_ext_features; uint32_t plus_ext2_features = 0, plus_ext3_features = 0; - uint32_t plus_kvm_features = 0, plus_svm_features = 0; + uint32_t plus_kvm_features = env->cpuid_kvm_features, plus_svm_features = 0; uint32_t plus_7_0_ebx_features = 0; /* Features to be removed */ uint32_t minus_features = 0, minus_ext_features = 0; @@ -1334,18 +1338,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, memcpy(x86_cpu_def, def, sizeof(*def)); } -#if defined(CONFIG_KVM) - plus_kvm_features = (1 << KVM_FEATURE_CLOCKSOURCE) | - (1 << KVM_FEATURE_NOP_IO_DELAY) | - (1 << KVM_FEATURE_MMU_OP) | - (1 << KVM_FEATURE_CLOCKSOURCE2) | - (1 << KVM_FEATURE_ASYNC_PF) | - (1 << KVM_FEATURE_STEAL_TIME) | - (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT); -#else - plus_kvm_features = 0; -#endif - featurestr = strtok(NULL, ","); while (featurestr) {
Replace setting default supported kvm features in cpu_x86_find_by_name() by default values in corresponding static properties. - Compile in kvm CPUID features only if CONFIG_KVM is defined. - Make "f-kvm_steal_tm" and "f-kvmclock_stable" CPUID features visible as properties. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- target-i386/cpu.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-)