Re: [RFC v5 09/12] module: introduce MODULE_INIT_ACCEL_CPU

[Claudio Fontana]

On 11/24/20 9:01 PM, Paolo Bonzini wrote:
> On 24/11/20 20:08, Eduardo Habkost wrote:
>>> Not a big advantage I agree,
>>> I think however there is one, in using the existing framework that exists, 
>>> for the purposes that it was built for.
>>>
>>> As I understand it, the global module init framework is supposed to mark 
>>> the major initialization steps,
>>> and this seems to fit the bill.
>> That seems to be the main source of disagreement.  I don't agree
>> that's the purpose of module_init().
>>
>> The module init framework is used to unconditionally register
>> module-provided entities like option names, QOM types, block
>> drivers, trace events, etc.  The entities registered by module
>> init functions represent a passive dynamically loadable piece of
>> code.
> 
> Indeed.  Think of module_init() as C++ global constructors.
> 
> Anything that has an "if" does not belong in module_init.
> 
> If you look at my review of the previous versions, I was not necessarily 
> against MODULE_INIT_ACCEL_CPU, but I was (and am) strongly against 
> calling it in the middle of the machine creation sequence.
> 
> Paolo
> 
> 

Hi Paolo,

in RFC v5 , module init for ACCEL_CPU is not conditional anymore, right?
But the fact that its behavior depends on current_accel() still disqualifies it?

It is called right after the accelerator is chosen and initialized in RFC v5, 
this still is "in the middle of the machine creation sequence"?

I am trying to find the actual things to fix, since when doing RFC v5 I tried 
to specifically address two points:

1) no if () inside module init functions

2) no proliferation of module init functions

which I accomplished via AccelClass extension to user mode, current_accel(), 
and class lookup.

If MODULE_INIT_ACCEL_CPU remains an option, where would you like to see the 
call so that it is not "in the middle"?

It is interesting for me to try to discern which meaning you folks give to 
MODULE_INIT.

Keep in mind, I will experiment with Eduardo's option of "one accel_init() to 
rule them all", without MODULE_INIT_ACCEL_CPU,
so I am not focused on using this no matter what.

Ciao,

Claudio