From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33596) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YcsCF-00067E-Aa for qemu-devel@nongnu.org; Tue, 31 Mar 2015 05:10:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YcsCA-0007Bu-FZ for qemu-devel@nongnu.org; Tue, 31 Mar 2015 05:10:55 -0400 Received: from e06smtp10.uk.ibm.com ([195.75.94.106]:56621) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YcsCA-0007Bb-51 for qemu-devel@nongnu.org; Tue, 31 Mar 2015 05:10:50 -0400 Received: from /spool/local by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 31 Mar 2015 10:10:48 +0100 Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 9FF11219004D for ; Tue, 31 Mar 2015 10:10:33 +0100 (BST) Received: from d06av10.portsmouth.uk.ibm.com (d06av10.portsmouth.uk.ibm.com [9.149.37.251]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t2V9AjNJ459220 for ; Tue, 31 Mar 2015 09:10:45 GMT Received: from d06av10.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av10.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t2V9AjoC026443 for ; Tue, 31 Mar 2015 03:10:45 -0600 Date: Tue, 31 Mar 2015 11:10:43 +0200 From: Michael Mueller Message-ID: <20150331111043.078443f8@bee> In-Reply-To: <20150330195020.GD7031@thinpad.lan.raisama.net> References: <1427725708-52100-1-git-send-email-mimu@linux.vnet.ibm.com> <1427725708-52100-12-git-send-email-mimu@linux.vnet.ibm.com> <20150330195020.GD7031@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: linux-s390@vger.kernel.org, Cornelia Huck , kvm@vger.kernel.org, Gleb Natapov , qemu-devel@nongnu.org, linux-kernel@vger.kernel.org, Christian Borntraeger , Alexander Graf , "Jason J. Herne" , Daniel Hansel , Paolo Bonzini , Andreas Faerber , Richard Henderson On Mon, 30 Mar 2015 16:50:20 -0300 Eduardo Habkost wrote: Hello Eduardo, > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > [...] > > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c > > index 829945d..1698b52 100644 > > --- a/target-s390x/cpu.c > > +++ b/target-s390x/cpu.c > > @@ -37,6 +37,11 @@ > > #define CR0_RESET 0xE0UL > > #define CR14_RESET 0xC2000000UL; > > > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > > +{ > > + return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > > +} > > + > > /* generate CPU information for cpu -? */ > > void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf) > > { > > @@ -74,6 +79,30 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) > > > > return entry; > > } > > + > > +CpuModelInfo *arch_query_cpu_model(Error **errp) > > +{ > > + CpuModelInfo *info; > > + S390CPUClass *cc; > > + > > + if (!s390_cpu_models_used()) { > > + return NULL; > > + } > > First question is: why you don't want to just return > { name: "none", accel: ... } > when TYPE_S390_CPU ("-cpu none") is used? That's a good idea, indeed! CPU model "none" is used as well in case option -cpu is omitted. > > Now, assuming that you really want to return NULL if -cpu none is used, > why don't you just ask for the first CPU (like you already do below) and > check if it is a named CPU model, instead of adding a new global? e.g.: > > static bool s390_cpu_class_is_model(S390CPUClass *cc) > { > return cpuid(cc->proc) != 0; > } > > CpuModelInfo *arch_query_cpu_model(Error **errp) > { > S390CPUClass *cc; > S390CPUClass *cc; > > cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0)); > if (!s390_cpu_class_is_model(cc)) { > return NULL; > } > [...] > } > > > > + info = g_try_new0(CpuModelInfo, 1); > > + if (!info) { > > + return NULL; > > + } > > + cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0)); > > + info->name = strdup_s390_cpu_name(cc); > > I don't think the current version of strdup_s390_cpu_name() can ever > return NULL. Do you expect it to return NULL in the future? No I do not expect this. I will drop the NULL test. > > > + if (!info->name) { > > + g_free(info); > > + return NULL; > > + } > > + if (kvm_enabled()) { > > + info->accel = ACCEL_ID_KVM; > > This could be a CPUState field, added automatically by > cpu_generic_init() using current_machine->accel. I will add a CPUState field and see how it works out... > > > + } > > + return info; > > +} > > #endif > > > > static void s390_cpu_set_pc(CPUState *cs, vaddr value) > > -- > > 1.8.3.1 > > > Thanks a lot, Michael