Message ID | 1475183699-11646-6-git-send-email-ehabkost@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Thu, Sep 29, 2016 at 18:14:53 -0300, Eduardo Habkost wrote: > Instead of translating the feature name entries when adding > property names, store the actual property names in the feature > name array. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > Changes series v3 -> v4: > * New patch added to series > --- > target-i386/cpu.c | 31 ++++++++++++++++--------------- > 1 file changed, 16 insertions(+), 15 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 4eaec0e..7795a7c 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -279,11 +279,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { > [FEAT_1_ECX] = { > .feat_names = { > "pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64", "monitor", > - "ds_cpl", "vmx", "smx", "est", > + "ds-cpl", "vmx", "smx", "est", > "tm2", "ssse3", "cid", NULL, > "fma", "cx16", "xtpr", "pdcm", > - NULL, "pcid", "dca", "sse4.1|sse4_1", > - "sse4.2|sse4_2", "x2apic", "movbe", "popcnt", > + NULL, "pcid", "dca", "sse4.1|sse4-1", > + "sse4.2|sse4-2", "x2apic", "movbe", "popcnt", > "tsc-deadline", "aes", "xsave", "osxsave", > "avx", "f16c", "rdrand", "hypervisor", > }, It wasn't quite obvious to me where this means we can't use the names with underscores when talking to QEMU. So I tried it and apparently underscores are just silently translated to dashes. It's backward compatible this way. However, QEMU will always give us the names with dashes, which means we have even more differences between libvirt's feature names and QEMU's feature names. So assuming we'll have an interface for querying supported CPU properties (and their aliases), shouldn't the old underscore names be added as aliases? This way, we could actually know that "ds-cpl" means "ds_cpl". Jirka
On Fri, Sep 30, 2016 at 09:29:58AM +0200, Jiri Denemark wrote: > On Thu, Sep 29, 2016 at 18:14:53 -0300, Eduardo Habkost wrote: > > Instead of translating the feature name entries when adding > > property names, store the actual property names in the feature > > name array. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > Changes series v3 -> v4: > > * New patch added to series > > --- > > target-i386/cpu.c | 31 ++++++++++++++++--------------- > > 1 file changed, 16 insertions(+), 15 deletions(-) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 4eaec0e..7795a7c 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -279,11 +279,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { > > [FEAT_1_ECX] = { > > .feat_names = { > > "pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64", "monitor", > > - "ds_cpl", "vmx", "smx", "est", > > + "ds-cpl", "vmx", "smx", "est", > > "tm2", "ssse3", "cid", NULL, > > "fma", "cx16", "xtpr", "pdcm", > > - NULL, "pcid", "dca", "sse4.1|sse4_1", > > - "sse4.2|sse4_2", "x2apic", "movbe", "popcnt", > > + NULL, "pcid", "dca", "sse4.1|sse4-1", > > + "sse4.2|sse4-2", "x2apic", "movbe", "popcnt", > > "tsc-deadline", "aes", "xsave", "osxsave", > > "avx", "f16c", "rdrand", "hypervisor", > > }, > > It wasn't quite obvious to me where this means we can't use the names > with underscores when talking to QEMU. So I tried it and apparently > underscores are just silently translated to dashes. It's backward > compatible this way. However, QEMU will always give us the names with > dashes, which means we have even more differences between libvirt's > feature names and QEMU's feature names. So assuming we'll have an > interface for querying supported CPU properties (and their aliases), > shouldn't the old underscore names be added as aliases? This way, we > could actually know that "ds-cpl" means "ds_cpl". Good idea. Instead of translating property names silently we can add aliases for the ones where underscores were actually used. I will look into it.
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 4eaec0e..7795a7c 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -279,11 +279,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { [FEAT_1_ECX] = { .feat_names = { "pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64", "monitor", - "ds_cpl", "vmx", "smx", "est", + "ds-cpl", "vmx", "smx", "est", "tm2", "ssse3", "cid", NULL, "fma", "cx16", "xtpr", "pdcm", - NULL, "pcid", "dca", "sse4.1|sse4_1", - "sse4.2|sse4_2", "x2apic", "movbe", "popcnt", + NULL, "pcid", "dca", "sse4.1|sse4-1", + "sse4.2|sse4-2", "x2apic", "movbe", "popcnt", "tsc-deadline", "aes", "xsave", "osxsave", "avx", "f16c", "rdrand", "hypervisor", }, @@ -303,7 +303,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */, NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */, "nx|xd", NULL, "mmxext", NULL /* mmx */, - NULL /* fxsr */, "fxsr_opt|ffxsr", "pdpe1gb", "rdtscp", + NULL /* fxsr */, "fxsr-opt|ffxsr", "pdpe1gb", "rdtscp", NULL, "lm|i64", "3dnowext", "3dnow", }, .cpuid_eax = 0x80000001, .cpuid_reg = R_EDX, @@ -311,13 +311,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { }, [FEAT_8000_0001_ECX] = { .feat_names = { - "lahf_lm", "cmp_legacy", "svm", "extapic", + "lahf-lm", "cmp-legacy", "svm", "extapic", "cr8legacy", "abm", "sse4a", "misalignsse", "3dnowprefetch", "osvw", "ibs", "xop", "skinit", "wdt", NULL, "lwp", - "fma4", "tce", NULL, "nodeid_msr", - NULL, "tbm", "topoext", "perfctr_core", - "perfctr_nb", NULL, NULL, NULL, + "fma4", "tce", NULL, "nodeid-msr", + NULL, "tbm", "topoext", "perfctr-core", + "perfctr-nb", NULL, NULL, NULL, NULL, NULL, NULL, NULL, }, .cpuid_eax = 0x80000001, .cpuid_reg = R_ECX, @@ -339,8 +339,8 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { }, [FEAT_KVM] = { .feat_names = { - "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock", - "kvm_asyncpf", "kvm_steal_time", "kvm_pv_eoi", "kvm_pv_unhalt", + "kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock", + "kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, @@ -400,9 +400,9 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { }, [FEAT_SVM] = { .feat_names = { - "npt", "lbrv", "svm_lock", "nrip_save", - "tsc_scale", "vmcb_clean", "flushbyasid", "decodeassists", - NULL, NULL, "pause_filter", NULL, + "npt", "lbrv", "svm-lock", "nrip-save", + "tsc-scale", "vmcb-clean", "flushbyasid", "decodeassists", + NULL, NULL, "pause-filter", NULL, "pfthreshold", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, @@ -414,7 +414,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { }, [FEAT_7_0_EBX] = { .feat_names = { - "fsgsbase", "tsc_adjust", NULL, "bmi1", + "fsgsbase", "tsc-adjust", NULL, "bmi1", "hle", "avx2", NULL, "smep", "bmi2", "erms", "invpcid", "rtm", NULL, NULL, "mpx", NULL, @@ -3334,7 +3334,8 @@ static void x86_cpu_register_feature_bit_props(X86CPU *cpu, names = g_strsplit(fi->feat_names[bitnr], "|", 0); - feat2prop(names[0]); + /* Property names should use "-" instead of "_" */ + assert(!strchr(names[0], '_')); x86_cpu_register_bit_prop(cpu, names[0], &cpu->env.features[w], bitnr); for (i = 1; names[i]; i++) {
Instead of translating the feature name entries when adding property names, store the actual property names in the feature name array. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes series v3 -> v4: * New patch added to series --- target-i386/cpu.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)