[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 20/27] target-i386: do not call x86_cpu_realize(
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 20/27] target-i386: do not call x86_cpu_realize() on cpu_x86_init() |
Date: |
Wed, 31 Oct 2012 15:10:33 -0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden> 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 <address@hidden>
> >> ---
> >> 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
--
Eduardo
- [Qemu-devel] [PATCH 14/27] pc: use FWCfgState* instead of void* for fw_cfg data, (continued)
- [Qemu-devel] [PATCH 14/27] pc: use FWCfgState* instead of void* for fw_cfg data, Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 26/27] pc: create separate init function for pc-1.3, Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 05/27] kvm: create kvm_arch_vcpu_id() function, Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 11/27] pc: add PC_DEFAULT_CPU_MODEL #define, Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 01/27] move I/O-related definitions from qemu-common.h to a new header (qemu-stdio.h), Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 20/27] target-i386: do not call x86_cpu_realize() on cpu_x86_init(), Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 23/27] pc: set fw_cfg data based on APIC ID calculation, Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 02/27] cpus.h: include qemu-stdio.h, Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 12/27] pc: add PCInitArgs parameter to pc_cpus_init(), Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 16/27] pc: pass PCInitArgs struct to pc_bios_init(), Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 21/27] fw_cfg: remove FW_CFG_MAX_CPUS from fw_cfg_init(), Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 25/27] target-i386: topology & APIC ID utility functions, Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 27/27] pc: generate APIC IDs according to CPU topology, Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 06/27] target-i386: kvm: set vcpu_id to APIC ID instead of CPU index, Eduardo Habkost, 2012/10/24
- [Qemu-devel] [PATCH 22/27] pc: set CPU APIC ID explicitly, Eduardo Habkost, 2012/10/24