[PATCH] usb: gadget: fsl: fix null pointer checking
fsl_ep_fifo_status() should return error if _ep->desc is null. Fixes: 75eaa498c99e (“usb: gadget: Correct NULL pointer checking in fsl gadget”) Signed-off-by: Ran Wang --- drivers/usb/gadget/udc/fsl_udc_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c index de528e3..ad6ff9c 100644 --- a/drivers/usb/gadget/udc/fsl_udc_core.c +++ b/drivers/usb/gadget/udc/fsl_udc_core.c @@ -1051,7 +1051,7 @@ static int fsl_ep_fifo_status(struct usb_ep *_ep) u32 bitmask; struct ep_queue_head *qh; - if (!_ep || _ep->desc || !(_ep->desc->bEndpointAddress&0xF)) + if (!_ep || !_ep->desc || !(_ep->desc->bEndpointAddress&0xF)) return -ENODEV; ep = container_of(_ep, struct fsl_ep, ep); -- 2.7.4
Re: [PATCH v6 02/11] mm/gup: Use functions to track lockless pgtbl walks on gup_pgd_range
Hi Michal, On 10/15/20 8:16 PM, Michal Suchánek wrote: Hello, On Thu, Feb 06, 2020 at 12:25:18AM -0300, Leonardo Bras wrote: On Thu, 2020-02-06 at 00:08 -0300, Leonardo Bras wrote: gup_pgd_range(addr, end, gup_flags, pages, ); - local_irq_enable(); + end_lockless_pgtbl_walk(IRQS_ENABLED); ret = nr; } Just noticed IRQS_ENABLED is not available on other archs than ppc64. I will fix this for v7. Has threre been v7? I cannot find it. Thanks Michal https://lore.kernel.org/linuxppc-dev/20200505071729.54912-1-aneesh.ku...@linux.ibm.com This series should help here. -aneesh
[GIT PULL] Please pull powerpc/linux.git powerpc-5.10-1 tag
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Hi Linus, Please pull powerpc updates for 5.10. Just two minor conflicts I'm aware of. The only slight subtlety is the conflict in kasan_init() where "int ret" needs to move out of the for_each_mem_range() and up to the function scope. Notable out of area changes: arch/sparc/kernel/smp_64.c# bafb056ce279 sparc64: remove mm_cpumask clearing to fix kthread_use_mm race fs/exec.c # d53c3dfb23c4 mm: fix exec activate_mm vs TLB shootdown and lazy tlb switching race include/uapi/linux/mman.h # e47168f3d1b1 powerpc/8xx: Support 16k hugepages with 4k pages include/uapi/asm-generic/hugetlb_encode.h # as above cheers The following changes since commit d012a7190fc1fd72ed48911e77ca97ba4521bccd: Linux 5.9-rc2 (2020-08-23 14:08:43 -0700) are available in the git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-5.10-1 for you to fetch changes up to ffd0b25ca049a477cb757e5bcf2d5e1664d12e5d: Revert "powerpc/pci: unmap legacy INTx interrupts when a PHB is removed" (2020-10-15 13:42:49 +1100) - -- powerpc updates for 5.10 - A series from Nick adding ARCH_WANT_IRQS_OFF_ACTIVATE_MM & selecting it for powerpc, as well as a related fix for sparc. - Remove support for PowerPC 601. - Some fixes for watchpoints & addition of a new ptrace flag for detecting ISA v3.1 (Power10) watchpoint features. - A fix for kernels using 4K pages and the hash MMU on bare metal Power9 systems with > 16TB of RAM, or RAM on the 2nd node. - A basic idle driver for shallow stop states on Power10. - Tweaks to our sched domains code to better inform the scheduler about the hardware topology on Power9/10, where two SMT4 cores can be presented by firmware as an SMT8 core. - A series doing further reworks & cleanups of our EEH code. - Addition of a filter for RTAS (firmware) calls done via sys_rtas(), to prevent root from overwriting kernel memory. - Other smaller features, fixes & cleanups. Thanks to: Alexey Kardashevskiy, Andrew Donnellan, Aneesh Kumar K.V, Athira Rajeev, Biwen Li, Cameron Berkenpas, Cédric Le Goater, Christophe Leroy, Christoph Hellwig, Colin Ian King, Daniel Axtens, David Dai, Finn Thain, Frederic Barrat, Gautham R. Shenoy, Greg Kurz, Gustavo Romero, Ira Weiny, Jason Yan, Joel Stanley, Jordan Niethe, Kajol Jain, Konrad Rzeszutek Wilk, Laurent Dufour, Leonardo Bras, Liu Shixin, Luca Ceresoli, Madhavan Srinivasan, Mahesh Salgaonkar, Nathan Lynch, Nicholas Mc Guire, Nicholas Piggin, Nick Desaulniers, Oliver O'Halloran, Pedro Miraglia Franco de Carvalho, Pratik Rajesh Sampat, Qian Cai, Qinglang Miao, Ravi Bangoria, Russell Currey, Satheesh Rajendran, Scott Cheloha, Segher Boessenkool, Srikar Dronamraju, Stan Johnson, Stephen Kitt, Stephen Rothwell, Thiago Jung Bauermann, Tyrel Datwyler, Vaibhav Jain, Vaidyanathan Srinivasan, Vasant Hegde, Wang Wensheng, Wolfram Sang, Yang Yingliang, zhengbin. - -- Andrew Donnellan (2): powerpc/rtas: Restrict RTAS requests from userspace selftests/powerpc: Add a rtas_filter selftest Aneesh Kumar K.V (11): powerpc/vmemmap: Fix memory leak with vmemmap list allocation failures. powerpc/vmemmap: Don't warn if we don't find a mapping vmemmap list entry powerpc/percpu: Update percpu bootmem allocator powerpc/64/mm: implement page mapping percpu first chunk allocator powerpc/book3s64/hash/4k: Support large linear mapping range with 4K powerpc/mm/book3s: Split radix and hash MAX_PHYSMEM limit powerepc/book3s64/hash: Align start/end address correctly with bolt mapping powerpc/drmem: Make lmb_size 64 bit powerpc/memhotplug: Make lmb size 64bit powerpc/book3s64/radix: Make radix_mem_block_size 64bit powerpc/lmb-size: Use addr #size-cells value when fetching lmb-size Athira Rajeev (1): powerpc/perf: Exclude pmc5/6 from the irrelevant PMU group constraints Biwen Li (2): powerpc/dts/t4240rdb: remove interrupts property powerc/dtc/t1024rdb: remove interrupts property Christoph Hellwig (1): powerpc: switch 85xx defconfigs from legacy ide to libata Christophe Leroy (52): powerpc/32s: Fix assembler warning about r0 powerpc/hwirq: Remove stale forward irq_chip declaration powerpc/irq: Drop forward declaration of struct irqaction powerpc/fpu: Drop cvt_fd() and cvt_df() powerpc: drop hard_reset_now() and poweroff_now() declaration powerpc: Drop _nmask_and_or_msr() powerpc: Remove flush_instruction_cache for book3s/32 powerpc: Move flush_instruction_cache() prototype in asm/cacheflush.h powerpc: Rewrite 4xx flush_cache_instruction() in C powerpc: Rewrite FSL_BOOKE flush_cache_instruction() in C powerpc: Remove
Re: linux-next: manual merge of the char-misc tree with the powerpc tree
Hi all,
On Tue, 6 Oct 2020 18:35:06 +1100 Stephen Rothwell
wrote:
>
> Today's linux-next merge of the char-misc tree got a conflict in:
>
> drivers/misc/ocxl/Kconfig
>
> between commit:
>
> dde6f18a8779 ("ocxl: Don't return trigger page when allocating an
> interrupt")
>
> from the powerpc tree and commit:
>
> 4b53a3c72116 ("ocxl: fix kconfig dependency warning for OCXL")
>
> from the char-misc tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> diff --cc drivers/misc/ocxl/Kconfig
> index 0d815b2a40b3,947294f6d7f4..
> --- a/drivers/misc/ocxl/Kconfig
> +++ b/drivers/misc/ocxl/Kconfig
> @@@ -9,9 -9,8 +9,9 @@@ config OCXL_BAS
>
> config OCXL
> tristate "OpenCAPI coherent accelerator support"
> -depends on PPC_POWERNV && PCI && EEH && HOTPLUG_PCI_POWERNV
> +depends on PPC_POWERNV && PCI && EEH && PPC_XIVE_NATIVE
> ++depends on HOTPLUG_PCI_POWERNV
> select OCXL_BASE
> - select HOTPLUG_PCI_POWERNV
> default m
> help
> Select this option to enable the ocxl driver for Open
This is now a conflict between the powerpc tree and Linus' tree.
--
Cheers,
Stephen Rothwell
pgpEUleqJ3kOy.pgp
Description: OpenPGP digital signature
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Thu, Oct 15, 2020 at 01:15:37PM +0300, Felipe Balbi wrote: > Serge Semin writes: > > > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote: > >> > >> Hi Serge, > >> > >> Serge Semin writes: > >> > In accordance with the DWC USB3 bindings the corresponding node name is > >> > suppose to comply with Generic USB HCD DT schema, which requires the USB > >> > > > >> DWC3 is not a simple HDC, though. > > > > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff, > > which are tuned by the DWC USB3 driver in the kernel. But after that the > > controller is registered as xhci-hcd device so it's serviced by the xHCI > > driver, > > in Dual-role or host-only builds, that's correct. We can also have > peripheral-only builds (both SW or HW versions) which means xhci isn't > even in the picture. It doesn't really matter though, or at least it does for what the new name might be, but the old one currently used is still pretty bad. The DT spec says that the node name is the class of the device. "usb" as the HCD binding mandates is one, but the current nodes currently have completely different names from one DT to another - which is already an issue - and most of them have dwc3 or some variant of it, which doesn't really qualify for a class name. Maxime signature.asc Description: PGP signature
[Bug 195755] rcu_sched detected stalls on CPUs/tasks: (detected by 0, t=6302 jiffies, g=11405, c=11404, q=1880), ppc64, G5
https://bugzilla.kernel.org/show_bug.cgi?id=195755 Marco Descher (ma...@descher.at) changed: What|Removed |Added CC||ma...@descher.at --- Comment #30 from Marco Descher (ma...@descher.at) --- I experienced this problem today, a freeze on an processor : 3 vendor_id : AuthenticAMD cpu family : 22 model : 48 model name : AMD GX-412TC SOC on Debian 10 4.19.0-11-amd64 #1 SMP Debian 4.19.146-1 (2020-09-17) x86_64 GNU/Linux with more and more sysrq messages coming up resulting in the following syslog entry Oct 15 11:43:45 gate kernel: [1545118.045973] rcu: INFO: rcu_sched detected stalls on CPUs/tasks: leading to the system becoming unreachable. Only after a reboot this works again. -- You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 07/20] dt-bindings: usb: xhci: Add Broadcom STB v2 compatible device
On 10/14/20 3:13 AM, Serge Semin wrote: > For some reason the "brcm,xhci-brcm-v2" compatible string has been missing > in the original bindings file. Add it to the Generic xHCI Controllers DT > schema since the controller driver expects it to be supported. > > Signed-off-by: Serge Semin Acked-by: Florian Fainelli -- Florian
Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
On 10/14/20 3:14 AM, Serge Semin wrote: > In accordance with the Generic EHCI/OHCI bindings the corresponding node > name is suppose to comply with the Generic USB HCD DT schema, which > requires the USB nodes to have the name acceptable by the regexp: > "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with > incompatible names. > > Signed-off-by: Serge Semin > > --- > > Please, test the patch out to make sure it doesn't brake the dependent DTS > files. I did only a manual grepping of the possible nodes dependencies. > --- > arch/arm/boot/dts/bcm5301x.dtsi| 4 ++-- > arch/arm/boot/dts/bcm53573.dtsi| 4 ++-- Acked-by: Florian Fainelli -- Florian
[PATCH 3/8] powerpc: Mark functions called inside uaccess blocks w/ 'notrace'
Functions called between user_*_access_begin() and user_*_access_end()
should be either inlined or marked 'notrace' to prevent leaving
userspace access exposed. Mark any such functions relevant to signal
handling so that subsequent patches can call them inside uaccess blocks.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/process.c | 20 ++--
arch/powerpc/mm/mem.c | 4 ++--
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index ba2c987b8403..bf5d9654bd2c 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -84,7 +84,7 @@ extern unsigned long _get_SP(void);
*/
bool tm_suspend_disabled __ro_after_init = false;
-static void check_if_tm_restore_required(struct task_struct *tsk)
+static void notrace check_if_tm_restore_required(struct task_struct *tsk)
{
/*
* If we are saving the current thread's registers, and the
@@ -151,7 +151,7 @@ void notrace __msr_check_and_clear(unsigned long bits)
EXPORT_SYMBOL(__msr_check_and_clear);
#ifdef CONFIG_PPC_FPU
-static void __giveup_fpu(struct task_struct *tsk)
+static void notrace __giveup_fpu(struct task_struct *tsk)
{
unsigned long msr;
@@ -163,7 +163,7 @@ static void __giveup_fpu(struct task_struct *tsk)
tsk->thread.regs->msr = msr;
}
-void giveup_fpu(struct task_struct *tsk)
+void notrace giveup_fpu(struct task_struct *tsk)
{
check_if_tm_restore_required(tsk);
@@ -177,7 +177,7 @@ EXPORT_SYMBOL(giveup_fpu);
* Make sure the floating-point register state in the
* the thread_struct is up to date for task tsk.
*/
-void flush_fp_to_thread(struct task_struct *tsk)
+void notrace flush_fp_to_thread(struct task_struct *tsk)
{
if (tsk->thread.regs) {
/*
@@ -234,7 +234,7 @@ static inline void __giveup_fpu(struct task_struct *tsk) { }
#endif /* CONFIG_PPC_FPU */
#ifdef CONFIG_ALTIVEC
-static void __giveup_altivec(struct task_struct *tsk)
+static void notrace __giveup_altivec(struct task_struct *tsk)
{
unsigned long msr;
@@ -246,7 +246,7 @@ static void __giveup_altivec(struct task_struct *tsk)
tsk->thread.regs->msr = msr;
}
-void giveup_altivec(struct task_struct *tsk)
+void notrace giveup_altivec(struct task_struct *tsk)
{
check_if_tm_restore_required(tsk);
@@ -285,7 +285,7 @@ EXPORT_SYMBOL(enable_kernel_altivec);
* Make sure the VMX/Altivec register state in the
* the thread_struct is up to date for task tsk.
*/
-void flush_altivec_to_thread(struct task_struct *tsk)
+void notrace flush_altivec_to_thread(struct task_struct *tsk)
{
if (tsk->thread.regs) {
preempt_disable();
@@ -300,7 +300,7 @@ EXPORT_SYMBOL_GPL(flush_altivec_to_thread);
#endif /* CONFIG_ALTIVEC */
#ifdef CONFIG_VSX
-static void __giveup_vsx(struct task_struct *tsk)
+static void notrace __giveup_vsx(struct task_struct *tsk)
{
unsigned long msr = tsk->thread.regs->msr;
@@ -317,7 +317,7 @@ static void __giveup_vsx(struct task_struct *tsk)
__giveup_altivec(tsk);
}
-static void giveup_vsx(struct task_struct *tsk)
+static void notrace giveup_vsx(struct task_struct *tsk)
{
check_if_tm_restore_required(tsk);
@@ -352,7 +352,7 @@ void enable_kernel_vsx(void)
}
EXPORT_SYMBOL(enable_kernel_vsx);
-void flush_vsx_to_thread(struct task_struct *tsk)
+void notrace flush_vsx_to_thread(struct task_struct *tsk)
{
if (tsk->thread.regs) {
preempt_disable();
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index ddc32cc1b6cf..da2345a2abc6 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -378,7 +378,7 @@ static inline bool flush_coherent_icache(unsigned long addr)
* @start: the start address
* @stop: the stop address (exclusive)
*/
-static void invalidate_icache_range(unsigned long start, unsigned long stop)
+static void notrace invalidate_icache_range(unsigned long start, unsigned long
stop)
{
unsigned long shift = l1_icache_shift();
unsigned long bytes = l1_icache_bytes();
@@ -402,7 +402,7 @@ static void invalidate_icache_range(unsigned long start,
unsigned long stop)
* @start: the start address
* @stop: the stop address (exclusive)
*/
-void flush_icache_range(unsigned long start, unsigned long stop)
+void notrace flush_icache_range(unsigned long start, unsigned long stop)
{
if (flush_coherent_icache(start))
return;
--
2.28.0
[PATCH 1/8] powerpc/uaccess: Add unsafe_copy_from_user
Implement raw_copy_from_user_allowed() which assumes that userspace read
access is open. Use this new function to implement raw_copy_from_user().
Finally, wrap the new function to follow the usual "unsafe_" convention
of taking a label argument. The new raw_copy_from_user_allowed() calls
__copy_tofrom_user() internally, but this is still safe to call in user
access blocks formed with user_*_access_begin()/user_*_access_end()
since asm functions are not instrumented for tracing.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/include/asm/uaccess.h | 28 +++-
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h
b/arch/powerpc/include/asm/uaccess.h
index 26781b044932..66940b4eb692 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -418,38 +418,45 @@ raw_copy_in_user(void __user *to, const void __user
*from, unsigned long n)
}
#endif /* __powerpc64__ */
-static inline unsigned long raw_copy_from_user(void *to,
- const void __user *from, unsigned long n)
+static inline unsigned long
+raw_copy_from_user_allowed(void *to, const void __user *from, unsigned long n)
{
- unsigned long ret;
if (__builtin_constant_p(n) && (n <= 8)) {
- ret = 1;
+ unsigned long ret = 1;
switch (n) {
case 1:
barrier_nospec();
- __get_user_size(*(u8 *)to, from, 1, ret);
+ __get_user_size_allowed(*(u8 *)to, from, 1, ret);
break;
case 2:
barrier_nospec();
- __get_user_size(*(u16 *)to, from, 2, ret);
+ __get_user_size_allowed(*(u16 *)to, from, 2, ret);
break;
case 4:
barrier_nospec();
- __get_user_size(*(u32 *)to, from, 4, ret);
+ __get_user_size_allowed(*(u32 *)to, from, 4, ret);
break;
case 8:
barrier_nospec();
- __get_user_size(*(u64 *)to, from, 8, ret);
+ __get_user_size_allowed(*(u64 *)to, from, 8, ret);
break;
}
if (ret == 0)
return 0;
}
+ return __copy_tofrom_user((__force void __user *)to, from, n);
+}
+
+static inline unsigned long
+raw_copy_from_user(void *to, const void __user *from, unsigned long n)
+{
+ unsigned long ret;
+
barrier_nospec();
allow_read_from_user(from, n);
- ret = __copy_tofrom_user((__force void __user *)to, from, n);
+ ret = raw_copy_from_user_allowed(to, from, n);
prevent_read_from_user(from, n);
return ret;
}
@@ -571,6 +578,9 @@ user_write_access_begin(const void __user *ptr, size_t len)
#define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
#define unsafe_put_user(x, p, e) __put_user_goto(x, p, e)
+#define unsafe_copy_from_user(d, s, l, e) \
+ unsafe_op_wrap(raw_copy_from_user_allowed(d, s, l), e)
+
#define unsafe_copy_to_user(d, s, l, e) \
do { \
u8 __user *_dst = (u8 __user *)(d); \
--
2.28.0
[PATCH 7/8] powerpc/signal64: Rewrite handle_rt_signal64() to minimise uaccess switches
From: Daniel Axtens
Add uaccess blocks and use the 'unsafe' versions of functions doing user
access where possible to reduce the number of times uaccess has to be
opened/closed.
There is no 'unsafe' version of copy_siginfo_to_user, so move it
slightly to allow for a "longer" uaccess block.
Signed-off-by: Daniel Axtens
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal_64.c | 54 -
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 6d4f7a5c4fbf..3b97e3681a8f 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -843,46 +843,42 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t
*set,
/* Save the thread's msr before get_tm_stackpointer() changes it */
unsigned long msr = regs->msr;
#endif
-
frame = get_sigframe(ksig, tsk, sizeof(*frame), 0);
- if (!access_ok(frame, sizeof(*frame)))
+ if (!user_write_access_begin(frame, sizeof(*frame)))
goto badframe;
- err |= __put_user(>info, >pinfo);
- err |= __put_user(>uc, >puc);
- err |= copy_siginfo_to_user(>info, >info);
- if (err)
- goto badframe;
+ unsafe_put_user(>info, >pinfo, badframe_block);
+ unsafe_put_user(>uc, >puc, badframe_block);
/* Create the ucontext. */
- err |= __put_user(0, >uc.uc_flags);
- err |= __save_altstack(>uc.uc_stack, regs->gpr[1]);
+ unsafe_put_user(0, >uc.uc_flags, badframe_block);
+ unsafe_save_altstack(>uc.uc_stack, regs->gpr[1], badframe_block);
+
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
if (MSR_TM_ACTIVE(msr)) {
/* The ucontext_t passed to userland points to the second
* ucontext_t (for transactional state) with its uc_link ptr.
*/
- err |= __put_user(>uc_transact, >uc.uc_link);
+ unsafe_put_user(>uc_transact, >uc.uc_link,
badframe_block);
+ user_write_access_end();
err |= setup_tm_sigcontexts(>uc.uc_mcontext,
>uc_transact.uc_mcontext,
tsk, ksig->sig, NULL,
(unsigned
long)ksig->ka.sa.sa_handler,
msr);
+ if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
+ goto badframe;
+
} else
#endif
{
- err |= __put_user(0, >uc.uc_link);
-
- if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
- return -EFAULT;
- err |= __unsafe_setup_sigcontext(>uc.uc_mcontext, tsk,
- ksig->sig, NULL,
- (unsigned
long)ksig->ka.sa.sa_handler, 1);
- user_write_access_end();
+ unsafe_put_user(0, >uc.uc_link, badframe_block);
+ unsafe_setup_sigcontext(>uc.uc_mcontext, tsk, ksig->sig,
+ NULL, (unsigned
long)ksig->ka.sa.sa_handler,
+ 1, badframe_block);
}
- err |= __copy_to_user(>uc.uc_sigmask, set, sizeof(*set));
- if (err)
- goto badframe;
+
+ unsafe_copy_to_user(>uc.uc_sigmask, set, sizeof(*set),
badframe_block);
/* Make sure signal handler doesn't get spurious FP exceptions */
tsk->thread.fp_state.fpscr = 0;
@@ -891,15 +887,17 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t
*set,
if (vdso64_rt_sigtramp && tsk->mm->context.vdso_base) {
regs->nip = tsk->mm->context.vdso_base + vdso64_rt_sigtramp;
} else {
- if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
- return -EFAULT;
- err |= __unsafe_setup_trampoline(__NR_rt_sigreturn,
>tramp[0]);
- user_write_access_end();
- if (err)
- goto badframe;
+ unsafe_setup_trampoline(__NR_rt_sigreturn, >tramp[0],
+ badframe_block);
regs->nip = (unsigned long) >tramp[0];
}
+ user_write_access_end();
+
+ /* Save the siginfo outside of the unsafe block. */
+ if (copy_siginfo_to_user(>info, >info))
+ goto badframe;
+
/* 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);
@@ -939,6 +937,8 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
return 0;
+badframe_block:
+ user_write_access_end();
badframe:
signal_fault(current, regs, "handle_rt_signal64", frame);
--
2.28.0
[PATCH 8/8] powerpc/signal64: Rewrite rt_sigreturn() to minimise uaccess switches
From: Daniel Axtens
Add uaccess blocks and use the 'unsafe' versions of functions doing user
access where possible to reduce the number of times uaccess has to be
opened/closed.
Signed-off-by: Daniel Axtens
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal_64.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 3b97e3681a8f..0f4ff7a5bfc1 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -779,18 +779,22 @@ SYSCALL_DEFINE0(rt_sigreturn)
*/
regs->msr &= ~MSR_TS_MASK;
- if (__get_user(msr, >uc_mcontext.gp_regs[PT_MSR]))
+ if (!user_read_access_begin(uc, sizeof(*uc)))
goto badframe;
+
+ unsafe_get_user(msr, >uc_mcontext.gp_regs[PT_MSR], badframe_block);
+
if (MSR_TM_ACTIVE(msr)) {
/* We recheckpoint on return. */
struct ucontext __user *uc_transact;
/* Trying to start TM on non TM system */
if (!cpu_has_feature(CPU_FTR_TM))
- goto badframe;
+ goto badframe_block;
+
+ unsafe_get_user(uc_transact, >uc_link, badframe_block);
+ user_read_access_end();
- if (__get_user(uc_transact, >uc_link))
- goto badframe;
if (restore_tm_sigcontexts(current, >uc_mcontext,
_transact->uc_mcontext))
goto badframe;
@@ -810,12 +814,13 @@ SYSCALL_DEFINE0(rt_sigreturn)
* causing a TM bad thing.
*/
current->thread.regs->msr &= ~MSR_TS_MASK;
+
+#ifndef CONFIG_PPC_TRANSACTIONAL_MEM
if (!user_read_access_begin(uc, sizeof(*uc)))
- return -EFAULT;
- if (__unsafe_restore_sigcontext(current, NULL, 1,
>uc_mcontext)) {
- user_read_access_end();
goto badframe;
- }
+#endif
+ unsafe_restore_sigcontext(current, NULL, 1, >uc_mcontext,
+ badframe_block);
user_read_access_end();
}
@@ -825,6 +830,8 @@ SYSCALL_DEFINE0(rt_sigreturn)
set_thread_flag(TIF_RESTOREALL);
return 0;
+badframe_block:
+ user_read_access_end();
badframe:
signal_fault(current, regs, "rt_sigreturn", uc);
--
2.28.0
[PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx,fpr}_from_user()
Reuse the "safe" implementation from signal.c except for calling
unsafe_copy_from_user() to copy into a local buffer. Unlike the
unsafe_copy_{vsx,fpr}_to_user() functions the "copy from" functions
cannot use unsafe_get_user() directly to bypass the local buffer since
doing so significantly reduces signal handling performance.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal.h | 33 +
1 file changed, 33 insertions(+)
diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
index 2559a681536e..e9aaeac0da37 100644
--- a/arch/powerpc/kernel/signal.h
+++ b/arch/powerpc/kernel/signal.h
@@ -53,6 +53,33 @@ unsigned long copy_ckfpr_from_user(struct task_struct *task,
void __user *from);
[i], label);\
} while (0)
+#define unsafe_copy_fpr_from_user(task, from, label) do {\
+ struct task_struct *__t = task; \
+ u64 __user *__f = (u64 __user *)from; \
+ u64 buf[ELF_NFPREG];\
+ int i; \
+ \
+ unsafe_copy_from_user(buf, __f, ELF_NFPREG * sizeof(double),\
+ label); \
+ for (i = 0; i < ELF_NFPREG - 1; i++)\
+ __t->thread.TS_FPR(i) = buf[i]; \
+ __t->thread.fp_state.fpscr = buf[i];\
+} while (0)
+
+#define unsafe_copy_vsx_from_user(task, from, label) do {\
+ struct task_struct *__t = task; \
+ u64 __user *__f = (u64 __user *)from; \
+ u64 buf[ELF_NVSRHALFREG]; \
+ int i; \
+ \
+ unsafe_copy_from_user(buf, __f, \
+ ELF_NVSRHALFREG * sizeof(double), \
+ label); \
+ for (i = 0; i < ELF_NVSRHALFREG ; i++) \
+ __t->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = buf[i]; \
+} while (0)
+
+
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
#define unsafe_copy_ckfpr_to_user(to, task, label) do {\
struct task_struct *__t = task; \
@@ -80,6 +107,10 @@ unsigned long copy_ckfpr_from_user(struct task_struct
*task, void __user *from);
unsafe_copy_to_user(to, (task)->thread.fp_state.fpr,\
ELF_NFPREG * sizeof(double), label)
+#define unsafe_copy_fpr_from_user(task, from, label) \
+ unsafe_copy_from_user((task)->thread.fp_state.fpr, from \
+ ELF_NFPREG * sizeof(double), label)
+
static inline unsigned long
copy_fpr_to_user(void __user *to, struct task_struct *task)
{
@@ -115,6 +146,8 @@ copy_ckfpr_from_user(struct task_struct *task, void __user
*from)
#else
#define unsafe_copy_fpr_to_user(to, task, label) do { } while (0)
+#define unsafe_copy_fpr_from_user(task, from, label) do { } while (0)
+
static inline unsigned long
copy_fpr_to_user(void __user *to, struct task_struct *task)
{
--
2.28.0
[PATCH 4/8] powerpc/signal64: Replace setup_sigcontext() w/ unsafe_setup_sigcontext()
Previously setup_sigcontext() performed a costly KUAP switch on every
uaccess operation. These repeated uaccess switches cause a significant
drop in signal handling performance.
Rewrite setup_sigcontext() to assume that a userspace write access window
is open. Replace all uaccess functions with their 'unsafe' versions
which avoid the repeated uaccess switches.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal_64.c | 71 -
1 file changed, 44 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 7df088b9ad0f..26934ceeb925 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -83,9 +83,13 @@ static elf_vrreg_t __user *sigcontext_vmx_regs(struct
sigcontext __user *sc)
* Set up the sigcontext for the signal frame.
*/
-static long setup_sigcontext(struct sigcontext __user *sc,
- struct task_struct *tsk, int signr, sigset_t *set,
- unsigned long handler, int ctx_has_vsx_region)
+#define unsafe_setup_sigcontext(sc, tsk, signr, set, handler, \
+ ctx_has_vsx_region, e) \
+ unsafe_op_wrap(__unsafe_setup_sigcontext(sc, tsk, signr, set, \
+ handler, ctx_has_vsx_region), e)
+static long notrace __unsafe_setup_sigcontext(struct sigcontext __user *sc,
+ struct task_struct *tsk, int signr,
sigset_t *set,
+ unsigned long handler, int
ctx_has_vsx_region)
{
/* When CONFIG_ALTIVEC is set, we _always_ setup v_regs even if the
* process never used altivec yet (MSR_VEC is zero in pt_regs of
@@ -101,21 +105,20 @@ static long setup_sigcontext(struct sigcontext __user *sc,
#endif
struct pt_regs *regs = tsk->thread.regs;
unsigned long msr = regs->msr;
- long err = 0;
/* Force usr to alway see softe as 1 (interrupts enabled) */
unsigned long softe = 0x1;
BUG_ON(tsk != current);
#ifdef CONFIG_ALTIVEC
- err |= __put_user(v_regs, >v_regs);
+ unsafe_put_user(v_regs, >v_regs, efault_out);
/* save altivec registers */
if (tsk->thread.used_vr) {
flush_altivec_to_thread(tsk);
/* Copy 33 vec registers (vr0..31 and vscr) to the stack */
- err |= __copy_to_user(v_regs, >thread.vr_state,
- 33 * sizeof(vector128));
+ unsafe_copy_to_user(v_regs, >thread.vr_state,
+ 33 * sizeof(vector128), efault_out);
/* set MSR_VEC in the MSR value in the frame to indicate that
sc->v_reg)
* contains valid data.
*/
@@ -130,13 +133,13 @@ static long setup_sigcontext(struct sigcontext __user *sc,
tsk->thread.vrsave = vrsave;
}
- err |= __put_user(vrsave, (u32 __user *)_regs[33]);
+ unsafe_put_user(vrsave, (u32 __user *)_regs[33], efault_out);
#else /* CONFIG_ALTIVEC */
- err |= __put_user(0, >v_regs);
+ unsafe_put_user(0, >v_regs, efault_out);
#endif /* CONFIG_ALTIVEC */
flush_fp_to_thread(tsk);
/* copy fpr regs and fpscr */
- err |= copy_fpr_to_user(>fp_regs, tsk);
+ unsafe_copy_fpr_to_user(>fp_regs, tsk, efault_out);
/*
* Clear the MSR VSX bit to indicate there is no valid state attached
@@ -152,24 +155,27 @@ static long setup_sigcontext(struct sigcontext __user *sc,
if (tsk->thread.used_vsr && ctx_has_vsx_region) {
flush_vsx_to_thread(tsk);
v_regs += ELF_NVRREG;
- err |= copy_vsx_to_user(v_regs, tsk);
+ unsafe_copy_vsx_to_user(v_regs, tsk, efault_out);
/* set MSR_VSX in the MSR value in the frame to
* indicate that sc->vs_reg) contains valid data.
*/
msr |= MSR_VSX;
}
#endif /* CONFIG_VSX */
- err |= __put_user(>gp_regs, >regs);
+ unsafe_put_user(>gp_regs, >regs, efault_out);
WARN_ON(!FULL_REGS(regs));
- err |= __copy_to_user(>gp_regs, regs, GP_REGS_SIZE);
- err |= __put_user(msr, >gp_regs[PT_MSR]);
- err |= __put_user(softe, >gp_regs[PT_SOFTE]);
- err |= __put_user(signr, >signal);
- err |= __put_user(handler, >handler);
+ unsafe_copy_to_user(>gp_regs, regs, GP_REGS_SIZE, efault_out);
+ unsafe_put_user(msr, >gp_regs[PT_MSR], efault_out);
+ unsafe_put_user(softe, >gp_regs[PT_SOFTE], efault_out);
+ unsafe_put_user(signr, >signal, efault_out);
+ unsafe_put_user(handler, >handler, efault_out);
if (set != NULL)
- err |= __put_user(set->sig[0], >oldmask);
+ unsafe_put_user(set->sig[0], >oldmask, efault_out);
- return err;
+ return 0;
+
+efault_out:
+ return -EFAULT;
}
#ifdef
[PATCH 5/8] powerpc/signal64: Replace restore_sigcontext() w/ unsafe_restore_sigcontext()
Previously restore_sigcontext() performed a costly KUAP switch on every
uaccess operation. These repeated uaccess switches cause a significant
drop in signal handling performance.
Rewrite restore_sigcontext() to assume that a userspace read access
window is open. Replace all uaccess functions with their 'unsafe'
versions which avoid the repeated uaccess switches.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal_64.c | 68 -
1 file changed, 41 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 26934ceeb925..bd92064e5576 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -318,14 +318,14 @@ static long setup_tm_sigcontexts(struct sigcontext __user
*sc,
/*
* Restore the sigcontext from the signal frame.
*/
-
-static long restore_sigcontext(struct task_struct *tsk, sigset_t *set, int sig,
- struct sigcontext __user *sc)
+#define unsafe_restore_sigcontext(tsk, set, sig, sc, e) \
+ unsafe_op_wrap(__unsafe_restore_sigcontext(tsk, set, sig, sc), e)
+static long notrace __unsafe_restore_sigcontext(struct task_struct *tsk,
sigset_t *set,
+ int sig, struct sigcontext
__user *sc)
{
#ifdef CONFIG_ALTIVEC
elf_vrreg_t __user *v_regs;
#endif
- unsigned long err = 0;
unsigned long save_r13 = 0;
unsigned long msr;
struct pt_regs *regs = tsk->thread.regs;
@@ -340,27 +340,28 @@ static long restore_sigcontext(struct task_struct *tsk,
sigset_t *set, int sig,
save_r13 = regs->gpr[13];
/* copy the GPRs */
- err |= __copy_from_user(regs->gpr, sc->gp_regs, sizeof(regs->gpr));
- err |= __get_user(regs->nip, >gp_regs[PT_NIP]);
+ unsafe_copy_from_user(regs->gpr, sc->gp_regs, sizeof(regs->gpr),
+ efault_out);
+ unsafe_get_user(regs->nip, >gp_regs[PT_NIP], efault_out);
/* get MSR separately, transfer the LE bit if doing signal return */
- err |= __get_user(msr, >gp_regs[PT_MSR]);
+ unsafe_get_user(msr, >gp_regs[PT_MSR], efault_out);
if (sig)
regs->msr = (regs->msr & ~MSR_LE) | (msr & MSR_LE);
- err |= __get_user(regs->orig_gpr3, >gp_regs[PT_ORIG_R3]);
- err |= __get_user(regs->ctr, >gp_regs[PT_CTR]);
- err |= __get_user(regs->link, >gp_regs[PT_LNK]);
- err |= __get_user(regs->xer, >gp_regs[PT_XER]);
- err |= __get_user(regs->ccr, >gp_regs[PT_CCR]);
+ unsafe_get_user(regs->orig_gpr3, >gp_regs[PT_ORIG_R3], efault_out);
+ unsafe_get_user(regs->ctr, >gp_regs[PT_CTR], efault_out);
+ unsafe_get_user(regs->link, >gp_regs[PT_LNK], efault_out);
+ unsafe_get_user(regs->xer, >gp_regs[PT_XER], efault_out);
+ unsafe_get_user(regs->ccr, >gp_regs[PT_CCR], efault_out);
/* Don't allow userspace to set SOFTE */
set_trap_norestart(regs);
- err |= __get_user(regs->dar, >gp_regs[PT_DAR]);
- err |= __get_user(regs->dsisr, >gp_regs[PT_DSISR]);
- err |= __get_user(regs->result, >gp_regs[PT_RESULT]);
+ unsafe_get_user(regs->dar, >gp_regs[PT_DAR], efault_out);
+ unsafe_get_user(regs->dsisr, >gp_regs[PT_DSISR], efault_out);
+ unsafe_get_user(regs->result, >gp_regs[PT_RESULT], efault_out);
if (!sig)
regs->gpr[13] = save_r13;
if (set != NULL)
- err |= __get_user(set->sig[0], >oldmask);
+ unsafe_get_user(set->sig[0], >oldmask, efault_out);
/*
* Force reload of FP/VEC.
@@ -370,29 +371,28 @@ static long restore_sigcontext(struct task_struct *tsk,
sigset_t *set, int sig,
regs->msr &= ~(MSR_FP | MSR_FE0 | MSR_FE1 | MSR_VEC | MSR_VSX);
#ifdef CONFIG_ALTIVEC
- err |= __get_user(v_regs, >v_regs);
- if (err)
- return err;
+ unsafe_get_user(v_regs, >v_regs, efault_out);
if (v_regs && !access_ok(v_regs, 34 * sizeof(vector128)))
return -EFAULT;
/* Copy 33 vec registers (vr0..31 and vscr) from the stack */
if (v_regs != NULL && (msr & MSR_VEC) != 0) {
- err |= __copy_from_user(>thread.vr_state, v_regs,
- 33 * sizeof(vector128));
+ unsafe_copy_from_user(>thread.vr_state, v_regs,
+ 33 * sizeof(vector128), efault_out);
tsk->thread.used_vr = true;
} else if (tsk->thread.used_vr) {
memset(>thread.vr_state, 0, 33 * sizeof(vector128));
}
/* Always get VRSAVE back */
if (v_regs != NULL)
- err |= __get_user(tsk->thread.vrsave, (u32 __user
*)_regs[33]);
+ unsafe_get_user(tsk->thread.vrsave, (u32 __user *)_regs[33],
+ efault_out);
else
tsk->thread.vrsave = 0;
if
[PATCH 6/8] powerpc/signal64: Replace setup_trampoline() w/ unsafe_setup_trampoline()
From: Daniel Axtens
Previously setup_trampoline() performed a costly KUAP switch on every
uaccess operation. These repeated uaccess switches cause a significant
drop in signal handling performance.
Rewrite setup_trampoline() to assume that a userspace write access
window is open. Replace all uaccess functions with their 'unsafe'
versions to avoid the repeated uaccess switches.
Signed-off-by: Daniel Axtens
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal_64.c | 32 +++-
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index bd92064e5576..6d4f7a5c4fbf 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -600,30 +600,33 @@ static long restore_tm_sigcontexts(struct task_struct
*tsk,
/*
* Setup the trampoline code on the stack
*/
-static long setup_trampoline(unsigned int syscall, unsigned int __user *tramp)
+#define unsafe_setup_trampoline(syscall, tramp, e) \
+ unsafe_op_wrap(__unsafe_setup_trampoline(syscall, tramp), e)
+static long notrace __unsafe_setup_trampoline(unsigned int syscall,
+ unsigned int __user *tramp)
{
int i;
- long err = 0;
/* bctrl # call the handler */
- err |= __put_user(PPC_INST_BCTRL, [0]);
+ unsafe_put_user(PPC_INST_BCTRL, [0], err);
/* addi r1, r1, __SIGNAL_FRAMESIZE # Pop the dummy stackframe */
- err |= __put_user(PPC_INST_ADDI | __PPC_RT(R1) | __PPC_RA(R1) |
- (__SIGNAL_FRAMESIZE & 0x), [1]);
+ unsafe_put_user(PPC_INST_ADDI | __PPC_RT(R1) | __PPC_RA(R1) |
+ (__SIGNAL_FRAMESIZE & 0x), [1], err);
/* li r0, __NR_[rt_]sigreturn| */
- err |= __put_user(PPC_INST_ADDI | (syscall & 0x), [2]);
+ unsafe_put_user(PPC_INST_ADDI | (syscall & 0x), [2], err);
/* sc */
- err |= __put_user(PPC_INST_SC, [3]);
+ unsafe_put_user(PPC_INST_SC, [3], err);
/* Minimal traceback info */
for (i=TRAMP_TRACEBACK; i < TRAMP_SIZE ;i++)
- err |= __put_user(0, [i]);
+ unsafe_put_user(0, [i], err);
- if (!err)
- flush_icache_range((unsigned long) [0],
- (unsigned long) [TRAMP_SIZE]);
+ flush_icache_range((unsigned long)[0],
+ (unsigned long)[TRAMP_SIZE]);
- return err;
+ return 0;
+err:
+ return 1;
}
/*
@@ -888,7 +891,10 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
if (vdso64_rt_sigtramp && tsk->mm->context.vdso_base) {
regs->nip = tsk->mm->context.vdso_base + vdso64_rt_sigtramp;
} else {
- err |= setup_trampoline(__NR_rt_sigreturn, >tramp[0]);
+ if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
+ return -EFAULT;
+ err |= __unsafe_setup_trampoline(__NR_rt_sigreturn,
>tramp[0]);
+ user_write_access_end();
if (err)
goto badframe;
regs->nip = (unsigned long) >tramp[0];
--
2.28.0
[PATCH 0/8] Improve signal performance on PPC64 with KUAP
As reported by Anton, there is a large penalty to signal handling
performance on radix systems using KUAP. The signal handling code
performs many user access operations, each of which needs to switch the
KUAP permissions bit to open and then close user access. This involves a
costly 'mtspr' operation [0].
There is existing work done on x86 and by Christopher Leroy for PPC32 to
instead open up user access in "blocks" using user_*_access_{begin,end}.
We can do the same in PPC64 to bring performance back up on KUAP-enabled
radix systems.
This series applies on top of Christophe Leroy's work for PPC32 [1] (I'm
sure patchwork won't be too happy about that).
The first two patches add some needed 'unsafe' versions of copy-from
functions. While these do not make use of asm-goto they still allow for
avoiding the repeated uaccess switches. The third patch adds 'notrace'
to any functions expected to be called in a uaccess block context.
Normally functions called in such a context should be inlined, but this
is not feasible everywhere. Marking them 'notrace' should provide _some_
protection against leaving the user access window open.
The next three patches rewrite some of the signal64 helper functions to
be 'unsafe'. Finally, the last two patches update the main signal
handling functions to make use of the new 'unsafe' helpers and eliminate
some additional uaccess switching.
I used the will-it-scale signal1 benchmark to measure and compare
performance [2]. The below results are from a P9 Blackbird system. Note
that currently hash does not support KUAP and is therefore used as the
"baseline" comparison. Bigger numbers are better:
signal1_threads -t1 -s10
| | hash | radix |
| --- | -- | -- |
| linuxppc/next | 289014 | 158408 |
| unsafe-signal64 | 298506 | 253053 |
[0]: https://github.com/linuxppc/issues/issues/277
[1]: https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=196278
[2]: https://github.com/antonblanchard/will-it-scale/blob/master/tests/signal1.c
Christopher M. Riedl (5):
powerpc/uaccess: Add unsafe_copy_from_user
powerpc/signal: Add unsafe_copy_{vsx,fpr}_from_user()
powerpc: Mark functions called inside uaccess blocks w/ 'notrace'
powerpc/signal64: Replace setup_sigcontext() w/
unsafe_setup_sigcontext()
powerpc/signal64: Replace restore_sigcontext() w/
unsafe_restore_sigcontext()
Daniel Axtens (3):
powerpc/signal64: Replace setup_trampoline() w/
unsafe_setup_trampoline()
powerpc/signal64: Rewrite handle_rt_signal64() to minimise uaccess
switches
powerpc/signal64: Rewrite rt_sigreturn() to minimise uaccess switches
arch/powerpc/include/asm/uaccess.h | 28 ++--
arch/powerpc/kernel/process.c | 20 +--
arch/powerpc/kernel/signal.h | 33 +
arch/powerpc/kernel/signal_64.c| 216 +
arch/powerpc/mm/mem.c | 4 +-
5 files changed, 194 insertions(+), 107 deletions(-)
--
2.28.0
Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
On 10/15/20 3:05 AM, Catalin Marinas wrote: > On Wed, Oct 14, 2020 at 03:21:16PM -0600, Khalid Aziz wrote: >> What FreeBSD does seems like a reasonable thing to do. Any way first >> thing to do is to update sparc to use arch_validate_flags() and update >> sparc_validate_prot() to not peek into vma without lock. > > If you go for arch_validate_flags(), I think sparc_validate_prot() > doesn't need the vma at all. Yes, the plan is to move vma flag check from sparc_validate_prot() to arch_validate_flags().. > > BTW, on the ADI topic, I think you have a race in do_swap_page() since > set_pte_at() is called before arch_do_swap_page(). So a thread in the > same process would see the new mapping but the tags have not been > updated yet. Unless sparc relies on the new user pte to be set, I think > you can just swap the two calls. > Thanks for pointing that out. I will take a look at it. -- Khalid
Re: [PATCH v6 02/11] mm/gup: Use functions to track lockless pgtbl walks on gup_pgd_range
Hello, On Thu, Feb 06, 2020 at 12:25:18AM -0300, Leonardo Bras wrote: > On Thu, 2020-02-06 at 00:08 -0300, Leonardo Bras wrote: > > gup_pgd_range(addr, end, gup_flags, pages, ); > > - local_irq_enable(); > > + end_lockless_pgtbl_walk(IRQS_ENABLED); > > ret = nr; > > } > > > > Just noticed IRQS_ENABLED is not available on other archs than ppc64. > I will fix this for v7. Has threre been v7? I cannot find it. Thanks Michal
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Thu, Oct 15, 2020 at 08:14:39AM +0200, Krzysztof Kozlowski wrote: > On Thu, Oct 15, 2020 at 02:51:05AM +0300, Serge Semin wrote: > > > > > > > So to speak thanks for suggesting it. I'll try it to validate the > > > > proposed > > > > changes. > > > > > > > > Two questions: > > > > 1) Any advise of a good inliner/command to compile all dtbs at once? Of > > > > course I > > > > can get all the updated dtsi'es, then find out all the dts'es which > > > > include > > > > them, then directly use dtc to compile the found dts'es... On the other > > > > hand I > > > > can just compile all dts'es, then compare old and new ones. The diff of > > > > the > > > > non-modified dtb'es will be just empty... > > > > > > > > make dtbs > > > > It's not that easy.) "make dtbs" will build dtbs only for enabled boards, > > which > > first need to be enabled in the kernel config. So I'll need to have a config > > with all the affected dts. The later is the same as if I just found all the > > affected dts and built them one-by-one by directly calling dtc. > > True. Sometimes allyesconfig for given arch might be helpful but not > always (e.g. for ARM it does not select all of ARMv4 and ARMv5 boards). > Most likely your approach is actually faster/more reliable. > > > > > > touch your dts or git stash pop > > > make dtbs > > > compare > > > diff for all unchanged will be simply empty, so easy to spot > > > > > > > 2) What crosc64 is? > > > > > > Ah, just an alias for cross compiling + ccache + kbuild out. I just > > > copied you my helpers, so you need to tweak them. > > > > > > > > > > > > > > > > > 2. Split it per arm architectures (and proper subject prefix - not > > > > > "arch") and subarchitectures so maintainers can pick it up. > > > > > > > > Why? The changes are simple and can be formatted as a single patch. > > > > I've seen > > > > tons of patches submitted like that, accepted and then merged. What you > > > > suggest > > > > is just much more work, which I don't see quite required. > > > > > > > > DTS changes go separate between arm64 and arm. There is nothing > > > unusual here - all changes are submitted like this. > > > Second topic is to split by subarchitectures which is necessary if you > > > want it to be picked up by maintainers. It also makes it easier to > > > review. > > > > The current patches are easy enough for review. The last three patches of > > the > > series is a collection of the one-type changes concerning the same type of > > nodes. So reviewing them won't cause any difficulty. But I assume that's not > > the main point in this discussion. > > > > > Sure, without split ber subarchitectures this could be picked > > > up by SoC folks but you did not even CC them. So if you do not want to > > > split it per subarchitectures for maintainers and you do not CC SoC, > > > then how do you believe this should be picked up? Out of the regular > > > patch submission way? That's not how the changes are handled. > > > > AFAIU there are another ways of merging comprehensive patches. If they get > > to collect > > all the Acked-by tags, they could be merged in, for instance, through Greg' > > or Rob' > > (for dts) repos, if of course they get to agree with doing that. Am I wrong? > > > > My hope was to ask Rob or Greg to get the patches merged in when they get > > to collect all the ackes, since I thought it was an option in such cases. > > So if > > they refuse to do so I'll have no choice but to split the series up into a > > smaller patches as you say. > > This is neither Rob's nor Greg's patch to pick up, but ARM SoC (which was > not CCed here). And most likely they won't pick it up because judging by > contents it is obvious it should go via ARM SoC. > > Sure, if there are dependencies between some patches they can go with > acks through unrelated trees, but this not the usual way. This is an > exception in the process to solve particular dependency problem. It has > drawbacks - increases the chances of annoying conflicts. > > The case here does not fall into this criteria - there is no dependency > of this patch on the others Therefore there is no reason to use the > unusual/exceptional way of handling patches. There is no reason why > this shouldn't go via either specific ARM subarchitecture maintainers or > via ARM SoC. Ok. I see your point. To sum it up I've studied the git log arch/ commit messages and it turns out even Rob has to split the cleanup changes like this ones. So thanks for your patience with stating your point. I'll split the last three patches up to be merged in via the corresponding archs/subarch'es repos. -Sergey > > > > > > 3. The subject title could be more accurate - there is no fix here > > > > > because there was no errors in the first place. Requirement of DWC > > > > > node names comes recently, so it is more alignment with dtschema. > > > > > Otherwise automatic-pickup-stable-bot might want to pick up... and it > > > > > should not go to
Re: [PATCH] powerpc: force inlining of csum_partial() to avoid multiple csum_partial() with GCC10
Le 15/10/2020 à 15:25, Segher Boessenkool a écrit : Hi! On Thu, Oct 15, 2020 at 10:52:20AM +, Christophe Leroy wrote: With gcc9 I get: With gcc10 I get: gcc10 defines multiple versions of csum_partial() which are just an unconditionnal branch to __csum_partial(). It doesn't inline it, yes. Could you open a GCC PR for this please? https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97445 Christophe
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Thu, Oct 15, 2020 at 01:15:37PM +0300, Felipe Balbi wrote: > Serge Semin writes: > > > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote: > >> > >> Hi Serge, > >> > >> Serge Semin writes: > >> > In accordance with the DWC USB3 bindings the corresponding node name is > >> > suppose to comply with Generic USB HCD DT schema, which requires the USB > >> > > > >> DWC3 is not a simple HDC, though. > > > > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff, > > which are tuned by the DWC USB3 driver in the kernel. But after that the > > controller is registered as xhci-hcd device so it's serviced by the xHCI > > driver, > > in Dual-role or host-only builds, that's correct. We can also have > peripheral-only builds (both SW or HW versions) which means xhci isn't > even in the picture. Hm, good point. In that case perhaps we'll need to apply the xHCI DT schema conditionally. Like this: - allOf: - - $ref: usb-xhci.yaml# + allOf: + - if: + properties: + dr_mode: + const: peripheral + then: + $ref: usb-hcd.yaml# + else: + $ref: usb-xhci.yaml# Note, we need to have the peripheral device being compatible with the usb-hcd.yaml schema to support the maximum-speed, dr_mode properties and to comply with the USB node naming constraint. -Sergey > > -- > balbi
Re: [PATCH] powerpc: force inlining of csum_partial() to avoid multiple csum_partial() with GCC10
Le 15/10/2020 à 15:25, Segher Boessenkool a écrit : Hi! On Thu, Oct 15, 2020 at 10:52:20AM +, Christophe Leroy wrote: With gcc9 I get: With gcc10 I get: gcc10 defines multiple versions of csum_partial() which are just an unconditionnal branch to __csum_partial(). It doesn't inline it, yes. Could you open a GCC PR for this please? Sure. I also have get_order() 75 times in my vmlinux, all the same as the following: c0016790 : c0016790: 38 63 ff ff addir3,r3,-1 c0016794: 54 63 a3 3e rlwinm r3,r3,20,12,31 c0016798: 7c 63 00 34 cntlzw r3,r3 c001679c: 20 63 00 20 subfic r3,r3,32 c00167a0: 4e 80 00 20 blr Christophe
Re: [PATCH] powerpc: force inlining of csum_partial() to avoid multiple csum_partial() with GCC10
Hi! On Thu, Oct 15, 2020 at 10:52:20AM +, Christophe Leroy wrote: > With gcc9 I get: > With gcc10 I get: > gcc10 defines multiple versions of csum_partial() which are just > an unconditionnal branch to __csum_partial(). It doesn't inline it, yes. Could you open a GCC PR for this please? > To enforce inlining of that branch to __csum_partial(), > mark csum_partial() as __always_inline. That should be fine of course, but it would be good if we could fix this in the compiler :-) Reviewed-by: Segher Boessenkool Segher
Re: [PATCH v2] ima: defer arch_ima_get_secureboot() call to IMA init time
On Wed, Oct 14, 2020 at 07:38:50AM -0400, Mimi Zohar wrote: > On Wed, 2020-10-14 at 17:35 +0800, Chester Lin wrote: > > Hi Ard & Mimi, > > > > On Tue, Oct 13, 2020 at 06:59:21PM +0200, Ard Biesheuvel wrote: > > > On Tue, 13 Oct 2020 at 18:46, Mimi Zohar wrote: > > > > > > > > [Cc'ing linuxppc-dev@lists.ozlabs.org] > > > > > > > > On Tue, 2020-10-13 at 10:18 +0200, Ard Biesheuvel wrote: > > > > > Chester reports that it is necessary to introduce a new way to pass > > > > > the EFI secure boot status between the EFI stub and the core kernel > > > > > on ARM systems. The usual way of obtaining this information is by > > > > > checking the SecureBoot and SetupMode EFI variables, but this can > > > > > only be done after the EFI variable workqueue is created, which > > > > > occurs in a subsys_initcall(), whereas arch_ima_get_secureboot() > > > > > is called much earlier by the IMA framework. > > > > > > > > > > However, the IMA framework itself is started as a late_initcall, > > > > > and the only reason the call to arch_ima_get_secureboot() occurs > > > > > so early is because it happens in the context of a __setup() > > > > > callback that parses the ima_appraise= command line parameter. > > > > > > > > > > So let's refactor this code a little bit, by using a core_param() > > > > > callback to capture the command line argument, and deferring any > > > > > reasoning based on its contents to the IMA init routine. > > > > > > > > > > Cc: Chester Lin > > > > > Cc: Mimi Zohar > > > > > Cc: Dmitry Kasatkin > > > > > Cc: James Morris > > > > > Cc: "Serge E. Hallyn" > > > > > Link: > > > > > https://lore.kernel.org/linux-arm-kernel/20200904072905.25332-2-c...@suse.com/ > > > > > Signed-off-by: Ard Biesheuvel > > > > > --- > > > > > v2: rebase onto series 'integrity: improve user feedback for invalid > > > > > bootparams' > > > > > > > > Thanks, Ard. Based on my initial, limited testing on Power, it looks > > > > good, but I'm hesistant to include it in the integrity 5.10 pull > > > > request without it having been in linux-next and some additional > > > > testing. It's now queued in the next-integrity-testing branch awaiting > > > > some tags. > > > > > > > > Tested-by: Chester Lin > > > > I have tested this patch on x86 VM. > > > > * System configuration: > > - Platform: QEMU/KVM > > - Firmware: EDK2/OVMF + secure boot enabled. > > - OS: SLE15-SP2 + SUSE's kernel-vanilla (=linux v5.9) + the follow commits > > from linux-next and upstream: > > * [PATCH v2] ima: defer arch_ima_get_secureboot() call to IMA init time > > https://www.spinics.net/lists/linux-efi/msg20645.html > > * e4d7e2df3a09 "ima: limit secure boot feedback scope for appraise" > > * 7fe2bb7e7e5c "integrity: invalid kernel parameters feedback" > > * 4afb28ab03d5 "ima: add check for enforced appraise option" > > > > * Logs with UEFI secure boot enabled: > > > > [0.00] Linux version 5.9.0-858-g865c50e1d279-1.g8764d18-vanilla > > (geeko@b > > uildhost) (gcc (SUSE Linux) 10.2.1 20200825 [revision > > c0746a1beb1ba073c7981eb09f > > 55b3d993b32e5c], GNU ld (GNU Binutils; openSUSE Tumbleweed) > > 2.34.0.20200325-1) # > > 1 SMP Wed Oct 14 04:00:11 UTC 2020 (8764d18) > > [0.00] Command line: > > BOOT_IMAGE=/boot/vmlinuz-5.9.0-858-g865c50e1d279-1. > > g8764d18-vanilla root=UUID=5304c03e-4d8a-4d67-873a-32a32e57cdeb > > console=ttyS0,11 > > 5200 resume=/dev/disk/by-path/pci-:04:00.0-part4 mitigations=auto > > ignore_log > > level crashkernel=192M,high crashkernel=72M,low ima_appraise=off > > [0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating > > point regi > > sters' > > [0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers' > > [0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers' > > > > > > [1.720309] ima: Secure boot enabled: ignoring ima_appraise=off option > > [1.720314] ima: No TPM chip found, activating TPM-bypass! > > [1.722129] ima: Allocated hash algorithm: sha256 > > > Thank you for testing the options aren't being set in secure boot mode. > My main concern, however, is that IMA doesn't go into TPM-bypass mode. > Does this system have a TPM? > > Mimi > Last time I didn't emulate a TPM device in KVM so that's why the kernel couldn't find a TPM chip. I have tested this patch again with a virtual TPM 1.2 by running swtpm and here are some logs: - [0.00] Linux version 5.9.0-858-g865c50e1d279-1.g8764d18-vanilla (geeko@b uildhost) (gcc (SUSE Linux) 10.2.1 20200825 [revision c0746a1beb1ba073c7981eb09f 55b3d993b32e5c], GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.34.0.20200325-1) # 1 SMP Wed Oct 14 04:00:11 UTC 2020 (8764d18) [0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-5.9.0-858-g865c50e1d279-1. g8764d18-vanilla root=UUID=5304c03e-4d8a-4d67-873a-32a32e57cdeb console=ttyS0,11 5200 resume=/dev/disk/by-path/pci-:04:00.0-part4 mitigations=auto
[PATCH] powerpc: force inlining of csum_partial() to avoid multiple csum_partial() with GCC10
ppc-linux-objdump -d vmlinux | grep -e "" -e "<__csum_partial>" With gcc9 I get: c0017ef8 <__csum_partial>: c00182fc: 4b ff fb fd bl c0017ef8 <__csum_partial> c0018478: 4b ff fa 80 b c0017ef8 <__csum_partial> c03e8458: 4b c2 fa a0 b c0017ef8 <__csum_partial> c03e8518: 4b c2 f9 e1 bl c0017ef8 <__csum_partial> c03ef410: 4b c2 8a e9 bl c0017ef8 <__csum_partial> c03f0b24: 4b c2 73 d5 bl c0017ef8 <__csum_partial> c04279a4: 4b bf 05 55 bl c0017ef8 <__csum_partial> c0429820: 4b be e6 d9 bl c0017ef8 <__csum_partial> c0429944: 4b be e5 b5 bl c0017ef8 <__csum_partial> c042b478: 4b be ca 81 bl c0017ef8 <__csum_partial> c042b554: 4b be c9 a5 bl c0017ef8 <__csum_partial> c045f15c: 4b bb 8d 9d bl c0017ef8 <__csum_partial> c0492190: 4b b8 5d 69 bl c0017ef8 <__csum_partial> c0492310: 4b b8 5b e9 bl c0017ef8 <__csum_partial> c0495594: 4b b8 29 65 bl c0017ef8 <__csum_partial> c049c420: 4b b7 ba d9 bl c0017ef8 <__csum_partial> c049c870: 4b b7 b6 89 bl c0017ef8 <__csum_partial> c049c930: 4b b7 b5 c9 bl c0017ef8 <__csum_partial> c04a9ca0: 4b b6 e2 59 bl c0017ef8 <__csum_partial> c04bdde4: 4b b5 a1 15 bl c0017ef8 <__csum_partial> c04be480: 4b b5 9a 79 bl c0017ef8 <__csum_partial> c04be710: 4b b5 97 e9 bl c0017ef8 <__csum_partial> c04c969c: 4b b4 e8 5d bl c0017ef8 <__csum_partial> c04ca2fc: 4b b4 db fd bl c0017ef8 <__csum_partial> c04cf5bc: 4b b4 89 3d bl c0017ef8 <__csum_partial> c04d0440: 4b b4 7a b9 bl c0017ef8 <__csum_partial> With gcc10 I get: c0018d08 <__csum_partial>: c0019020 : c0019020: 4b ff fc e8 b c0018d08 <__csum_partial> c001914c: 4b ff fe d4 b c0019020 c0019250: 4b ff fd d1 bl c0019020 c03e404c : c03e404c: 4b c3 4c bc b c0018d08 <__csum_partial> c03e4050: 4b ff ff fc b c03e404c c03e40fc: 4b ff ff 51 bl c03e404c c03e6680: 4b ff d9 cd bl c03e404c c03e68c4: 4b ff d7 89 bl c03e404c c03e7934: 4b ff c7 19 bl c03e404c c03e7bf8: 4b ff c4 55 bl c03e404c c03eb148: 4b ff 8f 05 bl c03e404c c03ecf68: 4b c2 bd a1 bl c0018d08 <__csum_partial> c04275b8 : c04275b8: 4b bf 17 50 b c0018d08 <__csum_partial> c0427884: 4b ff fd 35 bl c04275b8 c0427b18: 4b ff fa a1 bl c04275b8 c0427bd8: 4b ff f9 e1 bl c04275b8 c0427cd4: 4b ff f8 e5 bl c04275b8 c0427e34: 4b ff f7 85 bl c04275b8 c045a1c0: 4b bb eb 49 bl c0018d08 <__csum_partial> c0489464 : c0489464: 4b b8 f8 a4 b c0018d08 <__csum_partial> c04896b0: 4b ff fd b5 bl c0489464 c048982c: 4b ff fc 39 bl c0489464 c0490158: 4b b8 8b b1 bl c0018d08 <__csum_partial> c0492f0c : c0492f0c: 4b b8 5d fc b c0018d08 <__csum_partial> c049326c: 4b ff fc a1 bl c0492f0c c049333c: 4b ff fb d1 bl c0492f0c c0493b18: 4b ff f3 f5 bl c0492f0c c0493f50: 4b ff ef bd bl c0492f0c c0493ffc: 4b ff ef 11 bl c0492f0c c04a0f78: 4b b7 7d 91 bl c0018d08 <__csum_partial> c04b3e3c: 4b b6 4e cd bl c0018d08 <__csum_partial> c04b40d0 : c04b40d0: 4b b6 4c 38 b c0018d08 <__csum_partial> c04b4448: 4b ff fc 89 bl c04b40d0 c04b46f4: 4b ff f9 dd bl c04b40d0 c04bf448: 4b b5 98 c0 b c0018d08 <__csum_partial> c04c5264: 4b b5 3a a5 bl c0018d08 <__csum_partial> c04c61e4: 4b b5 2b 25 bl c0018d08 <__csum_partial> gcc10 defines multiple versions of csum_partial() which are just an unconditionnal branch to __csum_partial(). To enforce inlining of that branch to __csum_partial(), mark csum_partial() as __always_inline. With this patch with gcc10: c0018d08 <__csum_partial>: c0019148: 4b ff fb c0 b c0018d08 <__csum_partial> c001924c: 4b ff fa bd bl c0018d08
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
Serge Semin writes: > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote: >> >> Hi Serge, >> >> Serge Semin writes: >> > In accordance with the DWC USB3 bindings the corresponding node name is >> > suppose to comply with Generic USB HCD DT schema, which requires the USB >> > >> DWC3 is not a simple HDC, though. > > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff, > which are tuned by the DWC USB3 driver in the kernel. But after that the > controller is registered as xhci-hcd device so it's serviced by the xHCI > driver, in Dual-role or host-only builds, that's correct. We can also have peripheral-only builds (both SW or HW versions) which means xhci isn't even in the picture. -- balbi signature.asc Description: PGP signature
Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
Hi Serge,
On 10/14/20 12:14 PM, Serge Semin wrote:
In accordance with the Generic EHCI/OHCI bindings the corresponding node
name is suppose to comply with the Generic USB HCD DT schema, which
requires the USB nodes to have the name acceptable by the regexp:
"^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with
incompatible names.
Signed-off-by: Serge Semin
diff --git a/arch/arm/boot/dts/stm32mp151.dtsi
b/arch/arm/boot/dts/stm32mp151.dtsi
index bfe29023fbd5..576f7da564c5 100644
--- a/arch/arm/boot/dts/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/stm32mp151.dtsi
@@ -1404,7 +1404,7 @@ ethernet0: ethernet@5800a000 {
status = "disabled";
};
- usbh_ohci: usbh-ohci@5800c000 {
+ usbh_ohci: usb@5800c000 {
compatible = "generic-ohci";
reg = <0x5800c000 0x1000>;
clocks = < USBH>;
@@ -1413,7 +1413,7 @@ usbh_ohci: usbh-ohci@5800c000 {
status = "disabled";
};
- usbh_ehci: usbh-ehci@5800d000 {
+ usbh_ehci: usb@5800d000 {
compatible = "generic-ehci";
reg = <0x5800d000 0x1000>;
clocks = < USBH>;
For STM32MP151:
Acked-by: Amelie Delaunay
Thanks,
Amelie
Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
On Wed, Oct 14, 2020 at 03:21:16PM -0600, Khalid Aziz wrote: > On 10/13/20 3:16 AM, Catalin Marinas wrote: > > On Mon, Oct 12, 2020 at 01:14:50PM -0600, Khalid Aziz wrote: > >> On 10/12/20 11:22 AM, Catalin Marinas wrote: > >>> On Mon, Oct 12, 2020 at 11:03:33AM -0600, Khalid Aziz wrote: > On 10/10/20 5:09 AM, Catalin Marinas wrote: > > I still think sparc should avoid walking the vmas in > > arch_validate_prot(). The core code already has the vmas, though not > > when calling arch_validate_prot(). That's one of the reasons I added > > arch_validate_flags() with the MTE patches. For sparc, this could be > > (untested, just copied the arch_validate_prot() code): > > I am little uncomfortable with the idea of validating protection bits > inside the VMA walk loop in do_mprotect_pkey(). When ADI is being > enabled across multiple VMAs and arch_validate_flags() fails on a VMA > later, do_mprotect_pkey() will bail out with error leaving ADI enabled > on earlier VMAs. This will apply to protection bits other than ADI as > well of course. This becomes a partial failure of mprotect() call. I > think it should be all or nothing with mprotect() - when one calls > mprotect() from userspace, either the entire address range passed in > gets its protection bits updated or none of it does. That requires > validating protection bits upfront or undoing what earlier iterations of > VMA walk loop might have done. > >>> > >>> I thought the same initially but mprotect() already does this with the > >>> VM_MAY* flag checking. If you ask it for an mprotect() that crosses > >>> multiple vmas and one of them fails, it doesn't roll back the changes to > >>> the prior ones. I considered that a similar approach is fine for MTE > >>> (it's most likely a user error). > >> > >> You are right about the current behavior with VM_MAY* flags, but that is > >> not the right behavior. Adding more cases to this just perpetuates > >> incorrect behavior. It is not easy to roll back changes after VMAs have > >> potentially been split/merged which is probably why the current code > >> simply throws in the towel and returns with partially modified address > >> space. It is lot easier to do all the checks upfront and then proceed or > >> not proceed with modifying VMAs. One approach might be to call > >> arch_validate_flags() in a loop before modifying VMAs and walk all VMAs > >> with a read lock held. Current code also bails out with ENOMEM if it > >> finds a hole in the address range and leaves any modifications already > >> made in place. This is another case where a hole could have been > >> detected earlier. > > > > This should be ideal indeed though with the risk of breaking the current > > ABI (FWIW, FreeBSD seems to do a first pass to check for violations: > > https://github.com/freebsd/freebsd/blob/master/sys/vm/vm_map.c#L2630). > > I am not sure I understand where the ABI breakage would be. Are we aware > of apps that intentionally modify address space partially using the > current code? I hope there aren't any but you never know until you make the change and someone complains. Arguably, such user code is already broken since mprotect() doesn't even tell where the failure occurred. > What FreeBSD does seems like a reasonable thing to do. Any way first > thing to do is to update sparc to use arch_validate_flags() and update > sparc_validate_prot() to not peek into vma without lock. If you go for arch_validate_flags(), I think sparc_validate_prot() doesn't need the vma at all. BTW, on the ADI topic, I think you have a race in do_swap_page() since set_pte_at() is called before arch_do_swap_page(). So a thread in the same process would see the new mapping but the tags have not been updated yet. Unless sparc relies on the new user pte to be set, I think you can just swap the two calls. -- Catalin
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Thu, Oct 15, 2020 at 02:51:05AM +0300, Serge Semin wrote: > > > > > So to speak thanks for suggesting it. I'll try it to validate the proposed > > > changes. > > > > > > Two questions: > > > 1) Any advise of a good inliner/command to compile all dtbs at once? Of > > > course I > > > can get all the updated dtsi'es, then find out all the dts'es which > > > include > > > them, then directly use dtc to compile the found dts'es... On the other > > > hand I > > > can just compile all dts'es, then compare old and new ones. The diff of > > > the > > > non-modified dtb'es will be just empty... > > > > > make dtbs > > It's not that easy.) "make dtbs" will build dtbs only for enabled boards, > which > first need to be enabled in the kernel config. So I'll need to have a config > with all the affected dts. The later is the same as if I just found all the > affected dts and built them one-by-one by directly calling dtc. True. Sometimes allyesconfig for given arch might be helpful but not always (e.g. for ARM it does not select all of ARMv4 and ARMv5 boards). Most likely your approach is actually faster/more reliable. > > > touch your dts or git stash pop > > make dtbs > > compare > > diff for all unchanged will be simply empty, so easy to spot > > > > > 2) What crosc64 is? > > > > Ah, just an alias for cross compiling + ccache + kbuild out. I just > > copied you my helpers, so you need to tweak them. > > > > > > > > > > > > > 2. Split it per arm architectures (and proper subject prefix - not > > > > "arch") and subarchitectures so maintainers can pick it up. > > > > > > Why? The changes are simple and can be formatted as a single patch. I've > > > seen > > > tons of patches submitted like that, accepted and then merged. What you > > > suggest > > > is just much more work, which I don't see quite required. > > > > > DTS changes go separate between arm64 and arm. There is nothing > > unusual here - all changes are submitted like this. > > Second topic is to split by subarchitectures which is necessary if you > > want it to be picked up by maintainers. It also makes it easier to > > review. > > The current patches are easy enough for review. The last three patches of the > series is a collection of the one-type changes concerning the same type of > nodes. So reviewing them won't cause any difficulty. But I assume that's not > the main point in this discussion. > > > Sure, without split ber subarchitectures this could be picked > > up by SoC folks but you did not even CC them. So if you do not want to > > split it per subarchitectures for maintainers and you do not CC SoC, > > then how do you believe this should be picked up? Out of the regular > > patch submission way? That's not how the changes are handled. > > AFAIU there are another ways of merging comprehensive patches. If they get to > collect > all the Acked-by tags, they could be merged in, for instance, through Greg' > or Rob' > (for dts) repos, if of course they get to agree with doing that. Am I wrong? > > My hope was to ask Rob or Greg to get the patches merged in when they get > to collect all the ackes, since I thought it was an option in such cases. So > if > they refuse to do so I'll have no choice but to split the series up into a > smaller patches as you say. This is neither Rob's nor Greg's patch to pick up, but ARM SoC (which was not CCed here). And most likely they won't pick it up because judging by contents it is obvious it should go via ARM SoC. Sure, if there are dependencies between some patches they can go with acks through unrelated trees, but this not the usual way. This is an exception in the process to solve particular dependency problem. It has drawbacks - increases the chances of annoying conflicts. The case here does not fall into this criteria - there is no dependency of this patch on the others Therefore there is no reason to use the unusual/exceptional way of handling patches. There is no reason why this shouldn't go via either specific ARM subarchitecture maintainers or via ARM SoC. > > > > 3. The subject title could be more accurate - there is no fix here > > > > because there was no errors in the first place. Requirement of DWC > > > > node names comes recently, so it is more alignment with dtschema. > > > > Otherwise automatic-pickup-stable-bot might want to pick up... and it > > > > should not go to stable. > > > > > > Actually it is a fix, because the USB DT nodes should have been named > > > with "usb" > > > prefix in the first place. Legacy DWC USB3 bindings didn't define the > > > nodes > > > naming, but implied to be "usb"-prefixed by the USB HCD schema. The > > > Qualcomm > > > DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which > > > was > > > wrong in the first place. > > > > > Not following the naming convention of DT spec which was loosely > > enforced is not an error which should be "fixed". Simply wrong title. > > This is an alignment with dtschema or
