[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target-i386: Initialize CPUState::halted in cpu
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH] target-i386: Initialize CPUState::halted in cpu_reset |
Date: |
Wed, 27 Apr 2011 09:11:15 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2011-04-26 21:59, Blue Swirl wrote:
> On Tue, Apr 26, 2011 at 9:55 PM, Jan Kiszka <address@hidden> wrote:
>> On 2011-04-26 20:00, Blue Swirl wrote:
>>> On Tue, Apr 26, 2011 at 11:50 AM, Jan Kiszka <address@hidden> 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.
Something like this? The original patch has to be rebased on top.
I'm still struggling how to deal with apicbase long-term. I doesn't
belong to the APIC, but it's saved/restored there. Guess we should move
it to the CPU's vmstate. OTOH, changing vmstates only for the sake of
minor refactorings is also not very attractive.
Jan
---
hw/apic.c | 18 +++++++++++++-----
hw/apic.h | 2 +-
hw/pc.c | 14 +++++++-------
target-i386/helper.c | 3 ++-
target-i386/kvm.c | 5 +++--
5 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/hw/apic.c b/hw/apic.c
index 9febf40..31ac6cd 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -318,7 +318,7 @@ uint64_t cpu_get_apic_base(DeviceState *d)
trace_cpu_get_apic_base(s ? (uint64_t)s->apicbase: 0);
- return s ? s->apicbase : 0;
+ return s ? s->apicbase : MSR_IA32_APICBASE_BSP;
}
void cpu_set_apic_tpr(DeviceState *d, uint8_t val)
@@ -958,18 +958,26 @@ static const VMStateDescription vmstate_apic = {
}
};
+void apic_designate_bsp(DeviceState *d)
+{
+ APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+ if (!d) {
+ return;
+ }
+ s->apicbase |= MSR_IA32_APICBASE_BSP;
+}
+
static void apic_reset(DeviceState *d)
{
APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
- int bsp;
- bsp = cpu_is_bsp(s->cpu_env);
s->apicbase = 0xfee00000 |
- (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
+ (s->apicbase & MSR_IA32_APICBASE_BSP) | MSR_IA32_APICBASE_ENABLE;
apic_init_reset(d);
- if (bsp) {
+ if (s->apicbase & MSR_IA32_APICBASE_BSP) {
/*
* LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
* time typically by BIOS, so PIC interrupt can be delivered to the
diff --git a/hw/apic.h b/hw/apic.h
index 8a0c9d0..935144b 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -19,9 +19,9 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
uint8_t cpu_get_apic_tpr(DeviceState *s);
void apic_init_reset(DeviceState *s);
void apic_sipi(DeviceState *s);
+void apic_designate_bsp(DeviceState *d);
/* pc.c */
-int cpu_is_bsp(CPUState *env);
DeviceState *cpu_get_current_apic(void);
#endif
diff --git a/hw/pc.c b/hw/pc.c
index 6939c04..36ca238 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -852,12 +852,6 @@ void pc_init_ne2k_isa(NICInfo *nd)
nb_ne2k++;
}
-int cpu_is_bsp(CPUState *env)
-{
- /* We hard-wire the BSP to the first CPU. */
- return env->cpu_index == 0;
-}
-
DeviceState *cpu_get_current_apic(void)
{
if (cpu_single_env) {
@@ -918,7 +912,8 @@ static void pc_cpu_reset(void *opaque)
CPUState *env = opaque;
cpu_reset(env);
- env->halted = !cpu_is_bsp(env);
+ env->halted =
+ !(cpu_get_apic_base(env->apic_state) & MSR_IA32_APICBASE_BSP);
}
static CPUState *pc_new_cpu(const char *cpu_model)
@@ -933,6 +928,11 @@ static CPUState *pc_new_cpu(const char *cpu_model)
if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
env->cpuid_apic_id = env->cpu_index;
env->apic_state = apic_init(env, env->cpuid_apic_id);
+
+ /* We hard-wire the BSP to the first CPU. */
+ if (env->cpu_index == 0) {
+ apic_designate_bsp(env->apic_state);
+ }
}
qemu_register_reset(pc_cpu_reset, env);
pc_cpu_reset(env);
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 89df997..52b3a44 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1282,7 +1282,8 @@ 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);
+ env->halted =
+ !(cpu_get_apic_base(env->apic_state) & MSR_IA32_APICBASE_BSP);
}
void do_cpu_sipi(CPUState *env)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index a13599d..fef78c2 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -504,8 +504,9 @@ void kvm_arch_reset_vcpu(CPUState *env)
env->interrupt_injected = -1;
env->xcr0 = 1;
if (kvm_irqchip_in_kernel()) {
- env->mp_state = cpu_is_bsp(env) ? KVM_MP_STATE_RUNNABLE :
- KVM_MP_STATE_UNINITIALIZED;
+ env->mp_state =
+ cpu_get_apic_base(env->apic_state) & MSR_IA32_APICBASE_BSP ?
+ KVM_MP_STATE_RUNNABLE : KVM_MP_STATE_UNINITIALIZED;
} else {
env->mp_state = KVM_MP_STATE_RUNNABLE;
}
--
1.7.1
signature.asc
Description: OpenPGP digital signature