Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [RFC PATCH v2 2/2] LoongArch: No need to call RESTORE_ALL_AND_RET
 for all syscalls
To:     hev <r@hev.cc>
References: <1655275079-17651-1-git-send-email-yangtiezhu@loongson.cn>
 <1655275079-17651-3-git-send-email-yangtiezhu@loongson.cn>
 <CAHirt9g+0N6hJAf-xRrss-FRXiix4jE9=CGZC6+BDHO3dBunsg@mail.gmail.com>
Cc:     Huacai Chen <chenhuacai@kernel.org>,
        WANG Xuerui <kernel@xen0n.name>,
        Xuefeng Li <lixuefeng@loongson.cn>,
        Jianmin Lv <lvjianmin@loongson.cn>, Jun Yi <yijun@loongson.cn>,
        Rui Wang <wangrui@loongson.cn>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
From:   Tiezhu Yang <yangtiezhu@loongson.cn>
Message-ID: <b862fe85-97d8-000d-0d78-e749f4a29a2b@loongson.cn>
Date:   Wed, 15 Jun 2022 16:19:33 +0800
User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <CAHirt9g+0N6hJAf-xRrss-FRXiix4jE9=CGZC6+BDHO3dBunsg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 06/15/2022 02:57 PM, hev wrote:
> Hello,
>
> On Wed, Jun 15, 2022 at 2:38 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>
>> In handle_syscall, it is unnecessary to call RESTORE_ALL_AND_RET
>> for all syscalls.
>>
>> (1) If syscall number is __NR_clone and __NR_clone3,
>>     call RESTORE_STATIC_SOME_SP_AND_RET.
>> (2) If syscall number is __NR_rt_sigreturn and __NR_rt_sigsuspend,
>>     call RESTORE_TEMP_SOME_SP_AND_RET.
>> (3) The other syscalls call RESTORE_SOME_SP_AND_RET.
>>
>> With this patch, it can reduce many load instructions.
>>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>>  arch/loongarch/include/asm/stackframe.h | 17 +++++++++++++++++
>>  arch/loongarch/kernel/entry.S           | 24 +++++++++++++++++++++++-
>>  2 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/loongarch/include/asm/stackframe.h b/arch/loongarch/include/asm/stackframe.h
>> index 4ca9530..52649a5f 100644
>> --- a/arch/loongarch/include/asm/stackframe.h
>> +++ b/arch/loongarch/include/asm/stackframe.h
>> @@ -216,4 +216,21 @@
>>         RESTORE_SP_AND_RET \docfi
>>         .endm
>>
>> +       .macro  RESTORE_SOME_SP_AND_RET docfi=0
>> +       RESTORE_SOME \docfi
>> +       RESTORE_SP_AND_RET \docfi
>> +       .endm
>> +
>> +       .macro  RESTORE_STATIC_SOME_SP_AND_RET docfi=0
>> +       RESTORE_STATIC \docfi
>> +       RESTORE_SOME \docfi
>> +       RESTORE_SP_AND_RET \docfi
>> +       .endm
>> +
>> +       .macro  RESTORE_TEMP_SOME_SP_AND_RET docfi=0
>> +       RESTORE_TEMP \docfi
>> +       RESTORE_SOME \docfi
>> +       RESTORE_SP_AND_RET \docfi
>> +       .endm
>> +
>>  #endif /* _ASM_STACKFRAME_H */
>> diff --git a/arch/loongarch/kernel/entry.S b/arch/loongarch/kernel/entry.S
>> index 53ce2cb..58fe507 100644
>> --- a/arch/loongarch/kernel/entry.S
>> +++ b/arch/loongarch/kernel/entry.S
>> @@ -79,7 +79,29 @@ SYM_FUNC_START(handle_syscall)
>>         move    a0, sp
>>         bl      do_syscall
>>
>> -       RESTORE_ALL_AND_RET
>> +       /*
>> +        * Syscall number held in a7 which is stored in PT_R11.
>> +        * If syscall number is __NR_clone and __NR_clone3,
>> +        * call RESTORE_STATIC_SOME_SP_AND_RET.
>> +        * If syscall number is __NR_rt_sigreturn and __NR_rt_sigsuspend,
>> +        * call RESTORE_TEMP_SOME_SP_AND_RET.
>> +        * The other syscalls call RESTORE_SOME_SP_AND_RET.
>> +        */
>> +       cfi_ld  t3, PT_R11
>
> I think PT_R11 may be overwritten by the signal handler and the
> syscall number is now lost.
>

Thank you. Let me use TI_SYSCALL. I will send RFC v3 patch later.

arch/loongarch/include/asm/thread_info.h
struct thread_info {
	struct task_struct	*task;		/* main task structure */
	unsigned long		flags;		/* low level flags */
	unsigned long		tp_value;	/* thread pointer */
	__u32			cpu;		/* current CPU */
	int			preempt_count;	/* 0 => preemptible, <0 => BUG */
	struct pt_regs		*regs;
	unsigned long		syscall;	/* syscall number */
	unsigned long		syscall_work;	/* SYSCALL_WORK_ flags */
};

>> +       li.w    t4, __NR_clone
>> +       beq     t3, t4, 3f
>> +       li.w    t4, __NR_clone3
>> +       beq     t3, t4, 3f
>> +       li.w    t4, __NR_rt_sigreturn
>> +       beq     t3, t4, 4f
>> +       li.w    t4, __NR_rt_sigsuspend
>> +       beq     t3, t4, 4f
>> +
>> +       RESTORE_SOME_SP_AND_RET
>> +3:
>> +       RESTORE_STATIC_SOME_SP_AND_RET
>> +4:
>> +       RESTORE_TEMP_SOME_SP_AND_RET
>>  SYM_FUNC_END(handle_syscall)
>>
>>  SYM_CODE_START(ret_from_fork)
>> --
>> 2.1.0
>>
>
> Best regards,
> hev
>