[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU
From: |
Eduardo Habkost |
Subject: |
Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU |
Date: |
Wed, 18 Nov 2020 09:36:43 -0500 |
On Wed, Nov 18, 2020 at 03:05:42PM +0100, Paolo Bonzini wrote:
> 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 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);
What do you expect x86_cpu_accel_init() to do?
> }
>
> 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);
Why all this extra complexity if you can simply do:
ACCEL_GET_CLASS(acc)->finish_arch_specific_init();
?
> }
> 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.
I don't understand why a separate module init level is necessary
here.
Making sure module_call_init() is called at the correct moment is
not easier or safer than just making sure accel_init_machine()
(or another init function you create) is called at the correct
moment.
--
Eduardo
- Re: [RFC v3 4/9] i386: hvf: remove stale MAINTAINERS entry for old hvf stubs, (continued)
- [RFC v3 5/9] i386: move TCG accel files into tcg/, Claudio Fontana, 2020/11/18
- [RFC v3 3/9] i386: move hax accel files into hax/, Claudio Fontana, 2020/11/18
- [RFC v3 6/9] i386: move cpu dump out of helper.c into cpu-dump.c, Claudio Fontana, 2020/11/18
- [RFC v3 7/9] i386: move TCG cpu class initialization out of helper.c, Claudio Fontana, 2020/11/18
- [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Claudio Fontana, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Claudio Fontana, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Eduardo Habkost, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Claudio Fontana, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Paolo Bonzini, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU,
Eduardo Habkost <=
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Paolo Bonzini, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Eduardo Habkost, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Paolo Bonzini, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Eduardo Habkost, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Paolo Bonzini, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Eduardo Habkost, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Paolo Bonzini, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Eduardo Habkost, 2020/11/18
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Claudio Fontana, 2020/11/20
- Re: [RFC v3 8/9] module: introduce MODULE_INIT_ACCEL_CPU, Eduardo Habkost, 2020/11/20