LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] powerpc: Update MAINTAINERS
From: Michael Ellerman @ 2020-02-24 23:31 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras, linux-kernel

A while back Paul pointed out I'd been maintaining the tree more or
less solo for over five years, so perhaps it's time to update the
MAINTAINERS entry.

Ben & Paul still wrote most of the code, so keep them as Reviewers so
they still get Cc'ed on things. But if you're wondering why your patch
hasn't been merged that's my fault.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index fcd79fc38928..339bc3e53862 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9601,9 +9601,9 @@ F:	arch/powerpc/platforms/powermac/
 F:	drivers/macintosh/
 
 LINUX FOR POWERPC (32-BIT AND 64-BIT)
-M:	Benjamin Herrenschmidt <benh@kernel.crashing.org>
-M:	Paul Mackerras <paulus@samba.org>
 M:	Michael Ellerman <mpe@ellerman.id.au>
+R:	Benjamin Herrenschmidt <benh@kernel.crashing.org>
+R:	Paul Mackerras <paulus@samba.org>
 W:	https://github.com/linuxppc/linux/wiki
 L:	linuxppc-dev@lists.ozlabs.org
 Q:	http://patchwork.ozlabs.org/project/linuxppc-dev/list/
-- 
2.21.1


^ permalink raw reply related

* Re: [PATCH][next] toshiba: Replace zero-length array with flexible-array member
From: David Miller @ 2020-02-24 23:26 UTC (permalink / raw)
  To: gustavo; +Cc: geoff, linux-kernel, paulus, netdev, linuxppc-dev
In-Reply-To: <20200224163252.GA28066@embeddedor>

From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Mon, 24 Feb 2020 10:32:52 -0600

> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
> 
> struct foo {
>         int stuff;
>         struct boo array[];
> };
> 
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
> 
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
> 
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
> 
> This issue was found with the help of Coccinelle.
> 
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Applied.

^ permalink raw reply

* [PATCH] powerpc/vdso: remove deprecated VDS64_HAS_DESCRIPTORS references
From: Joe Lawrence @ 2020-02-24 21:18 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <2df70550695143a9b62c0146e8e3e17ac91c1863.camel@kernel.crashing.org>

The original 2005 patch that introduced the powerpc vdso, pre-git
("ppc64: Implement a vDSO and use it for signal trampoline") notes that:

  ... symbols exposed by the vDSO aren't "normal" function symbols, apps
  can't be expected to link against them directly, the vDSO's are both
  seen as if they were linked at 0 and the symbols just contain offsets
  to the various functions.  This is done on purpose to avoid a
  relocation step (ppc64 functions normally have descriptors with abs
  addresses in them).  When glibc uses those functions, it's expected to
  use it's own trampolines that know how to reach them.

Despite that explanation, there remains dead #ifdef
VDS64_HAS_DESCRIPTORS code-blocks that provide alternate function
definitions that setup function descriptors.

Since VDS64_HAS_DESCRIPTORS has been unused for all these years, we
might as well finally remove it from the codebase.

Link: https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-February/204430.html
Link: https://lore.kernel.org/lkml/1108002773.7733.196.camel@gaston/
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
 arch/powerpc/include/asm/vdso.h | 24 ------------------------
 arch/powerpc/kernel/vdso.c      |  5 -----
 2 files changed, 29 deletions(-)

diff --git a/arch/powerpc/include/asm/vdso.h b/arch/powerpc/include/asm/vdso.h
index b5e1f8f8a05c..2ff884853f97 100644
--- a/arch/powerpc/include/asm/vdso.h
+++ b/arch/powerpc/include/asm/vdso.h
@@ -13,9 +13,6 @@
 
 #define VDSO_VERSION_STRING	LINUX_2.6.15
 
-/* Define if 64 bits VDSO has procedure descriptors */
-#undef VDS64_HAS_DESCRIPTORS
-
 #ifndef __ASSEMBLY__
 
 /* Offsets relative to thread->vdso_base */
@@ -28,25 +25,6 @@ int vdso_getcpu_init(void);
 #else /* __ASSEMBLY__ */
 
 #ifdef __VDSO64__
-#ifdef VDS64_HAS_DESCRIPTORS
-#define V_FUNCTION_BEGIN(name)		\
-	.globl name;			\
-        .section ".opd","a";		\
-        .align 3;			\
-	name:				\
-	.quad .name,.TOC.@tocbase,0;	\
-	.previous;			\
-	.globl .name;			\
-	.type .name,@function; 		\
-	.name:				\
-
-#define V_FUNCTION_END(name)		\
-	.size .name,.-.name;
-
-#define V_LOCAL_FUNC(name) (.name)
-
-#else /* VDS64_HAS_DESCRIPTORS */
-
 #define V_FUNCTION_BEGIN(name)		\
 	.globl name;			\
 	name:				\
@@ -55,8 +33,6 @@ int vdso_getcpu_init(void);
 	.size name,.-name;
 
 #define V_LOCAL_FUNC(name) (name)
-
-#endif /* VDS64_HAS_DESCRIPTORS */
 #endif /* __VDSO64__ */
 
 #ifdef __VDSO32__
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index b9a108411c0d..d3b77c15f9ce 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -391,12 +391,7 @@ static unsigned long __init find_function64(struct lib64_elfinfo *lib,
 		       symname);
 		return 0;
 	}
-#ifdef VDS64_HAS_DESCRIPTORS
-	return *((u64 *)(vdso64_kbase + sym->st_value - VDSO64_LBASE)) -
-		VDSO64_LBASE;
-#else
 	return sym->st_value - VDSO64_LBASE;
-#endif
 }
 
 static int __init vdso_do_func_patch64(struct lib32_elfinfo *v32,
-- 
2.21.1


^ permalink raw reply related

* [PATCH] powerpc/kprobes: Use probe_address() to read instructions
From: Christophe Leroy @ 2020-02-24 18:02 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao
  Cc: linuxppc-dev, linux-kernel

In order to avoid Oopses, use probe_address() to read the
instruction at the address where the trap happened.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/kprobes.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 84567406b53d..a35320b79e16 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -276,14 +276,18 @@ int kprobe_handler(struct pt_regs *regs)
 
 	p = get_kprobe(addr);
 	if (!p) {
-		if (*addr != BREAKPOINT_INSTRUCTION) {
+		unsigned int instr;
+
+		if (probe_kernel_address(addr, instr))
+			goto no_kprobe;
+
+		if (instr != BREAKPOINT_INSTRUCTION) {
 			/*
 			 * PowerPC has multiple variants of the "trap"
 			 * instruction. If the current instruction is a
 			 * trap variant, it could belong to someone else
 			 */
-			kprobe_opcode_t cur_insn = *addr;
-			if (is_trap(cur_insn))
+			if (is_trap(instr))
 				goto no_kprobe;
 			/*
 			 * The breakpoint instruction was removed right
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2] powerpc/kprobes: Blacklist functions running with MMU disabled on PPC32
From: Christophe Leroy @ 2020-02-24 17:40 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao
  Cc: linuxppc-dev, linux-kernel

kprobe does not handle events happening in real mode, all
functions running with MMU disabled have to be blacklisted.

As already done for PPC64, do it for PPC32.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v2:
- Don't rename nonrecoverable as local, mark it noprobe instead.
- Add missing linux/kprobes.h include in pq2.c
---
 arch/powerpc/include/asm/ppc_asm.h           | 10 +++
 arch/powerpc/kernel/cpu_setup_6xx.S          |  4 +-
 arch/powerpc/kernel/entry_32.S               | 65 ++++++++------------
 arch/powerpc/kernel/fpu.S                    |  1 +
 arch/powerpc/kernel/idle_6xx.S               |  2 +-
 arch/powerpc/kernel/idle_e500.S              |  2 +-
 arch/powerpc/kernel/l2cr_6xx.S               |  2 +-
 arch/powerpc/kernel/misc.S                   |  2 +
 arch/powerpc/kernel/misc_32.S                |  4 +-
 arch/powerpc/kernel/swsusp_32.S              |  6 +-
 arch/powerpc/kernel/vector.S                 |  1 +
 arch/powerpc/mm/book3s32/hash_low.S          | 38 ++++++------
 arch/powerpc/mm/mem.c                        |  2 +
 arch/powerpc/platforms/52xx/lite5200_sleep.S |  2 +
 arch/powerpc/platforms/82xx/pq2.c            |  3 +
 arch/powerpc/platforms/83xx/suspend-asm.S    |  1 +
 arch/powerpc/platforms/powermac/cache.S      |  2 +
 arch/powerpc/platforms/powermac/sleep.S      | 13 ++--
 18 files changed, 86 insertions(+), 74 deletions(-)

diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index 6b03dff61a05..e8f34ba89497 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -267,8 +267,18 @@ GLUE(.,name):
 	.pushsection "_kprobe_blacklist","aw";		\
 	PPC_LONG (entry) ;				\
 	.popsection
+#define _NOKPROBE_ENTRY(entry)				\
+	_ASM_NOKPROBE_SYMBOL(entry)				\
+	_ENTRY(entry)
+#define _NOKPROBE_GLOBAL(entry)				\
+	_ASM_NOKPROBE_SYMBOL(entry)				\
+	_GLOBAL(entry)
 #else
 #define _ASM_NOKPROBE_SYMBOL(entry)
+#define _NOKPROBE_ENTRY(entry)				\
+	_ENTRY(entry)
+#define _NOKPROBE_GLOBAL(entry)				\
+	_GLOBAL(entry)
 #endif
 
 #define FUNC_START(name)	_GLOBAL(name)
diff --git a/arch/powerpc/kernel/cpu_setup_6xx.S b/arch/powerpc/kernel/cpu_setup_6xx.S
index f6517f67265a..1cb947268546 100644
--- a/arch/powerpc/kernel/cpu_setup_6xx.S
+++ b/arch/powerpc/kernel/cpu_setup_6xx.S
@@ -276,7 +276,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_NO_DPM)
  * in some 750 cpus where using a not yet initialized FPU register after
  * power on reset may hang the CPU
  */
-_GLOBAL(__init_fpu_registers)
+_NOKPROBE_GLOBAL(__init_fpu_registers)
 	mfmsr	r10
 	ori	r11,r10,MSR_FP
 	mtmsr	r11
@@ -381,7 +381,7 @@ _GLOBAL(__save_cpu_setup)
  * restore CPU state as backed up by the previous
  * function. This does not include cache setting
  */
-_GLOBAL(__restore_cpu_setup)
+_NOKPROBE_GLOBAL(__restore_cpu_setup)
 	/* Some CR fields are volatile, we back it up all */
 	mfcr	r7
 
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 16af0d8d90a8..f788d586254d 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -44,24 +44,21 @@
 	.align	12
 
 #ifdef CONFIG_BOOKE
-	.globl	mcheck_transfer_to_handler
-mcheck_transfer_to_handler:
+_NOKPROBE_ENTRY(mcheck_transfer_to_handler)
 	mfspr	r0,SPRN_DSRR0
 	stw	r0,_DSRR0(r11)
 	mfspr	r0,SPRN_DSRR1
 	stw	r0,_DSRR1(r11)
 	/* fall through */
 
-	.globl	debug_transfer_to_handler
-debug_transfer_to_handler:
+_NOKPROBE_ENTRY(debug_transfer_to_handler)
 	mfspr	r0,SPRN_CSRR0
 	stw	r0,_CSRR0(r11)
 	mfspr	r0,SPRN_CSRR1
 	stw	r0,_CSRR1(r11)
 	/* fall through */
 
-	.globl	crit_transfer_to_handler
-crit_transfer_to_handler:
+_NOKPROBE_ENTRY(crit_transfer_to_handler)
 #ifdef CONFIG_PPC_BOOK3E_MMU
 	mfspr	r0,SPRN_MAS0
 	stw	r0,MAS0(r11)
@@ -97,8 +94,7 @@ crit_transfer_to_handler:
 #endif
 
 #ifdef CONFIG_40x
-	.globl	crit_transfer_to_handler
-crit_transfer_to_handler:
+_NOKPROBE_ENTRY(crit_transfer_to_handler)
 	lwz	r0,crit_r10@l(0)
 	stw	r0,GPR10(r11)
 	lwz	r0,crit_r11@l(0)
@@ -124,13 +120,11 @@ crit_transfer_to_handler:
  * Note that we rely on the caller having set cr0.eq iff the exception
  * occurred in kernel mode (i.e. MSR:PR = 0).
  */
-	.globl	transfer_to_handler_full
-transfer_to_handler_full:
+_NOKPROBE_ENTRY(transfer_to_handler_full)
 	SAVE_NVGPRS(r11)
 	/* fall through */
 
-	.globl	transfer_to_handler
-transfer_to_handler:
+_NOKPROBE_ENTRY(transfer_to_handler)
 	stw	r2,GPR2(r11)
 	stw	r12,_NIP(r11)
 	stw	r9,_MSR(r11)
@@ -194,8 +188,7 @@ transfer_to_handler:
 	bt-	31-TLF_NAPPING,4f
 	bt-	31-TLF_SLEEPING,7f
 #endif /* CONFIG_PPC_BOOK3S_32 || CONFIG_E500 */
-	.globl transfer_to_handler_cont
-transfer_to_handler_cont:
+_NOKPROBE_ENTRY(transfer_to_handler_cont)
 3:
 	mflr	r9
 	tovirt_novmstack r2, r2 	/* set r2 to current */
@@ -297,6 +290,7 @@ reenable_mmu:
  * On kernel stack overflow, load up an initial stack pointer
  * and call StackOverflow(regs), which should not return.
  */
+_ASM_NOKPROBE_SYMBOL(stack_ovf)
 stack_ovf:
 	/* sometimes we use a statically-allocated stack, which is OK. */
 	lis	r12,_end@h
@@ -460,6 +454,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
 	lwz	r7,_NIP(r1)
 	lwz	r2,GPR2(r1)
 	lwz	r1,GPR1(r1)
+syscall_exit_finish:
 #if defined(CONFIG_PPC_8xx) && defined(CONFIG_PERF_EVENTS)
 	mtspr	SPRN_NRI, r0
 #endif
@@ -467,6 +462,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
 	mtspr	SPRN_SRR1,r8
 	SYNC
 	RFI
+_ASM_NOKPROBE_SYMBOL(syscall_exit_finish)
 #ifdef CONFIG_44x
 2:	li	r7,0
 	iccci	r0,r0
@@ -750,8 +746,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_SPE)
 	addi	r1,r1,INT_FRAME_SIZE
 	blr
 
-	.globl	fast_exception_return
-fast_exception_return:
+_NOKPROBE_ENTRY(fast_exception_return)
 #if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
 	andi.	r10,r9,MSR_RI		/* check for recoverable interrupt */
 	beq	1f			/* if not, we've got problems */
@@ -780,8 +775,8 @@ fast_exception_return:
 
 #if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
 /* check if the exception happened in a restartable section */
-1:	lis	r3,exc_exit_restart_end@ha
-	addi	r3,r3,exc_exit_restart_end@l
+1:	lis	r3,.Lexc_exit_restart_end@ha
+	addi	r3,r3,.Lexc_exit_restart_end@l
 	cmplw	r12,r3
 #ifdef CONFIG_PPC_BOOK3S_601
 	bge	2b
@@ -1005,15 +1000,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
 	LOAD_REG_IMMEDIATE(r10,MSR_KERNEL & ~MSR_RI)
 	SYNC
 	mtmsr	r10		/* clear the RI bit */
-	.globl exc_exit_restart
-exc_exit_restart:
+_NOKPROBE_ENTRY(exc_exit_restart)
 	lwz	r12,_NIP(r1)
 	mtspr	SPRN_SRR0,r12
 	mtspr	SPRN_SRR1,r9
 	REST_4GPRS(9, r1)
 	lwz	r1,GPR1(r1)
-	.globl exc_exit_restart_end
-exc_exit_restart_end:
+.Lexc_exit_restart_end:
 	SYNC
 	RFI
 
@@ -1033,17 +1026,15 @@ exc_exit_restart_end:
 	li	r10, 0
 	stw	r10, 8(r1)
 	REST_2GPRS(9, r1)
-	.globl exc_exit_restart
+_NOKPROBE_ENTRY(exc_exit_restart)
 exc_exit_restart:
 	lwz	r11,_NIP(r1)
 	lwz	r12,_MSR(r1)
-exc_exit_start:
 	mtspr	SPRN_SRR0,r11
 	mtspr	SPRN_SRR1,r12
 	REST_2GPRS(11, r1)
 	lwz	r1,GPR1(r1)
-	.globl exc_exit_restart_end
-exc_exit_restart_end:
+.Lexc_exit_restart_end:
 	PPC405_ERR77_SYNC
 	rfi
 	b	.			/* prevent prefetch past rfi */
@@ -1146,8 +1137,7 @@ exc_exit_restart_end:
 #endif
 
 #ifdef CONFIG_40x
-	.globl	ret_from_crit_exc
-ret_from_crit_exc:
+_NOKPROBE_ENTRY(ret_from_crit_exc)
 	mfspr	r9,SPRN_SPRG_THREAD
 	lis	r10,saved_ksp_limit@ha;
 	lwz	r10,saved_ksp_limit@l(r10);
@@ -1163,8 +1153,7 @@ ret_from_crit_exc:
 #endif /* CONFIG_40x */
 
 #ifdef CONFIG_BOOKE
-	.globl	ret_from_crit_exc
-ret_from_crit_exc:
+_NOKPROBE_ENTRY(ret_from_crit_exc)
 	mfspr	r9,SPRN_SPRG_THREAD
 	lwz	r10,SAVED_KSP_LIMIT(r1)
 	stw	r10,KSP_LIMIT(r9)
@@ -1172,8 +1161,7 @@ ret_from_crit_exc:
 	RESTORE_MMU_REGS;
 	RET_FROM_EXC_LEVEL(SPRN_CSRR0, SPRN_CSRR1, PPC_RFCI)
 
-	.globl	ret_from_debug_exc
-ret_from_debug_exc:
+_NOKPROBE_ENTRY(ret_from_debug_exc)
 	mfspr	r9,SPRN_SPRG_THREAD
 	lwz	r10,SAVED_KSP_LIMIT(r1)
 	stw	r10,KSP_LIMIT(r9)
@@ -1182,8 +1170,7 @@ ret_from_debug_exc:
 	RESTORE_MMU_REGS;
 	RET_FROM_EXC_LEVEL(SPRN_DSRR0, SPRN_DSRR1, PPC_RFDI)
 
-	.globl	ret_from_mcheck_exc
-ret_from_mcheck_exc:
+_NOKPROBE_ENTRY(ret_from_mcheck_exc)
 	mfspr	r9,SPRN_SPRG_THREAD
 	lwz	r10,SAVED_KSP_LIMIT(r1)
 	stw	r10,KSP_LIMIT(r9)
@@ -1277,9 +1264,10 @@ do_user_signal:			/* r10 contains MSR_KERNEL here */
  * that occurred at a place where taking an exception will lose
  * state information, such as the contents of SRR0 and SRR1.
  */
+_ASM_NOKPROBE_SYMBOL(nonrecoverable)
 nonrecoverable:
-	lis	r10,exc_exit_restart_end@ha
-	addi	r10,r10,exc_exit_restart_end@l
+	lis	r10,.Lexc_exit_restart_end@ha
+	addi	r10,r10,.Lexc_exit_restart_end@l
 	cmplw	r12,r10
 #ifdef CONFIG_PPC_BOOK3S_601
 	bgelr
@@ -1332,7 +1320,7 @@ ee_restarts:
  * On CHRP, the Run-Time Abstraction Services (RTAS) have to be
  * called with the MMU off.
  */
-_GLOBAL(enter_rtas)
+_NOKPROBE_GLOBAL(enter_rtas)
 	stwu	r1,-INT_FRAME_SIZE(r1)
 	mflr	r0
 	stw	r0,INT_FRAME_SIZE+4(r1)
@@ -1370,8 +1358,7 @@ _GLOBAL(enter_rtas)
 	mtspr	SPRN_SRR1,r9
 	RFI			/* return to caller */
 
-	.globl	machine_check_in_rtas
-machine_check_in_rtas:
+_NOKPROBE_ENTRY(machine_check_in_rtas)
 	twi	31,0,0
 	/* XXX load up BATs and panic */
 
diff --git a/arch/powerpc/kernel/fpu.S b/arch/powerpc/kernel/fpu.S
index 3235a8da6af7..1dfccf58fbb1 100644
--- a/arch/powerpc/kernel/fpu.S
+++ b/arch/powerpc/kernel/fpu.S
@@ -119,6 +119,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
 	/* restore registers and return */
 	/* we haven't used ctr or xer or lr */
 	blr
+_ASM_NOKPROBE_SYMBOL(load_up_fpu)
 
 /*
  * save_fpu(tsk)
diff --git a/arch/powerpc/kernel/idle_6xx.S b/arch/powerpc/kernel/idle_6xx.S
index 433d97bea1f3..37ca62fa0b75 100644
--- a/arch/powerpc/kernel/idle_6xx.S
+++ b/arch/powerpc/kernel/idle_6xx.S
@@ -149,7 +149,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
  * address of current.  R11 points to the exception frame (physical
  * address).  We have to preserve r10.
  */
-_GLOBAL(power_save_ppc32_restore)
+_NOKPROBE_GLOBAL(power_save_ppc32_restore)
 	lwz	r9,_LINK(r11)		/* interrupted in ppc6xx_idle: */
 	stw	r9,_NIP(r11)		/* make it do a blr */
 
diff --git a/arch/powerpc/kernel/idle_e500.S b/arch/powerpc/kernel/idle_e500.S
index 308f499e146c..8c9a781b7c11 100644
--- a/arch/powerpc/kernel/idle_e500.S
+++ b/arch/powerpc/kernel/idle_e500.S
@@ -78,7 +78,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CAN_NAP)
  * r11 points to the exception frame (physical address).
  * We have to preserve r10.
  */
-_GLOBAL(power_save_ppc32_restore)
+_NOKPROBE_GLOBAL(power_save_ppc32_restore)
 	lwz	r9,_LINK(r11)		/* interrupted in e500_idle */
 	stw	r9,_NIP(r11)		/* make it do a blr */
 
diff --git a/arch/powerpc/kernel/l2cr_6xx.S b/arch/powerpc/kernel/l2cr_6xx.S
index 2020d255585f..75e00baf0146 100644
--- a/arch/powerpc/kernel/l2cr_6xx.S
+++ b/arch/powerpc/kernel/l2cr_6xx.S
@@ -443,7 +443,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
  *
  * clobbers r3
  */
-_GLOBAL(__inval_enable_L1)
+_NOKPROBE_GLOBAL(__inval_enable_L1)
 	/* Enable and then Flash inval the instruction & data cache */
 	mfspr	r3,SPRN_HID0
 	ori	r3,r3, HID0_ICE|HID0_ICFI|HID0_DCE|HID0_DCI
diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S
index 974f65f79a8e..dae46463fc81 100644
--- a/arch/powerpc/kernel/misc.S
+++ b/arch/powerpc/kernel/misc.S
@@ -36,6 +36,8 @@ _GLOBAL(add_reloc_offset)
 	add	r3,r3,r5
 	mtlr	r0
 	blr
+_ASM_NOKPROBE_SYMBOL(reloc_offset)
+_ASM_NOKPROBE_SYMBOL(add_reloc_offset)
 
 	.align	3
 2:	PPC_LONG 1b
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index d80212be8698..07a2a303f0d9 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -233,7 +233,7 @@ _GLOBAL(_nmask_and_or_msr)
 /*
  * Do an IO access in real mode
  */
-_GLOBAL(real_readb)
+_NOKPROBE_GLOBAL(real_readb)
 	mfmsr	r7
 	rlwinm	r0,r7,0,~MSR_DR
 	sync
@@ -250,7 +250,7 @@ _GLOBAL(real_readb)
 	/*
  * Do an IO access in real mode
  */
-_GLOBAL(real_writeb)
+_NOKPROBE_GLOBAL(real_writeb)
 	mfmsr	r7
 	rlwinm	r0,r7,0,~MSR_DR
 	sync
diff --git a/arch/powerpc/kernel/swsusp_32.S b/arch/powerpc/kernel/swsusp_32.S
index cbdf86228eaa..2dfe1afd7fc1 100644
--- a/arch/powerpc/kernel/swsusp_32.S
+++ b/arch/powerpc/kernel/swsusp_32.S
@@ -176,7 +176,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 
 
 /* Resume code */
-_GLOBAL(swsusp_arch_resume)
+_NOKPROBE_GLOBAL(swsusp_arch_resume)
 
 #ifdef CONFIG_ALTIVEC
 	/* Stop pending alitvec streams and memory accesses */
@@ -367,7 +367,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 
 	/* restore the MSR and turn on the MMU */
 	lwz	r3,SL_MSR(r11)
-	bl	turn_on_mmu
+	bl	.Lturn_on_mmu
 	tovirt(r11,r11)
 
 	/* Restore TB */
@@ -399,7 +399,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 /* FIXME:This construct is actually not useful since we don't shut
  * down the instruction MMU, we could just flip back MSR-DR on.
  */
-turn_on_mmu:
+.Lturn_on_mmu:
 	mflr	r4
 	mtsrr0	r4
 	mtsrr1	r3
diff --git a/arch/powerpc/kernel/vector.S b/arch/powerpc/kernel/vector.S
index 25c14a0981bf..403ab2c32dc8 100644
--- a/arch/powerpc/kernel/vector.S
+++ b/arch/powerpc/kernel/vector.S
@@ -89,6 +89,7 @@ _GLOBAL(load_up_altivec)
 	REST_32VRS(0,r4,r6)
 	/* restore registers and return */
 	blr
+_ASM_NOKPROBE_SYMBOL(load_up_altivec)
 
 /*
  * save_altivec(tsk)
diff --git a/arch/powerpc/mm/book3s32/hash_low.S b/arch/powerpc/mm/book3s32/hash_low.S
index 2015c4f96238..004825d7070c 100644
--- a/arch/powerpc/mm/book3s32/hash_low.S
+++ b/arch/powerpc/mm/book3s32/hash_low.S
@@ -45,7 +45,7 @@ mmu_hash_lock:
  * Uses r0, r3 - r6, r8, r10, ctr, lr.
  */
 	.text
-_GLOBAL(hash_page)
+_NOKPROBE_GLOBAL(hash_page)
 #ifdef CONFIG_SMP
 	lis	r8, (mmu_hash_lock - PAGE_OFFSET)@h
 	ori	r8, r8, (mmu_hash_lock - PAGE_OFFSET)@l
@@ -81,7 +81,7 @@ _GLOBAL(hash_page)
 	rlwinm.	r8,r8,0,0,20		/* extract pt base address */
 #endif
 #ifdef CONFIG_SMP
-	beq-	hash_page_out		/* return if no mapping */
+	beq-	.Lhash_page_out		/* return if no mapping */
 #else
 	/* XXX it seems like the 601 will give a machine fault on the
 	   rfi if its alignment is wrong (bottom 4 bits of address are
@@ -109,11 +109,11 @@ _GLOBAL(hash_page)
 #if (PTE_FLAGS_OFFSET != 0)
 	addi	r8,r8,PTE_FLAGS_OFFSET
 #endif
-retry:
+.Lretry:
 	lwarx	r6,0,r8			/* get linux-style pte, flag word */
 	andc.	r5,r3,r6		/* check access & ~permission */
 #ifdef CONFIG_SMP
-	bne-	hash_page_out		/* return if access not permitted */
+	bne-	.Lhash_page_out		/* return if access not permitted */
 #else
 	bnelr-
 #endif
@@ -128,7 +128,7 @@ retry:
 #endif /* CONFIG_SMP */
 #endif /* CONFIG_PTE_64BIT */
 	stwcx.	r5,0,r8			/* attempt to update PTE */
-	bne-	retry			/* retry if someone got there first */
+	bne-	.Lretry			/* retry if someone got there first */
 
 	mfsrin	r3,r4			/* get segment reg for segment */
 #ifndef CONFIG_VMAP_STACK
@@ -156,7 +156,7 @@ retry:
 #endif
 
 #ifdef CONFIG_SMP
-hash_page_out:
+.Lhash_page_out:
 	eieio
 	lis	r8, (mmu_hash_lock - PAGE_OFFSET)@ha
 	li	r0,0
@@ -173,7 +173,7 @@ hash_page_out:
  * the accessed bit) have already been done and that there is actually
  * a hash table in use (i.e. we're not on a 603).
  */
-_GLOBAL(add_hash_page)
+_NOKPROBE_GLOBAL(add_hash_page)
 	mflr	r0
 	stw	r0,4(r1)
 
@@ -308,7 +308,7 @@ Hash_msk = (((1 << Hash_bits) - 1) * 64)
 #define HASH_LEFT	31-(LG_PTEG_SIZE+Hash_bits-1)
 #define HASH_RIGHT	31-LG_PTEG_SIZE
 
-_GLOBAL(create_hpte)
+_NOKPROBE_GLOBAL(create_hpte)
 	/* Convert linux-style PTE (r5) to low word of PPC-style PTE (r8) */
 	rlwinm	r8,r5,32-9,30,30	/* _PAGE_RW -> PP msb */
 	rlwinm	r0,r5,32-6,30,30	/* _PAGE_DIRTY -> PP msb */
@@ -360,7 +360,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_NEED_COHERENT)
 1:	LDPTEu	r6,HPTE_SIZE(r4)	/* get next PTE */
 	CMPPTE	0,r6,r5
 	bdnzf	2,1b			/* loop while ctr != 0 && !cr0.eq */
-	beq+	found_slot
+	beq+	.Lfound_slot
 
 	patch_site	0f, patch__hash_page_B
 	/* Search the secondary PTEG for a matching PTE */
@@ -372,7 +372,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_NEED_COHERENT)
 2:	LDPTEu	r6,HPTE_SIZE(r4)
 	CMPPTE	0,r6,r5
 	bdnzf	2,2b
-	beq+	found_slot
+	beq+	.Lfound_slot
 	xori	r5,r5,PTE_H		/* clear H bit again */
 
 	/* Search the primary PTEG for an empty slot */
@@ -381,7 +381,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_NEED_COHERENT)
 1:	LDPTEu	r6,HPTE_SIZE(r4)	/* get next PTE */
 	TST_V(r6)			/* test valid bit */
 	bdnzf	2,1b			/* loop while ctr != 0 && !cr0.eq */
-	beq+	found_empty
+	beq+	.Lfound_empty
 
 	/* update counter of times that the primary PTEG is full */
 	lis	r4, (primary_pteg_full - PAGE_OFFSET)@ha
@@ -399,7 +399,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_NEED_COHERENT)
 2:	LDPTEu	r6,HPTE_SIZE(r4)
 	TST_V(r6)
 	bdnzf	2,2b
-	beq+	found_empty
+	beq+	.Lfound_empty
 	xori	r5,r5,PTE_H		/* clear H bit again */
 
 	/*
@@ -437,9 +437,9 @@ END_FTR_SECTION_IFCLR(CPU_FTR_NEED_COHERENT)
 
 #ifndef CONFIG_SMP
 	/* Store PTE in PTEG */
-found_empty:
+.Lfound_empty:
 	STPTE	r5,0(r4)
-found_slot:
+.Lfound_slot:
 	STPTE	r8,HPTE_SIZE/2(r4)
 
 #else /* CONFIG_SMP */
@@ -460,8 +460,8 @@ found_slot:
  * We do however have to make sure that the PTE is never in an invalid
  * state with the V bit set.
  */
-found_empty:
-found_slot:
+.Lfound_empty:
+.Lfound_slot:
 	CLR_V(r5,r0)		/* clear V (valid) bit in PTE */
 	STPTE	r5,0(r4)
 	sync
@@ -493,7 +493,7 @@ htab_hash_searches:
  *
  * We assume that there is a hash table in use (Hash != 0).
  */
-_GLOBAL(flush_hash_pages)
+_NOKPROBE_GLOBAL(flush_hash_pages)
 	/*
 	 * We disable interrupts here, even on UP, because we want
 	 * the _PAGE_HASHPTE bit to be a reliable indication of
@@ -634,7 +634,7 @@ EXPORT_SYMBOL(flush_hash_pages)
 /*
  * Flush an entry from the TLB
  */
-_GLOBAL(_tlbie)
+_NOKPROBE_GLOBAL(_tlbie)
 #ifdef CONFIG_SMP
 	lwz	r8,TASK_CPU(r2)
 	oris	r8,r8,11
@@ -671,7 +671,7 @@ _GLOBAL(_tlbie)
 /*
  * Flush the entire TLB. 603/603e only
  */
-_GLOBAL(_tlbia)
+_NOKPROBE_GLOBAL(_tlbia)
 #if defined(CONFIG_SMP)
 	lwz	r8,TASK_CPU(r2)
 	oris	r8,r8,10
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index ef7b1119b2e2..7a4083a62efe 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -32,6 +32,7 @@
 #include <linux/vmalloc.h>
 #include <linux/memremap.h>
 #include <linux/dma-direct.h>
+#include <linux/kprobes.h>
 
 #include <asm/pgalloc.h>
 #include <asm/prom.h>
@@ -470,6 +471,7 @@ static void flush_dcache_icache_phys(unsigned long physaddr)
 		: "r" (nb), "r" (msr), "i" (bytes), "r" (msr0)
 		: "ctr", "memory");
 }
+NOKPROBE_SYMBOL(flush_dcache_icache_phys)
 #endif // !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
 
 /*
diff --git a/arch/powerpc/platforms/52xx/lite5200_sleep.S b/arch/powerpc/platforms/52xx/lite5200_sleep.S
index 3a9969c429b3..8ced276a6b47 100644
--- a/arch/powerpc/platforms/52xx/lite5200_sleep.S
+++ b/arch/powerpc/platforms/52xx/lite5200_sleep.S
@@ -193,6 +193,7 @@ sram_code_end:
 
 
 /* uboot jumps here on resume */
+_ASM_NOKPROBE_SYMBOL(lite5200_wakeup)
 lite5200_wakeup:
 	bl	restore_regs
 
@@ -335,6 +336,7 @@ save_regs:
 	LOAD_SR(n+2, addr+2);	\
 	LOAD_SR(n+3, addr+3);
 
+_ASM_NOKPROBE_SYMBOL(restore_regs)
 restore_regs:
 	lis	r4, registers@h
 	ori	r4, r4, registers@l
diff --git a/arch/powerpc/platforms/82xx/pq2.c b/arch/powerpc/platforms/82xx/pq2.c
index 1cdd5ed9d896..3b5cb39a564c 100644
--- a/arch/powerpc/platforms/82xx/pq2.c
+++ b/arch/powerpc/platforms/82xx/pq2.c
@@ -10,6 +10,8 @@
  * Copyright (c) 2006 MontaVista Software, Inc.
  */
 
+#include <linux/kprobes.h>
+
 #include <asm/cpm2.h>
 #include <asm/io.h>
 #include <asm/pci-bridge.h>
@@ -29,6 +31,7 @@ void __noreturn pq2_restart(char *cmd)
 
 	panic("Restart failed\n");
 }
+NOKPROBE_SYMBOL(pq2_restart)
 
 #ifdef CONFIG_PCI
 static int pq2_pci_exclude_device(struct pci_controller *hose,
diff --git a/arch/powerpc/platforms/83xx/suspend-asm.S b/arch/powerpc/platforms/83xx/suspend-asm.S
index 3acd7470dc5e..a7ba0f41ac34 100644
--- a/arch/powerpc/platforms/83xx/suspend-asm.S
+++ b/arch/powerpc/platforms/83xx/suspend-asm.S
@@ -365,6 +365,7 @@ boot_low:
 	isync
 1:	b	1b
 
+_ASM_NOKPROBE_SYMBOL(mpc83xx_deep_resume)
 mpc83xx_deep_resume:
 	lis	r4, 1f@h
 	ori	r4, r4, 1f@l
diff --git a/arch/powerpc/platforms/powermac/cache.S b/arch/powerpc/platforms/powermac/cache.S
index da69e0fcb4f1..e15f4aa22e9a 100644
--- a/arch/powerpc/platforms/powermac/cache.S
+++ b/arch/powerpc/platforms/powermac/cache.S
@@ -35,6 +35,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_L2CR)
 	b	__flush_disable_L1
 
 /* This is the code for G3 and 74[01]0 */
+_ASM_NOKPROBE_SYMBOL(flush_disable_75x)
 flush_disable_75x:
 	mflr	r10
 
@@ -186,6 +187,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 	blr
 
 /* This code is for 745x processors */
+_ASM_NOKPROBE_SYMBOL(flush_disable_745x)
 flush_disable_745x:
 	/* Turn off EE and DR in MSR */
 	mfmsr	r11
diff --git a/arch/powerpc/platforms/powermac/sleep.S b/arch/powerpc/platforms/powermac/sleep.S
index bd6085b470b7..2a6de1b345c0 100644
--- a/arch/powerpc/platforms/powermac/sleep.S
+++ b/arch/powerpc/platforms/powermac/sleep.S
@@ -177,8 +177,8 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 	 *    at address 0x84. I've have some trouble with those
 	 *    parameters however and I no longer use them.
 	 */
-	lis	r5,grackle_wake_up@ha
-	addi	r5,r5,grackle_wake_up@l
+	lis	r5,.Lgrackle_wake_up@ha
+	addi	r5,r5,.Lgrackle_wake_up@l
 	tophys(r5,r5)
 	stw	r5,SL_PC(r1)
 	lis	r4,KERNELBASE@h
@@ -202,6 +202,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 	stw r5,0(r3)
 
 	.globl	low_cpu_die
+_ASM_NOKPROBE_SYMBOL(low_cpu_die)
 low_cpu_die:
 	/* Flush & disable all caches */
 	bl	flush_disable_caches
@@ -244,7 +245,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_SPEC7450)
 	mtmsr	r2
 	isync
 	b	1b
-
 /*
  * Here is the resume code.
  */
@@ -255,6 +255,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_SPEC7450)
  * r4 has the physical address of SL_PC(sp) (unused)
  */
 _GLOBAL(core99_wake_up)
+_ASM_NOKPROBE_SYMBOL(core99_wake_up)
 	/* Make sure HID0 no longer contains any sleep bit and that data cache
 	 * is disabled
 	 */
@@ -287,7 +288,7 @@ _GLOBAL(core99_wake_up)
  * r1 has the physical address of SL_PC(sp).
  */
 
-grackle_wake_up:
+.Lgrackle_wake_up:
 
 	/* Restore the kernel's segment registers before
 	 * we do any r1 memory access as we are not sure they
@@ -407,7 +408,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 
 	/* restore the MSR and turn on the MMU */
 	lwz	r3,SL_MSR(r1)
-	bl	turn_on_mmu
+	bl	.Lturn_on_mmu
 
 	/* get back the stack pointer */
 	tovirt(r1,r1)
@@ -430,7 +431,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 	mtlr	r0
 	blr
 
-turn_on_mmu:
+.Lturn_on_mmu:
 	mflr	r4
 	tovirt(r4,r4)
 	mtsrr0	r4
-- 
2.25.0


^ permalink raw reply related

* [PATCH][next] toshiba: Replace zero-length array with flexible-array member
From: Gustavo A. R. Silva @ 2020-02-24 16:32 UTC (permalink / raw)
  To: Geoff Levand, David S. Miller, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Ishizaki Kou
  Cc: netdev, linuxppc-dev, linux-kernel, Gustavo A. R. Silva

The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:

struct foo {
        int stuff;
        struct boo array[];
};

By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.

Also, notice that, dynamic memory allocations won't be affected by
this change:

"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]

This issue was found with the help of Coccinelle.

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/ethernet/toshiba/ps3_gelic_net.h      | 2 +-
 drivers/net/ethernet/toshiba/ps3_gelic_wireless.h | 2 +-
 drivers/net/ethernet/toshiba/spider_net.h         | 2 +-
 drivers/net/ethernet/toshiba/tc35815.c            | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.h b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
index 805903dbddcc..68f324ed4eaf 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.h
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
@@ -308,7 +308,7 @@ struct gelic_port {
 	struct gelic_card *card;
 	struct net_device *netdev;
 	enum gelic_port_type type;
-	long priv[0]; /* long for alignment */
+	long priv[]; /* long for alignment */
 };
 
 static inline struct gelic_card *port_to_card(struct gelic_port *p)
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_wireless.h b/drivers/net/ethernet/toshiba/ps3_gelic_wireless.h
index 4041d946b649..1f203d1ae8db 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_wireless.h
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_wireless.h
@@ -158,7 +158,7 @@ struct gelic_eurus_scan_info {
 	__be32 reserved2;
 	__be32 reserved3;
 	__be32 reserved4;
-	u8 elements[0]; /* ie */
+	u8 elements[]; /* ie */
 } __packed;
 
 /* the hypervisor returns bbs up to 16 */
diff --git a/drivers/net/ethernet/toshiba/spider_net.h b/drivers/net/ethernet/toshiba/spider_net.h
index c0c68cbc898c..05b1a0736835 100644
--- a/drivers/net/ethernet/toshiba/spider_net.h
+++ b/drivers/net/ethernet/toshiba/spider_net.h
@@ -470,7 +470,7 @@ struct spider_net_card {
 	struct spider_net_extra_stats spider_stats;
 
 	/* Must be last item in struct */
-	struct spider_net_descr darray[0];
+	struct spider_net_descr darray[];
 };
 
 #endif
diff --git a/drivers/net/ethernet/toshiba/tc35815.c b/drivers/net/ethernet/toshiba/tc35815.c
index 3fd43d30b20d..b50c3ec3495b 100644
--- a/drivers/net/ethernet/toshiba/tc35815.c
+++ b/drivers/net/ethernet/toshiba/tc35815.c
@@ -367,7 +367,7 @@ struct TxFD {
 
 struct RxFD {
 	struct FDesc fd;
-	struct BDesc bd[0];	/* variable length */
+	struct BDesc bd[];	/* variable length */
 };
 
 struct FrFD {
-- 
2.25.0


^ permalink raw reply related

* [PATCH] crypto: Replace zero-length array with flexible-array member
From: Gustavo A. R. Silva @ 2020-02-24 16:21 UTC (permalink / raw)
  To: Horia Geantă, Aymen Sghaier, Herbert Xu, David S. Miller,
	Atul Gupta, Matthias Brugger, Breno Leitão, Nayna Jain,
	Paulo Flabiano Smorigo, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman
  Cc: Gustavo A. R. Silva, linux-kernel, linux-mediatek, linux-crypto,
	linuxppc-dev, linux-arm-kernel

The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:

struct foo {
        int stuff;
        struct boo array[];
};

By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.

Also, notice that, dynamic memory allocations won't be affected by
this change:

"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]

This issue was found with the help of Coccinelle.

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/crypto/caam/caamalg.c              | 2 +-
 drivers/crypto/caam/caamalg_qi.c           | 4 ++--
 drivers/crypto/caam/caamalg_qi2.h          | 6 +++---
 drivers/crypto/caam/caamhash.c             | 2 +-
 drivers/crypto/cavium/nitrox/nitrox_main.c | 2 +-
 drivers/crypto/chelsio/chcr_core.h         | 2 +-
 drivers/crypto/mediatek/mtk-sha.c          | 2 +-
 drivers/crypto/nx/nx.h                     | 2 +-
 drivers/crypto/omap-sham.c                 | 4 ++--
 include/crypto/if_alg.h                    | 2 +-
 10 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 03797f9b1050..b7bb7c30adeb 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -909,7 +909,7 @@ struct skcipher_edesc {
 	bool bklog;
 	dma_addr_t sec4_sg_dma;
 	struct sec4_sg_entry *sec4_sg;
-	u32 hw_desc[0];
+	u32 hw_desc[];
 };
 
 static void caam_unmap(struct device *dev, struct scatterlist *src,
diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index 4a29e0ef9d63..27e36bdf6163 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -783,7 +783,7 @@ struct aead_edesc {
 	unsigned int assoclen;
 	dma_addr_t assoclen_dma;
 	struct caam_drv_req drv_req;
-	struct qm_sg_entry sgt[0];
+	struct qm_sg_entry sgt[];
 };
 
 /*
@@ -803,7 +803,7 @@ struct skcipher_edesc {
 	int qm_sg_bytes;
 	dma_addr_t qm_sg_dma;
 	struct caam_drv_req drv_req;
-	struct qm_sg_entry sgt[0];
+	struct qm_sg_entry sgt[];
 };
 
 static struct caam_drv_ctx *get_drv_ctx(struct caam_ctx *ctx,
diff --git a/drivers/crypto/caam/caamalg_qi2.h b/drivers/crypto/caam/caamalg_qi2.h
index 706736776b47..f29cb7bd7dd3 100644
--- a/drivers/crypto/caam/caamalg_qi2.h
+++ b/drivers/crypto/caam/caamalg_qi2.h
@@ -114,7 +114,7 @@ struct aead_edesc {
 	dma_addr_t qm_sg_dma;
 	unsigned int assoclen;
 	dma_addr_t assoclen_dma;
-	struct dpaa2_sg_entry sgt[0];
+	struct dpaa2_sg_entry sgt[];
 };
 
 /*
@@ -132,7 +132,7 @@ struct skcipher_edesc {
 	dma_addr_t iv_dma;
 	int qm_sg_bytes;
 	dma_addr_t qm_sg_dma;
-	struct dpaa2_sg_entry sgt[0];
+	struct dpaa2_sg_entry sgt[];
 };
 
 /*
@@ -146,7 +146,7 @@ struct ahash_edesc {
 	dma_addr_t qm_sg_dma;
 	int src_nents;
 	int qm_sg_bytes;
-	struct dpaa2_sg_entry sgt[0];
+	struct dpaa2_sg_entry sgt[];
 };
 
 /**
diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 2fe852853d40..943bc0296267 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -536,7 +536,7 @@ struct ahash_edesc {
 	int sec4_sg_bytes;
 	bool bklog;
 	u32 hw_desc[DESC_JOB_IO_LEN_MAX / sizeof(u32)] ____cacheline_aligned;
-	struct sec4_sg_entry sec4_sg[0];
+	struct sec4_sg_entry sec4_sg[];
 };
 
 static inline void ahash_unmap(struct device *dev,
diff --git a/drivers/crypto/cavium/nitrox/nitrox_main.c b/drivers/crypto/cavium/nitrox/nitrox_main.c
index c4632d84c9a1..e91be9b8b083 100644
--- a/drivers/crypto/cavium/nitrox/nitrox_main.c
+++ b/drivers/crypto/cavium/nitrox/nitrox_main.c
@@ -71,7 +71,7 @@ struct ucode {
 	char version[VERSION_LEN - 1];
 	__be32 code_size;
 	u8 raz[12];
-	u64 code[0];
+	u64 code[];
 };
 
 /**
diff --git a/drivers/crypto/chelsio/chcr_core.h b/drivers/crypto/chelsio/chcr_core.h
index b41ef1abfe74..e480096754b5 100644
--- a/drivers/crypto/chelsio/chcr_core.h
+++ b/drivers/crypto/chelsio/chcr_core.h
@@ -68,7 +68,7 @@ struct _key_ctx {
 	__be32 ctx_hdr;
 	u8 salt[MAX_SALT];
 	__be64 iv_to_auth;
-	unsigned char key[0];
+	unsigned char key[];
 };
 
 #define KEYCTX_TX_WR_IV_S  55
diff --git a/drivers/crypto/mediatek/mtk-sha.c b/drivers/crypto/mediatek/mtk-sha.c
index 9e9f48bb7f85..bd6309e57ab8 100644
--- a/drivers/crypto/mediatek/mtk-sha.c
+++ b/drivers/crypto/mediatek/mtk-sha.c
@@ -107,7 +107,7 @@ struct mtk_sha_ctx {
 	u8 id;
 	u8 buf[SHA_BUF_SIZE] __aligned(sizeof(u32));
 
-	struct mtk_sha_hmac_ctx	base[0];
+	struct mtk_sha_hmac_ctx	base[];
 };
 
 struct mtk_sha_drv {
diff --git a/drivers/crypto/nx/nx.h b/drivers/crypto/nx/nx.h
index 91c54289124a..c6233173c612 100644
--- a/drivers/crypto/nx/nx.h
+++ b/drivers/crypto/nx/nx.h
@@ -37,7 +37,7 @@ struct max_sync_cop {
 	u32 fc;
 	u32 mode;
 	u32 triplets;
-	struct msc_triplet trip[0];
+	struct msc_triplet trip[];
 } __packed;
 
 struct alg_props {
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 4f915a4ef5b0..e4072cd38585 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -159,7 +159,7 @@ struct omap_sham_reqctx {
 	int			sg_len;
 	unsigned int		total;	/* total request */
 
-	u8			buffer[0] OMAP_ALIGNED;
+	u8			buffer[] OMAP_ALIGNED;
 };
 
 struct omap_sham_hmac_ctx {
@@ -176,7 +176,7 @@ struct omap_sham_ctx {
 	/* fallback stuff */
 	struct crypto_shash	*fallback;
 
-	struct omap_sham_hmac_ctx base[0];
+	struct omap_sham_hmac_ctx base[];
 };
 
 #define OMAP_SHAM_QUEUE_LENGTH	10
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 24cfa96f98ea..56527c85d122 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -66,7 +66,7 @@ struct af_alg_sgl {
 struct af_alg_tsgl {
 	struct list_head list;
 	unsigned int cur;		/* Last processed SG entry */
-	struct scatterlist sg[0];	/* Array of SGs forming the SGL */
+	struct scatterlist sg[];	/* Array of SGs forming the SGL */
 };
 
 #define MAX_SGL_ENTS ((4096 - sizeof(struct af_alg_tsgl)) / \
-- 
2.25.0


^ permalink raw reply related

* Re: vdso function descriptors (VDS64_HAS_DESCRIPTORS)?
From: Benjamin Herrenschmidt @ 2020-02-24 15:34 UTC (permalink / raw)
  To: Joe Lawrence, Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <885be270-7af8-490d-302c-270a6db94ffe@redhat.com>

On Mon, 2020-02-24 at 10:20 -0500, Joe Lawrence wrote:
> On
> > I don't remember why :-) I think I didn't want to mess with the OPD
> > fixup in glibc back then.
> > 
> 
> Does it make sense to just drop the unused VDS64_HAS_DESCRIPTORS code
> then?

I'd think so yes.

Cheers,
Ben.


^ permalink raw reply

* Re: vdso function descriptors (VDS64_HAS_DESCRIPTORS)?
From: Joe Lawrence @ 2020-02-24 15:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <587a0a1765ba1264b92044f7b1f9937eb40731f1.camel@kernel.crashing.org>

On 2/24/20 5:17 AM, Benjamin Herrenschmidt wrote:
> On Sat, 2020-02-22 at 18:07 -0600, Segher Boessenkool wrote:
>>>
>>> so I don't believe they are ever used by default -- in this case
>>> V_FUNCTION_BEGIN doesn't add to the .opd section with .name, .TOC base,
>>> etc.
>>>
>>> Manually setting VDS64_HAS_DESCRIPTORS results in a vdso64.so in which
>>> binutils tools like readelf properly report functions with symbol type
>>> FUNC instead of NOTYPE.
>>>
>>> Are there pieces of the build/etc toolchain unprepared for function
>>> descriptors?  I'm just trying to figure out why the code defaults to
>>> unsetting them.
>>
>> Because direct calls are faster than indirect calls?  Ben might have a
>> fuller explanation, cc:ing him.
> 
> I don't remember why :-) I think I didn't want to mess with the OPD
> fixup in glibc back then.
> 

Does it make sense to just drop the unused VDS64_HAS_DESCRIPTORS code then?

-- Joe


^ permalink raw reply

* Re: [RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)
From: Krzysztof Kozlowski @ 2020-02-24 14:44 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Rich Felker, Jiri Slaby, Geert Uytterhoeven, Michael S. Tsirkin,
	David Airlie, Jason Wang, DRI Development, virtualization,
	James E.J. Bottomley, netdev, Paul Mackerras, Linux-Arch,
	Dave Jiang, Yoshinori Sato, Helge Deller, Linux-sh list,
	Alexey Brodkin, Ben Skeggs, nouveau, Dave Airlie, Matt Turner,
	arcml, Nick Kossifidis, Allen Hubbe, Arnd Bergmann, alpha,
	Ivan Kokshaysky, Thomas Gleixner, Mauro Carvalho Chehab,
	Kalle Valo, Richard Henderson, Parisc List, Vineet Gupta,
	linux-wireless, Linux Kernel Mailing List, Luis Chamberlain,
	Daniel Vetter, Jon Mason, linux-ntb, Andrew Morton,
	Linux Media Mailing List, linuxppc-dev, David S. Miller
In-Reply-To: <CAMuHMdVev0PLF=8bD3nHrhcU9UgM-DAgdQpXi09PgvAxdbt24A@mail.gmail.com>

On Mon, Feb 24, 2020 at 01:54:00PM +0100, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Mon, Feb 24, 2020 at 1:47 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > On Thu, Feb 20, 2020 at 10:48:33AM +0100, Jiri Slaby wrote:
> > > On 19. 02. 20, 18:50, Krzysztof Kozlowski wrote:
> > > > The ioreadX() helpers have inconsistent interface.  On some architectures
> > > > void *__iomem address argument is a pointer to const, on some not.
> > > >
> > > > Implementations of ioreadX() do not modify the memory under the address
> > > > so they can be converted to a "const" version for const-safety and
> > > > consistency among architectures.
> > > >
> > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > > > Acked-by: Kalle Valo <kvalo@codeaurora.org>
> > > > ---
> > > >  drivers/net/wireless/ath/ath5k/ahb.c | 10 +++++-----
> > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
> > > > index 2c9cec8b53d9..8bd01df369fb 100644
> > > > --- a/drivers/net/wireless/ath/ath5k/ahb.c
> > > > +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> > > > @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
> > > >
> > > >     if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> > > >             /* Enable WMAC AHB arbitration */
> > > > -           reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> > > > +           reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> > >
> > > While I understand why the parameter of ioread32 should be const, I
> > > don't see a reason for these casts on the users' side. What does it
> > > bring except longer code to read?
> >
> > Because the argument is an int:
> >
> > drivers/net/wireless/ath/ath5k/ahb.c: In function ‘ath_ahb_probe’:
> > drivers/net/wireless/ath/ath5k/ahb.c:141:18: warning: passing argument 1 of ‘ioread32’ makes pointer from integer without a cast [-Wint-conversion]
> >    reg = ioread32(AR5K_AR2315_AHB_ARB_CTL);
> 
> That's an argument for keeping the cast to "void __iomem *", not for
> adding the "const", right?

Yes, correct. Maybe I misunderstood the question... The const on the
other hand does not have to be in the cast. It is merely for making it
consistent with interface. It is not required. I also mentioned it in
the cover letter:
"PAtches 5-9 are optional cleanup, without actual impact"

Feel free to ignore this one if it is not worth the effort.

Best regards,
Krzysztof


^ permalink raw reply

* RE: [RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)
From: David Laight @ 2020-02-24 14:00 UTC (permalink / raw)
  To: 'Geert Uytterhoeven', Krzysztof Kozlowski
  Cc: Rich Felker, Jiri Slaby, Geert Uytterhoeven, Michael S. Tsirkin,
	David Airlie, nouveau@lists.freedesktop.org, Jason Wang,
	DRI Development, virtualization@lists.linux-foundation.org,
	James E.J. Bottomley, Paul Mackerras, Linux-Arch, Dave Jiang,
	Yoshinori Sato, Helge Deller, Linux-sh list, Alexey Brodkin,
	Ben Skeggs, Dave Airlie, Matt Turner, arcml, Nick Kossifidis,
	Allen Hubbe, Arnd Bergmann, Jon Mason, Ivan Kokshaysky,
	Thomas Gleixner, Mauro Carvalho Chehab, Kalle Valo,
	Richard Henderson, Parisc List, netdev, linux-wireless,
	Linux Kernel Mailing List, Luis Chamberlain, Vineet Gupta,
	Daniel Vetter, alpha, linux-ntb@googlegroups.com, Andrew Morton,
	Linux Media Mailing List, linuxppc-dev, David S. Miller
In-Reply-To: <CAMuHMdVev0PLF=8bD3nHrhcU9UgM-DAgdQpXi09PgvAxdbt24A@mail.gmail.com>

From: Geert Uytterhoeven
> Sent: 24 February 2020 12:54
> To: Krzysztof Kozlowski <krzk@kernel.org>
...
> > > > diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
> > > > index 2c9cec8b53d9..8bd01df369fb 100644
> > > > --- a/drivers/net/wireless/ath/ath5k/ahb.c
> > > > +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> > > > @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
> > > >
> > > >     if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> > > >             /* Enable WMAC AHB arbitration */
> > > > -           reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> > > > +           reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> > >
> > > While I understand why the parameter of ioread32 should be const, I
> > > don't see a reason for these casts on the users' side. What does it
> > > bring except longer code to read?
> >
> > Because the argument is an int:
> >
> > drivers/net/wireless/ath/ath5k/ahb.c: In function ‘ath_ahb_probe’:
> > drivers/net/wireless/ath/ath5k/ahb.c:141:18: warning: passing argument 1 of ‘ioread32’ makes pointer
> from integer without a cast [-Wint-conversion]
> >    reg = ioread32(AR5K_AR2315_AHB_ARB_CTL);
> 
> That's an argument for keeping the cast to "void __iomem *", not for
> adding the "const", right?

Or more likely change the definitions to use a struct for the layout.
That also stops the constants being used in the wrong place.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)
From: Geert Uytterhoeven @ 2020-02-24 12:54 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rich Felker, Jiri Slaby, Geert Uytterhoeven, Michael S. Tsirkin,
	David Airlie, Jason Wang, DRI Development, virtualization,
	James E.J. Bottomley, netdev, Paul Mackerras, Linux-Arch,
	Dave Jiang, Yoshinori Sato, Helge Deller, Linux-sh list,
	Alexey Brodkin, Ben Skeggs, nouveau, Dave Airlie, Matt Turner,
	arcml, Nick Kossifidis, Allen Hubbe, Arnd Bergmann, alpha,
	Ivan Kokshaysky, Thomas Gleixner, Mauro Carvalho Chehab,
	Kalle Valo, Richard Henderson, Parisc List, Vineet Gupta,
	linux-wireless, Linux Kernel Mailing List, Luis Chamberlain,
	Daniel Vetter, Jon Mason, linux-ntb, Andrew Morton,
	Linux Media Mailing List, linuxppc-dev, David S. Miller
In-Reply-To: <20200224124744.GA1949@pi3>

Hi Krzysztof,

On Mon, Feb 24, 2020 at 1:47 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Thu, Feb 20, 2020 at 10:48:33AM +0100, Jiri Slaby wrote:
> > On 19. 02. 20, 18:50, Krzysztof Kozlowski wrote:
> > > The ioreadX() helpers have inconsistent interface.  On some architectures
> > > void *__iomem address argument is a pointer to const, on some not.
> > >
> > > Implementations of ioreadX() do not modify the memory under the address
> > > so they can be converted to a "const" version for const-safety and
> > > consistency among architectures.
> > >
> > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > > Acked-by: Kalle Valo <kvalo@codeaurora.org>
> > > ---
> > >  drivers/net/wireless/ath/ath5k/ahb.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
> > > index 2c9cec8b53d9..8bd01df369fb 100644
> > > --- a/drivers/net/wireless/ath/ath5k/ahb.c
> > > +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> > > @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
> > >
> > >     if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> > >             /* Enable WMAC AHB arbitration */
> > > -           reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> > > +           reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> >
> > While I understand why the parameter of ioread32 should be const, I
> > don't see a reason for these casts on the users' side. What does it
> > bring except longer code to read?
>
> Because the argument is an int:
>
> drivers/net/wireless/ath/ath5k/ahb.c: In function ‘ath_ahb_probe’:
> drivers/net/wireless/ath/ath5k/ahb.c:141:18: warning: passing argument 1 of ‘ioread32’ makes pointer from integer without a cast [-Wint-conversion]
>    reg = ioread32(AR5K_AR2315_AHB_ARB_CTL);

That's an argument for keeping the cast to "void __iomem *", not for
adding the "const", right?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)
From: Krzysztof Kozlowski @ 2020-02-24 12:47 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Rich Felker, Geert Uytterhoeven, Michael S. Tsirkin, David Airlie,
	Jason Wang, dri-devel, virtualization, James E.J. Bottomley,
	netdev, Paul Mackerras, linux-arch, Dave Jiang, Yoshinori Sato,
	Helge Deller, linux-sh, Alexey Brodkin, Ben Skeggs, nouveau,
	Dave Airlie, Matt Turner, linux-snps-arc, Nick Kossifidis,
	Allen Hubbe, Arnd Bergmann, linux-alpha, Ivan Kokshaysky,
	Thomas Gleixner, Mauro Carvalho Chehab, Kalle Valo,
	Richard Henderson, linux-parisc, Vineet Gupta, linux-wireless,
	linux-kernel, Luis Chamberlain, Daniel Vetter, Jon Mason,
	linux-ntb, Andrew Morton, linux-media, linuxppc-dev,
	David S. Miller
In-Reply-To: <518a9023-f802-17b3-fca5-582400bc34ae@gmail.com>

On Thu, Feb 20, 2020 at 10:48:33AM +0100, Jiri Slaby wrote:
> On 19. 02. 20, 18:50, Krzysztof Kozlowski wrote:
> > The ioreadX() helpers have inconsistent interface.  On some architectures
> > void *__iomem address argument is a pointer to const, on some not.
> > 
> > Implementations of ioreadX() do not modify the memory under the address
> > so they can be converted to a "const" version for const-safety and
> > consistency among architectures.
> > 
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > Acked-by: Kalle Valo <kvalo@codeaurora.org>
> > ---
> >  drivers/net/wireless/ath/ath5k/ahb.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
> > index 2c9cec8b53d9..8bd01df369fb 100644
> > --- a/drivers/net/wireless/ath/ath5k/ahb.c
> > +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> > @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
> >  
> >  	if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> >  		/* Enable WMAC AHB arbitration */
> > -		reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> > +		reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> 
> While I understand why the parameter of ioread32 should be const, I
> don't see a reason for these casts on the users' side. What does it
> bring except longer code to read?

Because the argument is an int:

drivers/net/wireless/ath/ath5k/ahb.c: In function ‘ath_ahb_probe’:
drivers/net/wireless/ath/ath5k/ahb.c:141:18: warning: passing argument 1 of ‘ioread32’ makes pointer from integer without a cast [-Wint-conversion]
   reg = ioread32(AR5K_AR2315_AHB_ARB_CTL);

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH] powerpc: Include .BTF section
From: Michael Ellerman @ 2020-02-24 11:03 UTC (permalink / raw)
  To: Naveen N. Rao; +Cc: linuxppc-dev
In-Reply-To: <1582516561.ya4ydk7t7x.naveen@linux.ibm.com>

"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> Michael Ellerman wrote:
>> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
>>> Selecting CONFIG_DEBUG_INFO_BTF results in the below warning from ld:
>>>   ld: warning: orphan section `.BTF' from `.btf.vmlinux.bin.o' being placed in section `.BTF'
>>>
>>> Include .BTF section in vmlinux explicitly to fix the same.
>> 
>> I don't see any other architectures doing this in their linker script.
>> Why are we special?
>
> I think this is due to commit 83a092cf95f28 ("powerpc: Link warning for 
> orphan sections"):
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=83a092cf95f28

Oh haha, right :}

> PS: I also see a linker warning for '.gnu.hash' if I enable 
> CONFIG_RELOCATABLE, but I couldn't decipher what that was for, and if it 
> should be retained.

Yeah, Joel tried to remove it but it caused breakage:

  https://lore.kernel.org/linuxppc-dev/87d0pwqweg.fsf@concordia.ellerman.id.au/


I never got around to looking at it.

cheers

^ permalink raw reply

* Re: [PATCH 3/7] docs: fix broken references to text files
From: Federico Vaga @ 2020-02-24  9:26 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-arch, linux-nfs, kvm, Jonathan Corbet, linux-rdma, netdev,
	Linux Doc Mailing List, linux-unionfs, kvm-ppc, linux-mm,
	dri-devel, linux-fsdevel, linuxppc-dev, kvmarm, linux-arm-kernel
In-Reply-To: <5cfeed6df208b74913312a1c97235ee615180f91.1582361737.git.mchehab+huawei@kernel.org>

On Saturday, February 22, 2020 10:00:03 AM CET Mauro Carvalho Chehab wrote:
> Several references got broken due to txt to ReST conversion.
> 
> Several of them can be automatically fixed with:
> 
> 	scripts/documentation-file-ref-check --fix
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---


>  26) If any ioctl's are added by the patch, then also update
> -    ``Documentation/ioctl/ioctl-number.rst``.
> +    ``Documentation/userspace-api/ioctl/ioctl-number.rst``.
> 
>  27) If your modified source code depends on or uses any of the kernel
>      APIs or features that are related to the following ``Kconfig`` symbols,
> diff --git a/Documentation/translations/it_IT/process/submit-checklist.rst
> b/Documentation/translations/it_IT/process/submit-checklist.rst index
> 995ee69fab11..3e575502690f 100644
> --- a/Documentation/translations/it_IT/process/submit-checklist.rst
> +++ b/Documentation/translations/it_IT/process/submit-checklist.rst
> @@ -117,7 +117,7 @@ sottomissione delle patch, in particolare
>      sorgenti che ne spieghi la logica: cosa fanno e perché.
> 
>  25) Se la patch aggiunge nuove chiamate ioctl, allora aggiornate
> -    ``Documentation/ioctl/ioctl-number.rst``.
> +    ``Documentation/userspace-api/ioctl/ioctl-number.rst``.


Acked-By: Federico Vaga <federico.vaga@vaga.pv.it>




^ permalink raw reply

* Re: [RFC PATCH v2 00/12] Reduce ifdef mess in ptrace
From: Michael Ellerman @ 2020-02-24 10:54 UTC (permalink / raw)
  To: Christophe Leroy, Michael Neuling
  Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <5b5d8f61-c9aa-1afd-6001-44a17f00c1a6@c-s.fr>

Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 24/02/2020 à 03:15, Michael Neuling a écrit :
>> Christophe,
>>> Le 28/06/2019 à 17:47, Christophe Leroy a écrit :
>>>> The purpose of this series is to reduce the amount of #ifdefs
>>>> in ptrace.c
>>>
>>> Any feedback on this series which aims at fixing the issue you opened at
>>> https://github.com/linuxppc/issues/issues/128 ?
>> 
>> Yeah, sorry my bad. You did all the hard work and I ignored it.
>> 
>> I like the approach and is a long the lines I was thinking. Putting it in a
>> ptrace subdir, splitting out adv_debug_regs, TM, SPE, Alitivec, VSX.
>> ppc_gethwdinfo() looks a lot nicer now too (that was some of the worst of it).
>> 
>> I've not gone through it with a fine tooth comb though. There is (rightly) a lot
>> of code moved around which could have introduced some issues.
>> 
>> It applies on v5.2 but are you planning on updating it to a newer base?
>> 
>
> As you noticed there is a lot of code moved around, and rebasing 
> produces a lot of conflicts. So I didn't want to spend hours to rebase 
> and rebase without being sure it was the right approach.
>
> Now that I got a positive feedback I'll consider rebasing it, hopping 
> that Michael will pick it up.

I would love to.

cheers

^ permalink raw reply

* Re: [RFC PATCH v2 04/12] powerpc/ptrace: split out VSX related functions.
From: Michael Ellerman @ 2020-02-24 10:51 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, mikey
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <920fe735d5f3dd882331b36a895bb756bd415fe7.1561735587.git.christophe.leroy@c-s.fr>

Christophe Leroy <christophe.leroy@c-s.fr> writes:
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-novsx.c b/arch/powerpc/kernel/ptrace/ptrace-novsx.c
> new file mode 100644
> index 000000000000..55fbbb4aa9d7
> --- /dev/null
> +++ b/arch/powerpc/kernel/ptrace/ptrace-novsx.c
> @@ -0,0 +1,83 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include <linux/kernel.h>
> +#include <linux/sched.h>
> +#include <linux/mm.h>
> +#include <linux/smp.h>
> +#include <linux/errno.h>
> +#include <linux/ptrace.h>
> +#include <linux/regset.h>
> +#include <linux/tracehook.h>
> +#include <linux/elf.h>
> +#include <linux/user.h>
> +#include <linux/security.h>
> +#include <linux/signal.h>
> +#include <linux/seccomp.h>
> +#include <linux/audit.h>
> +#include <trace/syscall.h>
> +#include <linux/hw_breakpoint.h>
> +#include <linux/perf_event.h>
> +#include <linux/context_tracking.h>
> +#include <linux/nospec.h>
> +
> +#include <linux/uaccess.h>
> +#include <linux/pkeys.h>
> +#include <asm/page.h>
> +#include <asm/pgtable.h>
> +#include <asm/switch_to.h>
> +#include <asm/tm.h>
> +#include <asm/asm-prototypes.h>
> +#include <asm/debug.h>
> +#include <asm/hw_breakpoint.h>

I suspect we probably don't need all those headers anymore. But I guess
we'll clean them up in future, as it's very tedious work to trim the list.

> +
> +#include <kernel/ptrace/ptrace-decl.h>

It's preferable to use:

#include "ptrace-decl.h"

cheers

^ permalink raw reply

* Re: [RFC PATCH v2 02/12] powerpc/ptrace: drop unnecessary #ifdefs CONFIG_PPC64
From: Michael Ellerman @ 2020-02-24 10:48 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, mikey
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <34af3942cd27f6b5365caae772fb8e0af44763d5.1561735587.git.christophe.leroy@c-s.fr>

Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Drop a bunch of #ifdefs CONFIG_PPC64 that are not vital.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/include/asm/ptrace.h      |  9 ++++-----
>  arch/powerpc/include/uapi/asm/ptrace.h | 12 ++++--------
>  arch/powerpc/kernel/ptrace/ptrace.c    | 24 +++---------------------
>  3 files changed, 11 insertions(+), 34 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index faa5a338ac5a..1506a9c61d50 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -36,11 +36,10 @@ struct pt_regs
>  			unsigned long link;
>  			unsigned long xer;
>  			unsigned long ccr;
> -#ifdef CONFIG_PPC64
> -			unsigned long softe;
> -#else
> -			unsigned long mq;
> -#endif
> +			union {
> +				unsigned long softe;
> +				unsigned long mq;
> +			};
>  			unsigned long trap;
>  			unsigned long dar;
>  			unsigned long dsisr;
> diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
> index f5f1ccc740fc..37d7befbb8dc 100644
> --- a/arch/powerpc/include/uapi/asm/ptrace.h
> +++ b/arch/powerpc/include/uapi/asm/ptrace.h
> @@ -43,12 +43,11 @@ struct pt_regs
>  	unsigned long link;
>  	unsigned long xer;
>  	unsigned long ccr;
> -#ifdef __powerpc64__
> -	unsigned long softe;		/* Soft enabled/disabled */
> -#else
> -	unsigned long mq;		/* 601 only (not used at present) */
> +	union {
> +		unsigned long softe;	/* Soft enabled/disabled */
> +		unsigned long mq;	/* 601 only (not used at present) */
>  					/* Used on APUS to hold IPL value. */
> -#endif
> +	};

As Andreas pointed out this is not safe as this is a uapi header.

>  	unsigned long trap;		/* Reason for being here */
>  	/* N.B. for critical exceptions on 4xx, the dar and dsisr
>  	   fields are overloaded to hold srr0 and srr1. */
> @@ -105,11 +104,8 @@ struct pt_regs
>  #define PT_LNK	36
>  #define PT_XER	37
>  #define PT_CCR	38
> -#ifndef __powerpc64__
>  #define PT_MQ	39
> -#else
>  #define PT_SOFTE 39
> -#endif

I'd also rather leave that as it is.

There's a slim chance it could break some code that already has either
of those defined.

If you need them both defined to make other code work in the kernel
that's fine, in the kernel header we can do:

// Ensure these are always defined inside the kernel to avoid #ifdefs
#ifdef CONFIG_PPC64
#define PT_MQ	39
#else
#define PT_SOFTE 39
#endif


>  #define PT_TRAP	40
>  #define PT_DAR	41
>  #define PT_DSISR 42
> diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
> index 684b0b315c32..0afb223c4d57 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> @@ -113,11 +113,8 @@ static const struct pt_regs_offset regoffset_table[] = {
>  	REG_OFFSET_NAME(link),
>  	REG_OFFSET_NAME(xer),
>  	REG_OFFSET_NAME(ccr),
> -#ifdef CONFIG_PPC64
>  	REG_OFFSET_NAME(softe),
> -#else
>  	REG_OFFSET_NAME(mq),
> -#endif

Pretty sure that will cause breakage. The offset is ABI.


cheers


^ permalink raw reply

* Re: vdso function descriptors (VDS64_HAS_DESCRIPTORS)?
From: Benjamin Herrenschmidt @ 2020-02-24 10:17 UTC (permalink / raw)
  To: Segher Boessenkool, Joe Lawrence; +Cc: linuxppc-dev
In-Reply-To: <20200223000715.GW22482@gate.crashing.org>

On Sat, 2020-02-22 at 18:07 -0600, Segher Boessenkool wrote:
> > 
> > so I don't believe they are ever used by default -- in this case
> > V_FUNCTION_BEGIN doesn't add to the .opd section with .name, .TOC base,
> > etc.
> > 
> > Manually setting VDS64_HAS_DESCRIPTORS results in a vdso64.so in which
> > binutils tools like readelf properly report functions with symbol type
> > FUNC instead of NOTYPE.
> > 
> > Are there pieces of the build/etc toolchain unprepared for function
> > descriptors?  I'm just trying to figure out why the code defaults to
> > unsetting them.
> 
> Because direct calls are faster than indirect calls?  Ben might have a
> fuller explanation, cc:ing him.

I don't remember why :-) I think I didn't want to mess with the OPD
fixup in glibc back then.

Ben.



^ permalink raw reply

* Re: [RESEND PATCH v5 2/5] arm64/crash_core: Export TCR_EL1.T1SZ in vmcoreinfo
From: Bhupesh Sharma @ 2020-02-24  6:25 UTC (permalink / raw)
  To: Amit Kachhap
  Cc: Mark Rutland, x86, Will Deacon, Linux Doc Mailing List,
	Catalin Marinas, Ard Biesheuvel, kexec mailing list,
	Linux Kernel Mailing List, Kazuhito Hagio, James Morse,
	Dave Anderson, bhupesh linux, linuxppc-dev, linux-arm-kernel,
	Steve Capper
In-Reply-To: <974f3601-25f8-f4e6-43a8-ff4275e9c174@arm.com>

Hi Amit,

On Fri, Feb 21, 2020 at 2:36 PM Amit Kachhap <amit.kachhap@arm.com> wrote:
>
> Hi Bhupesh,
>
> On 1/13/20 5:44 PM, Bhupesh Sharma wrote:
> > Hi James,
> >
> > On 01/11/2020 12:30 AM, Dave Anderson wrote:
> >>
> >> ----- Original Message -----
> >>> Hi Bhupesh,
> >>>
> >>> On 25/12/2019 19:01, Bhupesh Sharma wrote:
> >>>> On 12/12/2019 04:02 PM, James Morse wrote:
> >>>>> On 29/11/2019 19:59, Bhupesh Sharma wrote:
> >>>>>> vabits_actual variable on arm64 indicates the actual VA space size,
> >>>>>> and allows a single binary to support both 48-bit and 52-bit VA
> >>>>>> spaces.
> >>>>>>
> >>>>>> If the ARMv8.2-LVA optional feature is present, and we are running
> >>>>>> with a 64KB page size; then it is possible to use 52-bits of address
> >>>>>> space for both userspace and kernel addresses. However, any kernel
> >>>>>> binary that supports 52-bit must also be able to fall back to 48-bit
> >>>>>> at early boot time if the hardware feature is not present.
> >>>>>>
> >>>>>> Since TCR_EL1.T1SZ indicates the size offset of the memory region
> >>>>>> addressed by TTBR1_EL1 (and hence can be used for determining the
> >>>>>> vabits_actual value) it makes more sense to export the same in
> >>>>>> vmcoreinfo rather than vabits_actual variable, as the name of the
> >>>>>> variable can change in future kernel versions, but the architectural
> >>>>>> constructs like TCR_EL1.T1SZ can be used better to indicate intended
> >>>>>> specific fields to user-space.
> >>>>>>
> >>>>>> User-space utilities like makedumpfile and crash-utility, need to
> >>>>>> read/write this value from/to vmcoreinfo
> >>>>>
> >>>>> (write?)
> >>>>
> >>>> Yes, also write so that the vmcoreinfo from an (crashing) arm64
> >>>> system can
> >>>> be used for
> >>>> analysis of the root-cause of panic/crash on say an x86_64 host using
> >>>> utilities like
> >>>> crash-utility/gdb.
> >>>
> >>> I read this as as "User-space [...] needs to write to vmcoreinfo".
> >
> > That's correct. But for writing to vmcore dump in the kdump kernel, we
> > need to read the symbols from the vmcoreinfo in the primary kernel.
> >
> >>>>>> for determining if a virtual address lies in the linear map range.
> >>>>>
> >>>>> I think this is a fragile example. The debugger shouldn't need to know
> >>>>> this.
> >>>>
> >>>> Well that the current user-space utility design, so I am not sure we
> >>>> can
> >>>> tweak that too much.
> >>>>
> >>>>>> The user-space computation for determining whether an address lies in
> >>>>>> the linear map range is the same as we have in kernel-space:
> >>>>>>
> >>>>>>     #define __is_lm_address(addr)    (!(((u64)addr) &
> >>>>>> BIT(vabits_actual -
> >>>>>>     1)))
> >>>>>
> >>>>> This was changed with 14c127c957c1 ("arm64: mm: Flip kernel VA
> >>>>> space"). If
> >>>>> user-space
> >>>>> tools rely on 'knowing' the kernel memory layout, they must have to
> >>>>> constantly be fixed
> >>>>> and updated. This is a poor argument for adding this to something that
> >>>>> ends up as ABI.
> >>>>
> >>>> See above. The user-space has to rely on some ABI/guaranteed
> >>>> hardware-symbols which can be
> >>>> used for 'determining' the kernel memory layout.
> >>>
> >>> I disagree. Everything and anything in the kernel will change. The
> >>> ABI rules apply to
> >>> stuff exposed via syscalls and kernel filesystems. It does not apply
> >>> to kernel internals,
> >>> like the memory layout we used yesterday. 14c127c957c1 is a case in
> >>> point.
> >>>
> >>> A debugger trying to rely on this sort of thing would have to play
> >>> catchup whenever it
> >>> changes.
> >>
> >> Exactly.  That's the whole point.
> >>
> >> The crash utility and makedumpfile are not in the same league as other
> >> user-space tools.
> >> They have always had to "play catchup" precisely because they depend
> >> upon kernel internals,
> >> which constantly change.
> >
> > I agree with you and DaveA here. Software user-space debuggers are
> > dependent on kernel internals (which can change from time-to-time) and
> > will have to play catch-up (which has been the case since the very start).
> >
> > Unfortunately we don't have any clear ABI for software debugging tools -
> > may be something to look for in future.
> >
> > A case in point is gdb/kgdb, which still needs to run with KASLR
> > turned-off (nokaslr) for debugging, as it confuses gdb which resolve
> > kernel symbol address from symbol table of vmlinux. But we can
> > work-around the same in makedumpfile/crash by reading the 'kaslr_offset'
> > value. And I have several users telling me now they cannot use gdb on
> > KASLR enabled kernel to debug panics, but can makedumpfile + crash
> > combination to achieve the same.
> >
> > So, we should be looking to fix these utilities which are broken since
> > the 52-bit changes for arm64. Accordingly, I will try to send the v6
> > soon while incorporating the comments posted on the v5.
>
> Any update on the next v6 version. Since this patch series is fixing the
> current broken kdump so need this series to add some more fields in
> vmcoreinfo for Pointer Authentication work.

Sorry for the delay. I was caught up in some other urgent arm64
user-space issues.
I am preparing the v6 now and hopefully will be able to post it out
for review later today.

Thanks,
Bhupesh


^ permalink raw reply

* Re: [PATCH 1/2] powerpc/powernv: Treat an empty reboot string as default
From: Segher Boessenkool @ 2020-02-24 10:04 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: linuxppc-dev, Oliver O'Halloran
In-Reply-To: <767d85e7-7d8f-4a88-b16e-a4d69fb51343@ozlabs.ru>

On Mon, Feb 24, 2020 at 01:32:28PM +1100, Alexey Kardashevskiy wrote:
> On 17/02/2020 13:48, Oliver O'Halloran wrote:
> > Treat an empty reboot cmd string the same as a NULL string. This squashes a
> > spurious unsupported reboot message that sometimes gets out when using
> > xmon.

> > -		if (!cmd)
> > +		if (!cmd || !strlen(cmd))
> 
> nit: this does not matter here in practice but
> 
> if (!cmd || cmd[0] == '\0')
> 
> is faster (you do not care about the length anyway) and safer (@cmd can
> potentially be endless) ;)

No it isn't, this compiles to identical machine code.  (I tested with
GCC 9, and going back until 4.6 -- the generated code becomes
progressively worse (unrelated to this code, fwiw), but identical for
both cases all the time).


Segher

^ permalink raw reply

* Re: [PATCH v3 6/7] mm/memory_hotplug: Add pgprot_t to mhp_params
From: David Hildenbrand @ 2020-02-24  9:26 UTC (permalink / raw)
  To: Logan Gunthorpe, linux-kernel, linux-arm-kernel, linux-ia64,
	linuxppc-dev, linux-s390, linux-sh, platform-driver-x86, linux-mm,
	Dan Williams, Michal Hocko, Andrew Morton
  Cc: Eric Badger, Peter Zijlstra, Catalin Marinas, Dave Hansen,
	Michal Hocko, Ingo Molnar, Borislav Petkov, Andy Lutomirski,
	Thomas Gleixner, Will Deacon, Christoph Hellwig
In-Reply-To: <20200221182503.28317-7-logang@deltatee.com>

On 21.02.20 19:25, Logan Gunthorpe wrote:
> devm_memremap_pages() is currently used by the PCI P2PDMA code to create
> struct page mappings for IO memory. At present, these mappings are created
> with PAGE_KERNEL which implies setting the PAT bits to be WB. However, on
> x86, an mtrr register will typically override this and force the cache
> type to be UC-. In the case firmware doesn't set this register it is
> effectively WB and will typically result in a machine check exception
> when it's accessed.
> 
> Other arches are not currently likely to function correctly seeing they
> don't have any MTRR registers to fall back on.
> 
> To solve this, provide a way to specify the pgprot value explicitly to
> arch_add_memory().
> 
> Of the arches that support MEMORY_HOTPLUG: x86_64, and arm64 need a simple
> change to pass the pgprot_t down to their respective functions which set
> up the page tables. For x86_32, set the page tables explicitly using
> _set_memory_prot() (seeing they are already mapped). For ia64, s390 and
> sh, reject anything but PAGE_KERNEL settings -- this should be fine,
> for now, seeing these architectures don't support ZONE_DEVICE.
> 
> A check in __add_pages() is also added to ensure the pgprot parameter was
> set for all arches.
> 
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Acked-by: Michal Hocko <mhocko@suse.com>

[...]

> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index c5df1b3dada0..30d6c1b8847e 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -56,9 +56,11 @@ enum {
>  /*
>   * Extended parameters for memory hotplug:
>   * altmap: alternative allocator for memmap array (optional)
> + * pgprot: page protection flags to apply to newly added page tables (required)

s/added/created/?



-- 
Thanks,

David / dhildenb


^ permalink raw reply

* Re: [PATCH][next] soc: fsl: dpio: fix dereference of pointer p before null check
From: Dan Carpenter @ 2020-02-24  6:35 UTC (permalink / raw)
  To: Colin King
  Cc: linux-kernel, Roy Pledge, kernel-janitors, Youri Querry, Li Yang,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200221231143.30131-1-colin.king@canonical.com>

On Fri, Feb 21, 2020 at 11:11:43PM +0000, Colin King wrote:
> ---
>  drivers/soc/fsl/dpio/qbman-portal.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/fsl/dpio/qbman-portal.c b/drivers/soc/fsl/dpio/qbman-portal.c
> index 740ee0d19582..d1f49caa5b13 100644
> --- a/drivers/soc/fsl/dpio/qbman-portal.c
> +++ b/drivers/soc/fsl/dpio/qbman-portal.c
> @@ -249,10 +249,11 @@ struct qbman_swp *qbman_swp_init(const struct qbman_swp_desc *d)
>  	u32 mask_size;
>  	u32 eqcr_pi;
>  
> -	spin_lock_init(&p->access_spinlock);
> -
>  	if (!p)
>  		return NULL;
> +
> +	spin_lock_init(&p->access_spinlock);

Allocations in the declaration blog are not super common in the kernel,
but they're more bug prone.  Generally, it's not beautiful to call a
function which can fail in the allocation block.

regards,
dan carpenter


^ permalink raw reply

* RE: [PATCH v6 2/3] arm64: dts: ls1028a: Add PCIe controller DT nodes
From: Z.q. Hou @ 2020-02-24  9:22 UTC (permalink / raw)
  To: Michael Walle, Shawn Guo
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	lorenzo.pieralisi@arm.com, Xiaowei Bao, Roy Zang,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Leo Li,
	M.h. Lian, robh+dt@kernel.org, Mingkai Hu, bhelgaas@google.com,
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <a3aeabddc82ca86e3dca9c26081a0077@walle.cc>

Hi Michael and Shawn,

I'll update the patch with iommu-map property.

Thanks,
Zhiqiang

> -----Original Message-----
> From: Michael Walle <michael@walle.cc>
> Sent: 2020年2月24日 16:54
> To: Shawn Guo <shawnguo@kernel.org>
> Cc: Xiaowei Bao <xiaowei.bao@nxp.com>; Z.q. Hou
> <zhiqiang.hou@nxp.com>; bhelgaas@google.com;
> devicetree@vger.kernel.org; Leo Li <leoyang.li@nxp.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> linux-pci@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> lorenzo.pieralisi@arm.com; mark.rutland@arm.com; M.h. Lian
> <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> robh+dt@kernel.org; Roy Zang <roy.zang@nxp.com>
> Subject: Re: [PATCH v6 2/3] arm64: dts: ls1028a: Add PCIe controller DT
> nodes
> 
> Hi Shawn, all,
> 
> Am 2020-02-24 09:43, schrieb Shawn Guo:
> > On Mon, Feb 24, 2020 at 09:11:05AM +0100, Michael Walle wrote:
> >> Hi Xiaowei, Hi Shawn,
> >>
> >> > LS1028a implements 2 PCIe 3.0 controllers.
> >>
> >> Patch 1/3 and 3/3 are in Linus' tree but nobody seems to care about
> >> this patch anymore :(
> >>
> >> This doesn't work well with the IOMMU, because the iommu-map property
> >> is missing. The bootloader needs the &smmu phandle to fixup the
> >> entry.
> >> See
> >> below.
> >>
> >> Shawn, will you add this patch to your tree once its fixed,
> >> considering it just adds the device tree node for the LS1028A?
> >
> > The patch/thread is a bit aged.  You may want to send an updated patch
> > for discussion.
> 
> So should I just pick up the patch add my two fixes and send it again?
> What about
> the Signed-off-by tags? Leave them? Replace them? Add mine?
> 
> -michael

^ permalink raw reply

* Re: [PATCH v3 2/7] mm/memory_hotplug: Rename mhp_restrictions to mhp_params
From: David Hildenbrand @ 2020-02-24  9:11 UTC (permalink / raw)
  To: Logan Gunthorpe, linux-kernel, linux-arm-kernel, linux-ia64,
	linuxppc-dev, linux-s390, linux-sh, platform-driver-x86, linux-mm,
	Dan Williams, Michal Hocko, Andrew Morton
  Cc: Eric Badger, Peter Zijlstra, Catalin Marinas, Dave Hansen,
	Ingo Molnar, Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
	Will Deacon, Christoph Hellwig
In-Reply-To: <20200221182503.28317-3-logang@deltatee.com>

On 21.02.20 19:24, Logan Gunthorpe wrote:
> The mhp_restrictions struct really doesn't specify anything resembling
> a restriction anymore so rename it to be mhp_params as it is a list
> of extended parameters.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 
Thanks,

David / dhildenb


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox