[SeaBIOS] [PATCH v3 2/3] smp: refactor present CPU APIC ID detection and counting

Kevin O'Connor kevin at koconnor.net
Wed Aug 10 17:27:36 CEST 2016


On Wed, Aug 10, 2016 at 12:33:22PM +0200, Igor Mammedov wrote:
> On Mon, 8 Aug 2016 18:37:13 -0400
> "Kevin O'Connor" <kevin at koconnor.net> wrote:
> 
> > On Fri, Aug 05, 2016 at 12:47:28PM +0200, Igor Mammedov wrote:
> > > From: Kevin O'Connor <kevin at koconnor.net>
> > > 
> > > Signed-off-by: "Kevin O'Connor" <kevin at koconnor.net>
> > > Signed-off-by: Igor Mammedov <imammedo at redhat.com>
> > > ---
> > > v2:
> > >   * s/count_cpu/apic_id_init/
> > >   * call apic_id_init() after sending SIPI,
> > >     it will be needed for switching BSP into x2APIC mode
> > > ---
> > >  src/fw/smp.c | 32 +++++++++++++++++++-------------
> > >  1 file changed, 19 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/src/fw/smp.c b/src/fw/smp.c
> > > index 719d51d..2c5670c 100644
> > > --- a/src/fw/smp.c
> > > +++ b/src/fw/smp.c
> > > @@ -55,24 +55,32 @@ int apic_id_is_present(u8 apic_id)
> > >      return !!(FoundAPICIDs[apic_id/32] & (1ul << (apic_id % 32)));
> > >  }
> > >  
> > > +static int
> > > +apic_id_init(void)
> > > +{
> > > +    CountCPUs++;
> > > +
> > > +    // Track found apic id for use in legacy internal bios tables
> > > +    u32 eax, ebx, ecx, cpuid_features;
> > > +    cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> > > +    u8 apic_id = ebx>>24;
> > > +    FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> > > +
> > > +    return apic_id;
> > > +}
> > > +
> > >  void VISIBLE32FLAT
> > >  handle_smp(void)
> > >  {
> > >      if (!CONFIG_QEMU)
> > >          return;
> > >  
> > > -    // Detect apic_id
> > > -    u32 eax, ebx, ecx, cpuid_features;
> > > -    cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> > > -    u8 apic_id = ebx>>24;
> > > +    // Track this CPU and detect the apic_id
> > > +    int apic_id = apic_id_init();
> > >      dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);
> > >  
> > >      smp_write_msrs();
> > >  
> > > -    // Set bit on FoundAPICIDs
> > > -    FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> > > -
> > > -    CountCPUs++;
> > >  }
> > >  
> > >  // Atomic lock for shared stack across processors.
> > > @@ -93,11 +101,6 @@ smp_scan(void)
> > >          return;
> > >      }
> > >  
> > > -    // mark the BSP initial APIC ID as found, too:
> > > -    u8 apic_id = ebx>>24;
> > > -    FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> > > -    CountCPUs = 1;  
> > 
> > Now that scan_smp() is called from the resume path, we do have to
> > initialize CountCPUs here.  Probably best to not move the updating of
> > CountCPUs into apic_id_init().
> Indeed with this patch guest hung on resume.
> 
> adding CountCPUs = 0 here here fixes it,
> but I can just not move CountCPUs as you suggest.
> 
> What would you prefer?

I'd say the latter.

-Kevin



More information about the SeaBIOS mailing list