From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56610) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YqGf4-00035N-Lf for qemu-devel@nongnu.org; Thu, 07 May 2015 03:56:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YqGf1-0004Ok-AW for qemu-devel@nongnu.org; Thu, 07 May 2015 03:56:02 -0400 Received: from e06smtp10.uk.ibm.com ([195.75.94.106]:49019) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YqGf0-0004Oa-QW for qemu-devel@nongnu.org; Thu, 07 May 2015 03:55:59 -0400 Received: from /spool/local by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 7 May 2015 08:55:53 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 85CEE17D805F for ; Thu, 7 May 2015 08:56:34 +0100 (BST) Received: from d06av06.portsmouth.uk.ibm.com (d06av06.portsmouth.uk.ibm.com [9.149.37.217]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t477tnnI459008 for ; Thu, 7 May 2015 07:55:49 GMT Received: from d06av06.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av06.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t472npXm004353 for ; Wed, 6 May 2015 22:49:52 -0400 Date: Thu, 7 May 2015 09:55:45 +0200 From: Michael Mueller Message-ID: <20150507095545.6f14668e@bee> In-Reply-To: <20150506114150.GB17796@thinpad.lan.raisama.net> References: <1430146411-34632-1-git-send-email-mimu@linux.vnet.ibm.com> <1430146411-34632-3-git-send-email-mimu@linux.vnet.ibm.com> <20150505132602.GQ17796@thinpad.lan.raisama.net> <20150506115938.04d8a1c1@bee> <20150506114150.GB17796@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 02/17] Add accelerator id and model name to CPUState 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 Wed, 6 May 2015 08:41:50 -0300 Eduardo Habkost wrote: > On Wed, May 06, 2015 at 11:59:38AM +0200, Michael Mueller wrote: > > On Tue, 5 May 2015 10:26:02 -0300 > > Eduardo Habkost wrote: > > > > > On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote: > > > > The patch defines ids per accelerator and adds the accel_id and > > > > the model_name to the CPUState. The accel_id is initialized by > > > > common code, the model name needs to be initialized by target > > > > specific code. > > > > > > > > Signed-off-by: Michael Mueller > > > > Acked-by: Christian Borntraeger > > > > --- > > > > include/qom/cpu.h | 5 +++++ > > > > qapi-schema.json | 9 +++++++++ > > > > qom/cpu.c | 14 ++++++++++++++ > > > > 3 files changed, 28 insertions(+) > > > > > > > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > > > > index 9dafb48..4ffc050 100644 > > > > --- a/include/qom/cpu.h > > > > +++ b/include/qom/cpu.h > > > > @@ -236,6 +236,8 @@ struct kvm_run; > > > > * @mem_io_pc: Host Program Counter at which the memory was accessed. > > > > * @mem_io_vaddr: Target virtual address at which the memory was accessed. > > > > * @kvm_fd: vCPU file descriptor for KVM. > > > > + * @accel_id: accelerator id of this CPU. > > > > + * @model_name: model name of this CPU > > > > * > > > > * State of one CPU core or thread. > > > > */ > > > > @@ -313,6 +315,9 @@ struct CPUState { > > > > (absolute value) offset as small as possible. This reduces code > > > > size, especially for hosts without large memory offsets. */ > > > > volatile sig_atomic_t tcg_exit_req; > > > > + > > > > + AccelId accel_id; > > > > > > This can be a AccelState pointer, set on initialization, because we have > > > another user case for having a AccelState pointer: query-cpu-definition > > > implementations may create temporary CPU objects with a different accel > > > object to be able to probe for accel-specific data. > > > > > > (The pointer may become a link QOM property later.) > > > > > > > > > > + char *model_name; > > > > }; > > > > > > > > QTAILQ_HEAD(CPUTailQ, CPUState); > > > > diff --git a/qapi-schema.json b/qapi-schema.json > > > > index ac9594d..540e520 100644 > > > > --- a/qapi-schema.json > > > > +++ b/qapi-schema.json > > > > @@ -2515,6 +2515,15 @@ > > > > ## > > > > { 'command': 'query-machines', 'returns': ['MachineInfo'] } > > > > > > > > +# @AccelId > > > > +# > > > > +# Defines accelerator ids > > > > +# > > > > +# Since: 2.4 > > > > +## > > > > +{ 'enum': 'AccelId', > > > > + 'data': ['qtest', 'tcg', 'kvm', 'xen'] } > > > > + > > > > > > Not sure if it is better to have an enum or simply a string here. > > > > > > > ## > > > > # @CpuDefinitionInfo: > > > > # > > > > diff --git a/qom/cpu.c b/qom/cpu.c > > > > index 108bfa2..457afc7 100644 > > > > --- a/qom/cpu.c > > > > +++ b/qom/cpu.c > > > > @@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model) > > > > goto out; > > > > } > > > > > > > > + if (tcg_enabled()) { > > > > + cpu->accel_id = ACCEL_ID_TCG; > > > > + } else if (kvm_enabled()) { > > > > + cpu->accel_id = ACCEL_ID_KVM; > > > > + } > > > > +#ifdef CONFIG_XEN > > > > + else if (xen_enabled()) { > > > > + cpu->accel_id = ACCEL_ID_XEN; > > > > + } > > > > +#endif > > > > + else { > > > > + cpu->accel_id = ACCEL_ID_QTEST; > > > > + } > > > > > > You can simply use ACCEL_GET_CLASS(current_machine->accelerator)->name > > > here. If we really want an enum, we can add an AccelId field to > > > AccelClass, and initialize it properly on the accel class_init > > > functions. > > > > The AccelClass (ac = ACCEL_GET_CLASS(current_machine->accelerator) would be ok though. > > That will allow to access the ac->accel_id (not yet there) at places where required. > > > > I'm just not sure how to access current_machine here. > > It is a global variable declared in hw/boards.h. But it makes sense only > if !CONFIG_USER. > > But I am starting to wonder if we shouldn't simply expose accel info as > a /mmachine property, instead of per-CPU information. It may become > per-CPU in the future, but right now we really don't support having > multiple accelerators so why are we trying to pretend we do? > That brings me back into the direction I was before with the proposed query-cpu-model QMP call... but that's OK. In the end that holds true also for the cpu model name as well... > > > > > > > > CONFIG_USER may require some special code when returning the accelerator > > > ID as "tcg" because IIRC it doesn't use the QOM accel classes (yet). > > > > > > > + > > > > object_property_set_bool(OBJECT(cpu), true, "realized", &err); > > > > > > > > out: > > > > -- > > > > 1.8.3.1 > > > > > > > > > >