Re: [SeaBIOS] [PATCH 3/3] Take in account hot(un)plugged cpus on reboot

[Gleb Natapov]

On Thu, Mar 15, 2012 at 10:33:01AM -0400, Igor Mammedov wrote:
> ----- Original Message -----
> > From: "Gleb Natapov" 
> > To: "Igor Mammedov" 
> > Cc: "Kevin O'Connor" , "jan kiszka" , [e-mail]
> > Sent: Thursday, March 15, 2012 3:13:58 PM
> > Subject: Re: [SeaBIOS] [PATCH 3/3] Take in account hot(un)plugged cpus on reboot
> >
> > On Thu, Mar 15, 2012 at 10:08:17AM -0400, Igor Mammedov wrote:
> > > > Good point. You mean we need a list of apic ids of available cpus
> > > > right?
> > > > Can SeaBIOS build this list by reading apic id in
> > > > smp_ap_boot_code?
> > >
> > > I've looked at how tables are build. It seems that it's not hard
> > > and sufficient
> > > to build an equivalent temporary byte array of (un)available apic
> > > ids that
> > > corresponds to 0xaf00 bitmap at boot time, under following
> > > restriction:
> > > apic id shall be in range [0..max_cpus)
> >
> > Why this restriction? ap bootstrap code can grab array entry with
> > atomic
> > operation and store its apic id there.
>
> we need to know in advance how large array to allocate.
> and if apic id is to serve as index in this array and to correspond to
> appropriate bit in 0xaf00 bitmap.
>
> Why limiting apic id range to [0..max_cpus) is bad thing?

IIRC part of topology information is conveyed through apic ids, so you
cannot express some topology with limited apic ranges. That said
currently seabios assumes that apic ids are in this range so it would
not be a big dial to continue assuming it for now.
> > >  * this will allow to allocate array where apic id will serve
> > >  as index,
> > >  thus making it equvivalent to 0xaf00 bitmap.
> > >  * and easily fill this array in smp_ap_boot_code without
> > >  inventing
> > >  and using mutexes/spinlocks in smp_ap_boot_code.
> > >
> > >
> > > This will require cooperation from qemu side. apic id could be
> > > checked
> > > somewhere in device_add command handler call chain or at cpu object
> > > creation
> > > time.
> > >
> > > Jan,
> > >   What do you think about setting such restriction on apic id of
> > >   cpus in
> > >   qemu?
> >
> > We do have such restriction currently. A couple of years ago I
> > removed
> > it from the kernel, but qemu and current bios code still has it.

[PATCH 3/3] Take in account hot(un)plugged cpus on reboot

[Igor Mammedov]

All that said it leaves only
  [PATCH 1/3] Halt if number of started cpus are more then expected
sensible to re-post to prevent excessive cpu consumption in case of
error. And cmos_smp_count should be updated on (un)plug event in
cmos device in qemu.

On 03/15/2012 04:29 PM, Gleb Natapov wrote:
> On Thu, Mar 15, 2012 at 10:33:01AM -0400, Igor Mammedov wrote:
> > ----- Original Message -----
> > > From: "Gleb Natapov"
> > > To: "Igor Mammedov"
> > > Cc: "Kevin O'Connor", "jan kiszka", [e-mail]
> > > Sent: Thursday, March 15, 2012 3:13:58 PM
> > > Subject: Re: [SeaBIOS] [PATCH 3/3] Take in account hot(un)plugged cpus on reboot
> > >
> > > On Thu, Mar 15, 2012 at 10:08:17AM -0400, Igor Mammedov wrote:
> > > > > Good point. You mean we need a list of apic ids of available cpus
> > > > > right?
> > > > > Can SeaBIOS build this list by reading apic id in
> > > > > smp_ap_boot_code?
> > > >
> > > > I've looked at how tables are build. It seems that it's not hard
> > > > and sufficient
> > > > to build an equivalent temporary byte array of (un)available apic
> > > > ids that
> > > > corresponds to 0xaf00 bitmap at boot time, under following
> > > > restriction:
> > > > apic id shall be in range [0..max_cpus)
> > >
> > > Why this restriction? ap bootstrap code can grab array entry with
> > > atomic
> > > operation and store its apic id there.
> >
> > we need to know in advance how large array to allocate.
> > and if apic id is to serve as index in this array and to correspond to
> > appropriate bit in 0xaf00 bitmap.
> >
> > Why limiting apic id range to [0..max_cpus) is bad thing?
>
> IIRC part of topology information is conveyed through apic ids, so you
> cannot express some topology with limited apic ranges. That said
> currently seabios assumes that apic ids are in this range so it would
> not be a big dial to continue assuming it for now.
> > > >   * this will allow to allocate array where apic id will serve
> > > >   as index,
> > > >   thus making it equvivalent to 0xaf00 bitmap.
> > > >   * and easily fill this array in smp_ap_boot_code without
> > > >   inventing
> > > >   and using mutexes/spinlocks in smp_ap_boot_code.
> > > >
> > > >
> > > > This will require cooperation from qemu side. apic id could be
> > > > checked
> > > > somewhere in device_add command handler call chain or at cpu object
> > > > creation
> > > > time.
> > > >
> > > > Jan,
> > > >    What do you think about setting such restriction on apic id of
> > > >    cpus in
> > > >    qemu?
> > >
> > > We do have such restriction currently. A couple of years ago I
> > > removed
> > > it from the kernel, but qemu and current bios code still has it.