On 1/11/21 11:35 PM, Eduardo Habkost wrote:
> On Mon, Jan 11, 2021 at 05:13:27PM +0100, Claudio Fontana wrote:
>> Happy new year,
>
> Hi!
>
>>
>> picking up this topic again, i am looking at at now a different aspect of
>> this problem, of setting the right tcg ops for the right cpu class.
>>
>> This issue I am highlighting is present because different targets behave
>> differently in this regard.
>>
>> Ie, we have targets for which we always initialize all cpu classes, as a
>> result of different machine definitions.
>>
>> This is the case of arm, for example where we end up with backtraces like:
>>
>> arm_v7m_class_init
>> type_initialize
>> object_class_foreach_tramp
>> g_hash_table_foreach ()
>> object_class_foreach
>> object_class_get_list
>> select_machine ()
>> qemu_init
>> main
>>
>> with the arm_v7m_class_init called even if we are just going to use an
>> aarch64 cpu (so the class initializer for arm_v7m is called even for unused
>> cpus classes),
>>
>> while in other cases we have the target explicitly relying on the fact that
>> only the right cpu class is initialized, for example in cris we have code
>> like:
>
> This shouldn't matter at all, because class_init is not supposed
> to have any side effects outside the corresponding ObjectClass
> struct.
>
> So, I don't understand what you mean below:
>
>>
>> target/cris/cpu.c:
>>
>> static void crisv9_cpu_class_init(ObjectClass *oc, void *data)
>> {
>> CPUClass *cc = CPU_CLASS(oc);
>> CRISCPUClass *ccc = CRIS_CPU_CLASS(oc);
>>
>> ccc->vr = 9;
>> cc->do_interrupt = crisv10_cpu_do_interrupt;
>> cc->gdb_read_register = crisv10_cpu_gdb_read_register;
>> cc->tcg_initialize = cris_initialize_crisv10_tcg;
>> }
>>
>> where the class initialization of the cpu is explicitly setting the methods
>> of CPUClass, therefore implicitly relying on the fact that no other class
>> initializer screws things up.
>
> I don't see the problem here. Having all other class
> initializers being called should be completely OK, because each
> class has its own ObjectClass struct.
>
>
>>
>> Given this context, which one of these methods is "right"?
>> Should we rework things so that only used cpu classes are actually
>> initialized?
>
> This option wouldn't make sense. class_init is supposed to be
> called on demand on class lookup, and can be triggered by
> object_class_get_list(), object_class_by_name(), or similar
> functions. This is by design.
>
>
>> Or should we maybe not do these settings in cpu class_init at all, but
>> rather at cpu initfn time, or at cpu realize time?
>
> If you are talking about initializing
> ObjectClass/CPUClass/...Class fields, they can always be safely
> initialized in class_init.
Ok, so I can initialize the CPUClass fields for the tcg ops in the CPUClass
subclass class inits safely..
Thanks for clearing this up!
Claudio
>
> If you are talking about touching anything outside the class
> struct (like in CPUState), class_init is not the right place to
> do it.
>
