| Message ID | 1351101001-14589-21-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 24 Oct 2012 15:49:54 -0200 Eduardo Habkost <ehabkost@redhat.com> wrote: > The PC code will need to run additional steps when initializing the CPU > object, before x86_cpu_realize(). So, make cpu_x86_init() not call Killing cpu_x86_init() altogether will make future re-factoring even easier. For present its code could be duplicated in cpu_init() and pc.c, and with cpu subclasses cpu_init () would be reduced to cpu = object_new(X86CPU.QEMUxx); cpu.realize(); and pc_cpus_init() cpu = object_new(X86CPU.QEMUxx); make cpu a child of /machine (); apply custom properties (); cpu.realize(); I don't see any benefits in keeping cpu_x86_init() around and if we start touching it then just lets get rid of it in one step. > x86_cpu_realize(), and add two x86_cpu_realize() calls: > > - One on cpu_init(), that is called only by *-user > - One on pc_cpu_init(), that will include the more advanced PC CPU > initialization steps > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > hw/pc.c | 12 +++++++++++- > target-i386/cpu.h | 14 ++++++++++++++ > target-i386/helper.c | 11 ++++------- > 3 files changed, 29 insertions(+), 8 deletions(-) > > diff --git a/hw/pc.c b/hw/pc.c > index 85eab04..c209d3d 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -861,10 +861,20 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int > level) > static void pc_cpu_init(PCInitArgs *args, int cpu_index) > { > - if (!cpu_x86_init(args->qemu_args->cpu_model)) { > + Error *err = NULL; > + X86CPU *cpu; > + > + cpu = cpu_x86_init(args->qemu_args->cpu_model); > + if (!cpu) { > fprintf(stderr, "Unable to find x86 CPU definition\n"); > exit(1); > } > + > + x86_cpu_realize(OBJECT(cpu), &err); > + if (err) { > + error_report("pc_cpu_init: %s\n", error_get_pretty(err)); > + exit(1); > + } > } > > void pc_cpus_init(PCInitArgs *args) > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index 871c270..6853b17 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -21,6 +21,7 @@ > > #include "config.h" > #include "qemu-common.h" > +#include "qemu-error.h" > > #ifdef TARGET_X86_64 > #define TARGET_LONG_BITS 64 > @@ -1008,12 +1009,25 @@ uint64_t cpu_get_tsc(CPUX86State *env); > #define TARGET_VIRT_ADDR_SPACE_BITS 32 > #endif > > +/* Helper for simple CPU initialization (for target-independent code) > + * > + * Note that the PC code doesn't use this function, as it does additional > + * initialization steps between cpu_x86_init() and cpu_x86_realize() is > called. > + */ > static inline CPUX86State *cpu_init(const char *cpu_model) > { > + Error *err = NULL; > X86CPU *cpu = cpu_x86_init(cpu_model); > if (cpu == NULL) { > return NULL; > } > + > + x86_cpu_realize(OBJECT(cpu), &err); > + if (err) { > + error_report("cpu_init: %s\n", error_get_pretty(err)); > + return NULL; > + } > + > return &cpu->env; > } > > diff --git a/target-i386/helper.c b/target-i386/helper.c > index 1e5f61f..87a9221 100644 > --- a/target-i386/helper.c > +++ b/target-i386/helper.c > @@ -1240,11 +1240,14 @@ int cpu_x86_get_descr_debug(CPUX86State *env, > unsigned int selector, return 1; > } > > +/* Initialize X86CPU object > + * > + * Callers must eventually call x86_cpu_realize(), to finish > initialization. > + */ > X86CPU *cpu_x86_init(const char *cpu_model) > { > X86CPU *cpu; > CPUX86State *env; > - Error *err = NULL; > > cpu = X86_CPU(object_new(TYPE_X86_CPU)); > env = &cpu->env; > @@ -1255,12 +1258,6 @@ X86CPU *cpu_x86_init(const char *cpu_model) > return NULL; > } > > - x86_cpu_realize(OBJECT(cpu), &err); > - if (err) { > - error_report("cpu_x86_init: %s\n", error_get_pretty(err)); > - return NULL; > - } > - > return cpu; > } >
Am 31.10.2012 17:32, schrieb Igor Mammedov: > On Wed, 24 Oct 2012 15:49:54 -0200 > Eduardo Habkost <ehabkost@redhat.com> wrote: > >> The PC code will need to run additional steps when initializing the CPU >> object, before x86_cpu_realize(). So, make cpu_x86_init() not call > Killing cpu_x86_init() altogether will make future re-factoring even easier. > For present its code could be duplicated in cpu_init() and pc.c, > > and with cpu subclasses cpu_init () would be reduced to > cpu = object_new(X86CPU.QEMUxx); > cpu.realize(); > and pc_cpus_init() > cpu = object_new(X86CPU.QEMUxx); > make cpu a child of /machine (); > apply custom properties (); > cpu.realize(); > > I don't see any benefits in keeping cpu_x86_init() around and if we start > touching it then just lets get rid of it in one step. To my regret, CPU subclasses have moved to the end of your two queues. I was considering doing a proposal to fast-track that for symmetry with the other targets and gradually improve that with the pending properties series (that now depend on qdev, which I find rather nasty to review), but I have my hands quite full currently, no promises... Andreas > >> x86_cpu_realize(), and add two x86_cpu_realize() calls: >> >> - One on cpu_init(), that is called only by *-user >> - One on pc_cpu_init(), that will include the more advanced PC CPU >> initialization steps >> >> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> >> --- >> hw/pc.c | 12 +++++++++++- >> target-i386/cpu.h | 14 ++++++++++++++ >> target-i386/helper.c | 11 ++++------- >> 3 files changed, 29 insertions(+), 8 deletions(-) >> >> diff --git a/hw/pc.c b/hw/pc.c >> index 85eab04..c209d3d 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -861,10 +861,20 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int >> level) >> static void pc_cpu_init(PCInitArgs *args, int cpu_index) >> { >> - if (!cpu_x86_init(args->qemu_args->cpu_model)) { >> + Error *err = NULL; >> + X86CPU *cpu; >> + >> + cpu = cpu_x86_init(args->qemu_args->cpu_model); >> + if (!cpu) { >> fprintf(stderr, "Unable to find x86 CPU definition\n"); >> exit(1); >> } >> + >> + x86_cpu_realize(OBJECT(cpu), &err); >> + if (err) { >> + error_report("pc_cpu_init: %s\n", error_get_pretty(err)); >> + exit(1); >> + } >> } >> >> void pc_cpus_init(PCInitArgs *args) >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h >> index 871c270..6853b17 100644 >> --- a/target-i386/cpu.h >> +++ b/target-i386/cpu.h >> @@ -21,6 +21,7 @@ >> >> #include "config.h" >> #include "qemu-common.h" >> +#include "qemu-error.h" >> >> #ifdef TARGET_X86_64 >> #define TARGET_LONG_BITS 64 >> @@ -1008,12 +1009,25 @@ uint64_t cpu_get_tsc(CPUX86State *env); >> #define TARGET_VIRT_ADDR_SPACE_BITS 32 >> #endif >> >> +/* Helper for simple CPU initialization (for target-independent code) >> + * >> + * Note that the PC code doesn't use this function, as it does additional >> + * initialization steps between cpu_x86_init() and cpu_x86_realize() is >> called. >> + */ >> static inline CPUX86State *cpu_init(const char *cpu_model) >> { >> + Error *err = NULL; >> X86CPU *cpu = cpu_x86_init(cpu_model); >> if (cpu == NULL) { >> return NULL; >> } >> + >> + x86_cpu_realize(OBJECT(cpu), &err); >> + if (err) { >> + error_report("cpu_init: %s\n", error_get_pretty(err)); >> + return NULL; >> + } >> + >> return &cpu->env; >> } >> >> diff --git a/target-i386/helper.c b/target-i386/helper.c >> index 1e5f61f..87a9221 100644 >> --- a/target-i386/helper.c >> +++ b/target-i386/helper.c >> @@ -1240,11 +1240,14 @@ int cpu_x86_get_descr_debug(CPUX86State *env, >> unsigned int selector, return 1; >> } >> >> +/* Initialize X86CPU object >> + * >> + * Callers must eventually call x86_cpu_realize(), to finish >> initialization. >> + */ >> X86CPU *cpu_x86_init(const char *cpu_model) >> { >> X86CPU *cpu; >> CPUX86State *env; >> - Error *err = NULL; >> >> cpu = X86_CPU(object_new(TYPE_X86_CPU)); >> env = &cpu->env; >> @@ -1255,12 +1258,6 @@ X86CPU *cpu_x86_init(const char *cpu_model) >> return NULL; >> } >> >> - x86_cpu_realize(OBJECT(cpu), &err); >> - if (err) { >> - error_report("cpu_x86_init: %s\n", error_get_pretty(err)); >> - return NULL; >> - } >> - >> return cpu; >> } >> >
On Wed, Oct 31, 2012 at 05:32:33PM +0100, Igor Mammedov wrote: > On Wed, 24 Oct 2012 15:49:54 -0200 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > The PC code will need to run additional steps when initializing the CPU > > object, before x86_cpu_realize(). So, make cpu_x86_init() not call > Killing cpu_x86_init() altogether will make future re-factoring even easier. > For present its code could be duplicated in cpu_init() and pc.c, > > and with cpu subclasses cpu_init () would be reduced to > cpu = object_new(X86CPU.QEMUxx); > cpu.realize(); I suspect *-user supports +feature,-feature on the CPU model string as well. So both cases the cpu_model compat string parsing/property-setting and the CPU class lookup would be necessary. So cpu_init() wouldn't look so simple as above. > and pc_cpus_init() > cpu = object_new(X86CPU.QEMUxx); > make cpu a child of /machine (); > apply custom properties (); > cpu.realize(); > > I don't see any benefits in keeping cpu_x86_init() around and if we start > touching it then just lets get rid of it in one step. I believe the sequence that creates the CPU object will look like this on *-user: compat_normalize_cpu_model(cpu_model, &class_name, &features); class = type_get_by_name(class_name); cpu = object_new(class); cpu_x86_set_props(cpu, features); cpu.realize(); And on PC it will look like: compat_normalize_cpu_model(cpu_model, &class_name, &features); class = type_get_by_name(class_name); cpu = object_new(class); cpu_x86_set_props(cpu, features); cpu_init_steps_sepcific_for_pc(cpu); /* APIC ID, make a child of * /machine, whatever else. */ cpu.realize(); If cpu_init() was going to be just object_new() + cpu.realize(), I wouldn't mind duplicating the code. But I don't see a reason to duplicate code and not use a common function for the "cpu_model parsing + class lookup + object_new() + compat props setting" steps. I think cpu_x86_init() can be that common function. > > > x86_cpu_realize(), and add two x86_cpu_realize() calls: > > > > - One on cpu_init(), that is called only by *-user > > - One on pc_cpu_init(), that will include the more advanced PC CPU > > initialization steps > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > hw/pc.c | 12 +++++++++++- > > target-i386/cpu.h | 14 ++++++++++++++ > > target-i386/helper.c | 11 ++++------- > > 3 files changed, 29 insertions(+), 8 deletions(-) > > > > diff --git a/hw/pc.c b/hw/pc.c > > index 85eab04..c209d3d 100644 > > --- a/hw/pc.c > > +++ b/hw/pc.c > > @@ -861,10 +861,20 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int > > level) > > static void pc_cpu_init(PCInitArgs *args, int cpu_index) > > { > > - if (!cpu_x86_init(args->qemu_args->cpu_model)) { > > + Error *err = NULL; > > + X86CPU *cpu; > > + > > + cpu = cpu_x86_init(args->qemu_args->cpu_model); > > + if (!cpu) { > > fprintf(stderr, "Unable to find x86 CPU definition\n"); > > exit(1); > > } > > + > > + x86_cpu_realize(OBJECT(cpu), &err); > > + if (err) { > > + error_report("pc_cpu_init: %s\n", error_get_pretty(err)); > > + exit(1); > > + } > > } > > > > void pc_cpus_init(PCInitArgs *args) > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > > index 871c270..6853b17 100644 > > --- a/target-i386/cpu.h > > +++ b/target-i386/cpu.h > > @@ -21,6 +21,7 @@ > > > > #include "config.h" > > #include "qemu-common.h" > > +#include "qemu-error.h" > > > > #ifdef TARGET_X86_64 > > #define TARGET_LONG_BITS 64 > > @@ -1008,12 +1009,25 @@ uint64_t cpu_get_tsc(CPUX86State *env); > > #define TARGET_VIRT_ADDR_SPACE_BITS 32 > > #endif > > > > +/* Helper for simple CPU initialization (for target-independent code) > > + * > > + * Note that the PC code doesn't use this function, as it does additional > > + * initialization steps between cpu_x86_init() and cpu_x86_realize() is > > called. > > + */ > > static inline CPUX86State *cpu_init(const char *cpu_model) > > { > > + Error *err = NULL; > > X86CPU *cpu = cpu_x86_init(cpu_model); > > if (cpu == NULL) { > > return NULL; > > } > > + > > + x86_cpu_realize(OBJECT(cpu), &err); > > + if (err) { > > + error_report("cpu_init: %s\n", error_get_pretty(err)); > > + return NULL; > > + } > > + > > return &cpu->env; > > } > > > > diff --git a/target-i386/helper.c b/target-i386/helper.c > > index 1e5f61f..87a9221 100644 > > --- a/target-i386/helper.c > > +++ b/target-i386/helper.c > > @@ -1240,11 +1240,14 @@ int cpu_x86_get_descr_debug(CPUX86State *env, > > unsigned int selector, return 1; > > } > > > > +/* Initialize X86CPU object > > + * > > + * Callers must eventually call x86_cpu_realize(), to finish > > initialization. > > + */ > > X86CPU *cpu_x86_init(const char *cpu_model) > > { > > X86CPU *cpu; > > CPUX86State *env; > > - Error *err = NULL; > > > > cpu = X86_CPU(object_new(TYPE_X86_CPU)); > > env = &cpu->env; > > @@ -1255,12 +1258,6 @@ X86CPU *cpu_x86_init(const char *cpu_model) > > return NULL; > > } > > > > - x86_cpu_realize(OBJECT(cpu), &err); > > - if (err) { > > - error_report("cpu_x86_init: %s\n", error_get_pretty(err)); > > - return NULL; > > - } > > - > > return cpu; > > } > > >
On Wed, Oct 31, 2012 at 05:43:48PM +0100, Andreas Färber wrote: > Am 31.10.2012 17:32, schrieb Igor Mammedov: > > On Wed, 24 Oct 2012 15:49:54 -0200 > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > >> The PC code will need to run additional steps when initializing the CPU > >> object, before x86_cpu_realize(). So, make cpu_x86_init() not call > > Killing cpu_x86_init() altogether will make future re-factoring even easier. > > For present its code could be duplicated in cpu_init() and pc.c, > > > > and with cpu subclasses cpu_init () would be reduced to > > cpu = object_new(X86CPU.QEMUxx); > > cpu.realize(); > > and pc_cpus_init() > > cpu = object_new(X86CPU.QEMUxx); > > make cpu a child of /machine (); > > apply custom properties (); > > cpu.realize(); > > > > I don't see any benefits in keeping cpu_x86_init() around and if we start > > touching it then just lets get rid of it in one step. > > To my regret, CPU subclasses have moved to the end of your two queues. I > was considering doing a proposal to fast-track that for symmetry with > the other targets and gradually improve that with the pending properties > series (that now depend on qdev, which I find rather nasty to review), > but I have my hands quite full currently, no promises... I always planned to send the CPU classes code after the CPU properties are in, but maybe we can reverse the order. I am not sure if we have time for that. What would be a reasonable deadline to have a CPU classes series submitted, so it would be reasonable/feasible to review it and get it into 1.3? Yesterday? ;-) > > Andreas > > > > >> x86_cpu_realize(), and add two x86_cpu_realize() calls: > >> > >> - One on cpu_init(), that is called only by *-user > >> - One on pc_cpu_init(), that will include the more advanced PC CPU > >> initialization steps > >> > >> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > >> --- > >> hw/pc.c | 12 +++++++++++- > >> target-i386/cpu.h | 14 ++++++++++++++ > >> target-i386/helper.c | 11 ++++------- > >> 3 files changed, 29 insertions(+), 8 deletions(-) > >> > >> diff --git a/hw/pc.c b/hw/pc.c > >> index 85eab04..c209d3d 100644 > >> --- a/hw/pc.c > >> +++ b/hw/pc.c > >> @@ -861,10 +861,20 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int > >> level) > >> static void pc_cpu_init(PCInitArgs *args, int cpu_index) > >> { > >> - if (!cpu_x86_init(args->qemu_args->cpu_model)) { > >> + Error *err = NULL; > >> + X86CPU *cpu; > >> + > >> + cpu = cpu_x86_init(args->qemu_args->cpu_model); > >> + if (!cpu) { > >> fprintf(stderr, "Unable to find x86 CPU definition\n"); > >> exit(1); > >> } > >> + > >> + x86_cpu_realize(OBJECT(cpu), &err); > >> + if (err) { > >> + error_report("pc_cpu_init: %s\n", error_get_pretty(err)); > >> + exit(1); > >> + } > >> } > >> > >> void pc_cpus_init(PCInitArgs *args) > >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h > >> index 871c270..6853b17 100644 > >> --- a/target-i386/cpu.h > >> +++ b/target-i386/cpu.h > >> @@ -21,6 +21,7 @@ > >> > >> #include "config.h" > >> #include "qemu-common.h" > >> +#include "qemu-error.h" > >> > >> #ifdef TARGET_X86_64 > >> #define TARGET_LONG_BITS 64 > >> @@ -1008,12 +1009,25 @@ uint64_t cpu_get_tsc(CPUX86State *env); > >> #define TARGET_VIRT_ADDR_SPACE_BITS 32 > >> #endif > >> > >> +/* Helper for simple CPU initialization (for target-independent code) > >> + * > >> + * Note that the PC code doesn't use this function, as it does additional > >> + * initialization steps between cpu_x86_init() and cpu_x86_realize() is > >> called. > >> + */ > >> static inline CPUX86State *cpu_init(const char *cpu_model) > >> { > >> + Error *err = NULL; > >> X86CPU *cpu = cpu_x86_init(cpu_model); > >> if (cpu == NULL) { > >> return NULL; > >> } > >> + > >> + x86_cpu_realize(OBJECT(cpu), &err); > >> + if (err) { > >> + error_report("cpu_init: %s\n", error_get_pretty(err)); > >> + return NULL; > >> + } > >> + > >> return &cpu->env; > >> } > >> > >> diff --git a/target-i386/helper.c b/target-i386/helper.c > >> index 1e5f61f..87a9221 100644 > >> --- a/target-i386/helper.c > >> +++ b/target-i386/helper.c > >> @@ -1240,11 +1240,14 @@ int cpu_x86_get_descr_debug(CPUX86State *env, > >> unsigned int selector, return 1; > >> } > >> > >> +/* Initialize X86CPU object > >> + * > >> + * Callers must eventually call x86_cpu_realize(), to finish > >> initialization. > >> + */ > >> X86CPU *cpu_x86_init(const char *cpu_model) > >> { > >> X86CPU *cpu; > >> CPUX86State *env; > >> - Error *err = NULL; > >> > >> cpu = X86_CPU(object_new(TYPE_X86_CPU)); > >> env = &cpu->env; > >> @@ -1255,12 +1258,6 @@ X86CPU *cpu_x86_init(const char *cpu_model) > >> return NULL; > >> } > >> > >> - x86_cpu_realize(OBJECT(cpu), &err); > >> - if (err) { > >> - error_report("cpu_x86_init: %s\n", error_get_pretty(err)); > >> - return NULL; > >> - } > >> - > >> return cpu; > >> } > >> > > > > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
On Wed, 31 Oct 2012 17:43:48 +0100 Andreas Färber <afaerber@suse.de> wrote: > Am 31.10.2012 17:32, schrieb Igor Mammedov: > > On Wed, 24 Oct 2012 15:49:54 -0200 > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > >> The PC code will need to run additional steps when initializing the CPU > >> object, before x86_cpu_realize(). So, make cpu_x86_init() not call > > Killing cpu_x86_init() altogether will make future re-factoring even > > easier. For present its code could be duplicated in cpu_init() and pc.c, > > > > and with cpu subclasses cpu_init () would be reduced to > > cpu = object_new(X86CPU.QEMUxx); > > cpu.realize(); > > and pc_cpus_init() > > cpu = object_new(X86CPU.QEMUxx); > > make cpu a child of /machine (); > > apply custom properties (); > > cpu.realize(); > > > > I don't see any benefits in keeping cpu_x86_init() around and if we start > > touching it then just lets get rid of it in one step. > > To my regret, CPU subclasses have moved to the end of your two queues. I > was considering doing a proposal to fast-track that for symmetry with > the other targets and gradually improve that with the pending properties I could revive not qdev based version of cpu properties so we could move forward if Anthony won't object (he argued in favor of qdev based one on IRC). > series (that now depend on qdev, which I find rather nasty to review), Is there anything that could be done to improve review-ability of cpu-as-qdev > but I have my hands quite full currently, no promises... > > Andreas > > > > >> x86_cpu_realize(), and add two x86_cpu_realize() calls: > >> > >> - One on cpu_init(), that is called only by *-user > >> - One on pc_cpu_init(), that will include the more advanced PC CPU > >> initialization steps > >> > >> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > >> --- > >> hw/pc.c | 12 +++++++++++- > >> target-i386/cpu.h | 14 ++++++++++++++ > >> target-i386/helper.c | 11 ++++------- > >> 3 files changed, 29 insertions(+), 8 deletions(-) > >> > >> diff --git a/hw/pc.c b/hw/pc.c > >> index 85eab04..c209d3d 100644 > >> --- a/hw/pc.c > >> +++ b/hw/pc.c > >> @@ -861,10 +861,20 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, > >> int level) > >> static void pc_cpu_init(PCInitArgs *args, int cpu_index) > >> { > >> - if (!cpu_x86_init(args->qemu_args->cpu_model)) { > >> + Error *err = NULL; > >> + X86CPU *cpu; > >> + > >> + cpu = cpu_x86_init(args->qemu_args->cpu_model); > >> + if (!cpu) { > >> fprintf(stderr, "Unable to find x86 CPU definition\n"); > >> exit(1); > >> } > >> + > >> + x86_cpu_realize(OBJECT(cpu), &err); > >> + if (err) { > >> + error_report("pc_cpu_init: %s\n", error_get_pretty(err)); > >> + exit(1); > >> + } > >> } > >> > >> void pc_cpus_init(PCInitArgs *args) > >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h > >> index 871c270..6853b17 100644 > >> --- a/target-i386/cpu.h > >> +++ b/target-i386/cpu.h > >> @@ -21,6 +21,7 @@ > >> > >> #include "config.h" > >> #include "qemu-common.h" > >> +#include "qemu-error.h" > >> > >> #ifdef TARGET_X86_64 > >> #define TARGET_LONG_BITS 64 > >> @@ -1008,12 +1009,25 @@ uint64_t cpu_get_tsc(CPUX86State *env); > >> #define TARGET_VIRT_ADDR_SPACE_BITS 32 > >> #endif > >> > >> +/* Helper for simple CPU initialization (for target-independent code) > >> + * > >> + * Note that the PC code doesn't use this function, as it does > >> additional > >> + * initialization steps between cpu_x86_init() and cpu_x86_realize() is > >> called. > >> + */ > >> static inline CPUX86State *cpu_init(const char *cpu_model) > >> { > >> + Error *err = NULL; > >> X86CPU *cpu = cpu_x86_init(cpu_model); > >> if (cpu == NULL) { > >> return NULL; > >> } > >> + > >> + x86_cpu_realize(OBJECT(cpu), &err); > >> + if (err) { > >> + error_report("cpu_init: %s\n", error_get_pretty(err)); > >> + return NULL; > >> + } > >> + > >> return &cpu->env; > >> } > >> > >> diff --git a/target-i386/helper.c b/target-i386/helper.c > >> index 1e5f61f..87a9221 100644 > >> --- a/target-i386/helper.c > >> +++ b/target-i386/helper.c > >> @@ -1240,11 +1240,14 @@ int cpu_x86_get_descr_debug(CPUX86State *env, > >> unsigned int selector, return 1; > >> } > >> > >> +/* Initialize X86CPU object > >> + * > >> + * Callers must eventually call x86_cpu_realize(), to finish > >> initialization. > >> + */ > >> X86CPU *cpu_x86_init(const char *cpu_model) > >> { > >> X86CPU *cpu; > >> CPUX86State *env; > >> - Error *err = NULL; > >> > >> cpu = X86_CPU(object_new(TYPE_X86_CPU)); > >> env = &cpu->env; > >> @@ -1255,12 +1258,6 @@ X86CPU *cpu_x86_init(const char *cpu_model) > >> return NULL; > >> } > >> > >> - x86_cpu_realize(OBJECT(cpu), &err); > >> - if (err) { > >> - error_report("cpu_x86_init: %s\n", error_get_pretty(err)); > >> - return NULL; > >> - } > >> - > >> return cpu; > >> } > >> > > > >
diff --git a/hw/pc.c b/hw/pc.c index 85eab04..c209d3d 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -861,10 +861,20 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level) static void pc_cpu_init(PCInitArgs *args, int cpu_index) { - if (!cpu_x86_init(args->qemu_args->cpu_model)) { + Error *err = NULL; + X86CPU *cpu; + + cpu = cpu_x86_init(args->qemu_args->cpu_model); + if (!cpu) { fprintf(stderr, "Unable to find x86 CPU definition\n"); exit(1); } + + x86_cpu_realize(OBJECT(cpu), &err); + if (err) { + error_report("pc_cpu_init: %s\n", error_get_pretty(err)); + exit(1); + } } void pc_cpus_init(PCInitArgs *args) diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 871c270..6853b17 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -21,6 +21,7 @@ #include "config.h" #include "qemu-common.h" +#include "qemu-error.h" #ifdef TARGET_X86_64 #define TARGET_LONG_BITS 64 @@ -1008,12 +1009,25 @@ uint64_t cpu_get_tsc(CPUX86State *env); #define TARGET_VIRT_ADDR_SPACE_BITS 32 #endif +/* Helper for simple CPU initialization (for target-independent code) + * + * Note that the PC code doesn't use this function, as it does additional + * initialization steps between cpu_x86_init() and cpu_x86_realize() is called. + */ static inline CPUX86State *cpu_init(const char *cpu_model) { + Error *err = NULL; X86CPU *cpu = cpu_x86_init(cpu_model); if (cpu == NULL) { return NULL; } + + x86_cpu_realize(OBJECT(cpu), &err); + if (err) { + error_report("cpu_init: %s\n", error_get_pretty(err)); + return NULL; + } + return &cpu->env; } diff --git a/target-i386/helper.c b/target-i386/helper.c index 1e5f61f..87a9221 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -1240,11 +1240,14 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector, return 1; } +/* Initialize X86CPU object + * + * Callers must eventually call x86_cpu_realize(), to finish initialization. + */ X86CPU *cpu_x86_init(const char *cpu_model) { X86CPU *cpu; CPUX86State *env; - Error *err = NULL; cpu = X86_CPU(object_new(TYPE_X86_CPU)); env = &cpu->env; @@ -1255,12 +1258,6 @@ X86CPU *cpu_x86_init(const char *cpu_model) return NULL; } - x86_cpu_realize(OBJECT(cpu), &err); - if (err) { - error_report("cpu_x86_init: %s\n", error_get_pretty(err)); - return NULL; - } - return cpu; }
The PC code will need to run additional steps when initializing the CPU object, before x86_cpu_realize(). So, make cpu_x86_init() not call x86_cpu_realize(), and add two x86_cpu_realize() calls: - One on cpu_init(), that is called only by *-user - One on pc_cpu_init(), that will include the more advanced PC CPU initialization steps Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/pc.c | 12 +++++++++++- target-i386/cpu.h | 14 ++++++++++++++ target-i386/helper.c | 11 ++++------- 3 files changed, 29 insertions(+), 8 deletions(-)