[coyotos-dev] SMP Patch 1

[Jonathan S. Shapiro]

On Wed, 2007-10-24 at 13:04 -0400, Jeroen Visser wrote:

> How about I get you the transmap patch against the latest repository?

Great.

> After that I can integrate the good parts of my latest SMP work. Among
> other things it addresses:
> 
> low map:
> 
> The AP bootstrap code page has been augmented with a boot map page (2
> physical pages total for AP bootstrap). The boot map contains a copy
> of the kernel map, but with a low alias. The kernel map never contains
> a low mapping after arch_init.

I do not think that we need this. Look again at cpu_init_all_aps(). It
proceeds by temporarily re-establishing a low map. Once the APs are
initialized I don't think we need the low map.

> on-demand AP boot pages:
> 
> The AP boot pages can be mapped and unmapped on demand. Combined with
> the above we should be able to safely start APs (and perhaps reboot
> them) while running. If that is not required we can use this code to
> free up the AP bootstrap physical pages.

I would prefer to defer this until we have a reason to think that it is
required.

> MY_CPU(booted) field:
> 
> Set to 1 when the apboot has been run and interrupts have been
> enabled. The CPU is ready to dispatch its first process. In this mode
> the CPU sits in a "hlt" loop until MY_CPU(active) becomes 1. To
> actually start the CPU, set its active field to 1 and send it an IPI
> to break out of the hlt loop. CPU 0 waits for this field to transition
> from 0 -> 1 to detect AP spinup.

I was expecting to handle this a bit differently. If we can avoid it, I
think we do NOT want to enable AP interrupts that early, because
portions of the system are not yet fully initialized. This is why the
code I put in cpu_init_xxx and cpu_start_xxx uses a spinlock. The reason
we need MY_CPU(booted) is to make sure that we do not drop the low map
prematurely.

The idea here is that the AP enables its interrupts *after* it gets the
spinlock, which doesn't happen until everything in the kernel is fully
initialized.

> SMP command line options:
> 
> Options 'smp' and 'nosmp' controlling a variable named smp_enabled
> which defaults to false for now.

Please rename the variable to use_smp, but otherwise very good.

> AP bootstrap code cleanup:
> 
> Remove some unnecessary complexity. Use the new boot map page that was added.

Look at what I did and let me know if you still think that the AP boot
map is required. I hope that it is not, mainly because this would be a
bit simpler.

shap
-- 
Jonathan S. Shapiro, Ph.D.
Managing Director
The EROS Group, LLC
www.coyotos.org, www.eros-os.org