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`? -- Eduardo