Browse Source

Return signal pending info from syscall C handler.

This change removes the code from the system call assembly that checked
the current thread signal pending state. This code was brittle as it
involved reaching through the PROCESSOR_BLOCK and KTHREAD structures, and
defining an enum (ThreadSignalPending). Now a boolean is returned from the
C system call handler, which already had the current thread, and that is
checked in the assembly instead. It also makes the logic into a consistent
(SignalPending != NoSignalPending), which was inconsistent in the
assembly (sometimes being SignalPending == ThreadSignalPending).
Evan Green 6 years ago
parent
commit
b9bb096a26

+ 0 - 32
include/minoca/kernel/arm.inc

@@ -164,12 +164,6 @@ Environment:
 #define EXCEPTION_STACK_R0   0
 #define EXCEPTION_STACK_CPSR 4
 
-//
-// Processor block offsets.
-//
-
-#define PROCESSOR_BLOCK_RUNNING_THREAD 0x14
-
 //
 // Define the offsets for members of the TRAP_FRAME structure.
 //
@@ -208,18 +202,6 @@ Environment:
 
 #define PROCESSOR_CONTEXT_SIZE 192
 
-//
-// Thread structure offsets.
-//
-
-#define THREAD_SIGNAL_PENDING 84
-
-//
-// Define the thread signal pending state options.
-//
-
-#define ThreadSignalPending 2
-
 //
 // Define the instructions for DSB/ISB, which are different in ARMv6 vs ARMv7.
 //
@@ -485,17 +467,3 @@ Environment:
 
 .endm
 
-//
-// This macro can be used to get the current thread out of the processor block.
-// It is used by the system call assembly routines and purposely avoids R0.
-//
-
-.macro ARM_GET_CURRENT_THREAD
-    mrs     %r2, CPSR               @ Get the original status register.
-    cpsid   i                       @ Disable interrupts.
-    mrc     p15, 0, %r1, c13, c0, 4 @ Get TPIDRPRW.
-    ldr     %r1, [%r1, #PROCESSOR_BLOCK_RUNNING_THREAD] @ Read at offset.
-    msr     CPSR, %r2               @ Restore interrupts.
-
-.endm
-

+ 6 - 1
include/minoca/kernel/ke.h

@@ -3526,7 +3526,8 @@ INTN
 KeSystemCallHandler (
     ULONG SystemCallNumber,
     PVOID SystemCallParameter,
-    PTRAP_FRAME TrapFrame
+    PTRAP_FRAME TrapFrame,
+    PUINTN SignalPending
     );
 
 /*++
@@ -3548,6 +3549,10 @@ Arguments:
     TrapFrame - Supplies a pointer to the trap frame generated by this jump
         from user mode to kernel mode.
 
+    SignalPending - Supplies a pointer where a boolean will be returned
+        indicating if a signal is pending on the current thread or process that
+        needs to be dispatched.
+
 Return Value:
 
     STATUS_SUCCESS or positive integer on success.

+ 0 - 13
include/minoca/kernel/x64.inc

@@ -62,19 +62,6 @@ Environment:
 //
 
 #define PROCESSOR_BLOCK_TSS 0x10
-#define PROCESSOR_BLOCK_RUNNING_THREAD 0x20
-
-//
-// Thread structure offsets.
-//
-
-#define THREAD_SIGNAL_PENDING 0x94
-
-//
-// Define the thread signal pending state options.
-//
-
-#define ThreadSignalPending 2
 
 //
 // Definition for the TRAP_FRAME structure and the exception stack directly

+ 1 - 13
include/minoca/kernel/x86.inc

@@ -63,7 +63,6 @@ Environment:
 
 #define PROCESSOR_BLOCK_TSS 0x0C
 #define PROCESSOR_BLOCK_GDT 0x10
-#define PROCESSOR_BLOCK_RUNNING_THREAD 0x14
 
 //
 // Definition for the TRAP_FRAME structure and the exception stack directly
@@ -103,18 +102,6 @@ Environment:
 #define PROCESSOR_CONTEXT_SIZE 0x60
 #define SIGNAL_CONTEXT_SIZE 32
 
-//
-// Thread structure offsets.
-//
-
-#define THREAD_SIGNAL_PENDING 84
-
-//
-// Define the thread signal pending state options.
-//
-
-#define ThreadSignalPending 2
-
 //
 // Define the minimum and maximum external interrupt vectors.
 //
@@ -263,3 +250,4 @@ Environment:
 #define CFI_OFFSET(_Register, _Offset) .cfi_offset _Register, _Offset
 #define CFI_UNDEFINED(_Register) .cfi_undefined _Register
 #define CFI_SAME_VALUE(_Register) .cfi_same_value _Register
+

+ 20 - 16
kernel/armv7/trap.S

@@ -212,8 +212,8 @@ Return Value:
 
 FUNCTION ArpSoftwareInterruptEntry
     srsdb   %sp!, #ARM_MODE_SVC                 @ Push lr and spsr.
-    sub     %sp, #(TRAP_FRAME_SIZE - 8)         @ Make space for rest of frame.
-    mov     %r2, %sp                            @ Get stack/trap frame param.
+    sub     %sp, #(TRAP_FRAME_SIZE - 8 + 4)     @ Make space for rest of frame.
+    add     %r2, %sp, #4                        @ Get stack/trap frame param.
     cps     #ARM_MODE_SYSTEM                    @ Switch to system mode.
     str     %lr, [%r2, #TRAP_USERLR]            @ Save usermode LR.
     str     %sp, [%r2, #TRAP_USERSP]            @ Save usermode SP.
@@ -227,11 +227,11 @@ FUNCTION ArpSoftwareInterruptEntry
     str     %r0, [%r2, #TRAP_R2]
     str     %r1, [%r2, #TRAP_R1]
 
-    CFI_OFFSET(r1, TRAP_R1)
-    CFI_OFFSET(r0, TRAP_R2)
-    CFI_OFFSET(sp, TRAP_USERSP + 8)
-    CFI_OFFSET(lr, TRAP_USERLR + 8)
-    CFI_OFFSET(pc, TRAP_PC + 8)
+    CFI_OFFSET(r1, TRAP_R1 + 4)
+    CFI_OFFSET(r0, TRAP_R2 + 4)
+    CFI_OFFSET(sp, TRAP_USERSP + 4)
+    CFI_OFFSET(lr, TRAP_USERLR + 4)
+    CFI_OFFSET(pc, TRAP_PC + 4)
 
     //
     // Set the exception CPSR to something wild as a hint that this trap frame
@@ -248,22 +248,26 @@ FUNCTION ArpSoftwareInterruptEntry
     // user mode, and R2 == SP == trap frame.
     //
 
+    mov     %r3, %sp                            @ Set boolean pointer parameter.
     bl      KeSystemCallHandler                 @ Handle system call.
+    ldmia   %sp!, {%r1}                         @ Pop signal pending boolean.
+
+    CFI_OFFSET(r1, TRAP_R1)
+    CFI_OFFSET(r0, TRAP_R2)
+    CFI_OFFSET(sp, TRAP_USERSP)
+    CFI_OFFSET(lr, TRAP_USERLR)
+    CFI_OFFSET(pc, TRAP_PC)
 
     //
-    // Determine whether or not a signal is pending on the thread. Use the
-    // special macro to get the current thread so that R0 is not trashed. Then
-    // scrub the volatiles that will no longer be used so that user mode does
-    // not get any leaked kernel mode values.
+    // Determine whether or not a signal is pending on the thread. Go do the
+    // slow full save if there is one.
     //
 
-    ARM_GET_CURRENT_THREAD                      @ Get current thread in R1.
+    eor     %r12, %r12                          @ Scrub volatile R12.
+    tst     %r1, %r1                            @ See if r1 is FALSE.
+    bne     ArpSoftwareInterruptSignalCheck     @ Jump to dispatch signal.
     eor     %r2, %r2                            @ Scrub volatile R2.
     eor     %r3, %r3                            @ Scrub volatile R3.
-    eor     %r12, %r12                          @ Scrub volatile R12.
-    ldr     %r1, [%r1, #THREAD_SIGNAL_PENDING]  @ Load signal pending status.
-    cmp     %r1, #ThreadSignalPending           @ Compare to signal pending.
-    beq     ArpSoftwareInterruptSignalCheck     @ Jump to dispatch signal.
 
 ArpSoftwareInterruptRestore:
 

+ 13 - 1
kernel/ke/syscall.c

@@ -231,7 +231,8 @@ INTN
 KeSystemCallHandler (
     ULONG SystemCallNumber,
     PVOID SystemCallParameter,
-    PTRAP_FRAME TrapFrame
+    PTRAP_FRAME TrapFrame,
+    PUINTN SignalPending
     )
 
 /*++
@@ -253,6 +254,10 @@ Arguments:
     TrapFrame - Supplies a pointer to the trap frame generated by this jump
         from user mode to kernel mode.
 
+    SignalPending - Supplies a pointer where a boolean will be returned
+        indicating if a signal is pending on the current thread or process that
+        needs to be dispatched.
+
 Return Value:
 
     STATUS_SUCCESS or positive integer on success.
@@ -349,6 +354,13 @@ SystemCallHandlerEnd:
 
     PsCheckRuntimeTimers(Thread);
 
+    //
+    // Let the assembly know whether or not there's a signal pending so they
+    // don't have to do a bunch of complicated work.
+    //
+
+    *SignalPending = (Thread->SignalPending != ThreadNoSignalPending);
+
     //
     // Return to the previous thread state and cycle account.
     //

+ 18 - 14
kernel/x64/trap.S

@@ -435,13 +435,13 @@ FUNCTION(ArSyscallHandlerAsm)
     // know how to unwind from here.
     //
 
-    subq    $TRAP_FRAME_SIZE, %rsp  # Allocate space for a trap frame.
-    movq    %rcx, TRAP_RIP(%rsp)    # Save the user RIP, saved by syscall.
-    movq    %r11, TRAP_RFLAGS(%rsp) # Save RFLAGS, put there by syscall.
-    movq    %r10, TRAP_RSP(%rsp)    # Save the user RSP.
-    movq    %rdi, TRAP_RDI(%rsp)    # Save first parameter in case of restart.
-    movq    %rsi, TRAP_RSI(%rsp)    # Save second parameter in case of restart.
-    CFI_DEF_CFA(%rsp, 0)
+    subq    $(TRAP_FRAME_SIZE + 16), %rsp  # Allocate frame, bool (aligned).
+    movq    %rcx, TRAP_RIP+16(%rsp) # Save the user RIP, saved by syscall.
+    movq    %r11, TRAP_RFLAGS+16(%rsp) # Save RFLAGS, put there by syscall.
+    movq    %r10, TRAP_RSP+16(%rsp) # Save the user RSP.
+    movq    %rdi, TRAP_RDI+16(%rsp) # Save first parameter in case of restart.
+    movq    %rsi, TRAP_RSI+16(%rsp) # Save second parameter in case of restart.
+    CFI_DEF_CFA(%rsp, 16)
     CFI_OFFSET(%rip, TRAP_RIP)
     CFI_OFFSET(%rsp, TRAP_RSP)
     CFI_OFFSET(%rdi, TRAP_RDI)
@@ -452,20 +452,24 @@ FUNCTION(ArSyscallHandlerAsm)
     // 1) this is a user mode trap frame and 2) it's incomplete.
     //
 
-    movq    $USER_DS, TRAP_CS(%rsp)
-    movq    %rsp, %rdx              # Move fake trap frame to 3rd parameter.
+    movq    $USER_DS, TRAP_CS+16(%rsp)
+    movq    %rsp, %rcx              # Move boolean to 4th parameter.
+    leaq    16(%rsp), %rdx          # Move trap frame pointer to 3rd parameter.
     call    KeSystemCallHandler     # Call out to the handler.
 
     //
-    // If a signal is pending, then save the full trap frame and attempt to
+    // See if there is a signal pending, as reported by the signal handler
+    // function. If there is, then save the full trap frame and attempt to
     // dispatch the signal. Note that the trap frame was allocated on the
     // kernel stack and may be copied out to user mode. Don't leak kernel stack
     // values.
     //
 
-    movq    %gs:(PROCESSOR_BLOCK_RUNNING_THREAD), %rcx # Get current thread.
-    cmpl    $ThreadSignalPending, THREAD_SIGNAL_PENDING(%rcx) # Signal test.
-    jne     ArSyscallHandlerFastRestore # Exit via the fast restore path.
+    popq    %rcx                    # Get the signal pending boolean.
+    addq    $8, %rsp                # Pop the remaining alignment.
+    CFI_ADJUST_CFA_OFFSET(-16)      # Tell the debugger about the pops.
+    testl   %ecx, %ecx              # See if zero.
+    jz      ArSyscallHandlerRestore # Exit quickly if no signal pending.
 
     //
     // See if the trap frame is already complete from being upgraded by the
@@ -510,7 +514,7 @@ ArSyscallSignalDispatch:
     movq    %rsp, %rdi              # Set the trap frame as the first parameter.
     call    PsApplyPendingSignalsOrRestart # Dispatch signals or restart.
 
-ArSyscallHandlerFastRestore:
+ArSyscallHandlerRestore:
     cmpl    $USER_DS, TRAP_CS(%rsp) # Compare CS hint against non-full value.
     jne     ArSyscallFullRestore    # Go back and do a full restoration maybe.
 

+ 20 - 17
kernel/x86/trap.S

@@ -560,23 +560,26 @@ ArSystemCallHandlerAfterTrapSave:
     // necessary.
     //
 
+    subl    $4, %esp                # Make room for the boolean.
+    pushl   %esp                    # Push the boolean pointer parameter.
     pushl   %ebx                    # Push a pointer to the trap frame.
     pushl   %edx                    # Push system call parameter.
     pushl   %ecx                    # Push system call number.
-    CFI_ADJUST_CFA_OFFSET(12)       # Tell the debugger.
+    CFI_ADJUST_CFA_OFFSET(20)       # Tell the debugger.
     call    KeSystemCallHandler     # Call the main exception handler.
-    addl    $0xC, %esp              # Pop the parameters.
-    CFI_ADJUST_CFA_OFFSET(-12)      # Tell the debugger.
+    addl    $0x10, %esp             # Pop the parameters.
+    CFI_ADJUST_CFA_OFFSET(-16)      # Tell the debugger.
     movl    %eax, TRAP_EAX(%ebx)    # Save the return value in the trap frame.
 
     //
-    // If a signal is pending, then go attempt to dispatch it as the trap frame
-    // is already complete.
+    // See if there is a signal pending on the current thread, as returned by
+    // the system call handler.
     //
 
-    movl    %fs:(PROCESSOR_BLOCK_RUNNING_THREAD), %ecx # Get current thread.
-    cmpl    $ThreadSignalPending, THREAD_SIGNAL_PENDING(%ecx) # Signal test.
-    jnz     ArSystemCallHandlerExit
+    popl    %ecx                    # Get the signal pending boolean
+    CFI_ADJUST_CFA_OFFSET(-4)       # Tell the debugger.
+    testl   %ecx, %ecx              # See if it's zero.
+    jz      ArSystemCallHandlerExit # Skip the expensive signal stuff.
 
     //
     // Push the trap frame as the first parameter. The system call number and
@@ -671,22 +674,22 @@ FUNCTION(ArSysenterHandlerAsm)
     // Push the parameters for the system call handler and execute it.
     //
 
+    subl    $4, %esp                # Make room for the boolean.
+    pushl   %esp                    # Push the boolean pointer parameter.
     pushl   %ebx                    # Push a pointer to the sort-of trap frame.
     pushl   %edx                    # Push EDX, the system call parameter.
-    pushl   %ecx                    # Push saved ECX, the system call number.
+    pushl   %ecx                    # Push ECX, the system call number.
     call    KeSystemCallHandler     # Call out to the main service handler.
-    addl    $0xC, %esp              # Pop the system call parameter.
+    addl    $0x10, %esp             # Pop the function parameters.
 
     //
-    // If a signal is pending, then save the full trap frame and attempt to
-    // dispatch the signal. Note that the trap frame was allocated on the
-    // kernel stack and may be copied out to user mode. Don't leak kernel stack
-    // values.
+    // See if there is a signal pending on the current thread, as returned by
+    // the system call handler.
     //
 
-    movl    %fs:(PROCESSOR_BLOCK_RUNNING_THREAD), %ecx # Get current thread.
-    cmpl    $ThreadSignalPending, THREAD_SIGNAL_PENDING(%ecx) # Signal test.
-    jnz     ArSysenterHandlerRestore # Skip the expensive signal stuff.
+    popl    %ecx                    # Get the signal pending boolean
+    testl   %ecx, %ecx              # See if it's zero.
+    jz      ArSysenterHandlerRestore    # Skip the expensive signal stuff.
 
     //
     // See if the trap frame is already complete, as a result perhaps of the