Wait, are you suggesting that "doing things better than Microsoft" and "those idiots have no idea what they're doing" and "we shouldn't just be copying the instructions they use".... doesn't work? :O
Best regards, Alex Ionescu On Thu, Apr 14, 2016 at 8:09 AM, Thomas Faber <thomas.fa...@reactos.org> wrote: > Looks like we've encountered an issue with this > KiTrapReturnNoSegmentsRet8 approach. As seen in > https://jira.reactos.org/browse/CORE-11115, when a large number of > interrupts happen, this will cause us to run out of kernel stack. > > Since the popfd / ret 8 steps do not happen atomically, interrupts will > be re-enabled before those 12 bytes of stack are freed, thus many of > them happening in a row will cause us to run out. > > Thoughts? > > Thanks! > -Thomas > > > > On 2013-10-19 13:33, tkreu...@svn.reactos.org wrote: >> Author: tkreuzer >> Date: Sat Oct 19 11:33:34 2013 >> New Revision: 60701 >> >> URL: http://svn.reactos.org/svn/reactos?rev=60701&view=rev >> Log: >> [NTOSKRNL] >> - Introduce a new and faster way to return to kernel mode from traps by >> using a ret 8 instruction instead of an iret. >> - Make use of KiUserTrap where appropriate >> - Remove some pointless toplevel volatile >> >> Modified: >> trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S >> trunk/reactos/ntoskrnl/include/internal/i386/ke.h >> trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h >> trunk/reactos/ntoskrnl/ke/i386/cpu.c >> trunk/reactos/ntoskrnl/ke/i386/exp.c >> trunk/reactos/ntoskrnl/ke/i386/patpge.c >> trunk/reactos/ntoskrnl/ke/i386/trap.s >> trunk/reactos/ntoskrnl/ke/i386/traphdlr.c >> trunk/reactos/ntoskrnl/ke/i386/usercall.c >> trunk/reactos/ntoskrnl/ke/time.c >> >> Modified: trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S?rev=60701&r1=60700&r2=60701&view=diff >> ============================================================================== >> --- trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S [iso-8859-1] >> (original) >> +++ trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S [iso-8859-1] >> Sat Oct 19 11:33:34 2013 >> @@ -241,6 +241,7 @@ >> #define KI_EXIT_RET HEX(080) >> #define KI_EXIT_IRET HEX(100) >> #define KI_EDITED_FRAME HEX(200) >> +#define KI_EXIT_RET8 HEX(400) >> #define KI_RESTORE_VOLATILES (KI_RESTORE_EAX OR KI_RESTORE_ECX_EDX) >> >> MACRO(KiTrapExitStub, Name, Flags) >> @@ -248,15 +249,15 @@ >> PUBLIC @&Name&@4 >> @&Name&@4: >> >> - if (Flags AND KI_RESTORE_EFLAGS) >> + if (Flags AND KI_EXIT_RET8) OR (Flags AND KI_EXIT_IRET) >> + >> + /* This is the IRET frame */ >> + OffsetEsp = KTRAP_FRAME_EIP >> + >> + elseif (Flags AND KI_RESTORE_EFLAGS) >> >> /* We will pop EFlags off the stack */ >> OffsetEsp = KTRAP_FRAME_EFLAGS >> - >> - elseif (Flags AND KI_EXIT_IRET) >> - >> - /* This is the IRET frame */ >> - OffsetEsp = KTRAP_FRAME_EIP >> >> else >> >> @@ -335,6 +336,13 @@ >> >> if (Flags AND KI_RESTORE_EFLAGS) >> >> + if (Flags AND KI_EXIT_RET8) >> + >> + /* We are at the IRET frame, so push EFLAGS first */ >> + push dword ptr [esp + 8] >> + >> + endif >> + >> /* Restore EFLAGS */ >> popfd >> >> @@ -357,6 +365,11 @@ >> /* Return to kernel mode with a jmp */ >> jmp edx >> >> + elseif (Flags AND KI_EXIT_RET8) >> + >> + /* Return to kernel mode with a ret 8 */ >> + ret 8 >> + >> elseif (Flags AND KI_EXIT_RET) >> >> /* Return to kernel mode with a ret */ >> >> Modified: trunk/reactos/ntoskrnl/include/internal/i386/ke.h >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/i386/ke.h?rev=60701&r1=60700&r2=60701&view=diff >> ============================================================================== >> --- trunk/reactos/ntoskrnl/include/internal/i386/ke.h [iso-8859-1] (original) >> +++ trunk/reactos/ntoskrnl/include/internal/i386/ke.h [iso-8859-1] Sat Oct >> 19 11:33:34 2013 >> @@ -383,13 +383,13 @@ >> ULONG_PTR >> NTAPI >> Ki386EnableGlobalPage( >> - IN volatile ULONG_PTR Context >> + IN ULONG_PTR Context >> ); >> >> ULONG_PTR >> NTAPI >> Ki386EnableTargetLargePage( >> - IN volatile ULONG_PTR Context >> + IN ULONG_PTR Context >> ); >> >> BOOLEAN >> >> Modified: trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h?rev=60701&r1=60700&r2=60701&view=diff >> ============================================================================== >> --- trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h [iso-8859-1] >> (original) >> +++ trunk/reactos/ntoskrnl/include/internal/i386/trap_x.h [iso-8859-1] >> Sat Oct 19 11:33:34 2013 >> @@ -163,7 +163,7 @@ >> } >> >> /* Check DR values */ >> - if (TrapFrame->SegCs & MODE_MASK) >> + if (KiUserTrap(TrapFrame) >> { >> /* Check for active debugging */ >> if (KeGetCurrentThread()->Header.DebugActive) >> @@ -244,6 +244,7 @@ >> DECLSPEC_NORETURN VOID FASTCALL KiEditedTrapReturn(IN PKTRAP_FRAME >> TrapFrame); >> DECLSPEC_NORETURN VOID FASTCALL KiTrapReturn(IN PKTRAP_FRAME TrapFrame); >> DECLSPEC_NORETURN VOID FASTCALL KiTrapReturnNoSegments(IN PKTRAP_FRAME >> TrapFrame); >> +DECLSPEC_NORETURN VOID FASTCALL KiTrapReturnNoSegmentsRet8(IN PKTRAP_FRAME >> TrapFrame); >> >> typedef >> ATTRIB_NORETURN >> @@ -382,7 +383,7 @@ >> TrapFrame->Dr7 = 0; >> >> /* Check if the frame was from user mode or v86 mode */ >> - if ((TrapFrame->SegCs & MODE_MASK) || >> + if (KiUserTrap(TrapFrame) || >> (TrapFrame->EFlags & EFLAGS_V86_MASK)) >> { >> /* Check for active debugging */ >> @@ -411,7 +412,7 @@ >> TrapFrame->Dr7 = 0; >> >> /* Check if the frame was from user mode or v86 mode */ >> - if ((TrapFrame->SegCs & MODE_MASK) || >> + if (KiUserTrap(TrapFrame) || >> (TrapFrame->EFlags & EFLAGS_V86_MASK)) >> { >> /* Check for active debugging */ >> >> Modified: trunk/reactos/ntoskrnl/ke/i386/cpu.c >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/cpu.c?rev=60701&r1=60700&r2=60701&view=diff >> ============================================================================== >> --- trunk/reactos/ntoskrnl/ke/i386/cpu.c [iso-8859-1] (original) >> +++ trunk/reactos/ntoskrnl/ke/i386/cpu.c [iso-8859-1] Sat Oct 19 >> 11:33:34 2013 >> @@ -1535,7 +1535,7 @@ >> if (TargetAffinity) >> { >> /* Sanity check */ >> - ASSERT(Prcb == (volatile PKPRCB)KeGetCurrentPrcb()); >> + ASSERT(Prcb == KeGetCurrentPrcb()); >> >> /* FIXME: TODO */ >> ASSERTMSG("Not yet implemented\n", FALSE); >> >> Modified: trunk/reactos/ntoskrnl/ke/i386/exp.c >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/exp.c?rev=60701&r1=60700&r2=60701&view=diff >> ============================================================================== >> --- trunk/reactos/ntoskrnl/ke/i386/exp.c [iso-8859-1] (original) >> +++ trunk/reactos/ntoskrnl/ke/i386/exp.c [iso-8859-1] Sat Oct 19 >> 11:33:34 2013 >> @@ -137,7 +137,7 @@ >> KiEspFromTrapFrame(IN PKTRAP_FRAME TrapFrame) >> { >> /* Check if this is user-mode or V86 */ >> - if ((TrapFrame->SegCs & MODE_MASK) || >> + if (KiUserTrap(TrapFrame) || >> (TrapFrame->EFlags & EFLAGS_V86_MASK)) >> { >> /* Return it directly */ >> @@ -175,7 +175,7 @@ >> Previous = KiEspFromTrapFrame(TrapFrame); >> >> /* Check if this is user-mode or V86 */ >> - if ((TrapFrame->SegCs & MODE_MASK) || >> + if (KiUserTrap(TrapFrame) || >> (TrapFrame->EFlags & EFLAGS_V86_MASK)) >> { >> /* Write it directly */ >> @@ -225,7 +225,7 @@ >> /* Just return it */ >> return TrapFrame->HardwareSegSs; >> } >> - else if (TrapFrame->SegCs & MODE_MASK) >> + else if (KiUserTrap(TrapFrame)) >> { >> /* User mode, return the User SS */ >> return TrapFrame->HardwareSegSs | RPL_MASK; >> @@ -251,7 +251,7 @@ >> /* Just write it */ >> TrapFrame->HardwareSegSs = Ss; >> } >> - else if (TrapFrame->SegCs & MODE_MASK) >> + else if (KiUserTrap(TrapFrame)) >> { >> /* Usermode, save the User SS */ >> TrapFrame->HardwareSegSs = Ss | RPL_MASK; >> @@ -398,7 +398,7 @@ >> TrapFrame->V86Fs = Context->SegFs; >> TrapFrame->V86Gs = Context->SegGs; >> } >> - else if (!(TrapFrame->SegCs & MODE_MASK)) >> + else if (!KiUserTrap(TrapFrame)) >> { >> /* For kernel mode, write the standard values */ >> TrapFrame->SegDs = KGDT_R3_DATA | RPL_MASK; >> @@ -429,7 +429,7 @@ >> >> /* Handle the extended registers */ >> if (((ContextFlags & CONTEXT_EXTENDED_REGISTERS) == >> - CONTEXT_EXTENDED_REGISTERS) && (TrapFrame->SegCs & MODE_MASK)) >> + CONTEXT_EXTENDED_REGISTERS) && KiUserTrap(TrapFrame)) >> { >> /* Get the FX Area */ >> FxSaveArea = (PFX_SAVE_AREA)(TrapFrame + 1); >> @@ -463,7 +463,7 @@ >> >> /* Handle the floating point state */ >> if (((ContextFlags & CONTEXT_FLOATING_POINT) == >> - CONTEXT_FLOATING_POINT) && (TrapFrame->SegCs & MODE_MASK)) >> + CONTEXT_FLOATING_POINT) && KiUserTrap(TrapFrame)) >> { >> /* Get the FX Area */ >> FxSaveArea = (PFX_SAVE_AREA)(TrapFrame + 1); >> @@ -692,7 +692,7 @@ >> >> /* Handle extended registers */ >> if (((Context->ContextFlags & CONTEXT_EXTENDED_REGISTERS) == >> - CONTEXT_EXTENDED_REGISTERS) && (TrapFrame->SegCs & MODE_MASK)) >> + CONTEXT_EXTENDED_REGISTERS) && KiUserTrap(TrapFrame)) >> { >> /* Get the FX Save Area */ >> FxSaveArea = (PFX_SAVE_AREA)(TrapFrame + 1); >> @@ -712,7 +712,7 @@ >> >> /* Handle Floating Point */ >> if (((Context->ContextFlags & CONTEXT_FLOATING_POINT) == >> - CONTEXT_FLOATING_POINT) && (TrapFrame->SegCs & MODE_MASK)) >> + CONTEXT_FLOATING_POINT) && KiUserTrap(TrapFrame)) >> { >> /* Get the FX Save Area */ >> FxSaveArea = (PFX_SAVE_AREA)(TrapFrame + 1); >> @@ -1045,6 +1045,13 @@ >> } >> } >> _SEH2_END; >> + >> + DPRINT("First chance exception in %.16s, ExceptionCode: %lx, >> ExceptionAddress: %p, P0: %lx, P1: %lx\n", >> + PsGetCurrentProcess()->ImageFileName, >> + ExceptionRecord->ExceptionCode, >> + ExceptionRecord->ExceptionAddress, >> + ExceptionRecord->ExceptionInformation[0], >> + ExceptionRecord->ExceptionInformation[1]); >> } >> >> /* Try second chance */ >> @@ -1060,11 +1067,13 @@ >> } >> >> /* 3rd strike, kill the process */ >> - DPRINT1("Kill %.16s, ExceptionCode: %lx, ExceptionAddress: %p, >> BaseAddress: %p\n", >> + DPRINT1("Kill %.16s, ExceptionCode: %lx, ExceptionAddress: %p, >> BaseAddress: %p, P0: %lx, P1: %lx\n", >> PsGetCurrentProcess()->ImageFileName, >> ExceptionRecord->ExceptionCode, >> ExceptionRecord->ExceptionAddress, >> - PsGetCurrentProcess()->SectionBaseAddress); >> + PsGetCurrentProcess()->SectionBaseAddress, >> + ExceptionRecord->ExceptionInformation[0], >> + ExceptionRecord->ExceptionInformation[1]); >> >> ZwTerminateProcess(NtCurrentProcess(), >> ExceptionRecord->ExceptionCode); >> KeBugCheckEx(KMODE_EXCEPTION_NOT_HANDLED, >> >> Modified: trunk/reactos/ntoskrnl/ke/i386/patpge.c >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/patpge.c?rev=60701&r1=60700&r2=60701&view=diff >> ============================================================================== >> --- trunk/reactos/ntoskrnl/ke/i386/patpge.c [iso-8859-1] (original) >> +++ trunk/reactos/ntoskrnl/ke/i386/patpge.c [iso-8859-1] Sat Oct 19 >> 11:33:34 2013 >> @@ -20,9 +20,9 @@ >> ULONG_PTR >> NTAPI >> INIT_FUNCTION >> -Ki386EnableGlobalPage(IN volatile ULONG_PTR Context) >> -{ >> - volatile PLONG Count = (PLONG)Context; >> +Ki386EnableGlobalPage(IN ULONG_PTR Context) >> +{ >> + PLONG Count = (PLONG)Context; >> ULONG Cr4, Cr3; >> >> /* Disable interrupts */ >> @@ -144,7 +144,7 @@ >> if (PageTable) >> *PageTable = (PHARDWARE_PTE)(Pde->PageFrameNumber << >> PAGE_SHIFT); >> } >> - >> + >> return TRUE; >> } >> >> @@ -172,7 +172,7 @@ >> /* Get PTE of VirtualPtr, make it valid, and map PhysicalPtr there */ >> Pte = &PageTable[(VirtualPtr >> 12) & ((1 << PTE_BITS) - 1)]; >> Pte->Valid = 1; >> - Pte->PageFrameNumber = PhysicalPtr.QuadPart >> PAGE_SHIFT; >> + Pte->PageFrameNumber = (PFN_NUMBER)(PhysicalPtr.QuadPart >> PAGE_SHIFT); >> >> return TRUE; >> } >> @@ -189,7 +189,7 @@ >> PhysicalPtr = MmGetPhysicalAddress(VirtualPtr); >> >> /* Map its physical address in the page table provided by the caller */ >> - Pte->PageFrameNumber = PhysicalPtr.QuadPart >> PAGE_SHIFT; >> + Pte->PageFrameNumber = (PFN_NUMBER)(PhysicalPtr.QuadPart >> PAGE_SHIFT); >> } >> >> BOOLEAN >> >> Modified: trunk/reactos/ntoskrnl/ke/i386/trap.s >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/trap.s?rev=60701&r1=60700&r2=60701&view=diff >> ============================================================================== >> --- trunk/reactos/ntoskrnl/ke/i386/trap.s [iso-8859-1] (original) >> +++ trunk/reactos/ntoskrnl/ke/i386/trap.s [iso-8859-1] Sat Oct 19 >> 11:33:34 2013 >> @@ -166,6 +166,7 @@ >> KiTrapExitStub KiEditedTrapReturn, (KI_RESTORE_VOLATILES OR >> KI_RESTORE_EFLAGS OR KI_EDITED_FRAME OR KI_EXIT_RET) >> KiTrapExitStub KiTrapReturn, (KI_RESTORE_VOLATILES OR >> KI_RESTORE_SEGMENTS OR KI_EXIT_IRET) >> KiTrapExitStub KiTrapReturnNoSegments, (KI_RESTORE_VOLATILES OR >> KI_EXIT_IRET) >> +KiTrapExitStub KiTrapReturnNoSegmentsRet8,(KI_RESTORE_VOLATILES OR >> KI_RESTORE_EFLAGS OR KI_EXIT_RET8) >> >> #ifdef _MSC_VER >> EXTERN _PsConvertToGuiThread@0:PROC >> >> Modified: trunk/reactos/ntoskrnl/ke/i386/traphdlr.c >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/traphdlr.c?rev=60701&r1=60700&r2=60701&view=diff >> ============================================================================== >> --- trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] (original) >> +++ trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] Sat Oct 19 >> 11:33:34 2013 >> @@ -100,7 +100,7 @@ >> if (__builtin_expect(TrapFrame->Dr7 & ~DR7_RESERVED_MASK, 0)) >> { >> /* Check if the frame was from user mode or v86 mode */ >> - if ((TrapFrame->SegCs & MODE_MASK) || >> + if (KiUserTrap(TrapFrame) || >> (TrapFrame->EFlags & EFLAGS_V86_MASK)) >> { >> /* Handle debug registers */ >> @@ -124,13 +124,13 @@ >> if (TrapFrame->EFlags & EFLAGS_V86_MASK) >> KiTrapReturnNoSegments(TrapFrame); >> >> /* Check for user mode exit */ >> - if (TrapFrame->SegCs & MODE_MASK) KiTrapReturn(TrapFrame); >> + if (KiUserTrap(TrapFrame)) KiTrapReturn(TrapFrame); >> >> /* Check for edited frame */ >> if (KiIsFrameEdited(TrapFrame)) KiEditedTrapReturn(TrapFrame); >> >> /* Exit the trap to kernel mode */ >> - KiTrapReturnNoSegments(TrapFrame); >> + KiTrapReturnNoSegmentsRet8(TrapFrame); >> } >> >> DECLSPEC_NORETURN >> @@ -152,7 +152,7 @@ >> KeGetCurrentThread()->PreviousMode = >> (CCHAR)TrapFrame->PreviousPreviousMode; >> >> /* Check for user mode exit */ >> - if (TrapFrame->SegCs & MODE_MASK) >> + if (KiUserTrap(TrapFrame)) >> { >> /* Check if we were single stepping */ >> if (TrapFrame->EFlags & EFLAGS_TF) >> @@ -186,13 +186,13 @@ >> if (TrapFrame->EFlags & EFLAGS_V86_MASK) >> KiTrapReturnNoSegments(TrapFrame); >> >> /* Check for user mode exit */ >> - if (TrapFrame->SegCs & MODE_MASK) KiTrapReturn(TrapFrame); >> + if (KiUserTrap(TrapFrame)) KiTrapReturn(TrapFrame); >> >> /* Check for edited frame */ >> if (KiIsFrameEdited(TrapFrame)) KiEditedTrapReturn(TrapFrame); >> >> /* Exit the trap to kernel mode */ >> - KiTrapReturnNoSegments(TrapFrame); >> + KiTrapReturnNoSegmentsRet8(TrapFrame); >> } >> >> >> @@ -1250,10 +1250,10 @@ >> /* Call the access fault handler */ >> Status = MmAccessFault(TrapFrame->ErrCode & 1, >> (PVOID)Cr2, >> - TrapFrame->SegCs & MODE_MASK, >> + KiUserTrap(TrapFrame), >> TrapFrame); >> if (NT_SUCCESS(Status)) KiEoiHelper(TrapFrame); >> - >> + >> /* Check for syscall fault */ >> #if 0 >> if ((TrapFrame->Eip == (ULONG_PTR)CopyParams) || >> @@ -1541,7 +1541,7 @@ >> TrapFrame->Dr7 = 0; >> >> /* Check if the frame was from user mode */ >> - if (TrapFrame->SegCs & MODE_MASK) >> + if (KiUserTrap(TrapFrame)) >> { >> /* Check for active debugging */ >> if (KeGetCurrentThread()->Header.DebugActive & 0xFF) >> >> Modified: trunk/reactos/ntoskrnl/ke/i386/usercall.c >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/usercall.c?rev=60701&r1=60700&r2=60701&view=diff >> ============================================================================== >> --- trunk/reactos/ntoskrnl/ke/i386/usercall.c [iso-8859-1] (original) >> +++ trunk/reactos/ntoskrnl/ke/i386/usercall.c [iso-8859-1] Sat Oct 19 >> 11:33:34 2013 >> @@ -67,7 +67,7 @@ >> _SEH2_TRY >> { >> /* Sanity check */ >> - ASSERT((TrapFrame->SegCs & MODE_MASK) != KernelMode); >> + ASSERT(KiUserTrap(TrapFrame)); >> >> /* Get the aligned size */ >> AlignedEsp = Context.Esp & ~3; >> @@ -210,7 +210,7 @@ >> } >> >> >> -/* >> +/* >> * Stack layout for KiUserModeCallout: >> * ---------------------------------- >> * KCALLOUT_FRAME.ResultLength <= 2nd Parameter to KiCallUserMode >> >> Modified: trunk/reactos/ntoskrnl/ke/time.c >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/time.c?rev=60701&r1=60700&r2=60701&view=diff >> ============================================================================== >> --- trunk/reactos/ntoskrnl/ke/time.c [iso-8859-1] (original) >> +++ trunk/reactos/ntoskrnl/ke/time.c [iso-8859-1] Sat Oct 19 11:33:34 2013 >> @@ -153,7 +153,7 @@ >> >> /* Check if we came from user mode */ >> #ifndef _M_ARM >> - if ((TrapFrame->SegCs & MODE_MASK) || (TrapFrame->EFlags & >> EFLAGS_V86_MASK)) >> + if (KiUserTrap(TrapFrame) || (TrapFrame->EFlags & EFLAGS_V86_MASK)) >> #else >> if (TrapFrame->PreviousMode == UserMode) >> #endif >> >> > > > _______________________________________________ > Ros-dev mailing list > Ros-dev@reactos.org > http://www.reactos.org/mailman/listinfo/ros-dev _______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
