[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH] spapr: move registration of "host" C
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH] spapr: move registration of "host" CPU core type to machine code |
Date: |
Fri, 29 Sep 2017 08:41:11 +0200 |
On Wed, 27 Sep 2017 17:32:32 +0200
Greg Kurz <address@hidden> wrote:
> On Wed, 27 Sep 2017 14:19:56 +0200
> Igor Mammedov <address@hidden> wrote:
>
> > On Mon, 25 Sep 2017 23:47:30 +0200
> > Greg Kurz <address@hidden> wrote:
> >
> > > On Mon, 25 Sep 2017 17:48:57 +0200
> > > Greg Kurz <address@hidden> wrote:
> > >
> > > > On Mon, 25 Sep 2017 15:41:34 +0200
> > > > Igor Mammedov <address@hidden> wrote:
> > > >
> > > > > On Mon, 25 Sep 2017 11:47:33 +0200
> > > > > Greg Kurz <address@hidden> wrote:
> > > > >
> > > > > > The CPU core abstraction belongs to the machine code. This also gets
> > > > > > rid of some code duplication.
> > > > > >
> > > > > > Signed-off-by: Greg Kurz <address@hidden>
> > > > > > ---
> > > > > >
> > > > > > hw/ppc/spapr_cpu_core.h is also included elsewhere in
> > > > > > target/ppc/kvm.c
> > > > > > but this is already handled by the following cleanup patch:
> > > > > >
> > > > > > https://patchwork.ozlabs.org/patch/817598/
> > > > > > ---
> > > > > > hw/ppc/spapr.c | 4 ++++
> > > > > > hw/ppc/spapr_cpu_core.c | 34
> > > > > > ++++++++++++++++++++++------------
> > > > > > include/hw/ppc/spapr_cpu_core.h | 2 +-
> > > > > > target/ppc/kvm.c | 12 ------------
> > > > > > 4 files changed, 27 insertions(+), 25 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > > > > index 0ce3ec87ac59..e82c8532ffb0 100644
> > > > > > --- a/hw/ppc/spapr.c
> > > > > > +++ b/hw/ppc/spapr.c
> > > > > > @@ -2349,6 +2349,10 @@ static void ppc_spapr_init(MachineState
> > > > > > *machine)
> > > > > > }
> > > > > >
> > > > > > /* init CPUs */
> > > > > > + if (kvm_enabled()) {
> > > > > > + spapr_cpu_core_register_host_type();
> > > > > > + }
> > > > > why don't we create it statically in hw/ppc/spapr_cpu_core.c
> > > > > like it's done in x86, i.e.
> > > > >
> > > > > static void x86_cpu_register_types(void)
> > > > >
> > > > > {
> > > > >
> > > > > ...
> > > > > #ifdef CONFIG_KVM
> > > > >
> > > > > type_register_static(&host_x86_cpu_type_info);
> > > > >
> > > > > #endif
> > > > >
> > > > > }
> > > > > type_init(x86_cpu_register_types)
> > > > >
> > > > > and do the same for host CPU as well?
> > > > >
> > > >
> > > > Hi Igor,
> > > >
> > > > Not sure yet why we use dynamic types, but I'd be glad to dig a bit
> > > > more.
> > >
> > > So the problem is that it was decided to make the host CPU class a
> > > subclass of the host's CPU model, and this requires all the CPU model
> > > classes to be registered beforehand.
> > >
> > > commit 5ba4576b858c0d6056f59abb7e17a2b63f7905f3
> > > Author: Andreas Färber <address@hidden>
> > > Date: Sat Feb 23 11:22:12 2013 +0000
> > >
> > > target-ppc: Make host CPU a subclass of the host's CPU model
> > >
> > > This avoids assigning individual class fields and contributors
> > > forgetting to add field assignments in KVM-only code.
> > >
> > > ppc_cpu_class_find_by_pvr() requires the CPU model classes to be
> > > registered, so defer host CPU type registration to kvm_arch_init().
> > >
> > > Only register the host CPU type if there is a class with matching PVR.
> > > This lets us drop error handling from instance_init.
> > >
> > > Signed-off-by: Andreas Färber <address@hidden>
> > > Signed-off-by: Alexander Graf <address@hidden>
> > >
> > > I can't think of an alternate way to do this. Any suggestion ?
> > I don't see from this commit a reason why it can't be done in cpu-models.c
> > dependencies here are
> > mfpvr() - which probably should work without KVM
>
> Correct.
>
> > ppc_cpu_class_by_pvr() - should work fine if 'host' type is being
> > registered as the last among the other CPU types
> >
>
> We have:
>
> ppc_cpu_class_by_pvr()
> object_class_get_list()
> object_class_foreach()
> object_class_foreach_tramp()
> type_initialize()
> type_get_parent()
>
> type_initialize() recursively initializes all parent types, and
> type_get_parent() aborts if the parent type isn't registered yet,
> which may happen as long as all type_init() functions haven't been
> called => ppc_cpu_class_by_pvr() cannot be safely called from a
> type_init() function.
I don't get what you are trying to say,
could you be more specific about what parent type might be not
registered?
Re: [Qemu-ppc] [PATCH] spapr: move registration of "host" CPU core type to machine code, David Gibson, 2017/09/25
Re: [Qemu-ppc] [PATCH] spapr: move registration of "host" CPU core type to machine code, David Gibson, 2017/09/27
Re: [Qemu-ppc] [PATCH] spapr: move registration of "host" CPU core type to machine code, Igor Mammedov, 2017/09/27