Am 16.02.2012 00:16, schrieb Igor Mammedov: > Convert pc cpu to qdev device that is attached to icc bus, later > hot-plug ability of icc bus will allow to implement cpu hot-plug. > > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
This conflicts with CPU QOM'ification across targets (and no longer applies due to type_init() introduction). > --- > hw/pc.c | 62 +++++++++++++++++++++++++++++++++++++++++++------ > target-i386/cpu.h | 1 + > target-i386/helper.c | 26 ++++++++++++++------ > 3 files changed, 73 insertions(+), 16 deletions(-) > > diff --git a/hw/pc.c b/hw/pc.c > index 33d8090..b8db5dc 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -922,6 +922,12 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int > level) > } > } > > +typedef struct CPUPC { > + ICCBusDevice busdev; > + char *model; > + CPUState state; > +} CPUPC; I don't like this approach. For starters, using CPUState as field rather than pointer seems a bad idea to me (CPUX86State would only be slightly better). We should have a single code path through which we instantiate CPUs, currently: cpu_$arch_init(const char *cpu_model) With my series completed, this would return an X86CPU object. Depending on our liking, we could either place some ICC / APIC / whatever fields directly into that, or embed the X86CPU in an object such as yours above as a link<X86CPU>. I do feel however that the model string is misplaced there. Question is whether this ICC stuff is actually part of the CPU or part of the CPU wiring on the mainboard - I vaguely remember someone saying that this changed over time...? Having both depending on CPU subclass might also be an option, but I'd rather leave such decisions as a follow-up to the core QOM'ification. Andreas > + > static void pc_cpu_reset(void *opaque) > { > CPUState *env = opaque; > @@ -930,23 +936,63 @@ static void pc_cpu_reset(void *opaque) > env->halted = !cpu_is_bsp(env); > } > > -static CPUState *pc_new_cpu(const char *cpu_model) > +static DeviceState *pc_new_cpu(const char *cpu_model) > { > - CPUState *env; > + DeviceState *dev; > + BusState *b; > > - env = cpu_init(cpu_model); > - if (!env) { > - fprintf(stderr, "Unable to find x86 CPU definition\n"); > - exit(1); > + b = get_icc_bus(); > + dev = qdev_create(b, "cpu-pc"); > + if (!dev) { > + return NULL; > + } > + qdev_prop_set_string(dev, "model", g_strdup(cpu_model)); > + if (qdev_init(dev) < 0) { > + return NULL; > + } > + return dev; > +} > + > +static int cpu_device_init(ICCBusDevice *dev) > +{ > + CPUPC* cpu = DO_UPCAST(CPUPC, busdev, dev); > + CPUState *env = &cpu->state; > + > + if (cpu_x86_init_inplace(env, cpu->model) < 0) { > + return -1; > } > + > if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) { > env->apic_state = apic_init(env, env->cpuid_apic_id); > } > - qemu_register_reset(pc_cpu_reset, env); > + > + return 0; > +} > + > +static void cpu_device_reset(DeviceState *dev) { > + CPUPC *cpu = DO_UPCAST(CPUPC, busdev.qdev, dev); > + CPUState *env = &cpu->state; > pc_cpu_reset(env); > - return env; > } > > +static ICCBusDeviceInfo cpu_device_info = { > + .qdev.name = "cpu-pc", > + .qdev.size = sizeof(CPUPC), > + .qdev.reset = cpu_device_reset, > + .init = cpu_device_init, > + .qdev.props = (Property[]) { > + DEFINE_PROP_STRING("model", CPUPC, model), > + DEFINE_PROP_END_OF_LIST(), > + }, > +}; > + > +static void pc_register_devices(void) > +{ > + iccbus_register_devinfo(&cpu_device_info); > +} > + > +device_init(pc_register_devices); > + > void pc_cpus_init(const char *cpu_model) > { > int i; > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index 7e66bcf..830b65e 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -775,6 +775,7 @@ typedef struct CPUX86State { > } CPUX86State; > > CPUX86State *cpu_x86_init(const char *cpu_model); > +int cpu_x86_init_inplace(CPUX86State *env, const char *cpu_model); > int cpu_x86_exec(CPUX86State *s); > void cpu_x86_close(CPUX86State *s); > void x86_cpu_list (FILE *f, fprintf_function cpu_fprintf, const char > *optarg); > diff --git a/target-i386/helper.c b/target-i386/helper.c > index 2586aff..df2f5ba 100644 > --- a/target-i386/helper.c > +++ b/target-i386/helper.c > @@ -1235,12 +1235,14 @@ int cpu_x86_get_descr_debug(CPUX86State *env, > unsigned int selector, > return 1; > } > > -CPUX86State *cpu_x86_init(const char *cpu_model) > +int cpu_x86_init_inplace(CPUX86State *env, const char *cpu_model) > { > - CPUX86State *env; > static int inited; > > - env = g_malloc0(sizeof(CPUX86State)); > + if (cpu_x86_register(env, cpu_model) < 0) { > + fprintf(stderr, "Unable to find x86 CPU definition\n"); > + return -1; > + } > cpu_exec_init(env); > env->cpu_model_str = cpu_model; > > @@ -1253,18 +1255,26 @@ CPUX86State *cpu_x86_init(const char *cpu_model) > cpu_set_debug_excp_handler(breakpoint_handler); > #endif > } > - if (cpu_x86_register(env, cpu_model) < 0) { > - cpu_x86_close(env); > - return NULL; > - } > env->cpuid_apic_id = env->cpu_index; > mce_init(env); > > qemu_init_vcpu(env); > > - return env; > + return 0; > } > > +CPUX86State *cpu_x86_init(const char *cpu_model) > +{ > + CPUX86State *env; > + > + env = g_malloc0(sizeof(CPUX86State)); > + if (cpu_x86_init_inplace(env, cpu_model) < 0) { > + g_free(env); > + return NULL; > + } > + > + return env; > +} > #if !defined(CONFIG_USER_ONLY) > void do_cpu_init(CPUState *env) > { -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg