* [PATCH v4 02/10] powerpc/smp: Merge Power9 topology with Power topology
From: Srikar Dronamraju @ 2020-07-27 5:17 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling,
Srikar Dronamraju, Peter Zijlstra, Jordan Niethe, LKML,
Nicholas Piggin, Valentin Schneider, Oliver O'Halloran,
linuxppc-dev, Ingo Molnar
In-Reply-To: <20200727051805.16728-1-srikar@linux.vnet.ibm.com>
A new sched_domain_topology_level was added just for Power9. However the
same can be achieved by merging powerpc_topology with power9_topology
and makes the code more simpler especially when adding a new sched
domain.
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: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Jordan Niethe <jniethe5@gmail.com>
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog v1 -> v2:
Replaced a reference to cpu_smt_mask with per_cpu(cpu_sibling_map, cpu)
since cpu_smt_mask is only defined under CONFIG_SCHED_SMT
arch/powerpc/kernel/smp.c | 33 ++++++++++-----------------------
1 file changed, 10 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index edf94ca64eea..283a04e54f52 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1313,7 +1313,7 @@ int setup_profiling_timer(unsigned int multiplier)
}
#ifdef CONFIG_SCHED_SMT
-/* cpumask of CPUs with asymetric SMT dependancy */
+/* cpumask of CPUs with asymmetric SMT dependency */
static int powerpc_smt_flags(void)
{
int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
@@ -1326,14 +1326,6 @@ static int powerpc_smt_flags(void)
}
#endif
-static struct sched_domain_topology_level powerpc_topology[] = {
-#ifdef CONFIG_SCHED_SMT
- { cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
-#endif
- { cpu_cpu_mask, SD_INIT_NAME(DIE) },
- { NULL, },
-};
-
/*
* P9 has a slightly odd architecture where pairs of cores share an L2 cache.
* This topology makes it *much* cheaper to migrate tasks between adjacent cores
@@ -1351,7 +1343,13 @@ static int powerpc_shared_cache_flags(void)
*/
static const struct cpumask *shared_cache_mask(int cpu)
{
- return cpu_l2_cache_mask(cpu);
+ if (shared_caches)
+ return cpu_l2_cache_mask(cpu);
+
+ if (has_big_cores)
+ return cpu_smallcore_mask(cpu);
+
+ return per_cpu(cpu_sibling_map, cpu);
}
#ifdef CONFIG_SCHED_SMT
@@ -1361,7 +1359,7 @@ static const struct cpumask *smallcore_smt_mask(int cpu)
}
#endif
-static struct sched_domain_topology_level power9_topology[] = {
+static struct sched_domain_topology_level powerpc_topology[] = {
#ifdef CONFIG_SCHED_SMT
{ cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
#endif
@@ -1386,21 +1384,10 @@ void __init smp_cpus_done(unsigned int max_cpus)
#ifdef CONFIG_SCHED_SMT
if (has_big_cores) {
pr_info("Big cores detected but using small core scheduling\n");
- power9_topology[0].mask = smallcore_smt_mask;
powerpc_topology[0].mask = smallcore_smt_mask;
}
#endif
- /*
- * If any CPU detects that it's sharing a cache with another CPU then
- * use the deeper topology that is aware of this sharing.
- */
- if (shared_caches) {
- pr_info("Using shared cache scheduler topology\n");
- set_sched_topology(power9_topology);
- } else {
- pr_info("Using standard scheduler topology\n");
- set_sched_topology(powerpc_topology);
- }
+ set_sched_topology(powerpc_topology);
}
#ifdef CONFIG_HOTPLUG_CPU
--
2.17.1
^ permalink raw reply related
* [PATCH v4 00/10] Coregroup support on Powerpc
From: Srikar Dronamraju @ 2020-07-27 5:17 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Oliver OHalloran, Michael Neuling,
Srikar Dronamraju, Michael Ellerman, Peter Zijlstra,
Jordan Niethe, Anton Blanchard, LKML, Valentin Schneider,
Nick Piggin, linuxppc-dev, Ingo Molnar
Changelog v3 ->v4:
v3: https://lore.kernel.org/lkml/20200723085116.4731-1-srikar@linux.vnet.ibm.com/t/#u
powerpc/smp: Create coregroup domain
if coregroup_support doesn't exist, update MC mask to the next
smaller domain mask.
Changelog v2 -> v3:
v2: https://lore.kernel.org/linuxppc-dev/20200721113814.32284-1-srikar@linux.vnet.ibm.com/t/#u
powerpc/smp: Cache node for reuse
Removed node caching part. Rewrote the Commit msg (Michael Ellerman)
Renamed to powerpc/smp: Fix a warning under !NEED_MULTIPLE_NODES
powerpc/smp: Enable small core scheduling sooner
Rewrote changelog (Gautham)
Renamed to powerpc/smp: Move topology fixups into a new function
powerpc/smp: Create coregroup domain
Add optimization for mask updation under coregroup_support
Changelog v1 -> v2:
v1: https://lore.kernel.org/linuxppc-dev/20200714043624.5648-1-srikar@linux.vnet.ibm.com/t/#u
powerpc/smp: Merge Power9 topology with Power topology
Replaced a reference to cpu_smt_mask with per_cpu(cpu_sibling_map, cpu)
since cpu_smt_mask is only defined under CONFIG_SCHED_SMT
powerpc/smp: Enable small core scheduling sooner
Restored the previous info msg (Jordan)
Moved big core topology fixup to fixup_topology (Gautham)
powerpc/smp: Dont assume l2-cache to be superset of sibling
Set cpumask after verifying l2-cache. (Gautham)
powerpc/smp: Generalize 2nd sched domain
Moved shared_cache topology fixup to fixup_topology (Gautham)
Powerpc/numa: Detect support for coregroup
Explained Coregroup in commit msg (Michael Ellerman)
Powerpc/smp: Create coregroup domain
Moved coregroup topology fixup to fixup_topology (Gautham)
powerpc/smp: Implement cpu_to_coregroup_id
Move coregroup_enabled before getting associativity (Gautham)
powerpc/smp: Provide an ability to disable coregroup
Patch dropped (Michael Ellerman)
Cleanup of existing powerpc topologies and add coregroup support on
Powerpc. Coregroup is a group of (subset of) cores of a DIE that share
a resource.
Patch 7 of this patch series: "Powerpc/numa: Detect support for coregroup"
depends on
https://lore.kernel.org/linuxppc-dev/20200707140644.7241-1-srikar@linux.vnet.ibm.com/t/#u
However it should be easy to rebase the patch without the above patch.
This patch series is based on top of current powerpc/next tree + the
above patch.
On Power 8 Systems
------------------
$ tail /proc/cpuinfo
processor : 255
cpu : POWER8 (architected), altivec supported
clock : 3724.000000MHz
revision : 2.1 (pvr 004b 0201)
timebase : 512000000
platform : pSeries
model : IBM,8408-E8E
machine : CHRP IBM,8408-E8E
MMU : Hash
Before the patchset
-------------------
$ cat /proc/sys/kernel/sched_domain/cpu0/domain*/name
SMT
DIE
NUMA
NUMA
$ head /proc/schedstat
version 15
timestamp 4295534931
cpu0 0 0 0 0 0 0 41389823338 17682779896 14117
domain0 00000000,00000000,00000000,00000000,00000000,00000000,00000000,000000ff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain1 00000000,00000000,00000000,00000000,00000000,00000000,00000000,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain2 00000000,00000000,00000000,00000000,00000000,00000000,ffffffff,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain3 ffffffff,ffffffff,ffffffff,ffffffff,ffffffff,ffffffff,ffffffff,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
cpu1 0 0 0 0 0 0 27087859050 152273672 10396
domain0 00000000,00000000,00000000,00000000,00000000,00000000,00000000,000000ff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain1 00000000,00000000,00000000,00000000,00000000,00000000,00000000,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
After the patchset
------------------
$ cat /proc/sys/kernel/sched_domain/cpu0/domain*/name
SMT
DIE
NUMA
NUMA
$ head /proc/schedstat
version 15
timestamp 4295534931
cpu0 0 0 0 0 0 0 41389823338 17682779896 14117
domain0 00000000,00000000,00000000,00000000,00000000,00000000,00000000,000000ff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain1 00000000,00000000,00000000,00000000,00000000,00000000,00000000,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain2 00000000,00000000,00000000,00000000,00000000,00000000,ffffffff,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain3 ffffffff,ffffffff,ffffffff,ffffffff,ffffffff,ffffffff,ffffffff,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
cpu1 0 0 0 0 0 0 27087859050 152273672 10396
domain0 00000000,00000000,00000000,00000000,00000000,00000000,00000000,000000ff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain1 00000000,00000000,00000000,00000000,00000000,00000000,00000000,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
On Power 9 (with device-tree enablement to show coregroups).
(hunks for mimicing a coregroup was posted at
https://lore.kernel.org/linuxppc-dev/20200714043624.5648-1-srikar@linux.vnet.ibm.com/t/#m2cb09bb11c7a93257d6123d1d27edb8212f8af21)
-----------------------------------------------------------
$ tail /proc/cpuinfo
processor : 127
cpu : POWER9 (architected), altivec supported
clock : 3000.000000MHz
revision : 2.2 (pvr 004e 0202)
timebase : 512000000
platform : pSeries
model : IBM,9008-22L
machine : CHRP IBM,9008-22L
MMU : Hash
Before patchset
--------------
$ cat /proc/sys/kernel/sched_domain/cpu0/domain*/name
SMT
CACHE
DIE
NUMA
$ head /proc/schedstat
version 15
timestamp 4318242208
cpu0 0 0 0 0 0 0 28077107004 4773387362 78205
domain0 00000000,00000000,00000000,00000055 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain1 00000000,00000000,00000000,000000ff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain2 00000000,00000000,ffffffff,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain3 ffffffff,ffffffff,ffffffff,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
cpu1 0 0 0 0 0 0 24177439200 413887604 75393
domain0 00000000,00000000,00000000,000000aa 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain1 00000000,00000000,00000000,000000ff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
After patchset
--------------
$ cat /proc/sys/kernel/sched_domain/cpu0/domain*/name
SMT
CACHE
MC
DIE
NUMA
$ head /proc/schedstat
version 15
timestamp 4318242208
cpu0 0 0 0 0 0 0 28077107004 4773387362 78205
domain0 00000000,00000000,00000000,00000055 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain1 00000000,00000000,00000000,000000ff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain2 00000000,00000000,00000000,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain3 00000000,00000000,ffffffff,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
domain4 ffffffff,ffffffff,ffffffff,ffffffff 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
cpu1 0 0 0 0 0 0 24177439200 413887604 75393
domain0 00000000,00000000,00000000,000000aa 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <michaele@au1.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Nick Piggin <npiggin@au1.ibm.com>
Cc: Oliver OHalloran <oliveroh@au1.ibm.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Neuling <mikey@linux.ibm.com>
Cc: Anton Blanchard <anton@au1.ibm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
Cc: Jordan Niethe <jniethe5@gmail.com>
Srikar Dronamraju (10):
powerpc/smp: Fix a warning under !NEED_MULTIPLE_NODES
powerpc/smp: Merge Power9 topology with Power topology
powerpc/smp: Move powerpc_topology above
powerpc/smp: Move topology fixups into a new function
powerpc/smp: Dont assume l2-cache to be superset of sibling
powerpc/smp: Generalize 2nd sched domain
powerpc/numa: Detect support for coregroup
powerpc/smp: Allocate cpumask only after searching thread group
powerpc/smp: Create coregroup domain
powerpc/smp: Implement cpu_to_coregroup_id
arch/powerpc/include/asm/smp.h | 1 +
arch/powerpc/include/asm/topology.h | 10 ++
arch/powerpc/kernel/smp.c | 246 +++++++++++++++++-----------
arch/powerpc/mm/numa.c | 59 +++++--
4 files changed, 210 insertions(+), 106 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH v3 09/10] powerpc/smp: Create coregroup domain
From: Gautham R Shenoy @ 2020-07-27 4:39 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Nathan Lynch, Gautham R Shenoy, Michael Neuling, Peter Zijlstra,
LKML, Nicholas Piggin, Valentin Schneider, Oliver O'Halloran,
Jordan Niethe, linuxppc-dev, Ingo Molnar
In-Reply-To: <20200723085116.4731-10-srikar@linux.vnet.ibm.com>
On Thu, Jul 23, 2020 at 02:21:15PM +0530, Srikar Dronamraju wrote:
> Add percpu coregroup maps and masks to create coregroup domain.
> If a coregroup doesn't exist, the coregroup domain will be degenerated
> in favour of SMT/CACHE domain.
>
> 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: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Jordan Niethe <jniethe5@gmail.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> Changelog v2 -> v3:
> Add optimization for mask updation under coregroup_support
>
> Changelog v1 -> v2:
> Moved coregroup topology fixup to fixup_topology (Gautham)
>
> arch/powerpc/include/asm/topology.h | 10 +++++++
> arch/powerpc/kernel/smp.c | 44 +++++++++++++++++++++++++++++
> arch/powerpc/mm/numa.c | 5 ++++
> 3 files changed, 59 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
> index f0b6300e7dd3..6609174918ab 100644
> --- a/arch/powerpc/include/asm/topology.h
> +++ b/arch/powerpc/include/asm/topology.h
> @@ -88,12 +88,22 @@ static inline int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
>
> #if defined(CONFIG_NUMA) && defined(CONFIG_PPC_SPLPAR)
> extern int find_and_online_cpu_nid(int cpu);
> +extern int cpu_to_coregroup_id(int cpu);
> #else
> static inline int find_and_online_cpu_nid(int cpu)
> {
> return 0;
> }
>
> +static inline int cpu_to_coregroup_id(int cpu)
> +{
> +#ifdef CONFIG_SMP
> + return cpu_to_core_id(cpu);
> +#else
> + return 0;
> +#endif
> +}
> +
> #endif /* CONFIG_NUMA && CONFIG_PPC_SPLPAR */
>
> #include <asm-generic/topology.h>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 7d8d44cbab11..1faedde3e406 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -80,6 +80,7 @@ DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
> DEFINE_PER_CPU(cpumask_var_t, cpu_smallcore_map);
> DEFINE_PER_CPU(cpumask_var_t, cpu_l2_cache_map);
> DEFINE_PER_CPU(cpumask_var_t, cpu_core_map);
> +DEFINE_PER_CPU(cpumask_var_t, cpu_coregroup_map);
>
> EXPORT_PER_CPU_SYMBOL(cpu_sibling_map);
> EXPORT_PER_CPU_SYMBOL(cpu_l2_cache_map);
> @@ -91,6 +92,7 @@ enum {
> smt_idx,
> #endif
> bigcore_idx,
> + mc_idx,
> die_idx,
> };
>
> @@ -869,6 +871,21 @@ static const struct cpumask *smallcore_smt_mask(int cpu)
> }
> #endif
>
> +static struct cpumask *cpu_coregroup_mask(int cpu)
> +{
> + return per_cpu(cpu_coregroup_map, cpu);
> +}
> +
> +static bool has_coregroup_support(void)
> +{
> + return coregroup_enabled;
> +}
> +
> +static const struct cpumask *cpu_mc_mask(int cpu)
> +{
> + return cpu_coregroup_mask(cpu);
> +}
> +
> static const struct cpumask *cpu_bigcore_mask(int cpu)
> {
> return per_cpu(cpu_sibling_map, cpu);
> @@ -879,6 +896,7 @@ static struct sched_domain_topology_level powerpc_topology[] = {
> { cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
> #endif
> { cpu_bigcore_mask, SD_INIT_NAME(BIGCORE) },
> + { cpu_mc_mask, SD_INIT_NAME(MC) },
> { cpu_cpu_mask, SD_INIT_NAME(DIE) },
> { NULL, },
> };
[..snip..]
> @@ -1384,6 +1425,9 @@ int setup_profiling_timer(unsigned int multiplier)
>
> static void fixup_topology(void)
> {
> + if (!has_coregroup_support())
> + powerpc_topology[mc_idx].mask = cpu_bigcore_mask;
> +
> if (shared_caches) {
> pr_info("Using shared cache scheduler topology\n");
> powerpc_topology[bigcore_idx].mask = shared_cache_mask;
Suppose we consider a topology which does not have coregroup_support,
but has shared_caches. In that case, we would want our coregroup
domain to degenerate.
From the above code, after the fixup, our topology will look as
follows:
static struct sched_domain_topology_level powerpc_topology[] = {
{ cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
{ shared_cache_mask, powerpc_shared_cache_flags, SD_INIT_NAME(CACHE) },
{ cpu_bigcore_mask, SD_INIT_NAME(MC) },
{ cpu_cpu_mask, SD_INIT_NAME(DIE) },
{ NULL, },
So, in this case, the core-group domain (identified by MC) will
degenerate only if cpu_bigcore_mask() and shared_cache_mask() return
the same value. This may work for existing platforms, because either
shared_caches don't exist, or when they do, cpu_bigcore_mask and
shared_cache_mask return the same set of CPUs. But this may or may not
continue to hold good in the future.
Furthermore, if that is always going to be the case that in the
presence of shared_caches the cpu_bigcore_mask() and
shared_cache_mask() will always be the same, then why even define two
separate masks and not just have only the cpu_bigcore_mask() ?
The correct way would be to set the powerpc_topology[mc_idx].mask to
powerpc_topology[bigcore_idx].mask *after* we have fixedup the
big_core level.
--
Thanks and Regards
gautham.
^ permalink raw reply
* [PATCH v2 6/6] selftests/powerpc: Add test for pkey siginfo verification
From: Sandipan Das @ 2020-07-27 4:00 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <cover.1595821792.git.sandipan@linux.ibm.com>
Commit c46241a370a61 ("powerpc/pkeys: Check vma before
returning key fault error to the user") fixes a bug which
causes the kernel to set the wrong pkey in siginfo when a
pkey fault occurs after two competing threads that have
allocated different pkeys, one fully permissive and the
other restrictive, attempt to protect a common page at the
same time. This adds a test to detect the bug.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
tools/testing/selftests/powerpc/mm/.gitignore | 1 +
tools/testing/selftests/powerpc/mm/Makefile | 5 +-
.../selftests/powerpc/mm/pkey_siginfo.c | 332 ++++++++++++++++++
3 files changed, 337 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/powerpc/mm/pkey_siginfo.c
diff --git a/tools/testing/selftests/powerpc/mm/.gitignore b/tools/testing/selftests/powerpc/mm/.gitignore
index 8f841f925baa5..36ec2c4ccdea4 100644
--- a/tools/testing/selftests/powerpc/mm/.gitignore
+++ b/tools/testing/selftests/powerpc/mm/.gitignore
@@ -9,3 +9,4 @@ large_vm_fork_separation
bad_accesses
tlbie_test
pkey_exec_prot
+pkey_siginfo
diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile
index f9fa0ba7435c4..558b7ccc93932 100644
--- a/tools/testing/selftests/powerpc/mm/Makefile
+++ b/tools/testing/selftests/powerpc/mm/Makefile
@@ -3,7 +3,8 @@ noarg:
$(MAKE) -C ../
TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors wild_bctr \
- large_vm_fork_separation bad_accesses pkey_exec_prot
+ large_vm_fork_separation bad_accesses pkey_exec_prot \
+ pkey_siginfo
TEST_GEN_PROGS_EXTENDED := tlbie_test
TEST_GEN_FILES := tempfile
@@ -18,8 +19,10 @@ $(OUTPUT)/wild_bctr: CFLAGS += -m64
$(OUTPUT)/large_vm_fork_separation: CFLAGS += -m64
$(OUTPUT)/bad_accesses: CFLAGS += -m64
$(OUTPUT)/pkey_exec_prot: CFLAGS += -m64
+$(OUTPUT)/pkey_siginfo: CFLAGS += -m64
$(OUTPUT)/tempfile:
dd if=/dev/zero of=$@ bs=64k count=1
$(OUTPUT)/tlbie_test: LDLIBS += -lpthread
+$(OUTPUT)/pkey_siginfo: LDLIBS += -lpthread
diff --git a/tools/testing/selftests/powerpc/mm/pkey_siginfo.c b/tools/testing/selftests/powerpc/mm/pkey_siginfo.c
new file mode 100644
index 0000000000000..0c57f88f7f01a
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mm/pkey_siginfo.c
@@ -0,0 +1,332 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2020, Sandipan Das, IBM Corp.
+ *
+ * Test if the signal information reports the correct memory protection
+ * key upon getting a key access violation fault for a page that was
+ * attempted to be protected by two different keys from two competing
+ * threads at the same time.
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <signal.h>
+
+#include <unistd.h>
+#include <pthread.h>
+#include <sys/mman.h>
+
+#include "pkeys.h"
+
+#define PPC_INST_NOP 0x60000000
+#define PPC_INST_BLR 0x4e800020
+#define PROT_RWX (PROT_READ | PROT_WRITE | PROT_EXEC)
+
+#define NUM_ITERATIONS 1000000
+
+static volatile sig_atomic_t perm_pkey, rest_pkey;
+static volatile sig_atomic_t rights, fault_count;
+static volatile unsigned int *volatile fault_addr;
+static pthread_barrier_t iteration_barrier;
+
+static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
+{
+ void *pgstart;
+ size_t pgsize;
+ int pkey;
+
+ pkey = siginfo_pkey(sinfo);
+
+ /* Check if this fault originated from a pkey access violation */
+ if (sinfo->si_code != SEGV_PKUERR) {
+ sigsafe_err("got a fault for an unexpected reason\n");
+ _exit(1);
+ }
+
+ /* Check if this fault originated from the expected address */
+ if (sinfo->si_addr != (void *) fault_addr) {
+ sigsafe_err("got a fault for an unexpected address\n");
+ _exit(1);
+ }
+
+ /* Check if this fault originated from the restrictive pkey */
+ if (pkey != rest_pkey) {
+ sigsafe_err("got a fault for an unexpected pkey\n");
+ _exit(1);
+ }
+
+ /* Check if too many faults have occurred for the same iteration */
+ if (fault_count > 0) {
+ sigsafe_err("got too many faults for the same address\n");
+ _exit(1);
+ }
+
+ pgsize = getpagesize();
+ pgstart = (void *) ((unsigned long) fault_addr & ~(pgsize - 1));
+
+ /*
+ * If the current fault occurred due to lack of execute rights,
+ * reassociate the page with the exec-only pkey since execute
+ * rights cannot be changed directly for the faulting pkey as
+ * IAMR is inaccessible from userspace.
+ *
+ * Otherwise, if the current fault occurred due to lack of
+ * read-write rights, change the AMR permission bits for the
+ * pkey.
+ *
+ * This will let the test continue.
+ */
+ if (rights == PKEY_DISABLE_EXECUTE &&
+ mprotect(pgstart, pgsize, PROT_EXEC))
+ _exit(1);
+ else
+ pkey_set_rights(pkey, 0);
+
+ fault_count++;
+}
+
+struct region {
+ unsigned long rights;
+ unsigned int *base;
+ size_t size;
+};
+
+static void *protect(void *p)
+{
+ unsigned long rights;
+ unsigned int *base;
+ size_t size;
+ int tid, i;
+
+ tid = gettid();
+ base = ((struct region *) p)->base;
+ size = ((struct region *) p)->size;
+ FAIL_IF_EXIT(!base);
+
+ /* No read, write and execute restrictions */
+ rights = 0;
+
+ printf("tid %d, pkey permissions are %s\n", tid, pkey_rights(rights));
+
+ /* Allocate the permissive pkey */
+ perm_pkey = sys_pkey_alloc(0, rights);
+ FAIL_IF_EXIT(perm_pkey < 0);
+
+ /*
+ * Repeatedly try to protect the common region with a permissive
+ * pkey
+ */
+ for (i = 0; i < NUM_ITERATIONS; i++) {
+ /*
+ * Wait until the other thread has finished allocating the
+ * restrictive pkey or until the next iteration has begun
+ */
+ pthread_barrier_wait(&iteration_barrier);
+
+ /* Try to associate the permissive pkey with the region */
+ FAIL_IF_EXIT(sys_pkey_mprotect(base, size, PROT_RWX,
+ perm_pkey));
+ }
+
+ /* Free the permissive pkey */
+ sys_pkey_free(perm_pkey);
+
+ return NULL;
+}
+
+static void *protect_access(void *p)
+{
+ size_t size, numinsns;
+ unsigned int *base;
+ int tid, i;
+
+ tid = gettid();
+ base = ((struct region *) p)->base;
+ size = ((struct region *) p)->size;
+ rights = ((struct region *) p)->rights;
+ numinsns = size / sizeof(base[0]);
+ FAIL_IF_EXIT(!base);
+
+ /* Allocate the restrictive pkey */
+ rest_pkey = sys_pkey_alloc(0, rights);
+ FAIL_IF_EXIT(rest_pkey < 0);
+
+ printf("tid %d, pkey permissions are %s\n", tid, pkey_rights(rights));
+ printf("tid %d, %s randomly in range [%p, %p]\n", tid,
+ (rights == PKEY_DISABLE_EXECUTE) ? "execute" :
+ (rights == PKEY_DISABLE_WRITE) ? "write" : "read",
+ base, base + numinsns);
+
+ /*
+ * Repeatedly try to protect the common region with a restrictive
+ * pkey and read, write or execute from it
+ */
+ for (i = 0; i < NUM_ITERATIONS; i++) {
+ /*
+ * Wait until the other thread has finished allocating the
+ * permissive pkey or until the next iteration has begun
+ */
+ pthread_barrier_wait(&iteration_barrier);
+
+ /* Try to associate the restrictive pkey with the region */
+ FAIL_IF_EXIT(sys_pkey_mprotect(base, size, PROT_RWX,
+ rest_pkey));
+
+ /* Choose a random instruction word address from the region */
+ fault_addr = base + (rand() % numinsns);
+ fault_count = 0;
+
+ switch (rights) {
+ /* Read protection test */
+ case PKEY_DISABLE_ACCESS:
+ /*
+ * Read an instruction word from the region and
+ * verify if it has not been overwritten to
+ * something unexpected
+ */
+ FAIL_IF_EXIT(*fault_addr != PPC_INST_NOP &&
+ *fault_addr != PPC_INST_BLR);
+ break;
+
+ /* Write protection test */
+ case PKEY_DISABLE_WRITE:
+ /*
+ * Write an instruction word to the region and
+ * verify if the overwrite has succeeded
+ */
+ *fault_addr = PPC_INST_BLR;
+ FAIL_IF_EXIT(*fault_addr != PPC_INST_BLR);
+ break;
+
+ /* Execute protection test */
+ case PKEY_DISABLE_EXECUTE:
+ /* Jump to the region and execute instructions */
+ asm volatile(
+ "mtctr %0; bctrl"
+ : : "r"(fault_addr) : "ctr", "lr");
+ break;
+ }
+
+ /*
+ * Restore the restrictions originally imposed by the
+ * restrictive pkey as the signal handler would have
+ * cleared out the corresponding AMR bits
+ */
+ pkey_set_rights(rest_pkey, rights);
+ }
+
+ /* Free restrictive pkey */
+ sys_pkey_free(rest_pkey);
+
+ return NULL;
+}
+
+static void reset_pkeys(unsigned long rights)
+{
+ int pkeys[NR_PKEYS], i;
+
+ /* Exhaustively allocate all available pkeys */
+ for (i = 0; i < NR_PKEYS; i++)
+ pkeys[i] = sys_pkey_alloc(0, rights);
+
+ /* Free all allocated pkeys */
+ for (i = 0; i < NR_PKEYS; i++)
+ sys_pkey_free(pkeys[i]);
+}
+
+static int test(void)
+{
+ pthread_t prot_thread, pacc_thread;
+ struct sigaction act;
+ pthread_attr_t attr;
+ size_t numinsns;
+ struct region r;
+ int ret, i;
+
+ srand(time(NULL));
+ ret = pkeys_unsupported();
+ if (ret)
+ return ret;
+
+ /* Allocate the region */
+ r.size = getpagesize();
+ r.base = mmap(NULL, r.size, PROT_RWX,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ FAIL_IF(r.base == MAP_FAILED);
+
+ /*
+ * Fill the region with no-ops with a branch at the end
+ * for returning to the caller
+ */
+ numinsns = r.size / sizeof(r.base[0]);
+ for (i = 0; i < numinsns - 1; i++)
+ r.base[i] = PPC_INST_NOP;
+ r.base[i] = PPC_INST_BLR;
+
+ /* Setup SIGSEGV handler */
+ act.sa_handler = 0;
+ act.sa_sigaction = segv_handler;
+ FAIL_IF(sigprocmask(SIG_SETMASK, 0, &act.sa_mask) != 0);
+ act.sa_flags = SA_SIGINFO;
+ act.sa_restorer = 0;
+ FAIL_IF(sigaction(SIGSEGV, &act, NULL) != 0);
+
+ /*
+ * For these tests, the parent process should clear all bits of
+ * AMR and IAMR, i.e. impose no restrictions, for all available
+ * pkeys. This will be the base for the initial AMR and IAMR
+ * values for all the test thread pairs.
+ *
+ * If the AMR and IAMR bits of all available pkeys are cleared
+ * before running the tests and a fault is generated when
+ * attempting to read, write or execute instructions from a
+ * pkey protected region, the pkey responsible for this must be
+ * the one from the protect-and-access thread since the other
+ * one is fully permissive. Despite that, if the pkey reported
+ * by siginfo is not the restrictive pkey, then there must be a
+ * kernel bug.
+ */
+ reset_pkeys(0);
+
+ /* Setup barrier for protect and protect-and-access threads */
+ FAIL_IF(pthread_attr_init(&attr) != 0);
+ FAIL_IF(pthread_barrier_init(&iteration_barrier, NULL, 2) != 0);
+
+ /* Setup and start protect and protect-and-read threads */
+ puts("starting thread pair (protect, protect-and-read)");
+ r.rights = PKEY_DISABLE_ACCESS;
+ FAIL_IF(pthread_create(&prot_thread, &attr, &protect, &r) != 0);
+ FAIL_IF(pthread_create(&pacc_thread, &attr, &protect_access, &r) != 0);
+ FAIL_IF(pthread_join(prot_thread, NULL) != 0);
+ FAIL_IF(pthread_join(pacc_thread, NULL) != 0);
+
+ /* Setup and start protect and protect-and-write threads */
+ puts("starting thread pair (protect, protect-and-write)");
+ r.rights = PKEY_DISABLE_WRITE;
+ FAIL_IF(pthread_create(&prot_thread, &attr, &protect, &r) != 0);
+ FAIL_IF(pthread_create(&pacc_thread, &attr, &protect_access, &r) != 0);
+ FAIL_IF(pthread_join(prot_thread, NULL) != 0);
+ FAIL_IF(pthread_join(pacc_thread, NULL) != 0);
+
+ /* Setup and start protect and protect-and-execute threads */
+ puts("starting thread pair (protect, protect-and-execute)");
+ r.rights = PKEY_DISABLE_EXECUTE;
+ FAIL_IF(pthread_create(&prot_thread, &attr, &protect, &r) != 0);
+ FAIL_IF(pthread_create(&pacc_thread, &attr, &protect_access, &r) != 0);
+ FAIL_IF(pthread_join(prot_thread, NULL) != 0);
+ FAIL_IF(pthread_join(pacc_thread, NULL) != 0);
+
+ /* Cleanup */
+ FAIL_IF(pthread_attr_destroy(&attr) != 0);
+ FAIL_IF(pthread_barrier_destroy(&iteration_barrier) != 0);
+ munmap(r.base, r.size);
+
+ return 0;
+}
+
+int main(void)
+{
+ test_harness(test, "pkey_siginfo");
+}
--
2.25.1
^ permalink raw reply related
* [PATCH v2 4/6] selftests/powerpc: Add helper to exit on failure
From: Sandipan Das @ 2020-07-27 4:00 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <cover.1595821792.git.sandipan@linux.ibm.com>
This adds a helper similar to FAIL_IF() which lets a
program exit with code 1 (to indicate failure) when
the given condition is true.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
tools/testing/selftests/powerpc/include/utils.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/testing/selftests/powerpc/include/utils.h b/tools/testing/selftests/powerpc/include/utils.h
index 7f259f36e23bc..69d16875802da 100644
--- a/tools/testing/selftests/powerpc/include/utils.h
+++ b/tools/testing/selftests/powerpc/include/utils.h
@@ -72,6 +72,15 @@ do { \
} \
} while (0)
+#define FAIL_IF_EXIT(x) \
+do { \
+ if ((x)) { \
+ fprintf(stderr, \
+ "[FAIL] Test FAILED on line %d\n", __LINE__); \
+ _exit(1); \
+ } \
+} while (0)
+
/* The test harness uses this, yes it's gross */
#define MAGIC_SKIP_RETURN_VALUE 99
--
2.25.1
^ permalink raw reply related
* [PATCH v2 3/6] selftests/powerpc: Harden test for execute-disabled pkeys
From: Sandipan Das @ 2020-07-27 4:00 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <cover.1595821792.git.sandipan@linux.ibm.com>
Commit 192b6a7805989 ("powerpc/book3s64/pkeys: Fix
pkey_access_permitted() for execute disable pkey") fixed a
bug that caused repetitive faults for pkeys with no execute
rights alongside some combination of read and write rights.
This removes the last two cases of the test, which check
the behaviour of pkeys with read, write but no execute
rights and all the rights, in favour of checking all the
possible combinations of read, write and execute rights
to be able to detect bugs like the one mentioned above.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
.../selftests/powerpc/mm/pkey_exec_prot.c | 84 +++++++++----------
1 file changed, 41 insertions(+), 43 deletions(-)
diff --git a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
index 18ebfe6bae1c9..9e5c7f3f498a7 100644
--- a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
+++ b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
@@ -237,55 +237,53 @@ static int test(void)
*fault_addr = PPC_INST_NOP;
FAIL_IF(remaining_faults != 0 || fault_code != SEGV_ACCERR);
- /*
- * Jump to the executable region when AMR bits are set i.e.
- * the pkey permits neither read nor write access.
- *
- * This should generate a pkey fault based on IAMR bits which
- * are set to not permit execution. AMR bits should not affect
- * execution.
- *
- * This also checks if the overwrite of the first instruction
- * word from a trap to a no-op succeeded.
- */
- fault_addr = insns;
- fault_type = PKEY_DISABLE_EXECUTE;
- fault_pkey = pkey;
- remaining_faults = 1;
- FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
- printf("execute at %p, pkey permissions are %s\n", fault_addr,
- pkey_rights(rights));
- asm volatile("mtctr %0; bctrl" : : "r"(insns));
- FAIL_IF(remaining_faults != 0 || fault_code != SEGV_PKUERR);
-
- /*
- * Free the current pkey and allocate a new one that is
- * fully permissive.
- */
+ /* Free the current pkey */
sys_pkey_free(pkey);
+
rights = 0;
- pkey = sys_pkey_alloc(0, rights);
+ do {
+ /*
+ * Allocate pkeys with all valid combinations of read,
+ * write and execute restrictions.
+ */
+ pkey = sys_pkey_alloc(0, rights);
+ FAIL_IF(pkey < 0);
+
+ /*
+ * Jump to the executable region. AMR bits may or may not
+ * be set but they should not affect execution.
+ *
+ * This should generate pkey faults based on IAMR bits which
+ * may be set to restrict execution.
+ *
+ * The first iteration also checks if the overwrite of the
+ * first instruction word from a trap to a no-op succeeded.
+ */
+ fault_pkey = pkey;
+ fault_type = -1;
+ remaining_faults = 0;
+ if (rights & PKEY_DISABLE_EXECUTE) {
+ fault_type = PKEY_DISABLE_EXECUTE;
+ remaining_faults = 1;
+ }
- /*
- * Jump to the executable region when AMR bits are not set
- * i.e. the pkey permits read and write access.
- *
- * This should not generate any faults as the IAMR bits are
- * also not set and hence will the pkey will not restrict
- * execution.
- */
- fault_pkey = pkey;
- remaining_faults = 0;
- FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("execute at %p, pkey permissions are %s\n", fault_addr,
- pkey_rights(rights));
- asm volatile("mtctr %0; bctrl" : : "r"(insns));
- FAIL_IF(remaining_faults != 0);
+ FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
+ printf("execute at %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
+ asm volatile("mtctr %0; bctrl" : : "r"(insns));
+ FAIL_IF(remaining_faults != 0);
+ if (rights & PKEY_DISABLE_EXECUTE)
+ FAIL_IF(fault_code != SEGV_PKUERR);
+
+ /* Free the current pkey */
+ sys_pkey_free(pkey);
+
+ /* Find next valid combination of pkey rights */
+ rights = next_pkey_rights(rights);
+ } while (rights);
/* Cleanup */
munmap((void *) insns, pgsize);
- sys_pkey_free(pkey);
return 0;
}
--
2.25.1
^ permalink raw reply related
* [PATCH v2 5/6] selftests/powerpc: Add wrapper for gettid
From: Sandipan Das @ 2020-07-27 4:00 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <cover.1595821792.git.sandipan@linux.ibm.com>
The gettid() syscall wrapper was first introduced in
glibc 2.30. This adds a wrapper for use in distros
running older versions.
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
tools/testing/selftests/powerpc/include/utils.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/testing/selftests/powerpc/include/utils.h b/tools/testing/selftests/powerpc/include/utils.h
index 69d16875802da..71d2924f5b8b3 100644
--- a/tools/testing/selftests/powerpc/include/utils.h
+++ b/tools/testing/selftests/powerpc/include/utils.h
@@ -42,6 +42,16 @@ int perf_event_enable(int fd);
int perf_event_disable(int fd);
int perf_event_reset(int fd);
+#if !defined(__GLIBC_PREREQ) || !__GLIBC_PREREQ(2, 30)
+#include <unistd.h>
+#include <sys/syscall.h>
+
+static inline pid_t gettid(void)
+{
+ return syscall(SYS_gettid);
+}
+#endif
+
static inline bool have_hwcap(unsigned long ftr)
{
return ((unsigned long)get_auxv_entry(AT_HWCAP) & ftr) == ftr;
--
2.25.1
^ permalink raw reply related
* [PATCH v2 2/6] selftests/powerpc: Add pkey helpers for rights
From: Sandipan Das @ 2020-07-27 4:00 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <cover.1595821792.git.sandipan@linux.ibm.com>
This adds some new pkey-related helper to print
access rights of a pkey in the "rwx" format and
to generate different valid combinations of pkey
rights starting from a given combination.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
.../testing/selftests/powerpc/include/pkeys.h | 28 +++++++++++++++
.../selftests/powerpc/mm/pkey_exec_prot.c | 36 ++++++++++---------
2 files changed, 48 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/powerpc/include/pkeys.h b/tools/testing/selftests/powerpc/include/pkeys.h
index 9b53a97e664ea..6ba95039a0343 100644
--- a/tools/testing/selftests/powerpc/include/pkeys.h
+++ b/tools/testing/selftests/powerpc/include/pkeys.h
@@ -105,4 +105,32 @@ int siginfo_pkey(siginfo_t *si)
#endif
}
+#define pkey_rights(r) ({ \
+ static char buf[4] = "rwx"; \
+ unsigned int amr_bits; \
+ if ((r) & PKEY_DISABLE_EXECUTE) \
+ buf[2] = '-'; \
+ amr_bits = (r) & PKEY_BITS_MASK; \
+ if (amr_bits & PKEY_DISABLE_WRITE) \
+ buf[1] = '-'; \
+ if (amr_bits & PKEY_DISABLE_ACCESS & ~PKEY_DISABLE_WRITE) \
+ buf[0] = '-'; \
+ buf; \
+})
+
+unsigned long next_pkey_rights(unsigned long rights)
+{
+ if (rights == PKEY_DISABLE_ACCESS)
+ return PKEY_DISABLE_EXECUTE;
+ else if (rights == (PKEY_DISABLE_ACCESS | PKEY_DISABLE_EXECUTE))
+ return 0;
+
+ if ((rights & PKEY_BITS_MASK) == 0)
+ rights |= PKEY_DISABLE_WRITE;
+ else if ((rights & PKEY_BITS_MASK) == PKEY_DISABLE_WRITE)
+ rights |= PKEY_DISABLE_ACCESS;
+
+ return rights;
+}
+
#endif /* _SELFTESTS_POWERPC_PKEYS_H */
diff --git a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
index 1253ad6afba24..18ebfe6bae1c9 100644
--- a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
+++ b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
@@ -102,6 +102,7 @@ static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
static int test(void)
{
struct sigaction segv_act, trap_act;
+ unsigned long rights;
int pkey, ret, i;
ret = pkeys_unsupported();
@@ -150,7 +151,8 @@ static int test(void)
insns[numinsns - 1] = PPC_INST_BLR;
/* Allocate a pkey that restricts execution */
- pkey = sys_pkey_alloc(0, PKEY_DISABLE_EXECUTE);
+ rights = PKEY_DISABLE_EXECUTE;
+ pkey = sys_pkey_alloc(0, rights);
FAIL_IF(pkey < 0);
/*
@@ -175,8 +177,8 @@ static int test(void)
*/
remaining_faults = 0;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("read from %p, pkey is execute-disabled, access-enabled\n",
- (void *) fault_addr);
+ printf("read from %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
i = *fault_addr;
FAIL_IF(remaining_faults != 0);
@@ -192,12 +194,13 @@ static int test(void)
*/
remaining_faults = 1;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("write to %p, pkey is execute-disabled, access-enabled\n",
- (void *) fault_addr);
+ printf("write to %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
*fault_addr = PPC_INST_TRAP;
FAIL_IF(remaining_faults != 0 || fault_code != SEGV_ACCERR);
/* The following three cases will generate SEGV_PKUERR */
+ rights |= PKEY_DISABLE_ACCESS;
fault_type = PKEY_DISABLE_ACCESS;
fault_pkey = pkey;
@@ -211,9 +214,9 @@ static int test(void)
*/
remaining_faults = 1;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("read from %p, pkey is execute-disabled, access-disabled\n",
- (void *) fault_addr);
- pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
+ pkey_set_rights(pkey, rights);
+ printf("read from %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
i = *fault_addr;
FAIL_IF(remaining_faults != 0 || fault_code != SEGV_PKUERR);
@@ -228,9 +231,9 @@ static int test(void)
*/
remaining_faults = 2;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("write to %p, pkey is execute-disabled, access-disabled\n",
- (void *) fault_addr);
- pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
+ pkey_set_rights(pkey, rights);
+ printf("write to %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
*fault_addr = PPC_INST_NOP;
FAIL_IF(remaining_faults != 0 || fault_code != SEGV_ACCERR);
@@ -251,8 +254,8 @@ static int test(void)
remaining_faults = 1;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
- printf("execute at %p, pkey is execute-disabled, access-disabled\n",
- (void *) fault_addr);
+ printf("execute at %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
asm volatile("mtctr %0; bctrl" : : "r"(insns));
FAIL_IF(remaining_faults != 0 || fault_code != SEGV_PKUERR);
@@ -261,7 +264,8 @@ static int test(void)
* fully permissive.
*/
sys_pkey_free(pkey);
- pkey = sys_pkey_alloc(0, 0);
+ rights = 0;
+ pkey = sys_pkey_alloc(0, rights);
/*
* Jump to the executable region when AMR bits are not set
@@ -274,8 +278,8 @@ static int test(void)
fault_pkey = pkey;
remaining_faults = 0;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("execute at %p, pkey is execute-enabled, access-enabled\n",
- (void *) fault_addr);
+ printf("execute at %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
asm volatile("mtctr %0; bctrl" : : "r"(insns));
FAIL_IF(remaining_faults != 0);
--
2.25.1
^ permalink raw reply related
* [PATCH v2 1/6] selftests/powerpc: Move pkey helpers to headers
From: Sandipan Das @ 2020-07-27 4:00 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <cover.1595821792.git.sandipan@linux.ibm.com>
This moves all the pkey-related helpers to a new header
file and also a helper to print error messages in signal
handlers to the existing utils header file.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
.../testing/selftests/powerpc/include/pkeys.h | 108 ++++++++++++++++++
.../testing/selftests/powerpc/include/utils.h | 4 +
.../selftests/powerpc/mm/pkey_exec_prot.c | 100 +---------------
3 files changed, 114 insertions(+), 98 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/include/pkeys.h
diff --git a/tools/testing/selftests/powerpc/include/pkeys.h b/tools/testing/selftests/powerpc/include/pkeys.h
new file mode 100644
index 0000000000000..9b53a97e664ea
--- /dev/null
+++ b/tools/testing/selftests/powerpc/include/pkeys.h
@@ -0,0 +1,108 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2020, Sandipan Das, IBM Corp.
+ */
+
+#ifndef _SELFTESTS_POWERPC_PKEYS_H
+#define _SELFTESTS_POWERPC_PKEYS_H
+
+#include <sys/mman.h>
+
+#include "reg.h"
+#include "utils.h"
+
+/*
+ * Older versions of libc use the Intel-specific access rights.
+ * Hence, override the definitions as they might be incorrect.
+ */
+#undef PKEY_DISABLE_ACCESS
+#define PKEY_DISABLE_ACCESS 0x3
+
+#undef PKEY_DISABLE_WRITE
+#define PKEY_DISABLE_WRITE 0x2
+
+#undef PKEY_DISABLE_EXECUTE
+#define PKEY_DISABLE_EXECUTE 0x4
+
+/* Older versions of libc do not not define this */
+#ifndef SEGV_PKUERR
+#define SEGV_PKUERR 4
+#endif
+
+#define SI_PKEY_OFFSET 0x20
+
+#define SYS_pkey_mprotect 386
+#define SYS_pkey_alloc 384
+#define SYS_pkey_free 385
+
+#define PKEY_BITS_PER_PKEY 2
+#define NR_PKEYS 32
+#define PKEY_BITS_MASK ((1UL << PKEY_BITS_PER_PKEY) - 1)
+
+inline unsigned long pkeyreg_get(void)
+{
+ return mfspr(SPRN_AMR);
+}
+
+inline void pkeyreg_set(unsigned long amr)
+{
+ set_amr(amr);
+}
+
+void pkey_set_rights(int pkey, unsigned long rights)
+{
+ unsigned long amr, shift;
+
+ shift = (NR_PKEYS - pkey - 1) * PKEY_BITS_PER_PKEY;
+ amr = pkeyreg_get();
+ amr &= ~(PKEY_BITS_MASK << shift);
+ amr |= (rights & PKEY_BITS_MASK) << shift;
+ pkeyreg_set(amr);
+}
+
+int sys_pkey_mprotect(void *addr, size_t len, int prot, int pkey)
+{
+ return syscall(SYS_pkey_mprotect, addr, len, prot, pkey);
+}
+
+int sys_pkey_alloc(unsigned long flags, unsigned long rights)
+{
+ return syscall(SYS_pkey_alloc, flags, rights);
+}
+
+int sys_pkey_free(int pkey)
+{
+ return syscall(SYS_pkey_free, pkey);
+}
+
+int pkeys_unsupported(void)
+{
+ bool hash_mmu = false;
+ int pkey;
+
+ /* Protection keys are currently supported on Hash MMU only */
+ FAIL_IF(using_hash_mmu(&hash_mmu));
+ SKIP_IF(!hash_mmu);
+
+ /* Check if the system call is supported */
+ pkey = sys_pkey_alloc(0, 0);
+ SKIP_IF(pkey < 0);
+ sys_pkey_free(pkey);
+
+ return 0;
+}
+
+int siginfo_pkey(siginfo_t *si)
+{
+ /*
+ * In older versions of libc, siginfo_t does not have si_pkey as
+ * a member.
+ */
+#ifdef si_pkey
+ return si->si_pkey;
+#else
+ return *((int *)(((char *) si) + SI_PKEY_OFFSET));
+#endif
+}
+
+#endif /* _SELFTESTS_POWERPC_PKEYS_H */
diff --git a/tools/testing/selftests/powerpc/include/utils.h b/tools/testing/selftests/powerpc/include/utils.h
index 9dbe607cc5ec3..7f259f36e23bc 100644
--- a/tools/testing/selftests/powerpc/include/utils.h
+++ b/tools/testing/selftests/powerpc/include/utils.h
@@ -97,6 +97,10 @@ do { \
#define _str(s) #s
#define str(s) _str(s)
+#define sigsafe_err(msg) ({ \
+ ssize_t nbytes __attribute__((unused)); \
+ nbytes = write(STDERR_FILENO, msg, strlen(msg)); })
+
/* POWER9 feature */
#ifndef PPC_FEATURE2_ARCH_3_00
#define PPC_FEATURE2_ARCH_3_00 0x00800000
diff --git a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
index 7c7c93425c5e9..1253ad6afba24 100644
--- a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
+++ b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
@@ -14,83 +14,13 @@
#include <signal.h>
#include <unistd.h>
-#include <sys/mman.h>
-#include "reg.h"
-#include "utils.h"
-
-/*
- * Older versions of libc use the Intel-specific access rights.
- * Hence, override the definitions as they might be incorrect.
- */
-#undef PKEY_DISABLE_ACCESS
-#define PKEY_DISABLE_ACCESS 0x3
-
-#undef PKEY_DISABLE_WRITE
-#define PKEY_DISABLE_WRITE 0x2
-
-#undef PKEY_DISABLE_EXECUTE
-#define PKEY_DISABLE_EXECUTE 0x4
-
-/* Older versions of libc do not not define this */
-#ifndef SEGV_PKUERR
-#define SEGV_PKUERR 4
-#endif
-
-#define SI_PKEY_OFFSET 0x20
-
-#define SYS_pkey_mprotect 386
-#define SYS_pkey_alloc 384
-#define SYS_pkey_free 385
-
-#define PKEY_BITS_PER_PKEY 2
-#define NR_PKEYS 32
-#define PKEY_BITS_MASK ((1UL << PKEY_BITS_PER_PKEY) - 1)
+#include "pkeys.h"
#define PPC_INST_NOP 0x60000000
#define PPC_INST_TRAP 0x7fe00008
#define PPC_INST_BLR 0x4e800020
-#define sigsafe_err(msg) ({ \
- ssize_t nbytes __attribute__((unused)); \
- nbytes = write(STDERR_FILENO, msg, strlen(msg)); })
-
-static inline unsigned long pkeyreg_get(void)
-{
- return mfspr(SPRN_AMR);
-}
-
-static inline void pkeyreg_set(unsigned long amr)
-{
- set_amr(amr);
-}
-
-static void pkey_set_rights(int pkey, unsigned long rights)
-{
- unsigned long amr, shift;
-
- shift = (NR_PKEYS - pkey - 1) * PKEY_BITS_PER_PKEY;
- amr = pkeyreg_get();
- amr &= ~(PKEY_BITS_MASK << shift);
- amr |= (rights & PKEY_BITS_MASK) << shift;
- pkeyreg_set(amr);
-}
-
-static int sys_pkey_mprotect(void *addr, size_t len, int prot, int pkey)
-{
- return syscall(SYS_pkey_mprotect, addr, len, prot, pkey);
-}
-
-static int sys_pkey_alloc(unsigned long flags, unsigned long rights)
-{
- return syscall(SYS_pkey_alloc, flags, rights);
-}
-
-static int sys_pkey_free(int pkey)
-{
- return syscall(SYS_pkey_free, pkey);
-}
-
static volatile sig_atomic_t fault_pkey, fault_code, fault_type;
static volatile sig_atomic_t remaining_faults;
static volatile unsigned int *fault_addr;
@@ -110,16 +40,7 @@ static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
{
int signal_pkey;
- /*
- * In older versions of libc, siginfo_t does not have si_pkey as
- * a member.
- */
-#ifdef si_pkey
- signal_pkey = sinfo->si_pkey;
-#else
- signal_pkey = *((int *)(((char *) sinfo) + SI_PKEY_OFFSET));
-#endif
-
+ signal_pkey = siginfo_pkey(sinfo);
fault_code = sinfo->si_code;
/* Check if this fault originated from the expected address */
@@ -178,23 +99,6 @@ static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
remaining_faults--;
}
-static int pkeys_unsupported(void)
-{
- bool hash_mmu = false;
- int pkey;
-
- /* Protection keys are currently supported on Hash MMU only */
- FAIL_IF(using_hash_mmu(&hash_mmu));
- SKIP_IF(!hash_mmu);
-
- /* Check if the system call is supported */
- pkey = sys_pkey_alloc(0, 0);
- SKIP_IF(pkey < 0);
- sys_pkey_free(pkey);
-
- return 0;
-}
-
static int test(void)
{
struct sigaction segv_act, trap_act;
--
2.25.1
^ permalink raw reply related
* [PATCH v2 0/6] Improvements to pkey tests
From: Sandipan Das @ 2020-07-27 4:00 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
Based on recent bugs found in the pkey infrastructure, this
improves the test for execute-disabled pkeys and adds a new
test for detecting inconsistencies with the pkey reported by
the signal information upon getting a fault.
Previous versions can be found at:
v1: https://lore.kernel.org/linuxppc-dev/cover.1594897099.git.sandipan@linux.ibm.com/
Changes in v2:
- Added a wrapper for the gettid syscall based on suggestions
from Michael and Christophe.
Sandipan Das (6):
selftests/powerpc: Move pkey helpers to headers
selftests/powerpc: Add pkey helpers for rights
selftests/powerpc: Harden test for execute-disabled pkeys
selftests/powerpc: Add helper to exit on failure
selftests/powerpc: Add wrapper for gettid
selftests/powerpc: Add test for pkey siginfo verification
.../testing/selftests/powerpc/include/pkeys.h | 136 +++++++
.../testing/selftests/powerpc/include/utils.h | 23 ++
tools/testing/selftests/powerpc/mm/.gitignore | 1 +
tools/testing/selftests/powerpc/mm/Makefile | 5 +-
.../selftests/powerpc/mm/pkey_exec_prot.c | 210 +++--------
.../selftests/powerpc/mm/pkey_siginfo.c | 332 ++++++++++++++++++
6 files changed, 554 insertions(+), 153 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/include/pkeys.h
create mode 100644 tools/testing/selftests/powerpc/mm/pkey_siginfo.c
--
2.25.1
^ permalink raw reply
* Re: [PATCH v5 5/7] KVM: PPC: Book3S HV: migrate hot plugged memory
From: Bharata B Rao @ 2020-07-27 3:55 UTC (permalink / raw)
To: Ram Pai
Cc: ldufour, cclaudio, kvm-ppc, sathnaga, aneesh.kumar, sukadev,
linuxppc-dev, bauerman, david
In-Reply-To: <1595534844-16188-6-git-send-email-linuxram@us.ibm.com>
On Thu, Jul 23, 2020 at 01:07:22PM -0700, Ram Pai wrote:
> From: Laurent Dufour <ldufour@linux.ibm.com>
>
> When a memory slot is hot plugged to a SVM, PFNs associated with the
> GFNs in that slot must be migrated to the secure-PFNs, aka device-PFNs.
>
> Call kvmppc_uv_migrate_mem_slot() to accomplish this.
> Disable page-merge for all pages in the memory slot.
>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> [rearranged the code, and modified the commit log]
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
> arch/powerpc/include/asm/kvm_book3s_uvmem.h | 14 ++++++++++++++
> arch/powerpc/kvm/book3s_hv.c | 10 ++--------
> arch/powerpc/kvm/book3s_hv_uvmem.c | 23 +++++++++++++++++++----
> 3 files changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_uvmem.h b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
> index f229ab5..59c17ca 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_uvmem.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
> @@ -25,6 +25,10 @@ void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
> struct kvm *kvm, bool skip_page_out);
> int kvmppc_uv_migrate_mem_slot(struct kvm *kvm,
> const struct kvm_memory_slot *memslot);
> +int kvmppc_uvmem_memslot_create(struct kvm *kvm,
> + const struct kvm_memory_slot *new);
> +void kvmppc_uvmem_memslot_delete(struct kvm *kvm,
> + const struct kvm_memory_slot *old);
> #else
> static inline int kvmppc_uvmem_init(void)
> {
> @@ -84,5 +88,15 @@ static inline int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gfn)
> static inline void
> kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
> struct kvm *kvm, bool skip_page_out) { }
> +
> +static inline int kvmppc_uvmem_memslot_create(struct kvm *kvm,
> + const struct kvm_memory_slot *new)
> +{
> + return H_UNSUPPORTED;
> +}
> +
> +static inline void kvmppc_uvmem_memslot_delete(struct kvm *kvm,
> + const struct kvm_memory_slot *old) { }
> +
> #endif /* CONFIG_PPC_UV */
> #endif /* __ASM_KVM_BOOK3S_UVMEM_H__ */
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index d331b46..b1485ca 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4515,16 +4515,10 @@ static void kvmppc_core_commit_memory_region_hv(struct kvm *kvm,
>
> switch (change) {
> case KVM_MR_CREATE:
> - if (kvmppc_uvmem_slot_init(kvm, new))
> - return;
> - uv_register_mem_slot(kvm->arch.lpid,
> - new->base_gfn << PAGE_SHIFT,
> - new->npages * PAGE_SIZE,
> - 0, new->id);
> + kvmppc_uvmem_memslot_create(kvm, new);
Only concern is that kvmppc_uvmem_memslot_create() can fail due
to multiple reasons but we ignore them and go ahead with memory
hotplug.
May be this hasn't been observed in reality but if we can note this
as a TODO in the comments to dig further and explore the possibility
of recovering from here, then
Reviewed-by: Bharata B Rao <bharata@linux.ibm.com>
Regards,
Bharata.
^ permalink raw reply
* Re: [PATCH v5 6/7] KVM: PPC: Book3S HV: move kvmppc_svm_page_out up
From: Bharata B Rao @ 2020-07-27 3:49 UTC (permalink / raw)
To: Ram Pai
Cc: ldufour, cclaudio, kvm-ppc, sathnaga, aneesh.kumar, sukadev,
linuxppc-dev, bauerman, david
In-Reply-To: <1595534844-16188-7-git-send-email-linuxram@us.ibm.com>
On Thu, Jul 23, 2020 at 01:07:23PM -0700, Ram Pai wrote:
> From: Laurent Dufour <ldufour@linux.ibm.com>
>
> kvmppc_svm_page_out() will need to be called by kvmppc_uvmem_drop_pages()
> so move it upper in this file.
>
> Furthermore it will be interesting to call this function when already
> holding the kvm->arch.uvmem_lock, so prefix the original function with __
> and remove the locking in it, and introduce a wrapper which call that
> function with the lock held.
>
> There is no functional change.
>
> Cc: Ram Pai <linuxram@us.ibm.com>
> Cc: Bharata B Rao <bharata@linux.ibm.com>
> Cc: Paul Mackerras <paulus@ozlabs.org>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Reviewed-by: Bharata B Rao <bharata@linux.ibm.com>
Regards,
Bharata.
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3S HV: rework secure mem slot dropping
From: Bharata B Rao @ 2020-07-27 3:49 UTC (permalink / raw)
To: Laurent Dufour
Cc: aneesh.kumar, linuxram, cclaudio, kvm-ppc, sathnaga, sukadev,
linuxppc-dev, bauerman, david
In-Reply-To: <20200724083527.28027-1-ldufour@linux.ibm.com>
On Fri, Jul 24, 2020 at 10:35:27AM +0200, Laurent Dufour wrote:
> When a secure memslot is dropped, all the pages backed in the secure
> device (aka really backed by secure memory by the Ultravisor)
> should be paged out to a normal page. Previously, this was
> achieved by triggering the page fault mechanism which is calling
> kvmppc_svm_page_out() on each pages.
>
> This can't work when hot unplugging a memory slot because the memory
> slot is flagged as invalid and gfn_to_pfn() is then not trying to access
> the page, so the page fault mechanism is not triggered.
>
> Since the final goal is to make a call to kvmppc_svm_page_out() it seems
> simpler to call directly instead of triggering such a mechanism. This
> way kvmppc_uvmem_drop_pages() can be called even when hot unplugging a
> memslot.
>
> Since kvmppc_uvmem_drop_pages() is already holding kvm->arch.uvmem_lock,
> the call to __kvmppc_svm_page_out() is made. As
> __kvmppc_svm_page_out needs the vma pointer to migrate the pages,
> the VMA is fetched in a lazy way, to not trigger find_vma() all
> the time. In addition, the mmap_sem is held in read mode during
> that time, not in write mode since the virual memory layout is not
> impacted, and kvm->arch.uvmem_lock prevents concurrent operation
> on the secure device.
>
> Cc: Ram Pai <linuxram@us.ibm.com>
> Cc: Bharata B Rao <bharata@linux.ibm.com>
> Cc: Paul Mackerras <paulus@ozlabs.org>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> [modified the changelog description]
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> [modified check on the VMA in kvmppc_uvmem_drop_pages]
Reviewed-by: Bharata B Rao <bharata@linux.ibm.com>
Regards,
Bharata.
^ permalink raw reply
* Re: [PATCH 5/5] selftests/powerpc: Add test for pkey siginfo verification
From: Sandipan Das @ 2020-07-27 3:27 UTC (permalink / raw)
To: Michael Ellerman, Christophe Leroy
Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <87zh7lg7g5.fsf@mpe.ellerman.id.au>
On 27/07/20 5:22 am, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Michael Ellerman <mpe@ellerman.id.au> a écrit :
>>> Sandipan Das <sandipan@linux.ibm.com> writes:
>>>> diff --git a/tools/testing/selftests/powerpc/mm/pkey_siginfo.c
>>>> b/tools/testing/selftests/powerpc/mm/pkey_siginfo.c
>>>> new file mode 100644
>>>> index 0000000000000..58605c53d495d
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/powerpc/mm/pkey_siginfo.c
>>>> @@ -0,0 +1,332 @@
>>> ...
>>>> +
>>>> +static void *protect(void *p)
>>>> +{
>>>> + unsigned long rights;
>>>> + unsigned int *base;
>>>> + size_t size;
>>>> + int tid, i;
>>>> +
>>>> + tid = gettid();
>>>
>>> pkey_siginfo.c: In function 'protect':
>>> pkey_siginfo.c:103:8: error: implicit declaration of function
>>> 'gettid' [-Werror=implicit-function-declaration]
>>> tid = gettid();
>>> ^
>>>
>>>
>>> On Ubuntu 18.04 at least.
>>
>> See https://man7.org/linux/man-pages/man2/gettid.2.html
>>
>> Added in glibc 2.30
>
> Thanks, this seems to work:
>
> diff --git a/tools/testing/selftests/powerpc/include/utils.h b/tools/testing/selftests/powerpc/include/utils.h
> index 69d16875802d..71d2924f5b8b 100644
> --- a/tools/testing/selftests/powerpc/include/utils.h
> +++ b/tools/testing/selftests/powerpc/include/utils.h
> @@ -42,6 +42,16 @@ int perf_event_enable(int fd);
> int perf_event_disable(int fd);
> int perf_event_reset(int fd);
>
> +#if !defined(__GLIBC_PREREQ) || !__GLIBC_PREREQ(2, 30)
> +#include <unistd.h>
> +#include <sys/syscall.h>
> +
> +static inline pid_t gettid(void)
> +{
> + return syscall(SYS_gettid);
> +}
> +#endif
> +
> static inline bool have_hwcap(unsigned long ftr)
> {
> return ((unsigned long)get_auxv_entry(AT_HWCAP) & ftr) == ftr;
>
>
> cheers
>
Thanks for catching this. Will add these changes to v2.
- Sandipan
^ permalink raw reply
* linux-next: manual merge of the vfs tree with the powerpc tree
From: Stephen Rothwell @ 2020-07-27 1:32 UTC (permalink / raw)
To: Al Viro, Michael Ellerman
Cc: Aneesh Kumar K.V, Linux Next Mailing List, PowerPC,
Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 2065 bytes --]
Hi all,
Today's linux-next merge of the vfs tree got a conflict in:
arch/powerpc/kernel/ptrace/ptrace-view.c
between commit:
e0d8e991be64 ("powerpc/book3s64/kuap: Move UAMOR setup to key init function")
from the powerpc tree and commit:
5e39a71bddb3 ("powerpc: switch to ->regset_get()")
from the vfs tree.
I fixed it up (I thnk - see below) 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
diff --cc arch/powerpc/kernel/ptrace/ptrace-view.c
index ac7d480cb9c1,13208a9a02ca..000000000000
--- a/arch/powerpc/kernel/ptrace/ptrace-view.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-view.c
@@@ -486,23 -468,15 +468,15 @@@ static int pkey_active(struct task_stru
}
static int pkey_get(struct task_struct *target, const struct user_regset *regset,
- unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf)
+ struct membuf to)
{
- int ret;
-
BUILD_BUG_ON(TSO(amr) + sizeof(unsigned long) != TSO(iamr));
- BUILD_BUG_ON(TSO(iamr) + sizeof(unsigned long) != TSO(uamor));
if (!arch_pkeys_enabled())
return -ENODEV;
- ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &target->thread.amr,
- 0, 2 * sizeof(unsigned long));
- if (ret)
- return ret;
-
- ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &default_uamor,
- 2 * sizeof(unsigned long), 3 * sizeof(unsigned long));
- return ret;
- return membuf_write(&to, &target->thread.amr, ELF_NPKEY * sizeof(unsigned long));
++ membuf_write(&to, &target->thread.amr, 2 * sizeof(unsigned long));
++ return membuf_write(&to, &default_uamor, sizeof(unsigned long));
}
static int pkey_set(struct task_struct *target, const struct user_regset *regset,
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/rtas: Restrict RTAS requests from userspace
From: Daniel Axtens @ 2020-07-27 1:23 UTC (permalink / raw)
To: Andrew Donnellan, linuxppc-dev; +Cc: nathanl, leobras.c, stable
In-Reply-To: <20200702161932.18176-1-ajd@linux.ibm.com>
Andrew Donnellan <ajd@linux.ibm.com> writes:
> A number of userspace utilities depend on making calls to RTAS to retrieve
> information and update various things.
>
> The existing API through which we expose RTAS to userspace exposes more
> RTAS functionality than we actually need, through the sys_rtas syscall,
> which allows root (or anyone with CAP_SYS_ADMIN) to make any RTAS call they
> want with arbitrary arguments.
>
> Many RTAS calls take the address of a buffer as an argument, and it's up to
> the caller to specify the physical address of the buffer as an argument. We
> allocate a buffer (the "RMO buffer") in the Real Memory Area that RTAS can
> access, and then expose the physical address and size of this buffer in
> /proc/powerpc/rtas/rmo_buffer. Userspace is expected to read this address,
> poke at the buffer using /dev/mem, and pass an address in the RMO buffer to
> the RTAS call.
>
> However, there's nothing stopping the caller from specifying whatever
> address they want in the RTAS call, and it's easy to construct a series of
> RTAS calls that can overwrite arbitrary bytes (even without /dev/mem
> access).
>
> Additionally, there are some RTAS calls that do potentially dangerous
> things and for which there are no legitimate userspace use cases.
>
> In the past, this would not have been a particularly big deal as it was
> assumed that root could modify all system state freely, but with Secure
> Boot and lockdown we need to care about this.
>
> We can't fundamentally change the ABI at this point, however we can address
> this by implementing a filter that checks RTAS calls against a list
> of permitted calls and forces the caller to use addresses within the RMO
> buffer.
>
> The list is based off the list of calls that are used by the librtas
> userspace library, and has been tested with a number of existing userspace
> RTAS utilities. For compatibility with any applications we are not aware of
> that require other calls, the filter can be turned off at build time.
>
> Reported-by: Daniel Axtens <dja@axtens.net>
> Cc: stable@vger.kernel.org
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> ---
> arch/powerpc/Kconfig | 13 +++
> arch/powerpc/kernel/rtas.c | 198 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 211 insertions(+)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 9fa23eb320ff..0e2dfe497357 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -973,6 +973,19 @@ config PPC_SECVAR_SYSFS
> read/write operations on these variables. Say Y if you have
> secure boot enabled and want to expose variables to userspace.
>
> +config PPC_RTAS_FILTER
> + bool "Enable filtering of RTAS syscalls"
> + default y
> + depends on PPC_RTAS
> + help
> + The RTAS syscall API has security issues that could be used to
> + compromise system integrity. This option enforces restrictions on the
> + RTAS calls and arguments passed by userspace programs to mitigate
> + these issues.
> +
> + Say Y unless you know what you are doing and the filter is causing
> + problems for you.
> +
> endmenu
>
> config ISA_DMA_API
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index a09eba03f180..ec1cae52d8bd 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -324,6 +324,23 @@ int rtas_token(const char *service)
> }
> EXPORT_SYMBOL(rtas_token);
>
> +#ifdef CONFIG_PPC_RTAS_FILTER
> +
> +static char *rtas_token_name(int token)
> +{
> + struct property *prop;
> +
> + for_each_property_of_node(rtas.dev, prop) {
> + const __be32 *tokp = prop->value;
> +
> + if (tokp && be32_to_cpu(*tokp) == token)
> + return prop->name;
> + }
> + return NULL;
> +}
> +
> +#endif /* CONFIG_PPC_RTAS_FILTER */
> +
> int rtas_service_present(const char *service)
> {
> return rtas_token(service) != RTAS_UNKNOWN_SERVICE;
> @@ -1110,6 +1127,184 @@ struct pseries_errorlog *get_pseries_errorlog(struct rtas_error_log *log,
> return NULL;
> }
>
> +#ifdef CONFIG_PPC_RTAS_FILTER
> +
> +/*
> + * The sys_rtas syscall, as originally designed, allows root to pass
> + * arbitrary physical addresses to RTAS calls. A number of RTAS calls
> + * can be abused to write to arbitrary memory and do other things that
> + * are potentially harmful to system integrity, and thus should only
> + * be used inside the kernel and not exposed to userspace.
> + *
> + * All known legitimate users of the sys_rtas syscall will only ever
> + * pass addresses that fall within the RMO buffer, and use a known
> + * subset of RTAS calls.
> + *
> + * Accordingly, we filter RTAS requests to check that the call is
> + * permitted, and that provided pointers fall within the RMO buffer.
> + * The rtas_filters list contains an entry for each permitted call,
> + * with the indexes of the parameters which are expected to contain
> + * addresses and sizes of buffers allocated inside the RMO buffer.
> + */
> +struct rtas_filter {
> + const char name[32];
> +
> + /* Indexes into the args buffer, -1 if not used */
> + int rmo_buf_idx1;
> + int rmo_size_idx1;
> + int rmo_buf_idx2;
> + int rmo_size_idx2;
> +};
> +
> +struct rtas_filter rtas_filters[] = {
> + { "ibm,activate-firmware", -1, -1, -1, -1 },
> + { "ibm,configure-connector", 0, -1, 1, -1 }, /* Special cased, size 4096 */
> + { "display-character", -1, -1, -1, -1 },
> + { "ibm,display-message", 0, -1, -1, -1 },
> + { "ibm,errinjct", 2, -1, -1, -1 }, /* Fixed size of 1024 */
> + { "ibm,close-errinjct", -1, -1, -1, -1 },
> + { "ibm,open-errinct", -1, -1, -1, -1 },
> + { "ibm,get-config-addr-info2", -1, -1, -1, -1 },
> + { "ibm,get-dynamic-sensor-state", 1, -1, -1, -1 },
> + { "ibm,get-indices", 2, 3, -1, -1 },
> + { "get-power-level", -1, -1, -1, -1 },
> + { "get-sensor-state", -1, -1, -1, -1 },
> + { "ibm,get-system-parameter", 1, 2, -1, -1 },
> + { "get-time-of-day", -1, -1, -1, -1 },
> + { "ibm,get-vpd", 0, -1, 1, 2 },
> + { "ibm,lpar-perftools", 2, 3, -1, -1 },
> + { "ibm,platform-dump", 4, 5, -1, -1 },
> + { "ibm,read-slot-reset-state", -1, -1, -1, -1 },
> + { "ibm,scan-log-dump", 0, 1, -1, -1 },
> + { "ibm,set-dynamic-indicator", 2, -1, -1, -1 },
> + { "ibm,set-eeh-option", -1, -1, -1, -1 },
> + { "set-indicator", -1, -1, -1, -1 },
> + { "set-power-level", -1, -1, -1, -1 },
> + { "set-time-for-power-on", -1, -1, -1, -1 },
> + { "ibm,set-system-parameter", 1, -1, -1, -1 },
> + { "set-time-of-day", -1, -1, -1, -1 },
> + { "ibm,suspend-me", -1, -1, -1, -1 },
> + { "ibm,update-nodes", 0, -1, -1, -1 }, /* Fixed size of 4096 */
> + { "ibm,update-properties", 0, -1, -1, -1 }, /* Fixed size of 4096 */
> + { "ibm,physical-attestation", 0, 1, -1, -1 },
> +};
> +
> +static void dump_rtas_params(int token, int nargs, int nret,
> + struct rtas_args *args)
> +{
> + int i;
> + char *token_name = rtas_token_name(token);
> +
> + pr_err_ratelimited("sys_rtas: token=0x%x (%s), nargs=%d, nret=%d (called by %s)\n",
> + token, token_name ? token_name : "unknown", nargs,
> + nret, current->comm);
> + pr_err_ratelimited("sys_rtas: args: ");
> +
> + for (i = 0; i < nargs; i++) {
I wondered if it was possible for me to specify nargs == 0x7fffffff, but
the syscall definition in rtas.c limits nargs to 16.
Other than that, I checked:
- NULL return values from rtas_token_name were properly handled.
- the math around in_rmo_buf. It might be simpler to pass (addr, size)
rather than (start, end), but I think it's called correctly atm.
- I did a brief read-over of the basic logic, which makes sense to me.
I did not go through and compare the RTAS paramemter numbering with the
PAPR.
On that basis,
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
> + u32 arg = be32_to_cpu(args->args[i]);
> +
> + pr_cont("%08x ", arg);
> + if (arg >= rtas_rmo_buf &&
> + arg < (rtas_rmo_buf + RTAS_RMOBUF_MAX))
> + pr_cont("(buf+0x%lx) ", arg - rtas_rmo_buf);
> + }
> +
> + pr_cont("\n");
> +}
> +
> +static bool in_rmo_buf(u32 base, u32 end)
> +{
> + return base >= rtas_rmo_buf &&
> + base < (rtas_rmo_buf + RTAS_RMOBUF_MAX) &&
> + base <= end &&
> + end >= rtas_rmo_buf &&
> + end < (rtas_rmo_buf + RTAS_RMOBUF_MAX);
> +}
> +
> +static bool block_rtas_call(int token, int nargs,
> + struct rtas_args *args)
> +{
> + int i;
> + const char *reason;
> + char *token_name = rtas_token_name(token);
> +
> + if (!token_name)
> + goto err_notpermitted;
> +
> + for (i = 0; i < ARRAY_SIZE(rtas_filters); i++) {
> + struct rtas_filter *f = &rtas_filters[i];
> + u32 base, size, end;
> +
> + if (strcmp(token_name, f->name))
> + continue;
> +
> + if (f->rmo_buf_idx1 != -1) {
> + base = be32_to_cpu(args->args[f->rmo_buf_idx1]);
> + if (f->rmo_size_idx1 != -1)
> + size = be32_to_cpu(args->args[f->rmo_size_idx1]);
> + else if (!strcmp(token_name, "ibm,errinjct"))
> + size = 1024;
> + else if (!strcmp(token_name, "ibm,update-nodes") ||
> + !strcmp(token_name, "ibm,update-properties") ||
> + !strcmp(token_name, "ibm,configure-connector"))
> + size = 4096;
> + else
> + size = 1;
> +
> + end = base + size - 1;
> + if (!in_rmo_buf(base, end)) {
> + reason = "address pair 1 out of range";
> + goto err;
> + }
> + }
> +
> + if (f->rmo_buf_idx2 != -1) {
> + base = be32_to_cpu(args->args[f->rmo_buf_idx2]);
> + if (f->rmo_size_idx2 != -1)
> + size = be32_to_cpu(args->args[f->rmo_size_idx2]);
> + else if (!strcmp(token_name, "ibm,configure-connector"))
> + size = 4096;
> + else
> + size = 1;
> + end = base + size - 1;
> +
> + /*
> + * Special case for ibm,configure-connector where the
> + * address can be 0
> + */
> + if (!strcmp(token_name, "ibm,configure-connector") &&
> + base == 0)
> + return false;
> +
> + if (!in_rmo_buf(base, end)) {
> + reason = "address pair 2 out of range";
> + goto err;
> + }
> + }
> +
> + return false;
> + }
> +
> +err_notpermitted:
> + reason = "call not permitted";
> +
> +err:
> + pr_err_ratelimited("sys_rtas: RTAS call blocked - exploit attempt? (%s)\n",
> + reason);
> + dump_rtas_params(token, nargs, 0, args);
> + return true;
> +}
> +
> +#else
> +
> +static bool block_rtas_call(int token, int nargs,
> + struct rtas_args *args)
> +{
> + return false;
> +}
> +
> +#endif /* CONFIG_PPC_RTAS_FILTER */
> +
> /* We assume to be passed big endian arguments */
> SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
> {
> @@ -1147,6 +1342,9 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
> args.rets = &args.args[nargs];
> memset(args.rets, 0, nret * sizeof(rtas_arg_t));
>
> + if (block_rtas_call(token, nargs, &args))
> + return -EINVAL;
> +
> /* Need to handle ibm,suspend_me call specially */
> if (token == ibm_suspend_me_token) {
>
> --
> 2.20.1
^ permalink raw reply
* [PATCH] selftests/powerpc: Squash spurious errors due to device removal
From: Oliver O'Halloran @ 2020-07-27 1:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
For drivers that don't have the error handling callbacks we implement
recovery by removing the device and re-probing it. This causes the sysfs
directory for the PCI device to be removed which causes the following
spurious error to be printed when checking the PE state:
Breaking 0005:03:00.0...
./eeh-basic.sh: line 13: can't open /sys/bus/pci/devices/0005:03:00.0/eeh_pe_state: no such file
0005:03:00.0, waited 0/60
0005:03:00.0, waited 1/60
0005:03:00.0, waited 2/60
0005:03:00.0, waited 3/60
0005:03:00.0, waited 4/60
0005:03:00.0, waited 5/60
0005:03:00.0, waited 6/60
0005:03:00.0, waited 7/60
0005:03:00.0, Recovered after 8 seconds
We currently try to avoid this by checking if the PE state file exists
before reading from it. This is however inherently racy so re-work the
state checking so that we only read from the file once, and we squash any
errors that occur while reading.
Fixes: 85d86c8aa52e ("selftests/powerpc: Add basic EEH selftest")
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
tools/testing/selftests/powerpc/eeh/eeh-functions.sh | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/powerpc/eeh/eeh-functions.sh b/tools/testing/selftests/powerpc/eeh/eeh-functions.sh
index f52ed92b53e7..00dc32c0ed75 100755
--- a/tools/testing/selftests/powerpc/eeh/eeh-functions.sh
+++ b/tools/testing/selftests/powerpc/eeh/eeh-functions.sh
@@ -5,12 +5,17 @@ pe_ok() {
local dev="$1"
local path="/sys/bus/pci/devices/$dev/eeh_pe_state"
- if ! [ -e "$path" ] ; then
+ # if a driver doesn't support the error handling callbacks then the
+ # device is recovered by removing and re-probing it. This causes the
+ # sysfs directory to disappear so read the PE state once and squash
+ # any potential error messages
+ local eeh_state="$(cat $path 2>/dev/null)"
+ if [ -z "$eeh_state" ]; then
return 1;
fi
- local fw_state="$(cut -d' ' -f1 < $path)"
- local sw_state="$(cut -d' ' -f2 < $path)"
+ local fw_state="$(echo $eeh_state | cut -d' ' -f1)"
+ local sw_state="$(echo $eeh_state | cut -d' ' -f2)"
# If EEH_PE_ISOLATED or EEH_PE_RECOVERING are set then the PE is in an
# error state or being recovered. Either way, not ok.
--
2.26.2
^ permalink raw reply related
* Re: [PATCH] ASoC: fsl-asoc-card: Remove fsl_asoc_card_set_bias_level function
From: Nicolin Chen @ 2020-07-27 0:55 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, tiwai, lgirdwood,
perex, broonie, festevam, linux-kernel
In-Reply-To: <1595762417-2190-1-git-send-email-shengjiu.wang@nxp.com>
On Sun, Jul 26, 2020 at 07:20:17PM +0800, Shengjiu Wang wrote:
> With this case:
> aplay -Dhw:x 16khz.wav 24khz.wav
> There is sound distortion for 24khz.wav. The reason is that setting
> PLL of WM8962 with set_bias_level function, the bias level is not
> changed when 24khz.wav is played, then the PLL won't be reset, the
> clock is not correct, so distortion happens.
>
> The resolution of this issue is to remove fsl_asoc_card_set_bias_level.
> Move PLL configuration to hw_params and hw_free.
Hmm...using set_bias_level() instead of hw_params/hw_free() was
strongly suggested by Mark when I got imx-wm8962 machine driver
upstream. So we will need his input here, although I personally
don't have a problem with it...
> After removing fsl_asoc_card_set_bias_level, also test WM8960 case,
> it can work.
>
> Fixes: 708b4351f08c ("ASoC: fsl: Add Freescale Generic ASoC Sound Card with ASRC support")
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> sound/soc/fsl/fsl-asoc-card.c | 149 +++++++++++++++-------------------
> 1 file changed, 66 insertions(+), 83 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
> index 4848ba61d083..0517dbb3e908 100644
> --- a/sound/soc/fsl/fsl-asoc-card.c
> +++ b/sound/soc/fsl/fsl-asoc-card.c
> @@ -73,6 +73,7 @@ struct cpu_priv {
> * @codec_priv: CODEC private data
> * @cpu_priv: CPU private data
> * @card: ASoC card structure
> + * @is_stream_in_use: flags for release resource in hw_free
Would love to see something shorter... Could we reuse similar
one below, borrowing from fsl_ssi driver?
* @streams: Mask of current active streams: BIT(TX) and BIT(RX)
> static int fsl_asoc_card_audmux_init(struct device_node *np,
> struct fsl_asoc_card_priv *priv)
> {
> @@ -611,7 +600,6 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> /* Diversify the card configurations */
> if (of_device_is_compatible(np, "fsl,imx-audio-cs42888")) {
> codec_dai_name = "cs42888";
> - priv->card.set_bias_level = NULL;
Can check if set_bias_level is still being used with this change.
^ permalink raw reply
* Re: [PATCH 0/9] powerpc: delete duplicated words
From: Michael Ellerman @ 2020-07-26 23:55 UTC (permalink / raw)
To: Randy Dunlap, Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <b796e912-e945-3cb1-03f8-0f38009634a4@infradead.org>
Randy Dunlap <rdunlap@infradead.org> writes:
> On 7/26/20 7:29 AM, Christophe Leroy wrote:
>> Randy Dunlap <rdunlap@infradead.org> a écrit :
>>
>>> Drop duplicated words in arch/powerpc/ header files.
>>
>> How did you detect them ? Do you have some script for tgat, or you just read all comments ?
>
> Yes, it's a script that finds lots of false positives, so I have to check
> each and every one of them for validity.
>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>
>> You say you Cc Michael, but in fact you don't ... Allthough he is the powerpc maintainer
>
> Thanks for noticing that.
> [time passes]
> I checked all of my emails for this patch series and they say that Mike was Cc:ed
> on all of them.
>
> I am adding his email address back to this one.
> Mike, did you receive this patch series?
Yes.
There's a mailman option which drops me from being explicity on Cc,
because I'm subscribed to the list. Otherwise I get two copies of
everything.
So as long as it goes to linuxppc-dev I should see it, regardless of
whether I'm explicitly listed in Cc.
cheers
^ permalink raw reply
* Re: [PATCH 5/5] selftests/powerpc: Add test for pkey siginfo verification
From: Michael Ellerman @ 2020-07-26 23:52 UTC (permalink / raw)
To: Christophe Leroy
Cc: fweimer, aneesh.kumar, linuxram, Sandipan Das, linuxppc-dev,
bauerman
In-Reply-To: <20200726164009.Horde.J2bKDAPkYDkRMsx2Qnk8OQ8@messagerie.si.c-s.fr>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Michael Ellerman <mpe@ellerman.id.au> a écrit :
>> Sandipan Das <sandipan@linux.ibm.com> writes:
>>> diff --git a/tools/testing/selftests/powerpc/mm/pkey_siginfo.c
>>> b/tools/testing/selftests/powerpc/mm/pkey_siginfo.c
>>> new file mode 100644
>>> index 0000000000000..58605c53d495d
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/powerpc/mm/pkey_siginfo.c
>>> @@ -0,0 +1,332 @@
>> ...
>>> +
>>> +static void *protect(void *p)
>>> +{
>>> + unsigned long rights;
>>> + unsigned int *base;
>>> + size_t size;
>>> + int tid, i;
>>> +
>>> + tid = gettid();
>>
>> pkey_siginfo.c: In function 'protect':
>> pkey_siginfo.c:103:8: error: implicit declaration of function
>> 'gettid' [-Werror=implicit-function-declaration]
>> tid = gettid();
>> ^
>>
>>
>> On Ubuntu 18.04 at least.
>
> See https://man7.org/linux/man-pages/man2/gettid.2.html
>
> Added in glibc 2.30
Thanks, this seems to work:
diff --git a/tools/testing/selftests/powerpc/include/utils.h b/tools/testing/selftests/powerpc/include/utils.h
index 69d16875802d..71d2924f5b8b 100644
--- a/tools/testing/selftests/powerpc/include/utils.h
+++ b/tools/testing/selftests/powerpc/include/utils.h
@@ -42,6 +42,16 @@ int perf_event_enable(int fd);
int perf_event_disable(int fd);
int perf_event_reset(int fd);
+#if !defined(__GLIBC_PREREQ) || !__GLIBC_PREREQ(2, 30)
+#include <unistd.h>
+#include <sys/syscall.h>
+
+static inline pid_t gettid(void)
+{
+ return syscall(SYS_gettid);
+}
+#endif
+
static inline bool have_hwcap(unsigned long ftr)
{
return ((unsigned long)get_auxv_entry(AT_HWCAP) & ftr) == ftr;
cheers
^ permalink raw reply related
* Re: [PATCH v2 1/3] module: Rename module_alloc() to text_alloc() and move to kernel proper
From: Jarkko Sakkinen @ 2020-07-24 15:46 UTC (permalink / raw)
To: Jessica Yu
Cc: Catalin Marinas, Kefeng Wang, Paul Mackerras, Zong Li, Andi Kleen,
Paul Burton, Vincent Whitchurch, Petr Mladek, Brian Gerst,
Andy Lutomirski, Thomas Gleixner, Jiri Kosina, Anup Patel,
Linux Kernel Mailing List, Philipp Rudo, Torsten Duwe,
Masami Hiramatsu, Andrew Morton, Mark Rutland,
James E.J. Bottomley, Vincent Chen, Omar Sandoval, open list:S390,
Joe Lawrence, Helge Deller, John Fastabend, Anil S Keshavamurthy,
Yonghong Song, Iurii Zaikin, Andrii Nakryiko, Thomas Huth,
Vasily Gorbik, moderated list:ARM PORT, Daniel Axtens,
Damien Le Moal, Martin KaFai Lau, Song Liu, Josh Poimboeuf,
Heiko Carstens, Alexei Starovoitov, Atish Patra, Will Deacon,
Daniel Borkmann, Masahiro Yamada, Nayna Jain, Ley Foon Tan,
Christian Borntraeger, Sami Tolvanen, Naveen N. Rao, Mao Han,
Marco Elver, Steven Rostedt, Babu Moger, Borislav Petkov,
Greentime Hu, Ben Dooks, Guan Xuetao, Tiezhu Yang,
Thomas Bogendoerfer, open list:PARISC ARCHITECTURE,
open list:BPF JIT for MIPS 32-BIT AND 64-BIT, David S. Miller,
Thiago Jung Bauermann, Peter Zijlstra, David Howells,
Amit Daniel Kachhap, Sandipan Das, H. Peter Anvin,
open list:SPARC + UltraSPARC sparc/sparc64,
open list:RISC-V ARCHITECTURE, Miroslav Benes, Jiri Olsa,
Ard Biesheuvel, Vincenzo Frascino, Anders Roxell, Sven Schnelle,
maintainer:X86 ARCHITECTURE 32-BIT AND 64-BIT, Russell King,
Mike Rapoport, Ingo Molnar, Albert Ou, Paul E. McKenney,
Paul Walmsley, KP Singh, Dmitry Vyukov, Nick Hu,
open list:BPF JIT for MIPS 32-BIT AND 64-BIT, open list:MIPS,
Palmer Dabbelt, open list:LINUX FOR POWERPC 32-BIT AND 64-BIT
In-Reply-To: <20200724080508.GA17719@linux-8ccs>
On Fri, Jul 24, 2020 at 10:05:08AM +0200, Jessica Yu wrote:
> +++ Jarkko Sakkinen [24/07/20 10:36 +0300]:
> > On Thu, Jul 23, 2020 at 03:42:09PM +0300, Ard Biesheuvel wrote:
> > > On Thu, 23 Jul 2020 at 04:52, Jarkko Sakkinen
> > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > >
> > > > On Thu, Jul 16, 2020 at 06:49:09PM +0200, Christophe Leroy wrote:
> > > > > Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> a écrit :
> > > > >
> > > > > > Rename module_alloc() to text_alloc() and module_memfree() to
> > > > > > text_memfree(), and move them to kernel/text.c, which is unconditionally
> > > > > > compiled to the kernel proper. This allows kprobes, ftrace and bpf to
> > > > > > allocate space for executable code without requiring to compile the modules
> > > > > > support (CONFIG_MODULES=y) in.
> > > > >
> > > > > You are not changing enough in powerpc to have this work.
> > > > > On powerpc 32 bits (6xx), when STRICT_KERNEL_RWX is selected, the vmalloc
> > > > > space is set to NX (no exec) at segment level (ie by 256Mbytes zone) unless
> > > > > CONFIG_MODULES is selected.
> > > > >
> > > > > Christophe
> > > >
> > > > This has been deduced down to:
> > > >
> > > > https://lore.kernel.org/lkml/20200717030422.679972-1-jarkko.sakkinen@linux.intel.com/
> > > >
> > > > I.e. not intruding PPC anymore :-)
> > > >
> > >
> > > Ok, so after the elaborate discussion we had between Jessica, Russell,
> > > Peter, Will, Mark, you and myself, where we pointed out that
> > > a) a single text_alloc() abstraction for bpf, kprobes and ftrace does
> > > not fit other architectures very well, and
> > > b) that module_alloc() is not suitable as a default to base text_alloc() on,
> >
> > In the latest iteration (v5) it is conditionally available only if arch
> > defines and fallback has been removed.
> >
> > > you went ahead and implemented that anyway, but only cc'ing Peter,
> > > akpm, Masami and the mm list this time?
> >
> > No problems with that. Actually each patch gets everything that
> > get_maintainer.pl gives with a cc cmd script, not just the ones
> > explicitly listed in the patch.
> >
> > Should I explicitly CC you to the next version? I'm happy to grow
> > the list when requested.
>
> Yes, please CC everybody that was part of the discussion last time
> especially during v2, and please use a consistent CC list for the
> whole patchset. It is difficult to review when you only receive patch
> 1 out of 6 with no mention of text_alloc() anywhere and without being
> CC'd on the cover letter.
>
> Jessica
I'll make the following additions to the CC list (in alphabetical
order):
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Will Deacon <will@kernel.org>
This is also reflected now in the kprobes branch of
https://github.com/jsakkine-intel/linux-sgx.git. My candidate patches
are located in my SGX tree because I mainly use them when poking SGX.
The patches are kind of sidekick of SGX upstreaming process.
/Jarkko
^ permalink raw reply
* Re: [PATCH 0/9] powerpc: delete duplicated words
From: Joe Perches @ 2020-07-26 20:48 UTC (permalink / raw)
To: Randy Dunlap; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <4e505c35-8428-89bb-7f9b-bc819382c3cd@infradead.org>
On 2020-07-26 12:08, Randy Dunlap wrote:
> On 7/26/20 10:49 AM, Joe Perches wrote:
>> On Sun, 2020-07-26 at 10:23 -0700, Randy Dunlap wrote:
>>> On 7/26/20 7:29 AM, Christophe Leroy wrote:
>>>> Randy Dunlap <rdunlap@infradead.org> a écrit :
>>>>
>>>>> Drop duplicated words in arch/powerpc/ header files.
>>>>
>>>> How did you detect them ? Do you have some script for tgat, or you
>>>> just read all comments ?
>>>
>>> Yes, it's a script that finds lots of false positives, so I have to
>>> check
>>> each and every one of them for validity.
>>
>> And it's a lot of work too. (thanks Randy)
>>
>> It could be something like:
>>
>> $ grep-2.5.4 -nrP --include=*.[ch] '\b([A-Z]?[a-z]{2,}\b)[ \t]*(?:\n[
>> \t]*\*[ \t]*|)\1\b' * | \
>> grep -vP '\b(?:struct|enum|union)\s+([A-Z]?[a-z]{2,})\s+\*?\s*\1\b'
>> | \
>> grep -vP '\blong\s+long\b' | \
>> grep -vP '\b([A-Z]?[a-z]{2,})(?:\t+| {2,})\1\b'
>
> Hi Joe,
Hi Randy
> (what is grep-2.5.4 ?)
It's the last version of grep that allowed spanning multiple lines.
That's to find the comment second lines that start with *
> It looks like you tried a few iterations of this -- since it drops
> things
> like "long long". There are lots of data types that are repeated &
> valid.
> And many struct names, like "struct kref kref", "struct completion
> completion",
> and "struct mutex mutex". I handle (ignore) those manually
that's the first exclude pattern.
^ permalink raw reply
* [RESEND PATCH v5 11/11] ppc64/kexec_file: fix kexec load failure with lack of memory hole
From: Hari Bathini @ 2020-07-26 19:40 UTC (permalink / raw)
To: Michael Ellerman, Andrew Morton
Cc: Pingfan Liu, Kexec-ml, Mimi Zohar, Nayna Jain, Petr Tesarik,
Mahesh J Salgaonkar, Sourabh Jain, lkml, linuxppc-dev,
Thiago Jung Bauermann, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <159579157320.5790.6748078824637688685.stgit@hbathini>
The kexec purgatory has to run in real mode. Only the first memory
block maybe accessible in real mode. And, unlike the case with panic
kernel, no memory is set aside for regular kexec load. Another thing
to note is, the memory for crashkernel is reserved at an offset of
128MB. So, when crashkernel memory is reserved, the memory ranges to
load kexec segments shrink further as the generic code only looks for
memblock free memory ranges and in all likelihood only a tiny bit of
memory from 0 to 128MB would be available to load kexec segments.
With kdump being used by default in general, kexec file load is likely
to fail almost always. This can be fixed by changing the memory hole
lookup logic for regular kexec to use the same method as kdump. This
would mean that most kexec segments will overlap with crashkernel
memory region. That should still be ok as the pages, whose destination
address isn't available while loading, are placed in an intermediate
location till a flush to the actual destination address happens during
kexec boot sequence.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Tested-by: Pingfan Liu <piliu@redhat.com>
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
v4 -> v5:
* Unchanged.
v3 -> v4:
* Unchanged. Added Reviewed-by tag from Thiago.
v2 -> v3:
* Unchanged. Added Tested-by tag from Pingfan.
v1 -> v2:
* New patch to fix locating memory hole for kexec_file_load (kexec -s -l)
when memory is reserved for crashkernel.
arch/powerpc/kexec/file_load_64.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 296be7fc6440..7933b8990714 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -1122,13 +1122,6 @@ int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf)
u64 buf_min, buf_max;
int ret;
- /*
- * Use the generic kexec_locate_mem_hole for regular
- * kexec_file_load syscall
- */
- if (kbuf->image->type != KEXEC_TYPE_CRASH)
- return kexec_locate_mem_hole(kbuf);
-
/* Look up the exclude ranges list while locating the memory hole */
emem = &(kbuf->image->arch.exclude_ranges);
if (!(*emem) || ((*emem)->nr_ranges == 0)) {
@@ -1136,11 +1129,15 @@ int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf)
return kexec_locate_mem_hole(kbuf);
}
+ buf_min = kbuf->buf_min;
+ buf_max = kbuf->buf_max;
/* Segments for kdump kernel should be within crashkernel region */
- buf_min = (kbuf->buf_min < crashk_res.start ?
- crashk_res.start : kbuf->buf_min);
- buf_max = (kbuf->buf_max > crashk_res.end ?
- crashk_res.end : kbuf->buf_max);
+ if (kbuf->image->type == KEXEC_TYPE_CRASH) {
+ buf_min = (buf_min < crashk_res.start ?
+ crashk_res.start : buf_min);
+ buf_max = (buf_max > crashk_res.end ?
+ crashk_res.end : buf_max);
+ }
if (buf_min > buf_max) {
pr_err("Invalid buffer min and/or max values\n");
@@ -1177,15 +1174,13 @@ int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf)
int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
unsigned long buf_len)
{
- if (image->type == KEXEC_TYPE_CRASH) {
- int ret;
+ int ret;
- /* Get exclude memory ranges needed for setting up kdump segments */
- ret = get_exclude_memory_ranges(&(image->arch.exclude_ranges));
- if (ret) {
- pr_err("Failed to setup exclude memory ranges for buffer lookup\n");
- return ret;
- }
+ /* Get exclude memory ranges needed for setting up kexec segments */
+ ret = get_exclude_memory_ranges(&(image->arch.exclude_ranges));
+ if (ret) {
+ pr_err("Failed to setup exclude memory ranges for buffer lookup\n");
+ return ret;
}
return kexec_image_probe_default(image, buf, buf_len);
^ permalink raw reply related
* [RESEND PATCH v5 10/11] ppc64/kexec_file: add appropriate regions for memory reserve map
From: Hari Bathini @ 2020-07-26 19:40 UTC (permalink / raw)
To: Michael Ellerman, Andrew Morton
Cc: Pingfan Liu, Kexec-ml, Mimi Zohar, Nayna Jain, Petr Tesarik,
Mahesh J Salgaonkar, Sourabh Jain, lkml, linuxppc-dev,
Thiago Jung Bauermann, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <159579157320.5790.6748078824637688685.stgit@hbathini>
While initrd, elfcorehdr and backup regions are already added to the
reserve map, there are a few missing regions that need to be added to
the memory reserve map. Add them here. And now that all the changes
to load panic kernel are in place, claim likewise.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Tested-by: Pingfan Liu <piliu@redhat.com>
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
v4 -> v5:
* Unchanged.
v3 -> v4:
* Fixed a spellcheck and added Reviewed-by tag from Thiago.
v2 -> v3:
* Unchanged. Added Tested-by tag from Pingfan.
v1 -> v2:
* Updated add_rtas_mem_range() & add_opal_mem_range() callsites based on
the new prototype for these functions.
arch/powerpc/kexec/file_load_64.c | 58 ++++++++++++++++++++++++++++++++++---
1 file changed, 53 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 7a52f0634ce6..296be7fc6440 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -201,6 +201,34 @@ static int get_crash_memory_ranges(struct crash_mem **mem_ranges)
return ret;
}
+/**
+ * get_reserved_memory_ranges - Get reserve memory ranges. This list includes
+ * memory regions that should be added to the
+ * memory reserve map to ensure the region is
+ * protected from any mischief.
+ * @mem_ranges: Range list to add the memory ranges to.
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+static int get_reserved_memory_ranges(struct crash_mem **mem_ranges)
+{
+ int ret;
+
+ ret = add_rtas_mem_range(mem_ranges);
+ if (ret)
+ goto out;
+
+ ret = add_tce_mem_ranges(mem_ranges);
+ if (ret)
+ goto out;
+
+ ret = add_reserved_ranges(mem_ranges);
+out:
+ if (ret)
+ pr_err("Failed to setup reserved memory ranges\n");
+ return ret;
+}
+
/**
* __locate_mem_hole_top_down - Looks top down for a large enough memory hole
* in the memory regions between buf_min & buf_max
@@ -1007,8 +1035,8 @@ int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
unsigned long initrd_load_addr,
unsigned long initrd_len, const char *cmdline)
{
- struct crash_mem *umem = NULL;
- int ret;
+ struct crash_mem *umem = NULL, *rmem = NULL;
+ int i, nr_ranges, ret;
ret = setup_new_fdt(image, fdt, initrd_load_addr, initrd_len, cmdline);
if (ret)
@@ -1051,7 +1079,27 @@ int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
}
}
+ /* Update memory reserve map */
+ ret = get_reserved_memory_ranges(&rmem);
+ if (ret)
+ goto out;
+
+ nr_ranges = rmem ? rmem->nr_ranges : 0;
+ for (i = 0; i < nr_ranges; i++) {
+ u64 base, size;
+
+ base = rmem->ranges[i].start;
+ size = rmem->ranges[i].end - base + 1;
+ ret = fdt_add_mem_rsv(fdt, base, size);
+ if (ret) {
+ pr_err("Error updating memory reserve map: %s\n",
+ fdt_strerror(ret));
+ goto out;
+ }
+ }
+
out:
+ kfree(rmem);
kfree(umem);
return ret;
}
@@ -1134,10 +1182,10 @@ int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
/* Get exclude memory ranges needed for setting up kdump segments */
ret = get_exclude_memory_ranges(&(image->arch.exclude_ranges));
- if (ret)
+ if (ret) {
pr_err("Failed to setup exclude memory ranges for buffer lookup\n");
- /* Return this until all changes for panic kernel are in */
- return -EOPNOTSUPP;
+ return ret;
+ }
}
return kexec_image_probe_default(image, buf, buf_len);
^ permalink raw reply related
* [RESEND PATCH v5 09/11] ppc64/kexec_file: prepare elfcore header for crashing kernel
From: Hari Bathini @ 2020-07-26 19:39 UTC (permalink / raw)
To: Michael Ellerman, Andrew Morton
Cc: Pingfan Liu, Kexec-ml, Mimi Zohar, Nayna Jain, Petr Tesarik,
Mahesh J Salgaonkar, Sourabh Jain, lkml, linuxppc-dev,
Thiago Jung Bauermann, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <159579157320.5790.6748078824637688685.stgit@hbathini>
Prepare elf headers for the crashing kernel's core file using
crash_prepare_elf64_headers() and pass on this info to kdump
kernel by updating its command line with elfcorehdr parameter.
Also, add elfcorehdr location to reserve map to avoid it from
being stomped on while booting.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Tested-by: Pingfan Liu <piliu@redhat.com>
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
v4 -> v5:
* Unchanged. Added Reviewed-by tag from Thiago.
v3 -> v4:
* Added a FIXME tag to indicate issue in adding opal/rtas regions to
core image.
* Folded prepare_elf_headers() function into load_elfcorehdr_segment().
v2 -> v3:
* Unchanged. Added Tested-by tag from Pingfan.
v1 -> v2:
* Tried merging adjacent memory ranges on hitting maximum ranges limit
to reduce reallocations for memory ranges and also, minimize PT_LOAD
segments for elfcore.
* Updated add_rtas_mem_range() & add_opal_mem_range() callsites based on
the new prototype for these functions.
arch/powerpc/include/asm/kexec.h | 6 +
arch/powerpc/kexec/elf_64.c | 12 +++
arch/powerpc/kexec/file_load.c | 49 +++++++++++
arch/powerpc/kexec/file_load_64.c | 165 +++++++++++++++++++++++++++++++++++++
4 files changed, 232 insertions(+)
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index f9514ebeffaa..fe885bc3127e 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -108,12 +108,18 @@ struct kimage_arch {
unsigned long backup_start;
void *backup_buf;
+ unsigned long elfcorehdr_addr;
+ unsigned long elf_headers_sz;
+ void *elf_headers;
+
#ifdef CONFIG_IMA_KEXEC
phys_addr_t ima_buffer_addr;
size_t ima_buffer_size;
#endif
};
+char *setup_kdump_cmdline(struct kimage *image, char *cmdline,
+ unsigned long cmdline_len);
int setup_purgatory(struct kimage *image, const void *slave_code,
const void *fdt, unsigned long kernel_load_addr,
unsigned long fdt_load_addr);
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 76e2fc7e6dc3..d0e459bb2f05 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -35,6 +35,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
void *fdt;
const void *slave_code;
struct elfhdr ehdr;
+ char *modified_cmdline = NULL;
struct kexec_elf_info elf_info;
struct kexec_buf kbuf = { .image = image, .buf_min = 0,
.buf_max = ppc64_rma_size };
@@ -75,6 +76,16 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
pr_err("Failed to load kdump kernel segments\n");
goto out;
}
+
+ /* Setup cmdline for kdump kernel case */
+ modified_cmdline = setup_kdump_cmdline(image, cmdline,
+ cmdline_len);
+ if (!modified_cmdline) {
+ pr_err("Setting up cmdline for kdump kernel failed\n");
+ ret = -EINVAL;
+ goto out;
+ }
+ cmdline = modified_cmdline;
}
if (initrd != NULL) {
@@ -131,6 +142,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
pr_err("Error setting up the purgatory.\n");
out:
+ kfree(modified_cmdline);
kexec_free_elf_info(&elf_info);
/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
diff --git a/arch/powerpc/kexec/file_load.c b/arch/powerpc/kexec/file_load.c
index 38439aba27d7..d52c09729edd 100644
--- a/arch/powerpc/kexec/file_load.c
+++ b/arch/powerpc/kexec/file_load.c
@@ -18,10 +18,45 @@
#include <linux/kexec.h>
#include <linux/of_fdt.h>
#include <linux/libfdt.h>
+#include <asm/setup.h>
#include <asm/ima.h>
#define SLAVE_CODE_SIZE 256 /* First 0x100 bytes */
+/**
+ * setup_kdump_cmdline - Prepend "elfcorehdr=<addr> " to command line
+ * of kdump kernel for exporting the core.
+ * @image: Kexec image
+ * @cmdline: Command line parameters to update.
+ * @cmdline_len: Length of the cmdline parameters.
+ *
+ * kdump segment must be setup before calling this function.
+ *
+ * Returns new cmdline buffer for kdump kernel on success, NULL otherwise.
+ */
+char *setup_kdump_cmdline(struct kimage *image, char *cmdline,
+ unsigned long cmdline_len)
+{
+ int elfcorehdr_strlen;
+ char *cmdline_ptr;
+
+ cmdline_ptr = kzalloc(COMMAND_LINE_SIZE, GFP_KERNEL);
+ if (!cmdline_ptr)
+ return NULL;
+
+ elfcorehdr_strlen = sprintf(cmdline_ptr, "elfcorehdr=0x%lx ",
+ image->arch.elfcorehdr_addr);
+
+ if (elfcorehdr_strlen + cmdline_len > COMMAND_LINE_SIZE) {
+ pr_err("Appending elfcorehdr=<addr> exceeds cmdline size\n");
+ kfree(cmdline_ptr);
+ return NULL;
+ }
+
+ memcpy(cmdline_ptr + elfcorehdr_strlen, cmdline, cmdline_len);
+ return cmdline_ptr;
+}
+
/**
* setup_purgatory - initialize the purgatory's global variables
* @image: kexec image.
@@ -221,6 +256,20 @@ int setup_new_fdt(const struct kimage *image, void *fdt,
}
}
+ if (image->type == KEXEC_TYPE_CRASH) {
+ /*
+ * Avoid elfcorehdr from being stomped on in kdump kernel by
+ * setting up memory reserve map.
+ */
+ ret = fdt_add_mem_rsv(fdt, image->arch.elfcorehdr_addr,
+ image->arch.elf_headers_sz);
+ if (ret) {
+ pr_err("Error reserving elfcorehdr memory: %s\n",
+ fdt_strerror(ret));
+ goto err;
+ }
+ }
+
ret = setup_ima_buffer(image, fdt, chosen_node);
if (ret) {
pr_err("Error setting up the new device tree.\n");
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 88408b17a7f6..7a52f0634ce6 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -124,6 +124,83 @@ static int get_usable_memory_ranges(struct crash_mem **mem_ranges)
return ret;
}
+/**
+ * get_crash_memory_ranges - Get crash memory ranges. This list includes
+ * first/crashing kernel's memory regions that
+ * would be exported via an elfcore.
+ * @mem_ranges: Range list to add the memory ranges to.
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+static int get_crash_memory_ranges(struct crash_mem **mem_ranges)
+{
+ struct memblock_region *reg;
+ struct crash_mem *tmem;
+ int ret;
+
+ for_each_memblock(memory, reg) {
+ u64 base, size;
+
+ base = (u64)reg->base;
+ size = (u64)reg->size;
+
+ /* Skip backup memory region, which needs a separate entry */
+ if (base == BACKUP_SRC_START) {
+ if (size > BACKUP_SRC_SIZE) {
+ base = BACKUP_SRC_END + 1;
+ size -= BACKUP_SRC_SIZE;
+ } else
+ continue;
+ }
+
+ ret = add_mem_range(mem_ranges, base, size);
+ if (ret)
+ goto out;
+
+ /* Try merging adjacent ranges before reallocation attempt */
+ if ((*mem_ranges)->nr_ranges == (*mem_ranges)->max_nr_ranges)
+ sort_memory_ranges(*mem_ranges, true);
+ }
+
+ /* Reallocate memory ranges if there is no space to split ranges */
+ tmem = *mem_ranges;
+ if (tmem && (tmem->nr_ranges == tmem->max_nr_ranges)) {
+ tmem = realloc_mem_ranges(mem_ranges);
+ if (!tmem)
+ goto out;
+ }
+
+ /* Exclude crashkernel region */
+ ret = crash_exclude_mem_range(tmem, crashk_res.start, crashk_res.end);
+ if (ret)
+ goto out;
+
+ /*
+ * FIXME: For now, stay in parity with kexec-tools but if RTAS/OPAL
+ * regions are exported to save their context at the time of
+ * crash, they should actually be backed up just like the
+ * first 64K bytes of memory.
+ */
+ ret = add_rtas_mem_range(mem_ranges);
+ if (ret)
+ goto out;
+
+ ret = add_opal_mem_range(mem_ranges);
+ if (ret)
+ goto out;
+
+ /* create a separate program header for the backup region */
+ ret = add_mem_range(mem_ranges, BACKUP_SRC_START, BACKUP_SRC_SIZE);
+ if (ret)
+ goto out;
+
+ sort_memory_ranges(*mem_ranges, false);
+out:
+ if (ret)
+ pr_err("Failed to setup crash memory ranges\n");
+ return ret;
+}
+
/**
* __locate_mem_hole_top_down - Looks top down for a large enough memory hole
* in the memory regions between buf_min & buf_max
@@ -738,6 +815,81 @@ static int load_backup_segment(struct kimage *image, struct kexec_buf *kbuf)
return 0;
}
+/**
+ * update_backup_region_phdr - Update backup region's offset for the core to
+ * export the region appropriately.
+ * @image: Kexec image.
+ * @ehdr: ELF core header.
+ *
+ * Assumes an exclusive program header is setup for the backup region
+ * in the ELF headers
+ *
+ * Returns nothing.
+ */
+static void update_backup_region_phdr(struct kimage *image, Elf64_Ehdr *ehdr)
+{
+ Elf64_Phdr *phdr;
+ unsigned int i;
+
+ phdr = (Elf64_Phdr *)(ehdr + 1);
+ for (i = 0; i < ehdr->e_phnum; i++) {
+ if (phdr->p_paddr == BACKUP_SRC_START) {
+ phdr->p_offset = image->arch.backup_start;
+ pr_debug("Backup region offset updated to 0x%lx\n",
+ image->arch.backup_start);
+ return;
+ }
+ }
+}
+
+/**
+ * load_elfcorehdr_segment - Setup crash memory ranges and initialize elfcorehdr
+ * segment needed to load kdump kernel.
+ * @image: Kexec image.
+ * @kbuf: Buffer contents and memory parameters.
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf)
+{
+ struct crash_mem *cmem = NULL;
+ unsigned long headers_sz;
+ void *headers = NULL;
+ int ret;
+
+ ret = get_crash_memory_ranges(&cmem);
+ if (ret)
+ goto out;
+
+ /* Setup elfcorehdr segment */
+ ret = crash_prepare_elf64_headers(cmem, false, &headers, &headers_sz);
+ if (ret) {
+ pr_err("Failed to prepare elf headers for the core\n");
+ goto out;
+ }
+
+ /* Fix the offset for backup region in the ELF header */
+ update_backup_region_phdr(image, headers);
+
+ kbuf->buffer = headers;
+ kbuf->mem = KEXEC_BUF_MEM_UNKNOWN;
+ kbuf->bufsz = kbuf->memsz = headers_sz;
+ kbuf->top_down = false;
+
+ ret = kexec_add_buffer(kbuf);
+ if (ret) {
+ vfree(headers);
+ goto out;
+ }
+
+ image->arch.elfcorehdr_addr = kbuf->mem;
+ image->arch.elf_headers_sz = headers_sz;
+ image->arch.elf_headers = headers;
+out:
+ kfree(cmem);
+ return ret;
+}
+
/**
* load_crashdump_segments_ppc64 - Initialize the additional segements needed
* to load kdump kernel.
@@ -759,6 +911,15 @@ int load_crashdump_segments_ppc64(struct kimage *image,
}
pr_debug("Loaded the backup region at 0x%lx\n", kbuf->mem);
+ /* Load elfcorehdr segment - to export crashing kernel's vmcore */
+ ret = load_elfcorehdr_segment(image, kbuf);
+ if (ret) {
+ pr_err("Failed to load elfcorehdr segment\n");
+ return ret;
+ }
+ pr_debug("Loaded elf core header at 0x%lx, bufsz=0x%lx memsz=0x%lx\n",
+ image->arch.elfcorehdr_addr, kbuf->bufsz, kbuf->memsz);
+
return 0;
}
@@ -997,5 +1158,9 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
vfree(image->arch.backup_buf);
image->arch.backup_buf = NULL;
+ vfree(image->arch.elf_headers);
+ image->arch.elf_headers = NULL;
+ image->arch.elf_headers_sz = 0;
+
return kexec_image_post_load_cleanup_default(image);
}
^ 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