Re: [RFC v9 27/32] accel: replace struct CpusAccel with AccelOpsClass

["Alex Bennée" <alex.bennee@linaro.org>]

Claudio Fontana <cfontana@suse.de> writes:

> On 12/9/20 1:54 PM, Alex Bennée wrote:
>> 
>> Claudio Fontana <cfontana@suse.de> writes:
>> 
>>> centralize the registration of the cpus.c module
>>> accelerator operations in accel/accel-softmmu.c
>>>
>>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> <snip>
>>> diff --git a/accel/tcg/tcg-cpus.c b/accel/tcg/tcg-cpus.c
>>> index e335f9f155..38a58ab271 100644
>>> --- a/accel/tcg/tcg-cpus.c
>>> +++ b/accel/tcg/tcg-cpus.c
>>> @@ -35,6 +35,9 @@
>>>  #include "hw/boards.h"
>>>  
>>>  #include "tcg-cpus.h"
>>> +#include "tcg-cpus-mttcg.h"
>>> +#include "tcg-cpus-rr.h"
>>> +#include "tcg-cpus-icount.h"
>>>  
>>>  /* common functionality among all TCG variants */
>>>  
>>> @@ -80,3 +83,43 @@ void tcg_cpus_handle_interrupt(CPUState *cpu, int mask)
>>>          qatomic_set(&cpu_neg(cpu)->icount_decr.u16.high, -1);
>>>      }
>>>  }
>>> +
>>> +static void tcg_cpus_ops_init(AccelOpsClass *ops)
>>> +{
>>> +    if (qemu_tcg_mttcg_enabled()) {
>>> +        ops->create_vcpu_thread = mttcg_start_vcpu_thread;
>>> +        ops->kick_vcpu_thread = mttcg_kick_vcpu_thread;
>>> +        ops->handle_interrupt = tcg_cpus_handle_interrupt;
>>> +
>>> +    } else if (icount_enabled()) {
>>> +        ops->create_vcpu_thread = rr_start_vcpu_thread;
>>> +        ops->kick_vcpu_thread = rr_kick_vcpu_thread;
>>> +        ops->handle_interrupt = icount_handle_interrupt;
>>> +        ops->get_virtual_clock = icount_get;
>>> +        ops->get_elapsed_ticks = icount_get;
>>> +
>>> +    } else {
>>> +        ops->create_vcpu_thread = rr_start_vcpu_thread;
>>> +        ops->kick_vcpu_thread = rr_kick_vcpu_thread;
>>> +        ops->handle_interrupt = tcg_cpus_handle_interrupt;
>>> +    }
>>> +}
>> 
>> Aren't we going backwards here by having a global function aware of the
>> different accelerator types rather than encapsulating this is the
>> particular accelerator machinery?
>> 
>> <snip>
>> 
>
> Now I got your point.
>
> The ideal would be to have three classes. One called tcg-mttcg, one tcg-icount, one tcg-rr.
> The problem: backward compatibility I think, since currently we have only one accel, "tcg".
>
> But, hmm maybe fixable, I'll take a look.

Yeah I was wondering if we were going to have subclasses for each "type"
of TCG. But now I'm wondering if that even makes sense. Will we ever
want to built a TCG enabled binary that say doesn't do icount? Maybe
not - having a single AccelOpsClass which runs in 3 modes will probably
do for now.

>
> Thanks!
>
> Claudio


-- 
Alex Bennée