| Message ID | 4DB68768.3050700@siemens.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Apr 26, 2011 at 11:50 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > Instead of having an extra reset function at machine level and special > code for processing INIT, move the initialization of halted into the > cpu reset handler. Nack. A CPU is designated as a BSP at board level. CPUs do not need to know about this at all.
On 2011-04-26 20:00, Blue Swirl wrote: > On Tue, Apr 26, 2011 at 11:50 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> Instead of having an extra reset function at machine level and special >> code for processing INIT, move the initialization of halted into the >> cpu reset handler. > > Nack. A CPU is designated as a BSP at board level. CPUs do not need to > know about this at all. That's why we have cpu_is_bsp() in pc.c. Obviously, every CPU (which includes the APIC) must know if it is supposed to be BP or AP. It would be unable to enter the right state after reset otherwise (e.g. Intel SDM 9.1). cpu_is_bsp() is basically reporting the result of the MP init protocol in condensed from. Jan
On Tue, Apr 26, 2011 at 9:55 PM, Jan Kiszka <jan.kiszka@web.de> wrote: > On 2011-04-26 20:00, Blue Swirl wrote: >> On Tue, Apr 26, 2011 at 11:50 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >>> Instead of having an extra reset function at machine level and special >>> code for processing INIT, move the initialization of halted into the >>> cpu reset handler. >> >> Nack. A CPU is designated as a BSP at board level. CPUs do not need to >> know about this at all. > > That's why we have cpu_is_bsp() in pc.c. > > Obviously, every CPU (which includes the APIC) must know if it is > supposed to be BP or AP. It would be unable to enter the right state > after reset otherwise (e.g. Intel SDM 9.1). cpu_is_bsp() is basically > reporting the result of the MP init protocol in condensed from. Intel 64 and IA-32 Architectures Software Developer’s Manual vol 3A, 7.5.1 says that the protocol result is stored in APIC MSR. I think we should be using that instead. For example, the board could call cpu_designate_bsp() to set the BSP flag in MSR. Then cpu_is_bsp() would only check the MSR, which naturally belongs to the CPU/APIC domain.
diff --git a/hw/pc.c b/hw/pc.c index 6939c04..8ef86db 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -913,14 +913,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level) } } -static void pc_cpu_reset(void *opaque) -{ - CPUState *env = opaque; - - cpu_reset(env); - env->halted = !cpu_is_bsp(env); -} - static CPUState *pc_new_cpu(const char *cpu_model) { CPUState *env; @@ -934,8 +926,8 @@ static CPUState *pc_new_cpu(const char *cpu_model) env->cpuid_apic_id = env->cpu_index; env->apic_state = apic_init(env, env->cpuid_apic_id); } - qemu_register_reset(pc_cpu_reset, env); - pc_cpu_reset(env); + qemu_register_reset((QEMUResetHandler *)cpu_reset, env); + cpu_reset(env); return env; } diff --git a/target-i386/helper.c b/target-i386/helper.c index 89df997..56cca96 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -106,6 +106,10 @@ void cpu_reset(CPUX86State *env) env->dr[7] = DR7_FIXED_1; cpu_breakpoint_remove_all(env, BP_CPU); cpu_watchpoint_remove_all(env, BP_CPU); + +#if !defined(CONFIG_USER_ONLY) + env->halted = !cpu_is_bsp(env); +#endif } void cpu_x86_close(CPUX86State *env) @@ -1282,7 +1286,6 @@ void do_cpu_init(CPUState *env) env->interrupt_request = sipi; env->pat = pat; apic_init_reset(env->apic_state); - env->halted = !cpu_is_bsp(env); } void do_cpu_sipi(CPUState *env)
Instead of having an extra reset function at machine level and special code for processing INIT, move the initialization of halted into the cpu reset handler. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/pc.c | 12 ++---------- target-i386/helper.c | 5 ++++- 2 files changed, 6 insertions(+), 11 deletions(-)