From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53498) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUbEv-0000GS-Gq for qemu-devel@nongnu.org; Tue, 02 Aug 2016 11:04:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUbEq-0002nM-5E for qemu-devel@nongnu.org; Tue, 02 Aug 2016 11:04:16 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:59014 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUbEq-0002nI-07 for qemu-devel@nongnu.org; Tue, 02 Aug 2016 11:04:12 -0400 Received: from pps.filterd (m0098419.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u72ExXgD133823 for ; Tue, 2 Aug 2016 11:04:11 -0400 Received: from e06smtp05.uk.ibm.com (e06smtp05.uk.ibm.com [195.75.94.101]) by mx0b-001b2d01.pphosted.com with ESMTP id 24jh2ypf1t-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 02 Aug 2016 11:04:11 -0400 Received: from localhost by e06smtp05.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 2 Aug 2016 16:04:09 +0100 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 02470219004D for ; Tue, 2 Aug 2016 16:03:33 +0100 (BST) Received: from d06av11.portsmouth.uk.ibm.com (d06av11.portsmouth.uk.ibm.com [9.149.37.252]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u72F46nc32833734 for ; Tue, 2 Aug 2016 15:04:06 GMT Received: from d06av11.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av11.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u72F46ZY001116 for ; Tue, 2 Aug 2016 09:04:06 -0600 Date: Tue, 2 Aug 2016 17:04:05 +0200 From: David Hildenbrand In-Reply-To: <20160802134529.GE3337@thinpad.lan.raisama.net> References: <1470139155-53900-1-git-send-email-dahi@linux.vnet.ibm.com> <1470139155-53900-25-git-send-email-dahi@linux.vnet.ibm.com> <20160802134529.GE3337@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-Id: <20160802170405.57718508@thinkpad-w530> Subject: Re: [Qemu-devel] [Patch v1 24/29] qmp: add QMP interface "query-cpu-model-expansion" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, jdenemar@redhat.com, imammedo@redhat.com, cornelia.huck@de.ibm.com, borntraeger@de.ibm.com, fiuczy@linux.vnet.ibm.com, mimu@linux.vnet.ibm.com > > +# A CPU model consists of the name of a CPU definition, to which > > +# delta changes are applied (e.g. features added/removed). Most magic values > > +# that an architecture might require should be hidden behind the name. > > +# However, if required, architectures can expose relevant properties. > > +# > > +# @name: the name of the CPU definition the model is based on > > +# @props: #optional a dictionary of properties to be applied > > Should we make it explicit that we are talking about QOM > properties? Yes makes sense. > > > +# > > +# Since: 2.8.0 > > +## > > +{ 'struct': 'CpuModelInfo', > > + 'data': { 'name': 'str', > > + '*props': 'any' } } > > + > > +## > > +# @CpuModelExpansionType > > +# > > +# An enumeration of CPU model expansion types. > > +# > > +# @static: Expand to a static CPU model, a combination of a static base > > +# model name and property delta changes. As the static base model will > > +# never change, the expanded CPU model will be the same, independant of > > +# QEMU version or compatibility machines. Therefore, the resulting > > We could be more explicit about the guarantees: "independent of > QEMU version, machine type, machine options, and accelerator > options". agreed. > > > +# model can be used by tooling without having to specify a > > +# compatibility machine - e.g. when displaying the "host" model. > > +# All static CPU models are migration-safe. > > This is cool. Unfortunately we are not going to support it in x86 > very soon because we don't have any static CPU models. Well, it's all about finding a minimum set of features that one can work with. I assume e.g. a Phenom also always has a minimum set of features? > > > +# > > +# @full: Expand all properties. The produced model is not guaranteed to be > > +# migration-safe, but allows tooling to get an insight and work with > > +# model details. > > I wonder if we really need to document it broadly as "not > guaranteed to be migration-safe". The returned data will be > migration-safe (but not static) if the CPU model being expanded > is migration-safe, won't it? Actually I don't think so. Imagine expanding host: featA=true, featB=false Now, if going to another QEMU version, there might be featC known. So -host,featA=on,featB=off will implicitly enable featC and is therefore not be migration-safe. You would have to disable featC for compatibility machines on the host model. Is something like that done? I don't think so (and at least s390x won't do it right now). But, I can simply get rid of that remark. > > Also: I wonder what should be the return value for "name" when > expansion type is "full" and we don't have any static CPU models > (like on x86). e.g. returning: > { name: "host", props: { foo: "on", bar: "on", ... } > would make the interface not directly usable for the expansion of > . Maybe that means we have to add at least > one static CPU model to x86, too? I'd simply copy the name then. That's what I had in mind. (actually I don't do it on s390x because it's easier to just rely on the output of our conversion function). > > + > > + > > +## > > +# @query-cpu-model-expansion: > > +# > > +# Expands the given CPU model to different granularities, allowing tooling > > +# to get an understanding what a specific CPU model looks like in QEMU > > +# under a certain QEMU machine. > > Maybe "expands a given CPU model (or a combination of CPU model + > additional options) to different granularities". > > > +# > > +# Expanding CPU models is in general independant of the accelerator, except > > +# for models like "host" that explicitly rely on an accelerator and can > > +# vary in different configurations. > > Unfortunately this is not true in x86. Documenting it this way > might give people the wrong expectations. > > Specific guarantees from arch-specific code could be documented > somewhere, but: where? > > Maybe arch-specific guarantees should actually become > query-cpu-definitions fields (like we have "static" and > "migration-safe" today). If people are really interested in > accelerator-independent data, we need Okay, so this could be extended later than. > > > > This interface can therefore also be used > > +# to query the "host" CPU model. > > +# > > +# Note: This interface should not be used when global properties of CPU classes > > +# are changed (e.g. via "-cpu ..."). > > We could simply enumerate all cases that could affect the return > value. e.g.: > > # The data returned by this command may be affected by: > # > # * QEMU version: CPU models may look different depending on the > # QEMU version. (Except for CPU models reported as "static" > # in query-cpu-definitions.) > # * machine-type: CPU model expansion may look different depending > # on the machine-type. (Except for CPU models reported as "static" > # in query-cpu-definitions.) > # * machine options (including accelerator): in some > # architectures, CPU models may look different depending on > # machine and accelerator options. (Except for CPU models > # reported as "static" in query-cpu-definitions.) > # * "-cpu" arguments and global properties: arguments to the -cpu > # option and global properties may affect expansion of CPU > # models. Using query-cpu-model-expansion while using "-cpu" > # or global properties is not advised. > Yes, that's better. > > > +# > > +# s390x supports expanding of all CPU models with all expansion types. Other > > +# architectures are not supported yet. > > I think this paragraph is likely to get obsolete very soon (as > people may forget to update it when implementing the new > interface on other architectures). Also, the paragraph is not > true until patch 27/29 is applied. > > Maybe write it as "Some architectures may not support all > expansion types". Agreed. And most likely x86 won't support expanding all CPU models I assume? > > Patch 27/29 could add "s390x supports both 'static' and 'full' > expansion types". I wouldn't document it as "supports all > expansion types" because CpuModelExpansionType may be extended in > the future. Agreed. Thanks! David