[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: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH 20/27] target-i386: do not call x86_cpu_realize() on cpu_x86_init() |
Date: |
Wed, 31 Oct 2012 17:43:48 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121025 Thunderbird/16.0.2 |
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...
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
- [Qemu-devel] [PATCH 19/27] cpu_x86_init: check for x86_cpu_realize() errors, (continued)
- [Qemu-devel] [PATCH 19/27] cpu_x86_init: check for x86_cpu_realize() errors, Eduardo Habkost, 2012/10/24
- [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