Re: [Qemu-devel] [PATCH v4 05/11] target-i386: Remove underscores from property names

2016-09-30 Thread Eduardo Habkost 
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 
> > ---
> > 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.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v4 05/11] target-i386: Remove underscores from property names

2016-09-30 Thread Jiri Denemark 
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 
> ---
> 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

[Qemu-devel] [PATCH v4 05/11] target-i386: Remove underscores from property names

2016-09-29 Thread Eduardo Habkost 
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 
---
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",
 },
@@ -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 = 0x8001, .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 = 0x8001, .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], >env.features[w], bitnr);
 
 for (i = 1; names[i]; i++) {
-- 
2.7.4