[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH qom-next 5/6] target-i386: make initialize CPU i
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH qom-next 5/6] target-i386: make initialize CPU in QOM way |
Date: |
Thu, 24 May 2012 11:29:54 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 |
On 05/23/2012 11:27 PM, Andreas Färber wrote:
Am 23.05.2012 18:39, schrieb Igor Mammedov:
Make CPU creation/initialization consistent with QOM object
behavior in this, by moving tcg and apic initialization from board
level into CPU's initfn/realize calls and cpu_model property setter.
Which makes CPU object self-sufficient in respect of creation/initialization
and matches a typical object creation sequence, i.e.:
- create CPU instance
- set properties
- realize object - (x86_cpu_realize will be converted into realize
property setter, when it is implemented)
v2:
- fix moving of tcg_* initialization into cpu.c from helper.c
spotted-by:<Jan Kiszka address@hidden>
- make it compile/work on i386-linux-user target
Signed-off-by: Igor Mammedov<address@hidden>
---
hw/pc.c | 45 ++++------------------------
target-i386/cpu.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++-
target-i386/helper.c | 39 ------------------------
3 files changed, 85 insertions(+), 80 deletions(-)
diff --git a/hw/pc.c b/hw/pc.c
index 1ccfc6e..d7845ea 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -911,30 +891,17 @@ static void pc_cpu_reset(void *opaque)
cpu_reset(CPU(cpu));
}
-static X86CPU *pc_new_cpu(const char *cpu_model)
-{
- X86CPU *cpu;
- CPUX86State *env;
-
- cpu = cpu_x86_init(cpu_model);
- if (cpu == NULL) {
- exit(1);
- }
- env =&cpu->env;
- 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, cpu);
- pc_cpu_reset(cpu);
- return cpu;
-}
-
void pc_cpus_init(const char *cpu_model)
{
+ X86CPU *cpu;
int i;
for(i = 0; i< smp_cpus; i++) {
If we do changes here, please add the missing space. :)
I'll fix it.
- pc_new_cpu(cpu_model);
+ cpu = cpu_x86_init(cpu_model);
+ if (cpu == NULL) {
+ exit(1);
+ }
+ qemu_register_reset(pc_cpu_reset, cpu);
}
}
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e655129..99ef891 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1749,24 +1753,89 @@ static void x86_set_cpu_model(Object *obj, const char
*value, Error **errp)
if (cpu_x86_register(cpu, env->cpu_model_str)< 0) {
fprintf(stderr, "Unable to find x86 CPU definition\n");
error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+ return;
+ }
+
+#ifndef CONFIG_USER_ONLY
+ if (((env->cpuid_features& CPUID_APIC) || smp_cpus> 1)) {
+ if (kvm_irqchip_in_kernel()) {
+ env->apic_state = qdev_create(NULL, "kvm-apic");
+ } else if (xen_enabled()) {
+ env->apic_state = qdev_create(NULL, "xen-apic");
+ } else {
+ env->apic_state = qdev_create(NULL, "apic");
+ }
+ object_property_add_child(OBJECT(cpu), "apic",
+ OBJECT(env->apic_state), NULL);
+
+ qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
+ qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
I'd like to avoid re-adding this set_ptr(). We can cherry-pick my
link<X86CPU> property from QOM CPUState part 4 series.
sure, I'll add you link<X86CPU> patches and rebase on top of it
+ }
+#endif
+}
+
+#ifndef CONFIG_USER_ONLY
+static CPUDebugExcpHandler *prev_debug_excp_handler;
+
+static void breakpoint_handler(CPUX86State *env)
+{
+ CPUBreakpoint *bp;
+
+ if (env->watchpoint_hit) {
+ if (env->watchpoint_hit->flags& BP_CPU) {
+ env->watchpoint_hit = NULL;
+ if (check_hw_breakpoints(env, 0)) {
+ raise_exception_env(EXCP01_DB, env);
+ } else {
+ cpu_resume_from_signal(env, NULL);
+ }
+ }
+ } else {
+ QTAILQ_FOREACH(bp,&env->breakpoints, entry)
+ if (bp->pc == env->eip) {
+ if (bp->flags& BP_CPU) {
+ check_hw_breakpoints(env, 1);
+ raise_exception_env(EXCP01_DB, env);
+ }
+ break;
+ }
+ }
+ if (prev_debug_excp_handler) {
+ prev_debug_excp_handler(env);
}
}
+#endif
void x86_cpu_realize(Object *obj, Error **errp)
{
X86CPU *cpu = X86_CPU(obj);
+#ifndef CONFIG_USER_ONLY
+ CPUX86State *env =&cpu->env;
+
+ if (env->apic_state) {
+ if (qdev_init(env->apic_state)< 0) {
+ error_set(errp, QERR_DEVICE_INIT_FAILED,
+ object_get_typename(OBJECT(env->apic_state)));
+ return;
+ }
+ }
+#endif
mce_init(cpu);
- qemu_init_vcpu(&cpu->env);
+ qemu_init_vcpu(env);
This only works because currently qemu_init_vcpu() is a no-op macro that
doesn't use the parameter. Please don't change it back, I guess it's a
mismerge.
I'll fix it.
We can avoid the env variable if I finish merging Paolo's series - by
realizing the CPU the APIC as its child would get realized, too. Is the
ordering before mce_init() mandatory here or is it just to reduce the
#ifndef'ery?
mce_init() ordering here is not important and it is less #ifndef-s this way.
yep, with Paolo's realize patch whole #ifndef block should be thrown away.
I can split it into a separate patch, that could be easily discarded when
Paolo's realize is committed.
+ cpu_reset(CPU(cpu));
}
static void x86_cpu_initfn(Object *obj)
{
X86CPU *cpu = X86_CPU(obj);
CPUX86State *env =&cpu->env;
+ static int inited;
cpu_exec_init(env);
+ env->cpuid_apic_id = env->cpu_index;
+
object_property_add(obj, "family", "int",
x86_cpuid_version_get_family,
x86_cpuid_version_set_family, NULL, NULL, NULL);
@@ -1795,7 +1864,15 @@ static void x86_cpu_initfn(Object *obj)
object_property_add_str(obj, "cpu-model",
x86_get_cpu_model, x86_set_cpu_model, NULL);
- env->cpuid_apic_id = env->cpu_index;
+ /* init various static tables used in TCG mode */
+ if (tcg_enabled()&& !inited) {
+ inited = 1;
+ optimize_flags_init();
+#ifndef CONFIG_USER_ONLY
+ prev_debug_excp_handler =
+ cpu_set_debug_excp_handler(breakpoint_handler);
+#endif
+ }
Did you forget to put that into its own patch or did that not work?
My idea was to have it first in the series so that other changes here
and elsewhere can be rebased onto it.
Also I wonder whether it would better be placed into the class_init? I'd
tend towards initfn because that will not be invoked during type
enumeration.
Ok, I'll split it into separate patch.
On the second thought there is no compelling reason to move this hunk in cpu.c
It could be left at board level, just moved in the beginning of pc_cpus_init()
and called only once there.
}
static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
diff --git a/target-i386/helper.c b/target-i386/helper.c
index fbaeeea..38ac25d 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -941,34 +941,6 @@ int check_hw_breakpoints(CPUX86State *env, int
force_dr6_update)
return hit_enabled;
}
-static CPUDebugExcpHandler *prev_debug_excp_handler;
-
-static void breakpoint_handler(CPUX86State *env)
-{
- CPUBreakpoint *bp;
-
- if (env->watchpoint_hit) {
- if (env->watchpoint_hit->flags& BP_CPU) {
- env->watchpoint_hit = NULL;
- if (check_hw_breakpoints(env, 0))
- raise_exception_env(EXCP01_DB, env);
- else
- cpu_resume_from_signal(env, NULL);
- }
- } else {
- QTAILQ_FOREACH(bp,&env->breakpoints, entry)
- if (bp->pc == env->eip) {
- if (bp->flags& BP_CPU) {
- check_hw_breakpoints(env, 1);
- raise_exception_env(EXCP01_DB, env);
- }
- break;
- }
- }
- if (prev_debug_excp_handler)
- prev_debug_excp_handler(env);
-}
-
I wonder if that could rather stay here as non-static?
Any preference in what header file it should be?
--
-----
Thanks
Igor
- [Qemu-devel] [PATCH qom-next v2 0/6] target-i386: re-factor CPU creation/initialization to QOM, Igor Mammedov, 2012/05/23
- [Qemu-devel] [PATCH qom-next 2/6] target-i386: move cpu halted decision into x86_cpu_reset, Igor Mammedov, 2012/05/23
- [Qemu-devel] [PATCH qom-next 1/6] pc: Enable MSI support at APIC level, Igor Mammedov, 2012/05/23
- [Qemu-devel] [PATCH qom-next 5/6] target-i386: make initialize CPU in QOM way, Igor Mammedov, 2012/05/23
- [Qemu-devel] [PATCH qom-next 3/6] target-i386: add cpu-model property to x86_cpu, Igor Mammedov, 2012/05/23
- [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code, Igor Mammedov, 2012/05/23
- Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code, Peter Maydell, 2012/05/23
- Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code, Jan Kiszka, 2012/05/23
- Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code, Igor Mammedov, 2012/05/23
- Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code, Peter Maydell, 2012/05/23
- Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code, Igor Mammedov, 2012/05/24
- Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code, Peter Maydell, 2012/05/24
- Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code, Jan Kiszka, 2012/05/24
- Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code, Igor Mammedov, 2012/05/24