From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47033) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YpvSN-0003Z2-M0 for qemu-devel@nongnu.org; Wed, 06 May 2015 05:17:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YpvSK-00058G-2V for qemu-devel@nongnu.org; Wed, 06 May 2015 05:17:31 -0400 Received: from e06smtp14.uk.ibm.com ([195.75.94.110]:42340) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YpvSJ-00057z-MW for qemu-devel@nongnu.org; Wed, 06 May 2015 05:17:27 -0400 Received: from /spool/local by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 6 May 2015 10:17:26 +0100 Received: from b06cxnps4076.portsmouth.uk.ibm.com (d06relay13.portsmouth.uk.ibm.com [9.149.109.198]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 500242190061 for ; Wed, 6 May 2015 10:17:05 +0100 (BST) Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213]) by b06cxnps4076.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t469HNbU39321790 for ; Wed, 6 May 2015 09:17:23 GMT Received: from d06av03.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t469HMrG013528 for ; Wed, 6 May 2015 03:17:23 -0600 Date: Wed, 6 May 2015 11:17:20 +0200 From: Michael Mueller Message-ID: <20150506111720.46e93edd@bee> In-Reply-To: <20150505174101.GV17796@thinpad.lan.raisama.net> References: <1430146411-34632-1-git-send-email-mimu@linux.vnet.ibm.com> <1430146411-34632-2-git-send-email-mimu@linux.vnet.ibm.com> <20150505135547.GR17796@thinpad.lan.raisama.net> <20150505181216.5b754d24@bee> <20150505174101.GV17796@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Cornelia Huck , Gleb Natapov , qemu-devel@nongnu.org, Alexander Graf , Christian Borntraeger , "Jason J. Herne" , Daniel Hansel , Paolo Bonzini , Andreas Faerber , Richard Henderson On Tue, 5 May 2015 14:41:01 -0300 Eduardo Habkost wrote: > On Tue, May 05, 2015 at 06:12:16PM +0200, Michael Mueller wrote: > > On Tue, 5 May 2015 10:55:47 -0300 > > Eduardo Habkost wrote: > > > > > On Mon, Apr 27, 2015 at 04:53:15PM +0200, Michael Mueller wrote: > > > > This patch introduces the function cpu_desc_avail() which returns by > > > > default true if not architecture specific implemented. Its intention > > > > is to indicate if the cpu model description is available for display > > > > by list_cpus(). This change allows cpu model descriptions to become > > > > dynamically created by evaluating the runtime context instead of > > > > putting static cpu model information at display. > > > > > > Why are you deliberately breaking "-cpu ?" when cpu_desc_avail() is > > > false? > > > > In the s390x case cpu_desc_avail() is per se false in this code section of vl.c: > > > > /* Init CPU def lists, based on config > > * - Must be called after all the qemu_read_config_file() calls > > * - Must be called before list_cpu() > > * - Must be called before machine->init() > > */ > > (Side note: I believe the above outdated, I will send a patch to update > it.) Will be interesting to see what the change is, master is currently showing this code. > > > cpudef_init(); > > > > if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) { > > list_cpus(stdout, &fprintf, cpu_model); > > exit(0); > > } > > > > That is because the output does not solely depend on static definitions > > but also on runtime context. Here the host machine type this instance of > > QEMU is running on, at least for the KVM case. > > Is this a required feature? I would prefer to have the main() code > simple even if it means not having runnable information in "-cpu ?" by > now (about possible ways to implement this without cpu_desc_avail(), see > below). I think it is more than a desired feature because one might end up with a failed CPU object instantiation although the help screen claims to CPU model to be valid. > > > > > > Once the accelerator has been initialized AND the S390 cpu classes have > > been setup by means of the following code: > > > > static void kvm_setup_cpu_classes(KVMState *s) > > { > > S390MachineProps mach; > > > > if (!kvm_s390_get_machine_props(s, &mach)) { > > s390_setup_cpu_classes(ACCEL_CURRENT, &mach, > > s390_current_fac_list_mask()); > > s390_setup_cpu_aliases(); > > cpu_classes_initialized = true; > > } > > } > > > > cpu_desc_avail() becomes true. In case the selceted mode was "?" > > the list_cpu() is now done right before the cpu model is used as part > > of the cpu initialization (hw/s390-virtio.c): > > > > void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) > > { > > int i; > > > > if (cpu_model == NULL) { > > cpu_model = "none"; > > } > > > > if (is_help_option(cpu_model)) { > > list_cpus(stdout, &fprintf, cpu_model); > > exit(0); > > } > > > > ... > > for (i = 0; i < smp_cpus; i++) { > > ... > > cpu = cpu_s390x_init(cpu_model); > > ... > > } > > } > > > In other words, you just need to ensure that s390_cpu_list() run after > kvm_setup_cpu_classes(). ... which is part of the KVM/accel init process but it could of course make use of the ACCEL_TMP use case as query-cpu-definitions does. > > Can't you simply call s390_setup_cpu_classes(ACCEL_TEMP) inside > s390_init_cpus(), just like arch_query_cpu_definitions()? You could even > share code between both functions. That would not help with the current placement of list_cpus() in main() as it happens way to early. Not s390_init_cpus() is the issue, the context information has been processed already at that time. Currently I just kind of delay the list_cpus() until all required information is available. > > (In the future, we should be able to implement "-cpu ?" by simply > calling the query-cpu-definitions implementation.) Right but the -machine ,accel= options have to be processed already. > > > > > > > > > What exactly could cause cpu_desc_avail() to be false? If CPU model > > > information is not yet available when cpu_list() is called, it is a bug. > > > > > > > Here an example output that shows only runnable cpu models: > > > > $ ./s390x-softmmu/qemu-system-s390x -machine s390,accel=kvm -cpu ? > > s390 none > > s390 2064-ga1 IBM zSeries 900 GA1 > > s390 2064-ga2 IBM zSeries 900 GA2 > > s390 2064-ga3 IBM zSeries 900 GA3 > > s390 2064 (alias for 2064-ga3) > > s390 z900 (alias for 2064-ga3) > > s390 2066-ga1 IBM zSeries 800 GA1 > > s390 2066 (alias for 2066-ga1) > > s390 z800 (alias for 2066-ga1) > > s390 2084-ga1 IBM zSeries 990 GA1 > > s390 2084-ga2 IBM zSeries 990 GA2 > > s390 2084-ga3 IBM zSeries 990 GA3 > > s390 2084-ga4 IBM zSeries 990 GA4 > > s390 2084-ga5 IBM zSeries 990 GA5 > > s390 2084 (alias for 2084-ga5) > > s390 z990 (alias for 2084-ga5) > > s390 2086-ga1 IBM zSeries 890 GA1 > > s390 2086-ga2 IBM zSeries 890 GA2 > > s390 2086-ga3 IBM zSeries 890 GA3 > > s390 2086 (alias for 2086-ga3) > > s390 z890 (alias for 2086-ga3) > > s390 2094-ga1 IBM System z9 EC GA1 > > s390 z9-109 (alias for 2094-ga1) > > s390 2094-ga2 IBM System z9 EC GA2 > > s390 2094-ga3 IBM System z9 EC GA3 > > s390 2094 (alias for 2094-ga3) > > s390 z9 (alias for 2094-ga3) > > s390 z9-ec (alias for 2094-ga3) > > s390 2096-ga1 IBM System z9 BC GA1 > > s390 2096-ga2 IBM System z9 BC GA2 > > s390 2096 (alias for 2096-ga2) > > s390 z9-bc (alias for 2096-ga2) > > s390 2097-ga1 IBM System z10 EC GA1 > > s390 2097-ga2 IBM System z10 EC GA2 > > s390 2097-ga3 IBM System z10 EC GA3 > > s390 2097 (alias for 2097-ga3) > > s390 z10 (alias for 2097-ga3) > > s390 z10-ec (alias for 2097-ga3) > > s390 2098-ga1 IBM System z10 BC GA1 > > s390 2098-ga2 IBM System z10 BC GA2 > > s390 2098 (alias for 2098-ga2) > > s390 z10-bc (alias for 2098-ga2) > > s390 2817-ga1 IBM zEnterprise 196 GA1 > > s390 2817-ga2 IBM zEnterprise 196 GA2 > > s390 2817 (alias for 2817-ga2) > > s390 z196 (alias for 2817-ga2) > > s390 2818-ga1 IBM zEnterprise 114 GA1 > > s390 2818 (alias for 2818-ga1) > > s390 z114 (alias for 2818-ga1) > > s390 2827-ga1 IBM zEnterprise EC12 GA1 > > s390 2827-ga2 IBM zEnterprise EC12 GA2 > > s390 2827 (alias for 2827-ga2) > > s390 zEC12 (alias for 2827-ga2) > > s390 host (alias for 2827-ga2) > > s390 2828-ga1 IBM zEnterprise BC12 GA1 > > s390 2828 (alias for 2828-ga1) > > s390 zBC12 (alias for 2828-ga1) > > > > > [...] >