[coyotos-dev] SMP Issue: load correct kernel stack on trap

[Jonathan S. Shapiro]

Jeroen:

I had already started doing SMP support. Unfortunately, it looks like
your approach and mine went in opposite directions. I think you were
misled by a bunch of stale comments and documentation. Looking at the
patch was very useful to me, so I don't want you to be discouraged, but
we are not going to be able to use this patch directly.

Here are the issues:

0. Several parts of the patch are cosmetic changes or (in a few cases)
temporary development changes. These should be separated or removed.

1. Your code finds the current CPU from the current process. In the
interrupt handler this is necessary, but in C code it is not the best
way. We already had a mechanism for this. See target-hal/cpu.h and the
revised definition of MY_CPU. We chose to get the current process from
the current CPU.

We can change this if there is a reason to do it, but we thought this
approach would probably work better.

2. We chose to store the LEAST address of the stack in the CPU structure
because that is where the current CPU address needs to go. The top of
stack can be computed from cpu->stack+KSTACK_SIZE. You should only need
that when you are initializing stack for the APs.  Oh. Now I see why you
did it. There may be a better way, but that was certainly reasonable.
The addition of KSTACK_MASK and KSTACK_SIZE in asm-offsets.c should not
have been necessary -- you should have been able to use them directly
from config.h.

3. We decided to move all CPU-local data into the CPU structure. The
"percpu" region in ldscript is now empty, and will go away as we clean
up the SMP implementation.

4. We intend for all processors to have the same private page table and
private page directory, so there is no need to allocate per-AP tables in
hwmap.c. Is there a reason you did this that we may not have noticed?

5. We definitely care about SMP on non-PAE machines (for efficiency
reasons).

6. I have not explored your code enough to understand this, but we
definitely want a way to force SMP machines to operating with only one
CPU. Does your patch preserve a way to do this?


It is very possible that I have failed to understand a bunch of things
in your patch, so please do not hesitate to explain or correct me!


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