* [PATCH v4 2/5] powerpc/signal: Include the new stack frame inside the user access block
From: Christophe Leroy @ 2021-09-14 14:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
ebiederm, hch
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1718f38859d5366f82d5bef531f255cedf537b5d.1631629700.git.christophe.leroy@csgroup.eu>
Include the new stack frame inside the user access block and set it up
using unsafe_put_user().
On an mpc 8321 (book3s/32) the improvment is about 4% on a process
sending a signal to itself.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/signal_32.c | 29 +++++++++++++----------------
arch/powerpc/kernel/signal_64.c | 14 +++++++-------
2 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 0608581967f0..ff101e2b3bab 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -726,7 +726,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
struct rt_sigframe __user *frame;
struct mcontext __user *mctx;
struct mcontext __user *tm_mctx = NULL;
- unsigned long newsp = 0;
+ unsigned long __user *newsp;
unsigned long tramp;
struct pt_regs *regs = tsk->thread.regs;
/* Save the thread's msr before get_tm_stackpointer() changes it */
@@ -734,6 +734,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
/* Set up Signal Frame */
frame = get_sigframe(ksig, tsk, sizeof(*frame), 1);
+ newsp = (unsigned long __user *)((unsigned long)frame - (__SIGNAL_FRAMESIZE + 16));
mctx = &frame->uc.uc_mcontext;
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
tm_mctx = &frame->uc_transact.uc_mcontext;
@@ -743,7 +744,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
else
prepare_save_user_regs(1);
- if (!user_access_begin(frame, sizeof(*frame)))
+ if (!user_access_begin(newsp, __SIGNAL_FRAMESIZE + 16 + sizeof(*frame)))
goto badframe;
/* Put the siginfo & fill in most of the ucontext */
@@ -779,6 +780,9 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
}
unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
+ /* create a stack frame for the caller of the handler */
+ unsafe_put_user(regs->gpr[1], newsp, failed);
+
user_access_end();
if (copy_siginfo_to_user(&frame->info, &ksig->info))
@@ -790,13 +794,8 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
tsk->thread.fp_state.fpscr = 0; /* turn off all fp exceptions */
#endif
- /* create a stack frame for the caller of the handler */
- newsp = ((unsigned long)frame) - (__SIGNAL_FRAMESIZE + 16);
- if (put_user(regs->gpr[1], (u32 __user *)newsp))
- goto badframe;
-
/* Fill registers for signal handler */
- regs->gpr[1] = newsp;
+ regs->gpr[1] = (unsigned long)newsp;
regs->gpr[3] = ksig->sig;
regs->gpr[4] = (unsigned long)&frame->info;
regs->gpr[5] = (unsigned long)&frame->uc;
@@ -826,7 +825,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
struct sigframe __user *frame;
struct mcontext __user *mctx;
struct mcontext __user *tm_mctx = NULL;
- unsigned long newsp = 0;
+ unsigned long __user *newsp;
unsigned long tramp;
struct pt_regs *regs = tsk->thread.regs;
/* Save the thread's msr before get_tm_stackpointer() changes it */
@@ -834,6 +833,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
/* Set up Signal Frame */
frame = get_sigframe(ksig, tsk, sizeof(*frame), 1);
+ newsp = (unsigned long __user *)((unsigned long)frame - __SIGNAL_FRAMESIZE);
mctx = &frame->mctx;
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
tm_mctx = &frame->mctx_transact;
@@ -843,7 +843,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
else
prepare_save_user_regs(1);
- if (!user_access_begin(frame, sizeof(*frame)))
+ if (!user_access_begin(newsp, __SIGNAL_FRAMESIZE + sizeof(*frame)))
goto badframe;
sc = (struct sigcontext __user *) &frame->sctx;
@@ -873,6 +873,8 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
unsafe_put_user(PPC_RAW_SC(), &mctx->mc_pad[1], failed);
asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
}
+ /* create a stack frame for the caller of the handler */
+ unsafe_put_user(regs->gpr[1], newsp, failed);
user_access_end();
regs->link = tramp;
@@ -881,12 +883,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
tsk->thread.fp_state.fpscr = 0; /* turn off all fp exceptions */
#endif
- /* create a stack frame for the caller of the handler */
- newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
- if (put_user(regs->gpr[1], (u32 __user *)newsp))
- goto badframe;
-
- regs->gpr[1] = newsp;
+ regs->gpr[1] = (unsigned long)newsp;
regs->gpr[3] = ksig->sig;
regs->gpr[4] = (unsigned long) sc;
regs_set_return_ip(regs, (unsigned long) ksig->ka.sa.sa_handler);
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 7b1cd50bc4fb..d80ff83cacb9 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -847,13 +847,14 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
struct task_struct *tsk)
{
struct rt_sigframe __user *frame;
- unsigned long newsp = 0;
+ unsigned long __user *newsp;
long err = 0;
struct pt_regs *regs = tsk->thread.regs;
/* Save the thread's msr before get_tm_stackpointer() changes it */
unsigned long msr = regs->msr;
frame = get_sigframe(ksig, tsk, sizeof(*frame), 0);
+ newsp = (unsigned long __user *)((unsigned long)frame - __SIGNAL_FRAMESIZE);
/*
* This only applies when calling unsafe_setup_sigcontext() and must be
@@ -862,7 +863,7 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
if (!MSR_TM_ACTIVE(msr))
prepare_setup_sigcontext(tsk);
- if (!user_write_access_begin(frame, sizeof(*frame)))
+ if (!user_write_access_begin(newsp, __SIGNAL_FRAMESIZE + sizeof(*frame)))
goto badframe;
unsafe_put_user(&frame->info, &frame->pinfo, badframe_block);
@@ -900,6 +901,9 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
}
unsafe_copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set), badframe_block);
+ /* Allocate a dummy caller frame for the signal handler. */
+ unsafe_put_user(regs->gpr[1], newsp, badframe_block);
+
user_write_access_end();
/* Save the siginfo outside of the unsafe block. */
@@ -919,10 +923,6 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
regs_set_return_ip(regs, (unsigned long) &frame->tramp[0]);
}
- /* Allocate a dummy caller frame for the signal handler. */
- newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
- err |= put_user(regs->gpr[1], (unsigned long __user *)newsp);
-
/* Set up "regs" so we "return" to the signal handler. */
if (is_elf2_task()) {
regs->ctr = (unsigned long) ksig->ka.sa.sa_handler;
@@ -947,7 +947,7 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
/* enter the signal handler in native-endian mode */
regs_set_return_msr(regs, (regs->msr & ~MSR_LE) | (MSR_KERNEL & MSR_LE));
- regs->gpr[1] = newsp;
+ regs->gpr[1] = (unsigned long)newsp;
regs->gpr[3] = ksig->sig;
regs->result = 0;
if (ksig->ka.sa.sa_flags & SA_SIGINFO) {
--
2.31.1
^ permalink raw reply related
* [PATCH v4 3/5] signal: Add unsafe_copy_siginfo_to_user()
From: Christophe Leroy @ 2021-09-14 14:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
ebiederm, hch
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1718f38859d5366f82d5bef531f255cedf537b5d.1631629700.git.christophe.leroy@csgroup.eu>
In the same spirit as commit fb05121fd6a2 ("signal: Add
unsafe_get_compat_sigset()"), implement an 'unsafe' version of
copy_siginfo_to_user() in order to use it within user access blocks.
For that, also add an 'unsafe' version of clear_user().
This commit adds the generic fallback for unsafe_clear_user().
Architectures wanting to use unsafe_copy_siginfo_to_user() within a
user_access_begin() section have to make sure they have their
own unsafe_clear_user().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v3: Added precision about unsafe_clear_user() in commit message.
---
include/linux/signal.h | 15 +++++++++++++++
include/linux/uaccess.h | 1 +
kernel/signal.c | 5 -----
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 3f96a6374e4f..70ea7e741427 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -35,6 +35,21 @@ static inline void copy_siginfo_to_external(siginfo_t *to,
int copy_siginfo_to_user(siginfo_t __user *to, const kernel_siginfo_t *from);
int copy_siginfo_from_user(kernel_siginfo_t *to, const siginfo_t __user *from);
+static __always_inline char __user *si_expansion(const siginfo_t __user *info)
+{
+ return ((char __user *)info) + sizeof(struct kernel_siginfo);
+}
+
+#define unsafe_copy_siginfo_to_user(to, from, label) do { \
+ siginfo_t __user *__ucs_to = to; \
+ const kernel_siginfo_t *__ucs_from = from; \
+ char __user *__ucs_expansion = si_expansion(__ucs_to); \
+ \
+ unsafe_copy_to_user(__ucs_to, __ucs_from, \
+ sizeof(struct kernel_siginfo), label); \
+ unsafe_clear_user(__ucs_expansion, SI_EXPANSION_SIZE, label); \
+} while (0)
+
enum siginfo_layout {
SIL_KILL,
SIL_TIMER,
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index c05e903cef02..37073caac474 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -398,6 +398,7 @@ long strnlen_user_nofault(const void __user *unsafe_addr, long count);
#define unsafe_put_user(x,p,e) unsafe_op_wrap(__put_user(x,p),e)
#define unsafe_copy_to_user(d,s,l,e) unsafe_op_wrap(__copy_to_user(d,s,l),e)
#define unsafe_copy_from_user(d,s,l,e) unsafe_op_wrap(__copy_from_user(d,s,l),e)
+#define unsafe_clear_user(d, l, e) unsafe_op_wrap(__clear_user(d, l), e)
static inline unsigned long user_access_save(void) { return 0UL; }
static inline void user_access_restore(unsigned long flags) { }
#endif
diff --git a/kernel/signal.c b/kernel/signal.c
index 952741f6d0f9..23f168730b7e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3324,11 +3324,6 @@ enum siginfo_layout siginfo_layout(unsigned sig, int si_code)
return layout;
}
-static inline char __user *si_expansion(const siginfo_t __user *info)
-{
- return ((char __user *)info) + sizeof(struct kernel_siginfo);
-}
-
int copy_siginfo_to_user(siginfo_t __user *to, const kernel_siginfo_t *from)
{
char __user *expansion = si_expansion(to);
--
2.31.1
^ permalink raw reply related
* [PATCH v4 5/5] powerpc/signal: Use unsafe_copy_siginfo_to_user()
From: Christophe Leroy @ 2021-09-14 14:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
ebiederm, hch
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1718f38859d5366f82d5bef531f255cedf537b5d.1631629700.git.christophe.leroy@csgroup.eu>
Use unsafe_copy_siginfo_to_user() in order to do the copy
within the user access block.
On an mpc 8321 (book3s/32) the improvment is about 5% on a process
sending a signal to itself.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v4: Use another approach for compat: drop the unsafe_copy_siginfo_to_user32(), instead directly call copy_siginfo_to_external32() before user_access_begin()
v3: Don't leave compat aside, use the new unsafe_copy_siginfo_to_user32()
---
arch/powerpc/kernel/signal_32.c | 17 ++++++++---------
arch/powerpc/kernel/signal_64.c | 5 +----
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index ff101e2b3bab..f1f5dde0885f 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -710,12 +710,6 @@ static long restore_tm_user_regs(struct pt_regs *regs, struct mcontext __user *s
}
#endif
-#ifdef CONFIG_PPC64
-
-#define copy_siginfo_to_user copy_siginfo_to_user32
-
-#endif /* CONFIG_PPC64 */
-
/*
* Set up a signal frame for a "real-time" signal handler
* (one which gets siginfo).
@@ -731,6 +725,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
struct pt_regs *regs = tsk->thread.regs;
/* Save the thread's msr before get_tm_stackpointer() changes it */
unsigned long msr = regs->msr;
+ compat_siginfo_t uinfo;
/* Set up Signal Frame */
frame = get_sigframe(ksig, tsk, sizeof(*frame), 1);
@@ -744,6 +739,9 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
else
prepare_save_user_regs(1);
+ if (IS_ENABLED(CONFIG_COMPAT))
+ copy_siginfo_to_external32(&uinfo, &ksig->info);
+
if (!user_access_begin(newsp, __SIGNAL_FRAMESIZE + 16 + sizeof(*frame)))
goto badframe;
@@ -779,15 +777,16 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
}
unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
+ if (IS_ENABLED(CONFIG_COMPAT))
+ unsafe_copy_to_user(&frame->info, &uinfo, sizeof(frame->info), failed);
+ else
+ unsafe_copy_siginfo_to_user((void *)&frame->info, &ksig->info, failed);
/* create a stack frame for the caller of the handler */
unsafe_put_user(regs->gpr[1], newsp, failed);
user_access_end();
- if (copy_siginfo_to_user(&frame->info, &ksig->info))
- goto badframe;
-
regs->link = tramp;
#ifdef CONFIG_PPC_FPU_REGS
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index d80ff83cacb9..56c0c74aa28c 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -901,15 +901,12 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
}
unsafe_copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set), badframe_block);
+ unsafe_copy_siginfo_to_user(&frame->info, &ksig->info, badframe_block);
/* Allocate a dummy caller frame for the signal handler. */
unsafe_put_user(regs->gpr[1], newsp, badframe_block);
user_write_access_end();
- /* Save the siginfo outside of the unsafe block. */
- if (copy_siginfo_to_user(&frame->info, &ksig->info))
- goto badframe;
-
/* Make sure signal handler doesn't get spurious FP exceptions */
tsk->thread.fp_state.fpscr = 0;
--
2.31.1
^ permalink raw reply related
* [PATCH v4 4/5] powerpc/uaccess: Add unsafe_clear_user()
From: Christophe Leroy @ 2021-09-14 14:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
ebiederm, hch
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1718f38859d5366f82d5bef531f255cedf537b5d.1631629700.git.christophe.leroy@csgroup.eu>
Implement unsafe_clear_user() for powerpc.
It's a copy/paste of unsafe_copy_to_user() with value 0 as source.
It may be improved in a later patch by using 'dcbz' instruction
to zeroize full cache lines at once.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 22c79ab40006..962b675485ff 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -467,6 +467,26 @@ do { \
unsafe_put_user(*(u8*)(_src + _i), (u8 __user *)(_dst + _i), e); \
} while (0)
+#define unsafe_clear_user(d, l, e) \
+do { \
+ u8 __user *_dst = (u8 __user *)(d); \
+ size_t _len = (l); \
+ int _i; \
+ \
+ for (_i = 0; _i < (_len & ~(sizeof(u64) - 1)); _i += sizeof(u64)) \
+ unsafe_put_user(0, (u64 __user *)(_dst + _i), e); \
+ if (_len & 4) { \
+ unsafe_put_user(0, (u32 __user *)(_dst + _i), e); \
+ _i += 4; \
+ } \
+ if (_len & 2) { \
+ unsafe_put_user(0, (u16 __user *)(_dst + _i), e); \
+ _i += 2; \
+ } \
+ if (_len & 1) \
+ unsafe_put_user(0, (u8 __user *)(_dst + _i), e); \
+} while (0)
+
#define HAVE_GET_KERNEL_NOFAULT
#define __get_kernel_nofault(dst, src, type, err_label) \
--
2.31.1
^ permalink raw reply related
* [PATCH v4 1/5] powerpc/signal64: Access function descriptor with user access block
From: Christophe Leroy @ 2021-09-14 14:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
ebiederm, hch
Cc: linuxppc-dev, linux-kernel
Access the function descriptor of the handler within a
user access block.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v3: Flatten the change to avoid nested gotos.
---
arch/powerpc/kernel/signal_64.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 1831bba0582e..7b1cd50bc4fb 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -936,8 +936,13 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
func_descr_t __user *funct_desc_ptr =
(func_descr_t __user *) ksig->ka.sa.sa_handler;
- err |= get_user(regs->ctr, &funct_desc_ptr->entry);
- err |= get_user(regs->gpr[2], &funct_desc_ptr->toc);
+ if (!user_read_access_begin(funct_desc_ptr, sizeof(func_descr_t)))
+ goto badfunc;
+
+ unsafe_get_user(regs->ctr, &funct_desc_ptr->entry, badfunc_block);
+ unsafe_get_user(regs->gpr[2], &funct_desc_ptr->toc, badfunc_block);
+
+ user_read_access_end();
}
/* enter the signal handler in native-endian mode */
@@ -962,5 +967,12 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
badframe:
signal_fault(current, regs, "handle_rt_signal64", frame);
+ return 1;
+
+badfunc_block:
+ user_read_access_end();
+badfunc:
+ signal_fault(current, regs, __func__, (void __user *)ksig->ka.sa.sa_handler);
+
return 1;
}
--
2.31.1
^ permalink raw reply related
* Re: [PATCH RESEND v3 6/6] powerpc/signal: Use unsafe_copy_siginfo_to_user()
From: Christophe Leroy @ 2021-09-14 14:00 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: linux-kernel, hch, Paul Mackerras, linuxppc-dev
In-Reply-To: <87ilz4mgts.fsf@disp2133>
Le 13/09/2021 à 21:11, Eric W. Biederman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>
>> Le 13/09/2021 à 18:21, Eric W. Biederman a écrit :
>>> ebiederm@xmission.com (Eric W. Biederman) writes:
>>>
>>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>>>
>>>>> Use unsafe_copy_siginfo_to_user() in order to do the copy
>>>>> within the user access block.
>>>>>
>>>>> On an mpc 8321 (book3s/32) the improvment is about 5% on a process
>>>>> sending a signal to itself.
>>>
>>> If you can't make function calls from an unsafe macro there is another
>>> way to handle this that doesn't require everything to be inline.
>>>
>>> From a safety perspective it is probably even a better approach.
>>
>> Yes but that's exactly what I wanted to avoid for the native ppc32 case: this
>> double hop means useless pressure on the cache. The siginfo_t structure is 128
>> bytes large, that means 8 lines of cache on powerpc 8xx.
>>
>> But maybe it is acceptable to do that only for the compat case. Let me think
>> about it, it might be quite easy.
>
> The places get_signal is called tend to be well known. So I think we
> are safe from a capacity standpoint.
>
> I am not certain it makes a difference in capacity as there is a high
> probability that the stack was deeper recently than it is now which
> suggests the cache blocks might already be in the cache.
>
> My sense it is worth benchmarking before optimizing out the extra copy
> like that.
>
> On the extreme side there is simply building the entire sigframe on the
> stack and then just calling it copy_to_user. As the stack cache lines
> are likely to be hot, and copy_to_user is quite well optimized
> there is a real possibility that it is faster to build everything
> on the kernel stack, and then copy it to the user space stack.
>
> It is also possible that I am wrong and we may want to figure out how
> far up we can push the conversion to the 32bit siginfo format.
>
> If could move the work into collect_signal we could guarantee there
> would be no extra work. That would require adjusting the sigframe
> generation code on all of the architectures.
>
> There is a lot we can do but we need benchmarking to tell if it is
> worth it.
>
Sure, I'm benchmarking all the work I have been doing on signal code
with the following simple app that I run with 'perf stat':
#include <stdlib.h>
#include <signal.h>
void sigusr1(int sig) { }
int main(int argc, char **argv)
{
int i = 100000;
signal(SIGUSR1, sigusr1);
for (;i--;)
raise(SIGUSR1);
exit(0);
}
On an mpc8321 a 32 bits powerpc with KUAP enabled (KUAP is equivalent of
x86 SMAP)
Before changing copy_siginfo_to_user() to unsafe_copy_siginfo_to_user(),
'perf stat' reports 1983 msec (task-clock)
After my change I get 1900 msec.
With your approach I get 1930 msec, so we are loosing 36% of the benefit
of converting to the 'unsafe_' alternative.
So I think it is worth it.
Christophe
^ permalink raw reply
* Re: [RFC PATCH 0/8] Move task_struct::cpu back into thread_info
From: Mark Rutland @ 2021-09-14 13:55 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
linux-s390, Arnd Bergmann, Russell King, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, Albert Ou, Kees Cook, Vasily Gorbik,
Heiko Carstens, Keith Packard, Borislav Petkov, Andy Lutomirski,
Paul Walmsley, Thomas Gleixner, linux-arm-kernel, linuxppc-dev,
linux-kernel, Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-1-ardb@kernel.org>
On Tue, Sep 14, 2021 at 02:10:28PM +0200, Ard Biesheuvel wrote:
> Commit c65eacbe290b ("sched/core: Allow putting thread_info into
> task_struct") mentions that, along with moving thread_info into
> task_struct, the cpu field is moved out of the former into the latter,
> but does not explain why.
From what I recall of talking to Andy around that time, when converting
arm64 over, the theory was that over time we'd move more and more out of
thread_info and into task_struct or thread_struct, until task_struct
supplanted thread_info entirely, and that all became generic.
I think the key gain there was making things more *generic*, and there
are other ways we could do that in future without moving more into
task_struct (e.g. with a geenric thread_info and arch_thread_info inside
that).
With that in mind, and given the diffstat, I think this is worthwhile.
FWIW, for the series:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> While collaborating with Keith on adding THREAD_INFO_IN_TASK support to
> ARM, we noticed that keeping CPU in task_struct is problematic for
> architectures that define raw_smp_processor_id() in terms of this field,
> as it requires linux/sched.h to be included, which causes a lot of pain
> in terms of circular dependencies (or 'header soup', as the original
> commit refers to it).
>
> For examples of how existing architectures work around this, please
> refer to patches #6 or #7. In the former case, it uses an awful
> asm-offsets hack to index thread_info/current without using its type
> definition. The latter approach simply keeps a copy of the task_struct
> CPU field in thread_info, and keeps it in sync at context switch time.
>
> Patch #8 reverts this latter approach for ARM, but this code is still
> under review so it does not currently apply to mainline.
>
> We also discussed introducing yet another Kconfig symbol to indicate
> that the arch has THREAD_INFO_IN_TASK enabled but still prefers to keep
> its CPU field in thread_info, but simply keeping it in thread_info in
> all cases seems to be the cleanest approach here.
>
> Cc: Keith Packard <keithpac@amazon.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Albert Ou <aou@eecs.berkeley.edu>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-riscv@lists.infradead.org
> Cc: linux-s390@vger.kernel.org
>
> Ard Biesheuvel (8):
> arm64: add CPU field to struct thread_info
> x86: add CPU field to struct thread_info
> s390: add CPU field to struct thread_info
> powerpc: add CPU field to struct thread_info
> sched: move CPU field back into thread_info if THREAD_INFO_IN_TASK=y
> powerpc: smp: remove hack to obtain offset of task_struct::cpu
> riscv: rely on core code to keep thread_info::cpu updated
> ARM: rely on core code to keep thread_info::cpu updated
>
> arch/arm/include/asm/switch_to.h | 14 --------------
> arch/arm/kernel/smp.c | 3 ---
> arch/arm64/include/asm/thread_info.h | 1 +
> arch/arm64/kernel/asm-offsets.c | 2 +-
> arch/arm64/kernel/head.S | 2 +-
> arch/powerpc/Makefile | 11 -----------
> arch/powerpc/include/asm/smp.h | 17 +----------------
> arch/powerpc/include/asm/thread_info.h | 3 +++
> arch/powerpc/kernel/asm-offsets.c | 4 +---
> arch/powerpc/kernel/smp.c | 2 +-
> arch/riscv/kernel/asm-offsets.c | 1 -
> arch/riscv/kernel/entry.S | 5 -----
> arch/riscv/kernel/head.S | 1 -
> arch/s390/include/asm/thread_info.h | 1 +
> arch/x86/include/asm/thread_info.h | 3 +++
> include/linux/sched.h | 6 +-----
> kernel/sched/sched.h | 4 ----
> 17 files changed, 14 insertions(+), 66 deletions(-)
>
> --
> 2.30.2
>
^ permalink raw reply
* Re: [RFC PATCH 0/8] Move task_struct::cpu back into thread_info
From: Christophe Leroy @ 2021-09-14 13:49 UTC (permalink / raw)
To: Ard Biesheuvel, linux-kernel
Cc: Peter Zijlstra, Catalin Marinas, Paul Mackerras, linux-riscv,
Will Deacon, linux-s390, Arnd Bergmann, Russell King,
Christian Borntraeger, Ingo Molnar, Albert Ou, Kees Cook,
Vasily Gorbik, Heiko Carstens, Keith Packard, Borislav Petkov,
Andy Lutomirski, Paul Walmsley, Thomas Gleixner, linux-arm-kernel,
linuxppc-dev, Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-1-ardb@kernel.org>
Le 14/09/2021 à 14:10, Ard Biesheuvel a écrit :
> Commit c65eacbe290b ("sched/core: Allow putting thread_info into
> task_struct") mentions that, along with moving thread_info into
> task_struct, the cpu field is moved out of the former into the latter,
> but does not explain why.
I think it does explain why (init/Kconfig): "an arch will need to remove
all thread_info fields except flags".
IIUC initially the intention with THREAD_INFO_IN_TASK was to remove
everything from thread_info, but at the end it didn't happen it seems.
>
> While collaborating with Keith on adding THREAD_INFO_IN_TASK support to
> ARM, we noticed that keeping CPU in task_struct is problematic for
> architectures that define raw_smp_processor_id() in terms of this field,
> as it requires linux/sched.h to be included, which causes a lot of pain
> in terms of circular dependencies (or 'header soup', as the original
> commit refers to it).
>
> For examples of how existing architectures work around this, please
> refer to patches #6 or #7. In the former case, it uses an awful
> asm-offsets hack to index thread_info/current without using its type
> definition. The latter approach simply keeps a copy of the task_struct
> CPU field in thread_info, and keeps it in sync at context switch time.
It was a pain when implementing that on powerpc, so I really like your
idea, the series looks good to me.
>
> Patch #8 reverts this latter approach for ARM, but this code is still
> under review so it does not currently apply to mainline.
>
> We also discussed introducing yet another Kconfig symbol to indicate
> that the arch has THREAD_INFO_IN_TASK enabled but still prefers to keep
> its CPU field in thread_info, but simply keeping it in thread_info in
> all cases seems to be the cleanest approach here.
Yes, if we can avoid yet another config, that's better. We already have
so many configs that are supposed to be temporary and have lasted for
years if not decades.
Christophe
^ permalink raw reply
* Re: [PATCH trivial] powerpc/powernv/dump: Fix typo is comment
From: Joel Stanley @ 2021-09-14 13:29 UTC (permalink / raw)
To: Vasant Hegde; +Cc: linuxppc-dev
In-Reply-To: <20210914101646.30684-1-hegdevasant@linux.vnet.ibm.com>
On Tue, 14 Sept 2021 at 10:17, Vasant Hegde
<hegdevasant@linux.vnet.ibm.com> wrote:
>
> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
There's a typo in your commit message :)
> ---
> arch/powerpc/platforms/powernv/opal-dump.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-dump.c b/arch/powerpc/platforms/powernv/opal-dump.c
> index 00c5a59d82d9..717d1d30ade5 100644
> --- a/arch/powerpc/platforms/powernv/opal-dump.c
> +++ b/arch/powerpc/platforms/powernv/opal-dump.c
> @@ -419,7 +419,7 @@ void __init opal_platform_dump_init(void)
> int rc;
> int dump_irq;
>
> - /* ELOG not supported by firmware */
> + /* Dump not supported by firmware */
> if (!opal_check_token(OPAL_DUMP_READ))
> return;
>
> --
> 2.31.1
>
^ permalink raw reply
* Re: [PATCH bpf-next v2] bpf: Change value of MAX_TAIL_CALL_CNT from 32 to 33
From: Tiezhu Yang @ 2021-09-14 12:36 UTC (permalink / raw)
To: Daniel Borkmann, Shubham Bansal, Russell King, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Zi Shen Lim, Catalin Marinas,
Will Deacon, Paul Burton, Thomas Bogendoerfer, naveen.n.rao,
Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Luke Nelson, Xi Wang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
bjorn, davem, Johan Almbladh, Paul Chaignon
Cc: netdev, linux-kernel, linux-mips, sparclinux, bpf, linuxppc-dev,
linux-riscv, linux-arm-kernel
In-Reply-To: <0fb8d16f-67e7-7197-fce2-a4c17f1e5987@iogearbox.net>
On 09/14/2021 03:30 PM, Daniel Borkmann wrote:
> On 9/11/21 3:56 AM, Tiezhu Yang wrote:
>>
[...]
>> With this patch, it does not change the current limit 33,
>> MAX_TAIL_CALL_CNT
>> can reflect the actual max tail call count, the tailcall selftests
>> can work
>> well, and also the above failed testcase in test_bpf can be fixed for
>> the
>> interpreter (all archs) and the JIT (all archs except for x86).
>>
>> # uname -m
>> x86_64
>> # echo 1 > /proc/sys/net/core/bpf_jit_enable
>> # modprobe test_bpf
>> # dmesg | grep -w FAIL
>> Tail call error path, max count reached jited:1 ret 33 != 34 FAIL
>
> Could you also state in here which archs you have tested with this
> change? I
> presume /every/ arch which has a JIT?
OK, will do it in v3.
I have tested on x86 and mips.
>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>>
>> v2:
>> -- fix the typos in the commit message and update the commit message.
>> -- fix the failed tailcall selftests for x86 jit.
>> I am not quite sure the change on x86 is proper, with this change,
>> tailcall selftests passed, but tailcall limit test in test_bpf.ko
>> failed, I do not know the reason now, I think this is another
>> issue,
>> maybe someone more versed in x86 jit could take a look.
>
> There should be a series from Johan coming today with regards to
> test_bpf.ko
> that will fix the "tail call error path, max count reached" test which
> had an
> assumption in that R0 would always be valid for the fall-through and
> could be
> passed to the bpf_exit insn whereas it is not guaranteed and verifier,
> for
> example, forbids a subsequent access to R0 w/o reinit. For your
> testing, I
> would suggested to recheck once this series is out.
I will test the following patch on x86 and mips:
[PATCH bpf v4 13/14] bpf/tests: Fix error in tail call limit tests
[...]
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index 0fe6aac..74a9e61 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
>> @@ -402,7 +402,7 @@ static int get_pop_bytes(bool *callee_regs_used)
>> * ... bpf_tail_call(void *ctx, struct bpf_array *array, u64 index)
>> ...
>> * if (index >= array->map.max_entries)
>> * goto out;
>> - * if (++tail_call_cnt > MAX_TAIL_CALL_CNT)
>> + * if (tail_call_cnt++ == MAX_TAIL_CALL_CNT)
>
> Why such inconsistency to e.g. above with arm64 case but also compared to
> x86 32 bit which uses JAE? If so, we should cleanly follow the reference
> implementation (== interpreter) _everywhere_ and _not_ introduce
> additional
> variants/implementations across JITs.
In order tokeep consistencyand make as few changes as possible,
<javascript:void(0);>I will modify the check condition as follows:
#define MAX_TAIL_CALL_CNT 33
(1) for x86, arm64, ... (0 ~ 32)
tcc = 0;
if (tcc == MAX_TAIL_CALL_CNT)
goto out;
tcc++;
(2) for mips, riscv (33 ~ 1)
tcc = MAX_TAIL_CALL_CNT;
if (tcc == 0)
goto out;
tcc--;
[...]
^ permalink raw reply
* Re: linux-next: build failure after merge of the origin tree
From: Michael Ellerman @ 2021-09-14 12:21 UTC (permalink / raw)
To: Linus Torvalds, Stephen Rothwell
Cc: Linux Next Mailing List, PowerPC, Linux Kernel Mailing List
In-Reply-To: <CAHk-=wieb251-L9D-v3BeF-Cna8r5kLz2MeyXDS3mrNUmXNYrg@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Mon, Sep 13, 2021 at 7:08 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> That patch works for me - for the ppc64_defconfig build at least.
>
> Yeah, I just tested the allmodconfig case too, although I suspect it's
> essentially the same wrt the boot *.S files, so it probably doesn't
> matter.
>
> I'd like to have Michael or somebody who can actually run some tests
> on the end result ack that patch (or - even better - come up with
> something cleaner) before committing it.
>
> Because yeah, the build failure is annoying and I apologize, but I'd
> rather have the build fail overnight than commit something that builds
> but then is subtle buggy for some reason.
>
> But if I don't get any other comments by the time I'm up again
> tomorrow, I'll just commit it as "fixes the build".
I ended up doing a more minimal version of your change.
I sent it separately, or it's here:
https://lore.kernel.org/lkml/20210914121723.3756817-1-mpe@ellerman.id.au/
cheers
^ permalink raw reply
* [PATCH] powerpc/boot: Fix build failure since GCC 4.9 removal
From: Michael Ellerman @ 2021-09-14 12:17 UTC (permalink / raw)
To: torvalds; +Cc: sfr, linux-next, linuxppc-dev, linux-kernel
Stephen reported that the build was broken since commit
6d2ef226f2f1 ("compiler_attributes.h: drop __has_attribute() support for
gcc4"), with errors such as:
include/linux/compiler_attributes.h:296:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
296 | #if __has_attribute(__warning__)
| ^~~~~~~~~~~~~~~
make[2]: *** [arch/powerpc/boot/Makefile:225: arch/powerpc/boot/crt0.o] Error 1
But we expect __has_attribute() to always be defined now that we've
stopped using GCC 4.
Linus debugged it to the point of reading the GCC sources, and noticing
that the problem is that __has_attribute() is not defined when
preprocessing assembly files, which is what we're doing here.
Our assembly files don't include, or need, compiler_attributes.h, but
they are getting it unconditionally from the -include in BOOT_CFLAGS,
which is then added in its entirety to BOOT_AFLAGS.
That -include was added in commit 77433830ed16 ("powerpc: boot: include
compiler_attributes.h") so that we'd have "fallthrough" and other
attributes defined for the C files in arch/powerpc/boot. But it's not
needed for assembly files.
The minimal fix is to move the addition to BOOT_CFLAGS of -include
compiler_attributes.h until after we've copied BOOT_CFLAGS into
BOOT_AFLAGS. That avoids including compiler_attributes.h for asm files,
but makes no other change to BOOT_CFLAGS or BOOT_AFLAGS.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Debugged-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/boot/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
This seemed safer as a minimal fix, rather than doing a more
comprehensive separation of CFLAGS/AFLAGS. We can do that in a future
patch.
It passed my usual build/boot tests, including booting the built zImage
on some real hardware, so this is good to go from my POV.
cheers
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 6900d0ac2421..089ee3ea55c8 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -35,7 +35,6 @@ endif
BOOTCFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \
-pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
- -include $(srctree)/include/linux/compiler_attributes.h \
$(LINUXINCLUDE)
ifdef CONFIG_PPC64_BOOT_WRAPPER
@@ -70,6 +69,7 @@ ifeq ($(call cc-option-yn, -fstack-protector),y)
BOOTCFLAGS += -fno-stack-protector
endif
+BOOTCFLAGS += -include $(srctree)/include/linux/compiler_attributes.h
BOOTCFLAGS += -I$(objtree)/$(obj) -I$(srctree)/$(obj)
DTC_FLAGS ?= -p 1024
--
2.25.1
^ permalink raw reply related
* [RFC PATCH 8/8] ARM: rely on core code to keep thread_info::cpu updated
From: Ard Biesheuvel @ 2021-09-14 12:10 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
Ard Biesheuvel, linux-s390, Arnd Bergmann, Russell King,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Albert Ou,
Kees Cook, Vasily Gorbik, Heiko Carstens, Keith Packard,
Borislav Petkov, Andy Lutomirski, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, linuxppc-dev, Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-1-ardb@kernel.org>
Now that the core code switched back to using thread_info::cpu to keep
a task's CPU number, we no longer need to keep it in sync explicitly. So
just drop the code that does this.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
This patch applies onto [0], which we hope to get merged for v5.16
[0] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=arm32-ti-in-task-v5
arch/arm/include/asm/switch_to.h | 14 --------------
arch/arm/kernel/smp.c | 3 ---
2 files changed, 17 deletions(-)
diff --git a/arch/arm/include/asm/switch_to.h b/arch/arm/include/asm/switch_to.h
index db2be1f6550d..61e4a3c4ca6e 100644
--- a/arch/arm/include/asm/switch_to.h
+++ b/arch/arm/include/asm/switch_to.h
@@ -23,23 +23,9 @@
*/
extern struct task_struct *__switch_to(struct task_struct *, struct thread_info *, struct thread_info *);
-static inline void set_ti_cpu(struct task_struct *p)
-{
-#ifdef CONFIG_THREAD_INFO_IN_TASK
- /*
- * The core code no longer maintains the thread_info::cpu field once
- * CONFIG_THREAD_INFO_IN_TASK is in effect, but we rely on it for
- * raw_smp_processor_id(), which cannot access struct task_struct*
- * directly for reasons of circular #inclusion hell.
- */
- task_thread_info(p)->cpu = p->cpu;
-#endif
-}
-
#define switch_to(prev,next,last) \
do { \
__complete_pending_tlbi(); \
- set_ti_cpu(next); \
if (IS_ENABLED(CONFIG_CURRENT_POINTER_IN_TPIDRURO)) \
__this_cpu_write(__entry_task, next); \
last = __switch_to(prev,task_thread_info(prev), task_thread_info(next)); \
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index cde5b6d8bac5..97ee6b1567e9 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -154,9 +154,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
secondary_data.swapper_pg_dir = get_arch_pgd(swapper_pg_dir);
#endif
secondary_data.task = idle;
- if (IS_ENABLED(CONFIG_THREAD_INFO_IN_TASK))
- task_thread_info(idle)->cpu = cpu;
-
sync_cache_w(&secondary_data);
/*
--
2.30.2
^ permalink raw reply related
* [RFC PATCH 7/8] riscv: rely on core code to keep thread_info::cpu updated
From: Ard Biesheuvel @ 2021-09-14 12:10 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
Ard Biesheuvel, linux-s390, Arnd Bergmann, Russell King,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Albert Ou,
Kees Cook, Vasily Gorbik, Heiko Carstens, Keith Packard,
Borislav Petkov, Andy Lutomirski, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, linuxppc-dev, Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-1-ardb@kernel.org>
Now that the core code switched back to using thread_info::cpu to keep
a task's CPU number, we no longer need to keep it in sync explicitly. So
just drop the code that does this.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/riscv/kernel/asm-offsets.c | 1 -
arch/riscv/kernel/entry.S | 5 -----
arch/riscv/kernel/head.S | 1 -
3 files changed, 7 deletions(-)
diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index 90f8ce64fa6f..478d9f02dab5 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -33,7 +33,6 @@ void asm_offsets(void)
OFFSET(TASK_TI_PREEMPT_COUNT, task_struct, thread_info.preempt_count);
OFFSET(TASK_TI_KERNEL_SP, task_struct, thread_info.kernel_sp);
OFFSET(TASK_TI_USER_SP, task_struct, thread_info.user_sp);
- OFFSET(TASK_TI_CPU, task_struct, thread_info.cpu);
OFFSET(TASK_THREAD_F0, task_struct, thread.fstate.f[0]);
OFFSET(TASK_THREAD_F1, task_struct, thread.fstate.f[1]);
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 98f502654edd..459eb1714353 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -544,11 +544,6 @@ ENTRY(__switch_to)
REG_L s9, TASK_THREAD_S9_RA(a4)
REG_L s10, TASK_THREAD_S10_RA(a4)
REG_L s11, TASK_THREAD_S11_RA(a4)
- /* Swap the CPU entry around. */
- lw a3, TASK_TI_CPU(a0)
- lw a4, TASK_TI_CPU(a1)
- sw a3, TASK_TI_CPU(a1)
- sw a4, TASK_TI_CPU(a0)
/* The offset of thread_info in task_struct is zero. */
move tp, a1
ret
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index fce5184b22c3..d5ec30ef6f5d 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -317,7 +317,6 @@ clear_bss_done:
call setup_trap_vector
/* Restore C environment */
la tp, init_task
- sw zero, TASK_TI_CPU(tp)
la sp, init_thread_union + THREAD_SIZE
#ifdef CONFIG_KASAN
--
2.30.2
^ permalink raw reply related
* [RFC PATCH 6/8] powerpc: smp: remove hack to obtain offset of task_struct::cpu
From: Ard Biesheuvel @ 2021-09-14 12:10 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
Ard Biesheuvel, linux-s390, Arnd Bergmann, Russell King,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Albert Ou,
Kees Cook, Vasily Gorbik, Heiko Carstens, Keith Packard,
Borislav Petkov, Andy Lutomirski, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, linuxppc-dev, Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-1-ardb@kernel.org>
Instead of relying on awful hacks to obtain the offset of the cpu field
in struct task_struct, move it back into struct thread_info, which does
not create the same level of circular dependency hell when trying to
include the header file that defines it.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/powerpc/Makefile | 11 -----------
arch/powerpc/include/asm/smp.h | 17 +----------------
arch/powerpc/kernel/asm-offsets.c | 2 --
3 files changed, 1 insertion(+), 29 deletions(-)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index aa6808e70647..54cad1faa5d0 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -446,17 +446,6 @@ else
endif
endif
-ifdef CONFIG_SMP
-ifdef CONFIG_PPC32
-prepare: task_cpu_prepare
-
-PHONY += task_cpu_prepare
-task_cpu_prepare: prepare0
- $(eval KBUILD_CFLAGS += -D_TASK_CPU=$(shell awk '{if ($$2 == "TASK_CPU") print $$3;}' include/generated/asm-offsets.h))
-
-endif # CONFIG_PPC32
-endif # CONFIG_SMP
-
PHONY += checkbin
# Check toolchain versions:
# - gcc-4.6 is the minimum kernel-wide version so nothing required.
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 7ef1cd8168a0..007332a4a732 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -87,22 +87,7 @@ int is_cpu_dead(unsigned int cpu);
/* 32-bit */
extern int smp_hw_index[];
-/*
- * This is particularly ugly: it appears we can't actually get the definition
- * of task_struct here, but we need access to the CPU this task is running on.
- * Instead of using task_struct we're using _TASK_CPU which is extracted from
- * asm-offsets.h by kbuild to get the current processor ID.
- *
- * This also needs to be safeguarded when building asm-offsets.s because at
- * that time _TASK_CPU is not defined yet. It could have been guarded by
- * _TASK_CPU itself, but we want the build to fail if _TASK_CPU is missing
- * when building something else than asm-offsets.s
- */
-#ifdef GENERATING_ASM_OFFSETS
-#define raw_smp_processor_id() (0)
-#else
-#define raw_smp_processor_id() (*(unsigned int *)((void *)current + _TASK_CPU))
-#endif
+#define raw_smp_processor_id() (current_thread_info()->cpu)
#define hard_smp_processor_id() (smp_hw_index[smp_processor_id()])
static inline int get_hard_smp_processor_id(int cpu)
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index e37e4546034e..cc05522f50bf 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -9,8 +9,6 @@
* #defines from the assembly-language output.
*/
-#define GENERATING_ASM_OFFSETS /* asm/smp.h */
-
#include <linux/compat.h>
#include <linux/signal.h>
#include <linux/sched.h>
--
2.30.2
^ permalink raw reply related
* [RFC PATCH 5/8] sched: move CPU field back into thread_info if THREAD_INFO_IN_TASK=y
From: Ard Biesheuvel @ 2021-09-14 12:10 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
Ard Biesheuvel, linux-s390, Arnd Bergmann, Russell King,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Albert Ou,
Kees Cook, Vasily Gorbik, Heiko Carstens, Keith Packard,
Borislav Petkov, Andy Lutomirski, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, linuxppc-dev, Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-1-ardb@kernel.org>
THREAD_INFO_IN_TASK moved the CPU field out of thread_info, but this
causes some issues on architectures that define raw_smp_processor_id()
in terms of this field, due to the fact that #include'ing linux/sched.h
to get at struct task_struct is problematic in terms of circular
dependencies.
Given that thread_info and task_struct are the same data structure
anyway when THREAD_INFO_IN_TASK=y, let's move it back so that having
access to the type definition of struct thread_info is sufficient to
reference the CPU number of the current task.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm64/kernel/asm-offsets.c | 1 -
arch/arm64/kernel/head.S | 2 +-
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/kernel/smp.c | 2 +-
include/linux/sched.h | 6 +-----
kernel/sched/sched.h | 4 ----
6 files changed, 4 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index cee9f3e9f906..0bfc048221af 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -27,7 +27,6 @@
int main(void)
{
DEFINE(TSK_ACTIVE_MM, offsetof(struct task_struct, active_mm));
- DEFINE(TSK_CPU, offsetof(struct task_struct, cpu));
BLANK();
DEFINE(TSK_TI_CPU, offsetof(struct task_struct, thread_info.cpu));
DEFINE(TSK_TI_FLAGS, offsetof(struct task_struct, thread_info.flags));
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 17962452e31d..6a98f1a38c29 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -412,7 +412,7 @@ SYM_FUNC_END(__create_page_tables)
scs_load \tsk
adr_l \tmp1, __per_cpu_offset
- ldr w\tmp2, [\tsk, #TSK_CPU]
+ ldr w\tmp2, [\tsk, #TSK_TI_CPU]
ldr \tmp1, [\tmp1, \tmp2, lsl #3]
set_this_cpu_offset \tmp1
.endm
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index e563d3222d69..e37e4546034e 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -93,7 +93,7 @@ int main(void)
#endif /* CONFIG_PPC64 */
OFFSET(TASK_STACK, task_struct, stack);
#ifdef CONFIG_SMP
- OFFSET(TASK_CPU, task_struct, cpu);
+ OFFSET(TASK_CPU, task_struct, thread_info.cpu);
#endif
#ifdef CONFIG_LIVEPATCH
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 9cc7d3dbf439..512d875b45e0 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1223,7 +1223,7 @@ static void cpu_idle_thread_init(unsigned int cpu, struct task_struct *idle)
paca_ptrs[cpu]->kstack = (unsigned long)task_stack_page(idle) +
THREAD_SIZE - STACK_FRAME_OVERHEAD;
#endif
- idle->cpu = cpu;
+ task_thread_info(idle)->cpu = cpu;
secondary_current = current_set[cpu] = idle;
}
diff --git a/include/linux/sched.h b/include/linux/sched.h
index e12b524426b0..37aa521078e7 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -750,10 +750,6 @@ struct task_struct {
#ifdef CONFIG_SMP
int on_cpu;
struct __call_single_node wake_entry;
-#ifdef CONFIG_THREAD_INFO_IN_TASK
- /* Current CPU: */
- unsigned int cpu;
-#endif
unsigned int wakee_flips;
unsigned long wakee_flip_decay_ts;
struct task_struct *last_wakee;
@@ -2114,7 +2110,7 @@ static __always_inline bool need_resched(void)
static inline unsigned int task_cpu(const struct task_struct *p)
{
#ifdef CONFIG_THREAD_INFO_IN_TASK
- return READ_ONCE(p->cpu);
+ return READ_ONCE(p->thread_info.cpu);
#else
return READ_ONCE(task_thread_info(p)->cpu);
#endif
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 3d3e5793e117..79fcbad11450 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1926,11 +1926,7 @@ static inline void __set_task_cpu(struct task_struct *p, unsigned int cpu)
* per-task data have been completed by this moment.
*/
smp_wmb();
-#ifdef CONFIG_THREAD_INFO_IN_TASK
- WRITE_ONCE(p->cpu, cpu);
-#else
WRITE_ONCE(task_thread_info(p)->cpu, cpu);
-#endif
p->wake_cpu = cpu;
#endif
}
--
2.30.2
^ permalink raw reply related
* [RFC PATCH 4/8] powerpc: add CPU field to struct thread_info
From: Ard Biesheuvel @ 2021-09-14 12:10 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
Ard Biesheuvel, linux-s390, Arnd Bergmann, Russell King,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Albert Ou,
Kees Cook, Vasily Gorbik, Heiko Carstens, Keith Packard,
Borislav Petkov, Andy Lutomirski, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, linuxppc-dev, Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-1-ardb@kernel.org>
The CPU field will be moved back into thread_info even when
THREAD_INFO_IN_TASK is enabled, so add it back to powerpc's definition
of struct thread_info.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/powerpc/include/asm/thread_info.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index b4ec6c7dd72e..5725029aaa29 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -47,6 +47,9 @@
struct thread_info {
int preempt_count; /* 0 => preemptable,
<0 => BUG */
+#ifdef CONFIG_SMP
+ unsigned int cpu;
+#endif
unsigned long local_flags; /* private flags for thread */
#ifdef CONFIG_LIVEPATCH
unsigned long *livepatch_sp;
--
2.30.2
^ permalink raw reply related
* [RFC PATCH 3/8] s390: add CPU field to struct thread_info
From: Ard Biesheuvel @ 2021-09-14 12:10 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
Ard Biesheuvel, linux-s390, Arnd Bergmann, Russell King,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Albert Ou,
Kees Cook, Vasily Gorbik, Heiko Carstens, Keith Packard,
Borislav Petkov, Andy Lutomirski, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, linuxppc-dev, Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-1-ardb@kernel.org>
The CPU field will be moved back into thread_info even when
THREAD_INFO_IN_TASK is enabled, so add it back to s390's definition of
struct thread_info.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/s390/include/asm/thread_info.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/s390/include/asm/thread_info.h b/arch/s390/include/asm/thread_info.h
index e6674796aa6f..b2ffcb4fe000 100644
--- a/arch/s390/include/asm/thread_info.h
+++ b/arch/s390/include/asm/thread_info.h
@@ -37,6 +37,7 @@
struct thread_info {
unsigned long flags; /* low level flags */
unsigned long syscall_work; /* SYSCALL_WORK_ flags */
+ unsigned int cpu; /* current CPU */
};
/*
--
2.30.2
^ permalink raw reply related
* [RFC PATCH 2/8] x86: add CPU field to struct thread_info
From: Ard Biesheuvel @ 2021-09-14 12:10 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
Ard Biesheuvel, linux-s390, Arnd Bergmann, Russell King,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Albert Ou,
Kees Cook, Vasily Gorbik, Heiko Carstens, Keith Packard,
Borislav Petkov, Andy Lutomirski, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, linuxppc-dev, Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-1-ardb@kernel.org>
The CPU field will be moved back into thread_info even when
THREAD_INFO_IN_TASK is enabled, so add it back to x86's definition of
struct thread_info.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/x86/include/asm/thread_info.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index cf132663c219..ebec69c35e95 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -57,6 +57,9 @@ struct thread_info {
unsigned long flags; /* low level flags */
unsigned long syscall_work; /* SYSCALL_WORK_ flags */
u32 status; /* thread synchronous flags */
+#ifdef CONFIG_SMP
+ u32 cpu; /* current CPU */
+#endif
};
#define INIT_THREAD_INFO(tsk) \
--
2.30.2
^ permalink raw reply related
* [RFC PATCH 1/8] arm64: add CPU field to struct thread_info
From: Ard Biesheuvel @ 2021-09-14 12:10 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
Ard Biesheuvel, linux-s390, Arnd Bergmann, Russell King,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Albert Ou,
Kees Cook, Vasily Gorbik, Heiko Carstens, Keith Packard,
Borislav Petkov, Andy Lutomirski, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, linuxppc-dev, Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-1-ardb@kernel.org>
The CPU field will be moved back into thread_info even when
THREAD_INFO_IN_TASK is enabled, so add it back to arm64's definition of
struct thread_info.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/kernel/asm-offsets.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 6623c99f0984..c02bc8c183c3 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -42,6 +42,7 @@ struct thread_info {
void *scs_base;
void *scs_sp;
#endif
+ u32 cpu;
};
#define thread_saved_pc(tsk) \
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 551427ae8cc5..cee9f3e9f906 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -29,6 +29,7 @@ int main(void)
DEFINE(TSK_ACTIVE_MM, offsetof(struct task_struct, active_mm));
DEFINE(TSK_CPU, offsetof(struct task_struct, cpu));
BLANK();
+ DEFINE(TSK_TI_CPU, offsetof(struct task_struct, thread_info.cpu));
DEFINE(TSK_TI_FLAGS, offsetof(struct task_struct, thread_info.flags));
DEFINE(TSK_TI_PREEMPT, offsetof(struct task_struct, thread_info.preempt_count));
#ifdef CONFIG_ARM64_SW_TTBR0_PAN
--
2.30.2
^ permalink raw reply related
* [RFC PATCH 0/8] Move task_struct::cpu back into thread_info
From: Ard Biesheuvel @ 2021-09-14 12:10 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
Ard Biesheuvel, linux-s390, Arnd Bergmann, Russell King,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Albert Ou,
Kees Cook, Vasily Gorbik, Heiko Carstens, Keith Packard,
Borislav Petkov, Andy Lutomirski, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, linuxppc-dev, Palmer Dabbelt, Linus Torvalds
Commit c65eacbe290b ("sched/core: Allow putting thread_info into
task_struct") mentions that, along with moving thread_info into
task_struct, the cpu field is moved out of the former into the latter,
but does not explain why.
While collaborating with Keith on adding THREAD_INFO_IN_TASK support to
ARM, we noticed that keeping CPU in task_struct is problematic for
architectures that define raw_smp_processor_id() in terms of this field,
as it requires linux/sched.h to be included, which causes a lot of pain
in terms of circular dependencies (or 'header soup', as the original
commit refers to it).
For examples of how existing architectures work around this, please
refer to patches #6 or #7. In the former case, it uses an awful
asm-offsets hack to index thread_info/current without using its type
definition. The latter approach simply keeps a copy of the task_struct
CPU field in thread_info, and keeps it in sync at context switch time.
Patch #8 reverts this latter approach for ARM, but this code is still
under review so it does not currently apply to mainline.
We also discussed introducing yet another Kconfig symbol to indicate
that the arch has THREAD_INFO_IN_TASK enabled but still prefers to keep
its CPU field in thread_info, but simply keeping it in thread_info in
all cases seems to be the cleanest approach here.
Cc: Keith Packard <keithpac@amazon.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Ard Biesheuvel (8):
arm64: add CPU field to struct thread_info
x86: add CPU field to struct thread_info
s390: add CPU field to struct thread_info
powerpc: add CPU field to struct thread_info
sched: move CPU field back into thread_info if THREAD_INFO_IN_TASK=y
powerpc: smp: remove hack to obtain offset of task_struct::cpu
riscv: rely on core code to keep thread_info::cpu updated
ARM: rely on core code to keep thread_info::cpu updated
arch/arm/include/asm/switch_to.h | 14 --------------
arch/arm/kernel/smp.c | 3 ---
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/kernel/asm-offsets.c | 2 +-
arch/arm64/kernel/head.S | 2 +-
arch/powerpc/Makefile | 11 -----------
arch/powerpc/include/asm/smp.h | 17 +----------------
arch/powerpc/include/asm/thread_info.h | 3 +++
arch/powerpc/kernel/asm-offsets.c | 4 +---
arch/powerpc/kernel/smp.c | 2 +-
arch/riscv/kernel/asm-offsets.c | 1 -
arch/riscv/kernel/entry.S | 5 -----
arch/riscv/kernel/head.S | 1 -
arch/s390/include/asm/thread_info.h | 1 +
arch/x86/include/asm/thread_info.h | 3 +++
include/linux/sched.h | 6 +-----
kernel/sched/sched.h | 4 ----
17 files changed, 14 insertions(+), 66 deletions(-)
--
2.30.2
^ permalink raw reply
* Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
From: Borislav Petkov @ 2021-09-14 11:58 UTC (permalink / raw)
To: Michael Ellerman
Cc: Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh, kvm,
dri-devel, platform-driver-x86, Paul Mackerras, linux-s390,
Andi Kleen, Joerg Roedel, x86, amd-gfx, Christoph Hellwig,
linux-graphics-maintainer, Tom Lendacky, Tianyu Lan, kexec,
linux-kernel, iommu, linux-fsdevel, linuxppc-dev
In-Reply-To: <9d4fc3f8ea7b325aaa1879beab1286876f45d450.1631141919.git.thomas.lendacky@amd.com>
On Wed, Sep 08, 2021 at 05:58:35PM -0500, Tom Lendacky wrote:
> Introduce a powerpc version of the cc_platform_has() function. This will
> be used to replace the powerpc mem_encrypt_active() implementation, so
> the implementation will initially only support the CC_ATTR_MEM_ENCRYPT
> attribute.
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> arch/powerpc/platforms/pseries/Kconfig | 1 +
> arch/powerpc/platforms/pseries/Makefile | 2 ++
> arch/powerpc/platforms/pseries/cc_platform.c | 26 ++++++++++++++++++++
> 3 files changed, 29 insertions(+)
> create mode 100644 arch/powerpc/platforms/pseries/cc_platform.c
Michael,
can I get an ACK for the ppc bits to carry them through the tip tree
pls?
Btw, on a related note, cross-compiling this throws the following error here:
$ make CROSS_COMPILE=/home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/powerpc64-linux- V=1 ARCH=powerpc
...
/home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/powerpc64-linux-gcc -Wp,-MD,arch/powerpc/boot/.crt0.o.d -D__ASSEMBLY__ -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc -include ./include/linux/compiler_attributes.h -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -m32 -isystem /home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/../lib/gcc/powerpc64-linux/9.4.0/include -mbig-endian -nostdinc -c -o arch/powerpc/boot/crt0.o arch/powerpc/boot/crt0.S
In file included from <command-line>:
././include/linux/compiler_attributes.h:62:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
62 | #if __has_attribute(__assume_aligned__)
| ^~~~~~~~~~~~~~~
././include/linux/compiler_attributes.h:62:20: error: missing binary operator before token "("
62 | #if __has_attribute(__assume_aligned__)
| ^
././include/linux/compiler_attributes.h:88:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
88 | #if __has_attribute(__copy__)
| ^~~~~~~~~~~~~~~
...
Known issue?
This __has_attribute() thing is supposed to be supported
in gcc since 5.1 and I'm using the crosstool stuff from
https://www.kernel.org/pub/tools/crosstool/ and gcc-9.4 above is pretty
new so that should not happen actually.
But it does...
Hmmm.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH 1/3] perf: Add macros to specify onchip L2/L3 accesses
From: Peter Zijlstra @ 2021-09-14 11:49 UTC (permalink / raw)
To: Michael Ellerman
Cc: mark.rutland, atrajeev, ak, daniel, rnsastry, alexander.shishkin,
Kajol Jain, linux-kernel, acme, ast, linux-perf-users, yao.jin,
mingo, paulus, maddy, jolsa, namhyung, songliubraving,
linuxppc-dev, kan.liang
In-Reply-To: <87k0jjl9sp.fsf@mpe.ellerman.id.au>
On Tue, Sep 14, 2021 at 08:40:38PM +1000, Michael Ellerman wrote:
> Peter Zijlstra <peterz@infradead.org> writes:
> > I'm thinking we ought to keep hops as steps along the NUMA fabric, with
> > 0 hops being the local node. That only gets us:
> >
> > L2, remote=0, hops=HOPS_0 -- our L2
> > L2, remote=1, hops=HOPS_0 -- L2 on the local node but not ours
> > L2, remote=1, hops!=HOPS_0 -- L2 on a remote node
>
> Hmm. I'm not sure about tying it directly to NUMA hops. I worry we're
> going to see more and more systems where there's a hierarchy within the
> chip/package, in addition to the traditional NUMA hierarchy.
>
> Although then I guess it becomes a question of what exactly is a NUMA
> hop, maybe the answer is that on those future systems those
> intra-chip/package hops should be represented as NUMA hops.
>
> It's not like we have a hard definition of what a NUMA hop is?
Not really, typically whatever the BIOS/DT/whatever tables tell us. I
think in case of Power you're mostly making things up in software :-)
But yeah, I think we have plenty wriggle room there.
^ permalink raw reply
* Re: [PATCH 1/3] perf: Add macros to specify onchip L2/L3 accesses
From: Michael Ellerman @ 2021-09-14 10:40 UTC (permalink / raw)
To: Peter Zijlstra
Cc: mark.rutland, atrajeev, ak, daniel, rnsastry, alexander.shishkin,
Kajol Jain, linux-kernel, acme, ast, linux-perf-users, yao.jin,
mingo, paulus, maddy, jolsa, namhyung, songliubraving,
linuxppc-dev, kan.liang
In-Reply-To: <YTob/xfn1gt901K4@hirez.programming.kicks-ass.net>
Peter Zijlstra <peterz@infradead.org> writes:
> On Thu, Sep 09, 2021 at 10:45:54PM +1000, Michael Ellerman wrote:
>
>> > The 'new' composite doesnt have a hops field because the hardware that
>> > nessecitated that change doesn't report it, but we could easily add a
>> > field there.
>> >
>> > Suppose we add, mem_hops:3 (would 6 hops be too small?) and the
>> > corresponding PERF_MEM_HOPS_{NA, 0..6}
>>
>> It's really 7 if we use remote && hop = 0 to mean the first hop.
>
> I don't think we can do that, becaus of backward compat. Currently:
>
> lvl_num=DRAM, remote=1
>
> denites: "Remote DRAM of any distance". Effectively it would have the new
> hops field filled with zeros though, so if you then decode with the hops
> field added it suddenly becomes:
>
> lvl_num=DRAM, remote=1, hops=0
>
> and reads like: "Remote DRAM of 0 hops" which is quite daft. Therefore 0
> really must denote a 'N/A'.
Ah yeah, duh, it needs to be backward compatible.
>> If we're wanting to use some of the hop levels to represent
>> intra-chip/package hops then we could possibly use them all on a really
>> big system.
>>
>> eg. you could imagine something like:
>>
>> L2 | - local L2
>> L2 | REMOTE | HOPS_0 - L2 of neighbour core
>> L2 | REMOTE | HOPS_1 - L2 of near core on same chip (same 1/2 of chip)
>> L2 | REMOTE | HOPS_2 - L2 of far core on same chip (other 1/2 of chip)
>> L2 | REMOTE | HOPS_3 - L2 of sibling chip in same package
>> L2 | REMOTE | HOPS_4 - L2 on separate package 1 hop away
>> L2 | REMOTE | HOPS_5 - L2 on separate package 2 hops away
>> L2 | REMOTE | HOPS_6 - L2 on separate package 3 hops away
>>
>>
>> Whether it's useful to represent all those levels I'm not sure, but it's
>> probably good if we have the ability.
>
> I'm thinking we ought to keep hops as steps along the NUMA fabric, with
> 0 hops being the local node. That only gets us:
>
> L2, remote=0, hops=HOPS_0 -- our L2
> L2, remote=1, hops=HOPS_0 -- L2 on the local node but not ours
> L2, remote=1, hops!=HOPS_0 -- L2 on a remote node
Hmm. I'm not sure about tying it directly to NUMA hops. I worry we're
going to see more and more systems where there's a hierarchy within the
chip/package, in addition to the traditional NUMA hierarchy.
Although then I guess it becomes a question of what exactly is a NUMA
hop, maybe the answer is that on those future systems those
intra-chip/package hops should be represented as NUMA hops.
It's not like we have a hard definition of what a NUMA hop is?
>> I guess I'm 50/50 on whether that's enough levels, or whether we want
>> another bit to allow for future growth.
>
> Right, possibly safer to add one extra bit while we can.... I suppose.
Equally it's not _that_ hard to add another bit later (if there's still
one free), makes the API a little uglier to use, but not the end of the
world.
cheers
^ permalink raw reply
* [PATCH trivial] powerpc/powernv/dump: Fix typo is comment
From: Vasant Hegde @ 2021-09-14 10:16 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Vasant Hegde
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/opal-dump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/opal-dump.c b/arch/powerpc/platforms/powernv/opal-dump.c
index 00c5a59d82d9..717d1d30ade5 100644
--- a/arch/powerpc/platforms/powernv/opal-dump.c
+++ b/arch/powerpc/platforms/powernv/opal-dump.c
@@ -419,7 +419,7 @@ void __init opal_platform_dump_init(void)
int rc;
int dump_irq;
- /* ELOG not supported by firmware */
+ /* Dump not supported by firmware */
if (!opal_check_token(OPAL_DUMP_READ))
return;
--
2.31.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox