LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] powerpc/fsl-85xx: fix compile error
From: Christophe Leroy @ 2020-03-13 19:24 UTC (permalink / raw)
  To: 王文虎
  Cc: Kate Stewart, trivial, kernel, linux-kernel, Richard Fontana,
	Paul Mackerras, Thomas Gleixner, linuxppc-dev, Allison Randal
In-Reply-To: <AIkAvQC8CBiKb1HKth-6ZqrH.3.1584123458318.Hmail.wenhu.wang@vivo.com>



Le 13/03/2020 à 19:17, 王文虎 a écrit :
> 发件人:Christophe Leroy <christophe.leroy@c-s.fr>
> 发送日期:2020-03-14 01:45:11
> 收件人:WANG Wenhu <wenhu.wang@vivo.com>,Benjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,Richard Fontana <rfontana@redhat.com>,Kate Stewart <kstewart@linuxfoundation.org>,Allison Randal <allison@lohutok.net>,Thomas Gleixner <tglx@linutronix.de>,linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org
> 抄送人:kernel@vivo.com,trivial@kernel.org
> 主题:Re: [PATCH v2] powerpc/fsl-85xx: fix compile error>
>>
>> Le 13/03/2020 à 18:19, WANG Wenhu a écrit :
>>> Include "linux/of_address.h" to fix the compile error for
>>> mpc85xx_l2ctlr_of_probe() when compiling fsl_85xx_cache_sram.c.
>>>
>>>     CC      arch/powerpc/sysdev/fsl_85xx_l2ctlr.o
>>> arch/powerpc/sysdev/fsl_85xx_l2ctlr.c: In function ‘mpc85xx_l2ctlr_of_probe’:
>>> arch/powerpc/sysdev/fsl_85xx_l2ctlr.c:90:11: error: implicit declaration of function ‘of_iomap’; did you mean ‘pci_iomap’? [-Werror=implicit-function-declaration]
>>>     l2ctlr = of_iomap(dev->dev.of_node, 0);
>>>              ^~~~~~~~
>>>              pci_iomap
>>> arch/powerpc/sysdev/fsl_85xx_l2ctlr.c:90:9: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
>>>     l2ctlr = of_iomap(dev->dev.of_node, 0);
>>>            ^
>>> cc1: all warnings being treated as errors
>>> scripts/Makefile.build:267: recipe for target 'arch/powerpc/sysdev/fsl_85xx_l2ctlr.o' failed
>>> make[2]: *** [arch/powerpc/sysdev/fsl_85xx_l2ctlr.o] Error 1
>>>
>>> Fixes: commit 6db92cc9d07d ("powerpc/85xx: add cache-sram support")
>>
>> Shouldn't you Cc stable as well ?
> Pretty sure if it makes a difference(that I did not recognize).
> Does the inconsistency of Cc lead to a failure on classification
> or something else which may confuse you?

See 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/process/submitting-patches.rst?h=v5.1.9#n299

>>
>>> Signed-off-by: WANG Wenhu <wenhu.wang@vivo.com>
>>> ---
>>
>> What's the difference between v1 and v2 ?
> The label field modification: "Fixed" -> "Fixes", which now is
> identified successfully. Really sorry for the fault on v1.

Ok. Usually people tell here (just below the ---) what is the difference 
between the different versions. It helps people understand what the 
changes are.

Christophe

^ permalink raw reply

* Re:Re: [PATCH v2] powerpc/fsl-85xx: fix compile error
From: 王文虎 @ 2020-03-13 18:17 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Kate Stewart, trivial, kernel, linux-kernel, Richard Fontana,
	Paul Mackerras, Thomas Gleixner, linuxppc-dev, Allison Randal
In-Reply-To: <320843ad-32d5-8799-7c4a-150fa5fd7ef8@c-s.fr>

发件人:Christophe Leroy <christophe.leroy@c-s.fr>
发送日期:2020-03-14 01:45:11
收件人:WANG Wenhu <wenhu.wang@vivo.com>,Benjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,Richard Fontana <rfontana@redhat.com>,Kate Stewart <kstewart@linuxfoundation.org>,Allison Randal <allison@lohutok.net>,Thomas Gleixner <tglx@linutronix.de>,linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org
抄送人:kernel@vivo.com,trivial@kernel.org
主题:Re: [PATCH v2] powerpc/fsl-85xx: fix compile error>
>
>Le 13/03/2020 à 18:19, WANG Wenhu a écrit :
>> Include "linux/of_address.h" to fix the compile error for
>> mpc85xx_l2ctlr_of_probe() when compiling fsl_85xx_cache_sram.c.
>> 
>>    CC      arch/powerpc/sysdev/fsl_85xx_l2ctlr.o
>> arch/powerpc/sysdev/fsl_85xx_l2ctlr.c: In function ‘mpc85xx_l2ctlr_of_probe’:
>> arch/powerpc/sysdev/fsl_85xx_l2ctlr.c:90:11: error: implicit declaration of function ‘of_iomap’; did you mean ‘pci_iomap’? [-Werror=implicit-function-declaration]
>>    l2ctlr = of_iomap(dev->dev.of_node, 0);
>>             ^~~~~~~~
>>             pci_iomap
>> arch/powerpc/sysdev/fsl_85xx_l2ctlr.c:90:9: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
>>    l2ctlr = of_iomap(dev->dev.of_node, 0);
>>           ^
>> cc1: all warnings being treated as errors
>> scripts/Makefile.build:267: recipe for target 'arch/powerpc/sysdev/fsl_85xx_l2ctlr.o' failed
>> make[2]: *** [arch/powerpc/sysdev/fsl_85xx_l2ctlr.o] Error 1
>> 
>> Fixes: commit 6db92cc9d07d ("powerpc/85xx: add cache-sram support")
>
>Shouldn't you Cc stable as well ?
Pretty sure if it makes a difference(that I did not recognize). 
Does the inconsistency of Cc lead to a failure on classification
or something else which may confuse you?
>
>> Signed-off-by: WANG Wenhu <wenhu.wang@vivo.com>
>> ---
>
>What's the difference between v1 and v2 ?
The label field modification: "Fixed" -> "Fixes", which now is
identified successfully. Really sorry for the fault on v1.
>
>Christophe



^ permalink raw reply

* Re: [PATCH v2] powerpc/perf: Use SIER_USER_MASK while updating SPRN_SIER for EBB events
From: Segher Boessenkool @ 2020-03-13 18:06 UTC (permalink / raw)
  To: Athira Rajeev; +Cc: maddy, linuxppc-dev
In-Reply-To: <1584121747-6213-1-git-send-email-atrajeev@linux.vnet.ibm.com>

On Fri, Mar 13, 2020 at 01:49:07PM -0400, Athira Rajeev wrote:
> Sampled instruction address register (SIER), is a PMU register,

SIER stands for "Sampled Instruction Event Register", instead.  With that
change, your patch is totally clear :-)


Segher

^ permalink raw reply

* [PATCH v2] powerpc/perf: Use SIER_USER_MASK while updating SPRN_SIER for EBB events
From: Athira Rajeev @ 2020-03-13 17:49 UTC (permalink / raw)
  To: mpe; +Cc: maddy, linuxppc-dev

Sampled instruction address register (SIER), is a PMU register,
captures architecture state for a given sample. And sier_user_mask
defined in commit 330a1eb7775b ("powerpc/perf: Core EBB support for 64-bit
book3s") defines the architected bits that needs to be saved from the SPR.
Currently all of the bits from SIER are saved for EBB events. Patch fixes
this by ANDing the "sier_user_mask" to data from SIER in ebb_switch_out().
This will force save only architected bits from the SIER.

Fixes: 330a1eb7775b ("powerpc/perf: Core EBB support for 64-bit book3s")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
Changes in v2:
  - Make the commit message more clearer.

 arch/powerpc/perf/core-book3s.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 3086055..48b61cc 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -579,7 +579,7 @@ static void ebb_switch_out(unsigned long mmcr0)
 		return;
 
 	current->thread.siar  = mfspr(SPRN_SIAR);
-	current->thread.sier  = mfspr(SPRN_SIER);
+	current->thread.sier  = mfspr(SPRN_SIER) & SIER_USER_MASK;
 	current->thread.sdar  = mfspr(SPRN_SDAR);
 	current->thread.mmcr0 = mmcr0 & MMCR0_USER_MASK;
 	current->thread.mmcr2 = mfspr(SPRN_MMCR2) & MMCR2_USER_MASK;
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH v2] powerpc/fsl-85xx: fix compile error
From: Christophe Leroy @ 2020-03-13 17:45 UTC (permalink / raw)
  To: WANG Wenhu, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Richard Fontana, Kate Stewart, Allison Randal,
	Thomas Gleixner, linuxppc-dev, linux-kernel
  Cc: kernel, trivial
In-Reply-To: <20200313171953.63205-1-wenhu.wang@vivo.com>



Le 13/03/2020 à 18:19, WANG Wenhu a écrit :
> Include "linux/of_address.h" to fix the compile error for
> mpc85xx_l2ctlr_of_probe() when compiling fsl_85xx_cache_sram.c.
> 
>    CC      arch/powerpc/sysdev/fsl_85xx_l2ctlr.o
> arch/powerpc/sysdev/fsl_85xx_l2ctlr.c: In function ‘mpc85xx_l2ctlr_of_probe’:
> arch/powerpc/sysdev/fsl_85xx_l2ctlr.c:90:11: error: implicit declaration of function ‘of_iomap’; did you mean ‘pci_iomap’? [-Werror=implicit-function-declaration]
>    l2ctlr = of_iomap(dev->dev.of_node, 0);
>             ^~~~~~~~
>             pci_iomap
> arch/powerpc/sysdev/fsl_85xx_l2ctlr.c:90:9: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
>    l2ctlr = of_iomap(dev->dev.of_node, 0);
>           ^
> cc1: all warnings being treated as errors
> scripts/Makefile.build:267: recipe for target 'arch/powerpc/sysdev/fsl_85xx_l2ctlr.o' failed
> make[2]: *** [arch/powerpc/sysdev/fsl_85xx_l2ctlr.o] Error 1
> 
> Fixes: commit 6db92cc9d07d ("powerpc/85xx: add cache-sram support")

Shouldn't you Cc stable as well ?

> Signed-off-by: WANG Wenhu <wenhu.wang@vivo.com>
> ---

What's the difference between v1 and v2 ?

Christophe

^ permalink raw reply

* [PATCH v2] powerpc/fsl-85xx: fix compile error
From: WANG Wenhu @ 2020-03-13 17:19 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	WANG Wenhu, Richard Fontana, Kate Stewart, Allison Randal,
	Thomas Gleixner, linuxppc-dev, linux-kernel
  Cc: kernel, trivial

Include "linux/of_address.h" to fix the compile error for
mpc85xx_l2ctlr_of_probe() when compiling fsl_85xx_cache_sram.c.

  CC      arch/powerpc/sysdev/fsl_85xx_l2ctlr.o
arch/powerpc/sysdev/fsl_85xx_l2ctlr.c: In function ‘mpc85xx_l2ctlr_of_probe’:
arch/powerpc/sysdev/fsl_85xx_l2ctlr.c:90:11: error: implicit declaration of function ‘of_iomap’; did you mean ‘pci_iomap’? [-Werror=implicit-function-declaration]
  l2ctlr = of_iomap(dev->dev.of_node, 0);
           ^~~~~~~~
           pci_iomap
arch/powerpc/sysdev/fsl_85xx_l2ctlr.c:90:9: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
  l2ctlr = of_iomap(dev->dev.of_node, 0);
         ^
cc1: all warnings being treated as errors
scripts/Makefile.build:267: recipe for target 'arch/powerpc/sysdev/fsl_85xx_l2ctlr.o' failed
make[2]: *** [arch/powerpc/sysdev/fsl_85xx_l2ctlr.o] Error 1

Fixes: commit 6db92cc9d07d ("powerpc/85xx: add cache-sram support")
Signed-off-by: WANG Wenhu <wenhu.wang@vivo.com>
---
 arch/powerpc/sysdev/fsl_85xx_l2ctlr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c b/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c
index 2d0af0c517bb..7533572492f0 100644
--- a/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c
+++ b/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c
@@ -10,6 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_platform.h>
+#include <linux/of_address.h>
 #include <asm/io.h>
 
 #include "fsl_85xx_cache_ctlr.h"
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH 2/2] powerpc/smp: Use IS_ENABLED() to avoid #ifdef
From: Srikar Dronamraju @ 2020-03-13 14:54 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20200313112020.28235-2-mpe@ellerman.id.au>

* Michael Ellerman <mpe@ellerman.id.au> [2020-03-13 22:20:20]:

> We can avoid the #ifdef by using IS_ENABLED() in the existing
> condition check.
> 

Looks good to me.

Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>

> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/kernel/smp.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index aae61a3b3201..6d2a3a3666f0 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1189,18 +1189,17 @@ int get_physical_package_id(int cpu)
>  {
>  	int pkg_id = cpu_to_chip_id(cpu);
> 
> -#ifdef CONFIG_PPC_SPLPAR
>  	/*
>  	 * If the platform is PowerNV or Guest on KVM, ibm,chip-id is
>  	 * defined. Hence we would return the chip-id as the result of
>  	 * get_physical_package_id.
>  	 */
> -	if (pkg_id == -1 && firmware_has_feature(FW_FEATURE_LPAR)) {
> +	if (pkg_id == -1 && firmware_has_feature(FW_FEATURE_LPAR) &&
> +	    IS_ENABLED(CONFIG_PPC_SPLPAR)) {
>  		struct device_node *np = of_get_cpu_node(cpu, NULL);
>  		pkg_id = of_node_to_nid(np);
>  		of_node_put(np);
>  	}
> -#endif /* CONFIG_PPC_SPLPAR */
> 
>  	return pkg_id;
>  }
> -- 
> 2.21.1
> 

-- 
Thanks and Regards
Srikar Dronamraju


^ permalink raw reply

* Re: [PATCH 1/2] powerpc/smp: Drop superfluous NULL check
From: Srikar Dronamraju @ 2020-03-13 14:46 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20200313112020.28235-1-mpe@ellerman.id.au>

* Michael Ellerman <mpe@ellerman.id.au> [2020-03-13 22:20:19]:

> We don't need the NULL check of np, the result is the same because the
> OF helpers cope with NULL, of_node_to_nid(NULL) == NUMA_NO_NODE (-1).
> 

Looks good to me.

Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>

> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/kernel/smp.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 37c12e3bab9e..aae61a3b3201 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1197,11 +1197,8 @@ int get_physical_package_id(int cpu)
>  	 */
>  	if (pkg_id == -1 && firmware_has_feature(FW_FEATURE_LPAR)) {
>  		struct device_node *np = of_get_cpu_node(cpu, NULL);
> -
> -		if (np) {
> -			pkg_id = of_node_to_nid(np);
> -			of_node_put(np);
> -		}
> +		pkg_id = of_node_to_nid(np);
> +		of_node_put(np);
>  	}
>  #endif /* CONFIG_PPC_SPLPAR */
> 
> -- 
> 2.21.1
> 

-- 
Thanks and Regards
Srikar Dronamraju


^ permalink raw reply

* [PATCH] powerpc/pseries: Handle UE event for memcpy_mcsafe
From: Ganesh Goudar @ 2020-03-13 14:06 UTC (permalink / raw)
  To: mpe, linuxppc-dev; +Cc: mahesh, santosh, aneesh.kumar

If we hit UE at an instruction with a fixup entry, flag to
ignore the event and set nip to continue execution at the
fixup entry.
For powernv this changes are already made by commit
895e3dceeb97 ("powerpc/mce: Handle UE event for memcpy_mcsafe")

Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/ras.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 5d49d9d711da..6dc3074a34c5 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -10,6 +10,7 @@
 #include <linux/fs.h>
 #include <linux/reboot.h>
 #include <linux/irq_work.h>
+#include <linux/extable.h>
 
 #include <asm/machdep.h>
 #include <asm/rtas.h>
@@ -505,6 +506,7 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
 	int initiator = rtas_error_initiator(errp);
 	int severity = rtas_error_severity(errp);
 	u8 error_type, err_sub_type;
+	const struct exception_table_entry *entry;
 
 	if (initiator == RTAS_INITIATOR_UNKNOWN)
 		mce_err.initiator = MCE_INITIATOR_UNKNOWN;
@@ -558,6 +560,12 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
 	switch (mce_log->error_type) {
 	case MC_ERROR_TYPE_UE:
 		mce_err.error_type = MCE_ERROR_TYPE_UE;
+		entry = search_kernel_exception_table(regs->nip);
+		if (entry) {
+			mce_err.ignore_event = true;
+			regs->nip = extable_fixup(entry);
+			disposition = RTAS_DISP_FULLY_RECOVERED;
+		}
 		switch (err_sub_type) {
 		case MC_ERROR_UE_IFETCH:
 			mce_err.u.ue_error_type = MCE_UE_ERROR_IFETCH;
-- 
2.17.2


^ permalink raw reply related

* [PATCH] powerpc/pseries: Fix MCE handling on pseries
From: Ganesh Goudar @ 2020-03-13 14:04 UTC (permalink / raw)
  To: mpe, linuxppc-dev; +Cc: mahesh, npiggin

MCE handling on pSeries platform fails as recent rework to use common
code for pSeries and PowerNV in machine check error handling tries to
access per-cpu variables in realmode. The per-cpu variables may be
outside the RMO region on pSeries platform and needs translation to be
enabled for access. Just moving these per-cpu variable into RMO region
did'nt help because we queue some work to workqueues in real mode, which
again tries to touch per-cpu variables. Also fwnmi_release_errinfo()
cannot be called when translation is not enabled.

This patch fixes this by enabling translation in the exception handler
when all required real mode handling is done. This change only affects
the pSeries platform.

Without this fix below kernel crash is seen on injecting
SLB multihit:

BUG: Unable to handle kernel data access on read at 0xc00000027b205950
Faulting instruction address: 0xc00000000003b7e0
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: mcetest_slb(OE+) af_packet(E) xt_tcpudp(E) ip6t_rpfilter(E) ip6t_REJECT(E) ipt_REJECT(E) xt_conntrack(E) ip_set(E) nfnetlink(E) ebtable_nat(E) ebtable_broute(E) ip6table_nat(E) ip6table_mangle(E) ip6table_raw(E) ip6table_security(E) iptable_nat(E) nf_nat(E) nf_conntrack(E) nf_defrag_ipv6(E) nf_defrag_ipv4(E) iptable_mangle(E) iptable_raw(E) iptable_security(E) ebtable_filter(E) ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E) ip_tables(E) x_tables(E) xfs(E) ibmveth(E) vmx_crypto(E) gf128mul(E) uio_pdrv_genirq(E) uio(E) crct10dif_vpmsum(E) rtc_generic(E) btrfs(E) libcrc32c(E) xor(E) zstd_decompress(E) zstd_compress(E) raid6_pq(E) sr_mod(E) sd_mod(E) cdrom(E) ibmvscsi(E) scsi_transport_srp(E) crc32c_vpmsum(E) dm_mod(E) sg(E) scsi_mod(E)
CPU: 34 PID: 8154 Comm: insmod Kdump: loaded Tainted: G OE 5.5.0-mahesh #1
NIP: c00000000003b7e0 LR: c0000000000f2218 CTR: 0000000000000000
REGS: c000000007dcb960 TRAP: 0300 Tainted: G OE (5.5.0-mahesh)
MSR: 8000000000001003 <SF,ME,RI,LE> CR: 28002428 XER: 20040000
CFAR: c0000000000f2214 DAR: c00000027b205950 DSISR: 40000000 IRQMASK: 0
GPR00: c0000000000f2218 c000000007dcbbf0 c000000001544800 c000000007dcbd70
GPR04: 0000000000000001 c000000007dcbc98 c008000000d00258 c0080000011c0000
GPR08: 0000000000000000 0000000300000003 c000000001035950 0000000003000048
GPR12: 000000027a1d0000 c000000007f9c000 0000000000000558 0000000000000000
GPR16: 0000000000000540 c008000001110000 c008000001110540 0000000000000000
GPR20: c00000000022af10 c00000025480fd70 c008000001280000 c00000004bfbb300
GPR24: c000000001442330 c00800000800000d c008000008000000 4009287a77000510
GPR28: 0000000000000000 0000000000000002 c000000001033d30 0000000000000001
NIP [c00000000003b7e0] save_mce_event+0x30/0x240
LR [c0000000000f2218] pseries_machine_check_realmode+0x2c8/0x4f0
Call Trace:
Instruction dump:
3c4c0151 38429050 7c0802a6 60000000 fbc1fff0 fbe1fff8 f821ffd1 3d42ffaf
3fc2ffaf e98d0030 394a1150 3bdef530 <7d6a62aa> 1d2b0048 2f8b0063 380b0001
---[ end trace 46fd63f36bbdd940 ]---

Fixes: 9ca766f9891d ("powerpc/64s/pseries: machine check convert to use common event code")
Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
---
 arch/powerpc/kernel/exceptions-64s.S     | 12 ++++++++++++
 arch/powerpc/platforms/pseries/pseries.h |  1 +
 arch/powerpc/platforms/pseries/ras.c     | 11 +++++++++++
 3 files changed, 24 insertions(+)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ffc15f4f079d..93d0fddd8074 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -2405,3 +2405,15 @@ replay_interrupt_return:
 	blr
 
 _ASM_NOKPROBE_SYMBOL(__replay_interrupt)
+
+_GLOBAL(switch_to_virt_mode)
+	mflr	r10
+	mfmsr	r11
+	ori	r11,r11,MSR_IR|MSR_DR
+	/* Clear MSR_RI before setting SRR0 and SRR1 */
+	li	r12,0
+	mtmsrd	r12,1
+	mtspr	SPRN_SRR0,r10
+	mtspr	SPRN_SRR1,r11
+	RFI_TO_KERNEL
+	b	.
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 13fa370a87e4..768e3e0f137d 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -113,5 +113,6 @@ int dlpar_workqueue_init(void);
 
 void pseries_setup_rfi_flush(void);
 void pseries_lpar_read_hblkrm_characteristics(void);
+extern void switch_to_virt_mode(void);
 
 #endif /* _PSERIES_PSERIES_H */
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 1d7f973c647b..5d49d9d711da 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -683,6 +683,17 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
 #endif
 
 out:
+	/*
+	 * Enable translation as we will be accessing per-cpu variables
+	 * in save_mce_event() which may fall outside RMO region, also
+	 * leave it enabled because subsequently we will be queuing work
+	 * to workqueues where again per-cpu variables accessed, besides
+	 * fwnmi_release_errinfo() crashes when called in realmode on
+	 * pseries.
+	 * Note: All the realmode handling like flushing SLB entries for
+	 *       SLB multihit is done by now.
+	 */
+	switch_to_virt_mode();
 	save_mce_event(regs, disposition == RTAS_DISP_FULLY_RECOVERED,
 			&mce_err, regs->nip, eaddr, paddr);
 
-- 
2.17.2


^ permalink raw reply related

* Re: [PATCH v4 6/8] perf/tools: Enhance JSON/metric infrastructure to handle "?"
From: kajoljain @ 2020-03-13 12:26 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
	alexander.shishkin, anju, mamatha4, sukadev, ravi.bangoria, acme,
	jmario, namhyung, tglx, mpetlan, gregkh, linux-kernel,
	linux-perf-users, jolsa, linuxppc-dev
In-Reply-To: <20200312105214.GD311223@krava>



On 3/12/20 4:22 PM, Jiri Olsa wrote:
> On Mon, Mar 09, 2020 at 11:55:50AM +0530, Kajol Jain wrote:
> 
> SNIP
> 
>> +static int metricgroup__add_metric_runtime_param(struct strbuf *events,
>> +			struct list_head *group_list, struct pmu_event *pe)
>> +{
>> +	int i, count;
>> +	int ret = -EINVAL;
>> +
>> +	count = arch_get_runtimeparam();
>> +
>> +	/* This loop is added to create multiple
>> +	 * events depend on count value and add
>> +	 * those events to group_list.
>> +	 */
>> +
>> +	for (i = 0; i < count; i++) {
>> +		const char **ids;
>> +		int idnum;
>> +		struct egroup *eg;
>> +		char value[PATH_MAX];
>> +
>> +		expr__runtimeparam = i;
>> +
>> +		if (expr__find_other(pe->metric_expr,
>> +					NULL, &ids, &idnum) < 0)
>> +			return ret;
>> +
>> +		if (events->len > 0)
>> +			strbuf_addf(events, ",");
>> +
>> +		if (metricgroup__has_constraint(pe))
>> +			metricgroup__add_metric_non_group(events, ids, idnum);
>> +		else
>> +			metricgroup__add_metric_weak_group(events, ids, idnum);
>> +
>> +		eg = malloc(sizeof(struct egroup));
>> +		if (!eg) {
>> +			ret = -ENOMEM;
>> +			return ret;
>> +		}
>> +		sprintf(value, "%s%c%d", pe->metric_name, '_', i);
>> +		eg->ids = ids;
>> +		eg->idnum = idnum;
>> +		eg->metric_name = strdup(value);
>> +		eg->metric_expr = pe->metric_expr;
>> +		eg->metric_unit = pe->unit;
>> +		list_add_tail(&eg->nd, group_list);
>> +		ret = 0;
>> +
>> +		if (ret != 0)
>> +			break;
> 
> the inside loop is essentialy what you factor out to
> metricgroup__add_metric_param right? please nove
> addition of metricgroup__add_metric_param function
> into separate patch
> 
> jirka
> 

Ok will put it in seperate patch.

Thanks,
kajol
> 
>> +	}
>> +	return ret;
>> +}
>> +static int metricgroup__add_metric_param(struct strbuf *events,
>> +			struct list_head *group_list, struct pmu_event *pe)
>> +{
>> +
>> +	const char **ids;
>> +	int idnum;
>> +	struct egroup *eg;
>> +	int ret = -EINVAL;
>> +
>> +	if (expr__find_other(pe->metric_expr,
>> +					     NULL, &ids, &idnum) < 0)
>> +		return ret;
>> +	if (events->len > 0)
>> +		strbuf_addf(events, ",");
>> +
>> +	if (metricgroup__has_constraint(pe))
>> +		metricgroup__add_metric_non_group(events, ids, idnum);
>> +	else
>> +		metricgroup__add_metric_weak_group(events, ids, idnum);
>> +
>> +	eg = malloc(sizeof(struct egroup));
>> +	if (!eg)
>> +		ret = -ENOMEM;
>> +
>> +	eg->ids = ids;
>> +	eg->idnum = idnum;
>> +	eg->metric_name = pe->metric_name;
>> +	eg->metric_expr = pe->metric_expr;
>> +	eg->metric_unit = pe->unit;
>> +	list_add_tail(&eg->nd, group_list);
>> +	ret = 0;
>> +
>> +	return ret;
>> +}
> 
> SNIP
> 

^ permalink raw reply

* Re: [PATCH kernel] powerpc/prom_init: Pass the "os-term" message to hypervisor
From: Michael Ellerman @ 2020-03-13 11:57 UTC (permalink / raw)
  To: Alexey Kardashevskiy, linuxppc-dev
  Cc: Alexey Kardashevskiy, Ram Pai, kvm-ppc, Thiago Jung Bauermann,
	David Gibson
In-Reply-To: <20200312074404.87293-1-aik@ozlabs.ru>

Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> The "os-term" RTAS calls has one argument with a message address of
> OS termination cause. rtas_os_term() already passes it but the recently
> added prom_init's version of that missed it; it also does not fill args
> correctly.
>
> This passes the message address and initializes the number of arguments.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

I added:

Fixes: 6a9c930bd775 ("powerpc/prom_init: Add the ESM call to prom_init")

cheers


> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 577345382b23..673f13b87db1 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.nret = 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.17.1

^ permalink raw reply

* Re: [PATCH 1/3] powerpc/numa: Set numa_node for all possible cpus
From: Vlastimil Babka @ 2020-03-13 11:38 UTC (permalink / raw)
  To: Srikar Dronamraju, Joonsoo Kim
  Cc: Sachin Sant, linuxppc-dev, LKML, Michal Hocko,
	Linux Memory Management List, Kirill Tkhai, Mel Gorman,
	Joonsoo Kim, Kirill A. Shutemov, Andrew Morton, Linus Torvalds,
	Christopher Lameter
In-Reply-To: <20200313110440.GA25144@linux.vnet.ibm.com>

On 3/13/20 12:04 PM, Srikar Dronamraju wrote:
>> I lost all the memory about it. :)
>> Anyway, how about this?
>> 
>> 1. make node_present_pages() safer
>> static inline node_present_pages(nid)
>> {
>> if (!node_online(nid)) return 0;
>> return (NODE_DATA(nid)->node_present_pages);
>> }
>> 
> 
> Yes this would help.

Looks good, yeah.

>> 2. make node_to_mem_node() safer for all cases
>> In ppc arch's mem_topology_setup(void)
>> for_each_present_cpu(cpu) {
>>  numa_setup_cpu(cpu);
>>  mem_node = node_to_mem_node(numa_mem_id());
>>  if (!node_present_pages(mem_node)) {
>>   _node_numa_mem_[numa_mem_id()] = first_online_node;
>>  }
>> }
>> 
> 
> But here as discussed above, we miss the case of possible but not present nodes.
> For such nodes, the above change may not update, resulting in they still
> having 0. And node 0 can be only possible but not present.

So is there other way to do the setup so that node_to_mem_node() returns an
online+present node when called for any possible node?


^ permalink raw reply

* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Vlastimil Babka @ 2020-03-13 11:35 UTC (permalink / raw)
  To: Srikar Dronamraju, Michael Ellerman
  Cc: Sachin Sant, Michal Hocko, Pekka Enberg, Linux-Next Mailing List,
	Paul Mackerras, David Rientjes, Christopher Lameter, linuxppc-dev,
	Joonsoo Kim, Kirill Tkhai
In-Reply-To: <20200313111246.GB25144@linux.vnet.ibm.com>

On 3/13/20 12:12 PM, Srikar Dronamraju wrote:
> * Michael Ellerman <mpe@ellerman.id.au> [2020-03-13 21:48:06]:
> 
>> Sachin Sant <sachinp@linux.vnet.ibm.com> writes:
>> >> The patch below might work. Sachin can you test this? I tried faking up
>> >> a system with a memoryless node zero but couldn't get it to even start
>> >> booting.
>> >> 
>> > The patch did not help. The kernel crashed during
>> > the boot with the same call trace.
>> >
>> > BUG_ON() introduced with the patch was not triggered.
>> 
>> OK, that's weird.
>> 
>> I eventually managed to get a memoryless node going in sim, and it
>> appears to work there.
>> 
>> eg in dmesg:
>> 
>>   [    0.000000][    T0] numa:   NODE_DATA [mem 0x2000fffa2f80-0x2000fffa7fff]
>>   [    0.000000][    T0] numa:     NODE_DATA(0) on node 1
>>   [    0.000000][    T0] numa:   NODE_DATA [mem 0x2000fff9df00-0x2000fffa2f7f]
>>   ...
>>   [    0.000000][    T0] Early memory node ranges
>>   [    0.000000][    T0]   node   1: [mem 0x0000000000000000-0x00000000ffffffff]
>>   [    0.000000][    T0]   node   1: [mem 0x0000200000000000-0x00002000ffffffff]
>>   [    0.000000][    T0] Could not find start_pfn for node 0
>>   [    0.000000][    T0] Initmem setup node 0 [mem 0x0000000000000000-0x0000000000000000]
>>   [    0.000000][    T0] On node 0 totalpages: 0
>>   [    0.000000][    T0] Initmem setup node 1 [mem 0x0000000000000000-0x00002000ffffffff]
>>   [    0.000000][    T0] On node 1 totalpages: 131072
>>   
>>   # dmesg | grep set_numa
>>   [    0.000000][    T0] set_numa_mem: mem node for 0 = 1
>>   [    0.005654][    T0] set_numa_mem: mem node for 1 = 1
>> 
>> So is the problem more than just node zero having no memory?
>> 
> 
> The problem would happen with possible nodes which are not yet present. i.e
> no cpus, no memory attached to those nodes.
> 
> Please look at
> http://lore.kernel.org/lkml/20200312131438.GB3277@linux.vnet.ibm.com/t/#u
> for more details.
> 
> The summary being: pgdat/Node_Data for such nodes is not allocated. Hence

Michael's log shows that his pgdat is still allocated. But perhaps Sachin had
also your 3 patches from the other thread applied, in addition to Michael's
patch. So in his case pgdat for node 0 would indeed be no longer allocated, and
thus SLUB code was crashing in node_present_pages() instead.

> the node_present_pages(nid) called  where nid is a possible but not yet
> present node fails. Currently node_present_pages(nid) and node_to_mem_node
> don't seem to be equipped to handle possible but not present nodes.
> 
>> cheers
> 


^ permalink raw reply

* Re: [PATCH 1/3] powerpc/numa: Set numa_node for all possible cpus
From: Srikar Dronamraju @ 2020-03-13 11:22 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Sachin Sant, linuxppc-dev, LKML, Michal Hocko, linux-mm,
	Kirill Tkhai, Mel Gorman, Joonsoo Kim, Kirill A. Shutemov,
	Andrew Morton, Linus Torvalds, Christopher Lameter
In-Reply-To: <e115048c-be38-c298-b8d1-d4b513e7d2fb@suse.cz>

* Vlastimil Babka <vbabka@suse.cz> [2020-03-12 17:41:58]:

> On 3/12/20 5:13 PM, Srikar Dronamraju wrote:
> > * Vlastimil Babka <vbabka@suse.cz> [2020-03-12 14:51:38]:
> > 
> >> > * Vlastimil Babka <vbabka@suse.cz> [2020-03-12 10:30:50]:
> >> > 
> >> >> On 3/12/20 9:23 AM, Sachin Sant wrote:
> >> >> >> On 12-Mar-2020, at 10:57 AM, Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote:
> >> >> >> * Michal Hocko <mhocko@kernel.org> [2020-03-11 12:57:35]:
> >> >> >>> On Wed 11-03-20 16:32:35, Srikar Dronamraju wrote:
> >> I think we do need well defined and documented rules around node_to_mem_node(),
> >> cpu_to_node(), existence of NODE_DATA, various node_states bitmaps etc so
> >> everyone handles it the same, safe way.
> 
> So let's try to brainstorm how this would look like? What I mean are some rules
> like below, even if some details in my current understanding are most likely
> incorrect:
> 

Agree.

> with nid present in:
> N_POSSIBLE - pgdat might not exist, node_to_mem_node() must return some online
> node with memory so that we don't require everyone to search for it in slightly
> different ways
> N_ONLINE - pgdat must exist, there doesn't have to be present memory,
> node_to_mem_node() still has to return something else (?)

Right, think this has been taken care of at this time.

> N_NORMAL_MEMORY - there is present memory, node_to_mem_node() returns itself
> N_HIGH_MEMORY - node has present high memory
> 

dont see any problems with the above two to. That leaves us with N_POSSIBLE.

> > 
> > Other option would be to tweak Kirill Tkhai's patch such that we call
> > kvmalloc_node()/kzalloc_node() if node is online and call kvmalloc/kvzalloc
> > if the node is offline.
> 
> I really would like a solution that hides these ugly details from callers so
> they don't have to workaround the APIs we provide. kvmalloc_node() really
> shouldn't crash, and it should fallback automatically if we don't give it
> __GFP_THISNODE
> 

Agree thats its better to make API's robust where possible.

> However, taking a step back, memcg_alloc_shrinker_maps() is probably rather
> wasteful on systems with 256 possible nodes and only few present, by allocating
> effectively dead structures for each memcg.
> 

If we dont allocate now, we would have to allocate them when we online the
nodes. To me it looks better to allocate as soon as the nodes are onlined,

> SLUB tries to be smart, so it allocates the per-node per-cache structures only
> when the node goes online in slab_mem_going_online_callback(). This is why
> there's a crash when such non-existing structures are accessed for a node that's
> not online, and why they shouldn't be accessed.
> 
> Perhaps memcg should do the same on-demand allocation, if possible.
> 

Right.

-- 
Thanks and Regards
Srikar Dronamraju


^ permalink raw reply

* [PATCH 2/2] powerpc/smp: Use IS_ENABLED() to avoid #ifdef
From: Michael Ellerman @ 2020-03-13 11:20 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: srikar
In-Reply-To: <20200313112020.28235-1-mpe@ellerman.id.au>

We can avoid the #ifdef by using IS_ENABLED() in the existing
condition check.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/smp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index aae61a3b3201..6d2a3a3666f0 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1189,18 +1189,17 @@ int get_physical_package_id(int cpu)
 {
 	int pkg_id = cpu_to_chip_id(cpu);
 
-#ifdef CONFIG_PPC_SPLPAR
 	/*
 	 * If the platform is PowerNV or Guest on KVM, ibm,chip-id is
 	 * defined. Hence we would return the chip-id as the result of
 	 * get_physical_package_id.
 	 */
-	if (pkg_id == -1 && firmware_has_feature(FW_FEATURE_LPAR)) {
+	if (pkg_id == -1 && firmware_has_feature(FW_FEATURE_LPAR) &&
+	    IS_ENABLED(CONFIG_PPC_SPLPAR)) {
 		struct device_node *np = of_get_cpu_node(cpu, NULL);
 		pkg_id = of_node_to_nid(np);
 		of_node_put(np);
 	}
-#endif /* CONFIG_PPC_SPLPAR */
 
 	return pkg_id;
 }
-- 
2.21.1


^ permalink raw reply related

* [PATCH 1/2] powerpc/smp: Drop superfluous NULL check
From: Michael Ellerman @ 2020-03-13 11:20 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: srikar

We don't need the NULL check of np, the result is the same because the
OF helpers cope with NULL, of_node_to_nid(NULL) == NUMA_NO_NODE (-1).

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/smp.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 37c12e3bab9e..aae61a3b3201 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1197,11 +1197,8 @@ int get_physical_package_id(int cpu)
 	 */
 	if (pkg_id == -1 && firmware_has_feature(FW_FEATURE_LPAR)) {
 		struct device_node *np = of_get_cpu_node(cpu, NULL);
-
-		if (np) {
-			pkg_id = of_node_to_nid(np);
-			of_node_put(np);
-		}
+		pkg_id = of_node_to_nid(np);
+		of_node_put(np);
 	}
 #endif /* CONFIG_PPC_SPLPAR */
 
-- 
2.21.1


^ permalink raw reply related

* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Srikar Dronamraju @ 2020-03-13 11:12 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Sachin Sant, Michal Hocko, Pekka Enberg, Linux-Next Mailing List,
	Paul Mackerras, Vlastimil Babka, David Rientjes,
	Christopher Lameter, linuxppc-dev, Joonsoo Kim, Kirill Tkhai
In-Reply-To: <875zf8y1i1.fsf@mpe.ellerman.id.au>

* Michael Ellerman <mpe@ellerman.id.au> [2020-03-13 21:48:06]:

> Sachin Sant <sachinp@linux.vnet.ibm.com> writes:
> >> The patch below might work. Sachin can you test this? I tried faking up
> >> a system with a memoryless node zero but couldn't get it to even start
> >> booting.
> >> 
> > The patch did not help. The kernel crashed during
> > the boot with the same call trace.
> >
> > BUG_ON() introduced with the patch was not triggered.
> 
> OK, that's weird.
> 
> I eventually managed to get a memoryless node going in sim, and it
> appears to work there.
> 
> eg in dmesg:
> 
>   [    0.000000][    T0] numa:   NODE_DATA [mem 0x2000fffa2f80-0x2000fffa7fff]
>   [    0.000000][    T0] numa:     NODE_DATA(0) on node 1
>   [    0.000000][    T0] numa:   NODE_DATA [mem 0x2000fff9df00-0x2000fffa2f7f]
>   ...
>   [    0.000000][    T0] Early memory node ranges
>   [    0.000000][    T0]   node   1: [mem 0x0000000000000000-0x00000000ffffffff]
>   [    0.000000][    T0]   node   1: [mem 0x0000200000000000-0x00002000ffffffff]
>   [    0.000000][    T0] Could not find start_pfn for node 0
>   [    0.000000][    T0] Initmem setup node 0 [mem 0x0000000000000000-0x0000000000000000]
>   [    0.000000][    T0] On node 0 totalpages: 0
>   [    0.000000][    T0] Initmem setup node 1 [mem 0x0000000000000000-0x00002000ffffffff]
>   [    0.000000][    T0] On node 1 totalpages: 131072
>   
>   # dmesg | grep set_numa
>   [    0.000000][    T0] set_numa_mem: mem node for 0 = 1
>   [    0.005654][    T0] set_numa_mem: mem node for 1 = 1
> 
> So is the problem more than just node zero having no memory?
> 

The problem would happen with possible nodes which are not yet present. i.e
no cpus, no memory attached to those nodes.

Please look at
http://lore.kernel.org/lkml/20200312131438.GB3277@linux.vnet.ibm.com/t/#u
for more details.

The summary being: pgdat/Node_Data for such nodes is not allocated. Hence
the node_present_pages(nid) called  where nid is a possible but not yet
present node fails. Currently node_present_pages(nid) and node_to_mem_node
don't seem to be equipped to handle possible but not present nodes.

> cheers

-- 
Thanks and Regards
Srikar Dronamraju


^ permalink raw reply

* Re: [PATCH 1/3] powerpc/numa: Set numa_node for all possible cpus
From: Srikar Dronamraju @ 2020-03-13 11:04 UTC (permalink / raw)
  To: Joonsoo Kim
  Cc: Sachin Sant, linuxppc-dev, LKML, Michal Hocko,
	Linux Memory Management List, Kirill Tkhai, Mel Gorman,
	Joonsoo Kim, Kirill A. Shutemov, Andrew Morton, Linus Torvalds,
	Christopher Lameter, Vlastimil Babka
In-Reply-To: <CAAmzW4OFy51BhAT62tdVQD52NNMWm+UPgoGAX97omY7P+nJ+5w@mail.gmail.com>

* Joonsoo Kim <js1304@gmail.com> [2020-03-13 18:47:49]:

> > >>
> > >> > Also for a memoryless/cpuless node or possible but not present nodes,
> > >> > node_to_mem_node(node) will still end up as node (atleast on powerpc).
> > >>
> > >> I think that's the place where this would be best to fix.
> > >>
> > >
> > > Maybe. I thought about it but the current set_numa_mem semantics are apt
> > > for memoryless cpu node and not for possible nodes.  We could have upto 256
> > > possible nodes and only 2 nodes (1,2) with cpu and 1 node (1) with memory.
> > > node_to_mem_node seems to return what is set in set_numa_mem().
> > > set_numa_mem() seems to say set my numa_mem node for the current memoryless
> > > node to the param passed.
> > >
> > > But how do we set numa_mem for all the other 253 possible nodes, which
> > > probably will have 0 as default?
> > >
> > > Should we introduce another API such that we could update for all possible
> > > nodes?
> >
> > If we want to rely on node_to_mem_node() to give us something safe for each
> > possible node, then probably it would have to be like that, yeah.
> >
> > >> > I tried with this hunk below and it works.
> > >> >
> > >> > But I am not sure if we need to check at other places were
> > >> > node_present_pages is being called.
> > >>
> > >> I think this seems to defeat the purpose of node_to_mem_node()? Shouldn't it
> > >> return only nodes that are online with present memory?
> > >> CCing Joonsoo who seems to have introduced this in ad2c8144418c ("topology: add
> > >> support for node_to_mem_node() to determine the fallback node")
> > >>
> > >
> > > Agree
> 
> I lost all the memory about it. :)
> Anyway, how about this?
> 
> 1. make node_present_pages() safer
> static inline node_present_pages(nid)
> {
> if (!node_online(nid)) return 0;
> return (NODE_DATA(nid)->node_present_pages);
> }
> 

Yes this would help.

> 2. make node_to_mem_node() safer for all cases
> In ppc arch's mem_topology_setup(void)
> for_each_present_cpu(cpu) {
>  numa_setup_cpu(cpu);
>  mem_node = node_to_mem_node(numa_mem_id());
>  if (!node_present_pages(mem_node)) {
>   _node_numa_mem_[numa_mem_id()] = first_online_node;
>  }
> }
> 

But here as discussed above, we miss the case of possible but not present nodes.
For such nodes, the above change may not update, resulting in they still
having 0. And node 0 can be only possible but not present.

-- 
Thanks and Regards
Srikar Dronamraju


^ permalink raw reply

* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Michael Ellerman @ 2020-03-13 10:48 UTC (permalink / raw)
  To: Sachin Sant
  Cc: Srikar Dronamraju, Michal Hocko, Pekka Enberg,
	Linux-Next Mailing List, Paul Mackerras, Vlastimil Babka,
	David Rientjes, Christopher Lameter, linuxppc-dev, Joonsoo Kim,
	Kirill Tkhai
In-Reply-To: <F0FBAD4E-3257-4DFD-BFE2-4AD7D811CFB3@linux.vnet.ibm.com>

Sachin Sant <sachinp@linux.vnet.ibm.com> writes:
>> The patch below might work. Sachin can you test this? I tried faking up
>> a system with a memoryless node zero but couldn't get it to even start
>> booting.
>> 
> The patch did not help. The kernel crashed during
> the boot with the same call trace.
>
> BUG_ON() introduced with the patch was not triggered.

OK, that's weird.

I eventually managed to get a memoryless node going in sim, and it
appears to work there.

eg in dmesg:

  [    0.000000][    T0] numa:   NODE_DATA [mem 0x2000fffa2f80-0x2000fffa7fff]
  [    0.000000][    T0] numa:     NODE_DATA(0) on node 1
  [    0.000000][    T0] numa:   NODE_DATA [mem 0x2000fff9df00-0x2000fffa2f7f]
  ...
  [    0.000000][    T0] Early memory node ranges
  [    0.000000][    T0]   node   1: [mem 0x0000000000000000-0x00000000ffffffff]
  [    0.000000][    T0]   node   1: [mem 0x0000200000000000-0x00002000ffffffff]
  [    0.000000][    T0] Could not find start_pfn for node 0
  [    0.000000][    T0] Initmem setup node 0 [mem 0x0000000000000000-0x0000000000000000]
  [    0.000000][    T0] On node 0 totalpages: 0
  [    0.000000][    T0] Initmem setup node 1 [mem 0x0000000000000000-0x00002000ffffffff]
  [    0.000000][    T0] On node 1 totalpages: 131072
  
  # dmesg | grep set_numa
  [    0.000000][    T0] set_numa_mem: mem node for 0 = 1
  [    0.005654][    T0] set_numa_mem: mem node for 1 = 1

So is the problem more than just node zero having no memory?

cheers

^ permalink raw reply

* Re: [PATCH -next] PCI: rpaphp: remove set but not used variable 'value'
From: Michael Ellerman @ 2020-03-13 10:43 UTC (permalink / raw)
  To: Bjorn Helgaas, Chen Zhou
  Cc: tyreld, paulus, linuxppc-dev, linux-kernel, linux-pci
In-Reply-To: <20200312144157.GA110750@google.com>

Bjorn Helgaas <helgaas@kernel.org> writes:
> On Thu, Mar 12, 2020 at 09:38:02AM -0500, Bjorn Helgaas wrote:
>> On Thu, Mar 12, 2020 at 10:04:12PM +0800, Chen Zhou wrote:
>> > Fixes gcc '-Wunused-but-set-variable' warning:
>> > 
>> > drivers/pci/hotplug/rpaphp_core.c: In function is_php_type:
>> > drivers/pci/hotplug/rpaphp_core.c:291:16: warning:
>> > 	variable value set but not used [-Wunused-but-set-variable]
>> > 
>> > Reported-by: Hulk Robot <hulkci@huawei.com>
>> > Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
>> 
>> Michael, if you want this:
>> 
>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>> 
>> If you don't mind, edit the subject to follow the convention, e.g.,
>> 
>>   PCI: rpaphp: Remove unused variable 'value'
>> 
>> Apparently simple_strtoul() is deprecated and we're supposed to use
>> kstrtoul() instead.  Looks like kstrtoul() might simplify the code a
>> little, too, e.g.,
>> 
>>   if (kstrtoul(drc_type, 0, &value) == 0)
>>     return 1;
>> 
>>   return 0;
>
> I guess there are several other uses of simple_strtoul() in this file.
> Not sure if it's worth changing them all, just this one, or just the
> patch below as-is.

I'll take this patch as-is, and someone can send a follow-up to convert
the whole file to kstrtoul().

cheers

^ permalink raw reply

* [PATCH] powerpc/book3s/hash64/devmap: Use H_PAGE_THP_HUGE when setting up huge devmap pte entries
From: Aneesh Kumar K.V @ 2020-03-13  9:48 UTC (permalink / raw)
  To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, oohall

H_PAGE_THP_HUGE is used to differentiate between a THP hugepage and hugetlb
hugepage entries. The difference is w.r.t how we handle hash fault on these
address. THP address enables MPSS in segments. We want to manage devmap hugepage
entries similar to THP pt entries. Hence use H_PAGE_THP_HUGE for devmap huge pte
entries.

With current code while handling hash pte fault, we do set is_thp = true when
finding devmap pte huge pte entries.

Current code also does the below sequence we setting up huge devmap entries.
	entry = pmd_mkhuge(pfn_t_pmd(pfn, prot));
	if (pfn_t_devmap(pfn))
		entry = pmd_mkdevmap(entry);

In that case we would find both H_PAGE_THP_HUGE and PAGE_DEVMAP set for huge
devmap pte entries. This results in false positive error like below.

 kernel BUG at /home/kvaneesh/src/linux/mm/memory.c:4321!
 Oops: Exception in kernel mode, sig: 5 [#1]
 LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
 Modules linked in:
 CPU: 56 PID: 67996 Comm: t_mmap_dio Not tainted 5.6.0-rc4-59640-g371c804dedbc #128
 ....
 NIP [c00000000044c9e4] __follow_pte_pmd+0x264/0x900
 LR [c0000000005d45f8] dax_writeback_one+0x1a8/0x740
 Call Trace:
 [c000000c6e9f38c0] [c0000000013f4130] str_spec.74809+0x22ffb4/0x2d116c (unreliable)
 [c000000c6e9f3960] [c0000000005d45f8] dax_writeback_one+0x1a8/0x740
 [c000000c6e9f3a40] [c0000000005d4dfc] dax_writeback_mapping_range+0x26c/0x700
 [c000000c6e9f3b30] [c000000000666580] ext4_dax_writepages+0x150/0x5a0
 [c000000c6e9f3ba0] [c0000000003fe278] do_writepages+0x68/0x180
 [c000000c6e9f3c10] [c0000000003ecc58] __filemap_fdatawrite_range+0x138/0x180
 [c000000c6e9f3cc0] [c0000000003ede74] file_write_and_wait_range+0xa4/0x110
 [c000000c6e9f3d10] [c0000000006552d0] ext4_sync_file+0x370/0x6e0
 [c000000c6e9f3d70] [c00000000057d330] vfs_fsync_range+0x70/0xf0
 [c000000c6e9f3db0] [c00000000046a000] sys_msync+0x220/0x2e0
 [c000000c6e9f3e20] [c00000000000b478] system_call+0x5c/0x68
 Instruction dump:
 7a941564 392affff 7fbffc36 7a94f082 7d2907b4 78f4f00e 7fff4838 7bff1f24
 7e54f82a 7e74fa14 724900a0 40820410 <0b080000> 72490040 418201c8 2fb7000

This is because our pmd_trans_huge check doesn't exclude _PAGE_DEVMAP.

To make this all consistent, update pmd_mkdevmap to set H_PAGE_THP_HUGE and
pmd_trans_huge check now excludes _PAGE_DEVMAP correctly.

Fixes: ebd31197931d ("powerpc/mm: Add devmap support for ppc64")
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/hash-4k.h  | 6 ++++++
 arch/powerpc/include/asm/book3s/64/hash-64k.h | 8 +++++++-
 arch/powerpc/include/asm/book3s/64/pgtable.h  | 4 +++-
 arch/powerpc/include/asm/book3s/64/radix.h    | 5 +++++
 4 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h b/arch/powerpc/include/asm/book3s/64/hash-4k.h
index 8fd8599c9395..3f9ae3585ab9 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
@@ -156,6 +156,12 @@ extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
 extern int hash__has_transparent_hugepage(void);
 #endif
 
+static inline pmd_t hash__pmd_mkdevmap(pmd_t pmd)
+{
+	BUG();
+	return pmd;
+}
+
 #endif /* !__ASSEMBLY__ */
 
 #endif /* _ASM_POWERPC_BOOK3S_64_HASH_4K_H */
diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc/include/asm/book3s/64/hash-64k.h
index d1d9177d9ebd..0729c034e56f 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
@@ -246,7 +246,7 @@ static inline void mark_hpte_slot_valid(unsigned char *hpte_slot_array,
  */
 static inline int hash__pmd_trans_huge(pmd_t pmd)
 {
-	return !!((pmd_val(pmd) & (_PAGE_PTE | H_PAGE_THP_HUGE)) ==
+	return !!((pmd_val(pmd) & (_PAGE_PTE | H_PAGE_THP_HUGE | _PAGE_DEVMAP)) ==
 		  (_PAGE_PTE | H_PAGE_THP_HUGE));
 }
 
@@ -272,6 +272,12 @@ extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
 				       unsigned long addr, pmd_t *pmdp);
 extern int hash__has_transparent_hugepage(void);
 #endif /*  CONFIG_TRANSPARENT_HUGEPAGE */
+
+static inline pmd_t hash__pmd_mkdevmap(pmd_t pmd)
+{
+	return __pmd(pmd_val(pmd) | (_PAGE_PTE | H_PAGE_THP_HUGE | _PAGE_DEVMAP));
+}
+
 #endif	/* __ASSEMBLY__ */
 
 #endif /* _ASM_POWERPC_BOOK3S_64_HASH_64K_H */
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 201a69e6a355..368b136517e0 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1303,7 +1303,9 @@ extern void serialize_against_pte_lookup(struct mm_struct *mm);
 
 static inline pmd_t pmd_mkdevmap(pmd_t pmd)
 {
-	return __pmd(pmd_val(pmd) | (_PAGE_PTE | _PAGE_DEVMAP));
+	if (radix_enabled())
+		return radix__pmd_mkdevmap(pmd);
+	return hash__pmd_mkdevmap(pmd);
 }
 
 static inline int pmd_devmap(pmd_t pmd)
diff --git a/arch/powerpc/include/asm/book3s/64/radix.h b/arch/powerpc/include/asm/book3s/64/radix.h
index d97db3ad9aae..a1c60d5b50af 100644
--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h
@@ -263,6 +263,11 @@ static inline int radix__has_transparent_hugepage(void)
 }
 #endif
 
+static inline pmd_t radix__pmd_mkdevmap(pmd_t pmd)
+{
+	return __pmd(pmd_val(pmd) | (_PAGE_PTE | _PAGE_DEVMAP));
+}
+
 extern int __meminit radix__vmemmap_create_mapping(unsigned long start,
 					     unsigned long page_size,
 					     unsigned long phys);
-- 
2.24.1


^ permalink raw reply related

* Re: [PATCH 1/3] powerpc/numa: Set numa_node for all possible cpus
From: Joonsoo Kim @ 2020-03-13  9:47 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Sachin Sant, Srikar Dronamraju, linuxppc-dev, LKML, Michal Hocko,
	Linux Memory Management List, Kirill Tkhai, Mel Gorman,
	Joonsoo Kim, Kirill A. Shutemov, Andrew Morton, Linus Torvalds,
	Christopher Lameter
In-Reply-To: <e115048c-be38-c298-b8d1-d4b513e7d2fb@suse.cz>

2020년 3월 13일 (금) 오전 1:42, Vlastimil Babka <vbabka@suse.cz>님이 작성:
>
> On 3/12/20 5:13 PM, Srikar Dronamraju wrote:
> > * Vlastimil Babka <vbabka@suse.cz> [2020-03-12 14:51:38]:
> >
> >> > * Vlastimil Babka <vbabka@suse.cz> [2020-03-12 10:30:50]:
> >> >
> >> >> On 3/12/20 9:23 AM, Sachin Sant wrote:
> >> >> >> On 12-Mar-2020, at 10:57 AM, Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote:
> >> >> >> * Michal Hocko <mhocko@kernel.org> [2020-03-11 12:57:35]:
> >> >> >>> On Wed 11-03-20 16:32:35, Srikar Dronamraju wrote:
> >> >> >>>> To ensure a cpuless, memoryless dummy node is not online, powerpc need
> >> >> >>>> to make sure all possible but not present cpu_to_node are set to a
> >> >> >>>> proper node.
> >> >> >>>
> >> >> >>> Just curious, is this somehow related to
> >> >> >>> http://lkml.kernel.org/r/20200227182650.GG3771@dhcp22.suse.cz?
> >> >> >>>
> >> >> >>
> >> >> >> The issue I am trying to fix is a known issue in Powerpc since many years.
> >> >> >> So this surely not a problem after a75056fc1e7c (mm/memcontrol.c: allocate
> >> >> >> shrinker_map on appropriate NUMA node").
> >> >> >>
> >> >
> >> > While I am not an expert in the slub area, I looked at the patch
> >> > a75056fc1e7c and had some thoughts on why this could be causing this issue.
> >> >
> >> > On the system where the crash happens, the possible number of nodes is much
> >> > greater than the number of onlined nodes. The pdgat or the NODE_DATA is only
> >> > available for onlined nodes.
> >> >
> >> > With a75056fc1e7c memcg_alloc_shrinker_maps, we end up calling kzalloc_node
> >> > for all possible nodes and in ___slab_alloc we end up looking at the
> >> > node_present_pages which is NODE_DATA(nid)->node_present_pages.
> >> > i.e for a node whose pdgat struct is not allocated, we are trying to
> >> > dereference.
> >>
> >> From what we saw, the pgdat does exist, the problem is that slab's per-node data
> >> doesn't exist for a node that doesn't have present pages, as it would be a waste
> >> of memory.
> >
> > Just to be clear
> > Before my 3 patches to fix dummy node:
> > srikar@ltc-zzci-2 /sys/devices/system/node $ cat $PWD/possible
> > 0-31
> > srikar@ltc-zzci-2 /sys/devices/system/node $ cat $PWD/online
> > 0-1
>
> OK
>
> >>
> >> Uh actually you are probably right, the NODE_DATA doesn't exist anymore? In
> >> Sachin's first report [1] we have
> >>
> >> [    0.000000] numa:   NODE_DATA [mem 0x8bfedc900-0x8bfee3fff]
> >> [    0.000000] numa:     NODE_DATA(0) on node 1
> >> [    0.000000] numa:   NODE_DATA [mem 0x8bfed5200-0x8bfedc8ff]
> >>
> >
> > So even if pgdat would exist for nodes 0 and 1, there is no pgdat for the
> > rest 30 nodes.
>
> I see. Perhaps node_present_pages(node) is not safe in SLUB then and it should
> check online first, as you suggested.
>
> >> But in this thread, with your patches Sachin reports:
> >
> > and with my patches
> > srikar@ltc-zzci-2 /sys/devices/system/node $ cat $PWD/possible
> > 0-31
> > srikar@ltc-zzci-2 /sys/devices/system/node $ cat $PWD/online
> > 1
> >
> >>
> >> [    0.000000] numa:   NODE_DATA [mem 0x8bfedc900-0x8bfee3fff]
> >>
> >
> > so we only see one pgdat.
> >
> >> So I assume it's just node 1. In that case, node_present_pages is really dangerous.
> >>
> >> [1]
> >> https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
> >>
> >> > Also for a memoryless/cpuless node or possible but not present nodes,
> >> > node_to_mem_node(node) will still end up as node (atleast on powerpc).
> >>
> >> I think that's the place where this would be best to fix.
> >>
> >
> > Maybe. I thought about it but the current set_numa_mem semantics are apt
> > for memoryless cpu node and not for possible nodes.  We could have upto 256
> > possible nodes and only 2 nodes (1,2) with cpu and 1 node (1) with memory.
> > node_to_mem_node seems to return what is set in set_numa_mem().
> > set_numa_mem() seems to say set my numa_mem node for the current memoryless
> > node to the param passed.
> >
> > But how do we set numa_mem for all the other 253 possible nodes, which
> > probably will have 0 as default?
> >
> > Should we introduce another API such that we could update for all possible
> > nodes?
>
> If we want to rely on node_to_mem_node() to give us something safe for each
> possible node, then probably it would have to be like that, yeah.
>
> >> > I tried with this hunk below and it works.
> >> >
> >> > But I am not sure if we need to check at other places were
> >> > node_present_pages is being called.
> >>
> >> I think this seems to defeat the purpose of node_to_mem_node()? Shouldn't it
> >> return only nodes that are online with present memory?
> >> CCing Joonsoo who seems to have introduced this in ad2c8144418c ("topology: add
> >> support for node_to_mem_node() to determine the fallback node")
> >>
> >
> > Agree

I lost all the memory about it. :)
Anyway, how about this?

1. make node_present_pages() safer
static inline node_present_pages(nid)
{
if (!node_online(nid)) return 0;
return (NODE_DATA(nid)->node_present_pages);
}

2. make node_to_mem_node() safer for all cases
In ppc arch's mem_topology_setup(void)
for_each_present_cpu(cpu) {
 numa_setup_cpu(cpu);
 mem_node = node_to_mem_node(numa_mem_id());
 if (!node_present_pages(mem_node)) {
  _node_numa_mem_[numa_mem_id()] = first_online_node;
 }
}

With these two changes, we can uses node_present_pages() and node_to_mem_node()
as intended.

Thanks.

> >> I think we do need well defined and documented rules around node_to_mem_node(),
> >> cpu_to_node(), existence of NODE_DATA, various node_states bitmaps etc so
> >> everyone handles it the same, safe way.
>
> So let's try to brainstorm how this would look like? What I mean are some rules
> like below, even if some details in my current understanding are most likely
> incorrect:
>
> with nid present in:
> N_POSSIBLE - pgdat might not exist, node_to_mem_node() must return some online
> node with memory so that we don't require everyone to search for it in slightly
> different ways
> N_ONLINE - pgdat must exist, there doesn't have to be present memory,
> node_to_mem_node() still has to return something else (?)
> N_NORMAL_MEMORY - there is present memory, node_to_mem_node() returns itself
> N_HIGH_MEMORY - node has present high memory

^ permalink raw reply

* Re: [PATCH] ocxl: control via sysfs whether the FPGA is reloaded on a link reset
From: Frederic Barrat @ 2020-03-13  9:25 UTC (permalink / raw)
  To: Philippe Bergheaud, linuxppc-dev; +Cc: clombard, ajd, alastair
In-Reply-To: <20200311115216.532-1-felix@linux.ibm.com>



Le 11/03/2020 à 12:52, Philippe Bergheaud a écrit :
> Some opencapi FPGA images allow to control if the FPGA should be reloaded
> on the next adapter reset. If it is supported, the image specifies it
> through a Vendor Specific DVSEC in the config space of function 0.
> 
> This patch adds an interface to sysfs to control that behavior, if possible.
> 
> Signed-off-by: Philippe Bergheaud <felix@linux.ibm.com>
> ---
>   Documentation/ABI/testing/sysfs-class-ocxl | 10 ++++
>   drivers/misc/ocxl/config.c                 | 59 +++++++++++++++++++++-
>   drivers/misc/ocxl/ocxl_internal.h          |  6 +++
>   drivers/misc/ocxl/sysfs.c                  | 35 +++++++++++++
>   include/misc/ocxl-config.h                 |  1 +
>   5 files changed, 110 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-ocxl b/Documentation/ABI/testing/sysfs-class-ocxl
> index b5b1fa197592..b9ea671d5805 100644
> --- a/Documentation/ABI/testing/sysfs-class-ocxl
> +++ b/Documentation/ABI/testing/sysfs-class-ocxl
> @@ -33,3 +33,13 @@ Date:		January 2018
>   Contact:	linuxppc-dev@lists.ozlabs.org
>   Description:	read/write
>   		Give access the global mmio area for the AFU
> +
> +What:		/sys/class/ocxl/<afu name>/reload_on_reset
> +Date:		February 2020
> +Contact:	linuxppc-dev@lists.ozlabs.org
> +Description:	read/write
> +		Control whether the FPGA is reloaded on a link reset
> +			0	Do not reload FPGA image from flash
> +			1	Reload FPGA image from flash
> +			unavailable
> +				The device does not support this capability
> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
> index c8e19bfb5ef9..3488463c1640 100644
> --- a/drivers/misc/ocxl/config.c
> +++ b/drivers/misc/ocxl/config.c
> @@ -71,6 +71,20 @@ static int find_dvsec_afu_ctrl(struct pci_dev *dev, u8 afu_idx)
>   	return 0;
>   }
>   
> +/**
> + * get_function_0() - Find a related PCI device (function 0)
> + * @device: PCI device to match
> + *
> + * Returns a pointer to the related device, or null if not found
> + */
> +static struct pci_dev *get_function_0(struct pci_dev *dev)
> +{
> +	unsigned int devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 0);
> +
> +	return pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
> +					   dev->bus->number, devfn);
> +}
> +
>   static void read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
>   {
>   	u16 val;
> @@ -159,7 +173,7 @@ static int read_dvsec_afu_info(struct pci_dev *dev, struct ocxl_fn_config *fn)
>   static int read_dvsec_vendor(struct pci_dev *dev)
>   {
>   	int pos;
> -	u32 cfg, tlx, dlx;
> +	u32 cfg, tlx, dlx, reset_reload;
>   
>   	/*
>   	 * vendor specific DVSEC is optional
> @@ -178,11 +192,54 @@ static int read_dvsec_vendor(struct pci_dev *dev)
>   	pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_CFG_VERS, &cfg);
>   	pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_TLX_VERS, &tlx);
>   	pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_DLX_VERS, &dlx);
> +	pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD, &reset_reload);
>   
>   	dev_dbg(&dev->dev, "Vendor specific DVSEC:\n");
>   	dev_dbg(&dev->dev, "  CFG version = 0x%x\n", cfg);
>   	dev_dbg(&dev->dev, "  TLX version = 0x%x\n", tlx);
>   	dev_dbg(&dev->dev, "  DLX version = 0x%x\n", dlx);
> +	dev_dbg(&dev->dev, "  ResetReload = 0x%x\n", reset_reload);


We should apply a mask on bit 0 in case some future version starts using 
the extra bits.


> +	return 0;
> +}
> +
> +int ocxl_config_get_reset_reload(struct pci_dev *dev, int *val)
> +{
> +	int reset_reload = -1;
> +	int pos = 0;
> +	struct pci_dev *dev0 = get_function_0(dev);
> +


We could avoid calling get_function_0() if the pci_dev is already for 
function 0. But we know that all current images will be called for 
function != 0, so I guess that is fine.


The rest of the patch looks ok to me.

   Fred


> +	if (dev0)
> +		pos = find_dvsec(dev0, OCXL_DVSEC_VENDOR_ID);
> +
> +	if (pos)
> +		pci_read_config_dword(dev0,
> +				      pos + OCXL_DVSEC_VENDOR_RESET_RELOAD,
> +				      &reset_reload);
> +	if (reset_reload == -1)
> +		return reset_reload;
> +
> +	*val = reset_reload & BIT(0);
> +	return 0;
> +}
> +
> +int ocxl_config_set_reset_reload(struct pci_dev *dev, int val)
> +{
> +	int reset_reload = -1;
> +	int pos = 0;
> +	struct pci_dev *dev0 = get_function_0(dev);
> +
> +	if (dev0)
> +		pos = find_dvsec(dev0, OCXL_DVSEC_VENDOR_ID);
> +
> +	if (pos)
> +		pci_read_config_dword(dev0,
> +				      pos + OCXL_DVSEC_VENDOR_RESET_RELOAD,
> +				      &reset_reload);
> +	if (reset_reload == -1)
> +		return reset_reload;
> +
> +	val &= BIT(0);
> +	pci_write_config_dword(dev0, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD, val);
>   	return 0;
>   }
>   
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index 345bf843a38e..af9a84aeee6f 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -112,6 +112,12 @@ void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
>    */
>   int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
>   
> +/*
> + * Control whether the FPGA is reloaded on a link reset
> + */
> +int ocxl_config_get_reset_reload(struct pci_dev *dev, int *val);
> +int ocxl_config_set_reset_reload(struct pci_dev *dev, int val);
> +
>   /*
>    * Check if an AFU index is valid for the given function.
>    *
> diff --git a/drivers/misc/ocxl/sysfs.c b/drivers/misc/ocxl/sysfs.c
> index 58f1ba264206..8f69f7311343 100644
> --- a/drivers/misc/ocxl/sysfs.c
> +++ b/drivers/misc/ocxl/sysfs.c
> @@ -51,11 +51,46 @@ static ssize_t contexts_show(struct device *device,
>   			afu->pasid_count, afu->pasid_max);
>   }
>   
> +static ssize_t reload_on_reset_show(struct device *device,
> +		struct device_attribute *attr,
> +		char *buf)
> +{
> +	struct ocxl_afu *afu = to_afu(device);
> +	struct ocxl_fn *fn = afu->fn;
> +	struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
> +	int val;
> +
> +	if (ocxl_config_get_reset_reload(pci_dev, &val))
> +		return scnprintf(buf, PAGE_SIZE, "unavailable\n");
> +
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", val);
> +}
> +
> +static ssize_t reload_on_reset_store(struct device *device,
> +		struct device_attribute *attr,
> +		const char *buf, size_t count)
> +{
> +	struct ocxl_afu *afu = to_afu(device);
> +	struct ocxl_fn *fn = afu->fn;
> +	struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
> +	int rc, val;
> +
> +	rc = sscanf(buf, "%i", &val);
> +	if ((rc != 1) || !(val == 1 || val == 0))
> +		return -EINVAL;
> +
> +	if (ocxl_config_set_reset_reload(pci_dev, val))
> +		return -ENODEV;
> +
> +	return count;
> +}
> +
>   static struct device_attribute afu_attrs[] = {
>   	__ATTR_RO(global_mmio_size),
>   	__ATTR_RO(pp_mmio_size),
>   	__ATTR_RO(afu_version),
>   	__ATTR_RO(contexts),
> +	__ATTR_RW(reload_on_reset),
>   };
>   
>   static ssize_t global_mmio_read(struct file *filp, struct kobject *kobj,
> diff --git a/include/misc/ocxl-config.h b/include/misc/ocxl-config.h
> index 3526fa996a22..ccfd3b463517 100644
> --- a/include/misc/ocxl-config.h
> +++ b/include/misc/ocxl-config.h
> @@ -41,5 +41,6 @@
>   #define   OCXL_DVSEC_VENDOR_CFG_VERS            0x0C
>   #define   OCXL_DVSEC_VENDOR_TLX_VERS            0x10
>   #define   OCXL_DVSEC_VENDOR_DLX_VERS            0x20
> +#define   OCXL_DVSEC_VENDOR_RESET_RELOAD        0x38
>   
>   #endif /* _OCXL_CONFIG_H_ */
> 


^ permalink raw reply

* [PATCH v4 2/2] powerpc/powernv: Re-enable imc trace-mode in kernel
From: Anju T Sudhakar @ 2020-03-13  5:52 UTC (permalink / raw)
  To: mpe; +Cc: harihare, maddy, linuxppc-dev, anju
In-Reply-To: <20200313055238.8656-1-anju@linux.vnet.ibm.com>

commit <249fad734a25> ""powerpc/perf: Disable trace_imc pmu"
disables IMC(In-Memory Collection) trace-mode in kernel, since frequent
mode switching between accumulation mode and trace mode via the spr LDBAR
in the hardware can trigger a checkstop(system crash).

Patch to re-enable imc-trace mode in kernel.

The previous patch(1/2) in this series will address the mode switching issue
by implementing a global lock, and will restrict the usage of
accumulation and trace-mode at a time.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/opal-imc.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index 968b9a4d1cd9..7824cc364bc4 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -268,14 +268,7 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
 			domain = IMC_DOMAIN_THREAD;
 			break;
 		case IMC_TYPE_TRACE:
-			/*
-			 * FIXME. Using trace_imc events to monitor application
-			 * or KVM thread performance can cause a checkstop
-			 * (system crash).
-			 * Disable it for now.
-			 */
-			pr_info_once("IMC: disabling trace_imc PMU\n");
-			domain = -1;
+			domain = IMC_DOMAIN_TRACE;
 			break;
 		default:
 			pr_warn("IMC Unknown Device type \n");
-- 
2.20.1


^ permalink raw reply related


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