* Re: [PATCH v2 09/11] powerpc/smp: Optimize update_mask_by_l2
From: Qian Cai @ 2020-10-07 14:23 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling, Peter Zijlstra,
LKML, Nicholas Piggin, Ingo Molnar, Oliver O'Halloran,
Satheesh Rajendran, linuxppc-dev, Valentin Schneider
In-Reply-To: <20201007141745.GM12031@linux.vnet.ibm.com>
On Wed, 2020-10-07 at 19:47 +0530, Srikar Dronamraju wrote:
> Can you confirm if CONFIG_CPUMASK_OFFSTACK is enabled in your config?
Yes, https://gitlab.com/cailca/linux-mm/-/blob/master/powerpc.config
We tested here almost daily on linux-next.
^ permalink raw reply
* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Jann Horn @ 2020-10-07 14:42 UTC (permalink / raw)
To: Christoph Hellwig, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev
Cc: Dave Kleikamp, Catalin Marinas, kernel list, Linux-MM,
Khalid Aziz, sparclinux, Anthony Yznaga, Andrew Morton,
Will Deacon, David S. Miller, Linux ARM
In-Reply-To: <20201007123544.GA11433@infradead.org>
On Wed, Oct 7, 2020 at 2:35 PM Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Oct 07, 2020 at 09:39:31AM +0200, Jann Horn wrote:
> > diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
> > index 078608ec2e92..b1fabb97d138 100644
> > --- a/arch/powerpc/kernel/syscalls.c
> > +++ b/arch/powerpc/kernel/syscalls.c
> > @@ -43,7 +43,7 @@ static inline long do_mmap2(unsigned long addr, size_t len,
> > {
> > long ret = -EINVAL;
> >
> > - if (!arch_validate_prot(prot, addr))
> > + if (!arch_validate_prot(prot, addr, len))
>
> This call isn't under mmap lock. I also find it rather weird as the
> generic code only calls arch_validate_prot from mprotect, only powerpc
> also calls it from mmap.
>
> This seems to go back to commit ef3d3246a0d0
> ("powerpc/mm: Add Strong Access Ordering support")
I'm _guessing_ the idea in the generic case might be that mmap()
doesn't check unknown bits in the protection flags, and therefore
maybe people wanted to avoid adding new error cases that could be
caused by random high bits being set? So while the mprotect() case
checks the flags and refuses unknown values, the mmap() code just lets
the architecture figure out which bits are actually valid to set (via
arch_calc_vm_prot_bits()) and silently ignores the rest?
And powerpc apparently decided that they do want to error out on bogus
prot values passed to their version of mmap(), and in exchange, assume
in arch_calc_vm_prot_bits() that the protection bits are valid?
powerpc's arch_validate_prot() doesn't actually need the mmap lock, so
I think this is fine-ish for now (as in, while the code is a bit
unclean, I don't think I'm making it worse, and I don't think it's
actually buggy). In theory, we could move the arch_validate_prot()
call over into the mmap guts, where we're holding the lock, and gate
it on the architecture or on some feature CONFIG that powerpc can
activate in its Kconfig. But I'm not sure whether that'd be helping or
making things worse, so when I sent this patch, I deliberately left
the powerpc stuff as-is.
^ permalink raw reply
* [PATCH v3 01/11] powerpc/topology: Update topology_core_cpumask
From: Srikar Dronamraju @ 2020-10-07 18:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Qian Cai, Satheesh Rajendran,
Oliver O'Halloran, linuxppc-dev, Ingo Molnar
In-Reply-To: <20201007183800.27415-1-srikar@linux.vnet.ibm.com>
On Power, cpu_core_mask and cpu_cpu_mask refer to the same set of CPUs.
cpu_cpu_mask is needed by scheduler, hence look at deprecating
cpu_core_mask. Before deleting the cpu_core_mask, ensure its only user
is moved to cpu_cpu_mask.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
---
arch/powerpc/include/asm/topology.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index 6609174918ab..e0f232533c9d 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -122,7 +122,7 @@ int get_physical_package_id(int cpu);
#endif
#define topology_sibling_cpumask(cpu) (per_cpu(cpu_sibling_map, cpu))
-#define topology_core_cpumask(cpu) (per_cpu(cpu_core_map, cpu))
+#define topology_core_cpumask(cpu) (cpu_cpu_mask(cpu))
#define topology_core_id(cpu) (cpu_to_core_id(cpu))
#endif
--
2.17.1
^ permalink raw reply related
* [PATCH v3 02/11] powerpc/smp: Stop updating cpu_core_mask
From: Srikar Dronamraju @ 2020-10-07 18:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Qian Cai, Satheesh Rajendran,
Oliver O'Halloran, linuxppc-dev, Ingo Molnar
In-Reply-To: <20201007183800.27415-1-srikar@linux.vnet.ibm.com>
Anton Blanchard reported that his 4096 vcpu KVM guest took around 30
minutes to boot. He also analyzed it to the time taken to iterate while
setting the cpu_core_mask.
Further analysis shows that cpu_core_mask and cpu_cpu_mask for any CPU
would be equal on Power. However updating cpu_core_mask took forever to
update as its a per cpu cpumask variable. Instead cpu_cpu_mask was a per
NODE /per DIE cpumask that was shared by all the respective CPUs.
Also cpu_cpu_mask is needed from a scheduler perspective. However
cpu_core_map is an exported symbol. Hence stop updating cpu_core_map
and make it point to cpu_cpu_mask.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
---
arch/powerpc/include/asm/smp.h | 5 -----
arch/powerpc/kernel/smp.c | 33 +++++++--------------------------
2 files changed, 7 insertions(+), 31 deletions(-)
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 041f0b97c45b..40e121dd16af 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -119,11 +119,6 @@ static inline struct cpumask *cpu_sibling_mask(int cpu)
return per_cpu(cpu_sibling_map, cpu);
}
-static inline struct cpumask *cpu_core_mask(int cpu)
-{
- return per_cpu(cpu_core_map, cpu);
-}
-
static inline struct cpumask *cpu_l2_cache_mask(int cpu)
{
return per_cpu(cpu_l2_cache_map, cpu);
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 3d96752d6570..ec41491beca4 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -953,12 +953,17 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
local_memory_node(numa_cpu_lookup_table[cpu]));
}
#endif
+ /*
+ * cpu_core_map is now more updated and exists only since
+ * its been exported for long. It only will have a snapshot
+ * of cpu_cpu_mask.
+ */
+ cpumask_copy(per_cpu(cpu_core_map, cpu), cpu_cpu_mask(cpu));
}
/* Init the cpumasks so the boot CPU is related to itself */
cpumask_set_cpu(boot_cpuid, cpu_sibling_mask(boot_cpuid));
cpumask_set_cpu(boot_cpuid, cpu_l2_cache_mask(boot_cpuid));
- cpumask_set_cpu(boot_cpuid, cpu_core_mask(boot_cpuid));
if (has_coregroup_support())
cpumask_set_cpu(boot_cpuid, cpu_coregroup_mask(boot_cpuid));
@@ -1260,9 +1265,7 @@ static void remove_cpu_from_masks(int cpu)
{
int i;
- /* NB: cpu_core_mask is a superset of the others */
- for_each_cpu(i, cpu_core_mask(cpu)) {
- set_cpus_unrelated(cpu, i, cpu_core_mask);
+ for_each_cpu(i, cpu_cpu_mask(cpu)) {
set_cpus_unrelated(cpu, i, cpu_l2_cache_mask);
set_cpus_unrelated(cpu, i, cpu_sibling_mask);
if (has_big_cores)
@@ -1312,7 +1315,6 @@ EXPORT_SYMBOL_GPL(get_physical_package_id);
static void add_cpu_to_masks(int cpu)
{
int first_thread = cpu_first_thread_sibling(cpu);
- int pkg_id = get_physical_package_id(cpu);
int i;
/*
@@ -1320,7 +1322,6 @@ static void add_cpu_to_masks(int cpu)
* add it to it's own thread sibling mask.
*/
cpumask_set_cpu(cpu, cpu_sibling_mask(cpu));
- cpumask_set_cpu(cpu, cpu_core_mask(cpu));
for (i = first_thread; i < first_thread + threads_per_core; i++)
if (cpu_online(i))
@@ -1342,26 +1343,6 @@ static void add_cpu_to_masks(int cpu)
set_cpus_related(cpu, i, cpu_coregroup_mask);
}
}
-
- if (pkg_id == -1) {
- struct cpumask *(*mask)(int) = cpu_sibling_mask;
-
- /*
- * Copy the sibling mask into core sibling mask and
- * mark any CPUs on the same chip as this CPU.
- */
- if (shared_caches)
- mask = cpu_l2_cache_mask;
-
- for_each_cpu(i, mask(cpu))
- set_cpus_related(cpu, i, cpu_core_mask);
-
- return;
- }
-
- for_each_cpu(i, cpu_online_mask)
- if (get_physical_package_id(i) == pkg_id)
- set_cpus_related(cpu, i, cpu_core_mask);
}
/* Activate a secondary processor. */
--
2.17.1
^ permalink raw reply related
* [PATCH v3 00/11] Optimization to improve CPU online/offline on Powerpc
From: Srikar Dronamraju @ 2020-10-07 18:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Qian Cai, Oliver O'Halloran,
Satheesh Rajendran, linuxppc-dev, Ingo Molnar
Changelog v2->v3:
v1 link: https://lore.kernel.org/linuxppc-dev/20200921095653.9701-1-srikar@linux.vnet.ibm.com/t/#u
Use GFP_ATOMIC instead of GFP_KERNEL since allocations need to
atomic at the time of CPU HotPlug.
Reported by Qian Cai <cai@redhat.com>
Only changes in Patch 09 and Patch 11.
Changelog v1->v2:
v1 link: https://lore.kernel.org/linuxppc-dev/20200727075532.30058-1-srikar@linux.vnet.ibm.com/t/#u
Added five more patches on top of Seven.
Rebased to 19th Sept 2020 powerpc/next (based on v5.9-rc2)
Here are some optimizations and fixes to make CPU online/offline
faster and hence result in faster bootup.
Its based on top of my v5 coregroup support patchset.
https://lore.kernel.org/linuxppc-dev/20200810071834.92514-1-srikar@linux.vnet.ibm.com/t/#u
Anton reported that his 4096 cpu (1024 cores in a socket) was taking too
long to boot. He also analyzed that most of the time was being spent on
updating cpu_core_mask.
The first two patches should solve Anton's immediate problem.
On the unofficial patches, Anton reported that the boot time came from 30
mins to 6 seconds. (Basically a high core count in a single socket
configuration). Satheesh also reported similar numbers.
The rest are cleanups/optimizations.
Since cpu_core_mask is an exported symbol for a long duration, lets retain
as a snapshot of cpumask_of_node.
$ lscpu
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 1024
On-line CPU(s) list: 0-1023
Thread(s) per core: 8
Core(s) per socket: 8
Socket(s): 16
NUMA node(s): 16
Model: 2.0 (pvr 004d 0200)
Model name: POWER8 (architected), altivec supported
Hypervisor vendor: pHyp
Virtualization type: para
L1d cache: 64K
L1i cache: 32K
L2 cache: 512K
L3 cache: 8192K
NUMA node0 CPU(s): 0-63
NUMA node1 CPU(s): 64-127
NUMA node2 CPU(s): 128-191
NUMA node3 CPU(s): 192-255
NUMA node4 CPU(s): 256-319
NUMA node5 CPU(s): 320-383
NUMA node6 CPU(s): 384-447
NUMA node7 CPU(s): 448-511
NUMA node8 CPU(s): 512-575
NUMA node9 CPU(s): 576-639
NUMA node10 CPU(s): 640-703
NUMA node11 CPU(s): 704-767
NUMA node12 CPU(s): 768-831
NUMA node13 CPU(s): 832-895
NUMA node14 CPU(s): 896-959
NUMA node15 CPU(s): 960-1023
$ dmesg -k | grep -i -e Bringing -e Brought -e sysrq -e bug
With powerp/next
[ 0.000000] printk: debug: ignoring loglevel setting.
[ 0.354971] smp: Bringing up secondary CPUs ...
[ 233.354676] smp: Brought up 16 nodes, 1024 CPUs
[ 330.023073] sysrq: Changing Loglevel
[ 330.023101] sysrq: Loglevel set to 9
With +patchset
[ 0.000000] printk: debug: ignoring loglevel setting.
[ 0.351703] smp: Bringing up secondary CPUs ...
[ 4.059859] smp: Brought up 16 nodes, 1024 CPUs
[ 98.309015] sysrq: Changing Loglevel
[ 98.309044] sysrq: Loglevel set to 9
Observations:
CPU bringup time reduced to 4 seconds from 233 seconds on this 1024 CPU
system. This resulted in System boot up time reducing to 98 seconds from
330 seconds. The actual improvement would depend on your system topology.
Topology verification post patchset on a 2 node Power9 PowerVM LPAR
powerpc/next +patchset
------------ ---------
$ lscpu
Architecture: ppc64le Architecture: ppc64le
Byte Order: Little Endian Byte Order: Little Endian
CPU(s): 128 CPU(s): 128
On-line CPU(s) list: 0-127 On-line CPU(s) list: 0-127
Thread(s) per core: 8 Thread(s) per core: 8
Core(s) per socket: 8 Core(s) per socket: 8
Socket(s): 2 Socket(s): 2
NUMA node(s): 2 NUMA node(s): 2
Model: 2.2 (pvr 004e 0202) Model: 2.2 (pvr 004e 0202)
Model name: POWER9 (architected), altivec supported Model name: POWER9 (architected), altivec supported
Hypervisor vendor: pHyp Hypervisor vendor: pHyp
Virtualization type: para Virtualization type: para
L1d cache: 32K L1d cache: 32K
L1i cache: 32K L1i cache: 32K
L2 cache: 512K L2 cache: 512K
L3 cache: 10240K L3 cache: 10240K
NUMA node0 CPU(s): 0-63 NUMA node0 CPU(s): 0-63
NUMA node1 CPU(s): 64-127 NUMA node1 CPU(s): 64-127
$ tail -f /proc/cpuinfo
processor : 127 processor : 127
cpu : POWER9 (architected), altivec supported cpu : POWER9 (architected), altivec supported
clock : 3000.000000MHz clock : 3000.000000MHz
revision : 2.2 (pvr 004e 0202) revision : 2.2 (pvr 004e 0202)
timebase : 512000000 timebase : 512000000
platform : pSeries platform : pSeries
model : IBM,9008-22L model : IBM,9008-22L
machine : CHRP IBM,9008-22L machine : CHRP IBM,9008-22L
MMU : Radix MMU : Radix
$ grep . /proc/sys/kernel/sched_domain/cpu0/domain*/name
--------------------------------------------------------
/proc/sys/kernel/sched_domain/cpu0/domain0/name:SMT /proc/sys/kernel/sched_domain/cpu0/domain0/name:SMT
/proc/sys/kernel/sched_domain/cpu0/domain1/name:CACHE /proc/sys/kernel/sched_domain/cpu0/domain1/name:CACHE
/proc/sys/kernel/sched_domain/cpu0/domain2/name:DIE /proc/sys/kernel/sched_domain/cpu0/domain2/name:DIE
/proc/sys/kernel/sched_domain/cpu0/domain3/name:NUMA /proc/sys/kernel/sched_domain/cpu0/domain3/name:NUMA
$ grep . /proc/sys/kernel/sched_domain/cpu0/domain*/flags
---------------------------------------------------------
/proc/sys/kernel/sched_domain/cpu0/domain0/flags:2391 /proc/sys/kernel/sched_domain/cpu0/domain0/flags:2391
/proc/sys/kernel/sched_domain/cpu0/domain1/flags:2327 /proc/sys/kernel/sched_domain/cpu0/domain1/flags:2327
/proc/sys/kernel/sched_domain/cpu0/domain2/flags:2071 /proc/sys/kernel/sched_domain/cpu0/domain2/flags:2071
/proc/sys/kernel/sched_domain/cpu0/domain3/flags:12801 /proc/sys/kernel/sched_domain/cpu0/domain3/flags:12801
Post ppc64_cpu --smt=1
$ tail -f /proc/cpuinfo
processor : 120 processor : 120
cpu : POWER9 (architected), altivec supported cpu : POWER9 (architected), altivec supported
clock : 3000.000000MHz clock : 3000.000000MHz
revision : 2.2 (pvr 004e 0202) revision : 2.2 (pvr 004e 0202)
timebase : 512000000 timebase : 512000000
platform : pSeries platform : pSeries
model : IBM,9008-22L model : IBM,9008-22L
machine : CHRP IBM,9008-22L machine : CHRP IBM,9008-22L
MMU : Radix MMU : Radix
$ grep . /proc/sys/kernel/sched_domain/cpu0/domain*/name
--------------------------------------------------------
/proc/sys/kernel/sched_domain/cpu0/domain0/name:DIE /proc/sys/kernel/sched_domain/cpu0/domain0/name:DIE
/proc/sys/kernel/sched_domain/cpu0/domain1/name:NUMA /proc/sys/kernel/sched_domain/cpu0/domain1/name:NUMA
$ grep . /proc/sys/kernel/sched_domain/cpu0/domain*/flags
---------------------------------------------------------
/proc/sys/kernel/sched_domain/cpu0/domain0/flags:2071 /proc/sys/kernel/sched_domain/cpu0/domain0/flags:2071
/proc/sys/kernel/sched_domain/cpu0/domain1/flags:12801 /proc/sys/kernel/sched_domain/cpu0/domain1/flags:12801
Performance impact post +patchset
---------------------------------
100 iterations of ebizzy
Units: Records/second : higher is better
-----------------------------------------
kernel N Min Max Median Avg Stddev
powerpc/next 100 753917 870520 819054 817636.56 22649.7
+patchset 100 746258 874984 816681 813876.74 26424.351
100 iterations of perf bench sched pipe -l 10000000 (aka Hackbench)
units: usec/ops: lesser is better
--------------------------------
kernel N Min Max Median Avg Stddev
powerpc/next 100 13.845834 14.569539 14.06263 14.086167 0.17512607
+patchset 100 13.637611 18.097744 13.862656 13.9257 0.43872453
schbench Latency percentiles (usec)
units: usec : lesser is better
-----------------------------------
powerpc/next +patchset
50.0000th: 48 50.0000th: 49
75.0000th: 65 75.0000th: 66
90.0000th: 77 90.0000th: 79
95.0000th: 84 95.0000th: 85
*99.0000th: 101 *99.0000th: 99
99.5000th: 113 99.5000th: 104
99.9000th: 159 99.9000th: 129
min=0, max=15221 min=0, max=7666
100 interations of ppc64_cpu --smt=1 / ppc64_cpu --smt=8
Units: seconds : lesser is better
---------------------------------
ppc64_cpu --smt=1
kernel N Min Max Median Avg Stddev
powerpc/next 100 13.39 17.55 14.71 14.7658 0.69184745
+patchset 100 13.3 16.27 14.33 14.4179 0.5427433
ppc64_cpu --smt=8
kernel N Min Max Median Avg Stddev
powerpc/next 100 21.65 26.17 23.71 23.7111 0.8589786
+patchset 100 21.88 25.79 23.16 23.2945 0.86394839
Observations:
Performance of ebizzy/ perf_sched_bench / schbench remain the
same with and without the patchset.
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
Srikar Dronamraju (11):
powerpc/topology: Update topology_core_cpumask
powerpc/smp: Stop updating cpu_core_mask
powerpc/smp: Remove get_physical_package_id
powerpc/smp: Optimize remove_cpu_from_masks
powerpc/smp: Limit CPUs traversed to within a node.
powerpc/smp: Stop passing mask to update_mask_by_l2
powerpc/smp: Depend on cpu_l1_cache_map when adding CPUs
powerpc/smp: Check for duplicate topologies and consolidate
powerpc/smp: Optimize update_mask_by_l2
powerpc/smp: Move coregroup mask updation to a new function
powerpc/smp: Optimize update_coregroup_mask
arch/powerpc/include/asm/smp.h | 5 -
arch/powerpc/include/asm/topology.h | 7 +-
arch/powerpc/kernel/smp.c | 188 +++++++++++++++++++++++-------------
3 files changed, 122 insertions(+), 78 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v3 04/11] powerpc/smp: Optimize remove_cpu_from_masks
From: Srikar Dronamraju @ 2020-10-07 18:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Qian Cai, Satheesh Rajendran,
Oliver O'Halloran, linuxppc-dev, Ingo Molnar
In-Reply-To: <20201007183800.27415-1-srikar@linux.vnet.ibm.com>
While offlining a CPU, system currently iterate through all the CPUs in
the DIE to clear sibling, l2_cache and smallcore maps. However if there
are more cores in a DIE, system can end up spending more time iterating
through CPUs which are completely unrelated.
Optimize this by only iterating through smaller but relevant cpumap.
If shared_cache is set, cpu_l2_cache_map should be relevant else
cpu_sibling_map would be relevant.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
---
arch/powerpc/kernel/smp.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 8c095fe237b2..2e61a81aad88 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1263,14 +1263,21 @@ static bool update_mask_by_l2(int cpu, struct cpumask *(*mask_fn)(int))
#ifdef CONFIG_HOTPLUG_CPU
static void remove_cpu_from_masks(int cpu)
{
+ struct cpumask *(*mask_fn)(int) = cpu_sibling_mask;
int i;
- for_each_cpu(i, cpu_cpu_mask(cpu)) {
+ if (shared_caches)
+ mask_fn = cpu_l2_cache_mask;
+
+ for_each_cpu(i, mask_fn(cpu)) {
set_cpus_unrelated(cpu, i, cpu_l2_cache_mask);
set_cpus_unrelated(cpu, i, cpu_sibling_mask);
if (has_big_cores)
set_cpus_unrelated(cpu, i, cpu_smallcore_mask);
- if (has_coregroup_support())
+ }
+
+ if (has_coregroup_support()) {
+ for_each_cpu(i, cpu_coregroup_mask(cpu))
set_cpus_unrelated(cpu, i, cpu_coregroup_mask);
}
}
--
2.17.1
^ permalink raw reply related
* [PATCH v3 03/11] powerpc/smp: Remove get_physical_package_id
From: Srikar Dronamraju @ 2020-10-07 18:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Qian Cai, Satheesh Rajendran,
Oliver O'Halloran, linuxppc-dev, Ingo Molnar
In-Reply-To: <20201007183800.27415-1-srikar@linux.vnet.ibm.com>
Now that cpu_core_mask has been removed and topology_core_cpumask has
been updated to use cpu_cpu_mask, we no more need
get_physical_package_id.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
---
arch/powerpc/include/asm/topology.h | 5 -----
arch/powerpc/kernel/smp.c | 20 --------------------
2 files changed, 25 deletions(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index e0f232533c9d..e45219f74be0 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -114,12 +114,7 @@ static inline int cpu_to_coregroup_id(int cpu)
#ifdef CONFIG_PPC64
#include <asm/smp.h>
-#ifdef CONFIG_PPC_SPLPAR
-int get_physical_package_id(int cpu);
-#define topology_physical_package_id(cpu) (get_physical_package_id(cpu))
-#else
#define topology_physical_package_id(cpu) (cpu_to_chip_id(cpu))
-#endif
#define topology_sibling_cpumask(cpu) (per_cpu(cpu_sibling_map, cpu))
#define topology_core_cpumask(cpu) (cpu_cpu_mask(cpu))
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index ec41491beca4..8c095fe237b2 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1292,26 +1292,6 @@ static inline void add_cpu_to_smallcore_masks(int cpu)
}
}
-int get_physical_package_id(int cpu)
-{
- int pkg_id = cpu_to_chip_id(cpu);
-
- /*
- * 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) &&
- 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);
- }
-
- return pkg_id;
-}
-EXPORT_SYMBOL_GPL(get_physical_package_id);
-
static void add_cpu_to_masks(int cpu)
{
int first_thread = cpu_first_thread_sibling(cpu);
--
2.17.1
^ permalink raw reply related
* [PATCH v3 05/11] powerpc/smp: Limit CPUs traversed to within a node.
From: Srikar Dronamraju @ 2020-10-07 18:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Qian Cai, Satheesh Rajendran,
Oliver O'Halloran, linuxppc-dev, Ingo Molnar
In-Reply-To: <20201007183800.27415-1-srikar@linux.vnet.ibm.com>
All the arch specific topology cpumasks are within a node/DIE.
However when setting these per CPU cpumasks, system traverses through
all the online CPUs. This is redundant.
Reduce the traversal to only CPUs that are online in the node to which
the CPU belongs to.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
---
arch/powerpc/kernel/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 2e61a81aad88..c860c4950c9f 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1241,7 +1241,7 @@ static bool update_mask_by_l2(int cpu, struct cpumask *(*mask_fn)(int))
}
cpumask_set_cpu(cpu, mask_fn(cpu));
- for_each_cpu(i, cpu_online_mask) {
+ for_each_cpu_and(i, cpu_online_mask, cpu_cpu_mask(cpu)) {
/*
* when updating the marks the current CPU has not been marked
* online, but we need to update the cache masks
--
2.17.1
^ permalink raw reply related
* [PATCH v3 07/11] powerpc/smp: Depend on cpu_l1_cache_map when adding CPUs
From: Srikar Dronamraju @ 2020-10-07 18:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Qian Cai, Satheesh Rajendran,
Oliver O'Halloran, linuxppc-dev, Ingo Molnar
In-Reply-To: <20201007183800.27415-1-srikar@linux.vnet.ibm.com>
Currently on hotplug/hotunplug, CPU iterates through all the CPUs in
its core to find threads in its thread group. However this info is
already captured in cpu_l1_cache_map. Hence reduce iterations and
cleanup add_cpu_to_smallcore_masks function.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
---
arch/powerpc/kernel/smp.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 441c9c64b1e3..aeb219a4bf7a 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1285,16 +1285,15 @@ static void remove_cpu_from_masks(int cpu)
static inline void add_cpu_to_smallcore_masks(int cpu)
{
- struct cpumask *this_l1_cache_map = per_cpu(cpu_l1_cache_map, cpu);
- int i, first_thread = cpu_first_thread_sibling(cpu);
+ int i;
if (!has_big_cores)
return;
cpumask_set_cpu(cpu, cpu_smallcore_mask(cpu));
- for (i = first_thread; i < first_thread + threads_per_core; i++) {
- if (cpu_online(i) && cpumask_test_cpu(i, this_l1_cache_map))
+ for_each_cpu(i, per_cpu(cpu_l1_cache_map, cpu)) {
+ if (cpu_online(i))
set_cpus_related(i, cpu, cpu_smallcore_mask);
}
}
--
2.17.1
^ permalink raw reply related
* [PATCH v3 08/11] powerpc/smp: Check for duplicate topologies and consolidate
From: Srikar Dronamraju @ 2020-10-07 18:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Qian Cai, Oliver O'Halloran,
Satheesh Rajendran, linuxppc-dev, Ingo Molnar
In-Reply-To: <20201007183800.27415-1-srikar@linux.vnet.ibm.com>
CACHE and COREGROUP domains are now part of default topology. However on
systems that don't support CACHE or COREGROUP, these domains will
eventually be degenerated. The degeneration happens per CPU. Do note the
current fixup_topology() logic ensures that mask of a domain that is not
supported on the current platform is set to the previous domain.
Instead of waiting for the scheduler to degenerated try to consolidate
based on their masks and sd_flags. This is done just before setting
the scheduler topology.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
---
arch/powerpc/kernel/smp.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index aeb219a4bf7a..6f866e6b12f8 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1401,6 +1401,8 @@ int setup_profiling_timer(unsigned int multiplier)
static void fixup_topology(void)
{
+ int i;
+
#ifdef CONFIG_SCHED_SMT
if (has_big_cores) {
pr_info("Big cores detected but using small core scheduling\n");
@@ -1410,6 +1412,30 @@ static void fixup_topology(void)
if (!has_coregroup_support())
powerpc_topology[mc_idx].mask = powerpc_topology[cache_idx].mask;
+
+ /*
+ * Try to consolidate topology levels here instead of
+ * allowing scheduler to degenerate.
+ * - Dont consolidate if masks are different.
+ * - Dont consolidate if sd_flags exists and are different.
+ */
+ for (i = 1; i <= die_idx; i++) {
+ if (powerpc_topology[i].mask != powerpc_topology[i - 1].mask)
+ continue;
+
+ if (powerpc_topology[i].sd_flags && powerpc_topology[i - 1].sd_flags &&
+ powerpc_topology[i].sd_flags != powerpc_topology[i - 1].sd_flags)
+ continue;
+
+ if (!powerpc_topology[i - 1].sd_flags)
+ powerpc_topology[i - 1].sd_flags = powerpc_topology[i].sd_flags;
+
+ powerpc_topology[i].mask = powerpc_topology[i + 1].mask;
+ powerpc_topology[i].sd_flags = powerpc_topology[i + 1].sd_flags;
+#ifdef CONFIG_SCHED_DEBUG
+ powerpc_topology[i].name = powerpc_topology[i + 1].name;
+#endif
+ }
}
void __init smp_cpus_done(unsigned int max_cpus)
--
2.17.1
^ permalink raw reply related
* [PATCH v3 10/11] powerpc/smp: Move coregroup mask updation to a new function
From: Srikar Dronamraju @ 2020-10-07 18:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Qian Cai, Oliver O'Halloran,
Satheesh Rajendran, linuxppc-dev, Ingo Molnar
In-Reply-To: <20201007183800.27415-1-srikar@linux.vnet.ibm.com>
Move the logic for updating the coregroup mask of a CPU to its own
function. This will help in reworking the updation of coregroup mask in
subsequent patch.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
---
arch/powerpc/kernel/smp.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 17e90c2414af..b48ae4e306d3 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1338,6 +1338,23 @@ static inline void add_cpu_to_smallcore_masks(int cpu)
}
}
+static void update_coregroup_mask(int cpu)
+{
+ int first_thread = cpu_first_thread_sibling(cpu);
+ int coregroup_id = cpu_to_coregroup_id(cpu);
+ int i;
+
+ cpumask_set_cpu(cpu, cpu_coregroup_mask(cpu));
+ for_each_cpu_and(i, cpu_online_mask, cpu_cpu_mask(cpu)) {
+ int fcpu = cpu_first_thread_sibling(i);
+
+ if (fcpu == first_thread)
+ set_cpus_related(cpu, i, cpu_coregroup_mask);
+ else if (coregroup_id == cpu_to_coregroup_id(i))
+ set_cpus_related(cpu, i, cpu_coregroup_mask);
+ }
+}
+
static void add_cpu_to_masks(int cpu)
{
int first_thread = cpu_first_thread_sibling(cpu);
@@ -1356,19 +1373,8 @@ static void add_cpu_to_masks(int cpu)
add_cpu_to_smallcore_masks(cpu);
update_mask_by_l2(cpu);
- if (has_coregroup_support()) {
- int coregroup_id = cpu_to_coregroup_id(cpu);
-
- cpumask_set_cpu(cpu, cpu_coregroup_mask(cpu));
- for_each_cpu_and(i, cpu_online_mask, cpu_cpu_mask(cpu)) {
- int fcpu = cpu_first_thread_sibling(i);
-
- if (fcpu == first_thread)
- set_cpus_related(cpu, i, cpu_coregroup_mask);
- else if (coregroup_id == cpu_to_coregroup_id(i))
- set_cpus_related(cpu, i, cpu_coregroup_mask);
- }
- }
+ if (has_coregroup_support())
+ update_coregroup_mask(cpu);
}
/* Activate a secondary processor. */
--
2.17.1
^ permalink raw reply related
* [PATCH v3 09/11] powerpc/smp: Optimize update_mask_by_l2
From: Srikar Dronamraju @ 2020-10-07 18:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Qian Cai, Oliver O'Halloran,
Satheesh Rajendran, linuxppc-dev, Ingo Molnar
In-Reply-To: <20201007183800.27415-1-srikar@linux.vnet.ibm.com>
All threads of a SMT4 core can either be part of this CPU's l2-cache
mask or not related to this CPU l2-cache mask. Use this relation to
reduce the number of iterations needed to find all the CPUs that share
the same l2-cache.
Use a temporary mask to iterate through the CPUs that may share l2_cache
mask. Also instead of setting one CPU at a time into cpu_l2_cache_mask,
copy the SMT4/sub mask at one shot.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
---
Changelog v2->v3:
Use GFP_ATOMIC instead of GFP_KERNEL since allocations need to
atomic at the time of CPU HotPlug
Reported by Qian Cai <cai@redhat.com>
arch/powerpc/kernel/smp.c | 52 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 46 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 6f866e6b12f8..17e90c2414af 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -671,6 +671,28 @@ static void set_cpus_unrelated(int i, int j,
#endif
/*
+ * Extends set_cpus_related. Instead of setting one CPU at a time in
+ * dstmask, set srcmask at oneshot. dstmask should be super set of srcmask.
+ */
+static void or_cpumasks_related(int i, int j, struct cpumask *(*srcmask)(int),
+ struct cpumask *(*dstmask)(int))
+{
+ struct cpumask *mask;
+ int k;
+
+ mask = srcmask(j);
+ for_each_cpu(k, srcmask(i))
+ cpumask_or(dstmask(k), dstmask(k), mask);
+
+ if (i == j)
+ return;
+
+ mask = srcmask(i);
+ for_each_cpu(k, srcmask(j))
+ cpumask_or(dstmask(k), dstmask(k), mask);
+}
+
+/*
* parse_thread_groups: Parses the "ibm,thread-groups" device tree
* property for the CPU device node @dn and stores
* the parsed output in the thread_groups
@@ -1220,7 +1242,9 @@ static struct device_node *cpu_to_l2cache(int cpu)
static bool update_mask_by_l2(int cpu)
{
+ struct cpumask *(*submask_fn)(int) = cpu_sibling_mask;
struct device_node *l2_cache, *np;
+ cpumask_var_t mask;
int i;
l2_cache = cpu_to_l2cache(cpu);
@@ -1240,22 +1264,38 @@ static bool update_mask_by_l2(int cpu)
return false;
}
- cpumask_set_cpu(cpu, cpu_l2_cache_mask(cpu));
- for_each_cpu_and(i, cpu_online_mask, cpu_cpu_mask(cpu)) {
+ /* In CPU-hotplug path, hence use GFP_ATOMIC */
+ alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu));
+ cpumask_and(mask, cpu_online_mask, cpu_cpu_mask(cpu));
+
+ if (has_big_cores)
+ submask_fn = cpu_smallcore_mask;
+
+ /* Update l2-cache mask with all the CPUs that are part of submask */
+ or_cpumasks_related(cpu, cpu, submask_fn, cpu_l2_cache_mask);
+
+ /* Skip all CPUs already part of current CPU l2-cache mask */
+ cpumask_andnot(mask, mask, cpu_l2_cache_mask(cpu));
+
+ for_each_cpu(i, mask) {
/*
* when updating the marks the current CPU has not been marked
* online, but we need to update the cache masks
*/
np = cpu_to_l2cache(i);
- if (!np)
- continue;
- if (np == l2_cache)
- set_cpus_related(cpu, i, cpu_l2_cache_mask);
+ /* Skip all CPUs already part of current CPU l2-cache */
+ if (np == l2_cache) {
+ or_cpumasks_related(cpu, i, submask_fn, cpu_l2_cache_mask);
+ cpumask_andnot(mask, mask, submask_fn(i));
+ } else {
+ cpumask_andnot(mask, mask, cpu_l2_cache_mask(i));
+ }
of_node_put(np);
}
of_node_put(l2_cache);
+ free_cpumask_var(mask);
return true;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v3 11/11] powerpc/smp: Optimize update_coregroup_mask
From: Srikar Dronamraju @ 2020-10-07 18:38 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Oliver O'Halloran, Satheesh Rajendran,
linuxppc-dev, Ingo Molnar
In-Reply-To: <20201007183800.27415-1-srikar@linux.vnet.ibm.com>
All threads of a SMT4/SMT8 core can either be part of CPU's coregroup
mask or outside the coregroup. Use this relation to reduce the
number of iterations needed to find all the CPUs that share the same
coregroup
Use a temporary mask to iterate through the CPUs that may share
coregroup mask. Also instead of setting one CPU at a time into
cpu_coregroup_mask, copy the SMT4/SMT8/submask at one shot.
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog v2->v3:
Use GFP_ATOMIC instead of GFP_KERNEL since allocations need to
atomic at the time of CPU HotPlug
Reported by Qian Cai <cai@redhat.com>
arch/powerpc/kernel/smp.c | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index b48ae4e306d3..bbaea93dc558 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1340,19 +1340,34 @@ static inline void add_cpu_to_smallcore_masks(int cpu)
static void update_coregroup_mask(int cpu)
{
- int first_thread = cpu_first_thread_sibling(cpu);
+ struct cpumask *(*submask_fn)(int) = cpu_sibling_mask;
+ cpumask_var_t mask;
int coregroup_id = cpu_to_coregroup_id(cpu);
int i;
- cpumask_set_cpu(cpu, cpu_coregroup_mask(cpu));
- for_each_cpu_and(i, cpu_online_mask, cpu_cpu_mask(cpu)) {
- int fcpu = cpu_first_thread_sibling(i);
+ /* In CPU-hotplug path, hence use GFP_ATOMIC */
+ alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu));
+ cpumask_and(mask, cpu_online_mask, cpu_cpu_mask(cpu));
+
+ if (shared_caches)
+ submask_fn = cpu_l2_cache_mask;
- if (fcpu == first_thread)
- set_cpus_related(cpu, i, cpu_coregroup_mask);
- else if (coregroup_id == cpu_to_coregroup_id(i))
- set_cpus_related(cpu, i, cpu_coregroup_mask);
+ /* Update coregroup mask with all the CPUs that are part of submask */
+ or_cpumasks_related(cpu, cpu, submask_fn, cpu_coregroup_mask);
+
+ /* Skip all CPUs already part of coregroup mask */
+ cpumask_andnot(mask, mask, cpu_coregroup_mask(cpu));
+
+ for_each_cpu(i, mask) {
+ /* Skip all CPUs not part of this coregroup */
+ if (coregroup_id == cpu_to_coregroup_id(i)) {
+ or_cpumasks_related(cpu, i, submask_fn, cpu_coregroup_mask);
+ cpumask_andnot(mask, mask, submask_fn(i));
+ } else {
+ cpumask_andnot(mask, mask, cpu_coregroup_mask(i));
+ }
}
+ free_cpumask_var(mask);
}
static void add_cpu_to_masks(int cpu)
--
2.17.1
^ permalink raw reply related
* [PATCH v3 06/11] powerpc/smp: Stop passing mask to update_mask_by_l2
From: Srikar Dronamraju @ 2020-10-07 18:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, LKML, Nicholas Piggin,
Valentin Schneider, Qian Cai, Oliver O'Halloran,
Satheesh Rajendran, linuxppc-dev, Ingo Molnar
In-Reply-To: <20201007183800.27415-1-srikar@linux.vnet.ibm.com>
update_mask_by_l2 is called only once. But it passes cpu_l2_cache_mask
as parameter. Instead of passing cpu_l2_cache_mask, use it directly in
update_mask_by_l2.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
---
arch/powerpc/kernel/smp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index c860c4950c9f..441c9c64b1e3 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1218,7 +1218,7 @@ static struct device_node *cpu_to_l2cache(int cpu)
return cache;
}
-static bool update_mask_by_l2(int cpu, struct cpumask *(*mask_fn)(int))
+static bool update_mask_by_l2(int cpu)
{
struct device_node *l2_cache, *np;
int i;
@@ -1240,7 +1240,7 @@ static bool update_mask_by_l2(int cpu, struct cpumask *(*mask_fn)(int))
return false;
}
- cpumask_set_cpu(cpu, mask_fn(cpu));
+ cpumask_set_cpu(cpu, cpu_l2_cache_mask(cpu));
for_each_cpu_and(i, cpu_online_mask, cpu_cpu_mask(cpu)) {
/*
* when updating the marks the current CPU has not been marked
@@ -1251,7 +1251,7 @@ static bool update_mask_by_l2(int cpu, struct cpumask *(*mask_fn)(int))
continue;
if (np == l2_cache)
- set_cpus_related(cpu, i, mask_fn);
+ set_cpus_related(cpu, i, cpu_l2_cache_mask);
of_node_put(np);
}
@@ -1315,7 +1315,7 @@ static void add_cpu_to_masks(int cpu)
set_cpus_related(i, cpu, cpu_sibling_mask);
add_cpu_to_smallcore_masks(cpu);
- update_mask_by_l2(cpu, cpu_l2_cache_mask);
+ update_mask_by_l2(cpu);
if (has_coregroup_support()) {
int coregroup_id = cpu_to_coregroup_id(cpu);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Khalid Aziz @ 2020-10-07 20:14 UTC (permalink / raw)
To: Jann Horn, David S. Miller, sparclinux, Andrew Morton, linux-mm
Cc: Catalin Marinas, linuxppc-dev, linux-kernel, Christoph Hellwig,
Paul Mackerras, Anthony Yznaga, Will Deacon, linux-arm-kernel
In-Reply-To: <20201007073932.865218-1-jannh@google.com>
On 10/7/20 1:39 AM, Jann Horn wrote:
> arch_validate_prot() is a hook that can validate whether a given set of
> protection flags is valid in an mprotect() operation. It is given the set
> of protection flags and the address being modified.
>
> However, the address being modified can currently not actually be used in
> a meaningful way because:
>
> 1. Only the address is given, but not the length, and the operation can
> span multiple VMAs. Therefore, the callee can't actually tell which
> virtual address range, or which VMAs, are being targeted.
> 2. The mmap_lock is not held, meaning that if the callee were to check
> the VMA at @addr, that VMA would be unrelated to the one the
> operation is performed on.
>
> Currently, custom arch_validate_prot() handlers are defined by
> arm64, powerpc and sparc.
> arm64 and powerpc don't care about the address range, they just check the
> flags against CPU support masks.
> sparc's arch_validate_prot() attempts to look at the VMA, but doesn't take
> the mmap_lock.
>
> Change the function signature to also take a length, and move the
> arch_validate_prot() call in mm/mprotect.c down into the locked region.
>
> Cc: stable@vger.kernel.org
> Fixes: 9035cf9a97e4 ("mm: Add address parameter to arch_validate_prot()")
> Suggested-by: Khalid Aziz <khalid.aziz@oracle.com>
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> arch/arm64/include/asm/mman.h | 4 ++--
> arch/powerpc/include/asm/mman.h | 3 ++-
> arch/powerpc/kernel/syscalls.c | 2 +-
> arch/sparc/include/asm/mman.h | 6 ++++--
> include/linux/mman.h | 3 ++-
> mm/mprotect.c | 6 ++++--
> 6 files changed, 15 insertions(+), 9 deletions(-)
This looks good to me.
As Chris pointed out, the call to arch_validate_prot() from do_mmap2()
is made without holding mmap_lock. Lock is not acquired until
vm_mmap_pgoff(). This variance is uncomfortable but I am more
uncomfortable forcing all implementations of validate_prot to require
mmap_lock be held when non-sparc implementations do not have such need
yet. Since do_mmap2() is in powerpc specific code, for now this patch
solves a current problem. That leaves open the question of should
generic mmap call arch_validate_prot and return EINVAL for invalid
combination of protection bits, but that is better addressed in a
separate patch.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
>
> diff --git a/arch/arm64/include/asm/mman.h b/arch/arm64/include/asm/mman.h
> index 081ec8de9ea6..0876a87986dc 100644
> --- a/arch/arm64/include/asm/mman.h
> +++ b/arch/arm64/include/asm/mman.h
> @@ -23,7 +23,7 @@ static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
> #define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
>
> static inline bool arch_validate_prot(unsigned long prot,
> - unsigned long addr __always_unused)
> + unsigned long addr __always_unused, unsigned long len __always_unused)
> {
> unsigned long supported = PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM;
>
> @@ -32,6 +32,6 @@ static inline bool arch_validate_prot(unsigned long prot,
>
> return (prot & ~supported) == 0;
> }
> -#define arch_validate_prot(prot, addr) arch_validate_prot(prot, addr)
> +#define arch_validate_prot(prot, addr, len) arch_validate_prot(prot, addr, len)
>
> #endif /* ! __ASM_MMAN_H__ */
> diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
> index 7cb6d18f5cd6..65dd9b594985 100644
> --- a/arch/powerpc/include/asm/mman.h
> +++ b/arch/powerpc/include/asm/mman.h
> @@ -36,7 +36,8 @@ static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
> }
> #define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
>
> -static inline bool arch_validate_prot(unsigned long prot, unsigned long addr)
> +static inline bool arch_validate_prot(unsigned long prot, unsigned long addr,
> + unsigned long len)
> {
> if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
> return false;
> diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
> index 078608ec2e92..b1fabb97d138 100644
> --- a/arch/powerpc/kernel/syscalls.c
> +++ b/arch/powerpc/kernel/syscalls.c
> @@ -43,7 +43,7 @@ static inline long do_mmap2(unsigned long addr, size_t len,
> {
> long ret = -EINVAL;
>
> - if (!arch_validate_prot(prot, addr))
> + if (!arch_validate_prot(prot, addr, len))
> goto out;
>
> if (shift) {
> diff --git a/arch/sparc/include/asm/mman.h b/arch/sparc/include/asm/mman.h
> index f94532f25db1..e85222c76585 100644
> --- a/arch/sparc/include/asm/mman.h
> +++ b/arch/sparc/include/asm/mman.h
> @@ -52,9 +52,11 @@ static inline pgprot_t sparc_vm_get_page_prot(unsigned long vm_flags)
> return (vm_flags & VM_SPARC_ADI) ? __pgprot(_PAGE_MCD_4V) : __pgprot(0);
> }
>
> -#define arch_validate_prot(prot, addr) sparc_validate_prot(prot, addr)
> -static inline int sparc_validate_prot(unsigned long prot, unsigned long addr)
> +#define arch_validate_prot(prot, addr, len) sparc_validate_prot(prot, addr, len)
> +static inline int sparc_validate_prot(unsigned long prot, unsigned long addr,
> + unsigned long len)
> {
> + mmap_assert_write_locked(current->mm);
> if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_ADI))
> return 0;
> if (prot & PROT_ADI) {
> diff --git a/include/linux/mman.h b/include/linux/mman.h
> index 6f34c33075f9..5b4d554d3189 100644
> --- a/include/linux/mman.h
> +++ b/include/linux/mman.h
> @@ -96,7 +96,8 @@ static inline void vm_unacct_memory(long pages)
> *
> * Returns true if the prot flags are valid
> */
> -static inline bool arch_validate_prot(unsigned long prot, unsigned long addr)
> +static inline bool arch_validate_prot(unsigned long prot, unsigned long addr,
> + unsigned long len)
> {
> return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
> }
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index ce8b8a5eacbb..e2d6b51acbf8 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -533,14 +533,16 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
> end = start + len;
> if (end <= start)
> return -ENOMEM;
> - if (!arch_validate_prot(prot, start))
> - return -EINVAL;
>
> reqprot = prot;
>
> if (mmap_write_lock_killable(current->mm))
> return -EINTR;
>
> + error = -EINVAL;
> + if (!arch_validate_prot(prot, start, len))
> + goto out;
> +
> /*
> * If userspace did not allocate the pkey, do not let
> * them use it here.
>
> base-commit: c85fb28b6f999db9928b841f63f1beeb3074eeca
>
^ permalink raw reply
* Re: [PATCH 2/2] sparc: Check VMA range in sparc_validate_prot()
From: Khalid Aziz @ 2020-10-07 20:15 UTC (permalink / raw)
To: Jann Horn, David S. Miller, sparclinux, Andrew Morton, linux-mm
Cc: Catalin Marinas, linuxppc-dev, linux-kernel, Christoph Hellwig,
Paul Mackerras, Anthony Yznaga, Will Deacon, linux-arm-kernel
In-Reply-To: <20201007073932.865218-2-jannh@google.com>
On 10/7/20 1:39 AM, Jann Horn wrote:
> sparc_validate_prot() is called from do_mprotect_pkey() as
> arch_validate_prot(); it tries to ensure that an mprotect() call can't
> enable ADI on incompatible VMAs.
> The current implementation only checks that the VMA at the start address
> matches the rules for ADI mappings; instead, check all VMAs that will be
> affected by mprotect().
>
> (This hook is called before mprotect() makes sure that the specified range
> is actually covered by VMAs, and mprotect() returns specific error codes
> when that's not the case. In order for mprotect() to still generate the
> same error codes for mprotect(<unmapped_ptr>, <len>, ...|PROT_ADI), we need
> to *accept* cases where the range is not fully covered by VMAs.)
>
> Cc: stable@vger.kernel.org
> Fixes: 74a04967482f ("sparc64: Add support for ADI (Application Data Integrity)")
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> compile-tested only, I don't have a Sparc ADI setup - might be nice if some
> Sparc person could test this?
>
> arch/sparc/include/asm/mman.h | 50 +++++++++++++++++++++--------------
> 1 file changed, 30 insertions(+), 20 deletions(-)
Looks good to me.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
>
> diff --git a/arch/sparc/include/asm/mman.h b/arch/sparc/include/asm/mman.h
> index e85222c76585..6dced75567c3 100644
> --- a/arch/sparc/include/asm/mman.h
> +++ b/arch/sparc/include/asm/mman.h
> @@ -60,31 +60,41 @@ static inline int sparc_validate_prot(unsigned long prot, unsigned long addr,
> if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_ADI))
> return 0;
> if (prot & PROT_ADI) {
> + struct vm_area_struct *vma, *next;
> +
> if (!adi_capable())
> return 0;
>
> - if (addr) {
> - struct vm_area_struct *vma;
> + vma = find_vma(current->mm, addr);
> + /* if @addr is unmapped, let mprotect() deal with it */
> + if (!vma || vma->vm_start > addr)
> + return 1;
> + while (1) {
> + /* ADI can not be enabled on PFN
> + * mapped pages
> + */
> + if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
> + return 0;
>
> - vma = find_vma(current->mm, addr);
> - if (vma) {
> - /* ADI can not be enabled on PFN
> - * mapped pages
> - */
> - if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
> - return 0;
> + /* Mergeable pages can become unmergeable
> + * if ADI is enabled on them even if they
> + * have identical data on them. This can be
> + * because ADI enabled pages with identical
> + * data may still not have identical ADI
> + * tags on them. Disallow ADI on mergeable
> + * pages.
> + */
> + if (vma->vm_flags & VM_MERGEABLE)
> + return 0;
>
> - /* Mergeable pages can become unmergeable
> - * if ADI is enabled on them even if they
> - * have identical data on them. This can be
> - * because ADI enabled pages with identical
> - * data may still not have identical ADI
> - * tags on them. Disallow ADI on mergeable
> - * pages.
> - */
> - if (vma->vm_flags & VM_MERGEABLE)
> - return 0;
> - }
> + /* reached the end of the range without errors? */
> + if (addr+len <= vma->vm_end)
> + return 1;
> + next = vma->vm_next;
> + /* if a VMA hole follows, let mprotect() deal with it */
> + if (!next || next->vm_start != vma->vm_end)
> + return 1;
> + vma = next;
> }
> }
> return 1;
>
^ permalink raw reply
* Re: [PATCH] powerpc/pci: Fix PHB removal/rescan on PowerNV
From: Oliver O'Halloran @ 2020-10-08 2:23 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: Alexey Kardashevskiy, linuxppc-dev, linux-pci
In-Reply-To: <20200925092258.525079-1-clg@kaod.org>
On Fri, Sep 25, 2020 at 7:23 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> To fix an issue with PHB hotplug on pSeries machine (HPT/XIVE), commit
> 3a3181e16fbd introduced a PPC specific pcibios_remove_bus() routine to
> clear all interrupt mappings when the bus is removed. This routine
> frees an array allocated in pcibios_scan_phb().
>
> This broke PHB hotplug on PowerNV because, when a PHB is removed and
> re-scanned through sysfs, the PCI layer un-assigns and re-assigns
> resources to the PHB but does not destroy and recreate the PCI
> controller structure. Since pcibios_remove_bus() does not clear the
> 'irq_map' array pointer, a second removal of the PHB will try to free
> the array a second time and corrupt memory.
"PHB hotplug" and "hot-plugging devices under a PHB" are different
things. What you're saying here doesn't make a whole lot of sense to
me unless you're conflating the two. The distinction is important
since on pseries we can use DLPAR to add and remove actual PHBs (i.e.
the pci_controller) at runtime, but there's no corresponding mechanism
on PowerNV.
> Free the 'irq_map' array in pcibios_free_controller() to fix
> corruption and clear interrupt mapping after it has been
> disposed. This to avoid filling up the array with successive
> remove/rescan of a bus.
Even with this patch I think we're still broken. With this patch
applied the init path is something like:
per-phb init:
allocate phb->irq_map array
per-bus init:
nothing
per-device init:
pcibios_bus_add_device()
pci_read_irq_line()
pci_irq_map_register(pci_dev, virq)
*record the device's interrupt in phb->irq_map*
And the teardown path:
per-device teardown:
nothing
per-bus teardown:
pcibios_remove_bus()
pci_irq_map_dispose()
*walk phb->irq_map and dispose of each mapped interrupt*
per-phb teardown:
free(phb->irq_map)
There's a lot of asymmetry here, which is a problem in itself, but the
real issue is that when removing *any* pci_bus under a PHB we dispose
of the LSI\ for *every* device under that PHB. Not good.
Ideally we should be fixing this by having the per-device teardown
handle disposing the mapping. Unfortunately, there's no pcibios hook
that's called when removing a pci_dev. However, we can register a bus
notifier which will be called when the pci_dev is removed from its bus
and we already do that for the per-device EEH teardown and to handle
IOMMU TCE invalidation when the device is removed.
^ permalink raw reply
* [PATCH] powerpc/smp: Use GFP_ATOMIC while allocating tmp mask
From: Srikar Dronamraju @ 2020-10-08 3:42 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Srikar Dronamraju, Qian Cai, LKML,
Valentin Schneider, Peter Zijlstra, linuxppc-dev, Ingo Molnar
Qian Cai reported a regression where CPU Hotplug fails with the latest
powerpc/next
BUG: sleeping function called from invalid context at mm/slab.h:494
in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 0, name: swapper/88
no locks held by swapper/88/0.
irq event stamp: 18074448
hardirqs last enabled at (18074447): [<c0000000001a2a7c>] tick_nohz_idle_enter+0x9c/0x110
hardirqs last disabled at (18074448): [<c000000000106798>] do_idle+0x138/0x3b0
do_idle at kernel/sched/idle.c:253 (discriminator 1)
softirqs last enabled at (18074440): [<c0000000000bbec4>] irq_enter_rcu+0x94/0xa0
softirqs last disabled at (18074439): [<c0000000000bbea0>] irq_enter_rcu+0x70/0xa0
CPU: 88 PID: 0 Comm: swapper/88 Tainted: G W 5.9.0-rc8-next-20201007 #1
Call Trace:
[c00020000a4bfcf0] [c000000000649e98] dump_stack+0xec/0x144 (unreliable)
[c00020000a4bfd30] [c0000000000f6c34] ___might_sleep+0x2f4/0x310
[c00020000a4bfdb0] [c000000000354f94] slab_pre_alloc_hook.constprop.82+0x124/0x190
[c00020000a4bfe00] [c00000000035e9e8] __kmalloc_node+0x88/0x3a0
slab_alloc_node at mm/slub.c:2817
(inlined by) __kmalloc_node at mm/slub.c:4013
[c00020000a4bfe80] [c0000000006494d8] alloc_cpumask_var_node+0x38/0x80
kmalloc_node at include/linux/slab.h:577
(inlined by) alloc_cpumask_var_node at lib/cpumask.c:116
[c00020000a4bfef0] [c00000000003eedc] start_secondary+0x27c/0x800
update_mask_by_l2 at arch/powerpc/kernel/smp.c:1267
(inlined by) add_cpu_to_masks at arch/powerpc/kernel/smp.c:1387
(inlined by) start_secondary at arch/powerpc/kernel/smp.c:1420
[c00020000a4bff90] [c00000000000c468] start_secondary_resume+0x10/0x14
Allocating a temporary mask while performing a CPU Hotplug operation
with CONFIG_CPUMASK_OFFSTACK enabled, leads to calling a sleepable
function from a atomic context. Fix this by allocating the temporary
mask with GFP_ATOMIC flag.
If there is a failure to allocate a mask, scheduler is going to observe
that this CPU's topology is broken. Instead of having to speculate why
the topology is broken, add a WARN_ON_ONCE.
Fixes: 70a94089d7f7 ("powerpc/smp: Optimize update_coregroup_mask")
Fixes: 3ab33d6dc3e9 ("powerpc/smp: Optimize update_mask_by_l2")
Reported-by: Qian Cai <cai@redhat.com>
Suggested-by: Qian Cai <cai@redhat.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qian Cai <cai@redhat.com>
---
arch/powerpc/kernel/smp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 0dc1b85..1268558 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1264,7 +1264,8 @@ static bool update_mask_by_l2(int cpu)
return false;
}
- alloc_cpumask_var_node(&mask, GFP_KERNEL, cpu_to_node(cpu));
+ /* In CPU-hotplug path, hence use GFP_ATOMIC */
+ WARN_ON_ONCE(!alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu)));
cpumask_and(mask, cpu_online_mask, cpu_cpu_mask(cpu));
if (has_big_cores)
@@ -1344,7 +1345,8 @@ static void update_coregroup_mask(int cpu)
int coregroup_id = cpu_to_coregroup_id(cpu);
int i;
- alloc_cpumask_var_node(&mask, GFP_KERNEL, cpu_to_node(cpu));
+ /* In CPU-hotplug path, hence use GFP_ATOMIC */
+ WARN_ON_ONCE(!alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu)));
cpumask_and(mask, cpu_online_mask, cpu_cpu_mask(cpu));
if (shared_caches)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] powerpc/pci: Fix PHB removal/rescan on PowerNV
From: Cédric Le Goater @ 2020-10-08 4:37 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: Alexey Kardashevskiy, linuxppc-dev, linux-pci
In-Reply-To: <CAOSf1CGW7ocYm2BXFiy9Nmi+G+xwVcqQzTqPo_nss_tmpG_V=w@mail.gmail.com>
On 10/8/20 4:23 AM, Oliver O'Halloran wrote:
> On Fri, Sep 25, 2020 at 7:23 PM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> To fix an issue with PHB hotplug on pSeries machine (HPT/XIVE), commit
>> 3a3181e16fbd introduced a PPC specific pcibios_remove_bus() routine to
>> clear all interrupt mappings when the bus is removed. This routine
>> frees an array allocated in pcibios_scan_phb().
>>
>> This broke PHB hotplug on PowerNV because, when a PHB is removed and
>> re-scanned through sysfs, the PCI layer un-assigns and re-assigns
>> resources to the PHB but does not destroy and recreate the PCI
>> controller structure. Since pcibios_remove_bus() does not clear the
>> 'irq_map' array pointer, a second removal of the PHB will try to free
>> the array a second time and corrupt memory.
>
> "PHB hotplug" and "hot-plugging devices under a PHB" are different
> things. What you're saying here doesn't make a whole lot of sense to
> me unless you're conflating the two. The distinction is important
> since on pseries we can use DLPAR to add and remove actual PHBs (i.e.
> the pci_controller) at runtime, but there's no corresponding mechanism
> on PowerNV.
And it's even different on QEMU.
>> Free the 'irq_map' array in pcibios_free_controller() to fix
>> corruption and clear interrupt mapping after it has been
>> disposed. This to avoid filling up the array with successive
>> remove/rescan of a bus.
>
> Even with this patch I think we're still broken. With this patch
> applied the init path is something like:
>
> per-phb init:
> allocate phb->irq_map array
> per-bus init:
> nothing
> per-device init:
> pcibios_bus_add_device()
> pci_read_irq_line()
> pci_irq_map_register(pci_dev, virq)
> *record the device's interrupt in phb->irq_map*
>
> And the teardown path:
>
> per-device teardown:
> nothing
> per-bus teardown:
> pcibios_remove_bus()
> pci_irq_map_dispose()
> *walk phb->irq_map and dispose of each mapped interrupt*
> per-phb teardown:
> free(phb->irq_map)
>
> There's a lot of asymmetry here, which is a problem in itself, but the
> real issue is that when removing *any* pci_bus under a PHB we dispose
> of the LSI\ for *every* device under that PHB. Not good.
>
> Ideally we should be fixing this by having the per-device teardown
> handle disposing the mapping. Unfortunately, there's no pcibios hook
> that's called when removing a pci_dev. However, we can register a bus
> notifier which will be called when the pci_dev is removed from its bus
> and we already do that for the per-device EEH teardown and to handle
> IOMMU TCE invalidation when the device is removed.
I lack the knowledge here and I think some else should take over,
as I am not doing a good job.
Michael, can you drop the initial patch again :/ It is better not
to merge anything.
Thanks,
C.
^ permalink raw reply
* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Christoph Hellwig @ 2020-10-08 6:21 UTC (permalink / raw)
To: Jann Horn
Cc: Dave Kleikamp, Will Deacon, Linux-MM, kernel list,
Christoph Hellwig, Catalin Marinas, Khalid Aziz, Paul Mackerras,
sparclinux, Anthony Yznaga, Andrew Morton, linuxppc-dev,
David S. Miller, Linux ARM
In-Reply-To: <CAG48ez3kjTeVtQcjQerYYRs7sX5qq3O7SU-FEaYLNXisFmAeOg@mail.gmail.com>
On Wed, Oct 07, 2020 at 04:42:55PM +0200, Jann Horn wrote:
> > > @@ -43,7 +43,7 @@ static inline long do_mmap2(unsigned long addr, size_t len,
> > > {
> > > long ret = -EINVAL;
> > >
> > > - if (!arch_validate_prot(prot, addr))
> > > + if (!arch_validate_prot(prot, addr, len))
> >
> > This call isn't under mmap lock. I also find it rather weird as the
> > generic code only calls arch_validate_prot from mprotect, only powerpc
> > also calls it from mmap.
> >
> > This seems to go back to commit ef3d3246a0d0
> > ("powerpc/mm: Add Strong Access Ordering support")
>
> I'm _guessing_ the idea in the generic case might be that mmap()
> doesn't check unknown bits in the protection flags, and therefore
> maybe people wanted to avoid adding new error cases that could be
> caused by random high bits being set? So while the mprotect() case
> checks the flags and refuses unknown values, the mmap() code just lets
> the architecture figure out which bits are actually valid to set (via
> arch_calc_vm_prot_bits()) and silently ignores the rest?
>
> And powerpc apparently decided that they do want to error out on bogus
> prot values passed to their version of mmap(), and in exchange, assume
> in arch_calc_vm_prot_bits() that the protection bits are valid?
The problem really is that now programs behave different on powerpc
compared to all other architectures.
> powerpc's arch_validate_prot() doesn't actually need the mmap lock, so
> I think this is fine-ish for now (as in, while the code is a bit
> unclean, I don't think I'm making it worse, and I don't think it's
> actually buggy). In theory, we could move the arch_validate_prot()
> call over into the mmap guts, where we're holding the lock, and gate
> it on the architecture or on some feature CONFIG that powerpc can
> activate in its Kconfig. But I'm not sure whether that'd be helping or
> making things worse, so when I sent this patch, I deliberately left
> the powerpc stuff as-is.
For now I'd just duplicate the trivial logic from arch_validate_prot
in the powerpc version of do_mmap2 and add a comment that this check
causes a gratious incompatibility to all other architectures. And then
hope that the powerpc maintainers fix it up :)
^ permalink raw reply
* [PATCH v4 1/4] powerpc/sstep: Emulate prefixed instructions only when CPU_FTR_ARCH_31 is set
From: Ravi Bangoria @ 2020-10-08 7:27 UTC (permalink / raw)
To: mpe; +Cc: ravi.bangoria, bala24, paulus, sandipan, naveen.n.rao,
linuxppc-dev
In-Reply-To: <20201008072726.233086-1-ravi.bangoria@linux.ibm.com>
From: Balamuruhan S <bala24@linux.ibm.com>
Unconditional emulation of prefixed instructions will allow
emulation of them on Power10 predecessors which might cause
issues. Restrict that.
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/lib/sstep.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index e9dcaba9a4f8..e6242744c71b 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1346,6 +1346,9 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
switch (opcode) {
#ifdef __powerpc64__
case 1:
+ if (!cpu_has_feature(CPU_FTR_ARCH_31))
+ return -1;
+
prefix_r = GET_PREFIX_R(word);
ra = GET_PREFIX_RA(suffix);
rd = (suffix >> 21) & 0x1f;
@@ -2733,6 +2736,9 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
}
break;
case 1: /* Prefixed instructions */
+ if (!cpu_has_feature(CPU_FTR_ARCH_31))
+ return -1;
+
prefix_r = GET_PREFIX_R(word);
ra = GET_PREFIX_RA(suffix);
op->update_reg = ra;
--
2.26.2
^ permalink raw reply related
* [PATCH v4 0/4] powerpc/sstep: VSX 32-byte vector paired load/store instructions
From: Ravi Bangoria @ 2020-10-08 7:27 UTC (permalink / raw)
To: mpe; +Cc: ravi.bangoria, bala24, paulus, sandipan, naveen.n.rao,
linuxppc-dev
VSX vector paired instructions operates with octword (32-byte)
operand for loads and stores between storage and a pair of two
sequential Vector-Scalar Registers (VSRs). There are 4 word
instructions and 2 prefixed instructions that provides this
32-byte storage access operations - lxvp, lxvpx, stxvp, stxvpx,
plxvp, pstxvp.
Emulation infrastructure doesn't have support for these instructions,
to operate with 32-byte storage access and to operate with 2 VSX
registers. This patch series enables the instruction emulation
support and adds test cases for them respectively.
v3: https://lore.kernel.org/linuxppc-dev/20200731081637.1837559-1-bala24@linux.ibm.com/
Changes in v4:
-------------
* Patch #1 is (kind of) new.
* Patch #2 now enables both analyse_instr() and emulate_step()
unlike prev series where both were in separate patches.
* Patch #2 also has important fix for emulation on LE.
* Patch #3 and #4. Added XSP/XTP, D0/D1 instruction operands,
removed *_EX_OP, __PPC_T[P][X] macros which are incorrect,
and adhered to PPC_RAW_* convention.
* Added `CPU_FTR_ARCH_31` check in testcases to avoid failing
in p8/p9.
* Some consmetic changes.
* Rebased to powerpc/next
Changes in v3:
-------------
Worked on review comments and suggestions from Ravi and Naveen,
* Fix the do_vsx_load() to handle vsx instructions if MSR_FP/MSR_VEC
cleared in exception conditions and it reaches to read/write to
thread_struct member fp_state/vr_state respectively.
* Fix wrongly used `__vector128 v[2]` in struct vsx_reg as it should
hold a single vsx register size.
* Remove unnecessary `VSX_CHECK_VEC` flag set and condition to check
`VSX_LDLEFT` that is not applicable for these vsx instructions.
* Fix comments in emulate_vsx_load() that were misleading.
* Rebased on latest powerpc next branch.
Changes in v2:
-------------
* Fix suggestion from Sandipan, wrap ISA 3.1 instructions with
cpu_has_feature(CPU_FTR_ARCH_31) check.
* Rebase on latest powerpc next branch.
Balamuruhan S (4):
powerpc/sstep: Emulate prefixed instructions only when CPU_FTR_ARCH_31
is set
powerpc/sstep: Support VSX vector paired storage access instructions
powerpc/ppc-opcode: Add encoding macros for VSX vector paired
instructions
powerpc/sstep: Add testcases for VSX vector paired load/store
instructions
arch/powerpc/include/asm/ppc-opcode.h | 13 ++
arch/powerpc/lib/sstep.c | 152 +++++++++++++--
arch/powerpc/lib/test_emulate_step.c | 270 ++++++++++++++++++++++++++
3 files changed, 414 insertions(+), 21 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH v4 2/4] powerpc/sstep: Support VSX vector paired storage access instructions
From: Ravi Bangoria @ 2020-10-08 7:27 UTC (permalink / raw)
To: mpe; +Cc: ravi.bangoria, bala24, paulus, sandipan, naveen.n.rao,
linuxppc-dev
In-Reply-To: <20201008072726.233086-1-ravi.bangoria@linux.ibm.com>
From: Balamuruhan S <bala24@linux.ibm.com>
VSX Vector Paired instructions loads/stores an octword (32 bytes)
from/to storage into two sequential VSRs. Add emulation support
for these new instructions:
* Load VSX Vector Paired (lxvp)
* Load VSX Vector Paired Indexed (lxvpx)
* Prefixed Load VSX Vector Paired (plxvp)
* Store VSX Vector Paired (stxvp)
* Store VSX Vector Paired Indexed (stxvpx)
* Prefixed Store VSX Vector Paired (pstxvp)
Suggested-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/lib/sstep.c | 146 +++++++++++++++++++++++++++++++++------
1 file changed, 125 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index e6242744c71b..e39ee1651636 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -32,6 +32,10 @@ extern char system_call_vectored_emulate[];
#define XER_OV32 0x00080000U
#define XER_CA32 0x00040000U
+#ifdef CONFIG_VSX
+#define VSX_REGISTER_XTP(rd) ((((rd) & 1) << 5) | ((rd) & 0xfe))
+#endif
+
#ifdef CONFIG_PPC_FPU
/*
* Functions in ldstfp.S
@@ -279,6 +283,19 @@ static nokprobe_inline void do_byte_reverse(void *ptr, int nb)
up[1] = tmp;
break;
}
+ case 32: {
+ unsigned long *up = (unsigned long *)ptr;
+ unsigned long tmp;
+
+ tmp = byterev_8(up[0]);
+ up[0] = byterev_8(up[3]);
+ up[3] = tmp;
+ tmp = byterev_8(up[2]);
+ up[2] = byterev_8(up[1]);
+ up[1] = tmp;
+ break;
+ }
+
#endif
default:
WARN_ON_ONCE(1);
@@ -709,6 +726,8 @@ void emulate_vsx_load(struct instruction_op *op, union vsx_reg *reg,
reg->d[0] = reg->d[1] = 0;
switch (op->element_size) {
+ case 32:
+ /* [p]lxvp[x] */
case 16:
/* whole vector; lxv[x] or lxvl[l] */
if (size == 0)
@@ -717,7 +736,7 @@ void emulate_vsx_load(struct instruction_op *op, union vsx_reg *reg,
if (IS_LE && (op->vsx_flags & VSX_LDLEFT))
rev = !rev;
if (rev)
- do_byte_reverse(reg, 16);
+ do_byte_reverse(reg, size);
break;
case 8:
/* scalar loads, lxvd2x, lxvdsx */
@@ -793,6 +812,20 @@ void emulate_vsx_store(struct instruction_op *op, const union vsx_reg *reg,
size = GETSIZE(op->type);
switch (op->element_size) {
+ case 32:
+ /* [p]stxvp[x] */
+ if (size == 0)
+ break;
+ if (rev) {
+ /* reverse 32 bytes */
+ buf.d[0] = byterev_8(reg->d[3]);
+ buf.d[1] = byterev_8(reg->d[2]);
+ buf.d[2] = byterev_8(reg->d[1]);
+ buf.d[3] = byterev_8(reg->d[0]);
+ reg = &buf;
+ }
+ memcpy(mem, reg, size);
+ break;
case 16:
/* stxv, stxvx, stxvl, stxvll */
if (size == 0)
@@ -861,28 +894,43 @@ static nokprobe_inline int do_vsx_load(struct instruction_op *op,
bool cross_endian)
{
int reg = op->reg;
- u8 mem[16];
- union vsx_reg buf;
+ int i, j, nr_vsx_regs;
+ u8 mem[32];
+ union vsx_reg buf[2];
int size = GETSIZE(op->type);
if (!address_ok(regs, ea, size) || copy_mem_in(mem, ea, size, regs))
return -EFAULT;
- emulate_vsx_load(op, &buf, mem, cross_endian);
+ nr_vsx_regs = size / sizeof(__vector128);
+ emulate_vsx_load(op, buf, mem, cross_endian);
preempt_disable();
if (reg < 32) {
/* FP regs + extensions */
if (regs->msr & MSR_FP) {
- load_vsrn(reg, &buf);
+ for (i = 0; i < nr_vsx_regs; i++) {
+ j = IS_LE ? nr_vsx_regs - i - 1 : i;
+ load_vsrn(reg + i, &buf[j].v);
+ }
} else {
- current->thread.fp_state.fpr[reg][0] = buf.d[0];
- current->thread.fp_state.fpr[reg][1] = buf.d[1];
+ for (i = 0; i < nr_vsx_regs; i++) {
+ j = IS_LE ? nr_vsx_regs - i - 1 : i;
+ current->thread.fp_state.fpr[reg + i][0] = buf[j].d[0];
+ current->thread.fp_state.fpr[reg + i][1] = buf[j].d[1];
+ }
}
} else {
- if (regs->msr & MSR_VEC)
- load_vsrn(reg, &buf);
- else
- current->thread.vr_state.vr[reg - 32] = buf.v;
+ if (regs->msr & MSR_VEC) {
+ for (i = 0; i < nr_vsx_regs; i++) {
+ j = IS_LE ? nr_vsx_regs - i - 1 : i;
+ load_vsrn(reg + i, &buf[j].v);
+ }
+ } else {
+ for (i = 0; i < nr_vsx_regs; i++) {
+ j = IS_LE ? nr_vsx_regs - i - 1 : i;
+ current->thread.vr_state.vr[reg - 32 + i] = buf[j].v;
+ }
+ }
}
preempt_enable();
return 0;
@@ -893,30 +941,45 @@ static nokprobe_inline int do_vsx_store(struct instruction_op *op,
bool cross_endian)
{
int reg = op->reg;
- u8 mem[16];
- union vsx_reg buf;
+ int i, j, nr_vsx_regs;
+ u8 mem[32];
+ union vsx_reg buf[2];
int size = GETSIZE(op->type);
if (!address_ok(regs, ea, size))
return -EFAULT;
+ nr_vsx_regs = size / sizeof(__vector128);
preempt_disable();
if (reg < 32) {
/* FP regs + extensions */
if (regs->msr & MSR_FP) {
- store_vsrn(reg, &buf);
+ for (i = 0; i < nr_vsx_regs; i++) {
+ j = IS_LE ? nr_vsx_regs - i - 1 : i;
+ store_vsrn(reg + i, &buf[j].v);
+ }
} else {
- buf.d[0] = current->thread.fp_state.fpr[reg][0];
- buf.d[1] = current->thread.fp_state.fpr[reg][1];
+ for (i = 0; i < nr_vsx_regs; i++) {
+ j = IS_LE ? nr_vsx_regs - i - 1 : i;
+ buf[j].d[0] = current->thread.fp_state.fpr[reg + i][0];
+ buf[j].d[1] = current->thread.fp_state.fpr[reg + i][1];
+ }
}
} else {
- if (regs->msr & MSR_VEC)
- store_vsrn(reg, &buf);
- else
- buf.v = current->thread.vr_state.vr[reg - 32];
+ if (regs->msr & MSR_VEC) {
+ for (i = 0; i < nr_vsx_regs; i++) {
+ j = IS_LE ? nr_vsx_regs - i - 1 : i;
+ store_vsrn(reg + i, &buf[j].v);
+ }
+ } else {
+ for (i = 0; i < nr_vsx_regs; i++) {
+ j = IS_LE ? nr_vsx_regs - i - 1 : i;
+ buf[j].v = current->thread.vr_state.vr[reg - 32 + i];
+ }
+ }
}
preempt_enable();
- emulate_vsx_store(op, &buf, mem, cross_endian);
+ emulate_vsx_store(op, buf, mem, cross_endian);
return copy_mem_out(mem, ea, size, regs);
}
#endif /* CONFIG_VSX */
@@ -2403,6 +2466,14 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
op->vsx_flags = VSX_SPLAT;
break;
+ case 333: /* lxvpx */
+ if (!cpu_has_feature(CPU_FTR_ARCH_31))
+ return -1;
+ op->reg = VSX_REGISTER_XTP(rd);
+ op->type = MKOP(LOAD_VSX, 0, 32);
+ op->element_size = 32;
+ break;
+
case 364: /* lxvwsx */
op->reg = rd | ((word & 1) << 5);
op->type = MKOP(LOAD_VSX, 0, 4);
@@ -2431,6 +2502,13 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
VSX_CHECK_VEC;
break;
}
+ case 461: /* stxvpx */
+ if (!cpu_has_feature(CPU_FTR_ARCH_31))
+ return -1;
+ op->reg = VSX_REGISTER_XTP(rd);
+ op->type = MKOP(STORE_VSX, 0, 32);
+ op->element_size = 32;
+ break;
case 524: /* lxsspx */
op->reg = rd | ((word & 1) << 5);
op->type = MKOP(LOAD_VSX, 0, 4);
@@ -2672,6 +2750,22 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
#endif
#ifdef CONFIG_VSX
+ case 6:
+ if (!cpu_has_feature(CPU_FTR_ARCH_31))
+ return -1;
+ op->ea = dqform_ea(word, regs);
+ op->reg = VSX_REGISTER_XTP(rd);
+ op->element_size = 32;
+ switch (word & 0xf) {
+ case 0: /* lxvp */
+ op->type = MKOP(LOAD_VSX, 0, 32);
+ break;
+ case 1: /* stxvp */
+ op->type = MKOP(STORE_VSX, 0, 32);
+ break;
+ }
+ break;
+
case 61: /* stfdp, lxv, stxsd, stxssp, stxv */
switch (word & 7) {
case 0: /* stfdp with LSB of DS field = 0 */
@@ -2803,12 +2897,22 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
case 57: /* pld */
op->type = MKOP(LOAD, PREFIXED, 8);
break;
+ case 58: /* plxvp */
+ op->reg = VSX_REGISTER_XTP(rd);
+ op->type = MKOP(LOAD_VSX, PREFIXED, 32);
+ op->element_size = 32;
+ break;
case 60: /* stq */
op->type = MKOP(STORE, PREFIXED, 16);
break;
case 61: /* pstd */
op->type = MKOP(STORE, PREFIXED, 8);
break;
+ case 62: /* pstxvp */
+ op->reg = VSX_REGISTER_XTP(rd);
+ op->type = MKOP(STORE_VSX, PREFIXED, 32);
+ op->element_size = 32;
+ break;
}
break;
case 1: /* Type 01 Eight-Byte Register-to-Register */
--
2.26.2
^ permalink raw reply related
* [PATCH v4 3/4] powerpc/ppc-opcode: Add encoding macros for VSX vector paired instructions
From: Ravi Bangoria @ 2020-10-08 7:27 UTC (permalink / raw)
To: mpe; +Cc: ravi.bangoria, bala24, paulus, sandipan, naveen.n.rao,
linuxppc-dev
In-Reply-To: <20201008072726.233086-1-ravi.bangoria@linux.ibm.com>
From: Balamuruhan S <bala24@linux.ibm.com>
Add instruction encodings, DQ, D0, D1 immediate, XTP, XSP operands as
macros for new VSX vector paired instructions,
* Load VSX Vector Paired (lxvp)
* Load VSX Vector Paired Indexed (lxvpx)
* Prefixed Load VSX Vector Paired (plxvp)
* Store VSX Vector Paired (stxvp)
* Store VSX Vector Paired Indexed (stxvpx)
* Prefixed Store VSX Vector Paired (pstxvp)
Suggested-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/ppc-opcode.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index a6e3700c4566..5e7918ca4fb7 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -78,6 +78,9 @@
#define IMM_L(i) ((uintptr_t)(i) & 0xffff)
#define IMM_DS(i) ((uintptr_t)(i) & 0xfffc)
+#define IMM_DQ(i) ((uintptr_t)(i) & 0xfff0)
+#define IMM_D0(i) (((uintptr_t)(i) >> 16) & 0x3ffff)
+#define IMM_D1(i) IMM_L(i)
/*
* 16-bit immediate helper macros: HA() is for use with sign-extending instrs
@@ -295,6 +298,8 @@
#define __PPC_XB(b) ((((b) & 0x1f) << 11) | (((b) & 0x20) >> 4))
#define __PPC_XS(s) ((((s) & 0x1f) << 21) | (((s) & 0x20) >> 5))
#define __PPC_XT(s) __PPC_XS(s)
+#define __PPC_XSP(s) ((((s) & 0x1e) | (((s) >> 5) & 0x1)) << 21)
+#define __PPC_XTP(s) __PPC_XSP(s)
#define __PPC_T_TLB(t) (((t) & 0x3) << 21)
#define __PPC_WC(w) (((w) & 0x3) << 21)
#define __PPC_WS(w) (((w) & 0x1f) << 11)
@@ -395,6 +400,14 @@
#define PPC_RAW_XVCPSGNDP(t, a, b) ((0xf0000780 | VSX_XX3((t), (a), (b))))
#define PPC_RAW_VPERMXOR(vrt, vra, vrb, vrc) \
((0x1000002d | ___PPC_RT(vrt) | ___PPC_RA(vra) | ___PPC_RB(vrb) | (((vrc) & 0x1f) << 6)))
+#define PPC_RAW_LXVP(xtp, a, i) (0x18000000 | __PPC_XTP(xtp) | ___PPC_RA(a) | IMM_DQ(i))
+#define PPC_RAW_STXVP(xsp, a, i) (0x18000001 | __PPC_XSP(xsp) | ___PPC_RA(a) | IMM_DQ(i))
+#define PPC_RAW_LXVPX(xtp, a, b) (0x7c00029a | __PPC_XTP(xtp) | ___PPC_RA(a) | ___PPC_RB(b))
+#define PPC_RAW_STXVPX(xsp, a, b) (0x7c00039a | __PPC_XSP(xsp) | ___PPC_RA(a) | ___PPC_RB(b))
+#define PPC_RAW_PLXVP(xtp, i, a, pr) \
+ ((PPC_PREFIX_8LS | __PPC_PRFX_R(pr) | IMM_D0(i)) << 32 | (0xe8000000 | __PPC_XTP(xtp) | ___PPC_RA(a) | IMM_D1(i)))
+#define PPC_RAW_PSTXVP(xsp, i, a, pr) \
+ ((PPC_PREFIX_8LS | __PPC_PRFX_R(pr) | IMM_D0(i)) << 32 | (0xf8000000 | __PPC_XSP(xsp) | ___PPC_RA(a) | IMM_D1(i)))
#define PPC_RAW_NAP (0x4c000364)
#define PPC_RAW_SLEEP (0x4c0003a4)
#define PPC_RAW_WINKLE (0x4c0003e4)
--
2.26.2
^ permalink raw reply related
* [PATCH v4 4/4] powerpc/sstep: Add testcases for VSX vector paired load/store instructions
From: Ravi Bangoria @ 2020-10-08 7:27 UTC (permalink / raw)
To: mpe; +Cc: ravi.bangoria, bala24, paulus, sandipan, naveen.n.rao,
linuxppc-dev
In-Reply-To: <20201008072726.233086-1-ravi.bangoria@linux.ibm.com>
From: Balamuruhan S <bala24@linux.ibm.com>
Add testcases for VSX vector paired load/store instructions.
Sample o/p:
emulate_step_test: lxvp : PASS
emulate_step_test: stxvp : PASS
emulate_step_test: lxvpx : PASS
emulate_step_test: stxvpx : PASS
emulate_step_test: plxvp : PASS
emulate_step_test: pstxvp : PASS
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/lib/test_emulate_step.c | 270 +++++++++++++++++++++++++++
1 file changed, 270 insertions(+)
diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
index 0a201b771477..783d1b85ecfe 100644
--- a/arch/powerpc/lib/test_emulate_step.c
+++ b/arch/powerpc/lib/test_emulate_step.c
@@ -612,6 +612,273 @@ static void __init test_lxvd2x_stxvd2x(void)
}
#endif /* CONFIG_VSX */
+#ifdef CONFIG_VSX
+static void __init test_lxvp_stxvp(void)
+{
+ struct pt_regs regs;
+ union {
+ vector128 a;
+ u32 b[4];
+ } c[2];
+ u32 cached_b[8];
+ int stepped = -1;
+
+ if (!cpu_has_feature(CPU_FTR_ARCH_31)) {
+ show_result("lxvp", "SKIP (!CPU_FTR_ARCH_31)");
+ show_result("stxvp", "SKIP (!CPU_FTR_ARCH_31)");
+ return;
+ }
+
+ init_pt_regs(®s);
+
+ /*** lxvp ***/
+
+ cached_b[0] = c[0].b[0] = 18233;
+ cached_b[1] = c[0].b[1] = 34863571;
+ cached_b[2] = c[0].b[2] = 834;
+ cached_b[3] = c[0].b[3] = 6138911;
+ cached_b[4] = c[1].b[0] = 1234;
+ cached_b[5] = c[1].b[1] = 5678;
+ cached_b[6] = c[1].b[2] = 91011;
+ cached_b[7] = c[1].b[3] = 121314;
+
+ regs.gpr[4] = (unsigned long)&c[0].a;
+
+ /*
+ * lxvp XTp,DQ(RA)
+ * XTp = 32xTX + 2xTp
+ * let TX=1 Tp=1 RA=4 DQ=0
+ */
+ stepped = emulate_step(®s, ppc_inst(PPC_RAW_LXVP(34, 4, 0)));
+
+ if (stepped == 1 && cpu_has_feature(CPU_FTR_VSX)) {
+ show_result("lxvp", "PASS");
+ } else {
+ if (!cpu_has_feature(CPU_FTR_VSX))
+ show_result("lxvp", "PASS (!CPU_FTR_VSX)");
+ else
+ show_result("lxvp", "FAIL");
+ }
+
+ /*** stxvp ***/
+
+ c[0].b[0] = 21379463;
+ c[0].b[1] = 87;
+ c[0].b[2] = 374234;
+ c[0].b[3] = 4;
+ c[1].b[0] = 90;
+ c[1].b[1] = 122;
+ c[1].b[2] = 555;
+ c[1].b[3] = 32144;
+
+ /*
+ * stxvp XSp,DQ(RA)
+ * XSp = 32xSX + 2xSp
+ * let SX=1 Sp=1 RA=4 DQ=0
+ */
+ stepped = emulate_step(®s, ppc_inst(PPC_RAW_STXVP(34, 4, 0)));
+
+ if (stepped == 1 && cached_b[0] == c[0].b[0] && cached_b[1] == c[0].b[1] &&
+ cached_b[2] == c[0].b[2] && cached_b[3] == c[0].b[3] &&
+ cached_b[4] == c[1].b[0] && cached_b[5] == c[1].b[1] &&
+ cached_b[6] == c[1].b[2] && cached_b[7] == c[1].b[3] &&
+ cpu_has_feature(CPU_FTR_VSX)) {
+ show_result("stxvp", "PASS");
+ } else {
+ if (!cpu_has_feature(CPU_FTR_VSX))
+ show_result("stxvp", "PASS (!CPU_FTR_VSX)");
+ else
+ show_result("stxvp", "FAIL");
+ }
+}
+#else
+static void __init test_lxvp_stxvp(void)
+{
+ show_result("lxvp", "SKIP (CONFIG_VSX is not set)");
+ show_result("stxvp", "SKIP (CONFIG_VSX is not set)");
+}
+#endif /* CONFIG_VSX */
+
+#ifdef CONFIG_VSX
+static void __init test_lxvpx_stxvpx(void)
+{
+ struct pt_regs regs;
+ union {
+ vector128 a;
+ u32 b[4];
+ } c[2];
+ u32 cached_b[8];
+ int stepped = -1;
+
+ if (!cpu_has_feature(CPU_FTR_ARCH_31)) {
+ show_result("lxvpx", "SKIP (!CPU_FTR_ARCH_31)");
+ show_result("stxvpx", "SKIP (!CPU_FTR_ARCH_31)");
+ return;
+ }
+
+ init_pt_regs(®s);
+
+ /*** lxvpx ***/
+
+ cached_b[0] = c[0].b[0] = 18233;
+ cached_b[1] = c[0].b[1] = 34863571;
+ cached_b[2] = c[0].b[2] = 834;
+ cached_b[3] = c[0].b[3] = 6138911;
+ cached_b[4] = c[1].b[0] = 1234;
+ cached_b[5] = c[1].b[1] = 5678;
+ cached_b[6] = c[1].b[2] = 91011;
+ cached_b[7] = c[1].b[3] = 121314;
+
+ regs.gpr[3] = (unsigned long)&c[0].a;
+ regs.gpr[4] = 0;
+
+ /*
+ * lxvpx XTp,RA,RB
+ * XTp = 32xTX + 2xTp
+ * let TX=1 Tp=1 RA=3 RB=4
+ */
+ stepped = emulate_step(®s, ppc_inst(PPC_RAW_LXVPX(34, 3, 4)));
+
+ if (stepped == 1 && cpu_has_feature(CPU_FTR_VSX)) {
+ show_result("lxvpx", "PASS");
+ } else {
+ if (!cpu_has_feature(CPU_FTR_VSX))
+ show_result("lxvpx", "PASS (!CPU_FTR_VSX)");
+ else
+ show_result("lxvpx", "FAIL");
+ }
+
+ /*** stxvpx ***/
+
+ c[0].b[0] = 21379463;
+ c[0].b[1] = 87;
+ c[0].b[2] = 374234;
+ c[0].b[3] = 4;
+ c[1].b[0] = 90;
+ c[1].b[1] = 122;
+ c[1].b[2] = 555;
+ c[1].b[3] = 32144;
+
+ /*
+ * stxvpx XSp,RA,RB
+ * XSp = 32xSX + 2xSp
+ * let SX=1 Sp=1 RA=3 RB=4
+ */
+ stepped = emulate_step(®s, ppc_inst(PPC_RAW_STXVPX(34, 3, 4)));
+
+ if (stepped == 1 && cached_b[0] == c[0].b[0] && cached_b[1] == c[0].b[1] &&
+ cached_b[2] == c[0].b[2] && cached_b[3] == c[0].b[3] &&
+ cached_b[4] == c[1].b[0] && cached_b[5] == c[1].b[1] &&
+ cached_b[6] == c[1].b[2] && cached_b[7] == c[1].b[3] &&
+ cpu_has_feature(CPU_FTR_VSX)) {
+ show_result("stxvpx", "PASS");
+ } else {
+ if (!cpu_has_feature(CPU_FTR_VSX))
+ show_result("stxvpx", "PASS (!CPU_FTR_VSX)");
+ else
+ show_result("stxvpx", "FAIL");
+ }
+}
+#else
+static void __init test_lxvpx_stxvpx(void)
+{
+ show_result("lxvpx", "SKIP (CONFIG_VSX is not set)");
+ show_result("stxvpx", "SKIP (CONFIG_VSX is not set)");
+}
+#endif /* CONFIG_VSX */
+
+#ifdef CONFIG_VSX
+static void __init test_plxvp_pstxvp(void)
+{
+ struct ppc_inst instr;
+ struct pt_regs regs;
+ union {
+ vector128 a;
+ u32 b[4];
+ } c[2];
+ u32 cached_b[8];
+ int stepped = -1;
+
+ if (!cpu_has_feature(CPU_FTR_ARCH_31)) {
+ show_result("plxvp", "SKIP (!CPU_FTR_ARCH_31)");
+ show_result("pstxvp", "SKIP (!CPU_FTR_ARCH_31)");
+ return;
+ }
+
+ /*** plxvp ***/
+
+ cached_b[0] = c[0].b[0] = 18233;
+ cached_b[1] = c[0].b[1] = 34863571;
+ cached_b[2] = c[0].b[2] = 834;
+ cached_b[3] = c[0].b[3] = 6138911;
+ cached_b[4] = c[1].b[0] = 1234;
+ cached_b[5] = c[1].b[1] = 5678;
+ cached_b[6] = c[1].b[2] = 91011;
+ cached_b[7] = c[1].b[3] = 121314;
+
+ init_pt_regs(®s);
+ regs.gpr[3] = (unsigned long)&c[0].a;
+
+ /*
+ * plxvp XTp,D(RA),R
+ * XTp = 32xTX + 2xTp
+ * let RA=3 R=0 D=d0||d1=0 R=0 Tp=1 TX=1
+ */
+ instr = ppc_inst_prefix(PPC_RAW_PLXVP(34, 0, 3, 0) >> 32,
+ PPC_RAW_PLXVP(34, 0, 3, 0) & 0xffffffff);
+
+ stepped = emulate_step(®s, instr);
+ if (stepped == 1 && cpu_has_feature(CPU_FTR_VSX)) {
+ show_result("plxvp", "PASS");
+ } else {
+ if (!cpu_has_feature(CPU_FTR_VSX))
+ show_result("plxvp", "PASS (!CPU_FTR_VSX)");
+ else
+ show_result("plxvp", "FAIL");
+ }
+
+ /*** pstxvp ***/
+
+ c[0].b[0] = 21379463;
+ c[0].b[1] = 87;
+ c[0].b[2] = 374234;
+ c[0].b[3] = 4;
+ c[1].b[0] = 90;
+ c[1].b[1] = 122;
+ c[1].b[2] = 555;
+ c[1].b[3] = 32144;
+
+ /*
+ * pstxvp XSp,D(RA),R
+ * XSp = 32xSX + 2xSp
+ * let RA=3 D=d0||d1=0 R=0 Sp=1 SX=1
+ */
+ instr = ppc_inst_prefix(PPC_RAW_PSTXVP(34, 0, 3, 0) >> 32,
+ PPC_RAW_PSTXVP(34, 0, 3, 0) & 0xffffffff);
+
+ stepped = emulate_step(®s, instr);
+
+ if (stepped == 1 && cached_b[0] == c[0].b[0] && cached_b[1] == c[0].b[1] &&
+ cached_b[2] == c[0].b[2] && cached_b[3] == c[0].b[3] &&
+ cached_b[4] == c[1].b[0] && cached_b[5] == c[1].b[1] &&
+ cached_b[6] == c[1].b[2] && cached_b[7] == c[1].b[3] &&
+ cpu_has_feature(CPU_FTR_VSX)) {
+ show_result("pstxvp", "PASS");
+ } else {
+ if (!cpu_has_feature(CPU_FTR_VSX))
+ show_result("pstxvp", "PASS (!CPU_FTR_VSX)");
+ else
+ show_result("pstxvp", "FAIL");
+ }
+}
+#else
+static void __init test_plxvp_pstxvp(void)
+{
+ show_result("plxvp", "SKIP (CONFIG_VSX is not set)");
+ show_result("pstxvp", "SKIP (CONFIG_VSX is not set)");
+}
+#endif /* CONFIG_VSX */
+
static void __init run_tests_load_store(void)
{
test_ld();
@@ -628,6 +895,9 @@ static void __init run_tests_load_store(void)
test_plfd_pstfd();
test_lvx_stvx();
test_lxvd2x_stxvd2x();
+ test_lxvp_stxvp();
+ test_lxvpx_stxvpx();
+ test_plxvp_pstxvp();
}
struct compute_test {
--
2.26.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox