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

[Igor Mammedov]

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?



> 
> -Kevin