Re: [Qemu-devel] [PATCH v3 2/3] exec: rename cpu_exec_init() as cpu_exec_realizefn()

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 17 Oct 2016 17:20:22 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Sat, Oct 15, 2016 at 12:52:48AM +0200, Laurent Vivier wrote:
> > Modify all CPUs to call it from XXX_cpu_realizefn() function.
> > 
> > Remove all the cannot_destroy_with_object_finalize_yet as
> > unsafe references have been moved to cpu_exec_realizefn().
> > (tested with QOM command provided by commit 4c315c27)
> > 
> > for arm:
> > 
> > Setting of cpu->mp_affinity is moved from arm_cpu_initfn()
> > to arm_cpu_realizefn() as setting of cpu_index is now done
> > in cpu_exec_realizefn().
> > 
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>  
> [...]
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index 1b9540e..364a45d 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -441,22 +441,11 @@ static void arm_cpu_initfn(Object *obj)
> >      CPUState *cs = CPU(obj);
> >      ARMCPU *cpu = ARM_CPU(obj);
> >      static bool inited;
> > -    uint32_t Aff1, Aff0;
> >  
> >      cs->env_ptr = &cpu->env;
> > -    cpu_exec_init(cs, &error_abort);
> >      cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
> >                                           g_free, g_free);
> >  
> > -    /* This cpu-id-to-MPIDR affinity is used only for TCG; KVM will override it.
> > -     * We don't support setting cluster ID ([16..23]) (known as Aff2
> > -     * in later ARM ARM versions), or any of the higher affinity level fields,
> > -     * so these bits always RAZ.
> > -     */
> > -    Aff1 = cs->cpu_index / ARM_CPUS_PER_CLUSTER;
> > -    Aff0 = cs->cpu_index % ARM_CPUS_PER_CLUSTER;
> > -    cpu->mp_affinity = (Aff1 << ARM_AFF1_SHIFT) | Aff0;
> > -
> >  #ifndef CONFIG_USER_ONLY
> >      /* Our inbound IRQ and FIQ lines */
> >      if (kvm_enabled()) {  
> [...]
> > @@ -631,6 +628,15 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> >          set_feature(env, ARM_FEATURE_THUMB_DSP);
> >      }
> >  
> > +    /* This cpu-id-to-MPIDR affinity is used only for TCG; KVM will override it.
> > +     * We don't support setting cluster ID ([16..23]) (known as Aff2
> > +     * in later ARM ARM versions), or any of the higher affinity level fields,
> > +     * so these bits always RAZ.
> > +     */
> > +    Aff1 = cs->cpu_index / ARM_CPUS_PER_CLUSTER;
> > +    Aff0 = cs->cpu_index % ARM_CPUS_PER_CLUSTER;
> > +    cpu->mp_affinity = (Aff1 << ARM_AFF1_SHIFT) | Aff0;
> > +  
>
> This will override any value set in the "mp-affinity" property,
> The mp-affinity property can be set by the user in the
> command-line, and it is also set by machvirt_init() in
> hw/arm/virt.c.
> 
> Considering that each CPU is supposed to have a different value,
> I doubt there are existing use cases for mp-affinity being set
> directly by the user.
> 
> I suggest having a "cluster-size" property, instead of
> "mp-affinity". This way the mp_affinity field can be calculated
> on realize, based on the configured cluster-size.
CCing Drew as he knows more about MPIDR stuff more.

> 
> >      if (cpu->reset_hivecs) {
> >              cpu->reset_sctlr |= (1 << 13);
> >      }  
> [...]
>