[PATCH] target-i386: Improve x86_cpu_list output

[PATCH] target-i386: Improve x86_cpu_list output

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]

From: Jan Kiszka <jan.kiszka@siemens.com>

Several issues fixed:
 - We were missing a bunch of feature lists. Fix this by simply dumping
   the meta list feature_word_info.
 - kvm_enabled() cannot be true at this point because accelerators are
   initialized much later during init. Simply dump unconditionally.
 - Add explanation for "host" CPU type.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 target-i386/cpu.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index dfcf86e..6e742f0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1453,18 +1453,16 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
         snprintf(buf, sizeof(buf), "%s", def->name);
         (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
     }
-    if (kvm_enabled()) {
-        (*cpu_fprintf)(f, "x86 %16s\n", "[host]");
-    }
+    (*cpu_fprintf)(f, "x86 %16s  %-48s\n", "host",
+                   "KVM processor with all supported host features");
+
     (*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
-    listflags(buf, sizeof(buf), (uint32_t)~0, feature_name, 1);
-    (*cpu_fprintf)(f, "  %s\n", buf);
-    listflags(buf, sizeof(buf), (uint32_t)~0, ext_feature_name, 1);
-    (*cpu_fprintf)(f, "  %s\n", buf);
-    listflags(buf, sizeof(buf), (uint32_t)~0, ext2_feature_name, 1);
-    (*cpu_fprintf)(f, "  %s\n", buf);
-    listflags(buf, sizeof(buf), (uint32_t)~0, ext3_feature_name, 1);
-    (*cpu_fprintf)(f, "  %s\n", buf);
+    for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
+        FeatureWordInfo *fw = &feature_word_info[i];
+
+        listflags(buf, sizeof(buf), (uint32_t)~0, fw->feat_names, 1);
+        (*cpu_fprintf)(f, "  %s\n", buf);
+    }
 }
 
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
-- 
1.7.3.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH] target-i386: Improve x86_cpu_list output

[Eduardo Habkost]

On Sat, Feb 23, 2013 at 04:45:00PM +0100, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Several issues fixed:
>  - We were missing a bunch of feature lists. Fix this by simply dumping
>    the meta list feature_word_info.
>  - kvm_enabled() cannot be true at this point because accelerators are
>    initialized much later during init. Simply dump unconditionally.
>  - Add explanation for "host" CPU type.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  target-i386/cpu.c |   20 +++++++++-----------
>  1 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index dfcf86e..6e742f0 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1453,18 +1453,16 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>          snprintf(buf, sizeof(buf), "%s", def->name);
>          (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
>      }
> -    if (kvm_enabled()) {
> -        (*cpu_fprintf)(f, "x86 %16s\n", "[host]");
> -    }
> +    (*cpu_fprintf)(f, "x86 %16s  %-48s\n", "host",
> +                   "KVM processor with all supported host features");
> +
>      (*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
> -    listflags(buf, sizeof(buf), (uint32_t)~0, feature_name, 1);
> -    (*cpu_fprintf)(f, "  %s\n", buf);
> -    listflags(buf, sizeof(buf), (uint32_t)~0, ext_feature_name, 1);
> -    (*cpu_fprintf)(f, "  %s\n", buf);
> -    listflags(buf, sizeof(buf), (uint32_t)~0, ext2_feature_name, 1);
> -    (*cpu_fprintf)(f, "  %s\n", buf);
> -    listflags(buf, sizeof(buf), (uint32_t)~0, ext3_feature_name, 1);
> -    (*cpu_fprintf)(f, "  %s\n", buf);
> +    for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
> +        FeatureWordInfo *fw = &feature_word_info[i];
> +
> +        listflags(buf, sizeof(buf), (uint32_t)~0, fw->feat_names, 1);

To be done later: we can kill the 'fbits' parameter on listflags(). All
callers are using ~0, now (I don't even remember why the parameter
existed).

I was planning to keep the posibility of having FeatureWordInfo entries
without feat_names set. For example, we may want to make a specific
feature word supported by "-cpu host" before we make each bit
configurable individually in the command-line.

On the other hand, requiring feat_names to be always set would help
making the table more explicit. If we really want it (which I doubt), we
can have the same effect of feat_names==NULL by simply having a feature
name array with only NULL entries. So:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> +        (*cpu_fprintf)(f, "  %s\n", buf);
> +    }
>  }
>  
>  CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> -- 
> 1.7.3.4
> 



-- 
Eduardo

Re: [PATCH] target-i386: Improve x86_cpu_list output

[Igor Mammedov]

On Sat, 23 Feb 2013 16:45:00 +0100
Jan Kiszka <jan.kiszka@web.de> wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Several issues fixed:
>  - We were missing a bunch of feature lists. Fix this by simply dumping
>    the meta list feature_word_info.
>  - kvm_enabled() cannot be true at this point because accelerators are
>    initialized much later during init. Simply dump unconditionally.
Why not to move list_cpu after accelerators are initialized?

>  - Add explanation for "host" CPU type.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  target-i386/cpu.c |   20 +++++++++-----------
>  1 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index dfcf86e..6e742f0 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1453,18 +1453,16 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>          snprintf(buf, sizeof(buf), "%s", def->name);
>          (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
>      }
> -    if (kvm_enabled()) {
> -        (*cpu_fprintf)(f, "x86 %16s\n", "[host]");
> -    }
> +    (*cpu_fprintf)(f, "x86 %16s  %-48s\n", "host",
> +                   "KVM processor with all supported host features");
> +
that would make 'host' visible to users even if QEMU compiled without KVM
support. No big harm, but autotest could get confused when it gets 'host' CPU
but QEMU doesn't run because it's not really supported.

>      (*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
> -    listflags(buf, sizeof(buf), (uint32_t)~0, feature_name, 1);
> -    (*cpu_fprintf)(f, "  %s\n", buf);
> -    listflags(buf, sizeof(buf), (uint32_t)~0, ext_feature_name, 1);
> -    (*cpu_fprintf)(f, "  %s\n", buf);
> -    listflags(buf, sizeof(buf), (uint32_t)~0, ext2_feature_name, 1);
> -    (*cpu_fprintf)(f, "  %s\n", buf);
> -    listflags(buf, sizeof(buf), (uint32_t)~0, ext3_feature_name, 1);
> -    (*cpu_fprintf)(f, "  %s\n", buf);
> +    for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
> +        FeatureWordInfo *fw = &feature_word_info[i];
> +
> +        listflags(buf, sizeof(buf), (uint32_t)~0, fw->feat_names, 1);
> +        (*cpu_fprintf)(f, "  %s\n", buf);
> +    }
>  }
>  
>  CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> -- 
> 1.7.3.4
> 


-- 
Regards,
  Igor

Re: [PATCH] target-i386: Improve x86_cpu_list output

[Eduardo Habkost]

On Tue, Feb 26, 2013 at 10:57:56PM +0100, Igor Mammedov wrote:
> On Sat, 23 Feb 2013 16:45:00 +0100
> Jan Kiszka <jan.kiszka@web.de> wrote:
> 
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > Several issues fixed:
> >  - We were missing a bunch of feature lists. Fix this by simply dumping
> >    the meta list feature_word_info.
> >  - kvm_enabled() cannot be true at this point because accelerators are
> >    initialized much later during init. Simply dump unconditionally.
> Why not to move list_cpu after accelerators are initialized?

Because help output is simply documentation and shouldn't depend on any
other config option parsing or accelerator initialization at all?

> 
> >  - Add explanation for "host" CPU type.
> > 
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > ---
> >  target-i386/cpu.c |   20 +++++++++-----------
> >  1 files changed, 9 insertions(+), 11 deletions(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index dfcf86e..6e742f0 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1453,18 +1453,16 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> >          snprintf(buf, sizeof(buf), "%s", def->name);
> >          (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
> >      }
> > -    if (kvm_enabled()) {
> > -        (*cpu_fprintf)(f, "x86 %16s\n", "[host]");
> > -    }
> > +    (*cpu_fprintf)(f, "x86 %16s  %-48s\n", "host",
> > +                   "KVM processor with all supported host features");
> > +
> that would make 'host' visible to users even if QEMU compiled without KVM
> support. No big harm, but autotest could get confused when it gets 'host' CPU
> but QEMU doesn't run because it's not really supported.

Then we have to fix the autotest test code to not try it without KVM.
:-)

Help output is not a probing mechanism (although we often misuse it as
if it were), and I expect help output to be static and not depend on any
subsystem initialization.

For example: an user without permission to open /dev/kvm should be still
able to see help text explaining that "-cpu host" is available when KVM
is enabled and working.


> 
> >      (*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
> > -    listflags(buf, sizeof(buf), (uint32_t)~0, feature_name, 1);
> > -    (*cpu_fprintf)(f, "  %s\n", buf);
> > -    listflags(buf, sizeof(buf), (uint32_t)~0, ext_feature_name, 1);
> > -    (*cpu_fprintf)(f, "  %s\n", buf);
> > -    listflags(buf, sizeof(buf), (uint32_t)~0, ext2_feature_name, 1);
> > -    (*cpu_fprintf)(f, "  %s\n", buf);
> > -    listflags(buf, sizeof(buf), (uint32_t)~0, ext3_feature_name, 1);
> > -    (*cpu_fprintf)(f, "  %s\n", buf);
> > +    for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
> > +        FeatureWordInfo *fw = &feature_word_info[i];
> > +
> > +        listflags(buf, sizeof(buf), (uint32_t)~0, fw->feat_names, 1);
> > +        (*cpu_fprintf)(f, "  %s\n", buf);
> > +    }
> >  }
> >  
> >  CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> > -- 
> > 1.7.3.4
> > 
> 
> 
> -- 
> Regards,
>   Igor

-- 
Eduardo

Re: [PATCH] target-i386: Improve x86_cpu_list output

[Igor Mammedov]

On Wed, 27 Feb 2013 00:26:38 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Tue, Feb 26, 2013 at 10:57:56PM +0100, Igor Mammedov wrote:
> > On Sat, 23 Feb 2013 16:45:00 +0100
> > Jan Kiszka <jan.kiszka@web.de> wrote:
> > 
> > > From: Jan Kiszka <jan.kiszka@siemens.com>
> > > 
> > > Several issues fixed:
> > >  - We were missing a bunch of feature lists. Fix this by simply dumping
> > >    the meta list feature_word_info.
> > >  - kvm_enabled() cannot be true at this point because accelerators are
> > >    initialized much later during init. Simply dump unconditionally.
> > Why not to move list_cpu after accelerators are initialized?
> 
> Because help output is simply documentation and shouldn't depend on any
> other config option parsing or accelerator initialization at all?
Don't see reason why it shouldn't.
It's not a man page but a program and can do pretty much everything.

> 
> > 
> > >  - Add explanation for "host" CPU type.
> > > 
> > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > ---
> > >  target-i386/cpu.c |   20 +++++++++-----------
> > >  1 files changed, 9 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > > index dfcf86e..6e742f0 100644
> > > --- a/target-i386/cpu.c
> > > +++ b/target-i386/cpu.c
> > > @@ -1453,18 +1453,16 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> > >          snprintf(buf, sizeof(buf), "%s", def->name);
> > >          (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
> > >      }
> > > -    if (kvm_enabled()) {
> > > -        (*cpu_fprintf)(f, "x86 %16s\n", "[host]");
> > > -    }
> > > +    (*cpu_fprintf)(f, "x86 %16s  %-48s\n", "host",
> > > +                   "KVM processor with all supported host features");
> > > +
> > that would make 'host' visible to users even if QEMU compiled without KVM
> > support. No big harm, but autotest could get confused when it gets 'host' CPU
> > but QEMU doesn't run because it's not really supported.
> 
> Then we have to fix the autotest test code to not try it without KVM.
> :-)
> 
> Help output is not a probing mechanism (although we often misuse it as
> if it were), and I expect help output to be static and not depend on any
> subsystem initialization.
Then fix help output and add to "host" line something like " is available with
-enable-kvm on command line and if your build was compiled --enable-kvm
configure option", otherwise 'host' is misleading.
Now even without 'host' in output of -cpu 'help', question why 'host' is not
found periodically pops up on IRC. This change will just increase frequency of
it.

> 
> For example: an user without permission to open /dev/kvm should be still
> able to see help text explaining that "-cpu host" is available when KVM
> is enabled and working.
which is not correct, since it isn't available.

> 
> 
> > 
> > >      (*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
> > > -    listflags(buf, sizeof(buf), (uint32_t)~0, feature_name, 1);
> > > -    (*cpu_fprintf)(f, "  %s\n", buf);
> > > -    listflags(buf, sizeof(buf), (uint32_t)~0, ext_feature_name, 1);
> > > -    (*cpu_fprintf)(f, "  %s\n", buf);
> > > -    listflags(buf, sizeof(buf), (uint32_t)~0, ext2_feature_name, 1);
> > > -    (*cpu_fprintf)(f, "  %s\n", buf);
> > > -    listflags(buf, sizeof(buf), (uint32_t)~0, ext3_feature_name, 1);
> > > -    (*cpu_fprintf)(f, "  %s\n", buf);
> > > +    for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
> > > +        FeatureWordInfo *fw = &feature_word_info[i];
> > > +
> > > +        listflags(buf, sizeof(buf), (uint32_t)~0, fw->feat_names, 1);
> > > +        (*cpu_fprintf)(f, "  %s\n", buf);
> > > +    }
> > >  }
> > >  
> > >  CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> > > -- 
> > > 1.7.3.4
> > > 
> > 
> > 
> > -- 
> > Regards,
> >   Igor
> 
> -- 
> Eduardo


-- 
Regards,
  Igor

Re: [PATCH] target-i386: Improve x86_cpu_list output

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3034 bytes --]

On 2013-02-27 08:37, Igor Mammedov wrote:
> On Wed, 27 Feb 2013 00:26:38 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
>> On Tue, Feb 26, 2013 at 10:57:56PM +0100, Igor Mammedov wrote:
>>> On Sat, 23 Feb 2013 16:45:00 +0100
>>> Jan Kiszka <jan.kiszka@web.de> wrote:
>>>
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> Several issues fixed:
>>>>  - We were missing a bunch of feature lists. Fix this by simply dumping
>>>>    the meta list feature_word_info.
>>>>  - kvm_enabled() cannot be true at this point because accelerators are
>>>>    initialized much later during init. Simply dump unconditionally.
>>> Why not to move list_cpu after accelerators are initialized?
>>
>> Because help output is simply documentation and shouldn't depend on any
>> other config option parsing or accelerator initialization at all?
> Don't see reason why it shouldn't.
> It's not a man page but a program and can do pretty much everything.

Actually, requiring "-enable-kvm -cpu ?" to list the host type would be
counterproductive - hardly any user will find this out, at best by
chance. However ...

> 
>>
>>>
>>>>  - Add explanation for "host" CPU type.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>  target-i386/cpu.c |   20 +++++++++-----------
>>>>  1 files changed, 9 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>>>> index dfcf86e..6e742f0 100644
>>>> --- a/target-i386/cpu.c
>>>> +++ b/target-i386/cpu.c
>>>> @@ -1453,18 +1453,16 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>>>>          snprintf(buf, sizeof(buf), "%s", def->name);
>>>>          (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
>>>>      }
>>>> -    if (kvm_enabled()) {
>>>> -        (*cpu_fprintf)(f, "x86 %16s\n", "[host]");
>>>> -    }
>>>> +    (*cpu_fprintf)(f, "x86 %16s  %-48s\n", "host",
>>>> +                   "KVM processor with all supported host features");
>>>> +
>>> that would make 'host' visible to users even if QEMU compiled without KVM
>>> support. No big harm, but autotest could get confused when it gets 'host' CPU
>>> but QEMU doesn't run because it's not really supported.
>>
>> Then we have to fix the autotest test code to not try it without KVM.
>> :-)
>>
>> Help output is not a probing mechanism (although we often misuse it as
>> if it were), and I expect help output to be static and not depend on any
>> subsystem initialization.
> Then fix help output and add to "host" line something like " is available with
> -enable-kvm on command line and if your build was compiled --enable-kvm
> configure option", otherwise 'host' is misleading.
> Now even without 'host' in output of -cpu 'help', question why 'host' is not
> found periodically pops up on IRC. This change will just increase frequency of
> it.

...I will add "(only available in KVM mode)" here and wrap these lines
in #ifdef CONFIG_KVM. That should be more acceptable, no?

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH] target-i386: Improve x86_cpu_list output

[Andreas Färber]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 27.02.2013 08:52, schrieb Jan Kiszka:
> On 2013-02-27 08:37, Igor Mammedov wrote:
>> On Wed, 27 Feb 2013 00:26:38 -0300 Eduardo Habkost
>> <ehabkost@redhat.com> wrote:
>> 
>>> On Tue, Feb 26, 2013 at 10:57:56PM +0100, Igor Mammedov wrote:
>>>> On Sat, 23 Feb 2013 16:45:00 +0100 Jan Kiszka
>>>> <jan.kiszka@web.de> wrote:
>>>> 
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> 
>>>>> Several issues fixed: - We were missing a bunch of feature
>>>>> lists. Fix this by simply dumping the meta list
>>>>> feature_word_info. - kvm_enabled() cannot be true at this
>>>>> point because accelerators are initialized much later
>>>>> during init. Simply dump unconditionally.
>>>> Why not to move list_cpu after accelerators are initialized?
>>> 
>>> Because help output is simply documentation and shouldn't
>>> depend on any other config option parsing or accelerator
>>> initialization at all?
>> Don't see reason why it shouldn't. It's not a man page but a
>> program and can do pretty much everything.
> 
> Actually, requiring "-enable-kvm -cpu ?" to list the host type
> would be counterproductive - hardly any user will find this out, at
> best by chance. However ...
> 
>> 
>>> 
>>>> 
>>>>> - Add explanation for "host" CPU type.
>>>>> 
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- 
>>>>> target-i386/cpu.c |   20 +++++++++----------- 1 files
>>>>> changed, 9 insertions(+), 11 deletions(-)
>>>>> 
>>>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c index
>>>>> dfcf86e..6e742f0 100644 --- a/target-i386/cpu.c +++
>>>>> b/target-i386/cpu.c @@ -1453,18 +1453,16 @@ void
>>>>> x86_cpu_list(FILE *f, fprintf_function cpu_fprintf) 
>>>>> snprintf(buf, sizeof(buf), "%s", def->name); 
>>>>> (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf,
>>>>> def->model_id); } -    if (kvm_enabled()) { -
>>>>> (*cpu_fprintf)(f, "x86 %16s\n", "[host]"); -    } +
>>>>> (*cpu_fprintf)(f, "x86 %16s  %-48s\n", "host", +
>>>>> "KVM processor with all supported host features"); +
>>>> that would make 'host' visible to users even if QEMU compiled
>>>> without KVM support. No big harm, but autotest could get
>>>> confused when it gets 'host' CPU but QEMU doesn't run because
>>>> it's not really supported.
>>> 
>>> Then we have to fix the autotest test code to not try it
>>> without KVM. :-)
>>> 
>>> Help output is not a probing mechanism (although we often
>>> misuse it as if it were), and I expect help output to be static
>>> and not depend on any subsystem initialization.
>> Then fix help output and add to "host" line something like " is
>> available with -enable-kvm on command line and if your build was
>> compiled --enable-kvm configure option", otherwise 'host' is
>> misleading. Now even without 'host' in output of -cpu 'help',
>> question why 'host' is not found periodically pops up on IRC.
>> This change will just increase frequency of it.
> 
> ...I will add "(only available in KVM mode)" here and wrap these
> lines in #ifdef CONFIG_KVM. That should be more acceptable, no?

I was about to ask for #ifdef CONFIG_KVM, yes please.

Can you move that into a second patch though? Then I can apply the
feature words fixes earlier. -cpu host has been a controversial and
known-broken topic for a long time already.

Thanks,
Andreas

- -- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nrnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend￶rffer; HRB 16746 AG Nrnberg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)

iQIcBAEBAgAGBQJRLbzhAAoJEPou0S0+fgE/1lgQAI4RZM+hBIkVgFW9laosOmn6
lP7M+fy5037V4rGPdLuAY+F7BsuH3CBzr11o+QGJCmFJo+1IUI6AvnQNMQkA6S06
b/jmchu+LxQqxPXYF2v5F0MmznLpBOtX4C9A8NaPEW8+nXY7O0UQ1feRiN+vuA6L
PQnRQK6znsxhR/1dnZPEZMHtAqHN2VSYUD/VfUBzw1gYgXq7iyFIfetGEY9op3e4
ZuPh37pd0hkKebIoz7hUKlMf5OuM8ANsEt/exvZbnhH7nT/2UJhITFYLZXrCWqnB
KJoFlvQJjQ2VE8l18cr+fX0MVDpQKKks+svzVIasiPff4WM//Y+cnz44q8/JPlWB
XEAJTKnZHXECeunIw1bGafKZvf/Grrj7QaUGWPStohZO0+d5rkYgKCBNzsHBN468
Z5biu7StZUNW+r2z3YxdF64zg0wT1tXyN4CzI5NHNNKymmuyXt/b/srkGuNhronB
k7AsV4YqI8A7pQRfY9vc3RG17+Mzcbj+uHlnSxunwV8c6SiZ5zXoHpNR3oYnr6z7
5WB3LH9Yre2dcf0xBp+15SzMHkjd7XoGX5viVU1dbumNTvUValLRasQHshaNbdSP
skobPfFAJmXXqZabaGwgtfzo3ivRcFjzikbvAFacVtnx6zHpgpp+PdSZRWv+79uM
fr0q+VNKbnVfnYRyVh3k
=cbHS
-----END PGP SIGNATURE-----

Re: [PATCH] target-i386: Improve x86_cpu_list output

[Igor Mammedov]

On Wed, 27 Feb 2013 08:52:50 +0100
Jan Kiszka <jan.kiszka@web.de> wrote:

> On 2013-02-27 08:37, Igor Mammedov wrote:
> > On Wed, 27 Feb 2013 00:26:38 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > 
> >> On Tue, Feb 26, 2013 at 10:57:56PM +0100, Igor Mammedov wrote:
> >>> On Sat, 23 Feb 2013 16:45:00 +0100
> >>> Jan Kiszka <jan.kiszka@web.de> wrote:
> >>>
> >>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>
> >>>> Several issues fixed:
> >>>>  - We were missing a bunch of feature lists. Fix this by simply dumping
> >>>>    the meta list feature_word_info.
> >>>>  - kvm_enabled() cannot be true at this point because accelerators are
> >>>>    initialized much later during init. Simply dump unconditionally.
> >>> Why not to move list_cpu after accelerators are initialized?
> >>
> >> Because help output is simply documentation and shouldn't depend on any
> >> other config option parsing or accelerator initialization at all?
> > Don't see reason why it shouldn't.
> > It's not a man page but a program and can do pretty much everything.
> 
> Actually, requiring "-enable-kvm -cpu ?" to list the host type would be
> counterproductive - hardly any user will find this out, at best by
> chance. However ...
> 
> > 
> >>
> >>>
> >>>>  - Add explanation for "host" CPU type.
> >>>>
> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>> ---
> >>>>  target-i386/cpu.c |   20 +++++++++-----------
> >>>>  1 files changed, 9 insertions(+), 11 deletions(-)
> >>>>
> >>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> >>>> index dfcf86e..6e742f0 100644
> >>>> --- a/target-i386/cpu.c
> >>>> +++ b/target-i386/cpu.c
> >>>> @@ -1453,18 +1453,16 @@ void x86_cpu_list(FILE *f, fprintf_function
> >>>> cpu_fprintf) snprintf(buf, sizeof(buf), "%s", def->name);
> >>>>          (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
> >>>>      }
> >>>> -    if (kvm_enabled()) {
> >>>> -        (*cpu_fprintf)(f, "x86 %16s\n", "[host]");
> >>>> -    }
> >>>> +    (*cpu_fprintf)(f, "x86 %16s  %-48s\n", "host",
> >>>> +                   "KVM processor with all supported host features");
> >>>> +
> >>> that would make 'host' visible to users even if QEMU compiled without
> >>> KVM support. No big harm, but autotest could get confused when it gets
> >>> 'host' CPU but QEMU doesn't run because it's not really supported.
> >>
> >> Then we have to fix the autotest test code to not try it without KVM.
> >> :-)
> >>
> >> Help output is not a probing mechanism (although we often misuse it as
> >> if it were), and I expect help output to be static and not depend on any
> >> subsystem initialization.
> > Then fix help output and add to "host" line something like " is available
> > with -enable-kvm on command line and if your build was compiled
> > --enable-kvm configure option", otherwise 'host' is misleading.
> > Now even without 'host' in output of -cpu 'help', question why 'host' is
> > not found periodically pops up on IRC. This change will just increase
> > frequency of it.
> 
> ...I will add "(only available in KVM mode)" here and wrap these lines
> in #ifdef CONFIG_KVM. That should be more acceptable, no?
yes, it will be better.

> Jan
> 
>