LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx,fpr}_from_user()
From: Christopher M. Riedl @ 2020-10-15 15:01 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20201015150159.28933-1-cmr@codefail.de>

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 <cmr@codefail.de>
---
 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);
 				&buf[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


^ permalink raw reply related

* [PATCH 4/8] powerpc/signal64: Replace setup_sigcontext() w/ unsafe_setup_sigcontext()
From: Christopher M. Riedl @ 2020-10-15 15:01 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20201015150159.28933-1-cmr@codefail.de>

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 <cmr@codefail.de>
---
 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, &sc->v_regs);
+	unsafe_put_user(v_regs, &sc->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, &tsk->thread.vr_state,
-				      33 * sizeof(vector128));
+		unsafe_copy_to_user(v_regs, &tsk->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 *)&v_regs[33]);
+	unsafe_put_user(vrsave, (u32 __user *)&v_regs[33], efault_out);
 #else /* CONFIG_ALTIVEC */
-	err |= __put_user(0, &sc->v_regs);
+	unsafe_put_user(0, &sc->v_regs, efault_out);
 #endif /* CONFIG_ALTIVEC */
 	flush_fp_to_thread(tsk);
 	/* copy fpr regs and fpscr */
-	err |= copy_fpr_to_user(&sc->fp_regs, tsk);
+	unsafe_copy_fpr_to_user(&sc->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(&sc->gp_regs, &sc->regs);
+	unsafe_put_user(&sc->gp_regs, &sc->regs, efault_out);
 	WARN_ON(!FULL_REGS(regs));
-	err |= __copy_to_user(&sc->gp_regs, regs, GP_REGS_SIZE);
-	err |= __put_user(msr, &sc->gp_regs[PT_MSR]);
-	err |= __put_user(softe, &sc->gp_regs[PT_SOFTE]);
-	err |= __put_user(signr, &sc->signal);
-	err |= __put_user(handler, &sc->handler);
+	unsafe_copy_to_user(&sc->gp_regs, regs, GP_REGS_SIZE, efault_out);
+	unsafe_put_user(msr, &sc->gp_regs[PT_MSR], efault_out);
+	unsafe_put_user(softe, &sc->gp_regs[PT_SOFTE], efault_out);
+	unsafe_put_user(signr, &sc->signal, efault_out);
+	unsafe_put_user(handler, &sc->handler, efault_out);
 	if (set != NULL)
-		err |=  __put_user(set->sig[0], &sc->oldmask);
+		unsafe_put_user(set->sig[0], &sc->oldmask, efault_out);
 
-	return err;
+	return 0;
+
+efault_out:
+	return -EFAULT;
 }
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -655,12 +661,15 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
 		ctx_has_vsx_region = 1;
 
 	if (old_ctx != NULL) {
-		if (!access_ok(old_ctx, ctx_size)
-		    || setup_sigcontext(&old_ctx->uc_mcontext, current, 0, NULL, 0,
-					ctx_has_vsx_region)
-		    || __copy_to_user(&old_ctx->uc_sigmask,
-				      &current->blocked, sizeof(sigset_t)))
+		if (!user_write_access_begin(old_ctx, ctx_size))
 			return -EFAULT;
+
+		unsafe_setup_sigcontext(&old_ctx->uc_mcontext, current, 0, NULL,
+					0, ctx_has_vsx_region, efault_out);
+		unsafe_copy_to_user(&old_ctx->uc_sigmask, &current->blocked,
+				    sizeof(sigset_t), efault_out);
+
+		user_write_access_end();
 	}
 	if (new_ctx == NULL)
 		return 0;
@@ -689,6 +698,10 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
 	/* This returns like rt_sigreturn */
 	set_thread_flag(TIF_RESTOREALL);
 	return 0;
+
+efault_out:
+	user_write_access_end();
+	return -EFAULT;
 }
 
 
@@ -842,9 +855,13 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 #endif
 	{
 		err |= __put_user(0, &frame->uc.uc_link);
-		err |= setup_sigcontext(&frame->uc.uc_mcontext, tsk, ksig->sig,
-					NULL, (unsigned long)ksig->ka.sa.sa_handler,
-					1);
+
+		if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
+			return -EFAULT;
+		err |= __unsafe_setup_sigcontext(&frame->uc.uc_mcontext, tsk,
+						ksig->sig, NULL,
+						(unsigned long)ksig->ka.sa.sa_handler, 1);
+		user_write_access_end();
 	}
 	err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));
 	if (err)
-- 
2.28.0


^ permalink raw reply related

* [PATCH 5/8] powerpc/signal64: Replace restore_sigcontext() w/ unsafe_restore_sigcontext()
From: Christopher M. Riedl @ 2020-10-15 15:01 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20201015150159.28933-1-cmr@codefail.de>

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 <cmr@codefail.de>
---
 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, &sc->gp_regs[PT_NIP]);
+	unsafe_copy_from_user(regs->gpr, sc->gp_regs, sizeof(regs->gpr),
+			      efault_out);
+	unsafe_get_user(regs->nip, &sc->gp_regs[PT_NIP], efault_out);
 	/* get MSR separately, transfer the LE bit if doing signal return */
-	err |= __get_user(msr, &sc->gp_regs[PT_MSR]);
+	unsafe_get_user(msr, &sc->gp_regs[PT_MSR], efault_out);
 	if (sig)
 		regs->msr = (regs->msr & ~MSR_LE) | (msr & MSR_LE);
-	err |= __get_user(regs->orig_gpr3, &sc->gp_regs[PT_ORIG_R3]);
-	err |= __get_user(regs->ctr, &sc->gp_regs[PT_CTR]);
-	err |= __get_user(regs->link, &sc->gp_regs[PT_LNK]);
-	err |= __get_user(regs->xer, &sc->gp_regs[PT_XER]);
-	err |= __get_user(regs->ccr, &sc->gp_regs[PT_CCR]);
+	unsafe_get_user(regs->orig_gpr3, &sc->gp_regs[PT_ORIG_R3], efault_out);
+	unsafe_get_user(regs->ctr, &sc->gp_regs[PT_CTR], efault_out);
+	unsafe_get_user(regs->link, &sc->gp_regs[PT_LNK], efault_out);
+	unsafe_get_user(regs->xer, &sc->gp_regs[PT_XER], efault_out);
+	unsafe_get_user(regs->ccr, &sc->gp_regs[PT_CCR], efault_out);
 	/* Don't allow userspace to set SOFTE */
 	set_trap_norestart(regs);
-	err |= __get_user(regs->dar, &sc->gp_regs[PT_DAR]);
-	err |= __get_user(regs->dsisr, &sc->gp_regs[PT_DSISR]);
-	err |= __get_user(regs->result, &sc->gp_regs[PT_RESULT]);
+	unsafe_get_user(regs->dar, &sc->gp_regs[PT_DAR], efault_out);
+	unsafe_get_user(regs->dsisr, &sc->gp_regs[PT_DSISR], efault_out);
+	unsafe_get_user(regs->result, &sc->gp_regs[PT_RESULT], efault_out);
 
 	if (!sig)
 		regs->gpr[13] = save_r13;
 	if (set != NULL)
-		err |=  __get_user(set->sig[0], &sc->oldmask);
+		unsafe_get_user(set->sig[0], &sc->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, &sc->v_regs);
-	if (err)
-		return err;
+	unsafe_get_user(v_regs, &sc->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(&tsk->thread.vr_state, v_regs,
-					33 * sizeof(vector128));
+		unsafe_copy_from_user(&tsk->thread.vr_state, v_regs,
+				      33 * sizeof(vector128), efault_out);
 		tsk->thread.used_vr = true;
 	} else if (tsk->thread.used_vr) {
 		memset(&tsk->thread.vr_state, 0, 33 * sizeof(vector128));
 	}
 	/* Always get VRSAVE back */
 	if (v_regs != NULL)
-		err |= __get_user(tsk->thread.vrsave, (u32 __user *)&v_regs[33]);
+		unsafe_get_user(tsk->thread.vrsave, (u32 __user *)&v_regs[33],
+				efault_out);
 	else
 		tsk->thread.vrsave = 0;
 	if (cpu_has_feature(CPU_FTR_ALTIVEC))
 		mtspr(SPRN_VRSAVE, tsk->thread.vrsave);
 #endif /* CONFIG_ALTIVEC */
 	/* restore floating point */
-	err |= copy_fpr_from_user(tsk, &sc->fp_regs);
+	unsafe_copy_fpr_from_user(tsk, &sc->fp_regs, efault_out);
 #ifdef CONFIG_VSX
 	/*
 	 * Get additional VSX data. Update v_regs to point after the
@@ -401,14 +401,17 @@ static long restore_sigcontext(struct task_struct *tsk, sigset_t *set, int sig,
 	 */
 	v_regs += ELF_NVRREG;
 	if ((msr & MSR_VSX) != 0) {
-		err |= copy_vsx_from_user(tsk, v_regs);
+		unsafe_copy_vsx_from_user(tsk, v_regs, efault_out);
 		tsk->thread.used_vsr = true;
 	} else {
 		for (i = 0; i < 32 ; i++)
 			tsk->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = 0;
 	}
 #endif
-	return err;
+	return 0;
+
+efault_out:
+	return -EFAULT;
 }
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -692,8 +695,14 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
 	if (__copy_from_user(&set, &new_ctx->uc_sigmask, sizeof(set)))
 		do_exit(SIGSEGV);
 	set_current_blocked(&set);
-	if (restore_sigcontext(current, NULL, 0, &new_ctx->uc_mcontext))
+
+	if (!user_read_access_begin(new_ctx, ctx_size))
+		return -EFAULT;
+	if (__unsafe_restore_sigcontext(current, NULL, 0, &new_ctx->uc_mcontext)) {
+		user_read_access_end();
 		do_exit(SIGSEGV);
+	}
+	user_read_access_end();
 
 	/* This returns like rt_sigreturn */
 	set_thread_flag(TIF_RESTOREALL);
@@ -798,8 +807,13 @@ SYSCALL_DEFINE0(rt_sigreturn)
 		 * causing a TM bad thing.
 		 */
 		current->thread.regs->msr &= ~MSR_TS_MASK;
-		if (restore_sigcontext(current, NULL, 1, &uc->uc_mcontext))
+		if (!user_read_access_begin(uc, sizeof(*uc)))
+			return -EFAULT;
+		if (__unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext)) {
+			user_read_access_end();
 			goto badframe;
+		}
+		user_read_access_end();
 	}
 
 	if (restore_altstack(&uc->uc_stack))
-- 
2.28.0


^ permalink raw reply related

* [PATCH 6/8] powerpc/signal64: Replace setup_trampoline() w/ unsafe_setup_trampoline()
From: Christopher M. Riedl @ 2020-10-15 15:01 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Daniel Axtens
In-Reply-To: <20201015150159.28933-1-cmr@codefail.de>

From: Daniel Axtens <dja@axtens.net>

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 <dja@axtens.net>
Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
---
 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, &tramp[0]);
+	unsafe_put_user(PPC_INST_BCTRL, &tramp[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 & 0xffff), &tramp[1]);
+	unsafe_put_user(PPC_INST_ADDI | __PPC_RT(R1) | __PPC_RA(R1) |
+			  (__SIGNAL_FRAMESIZE & 0xffff), &tramp[1], err);
 	/* li r0, __NR_[rt_]sigreturn| */
-	err |= __put_user(PPC_INST_ADDI | (syscall & 0xffff), &tramp[2]);
+	unsafe_put_user(PPC_INST_ADDI | (syscall & 0xffff), &tramp[2], err);
 	/* sc */
-	err |= __put_user(PPC_INST_SC, &tramp[3]);
+	unsafe_put_user(PPC_INST_SC, &tramp[3], err);
 
 	/* Minimal traceback info */
 	for (i=TRAMP_TRACEBACK; i < TRAMP_SIZE ;i++)
-		err |= __put_user(0, &tramp[i]);
+		unsafe_put_user(0, &tramp[i], err);
 
-	if (!err)
-		flush_icache_range((unsigned long) &tramp[0],
-			   (unsigned long) &tramp[TRAMP_SIZE]);
+	flush_icache_range((unsigned long)&tramp[0],
+			   (unsigned long)&tramp[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, &frame->tramp[0]);
+		if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
+			return -EFAULT;
+		err |= __unsafe_setup_trampoline(__NR_rt_sigreturn, &frame->tramp[0]);
+		user_write_access_end();
 		if (err)
 			goto badframe;
 		regs->nip = (unsigned long) &frame->tramp[0];
-- 
2.28.0


^ permalink raw reply related

* [PATCH 0/8] Improve signal performance on PPC64 with KUAP
From: Christopher M. Riedl @ 2020-10-15 15:01 UTC (permalink / raw)
  To: linuxppc-dev

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


^ permalink raw reply

* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Khalid Aziz @ 2020-10-15 14:53 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Jann Horn, linuxppc-dev, linux-kernel, Christoph Hellwig,
	linux-mm, Paul Mackerras, sparclinux, Anthony Yznaga,
	Andrew Morton, Will Deacon, David S. Miller, linux-arm-kernel
In-Reply-To: <20201015084936.GC20197@gaia>

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



^ permalink raw reply

* Re: [PATCH v6 02/11] mm/gup: Use functions to track lockless pgtbl walks on gup_pgd_range
From: Michal Suchánek @ 2020-10-15 14:46 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: linux-mm, Paul Mackerras, Mike Rapoport, linux-arch, Steven Price,
	Mahesh Salgaonkar, Arnd Bergmann, Robin Murphy, kvm-ppc,
	Nicholas Piggin, Thomas Gleixner, Reza Arbab, Allison Randal,
	Christophe Leroy, Greg Kroah-Hartman, linux-kernel,
	Aneesh Kumar K.V, Andrew Morton, linuxppc-dev
In-Reply-To: <760c238043196e0628c8c0eff48a8e938ef539ba.camel@linux.ibm.com>

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, &nr);
> > -               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

^ permalink raw reply

* Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
From: Serge Semin @ 2020-10-15 14:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Andrew Lunn, linux-usb, Neil Armstrong, Tony Lindgren,
	Bjorn Andersson, Pavel Parkhomenko,
	linux-samsung-soc@vger.kernel.org, Kevin Hilman, Gregory Clement,
	Wei Xu, Chen-Yu Tsai, Kukjin Kim, Andy Gross, linux-arm-msm,
	linux-snps-arc, Sebastian Hesselbarth, devicetree, Jason Cooper,
	Mathias Nyman, linux-kernel@vger.kernel.org, Lad Prabhakar,
	Maxime Ripard, Alexey Malahov, Rob Herring, Santosh Shilimkar,
	linux-omap, linux-arm-kernel, Roger Quadros, Felipe Balbi,
	linux-mips, Greg Kroah-Hartman, Yoshihiro Shimoda, linuxppc-dev,
	Patrice Chotard, Serge Semin, Li Yang, Manu Gautam,
	Benoît Cousson, Shawn Guo
In-Reply-To: <20201015061439.GA2926@kozik-lap>

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 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 correcting naming convention.
> > > Not fixing errors.
> > 
> > From your perspective it wasn't an error, from mine and most likely Rob' it
> > was.) Anyway as I said I don't care that much about preserving the subject
> > wording, so what about the next one:
> > <arch>: <subarch>: Harmonize DWC USB3 nodes name with DT schema
> > ?
> 
> Looks good.
> 
> Best regards,
> Krzysztof
> 

^ permalink raw reply

* Re: [PATCH] powerpc: force inlining of csum_partial() to avoid multiple csum_partial() with GCC10
From: Christophe Leroy @ 2020-10-15 13:59 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20201015132512.GG2672@gate.crashing.org>



Le 15/10/2020 à 15:25, Segher Boessenkool a écrit :
> Hi!
> 
> On Thu, Oct 15, 2020 at 10:52:20AM +0000, Christophe Leroy wrote:
>> With gcc9 I get:
> 
> <snip>
> 
>> With gcc10 I get:
> 
> <snip>
> 
>> 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

^ permalink raw reply

* Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
From: Serge Semin @ 2020-10-15 13:53 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Andrew Lunn, linux-usb, Neil Armstrong, Tony Lindgren,
	Bjorn Andersson, Wei Xu, linux-samsung-soc, Kevin Hilman,
	Gregory Clement, Krzysztof Kozlowski, Chen-Yu Tsai, Kukjin Kim,
	Andy Gross, linux-arm-msm, linux-snps-arc, Sebastian Hesselbarth,
	devicetree, Jason Cooper, Mathias Nyman, linux-kernel,
	Lad Prabhakar, Maxime Ripard, Alexey Malahov, Rob Herring,
	Santosh Shilimkar, linux-omap, linux-arm-kernel, Roger Quadros,
	linux-mips, Greg Kroah-Hartman, Yoshihiro Shimoda, linuxppc-dev,
	Patrice Chotard, Serge Semin, Li Yang, Manu Gautam,
	Benoît Cousson, Shawn Guo, Pavel Parkhomenko
In-Reply-To: <875z7blrqu.fsf@kernel.org>

On Thu, Oct 15, 2020 at 01:15:37PM +0300, Felipe Balbi wrote:
> Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:
> 
> > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
> >> 
> >> Hi Serge,
> >> 
> >> Serge Semin <Sergey.Semin@baikalelectronics.ru> 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



^ permalink raw reply

* Re: [PATCH] powerpc: force inlining of csum_partial() to avoid multiple csum_partial() with GCC10
From: Christophe Leroy @ 2020-10-15 13:32 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20201015132512.GG2672@gate.crashing.org>



Le 15/10/2020 à 15:25, Segher Boessenkool a écrit :
> Hi!
> 
> On Thu, Oct 15, 2020 at 10:52:20AM +0000, Christophe Leroy wrote:
>> With gcc9 I get:
> 
> <snip>
> 
>> With gcc10 I get:
> 
> <snip>
> 
>> 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 <get_order>:
c0016790:	38 63 ff ff 	addi    r3,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

^ permalink raw reply

* Re: [PATCH] powerpc: force inlining of csum_partial() to avoid multiple csum_partial() with GCC10
From: Segher Boessenkool @ 2020-10-15 13:25 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <a1d31f84ddb0926813b17fcd5cc7f3fa7b4deac2.1602759123.git.christophe.leroy@csgroup.eu>

Hi!

On Thu, Oct 15, 2020 at 10:52:20AM +0000, Christophe Leroy wrote:
> With gcc9 I get:

<snip>

> With gcc10 I get:

<snip>

> 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@kernel.crashing.org>


Segher

^ permalink raw reply

* Re: [PATCH v2] ima: defer arch_ima_get_secureboot() call to IMA init time
From: Chester Lin @ 2020-10-15 12:16 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: clin, linux-efi, Dmitry Kasatkin, James Morris, jlee,
	linux-security-module, linux-integrity,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Ard Biesheuvel,
	Serge E. Hallyn
In-Reply-To: <98b04c130708893ebefdf81e127a66356b4a6129.camel@linux.ibm.com>

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 <zohar@linux.ibm.com> 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 <clin@suse.com>
> > > > > Cc: Mimi Zohar <zohar@linux.ibm.com>
> > > > > Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> > > > > Cc: James Morris <jmorris@namei.org>
> > > > > Cc: "Serge E. Hallyn" <serge@hallyn.com>
> > > > > Link: https://lore.kernel.org/linux-arm-kernel/20200904072905.25332-2-clin@suse.com/
> > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > ---
> > > > > 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 <clin@suse.com>
> > 
> > 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.000000] 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.000000] 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-0000:04:00.0-part4 mitigations=auto ignore_log
> >   level crashkernel=192M,high crashkernel=72M,low ima_appraise=off
> >   [    0.000000] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point regi
> >   sters'
> >   [    0.000000] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
> >   [    0.000000] 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.000000] 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.000000] 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-0000:04:00.0-part4 mitigations=auto ignore_log
level crashkernel=192M,high crashkernel=72M,low ima_appraise=off
[    0.000000] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point regi
sters'
.....
.....
[    0.000000] efi: EFI v2.60 by EDK II
[    0.000000] efi: SMBIOS=0x7fe82000 ACPI=0x7feb0000 ACPI 2.0=0x7feb0014 MEMATTT
R=0x7ef89698 RNG=0x7ea12e18
[    0.000000] efi: seeding entropy pool
[    0.000000] random: fast init done
[    0.000000] SMBIOS 2.8 present.
[    0.000000] DMI: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
[    0.000000] Hypervisor detected: KVM
.....
.....
[    1.957625] Linux agpgart interface v0.103
[    1.961903] tpm_tis 00:04: 1.2 TPM (device-id 0x1, rev-id 1)
[    1.966306] tpm tpm0: starting up the TPM manually
[    1.981753] ahci 0000:00:1f.2: version 3.0
[    1.984503] PCI Interrupt Link [GSIA] enabled at IRQ 16
.....
.....
[    2.086645] ima: Secure boot enabled: ignoring ima_appraise=off option
[    2.086650] ima: Allocated hash algorithm: sha256
[    2.100662] evm: Initialising EVM extended attributes:
[    2.100689] audit: type=1807 audit(1602760724.564:2): action=measure func=KEX
EC_KERNEL_CHECK res=1
[    2.101849] evm: security.selinux
[    2.101849] evm: security.apparmor
[    2.101849] evm: security.ima
[    2.101850] evm: security.capability
[    2.101850] evm: HMAC attrs: 0x1
[    2.102083] PM:   Magic number: 8:588:327
[    2.106723] audit: type=1807 audit(1602760724.564:3): action=measure func=MODD
ULE_CHECK res=1
[    2.118761] RAS: Correctable Errors collector initialized.
.....
-----


In terms of TPM 2.0, swtpm seems to have issues while running the selftest so
I verified this patch on my laptop as well because it has a real TPM 2.0 chip:

-----
[    0.000000] microcode: microcode updated early to revision 0xd6, date = 2020-
04-27
[    0.000000] 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 1404:00:11 UTC 2020 (8764d18)
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.9.0-858-g865c50e1d279-1.
g8764d18-vanilla root=UUID=2184bbb7-780e-48a2-98b8-3a8cd3366e5e splash=silent re
sume=/dev/disk/by-id/ata-INTEL_SSDSCKKF512G8_SATA_512GB_BTLA82850G3E512K-part3 i
gnore_loglevel crashkernel=222M,high crashkernel=72M,low ima_appraise=log
[    0.000000] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point regi
sters'
.....
.....
[    0.000000] efi: EFI v2.60 by American Megatrends
[    0.000000] efi: ACPI=0x7a34e000 ACPI 2.0=0x7a34e000 SMBIOS=0xf0000 SMBIOS 3.
0=0xf0020 TPMFinalLog=0x7ac8b000 ESRT=0x7b0e0018 MEMATTR=0x7762d018 RNG=0x7b133f
18 TPMEventLog=0x6ab96018
[    0.000000] efi: seeding entropy pool
[    0.000000] random: fast init done
[    0.000000] SMBIOS 3.1.0 present.
[    0.000000] DMI: Dell Inc. Latitude 7490/0KP0FT, BIOS 1.14.0 01/22/2020
.....
.....
[    7.182711] Non-volatile memory driver v1.3
[    7.182748] Linux agpgart interface v0.103
[    7.210826] tpm_tis MSFT0101:00: 2.0 TPM (device-id 0xFC, rev-id 1)
[    7.234324] ahci 0000:00:17.0: version 3.0
.....
.....
[    7.288573] integrity: Loading X.509 certificate: UEFI:MokListRT
[    7.289299] integrity: Loaded X.509 cert 'openSUSE Secure Boot CA: 6842600de22c4c477e95be23dfea9513e5971762'
[    7.289913] ima: Secure boot enabled: ignoring ima_appraise=log option
[    7.289921] ima: Allocated hash algorithm: sha256
[    7.314854] evm: Initialising EVM extended attributes:
[    7.314878] audit: type=1807 audit(1602778176.572:2): action=measure func=KEXEC_KERNEL_CHECK res=1
[    7.316656] evm: security.selinux
[    7.316659] evm: security.apparmor
[    7.318493] audit: type=1807 audit(1602778176.572:3): action=measure func=MODULE_CHECK res=1
[    7.320092] evm: security.ima
[    7.320093] evm: security.capability
[    7.322554] evm: HMAC attrs: 0x1


^ permalink raw reply

* [PATCH] powerpc: force inlining of csum_partial() to avoid multiple csum_partial() with GCC10
From: Christophe Leroy @ 2020-10-15 10:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

	ppc-linux-objdump -d vmlinux | grep -e "<csum_partial>" -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 <csum_partial>:
	c0019020:	4b ff fc e8 	b       c0018d08 <__csum_partial>
	c001914c:	4b ff fe d4 	b       c0019020 <csum_partial>
	c0019250:	4b ff fd d1 	bl      c0019020 <csum_partial>
	c03e404c <csum_partial>:
	c03e404c:	4b c3 4c bc 	b       c0018d08 <__csum_partial>
	c03e4050:	4b ff ff fc 	b       c03e404c <csum_partial>
	c03e40fc:	4b ff ff 51 	bl      c03e404c <csum_partial>
	c03e6680:	4b ff d9 cd 	bl      c03e404c <csum_partial>
	c03e68c4:	4b ff d7 89 	bl      c03e404c <csum_partial>
	c03e7934:	4b ff c7 19 	bl      c03e404c <csum_partial>
	c03e7bf8:	4b ff c4 55 	bl      c03e404c <csum_partial>
	c03eb148:	4b ff 8f 05 	bl      c03e404c <csum_partial>
	c03ecf68:	4b c2 bd a1 	bl      c0018d08 <__csum_partial>
	c04275b8 <csum_partial>:
	c04275b8:	4b bf 17 50 	b       c0018d08 <__csum_partial>
	c0427884:	4b ff fd 35 	bl      c04275b8 <csum_partial>
	c0427b18:	4b ff fa a1 	bl      c04275b8 <csum_partial>
	c0427bd8:	4b ff f9 e1 	bl      c04275b8 <csum_partial>
	c0427cd4:	4b ff f8 e5 	bl      c04275b8 <csum_partial>
	c0427e34:	4b ff f7 85 	bl      c04275b8 <csum_partial>
	c045a1c0:	4b bb eb 49 	bl      c0018d08 <__csum_partial>
	c0489464 <csum_partial>:
	c0489464:	4b b8 f8 a4 	b       c0018d08 <__csum_partial>
	c04896b0:	4b ff fd b5 	bl      c0489464 <csum_partial>
	c048982c:	4b ff fc 39 	bl      c0489464 <csum_partial>
	c0490158:	4b b8 8b b1 	bl      c0018d08 <__csum_partial>
	c0492f0c <csum_partial>:
	c0492f0c:	4b b8 5d fc 	b       c0018d08 <__csum_partial>
	c049326c:	4b ff fc a1 	bl      c0492f0c <csum_partial>
	c049333c:	4b ff fb d1 	bl      c0492f0c <csum_partial>
	c0493b18:	4b ff f3 f5 	bl      c0492f0c <csum_partial>
	c0493f50:	4b ff ef bd 	bl      c0492f0c <csum_partial>
	c0493ffc:	4b ff ef 11 	bl      c0492f0c <csum_partial>
	c04a0f78:	4b b7 7d 91 	bl      c0018d08 <__csum_partial>
	c04b3e3c:	4b b6 4e cd 	bl      c0018d08 <__csum_partial>
	c04b40d0 <csum_partial>:
	c04b40d0:	4b b6 4c 38 	b       c0018d08 <__csum_partial>
	c04b4448:	4b ff fc 89 	bl      c04b40d0 <csum_partial>
	c04b46f4:	4b ff f9 dd 	bl      c04b40d0 <csum_partial>
	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 <__csum_partial>
	c03e40ec:	4b c3 4c 1d 	bl      c0018d08 <__csum_partial>
	c03e4120:	4b c3 4b e8 	b       c0018d08 <__csum_partial>
	c03eb004:	4b c2 dd 05 	bl      c0018d08 <__csum_partial>
	c03ecef4:	4b c2 be 15 	bl      c0018d08 <__csum_partial>
	c0427558:	4b bf 17 b1 	bl      c0018d08 <__csum_partial>
	c04286e4:	4b bf 06 25 	bl      c0018d08 <__csum_partial>
	c0428cd8:	4b bf 00 31 	bl      c0018d08 <__csum_partial>
	c0428d84:	4b be ff 85 	bl      c0018d08 <__csum_partial>
	c045a17c:	4b bb eb 8d 	bl      c0018d08 <__csum_partial>
	c0489450:	4b b8 f8 b9 	bl      c0018d08 <__csum_partial>
	c0491860:	4b b8 74 a9 	bl      c0018d08 <__csum_partial>
	c0492eec:	4b b8 5e 1d 	bl      c0018d08 <__csum_partial>
	c04a0eac:	4b b7 7e 5d 	bl      c0018d08 <__csum_partial>
	c04b3e34:	4b b6 4e d5 	bl      c0018d08 <__csum_partial>
	c04b426c:	4b b6 4a 9d 	bl      c0018d08 <__csum_partial>
	c04b463c:	4b b6 46 cd 	bl      c0018d08 <__csum_partial>
	c04c004c:	4b b5 8c bd 	bl      c0018d08 <__csum_partial>
	c04c0368:	4b b5 89 a1 	bl      c0018d08 <__csum_partial>
	c04c5254:	4b b5 3a b5 	bl      c0018d08 <__csum_partial>
	c04c60d4:	4b b5 2c 35 	bl      c0018d08 <__csum_partial>

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/checksum.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/checksum.h b/arch/powerpc/include/asm/checksum.h
index 9cce06194dcc..cab8fec60005 100644
--- a/arch/powerpc/include/asm/checksum.h
+++ b/arch/powerpc/include/asm/checksum.h
@@ -164,7 +164,7 @@ static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
  */
 __wsum __csum_partial(const void *buff, int len, __wsum sum);
 
-static inline __wsum csum_partial(const void *buff, int len, __wsum sum)
+static __always_inline __wsum csum_partial(const void *buff, int len, __wsum sum)
 {
 	if (__builtin_constant_p(len) && len <= 16 && (len & 1) == 0) {
 		if (len == 2)
-- 
2.25.0


^ permalink raw reply related

* Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
From: Felipe Balbi @ 2020-10-15 10:15 UTC (permalink / raw)
  To: Serge Semin
  Cc: Andrew Lunn, linux-usb, Neil Armstrong, Tony Lindgren,
	Bjorn Andersson, Wei Xu, linux-samsung-soc, Kevin Hilman,
	Gregory Clement, Krzysztof Kozlowski, Chen-Yu Tsai, Kukjin Kim,
	Andy Gross, linux-arm-msm, linux-snps-arc, Sebastian Hesselbarth,
	devicetree, Jason Cooper, Mathias Nyman, linux-kernel,
	Lad Prabhakar, Maxime Ripard, Alexey Malahov, Rob Herring,
	Santosh Shilimkar, linux-omap, linux-arm-kernel, Roger Quadros,
	linux-mips, Greg Kroah-Hartman, Yoshihiro Shimoda, linuxppc-dev,
	Patrice Chotard, Serge Semin, Li Yang, Manu Gautam,
	Benoît Cousson, Shawn Guo, Pavel Parkhomenko
In-Reply-To: <20201014143720.yny3jco5pkb7dr4b@mobilestation>

[-- Attachment #1: Type: text/plain, Size: 831 bytes --]

Serge Semin <Sergey.Semin@baikalelectronics.ru> writes:

> On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
>> 
>> Hi Serge,
>> 
>> Serge Semin <Sergey.Semin@baikalelectronics.ru> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

^ permalink raw reply

* Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
From: Amelie DELAUNAY @ 2020-10-15  8:05 UTC (permalink / raw)
  To: Serge Semin, Mathias Nyman, Felipe Balbi, Greg Kroah-Hartman,
	Rob Herring, Alexey Brodkin, Vineet Gupta, Hauke Mehrtens,
	Rafał Miłecki, bcm-kernel-feedback-list, Wei Xu,
	Vladimir Zapolskiy, Maxime Coquelin, Alexandre Torgue,
	Paul Cercueil, Thomas Bogendoerfer, Matthias Brugger,
	Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
  Cc: devicetree, linux-snps-arc, linux-mips, Neil Armstrong,
	Kevin Hilman, Yoshihiro Shimoda, linux-usb, linux-kernel,
	Lad Prabhakar, Serge Semin, Bjorn Andersson, Manu Gautam,
	Andy Gross, linux-mediatek, Pavel Parkhomenko, Alexey Malahov,
	linuxppc-dev, linux-stm32, linux-arm-kernel, Roger Quadros
In-Reply-To: <20201014101402.18271-19-Sergey.Semin@baikalelectronics.ru>

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<Sergey.Semin@baikalelectronics.ru>
> 
> 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 = <&rcc 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 = <&rcc USBH>;

For STM32MP151:

Acked-by: Amelie Delaunay <amelie.delaunay@st.com>

Thanks,
Amelie

^ permalink raw reply

* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Catalin Marinas @ 2020-10-15  9:05 UTC (permalink / raw)
  To: Khalid Aziz
  Cc: Jann Horn, linuxppc-dev, linux-kernel, Christoph Hellwig,
	linux-mm, Paul Mackerras, sparclinux, Anthony Yznaga,
	Andrew Morton, Will Deacon, David S. Miller, linux-arm-kernel
In-Reply-To: <e4c2c56b-3dbe-73dd-ea72-a5378de7de6a@oracle.com>

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

^ permalink raw reply

* Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
From: Krzysztof Kozlowski @ 2020-10-15  6:14 UTC (permalink / raw)
  To: Serge Semin
  Cc: Andrew Lunn, linux-usb, Neil Armstrong, Tony Lindgren,
	Bjorn Andersson, Pavel Parkhomenko,
	linux-samsung-soc@vger.kernel.org, Kevin Hilman, Gregory Clement,
	Wei Xu, Chen-Yu Tsai, Kukjin Kim, Andy Gross, linux-arm-msm,
	linux-snps-arc, Sebastian Hesselbarth, devicetree, Jason Cooper,
	Mathias Nyman, linux-kernel@vger.kernel.org, Lad Prabhakar,
	Maxime Ripard, Alexey Malahov, Rob Herring, Santosh Shilimkar,
	linux-omap, linux-arm-kernel, Roger Quadros, Felipe Balbi,
	linux-mips, Greg Kroah-Hartman, Yoshihiro Shimoda, linuxppc-dev,
	Patrice Chotard, Serge Semin, Li Yang, Manu Gautam,
	Benoît Cousson, Shawn Guo
In-Reply-To: <20201014235105.kj4rtwiidph7gyen@mobilestation>

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 correcting naming convention.
> > Not fixing errors.
> 
> From your perspective it wasn't an error, from mine and most likely Rob' it
> was.) Anyway as I said I don't care that much about preserving the subject
> wording, so what about the next one:
> <arch>: <subarch>: Harmonize DWC USB3 nodes name with DT schema
> ?

Looks good.

Best regards,
Krzysztof


^ permalink raw reply

* [PATCH 2/2] ASoC: fsl_spdif: Add support for i.MX8QM platform
From: Shengjiu Wang @ 2020-10-15  5:28 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
	alsa-devel, lgirdwood, robh+dt, devicetree
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1602739728-4433-1-git-send-email-shengjiu.wang@nxp.com>

On i.MX8QM, there are separate interrupts for TX and RX.

As the EDMA can't be configured to swing back to first FIFO
after writing the second FIFO, so we need to force the burst
size to be 2 on i.MX8QM. And EDMA don't support to shift
the data from S24_LE to S16_LE, so the supported TX format
is also different on i.MX8QM.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_spdif.c | 57 ++++++++++++++++++++++++++++++++-------
 1 file changed, 47 insertions(+), 10 deletions(-)

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index f41496cf5b63..5fa178f3f497 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -49,10 +49,18 @@ static u8 srpc_dpll_locked[] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0xa, 0xb };
  * @imx: for imx platform
  * @shared_root_clock: flag of sharing a clock source with others;
  *                     so the driver shouldn't set root clock rate
+ * @interrupts: interrupt number
+ * @tx_burst: tx maxburst size
+ * @rx_burst: rx maxburst size
+ * @tx_formats: tx supported data format
  */
 struct fsl_spdif_soc_data {
 	bool imx;
 	bool shared_root_clock;
+	u32 interrupts;
+	u32 tx_burst;
+	u32 rx_burst;
+	u64 tx_formats;
 };
 
 /*
@@ -128,16 +136,38 @@ struct fsl_spdif_priv {
 static struct fsl_spdif_soc_data fsl_spdif_vf610 = {
 	.imx = false,
 	.shared_root_clock = false,
+	.interrupts = 1,
+	.tx_burst = FSL_SPDIF_TXFIFO_WML,
+	.rx_burst = FSL_SPDIF_RXFIFO_WML,
+	.tx_formats = FSL_SPDIF_FORMATS_PLAYBACK,
 };
 
 static struct fsl_spdif_soc_data fsl_spdif_imx35 = {
 	.imx = true,
 	.shared_root_clock = false,
+	.interrupts = 1,
+	.tx_burst = FSL_SPDIF_TXFIFO_WML,
+	.rx_burst = FSL_SPDIF_RXFIFO_WML,
+	.tx_formats = FSL_SPDIF_FORMATS_PLAYBACK,
 };
 
 static struct fsl_spdif_soc_data fsl_spdif_imx6sx = {
 	.imx = true,
 	.shared_root_clock = true,
+	.interrupts = 1,
+	.tx_burst = FSL_SPDIF_TXFIFO_WML,
+	.rx_burst = FSL_SPDIF_RXFIFO_WML,
+	.tx_formats = FSL_SPDIF_FORMATS_PLAYBACK,
+
+};
+
+static struct fsl_spdif_soc_data fsl_spdif_imx8qm = {
+	.imx = true,
+	.shared_root_clock = true,
+	.interrupts = 2,
+	.tx_burst = 2,		/* Applied for EDMA */
+	.rx_burst = 2,		/* Applied for EDMA */
+	.tx_formats = SNDRV_PCM_FMTBIT_S24_LE,  /* Applied for EDMA */
 };
 
 /* Check if clk is a root clock that does not share clock source with others */
@@ -1283,6 +1313,8 @@ static int fsl_spdif_probe(struct platform_device *pdev)
 	/* Initialize this copy of the CPU DAI driver structure */
 	memcpy(&spdif_priv->cpu_dai_drv, &fsl_spdif_dai, sizeof(fsl_spdif_dai));
 	spdif_priv->cpu_dai_drv.name = dev_name(&pdev->dev);
+	spdif_priv->cpu_dai_drv.playback.formats =
+				spdif_priv->soc->tx_formats;
 
 	/* Get the addresses and IRQ */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1297,15 +1329,19 @@ static int fsl_spdif_probe(struct platform_device *pdev)
 		return PTR_ERR(spdif_priv->regmap);
 	}
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
+	for (i = 0; i < spdif_priv->soc->interrupts; i++) {
+		irq = platform_get_irq(pdev, i);
+		if (irq < 0) {
+			dev_err(&pdev->dev, "no irq for node %s\n", pdev->name);
+			return irq;
+		}
 
-	ret = devm_request_irq(&pdev->dev, irq, spdif_isr, 0,
-			       dev_name(&pdev->dev), spdif_priv);
-	if (ret) {
-		dev_err(&pdev->dev, "could not claim irq %u\n", irq);
-		return ret;
+		ret = devm_request_irq(&pdev->dev, irq, spdif_isr, 0,
+				       dev_name(&pdev->dev), spdif_priv);
+		if (ret) {
+			dev_err(&pdev->dev, "could not claim irq %u\n", irq);
+			return ret;
+		}
 	}
 
 	/* Get system clock for rx clock rate calculation */
@@ -1354,8 +1390,8 @@ static int fsl_spdif_probe(struct platform_device *pdev)
 
 	spdif_priv->dpll_locked = false;
 
-	spdif_priv->dma_params_tx.maxburst = FSL_SPDIF_TXFIFO_WML;
-	spdif_priv->dma_params_rx.maxburst = FSL_SPDIF_RXFIFO_WML;
+	spdif_priv->dma_params_tx.maxburst = spdif_priv->soc->tx_burst;
+	spdif_priv->dma_params_rx.maxburst = spdif_priv->soc->rx_burst;
 	spdif_priv->dma_params_tx.addr = res->start + REG_SPDIF_STL;
 	spdif_priv->dma_params_rx.addr = res->start + REG_SPDIF_SRL;
 
@@ -1468,6 +1504,7 @@ static const struct of_device_id fsl_spdif_dt_ids[] = {
 	{ .compatible = "fsl,imx35-spdif", .data = &fsl_spdif_imx35, },
 	{ .compatible = "fsl,vf610-spdif", .data = &fsl_spdif_vf610, },
 	{ .compatible = "fsl,imx6sx-spdif", .data = &fsl_spdif_imx6sx, },
+	{ .compatible = "fsl,imx8qm-spdif", .data = &fsl_spdif_imx8qm, },
 	{}
 };
 MODULE_DEVICE_TABLE(of, fsl_spdif_dt_ids);
-- 
2.27.0


^ permalink raw reply related

* [PATCH 1/2] ASoC: dt-bindings: fsl_spdif: Add new compatible string for i.MX8QM
From: Shengjiu Wang @ 2020-10-15  5:28 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
	alsa-devel, lgirdwood, robh+dt, devicetree
  Cc: linuxppc-dev, linux-kernel

Add new compatible string "fsl,imx8qm-spdif" for supporting spdif
module on i.MX8QM.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 Documentation/devicetree/bindings/sound/fsl,spdif.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
index 2ac671f5cb9b..50449b6d1048 100644
--- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
@@ -20,6 +20,7 @@ properties:
       - fsl,imx35-spdif
       - fsl,vf610-spdif
       - fsl,imx6sx-spdif
+      - fsl,imx8qm-spdif
 
   reg:
     maxItems: 1
-- 
2.27.0


^ permalink raw reply related

* Re: [PATCH v4 00/13] mm/debug_vm_pgtable fixes
From: Anshuman Khandual @ 2020-10-15  2:59 UTC (permalink / raw)
  To: Andrew Morton, Aneesh Kumar K.V; +Cc: linux-mm, linuxppc-dev
In-Reply-To: <20201014133607.fbf63d060e331fcd6007b624@linux-foundation.org>



On 10/15/2020 02:06 AM, Andrew Morton wrote:
> On Wed, 14 Oct 2020 08:45:16 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
> 
>>> Against mm-debug_vm_pgtable-avoid-none-pte-in-pte_clear_test.patch:
>>>
>>> https://lkml.kernel.org/r/87zh5wx51b.fsf@linux.ibm.com
>>
>>
>> yes this one we should get fixed. I was hoping someone familiar with 
>> Riscv pte updates rules would pitch in. IIUC we need to update 
>> RANDON_ORVALUE similar to how we updated it for s390 and ppc64.
>>
>>
>>   Alternatively we can do this
>>
>> modified   mm/debug_vm_pgtable.c
>> @@ -548,7 +548,7 @@ static void __init pte_clear_tests(struct mm_struct 
>> *mm, pte_t *ptep,
>>   	pte_t pte = pfn_pte(pfn, prot);
>>
>>   	pr_debug("Validating PTE clear\n");
>> -	pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
>> +//	pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
>>   	set_pte_at(mm, vaddr, ptep, pte);
>>   	barrier();
>>   	pte_clear(mm, vaddr, ptep);
>>
>> till we get that feedback from RiscV team?
> 
> Would it be better to do
> 
> #ifdef CONFIG_S390
> 	pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
> #endif

I would suggest just dropping RANDOM_ORVALUE from pte_clear_tests()
possibly with a comment saying it needs to be fixed on RISCV before
being added back later.

	pte = __pte(pte_val(pte));

OR

Disable RANDOM_ORVALUE only for RISCV.

#ifndef CONFIG_RISCV
	pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
#endif

^ permalink raw reply

* Re: [PATCH -next] Revert "powerpc/pci: unmap legacy INTx interrupts when a PHB is removed"
From: Stephen Rothwell @ 2020-10-15  0:18 UTC (permalink / raw)
  To: Oliver O'Halloran
  Cc: Alexey Kardashevskiy, Linux Kernel Mailing List,
	Linux-Next Mailing List, Qian Cai, Cédric Le Goater,
	linuxppc-dev
In-Reply-To: <CAOSf1CFT_Y67Q8caH2uFOYtwpRgFozh30ZWWZzzR-x18LBsG8g@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 524 bytes --]

Hi Oliver,

On Thu, 15 Oct 2020 10:00:49 +1100 "Oliver O'Halloran" <oohall@gmail.com> wrote:
>
> On Thu, Oct 15, 2020 at 5:28 AM Qian Cai <cai@lca.pw> wrote:
> >
> > This reverts commit 3a3181e16fbde752007759f8759d25e0ff1fc425 which
> > causes memory corruptions on POWER9 NV.  
> 
> I was going to post this along with a fix for Cedric's original bug,
> but I can do that separately so:
> 
> Acked-by: Oliver O'Halloran <oohall@gmail.com>

I added that to linux-next today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
From: Serge Semin @ 2020-10-14 23:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Andrew Lunn, linux-usb, Neil Armstrong, Tony Lindgren,
	Bjorn Andersson, Pavel Parkhomenko,
	linux-samsung-soc@vger.kernel.org, Kevin Hilman, Gregory Clement,
	Wei Xu, Chen-Yu Tsai, Kukjin Kim, Andy Gross, linux-arm-msm,
	linux-snps-arc, Sebastian Hesselbarth, devicetree, Jason Cooper,
	Mathias Nyman, linux-kernel@vger.kernel.org, Lad Prabhakar,
	Maxime Ripard, Alexey Malahov, Rob Herring, Santosh Shilimkar,
	linux-omap, linux-arm-kernel, Roger Quadros, Felipe Balbi,
	linux-mips, Greg Kroah-Hartman, Yoshihiro Shimoda, linuxppc-dev,
	Patrice Chotard, Serge Semin, Li Yang, Manu Gautam,
	Benoît Cousson, Shawn Guo
In-Reply-To: <CAJKOXPcHi_=jea=0YrPNo4dh6k03+63Tc2Uo+sd0u8+XPdQjOw@mail.gmail.com>

On Wed, Oct 14, 2020 at 10:04:32PM +0200, Krzysztof Kozlowski wrote:
> On Wed, 14 Oct 2020 at 19:16, Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote:
> > > On Wed, 14 Oct 2020 at 12:23, Serge Semin
> > > <Sergey.Semin@baikalelectronics.ru> wrote:
> > > >
> > > > 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
> > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > > >
> > > > Note we don't change the DWC USB3-compatible nodes names of
> > > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > > in-source comment says that the nodes name need to be preserved as
> > > > "^dwusb@.*" for some backward compatibility.
> > > >
> > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > >
> > > > ---
> > > >
> > > > 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.
> > >
> >
> > > 1. It is you who should compare the decompiled DTS, not us. For example:
> > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > > scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done
> > >
> > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > > fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
> > > dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
> > > dts-new/${i#dts-old/}.fdt ; done
> >
> > So basically you suggest first to compile the old and new dts files, then to
> > de-compile them, then diff old and new fdt's, and visually compare the results.
> > Personally it isn't that much better than what I did, since each old and new
> > dtbs will for sure differ due to the node names change suggested in this patch.
> > So it will lead to the visual debugging too, which isn't that effective. But
> > your approach is still more demonstrative to make sure that I didn't loose any
> > nodes redefinition, since in the occasion the old and new de-compiled nodes will
> > differ not only by the node names but with an additional old named node.
> 

> My suggestion is to compare the entire, effective DTS after all
> inclusions. Maybe you did it already, I don't know.

Only by grepping the dts'es, which include the dtsi'es modified in this patch.
So your suggestion of compiling and de-compiling has been indeed relevant.

> The point is that
> when you change node names in DTSI but you miss one in DTS, you end up
> with two nodes.

Yep, that's exactly what I meant when I said that your approach was more
demonstrative, etc.

> This is much easier to spot with dtxdiff or with
> fdtdump (which behaves better for node moves).
> 

> Indeed it is still a visual comparison - if you have any ideas how to
> automate it (e.g. ignore phandle changes), please share. It would
> solve my testings as well.

Alas I don't. That's why to save my time of coming up with an automated solution
I did a very thorough modification making sure that each affected node isn't
updated in the corresponding dts'es and asked to test the patches out.

Anyway the approach suggested by you will indeed give us a better understanding
of my patches correctness. So I'll use it before sending v3. Thanks.

> But asking others to test because you do
> not want to check it is not the best way to handle such changes.
> 
> >
> > 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.

> 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.

> 
> >
> > >
> > > 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 correcting naming convention.
> Not fixing errors.

From your perspective it wasn't an error, from mine and most likely Rob' it
was.) Anyway as I said I don't care that much about preserving the subject
wording, so what about the next one:
<arch>: <subarch>: Harmonize DWC USB3 nodes name with DT schema
?

-Sergey

> 
> >
> > Regarding automatic-pickup-stable-bot if it exists I don't think it scans all the
> > emails, but most likely the stable@vger.kernel.org list only or the emails
> > having the "Fixes:" tag. If I am wrong please give me a link to the bot sources
> > or refer to a doc where I can read about the way it works, to take it into
> > account in future commits. Just to note I submitted patches which did some fixes,
> > had the word "fix" in the subject but weren't selected to be backported to the
> > stable kernel.
> 
> You mixed up bots. The regular stable bot picks commits with cc-stable
> or with "Fixes". The auto-pickup bot picks all commits (not emails...
> why would it look at emails?) looking like a fix. Wording could be one
> of the hints used in the heuristic. Anyway, this is not a fix,
> regardless of autosel, so the wording is not correct.
> 
> Just Google for AUTOSEL. You can then ask Sasha for sources...
> 
> > Anyway I don't really care that much about the subject text using the word "fix"
> > or some else. So if you suggest some better alternative, I'd be glad to consider
> > it.
> 
> I already did. One example is: alignment with dtschema.
> 
> Best regards,
> Krzysztof

^ permalink raw reply

* Re: [PATCH -next] Revert "powerpc/pci: unmap legacy INTx interrupts when a PHB is removed"
From: Oliver O'Halloran @ 2020-10-14 23:00 UTC (permalink / raw)
  To: Qian Cai
  Cc: Stephen Rothwell, Alexey Kardashevskiy, Linux Kernel Mailing List,
	Linux-Next Mailing List, Cédric Le Goater, linuxppc-dev
In-Reply-To: <20201014182811.12027-1-cai@lca.pw>

On Thu, Oct 15, 2020 at 5:28 AM Qian Cai <cai@lca.pw> wrote:
>
> This reverts commit 3a3181e16fbde752007759f8759d25e0ff1fc425 which
> causes memory corruptions on POWER9 NV.

I was going to post this along with a fix for Cedric's original bug,
but I can do that separately so:

Acked-by: Oliver O'Halloran <oohall@gmail.com>

^ permalink raw reply

* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Jann Horn @ 2020-10-14 22:29 UTC (permalink / raw)
  To: Khalid Aziz
  Cc: Catalin Marinas, linuxppc-dev, kernel list, Christoph Hellwig,
	Linux-MM, Paul Mackerras, sparclinux, Anthony Yznaga,
	Andrew Morton, Will Deacon, David S. Miller, Linux ARM
In-Reply-To: <e4c2c56b-3dbe-73dd-ea72-a5378de7de6a@oracle.com>

On Wed, Oct 14, 2020 at 11:24 PM Khalid Aziz <khalid.aziz@oracle.com> wrote:
[...]
> current code? 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. I
> can do that unless Jann wants to rework this 2 patch series with these
> changes.

Ah, if you're willing to take care of that, that'd be nice, please do. :)

^ 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