Hi
On Fri, Nov 30, 2018 at 3:48 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Tue, 27 Nov 2018 13:27:57 +0400
> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
>
> > Replace arm_cpu_post_init() instance callback by calling it from leaf
> > classes, to avoid potential the ordering issue with interfaces
> > post-init.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Suggested-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > target/arm/cpu.h | 2 ++
> > target/arm/cpu.c | 15 ++++++++++++---
> > target/arm/cpu64.c | 11 ++++++++++-
> > 3 files changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index 2a73fed9a0..84fba2b24b 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -884,6 +884,8 @@ static inline ARMCPU *arm_env_get_cpu(CPUARMState *env)
> > return container_of(env, ARMCPU, env);
> > }
> >
> > +void arm_cpu_post_init(Object *obj);
> > +
> > uint64_t arm_cpu_mp_affinity(int idx, uint8_t clustersz);
> >
> > #define ENV_GET_CPU(e) CPU(arm_env_get_cpu(e))
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index 60411f6bfe..8a4aae7438 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -734,7 +734,7 @@ static Property arm_cpu_pmsav7_dregion_property =
> > static Property arm_cpu_initsvtor_property =
> > DEFINE_PROP_UINT32("init-svtor", ARMCPU, init_svtor, 0);
> >
> > -static void arm_cpu_post_init(Object *obj)
> > +void arm_cpu_post_init(Object *obj)
> > {
> > ARMCPU *cpu = ARM_CPU(obj);
> >
> > @@ -2094,6 +2094,7 @@ static void arm_host_initfn(Object *obj)
> > ARMCPU *cpu = ARM_CPU(obj);
> >
> > kvm_arm_set_cpu_features_from_host(cpu);
> > + arm_cpu_post_init(ARM_CPU(obj));
> > }
> >
> > static const TypeInfo host_arm_cpu_type_info = {
> > @@ -2108,14 +2109,23 @@ static const TypeInfo host_arm_cpu_type_info = {
> >
> > #endif
> >
> > +static void arm_cpu_instance_init(Object *obj)
> > +{
> > + const ARMCPUInfo *info = object_class_get_class_data(object_get_class(obj));
> > +
> > + info->initfn(obj);
> > + arm_cpu_post_init(obj);
> > +}
> now imagine leaf cpu class call chain:
> before patch:
> arm-cpu::initfn()
> cortex-a8::initfn()
> set feature AAA
> device_post_init()
> -> arm_cpu_post_init()
> if (AAA)
> do something
>
> after patch:
> arm-cpu::initfn()
> -> arm_cpu_post_init()
> if (AAA)
> do something <--- won't happen anymore
> cortex-a8::initfn()
>
Hmm? No, after the patch cortex-a8::initfn() is called before
arm_cpu_post_init().
> arm_cpu_post_init() helper has to go to leaf classes only
That's what the patch does, unless I am missing something.
arm_cpu_instance_init() is called on the leaf class, calling the
leaf-cpu init (cortex-8a::initfn() etc) before calling
arm_cpu_post_init()
>
> > +
> > static void cpu_register(const ARMCPUInfo *info)
> > {
> > TypeInfo type_info = {
> > .parent = TYPE_ARM_CPU,
> > .instance_size = sizeof(ARMCPU),
> > - .instance_init = info->initfn,
> > + .instance_init = arm_cpu_instance_init,
> > .class_size = sizeof(ARMCPUClass),
> > .class_init = info->class_init,
> > + .class_data = (void *)info,
> > };
> >
> > type_info.name = g_strdup_printf("%s-" TYPE_ARM_CPU, info->name);
> > @@ -2128,7 +2138,6 @@ static const TypeInfo arm_cpu_type_info = {
> > .parent = TYPE_CPU,
> > .instance_size = sizeof(ARMCPU),
> > .instance_init = arm_cpu_initfn,
> > - .instance_post_init = arm_cpu_post_init,
> > .instance_finalize = arm_cpu_finalizefn,
> > .abstract = true,
> > .class_size = sizeof(ARMCPUClass),
> > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> > index 873f059bf2..dbfc3ee490 100644
> > --- a/target/arm/cpu64.c
> > +++ b/target/arm/cpu64.c
> > @@ -447,14 +447,23 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
> > cc->gdb_arch_name = aarch64_gdb_arch_name;
> > }
> >
> > +static void aarch64_cpu_instance_init(Object *obj)
> > +{
> > + const ARMCPUInfo *info = object_class_get_class_data(object_get_class(obj));
> > +
> > + info->initfn(obj);
> > + arm_cpu_post_init(obj);
> > +}
> > +
> > static void aarch64_cpu_register(const ARMCPUInfo *info)
> > {
> > TypeInfo type_info = {
> > .parent = TYPE_AARCH64_CPU,
> > .instance_size = sizeof(ARMCPU),
> > - .instance_init = info->initfn,
> > + .instance_init = aarch64_cpu_instance_init,
> > .class_size = sizeof(ARMCPUClass),
> > .class_init = info->class_init,
> > + .class_data = (void *)info,
> > };
> >
> > type_info.name = g_strdup_printf("%s-" TYPE_ARM_CPU, info->name);
>
>
--
Marc-André Lureau