From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Mammedov Subject: Re: [PATCH] target-i386: Improve x86_cpu_list output Date: Wed, 27 Feb 2013 08:37:21 +0100 Message-ID: <20130227083721.720ee6a3@thinkpad.mammed.net> References: <5128E3FC.8010206@web.de> <20130226225756.09a78aeb@thinkpad.mammed.net> <20130227032638.GJ8494@otherpad.lan.raisama.net> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Anthony Liguori , Jan Kiszka , qemu-devel , kvm , Andreas =?UTF-8?B?RsOkcmJlcg==?= To: Eduardo Habkost Return-path: In-Reply-To: <20130227032638.GJ8494@otherpad.lan.raisama.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org List-Id: kvm.vger.kernel.org On Wed, 27 Feb 2013 00:26:38 -0300 Eduardo Habkost 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 wrote: > > > > > From: Jan Kiszka > > > > > > 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 > > > --- > > > 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