• Guo Hui's avatar
    arm64: syscall: unmask DAIF for tracing status · 1da185fc
    Guo Hui authored
    The following code:
    static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
                                const syscall_fn_t syscall_table[])
    {
        ...
    
        if (!has_syscall_work(flags) && !IS_ENABLED(CONFIG_DEBUG_RSEQ)) {
            local_daif_mask();
            flags = read_thread_flags(); -------------------------------- A
            if (!has_syscall_work(flags) && !(flags & _TIF_SINGLESTEP))
                 return;
            local_daif_restore(DAIF_PROCCTX);
         }
    
    trace_exit:
         syscall_trace_exit(regs); ------- B
    }
    
    1. The flags in the if conditional statement should be used
    in the same way as the flags in the function syscall_trace_exit,
    because DAIF is not shielded in the function syscall_trace_exit,
    so when the flags are obtained in line A of the code,
    there is no need to shield DAIF.
    Don't care about the modification of flags after line A.
    
    2. Masking DAIF caused syscall performance to deteriorate by about 10%.
    The Unixbench single core syscall performance data is as follows:
    
    Machine: Kunpeng 920
    
    Mask DAIF:
    System Call Overhead                    1172314.1 lps   (10.0 s, 7 samples)
    
    System Benchmarks Partial Index          BASELINE       RESULT    INDEX
    System Call Overhead                      15000.0    1172314.1    781.5
                                                                   ========
    System Benchmarks Index Score (Partial Only)                      781.5
    
    Unmask DAIF:
    System Call Overhead                    1287944.6 lps   (10.0 s, 7 samples)
    
    System Benchmarks Partial Index          BASELINE       RESULT    INDEX
    System Call Overhead                      15000.0    1287944.6    858.6
                                                                   ========
    System Benchmarks Index Score (Partial Only)                      858.6
    
    Rationale from Mark Rutland as for why this is safe:
    
    This masking is an artifact of the old "ret_fast_syscall" assembly that
    was converted to C in commit:
    
      f37099b6 ("arm64: convert syscall trace logic to C")
    
    The assembly would mask DAIF, check the thread flags, and fall through
    to kernel_exit without unmasking if no tracing was needed. The
    conversion copied this masking into the C version, though this wasn't
    strictly necessary.
    
    As (in general) thread flags can be manipulated by other threads, it's
    not safe to manipulate the thread flags with plain reads and writes, and
    since commit:
    
      342b3808 ("arm64: Snapshot thread flags")
    
    ... we use read_thread_flags() to read the flags atomically.
    
    With this, there is no need to mask DAIF transiently around reading the
    flags, as we only decide whether to trace while DAIF is masked, and the
    actual tracing occurs with DAIF unmasked. When el0_svc_common() returns
    its caller will unconditionally mask DAIF via exit_to_user_mode(), so
    the masking is redundant.
    Signed-off-by: default avatarGuo Hui <guohui@uniontech.com>
    Reviewed-by: default avatarMark Rutland <mark.rutland@arm.com>
    Link: https://lore.kernel.org/r/20230526024715.8773-1-guohui@uniontech.com
    [catalin.marinas@arm.com: updated comment with Mark's rationale]
    Signed-off-by: default avatarCatalin Marinas <catalin.marinas@arm.com>
    1da185fc
syscall.c 5.67 KB