[powerpc:next-test] BUILD SUCCESS dbbd7a68c8d47293bd8d027841b06dd5137e0fcc

2020-08-03 Thread kernel test robot
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git  
next-test
branch HEAD: dbbd7a68c8d47293bd8d027841b06dd5137e0fcc  powerpc: Fix P10 PVR 
revision in /proc/cpuinfo for SMT4 cores

elapsed time: 986m

configs tested: 99
configs skipped: 6

The following configs have been built successfully.
More configs may be tested in the coming days.

arm defconfig
arm64allyesconfig
arm64   defconfig
arm  allyesconfig
arm  allmodconfig
ia64generic_defconfig
m68k   bvme6000_defconfig
arm  pxa168_defconfig
powerpc  mpc885_ads_defconfig
m68k alldefconfig
mips  decstation_64_defconfig
armneponset_defconfig
arm  jornada720_defconfig
sh magicpanelr2_defconfig
arm  moxart_defconfig
shsh7763rdp_defconfig
sh microdev_defconfig
armmagician_defconfig
arm   aspeed_g4_defconfig
arc  axs101_defconfig
powerpcamigaone_defconfig
sh   sh7770_generic_defconfig
mipsbcm47xx_defconfig
h8300h8300h-sim_defconfig
nds32 allnoconfig
powerpc  alldefconfig
mips  bmips_stb_defconfig
arm pxa_defconfig
sh sh03_defconfig
mips db1xxx_defconfig
armtrizeps4_defconfig
parisc   alldefconfig
mips   ip32_defconfig
ia64 allmodconfig
ia64defconfig
ia64 allyesconfig
m68k allmodconfig
m68kdefconfig
m68k allyesconfig
nios2   defconfig
arc  allyesconfig
c6x  allyesconfig
nds32   defconfig
nios2allyesconfig
cskydefconfig
alpha   defconfig
alphaallyesconfig
xtensa   allyesconfig
h8300allyesconfig
arc defconfig
sh   allmodconfig
parisc  defconfig
s390 allyesconfig
parisc   allyesconfig
s390defconfig
i386 allyesconfig
sparcallyesconfig
sparc   defconfig
i386defconfig
mips allyesconfig
mips allmodconfig
powerpc  allyesconfig
powerpc  allmodconfig
powerpc   allnoconfig
powerpc defconfig
i386 randconfig-a004-20200803
i386 randconfig-a005-20200803
i386 randconfig-a001-20200803
i386 randconfig-a002-20200803
i386 randconfig-a003-20200803
i386 randconfig-a006-20200803
x86_64   randconfig-a006-20200804
x86_64   randconfig-a001-20200804
x86_64   randconfig-a004-20200804
x86_64   randconfig-a005-20200804
x86_64   randconfig-a002-20200804
x86_64   randconfig-a003-20200804
x86_64   randconfig-a013-20200803
x86_64   randconfig-a011-20200803
x86_64   randconfig-a012-20200803
x86_64   randconfig-a016-20200803
x86_64   randconfig-a015-20200803
x86_64   randconfig-a014-20200803
i386 randconfig-a011-20200803
i386 randconfig-a012-20200803
i386 randconfig-a015-20200803
i386 randconfig-a014-20200803
i386 randconfig-a013-20200803
i386 randconfig-a016-20200803
riscvallyesconfig
riscv allnoconfig
riscv   defconfig
riscvallmodconfig
x86_64   rhel
x86_64   allyesconfig
x86_64rhel-7.6-kselftests
x86_64  defconfig
x86_64   rhel-8.3
x86_64  kexec

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org

[powerpc:next] BUILD SUCCESS 2075ec9896c5aef01e837198381d04cfa6452317

2020-08-03 Thread kernel test robot
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git  
next
branch HEAD: 2075ec9896c5aef01e837198381d04cfa6452317  powerpc/powernv/sriov: 
Fix use of uninitialised variable

elapsed time: 986m

configs tested: 94
configs skipped: 5

The following configs have been built successfully.
More configs may be tested in the coming days.

arm defconfig
arm64allyesconfig
arm64   defconfig
arm  allyesconfig
arm  allmodconfig
ia64generic_defconfig
m68k   bvme6000_defconfig
arm  pxa168_defconfig
arm pxa_defconfig
archsdk_defconfig
armhisi_defconfig
arc   tb10x_defconfig
arc  axs103_defconfig
arm   aspeed_g4_defconfig
arc  axs101_defconfig
powerpcamigaone_defconfig
sh   sh7770_generic_defconfig
mipsbcm47xx_defconfig
h8300h8300h-sim_defconfig
powerpc  alldefconfig
mips  bmips_stb_defconfig
armmagician_defconfig
sh sh03_defconfig
mips db1xxx_defconfig
armtrizeps4_defconfig
parisc   alldefconfig
mips   ip32_defconfig
ia64 allmodconfig
ia64defconfig
ia64 allyesconfig
m68k allmodconfig
m68kdefconfig
m68k allyesconfig
nios2   defconfig
arc  allyesconfig
nds32 allnoconfig
c6x  allyesconfig
nds32   defconfig
nios2allyesconfig
cskydefconfig
alpha   defconfig
alphaallyesconfig
xtensa   allyesconfig
h8300allyesconfig
arc defconfig
sh   allmodconfig
parisc  defconfig
s390 allyesconfig
parisc   allyesconfig
s390defconfig
i386 allyesconfig
sparcallyesconfig
sparc   defconfig
i386defconfig
mips allyesconfig
mips allmodconfig
powerpc  allyesconfig
powerpc  allmodconfig
powerpc   allnoconfig
powerpc defconfig
i386 randconfig-a004-20200803
i386 randconfig-a005-20200803
i386 randconfig-a001-20200803
i386 randconfig-a002-20200803
i386 randconfig-a003-20200803
i386 randconfig-a006-20200803
x86_64   randconfig-a013-20200803
x86_64   randconfig-a011-20200803
x86_64   randconfig-a012-20200803
x86_64   randconfig-a016-20200803
x86_64   randconfig-a015-20200803
x86_64   randconfig-a014-20200803
i386 randconfig-a011-20200803
i386 randconfig-a012-20200803
i386 randconfig-a015-20200803
i386 randconfig-a014-20200803
i386 randconfig-a013-20200803
i386 randconfig-a016-20200803
x86_64   randconfig-a006-20200804
x86_64   randconfig-a001-20200804
x86_64   randconfig-a004-20200804
x86_64   randconfig-a005-20200804
x86_64   randconfig-a002-20200804
x86_64   randconfig-a003-20200804
riscvallyesconfig
riscv allnoconfig
riscv   defconfig
riscvallmodconfig
x86_64   rhel
x86_64   allyesconfig
x86_64rhel-7.6-kselftests
x86_64  defconfig
x86_64   rhel-8.3
x86_64  kexec

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


[powerpc:merge] BUILD SUCCESS a3dcfbc2456df1a2d416b7d0b627d9cededa1432

2020-08-03 Thread kernel test robot
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git  
merge
branch HEAD: a3dcfbc2456df1a2d416b7d0b627d9cededa1432  Automatic merge of 
'master', 'next' and 'fixes' (2020-08-03 22:40)

elapsed time: 987m

configs tested: 82
configs skipped: 7

The following configs have been built successfully.
More configs may be tested in the coming days.

arm defconfig
arm  allyesconfig
arm  allmodconfig
arm64allyesconfig
arm64   defconfig
ia64generic_defconfig
m68k   bvme6000_defconfig
arm  pxa168_defconfig
powerpc   maple_defconfig
mipsmalta_qemu_32r6_defconfig
arm64alldefconfig
armvt8500_v6_v7_defconfig
armlart_defconfig
mipsomega2p_defconfig
mips loongson1c_defconfig
xtensa   alldefconfig
nds32 allnoconfig
powerpc  alldefconfig
mips  bmips_stb_defconfig
armmagician_defconfig
parisc   alldefconfig
mips   ip32_defconfig
ia64 allmodconfig
ia64defconfig
ia64 allyesconfig
m68k allmodconfig
m68kdefconfig
m68k allyesconfig
nds32   defconfig
nios2allyesconfig
cskydefconfig
alpha   defconfig
alphaallyesconfig
xtensa   allyesconfig
h8300allyesconfig
arc defconfig
sh   allmodconfig
parisc  defconfig
s390 allyesconfig
parisc   allyesconfig
s390defconfig
i386 allyesconfig
sparcallyesconfig
sparc   defconfig
i386defconfig
nios2   defconfig
arc  allyesconfig
c6x  allyesconfig
mips allyesconfig
mips allmodconfig
powerpc  allyesconfig
powerpc  allmodconfig
powerpc   allnoconfig
powerpc defconfig
i386 randconfig-a004-20200803
i386 randconfig-a005-20200803
i386 randconfig-a001-20200803
i386 randconfig-a002-20200803
i386 randconfig-a003-20200803
i386 randconfig-a006-20200803
x86_64   randconfig-a013-20200803
x86_64   randconfig-a011-20200803
x86_64   randconfig-a012-20200803
x86_64   randconfig-a016-20200803
x86_64   randconfig-a015-20200803
x86_64   randconfig-a014-20200803
i386 randconfig-a011-20200803
i386 randconfig-a012-20200803
i386 randconfig-a015-20200803
i386 randconfig-a014-20200803
i386 randconfig-a013-20200803
i386 randconfig-a016-20200803
riscvallyesconfig
riscv allnoconfig
riscv   defconfig
riscvallmodconfig
x86_64   rhel
x86_64   allyesconfig
x86_64rhel-7.6-kselftests
x86_64  defconfig
x86_64   rhel-8.3
x86_64  kexec

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


[PATCH 2/2] powerpc/topology: Override cpu_smt_mask

2020-08-03 Thread Srikar Dronamraju
On Power9 a pair of cores can be presented by the firmware as a big-core
for backward compatibility reasons, with 4 threads per (small) core and 8
threads per big-core. cpu_smt_mask() should generally point to the cpu mask
of the (small)core.

In order to maintain userspace backward compatibility (with Power8 chips in
case of Power9) in enterprise Linux systems, the topology_sibling_cpumask
has to be set to big-core. Hence override the default cpu_smt_mask() to be
powerpc specific allowing for better scheduling behaviour on Power.

Cc: linuxppc-dev 
Cc: LKML 
Cc: Michael Ellerman 
Cc: Michael Neuling 
Cc: Gautham R Shenoy 
Cc: Ingo Molnar 
Cc: Peter Zijlstra 
Cc: Valentin Schneider 
Cc: Dietmar Eggemann 
Cc: Mel Gorman 
Cc: Vincent Guittot 
Cc: Vaidyanathan Srinivasan 
Signed-off-by: Srikar Dronamraju 
---
 arch/powerpc/include/asm/cputhreads.h |  1 -
 arch/powerpc/include/asm/smp.h| 13 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cputhreads.h 
b/arch/powerpc/include/asm/cputhreads.h
index deb99fd6e060..98c8bd155bf9 100644
--- a/arch/powerpc/include/asm/cputhreads.h
+++ b/arch/powerpc/include/asm/cputhreads.h
@@ -23,7 +23,6 @@
 extern int threads_per_core;
 extern int threads_per_subcore;
 extern int threads_shift;
-extern bool has_big_cores;
 extern cpumask_t threads_core_mask;
 #else
 #define threads_per_core   1
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 9cd0765633c5..d4bc28accb28 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -131,6 +131,19 @@ static inline struct cpumask *cpu_smallcore_mask(int cpu)
 
 extern int cpu_to_core_id(int cpu);
 
+extern bool has_big_cores;
+
+#define cpu_smt_mask cpu_smt_mask
+#ifdef CONFIG_SCHED_SMT
+static inline const struct cpumask *cpu_smt_mask(int cpu)
+{
+   if (has_big_cores)
+   return per_cpu(cpu_smallcore_map, cpu);
+
+   return per_cpu(cpu_sibling_map, cpu);
+}
+#endif /* CONFIG_SCHED_SMT */
+
 /* Since OpenPIC has only 4 IPIs, we use slightly different message numbers.
  *
  * Make sure this matches openpic_request_IPIs in open_pic.c, or what shows up
-- 
2.18.2



[PATCH 1/2] sched/topology: Allow archs to override cpu_smt_mask

2020-08-03 Thread Srikar Dronamraju
cpu_smt_mask tracks topology_sibling_cpumask. This would be good for
most architectures. One of the users of cpu_smt_mask(), would be to
identify idle-cores. On Power9, a pair of cores can be presented by the
firmware as a big-core for backward compatibility reasons.

In order to maintain userspace backward compatibility with previous
versions of processor, (since Power8 had SMT8 cores), Power9 onwards there
is option to the firmware to advertise a pair of SMT4 cores as a fused
cores (referred to as the big_core mode in the Linux Kernel). On Power9
this pair shares the L2 cache as well. However, from the scheduler's point
of view, a core should be determined by SMT4. The load-balancer already
does this. Hence allow PowerPc architecture to override the default
cpu_smt_mask() to point to the SMT4 cores in a big_core mode.

Cc: linuxppc-dev 
Cc: LKML 
Cc: Michael Ellerman 
Cc: Michael Neuling 
Cc: Gautham R Shenoy 
Cc: Ingo Molnar 
Cc: Peter Zijlstra 
Cc: Valentin Schneider 
Cc: Dietmar Eggemann 
Cc: Mel Gorman 
Cc: Vincent Guittot 
Cc: Vaidyanathan Srinivasan 
Signed-off-by: Srikar Dronamraju 
---
 include/linux/topology.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/topology.h b/include/linux/topology.h
index 608fa4aadf0e..ad03df1cc266 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -198,7 +198,7 @@ static inline int cpu_to_mem(int cpu)
 #define topology_die_cpumask(cpu)  cpumask_of(cpu)
 #endif

-#ifdef CONFIG_SCHED_SMT
+#if defined(CONFIG_SCHED_SMT) && !defined(cpu_smt_mask)
 static inline const struct cpumask *cpu_smt_mask(int cpu)
 {
return topology_sibling_cpumask(cpu);
-- 
2.18.2



[PATCH] powerpc/pseries/hotplug-cpu: increase wait time for vCPU death

2020-08-03 Thread Michael Roth
For a power9 KVM guest with XIVE enabled, running a test loop
where we hotplug 384 vcpus and then unplug them, the following traces
can be seen (generally within a few loops) either from the unplugged
vcpu:

  [ 1767.353447] cpu 65 (hwid 65) Ready to die...
  [ 1767.952096] Querying DEAD? cpu 66 (66) shows 2
  [ 1767.952311] list_del corruption. next->prev should be c00a02470208, 
but was c00a02470048
  [ 1767.952322] [ cut here ]
  [ 1767.952323] kernel BUG at lib/list_debug.c:56!
  [ 1767.952325] Oops: Exception in kernel mode, sig: 5 [#1]
  [ 1767.952326] LE SMP NR_CPUS=2048 NUMA pSeries
  [ 1767.952328] Modules linked in: fuse nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 
nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct 
nf_tables_set nft_chain_nat_ipv6 nf_nat_ipv6 nft_chain_route_ipv6 
nft_chain_nat_ipv4 nf_nat_ipv4 nf_nat nf_conntrack nf_defrag_ipv6 
nf_defrag_ipv4 nft_chain_route_ipv4 ip6_tables nft_compat ip_set nf_tables 
nfnetlink uio_pdrv_genirq ip_tables xfs libcrc32c sd_mod sg xts vmx_crypto 
virtio_net net_failover failover virtio_scsi dm_multipath dm_mirror 
dm_region_hash dm_log dm_mod be2iscsi bnx2i cnic uio cxgb4i cxgb4 libcxgbi 
libcxgb qla4xxx iscsi_boot_sysfs iscsi_tcp libiscsi_tcp libiscsi 
scsi_transport_iscsi
  [ 1767.952352] CPU: 66 PID: 0 Comm: swapper/66 Kdump: loaded Not tainted 
4.18.0-221.el8.ppc64le #1
  [ 1767.952354] NIP:  c07ab50c LR: c07ab508 CTR: 
03ac
  [ 1767.952355] REGS: c009e5a17840 TRAP: 0700   Not tainted  
(4.18.0-221.el8.ppc64le)
  [ 1767.952355] MSR:  8282b033   CR: 
28000842  XER: 2004
  [ 1767.952360] CFAR: c01ffe64 IRQMASK: 1
  [ 1767.952360] GPR00: c07ab508 c009e5a17ac0 c1ac0700 
0054
  [ 1767.952360] GPR04: c009f056cf90 c009f05f5628 c009ed40d654 
c1c90700
  [ 1767.952360] GPR08: 0007 c009f0573980 0009ef2b 
7562202c38303230
  [ 1767.952360] GPR12:  c007fff6ce80 c00a02470208 

  [ 1767.952360] GPR16: 0001 c009f05fbb00 0800 
c009ff3d4980
  [ 1767.952360] GPR20: c009f05fbb10 5deadbeef100 5deadbeef200 
00187961
  [ 1767.952360] GPR24: c009e5a17b78  0001 

  [ 1767.952360] GPR28: c00a02470200 c009f05fbb10 c009f05fbb10 

  [ 1767.952375] NIP [c07ab50c] __list_del_entry_valid+0xac/0x100
  [ 1767.952376] LR [c07ab508] __list_del_entry_valid+0xa8/0x100
  [ 1767.952377] Call Trace:
  [ 1767.952378] [c009e5a17ac0] [c07ab508] 
__list_del_entry_valid+0xa8/0x100 (unreliable)
  [ 1767.952381] [c009e5a17b20] [c0476e18] 
free_pcppages_bulk+0x1f8/0x940
  [ 1767.952383] [c009e5a17c20] [c047a9a0] 
free_unref_page+0xd0/0x100
  [ 1767.952386] [c009e5a17c50] [c00bc2a8] 
xive_spapr_cleanup_queue+0x148/0x1b0
  [ 1767.952388] [c009e5a17cf0] [c00b96dc] 
xive_teardown_cpu+0x1bc/0x240
  [ 1767.952391] [c009e5a17d30] [c010bf28] 
pseries_mach_cpu_die+0x78/0x2f0
  [ 1767.952393] [c009e5a17de0] [c005c8d8] cpu_die+0x48/0x70
  [ 1767.952394] [c009e5a17e00] [c0021cf0] 
arch_cpu_idle_dead+0x20/0x40
  [ 1767.952397] [c009e5a17e20] [c01b4294] do_idle+0x2f4/0x4c0
  [ 1767.952399] [c009e5a17ea0] [c01b46a8] 
cpu_startup_entry+0x38/0x40
  [ 1767.952400] [c009e5a17ed0] [c005c43c] 
start_secondary+0x7bc/0x8f0
  [ 1767.952403] [c009e5a17f90] [c000ac70] 
start_secondary_prolog+0x10/0x14

or on the worker thread handling the unplug:

  [ 1538.253044] pseries-hotplug-cpu: Attempting to remove CPU , drc 
index: 113a
  [ 1538.360259] Querying DEAD? cpu 314 (314) shows 2
  [ 1538.360736] BUG: Bad page state in process kworker/u768:3  pfn:95de1
  [ 1538.360746] cpu 314 (hwid 314) Ready to die...
  [ 1538.360784] page:c00a02577840 refcount:0 mapcount:-128 
mapping: index:0x0
  [ 1538.361881] flags: 0x5c()
  [ 1538.361908] raw: 005c 5deadbeef100 5deadbeef200 

  [ 1538.361955] raw:   ff7f 

  [ 1538.362002] page dumped because: nonzero mapcount
  [ 1538.362033] Modules linked in: kvm xt_CHECKSUM ipt_MASQUERADE xt_conntrack 
ipt_REJECT nft_counter nf_nat_tftp nft_objref nf_conntrack_tftp tun bridge stp 
llc nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet 
nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nf_tables_set nft_chain_nat 
nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip6_tables nft_compat ip_set 
nf_tables nfnetlink sunrpc xts vmx_crypto ip_tables xfs libcrc32c sd_mod sg 
virtio_net net_failover virtio_scsi failover dm_mirror dm_region_hash dm_log 
dm_mod
  [ 1538.362613] CPU: 0 PID: 548 Comm: kworker/u768:3 Kdump: loaded Not tainted 

Re: [PATCH 5/6] powerpc/powernv/pci: Drop unused parent variable

2020-08-03 Thread Joel Stanley
On Tue, 4 Aug 2020 at 01:06, Oliver O'Halloran  wrote:
>
> The "parent" variable in pnv_pci_ioda_configure_pe() isn't used for
> anything anymore and can be dropped.
>
> Signed-off-by: Oliver O'Halloran 

Reviewed-by: Joel Stanley 


Re: [PATCH 4/6] powerpc/powernv: Fix spurious kerneldoc warnings in opal-prd.c

2020-08-03 Thread Joel Stanley
On Tue, 4 Aug 2020 at 01:03, Oliver O'Halloran  wrote:
>
> Comments opening with /** are parsed by kerneldoc and this causes the
> following warning to be printed:
>
> arch/powerpc/platforms/powernv/opal-prd.c:31: warning: cannot 
> understand
> function prototype: 'struct opal_prd_msg_queue_item '
>
> opal_prd_mesg_queue_item is an internal data structure so there's no real
> need for it to be documented at all. Fix up the comment to squash the
> warning.
>
> Signed-off-by: Oliver O'Halloran 

Reviewed-by: Joel Stanley 


Re: [PATCH 3/6] powerpc/powernv: Staticify functions without prototypes

2020-08-03 Thread Joel Stanley
On Tue, 4 Aug 2020 at 01:01, Oliver O'Halloran  wrote:
>
> There's a few scattered in the powernv platform.
>
> Signed-off-by: Oliver O'Halloran 

> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -38,7 +38,7 @@
>
>  static int eeh_event_irq = -EINVAL;
>
> -void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
> +static void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
>  {
> dev_dbg(>dev, "EEH: Setting up device\n");
> eeh_probe_device(pdev);

This one could even be deleted as eeh_probe_device has it's own dev_dbg.

Reviewed-by: Joel Stanley 


Re: [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

2020-08-03 Thread Joel Stanley
On Tue, 4 Aug 2020 at 00:57, Oliver O'Halloran  wrote:
>
> When building with W=1 we get the following warning:
>
>  arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
>  arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
> empty body in an ‘if’ statement [-Werror=empty-body]
>276 |  cpu, srr1);
>|^
>  cc1: all warnings being treated as errors
>
> The full context is this block:
>
>  if (srr1 && !generic_check_cpu_restart(cpu))
> DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
> cpu, srr1);
>
> When building with DEBUG undefined DBG() expands to nothing and GCC emits
> the warning due to the lack of braces around an empty statement.
>
> Signed-off-by: Oliver O'Halloran 
> ---
> We could add the braces too. That might even be better since it's a multi-line
> if block even though it's only a single statement.

Or you could put it all on one line, now that our 120 line overlords
have taken over.

Reviewed-by: Joel Stanley 

Messy:

$ git grep "define DBG(" arch/powerpc/ |grep -v print
arch/powerpc/kernel/crash_dump.c:#define DBG(fmt...)
arch/powerpc/kernel/iommu.c:#define DBG(...)
arch/powerpc/kernel/legacy_serial.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/kernel/prom.c:#define DBG(fmt...)
arch/powerpc/kernel/setup-common.c:#define DBG(fmt...)
arch/powerpc/kernel/setup_32.c:#define DBG(fmt...)
arch/powerpc/kernel/smp.c:#define DBG(fmt...)
arch/powerpc/kernel/vdso.c:#define DBG(fmt...)
arch/powerpc/kvm/book3s_hv_rm_xive.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/mm/book3s64/hash_utils.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc832x_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc832x_rdb.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc836x_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/85xx/mpc85xx_ds.c:#define DBG(fmt, args...)
arch/powerpc/platforms/85xx/mpc85xx_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/85xx/mpc85xx_rdb.c:#define DBG(fmt, args...)
arch/powerpc/platforms/86xx/mpc86xx_hpcn.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/platforms/cell/setup.c:#define DBG(fmt...)
arch/powerpc/platforms/cell/smp.c:#define DBG(fmt...)
arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c:#define DBG(fmt...)
do { } while(0)
arch/powerpc/platforms/maple/pci.c:#define DBG(x...)
arch/powerpc/platforms/maple/setup.c:#define DBG(fmt...)
arch/powerpc/platforms/maple/time.c:#define DBG(x...)
arch/powerpc/platforms/powermac/bootx_init.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/platforms/powermac/feature.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...) do {\
arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...)
arch/powerpc/platforms/powermac/nvram.c:#define DBG(x...)
arch/powerpc/platforms/powermac/pci.c:#define DBG(x...)
arch/powerpc/platforms/powermac/pfunc_base.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/pfunc_core.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/smp.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/time.c:#define DBG(x...)
arch/powerpc/platforms/powernv/smp.c:#define DBG(fmt...)
arch/powerpc/sysdev/dart_iommu.c:#define DBG(...)
arch/powerpc/sysdev/ge/ge_pic.c:#define DBG(fmt...) do { } while (0)
arch/powerpc/sysdev/mpic.c:#define DBG(fmt...)
arch/powerpc/sysdev/tsi108_dev.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/sysdev/tsi108_pci.c:#define DBG(x...)


> ---
>  arch/powerpc/platforms/powernv/smp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/smp.c 
> b/arch/powerpc/platforms/powernv/smp.c
> index b2ba3e95bda7..bbf361f23ae8 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -43,7 +43,7 @@
>  #include 
>  #define DBG(fmt...) udbg_printf(fmt)
>  #else
> -#define DBG(fmt...)
> +#define DBG(fmt...) do { } while (0)
>  #endif
>
>  static void pnv_smp_setup_cpu(int cpu)
> --
> 2.26.2
>


Re: [merge] Build failure selftest/powerpc/mm/pkey_exec_prot

2020-08-03 Thread Michael Ellerman
Sandipan Das  writes:
> On 03/08/20 4:32 pm, Michael Ellerman wrote:
>> Sachin Sant  writes:
 On 02-Aug-2020, at 10:58 PM, Sandipan Das  wrote:
 On 02/08/20 4:45 pm, Sachin Sant wrote:
> pkey_exec_prot test from linuxppc merge branch (3f68564f1f5a) fails to
> build due to following error:
>
> gcc -std=gnu99 -O2 -Wall -Werror 
> -DGIT_VERSION='"v5.8-rc7-1276-g3f68564f1f5a"' 
> -I/home/sachin/linux/tools/testing/selftests/powerpc/include  -m64
> pkey_exec_prot.c 
> /home/sachin/linux/tools/testing/selftests/kselftest_harness.h 
> /home/sachin/linux/tools/testing/selftests/kselftest.h ../harness.c 
> ../utils.c  -o 
> /home/sachin/linux/tools/testing/selftests/powerpc/mm/pkey_exec_prot
> In file included from pkey_exec_prot.c:18:
> /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:34: 
> error: "SYS_pkey_mprotect" redefined [-Werror]
> #define SYS_pkey_mprotect 386
>
> In file included from /usr/include/sys/syscall.h:31,
> from 
> /home/sachin/linux/tools/testing/selftests/powerpc/include/utils.h:47,
> from 
> /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:12,
> from pkey_exec_prot.c:18:
> /usr/include/bits/syscall.h:1583: note: this is the location of the 
> previous definition
> # define SYS_pkey_mprotect __NR_pkey_mprotect
>
> commit 128d3d021007 introduced this error.
> selftests/powerpc: Move pkey helpers to headers
>
> Possibly the # defines for sys calls can be retained in pkey_exec_prot.c 
> or
>

 I am unable to reproduce this on the latest merge branch (HEAD at 
 f59195f7faa4).
 I don't see any redefinitions in pkey_exec_prot.c either.

>>>
>>> I can still see this problem on latest merge branch.
>>> I have following gcc version
>>>
>>> gcc version 8.3.1 20191121
>> 
>> What libc version? Or just the distro & version?
>
> Sachin observed this on RHEL 8.2 with glibc-2.28.
> I couldn't reproduce it on Ubuntu 20.04 and Fedora 32 and both these distros
> are using glibc-2.31.

OK odd. Usually it's newer glibc that hits this problem.

I guess on RHEL 8.2 we're getting the asm-generic version? But that
would be quite wrong if that's what's happening.

cheers


[PATCH 6/6] powerpc/nx: Don't pack struct coprocessor_request_block

2020-08-03 Thread Oliver O'Halloran
Building with W=1 results in the following warning:

In file included from arch/powerpc/platforms/powernv/vas-fault.c:16:
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
  159 | } __packed;
  | ^
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
cc1: all warnings being treated as errors

This happens because coprocessor_request_block includes several
sub-structures with an alignment specified using the __aligned(XX)
attribute. The problem comes from coprocessor_request_block having the
__packed attribute. Packing the structure causes the preferred alignment of
the nested structures to be ignored and we get the warnings as a result.

This isn't a problem in practice since the struct is defined with explicit
padding in the form of reserved fields, but we'd like to get rid of the
spurious warnings. The simplest solution is to remove the packed attribute
and use a BUILD_BUG_ON() to ensure the struct is the correct (expected by
HW) size compile time.

Also add a __aligned(128) to the request block structure since Book4 for P8
suggests the HW requires it to be aligned to a 128 byte boundary. There's a
similar requirement for P9 since the COPY and PASTE instructions used to
invoke VAS/NX accelerators operates on a cache line boundary.

Cc: Dan Streetman 
Cc: Haren Myneni 
Signed-off-by: Oliver O'Halloran 
---
 arch/powerpc/include/asm/icswx.h | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/include/asm/icswx.h
index b0c70a35fd0e..f6599ccb3012 100644
--- a/arch/powerpc/include/asm/icswx.h
+++ b/arch/powerpc/include/asm/icswx.h
@@ -156,8 +156,7 @@ struct coprocessor_request_block {
u8 reserved[32];
 
struct coprocessor_status_block csb;
-} __packed;
-
+} __aligned(128);
 
 /* RFC02167 Initiate Coprocessor Instructions document
  * Chapter 8.2.1.1.1 RS
@@ -188,6 +187,9 @@ static inline int icswx(__be32 ccw, struct 
coprocessor_request_block *crb)
__be64 ccw_reg = ccw;
u32 cr;
 
+   /* NB: the same structures are used by VAS-NX */
+   BUILD_BUG_ON(sizeof(*crb) != 128);
+
__asm__ __volatile__(
PPC_ICSWX(%1,0,%2) "\n"
"mfcr %0\n"
-- 
2.26.2



[PATCH 5/6] powerpc/powernv/pci: Drop unused parent variable

2020-08-03 Thread Oliver O'Halloran
The "parent" variable in pnv_pci_ioda_configure_pe() isn't used for
anything anymore and can be dropped.

Signed-off-by: Oliver O'Halloran 
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c 
b/arch/powerpc/platforms/powernv/pci-ioda.c
index c9c25fb0783c..6d48155bd885 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -894,7 +894,6 @@ int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct 
pnv_ioda_pe *pe)
 
 int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
 {
-   struct pci_dev *parent;
uint8_t bcomp, dcomp, fcomp;
long rc, rid_end, rid;
 
@@ -904,7 +903,6 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct 
pnv_ioda_pe *pe)
 
dcomp = OPAL_IGNORE_RID_DEVICE_NUMBER;
fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER;
-   parent = pe->pbus->self;
if (pe->flags & PNV_IODA_PE_BUS_ALL)
count = resource_size(>pbus->busn_res);
else
@@ -925,12 +923,6 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct 
pnv_ioda_pe *pe)
}
rid_end = pe->rid + (count << 8);
} else {
-#ifdef CONFIG_PCI_IOV
-   if (pe->flags & PNV_IODA_PE_VF)
-   parent = pe->parent_dev;
-   else
-#endif /* CONFIG_PCI_IOV */
-   parent = pe->pdev->bus->self;
bcomp = OpalPciBusAll;
dcomp = OPAL_COMPARE_RID_DEVICE_NUMBER;
fcomp = OPAL_COMPARE_RID_FUNCTION_NUMBER;
-- 
2.26.2



[PATCH 4/6] powerpc/powernv: Fix spurious kerneldoc warnings in opal-prd.c

2020-08-03 Thread Oliver O'Halloran
Comments opening with /** are parsed by kerneldoc and this causes the
following warning to be printed:

arch/powerpc/platforms/powernv/opal-prd.c:31: warning: cannot understand
function prototype: 'struct opal_prd_msg_queue_item '

opal_prd_mesg_queue_item is an internal data structure so there's no real
need for it to be documented at all. Fix up the comment to squash the
warning.

Signed-off-by: Oliver O'Halloran 
---
 arch/powerpc/platforms/powernv/opal-prd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/opal-prd.c 
b/arch/powerpc/platforms/powernv/opal-prd.c
index 45f4223a790f..deddaebf8c14 100644
--- a/arch/powerpc/platforms/powernv/opal-prd.c
+++ b/arch/powerpc/platforms/powernv/opal-prd.c
@@ -24,7 +24,7 @@
 #include 
 
 
-/**
+/*
  * The msg member must be at the end of the struct, as it's followed by the
  * message data.
  */
-- 
2.26.2



[PATCH 3/6] powerpc/powernv: Staticify functions without prototypes

2020-08-03 Thread Oliver O'Halloran
There's a few scattered in the powernv platform.

Signed-off-by: Oliver O'Halloran 
---
 arch/powerpc/platforms/powernv/eeh-powernv.c | 4 ++--
 arch/powerpc/platforms/powernv/rng.c | 2 +-
 arch/powerpc/platforms/powernv/vas-window.c  | 9 -
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c 
b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 9af8c3b98853..663bd69ac51b 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -38,7 +38,7 @@
 
 static int eeh_event_irq = -EINVAL;
 
-void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
+static void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 {
dev_dbg(>dev, "EEH: Setting up device\n");
eeh_probe_device(pdev);
@@ -190,7 +190,7 @@ PNV_EEH_DBGFS_ENTRY(inbB, 0xE10);
 
 #endif /* CONFIG_DEBUG_FS */
 
-void pnv_eeh_enable_phbs(void)
+static void pnv_eeh_enable_phbs(void)
 {
struct pci_controller *hose;
struct pnv_phb *phb;
diff --git a/arch/powerpc/platforms/powernv/rng.c 
b/arch/powerpc/platforms/powernv/rng.c
index 8035caf6e297..72c25295c1c2 100644
--- a/arch/powerpc/platforms/powernv/rng.c
+++ b/arch/powerpc/platforms/powernv/rng.c
@@ -65,7 +65,7 @@ int powernv_get_random_real_mode(unsigned long *v)
return 1;
 }
 
-int powernv_get_random_darn(unsigned long *v)
+static int powernv_get_random_darn(unsigned long *v)
 {
unsigned long val;
 
diff --git a/arch/powerpc/platforms/powernv/vas-window.c 
b/arch/powerpc/platforms/powernv/vas-window.c
index 6434f9cb5aed..5f5fe63a3d1c 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -186,7 +186,7 @@ static void unmap_winctx_mmio_bars(struct vas_window 
*window)
  * OS/User Window Context (UWC) MMIO Base Address Region for the given window.
  * Map these bus addresses and save the mapped kernel addresses in @window.
  */
-int map_winctx_mmio_bars(struct vas_window *window)
+static int map_winctx_mmio_bars(struct vas_window *window)
 {
int len;
u64 start;
@@ -214,7 +214,7 @@ int map_winctx_mmio_bars(struct vas_window *window)
  *  registers are not sequential. And, we can only write to offsets
  *  with valid registers.
  */
-void reset_window_regs(struct vas_window *window)
+static void reset_window_regs(struct vas_window *window)
 {
write_hvwc_reg(window, VREG(LPID), 0ULL);
write_hvwc_reg(window, VREG(PID), 0ULL);
@@ -357,7 +357,8 @@ static void init_rsvd_tx_buf_count(struct vas_window *txwin,
  * as a one-time task? That could work for NX but what about other
  * receivers?  Let the receivers tell us the rx-fifo buffers for now.
  */
-int init_winctx_regs(struct vas_window *window, struct vas_winctx *winctx)
+static void init_winctx_regs(struct vas_window *window,
+struct vas_winctx *winctx)
 {
u64 val;
int fifo_size;
@@ -499,8 +500,6 @@ int init_winctx_regs(struct vas_window *window, struct 
vas_winctx *winctx)
val = SET_FIELD(VAS_WINCTL_NX_WIN, val, winctx->nx_win);
val = SET_FIELD(VAS_WINCTL_OPEN, val, 1);
write_hvwc_reg(window, VREG(WINCTL), val);
-
-   return 0;
 }
 
 static void vas_release_window_id(struct ida *ida, int winid)
-- 
2.26.2



[PATCH 2/6] powerpc/powernv: Include asm/powernv.h from the local powernv.h

2020-08-03 Thread Oliver O'Halloran
The asm/powernv.h header provides prototypes for functions which need to be
called by non-powernv platform code. Also include it in the powernv.h
that's local to the platform directory to squash some warnings about
non-static functions missing prototypes.

Also include powernv.h since from opal-memcons.c since it has the
prototypes for the memcons wrangling functions which are used for the opal
and ultravisor msglog.

Signed-off-by: Oliver O'Halloran 
---
 arch/powerpc/platforms/powernv/opal-msglog.c | 2 ++
 arch/powerpc/platforms/powernv/powernv.h | 7 +++
 2 files changed, 9 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/opal-msglog.c 
b/arch/powerpc/platforms/powernv/opal-msglog.c
index d26da19a611f..d3b6e135c18b 100644
--- a/arch/powerpc/platforms/powernv/opal-msglog.c
+++ b/arch/powerpc/platforms/powernv/opal-msglog.c
@@ -12,6 +12,8 @@
 #include 
 #include 
 
+#include "powernv.h"
+
 /* OPAL in-memory console. Defined in OPAL source at core/console.c */
 struct memcons {
__be64 magic;
diff --git a/arch/powerpc/platforms/powernv/powernv.h 
b/arch/powerpc/platforms/powernv/powernv.h
index 1aa51c4fa904..11df4e16a1cc 100644
--- a/arch/powerpc/platforms/powernv/powernv.h
+++ b/arch/powerpc/platforms/powernv/powernv.h
@@ -2,6 +2,13 @@
 #ifndef _POWERNV_H
 #define _POWERNV_H
 
+/*
+ * There's various hacks scattered throughout the generic powerpc arch code
+ * that needs to call into powernv platform stuff. The prototypes for those
+ * functions are in asm/powernv.h
+ */
+#include 
+
 #ifdef CONFIG_SMP
 extern void pnv_smp_init(void);
 #else
-- 
2.26.2



[PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

2020-08-03 Thread Oliver O'Halloran
When building with W=1 we get the following warning:

 arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
 arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
empty body in an ‘if’ statement [-Werror=empty-body]
   276 |  cpu, srr1);
   |^
 cc1: all warnings being treated as errors

The full context is this block:

 if (srr1 && !generic_check_cpu_restart(cpu))
DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
cpu, srr1);

When building with DEBUG undefined DBG() expands to nothing and GCC emits
the warning due to the lack of braces around an empty statement.

Signed-off-by: Oliver O'Halloran 
---
We could add the braces too. That might even be better since it's a multi-line
if block even though it's only a single statement.
---
 arch/powerpc/platforms/powernv/smp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/smp.c 
b/arch/powerpc/platforms/powernv/smp.c
index b2ba3e95bda7..bbf361f23ae8 100644
--- a/arch/powerpc/platforms/powernv/smp.c
+++ b/arch/powerpc/platforms/powernv/smp.c
@@ -43,7 +43,7 @@
 #include 
 #define DBG(fmt...) udbg_printf(fmt)
 #else
-#define DBG(fmt...)
+#define DBG(fmt...) do { } while (0)
 #endif
 
 static void pnv_smp_setup_cpu(int cpu)
-- 
2.26.2



Clean up W=1 for the powernv platform

2020-08-03 Thread Oliver O'Halloran
Fixes the (mostly) suprious errors we get when building powernv with
W=1. More work is required to build all of powerpc with W=1, let alone
W=2.




Re: powerpc: build failures in Linus' tree

2020-08-03 Thread Michael Ellerman
Stephen Rothwell  writes:
> On Mon, 03 Aug 2020 21:18:00 +1000 Michael Ellerman  
> wrote:
>>
>> If we just move the include of asm/paca.h below asm-generic/percpu.h
>> then it avoids the bad circular dependency and we still have paca.h
>> included from percpu.h as before.
>> 
>> eg:
>> 
>> diff --git a/arch/powerpc/include/asm/percpu.h 
>> b/arch/powerpc/include/asm/percpu.h
>> index dce863a7635c..8e5b7d0b851c 100644
>> --- a/arch/powerpc/include/asm/percpu.h
>> +++ b/arch/powerpc/include/asm/percpu.h
>> @@ -10,8 +10,6 @@
>>  
>>  #ifdef CONFIG_SMP
>>  
>> -#include 
>> -
>>  #define __my_cpu_offset local_paca->data_offset
>>  
>>  #endif /* CONFIG_SMP */
>> @@ -19,4 +17,6 @@
>>  
>>  #include 
>>  
>> +#include 
>> +
>>  #endif /* _ASM_POWERPC_PERCPU_H_ */
>> 
>> 
>> So I think I'm inclined to merge that as a minimal fix that's easy to
>> backport.
>> 
>> cheers
>
> Looks ok, except does it matter that the include used to be only done
> if __powerpc64__ and CONFIG_SMP are defined?

Basically all of paca.h is inside #ifdef CONFIG_PPC64.

SMP "shouldn't matter", but I tested a SMP=n build and it's clean, so I
think it's good. Of course there's really no guarantees with these
header tangles.

cheers


Re: [PATCH v2 15/16] powerpc/powernv/sriov: Make single PE mode a per-BAR setting

2020-08-03 Thread Segher Boessenkool
On Mon, Aug 03, 2020 at 03:57:11PM +1000, Michael Ellerman wrote:
> > I would assume the function should still be generated since those checks
> > are relevant, just the return value is bogus.
> 
> Yeah, just sometimes missing warnings boil down to the compiler eliding
> whole sections of code, if it can convince itself they're unreachable.

Including when the compiler considers they must be unreachable because
they would perform undefined behaviour, like, act on uninitialised
values.  Such code is removed by cddce ("control dependence dead code
elimination", enabled by -ftree-dce at -O2 or above).

> AFAICS there's nothing weird going on here that should confuse GCC, it's
> about as straight forward as it gets.

Yes.  Please open a bug?

> Actually I can reproduce it with:
> 
> $ cat > test.c < int foo(void *p)
> {
> unsigned align;
> 
> if (!p)
> return align;
> 
> return 0;
> }
> EOF
> 
> $ gcc -Wall -Wno-maybe-uninitialized -c test.c
> $
> 
> No warning.

The whole if() is deleted pretty early.

===
static int foo(void *p)
{
unsigned align;

if (!p)
return align;

return 42;
}
int bork(void) { return foo(0); }
===

doesn't warn either, although that always uses the uninitialised var
(actually, that code is *removed*, and it always does that "return 42").

> But I guess that's behaving as documented. The compiler can't prove that
> foo() will be called with p == NULL, so it doesn't warn.

-Wmaybe-uninitialized doesn't warn for this either, btw.


Segher


Re: [PATCHv4 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents

2020-08-03 Thread Laurent Dufour

Le 30/07/2020 à 15:33, Pingfan Liu a écrit :

A bug is observed on pseries by taking the following steps on rhel:
-1. drmgr -c mem -r -q 5
-2. echo c > /proc/sysrq-trigger

And then, the failure looks like:
kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/
kdump: saving vmcore-dmesg.txt
kdump: saving vmcore-dmesg.txt complete
kdump: saving vmcore
  Checking for memory holes : [  0.0 %] /   
Checking for memory holes : [100.0 %] | 
  Excluding unnecessary pages   : [100.0 %] \   
Copying data  : [  0.3 %] - 
 eta: 38s[   44.337636] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 
access=0x8004 current=makedumpfile
[   44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 
psize 2 pte=0xc0005504
[   44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 
access=0x8004 current=makedumpfile
[   44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 
psize 2 pte=0xc0005504
[   44.337708] makedumpfile[469]: unhandled signal 7 at 7fffba40 nip 
7fffbbc4d7fc lr 00011356ca3c code 2
[   44.338548] Core dump to |/bin/false pipe failed
/lib/kdump-lib-initramfs.sh: line 98:   469 Bus error   
$CORE_COLLECTOR /proc/vmcore 
$_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete
kdump: saving vmcore failed

* Root cause *
   After analyzing, it turns out that in the current implementation,
when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as
the code __remove_memory() comes before drmem_update_dt().
So in kdump kernel, when read_from_oldmem() resorts to
pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to
non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, as it
can be observed "Bus error"

 From a viewpoint of listener and publisher, the publisher notifies the
listener before data is ready.  This introduces a problem where udev
launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before
updating. And in capture kernel, makedumpfile will access the memory based
on the stale dt info, and hit a SIGBUS error due to an un-existed lmb.

* Fix *
This bug is introduced by commit 063b8b1251fd
("powerpc/pseries/memory-hotplug: Only update DT once per memory DLPAR
request"), which tried to combine all the dt updating into one.

To fix this issue, meanwhile not to introduce a quadratic runtime
complexity by the model:
   dlpar_memory_add_by_count
 for_each_drmem_lmb <--
   dlpar_add_lmb
 drmem_update_dt(_v1|_v2)
   for_each_drmem_lmb   <--
The dt should still be only updated once, and just before the last memory
online/offline event is ejected to user space. Achieve this by tracing the
num of lmb added or removed.

Signed-off-by: Pingfan Liu 
Cc: Michael Ellerman 
Cc: Hari Bathini 
Cc: Nathan Lynch 
Cc: Nathan Fontenot 
Cc: ke...@lists.infradead.org
To: linuxppc-dev@lists.ozlabs.org
---
v3 -> v4: resolve a quadratic runtime complexity issue.
   This series is applied on next-test branch
  arch/powerpc/platforms/pseries/hotplug-memory.c | 88 ++---
  1 file changed, 66 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c 
b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 1a3ac3b..e07d5b1 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -350,13 +350,13 @@ static bool lmb_is_removable(struct drmem_lmb *lmb)
return true;
  }
  
-static int dlpar_add_lmb(struct drmem_lmb *);

+static int dlpar_add_lmb(struct drmem_lmb *lmb, bool dt_update);
  
-static int dlpar_remove_lmb(struct drmem_lmb *lmb)

+static int dlpar_remove_lmb(struct drmem_lmb *lmb, bool dt_update)
  {
unsigned long block_sz;
phys_addr_t base_addr;
-   int rc, nid;
+   int rc, ret, nid;
  
  	if (!lmb_is_removable(lmb))

return -EINVAL;
@@ -372,6 +372,11 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+   if (dt_update) {
+   ret = drmem_update_dt();
+   if (ret)
+   pr_warn("%s fail to update dt, but continue\n", 
__func__);
+   }
  
  	__remove_memory(nid, base_addr, block_sz);
  
@@ -387,6 +392,7 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)

int lmbs_removed = 0;
int lmbs_available = 0;
int rc;
+   bool dt_update = false;
  
  	pr_info("Attempting to hot-remove %d LMB(s)\n", lmbs_to_remove);
  
@@ -409,7 +415,7 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)

}
  
  	for_each_drmem_lmb(lmb) {

-   rc = dlpar_remove_lmb(lmb);
+   

RE: [PATCH v2] selftests/powerpc: Fix pkey syscall redefinitions

2020-08-03 Thread David Laight
> > +#undef SYS_pkey_mprotect
> >  #define SYS_pkey_mprotect  386
> 
> We shouldn't undef them.
> 
> They should obviously never change, but if the system headers already
> have a definition then we should use that, so I think it should be:
> 
> #ifndef SYS_pkey_mprotect
> #define SYS_pkey_mprotect 386
> #endif

If the definitions are identical the compiler won't complain.
So you probably actually want a matching definition so that,
provided at least one compile picks up both headers, you know
that the definitions actually match.

David

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



Re: [PATCH v3] selftests/powerpc: Fix pkey syscall redefinitions

2020-08-03 Thread Sachin Sant



> On 03-Aug-2020, at 4:53 PM, Sandipan Das  wrote:
> 
> On some distros, there are conflicts w.r.t to redefinition
> of pkey syscall numbers which cause build failures. This
> fixes them.
> 
> Reported-by: Sachin Sant 
> Signed-off-by: Sandipan Das 
> —

Thanks for the fix.

Tested-by: Sachin Sant 



Re: [PATCHv4 1/2] powerpc/pseries: group lmb operation and memblock's

2020-08-03 Thread Laurent Dufour

@@ -603,6 +606,8 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
}

lmb_set_nid(lmb);
+   lmb->flags |= DRCONF_MEM_ASSIGNED;
+
block_sz = memory_block_size_bytes();

/* Add the memory */


Since the lmb->flags is now set earlier, you should unset it in the case the 
call to __add_memory() fails, something like:


@@ -614,6 +614,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
rc = __add_memory(lmb->nid, lmb->base_addr, block_sz);
if (rc) {
invalidate_lmb_associativity_index(lmb);
+   lmb->flags &= ~DRCONF_MEM_ASSIGNED;
return rc;
}


@@ -614,11 +619,14 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)

rc = dlpar_online_lmb(lmb);
if (rc) {
-   __remove_memory(lmb->nid, lmb->base_addr, block_sz);
+   int nid = lmb->nid;
+   phys_addr_t base_addr = lmb->base_addr;
+
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
-   } else {
-   lmb->flags |= DRCONF_MEM_ASSIGNED;
+   lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+
+   __remove_memory(nid, base_addr, block_sz);
}

return rc;


Re: [PATCH] powerpc: Fix P10 PVR revision in /proc/cpuinfo for SMT4 cores

2020-08-03 Thread Michael Ellerman
Michael Neuling  writes:
> On POWER10 bit 12 in the PVR indicates if the core is SMT4 or
> SMT8. Bit 12 is set for SMT4.
>
> Without this patch, /proc/cpuinfo on a SMT4 DD1 POWER10 looks like
> this:
> cpu : POWER10, altivec supported
> revision: 17.0 (pvr 0080 1100)
>
> Signed-off-by: Michael Neuling 
> ---
>  arch/powerpc/kernel/setup-common.c | 1 +
>  1 file changed, 1 insertion(+)

This should have a Fixes: pointing at something so it gets backported.

cheers


Re: powerpc: build failures in Linus' tree

2020-08-03 Thread Stephen Rothwell
Hi Michael,

On Mon, 03 Aug 2020 21:18:00 +1000 Michael Ellerman  wrote:
>
> If we just move the include of asm/paca.h below asm-generic/percpu.h
> then it avoids the bad circular dependency and we still have paca.h
> included from percpu.h as before.
> 
> eg:
> 
> diff --git a/arch/powerpc/include/asm/percpu.h 
> b/arch/powerpc/include/asm/percpu.h
> index dce863a7635c..8e5b7d0b851c 100644
> --- a/arch/powerpc/include/asm/percpu.h
> +++ b/arch/powerpc/include/asm/percpu.h
> @@ -10,8 +10,6 @@
>  
>  #ifdef CONFIG_SMP
>  
> -#include 
> -
>  #define __my_cpu_offset local_paca->data_offset
>  
>  #endif /* CONFIG_SMP */
> @@ -19,4 +17,6 @@
>  
>  #include 
>  
> +#include 
> +
>  #endif /* _ASM_POWERPC_PERCPU_H_ */
> 
> 
> So I think I'm inclined to merge that as a minimal fix that's easy to
> backport.
> 
> cheers

Looks ok, except does it matter that the include used to be only done
if __powerpc64__ and CONFIG_SMP are defined?

-- 
Cheers,
Stephen Rothwell


pgpvv9dQuUWY2.pgp
Description: OpenPGP digital signature


Re: powerpc: build failures in Linus' tree

2020-08-03 Thread Willy Tarreau
On Mon, Aug 03, 2020 at 09:18:00PM +1000, Michael Ellerman wrote:
> If we just move the include of asm/paca.h below asm-generic/percpu.h
> then it avoids the bad circular dependency and we still have paca.h
> included from percpu.h as before.
> 
> eg:
> 
> diff --git a/arch/powerpc/include/asm/percpu.h 
> b/arch/powerpc/include/asm/percpu.h
> index dce863a7635c..8e5b7d0b851c 100644
> --- a/arch/powerpc/include/asm/percpu.h
> +++ b/arch/powerpc/include/asm/percpu.h
> @@ -10,8 +10,6 @@
>  
>  #ifdef CONFIG_SMP
>  
> -#include 
> -
>  #define __my_cpu_offset local_paca->data_offset
>  
>  #endif /* CONFIG_SMP */
> @@ -19,4 +17,6 @@
>  
>  #include 
>  
> +#include 
> +
>  #endif /* _ASM_POWERPC_PERCPU_H_ */
> 
> 
> So I think I'm inclined to merge that as a minimal fix that's easy to
> backport.

This totally makes sense indeed!
Willy


Re: [PATCH v2] selftests/powerpc: Fix pkey syscall redefinitions

2020-08-03 Thread Sandipan Das



On 03/08/20 4:34 pm, Michael Ellerman wrote:
> Sandipan Das  writes:
>> On some distros, there are conflicts w.r.t to redefinition
>> of pkey syscall numbers which cause build failures. This
>> fixes them.
>>
>> Reported-by: Sachin Sant 
>> Signed-off-by: Sandipan Das 
>> ---
>> Previous versions can be found at:
>> v1: 
>> https://lore.kernel.org/linuxppc-dev/20200803074043.466809-1-sandi...@linux.ibm.com/
>>
>> Changes in v2:
>> - Fix incorrect commit message.
>>
>> ---
>>  tools/testing/selftests/powerpc/include/pkeys.h | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/tools/testing/selftests/powerpc/include/pkeys.h 
>> b/tools/testing/selftests/powerpc/include/pkeys.h
>> index 6ba95039a034..26eef5c1f8ea 100644
>> --- a/tools/testing/selftests/powerpc/include/pkeys.h
>> +++ b/tools/testing/selftests/powerpc/include/pkeys.h
>> @@ -31,8 +31,13 @@
>>  
>>  #define SI_PKEY_OFFSET  0x20
>>  
>> +#undef SYS_pkey_mprotect
>>  #define SYS_pkey_mprotect   386
> 
> We shouldn't undef them.
> 
> They should obviously never change, but if the system headers already
> have a definition then we should use that, so I think it should be:
> 
> #ifndef SYS_pkey_mprotect
> #define SYS_pkey_mprotect 386
> #endif
> 

Agreed. This had me confused.

$ grep -nr "#define __NR_pkey_" /usr/include/
/usr/include/asm-generic/unistd.h:767:#define __NR_pkey_mprotect 288
/usr/include/asm-generic/unistd.h:769:#define __NR_pkey_alloc 289
/usr/include/asm-generic/unistd.h:771:#define __NR_pkey_free 290
/usr/include/powerpc64le-linux-gnu/asm/unistd_32.h:374:#define __NR_pkey_alloc  
384
/usr/include/powerpc64le-linux-gnu/asm/unistd_32.h:375:#define __NR_pkey_free   
385
/usr/include/powerpc64le-linux-gnu/asm/unistd_32.h:376:#define 
__NR_pkey_mprotect   386
/usr/include/powerpc64le-linux-gnu/asm/unistd_64.h:365:#define __NR_pkey_alloc  
384
/usr/include/powerpc64le-linux-gnu/asm/unistd_64.h:366:#define __NR_pkey_free   
385
/usr/include/powerpc64le-linux-gnu/asm/unistd_64.h:367:#define 
__NR_pkey_mprotect   386
...

But it looks like including unistd.h from a C program picks the
right values.


- Sandipan


Re: [PATCH] powerpc/boot: Use address-of operator on section symbols

2020-08-03 Thread Geert Uytterhoeven
Hi Michael,

On Mon, Aug 3, 2020 at 1:09 PM Michael Ellerman  wrote:
> Geert Uytterhoeven  writes:
> > On Mon, Jul 20, 2020 at 11:03 PM Segher Boessenkool
> >  wrote:
> >> On Sat, Jul 18, 2020 at 09:50:50AM +0200, Geert Uytterhoeven wrote:
> >> > On Wed, Jun 24, 2020 at 6:02 AM Nathan Chancellor
> >> >  wrote:
> >> > > /* If we have an image attached to us, it overrides anything
> >> > >  * supplied by the loader. */
> >> > > -   if (_initrd_end > _initrd_start) {
> >> > > +   if (&_initrd_end > &_initrd_start) {
> >> >
> >> > Are you sure that fix is correct?
> >> >
> >> > extern char _initrd_start[];
> >> > extern char _initrd_end[];
> >> > extern char _esm_blob_start[];
> >> > extern char _esm_blob_end[];
> >> >
> >> > Of course the result of their comparison is a constant, as the addresses
> >> > are constant.  If clangs warns about it, perhaps that warning should be 
> >> > moved
> >> > to W=1?
> >> >
> >> > But adding "&" is not correct, according to C.
> >>
> >> Why not?
> >>
> >> 6.5.3.2/3
> >> The unary & operator yields the address of its operand.  [...]
> >> Otherwise, the result is a pointer to the object or function designated
> >> by its operand.
> >>
> >> This is the same as using the name of an array without anything else,
> >> yes.  It is a bit clearer if it would not be declared as array, perhaps,
> >> but it is correct just fine like this.
> >
> > Thanks, I stand corrected.
> >
> > Regardless, the comparison is still a comparison between two constant
> > addresses, so my fear is that the compiler will start generating
> > warnings for that in the near or distant future, making this change
> > futile.
>
> They're not constant at compile time though. So I don't think the
> compiler could (sensibly) warn about that? (surely!)

They're constant, but the compiler doesn't know their value.
That doesn't change by (not) using the address-of operator.

Gr{oetje,eeting}s,

Geert

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

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


Re: [merge] Build failure selftest/powerpc/mm/pkey_exec_prot

2020-08-03 Thread Sandipan Das
Hi Michael,

On 03/08/20 4:32 pm, Michael Ellerman wrote:
> Sachin Sant  writes:
>>> On 02-Aug-2020, at 10:58 PM, Sandipan Das  wrote:
>>> On 02/08/20 4:45 pm, Sachin Sant wrote:
 pkey_exec_prot test from linuxppc merge branch (3f68564f1f5a) fails to
 build due to following error:

 gcc -std=gnu99 -O2 -Wall -Werror 
 -DGIT_VERSION='"v5.8-rc7-1276-g3f68564f1f5a"' 
 -I/home/sachin/linux/tools/testing/selftests/powerpc/include  -m64
 pkey_exec_prot.c 
 /home/sachin/linux/tools/testing/selftests/kselftest_harness.h 
 /home/sachin/linux/tools/testing/selftests/kselftest.h ../harness.c 
 ../utils.c  -o 
 /home/sachin/linux/tools/testing/selftests/powerpc/mm/pkey_exec_prot
 In file included from pkey_exec_prot.c:18:
 /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:34: 
 error: "SYS_pkey_mprotect" redefined [-Werror]
 #define SYS_pkey_mprotect 386

 In file included from /usr/include/sys/syscall.h:31,
 from 
 /home/sachin/linux/tools/testing/selftests/powerpc/include/utils.h:47,
 from 
 /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:12,
 from pkey_exec_prot.c:18:
 /usr/include/bits/syscall.h:1583: note: this is the location of the 
 previous definition
 # define SYS_pkey_mprotect __NR_pkey_mprotect

 commit 128d3d021007 introduced this error.
 selftests/powerpc: Move pkey helpers to headers

 Possibly the # defines for sys calls can be retained in pkey_exec_prot.c or

>>>
>>> I am unable to reproduce this on the latest merge branch (HEAD at 
>>> f59195f7faa4).
>>> I don't see any redefinitions in pkey_exec_prot.c either.
>>>
>>
>> I can still see this problem on latest merge branch.
>> I have following gcc version
>>
>> gcc version 8.3.1 20191121
> 
> What libc version? Or just the distro & version?
> 

Sachin observed this on RHEL 8.2 with glibc-2.28.
I couldn't reproduce it on Ubuntu 20.04 and Fedora 32 and both these distros
are using glibc-2.31.


- Sandipan


[PATCH v3] selftests/powerpc: Fix pkey syscall redefinitions

2020-08-03 Thread Sandipan Das
On some distros, there are conflicts w.r.t to redefinition
of pkey syscall numbers which cause build failures. This
fixes them.

Reported-by: Sachin Sant 
Signed-off-by: Sandipan Das 
---
Previous versions can be found at:
v2: 
https://lore.kernel.org/linuxppc-dev/566dde119ce71f00f9642807ba30ceb7f54c9bfa.1596441105.git.sandi...@linux.ibm.com/
v1: 
https://lore.kernel.org/linuxppc-dev/20200803074043.466809-1-sandi...@linux.ibm.com/

Changes in v3:
- Use ifndef...endif instead of undef as suggested by
  Michael.

Changes in v2:
- Fix incorrect commit message.

---
 tools/testing/selftests/powerpc/include/pkeys.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/tools/testing/selftests/powerpc/include/pkeys.h 
b/tools/testing/selftests/powerpc/include/pkeys.h
index 6ba95039a034..54bf9aa9e1e1 100644
--- a/tools/testing/selftests/powerpc/include/pkeys.h
+++ b/tools/testing/selftests/powerpc/include/pkeys.h
@@ -31,9 +31,17 @@
 
 #define SI_PKEY_OFFSET 0x20
 
+#ifndef SYS_pkey_mprotect
 #define SYS_pkey_mprotect  386
+#endif
+
+#ifndef SYS_pkey_alloc
 #define SYS_pkey_alloc 384
+#endif
+
+#ifndef SYS_pkey_free
 #define SYS_pkey_free  385
+#endif
 
 #define PKEY_BITS_PER_PKEY 2
 #define NR_PKEYS   32
-- 
2.25.1



Re: powerpc: build failures in Linus' tree

2020-08-03 Thread Michael Ellerman
Willy Tarreau  writes:
> On Sun, Aug 02, 2020 at 07:20:19PM +0200, Willy Tarreau wrote:
>> On Sun, Aug 02, 2020 at 08:48:42PM +1000, Stephen Rothwell wrote:
>> > Hi all,
>> > 
>> > We are getting build failures in some PowerPC configs for Linus' tree.
>> > See e.g. http://kisskb.ellerman.id.au/kisskb/buildresult/14306515/
>> > 
>> > In file included from /kisskb/src/arch/powerpc/include/asm/paca.h:18,
>> >  from /kisskb/src/arch/powerpc/include/asm/percpu.h:13,
>> >  from /kisskb/src/include/linux/random.h:14,
>> >  from /kisskb/src/include/linux/net.h:18,
>> >  from /kisskb/src/net/ipv6/ip6_fib.c:20:
>> > /kisskb/src/arch/powerpc/include/asm/mmu.h:139:22: error: unknown type 
>> > name 'next_tlbcam_idx'
>> >   139 | DECLARE_PER_CPU(int, next_tlbcam_idx);
>> > 
>> > I assume this is caused by commit
>> > 
>> >   1c9df907da83 ("random: fix circular include dependency on arm64 after 
>> > addition of percpu.h")
>> > 
>> > But I can't see how, sorry.
>> 
>> So there, asm/mmu.h includes asm/percpu.h, which includes asm/paca.h, which
>> includes asm/mmu.h.
>> 
>> I suspect that we can remove asm/paca.h from asm/percpu.h as it *seems*
>> to be only used by the #define __my_cpu_offset but I don't know if anything
>> will break further, especially if this __my_cpu_offset is used anywhere
>> without this paca definition.
>
> I tried this and it fixed 5.8 for me with your config above. I'm appending
> a patch that does just this. I didn't test other configs as I don't know
> which ones to test though. If it fixes the problem for you, maybe it can
> be picked by the PPC maintainers.
>
> Willy
> From bcd64a7d0f3445c9a75d3b4dc4837d2ce61660c9 Mon Sep 17 00:00:00 2001
> From: Willy Tarreau 
> Date: Mon, 3 Aug 2020 05:27:57 +0200
> Subject: powerpc: fix circular dependency in percpu.h
>
> After random.h started to include percpu.h (commit f227e3e), several
> archs broke in circular dependencies around percpu.h.
>
> In https://lore.kernel.org/lkml/20200802204842.36bca...@canb.auug.org.au/
> Stephen Rothwell reported breakage for powerpc with CONFIG_PPC_FSL_BOOK3E.
>
> It turns out that asm/percpu.h includes asm/paca.h, which itself
> includes mmu.h, which includes percpu.h when CONFIG_PPC_FSL_BOOK3E=y.
>
> Percpu seems to include asm/paca.h only for local_paca which is used in
> the __my_cpu_offset macro. Removing this include solves the issue for
> this config.
>
> Reported-by: Stephen Rothwell 
> Fixes: f227e3e ("random32: update the net random state on interrupt and 
> activity")
> Link: https://lore.kernel.org/lkml/20200802204842.36bca...@canb.auug.org.au/
> Cc: Linus Torvalds 
> Cc: Michael Ellerman 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Signed-off-by: Willy Tarreau 
> ---
>  arch/powerpc/include/asm/percpu.h | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/percpu.h 
> b/arch/powerpc/include/asm/percpu.h
> index dce863a..cd3f6e5 100644
> --- a/arch/powerpc/include/asm/percpu.h
> +++ b/arch/powerpc/include/asm/percpu.h
> @@ -10,8 +10,6 @@
>  
>  #ifdef CONFIG_SMP
>  
> -#include 
> -
>  #define __my_cpu_offset local_paca->data_offset
>  
>  #endif /* CONFIG_SMP */

If we just move the include of asm/paca.h below asm-generic/percpu.h
then it avoids the bad circular dependency and we still have paca.h
included from percpu.h as before.

eg:

diff --git a/arch/powerpc/include/asm/percpu.h 
b/arch/powerpc/include/asm/percpu.h
index dce863a7635c..8e5b7d0b851c 100644
--- a/arch/powerpc/include/asm/percpu.h
+++ b/arch/powerpc/include/asm/percpu.h
@@ -10,8 +10,6 @@
 
 #ifdef CONFIG_SMP
 
-#include 
-
 #define __my_cpu_offset local_paca->data_offset
 
 #endif /* CONFIG_SMP */
@@ -19,4 +17,6 @@
 
 #include 
 
+#include 
+
 #endif /* _ASM_POWERPC_PERCPU_H_ */


So I think I'm inclined to merge that as a minimal fix that's easy to
backport.

cheers


Re: [PATCH] powerpc/boot: Use address-of operator on section symbols

2020-08-03 Thread Michael Ellerman
Geert Uytterhoeven  writes:
> On Mon, Jul 20, 2020 at 11:03 PM Segher Boessenkool
>  wrote:
>> On Sat, Jul 18, 2020 at 09:50:50AM +0200, Geert Uytterhoeven wrote:
>> > On Wed, Jun 24, 2020 at 6:02 AM Nathan Chancellor
>> >  wrote:
>> > > /* If we have an image attached to us, it overrides anything
>> > >  * supplied by the loader. */
>> > > -   if (_initrd_end > _initrd_start) {
>> > > +   if (&_initrd_end > &_initrd_start) {
>> >
>> > Are you sure that fix is correct?
>> >
>> > extern char _initrd_start[];
>> > extern char _initrd_end[];
>> > extern char _esm_blob_start[];
>> > extern char _esm_blob_end[];
>> >
>> > Of course the result of their comparison is a constant, as the addresses
>> > are constant.  If clangs warns about it, perhaps that warning should be 
>> > moved
>> > to W=1?
>> >
>> > But adding "&" is not correct, according to C.
>>
>> Why not?
>>
>> 6.5.3.2/3
>> The unary & operator yields the address of its operand.  [...]
>> Otherwise, the result is a pointer to the object or function designated
>> by its operand.
>>
>> This is the same as using the name of an array without anything else,
>> yes.  It is a bit clearer if it would not be declared as array, perhaps,
>> but it is correct just fine like this.
>
> Thanks, I stand corrected.
>
> Regardless, the comparison is still a comparison between two constant
> addresses, so my fear is that the compiler will start generating
> warnings for that in the near or distant future, making this change
> futile.

They're not constant at compile time though. So I don't think the
compiler could (sensibly) warn about that? (surely!)

cheers



Re: [PATCH v2] selftests/powerpc: Fix pkey syscall redefinitions

2020-08-03 Thread Michael Ellerman
Sandipan Das  writes:
> On some distros, there are conflicts w.r.t to redefinition
> of pkey syscall numbers which cause build failures. This
> fixes them.
>
> Reported-by: Sachin Sant 
> Signed-off-by: Sandipan Das 
> ---
> Previous versions can be found at:
> v1: 
> https://lore.kernel.org/linuxppc-dev/20200803074043.466809-1-sandi...@linux.ibm.com/
>
> Changes in v2:
> - Fix incorrect commit message.
>
> ---
>  tools/testing/selftests/powerpc/include/pkeys.h | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/tools/testing/selftests/powerpc/include/pkeys.h 
> b/tools/testing/selftests/powerpc/include/pkeys.h
> index 6ba95039a034..26eef5c1f8ea 100644
> --- a/tools/testing/selftests/powerpc/include/pkeys.h
> +++ b/tools/testing/selftests/powerpc/include/pkeys.h
> @@ -31,8 +31,13 @@
>  
>  #define SI_PKEY_OFFSET   0x20
>  
> +#undef SYS_pkey_mprotect
>  #define SYS_pkey_mprotect386

We shouldn't undef them.

They should obviously never change, but if the system headers already
have a definition then we should use that, so I think it should be:

#ifndef SYS_pkey_mprotect
#define SYS_pkey_mprotect   386
#endif

cheers


Re: [merge] Build failure selftest/powerpc/mm/pkey_exec_prot

2020-08-03 Thread Michael Ellerman
Sachin Sant  writes:
>> On 02-Aug-2020, at 10:58 PM, Sandipan Das  wrote:
>> On 02/08/20 4:45 pm, Sachin Sant wrote:
>>> pkey_exec_prot test from linuxppc merge branch (3f68564f1f5a) fails to
>>> build due to following error:
>>> 
>>> gcc -std=gnu99 -O2 -Wall -Werror 
>>> -DGIT_VERSION='"v5.8-rc7-1276-g3f68564f1f5a"' 
>>> -I/home/sachin/linux/tools/testing/selftests/powerpc/include  -m64
>>> pkey_exec_prot.c 
>>> /home/sachin/linux/tools/testing/selftests/kselftest_harness.h 
>>> /home/sachin/linux/tools/testing/selftests/kselftest.h ../harness.c 
>>> ../utils.c  -o 
>>> /home/sachin/linux/tools/testing/selftests/powerpc/mm/pkey_exec_prot
>>> In file included from pkey_exec_prot.c:18:
>>> /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:34: 
>>> error: "SYS_pkey_mprotect" redefined [-Werror]
>>> #define SYS_pkey_mprotect 386
>>> 
>>> In file included from /usr/include/sys/syscall.h:31,
>>> from 
>>> /home/sachin/linux/tools/testing/selftests/powerpc/include/utils.h:47,
>>> from 
>>> /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:12,
>>> from pkey_exec_prot.c:18:
>>> /usr/include/bits/syscall.h:1583: note: this is the location of the 
>>> previous definition
>>> # define SYS_pkey_mprotect __NR_pkey_mprotect
>>> 
>>> commit 128d3d021007 introduced this error.
>>> selftests/powerpc: Move pkey helpers to headers
>>> 
>>> Possibly the # defines for sys calls can be retained in pkey_exec_prot.c or
>>> 
>> 
>> I am unable to reproduce this on the latest merge branch (HEAD at 
>> f59195f7faa4).
>> I don't see any redefinitions in pkey_exec_prot.c either.
>> 
>
> I can still see this problem on latest merge branch.
> I have following gcc version
>
> gcc version 8.3.1 20191121

What libc version? Or just the distro & version?

cheers

> # git show
> commit f59195f7faa4896b7c1d947ac2dba29ec18ad569 (HEAD -> merge, origin/merge)
> Merge: 70ce795dac09 ac3a0c847296
> Author: Michael Ellerman 
> Date:   Sun Aug 2 23:18:03 2020 +1000
>
> Automatic merge of 'master', 'next' and 'fixes' (2020-08-02 23:18)
>
> # make -C powerpc
> ……
> …...
> BUILD_TARGET=/home/sachin/linux/tools/testing/selftests/powerpc/mm; mkdir -p 
> $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C mm all
> make[1]: Entering directory 
> '/home/sachin/linux/tools/testing/selftests/powerpc/mm'
> gcc -std=gnu99 -O2 -Wall -Werror 
> -DGIT_VERSION='"v5.8-rc7-1456-gf59195f7faa4"' 
> -I/home/sachin/linux/tools/testing/selftests/powerpc/include  -m64
> pkey_exec_prot.c 
> /home/sachin/linux/tools/testing/selftests/kselftest_harness.h 
> /home/sachin/linux/tools/testing/selftests/kselftest.h ../harness.c 
> ../utils.c  -o 
> /home/sachin/linux/tools/testing/selftests/powerpc/mm/pkey_exec_prot
> In file included from pkey_exec_prot.c:18:
> /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:34: error: 
> "SYS_pkey_mprotect" redefined [-Werror]
>  #define SYS_pkey_mprotect 386
>  
> In file included from /usr/include/sys/syscall.h:31,
>  from 
> /home/sachin/linux/tools/testing/selftests/powerpc/include/utils.h:47,
>  from 
> /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:12,
>  from pkey_exec_prot.c:18:
> /usr/include/bits/syscall.h:1583: note: this is the location of the previous 
> definition
>  # define SYS_pkey_mprotect __NR_pkey_mprotect


Re: [PATCH] powerpc: Fix P10 PVR revision in /proc/cpuinfo for SMT4 cores

2020-08-03 Thread Vaidyanathan Srinivasan
* Michael Neuling  [2020-08-03 13:56:00]:

> On POWER10 bit 12 in the PVR indicates if the core is SMT4 or
> SMT8. Bit 12 is set for SMT4.
> 
> Without this patch, /proc/cpuinfo on a SMT4 DD1 POWER10 looks like
> this:
> cpu : POWER10, altivec supported
> revision: 17.0 (pvr 0080 1100)
> 
> Signed-off-by: Michael Neuling 

Reviewed-by: Vaidyanathan Srinivasan 

> ---
>  arch/powerpc/kernel/setup-common.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/powerpc/kernel/setup-common.c 
> b/arch/powerpc/kernel/setup-common.c
> index b198b0ff25..808ec9fab6 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -311,6 +311,7 @@ static int show_cpuinfo(struct seq_file *m, void *v)
>   min = pvr & 0xFF;
>   break;
>   case 0x004e: /* POWER9 bits 12-15 give chip type */
> + case 0x0080: /* POWER10 bit 12 gives SMT8/4 */

Correct. P9 and P10 have chip type (smt4 vs smt8 core) encoded in bits
PVR chip type bits 12-15.

Thanks for the fix.

--Vaidy



Re: [PATCH] powerpc/boot: Use address-of operator on section symbols

2020-08-03 Thread Geert Uytterhoeven
Hi Segher,

On Mon, Jul 20, 2020 at 11:03 PM Segher Boessenkool
 wrote:
> On Sat, Jul 18, 2020 at 09:50:50AM +0200, Geert Uytterhoeven wrote:
> > On Wed, Jun 24, 2020 at 6:02 AM Nathan Chancellor
> >  wrote:
> > > /* If we have an image attached to us, it overrides anything
> > >  * supplied by the loader. */
> > > -   if (_initrd_end > _initrd_start) {
> > > +   if (&_initrd_end > &_initrd_start) {
> >
> > Are you sure that fix is correct?
> >
> > extern char _initrd_start[];
> > extern char _initrd_end[];
> > extern char _esm_blob_start[];
> > extern char _esm_blob_end[];
> >
> > Of course the result of their comparison is a constant, as the addresses
> > are constant.  If clangs warns about it, perhaps that warning should be 
> > moved
> > to W=1?
> >
> > But adding "&" is not correct, according to C.
>
> Why not?
>
> 6.5.3.2/3
> The unary & operator yields the address of its operand.  [...]
> Otherwise, the result is a pointer to the object or function designated
> by its operand.
>
> This is the same as using the name of an array without anything else,
> yes.  It is a bit clearer if it would not be declared as array, perhaps,
> but it is correct just fine like this.

Thanks, I stand corrected.

Regardless, the comparison is still a comparison between two constant
addresses, so my fear is that the compiler will start generating
warnings for that in the near or distant future, making this change
futile.

Gr{oetje,eeting}s,

Geert

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

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


Re: Build regressions/improvements in v5.8

2020-08-03 Thread Geert Uytterhoeven
On Mon, Aug 3, 2020 at 11:53 AM Geert Uytterhoeven  wrote:
> JFYI, when comparing v5.8[1] to v5.8-rc7[3], the summaries are:
>   - build errors: +2/-3

  + /kisskb/src/arch/powerpc/include/asm/mmu.h: error: unknown type
name 'next_tlbcam_idx':  => 139:22

v5.8/powerpc-gcc4.9/corenet64_smp_defconfig
v5.8/powerpc-gcc4.9/ppc64_book3e_allmodconfig
v5.8/powerpc-gcc4.9/ppc64e_defconfig
v5.8/powerpc-gcc9/ppc64_book3e_allmodconfig
(fix available)

  + /kisskb/src/arch/sparc/include/asm/trap_block.h: error: 'NR_CPUS'
undeclared here (not in a function):  => 54:39

v5.8/sparc64/sparc64-allmodconfig
v5.8/sparc64/sparc64-defconfig

> [1] 
> http://kisskb.ellerman.id.au/kisskb/branch/linus/head/bcf876870b95592b52519ed4aafcf9d95999bc9c/
>  (192 out of 194 configs)
> [3] 
> http://kisskb.ellerman.id.au/kisskb/branch/linus/head/92ed301919932f13b9172e525674157e983d/
>  (192 out of 194 configs)

Gr{oetje,eeting}s,

Geert

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

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


[PATCH v2] selftests/powerpc: Fix pkey syscall redefinitions

2020-08-03 Thread Sandipan Das
On some distros, there are conflicts w.r.t to redefinition
of pkey syscall numbers which cause build failures. This
fixes them.

Reported-by: Sachin Sant 
Signed-off-by: Sandipan Das 
---
Previous versions can be found at:
v1: 
https://lore.kernel.org/linuxppc-dev/20200803074043.466809-1-sandi...@linux.ibm.com/

Changes in v2:
- Fix incorrect commit message.

---
 tools/testing/selftests/powerpc/include/pkeys.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/powerpc/include/pkeys.h 
b/tools/testing/selftests/powerpc/include/pkeys.h
index 6ba95039a034..26eef5c1f8ea 100644
--- a/tools/testing/selftests/powerpc/include/pkeys.h
+++ b/tools/testing/selftests/powerpc/include/pkeys.h
@@ -31,8 +31,13 @@
 
 #define SI_PKEY_OFFSET 0x20
 
+#undef SYS_pkey_mprotect
 #define SYS_pkey_mprotect  386
+
+#undef SYS_pkey_alloc
 #define SYS_pkey_alloc 384
+
+#undef SYS_pkey_free
 #define SYS_pkey_free  385
 
 #define PKEY_BITS_PER_PKEY 2
-- 
2.25.1



[PATCH] powerpc/powernv/sriov: Fix use of uninitialised variable

2020-08-03 Thread Oliver O'Halloran
Initialising the value before using it is generally regarded as a good
idea so do that.

Fixes: 4c51f3e1e870 ("powerpc/powernv/sriov: Make single PE mode a per-BAR 
setting")
Reported-by: Nathan Chancellor 
Signed-off-by: Oliver O'Halloran 
---
 arch/powerpc/platforms/powernv/pci-sriov.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c 
b/arch/powerpc/platforms/powernv/pci-sriov.c
index 7894745fd4f8..c4434f20f42f 100644
--- a/arch/powerpc/platforms/powernv/pci-sriov.c
+++ b/arch/powerpc/platforms/powernv/pci-sriov.c
@@ -253,9 +253,9 @@ void pnv_pci_ioda_fixup_iov(struct pci_dev *pdev)
 resource_size_t pnv_pci_iov_resource_alignment(struct pci_dev *pdev,
  int resno)
 {
+   resource_size_t align = pci_iov_resource_size(pdev, resno);
struct pnv_phb *phb = pci_bus_to_pnvhb(pdev->bus);
struct pnv_iov_data *iov = pnv_iov_get(pdev);
-   resource_size_t align;
 
/*
 * iov can be null if we have an SR-IOV device with IOV BAR that can't
@@ -266,8 +266,6 @@ resource_size_t pnv_pci_iov_resource_alignment(struct 
pci_dev *pdev,
if (!iov)
return align;
 
-   align = pci_iov_resource_size(pdev, resno);
-
/*
 * If we're using single mode then we can just use the native VF BAR
 * alignment. We validated that it's possible to use a single PE
-- 
2.26.2



[PATCH] selftests/powerpc: Fix pkey syscall redefinitions

2020-08-03 Thread Sandipan Das
Some distros have the pkey syscall numbers defined under
unistd.h. This conflicts with the definitions in the pkeys
selftest header and causes build failures.

E.g. this works
  $ grep -nr "SYS_pkey" /usr/include/
  /usr/include/bits/syscall.h:1575:# define SYS_pkey_alloc __NR_pkey_alloc
  /usr/include/bits/syscall.h:1579:# define SYS_pkey_free __NR_pkey_free
  /usr/include/bits/syscall.h:1583:# define SYS_pkey_mprotect __NR_pkey_mprotect

While this does not
  $ grep -nr "SYS_pkey" /usr/include/
  /usr/include/bits/syscall.h:1575:# define SYS_pkey_alloc __NR_pkey_alloc
  /usr/include/bits/syscall.h:1579:# define SYS_pkey_free __NR_pkey_free
  /usr/include/bits/syscall.h:1583:# define SYS_pkey_mprotect __NR_pkey_mprotect
  /usr/include/asm-generic/unistd.h:728:__SYSCALL(__NR_pkey_mprotect, 
sys_pkey_mprotect)
  /usr/include/asm-generic/unistd.h:730:__SYSCALL(__NR_pkey_alloc,
sys_pkey_alloc)
  /usr/include/asm-generic/unistd.h:732:__SYSCALL(__NR_pkey_free, 
sys_pkey_free)

Reported-by: Sachin Sant 
Signed-off-by: Sandipan Das 
---
 tools/testing/selftests/powerpc/include/pkeys.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/powerpc/include/pkeys.h 
b/tools/testing/selftests/powerpc/include/pkeys.h
index 6ba95039a034..26eef5c1f8ea 100644
--- a/tools/testing/selftests/powerpc/include/pkeys.h
+++ b/tools/testing/selftests/powerpc/include/pkeys.h
@@ -31,8 +31,13 @@
 
 #define SI_PKEY_OFFSET 0x20
 
+#undef SYS_pkey_mprotect
 #define SYS_pkey_mprotect  386
+
+#undef SYS_pkey_alloc
 #define SYS_pkey_alloc 384
+
+#undef SYS_pkey_free
 #define SYS_pkey_free  385
 
 #define PKEY_BITS_PER_PKEY 2
-- 
2.25.1



Re: [PATCH 2/2 v2] powerpc/powernv: Enable and setup PCI P2P

2020-08-03 Thread Oliver O'Halloran
On Thu, Apr 30, 2020 at 11:15 PM Max Gurtovoy  wrote:
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c 
> b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 57d3a6a..9ecc576 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -3706,18 +3706,208 @@ static void pnv_pci_ioda_dma_bus_setup(struct 
> pci_bus *bus)
> }
>  }
>
> +#ifdef CONFIG_PCI_P2PDMA
> +static DEFINE_MUTEX(p2p_mutex);
> +
> +static bool pnv_pci_controller_owns_addr(struct pci_controller *hose,
> +phys_addr_t addr, size_t size)
> +{
> +   int i;
> +
> +   /*
> +* It seems safe to assume the full range is under the same PHB, so we
> +* can ignore the size.
> +*/
> +   for (i = 0; i < ARRAY_SIZE(hose->mem_resources); i++) {
> +   struct resource *res = >mem_resources[i];
> +
> +   if (res->flags && addr >= res->start && addr < res->end)
> +   return true;
> +   }
> +   return false;
> +}
> +
> +/*
> + * find the phb owning a mmio address if not owned locally
> + */
> +static struct pnv_phb *pnv_pci_find_owning_phb(struct pci_dev *pdev,
> +  phys_addr_t addr, size_t size)
> +{
> +   struct pci_controller *hose;
> +
> +   /* fast path */
> +   if (pnv_pci_controller_owns_addr(pdev->bus->sysdata, addr, size))
> +   return NULL;

Do we actually need this fast path? It's going to be slow either way.
Also if a device is doing p2p to another device under the same PHB
then it should not be happening via the root complex. Is this a case
you've tested?

> +   list_for_each_entry(hose, _list, list_node) {
> +   struct pnv_phb *phb = hose->private_data;
> +
> +   if (phb->type != PNV_PHB_NPU_NVLINK &&
> +   phb->type != PNV_PHB_NPU_OCAPI) {
> +   if (pnv_pci_controller_owns_addr(hose, addr, size))
> +   return phb;
> +   }
> +   }
> +   return NULL;
> +}
> +
> +static u64 pnv_pci_dma_dir_to_opal_p2p(enum dma_data_direction dir)
> +{
> +   if (dir == DMA_TO_DEVICE)
> +   return OPAL_PCI_P2P_STORE;
> +   else if (dir == DMA_FROM_DEVICE)
> +   return OPAL_PCI_P2P_LOAD;
> +   else if (dir == DMA_BIDIRECTIONAL)
> +   return OPAL_PCI_P2P_LOAD | OPAL_PCI_P2P_STORE;
> +   else
> +   return 0;
> +}
> +
> +static int pnv_pci_ioda_enable_p2p(struct pci_dev *initiator,
> +  struct pnv_phb *phb_target,
> +  enum dma_data_direction dir)
> +{
> +   struct pci_controller *hose;
> +   struct pnv_phb *phb_init;
> +   struct pnv_ioda_pe *pe_init;
> +   u64 desc;
> +   int rc;
> +
> +   if (!opal_check_token(OPAL_PCI_SET_P2P))
> +   return -ENXIO;
> +

> +   hose = pci_bus_to_host(initiator->bus);
> +   phb_init = hose->private_data;

You can use the pci_bus_to_pnvhb() helper

> +
> +   pe_init = pnv_ioda_get_pe(initiator);
> +   if (!pe_init)
> +   return -ENODEV;
> +
> +   if (!pe_init->tce_bypass_enabled)
> +   return -EINVAL;
> +
> +   /*
> +* Configuring the initiator's PHB requires to adjust its TVE#1
> +* setting. Since the same device can be an initiator several times 
> for
> +* different target devices, we need to keep a reference count to know
> +* when we can restore the default bypass setting on its TVE#1 when
> +* disabling. Opal is not tracking PE states, so we add a reference
> +* count on the PE in linux.
> +*
> +* For the target, the configuration is per PHB, so we keep a
> +* target reference count on the PHB.
> +*/

This irks me a bit because configuring the DMA address limits for the
TVE is the kernel's job. What we really should be doing is using
opal_pci_map_pe_dma_window_real() to set the bypass-mode address limit
for the TVE to something large enough to hit the MMIO ranges rather
than having set_p2p do it as a side effect. Unfortunately, for some
reason skiboot doesn't implement support for enabling 56bit addressing
using opal_pci_map_pe_dma_window_real() and we do need to support
older kernel's which used this stuff so I guess we're stuck with it
for now. It'd be nice if we could fix this in the longer term
though...

> +   mutex_lock(_mutex);
> +
> +   desc = OPAL_PCI_P2P_ENABLE | pnv_pci_dma_dir_to_opal_p2p(dir);
> +   /* always go to opal to validate the configuration */
> +   rc = opal_pci_set_p2p(phb_init->opal_id, phb_target->opal_id, desc,
> + pe_init->pe_number);
> +   if (rc != OPAL_SUCCESS) {
> +   rc = -EIO;
> +   goto out;
> +   }
> +
> +   pe_init->p2p_initiator_count++;
> +   

linux-next: manual merge of the char-misc tree with the powerpc tree

2020-08-03 Thread Stephen Rothwell
Hi all,

Today's linux-next merge of the char-misc tree got a conflict in:

  drivers/misc/ocxl/config.c

between commit:

  3591538a31af ("ocxl: Address kernel doc errors & warnings")

from the powerpc tree and commit:

  28fc491e9be6 ("misc: ocxl: config: Provide correct formatting to function 
headers")

from the char-misc tree.

I fixed it up (as it was just differences in comments, I just arbitrarily
chose the latter version) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell


pgpOZLQA1Gmp_.pgp
Description: OpenPGP digital signature


Re: [PATCH v4 09/10] Powerpc/smp: Create coregroup domain

2020-08-03 Thread Srikar Dronamraju
> > Also in the current P9 itself, two neighbouring core-pairs form a quad.
> > Cache latency within a quad is better than a latency to a distant core-pair.
> > Cache latency within a core pair is way better than latency within a quad.
> > So if we have only 4 threads running on a DIE all of them accessing the same
> > cache-lines, then we could probably benefit if all the tasks were to run
> > within the quad aka MC/Coregroup.
> >
> 
> Did you test this? WRT load balance we do try to balance "load" over the
> different domain spans, so if you represent quads as their own MC domain,
> you would AFAICT end up spreading tasks over the quads (rather than packing
> them) when balancing at e.g. DIE level. The desired behaviour might be
> hackable with some more ASYM_PACKING, but I'm not sure I should be
> suggesting that :-)
> 

Agree, load balance will try to spread the load across the quads. In my hack,
I was explicitly marking QUAD domains as !SD_PREFER_SIBLING + relaxing few
load spreading rules when SD_PREFER_SIBLING was not set. And this was on a
slightly older kernel (without recent Vincent's load balance overhaul).

> > I have found some benchmarks which are latency sensitive to benefit by
> > having a grouping a quad level (using kernel hacks and not backed by
> > firmware changes). Gautham also found similar results in his experiments
> > but he only used binding within the stock kernel.
> >
> 
> IIUC you reflect this "fabric quirk" (i.e. coregroups) using this DT
> binding thing.
> 
> That's also where things get interesting (for me) because I experienced
> something similar on another arm64 platform (ThunderX1). This was more
> about cache bandwidth than cache latency, but IMO it's in the same bag of
> fabric quirks. I blabbered a bit about this at last LPC [1], but kind of
> gave up on it given the TX1 was the only (arm64) platform where I could get
> both significant and reproducible results.
> 
> Now, if you folks are seeing this on completely different hardware and have
> "real" workloads that truly benefit from this kind of domain partitioning,
> this might be another incentive to try and sort of generalize this. That's
> outside the scope of your series, but your findings give me some hope!
> 
> I think what I had in mind back then was that if enough folks cared about
> it, we might get some bits added to the ACPI spec; something along the
> lines of proximity domains for the caches described in PPTT, IOW a cache
> distance matrix. I don't really know what it'll take to get there, but I
> figured I'd dump this in case someone's listening :-)
> 

Very interesting.

> > I am not setting SD_SHARE_PKG_RESOURCES in MC/Coregroup sd_flags as in MC
> > domain need not be LLC domain for Power.
> 
> From what I understood your MC domain does seem to map to LLC; but in any
> case, shouldn't you set that flag at least for BIGCORE (i.e. L2)? AIUI with
> your changes your sd_llc is gonna be SMT, and that's not going to be a very
> big mask. IMO you do want to correctly reflect your LLC situation via this
> flag to make cpus_share_cache() work properly.

I detect if the LLC is shared at BIGCORE, and if they are shared at BIGCORE,
then I dynamically rename the DOMAIN as CACHE and enable
SD_SHARE_PKG_RESOURCES in that domain.

> 
> [1]: https://linuxplumbersconf.org/event/4/contributions/484/

Thanks for the pointer.

-- 
Thanks and Regards
Srikar Dronamraju