Re: all class init functions for all types in QEMU are called in select_machine(). Expected?

[Claudio Fontana]

On 3/12/21 10:45 AM, Philippe Mathieu-Daudé wrote:
> On 3/12/21 10:31 AM, Claudio Fontana wrote:
>> Hello Paolo and all,
>>
>> while debugging a class init ordering issue, I noticed that
>>
>> _all_ class init functions for all types registered in the QEMU QOM are 
>> called in select_machine().
>> Expected?
>>
>> In particular it happens here:
>>
>> static MachineClass *select_machine(void)
>> {
>>     GSList *machines = object_class_get_list(TYPE_MACHINE, false);
>>
>>
>> object_class_get_list() ->
>>   object_class_foreach() ->
>>     g_hash_table_foreach() ->
>>       object_class_foreach_tramp ->
>>         type_initialize(type);
>>
>>
>> Is this really desired? It looks suspect to me.
>>
>> (gdb) bt
>> #0  0x0000555555db613f in arm_v7m_class_init (oc=0x555556dca320, 
>> data=0x555556a926e0 <arm_tcg_cpus+288>)
>>     at ../target/arm/tcg/tcg-cpu-models.c:849
>> #1  0x0000555555f1deba in type_initialize (ti=0x555556d5b2f0) at 
>> ../qom/object.c:364
>> #2  0x0000555555f1f62a in object_class_foreach_tramp (key=0x555556d5b470, 
>> value=0x555556d5b2f0, opaque=0x7fffffffda20)
>>     at ../qom/object.c:1069
>> #3  0x00007ffff6562000 in g_hash_table_foreach () at 
>> /usr/lib64/libglib-2.0.so.0
>> #4  0x0000555555f1f709 in object_class_foreach (fn=
>>     0x555555f1f866 <object_class_get_list_tramp>, 
>> implements_type=0x555556381b09 "machine", include_abstract=false, 
>> opaque=0x7fffffffda70)
>>     at ../qom/object.c:1091
>> #5  0x0000555555f1f8e4 in object_class_get_list 
>> (implements_type=0x555556381b09 "machine", include_abstract=false) at 
>> ../qom/object.c:1148
>> #6  0x0000555555debe94 in select_machine () at ../softmmu/vl.c:1607
>>
>>
>> If not here, where should be the right place, for example, for CPU class 
>> inits to be called?
>>
>> At the very least I would put a comment there around the beginning of 
>> select_machine() saying:
>>
>> /* all types, all classes in QOM are initialized here, as a result of the 
>> object_class_get_list call */
>>
>> Wdyt?
> 
> Are you trying to register types conditionally?
> 

Not really, but I have been using the accel class init function on x86 to 
register the TCG OPS,

and this instead requires a bit more thought for ARM,

because we currently register for the ARM M Profile the TCG Ops at 
arm_v7m_class_init time,
which is called already at select_machine() time,

so when we select the accelerator, and we call the tcg_cpu_class_init, we run 
the risk of overriding the existing tcg_ops,
with the current result that we have to do:

static void tcg_cpu_class_init(CPUClass *cc)
{
    /*                                                                          
                                                            
     * do not overwrite the TCG ops, if already set by the                      
                                                            
     * arm cpu class (this is the case for the M profile CPUs).                 
                                                            
     *                                                                          
                                                            
     * Otherwise, provide the default ARM TCG behavior here.                    
                                                            
     */
    if (!cc->tcg_ops) {
        cc->tcg_ops = &arm_tcg_ops;
    }
}

It's a fine result for me, but we do have an "if" inside a class_init.

Ideas? Looks horrible?

THanks,

Claudio