* [PATCH v3 1/3] x86/fred: Allow variable-sized event frame
2025-03-19 7:10 [PATCH v3 0/3] x86: Allow variable-sized event frame Xin Li (Intel)
@ 2025-03-19 7:10 ` Xin Li (Intel)
2025-03-21 5:37 ` [PATCH v3A " Xin Li (Intel)
2025-03-19 7:10 ` [PATCH v3 2/3] x86: Remove the padding space at top of the init stack Xin Li (Intel)
2025-03-19 7:10 ` [PATCH v3 3/3] x86: Zap TOP_OF_KERNEL_STACK_PADDING on x86_64 Xin Li (Intel)
2 siblings, 1 reply; 9+ messages in thread
From: Xin Li (Intel) @ 2025-03-19 7:10 UTC (permalink / raw)
To: linux-kernel
Cc: luto, tglx, mingo, bp, dave.hansen, x86, hpa, peterz, brgerst
A FRED event frame could contain different amount of information for
different event types, or perhaps even for different instances of the
same event type. Thus the size of an event frame pushed by a FRED CPU
is not fixed and the address of the pt_regs structure that is used to
save a user level context of current task is not at a fixed offset
from top of current task kernel stack.
Add a new field named 'user_pt_regs' in the thread_info structure to
save the address of user level context pt_regs structure, thus to
eliminate the need of any advance information of event frame size
and allow a FRED CPU to push variable-sized event frame.
For IDT user level event delivery, a pt_regs structure is pushed by
hardware and software _always_ at a fixed offset from top of current
task kernel stack, so simply initialize user_pt_regs to point to the
pt_regs structure no matter whether one is pushed or not.
While for FRED user level event delivery, user_pt_regs is updated with
a pt_regs structure pointer generated in asm_fred_entrypoint_user().
Suggested-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Signed-off-by: Xin Li (Intel) <xin@zytor.com>
---
Change in v3:
* Replace "(struct pt_regs *)TOP_OF_INIT_STACK - 1" with
(struct pt_regs *)__top_init_kernel_stack (Brian Gerst).
---
arch/x86/entry/entry_fred.c | 10 ++++++++++
arch/x86/include/asm/processor.h | 12 ++++++------
arch/x86/include/asm/thread_info.h | 9 ++++++---
arch/x86/kernel/process.c | 22 ++++++++++++++++++++++
include/linux/thread_info.h | 1 +
kernel/fork.c | 6 ++++++
6 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
index f004a4dc74c2..a5f5bdd16ad8 100644
--- a/arch/x86/entry/entry_fred.c
+++ b/arch/x86/entry/entry_fred.c
@@ -228,6 +228,16 @@ __visible noinstr void fred_entry_from_user(struct pt_regs *regs)
/* Invalidate orig_ax so that syscall_get_nr() works correctly */
regs->orig_ax = -1;
+ /*
+ * A FRED event frame could contain different amount of information
+ * for different event types, or perhaps even for different instances
+ * of the same event type. Thus the size of an event frame pushed by
+ * a FRED CPU is not fixed and the address of the pt_regs structure
+ * that is used to save a user level context of current task is not
+ * at a fixed offset from top of current task stack.
+ */
+ current->thread_info.user_pt_regs = regs;
+
switch (regs->fred_ss.type) {
case EVENT_TYPE_EXTINT:
return fred_extint(regs);
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 5d2f7e5aff26..7ff3443eb57d 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -646,12 +646,12 @@ static __always_inline void prefetchw(const void *x)
#define task_top_of_stack(task) ((unsigned long)(task_pt_regs(task) + 1))
-#define task_pt_regs(task) \
-({ \
- unsigned long __ptr = (unsigned long)task_stack_page(task); \
- __ptr += THREAD_SIZE - TOP_OF_KERNEL_STACK_PADDING; \
- ((struct pt_regs *)__ptr) - 1; \
-})
+/*
+ * Note, this can't be converted to an inline function as this header
+ * file defines 'struct thread_struct' which is used in the task_struct
+ * structure definition.
+ */
+#define task_pt_regs(task) ((task)->thread_info.user_pt_regs)
#ifdef CONFIG_X86_32
#define INIT_THREAD { \
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 9282465eea21..915a6839bd61 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -56,6 +56,7 @@
*/
#ifndef __ASSEMBLER__
struct task_struct;
+struct pt_regs;
#include <asm/cpufeature.h>
#include <linux/atomic.h>
@@ -66,11 +67,13 @@ struct thread_info {
#ifdef CONFIG_SMP
u32 cpu; /* current CPU */
#endif
+ struct pt_regs *user_pt_regs;
};
-#define INIT_THREAD_INFO(tsk) \
-{ \
- .flags = 0, \
+#define INIT_THREAD_INFO(tsk) \
+{ \
+ .flags = 0, \
+ .user_pt_regs = (struct pt_regs *)__top_init_kernel_stack, \
}
#else /* !__ASSEMBLER__ */
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 91f6ff618852..58c1cd4ca60a 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -108,6 +108,28 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
return 0;
}
+/*
+ * Initialize thread_info.user_pt_regs for IDT event delivery.
+ *
+ * For IDT user level event delivery, a pt_regs structure is pushed by both
+ * hardware and software and always resides at a fixed offset from top of
+ * current task kernel stack, thus thread_info.user_pt_regs is a per-task
+ * constant and NEVER changes after initialization.
+ *
+ * While for FRED user level event delivery, user_pt_regs is updated in
+ * fred_entry_from_user() immediately after user level event delivery.
+ *
+ * Note: thread_info.user_pt_regs of the init task is initialized at build
+ * time.
+ */
+void arch_init_user_pt_regs(struct task_struct *tsk)
+{
+ unsigned long top_of_stack = (unsigned long)task_stack_page(tsk) + THREAD_SIZE;
+
+ top_of_stack -= TOP_OF_KERNEL_STACK_PADDING;
+ tsk->thread_info.user_pt_regs = (struct pt_regs *)top_of_stack - 1;
+}
+
#ifdef CONFIG_X86_64
void arch_release_task_struct(struct task_struct *tsk)
{
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index cf2446c9c30d..324667e0b8dd 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -273,6 +273,7 @@ void arch_task_cache_init(void); /* for CONFIG_SH */
void arch_release_task_struct(struct task_struct *tsk);
int arch_dup_task_struct(struct task_struct *dst,
struct task_struct *src);
+void arch_init_user_pt_regs(struct task_struct *tsk);
#endif /* __KERNEL__ */
diff --git a/kernel/fork.c b/kernel/fork.c
index 8695c345ca49..21af95645d84 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1105,6 +1105,10 @@ int __weak arch_dup_task_struct(struct task_struct *dst,
return 0;
}
+void __weak arch_init_user_pt_regs(struct task_struct *tsk)
+{
+}
+
void set_task_stack_end_magic(struct task_struct *tsk)
{
unsigned long *stackend;
@@ -1132,6 +1136,8 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
if (err)
goto free_tsk;
+ arch_init_user_pt_regs(tsk);
+
#ifdef CONFIG_THREAD_INFO_IN_TASK
refcount_set(&tsk->stack_refcount, 1);
#endif
--
2.48.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v3A 1/3] x86/fred: Allow variable-sized event frame
2025-03-19 7:10 ` [PATCH v3 1/3] x86/fred: " Xin Li (Intel)
@ 2025-03-21 5:37 ` Xin Li (Intel)
0 siblings, 0 replies; 9+ messages in thread
From: Xin Li (Intel) @ 2025-03-21 5:37 UTC (permalink / raw)
To: linux-kernel
Cc: luto, tglx, mingo, bp, dave.hansen, x86, hpa, peterz, brgerst
A FRED event frame could contain different amount of information for
different event types, or perhaps even for different instances of the
same event type. Thus the size of an event frame pushed by a FRED CPU
is not fixed and the address of the pt_regs structure that is used to
save a user level context of current task is not at a fixed offset
from top of current task kernel stack.
Add a new field named 'user_pt_regs' in the thread_info structure to
save the address of user level context pt_regs structure, thus to
eliminate the need of any advance information of event frame size
and allow a FRED CPU to push variable-sized event frame.
For IDT user level event delivery, a pt_regs structure is pushed by
hardware and software _always_ at a fixed offset from top of current
task kernel stack, so simply initialize user_pt_regs to point to the
pt_regs structure no matter whether one is pushed or not.
While for FRED user level event delivery, user_pt_regs is updated with
a pt_regs structure pointer generated in asm_fred_entrypoint_user().
Suggested-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Signed-off-by: Xin Li (Intel) <xin@zytor.com>
---
Change in v3A:
* Add declaration for __top_init_kernel_stack[] (Intel lkp).
Change in v3:
* Replace "(struct pt_regs *)TOP_OF_INIT_STACK - 1" with
(struct pt_regs *)__top_init_kernel_stack (Brian Gerst).
---
arch/x86/entry/entry_fred.c | 10 ++++++++++
arch/x86/include/asm/processor.h | 12 ++++++------
arch/x86/include/asm/thread_info.h | 11 ++++++++---
arch/x86/kernel/process.c | 22 ++++++++++++++++++++++
include/linux/thread_info.h | 1 +
kernel/fork.c | 6 ++++++
6 files changed, 53 insertions(+), 9 deletions(-)
diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
index f004a4dc74c2..a5f5bdd16ad8 100644
--- a/arch/x86/entry/entry_fred.c
+++ b/arch/x86/entry/entry_fred.c
@@ -228,6 +228,16 @@ __visible noinstr void fred_entry_from_user(struct pt_regs *regs)
/* Invalidate orig_ax so that syscall_get_nr() works correctly */
regs->orig_ax = -1;
+ /*
+ * A FRED event frame could contain different amount of information
+ * for different event types, or perhaps even for different instances
+ * of the same event type. Thus the size of an event frame pushed by
+ * a FRED CPU is not fixed and the address of the pt_regs structure
+ * that is used to save a user level context of current task is not
+ * at a fixed offset from top of current task stack.
+ */
+ current->thread_info.user_pt_regs = regs;
+
switch (regs->fred_ss.type) {
case EVENT_TYPE_EXTINT:
return fred_extint(regs);
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 5d2f7e5aff26..7ff3443eb57d 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -646,12 +646,12 @@ static __always_inline void prefetchw(const void *x)
#define task_top_of_stack(task) ((unsigned long)(task_pt_regs(task) + 1))
-#define task_pt_regs(task) \
-({ \
- unsigned long __ptr = (unsigned long)task_stack_page(task); \
- __ptr += THREAD_SIZE - TOP_OF_KERNEL_STACK_PADDING; \
- ((struct pt_regs *)__ptr) - 1; \
-})
+/*
+ * Note, this can't be converted to an inline function as this header
+ * file defines 'struct thread_struct' which is used in the task_struct
+ * structure definition.
+ */
+#define task_pt_regs(task) ((task)->thread_info.user_pt_regs)
#ifdef CONFIG_X86_32
#define INIT_THREAD { \
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 9282465eea21..07c6a6a92c65 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -56,6 +56,7 @@
*/
#ifndef __ASSEMBLER__
struct task_struct;
+struct pt_regs;
#include <asm/cpufeature.h>
#include <linux/atomic.h>
@@ -66,11 +67,15 @@ struct thread_info {
#ifdef CONFIG_SMP
u32 cpu; /* current CPU */
#endif
+ struct pt_regs *user_pt_regs;
};
-#define INIT_THREAD_INFO(tsk) \
-{ \
- .flags = 0, \
+extern unsigned long __top_init_kernel_stack[];
+
+#define INIT_THREAD_INFO(tsk) \
+{ \
+ .flags = 0, \
+ .user_pt_regs = (struct pt_regs *)__top_init_kernel_stack, \
}
#else /* !__ASSEMBLER__ */
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 91f6ff618852..58c1cd4ca60a 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -108,6 +108,28 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
return 0;
}
+/*
+ * Initialize thread_info.user_pt_regs for IDT event delivery.
+ *
+ * For IDT user level event delivery, a pt_regs structure is pushed by both
+ * hardware and software and always resides at a fixed offset from top of
+ * current task kernel stack, thus thread_info.user_pt_regs is a per-task
+ * constant and NEVER changes after initialization.
+ *
+ * While for FRED user level event delivery, user_pt_regs is updated in
+ * fred_entry_from_user() immediately after user level event delivery.
+ *
+ * Note: thread_info.user_pt_regs of the init task is initialized at build
+ * time.
+ */
+void arch_init_user_pt_regs(struct task_struct *tsk)
+{
+ unsigned long top_of_stack = (unsigned long)task_stack_page(tsk) + THREAD_SIZE;
+
+ top_of_stack -= TOP_OF_KERNEL_STACK_PADDING;
+ tsk->thread_info.user_pt_regs = (struct pt_regs *)top_of_stack - 1;
+}
+
#ifdef CONFIG_X86_64
void arch_release_task_struct(struct task_struct *tsk)
{
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index cf2446c9c30d..324667e0b8dd 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -273,6 +273,7 @@ void arch_task_cache_init(void); /* for CONFIG_SH */
void arch_release_task_struct(struct task_struct *tsk);
int arch_dup_task_struct(struct task_struct *dst,
struct task_struct *src);
+void arch_init_user_pt_regs(struct task_struct *tsk);
#endif /* __KERNEL__ */
diff --git a/kernel/fork.c b/kernel/fork.c
index e27fe5d5a15c..2b0367368afb 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1101,6 +1101,10 @@ int __weak arch_dup_task_struct(struct task_struct *dst,
return 0;
}
+void __weak arch_init_user_pt_regs(struct task_struct *tsk)
+{
+}
+
void set_task_stack_end_magic(struct task_struct *tsk)
{
unsigned long *stackend;
@@ -1128,6 +1132,8 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
if (err)
goto free_tsk;
+ arch_init_user_pt_regs(tsk);
+
#ifdef CONFIG_THREAD_INFO_IN_TASK
refcount_set(&tsk->stack_refcount, 1);
#endif
--
2.48.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] x86: Remove the padding space at top of the init stack
2025-03-19 7:10 [PATCH v3 0/3] x86: Allow variable-sized event frame Xin Li (Intel)
2025-03-19 7:10 ` [PATCH v3 1/3] x86/fred: " Xin Li (Intel)
@ 2025-03-19 7:10 ` Xin Li (Intel)
2025-03-19 7:10 ` [PATCH v3 3/3] x86: Zap TOP_OF_KERNEL_STACK_PADDING on x86_64 Xin Li (Intel)
2 siblings, 0 replies; 9+ messages in thread
From: Xin Li (Intel) @ 2025-03-19 7:10 UTC (permalink / raw)
To: linux-kernel
Cc: luto, tglx, mingo, bp, dave.hansen, x86, hpa, peterz, brgerst
Because the owner of the init stack, init task, doesn't have any user
level context, there will NEVER be an actual pt_regs structure pushed
at top of the init stack.
However a zeroed pt_regs structure is created at build time and kept
at top of the init stack for task_pt_regs() to function properly with
the init task in the same manner as a normal task with user level
context.
Besides, task_pt_regs() no longer converts a fixed offset from top of
a task kernel stack to a pt_regs structure pointer, but rather returns
whatever in the thread_info.user_pt_regs field, which is initialized
at build time to '(struct pt_regs *)TOP_OF_INIT_STACK - 1' for the
init task.
As a result, there is no point to reserve any padding space at top of
the init stack, so remove the padding space.
Signed-off-by: Xin Li (Intel) <xin@zytor.com>
---
arch/x86/include/asm/processor.h | 16 ++++++++++++++--
arch/x86/kernel/vmlinux.lds.S | 7 +++++--
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 7ff3443eb57d..a88ddf5614f2 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -641,8 +641,20 @@ static __always_inline void prefetchw(const void *x)
"m" (*(const char *)x));
}
-#define TOP_OF_INIT_STACK ((unsigned long)&init_stack + sizeof(init_stack) - \
- TOP_OF_KERNEL_STACK_PADDING)
+extern unsigned long __end_init_stack[];
+
+/*
+ * No need to reserve extra padding space above the pt_regs structure
+ * at top of the init stack, because its owner init task doesn't have
+ * any user level context, thus there will NEVER be an actual pt_regs
+ * structure pushed at top of the init stack.
+ *
+ * However a zeroed pt_regs structure is created at build time and kept
+ * at top of the init stack for task_pt_regs() to function properly with
+ * the init task in the same manner as a normal task with user level
+ * context.
+ */
+#define TOP_OF_INIT_STACK ((unsigned long)&__end_init_stack)
#define task_top_of_stack(task) ((unsigned long)(task_pt_regs(task) + 1))
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index ccdc45e5b759..b2df61293a20 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -178,8 +178,11 @@ SECTIONS
/* init_task */
INIT_TASK_DATA(THREAD_SIZE)
- /* equivalent to task_pt_regs(&init_task) */
- __top_init_kernel_stack = __end_init_stack - TOP_OF_KERNEL_STACK_PADDING - PTREGS_SIZE;
+ /*
+ * task_pt_regs(&init_task) is:
+ * '(struct pt_regs *)&__end_init_stack - 1'
+ */
+ __top_init_kernel_stack = __end_init_stack - PTREGS_SIZE;
#ifdef CONFIG_X86_32
/* 32 bit has nosave before _edata */
--
2.48.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] x86: Zap TOP_OF_KERNEL_STACK_PADDING on x86_64
2025-03-19 7:10 [PATCH v3 0/3] x86: Allow variable-sized event frame Xin Li (Intel)
2025-03-19 7:10 ` [PATCH v3 1/3] x86/fred: " Xin Li (Intel)
2025-03-19 7:10 ` [PATCH v3 2/3] x86: Remove the padding space at top of the init stack Xin Li (Intel)
@ 2025-03-19 7:10 ` Xin Li (Intel)
2025-03-19 19:17 ` Brian Gerst
2 siblings, 1 reply; 9+ messages in thread
From: Xin Li (Intel) @ 2025-03-19 7:10 UTC (permalink / raw)
To: linux-kernel
Cc: luto, tglx, mingo, bp, dave.hansen, x86, hpa, peterz, brgerst
Because task_pt_regs() is now just an alias of thread_info.user_pt_regs,
and no matter whether FRED is enabled or not a user level event frame on
x86_64 is always pushed from top of current task kernel stack, i.e.,
'(unsigned long)task_stack_page(task) + THREAD_SIZE', there is no meaning
to keep TOP_OF_KERNEL_STACK_PADDING on x86_64, thus remove it.
Signed-off-by: Xin Li (Intel) <xin@zytor.com>
---
Change in v2:
* Rebase on latest tip/master.
---
arch/x86/include/asm/fred.h | 2 +-
arch/x86/include/asm/processor.h | 6 ++++--
arch/x86/include/asm/thread_info.h | 10 ----------
arch/x86/kernel/process.c | 3 +--
4 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/arch/x86/include/asm/fred.h b/arch/x86/include/asm/fred.h
index 2a29e5216881..f9cca8c2c73e 100644
--- a/arch/x86/include/asm/fred.h
+++ b/arch/x86/include/asm/fred.h
@@ -97,7 +97,7 @@ static __always_inline void fred_sync_rsp0(unsigned long rsp0)
static __always_inline void fred_update_rsp0(void)
{
- unsigned long rsp0 = (unsigned long) task_stack_page(current) + THREAD_SIZE;
+ unsigned long rsp0 = task_top_of_stack(current);
if (cpu_feature_enabled(X86_FEATURE_FRED) && (__this_cpu_read(fred_rsp0) != rsp0)) {
wrmsrns(MSR_IA32_FRED_RSP0, rsp0);
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index a88ddf5614f2..3b7adefe05ab 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -656,8 +656,6 @@ extern unsigned long __end_init_stack[];
*/
#define TOP_OF_INIT_STACK ((unsigned long)&__end_init_stack)
-#define task_top_of_stack(task) ((unsigned long)(task_pt_regs(task) + 1))
-
/*
* Note, this can't be converted to an inline function as this header
* file defines 'struct thread_struct' which is used in the task_struct
@@ -666,6 +664,9 @@ extern unsigned long __end_init_stack[];
#define task_pt_regs(task) ((task)->thread_info.user_pt_regs)
#ifdef CONFIG_X86_32
+#define task_top_of_stack(task) ((unsigned long)task_stack_page(task) + THREAD_SIZE \
+ - TOP_OF_KERNEL_STACK_PADDING)
+
#define INIT_THREAD { \
.sp0 = TOP_OF_INIT_STACK, \
.sysenter_cs = __KERNEL_CS, \
@@ -673,6 +674,7 @@ extern unsigned long __end_init_stack[];
#else
extern unsigned long __top_init_kernel_stack[];
+#define task_top_of_stack(task) ((unsigned long)task_stack_page(task) + THREAD_SIZE)
#define INIT_THREAD { \
.sp = (unsigned long)&__top_init_kernel_stack, \
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 915a6839bd61..d8ccca04b4d9 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -30,10 +30,6 @@
*
* In vm86 mode, the hardware frame is much longer still, so add 16
* bytes to make room for the real-mode segments.
- *
- * x86-64 has a fixed-length stack frame, but it depends on whether
- * or not FRED is enabled. Future versions of FRED might make this
- * dynamic, but for now it is always 2 words longer.
*/
#ifdef CONFIG_X86_32
# ifdef CONFIG_VM86
@@ -41,12 +37,6 @@
# else
# define TOP_OF_KERNEL_STACK_PADDING 8
# endif
-#else /* x86-64 */
-# ifdef CONFIG_X86_FRED
-# define TOP_OF_KERNEL_STACK_PADDING (2 * 8)
-# else
-# define TOP_OF_KERNEL_STACK_PADDING 0
-# endif
#endif
/*
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 58c1cd4ca60a..51020caac332 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -124,9 +124,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
*/
void arch_init_user_pt_regs(struct task_struct *tsk)
{
- unsigned long top_of_stack = (unsigned long)task_stack_page(tsk) + THREAD_SIZE;
+ unsigned long top_of_stack = task_top_of_stack(tsk);
- top_of_stack -= TOP_OF_KERNEL_STACK_PADDING;
tsk->thread_info.user_pt_regs = (struct pt_regs *)top_of_stack - 1;
}
--
2.48.1
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 3/3] x86: Zap TOP_OF_KERNEL_STACK_PADDING on x86_64
2025-03-19 7:10 ` [PATCH v3 3/3] x86: Zap TOP_OF_KERNEL_STACK_PADDING on x86_64 Xin Li (Intel)
@ 2025-03-19 19:17 ` Brian Gerst
2025-03-21 5:47 ` Xin Li
0 siblings, 1 reply; 9+ messages in thread
From: Brian Gerst @ 2025-03-19 19:17 UTC (permalink / raw)
To: Xin Li (Intel)
Cc: linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, hpa, peterz
On Wed, Mar 19, 2025 at 3:10 AM Xin Li (Intel) <xin@zytor.com> wrote:
>
> Because task_pt_regs() is now just an alias of thread_info.user_pt_regs,
> and no matter whether FRED is enabled or not a user level event frame on
> x86_64 is always pushed from top of current task kernel stack, i.e.,
> '(unsigned long)task_stack_page(task) + THREAD_SIZE', there is no meaning
> to keep TOP_OF_KERNEL_STACK_PADDING on x86_64, thus remove it.
>
> Signed-off-by: Xin Li (Intel) <xin@zytor.com>
> ---
>
> Change in v2:
> * Rebase on latest tip/master.
> ---
> arch/x86/include/asm/fred.h | 2 +-
> arch/x86/include/asm/processor.h | 6 ++++--
> arch/x86/include/asm/thread_info.h | 10 ----------
> arch/x86/kernel/process.c | 3 +--
> 4 files changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/include/asm/fred.h b/arch/x86/include/asm/fred.h
> index 2a29e5216881..f9cca8c2c73e 100644
> --- a/arch/x86/include/asm/fred.h
> +++ b/arch/x86/include/asm/fred.h
> @@ -97,7 +97,7 @@ static __always_inline void fred_sync_rsp0(unsigned long rsp0)
>
> static __always_inline void fred_update_rsp0(void)
> {
> - unsigned long rsp0 = (unsigned long) task_stack_page(current) + THREAD_SIZE;
> + unsigned long rsp0 = task_top_of_stack(current);
>
> if (cpu_feature_enabled(X86_FEATURE_FRED) && (__this_cpu_read(fred_rsp0) != rsp0)) {
> wrmsrns(MSR_IA32_FRED_RSP0, rsp0);
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index a88ddf5614f2..3b7adefe05ab 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -656,8 +656,6 @@ extern unsigned long __end_init_stack[];
> */
> #define TOP_OF_INIT_STACK ((unsigned long)&__end_init_stack)
>
> -#define task_top_of_stack(task) ((unsigned long)(task_pt_regs(task) + 1))
> -
> /*
> * Note, this can't be converted to an inline function as this header
> * file defines 'struct thread_struct' which is used in the task_struct
> @@ -666,6 +664,9 @@ extern unsigned long __end_init_stack[];
> #define task_pt_regs(task) ((task)->thread_info.user_pt_regs)
>
> #ifdef CONFIG_X86_32
> +#define task_top_of_stack(task) ((unsigned long)task_stack_page(task) + THREAD_SIZE \
> + - TOP_OF_KERNEL_STACK_PADDING)
> +
> #define INIT_THREAD { \
> .sp0 = TOP_OF_INIT_STACK, \
> .sysenter_cs = __KERNEL_CS, \
> @@ -673,6 +674,7 @@ extern unsigned long __end_init_stack[];
>
> #else
> extern unsigned long __top_init_kernel_stack[];
> +#define task_top_of_stack(task) ((unsigned long)task_stack_page(task) + THREAD_SIZE)
>
> #define INIT_THREAD { \
> .sp = (unsigned long)&__top_init_kernel_stack, \
> diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
> index 915a6839bd61..d8ccca04b4d9 100644
> --- a/arch/x86/include/asm/thread_info.h
> +++ b/arch/x86/include/asm/thread_info.h
> @@ -30,10 +30,6 @@
> *
> * In vm86 mode, the hardware frame is much longer still, so add 16
> * bytes to make room for the real-mode segments.
> - *
> - * x86-64 has a fixed-length stack frame, but it depends on whether
> - * or not FRED is enabled. Future versions of FRED might make this
> - * dynamic, but for now it is always 2 words longer.
> */
> #ifdef CONFIG_X86_32
> # ifdef CONFIG_VM86
> @@ -41,12 +37,6 @@
> # else
> # define TOP_OF_KERNEL_STACK_PADDING 8
> # endif
> -#else /* x86-64 */
> -# ifdef CONFIG_X86_FRED
> -# define TOP_OF_KERNEL_STACK_PADDING (2 * 8)
> -# else
> -# define TOP_OF_KERNEL_STACK_PADDING 0
> -# endif
> #endif
>
> /*
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 58c1cd4ca60a..51020caac332 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -124,9 +124,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
> */
> void arch_init_user_pt_regs(struct task_struct *tsk)
> {
> - unsigned long top_of_stack = (unsigned long)task_stack_page(tsk) + THREAD_SIZE;
> + unsigned long top_of_stack = task_top_of_stack(tsk);
>
> - top_of_stack -= TOP_OF_KERNEL_STACK_PADDING;
> tsk->thread_info.user_pt_regs = (struct pt_regs *)top_of_stack - 1;
> }
>
> --
> 2.48.1
>
I'm not sure it's worth fully removing TOP_OF_KERNEL_STACK_PADDING for
64-bit if it results in needing separate definitions of
task_top_of_stack(). Leaving it at zero is fine. The other changes
are fine though.
Brian Gerst
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 3/3] x86: Zap TOP_OF_KERNEL_STACK_PADDING on x86_64
2025-03-19 19:17 ` Brian Gerst
@ 2025-03-21 5:47 ` Xin Li
2025-03-21 8:14 ` H. Peter Anvin
0 siblings, 1 reply; 9+ messages in thread
From: Xin Li @ 2025-03-21 5:47 UTC (permalink / raw)
To: Brian Gerst
Cc: linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, hpa, peterz
On 3/19/2025 12:17 PM, Brian Gerst wrote:
> I'm not sure it's worth fully removing TOP_OF_KERNEL_STACK_PADDING for
> 64-bit if it results in needing separate definitions of
> task_top_of_stack(). Leaving it at zero is fine. The other changes
> are fine though.
Let's leave it to x86 maintainers ;-)
But to me, TOP_OF_KERNEL_STACK_PADDING no longer makes sense on 64-bit,
and it makes it simpler to remove it. On the other side, 32-bit is to
be zapped...
Thanks!
Xin
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 3/3] x86: Zap TOP_OF_KERNEL_STACK_PADDING on x86_64
2025-03-21 5:47 ` Xin Li
@ 2025-03-21 8:14 ` H. Peter Anvin
2025-05-12 6:13 ` Xin Li
0 siblings, 1 reply; 9+ messages in thread
From: H. Peter Anvin @ 2025-03-21 8:14 UTC (permalink / raw)
To: Xin Li, Brian Gerst
Cc: linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, peterz
On March 20, 2025 10:47:25 PM PDT, Xin Li <xin@zytor.com> wrote:
>On 3/19/2025 12:17 PM, Brian Gerst wrote:
>> I'm not sure it's worth fully removing TOP_OF_KERNEL_STACK_PADDING for
>> 64-bit if it results in needing separate definitions of
>> task_top_of_stack(). Leaving it at zero is fine. The other changes
>> are fine though.
>
>Let's leave it to x86 maintainers ;-)
>
>But to me, TOP_OF_KERNEL_STACK_PADDING no longer makes sense on 64-bit,
>and it makes it simpler to remove it. On the other side, 32-bit is to
>be zapped...
>
>Thanks!
> Xin
>
For what it's worth, it was there as 0 before the FRED changes, so ...
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] x86: Zap TOP_OF_KERNEL_STACK_PADDING on x86_64
2025-03-21 8:14 ` H. Peter Anvin
@ 2025-05-12 6:13 ` Xin Li
0 siblings, 0 replies; 9+ messages in thread
From: Xin Li @ 2025-05-12 6:13 UTC (permalink / raw)
To: H. Peter Anvin, Brian Gerst
Cc: linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, peterz
On 3/21/2025 1:14 AM, H. Peter Anvin wrote:
> On March 20, 2025 10:47:25 PM PDT, Xin Li <xin@zytor.com> wrote:
>> On 3/19/2025 12:17 PM, Brian Gerst wrote:
>>> I'm not sure it's worth fully removing TOP_OF_KERNEL_STACK_PADDING for
>>> 64-bit if it results in needing separate definitions of
>>> task_top_of_stack(). Leaving it at zero is fine. The other changes
>>> are fine though.
>>
>> Let's leave it to x86 maintainers ;-)
>>
>> But to me, TOP_OF_KERNEL_STACK_PADDING no longer makes sense on 64-bit,
>> and it makes it simpler to remove it. On the other side, 32-bit is to
>> be zapped...
>>
>> Thanks!
>> Xin
>>
>
> For what it's worth, it was there as 0 before the FRED changes, so ...
>
Okay, I will drop this patch and send the patch set as v4.
Thanks!
Xin
^ permalink raw reply [flat|nested] 9+ messages in thread