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 <cfont...@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
base.
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)
{
x86_cpu_accel_init(&kvm_cpu_accel);
}
static void kvm_cpu_accel_register(void)
{
accel_register_call(TYPE_KVM, kvm_cpu_accel_init);
}
accel_cpu_init(kvm_cpu_accel_register);
...
void
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);
}
void
accel_do_call(void *data, void *unused)
{
void (*fn)(void) = data;
data();
}
int accel_init_machine(AccelState *accel, MachineState *ms)
{
...
if (ret < 0) {
ms->accelerator = NULL;
*(acc->allowed) = false;
object_unref(OBJECT(accel));
} else {
object_set_accelerator_compat_props(acc->compat_props);
g_slist_foreach(acc->setup_calls, accel_do_call, NULL);
}
return ret;
}
where the module_call_init would be right after MODULE_INIT_QOM
Paolo
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:
cpus_register_accel(current_machine->accelerator->cpu_accel_ops);
_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.
Thanks,
Claudio
