Kernel stack padding looks like obey MIPS o32 Calling Convention, as
LoongArch is inspired by MIPS and keep it. Remove it avoid not clear
code.
Link: https://lore.kernel.org/loongarch/[email protected]/
Signed-off-by: Jinyang He <[email protected]>
---
v2: Remove TOP_OF_KERNEL_STACK_PADDING
Remove 'init stack pointer' in head.S
arch/loongarch/include/asm/processor.h | 2 +-
arch/loongarch/include/asm/ptrace.h | 2 +-
arch/loongarch/kernel/head.S | 3 +--
arch/loongarch/kernel/process.c | 4 ++--
arch/loongarch/kernel/switch.S | 2 +-
5 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/arch/loongarch/include/asm/processor.h b/arch/loongarch/include/asm/processor.h
index 6954dc5d24e9..7184f1dc61f2 100644
--- a/arch/loongarch/include/asm/processor.h
+++ b/arch/loongarch/include/asm/processor.h
@@ -191,7 +191,7 @@ static inline void flush_thread(void)
unsigned long __get_wchan(struct task_struct *p);
#define __KSTK_TOS(tsk) ((unsigned long)task_stack_page(tsk) + \
- THREAD_SIZE - 32 - sizeof(struct pt_regs))
+ THREAD_SIZE - sizeof(struct pt_regs))
#define task_pt_regs(tsk) ((struct pt_regs *)__KSTK_TOS(tsk))
#define KSTK_EIP(tsk) (task_pt_regs(tsk)->csr_era)
#define KSTK_ESP(tsk) (task_pt_regs(tsk)->regs[3])
diff --git a/arch/loongarch/include/asm/ptrace.h b/arch/loongarch/include/asm/ptrace.h
index 7437b9366c3b..59c4608de91d 100644
--- a/arch/loongarch/include/asm/ptrace.h
+++ b/arch/loongarch/include/asm/ptrace.h
@@ -133,7 +133,7 @@ static inline void die_if_kernel(const char *str, struct pt_regs *regs)
#define current_pt_regs() \
({ \
unsigned long sp = (unsigned long)__builtin_frame_address(0); \
- (struct pt_regs *)((sp | (THREAD_SIZE - 1)) + 1 - 32) - 1; \
+ (struct pt_regs *)((sp | (THREAD_SIZE - 1)) + 1) - 1; \
})
/* Helpers for working with the user stack pointer */
diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S
index 97425779ce9f..84970e266658 100644
--- a/arch/loongarch/kernel/head.S
+++ b/arch/loongarch/kernel/head.S
@@ -84,10 +84,9 @@ SYM_CODE_START(kernel_entry) # kernel entry point
la.pcrel tp, init_thread_union
/* Set the SP after an empty pt_regs. */
- PTR_LI sp, (_THREAD_SIZE - 32 - PT_SIZE)
+ PTR_LI sp, (_THREAD_SIZE - PT_SIZE)
PTR_ADD sp, sp, tp
set_saved_sp sp, t0, t1
- PTR_ADDI sp, sp, -4 * SZREG # init stack pointer
bl start_kernel
ASM_BUG()
diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c
index 1256e3582475..2526b68f1c0f 100644
--- a/arch/loongarch/kernel/process.c
+++ b/arch/loongarch/kernel/process.c
@@ -129,7 +129,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
unsigned long clone_flags = args->flags;
struct pt_regs *childregs, *regs = current_pt_regs();
- childksp = (unsigned long)task_stack_page(p) + THREAD_SIZE - 32;
+ childksp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
/* set up new TSS. */
childregs = (struct pt_regs *) childksp - 1;
@@ -236,7 +236,7 @@ bool in_task_stack(unsigned long stack, struct task_struct *task,
struct stack_info *info)
{
unsigned long begin = (unsigned long)task_stack_page(task);
- unsigned long end = begin + THREAD_SIZE - 32;
+ unsigned long end = begin + THREAD_SIZE;
if (stack < begin || stack >= end)
return false;
diff --git a/arch/loongarch/kernel/switch.S b/arch/loongarch/kernel/switch.S
index 43ebbc3990f7..202a163cb32f 100644
--- a/arch/loongarch/kernel/switch.S
+++ b/arch/loongarch/kernel/switch.S
@@ -26,7 +26,7 @@ SYM_FUNC_START(__switch_to)
move tp, a2
cpu_restore_nonscratch a1
- li.w t0, _THREAD_SIZE - 32
+ li.w t0, _THREAD_SIZE
PTR_ADD t0, t0, tp
set_saved_sp t0, t1, t2
--
2.34.3
On 2022/10/26 11:55, Jinyang He wrote:
> Kernel stack padding looks like obey MIPS o32 Calling Convention, as
> LoongArch is inspired by MIPS and keep it. Remove it avoid not clear
> code.
Just some improvement to the commit message so it's clearer:
"The current LoongArch kernel stack is padded as if obeying the MIPS o32
calling convention, signifying the port's MIPS lineage but no longer
making sense. Remove the padding for clarity."
>
> Link: https://lore.kernel.org/loongarch/[email protected]/
>
I think this blank line between the "Link" and S-o-b tags could be removed.
> Signed-off-by: Jinyang He <[email protected]>
> ---
> v2: Remove TOP_OF_KERNEL_STACK_PADDING
> Remove 'init stack pointer' in head.S
>
> arch/loongarch/include/asm/processor.h | 2 +-
> arch/loongarch/include/asm/ptrace.h | 2 +-
> arch/loongarch/kernel/head.S | 3 +--
> arch/loongarch/kernel/process.c | 4 ++--
> arch/loongarch/kernel/switch.S | 2 +-
> 5 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/processor.h b/arch/loongarch/include/asm/processor.h
> index 6954dc5d24e9..7184f1dc61f2 100644
> --- a/arch/loongarch/include/asm/processor.h
> +++ b/arch/loongarch/include/asm/processor.h
> @@ -191,7 +191,7 @@ static inline void flush_thread(void)
> unsigned long __get_wchan(struct task_struct *p);
>
> #define __KSTK_TOS(tsk) ((unsigned long)task_stack_page(tsk) + \
> - THREAD_SIZE - 32 - sizeof(struct pt_regs))
> + THREAD_SIZE - sizeof(struct pt_regs))
> #define task_pt_regs(tsk) ((struct pt_regs *)__KSTK_TOS(tsk))
> #define KSTK_EIP(tsk) (task_pt_regs(tsk)->csr_era)
> #define KSTK_ESP(tsk) (task_pt_regs(tsk)->regs[3])
> diff --git a/arch/loongarch/include/asm/ptrace.h b/arch/loongarch/include/asm/ptrace.h
> index 7437b9366c3b..59c4608de91d 100644
> --- a/arch/loongarch/include/asm/ptrace.h
> +++ b/arch/loongarch/include/asm/ptrace.h
> @@ -133,7 +133,7 @@ static inline void die_if_kernel(const char *str, struct pt_regs *regs)
> #define current_pt_regs() \
> ({ \
> unsigned long sp = (unsigned long)__builtin_frame_address(0); \
> - (struct pt_regs *)((sp | (THREAD_SIZE - 1)) + 1 - 32) - 1; \
> + (struct pt_regs *)((sp | (THREAD_SIZE - 1)) + 1) - 1; \
> })
>
> /* Helpers for working with the user stack pointer */
> diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S
> index 97425779ce9f..84970e266658 100644
> --- a/arch/loongarch/kernel/head.S
> +++ b/arch/loongarch/kernel/head.S
> @@ -84,10 +84,9 @@ SYM_CODE_START(kernel_entry) # kernel entry point
>
> la.pcrel tp, init_thread_union
> /* Set the SP after an empty pt_regs. */
> - PTR_LI sp, (_THREAD_SIZE - 32 - PT_SIZE)
> + PTR_LI sp, (_THREAD_SIZE - PT_SIZE)
> PTR_ADD sp, sp, tp
> set_saved_sp sp, t0, t1
> - PTR_ADDI sp, sp, -4 * SZREG # init stack pointer
>
> bl start_kernel
> ASM_BUG()
> diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c
> index 1256e3582475..2526b68f1c0f 100644
> --- a/arch/loongarch/kernel/process.c
> +++ b/arch/loongarch/kernel/process.c
> @@ -129,7 +129,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
> unsigned long clone_flags = args->flags;
> struct pt_regs *childregs, *regs = current_pt_regs();
>
> - childksp = (unsigned long)task_stack_page(p) + THREAD_SIZE - 32;
> + childksp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
>
> /* set up new TSS. */
> childregs = (struct pt_regs *) childksp - 1;
> @@ -236,7 +236,7 @@ bool in_task_stack(unsigned long stack, struct task_struct *task,
> struct stack_info *info)
> {
> unsigned long begin = (unsigned long)task_stack_page(task);
> - unsigned long end = begin + THREAD_SIZE - 32;
> + unsigned long end = begin + THREAD_SIZE;
>
> if (stack < begin || stack >= end)
> return false;
> diff --git a/arch/loongarch/kernel/switch.S b/arch/loongarch/kernel/switch.S
> index 43ebbc3990f7..202a163cb32f 100644
> --- a/arch/loongarch/kernel/switch.S
> +++ b/arch/loongarch/kernel/switch.S
> @@ -26,7 +26,7 @@ SYM_FUNC_START(__switch_to)
> move tp, a2
> cpu_restore_nonscratch a1
>
> - li.w t0, _THREAD_SIZE - 32
> + li.w t0, _THREAD_SIZE
> PTR_ADD t0, t0, tp
> set_saved_sp t0, t1, t2
>
Otherwise LGTM, assuming you have tested it before sending (I don't
currently have the capacity to immediately test this for you)...
Reviewed-by: WANG Xuerui <[email protected]>
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
Hi, Xuerui,
在 2022/10/26 15:27, WANG Xuerui 写道:
> On 2022/10/26 11:55, Jinyang He wrote:
>> Kernel stack padding looks like obey MIPS o32 Calling Convention, as
>> LoongArch is inspired by MIPS and keep it. Remove it avoid not clear
>> code.
>
> Just some improvement to the commit message so it's clearer:
>
> "The current LoongArch kernel stack is padded as if obeying the MIPS
> o32 calling convention, signifying the port's MIPS lineage but no
> longer making sense. Remove the padding for clarity."
Thanks, I'll improve it and send it later.
>
>>
>> Link:
>> https://lore.kernel.org/loongarch/[email protected]/
>>
>
> I think this blank line between the "Link" and S-o-b tags could be
> removed.
Ok.
>
>> Signed-off-by: Jinyang He <[email protected]>
>> ---
>> v2: Remove TOP_OF_KERNEL_STACK_PADDING
>> Remove 'init stack pointer' in head.S
>>
>> arch/loongarch/include/asm/processor.h | 2 +-
>> arch/loongarch/include/asm/ptrace.h | 2 +-
>> arch/loongarch/kernel/head.S | 3 +--
>> arch/loongarch/kernel/process.c | 4 ++--
>> arch/loongarch/kernel/switch.S | 2 +-
>> 5 files changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/processor.h
>> b/arch/loongarch/include/asm/processor.h
>> index 6954dc5d24e9..7184f1dc61f2 100644
>> --- a/arch/loongarch/include/asm/processor.h
>> +++ b/arch/loongarch/include/asm/processor.h
>> @@ -191,7 +191,7 @@ static inline void flush_thread(void)
>> unsigned long __get_wchan(struct task_struct *p);
>> #define __KSTK_TOS(tsk) ((unsigned long)task_stack_page(tsk) + \
>> - THREAD_SIZE - 32 - sizeof(struct pt_regs))
>> + THREAD_SIZE - sizeof(struct pt_regs))
>> #define task_pt_regs(tsk) ((struct pt_regs *)__KSTK_TOS(tsk))
>> #define KSTK_EIP(tsk) (task_pt_regs(tsk)->csr_era)
>> #define KSTK_ESP(tsk) (task_pt_regs(tsk)->regs[3])
>> diff --git a/arch/loongarch/include/asm/ptrace.h
>> b/arch/loongarch/include/asm/ptrace.h
>> index 7437b9366c3b..59c4608de91d 100644
>> --- a/arch/loongarch/include/asm/ptrace.h
>> +++ b/arch/loongarch/include/asm/ptrace.h
>> @@ -133,7 +133,7 @@ static inline void die_if_kernel(const char *str,
>> struct pt_regs *regs)
>> #define current_pt_regs() \
>> ({ \
>> unsigned long sp = (unsigned long)__builtin_frame_address(0); \
>> - (struct pt_regs *)((sp | (THREAD_SIZE - 1)) + 1 - 32) - 1; \
>> + (struct pt_regs *)((sp | (THREAD_SIZE - 1)) + 1) - 1; \
>> })
>> /* Helpers for working with the user stack pointer */
>> diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S
>> index 97425779ce9f..84970e266658 100644
>> --- a/arch/loongarch/kernel/head.S
>> +++ b/arch/loongarch/kernel/head.S
>> @@ -84,10 +84,9 @@ SYM_CODE_START(kernel_entry) # kernel
>> entry point
>> la.pcrel tp, init_thread_union
>> /* Set the SP after an empty pt_regs. */
>> - PTR_LI sp, (_THREAD_SIZE - 32 - PT_SIZE)
>> + PTR_LI sp, (_THREAD_SIZE - PT_SIZE)
>> PTR_ADD sp, sp, tp
>> set_saved_sp sp, t0, t1
>> - PTR_ADDI sp, sp, -4 * SZREG # init stack pointer
>> bl start_kernel
>> ASM_BUG()
>> diff --git a/arch/loongarch/kernel/process.c
>> b/arch/loongarch/kernel/process.c
>> index 1256e3582475..2526b68f1c0f 100644
>> --- a/arch/loongarch/kernel/process.c
>> +++ b/arch/loongarch/kernel/process.c
>> @@ -129,7 +129,7 @@ int copy_thread(struct task_struct *p, const
>> struct kernel_clone_args *args)
>> unsigned long clone_flags = args->flags;
>> struct pt_regs *childregs, *regs = current_pt_regs();
>> - childksp = (unsigned long)task_stack_page(p) + THREAD_SIZE - 32;
>> + childksp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
>> /* set up new TSS. */
>> childregs = (struct pt_regs *) childksp - 1;
>> @@ -236,7 +236,7 @@ bool in_task_stack(unsigned long stack, struct
>> task_struct *task,
>> struct stack_info *info)
>> {
>> unsigned long begin = (unsigned long)task_stack_page(task);
>> - unsigned long end = begin + THREAD_SIZE - 32;
>> + unsigned long end = begin + THREAD_SIZE;
>> if (stack < begin || stack >= end)
>> return false;
>> diff --git a/arch/loongarch/kernel/switch.S
>> b/arch/loongarch/kernel/switch.S
>> index 43ebbc3990f7..202a163cb32f 100644
>> --- a/arch/loongarch/kernel/switch.S
>> +++ b/arch/loongarch/kernel/switch.S
>> @@ -26,7 +26,7 @@ SYM_FUNC_START(__switch_to)
>> move tp, a2
>> cpu_restore_nonscratch a1
>> - li.w t0, _THREAD_SIZE - 32
>> + li.w t0, _THREAD_SIZE
>> PTR_ADD t0, t0, tp
>> set_saved_sp t0, t1, t2
>
> Otherwise LGTM, assuming you have tested it before sending (I don't
> currently have the capacity to immediately test this for you)...
>
> Reviewed-by: WANG Xuerui <[email protected]>
Emm, to be honest, I didn't do tests like LTP. I ususally do development
on CLFS-LoongArch while kernel has this patch. And it works normally.
Thanks,
Jinyang
On 2022/10/26 21:24, Jinyang He wrote:
> Emm, to be honest, I didn't do tests like LTP. I ususally do development
> on CLFS-LoongArch while kernel has this patch. And it works normally.
For changes touching fundamental parts that make the system work at all,
like this, "it continues to boot and work" usually is enough for
confirming correctness. Although some harder test runs would certainly
be better ;-)
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/