[SeaBIOS] [PATCH 4/4] cleanup smp_setup()

Kevin O'Connor kevin at koconnor.net
Wed May 11 16:08:11 CEST 2016


On Wed, May 11, 2016 at 11:50:36AM +0200, Igor Mammedov wrote:
> On Tue, 10 May 2016 16:43:34 +0200
> Igor Mammedov <imammedo at redhat.com> wrote:
> 
> > MaxCountCPUs could never be 0 or less CountCPUs
> > anymore, remove code that wouldn't be executed.
> > 
> > Signed-off-by: Igor Mammedov <imammedo at redhat.com>
> > ---
> >  src/fw/smp.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/src/fw/smp.c b/src/fw/smp.c
> > index 18a4c77..4d82502 100644
> > --- a/src/fw/smp.c
> > +++ b/src/fw/smp.c
> > @@ -175,8 +175,6 @@ smp_setup(void)
> >      *(u64*)BUILD_AP_BOOT_ADDR = old;
> Also looking at usage of BUILD_AP_BOOT_ADDR, it doesn't seem
> to user anywhere beside of smp_setup() and
> smp_setup every time overwrites it to point to entry_smp trampoline
> So saving/restoring 'old' value doesn't look like necessary,
> should it be cleaned up as well?

That's in there because technically malloc_tmplow() could give out
that address range (see alloc_add(&ZoneTmpLow) in malloc_preinit).
So, the idea is to save/restore the area so that the trampoline
doesn't alter anything stored there.  In practice nothing that early
in the boot would use enough malloc_tmplow space to give out that
address, but it doesn't hurt to save/restore either.


> >      handle_x2apic();
> > -    if (!MaxCountCPUs || MaxCountCPUs < CountCPUs)
> > -        MaxCountCPUs = CountCPUs;

BTW, the "MaxCountCPUs < CountCPUs" check was in there for really old
versions of QEMU that didn't populate "etc/max-cpus".  Unless you're
sure that "etc/max-cpus" predates CMOS_BIOS_SMP_COUNT, I'd be inclined
to leave the check in there.  (No need to resend the series if not
sure).

-Kevin



More information about the SeaBIOS mailing list