On 11/24/20 6:08 PM, Eduardo Habkost wrote:
> On Tue, Nov 24, 2020 at 05:22:07PM +0100, Claudio Fontana wrote:
>> apply this to the registration of the cpus accel interfaces,
>>
>> but this will be also in preparation for later use of this
>> new module init step to also register per-accel x86 cpu type
>> interfaces.
>>
>> Signed-off-by: Claudio Fontana <cfont...@suse.de>
>> ---
> [...]
>> diff --git a/accel/qtest/qtest.c b/accel/qtest/qtest.c
>> index b4e731cb2b..482f89729f 100644
>> --- a/accel/qtest/qtest.c
>> +++ b/accel/qtest/qtest.c
>> @@ -32,7 +32,6 @@ const CpusAccel qtest_cpus = {
>>
>> static int qtest_init_accel(MachineState *ms)
>> {
>> - cpus_register_accel(&qtest_cpus);
>> return 0;
>> }
>>
>> @@ -58,3 +57,12 @@ static void qtest_type_init(void)
>> }
>>
>> type_init(qtest_type_init);
>> +
>> +static void qtest_accel_cpu_init(void)
>> +{
>> + if (qtest_enabled()) {
>> + cpus_register_accel(&qtest_cpus);
>> + }
>> +}
>> +
>> +accel_cpu_init(qtest_accel_cpu_init);
>
> I don't understand why this (and the similar changes on other
> accelerators) is an improvement.
>
> You are replacing a trivial AccelClass-specific init method with
> a module_init() function that has a hidden dependency on runtime
> state.
>
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.
The "hidden" dependency on the fact that accels need to be initialized at that
time, is not hidden at all I think,
it is what this module init step is all about.
It is explicitly meaning, "_now that the current accelerator is chosen_,
perform these initializations".
But, as you mentioned elsewhere, I will in the meantime anyway squash these
things so they do not start fragmented at all, and centralize immediately.
Thanks,
Claudio
