Message ID | 1338329426-28118-8-git-send-email-imammedo@redhat.com |
---|---|
State | New |
Headers | show |
Am 30.05.2012 00:10, schrieb Igor Mammedov: > From: Igor Mammedov <niallain@gmail.com> > > MP initialization protocol differs between cpu families, and for P6 and > onward models it is up to CPU to decide if it will be BSP using this > protocol, so try to model this. However there is no point in implementing > MP initialization protocol in qemu. Thus first CPU is always marked as BSP. > > This patch: > - moves decision to designate BSP from board into cpu, making cpu > self-sufficient in this regard. Later it will allow to cleanup hw/pc.c > and remove cpu_reset and wrappers from there. > - stores flag that CPU is BSP in IA32_APIC_BASE to model behavior > described in Inted SDM vol 3a part 1 chapter 8.4.1 > - uses MSR_IA32_APICBASE_BSP flag in apic_base for checking if cpu is BSP > > patch is based on Jan Kiszka's proposal: > http://thread.gmane.org/gmane.comp.emulators.qemu/100806 > > v2: > - fix build for i386-linux-user > spotted-by: Peter Maydell <peter.maydell@linaro.org> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/apic.h | 2 +- > hw/apic_common.c | 18 ++++++++++++------ > hw/pc.c | 9 --------- > target-i386/cpu.c | 9 +++++++++ > target-i386/helper.c | 1 - > target-i386/kvm.c | 5 +++-- > 6 files changed, 25 insertions(+), 19 deletions(-) > > diff --git a/hw/apic.h b/hw/apic.h > index 62179ce..d961ed4 100644 > --- a/hw/apic.h > +++ b/hw/apic.h > @@ -20,9 +20,9 @@ void apic_init_reset(DeviceState *s); > void apic_sipi(DeviceState *s); > void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip, > TPRAccess access); > +void apic_designate_bsp(DeviceState *d); > > /* pc.c */ > -int cpu_is_bsp(CPUX86State *env); > DeviceState *cpu_get_current_apic(void); > > #endif > diff --git a/hw/apic_common.c b/hw/apic_common.c > index 46a9ff7..98ad6f0 100644 > --- a/hw/apic_common.c > +++ b/hw/apic_common.c > @@ -43,8 +43,8 @@ uint64_t cpu_get_apic_base(DeviceState *d) > trace_cpu_get_apic_base((uint64_t)s->apicbase); > return s->apicbase; > } else { > - trace_cpu_get_apic_base(0); > - return 0; > + trace_cpu_get_apic_base(MSR_IA32_APICBASE_BSP); > + return MSR_IA32_APICBASE_BSP; > } > } > > @@ -201,22 +201,28 @@ void apic_init_reset(DeviceState *d) > s->timer_expiry = -1; > } > > +void apic_designate_bsp(DeviceState *d) > +{ > + if (d) { This check looks odd. The function call is already guarded with cpu_index == 0. What other case would lead to d == NULL and can't we check that at the call site? > + APICCommonState *s = APIC_COMMON(d); If this is a QOM cast macro, it will implicitly assert d != NULL, no? Andreas > + s->apicbase |= MSR_IA32_APICBASE_BSP; > + } > +} > + > static void apic_reset_common(DeviceState *d) > { > APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); > APICCommonClass *info = APIC_COMMON_GET_CLASS(s); > - bool 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; > > s->vapic_paddr = 0; > info->vapic_base_update(s); > > 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/pc.c b/hw/pc.c > index 6bb3d2a..2f681db 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -870,12 +870,6 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd) > nb_ne2k++; > } > > -int cpu_is_bsp(CPUX86State *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) { > @@ -942,10 +936,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level) > static void pc_cpu_reset(void *opaque) > { > X86CPU *cpu = opaque; > - CPUX86State *env = &cpu->env; > - > cpu_reset(CPU(cpu)); > - env->halted = !cpu_is_bsp(env); > } > > static X86CPU *pc_new_cpu(const char *cpu_model) > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 41a0436..f029f2a 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1704,6 +1704,15 @@ static void x86_cpu_reset(CPUState *s) > 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) > + /* We hard-wire the BSP to the first CPU. */ > + if (env->cpu_index == 0) { > + apic_designate_bsp(env->apic_state); > + } > + > + env->halted = !(cpu_get_apic_base(env->apic_state) & MSR_IA32_APICBASE_BSP); > +#endif > } > > static void mce_init(X86CPU *cpu) > diff --git a/target-i386/helper.c b/target-i386/helper.c > index a94be0a..fd20fd4 100644 > --- a/target-i386/helper.c > +++ b/target-i386/helper.c > @@ -1179,7 +1179,6 @@ void do_cpu_init(X86CPU *cpu) > env->interrupt_request = sipi; > env->pat = pat; > apic_init_reset(env->apic_state); > - env->halted = !cpu_is_bsp(env); > } > > void do_cpu_sipi(X86CPU *cpu) > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index 0d0d8f6..09621e5 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -583,8 +583,9 @@ void kvm_arch_reset_vcpu(CPUX86State *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; > }
On 05/30/2012 02:11 PM, Andreas Färber wrote: > Am 30.05.2012 00:10, schrieb Igor Mammedov: >> From: Igor Mammedov<niallain@gmail.com> >> >> MP initialization protocol differs between cpu families, and for P6 and >> onward models it is up to CPU to decide if it will be BSP using this >> protocol, so try to model this. However there is no point in implementing >> MP initialization protocol in qemu. Thus first CPU is always marked as BSP. >> >> This patch: >> - moves decision to designate BSP from board into cpu, making cpu >> self-sufficient in this regard. Later it will allow to cleanup hw/pc.c >> and remove cpu_reset and wrappers from there. >> - stores flag that CPU is BSP in IA32_APIC_BASE to model behavior >> described in Inted SDM vol 3a part 1 chapter 8.4.1 >> - uses MSR_IA32_APICBASE_BSP flag in apic_base for checking if cpu is BSP >> >> patch is based on Jan Kiszka's proposal: >> http://thread.gmane.org/gmane.comp.emulators.qemu/100806 >> >> v2: >> - fix build for i386-linux-user >> spotted-by: Peter Maydell<peter.maydell@linaro.org> >> >> Signed-off-by: Igor Mammedov<imammedo@redhat.com> >> --- >> hw/apic.h | 2 +- >> hw/apic_common.c | 18 ++++++++++++------ >> hw/pc.c | 9 --------- >> target-i386/cpu.c | 9 +++++++++ >> target-i386/helper.c | 1 - >> target-i386/kvm.c | 5 +++-- >> 6 files changed, 25 insertions(+), 19 deletions(-) >> >> diff --git a/hw/apic.h b/hw/apic.h >> index 62179ce..d961ed4 100644 >> --- a/hw/apic.h >> +++ b/hw/apic.h >> @@ -20,9 +20,9 @@ void apic_init_reset(DeviceState *s); >> void apic_sipi(DeviceState *s); >> void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip, >> TPRAccess access); >> +void apic_designate_bsp(DeviceState *d); >> >> /* pc.c */ >> -int cpu_is_bsp(CPUX86State *env); >> DeviceState *cpu_get_current_apic(void); >> >> #endif >> diff --git a/hw/apic_common.c b/hw/apic_common.c >> index 46a9ff7..98ad6f0 100644 >> --- a/hw/apic_common.c >> +++ b/hw/apic_common.c >> @@ -43,8 +43,8 @@ uint64_t cpu_get_apic_base(DeviceState *d) >> trace_cpu_get_apic_base((uint64_t)s->apicbase); >> return s->apicbase; >> } else { >> - trace_cpu_get_apic_base(0); >> - return 0; >> + trace_cpu_get_apic_base(MSR_IA32_APICBASE_BSP); >> + return MSR_IA32_APICBASE_BSP; >> } >> } >> >> @@ -201,22 +201,28 @@ void apic_init_reset(DeviceState *d) >> s->timer_expiry = -1; >> } >> >> +void apic_designate_bsp(DeviceState *d) >> +{ >> + if (d) { > > This check looks odd. The function call is already guarded with > cpu_index == 0. What other case would lead to d == NULL and can't we > check that at the call site? cpu_index == 0 doesn't imply that cpu has apic hence the check. And we for sure can check it call site, would you like to do it there? PS: there are many checks for this condition in APIC code, so may be it should be there style-wise at least? > >> + APICCommonState *s = APIC_COMMON(d); > > If this is a QOM cast macro, it will implicitly assert d != NULL, no? it actually will lead to null pointer dereference, like this: OBJECT_CHECK(type, obj, name) \ ((type *)object_dynamic_cast_assert(OBJECT(obj), (name))) => object_dynamic_cast(obj, typename) => object_is_type(obj, target_type) => type_is_ancestor(obj->class->type, target_type); ^^^ > > Andreas > >> + s->apicbase |= MSR_IA32_APICBASE_BSP; >> + } >> +} >> + >> static void apic_reset_common(DeviceState *d) >> { >> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); >> APICCommonClass *info = APIC_COMMON_GET_CLASS(s); >> - bool 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; >> >> s->vapic_paddr = 0; >> info->vapic_base_update(s); >> >> 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/pc.c b/hw/pc.c >> index 6bb3d2a..2f681db 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -870,12 +870,6 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd) >> nb_ne2k++; >> } >> >> -int cpu_is_bsp(CPUX86State *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) { >> @@ -942,10 +936,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level) >> static void pc_cpu_reset(void *opaque) >> { >> X86CPU *cpu = opaque; >> - CPUX86State *env =&cpu->env; >> - >> cpu_reset(CPU(cpu)); >> - env->halted = !cpu_is_bsp(env); >> } >> >> static X86CPU *pc_new_cpu(const char *cpu_model) >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> index 41a0436..f029f2a 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c >> @@ -1704,6 +1704,15 @@ static void x86_cpu_reset(CPUState *s) >> 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) >> + /* We hard-wire the BSP to the first CPU. */ >> + if (env->cpu_index == 0) { >> + apic_designate_bsp(env->apic_state); >> + } >> + >> + env->halted = !(cpu_get_apic_base(env->apic_state)& MSR_IA32_APICBASE_BSP); >> +#endif >> } >> >> static void mce_init(X86CPU *cpu) >> diff --git a/target-i386/helper.c b/target-i386/helper.c >> index a94be0a..fd20fd4 100644 >> --- a/target-i386/helper.c >> +++ b/target-i386/helper.c >> @@ -1179,7 +1179,6 @@ void do_cpu_init(X86CPU *cpu) >> env->interrupt_request = sipi; >> env->pat = pat; >> apic_init_reset(env->apic_state); >> - env->halted = !cpu_is_bsp(env); >> } >> >> void do_cpu_sipi(X86CPU *cpu) >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c >> index 0d0d8f6..09621e5 100644 >> --- a/target-i386/kvm.c >> +++ b/target-i386/kvm.c >> @@ -583,8 +583,9 @@ void kvm_arch_reset_vcpu(CPUX86State *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; >> } > >
Am 30.05.2012 16:13, schrieb Igor Mammedov: > On 05/30/2012 02:11 PM, Andreas Färber wrote: >> Am 30.05.2012 00:10, schrieb Igor Mammedov: >>> From: Igor Mammedov<niallain@gmail.com> >>> >>> MP initialization protocol differs between cpu families, and for P6 and >>> onward models it is up to CPU to decide if it will be BSP using this >>> protocol, so try to model this. However there is no point in >>> implementing >>> MP initialization protocol in qemu. Thus first CPU is always marked >>> as BSP. >>> >>> This patch: >>> - moves decision to designate BSP from board into cpu, making cpu >>> self-sufficient in this regard. Later it will allow to cleanup hw/pc.c >>> and remove cpu_reset and wrappers from there. >>> - stores flag that CPU is BSP in IA32_APIC_BASE to model behavior >>> described in Inted SDM vol 3a part 1 chapter 8.4.1 >>> - uses MSR_IA32_APICBASE_BSP flag in apic_base for checking if cpu >>> is BSP >>> >>> patch is based on Jan Kiszka's proposal: >>> http://thread.gmane.org/gmane.comp.emulators.qemu/100806 >>> >>> v2: >>> - fix build for i386-linux-user >>> spotted-by: Peter Maydell<peter.maydell@linaro.org> >>> >>> Signed-off-by: Igor Mammedov<imammedo@redhat.com> >>> --- >>> hw/apic.h | 2 +- >>> hw/apic_common.c | 18 ++++++++++++------ >>> hw/pc.c | 9 --------- >>> target-i386/cpu.c | 9 +++++++++ >>> target-i386/helper.c | 1 - >>> target-i386/kvm.c | 5 +++-- >>> 6 files changed, 25 insertions(+), 19 deletions(-) >>> >>> diff --git a/hw/apic.h b/hw/apic.h >>> index 62179ce..d961ed4 100644 >>> --- a/hw/apic.h >>> +++ b/hw/apic.h >>> @@ -20,9 +20,9 @@ void apic_init_reset(DeviceState *s); >>> void apic_sipi(DeviceState *s); >>> void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip, >>> TPRAccess access); >>> +void apic_designate_bsp(DeviceState *d); >>> >>> /* pc.c */ >>> -int cpu_is_bsp(CPUX86State *env); >>> DeviceState *cpu_get_current_apic(void); >>> >>> #endif >>> diff --git a/hw/apic_common.c b/hw/apic_common.c >>> index 46a9ff7..98ad6f0 100644 >>> --- a/hw/apic_common.c >>> +++ b/hw/apic_common.c >>> @@ -43,8 +43,8 @@ uint64_t cpu_get_apic_base(DeviceState *d) >>> trace_cpu_get_apic_base((uint64_t)s->apicbase); >>> return s->apicbase; >>> } else { >>> - trace_cpu_get_apic_base(0); >>> - return 0; >>> + trace_cpu_get_apic_base(MSR_IA32_APICBASE_BSP); >>> + return MSR_IA32_APICBASE_BSP; >>> } >>> } >>> >>> @@ -201,22 +201,28 @@ void apic_init_reset(DeviceState *d) >>> s->timer_expiry = -1; >>> } >>> >>> +void apic_designate_bsp(DeviceState *d) >>> +{ >>> + if (d) { >> >> This check looks odd. The function call is already guarded with >> cpu_index == 0. What other case would lead to d == NULL and can't we >> check that at the call site? > > cpu_index == 0 doesn't imply that cpu has apic hence the check. And we > for sure > can check it call site, would you like to do it there? > PS: > there are many checks for this condition in APIC code, so may be it should > be there style-wise at least? What I referred to as odd was the indentation of the actual implementation. So I'd be fine with either checking at the call site or if (d == NULL) { return; } ... >>> + APICCommonState *s = APIC_COMMON(d); >> >> If this is a QOM cast macro, it will implicitly assert d != NULL, no? > > it actually will lead to null pointer dereference, like this: > OBJECT_CHECK(type, obj, name) \ > ((type *)object_dynamic_cast_assert(OBJECT(obj), (name))) > => object_dynamic_cast(obj, typename) > => object_is_type(obj, target_type) > => type_is_ancestor(obj->class->type, target_type); > ^^^ Mind to send a patch fixing that? :-) I'd suppose the _is_... functions should probably return false and the cast should assert. Anthony? Andreas
On 05/30/2012 04:54 PM, Andreas Färber wrote: > Am 30.05.2012 16:13, schrieb Igor Mammedov: >> On 05/30/2012 02:11 PM, Andreas Färber wrote: >>> Am 30.05.2012 00:10, schrieb Igor Mammedov: >>>> From: Igor Mammedov<niallain@gmail.com> >>>> >>>> MP initialization protocol differs between cpu families, and for P6 and >>>> onward models it is up to CPU to decide if it will be BSP using this >>>> protocol, so try to model this. However there is no point in >>>> implementing >>>> MP initialization protocol in qemu. Thus first CPU is always marked >>>> as BSP. >>>> >>>> This patch: >>>> - moves decision to designate BSP from board into cpu, making cpu >>>> self-sufficient in this regard. Later it will allow to cleanup hw/pc.c >>>> and remove cpu_reset and wrappers from there. >>>> - stores flag that CPU is BSP in IA32_APIC_BASE to model behavior >>>> described in Inted SDM vol 3a part 1 chapter 8.4.1 >>>> - uses MSR_IA32_APICBASE_BSP flag in apic_base for checking if cpu >>>> is BSP >>>> >>>> patch is based on Jan Kiszka's proposal: >>>> http://thread.gmane.org/gmane.comp.emulators.qemu/100806 >>>> >>>> v2: >>>> - fix build for i386-linux-user >>>> spotted-by: Peter Maydell<peter.maydell@linaro.org> >>>> >>>> Signed-off-by: Igor Mammedov<imammedo@redhat.com> >>>> --- >>>> hw/apic.h | 2 +- >>>> hw/apic_common.c | 18 ++++++++++++------ >>>> hw/pc.c | 9 --------- >>>> target-i386/cpu.c | 9 +++++++++ >>>> target-i386/helper.c | 1 - >>>> target-i386/kvm.c | 5 +++-- >>>> 6 files changed, 25 insertions(+), 19 deletions(-) >>>> >>>> diff --git a/hw/apic.h b/hw/apic.h >>>> index 62179ce..d961ed4 100644 >>>> --- a/hw/apic.h >>>> +++ b/hw/apic.h >>>> @@ -20,9 +20,9 @@ void apic_init_reset(DeviceState *s); >>>> void apic_sipi(DeviceState *s); >>>> void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip, >>>> TPRAccess access); >>>> +void apic_designate_bsp(DeviceState *d); >>>> >>>> /* pc.c */ >>>> -int cpu_is_bsp(CPUX86State *env); >>>> DeviceState *cpu_get_current_apic(void); >>>> >>>> #endif >>>> diff --git a/hw/apic_common.c b/hw/apic_common.c >>>> index 46a9ff7..98ad6f0 100644 >>>> --- a/hw/apic_common.c >>>> +++ b/hw/apic_common.c >>>> @@ -43,8 +43,8 @@ uint64_t cpu_get_apic_base(DeviceState *d) >>>> trace_cpu_get_apic_base((uint64_t)s->apicbase); >>>> return s->apicbase; >>>> } else { >>>> - trace_cpu_get_apic_base(0); >>>> - return 0; >>>> + trace_cpu_get_apic_base(MSR_IA32_APICBASE_BSP); >>>> + return MSR_IA32_APICBASE_BSP; >>>> } >>>> } >>>> >>>> @@ -201,22 +201,28 @@ void apic_init_reset(DeviceState *d) >>>> s->timer_expiry = -1; >>>> } >>>> >>>> +void apic_designate_bsp(DeviceState *d) >>>> +{ >>>> + if (d) { >>> >>> This check looks odd. The function call is already guarded with >>> cpu_index == 0. What other case would lead to d == NULL and can't we >>> check that at the call site? >> >> cpu_index == 0 doesn't imply that cpu has apic hence the check. And we >> for sure >> can check it call site, would you like to do it there? >> PS: >> there are many checks for this condition in APIC code, so may be it should >> be there style-wise at least? > > What I referred to as odd was the indentation of the actual implementation. > > So I'd be fine with either checking at the call site or > > if (d == NULL) { > return; > } > > ... > >>>> + APICCommonState *s = APIC_COMMON(d); >>> >>> If this is a QOM cast macro, it will implicitly assert d != NULL, no? >> >> it actually will lead to null pointer dereference, like this: >> OBJECT_CHECK(type, obj, name) \ >> ((type *)object_dynamic_cast_assert(OBJECT(obj), (name))) >> => object_dynamic_cast(obj, typename) >> => object_is_type(obj, target_type) >> => type_is_ancestor(obj->class->type, target_type); >> ^^^ > > Mind to send a patch fixing that? :-) > I'd suppose the _is_... functions should probably return false and the > cast should assert. Anthony? Maybe asserting right at the beginning of object_dynamic_cast_assert would be better than hiding it somewhere deep in call chain, because later someone could add access to obj before *_is_* functions and with explicit assert in the beginnig of object_dynamic_cast_assert() it would be hard to mess things up. > > Andreas >
diff --git a/hw/apic.h b/hw/apic.h index 62179ce..d961ed4 100644 --- a/hw/apic.h +++ b/hw/apic.h @@ -20,9 +20,9 @@ void apic_init_reset(DeviceState *s); void apic_sipi(DeviceState *s); void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip, TPRAccess access); +void apic_designate_bsp(DeviceState *d); /* pc.c */ -int cpu_is_bsp(CPUX86State *env); DeviceState *cpu_get_current_apic(void); #endif diff --git a/hw/apic_common.c b/hw/apic_common.c index 46a9ff7..98ad6f0 100644 --- a/hw/apic_common.c +++ b/hw/apic_common.c @@ -43,8 +43,8 @@ uint64_t cpu_get_apic_base(DeviceState *d) trace_cpu_get_apic_base((uint64_t)s->apicbase); return s->apicbase; } else { - trace_cpu_get_apic_base(0); - return 0; + trace_cpu_get_apic_base(MSR_IA32_APICBASE_BSP); + return MSR_IA32_APICBASE_BSP; } } @@ -201,22 +201,28 @@ void apic_init_reset(DeviceState *d) s->timer_expiry = -1; } +void apic_designate_bsp(DeviceState *d) +{ + if (d) { + APICCommonState *s = APIC_COMMON(d); + s->apicbase |= MSR_IA32_APICBASE_BSP; + } +} + static void apic_reset_common(DeviceState *d) { APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); APICCommonClass *info = APIC_COMMON_GET_CLASS(s); - bool 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; s->vapic_paddr = 0; info->vapic_base_update(s); 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/pc.c b/hw/pc.c index 6bb3d2a..2f681db 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -870,12 +870,6 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd) nb_ne2k++; } -int cpu_is_bsp(CPUX86State *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) { @@ -942,10 +936,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level) static void pc_cpu_reset(void *opaque) { X86CPU *cpu = opaque; - CPUX86State *env = &cpu->env; - cpu_reset(CPU(cpu)); - env->halted = !cpu_is_bsp(env); } static X86CPU *pc_new_cpu(const char *cpu_model) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 41a0436..f029f2a 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1704,6 +1704,15 @@ static void x86_cpu_reset(CPUState *s) 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) + /* We hard-wire the BSP to the first CPU. */ + if (env->cpu_index == 0) { + apic_designate_bsp(env->apic_state); + } + + env->halted = !(cpu_get_apic_base(env->apic_state) & MSR_IA32_APICBASE_BSP); +#endif } static void mce_init(X86CPU *cpu) diff --git a/target-i386/helper.c b/target-i386/helper.c index a94be0a..fd20fd4 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -1179,7 +1179,6 @@ void do_cpu_init(X86CPU *cpu) env->interrupt_request = sipi; env->pat = pat; apic_init_reset(env->apic_state); - env->halted = !cpu_is_bsp(env); } void do_cpu_sipi(X86CPU *cpu) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 0d0d8f6..09621e5 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -583,8 +583,9 @@ void kvm_arch_reset_vcpu(CPUX86State *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; }