Patchwork [v4,03/10] x86: Extend validity of cpu_is_bsp

login
register
mail settings
Submitter Jan Kiszka
Date March 1, 2010, 5:17 p.m.
Message ID <ca058ae38e22e417f9847e487cf657761ffbaab7.1267463833.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/46576/
State New
Headers show

Comments

Jan Kiszka - March 1, 2010, 5:17 p.m.
As we hard-wire the BSP to CPU 0 anyway and cpuid_apic_id equals
cpu_index, cpu_is_bsp can also be based on the latter directly. This
will help an early user of it: KVM while initializing mp_state.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/pc.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Gleb Natapov - March 3, 2010, 4 p.m.
On Mon, Mar 01, 2010 at 06:17:22PM +0100, Jan Kiszka wrote:
> As we hard-wire the BSP to CPU 0 anyway and cpuid_apic_id equals
> cpu_index, cpu_is_bsp can also be based on the latter directly. This
> will help an early user of it: KVM while initializing mp_state.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/pc.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index b90a79e..58c32ea 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -767,7 +767,8 @@ static void pc_init_ne2k_isa(NICInfo *nd)
>  
>  int cpu_is_bsp(CPUState *env)
>  {
> -    return env->cpuid_apic_id == 0;
> +    /* We hard-wire the BSP to the first CPU. */
> +    return env->cpu_index == 0;
>  }
We should not assume that. The function was written like that
specifically so the code around it will not rely on this assumption.
Now you change that specifically to write code that will do incorrect
assumptions. I don't see the logic here.

>  
>  CPUState *pc_new_cpu(const char *cpu_model)
> -- 
> 1.6.0.2

--
			Gleb.
Jan Kiszka - March 3, 2010, 11:34 p.m.
Gleb Natapov wrote:
> On Mon, Mar 01, 2010 at 06:17:22PM +0100, Jan Kiszka wrote:
>> As we hard-wire the BSP to CPU 0 anyway and cpuid_apic_id equals
>> cpu_index, cpu_is_bsp can also be based on the latter directly. This
>> will help an early user of it: KVM while initializing mp_state.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  hw/pc.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index b90a79e..58c32ea 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -767,7 +767,8 @@ static void pc_init_ne2k_isa(NICInfo *nd)
>>  
>>  int cpu_is_bsp(CPUState *env)
>>  {
>> -    return env->cpuid_apic_id == 0;
>> +    /* We hard-wire the BSP to the first CPU. */
>> +    return env->cpu_index == 0;
>>  }
> We should not assume that. The function was written like that
> specifically so the code around it will not rely on this assumption.
> Now you change that specifically to write code that will do incorrect
> assumptions. I don't see the logic here.

The logic is that we do not support any other mapping yet - with or
without this change. Without it, we complicate the APIC initialization
for (so far) no good reason. Once we want to support different BSP
assignments, we need to go through the code and rework some parts anyway.

Jan
Gleb Natapov - March 4, 2010, 6:47 a.m.
On Thu, Mar 04, 2010 at 12:34:22AM +0100, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Mon, Mar 01, 2010 at 06:17:22PM +0100, Jan Kiszka wrote:
> >> As we hard-wire the BSP to CPU 0 anyway and cpuid_apic_id equals
> >> cpu_index, cpu_is_bsp can also be based on the latter directly. This
> >> will help an early user of it: KVM while initializing mp_state.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  hw/pc.c |    3 ++-
> >>  1 files changed, 2 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/hw/pc.c b/hw/pc.c
> >> index b90a79e..58c32ea 100644
> >> --- a/hw/pc.c
> >> +++ b/hw/pc.c
> >> @@ -767,7 +767,8 @@ static void pc_init_ne2k_isa(NICInfo *nd)
> >>  
> >>  int cpu_is_bsp(CPUState *env)
> >>  {
> >> -    return env->cpuid_apic_id == 0;
> >> +    /* We hard-wire the BSP to the first CPU. */
> >> +    return env->cpu_index == 0;
> >>  }
> > We should not assume that. The function was written like that
> > specifically so the code around it will not rely on this assumption.
> > Now you change that specifically to write code that will do incorrect
> > assumptions. I don't see the logic here.
> 
> The logic is that we do not support any other mapping yet - with or
> without this change. Without it, we complicate the APIC initialization
> for (so far) no good reason. Once we want to support different BSP
> assignments, we need to go through the code and rework some parts anyway.
> 
As far as I remember the only part that was missing was a command line to
specify apic IDs for each CPU and what CPU is BSP. The code was ready
otherwise. I's very sad if this was broken by other modifications. But
changes like that actually pushes us back from our goal. Why not rework
code so it will work with correct cpu_is_bsp() function instead of
introducing this hack?

--
			Gleb.
Jan Kiszka - March 4, 2010, 8:23 a.m.
Gleb Natapov wrote:
> On Thu, Mar 04, 2010 at 12:34:22AM +0100, Jan Kiszka wrote:
>> Gleb Natapov wrote:
>>> On Mon, Mar 01, 2010 at 06:17:22PM +0100, Jan Kiszka wrote:
>>>> As we hard-wire the BSP to CPU 0 anyway and cpuid_apic_id equals
>>>> cpu_index, cpu_is_bsp can also be based on the latter directly. This
>>>> will help an early user of it: KVM while initializing mp_state.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>  hw/pc.c |    3 ++-
>>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/hw/pc.c b/hw/pc.c
>>>> index b90a79e..58c32ea 100644
>>>> --- a/hw/pc.c
>>>> +++ b/hw/pc.c
>>>> @@ -767,7 +767,8 @@ static void pc_init_ne2k_isa(NICInfo *nd)
>>>>  
>>>>  int cpu_is_bsp(CPUState *env)
>>>>  {
>>>> -    return env->cpuid_apic_id == 0;
>>>> +    /* We hard-wire the BSP to the first CPU. */
>>>> +    return env->cpu_index == 0;
>>>>  }
>>> We should not assume that. The function was written like that
>>> specifically so the code around it will not rely on this assumption.
>>> Now you change that specifically to write code that will do incorrect
>>> assumptions. I don't see the logic here.
>> The logic is that we do not support any other mapping yet - with or
>> without this change. Without it, we complicate the APIC initialization
>> for (so far) no good reason. Once we want to support different BSP
>> assignments, we need to go through the code and rework some parts anyway.
>>
> As far as I remember the only part that was missing was a command line to
> specify apic IDs for each CPU and what CPU is BSP. The code was ready
> otherwise. I's very sad if this was broken by other modifications. But
> changes like that actually pushes us back from our goal. Why not rework
> code so it will work with correct cpu_is_bsp() function instead of
> introducing this hack?

If you can confirm that there is a serious use case behind it, I will
look into this again. But so far, I did not find it.

Jan
Gleb Natapov - March 4, 2010, 8:35 a.m.
On Thu, Mar 04, 2010 at 09:23:46AM +0100, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Thu, Mar 04, 2010 at 12:34:22AM +0100, Jan Kiszka wrote:
> >> Gleb Natapov wrote:
> >>> On Mon, Mar 01, 2010 at 06:17:22PM +0100, Jan Kiszka wrote:
> >>>> As we hard-wire the BSP to CPU 0 anyway and cpuid_apic_id equals
> >>>> cpu_index, cpu_is_bsp can also be based on the latter directly. This
> >>>> will help an early user of it: KVM while initializing mp_state.
> >>>>
> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>> ---
> >>>>  hw/pc.c |    3 ++-
> >>>>  1 files changed, 2 insertions(+), 1 deletions(-)
> >>>>
> >>>> diff --git a/hw/pc.c b/hw/pc.c
> >>>> index b90a79e..58c32ea 100644
> >>>> --- a/hw/pc.c
> >>>> +++ b/hw/pc.c
> >>>> @@ -767,7 +767,8 @@ static void pc_init_ne2k_isa(NICInfo *nd)
> >>>>  
> >>>>  int cpu_is_bsp(CPUState *env)
> >>>>  {
> >>>> -    return env->cpuid_apic_id == 0;
> >>>> +    /* We hard-wire the BSP to the first CPU. */
> >>>> +    return env->cpu_index == 0;
> >>>>  }
> >>> We should not assume that. The function was written like that
> >>> specifically so the code around it will not rely on this assumption.
> >>> Now you change that specifically to write code that will do incorrect
> >>> assumptions. I don't see the logic here.
> >> The logic is that we do not support any other mapping yet - with or
> >> without this change. Without it, we complicate the APIC initialization
> >> for (so far) no good reason. Once we want to support different BSP
> >> assignments, we need to go through the code and rework some parts anyway.
> >>
> > As far as I remember the only part that was missing was a command line to
> > specify apic IDs for each CPU and what CPU is BSP. The code was ready
> > otherwise. I's very sad if this was broken by other modifications. But
> > changes like that actually pushes us back from our goal. Why not rework
> > code so it will work with correct cpu_is_bsp() function instead of
> > introducing this hack?
> 
> If you can confirm that there is a serious use case behind it, I will
> look into this again. But so far, I did not find it.
> 
Firs of all it is correctness issue. We should emulate x86 platform and
nothing there says that BSP apic id is zero. Second part of CPU topology
information is encoded in apic id. i.e when socket/core/ht topology is
used we can't just arbitrary specify apic ids for each logical cpu, we
should follow the rules described in SDM. For instance when more then 16
CPUs are present AMD advices to start numbering apic ids from 16 and leave
first 16 IDs for IOAPICs. And third introduction of this hack shows that
something is done wrong in other places of the code. Somewhere
initialization order is incorrect.

--
			Gleb.
Jan Kiszka - March 4, 2010, 11:35 a.m.
Gleb Natapov wrote:
> On Thu, Mar 04, 2010 at 09:23:46AM +0100, Jan Kiszka wrote:
>> Gleb Natapov wrote:
>>> On Thu, Mar 04, 2010 at 12:34:22AM +0100, Jan Kiszka wrote:
>>>> Gleb Natapov wrote:
>>>>> On Mon, Mar 01, 2010 at 06:17:22PM +0100, Jan Kiszka wrote:
>>>>>> As we hard-wire the BSP to CPU 0 anyway and cpuid_apic_id equals
>>>>>> cpu_index, cpu_is_bsp can also be based on the latter directly. This
>>>>>> will help an early user of it: KVM while initializing mp_state.
>>>>>>
>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>> ---
>>>>>>  hw/pc.c |    3 ++-
>>>>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>>>>
>>>>>> diff --git a/hw/pc.c b/hw/pc.c
>>>>>> index b90a79e..58c32ea 100644
>>>>>> --- a/hw/pc.c
>>>>>> +++ b/hw/pc.c
>>>>>> @@ -767,7 +767,8 @@ static void pc_init_ne2k_isa(NICInfo *nd)
>>>>>>  
>>>>>>  int cpu_is_bsp(CPUState *env)
>>>>>>  {
>>>>>> -    return env->cpuid_apic_id == 0;
>>>>>> +    /* We hard-wire the BSP to the first CPU. */
>>>>>> +    return env->cpu_index == 0;
>>>>>>  }
>>>>> We should not assume that. The function was written like that
>>>>> specifically so the code around it will not rely on this assumption.
>>>>> Now you change that specifically to write code that will do incorrect
>>>>> assumptions. I don't see the logic here.
>>>> The logic is that we do not support any other mapping yet - with or
>>>> without this change. Without it, we complicate the APIC initialization
>>>> for (so far) no good reason. Once we want to support different BSP
>>>> assignments, we need to go through the code and rework some parts anyway.
>>>>
>>> As far as I remember the only part that was missing was a command line to
>>> specify apic IDs for each CPU and what CPU is BSP. The code was ready
>>> otherwise. I's very sad if this was broken by other modifications. But
>>> changes like that actually pushes us back from our goal. Why not rework
>>> code so it will work with correct cpu_is_bsp() function instead of
>>> introducing this hack?
>> If you can confirm that there is a serious use case behind it, I will
>> look into this again. But so far, I did not find it.
>>
> Firs of all it is correctness issue. We should emulate x86 platform and
> nothing there says that BSP apic id is zero. Second part of CPU topology
> information is encoded in apic id. i.e when socket/core/ht topology is
> used we can't just arbitrary specify apic ids for each logical cpu, we
> should follow the rules described in SDM. For instance when more then 16
> CPUs are present AMD advices to start numbering apic ids from 16 and leave
> first 16 IDs for IOAPICs. And third introduction of this hack shows that
> something is done wrong in other places of the code. Somewhere
> initialization order is incorrect.

Well, it looks like we need to answer two questions: How shall to user
specify the BSP? And how to reliably map this on QEMU's internal
cpu_index? Depending on this, apic numbering may or may not be an
orthogonal issue.

BTW, do real systems allow to hot plug BSP as well? Or how is the case
handled when you unplug the BSP and then reboot the box?

Jan
Gleb Natapov - March 4, 2010, 12:03 p.m.
On Thu, Mar 04, 2010 at 12:35:45PM +0100, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Thu, Mar 04, 2010 at 09:23:46AM +0100, Jan Kiszka wrote:
> >> Gleb Natapov wrote:
> >>> On Thu, Mar 04, 2010 at 12:34:22AM +0100, Jan Kiszka wrote:
> >>>> Gleb Natapov wrote:
> >>>>> On Mon, Mar 01, 2010 at 06:17:22PM +0100, Jan Kiszka wrote:
> >>>>>> As we hard-wire the BSP to CPU 0 anyway and cpuid_apic_id equals
> >>>>>> cpu_index, cpu_is_bsp can also be based on the latter directly. This
> >>>>>> will help an early user of it: KVM while initializing mp_state.
> >>>>>>
> >>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>> ---
> >>>>>>  hw/pc.c |    3 ++-
> >>>>>>  1 files changed, 2 insertions(+), 1 deletions(-)
> >>>>>>
> >>>>>> diff --git a/hw/pc.c b/hw/pc.c
> >>>>>> index b90a79e..58c32ea 100644
> >>>>>> --- a/hw/pc.c
> >>>>>> +++ b/hw/pc.c
> >>>>>> @@ -767,7 +767,8 @@ static void pc_init_ne2k_isa(NICInfo *nd)
> >>>>>>  
> >>>>>>  int cpu_is_bsp(CPUState *env)
> >>>>>>  {
> >>>>>> -    return env->cpuid_apic_id == 0;
> >>>>>> +    /* We hard-wire the BSP to the first CPU. */
> >>>>>> +    return env->cpu_index == 0;
> >>>>>>  }
> >>>>> We should not assume that. The function was written like that
> >>>>> specifically so the code around it will not rely on this assumption.
> >>>>> Now you change that specifically to write code that will do incorrect
> >>>>> assumptions. I don't see the logic here.
> >>>> The logic is that we do not support any other mapping yet - with or
> >>>> without this change. Without it, we complicate the APIC initialization
> >>>> for (so far) no good reason. Once we want to support different BSP
> >>>> assignments, we need to go through the code and rework some parts anyway.
> >>>>
> >>> As far as I remember the only part that was missing was a command line to
> >>> specify apic IDs for each CPU and what CPU is BSP. The code was ready
> >>> otherwise. I's very sad if this was broken by other modifications. But
> >>> changes like that actually pushes us back from our goal. Why not rework
> >>> code so it will work with correct cpu_is_bsp() function instead of
> >>> introducing this hack?
> >> If you can confirm that there is a serious use case behind it, I will
> >> look into this again. But so far, I did not find it.
> >>
> > Firs of all it is correctness issue. We should emulate x86 platform and
> > nothing there says that BSP apic id is zero. Second part of CPU topology
> > information is encoded in apic id. i.e when socket/core/ht topology is
> > used we can't just arbitrary specify apic ids for each logical cpu, we
> > should follow the rules described in SDM. For instance when more then 16
> > CPUs are present AMD advices to start numbering apic ids from 16 and leave
> > first 16 IDs for IOAPICs. And third introduction of this hack shows that
> > something is done wrong in other places of the code. Somewhere
> > initialization order is incorrect.
> 
> Well, it looks like we need to answer two questions: How shall to user
> specify the BSP? And how to reliably map this on QEMU's internal
> cpu_index? Depending on this, apic numbering may or may not be an
> orthogonal issue.
> 
Two good question :) We can extend -cpu command to let as specify base
apic id for each socket. Apic ids of logical cpus are derived from this
base acpi id depending on where in hierarchy the logical cpu resided.
cpu_index thing in QEMU is pretty messy. The way non x86 arches use it
make it hard to cleanup, so this is why I didn't want to rely on it at
all and use acpi id instead. Thinking about it cpu_is_bsp() should
really check BSP bit in apic base register. 


> BTW, do real systems allow to hot plug BSP as well? Or how is the case
> handled when you unplug the BSP and then reboot the box?
> 
Did you mean hot unplug BSP? OS determines what CPU is BSP by checking
BSP bit in APIC base register. My guess is that there is some pin on CPU
which value is mirrored as BSP bit in APIC base register. Board may have
some logic to check what sockets are populated and chose one of them as
BSP by pulling its pin up. But this is only guess.

--
			Gleb.
Gleb Natapov - March 7, 2010, 6:36 a.m.
On Thu, Mar 04, 2010 at 02:03:54PM +0200, Gleb Natapov wrote:
> > BTW, do real systems allow to hot plug BSP as well? Or how is the case
> > handled when you unplug the BSP and then reboot the box?
> > 
> Did you mean hot unplug BSP? OS determines what CPU is BSP by checking
> BSP bit in APIC base register. My guess is that there is some pin on CPU
> which value is mirrored as BSP bit in APIC base register. Board may have
> some logic to check what sockets are populated and chose one of them as
> BSP by pulling its pin up. But this is only guess.
> 
Actually this is much more simple:
SDM 8.4.1:
 The MP initialization protocol defines two classes of processors: the
 bootstrap processor (BSP) and the application processors (APs). Following
 a power-up or RESET of an MP system, system hardware dynamically selects
 one of the processors on the system bus as the BSP. The remaining
 processors are designated as APs.
And by "hardware" they mean CPUs themselves over apic BUS.

--
			Gleb.
Jan Kiszka - March 7, 2010, 10:48 a.m.
Gleb Natapov wrote:
> On Thu, Mar 04, 2010 at 02:03:54PM +0200, Gleb Natapov wrote:
>>> BTW, do real systems allow to hot plug BSP as well? Or how is the case
>>> handled when you unplug the BSP and then reboot the box?
>>>
>> Did you mean hot unplug BSP? OS determines what CPU is BSP by checking
>> BSP bit in APIC base register. My guess is that there is some pin on CPU
>> which value is mirrored as BSP bit in APIC base register. Board may have
>> some logic to check what sockets are populated and chose one of them as
>> BSP by pulling its pin up. But this is only guess.
>>
> Actually this is much more simple:
> SDM 8.4.1:
>  The MP initialization protocol defines two classes of processors: the
>  bootstrap processor (BSP) and the application processors (APs). Following
>  a power-up or RESET of an MP system, system hardware dynamically selects
>  one of the processors on the system bus as the BSP. The remaining
>  processors are designated as APs.
> And by "hardware" they mean CPUs themselves over apic BUS.

That should be straightforward, will have a look. I just want to ensure
that cpu_is_bsp delivers a valid result all the time. Otherwise we will
run into ordering issues again once some new user of this services shows up.

Jan
Jan Kiszka - March 7, 2010, 1:44 p.m.
Jan Kiszka wrote:
> Gleb Natapov wrote:
>> On Thu, Mar 04, 2010 at 02:03:54PM +0200, Gleb Natapov wrote:
>>>> BTW, do real systems allow to hot plug BSP as well? Or how is the case
>>>> handled when you unplug the BSP and then reboot the box?
>>>>
>>> Did you mean hot unplug BSP? OS determines what CPU is BSP by checking
>>> BSP bit in APIC base register. My guess is that there is some pin on CPU
>>> which value is mirrored as BSP bit in APIC base register. Board may have
>>> some logic to check what sockets are populated and chose one of them as
>>> BSP by pulling its pin up. But this is only guess.
>>>
>> Actually this is much more simple:
>> SDM 8.4.1:
>>  The MP initialization protocol defines two classes of processors: the
>>  bootstrap processor (BSP) and the application processors (APs). Following
>>  a power-up or RESET of an MP system, system hardware dynamically selects
>>  one of the processors on the system bus as the BSP. The remaining
>>  processors are designated as APs.
>> And by "hardware" they mean CPUs themselves over apic BUS.
> 
> That should be straightforward, will have a look. I just want to ensure
> that cpu_is_bsp delivers a valid result all the time. Otherwise we will
> run into ordering issues again once some new user of this services shows up.
> 

...but you have to explain to me how CPU hotplugging in current qemu-kvm
is supposed to work. 'cpu_set n offline' does not have any visible
effect: Linux still allows to bring such a CPU online again, and it is
also brought up again after reset.

Besides this, I would probably need some other OS to test offlining the
BSP - Linux does not appear to support it.

Jan
Gleb Natapov - March 7, 2010, 1:50 p.m.
On Sun, Mar 07, 2010 at 02:44:50PM +0100, Jan Kiszka wrote:
> Jan Kiszka wrote:
> > Gleb Natapov wrote:
> >> On Thu, Mar 04, 2010 at 02:03:54PM +0200, Gleb Natapov wrote:
> >>>> BTW, do real systems allow to hot plug BSP as well? Or how is the case
> >>>> handled when you unplug the BSP and then reboot the box?
> >>>>
> >>> Did you mean hot unplug BSP? OS determines what CPU is BSP by checking
> >>> BSP bit in APIC base register. My guess is that there is some pin on CPU
> >>> which value is mirrored as BSP bit in APIC base register. Board may have
> >>> some logic to check what sockets are populated and chose one of them as
> >>> BSP by pulling its pin up. But this is only guess.
> >>>
> >> Actually this is much more simple:
> >> SDM 8.4.1:
> >>  The MP initialization protocol defines two classes of processors: the
> >>  bootstrap processor (BSP) and the application processors (APs). Following
> >>  a power-up or RESET of an MP system, system hardware dynamically selects
> >>  one of the processors on the system bus as the BSP. The remaining
> >>  processors are designated as APs.
> >> And by "hardware" they mean CPUs themselves over apic BUS.
> > 
> > That should be straightforward, will have a look. I just want to ensure
> > that cpu_is_bsp delivers a valid result all the time. Otherwise we will
> > run into ordering issues again once some new user of this services shows up.
> > 
> 
> ...but you have to explain to me how CPU hotplugging in current qemu-kvm
> is supposed to work. 'cpu_set n offline' does not have any visible
> effect: Linux still allows to bring such a CPU online again, and it is
> also brought up again after reset.
Current BIOS does not support cpu hotplug/unplug. If after reset
unplugged vcpu is brought up again then there is a bug there somewhere.

> 
> Besides this, I would probably need some other OS to test offlining the
> BSP - Linux does not appear to support it.
> 

--
			Gleb.

Patch

diff --git a/hw/pc.c b/hw/pc.c
index b90a79e..58c32ea 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -767,7 +767,8 @@  static void pc_init_ne2k_isa(NICInfo *nd)
 
 int cpu_is_bsp(CPUState *env)
 {
-    return env->cpuid_apic_id == 0;
+    /* We hard-wire the BSP to the first CPU. */
+    return env->cpu_index == 0;
 }
 
 CPUState *pc_new_cpu(const char *cpu_model)