arm64: syscall: unmask DAIF for tracing status

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:

  f37099b6992a0b81 ("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:

  342b3808786518ce ("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: Guo Hui <guohui@uniontech.com>
Reviewed-by: Mark 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: Catalin Marinas <catalin.marinas@arm.com>

diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
index da84cf855c4446a30057e2fd5db921c997ca106e..5a668d7f3c1f71a5d211a3ec247e05ee7c3927e9 100644 (file)
--- a/arch/arm64/kernel/syscall.c
+++ b/arch/arm64/kernel/syscall.c
@@ -147,11 +147,9 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
         * exit regardless, as the old entry assembly did.
         */
        if (!has_syscall_work(flags) && !IS_ENABLED(CONFIG_DEBUG_RSEQ)) {
-               local_daif_mask();
                flags = read_thread_flags();
                if (!has_syscall_work(flags) && !(flags & _TIF_SINGLESTEP))
                        return;
-               local_daif_restore(DAIF_PROCCTX);
        }
 
 trace_exit: