Kernel-mode stack layout (FPU save area, trap frames, etc.)

[KJK::Hyperion]

I've finally taken the time to write it all down:

http://mok.lvcm.com/cgi-bin/reactos/roswiki?KernelModeStackLayout

Let me know if you see any mistakes or omissions

[Hartmut Birr]

Hi,

I think we need two floating save areas, one for user mode and one for
kernel mode. The area for kernel mode is necessary because win32k and
freetype are using the fpu. The kernel mode area is not necessary if we
protect some function in win32k with
KeSaveFloatingPointState/KeRestoreFloatingPointState. The start of the areas
should be calculate as a offset from the top of the kernel stack and not as
offset from the user mode trap frame. The state of the fpu must be saved on
a thread switch and if the fpu was used. At this point the start of the user
mode trap frame is only known as an offset from the top of the stack.

- Hartmut
> -----Original Message-----
> From: [e-mail] 
> [mailto:[e-mail]] On Behalf Of KJK::Hyperion
> Sent: Wednesday, October 13, 2004 8:13 AM
> To: [e-mail]
> Subject: [ros-dev] Kernel-mode stack layout (FPU save area, 
> trap frames,etc.)
>
> I've finally taken the time to write it all down:
>
> http://mok.lvcm.com/cgi-bin/reactos/roswiki?KernelModeStackLayout
>
> Let me know if you see any mistakes or omissions

[KJK::Hyperion]

At 19.02 15/10/2004, you wrote:
> I think we need two floating save areas, one for user mode and one for 
> kernel mode. The area for kernel mode is necessary because win32k and 
> freetype are using the fpu. The kernel mode area is not necessary if we 
> protect some function in win32k with 
> KeSaveFloatingPointState/KeRestoreFloatingPointState.

I think kernel-mode components using the FPU on x86 are supposed to save 
the state themselves. Win32k actually uses a software implementation of 
fixed-point numbers (FLOATOBJ), though. FreeType isn't a problem, since we 
already have "enter FreeType" and "leave FreeType" functions in win32k, 
which at the moment acquire a mutex (FreeType isn't thread-safe), but could 
be extended to save the FPU state too (I'm not so cool about it, anyway. It 
all sounds very expensive)
> The start of the areas should be calculate as a offset from the top of the 
> kernel stack and not as offset from the user mode trap frame.

Offset from the user-mode trap frame is how Windows addresses it
> The state of the fpu must be saved on a thread switch and if the fpu was 
> used. At this point the start of the user mode trap frame is only known as 
> an offset from the top of the stack.

Consider that the user-mode trap frame, in turn, is at a fixed offset from 
the stack base, since the FPU save area and the trap frame have a fixed 
size. And, at thread switch, you need to load the correct value for the 
ring 0 SS:ESP in the TSS, and this will be a fixed offset from the 
user-mode trap frame too (&TrapFrame->HardwareEsp, IIRC)

[Hartmut Birr]

> -----Original Message-----
> From: [e-mail] 
> [mailto:[e-mail]] On Behalf Of KJK::Hyperion
> Sent: Friday, October 15, 2004 8:27 PM
> To: ReactOS Development List
> Subject: RE: [ros-dev] Kernel-mode stack layout (FPU save 
> area, trapframes, etc.)
>
> Win32k actually uses a software 
> implementation of 
> fixed-point numbers (FLOATOBJ), though.

This is not correct. Many transformations are using double values. If you
look into win32k.map, you will found hundreds of fpu instructions.

- Hartmut

[KJK::Hyperion]

At 21.07 15/10/2004, you wrote:
> This is not correct. Many transformations are using double values. If you 
> look into win32k.map, you will found hundreds of fpu instructions.

I meant in Windows

[Anich Gregor]

Hello!

I have tried to get some initial FPU save code working the last few days, but 
it always crashed on fsave or after frstor (math-fault)... I tried to reseve 
an FX_SAVE_AREA on top of the kernel stack by changing 
Ke386InitThreadWithContext to decrement the kernel stack pointer by sizeof 
(FX_SAVE_AREA) and changing the PsBeginThreadWithContext (in bthread.S) to 
not pop (frstor + decl esp) a FLOATING_AREA from the initial stack, so the 
FX_SAVE_AREA which is at the top of the stack should not be touched since the 
stack grows downwards, but it got either overwritten at some point or 
InitialThread in KTHREAD changes from time to time... Do you know if I should 
change the allocation size for the stack to MM_STACK_SIZE + sizeof 
(FX_SAVE_AREA) and use InitialThread instead of InitialThread - sizeof 
(FX_SAVE_AREA) for the FX_SAVE_AREA?

- blight

On Friday 15 October 2004 19:02, Hartmut Birr wrote:
> Hi,
>
> I think we need two floating save areas, one for user mode and one for
> kernel mode. The area for kernel mode is necessary because win32k and
> freetype are using the fpu. The kernel mode area is not necessary if we
> protect some function in win32k with
> KeSaveFloatingPointState/KeRestoreFloatingPointState. The start of the
> areas should be calculate as a offset from the top of the kernel stack and
> not as offset from the user mode trap frame. The state of the fpu must be
> saved on a thread switch and if the fpu was used. At this point the start
> of the user mode trap frame is only known as an offset from the top of the
> stack.
> Hartmut
> > -----Original Message-----
> > From: [e-mail]
> > [mailto:[e-mail]] On Behalf Of KJK::Hyperion
> > Sent: Wednesday, October 13, 2004 8:13 AM
> > To: [e-mail]
> > Subject: [ros-dev] Kernel-mode stack layout (FPU save area,
> > trap frames,etc.)
> >
> > I've finally taken the time to write it all down:
> >
> > http://mok.lvcm.com/cgi-bin/reactos/roswiki?KernelModeStackLayout
> >
> > Let me know if you see any mistakes or omissions

[Hartmut Birr]

Hi,

you must change all position where a kernel stack is initialized:
- In multiboot.S is setup the initial stack
- In tss.c is setup the tss with the initial stack
- In w32call.c is setup a new kernel stack for the callback function.
- In kthread.c are setup some values on the top of the stack
- In bthread.S starts the execution of a new thread

It seems that you have changed only the last two points. The size of the
stack must not be changed.

- Hartmut
> -----Original Message-----
> From: [e-mail] 
> [mailto:[e-mail]] On Behalf Of Anich Gregor
> Sent: Saturday, October 16, 2004 1:52 PM
> To: [e-mail]
> Subject: Re: [ros-dev] Kernel-mode stack layout (FPU save 
> area, trap frames,etc.)
>
> Hello!
>
> I have tried to get some initial FPU save code working the 
> last few days, but 
> it always crashed on fsave or after frstor (math-fault)... I 
> tried to reseve 
> an FX_SAVE_AREA on top of the kernel stack by changing 
> Ke386InitThreadWithContext to decrement the kernel stack 
> pointer by sizeof 
> (FX_SAVE_AREA) and changing the PsBeginThreadWithContext (in 
> bthread.S) to 
> not pop (frstor + decl esp) a FLOATING_AREA from the initial 
> stack, so the 
> FX_SAVE_AREA which is at the top of the stack should not be 
> touched since the 
> stack grows downwards, but it got either overwritten at some point or 
> InitialThread in KTHREAD changes from time to time... Do you 
> know if I should 
> change the allocation size for the stack to MM_STACK_SIZE + sizeof 
> (FX_SAVE_AREA) and use InitialThread instead of InitialThread
> sizeof
>
> (FX_SAVE_AREA) for the FX_SAVE_AREA?
> blight
>
> On Friday 15 October 2004 19:02, Hartmut Birr wrote:
> > Hi,
> >
> > I think we need two floating save areas, one for user mode
>
> and one for
> > kernel mode. The area for kernel mode is necessary because
>
> win32k and
> > freetype are using the fpu. The kernel mode area is not
>
> necessary if we
> > protect some function in win32k with
> > KeSaveFloatingPointState/KeRestoreFloatingPointState. The
>
> start of the
> > areas should be calculate as a offset from the top of the
>
> kernel stack and
> > not as offset from the user mode trap frame. The state of
>
> the fpu must be
> > saved on a thread switch and if the fpu was used. At this
>
> point the start
> > of the user mode trap frame is only known as an offset from
>
> the top of the
> > stack.
> > Hartmut
> > > -----Original Message-----
> > > From: [e-mail]
> > > [mailto:[e-mail]] On Behalf Of KJK::Hyperion
> > > Sent: Wednesday, October 13, 2004 8:13 AM
> > > To: [e-mail]
> > > Subject: [ros-dev] Kernel-mode stack layout (FPU save area,
> > > trap frames,etc.)
> > >
> > > I've finally taken the time to write it all down:
>
> http://mok.lvcm.com/cgi-bin/reactos/roswiki?KernelModeStackLayout
> > > Let me know if you see any mistakes or omissions
>
> Ros-dev mailing list
> [e-mail]
> http://reactos.com:8080/mailman/listinfo/ros-dev

[Anich Gregor]

Hi!

I have changed all the places you mentioned (I hope) and even more - still the 
area gets written into... when I did a backtrace from the place where the 
memory was written it seemed to be in irq_handler_0. I did not see where the 
CPU was pointed to a TSS which it would take the stack pointer for the irq 
handler from, but I have changed all places where a stack pointer is setup in 
a TSS I think.
I have attached the changes which I have made so far, it's very unstable, some 
things are wrong and it does none of the important work (fsave on taskswitch 
is commented out because it crashed everytime, frstor with a corrupted FPU 
save area caused math-faults to raise after the not-present fault which also 
crashed somewhere in KiDispatchException - maybe because they were raised in 
KernelMode and not handled, so I replaced it by finit for now, ...)

In ke/i386/exp.c ExceptionToNtStatus and ExceptionTypeStrings are wrong I 
think, interrupt #15 is reserved by intel, #16 is math fault and #17 is 
alignment check. At the moment #15 is missing in the table, so #15 is math 
fault, #16 is alignment check and so on.
Maybe "if (ExceptionNr < 16)" (around line 170) should be changed to something 
like "if (ExceptionNr < ARRAY_SIZE(ExceptionToNtStatus)" (there are two other 
places where "ExceptionNr < 19" is hardcoded, and we could use "ExceptionNr < 
ARRAY_SIZE(ExceptionTypeStrings)".
Do we have any macro like ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0])) ?

I was also wondering about line 137 in tss.c (Tss->Esp0 = 
(ULONG)Ki386InitialStackArray[Id];) because it sets the stack pointer of the 
Tss to Ki386InitialStackArray[Id], which is set to &init stack on line 217 
(Ki386InitialStackArray[0] = (PVOID)&init_stack;) - shouldn't it be set to 
&init_stack_top?

- blight

On Saturday 16 October 2004 14:33, Hartmut Birr wrote:
> Hi,
>
> you must change all position where a kernel stack is initialized:
> In multiboot.S is setup the initial stack
> In tss.c is setup the tss with the initial stack
> In w32call.c is setup a new kernel stack for the callback function.
> In kthread.c are setup some values on the top of the stack
> In bthread.S starts the execution of a new thread
>
> It seems that you have changed only the last two points. The size of the
> stack must not be changed.
> Hartmut

[KJK::Hyperion]

At 16.47 21/10/2004, you wrote:
> I have changed all the places you mentioned (I hope) and even more - still 
> the area gets written into... when I did a backtrace from the place where 
> the memory was written it seemed to be in irq_handler_0. I did not see 
> where the CPU was pointed to a TSS which it would take the stack pointer 
> for the irq handler from,

on thread switch, all the appropriate per-thread attributes (mostly 
kernel-mode stack stuff) are copied in the global CPU state (either in the 
KPCR or in CPU-specific structures like the TSS). In the thread switch code 
you should see an interrupt-critical region (cli/sti) where this operation 
is performed
> Do we have any macro like ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0])) ?

winnt.h and ntdef.h define an undocumented RTL_NUMBER_OF macro for Windows 
XP or higher (?)

[Anich Gregor]

On Thursday 21 October 2004 17:40, KJK::Hyperion wrote:
> on thread switch, all the appropriate per-thread attributes (mostly
> kernel-mode stack stuff) are copied in the global CPU state (either in the
> KPCR or in CPU-specific structures like the TSS). In the thread switch code
> you should see an interrupt-critical region (cli/sti) where this operation
> is performed

Hi!

I have found in tskswitch.S a place which i did not change, and changing it 
fixed corruption of the FPU save area. I will continue working on it these 
days and try to do the next step ;)
Thank you!

- blight