On 11/26/20 2:44 PM, Eduardo Habkost wrote:
> On Thu, Nov 26, 2020 at 11:57:28AM +0100, Claudio Fontana wrote:
>> On 11/24/20 10:31 PM, Eduardo Habkost wrote:
>>> On Tue, Nov 24, 2020 at 09:13:13PM +0100, Paolo Bonzini wrote:
>>>> On 24/11/20 17:22, Claudio Fontana wrote:
>>>>> +static void x86_cpu_accel_init(void)
>>>>> {
>>>>> - X86CPUAccelClass *acc;
>>>>> + const char *ac_name;
>>>>> + ObjectClass *ac;
>>>>> + char *xac_name;
>>>>> + ObjectClass *xac;
>>>>> - acc = X86_CPU_ACCEL_CLASS(object_class_by_name(accel_name));
>>>>> - g_assert(acc != NULL);
>>>>> + ac = object_get_class(OBJECT(current_accel()));
>>>>> + g_assert(ac != NULL);
>>>>> + ac_name = object_class_get_name(ac);
>>>>> + g_assert(ac_name != NULL);
>>>>> - object_class_foreach(x86_cpu_accel_init_aux, TYPE_X86_CPU, false,
>>>>> &acc);
>>>>> + xac_name = g_strdup_printf("%s-%s", ac_name, TYPE_X86_CPU);
>>>>> + xac = object_class_by_name(xac_name);
>>>>> + g_free(xac_name);
>>>>> +
>>>>> + if (xac) {
>>>>> + object_class_foreach(x86_cpu_accel_init_aux, TYPE_X86_CPU,
>>>>> false, xac);
>>>>> + }
>>>>> }
>>>>> +
>>>>> +accel_cpu_init(x86_cpu_accel_init);
>>>>
>>>> If this and cpus_accel_ops_init are the only call to accel_cpu_init, I'd
>>>> rather make them functions in CPUClass (which you find and call via
>>>> CPU_RESOLVING_TYPE) and AccelClass respectively.
>>>
>>> Making x86_cpu_accel_init() be a CPUClass method sounds like a
>>> good idea. This way we won't need a arch_cpu_accel_init() stub
>>> for non-x86.
>>>
>>> accel.c can't use cpu.h, correct? We can add a:
>>>
>>> CPUClass *arch_base_cpu_type(void)
>>> {
>>> return object_class_by_name(CPU_RESOLVING_TYPE);
>>> }
>>>
>>> function to arch_init.c, to allow target-independent code call
>>> target-specific code.
>>>
>>
>> Hi Eduardo,
>>
>> we can't use arch-init because it is softmmu only, but we could put this in
>> $(top_srcdir)/cpu.c
>
> That would work, too.
>
>>
>> however, it would be very useful to put a:
>>
>> #define TYPE_ACCEL_CPU "accel-" CPU_RESOLVING_TYPE
>> #define ACCEL_CPU_NAME(name) (name "-" TYPE_ACCEL_CPU)
>>
>> in an H file somewhere, for convenience for the programmer that
>> has to implement subclasses in target/xxx/
>
> Absolutely.
>
>>
>> But it is tough to find a header where CPU_RESOLVING_TYPE can be used.
>
> cpu-all.h?
>
>>
>> We could I guess just use plain "cpu" instead of CPU_RESOLVING_TYPE,
>> maybe that would be acceptable too? The interface ends up in CPUClass, so
>> maybe ok?
>>
>> So we'd end up having
>>
>> accel-cpu
>>
>> instead of the previous
>>
>> accel-x86_64-cpu
>>
>> on top of the hierarchy.
>
> It seems OK to have a accel-cpu type at the top, but I don't see
> why it solves the problem above. What exactly would be the value
> of `kvm_cpu_accel.name`?
>
It does solve the problem, because we can put then all AccelOpsClass and
AccelCPUClass stuff in accel.h,
resolve everything in accel/accel-*.c, and make a generic solution fairly
self-contained (already tested, will post soonish).
But I'll try cpu-all.h if it's preferred to have accel-x86_64-cpu,
accel-XXX-cpu on top, I wonder what the preference would be?
Ciao,
Claudio
