[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to C
From: |
Michael Mueller |
Subject: |
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState |
Date: |
Thu, 7 May 2015 09:55:45 +0200 |
On Wed, 6 May 2015 08:41:50 -0300
Eduardo Habkost <address@hidden> 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 <address@hidden> 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 <address@hidden>
> > > > Acked-by: Christian Borntraeger <address@hidden>
> > > > ---
> > > > 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
> > > >
> > >
> >
>