LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Christian Borntraeger @ 2020-03-25  9:38 UTC (permalink / raw)
  To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.),
	Mike Kravetz, Dave Hansen, linux-mm, linux-kernel,
	linux-arm-kernel, linuxppc-dev, linux-riscv, linux-s390,
	sparclinux, linux-doc
  Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
	Catalin Marinas, Dave Hansen, Heiko Carstens, Ingo Molnar,
	Palmer Dabbelt, Paul Walmsley, Paul Mackerras, Thomas Gleixner,
	Will Deacon, David S.Miller
In-Reply-To: <5ea6313e-ec4f-a043-632b-ef2901ce2cc9@huawei.com>



On 25.03.20 03:58, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote:
[...]
> Hi Mike,
> 
> Inspired by Dave's opinion, it seems the x86-specific hugepages_supported should
> also need to use cpu_feature_enabled instead.
> 
> Also, I wonder if the hugepages_supported is correct ? There're two arch
> specific hugepages_supported:
> x86:
> #define hugepages_supported() boot_cpu_has(X86_FEATURE_PSE)
> and
> s390:
> #define hugepages_supported() (MACHINE_HAS_EDAT1)
> 
> Is it possible that x86 has X86_FEATURE_GBPAGES but hasn't X86_FEATURE_GBPAGES
> or s390 has MACHINE_HAS_EDAT2 but hasn't MACHINE_HAS_EDAT1 ?

The s390 architecture says that 

When EDAT-2 applies, the following function is available in the DAT process:
 - EDAT-1 applies.
[..]

So if the machine has EDAT-2 it also has EDAT-1.


^ permalink raw reply

* Re: [PATCH] powerpc/prom_init: Include the termination message in ibm, os-term RTAS call
From: Michael Ellerman @ 2020-03-25 10:06 UTC (permalink / raw)
  To: Fabiano Rosas, linuxppc-dev; +Cc: linuxram, paulus, kvm-ppc
In-Reply-To: <20200324201211.1055236-1-farosas@linux.ibm.com>

Fabiano Rosas <farosas@linux.ibm.com> writes:

> QEMU can now print the ibm,os-term message[1], so let's include it in
> the RTAS call. E.g.:
>
>   qemu-system-ppc64: OS terminated: Switch to secure mode failed.
>
> 1- https://git.qemu.org/?p=qemu.git;a=commitdiff;h=a4c3791ae0
>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  arch/powerpc/kernel/prom_init.c | 3 +++
>  1 file changed, 3 insertions(+)

I have this queued:
  https://patchwork.ozlabs.org/patch/1253390/

Which I think does the same thing?

cheers

> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 577345382b23..d543fb6d29c5 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -1773,6 +1773,9 @@ static void __init prom_rtas_os_term(char *str)
>  	if (token == 0)
>  		prom_panic("Could not get token for ibm,os-term\n");
>  	os_term_args.token = cpu_to_be32(token);
> +	os_term_args.nargs = cpu_to_be32(1);
> +	os_term_args.args[0] = cpu_to_be32(__pa(str));
> +
>  	prom_rtas_hcall((uint64_t)&os_term_args);
>  }
>  #endif /* CONFIG_PPC_SVM */
> -- 
> 2.23.0

^ permalink raw reply

* Re: [PATCH v4] powerpc: Replace setup_irq() by request_irq()
From: Michael Ellerman @ 2020-03-25 10:22 UTC (permalink / raw)
  To: afzal mohammed, Thomas Gleixner
  Cc: linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200324110637.GA5836@afzalpc>

afzal mohammed <afzal.mohd.ma@gmail.com> writes:
> Hi Michael Ellerman,
> On Thu, Mar 12, 2020 at 12:12:55PM +0530, afzal mohammed wrote:
>> request_irq() is preferred over setup_irq(). Invocations of setup_irq()
>> occur after memory allocators are ready.
>> 
>> Per tglx[1], setup_irq() existed in olden days when allocators were not
>> ready by the time early interrupts were initialized.
>> 
>> Hence replace setup_irq() by request_irq().
>> 
>> [1] https://lkml.kernel.org/r/alpine.DEB.2.20.1710191609480.1971@nanos
>> 
>> Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
>
> This patch is seen in next-test branch for last 4-5 days, i don't know
> exactly how powerpc workflow happens, so a question - this would be
> appear in linux-next soon right ? (for last 4-5 days i had been daily
> checking -next, but not appearing there).

Yeah it will appear in next "soon".

It's been stuck behind a big series that has hit some bugs during
testing, so that has delayed me pushing the whole branch.

> Sorry for the query for this trivial patch, i am asking because Thomas
> had mentioned [1] to get setup_irq() cleanup thr' respective
> maintainers (earlier it was part of tree-wide series), check -next after
> -rc6 & resubmit ignored ones to him, this patch is neither in -next,
> neither ignored, so i am at a loss what to do :(

That's OK. I will take this one, you can stop worrying about it.

It should appear in next tomorrow or Friday.

cheers

^ permalink raw reply

* [PATCH v2 00/12] powerpc/64: machine check and system reset fixes
From: Nicholas Piggin @ 2020-03-25 10:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin

There's a bunch of problems we hit bringing up fwnmi sreset and testing
with mce injection on QEMU. Mostly pseries issues.

This series of fixes applies on top of next-test, the machine
check reconcile patch won't apply cleanly to previous kernels but
it might want to be backported. We can do that after upstreaming.

This doesn't solve all known problems yet, but fwnmi machine check
and system reset injection in QEMU is significantly better. There
will be more to come but these should be ready for review now.

Thanks,
Nick

Nicholas Piggin (12):
  powerpc/64s/exceptions: Fix in_mce accounting in unrecoverable path
  powerpc/64s/exceptions: Change irq reconcile for NMIs from reusing
    _DAR to RESULT
  powerpc/64s/exceptions: machine check reconcile irq state
  powerpc/pseries/ras: avoid calling rtas_token in NMI paths
  powerpc/pseries/ras: FWNMI_VALID off by one
  powerpc/pseries/ras: fwnmi avoid modifying r3 in error case
  powerpc/pseries/ras: fwnmi sreset should not interlock
  powerpc/pseries: limit machine check stack to 4GB
  powerpc/pseries: machine check use rtas_call_unlocked with args on
    stack
  powerpc/64s: machine check interrupt update NMI accounting
  powerpc/64s: machine check do not trace real-mode handler
  powerpc/64s: system reset do not trace

 arch/powerpc/include/asm/firmware.h    |  1 +
 arch/powerpc/kernel/exceptions-64s.S   | 33 +++++++++++---
 arch/powerpc/kernel/mce.c              | 13 +++++-
 arch/powerpc/kernel/process.c          |  2 +-
 arch/powerpc/kernel/setup_64.c         | 15 ++++++-
 arch/powerpc/kernel/traps.c            | 18 +++-----
 arch/powerpc/platforms/pseries/ras.c   | 62 +++++++++++++++++++-------
 arch/powerpc/platforms/pseries/setup.c | 13 ++++--
 8 files changed, 118 insertions(+), 39 deletions(-)

-- 
2.23.0


^ permalink raw reply

* [PATCH v2 01/12] powerpc/64s/exceptions: Fix in_mce accounting in unrecoverable path
From: Nicholas Piggin @ 2020-03-25 10:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/exceptions-64s.S | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 6a936c9199d6..67cbcb2d0c7f 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1335,6 +1335,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
 	andc	r10,r10,r3
 	mtmsrd	r10
 
+	lhz	r12,PACA_IN_MCE(r13)
+	subi	r12,r12,1
+	sth	r12,PACA_IN_MCE(r13)
+
 	/* Invoke machine_check_exception to print MCE event and panic. */
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	machine_check_exception
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 02/12] powerpc/64s/exceptions: Change irq reconcile for NMIs from reusing _DAR to RESULT
From: Nicholas Piggin @ 2020-03-25 10:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

A spare interrupt stack slot is needed to save irq state when
reconciling NMIs (sreset and decrementer soft-nmi). _DAR is used
for this, but we want to reconcile machine checks as well, which
do use _DAR. Switch to using RESULT instead, as it's used by
system calls.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/exceptions-64s.S | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 67cbcb2d0c7f..257d24437130 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1011,13 +1011,13 @@ EXC_COMMON_BEGIN(system_reset_common)
 	 * the right thing. We do not want to reconcile because that goes
 	 * through irq tracing which we don't want in NMI.
 	 *
-	 * Save PACAIRQHAPPENED to _DAR (otherwise unused), and set HARD_DIS
+	 * Save PACAIRQHAPPENED to RESULT (otherwise unused), and set HARD_DIS
 	 * as we are running with MSR[EE]=0.
 	 */
 	li	r10,IRQS_ALL_DISABLED
 	stb	r10,PACAIRQSOFTMASK(r13)
 	lbz	r10,PACAIRQHAPPENED(r13)
-	std	r10,_DAR(r1)
+	std	r10,RESULT(r1)
 	ori	r10,r10,PACA_IRQ_HARD_DIS
 	stb	r10,PACAIRQHAPPENED(r13)
 
@@ -1038,7 +1038,7 @@ EXC_COMMON_BEGIN(system_reset_common)
 	/*
 	 * Restore soft mask settings.
 	 */
-	ld	r10,_DAR(r1)
+	ld	r10,RESULT(r1)
 	stb	r10,PACAIRQHAPPENED(r13)
 	ld	r10,SOFTE(r1)
 	stb	r10,PACAIRQSOFTMASK(r13)
@@ -2809,7 +2809,7 @@ EXC_COMMON_BEGIN(soft_nmi_common)
 	li	r10,IRQS_ALL_DISABLED
 	stb	r10,PACAIRQSOFTMASK(r13)
 	lbz	r10,PACAIRQHAPPENED(r13)
-	std	r10,_DAR(r1)
+	std	r10,RESULT(r1)
 	ori	r10,r10,PACA_IRQ_HARD_DIS
 	stb	r10,PACAIRQHAPPENED(r13)
 
@@ -2823,7 +2823,7 @@ EXC_COMMON_BEGIN(soft_nmi_common)
 	/*
 	 * Restore soft mask settings.
 	 */
-	ld	r10,_DAR(r1)
+	ld	r10,RESULT(r1)
 	stb	r10,PACAIRQHAPPENED(r13)
 	ld	r10,SOFTE(r1)
 	stb	r10,PACAIRQSOFTMASK(r13)
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 03/12] powerpc/64s/exceptions: machine check reconcile irq state
From: Nicholas Piggin @ 2020-03-25 10:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

pseries fwnmi machine check code pops the soft-irq checks in rtas_call
(after the previous patch to remove rtas_token from this call path).
Rather than play whack a mole with these and forever having fragile
code, it seems better to have the early machine check handler perform
the same kind of reconcile as the other NMI interrupts.

  WARNING: CPU: 0 PID: 493 at arch/powerpc/kernel/irq.c:343
  CPU: 0 PID: 493 Comm: a Tainted: G        W
  NIP:  c00000000001ed2c LR: c000000000042c40 CTR: 0000000000000000
  REGS: c0000001fffd38b0 TRAP: 0700   Tainted: G        W
  MSR:  8000000000021003 <SF,ME,RI,LE>  CR: 28000488  XER: 00000000
  CFAR: c00000000001ec90 IRQMASK: 0
  GPR00: c000000000043820 c0000001fffd3b40 c0000000012ba300 0000000000000000
  GPR04: 0000000048000488 0000000000000000 0000000000000000 00000000deadbeef
  GPR08: 0000000000000080 0000000000000000 0000000000000000 0000000000001001
  GPR12: 0000000000000000 c0000000014a0000 0000000000000000 0000000000000000
  GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  GPR24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  GPR28: 0000000000000000 0000000000000001 c000000001360810 0000000000000000
  NIP [c00000000001ed2c] arch_local_irq_restore.part.0+0xac/0x100
  LR [c000000000042c40] unlock_rtas+0x30/0x90
  Call Trace:
  [c0000001fffd3b40] [c000000001360810] 0xc000000001360810 (unreliable)
  [c0000001fffd3b60] [c000000000043820] rtas_call+0x1c0/0x280
  [c0000001fffd3bb0] [c0000000000dc328] fwnmi_release_errinfo+0x38/0x70
  [c0000001fffd3c10] [c0000000000dcd8c] pseries_machine_check_realmode+0x1dc/0x540
  [c0000001fffd3cd0] [c00000000003fe04] machine_check_early+0x54/0x70
  [c0000001fffd3d00] [c000000000008384] machine_check_early_common+0x134/0x1f0
  --- interrupt: 200 at 0x13f1307c8
      LR = 0x7fff888b8528
  Instruction dump:
  60000000 7d2000a6 71298000 41820068 39200002 7d210164 4bffff9c 60000000
  60000000 7d2000a6 71298000 4c820020 <0fe00000> 4e800020 60000000 60000000

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/exceptions-64s.S | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 257d24437130..687f1389a5e5 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1186,11 +1186,30 @@ END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
 	li	r10,MSR_RI
 	mtmsrd	r10,1
 
+	/*
+	 * Set IRQS_ALL_DISABLED and save PACAIRQHAPPENED (see
+	 * system_reset_common)
+	 */
+	li	r10,IRQS_ALL_DISABLED
+	stb	r10,PACAIRQSOFTMASK(r13)
+	lbz	r10,PACAIRQHAPPENED(r13)
+	std	r10,RESULT(r1)
+	ori	r10,r10,PACA_IRQ_HARD_DIS
+	stb	r10,PACAIRQHAPPENED(r13)
+
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	machine_check_early
 	std	r3,RESULT(r1)	/* Save result */
 	ld	r12,_MSR(r1)
 
+	/*
+	 * Restore soft mask settings.
+	 */
+	ld	r10,RESULT(r1)
+	stb	r10,PACAIRQHAPPENED(r13)
+	ld	r10,SOFTE(r1)
+	stb	r10,PACAIRQSOFTMASK(r13)
+
 #ifdef CONFIG_PPC_P7_NAP
 	/*
 	 * Check if thread was in power saving mode. We come here when any
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 04/12] powerpc/pseries/ras: avoid calling rtas_token in NMI paths
From: Nicholas Piggin @ 2020-03-25 10:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

In the interest of reducing code and possible failures in the
machine check and system reset paths, grab the "ibm,nmi-interlock"
token at init time.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/firmware.h    |  1 +
 arch/powerpc/platforms/pseries/ras.c   |  2 +-
 arch/powerpc/platforms/pseries/setup.c | 13 ++++++++++---
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index ca33f4ef6cb4..6003c2e533a0 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -128,6 +128,7 @@ extern void machine_check_fwnmi(void);
 
 /* This is true if we are using the firmware NMI handler (typically LPAR) */
 extern int fwnmi_active;
+extern int ibm_nmi_interlock_token;
 
 extern unsigned int __start___fw_ftr_fixup, __stop___fw_ftr_fixup;
 
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 1d7f973c647b..c74d5e740922 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -458,7 +458,7 @@ static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
  */
 static void fwnmi_release_errinfo(void)
 {
-	int ret = rtas_call(rtas_token("ibm,nmi-interlock"), 0, 1, NULL);
+	int ret = rtas_call(ibm_nmi_interlock_token, 0, 1, NULL);
 	if (ret != 0)
 		printk(KERN_ERR "FWNMI: nmi-interlock failed: %d\n", ret);
 }
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 17d17f064a2d..c31acd7ce0c0 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -83,6 +83,7 @@ unsigned long CMO_PageSize = (ASM_CONST(1) << IOMMU_PAGE_SHIFT_4K);
 EXPORT_SYMBOL(CMO_PageSize);
 
 int fwnmi_active;  /* TRUE if an FWNMI handler is present */
+int ibm_nmi_interlock_token;
 
 static void pSeries_show_cpuinfo(struct seq_file *m)
 {
@@ -113,9 +114,14 @@ static void __init fwnmi_init(void)
 	struct slb_entry *slb_ptr;
 	size_t size;
 #endif
+	int ibm_nmi_register_token;
 
-	int ibm_nmi_register = rtas_token("ibm,nmi-register");
-	if (ibm_nmi_register == RTAS_UNKNOWN_SERVICE)
+	ibm_nmi_register_token = rtas_token("ibm,nmi-register");
+	if (ibm_nmi_register_token == RTAS_UNKNOWN_SERVICE)
+		return;
+
+	ibm_nmi_interlock_token = rtas_token("ibm,nmi-interlock");
+	if (WARN_ON(ibm_nmi_interlock_token == RTAS_UNKNOWN_SERVICE))
 		return;
 
 	/* If the kernel's not linked at zero we point the firmware at low
@@ -123,7 +129,8 @@ static void __init fwnmi_init(void)
 	system_reset_addr  = __pa(system_reset_fwnmi) - PHYSICAL_START;
 	machine_check_addr = __pa(machine_check_fwnmi) - PHYSICAL_START;
 
-	if (0 == rtas_call(ibm_nmi_register, 2, 1, NULL, system_reset_addr,
+	if (0 == rtas_call(ibm_nmi_register_token, 2, 1, NULL,
+				system_reset_addr,
 				machine_check_addr))
 		fwnmi_active = 1;
 
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 05/12] powerpc/pseries/ras: FWNMI_VALID off by one
From: Nicholas Piggin @ 2020-03-25 10:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

This was discovered developing qemu fwnmi sreset support. This
off-by-one bug means the last 16 bytes of the rtas area can not
be used for a 16 byte save area.

It's not a serious bug, and QEMU implementation has to retain a
workaround for old kernels, but it's good to tighten it.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/pseries/ras.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index c74d5e740922..9a37bda47468 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -395,10 +395,11 @@ static irqreturn_t ras_error_interrupt(int irq, void *dev_id)
 /*
  * Some versions of FWNMI place the buffer inside the 4kB page starting at
  * 0x7000. Other versions place it inside the rtas buffer. We check both.
+ * Minimum size of the buffer is 16 bytes.
  */
 #define VALID_FWNMI_BUFFER(A) \
-	((((A) >= 0x7000) && ((A) < 0x7ff0)) || \
-	(((A) >= rtas.base) && ((A) < (rtas.base + rtas.size - 16))))
+	((((A) >= 0x7000) && ((A) <= 0x8000 - 16)) || \
+	(((A) >= rtas.base) && ((A) <= (rtas.base + rtas.size - 16))))
 
 static inline struct rtas_error_log *fwnmi_get_errlog(void)
 {
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 06/12] powerpc/pseries/ras: fwnmi avoid modifying r3 in error case
From: Nicholas Piggin @ 2020-03-25 10:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

If there is some error with the fwnmi save area, r3 has already been
modified which doesn't help with debugging.

Only update r3 when to restore the saved value.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/pseries/ras.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 9a37bda47468..a40598e6e525 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -423,18 +423,19 @@ static inline struct rtas_error_log *fwnmi_get_errlog(void)
  */
 static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
 {
+	unsigned long savep_ra;
 	unsigned long *savep;
 	struct rtas_error_log *h;
 
 	/* Mask top two bits */
-	regs->gpr[3] &= ~(0x3UL << 62);
+	savep_ra = regs->gpr[3] & ~(0x3UL << 62);
 
-	if (!VALID_FWNMI_BUFFER(regs->gpr[3])) {
+	if (!VALID_FWNMI_BUFFER(savep_ra)) {
 		printk(KERN_ERR "FWNMI: corrupt r3 0x%016lx\n", regs->gpr[3]);
 		return NULL;
 	}
 
-	savep = __va(regs->gpr[3]);
+	savep = __va(savep_ra);
 	regs->gpr[3] = be64_to_cpu(savep[0]);	/* restore original r3 */
 
 	h = (struct rtas_error_log *)&savep[1];
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 07/12] powerpc/pseries/ras: fwnmi sreset should not interlock
From: Nicholas Piggin @ 2020-03-25 10:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

PAPR does not specify that fwnmi sreset should be interlocked, and
PowerVM (and therefore now QEMU) do not require it.

These "ibm,nmi-interlock" calls are ignored by firmware, but there
is a possibility that the sreset could have interrupted a machine
check and release the machine check's interlock too early, corrupting
it if another machine check came in.

This is an extremely rare case, but it should be fixed for clarity
and reducing the code executed in the sreset path. Firmware also
does not provide error information for the sreset case to look at, so
remove that comment.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/pseries/ras.c | 48 ++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index a40598e6e525..833ae34b7fec 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -406,6 +406,20 @@ static inline struct rtas_error_log *fwnmi_get_errlog(void)
 	return (struct rtas_error_log *)local_paca->mce_data_buf;
 }
 
+static unsigned long *fwnmi_get_savep(struct pt_regs *regs)
+{
+	unsigned long savep_ra;
+
+	/* Mask top two bits */
+	savep_ra = regs->gpr[3] & ~(0x3UL << 62);
+	if (!VALID_FWNMI_BUFFER(savep_ra)) {
+		printk(KERN_ERR "FWNMI: corrupt r3 0x%016lx\n", regs->gpr[3]);
+		return NULL;
+	}
+
+	return __va(savep_ra);
+}
+
 /*
  * Get the error information for errors coming through the
  * FWNMI vectors.  The pt_regs' r3 will be updated to reflect
@@ -423,20 +437,15 @@ static inline struct rtas_error_log *fwnmi_get_errlog(void)
  */
 static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
 {
-	unsigned long savep_ra;
 	unsigned long *savep;
 	struct rtas_error_log *h;
 
-	/* Mask top two bits */
-	savep_ra = regs->gpr[3] & ~(0x3UL << 62);
-
-	if (!VALID_FWNMI_BUFFER(savep_ra)) {
-		printk(KERN_ERR "FWNMI: corrupt r3 0x%016lx\n", regs->gpr[3]);
+	savep = fwnmi_get_savep(regs);
+	if (!savep)
 		return NULL;
-	}
 
-	savep = __va(savep_ra);
-	regs->gpr[3] = be64_to_cpu(savep[0]);	/* restore original r3 */
+	/* restore original r3 */
+	regs->gpr[3] = be64_to_cpu(savep[0]);
 
 	h = (struct rtas_error_log *)&savep[1];
 	/* Use the per cpu buffer from paca to store rtas error log */
@@ -483,11 +492,22 @@ int pSeries_system_reset_exception(struct pt_regs *regs)
 #endif
 
 	if (fwnmi_active) {
-		struct rtas_error_log *errhdr = fwnmi_get_errinfo(regs);
-		if (errhdr) {
-			/* XXX Should look at FWNMI information */
-		}
-		fwnmi_release_errinfo();
+		unsigned long *savep;
+
+		/*
+		 * Firmware (PowerVM and KVM) saves r3 to a save area like
+		 * machine check, which is not exactly what PAPR (2.9)
+		 * suggests but there is no way to detect otherwise, so this
+		 * is the interface now.
+		 *
+		 * System resets do not save any error log or require an
+		 * "ibm,nmi-interlock" rtas call to release.
+		 */
+
+		savep = fwnmi_get_savep(regs);
+		/* restore original r3 */
+		if (savep)
+			regs->gpr[3] = be64_to_cpu(savep[0]);
 	}
 
 	if (smp_handle_nmi_ipi(regs))
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 08/12] powerpc/pseries: limit machine check stack to 4GB
From: Nicholas Piggin @ 2020-03-25 10:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

This allows rtas_args to be put on the machine check stack, which
avoids a lot of complications with re-entrancy deadlocks.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/setup_64.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 3bf03666ee09..ca1041f8a578 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -695,6 +695,9 @@ void __init exc_lvl_early_init(void)
 void __init emergency_stack_init(void)
 {
 	u64 limit;
+#ifdef CONFIG_PPC_BOOK3S_64
+	u64 mce_limit;
+#endif
 	unsigned int i;
 
 	/*
@@ -713,6 +716,16 @@ void __init emergency_stack_init(void)
 	 */
 	limit = min(ppc64_bolted_size(), ppc64_rma_size);
 
+	/*
+	 * Machine check on pseries calls rtas, but can't use the static
+	 * rtas_args due to a machine check hitting while the lock is held.
+	 * rtas args have to be under 4GB, so the machine check stack is
+	 * limited to 4GB so args can be put on stack.
+	 */
+	mce_limit = limit;
+	if (firmware_has_feature(FW_FEATURE_LPAR) && mce_limit > 4UL*1024*1024*1024)
+		mce_limit = 4UL*1024*1024*1024;
+
 	for_each_possible_cpu(i) {
 		paca_ptrs[i]->emergency_sp = alloc_stack(limit, i) + THREAD_SIZE;
 
@@ -721,7 +734,7 @@ void __init emergency_stack_init(void)
 		paca_ptrs[i]->nmi_emergency_sp = alloc_stack(limit, i) + THREAD_SIZE;
 
 		/* emergency stack for machine check exception handling. */
-		paca_ptrs[i]->mc_emergency_sp = alloc_stack(limit, i) + THREAD_SIZE;
+		paca_ptrs[i]->mc_emergency_sp = alloc_stack(mce_limit, i) + THREAD_SIZE;
 #endif
 	}
 }
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 09/12] powerpc/pseries: machine check use rtas_call_unlocked with args on stack
From: Nicholas Piggin @ 2020-03-25 10:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

With the previous patch, machine checks can use rtas_call_unlocked
which avoids the rtas spinlock which would deadlock if a machine
check hits while making an rtas call.

This also avoids the complex rtas error logging which has more rtas calls
and includes kmalloc (which can return memory beyond RMA, which would
also crash).

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/pseries/ras.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 833ae34b7fec..6938261e4569 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -469,7 +469,15 @@ static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
  */
 static void fwnmi_release_errinfo(void)
 {
-	int ret = rtas_call(ibm_nmi_interlock_token, 0, 1, NULL);
+	struct rtas_args rtas_args;
+	int ret;
+
+	/*
+	 * On pseries, the machine check stack is limited to under 4GB, so
+	 * args can be on-stack.
+	 */
+	rtas_call_unlocked(&rtas_args, ibm_nmi_interlock_token, 0, 1, NULL);
+	ret = be32_to_cpu(rtas_args.rets[0]);
 	if (ret != 0)
 		printk(KERN_ERR "FWNMI: nmi-interlock failed: %d\n", ret);
 }
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 10/12] powerpc/64s: machine check interrupt update NMI accounting
From: Nicholas Piggin @ 2020-03-25 10:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

machine_check_early is taken as an NMI, so nmi_enter is used there.
machine_check_exception is no longer taken as an NMI (it's invoked
via irq_work in the case a machine check hits in kernel mode), so
remove the nmi_enter from that case.

In NMI context, hash faults don't try to refill the hash table, which
can lead to crashes accessing non-pinned kernel pages. System reset
still has this potential problem.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/mce.c     |  7 +++++++
 arch/powerpc/kernel/process.c |  2 +-
 arch/powerpc/kernel/traps.c   | 13 +------------
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index 34c1001e9e8b..c1684be0d8b7 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -560,6 +560,9 @@ EXPORT_SYMBOL_GPL(machine_check_print_event_info);
 long machine_check_early(struct pt_regs *regs)
 {
 	long handled = 0;
+	bool nested = in_nmi();
+	if (!nested)
+		nmi_enter();
 
 	hv_nmi_check_nonrecoverable(regs);
 
@@ -568,6 +571,10 @@ long machine_check_early(struct pt_regs *regs)
 	 */
 	if (ppc_md.machine_check_early)
 		handled = ppc_md.machine_check_early(regs);
+
+	if (!nested)
+		nmi_exit();
+
 	return handled;
 }
 
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index d27bf367e929..c21344c1a151 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1421,7 +1421,7 @@ void show_regs(struct pt_regs * regs)
 		pr_cont("DAR: "REG" DSISR: %08lx ", regs->dar, regs->dsisr);
 #endif
 #ifdef CONFIG_PPC64
-	pr_cont("IRQMASK: %lx ", regs->softe);
+	pr_cont("IRQMASK: %lx IN_NMI:%d IN_MCE:%d", regs->softe, (int)get_paca()->in_nmi, (int)get_paca()->in_mce);
 #endif
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	if (MSR_TM_ACTIVE(regs->msr))
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 82a3438300fd..1845fd7e161a 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -823,9 +823,6 @@ int machine_check_generic(struct pt_regs *regs)
 void machine_check_exception(struct pt_regs *regs)
 {
 	int recover = 0;
-	bool nested = in_nmi();
-	if (!nested)
-		nmi_enter();
 
 	__this_cpu_inc(irq_stat.mce_exceptions);
 
@@ -851,20 +848,12 @@ void machine_check_exception(struct pt_regs *regs)
 	if (check_io_access(regs))
 		goto bail;
 
-	if (!nested)
-		nmi_exit();
-
 	die("Machine check", regs, SIGBUS);
 
+bail:
 	/* Must die if the interrupt is not recoverable */
 	if (!(regs->msr & MSR_RI))
 		nmi_panic(regs, "Unrecoverable Machine check");
-
-	return;
-
-bail:
-	if (!nested)
-		nmi_exit();
 }
 
 void SMIException(struct pt_regs *regs)
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 11/12] powerpc/64s: machine check do not trace real-mode handler
From: Nicholas Piggin @ 2020-03-25 10:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

Rather than notrace annotations throughout a significant part of the
machine check code across kernel/ pseries/ and powernv/ which can
easily be broken and is infrequently tested, use paca->ftrace_enabled
to blanket-disable tracing of the real-mode non-maskable handler.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/mce.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index c1684be0d8b7..36e99adb8710 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -557,10 +557,13 @@ EXPORT_SYMBOL_GPL(machine_check_print_event_info);
  *
  * regs->nip and regs->msr contains srr0 and ssr1.
  */
-long machine_check_early(struct pt_regs *regs)
+long notrace machine_check_early(struct pt_regs *regs)
 {
 	long handled = 0;
 	bool nested = in_nmi();
+	u8 ftrace_enabled = local_paca->ftrace_enabled;
+
+	local_paca->ftrace_enabled = 0;
 	if (!nested)
 		nmi_enter();
 
@@ -574,6 +577,7 @@ long machine_check_early(struct pt_regs *regs)
 
 	if (!nested)
 		nmi_exit();
+	local_paca->ftrace_enabled = ftrace_enabled;
 
 	return handled;
 }
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 12/12] powerpc/64s: system reset do not trace
From: Nicholas Piggin @ 2020-03-25 10:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200325103410.157573-1-npiggin@gmail.com>

Similarly to the previous patch, do not trace system reset. This code
is used when there is a crash or hang, and tracing disturbs the system
more and has been known to crash in the crash handling path.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/traps.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 1845fd7e161a..ed7b7a6e2dc0 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -443,6 +443,9 @@ void system_reset_exception(struct pt_regs *regs)
 	unsigned long hsrr0, hsrr1;
 	bool nested = in_nmi();
 	bool saved_hsrrs = false;
+	u8 ftrace_enabled = local_paca->ftrace_enabled;
+
+	local_paca->ftrace_enabled = 0;
 
 	/*
 	 * Avoid crashes in case of nested NMI exceptions. Recoverability
@@ -524,6 +527,8 @@ void system_reset_exception(struct pt_regs *regs)
 	if (!nested)
 		nmi_exit();
 
+	local_paca->ftrace_enabled = ftrace_enabled;
+
 	/* What should we do here? We could issue a shutdown or hard reset. */
 }
 
-- 
2.23.0


^ permalink raw reply related

* Re: [PATCH] cpufreq: powernv: Fix frame-size-overflow in powernv_cpufreq_work_fn
From: Rafael J. Wysocki @ 2020-03-25 10:36 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: ego, pratik.r.sampat, linux-pm, linux-kernel,
	Pratik Rajesh Sampat, linuxppc-dev, dja
In-Reply-To: <87h7yexnu7.fsf@mpe.ellerman.id.au>

On Tuesday, March 24, 2020 7:34:56 AM CET Michael Ellerman wrote:
> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
> > On Monday, March 16, 2020 2:57:43 PM CET Pratik Rajesh Sampat wrote:
> >> The patch avoids allocating cpufreq_policy on stack hence fixing frame
> >> size overflow in 'powernv_cpufreq_work_fn'
> >> 
> >> Fixes: 227942809b52 ("cpufreq: powernv: Restore cpu frequency to policy->cur on unthrottling")
> >> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> >
> > Any objections or concerns here?
> >
> > If not, I'll queue it up.
> 
> I have it in my testing branch,

Great!

> but if you pick it up I can drop it.

Let it go in through your tree.

Cheers!




^ permalink raw reply

* [PATCH v2] powerpc/64: mark emergency stacks valid to unwind
From: Nicholas Piggin @ 2020-03-25 10:41 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

Before:

  WARNING: CPU: 0 PID: 494 at arch/powerpc/kernel/irq.c:343
  CPU: 0 PID: 494 Comm: a Tainted: G        W
  NIP:  c00000000001ed2c LR: c000000000d13190 CTR: c00000000003f910
  REGS: c0000001fffd3870 TRAP: 0700   Tainted: G        W
  MSR:  8000000000021003 <SF,ME,RI,LE>  CR: 28000488  XER: 00000000
  CFAR: c00000000001ec90 IRQMASK: 0
  GPR00: c000000000aeb12c c0000001fffd3b00 c0000000012ba300 0000000000000000
  GPR04: 0000000000000000 0000000000000000 000000010bd207c8 6b00696e74657272
  GPR08: 0000000000000000 0000000000000000 0000000000000000 efbeadde00000000
  GPR12: 0000000000000000 c0000000014a0000 0000000000000000 0000000000000000
  GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  GPR24: 0000000000000000 0000000000000000 0000000000000000 000000010bd207bc
  GPR28: 0000000000000000 c00000000148a898 0000000000000000 c0000001ffff3f50
  NIP [c00000000001ed2c] arch_local_irq_restore.part.0+0xac/0x100
  LR [c000000000d13190] _raw_spin_unlock_irqrestore+0x50/0xc0
  Call Trace:
  Instruction dump:
  60000000 7d2000a6 71298000 41820068 39200002 7d210164 4bffff9c 60000000
  60000000 7d2000a6 71298000 4c820020 <0fe00000> 4e800020 60000000 60000000

After:

  WARNING: CPU: 0 PID: 499 at arch/powerpc/kernel/irq.c:343
  CPU: 0 PID: 499 Comm: a Not tainted
  NIP:  c00000000001ed2c LR: c000000000d13210 CTR: c00000000003f980
  REGS: c0000001fffd3870 TRAP: 0700   Not tainted
  MSR:  8000000000021003 <SF,ME,RI,LE>  CR: 28000488  XER: 00000000
  CFAR: c00000000001ec90 IRQMASK: 0
  GPR00: c000000000aeb1ac c0000001fffd3b00 c0000000012ba300 0000000000000000
  GPR04: 0000000000000000 0000000000000000 00000001347607c8 6b00696e74657272
  GPR08: 0000000000000000 0000000000000000 0000000000000000 efbeadde00000000
  GPR12: 0000000000000000 c0000000014a0000 0000000000000000 0000000000000000
  GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  GPR24: 0000000000000000 0000000000000000 0000000000000000 00000001347607bc
  GPR28: 0000000000000000 c00000000148a898 0000000000000000 c0000001ffff3f50
  NIP [c00000000001ed2c] arch_local_irq_restore.part.0+0xac/0x100
  LR [c000000000d13210] _raw_spin_unlock_irqrestore+0x50/0xc0
  Call Trace:
  [c0000001fffd3b20] [c000000000aeb1ac] of_find_property+0x6c/0x90
  [c0000001fffd3b70] [c000000000aeb1f0] of_get_property+0x20/0x40
  [c0000001fffd3b90] [c000000000042cdc] rtas_token+0x3c/0x70
  [c0000001fffd3bb0] [c0000000000dc318] fwnmi_release_errinfo+0x28/0x70
  [c0000001fffd3c10] [c0000000000dcd8c] pseries_machine_check_realmode+0x1dc/0x540
  [c0000001fffd3cd0] [c00000000003fe04] machine_check_early+0x54/0x70
  [c0000001fffd3d00] [c000000000008384] machine_check_early_common+0x134/0x1f0
  --- interrupt: 200 at 0x1347607c8
      LR = 0x7fffafbd8328
  Instruction dump:
  60000000 7d2000a6 71298000 41820068 39200002 7d210164 4bffff9c 60000000
  60000000 7d2000a6 71298000 4c820020 <0fe00000> 4e800020 60000000 60000000

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
No change but I removed this from the RAS series because it's
orthogonal and hopefully a good simple change on its own.

 arch/powerpc/kernel/process.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 1dea4d280f6f..d27bf367e929 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1983,6 +1983,32 @@ static inline int valid_irq_stack(unsigned long sp, struct task_struct *p,
 	return 0;
 }
 
+static inline int valid_emergency_stack(unsigned long sp, struct task_struct *p,
+					unsigned long nbytes)
+{
+#ifdef CONFIG_PPC64
+	unsigned long stack_page;
+	unsigned long cpu = task_cpu(p);
+
+	stack_page = (unsigned long)paca_ptrs[cpu]->emergency_sp - THREAD_SIZE;
+	if (sp >= stack_page && sp <= stack_page + THREAD_SIZE - nbytes)
+		return 1;
+
+# ifdef CONFIG_PPC_BOOK3S_64
+	stack_page = (unsigned long)paca_ptrs[cpu]->nmi_emergency_sp - THREAD_SIZE;
+	if (sp >= stack_page && sp <= stack_page + THREAD_SIZE - nbytes)
+		return 1;
+
+	stack_page = (unsigned long)paca_ptrs[cpu]->mc_emergency_sp - THREAD_SIZE;
+	if (sp >= stack_page && sp <= stack_page + THREAD_SIZE - nbytes)
+		return 1;
+# endif
+#endif
+
+	return 0;
+}
+
+
 int validate_sp(unsigned long sp, struct task_struct *p,
 		       unsigned long nbytes)
 {
@@ -1994,7 +2020,10 @@ int validate_sp(unsigned long sp, struct task_struct *p,
 	if (sp >= stack_page && sp <= stack_page + THREAD_SIZE - nbytes)
 		return 1;
 
-	return valid_irq_stack(sp, p, nbytes);
+	if (valid_irq_stack(sp, p, nbytes))
+		return 1;
+
+	return valid_emergency_stack(sp, p, nbytes);
 }
 
 EXPORT_SYMBOL(validate_sp);
-- 
2.23.0


^ permalink raw reply related

* Re: [PATCH] powerpc/boot: Delete unneeded .globl _zimage_start
From: Michael Ellerman @ 2020-03-25 10:42 UTC (permalink / raw)
  To: Fangrui Song, linuxppc-dev
  Cc: clang-built-linux, Nick Desaulniers, Joel Stanley, Fangrui Song
In-Reply-To: <20200325051820.163253-1-maskray@google.com>

Fangrui Song <maskray@google.com> writes:
> .globl sets the symbol binding to STB_GLOBAL while .weak sets the
> binding to STB_WEAK. They should not be used together. It is accidetal
> rather then intentional that GNU as let .weak override .globl while
> clang integrated assembler let the last win.

> Fixes: cd197ffcf10b "[POWERPC] zImage: Cleanup and improve zImage entry point"
> Fixes: ee9d21b3b358 "powerpc/boot: Ensure _zimage_start is a weak symbol"
> Link: https://github.com/ClangBuiltLinux/linux/issues/937
> Signed-off-by: Fangrui Song <maskray@google.com>

So it seems the patch is OK but people don't agree with the explanation
in the changelog.

Please send a v2 that captures the responses from Segher and Alan.

cheers

> diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
> index 92608f34d312..1d83966f5ef6 100644
> --- a/arch/powerpc/boot/crt0.S
> +++ b/arch/powerpc/boot/crt0.S
> @@ -44,9 +44,6 @@ p_end:		.long	_end
>  p_pstack:	.long	_platform_stack_top
>  #endif
>  
> -	.globl	_zimage_start
> -	/* Clang appears to require the .weak directive to be after the symbol
> -	 * is defined. See https://bugs.llvm.org/show_bug.cgi?id=38921  */
>  	.weak	_zimage_start
>  _zimage_start:
>  	.globl	_zimage_start_lib
> -- 
> 2.25.1.696.g5e7596f4ac-goog

^ permalink raw reply

* Re: [PATCH v8 03/14] powerpc/vas: Define nx_fault_stamp in coprocessor_request_block
From: Michael Ellerman @ 2020-03-25 10:44 UTC (permalink / raw)
  To: Haren Myneni
  Cc: mikey, ajd, Nicholas Piggin, hch, oohall, sukadev, linuxppc-dev,
	herbert
In-Reply-To: <1584987237.9256.15333.camel@hbabu-laptop>

Haren Myneni <haren@linux.ibm.com> writes:
> On Mon, 2020-03-23 at 22:32 +1100, Michael Ellerman wrote:
>> Nicholas Piggin <npiggin@gmail.com> writes:
>> > Haren Myneni's on March 19, 2020 4:13 pm:
>> >> 
>> >> Kernel sets fault address and status in CRB for NX page fault on user
>> >> space address after processing page fault. User space gets the signal
>> >> and handles the fault mentioned in CRB by bringing the page in to
>> >> memory and send NX request again.
>> >> 
>> >> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
>> >> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
>> >> ---
>> >>  arch/powerpc/include/asm/icswx.h | 18 +++++++++++++++++-
>> >>  1 file changed, 17 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/include/asm/icswx.h
>> >> index 9872f85..b233d1e 100644
>> >> --- a/arch/powerpc/include/asm/icswx.h
>> >> +++ b/arch/powerpc/include/asm/icswx.h
>> >
>> > "icswx" is not a thing anymore, after 6ff4d3e96652 ("powerpc: Remove old 
>> > unused icswx based coprocessor support").
>> 
>> Yeah that commit ripped out some parts of the previous attempt at a user
>> visible API for this sort of "coprocessor" stuff. VAS is yet another
>> attempt to do something useful with most of the same pieces but some
>> slightly different details.
>> 
>> > I guess NX is reusing some 
>> > things from it, but it would be good to get rid of the cruft and re-name
>> > this file and and relevant names.
>> 
>> > NX already uses this file, so I guesss that can happen after this series.
>> 
>> A lot of the CRB/CSB stuff is still the same, and P8 still uses icswx.
>> But I'd be happy if the header was renamed eventually, as icswx is now a
>> legacy name.
>
> We can move all macros and struct definitions to vas.h and remove
> icswx.h. Can I do this after this series? 

Well they're still needed by the non-vas Power8 code, so that wouldn't
be quite right either :)

But yeah we can do whatever movement later as a cleanup.

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/prom_init: Include the termination message in ibm,os-term RTAS call
From: Greg Kurz @ 2020-03-25 10:34 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxram, paulus, linuxppc-dev, kvm-ppc, Fabiano Rosas
In-Reply-To: <87zhc4wxy9.fsf@mpe.ellerman.id.au>

On Wed, 25 Mar 2020 21:06:22 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Fabiano Rosas <farosas@linux.ibm.com> writes:
> 
> > QEMU can now print the ibm,os-term message[1], so let's include it in
> > the RTAS call. E.g.:
> >
> >   qemu-system-ppc64: OS terminated: Switch to secure mode failed.
> >
> > 1- https://git.qemu.org/?p=qemu.git;a=commitdiff;h=a4c3791ae0
> >
> > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> > ---
> >  arch/powerpc/kernel/prom_init.c | 3 +++
> >  1 file changed, 3 insertions(+)
> 
> I have this queued:
>   https://patchwork.ozlabs.org/patch/1253390/
> 
> Which I think does the same thing?
> 

Alexey's patch also sets os_term_args.nret as indicated in PAPR.
Even if QEMU's handler for "ibm,os-term" doesn't seem to have
a use for nret, I think it's better to stick to the spec.

Cheers,

--
Greg

> cheers
> 
> > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> > index 577345382b23..d543fb6d29c5 100644
> > --- a/arch/powerpc/kernel/prom_init.c
> > +++ b/arch/powerpc/kernel/prom_init.c
> > @@ -1773,6 +1773,9 @@ static void __init prom_rtas_os_term(char *str)
> >  	if (token == 0)
> >  		prom_panic("Could not get token for ibm,os-term\n");
> >  	os_term_args.token = cpu_to_be32(token);
> > +	os_term_args.nargs = cpu_to_be32(1);
> > +	os_term_args.args[0] = cpu_to_be32(__pa(str));
> > +
> >  	prom_rtas_hcall((uint64_t)&os_term_args);
> >  }
> >  #endif /* CONFIG_PPC_SVM */
> > -- 
> > 2.23.0


^ permalink raw reply

* Documentation/locking/locktypes: Further clarifications and wordsmithing
From: Thomas Gleixner @ 2020-03-25 12:27 UTC (permalink / raw)
  To: paulmck
  Cc: linux-usb, linux-ia64, Peter Zijlstra, linux-pci,
	Sebastian Siewior, Oleg Nesterov, Guo Ren, Joel Fernandes,
	Vincent Chen, Ingo Molnar, Davidlohr Bueso, linux-acpi,
	Brian Cain, Jonathan Corbet, linux-hexagon, Rafael J. Wysocki,
	linux-csky, Linus Torvalds, Darren Hart, Zhang Rui, Len Brown,
	Fenghua Yu, Arnd Bergmann, linux-pm, linuxppc-dev, Greentime Hu,
	Bjorn Helgaas, Kurt Schwemmer, platform-driver-x86, Kalle Valo,
	kbuild test robot, Felipe Balbi, Michal Simek, Tony Luck, Nick Hu,
	Geoff Levand, netdev, Randy Dunlap, linux-wireless, LKML,
	Davidlohr Bueso, Greg Kroah-Hartman, Logan Gunthorpe,
	David S. Miller, Andy Shevchenko
In-Reply-To: <20200325002811.GO19865@paulmck-ThinkPad-P72>

The documentation of rw_semaphores is wrong as it claims that the non-owner
reader release is not supported by RT. That's just history biased memory
distortion.

Split the 'Owner semantics' section up and add separate sections for
semaphore and rw_semaphore to reflect reality.

Aside of that the following updates are done:

 - Add pseudo code to document the spinlock state preserving mechanism on
   PREEMPT_RT

 - Wordsmith the bitspinlock and lock nesting sections

Co-developed-by: Paul McKenney <paulmck@kernel.org>
Signed-off-by: Paul McKenney <paulmck@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 Documentation/locking/locktypes.rst |  150 +++++++++++++++++++++++-------------
 1 file changed, 99 insertions(+), 51 deletions(-)

--- a/Documentation/locking/locktypes.rst
+++ b/Documentation/locking/locktypes.rst
@@ -67,6 +67,17 @@ Spinning locks implicitly disable preemp
  _irqsave/restore()   Save and disable / restore interrupt disabled state
  ===================  ====================================================
 
+Owner semantics
+===============
+
+The aforementioned lock types except semaphores have strict owner
+semantics:
+
+  The context (task) that acquired the lock must release it.
+
+rw_semaphores have a special interface which allows non-owner release for
+readers.
+
 
 rtmutex
 =======
@@ -83,6 +94,51 @@ interrupt handlers and soft interrupts.
 and rwlock_t to be implemented via RT-mutexes.
 
 
+semaphore
+=========
+
+semaphore is a counting semaphore implementation.
+
+Semaphores are often used for both serialization and waiting, but new use
+cases should instead use separate serialization and wait mechanisms, such
+as mutexes and completions.
+
+semaphores and PREEMPT_RT
+----------------------------
+
+PREEMPT_RT does not change the semaphore implementation because counting
+semaphores have no concept of owners, thus preventing PREEMPT_RT from
+providing priority inheritance for semaphores.  After all, an unknown
+owner cannot be boosted. As a consequence, blocking on semaphores can
+result in priority inversion.
+
+
+rw_semaphore
+============
+
+rw_semaphore is a multiple readers and single writer lock mechanism.
+
+On non-PREEMPT_RT kernels the implementation is fair, thus preventing
+writer starvation.
+
+rw_semaphore complies by default with the strict owner semantics, but there
+exist special-purpose interfaces that allow non-owner release for readers.
+These work independent of the kernel configuration.
+
+rw_semaphore and PREEMPT_RT
+---------------------------
+
+PREEMPT_RT kernels map rw_semaphore to a separate rt_mutex-based
+implementation, thus changing the fairness:
+
+ Because an rw_semaphore writer cannot grant its priority to multiple
+ readers, a preempted low-priority reader will continue holding its lock,
+ thus starving even high-priority writers.  In contrast, because readers
+ can grant their priority to a writer, a preempted low-priority writer will
+ have its priority boosted until it releases the lock, thus preventing that
+ writer from starving readers.
+
+
 raw_spinlock_t and spinlock_t
 =============================
 
@@ -102,7 +158,7 @@ critical section is tiny, thus avoiding
 spinlock_t
 ----------
 
-The semantics of spinlock_t change with the state of CONFIG_PREEMPT_RT.
+The semantics of spinlock_t change with the state of PREEMPT_RT.
 
 On a non PREEMPT_RT enabled kernel spinlock_t is mapped to raw_spinlock_t
 and has exactly the same semantics.
@@ -140,7 +196,16 @@ On a PREEMPT_RT enabled kernel spinlock_
    kernels leave task state untouched.  However, PREEMPT_RT must change
    task state if the task blocks during acquisition.  Therefore, it saves
    the current task state before blocking and the corresponding lock wakeup
-   restores it.
+   restores it, as shown below::
+
+    task->state = TASK_INTERRUPTIBLE
+     lock()
+       block()
+         task->saved_state = task->state
+	 task->state = TASK_UNINTERRUPTIBLE
+	 schedule()
+					lock wakeup
+					  task->state = task->saved_state
 
    Other types of wakeups would normally unconditionally set the task state
    to RUNNING, but that does not work here because the task must remain
@@ -148,7 +213,22 @@ On a PREEMPT_RT enabled kernel spinlock_
    wakeup attempts to awaken a task blocked waiting for a spinlock, it
    instead sets the saved state to RUNNING.  Then, when the lock
    acquisition completes, the lock wakeup sets the task state to the saved
-   state, in this case setting it to RUNNING.
+   state, in this case setting it to RUNNING::
+
+    task->state = TASK_INTERRUPTIBLE
+     lock()
+       block()
+         task->saved_state = task->state
+	 task->state = TASK_UNINTERRUPTIBLE
+	 schedule()
+					non lock wakeup
+					  task->saved_state = TASK_RUNNING
+
+					lock wakeup
+					  task->state = task->saved_state
+
+   This ensures that the real wakeup cannot be lost.
+
 
 rwlock_t
 ========
@@ -228,17 +308,16 @@ while holding normal non-raw spinlocks b
 bit spinlocks
 -------------
 
-Bit spinlocks are problematic for PREEMPT_RT as they cannot be easily
-substituted by an RT-mutex based implementation for obvious reasons.
-
-The semantics of bit spinlocks are preserved on PREEMPT_RT kernels and the
-caveats vs. raw_spinlock_t apply.
-
-Some bit spinlocks are substituted by regular spinlock_t for PREEMPT_RT but
-this requires conditional (#ifdef'ed) code changes at the usage site while
-the spinlock_t substitution is simply done by the compiler and the
-conditionals are restricted to header files and core implementation of the
-locking primitives and the usage sites do not require any changes.
+PREEMPT_RT cannot substitute bit spinlocks because a single bit is too
+small to accommodate an RT-mutex.  Therefore, the semantics of bit
+spinlocks are preserved on PREEMPT_RT kernels, so that the raw_spinlock_t
+caveats also apply to bit spinlocks.
+
+Some bit spinlocks are replaced with regular spinlock_t for PREEMPT_RT
+using conditional (#ifdef'ed) code changes at the usage site.  In contrast,
+usage-site changes are not needed for the spinlock_t substitution.
+Instead, conditionals in header files and the core locking implemementation
+enable the compiler to do the substitution transparently.
 
 
 Lock type nesting rules
@@ -254,46 +333,15 @@ Lock type nesting rules
 
   - Spinning lock types can nest inside sleeping lock types.
 
-These rules apply in general independent of CONFIG_PREEMPT_RT.
+These constraints apply both in PREEMPT_RT and otherwise.
 
-As PREEMPT_RT changes the lock category of spinlock_t and rwlock_t from
-spinning to sleeping this has obviously restrictions how they can nest with
-raw_spinlock_t.
-
-This results in the following nest ordering:
+The fact that PREEMPT_RT changes the lock category of spinlock_t and
+rwlock_t from spinning to sleeping means that they cannot be acquired while
+holding a raw spinlock.  This results in the following nesting ordering:
 
   1) Sleeping locks
   2) spinlock_t and rwlock_t
   3) raw_spinlock_t and bit spinlocks
 
-Lockdep is aware of these constraints to ensure that they are respected.
-
-
-Owner semantics
-===============
-
-Most lock types in the Linux kernel have strict owner semantics, i.e. the
-context (task) which acquires a lock has to release it.
-
-There are two exceptions:
-
-  - semaphores
-  - rwsems
-
-semaphores have no owner semantics for historical reason, and as such
-trylock and release operations can be called from any context. They are
-often used for both serialization and waiting purposes. That's generally
-discouraged and should be replaced by separate serialization and wait
-mechanisms, such as mutexes and completions.
-
-rwsems have grown interfaces which allow non owner release for special
-purposes. This usage is problematic on PREEMPT_RT because PREEMPT_RT
-substitutes all locking primitives except semaphores with RT-mutex based
-implementations to provide priority inheritance for all lock types except
-the truly spinning ones. Priority inheritance on ownerless locks is
-obviously impossible.
-
-For now the rwsem non-owner release excludes code which utilizes it from
-being used on PREEMPT_RT enabled kernels. In same cases this can be
-mitigated by disabling portions of the code, in other cases the complete
-functionality has to be disabled until a workable solution has been found.
+Lockdep will complain if these constraints are violated, both in
+PREEMPT_RT and otherwise.

^ permalink raw reply

* Re: [PATCH] powerpc/prom_init: Include the termination message in ibm,os-term RTAS call
From: Ram Pai @ 2020-03-25 13:24 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: paulus, linuxppc-dev, kvm-ppc
In-Reply-To: <20200324201211.1055236-1-farosas@linux.ibm.com>

On Tue, Mar 24, 2020 at 05:12:11PM -0300, Fabiano Rosas wrote:
> QEMU can now print the ibm,os-term message[1], so let's include it in
> the RTAS call. E.g.:
> 
>   qemu-system-ppc64: OS terminated: Switch to secure mode failed.
> 
> 1- https://git.qemu.org/?p=qemu.git;a=commitdiff;h=a4c3791ae0
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  arch/powerpc/kernel/prom_init.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 577345382b23..d543fb6d29c5 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -1773,6 +1773,9 @@ static void __init prom_rtas_os_term(char *str)
>  	if (token == 0)
>  		prom_panic("Could not get token for ibm,os-term\n");
>  	os_term_args.token = cpu_to_be32(token);
> +	os_term_args.nargs = cpu_to_be32(1);
> +	os_term_args.args[0] = cpu_to_be32(__pa(str));
> +

Reviewed-by: Ram Pai <linuxram@us.ibm.com>

RP


^ permalink raw reply

* Re: [PATCH] powerpc/prom_init: Include the termination message in ibm, os-term RTAS call
From: Fabiano Rosas @ 2020-03-25 13:24 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, paulus, kvm-ppc
In-Reply-To: <87zhc4wxy9.fsf@mpe.ellerman.id.au>

Michael Ellerman <mpe@ellerman.id.au> writes:

> Fabiano Rosas <farosas@linux.ibm.com> writes:
>
>> QEMU can now print the ibm,os-term message[1], so let's include it in
>> the RTAS call. E.g.:
>>
>>   qemu-system-ppc64: OS terminated: Switch to secure mode failed.
>>
>> 1- https://git.qemu.org/?p=qemu.git;a=commitdiff;h=a4c3791ae0
>>
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>>  arch/powerpc/kernel/prom_init.c | 3 +++
>>  1 file changed, 3 insertions(+)
>
> I have this queued:
>   https://patchwork.ozlabs.org/patch/1253390/
>
> Which I think does the same thing?
>

Ah, all good then. Nothing to see here... =)

> cheers
>
>> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
>> index 577345382b23..d543fb6d29c5 100644
>> --- a/arch/powerpc/kernel/prom_init.c
>> +++ b/arch/powerpc/kernel/prom_init.c
>> @@ -1773,6 +1773,9 @@ static void __init prom_rtas_os_term(char *str)
>>  	if (token == 0)
>>  		prom_panic("Could not get token for ibm,os-term\n");
>>  	os_term_args.token = cpu_to_be32(token);
>> +	os_term_args.nargs = cpu_to_be32(1);
>> +	os_term_args.args[0] = cpu_to_be32(__pa(str));
>> +
>>  	prom_rtas_hcall((uint64_t)&os_term_args);
>>  }
>>  #endif /* CONFIG_PPC_SVM */
>> -- 
>> 2.23.0

^ permalink raw reply

* Re: Documentation/locking/locktypes: Further clarifications and wordsmithing
From: Sebastian Siewior @ 2020-03-25 16:02 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-usb, linux-ia64, Peter Zijlstra, linux-pci, Oleg Nesterov,
	Guo Ren, Joel Fernandes, Vincent Chen, Ingo Molnar,
	Davidlohr Bueso, linux-acpi, Brian Cain, Jonathan Corbet,
	linux-hexagon, Rafael J. Wysocki, linux-csky, Linus Torvalds,
	Darren Hart, Zhang Rui, Len Brown, Fenghua Yu, paulmck, linux-pm,
	linuxppc-dev, Greentime Hu, Bjorn Helgaas, Kurt Schwemmer,
	platform-driver-x86, Kalle Valo, kbuild test robot, Felipe Balbi,
	Michal Simek, Tony Luck, Nick Hu, Geoff Levand,
	Greg Kroah-Hartman, Randy Dunlap, linux-wireless, LKML,
	Davidlohr Bueso, Arnd Bergmann, netdev, Logan Gunthorpe,
	David S. Miller, Andy Shevchenko
In-Reply-To: <87wo78y5yy.fsf@nanos.tec.linutronix.de>

On 2020-03-25 13:27:49 [+0100], Thomas Gleixner wrote:
> The documentation of rw_semaphores is wrong as it claims that the non-owner
> reader release is not supported by RT. That's just history biased memory
> distortion.
> 
> Split the 'Owner semantics' section up and add separate sections for
> semaphore and rw_semaphore to reflect reality.
> 
> Aside of that the following updates are done:
> 
>  - Add pseudo code to document the spinlock state preserving mechanism on
>    PREEMPT_RT
> 
>  - Wordsmith the bitspinlock and lock nesting sections
> 
> Co-developed-by: Paul McKenney <paulmck@kernel.org>
> Signed-off-by: Paul McKenney <paulmck@kernel.org>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

> --- a/Documentation/locking/locktypes.rst
> +++ b/Documentation/locking/locktypes.rst
> +rw_semaphore
> +============
> +
> +rw_semaphore is a multiple readers and single writer lock mechanism.
> +
> +On non-PREEMPT_RT kernels the implementation is fair, thus preventing
> +writer starvation.
> +
> +rw_semaphore complies by default with the strict owner semantics, but there
> +exist special-purpose interfaces that allow non-owner release for readers.
> +These work independent of the kernel configuration.

This reads funny, could be my English. "This works independent …" maybe?

Sebastian

^ 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