[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU

From: Paolo Bonzini
Subject: Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU
Date: Wed, 18 Nov 2020 15:05:42 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0

On 18/11/20 14:48, Claudio Fontana wrote:
On 11/18/20 1:48 PM, Eduardo Habkost wrote:
On Wed, Nov 18, 2020 at 11:29:35AM +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 defer the registration of the cpu models,
in order to make them subclasses of a per-accel cpu type.

Signed-off-by: Claudio Fontana <cfontana@suse.de>
+    /*
+     * accelerator has been chosen and initialized, now it is time to
+     * register the cpu accel interface.
+     */
+    module_call_init(MODULE_INIT_ACCEL_CPU);

I don't get why we would use a new module initialization level

To have a clear point in time after which all accelerator interface 
initialization is done.
It avoids to have to hunt down the registration points spread around the code 
I'd turn it around, why not?

I see two disadvantages:

1) you have to hunt down accel_cpu_inits instead of looking at accelerator classes. :)

2) all callbacks have an "if (*_enabled())" around the actual meat. Another related issue is that usually the module_call_init are unconditional.

I think the idea of using module_call_init is good however.  What about:

static void kvm_cpu_accel_init(void)

static void kvm_cpu_accel_register(void)
    accel_register_call(TYPE_KVM, kvm_cpu_accel_init);


accel_register_call(const char *qom_type, void (*fn)(void))
    AccelClass *acc = ACCEL_CLASS(object_class_by_name(qom_type));

    acc->setup_calls = g_slist_append(acc->setup_calls, (void *)fn);

accel_do_call(void *data, void *unused)
    void (*fn)(void) = data;


int accel_init_machine(AccelState *accel, MachineState *ms)
    if (ret < 0) {
        ms->accelerator = NULL;
        *(acc->allowed) = false;
    } else {
        g_slist_foreach(acc->setup_calls, accel_do_call, NULL);
    return ret;

where the module_call_init would be right after MODULE_INIT_QOM


for this.  If the accelerator object was already created, we can
just ask the existing accel object to do whatever initialization
step is necessary.

e.g. we can add a AccelClass.cpu_accel_ops field, and call:


_When_ this is done is the question, in my view, where the call to the 
registration is placed.

After adding additonal operations that have to be done at "accelerator-chosen" 
time, it becomes more and more difficult to trace them around the codebase.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]