Re: [RFC v6 10/11] accel: introduce AccelCPUClass extending CPUClass

[Eduardo Habkost]

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.

If you are talking about touching anything outside the class
struct (like in CPUState), class_init is not the right place to
do it.

-- 
Eduardo