* [PATCH v5 4/6] sched/fair: Provide update_sg_lb_stats() with sched domain statistics
From: Ricardo Neri @ 2021-09-11 1:18 UTC (permalink / raw)
To: Peter Zijlstra (Intel), Ingo Molnar, Juri Lelli, Vincent Guittot
Cc: Len Brown, Aubrey Li, Rafael J . Wysocki, Srikar Dronamraju,
Ravi V. Shankar, linuxppc-dev, Aubrey Li, Nicholas Piggin,
Ricardo Neri, Steven Rostedt, Quentin Perret, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Srinivas Pandruvada,
Joel Fernandes (Google), Tim Chen, Dietmar Eggemann, linux-kernel,
Ricardo Neri
In-Reply-To: <20210911011819.12184-1-ricardo.neri-calderon@linux.intel.com>
Before deciding to pull tasks when using asymmetric packing of tasks,
on some architectures (e.g., x86) it is necessary to know not only the
state of dst_cpu but also of its SMT siblings. The decision to classify
a candidate busiest group as group_asym_packing is done in
update_sg_lb_stats(). Give this function access to the scheduling domain
statistics, which contains the statistics of the local group.
Cc: Aubrey Li <aubrey.li@intel.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Quentin Perret <qperret@google.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Reviewed-by: Len Brown <len.brown@intel.com>
Originally-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v4:
* None
Changes since v3:
* None
Changes since v2:
* Introduced this patch.
Changes since v1:
* N/A
---
kernel/sched/fair.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 7a054f528bcc..c5851260b4d8 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8605,6 +8605,7 @@ group_type group_classify(unsigned int imbalance_pct,
* @sg_status: Holds flag indicating the status of the sched_group
*/
static inline void update_sg_lb_stats(struct lb_env *env,
+ struct sd_lb_stats *sds,
struct sched_group *group,
struct sg_lb_stats *sgs,
int *sg_status)
@@ -8613,7 +8614,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
memset(sgs, 0, sizeof(*sgs));
- local_group = cpumask_test_cpu(env->dst_cpu, sched_group_span(group));
+ local_group = group == sds->local;
for_each_cpu_and(i, sched_group_span(group), env->cpus) {
struct rq *rq = cpu_rq(i);
@@ -9176,7 +9177,7 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
update_group_capacity(env->sd, env->dst_cpu);
}
- update_sg_lb_stats(env, sg, sgs, &sg_status);
+ update_sg_lb_stats(env, sds, sg, sgs, &sg_status);
if (local_group)
goto next_group;
--
2.17.1
^ permalink raw reply related
* [PATCH v5 5/6] sched/fair: Carve out logic to mark a group for asymmetric packing
From: Ricardo Neri @ 2021-09-11 1:18 UTC (permalink / raw)
To: Peter Zijlstra (Intel), Ingo Molnar, Juri Lelli, Vincent Guittot
Cc: Len Brown, Aubrey Li, Rafael J . Wysocki, Srikar Dronamraju,
Ravi V. Shankar, linuxppc-dev, Aubrey Li, Nicholas Piggin,
Ricardo Neri, Steven Rostedt, Quentin Perret, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Srinivas Pandruvada,
Joel Fernandes (Google), Tim Chen, Dietmar Eggemann, linux-kernel,
Ricardo Neri
In-Reply-To: <20210911011819.12184-1-ricardo.neri-calderon@linux.intel.com>
Create a separate function, sched_asym(). A subsequent changeset will
introduce logic to deal with SMT in conjunction with asmymmetric
packing. Such logic will need the statistics of the scheduling
group provided as argument. Update them before calling sched_asym().
Cc: Aubrey Li <aubrey.li@intel.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Quentin Perret <qperret@google.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Reviewed-by: Len Brown <len.brown@intel.com>
Co-developed-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v4:
* None
Changes since v3:
* Remove a redundant check for the local group in sched_asym().
(Dietmar)
* Reworded commit message for clarity. (Len)
Changes since v2:
* Introduced this patch.
Changes since v1:
* N/A
---
kernel/sched/fair.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index c5851260b4d8..26db017c14a3 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8597,6 +8597,13 @@ group_type group_classify(unsigned int imbalance_pct,
return group_has_spare;
}
+static inline bool
+sched_asym(struct lb_env *env, struct sd_lb_stats *sds, struct sg_lb_stats *sgs,
+ struct sched_group *group)
+{
+ return sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu);
+}
+
/**
* update_sg_lb_stats - Update sched_group's statistics for load balancing.
* @env: The load balancing environment.
@@ -8657,18 +8664,17 @@ static inline void update_sg_lb_stats(struct lb_env *env,
}
}
+ sgs->group_capacity = group->sgc->capacity;
+
+ sgs->group_weight = group->group_weight;
+
/* Check if dst CPU is idle and preferred to this group */
if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
- env->idle != CPU_NOT_IDLE &&
- sgs->sum_h_nr_running &&
- sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu)) {
+ env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
+ sched_asym(env, sds, sgs, group)) {
sgs->group_asym_packing = 1;
}
- sgs->group_capacity = group->sgc->capacity;
-
- sgs->group_weight = group->group_weight;
-
sgs->group_type = group_classify(env->sd->imbalance_pct, group, sgs);
/* Computing avg_load makes sense only when group is overloaded */
--
2.17.1
^ permalink raw reply related
* [PATCH v5 6/6] sched/fair: Consider SMT in ASYM_PACKING load balance
From: Ricardo Neri @ 2021-09-11 1:18 UTC (permalink / raw)
To: Peter Zijlstra (Intel), Ingo Molnar, Juri Lelli, Vincent Guittot
Cc: Len Brown, Aubrey Li, Rafael J . Wysocki, Srikar Dronamraju,
Ravi V. Shankar, linuxppc-dev, Aubrey Li, Nicholas Piggin,
Ricardo Neri, Steven Rostedt, Quentin Perret, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Srinivas Pandruvada,
Joel Fernandes (Google), Tim Chen, Dietmar Eggemann, linux-kernel,
Ricardo Neri
In-Reply-To: <20210911011819.12184-1-ricardo.neri-calderon@linux.intel.com>
When deciding to pull tasks in ASYM_PACKING, it is necessary not only to
check for the idle state of the destination CPU, dst_cpu, but also of
its SMT siblings.
If dst_cpu is idle but its SMT siblings are busy, performance suffers
if it pulls tasks from a medium priority CPU that does not have SMT
siblings.
Implement asym_smt_can_pull_tasks() to inspect the state of the SMT
siblings of both dst_cpu and the CPUs in the candidate busiest group.
Cc: Aubrey Li <aubrey.li@intel.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Quentin Perret <qperret@google.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Reviewed-by: Len Brown <len.brown@intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v4:
* Use sg_lb_stats::sum_nr_running the idle state of a scheduling group.
(Vincent, Peter)
* Do not even idle CPUs in asym_smt_can_pull_tasks(). (Vincent)
* Updated function documentation and corrected a typo.
Changes since v3:
* Removed the arch_asym_check_smt_siblings() hook. Discussions with the
powerpc folks showed that this patch should not impact them. Also, more
recent powerpc processor no longer use asym_packing. (PeterZ)
* Removed unnecessary local variable in asym_can_pull_tasks(). (Dietmar)
* Removed unnecessary check for local CPUs when the local group has zero
utilization. (Joel)
* Renamed asym_can_pull_tasks() as asym_smt_can_pull_tasks() to reflect
the fact that it deals with SMT cases.
* Made asym_smt_can_pull_tasks() return false for !CONFIG_SCHED_SMT so
that callers can deal with non-SMT cases.
Changes since v2:
* Reworded the commit message to reflect updates in code.
* Corrected misrepresentation of dst_cpu as the CPU doing the load
balancing. (PeterZ)
* Removed call to arch_asym_check_smt_siblings() as it is now called in
sched_asym().
Changes since v1:
* Don't bailout in update_sd_pick_busiest() if dst_cpu cannot pull
tasks. Instead, reclassify the candidate busiest group, as it
may still be selected. (PeterZ)
* Avoid an expensive and unnecessary call to cpumask_weight() when
determining if a sched_group is comprised of SMT siblings.
(PeterZ).
---
kernel/sched/fair.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 94 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 26db017c14a3..8d763dd0174b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8597,10 +8597,98 @@ group_type group_classify(unsigned int imbalance_pct,
return group_has_spare;
}
+/**
+ * asym_smt_can_pull_tasks - Check whether the load balancing CPU can pull tasks
+ * @dst_cpu: Destination CPU of the load balancing
+ * @sds: Load-balancing data with statistics of the local group
+ * @sgs: Load-balancing statistics of the candidate busiest group
+ * @sg: The candidate busiest group
+ *
+ * Check the state of the SMT siblings of both @sds::local and @sg and decide
+ * if @dst_cpu can pull tasks.
+ *
+ * If @dst_cpu does not have SMT siblings, it can pull tasks if two or more of
+ * the SMT siblings of @sg are busy. If only one CPU in @sg is busy, pull tasks
+ * only if @dst_cpu has higher priority.
+ *
+ * If both @dst_cpu and @sg have SMT siblings, and @sg has exactly one more
+ * busy CPU than @sds::local, let @dst_cpu pull tasks if it has higher priority.
+ * Bigger imbalances in the number of busy CPUs will be dealt with in
+ * update_sd_pick_busiest().
+ *
+ * If @sg does not have SMT siblings, only pull tasks if all of the SMT siblings
+ * of @dst_cpu are idle and @sg has lower priority.
+ */
+static bool asym_smt_can_pull_tasks(int dst_cpu, struct sd_lb_stats *sds,
+ struct sg_lb_stats *sgs,
+ struct sched_group *sg)
+{
+#ifdef CONFIG_SCHED_SMT
+ bool local_is_smt, sg_is_smt;
+ int sg_busy_cpus;
+
+ local_is_smt = sds->local->flags & SD_SHARE_CPUCAPACITY;
+ sg_is_smt = sg->flags & SD_SHARE_CPUCAPACITY;
+
+ sg_busy_cpus = sgs->group_weight - sgs->idle_cpus;
+
+ if (!local_is_smt) {
+ /*
+ * If we are here, @dst_cpu is idle and does not have SMT
+ * siblings. Pull tasks if candidate group has two or more
+ * busy CPUs.
+ */
+ if (sg_is_smt && sg_busy_cpus >= 2)
+ return true;
+
+ /*
+ * @dst_cpu does not have SMT siblings. @sg may have SMT
+ * siblings and only one is busy. In such case, @dst_cpu
+ * can help if it has higher priority and is idle (i.e.,
+ * it has no running tasks).
+ */
+ return !sds->local_stat.sum_nr_running &&
+ sched_asym_prefer(dst_cpu, sg->asym_prefer_cpu);
+ }
+
+ /* @dst_cpu has SMT siblings. */
+
+ if (sg_is_smt) {
+ int local_busy_cpus = sds->local->group_weight -
+ sds->local_stat.idle_cpus;
+ int busy_cpus_delta = sg_busy_cpus - local_busy_cpus;
+
+ if (busy_cpus_delta == 1)
+ return sched_asym_prefer(dst_cpu,
+ sg->asym_prefer_cpu);
+
+ return false;
+ }
+
+ /*
+ * @sg does not have SMT siblings. Ensure that @sds::local does not end
+ * up with more than one busy SMT sibling and only pull tasks if there
+ * are not busy CPUs (i.e., no CPU has running tasks).
+ */
+ if (!sds->local_stat.sum_nr_running)
+ return sched_asym_prefer(dst_cpu, sg->asym_prefer_cpu);
+
+ return false;
+#else
+ /* Always return false so that callers deal with non-SMT cases. */
+ return false;
+#endif
+}
+
static inline bool
sched_asym(struct lb_env *env, struct sd_lb_stats *sds, struct sg_lb_stats *sgs,
struct sched_group *group)
{
+ /* Only do SMT checks if either local or candidate have SMT siblings */
+ if ((sds->local->flags & SD_SHARE_CPUCAPACITY) ||
+ (group->flags & SD_SHARE_CPUCAPACITY))
+ return asym_smt_can_pull_tasks(env->dst_cpu, sds, sgs, group);
+
return sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu);
}
@@ -9606,6 +9694,12 @@ static struct rq *find_busiest_queue(struct lb_env *env,
nr_running == 1)
continue;
+ /* Make sure we only pull tasks from a CPU of lower priority */
+ if ((env->sd->flags & SD_ASYM_PACKING) &&
+ sched_asym_prefer(i, env->dst_cpu) &&
+ nr_running == 1)
+ continue;
+
switch (env->migration_type) {
case migrate_load:
/*
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next v2] bpf: Change value of MAX_TAIL_CALL_CNT from 32 to 33
From: Tiezhu Yang @ 2021-09-11 1:56 UTC (permalink / raw)
To: Shubham Bansal, Russell King, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Zi Shen Lim, Catalin Marinas,
Will Deacon, Paul Burton, Thomas Bogendoerfer, naveen.n.rao,
Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Luke Nelson, Xi Wang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
bjorn, davem, Johan Almbladh, Paul Chaignon
Cc: netdev, linux-kernel, linux-mips, sparclinux, bpf, linuxppc-dev,
linux-riscv, linux-arm-kernel
In the current code, the actual max tail call count is 33 which is greater
than MAX_TAIL_CALL_CNT (defined as 32), the actual limit is not consistent
with the meaning of MAX_TAIL_CALL_CNT, there is some confusion and need to
spend some time to think about the reason at the first glance.
We can see the historical evolution from commit 04fd61ab36ec ("bpf: allow
bpf programs to tail-call other bpf programs") and commit f9dabe016b63
("bpf: Undo off-by-one in interpreter tail call count limit").
In order to avoid changing existing behavior, the actual limit is 33 now,
this is reasonable.
After commit 874be05f525e ("bpf, tests: Add tail call test suite"), we can
see there exists failed testcase.
On all archs when CONFIG_BPF_JIT_ALWAYS_ON is not set:
# echo 0 > /proc/sys/net/core/bpf_jit_enable
# modprobe test_bpf
# dmesg | grep -w FAIL
Tail call error path, max count reached jited:0 ret 34 != 33 FAIL
On some archs:
# echo 1 > /proc/sys/net/core/bpf_jit_enable
# modprobe test_bpf
# dmesg | grep -w FAIL
Tail call error path, max count reached jited:1 ret 34 != 33 FAIL
So it is necessary to change the value of MAX_TAIL_CALL_CNT from 32 to 33,
then do some small changes of the related code.
With this patch, it does not change the current limit 33, MAX_TAIL_CALL_CNT
can reflect the actual max tail call count, the tailcall selftests can work
well, and also the above failed testcase in test_bpf can be fixed for the
interpreter (all archs) and the JIT (all archs except for x86).
# uname -m
x86_64
# echo 1 > /proc/sys/net/core/bpf_jit_enable
# modprobe test_bpf
# dmesg | grep -w FAIL
Tail call error path, max count reached jited:1 ret 33 != 34 FAIL
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
v2:
-- fix the typos in the commit message and update the commit message.
-- fix the failed tailcall selftests for x86 jit.
I am not quite sure the change on x86 is proper, with this change,
tailcall selftests passed, but tailcall limit test in test_bpf.ko
failed, I do not know the reason now, I think this is another issue,
maybe someone more versed in x86 jit could take a look.
arch/arm/net/bpf_jit_32.c | 11 ++++++-----
arch/arm64/net/bpf_jit_comp.c | 7 ++++---
arch/mips/net/ebpf_jit.c | 4 ++--
arch/powerpc/net/bpf_jit_comp32.c | 4 ++--
arch/powerpc/net/bpf_jit_comp64.c | 12 ++++++------
arch/riscv/net/bpf_jit_comp32.c | 4 ++--
arch/riscv/net/bpf_jit_comp64.c | 4 ++--
arch/sparc/net/bpf_jit_comp_64.c | 8 ++++----
arch/x86/net/bpf_jit_comp.c | 10 +++++-----
include/linux/bpf.h | 2 +-
kernel/bpf/core.c | 4 ++--
11 files changed, 36 insertions(+), 34 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index a951276..39d9ae9 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -1180,18 +1180,19 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
/* tmp2[0] = array, tmp2[1] = index */
- /* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
- * goto out;
+ /*
* tail_call_cnt++;
+ * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+ * goto out;
*/
+ tc = arm_bpf_get_reg64(tcc, tmp, ctx);
+ emit(ARM_ADDS_I(tc[1], tc[1], 1), ctx);
+ emit(ARM_ADC_I(tc[0], tc[0], 0), ctx);
lo = (u32)MAX_TAIL_CALL_CNT;
hi = (u32)((u64)MAX_TAIL_CALL_CNT >> 32);
- tc = arm_bpf_get_reg64(tcc, tmp, ctx);
emit(ARM_CMP_I(tc[0], hi), ctx);
_emit(ARM_COND_EQ, ARM_CMP_I(tc[1], lo), ctx);
_emit(ARM_COND_HI, ARM_B(jmp_offset), ctx);
- emit(ARM_ADDS_I(tc[1], tc[1], 1), ctx);
- emit(ARM_ADC_I(tc[0], tc[0], 0), ctx);
arm_bpf_put_reg64(tcc, tmp, ctx);
/* prog = array->ptrs[index]
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 41c23f4..5d6c843 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -286,14 +286,15 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
emit(A64_CMP(0, r3, tmp), ctx);
emit(A64_B_(A64_COND_CS, jmp_offset), ctx);
- /* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
- * goto out;
+ /*
* tail_call_cnt++;
+ * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+ * goto out;
*/
+ emit(A64_ADD_I(1, tcc, tcc, 1), ctx);
emit_a64_mov_i64(tmp, MAX_TAIL_CALL_CNT, ctx);
emit(A64_CMP(1, tcc, tmp), ctx);
emit(A64_B_(A64_COND_HI, jmp_offset), ctx);
- emit(A64_ADD_I(1, tcc, tcc, 1), ctx);
/* prog = array->ptrs[index];
* if (prog == NULL)
diff --git a/arch/mips/net/ebpf_jit.c b/arch/mips/net/ebpf_jit.c
index 3a73e93..029fc34 100644
--- a/arch/mips/net/ebpf_jit.c
+++ b/arch/mips/net/ebpf_jit.c
@@ -617,14 +617,14 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx, int this_idx)
b_off = b_imm(this_idx + 1, ctx);
emit_instr(ctx, bne, MIPS_R_AT, MIPS_R_ZERO, b_off);
/*
- * if (TCC-- < 0)
+ * if (--TCC < 0)
* goto out;
*/
/* Delay slot */
tcc_reg = (ctx->flags & EBPF_TCC_IN_V1) ? MIPS_R_V1 : MIPS_R_S4;
emit_instr(ctx, daddiu, MIPS_R_T5, tcc_reg, -1);
b_off = b_imm(this_idx + 1, ctx);
- emit_instr(ctx, bltz, tcc_reg, b_off);
+ emit_instr(ctx, bltz, MIPS_R_T5, b_off);
/*
* prog = array->ptrs[index];
* if (prog == NULL)
diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
index beb12cb..b5585ad 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -221,12 +221,12 @@ static void bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32
PPC_BCC(COND_GE, out);
/*
+ * tail_call_cnt++;
* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
* goto out;
*/
- EMIT(PPC_RAW_CMPLWI(_R0, MAX_TAIL_CALL_CNT));
- /* tail_call_cnt++; */
EMIT(PPC_RAW_ADDIC(_R0, _R0, 1));
+ EMIT(PPC_RAW_CMPLWI(_R0, MAX_TAIL_CALL_CNT));
PPC_BCC(COND_GT, out);
/* prog = array->ptrs[index]; */
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index b87a63d..bb15cc4 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -227,6 +227,12 @@ static void bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32
PPC_BCC(COND_GE, out);
/*
+ * tail_call_cnt++;
+ */
+ EMIT(PPC_RAW_ADDI(b2p[TMP_REG_1], b2p[TMP_REG_1], 1));
+ PPC_BPF_STL(b2p[TMP_REG_1], 1, bpf_jit_stack_tailcallcnt(ctx));
+
+ /*
* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
* goto out;
*/
@@ -234,12 +240,6 @@ static void bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32
EMIT(PPC_RAW_CMPLWI(b2p[TMP_REG_1], MAX_TAIL_CALL_CNT));
PPC_BCC(COND_GT, out);
- /*
- * tail_call_cnt++;
- */
- EMIT(PPC_RAW_ADDI(b2p[TMP_REG_1], b2p[TMP_REG_1], 1));
- PPC_BPF_STL(b2p[TMP_REG_1], 1, bpf_jit_stack_tailcallcnt(ctx));
-
/* prog = array->ptrs[index]; */
EMIT(PPC_RAW_MULI(b2p[TMP_REG_1], b2p_index, 8));
EMIT(PPC_RAW_ADD(b2p[TMP_REG_1], b2p[TMP_REG_1], b2p_bpf_array));
diff --git a/arch/riscv/net/bpf_jit_comp32.c b/arch/riscv/net/bpf_jit_comp32.c
index e649742..1608d94 100644
--- a/arch/riscv/net/bpf_jit_comp32.c
+++ b/arch/riscv/net/bpf_jit_comp32.c
@@ -800,12 +800,12 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
/*
* temp_tcc = tcc - 1;
- * if (tcc < 0)
+ * if (temp_tcc < 0)
* goto out;
*/
emit(rv_addi(RV_REG_T1, RV_REG_TCC, -1), ctx);
off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
- emit_bcc(BPF_JSLT, RV_REG_TCC, RV_REG_ZERO, off, ctx);
+ emit_bcc(BPF_JSLT, RV_REG_T1, RV_REG_ZERO, off, ctx);
/*
* prog = array->ptrs[index];
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 3af4131..6e9ba83 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -311,12 +311,12 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
emit_branch(BPF_JGE, RV_REG_A2, RV_REG_T1, off, ctx);
- /* if (TCC-- < 0)
+ /* if (--TCC < 0)
* goto out;
*/
emit_addi(RV_REG_T1, tcc, -1, ctx);
off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
- emit_branch(BPF_JSLT, tcc, RV_REG_ZERO, off, ctx);
+ emit_branch(BPF_JSLT, RV_REG_T1, RV_REG_ZERO, off, ctx);
/* prog = array->ptrs[index];
* if (!prog)
diff --git a/arch/sparc/net/bpf_jit_comp_64.c b/arch/sparc/net/bpf_jit_comp_64.c
index 9a2f20c..50d914c 100644
--- a/arch/sparc/net/bpf_jit_comp_64.c
+++ b/arch/sparc/net/bpf_jit_comp_64.c
@@ -863,6 +863,10 @@ static void emit_tail_call(struct jit_ctx *ctx)
emit_branch(BGEU, ctx->idx, ctx->idx + OFFSET1, ctx);
emit_nop(ctx);
+ emit_alu_K(ADD, tmp, 1, ctx);
+ off = BPF_TAILCALL_CNT_SP_OFF;
+ emit(ST32 | IMMED | RS1(SP) | S13(off) | RD(tmp), ctx);
+
off = BPF_TAILCALL_CNT_SP_OFF;
emit(LD32 | IMMED | RS1(SP) | S13(off) | RD(tmp), ctx);
emit_cmpi(tmp, MAX_TAIL_CALL_CNT, ctx);
@@ -870,10 +874,6 @@ static void emit_tail_call(struct jit_ctx *ctx)
emit_branch(BGU, ctx->idx, ctx->idx + OFFSET2, ctx);
emit_nop(ctx);
- emit_alu_K(ADD, tmp, 1, ctx);
- off = BPF_TAILCALL_CNT_SP_OFF;
- emit(ST32 | IMMED | RS1(SP) | S13(off) | RD(tmp), ctx);
-
emit_alu3_K(SLL, bpf_index, 3, tmp, ctx);
emit_alu(ADD, bpf_array, tmp, ctx);
off = offsetof(struct bpf_array, ptrs);
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 0fe6aac..74a9e61 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -402,7 +402,7 @@ static int get_pop_bytes(bool *callee_regs_used)
* ... bpf_tail_call(void *ctx, struct bpf_array *array, u64 index) ...
* if (index >= array->map.max_entries)
* goto out;
- * if (++tail_call_cnt > MAX_TAIL_CALL_CNT)
+ * if (tail_call_cnt++ == MAX_TAIL_CALL_CNT)
* goto out;
* prog = array->ptrs[index];
* if (prog == NULL)
@@ -452,13 +452,13 @@ static void emit_bpf_tail_call_indirect(u8 **pprog, bool *callee_regs_used,
EMIT2(X86_JBE, OFFSET1); /* jbe out */
/*
- * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+ * if (tail_call_cnt++ == MAX_TAIL_CALL_CNT)
* goto out;
*/
EMIT2_off32(0x8B, 0x85, tcc_off); /* mov eax, dword ptr [rbp - tcc_off] */
EMIT3(0x83, 0xF8, MAX_TAIL_CALL_CNT); /* cmp eax, MAX_TAIL_CALL_CNT */
#define OFFSET2 (off2 + RETPOLINE_RCX_BPF_JIT_SIZE)
- EMIT2(X86_JA, OFFSET2); /* ja out */
+ EMIT2(X86_JE, OFFSET2); /* je out */
EMIT3(0x83, 0xC0, 0x01); /* add eax, 1 */
EMIT2_off32(0x89, 0x85, tcc_off); /* mov dword ptr [rbp - tcc_off], eax */
@@ -530,12 +530,12 @@ static void emit_bpf_tail_call_direct(struct bpf_jit_poke_descriptor *poke,
}
/*
- * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+ * if (tail_call_cnt++ == MAX_TAIL_CALL_CNT)
* goto out;
*/
EMIT2_off32(0x8B, 0x85, tcc_off); /* mov eax, dword ptr [rbp - tcc_off] */
EMIT3(0x83, 0xF8, MAX_TAIL_CALL_CNT); /* cmp eax, MAX_TAIL_CALL_CNT */
- EMIT2(X86_JA, off1); /* ja out */
+ EMIT2(X86_JE, off1); /* je out */
EMIT3(0x83, 0xC0, 0x01); /* add eax, 1 */
EMIT2_off32(0x89, 0x85, tcc_off); /* mov dword ptr [rbp - tcc_off], eax */
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index f4c16f1..224cc7e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1046,7 +1046,7 @@ struct bpf_array {
};
#define BPF_COMPLEXITY_LIMIT_INSNS 1000000 /* yes. 1M insns */
-#define MAX_TAIL_CALL_CNT 32
+#define MAX_TAIL_CALL_CNT 33
#define BPF_F_ACCESS_MASK (BPF_F_RDONLY | \
BPF_F_RDONLY_PROG | \
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 9f4636d..8edb1c3 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1564,10 +1564,10 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
if (unlikely(index >= array->map.max_entries))
goto out;
- if (unlikely(tail_call_cnt > MAX_TAIL_CALL_CNT))
- goto out;
tail_call_cnt++;
+ if (unlikely(tail_call_cnt > MAX_TAIL_CALL_CNT))
+ goto out;
prog = READ_ONCE(array->ptrs[index]);
if (!prog)
--
2.1.0
^ permalink raw reply related
* [PATCH v6 1/4] powerpc/64s: Introduce temporary mm for Radix MMU
From: Christopher M. Riedl @ 2021-09-11 2:29 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-hardening
In-Reply-To: <20210911022904.30962-1-cmr@bluescreens.de>
x86 supports the notion of a temporary mm which restricts access to
temporary PTEs to a single CPU. A temporary mm is useful for situations
where a CPU needs to perform sensitive operations (such as patching a
STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing
said mappings to other CPUs. Another benefit is that other CPU TLBs do
not need to be flushed when the temporary mm is torn down.
Mappings in the temporary mm can be set in the userspace portion of the
address-space.
Interrupts must be disabled while the temporary mm is in use. HW
breakpoints, which may have been set by userspace as watchpoints on
addresses now within the temporary mm, are saved and disabled when
loading the temporary mm. The HW breakpoints are restored when unloading
the temporary mm. All HW breakpoints are indiscriminately disabled while
the temporary mm is in use - this may include breakpoints set by perf.
Based on x86 implementation:
commit cefa929c034e
("x86/mm: Introduce temporary mm structs")
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
v6: * Use {start,stop}_using_temporary_mm() instead of
{use,unuse}_temporary_mm() as suggested by Christophe.
v5: * Drop support for using a temporary mm on Book3s64 Hash MMU.
v4: * Pass the prev mm instead of NULL to switch_mm_irqs_off() when
using/unusing the temp mm as suggested by Jann Horn to keep
the context.active counter in-sync on mm/nohash.
* Disable SLB preload in the temporary mm when initializing the
temp_mm struct.
* Include asm/debug.h header to fix build issue with
ppc44x_defconfig.
---
arch/powerpc/include/asm/debug.h | 1 +
arch/powerpc/kernel/process.c | 5 +++
arch/powerpc/lib/code-patching.c | 56 ++++++++++++++++++++++++++++++++
3 files changed, 62 insertions(+)
diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
index 86a14736c76c..dfd82635ea8b 100644
--- a/arch/powerpc/include/asm/debug.h
+++ b/arch/powerpc/include/asm/debug.h
@@ -46,6 +46,7 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
#endif
void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk);
+void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk);
bool ppc_breakpoint_available(void);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
extern void do_send_trap(struct pt_regs *regs, unsigned long address,
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 50436b52c213..6aa1f5c4d520 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -865,6 +865,11 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk)
return 0;
}
+void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk)
+{
+ memcpy(brk, this_cpu_ptr(¤t_brk[nr]), sizeof(*brk));
+}
+
void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
{
memcpy(this_cpu_ptr(¤t_brk[nr]), brk, sizeof(*brk));
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index f9a3019e37b4..8d61a7d35b89 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -17,6 +17,9 @@
#include <asm/code-patching.h>
#include <asm/setup.h>
#include <asm/inst.h>
+#include <asm/mmu_context.h>
+#include <asm/debug.h>
+#include <asm/tlb.h>
static int __patch_instruction(u32 *exec_addr, struct ppc_inst instr, u32 *patch_addr)
{
@@ -45,6 +48,59 @@ int raw_patch_instruction(u32 *addr, struct ppc_inst instr)
}
#ifdef CONFIG_STRICT_KERNEL_RWX
+
+struct temp_mm {
+ struct mm_struct *temp;
+ struct mm_struct *prev;
+ struct arch_hw_breakpoint brk[HBP_NUM_MAX];
+};
+
+static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm)
+{
+ /* We currently only support temporary mm on the Book3s64 Radix MMU */
+ WARN_ON(!radix_enabled());
+
+ temp_mm->temp = mm;
+ temp_mm->prev = NULL;
+ memset(&temp_mm->brk, 0, sizeof(temp_mm->brk));
+}
+
+static inline void start_using_temporary_mm(struct temp_mm *temp_mm)
+{
+ lockdep_assert_irqs_disabled();
+
+ temp_mm->prev = current->active_mm;
+ switch_mm_irqs_off(temp_mm->prev, temp_mm->temp, current);
+
+ WARN_ON(!mm_is_thread_local(temp_mm->temp));
+
+ if (ppc_breakpoint_available()) {
+ struct arch_hw_breakpoint null_brk = {0};
+ int i = 0;
+
+ for (; i < nr_wp_slots(); ++i) {
+ __get_breakpoint(i, &temp_mm->brk[i]);
+ if (temp_mm->brk[i].type != 0)
+ __set_breakpoint(i, &null_brk);
+ }
+ }
+}
+
+static inline void stop_using_temporary_mm(struct temp_mm *temp_mm)
+{
+ lockdep_assert_irqs_disabled();
+
+ switch_mm_irqs_off(temp_mm->temp, temp_mm->prev, current);
+
+ if (ppc_breakpoint_available()) {
+ int i = 0;
+
+ for (; i < nr_wp_slots(); ++i)
+ if (temp_mm->brk[i].type != 0)
+ __set_breakpoint(i, &temp_mm->brk[i]);
+ }
+}
+
static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
static int text_area_cpu_up(unsigned int cpu)
--
2.32.0
^ permalink raw reply related
* [PATCH v6 0/4] Use per-CPU temporary mappings for patching on Radix MMU
From: Christopher M. Riedl @ 2021-09-11 2:29 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-hardening
When compiled with CONFIG_STRICT_KERNEL_RWX, the kernel must create
temporary mappings when patching itself. These mappings temporarily
override the strict RWX text protections to permit a write. Currently,
powerpc allocates a per-CPU VM area for patching. Patching occurs as
follows:
1. Map page in per-CPU VM area w/ PAGE_KERNEL protection
2. Patch text
3. Remove the temporary mapping
While the VM area is per-CPU, the mapping is actually inserted into the
kernel page tables. Presumably, this could allow another CPU to access
the normally write-protected text - either malicously or accidentally -
via this same mapping if the address of the VM area is known. Ideally,
the mapping should be kept local to the CPU doing the patching [0].
x86 introduced "temporary mm" structs which allow the creation of mappings
local to a particular CPU [1]. This series intends to bring the notion of a
temporary mm to powerpc's Book3s64 Radix MMU and harden it by using such a
mapping for patching a kernel with strict RWX permissions.
Tested boot and ftrace:
- QEMU+KVM (host: POWER9 Blackbird): Radix MMU w/ KUAP
- QEMU+KVM (host: POWER9 Blackbird): Hash MMU
Tested boot:
- QEMU+TCG: ppc44x (bamboo)
- QEMU+TCG: g5 (mac99)
I also tested with various extra config options enabled as suggested in
section 12) in Documentation/process/submit-checklist.rst.
v6: * Split series to separate powerpc percpu temporary mm
implementation and LKDTM test (still working on this one) and
implement some of Christophe Leroy's feedback.
* Rebase on linuxppc/next: powerpc-5.15-1
v5: * Only support Book3s64 Radix MMU for now. There are some issues with
the previous implementation on the Hash MMU as pointed out by Nick
Piggin. Fixing these is not trivial so we only support the Radix MMU
for now.
v4: * It's time to revisit this series again since @jpn and @mpe fixed
our known STRICT_*_RWX bugs on powerpc/64s.
* Rebase on linuxppc/next:
commit ee1bc694fbaec ("powerpc/kvm: Fix build error when PPC_MEM_KEYS/PPC_PSERIES=n")
* Completely rework how map_patch() works on book3s64 Hash MMU
* Split the LKDTM x86_64 and powerpc bits into separate patches
* Annotate commit messages with changes from v3 instead of
listing them here completely out-of context...
v3: * Rebase on linuxppc/next: commit 9123e3a74ec7 ("Linux 5.9-rc1")
* Move temporary mm implementation into code-patching.c where it
belongs
* Implement LKDTM hijacker test on x86_64 (on IBM time oof) Do
* not use address zero for the patching address in the
temporary mm (thanks @dja for pointing this out!)
* Wrap the LKDTM test w/ CONFIG_SMP as suggested by Christophe
Leroy
* Comments to clarify PTE pre-allocation and patching addr
selection
v2: * Rebase on linuxppc/next:
commit 105fb38124a4 ("powerpc/8xx: Modify ptep_get()")
* Always dirty pte when mapping patch
* Use `ppc_inst_len` instead of `sizeof` on instructions
* Declare LKDTM patching addr accessor in header where it belongs
v1: * Rebase on linuxppc/next (4336b9337824)
* Save and restore second hw watchpoint
* Use new ppc_inst_* functions for patching check and in LKDTM test
rfc-v2: * Many fixes and improvements mostly based on extensive feedback
and testing by Christophe Leroy (thanks!).
* Make patching_mm and patching_addr static and move
'__ro_after_init' to after the variable name (more common in
other parts of the kernel)
* Use 'asm/debug.h' header instead of 'asm/hw_breakpoint.h' to
fix PPC64e compile
* Add comment explaining why we use BUG_ON() during the init
call to setup for patching later
* Move ptep into patch_mapping to avoid walking page tables a
second time when unmapping the temporary mapping
* Use KUAP under non-radix, also manually dirty the PTE for patch
mapping on non-BOOK3S_64 platforms
* Properly return any error from __patch_instruction
* Do not use 'memcmp' where a simple comparison is appropriate
* Simplify expression for patch address by removing pointer maths
* Add LKDTM test
[0]: https://github.com/linuxppc/issues/issues/224
[1]: https://lore.kernel.org/kernel-hardening/20190426232303.28381-1-nadav.amit@gmail.com/
Christopher M. Riedl (4):
powerpc/64s: Introduce temporary mm for Radix MMU
powerpc: Rework and improve STRICT_KERNEL_RWX patching
powerpc: Use WARN_ON and fix check in poking_init
powerpc/64s: Initialize and use a temporary mm for patching on Radix
arch/powerpc/include/asm/debug.h | 1 +
arch/powerpc/kernel/process.c | 5 +
arch/powerpc/lib/code-patching.c | 215 ++++++++++++++++++++++++++-----
3 files changed, 191 insertions(+), 30 deletions(-)
--
2.32.0
^ permalink raw reply
* [PATCH v6 4/4] powerpc/64s: Initialize and use a temporary mm for patching on Radix
From: Christopher M. Riedl @ 2021-09-11 2:29 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-hardening
In-Reply-To: <20210911022904.30962-1-cmr@bluescreens.de>
When code patching a STRICT_KERNEL_RWX kernel the page containing the
address to be patched is temporarily mapped as writeable. Currently, a
per-cpu vmalloc patch area is used for this purpose. While the patch
area is per-cpu, the temporary page mapping is inserted into the kernel
page tables for the duration of patching. The mapping is exposed to CPUs
other than the patching CPU - this is undesirable from a hardening
perspective. Use a temporary mm instead which keeps the mapping local to
the CPU doing the patching.
Use the `poking_init` init hook to prepare a temporary mm and patching
address. Initialize the temporary mm by copying the init mm. Choose a
randomized patching address inside the temporary mm userspace address
space. The patching address is randomized between PAGE_SIZE and
DEFAULT_MAP_WINDOW-PAGE_SIZE.
Bits of entropy with 64K page size on BOOK3S_64:
bits of entropy = log2(DEFAULT_MAP_WINDOW_USER64 / PAGE_SIZE)
PAGE_SIZE=64K, DEFAULT_MAP_WINDOW_USER64=128TB
bits of entropy = log2(128TB / 64K)
bits of entropy = 31
The upper limit is DEFAULT_MAP_WINDOW due to how the Book3s64 Hash MMU
operates - by default the space above DEFAULT_MAP_WINDOW is not
available. Currently the Hash MMU does not use a temporary mm so
technically this upper limit isn't necessary; however, a larger
randomization range does not further "harden" this overall approach and
future work may introduce patching with a temporary mm on Hash as well.
Randomization occurs only once during initialization at boot for each
possible CPU in the system.
Introduce two new functions, map_patch_mm() and unmap_patch_mm(), to
respectively create and remove the temporary mapping with write
permissions at patching_addr. Map the page with PAGE_KERNEL to set
EAA[0] for the PTE which ignores the AMR (so no need to unlock/lock
KUAP) according to PowerISA v3.0b Figure 35 on Radix.
Based on x86 implementation:
commit 4fc19708b165
("x86/alternatives: Initialize temporary mm for patching")
and:
commit b3fd8e83ada0
("x86/alternatives: Use temporary mm for text poking")
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
v6: * Small clean-ups (naming, formatting, style, etc).
* Call stop_using_temporary_mm() before pte_unmap_unlock() after
patching.
* Replace BUG_ON()s in poking_init() w/ WARN_ON()s.
v5: * Only support Book3s64 Radix MMU for now.
* Use a per-cpu datastructure to hold the patching_addr and
patching_mm to avoid the need for a synchronization lock/mutex.
v4: * In the previous series this was two separate patches: one to init
the temporary mm in poking_init() (unused in powerpc at the time)
and the other to use it for patching (which removed all the
per-cpu vmalloc code). Now that we use poking_init() in the
existing per-cpu vmalloc approach, that separation doesn't work
as nicely anymore so I just merged the two patches into one.
* Preload the SLB entry and hash the page for the patching_addr
when using Hash on book3s64 to avoid taking an SLB and Hash fault
during patching. The previous implementation was a hack which
changed current->mm to allow the SLB and Hash fault handlers to
work with the temporary mm since both of those code-paths always
assume mm == current->mm.
* Also (hmm - seeing a trend here) with the book3s64 Hash MMU we
have to manage the mm->context.active_cpus counter and mm cpumask
since they determine (via mm_is_thread_local()) if the TLB flush
in pte_clear() is local or not - it should always be local when
we're using the temporary mm. On book3s64's Radix MMU we can
just call local_flush_tlb_mm().
* Use HPTE_USE_KERNEL_KEY on Hash to avoid costly lock/unlock of
KUAP.
---
arch/powerpc/lib/code-patching.c | 119 +++++++++++++++++++++++++++++--
1 file changed, 112 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index e802e42c2789..af8e2a02a9dd 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -11,6 +11,7 @@
#include <linux/cpuhotplug.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <linux/random.h>
#include <asm/tlbflush.h>
#include <asm/page.h>
@@ -103,6 +104,7 @@ static inline void stop_using_temporary_mm(struct temp_mm *temp_mm)
static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
static DEFINE_PER_CPU(unsigned long, cpu_patching_addr);
+static DEFINE_PER_CPU(struct mm_struct *, cpu_patching_mm);
static int text_area_cpu_up(unsigned int cpu)
{
@@ -126,8 +128,48 @@ static int text_area_cpu_down(unsigned int cpu)
return 0;
}
+static __always_inline void __poking_init_temp_mm(void)
+{
+ int cpu;
+ spinlock_t *ptl; /* for protecting pte table */
+ pte_t *ptep;
+ struct mm_struct *patching_mm;
+ unsigned long patching_addr;
+
+ for_each_possible_cpu(cpu) {
+ patching_mm = copy_init_mm();
+ WARN_ON(!patching_mm);
+ per_cpu(cpu_patching_mm, cpu) = patching_mm;
+
+ /*
+ * Choose a randomized, page-aligned address from the range:
+ * [PAGE_SIZE, DEFAULT_MAP_WINDOW - PAGE_SIZE] The lower
+ * address bound is PAGE_SIZE to avoid the zero-page. The
+ * upper address bound is DEFAULT_MAP_WINDOW - PAGE_SIZE to
+ * stay under DEFAULT_MAP_WINDOW with the Book3s64 Hash MMU.
+ */
+ patching_addr = PAGE_SIZE + ((get_random_long() & PAGE_MASK)
+ % (DEFAULT_MAP_WINDOW - 2 * PAGE_SIZE));
+ per_cpu(cpu_patching_addr, cpu) = patching_addr;
+
+ /*
+ * PTE allocation uses GFP_KERNEL which means we need to
+ * pre-allocate the PTE here because we cannot do the
+ * allocation during patching when IRQs are disabled.
+ */
+ ptep = get_locked_pte(patching_mm, patching_addr, &ptl);
+ WARN_ON(!ptep);
+ pte_unmap_unlock(ptep, ptl);
+ }
+}
+
void __init poking_init(void)
{
+ if (radix_enabled()) {
+ __poking_init_temp_mm();
+ return;
+ }
+
WARN_ON(cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
"powerpc/text_poke:online", text_area_cpu_up,
text_area_cpu_down) < 0);
@@ -197,30 +239,93 @@ static inline int unmap_patch_area(void)
return 0;
}
+struct patch_mapping {
+ spinlock_t *ptl; /* for protecting pte table */
+ pte_t *ptep;
+ struct temp_mm temp_mm;
+};
+
+/*
+ * This can be called for kernel text or a module.
+ */
+static int map_patch_mm(const void *addr, struct patch_mapping *patch_mapping)
+{
+ struct page *page;
+ struct mm_struct *patching_mm = __this_cpu_read(cpu_patching_mm);
+ unsigned long patching_addr = __this_cpu_read(cpu_patching_addr);
+
+ if (is_vmalloc_or_module_addr(addr))
+ page = vmalloc_to_page(addr);
+ else
+ page = virt_to_page(addr);
+
+ patch_mapping->ptep = get_locked_pte(patching_mm, patching_addr,
+ &patch_mapping->ptl);
+ if (unlikely(!patch_mapping->ptep)) {
+ pr_warn("map patch: failed to allocate pte for patching\n");
+ return -1;
+ }
+
+ set_pte_at(patching_mm, patching_addr, patch_mapping->ptep,
+ pte_mkdirty(mk_pte(page, PAGE_KERNEL)));
+
+ init_temp_mm(&patch_mapping->temp_mm, patching_mm);
+ start_using_temporary_mm(&patch_mapping->temp_mm);
+
+ return 0;
+}
+
+static int unmap_patch_mm(struct patch_mapping *patch_mapping)
+{
+ struct mm_struct *patching_mm = __this_cpu_read(cpu_patching_mm);
+ unsigned long patching_addr = __this_cpu_read(cpu_patching_addr);
+
+ pte_clear(patching_mm, patching_addr, patch_mapping->ptep);
+
+ local_flush_tlb_mm(patching_mm);
+ stop_using_temporary_mm(&patch_mapping->temp_mm);
+
+ pte_unmap_unlock(patch_mapping->ptep, patch_mapping->ptl);
+
+ return 0;
+}
+
static int do_patch_instruction(u32 *addr, struct ppc_inst instr)
{
int err, rc = 0;
u32 *patch_addr = NULL;
unsigned long flags;
+ struct patch_mapping patch_mapping;
/*
- * During early early boot patch_instruction is called
- * when text_poke_area is not ready, but we still need
- * to allow patching. We just do the plain old patching
+ * During early early boot patch_instruction is called when the
+ * patching_mm/text_poke_area is not ready, but we still need to allow
+ * patching. We just do the plain old patching.
*/
- if (!this_cpu_read(text_poke_area))
- return raw_patch_instruction(addr, instr);
+ if (radix_enabled()) {
+ if (!this_cpu_read(cpu_patching_mm))
+ return raw_patch_instruction(addr, instr);
+ } else {
+ if (!this_cpu_read(text_poke_area))
+ return raw_patch_instruction(addr, instr);
+ }
local_irq_save(flags);
- err = map_patch_area(addr);
+ if (radix_enabled())
+ err = map_patch_mm(addr, &patch_mapping);
+ else
+ err = map_patch_area(addr);
if (err)
goto out;
patch_addr = (u32 *)(__this_cpu_read(cpu_patching_addr) | offset_in_page(addr));
rc = __patch_instruction(addr, instr, patch_addr);
- err = unmap_patch_area();
+ if (radix_enabled())
+ err = unmap_patch_mm(&patch_mapping);
+ else
+ err = unmap_patch_area();
out:
local_irq_restore(flags);
--
2.32.0
^ permalink raw reply related
* [PATCH v6 3/4] powerpc: Use WARN_ON and fix check in poking_init
From: Christopher M. Riedl @ 2021-09-11 2:29 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-hardening
In-Reply-To: <20210911022904.30962-1-cmr@bluescreens.de>
The latest kernel docs list BUG_ON() as 'deprecated' and that they
should be replaced with WARN_ON() (or pr_warn()) when possible. The
BUG_ON() in poking_init() warrants a WARN_ON() rather than a pr_warn()
since the error condition is deemed "unreachable".
Also take this opportunity to fix the failure check in the WARN_ON():
cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, ...) returns a positive integer
on success and a negative integer on failure.
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
v6: * New to series - based on Christophe's relentless feedback in the
crusade against BUG_ON()s :)
---
arch/powerpc/lib/code-patching.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 8d0bb86125d5..e802e42c2789 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -126,16 +126,11 @@ static int text_area_cpu_down(unsigned int cpu)
return 0;
}
-/*
- * Although BUG_ON() is rude, in this case it should only happen if ENOMEM, and
- * we judge it as being preferable to a kernel that will crash later when
- * someone tries to use patch_instruction().
- */
void __init poking_init(void)
{
- BUG_ON(!cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+ WARN_ON(cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
"powerpc/text_poke:online", text_area_cpu_up,
- text_area_cpu_down));
+ text_area_cpu_down) < 0);
}
/*
--
2.32.0
^ permalink raw reply related
* [PATCH v6 2/4] powerpc: Rework and improve STRICT_KERNEL_RWX patching
From: Christopher M. Riedl @ 2021-09-11 2:29 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-hardening
In-Reply-To: <20210911022904.30962-1-cmr@bluescreens.de>
Rework code-patching with STRICT_KERNEL_RWX to prepare for a later patch
which uses a temporary mm for patching under the Book3s64 Radix MMU.
Make improvements by adding a WARN_ON when the patchsite doesn't match
after patching and return the error from __patch_instruction() properly.
Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
---
v6: * Remove the pr_warn() message from unmap_patch_area().
v5: * New to series.
---
arch/powerpc/lib/code-patching.c | 35 ++++++++++++++++----------------
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 8d61a7d35b89..8d0bb86125d5 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -102,6 +102,7 @@ static inline void stop_using_temporary_mm(struct temp_mm *temp_mm)
}
static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
+static DEFINE_PER_CPU(unsigned long, cpu_patching_addr);
static int text_area_cpu_up(unsigned int cpu)
{
@@ -114,6 +115,7 @@ static int text_area_cpu_up(unsigned int cpu)
return -1;
}
this_cpu_write(text_poke_area, area);
+ this_cpu_write(cpu_patching_addr, (unsigned long)area->addr);
return 0;
}
@@ -139,7 +141,7 @@ void __init poking_init(void)
/*
* This can be called for kernel text or a module.
*/
-static int map_patch_area(void *addr, unsigned long text_poke_addr)
+static int map_patch_area(void *addr)
{
unsigned long pfn;
int err;
@@ -149,17 +151,20 @@ static int map_patch_area(void *addr, unsigned long text_poke_addr)
else
pfn = __pa_symbol(addr) >> PAGE_SHIFT;
- err = map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);
+ err = map_kernel_page(__this_cpu_read(cpu_patching_addr),
+ (pfn << PAGE_SHIFT), PAGE_KERNEL);
- pr_devel("Mapped addr %lx with pfn %lx:%d\n", text_poke_addr, pfn, err);
+ pr_devel("Mapped addr %lx with pfn %lx:%d\n",
+ __this_cpu_read(cpu_patching_addr), pfn, err);
if (err)
return -1;
return 0;
}
-static inline int unmap_patch_area(unsigned long addr)
+static inline int unmap_patch_area(void)
{
+ unsigned long addr = __this_cpu_read(cpu_patching_addr);
pte_t *ptep;
pmd_t *pmdp;
pud_t *pudp;
@@ -199,11 +204,9 @@ static inline int unmap_patch_area(unsigned long addr)
static int do_patch_instruction(u32 *addr, struct ppc_inst instr)
{
- int err;
+ int err, rc = 0;
u32 *patch_addr = NULL;
unsigned long flags;
- unsigned long text_poke_addr;
- unsigned long kaddr = (unsigned long)addr;
/*
* During early early boot patch_instruction is called
@@ -215,24 +218,20 @@ static int do_patch_instruction(u32 *addr, struct ppc_inst instr)
local_irq_save(flags);
- text_poke_addr = (unsigned long)__this_cpu_read(text_poke_area)->addr;
- if (map_patch_area(addr, text_poke_addr)) {
- err = -1;
+ err = map_patch_area(addr);
+ if (err)
goto out;
- }
- patch_addr = (u32 *)(text_poke_addr + (kaddr & ~PAGE_MASK));
+ patch_addr = (u32 *)(__this_cpu_read(cpu_patching_addr) | offset_in_page(addr));
+ rc = __patch_instruction(addr, instr, patch_addr);
- __patch_instruction(addr, instr, patch_addr);
-
- err = unmap_patch_area(text_poke_addr);
- if (err)
- pr_warn("failed to unmap %lx\n", text_poke_addr);
+ err = unmap_patch_area();
out:
local_irq_restore(flags);
+ WARN_ON(!ppc_inst_equal(ppc_inst_read(addr), instr));
- return err;
+ return rc ? rc : err;
}
#else /* !CONFIG_STRICT_KERNEL_RWX */
--
2.32.0
^ permalink raw reply related
* Re: [PATCH v6 1/4] powerpc/64s: Introduce temporary mm for Radix MMU
From: Jordan Niethe @ 2021-09-11 8:26 UTC (permalink / raw)
To: Christopher M. Riedl; +Cc: linuxppc-dev, linux-hardening
In-Reply-To: <20210911022904.30962-2-cmr@bluescreens.de>
On Sat, Sep 11, 2021 at 12:35 PM Christopher M. Riedl
<cmr@bluescreens.de> wrote:
>
> x86 supports the notion of a temporary mm which restricts access to
> temporary PTEs to a single CPU. A temporary mm is useful for situations
> where a CPU needs to perform sensitive operations (such as patching a
> STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing
> said mappings to other CPUs. Another benefit is that other CPU TLBs do
> not need to be flushed when the temporary mm is torn down.
>
> Mappings in the temporary mm can be set in the userspace portion of the
> address-space.
>
> Interrupts must be disabled while the temporary mm is in use. HW
> breakpoints, which may have been set by userspace as watchpoints on
> addresses now within the temporary mm, are saved and disabled when
> loading the temporary mm. The HW breakpoints are restored when unloading
> the temporary mm. All HW breakpoints are indiscriminately disabled while
> the temporary mm is in use - this may include breakpoints set by perf.
I had thought CPUs with a DAWR might not need to do this because the
privilege level that breakpoints trigger on can be configured. But it
turns out in ptrace, etc we use HW_BRK_TYPE_PRIV_ALL.
>
> Based on x86 implementation:
>
> commit cefa929c034e
> ("x86/mm: Introduce temporary mm structs")
>
> Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
>
> ---
>
> v6: * Use {start,stop}_using_temporary_mm() instead of
> {use,unuse}_temporary_mm() as suggested by Christophe.
>
> v5: * Drop support for using a temporary mm on Book3s64 Hash MMU.
>
> v4: * Pass the prev mm instead of NULL to switch_mm_irqs_off() when
> using/unusing the temp mm as suggested by Jann Horn to keep
> the context.active counter in-sync on mm/nohash.
> * Disable SLB preload in the temporary mm when initializing the
> temp_mm struct.
> * Include asm/debug.h header to fix build issue with
> ppc44x_defconfig.
> ---
> arch/powerpc/include/asm/debug.h | 1 +
> arch/powerpc/kernel/process.c | 5 +++
> arch/powerpc/lib/code-patching.c | 56 ++++++++++++++++++++++++++++++++
> 3 files changed, 62 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
> index 86a14736c76c..dfd82635ea8b 100644
> --- a/arch/powerpc/include/asm/debug.h
> +++ b/arch/powerpc/include/asm/debug.h
> @@ -46,6 +46,7 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
> #endif
>
> void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk);
> +void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk);
> bool ppc_breakpoint_available(void);
> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> extern void do_send_trap(struct pt_regs *regs, unsigned long address,
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 50436b52c213..6aa1f5c4d520 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -865,6 +865,11 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk)
> return 0;
> }
>
> +void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk)
> +{
> + memcpy(brk, this_cpu_ptr(¤t_brk[nr]), sizeof(*brk));
> +}
The breakpoint code is already a little hard to follow. I'm worried
doing this might spread breakpoint handling into more places in the
future.
What about something like having a breakpoint_pause() function which
clears the hardware registers only and then a breakpoint_resume()
function that copies from current_brk[] back to the hardware
registers?
Then we don't have to make another copy of the breakpoint state.
> +
> void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
> {
> memcpy(this_cpu_ptr(¤t_brk[nr]), brk, sizeof(*brk));
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index f9a3019e37b4..8d61a7d35b89 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
Sorry I might have missed it, but what was the reason for not putting
this stuff in mmu_context.h?
> @@ -17,6 +17,9 @@
> #include <asm/code-patching.h>
> #include <asm/setup.h>
> #include <asm/inst.h>
> +#include <asm/mmu_context.h>
> +#include <asm/debug.h>
> +#include <asm/tlb.h>
>
> static int __patch_instruction(u32 *exec_addr, struct ppc_inst instr, u32 *patch_addr)
> {
> @@ -45,6 +48,59 @@ int raw_patch_instruction(u32 *addr, struct ppc_inst instr)
> }
>
> #ifdef CONFIG_STRICT_KERNEL_RWX
> +
> +struct temp_mm {
> + struct mm_struct *temp;
> + struct mm_struct *prev;
> + struct arch_hw_breakpoint brk[HBP_NUM_MAX];
^ Then we wouldn't need this.
> +};
> +
> +static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm)
> +{
> + /* We currently only support temporary mm on the Book3s64 Radix MMU */
> + WARN_ON(!radix_enabled());
> +
> + temp_mm->temp = mm;
> + temp_mm->prev = NULL;
> + memset(&temp_mm->brk, 0, sizeof(temp_mm->brk));
> +}
> +
> +static inline void start_using_temporary_mm(struct temp_mm *temp_mm)
> +{
> + lockdep_assert_irqs_disabled();
> +
> + temp_mm->prev = current->active_mm;
> + switch_mm_irqs_off(temp_mm->prev, temp_mm->temp, current);
Now that we only support radix it should be fine again to have it like this:
switch_mm_irqs_off(NULL, temp_mm->temp, current);?
It was changed from that because it would cause issues on nohash I thought.
> +
> + WARN_ON(!mm_is_thread_local(temp_mm->temp));
> +
> + if (ppc_breakpoint_available()) {
> + struct arch_hw_breakpoint null_brk = {0};
> + int i = 0;
> +
> + for (; i < nr_wp_slots(); ++i) {
> + __get_breakpoint(i, &temp_mm->brk[i]);
> + if (temp_mm->brk[i].type != 0)
> + __set_breakpoint(i, &null_brk);
> + }
> + }
> +}
> +
> +static inline void stop_using_temporary_mm(struct temp_mm *temp_mm)
> +{
> + lockdep_assert_irqs_disabled();
> +
> + switch_mm_irqs_off(temp_mm->temp, temp_mm->prev, current);
> +
> + if (ppc_breakpoint_available()) {
> + int i = 0;
> +
> + for (; i < nr_wp_slots(); ++i)
> + if (temp_mm->brk[i].type != 0)
> + __set_breakpoint(i, &temp_mm->brk[i]);
> + }
> +}
> +
> static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
>
> static int text_area_cpu_up(unsigned int cpu)
> --
> 2.32.0
>
Thanks,
Jordan
^ permalink raw reply
* Re: [PATCH v6 4/4] powerpc/64s: Initialize and use a temporary mm for patching on Radix
From: Jordan Niethe @ 2021-09-11 9:14 UTC (permalink / raw)
To: Christopher M. Riedl; +Cc: linuxppc-dev, linux-hardening
In-Reply-To: <20210911022904.30962-5-cmr@bluescreens.de>
On Sat, Sep 11, 2021 at 12:39 PM Christopher M. Riedl
<cmr@bluescreens.de> wrote:
>
> When code patching a STRICT_KERNEL_RWX kernel the page containing the
> address to be patched is temporarily mapped as writeable. Currently, a
> per-cpu vmalloc patch area is used for this purpose. While the patch
> area is per-cpu, the temporary page mapping is inserted into the kernel
> page tables for the duration of patching. The mapping is exposed to CPUs
> other than the patching CPU - this is undesirable from a hardening
> perspective. Use a temporary mm instead which keeps the mapping local to
> the CPU doing the patching.
>
> Use the `poking_init` init hook to prepare a temporary mm and patching
> address. Initialize the temporary mm by copying the init mm. Choose a
> randomized patching address inside the temporary mm userspace address
> space. The patching address is randomized between PAGE_SIZE and
> DEFAULT_MAP_WINDOW-PAGE_SIZE.
>
> Bits of entropy with 64K page size on BOOK3S_64:
>
> bits of entropy = log2(DEFAULT_MAP_WINDOW_USER64 / PAGE_SIZE)
>
> PAGE_SIZE=64K, DEFAULT_MAP_WINDOW_USER64=128TB
> bits of entropy = log2(128TB / 64K)
> bits of entropy = 31
>
> The upper limit is DEFAULT_MAP_WINDOW due to how the Book3s64 Hash MMU
> operates - by default the space above DEFAULT_MAP_WINDOW is not
> available. Currently the Hash MMU does not use a temporary mm so
> technically this upper limit isn't necessary; however, a larger
> randomization range does not further "harden" this overall approach and
> future work may introduce patching with a temporary mm on Hash as well.
>
> Randomization occurs only once during initialization at boot for each
> possible CPU in the system.
>
> Introduce two new functions, map_patch_mm() and unmap_patch_mm(), to
> respectively create and remove the temporary mapping with write
> permissions at patching_addr. Map the page with PAGE_KERNEL to set
> EAA[0] for the PTE which ignores the AMR (so no need to unlock/lock
> KUAP) according to PowerISA v3.0b Figure 35 on Radix.
>
> Based on x86 implementation:
>
> commit 4fc19708b165
> ("x86/alternatives: Initialize temporary mm for patching")
>
> and:
>
> commit b3fd8e83ada0
> ("x86/alternatives: Use temporary mm for text poking")
>
> Signed-off-by: Christopher M. Riedl <cmr@bluescreens.de>
>
> ---
>
> v6: * Small clean-ups (naming, formatting, style, etc).
> * Call stop_using_temporary_mm() before pte_unmap_unlock() after
> patching.
> * Replace BUG_ON()s in poking_init() w/ WARN_ON()s.
>
> v5: * Only support Book3s64 Radix MMU for now.
> * Use a per-cpu datastructure to hold the patching_addr and
> patching_mm to avoid the need for a synchronization lock/mutex.
>
> v4: * In the previous series this was two separate patches: one to init
> the temporary mm in poking_init() (unused in powerpc at the time)
> and the other to use it for patching (which removed all the
> per-cpu vmalloc code). Now that we use poking_init() in the
> existing per-cpu vmalloc approach, that separation doesn't work
> as nicely anymore so I just merged the two patches into one.
> * Preload the SLB entry and hash the page for the patching_addr
> when using Hash on book3s64 to avoid taking an SLB and Hash fault
> during patching. The previous implementation was a hack which
> changed current->mm to allow the SLB and Hash fault handlers to
> work with the temporary mm since both of those code-paths always
> assume mm == current->mm.
> * Also (hmm - seeing a trend here) with the book3s64 Hash MMU we
> have to manage the mm->context.active_cpus counter and mm cpumask
> since they determine (via mm_is_thread_local()) if the TLB flush
> in pte_clear() is local or not - it should always be local when
> we're using the temporary mm. On book3s64's Radix MMU we can
> just call local_flush_tlb_mm().
> * Use HPTE_USE_KERNEL_KEY on Hash to avoid costly lock/unlock of
> KUAP.
> ---
> arch/powerpc/lib/code-patching.c | 119 +++++++++++++++++++++++++++++--
> 1 file changed, 112 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index e802e42c2789..af8e2a02a9dd 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -11,6 +11,7 @@
> #include <linux/cpuhotplug.h>
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> +#include <linux/random.h>
>
> #include <asm/tlbflush.h>
> #include <asm/page.h>
> @@ -103,6 +104,7 @@ static inline void stop_using_temporary_mm(struct temp_mm *temp_mm)
>
> static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
> static DEFINE_PER_CPU(unsigned long, cpu_patching_addr);
> +static DEFINE_PER_CPU(struct mm_struct *, cpu_patching_mm);
>
> static int text_area_cpu_up(unsigned int cpu)
> {
> @@ -126,8 +128,48 @@ static int text_area_cpu_down(unsigned int cpu)
> return 0;
> }
>
> +static __always_inline void __poking_init_temp_mm(void)
> +{
> + int cpu;
> + spinlock_t *ptl; /* for protecting pte table */
ptl is just used so we don't have to open code allocating a pte in
patching_mm isn't it?
> + pte_t *ptep;
> + struct mm_struct *patching_mm;
> + unsigned long patching_addr;
> +
> + for_each_possible_cpu(cpu) {
> + patching_mm = copy_init_mm();
> + WARN_ON(!patching_mm);
Would it be okay to just let the mmu handle null pointer dereferences?
> + per_cpu(cpu_patching_mm, cpu) = patching_mm;
> +
> + /*
> + * Choose a randomized, page-aligned address from the range:
> + * [PAGE_SIZE, DEFAULT_MAP_WINDOW - PAGE_SIZE] The lower
> + * address bound is PAGE_SIZE to avoid the zero-page. The
> + * upper address bound is DEFAULT_MAP_WINDOW - PAGE_SIZE to
> + * stay under DEFAULT_MAP_WINDOW with the Book3s64 Hash MMU.
> + */
> + patching_addr = PAGE_SIZE + ((get_random_long() & PAGE_MASK)
> + % (DEFAULT_MAP_WINDOW - 2 * PAGE_SIZE));
> + per_cpu(cpu_patching_addr, cpu) = patching_addr;
On x86 the randomization depends on CONFIG_RANDOMIZE_BASE. Should it
be controllable here too?
> +
> + /*
> + * PTE allocation uses GFP_KERNEL which means we need to
> + * pre-allocate the PTE here because we cannot do the
> + * allocation during patching when IRQs are disabled.
> + */
> + ptep = get_locked_pte(patching_mm, patching_addr, &ptl);
> + WARN_ON(!ptep);
> + pte_unmap_unlock(ptep, ptl);
> + }
> +}
> +
> void __init poking_init(void)
> {
> + if (radix_enabled()) {
> + __poking_init_temp_mm();
Should this also be done with cpuhp_setup_state()?
> + return;
> + }
> +
> WARN_ON(cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> "powerpc/text_poke:online", text_area_cpu_up,
> text_area_cpu_down) < 0);
> @@ -197,30 +239,93 @@ static inline int unmap_patch_area(void)
> return 0;
> }
>
> +struct patch_mapping {
> + spinlock_t *ptl; /* for protecting pte table */
> + pte_t *ptep;
> + struct temp_mm temp_mm;
> +};
> +
> +/*
> + * This can be called for kernel text or a module.
> + */
> +static int map_patch_mm(const void *addr, struct patch_mapping *patch_mapping)
> +{
> + struct page *page;
> + struct mm_struct *patching_mm = __this_cpu_read(cpu_patching_mm);
> + unsigned long patching_addr = __this_cpu_read(cpu_patching_addr);
> +
> + if (is_vmalloc_or_module_addr(addr))
> + page = vmalloc_to_page(addr);
> + else
> + page = virt_to_page(addr);
> +
> + patch_mapping->ptep = get_locked_pte(patching_mm, patching_addr,
> + &patch_mapping->ptl);
> + if (unlikely(!patch_mapping->ptep)) {
> + pr_warn("map patch: failed to allocate pte for patching\n");
> + return -1;
> + }
> +
> + set_pte_at(patching_mm, patching_addr, patch_mapping->ptep,
> + pte_mkdirty(mk_pte(page, PAGE_KERNEL)));
> +
> + init_temp_mm(&patch_mapping->temp_mm, patching_mm);
> + start_using_temporary_mm(&patch_mapping->temp_mm);
> +
> + return 0;
> +}
> +
> +static int unmap_patch_mm(struct patch_mapping *patch_mapping)
> +{
> + struct mm_struct *patching_mm = __this_cpu_read(cpu_patching_mm);
> + unsigned long patching_addr = __this_cpu_read(cpu_patching_addr);
> +
> + pte_clear(patching_mm, patching_addr, patch_mapping->ptep);
> +
> + local_flush_tlb_mm(patching_mm);
> + stop_using_temporary_mm(&patch_mapping->temp_mm);
> +
> + pte_unmap_unlock(patch_mapping->ptep, patch_mapping->ptl);
> +
> + return 0;
> +}
> +
> static int do_patch_instruction(u32 *addr, struct ppc_inst instr)
> {
> int err, rc = 0;
> u32 *patch_addr = NULL;
> unsigned long flags;
> + struct patch_mapping patch_mapping;
>
> /*
> - * During early early boot patch_instruction is called
> - * when text_poke_area is not ready, but we still need
> - * to allow patching. We just do the plain old patching
> + * During early early boot patch_instruction is called when the
> + * patching_mm/text_poke_area is not ready, but we still need to allow
> + * patching. We just do the plain old patching.
> */
> - if (!this_cpu_read(text_poke_area))
> - return raw_patch_instruction(addr, instr);
> + if (radix_enabled()) {
> + if (!this_cpu_read(cpu_patching_mm))
> + return raw_patch_instruction(addr, instr);
> + } else {
> + if (!this_cpu_read(text_poke_area))
> + return raw_patch_instruction(addr, instr);
> + }
Would testing cpu_patching_addr handler both of these cases?
Then I think it might be clearer to do something like this:
if (radix_enabled()) {
return patch_instruction_mm(addr, instr);
}
patch_instruction_mm() would combine map_patch_mm(), then patching and
unmap_patch_mm() into one function.
IMO, a bit of code duplication would be cleaner than checking multiple
times for radix_enabled() and having struct patch_mapping especially
for maintaining state.
>
> local_irq_save(flags);
>
> - err = map_patch_area(addr);
> + if (radix_enabled())
> + err = map_patch_mm(addr, &patch_mapping);
> + else
> + err = map_patch_area(addr);
> if (err)
> goto out;
>
> patch_addr = (u32 *)(__this_cpu_read(cpu_patching_addr) | offset_in_page(addr));
> rc = __patch_instruction(addr, instr, patch_addr);
>
> - err = unmap_patch_area();
> + if (radix_enabled())
> + err = unmap_patch_mm(&patch_mapping);
> + else
> + err = unmap_patch_area();
>
> out:
> local_irq_restore(flags);
> --
> 2.32.0
>
Thanks,
Jordan
^ permalink raw reply
* Re: [PATCH v3 3/8] x86/sev: Add an x86 version of cc_platform_has()
From: Borislav Petkov @ 2021-09-11 10:10 UTC (permalink / raw)
To: Tom Lendacky
Cc: Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh, kvm,
Peter Zijlstra, Dave Hansen, dri-devel, platform-driver-x86,
linux-s390, Andi Kleen, Joerg Roedel, x86, amd-gfx,
Christoph Hellwig, Ingo Molnar, linux-graphics-maintainer,
Tianyu Lan, Andy Lutomirski, Thomas Gleixner, kexec, linux-kernel,
iommu, linux-fsdevel, linuxppc-dev
In-Reply-To: <f9951644147e27772bf4512325e8ba6472e363b7.1631141919.git.thomas.lendacky@amd.com>
On Wed, Sep 08, 2021 at 05:58:34PM -0500, Tom Lendacky wrote:
> diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c
> new file mode 100644
> index 000000000000..3c9bacd3c3f3
> --- /dev/null
> +++ b/arch/x86/kernel/cc_platform.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Confidential Computing Platform Capability checks
> + *
> + * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + *
> + * Author: Tom Lendacky <thomas.lendacky@amd.com>
> + */
> +
> +#include <linux/export.h>
> +#include <linux/cc_platform.h>
> +#include <linux/mem_encrypt.h>
> +
> +bool cc_platform_has(enum cc_attr attr)
> +{
> + if (sme_me_mask)
Why are you still checking the sme_me_mask here? AFAIR, we said that
we'll do that only when the KVM folks come with a valid use case...
> + return amd_cc_platform_has(attr);
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(cc_platform_has);
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index ff08dc463634..18fe19916bc3 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -20,6 +20,7 @@
> #include <linux/bitops.h>
> #include <linux/dma-mapping.h>
> #include <linux/virtio_config.h>
> +#include <linux/cc_platform.h>
>
> #include <asm/tlbflush.h>
> #include <asm/fixmap.h>
> @@ -389,6 +390,26 @@ bool noinstr sev_es_active(void)
> return sev_status & MSR_AMD64_SEV_ES_ENABLED;
> }
>
> +bool amd_cc_platform_has(enum cc_attr attr)
> +{
> + switch (attr) {
> + case CC_ATTR_MEM_ENCRYPT:
> + return sme_me_mask != 0;
No need for the "!= 0"
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH 06/10] powerpc: remove GCC version check for UPD_CONSTR
From: Michael Ellerman @ 2021-09-11 10:43 UTC (permalink / raw)
To: Nathan Chancellor, Nick Desaulniers, Andrew Morton
Cc: Stephen Rothwell, linuxppc-dev, Arnd Bergmann, Masahiro Yamada,
llvm, Rasmus Villemoes, linux-kernel, Paul Mackerras, Joe Perches,
Linus Torvalds
In-Reply-To: <b940bd2c-21d2-dfe9-e171-e265085a2b11@kernel.org>
Nathan Chancellor <nathan@kernel.org> writes:
> On 9/10/2021 4:40 PM, Nick Desaulniers wrote:
>> Now that GCC 5.1 is the minimum supported version, we can drop this
>> workaround for older versions of GCC. This adversely affected clang,
>> too.
>>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Segher Boessenkool <segher@kernel.crashing.org>
>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>> ---
>> arch/powerpc/include/asm/asm-const.h | 10 ----------
>> 1 file changed, 10 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/asm-const.h b/arch/powerpc/include/asm/asm-const.h
>> index 0ce2368bd20f..dbfa5e1e3198 100644
>> --- a/arch/powerpc/include/asm/asm-const.h
>> +++ b/arch/powerpc/include/asm/asm-const.h
>> @@ -12,16 +12,6 @@
>> # define ASM_CONST(x) __ASM_CONST(x)
>> #endif
>>
>> -/*
>> - * Inline assembly memory constraint
>> - *
>> - * GCC 4.9 doesn't properly handle pre update memory constraint "m<>"
>> - *
>> - */
>> -#if defined(GCC_VERSION) && GCC_VERSION < 50000
>> -#define UPD_CONSTR ""
>> -#else
>> #define UPD_CONSTR "<>"
>> -#endif
>
> The only reason this exists is because of commit 592bbe9c505d
> ("powerpc/uaccess: Don't use "m<>" constraint with GCC 4.9"). It is
> probably just worth sinking "<>" into all of the callsites and removing
> UPD_CONSTR.
Yeah that would be great if you're doing a v2. Or we can do it as a
follow-up.
cheers
^ permalink raw reply
* Re: [PATCH 1/1] powerpc: Drop superfluous pci_dev_is_added() calls
From: Michael Ellerman @ 2021-09-11 11:09 UTC (permalink / raw)
To: Niklas Schnelle, Bjorn Helgaas
Cc: linux-arch, linux-s390, linux-kernel, Oliver O'Halloran,
linux-pci, linuxppc-dev
In-Reply-To: <20210910141940.2598035-2-schnelle@linux.ibm.com>
Niklas Schnelle <schnelle@linux.ibm.com> writes:
> On powerpc, pci_dev_is_added() is called as part of SR-IOV fixups
> that are done under pcibios_add_device() which in turn is only called in
> pci_device_add() whih is called when a PCI device is scanned.
Thanks for cleaning this up for us.
> Now pci_dev_assign_added() is called in pci_bus_add_device() which is
> only called after scanning the device. Thus pci_dev_is_added() is always
> false and can be dropped.
My only query is whether we can pin down when that changed.
Oliver said:
The use of pci_dev_is_added() in arch/powerpc was because in the past
pci_bus_add_device() could be called before pci_device_add(). That was
fixed a while ago so It should be safe to remove those calls now.
I trawled back through the history a bit but I can't remember/find which
commit changed that, Oliver can you remember?
cheers
> diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c b/arch/powerpc/platforms/powernv/pci-sriov.c
> index 28aac933a439..deddbb233fde 100644
> --- a/arch/powerpc/platforms/powernv/pci-sriov.c
> +++ b/arch/powerpc/platforms/powernv/pci-sriov.c
> @@ -9,9 +9,6 @@
>
> #include "pci.h"
>
> -/* for pci_dev_is_added() */
> -#include "../../../../drivers/pci/pci.h"
> -
> /*
> * The majority of the complexity in supporting SR-IOV on PowerNV comes from
> * the need to put the MMIO space for each VF into a separate PE. Internally
> @@ -228,9 +225,6 @@ static void pnv_pci_ioda_fixup_iov_resources(struct pci_dev *pdev)
>
> void pnv_pci_ioda_fixup_iov(struct pci_dev *pdev)
> {
> - if (WARN_ON(pci_dev_is_added(pdev)))
> - return;
> -
> if (pdev->is_virtfn) {
> struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
>
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index f79126f16258..2188054470c1 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -74,7 +74,6 @@
> #include <asm/hvconsole.h>
>
> #include "pseries.h"
> -#include "../../../../drivers/pci/pci.h"
>
> DEFINE_STATIC_KEY_FALSE(shared_processor);
> EXPORT_SYMBOL(shared_processor);
> @@ -750,7 +749,7 @@ static void pseries_pci_fixup_iov_resources(struct pci_dev *pdev)
> const int *indexes;
> struct device_node *dn = pci_device_to_OF_node(pdev);
>
> - if (!pdev->is_physfn || pci_dev_is_added(pdev))
> + if (!pdev->is_physfn)
> return;
> /*Firmware must support open sriov otherwise dont configure*/
> indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH 1/1] selftests/powerpc: Add memmove_64 test
From: Michael Ellerman @ 2021-09-11 11:26 UTC (permalink / raw)
To: Ritesh Harjani, linuxppc-dev
Cc: Aneesh Kumar K . V, Ritesh Harjani, Vaibhav Jain
In-Reply-To: <c152ad80dc3a80cd362f6cbbccd626798ab0d5db.1629300331.git.riteshh@linux.ibm.com>
Ritesh Harjani <riteshh@linux.ibm.com> writes:
> While debugging an issue, we wanted to check whether the arch specific
> kernel memmove implementation is correct. This selftest could help test that.
>
> Suggested-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> ---
> tools/testing/selftests/powerpc/Makefile | 1 +
> .../selftests/powerpc/memmoveloop/.gitignore | 2 +
> .../selftests/powerpc/memmoveloop/Makefile | 19 +++++++
> .../powerpc/memmoveloop/asm/asm-compat.h | 0
> .../powerpc/memmoveloop/asm/export.h | 4 ++
> .../powerpc/memmoveloop/asm/feature-fixups.h | 0
> .../selftests/powerpc/memmoveloop/asm/kasan.h | 0
> .../powerpc/memmoveloop/asm/ppc_asm.h | 39 +++++++++++++
> .../powerpc/memmoveloop/asm/processor.h | 0
> .../selftests/powerpc/memmoveloop/mem_64.S | 1 +
> .../selftests/powerpc/memmoveloop/memcpy_64.S | 1 +
> .../selftests/powerpc/memmoveloop/stubs.S | 8 +++
> .../selftests/powerpc/memmoveloop/validate.c | 56 +++++++++++++++++++
> 13 files changed, 131 insertions(+)
> create mode 100644 tools/testing/selftests/powerpc/memmoveloop/.gitignore
> create mode 100644 tools/testing/selftests/powerpc/memmoveloop/Makefile
> create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/asm-compat.h
> create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/export.h
> create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/feature-fixups.h
> create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/kasan.h
> create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/ppc_asm.h
> create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/processor.h
> create mode 120000 tools/testing/selftests/powerpc/memmoveloop/mem_64.S
> create mode 120000 tools/testing/selftests/powerpc/memmoveloop/memcpy_64.S
> create mode 100644 tools/testing/selftests/powerpc/memmoveloop/stubs.S
> create mode 100644 tools/testing/selftests/powerpc/memmoveloop/validate.c
You've copied a lot of tools/testing/selftests/powerpc/copyloops
I'd be happier if you integrated the memmove test into that existing
code. I realise memmove is not technically a copy loop, but it's close
enough.
Did you try that and hit some roadblock?
cheers
> diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
> index 0830e63818c1..d110b3e5cbcd 100644
> --- a/tools/testing/selftests/powerpc/Makefile
> +++ b/tools/testing/selftests/powerpc/Makefile
> @@ -16,6 +16,7 @@ export CFLAGS
> SUB_DIRS = alignment \
> benchmarks \
> cache_shape \
> + memmoveloop \
> copyloops \
> dscr \
> mm \
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/.gitignore b/tools/testing/selftests/powerpc/memmoveloop/.gitignore
> new file mode 100644
> index 000000000000..56c1426013d5
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/memmoveloop/.gitignore
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +memmove_64
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/Makefile b/tools/testing/selftests/powerpc/memmoveloop/Makefile
> new file mode 100644
> index 000000000000..d58d8c100099
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/memmoveloop/Makefile
> @@ -0,0 +1,19 @@
> +# SPDX-License-Identifier: GPL-2.0
> +CFLAGS += -m64
> +CFLAGS += -I$(CURDIR)
> +CFLAGS += -D SELFTEST
> +CFLAGS += -maltivec
> +
> +ASFLAGS = $(CFLAGS) -Wa,-mpower4
> +
> +TEST_GEN_PROGS := memmove_64
> +EXTRA_SOURCES := validate.c ../harness.c stubs.S
> +CPPFLAGS += -D TEST_MEMMOVE=test_memmove
> +
> +top_srcdir = ../../../../..
> +include ../../lib.mk
> +
> +$(OUTPUT)/memmove_64: mem_64.S memcpy_64.S $(EXTRA_SOURCES)
> + $(CC) $(CPPFLAGS) $(CFLAGS) \
> + -D TEST_MEMMOVE=test_memmove \
> + -o $@ $^
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/asm-compat.h b/tools/testing/selftests/powerpc/memmoveloop/asm/asm-compat.h
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/export.h b/tools/testing/selftests/powerpc/memmoveloop/asm/export.h
> new file mode 100644
> index 000000000000..e6b80d5fbd14
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/memmoveloop/asm/export.h
> @@ -0,0 +1,4 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#define EXPORT_SYMBOL(x)
> +#define EXPORT_SYMBOL_GPL(x)
> +#define EXPORT_SYMBOL_KASAN(x)
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/feature-fixups.h b/tools/testing/selftests/powerpc/memmoveloop/asm/feature-fixups.h
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/kasan.h b/tools/testing/selftests/powerpc/memmoveloop/asm/kasan.h
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/ppc_asm.h b/tools/testing/selftests/powerpc/memmoveloop/asm/ppc_asm.h
> new file mode 100644
> index 000000000000..117005c56e19
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/memmoveloop/asm/ppc_asm.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __SELFTESTS_POWERPC_PPC_ASM_H
> +#define __SELFTESTS_POWERPC_PPC_ASM_H
> +#include <ppc-asm.h>
> +
> +#define CONFIG_ALTIVEC
> +
> +#define r1 1
> +
> +#define R14 r14
> +#define R15 r15
> +#define R16 r16
> +#define R17 r17
> +#define R18 r18
> +#define R19 r19
> +#define R20 r20
> +#define R21 r21
> +#define R22 r22
> +#define R29 r29
> +#define R30 r30
> +#define R31 r31
> +
> +#define STACKFRAMESIZE 256
> +#define STK_REG(i) (112 + ((i)-14)*8)
> +
> +#define _GLOBAL(A) FUNC_START(test_ ## A)
> +#define _GLOBAL_TOC(A) _GLOBAL(A)
> +#define _GLOBAL_TOC_KASAN(A) _GLOBAL(A)
> +#define _GLOBAL_KASAN(A) _GLOBAL(A)
> +
> +#define PPC_MTOCRF(A, B) mtocrf A, B
> +
> +#define BEGIN_FTR_SECTION
> +#define FTR_SECTION_ELSE
> +#define ALT_FTR_SECTION_END_IFCLR(x)
> +#define ALT_FTR_SECTION_END(x, y)
> +#define END_FTR_SECTION_IFCLR(x)
> +
> +#endif /* __SELFTESTS_POWERPC_PPC_ASM_H */
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/processor.h b/tools/testing/selftests/powerpc/memmoveloop/asm/processor.h
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/mem_64.S b/tools/testing/selftests/powerpc/memmoveloop/mem_64.S
> new file mode 120000
> index 000000000000..db254c9a5f5c
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/memmoveloop/mem_64.S
> @@ -0,0 +1 @@
> +../../../../../arch/powerpc/lib/mem_64.S
> \ No newline at end of file
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/memcpy_64.S b/tools/testing/selftests/powerpc/memmoveloop/memcpy_64.S
> new file mode 120000
> index 000000000000..cce33fb6f9d8
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/memmoveloop/memcpy_64.S
> @@ -0,0 +1 @@
> +../../../../../arch/powerpc/lib/memcpy_64.S
> \ No newline at end of file
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/stubs.S b/tools/testing/selftests/powerpc/memmoveloop/stubs.S
> new file mode 100644
> index 000000000000..d9baa832fa49
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/memmoveloop/stubs.S
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <asm/ppc_asm.h>
> +
> +FUNC_START(memcpy)
> + b test_memcpy
> +
> +FUNC_START(backwards_memcpy)
> + b test_backwards_memcpy
> diff --git a/tools/testing/selftests/powerpc/memmoveloop/validate.c b/tools/testing/selftests/powerpc/memmoveloop/validate.c
> new file mode 100644
> index 000000000000..52f7d32bb3fe
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/memmoveloop/validate.c
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <malloc.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <assert.h>
> +#include "utils.h"
> +
> +void *TEST_MEMMOVE(const void *s1, const void *s2, size_t n);
> +
> +#define BUF_LEN 65536
> +#define MAX_OFFSET 512
> +
> +size_t max(size_t a, size_t b)
> +{
> + if (a >= b) return a;
> + return b;
> +}
> +
> +static int testcase_run(void)
> +{
> + size_t i, src_off, dst_off, len;
> +
> + char *usermap = memalign(BUF_LEN, BUF_LEN);
> + char *kernelmap = memalign(BUF_LEN, BUF_LEN);
> +
> + assert(usermap != NULL);
> + assert(kernelmap != NULL);
> +
> + memset(usermap, 0, BUF_LEN);
> + memset(kernelmap, 0, BUF_LEN);
> +
> + for (i = 0; i < BUF_LEN; i++) {
> + usermap[i] = i & 0xff;
> + kernelmap[i] = i & 0xff;
> + }
> +
> + for (src_off = 0; src_off < MAX_OFFSET; src_off++) {
> + for (dst_off = 0; dst_off < MAX_OFFSET; dst_off++) {
> + for (len = 1; len < MAX_OFFSET - max(src_off, dst_off); len++) {
> +
> + memmove(usermap + dst_off, usermap + src_off, len);
> + TEST_MEMMOVE(kernelmap + dst_off, kernelmap + src_off, len);
> + if (memcmp(usermap, kernelmap, MAX_OFFSET) != 0) {
> + printf("memmove failed at %ld %ld %ld\n", src_off, dst_off, len);
> + abort();
> + }
> + }
> + }
> + }
> + return 0;
> +}
> +
> +int main(void)
> +{
> + return test_harness(testcase_run, "memmove");
> +}
> --
> 2.31.1
^ permalink raw reply
* Re: [PATCH AUTOSEL 5.14 38/99] KVM: PPC: Book3S HV: XICS: Fix mapping of passthrough interrupts
From: Sasha Levin @ 2021-09-11 14:35 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm-ppc, linuxppc-dev, linux-kernel, stable
In-Reply-To: <27739836-bad2-6b3f-7f40-e84663fbbf24@kaod.org>
On Fri, Sep 10, 2021 at 07:48:18AM +0200, Cédric Le Goater wrote:
>On 9/10/21 2:14 AM, Sasha Levin wrote:
>> From: Cédric Le Goater <clg@kaod.org>
>>
>> [ Upstream commit 1753081f2d445f9157550692fcc4221cd3ff0958 ]
>>
>> PCI MSIs now live in an MSI domain but the underlying calls, which
>> will EOI the interrupt in real mode, need an HW IRQ number mapped in
>> the XICS IRQ domain. Grab it there.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> Link: https://lore.kernel.org/r/20210701132750.1475580-31-clg@kaod.org
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>
>
>Why are we backporting this patch in stable trees ?
>
>It should be fine but to compile, we need a partial backport of commit
>51be9e51a800 ("KVM: PPC: Book3S HV: XIVE: Fix mapping of passthrough
>interrupts") which exports irq_get_default_host().
Or, I can drop it if it makes no sense?
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH 06/10] powerpc: remove GCC version check for UPD_CONSTR
From: Christophe Leroy @ 2021-09-11 15:34 UTC (permalink / raw)
To: Nick Desaulniers, Andrew Morton
Cc: Arnd Bergmann, linuxppc-dev, Stephen Rothwell, Masahiro Yamada,
llvm, Rasmus Villemoes, linux-kernel, Nathan Chancellor,
Paul Mackerras, Joe Perches, Linus Torvalds
In-Reply-To: <20210910234047.1019925-7-ndesaulniers@google.com>
Le 11/09/2021 à 01:40, Nick Desaulniers a écrit :
> Now that GCC 5.1 is the minimum supported version, we can drop this
> workaround for older versions of GCC. This adversely affected clang,
> too.
Why do you say that GCC 5.1 is the minimum supported ?
As far as I can see, the minimum supported is still 4.9, see
https://github.com/torvalds/linux/blob/master/Documentation/process/changes.rst
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Segher Boessenkool <segher@kernel.crashing.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> arch/powerpc/include/asm/asm-const.h | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/asm-const.h b/arch/powerpc/include/asm/asm-const.h
> index 0ce2368bd20f..dbfa5e1e3198 100644
> --- a/arch/powerpc/include/asm/asm-const.h
> +++ b/arch/powerpc/include/asm/asm-const.h
> @@ -12,16 +12,6 @@
> # define ASM_CONST(x) __ASM_CONST(x)
> #endif
>
> -/*
> - * Inline assembly memory constraint
> - *
> - * GCC 4.9 doesn't properly handle pre update memory constraint "m<>"
> - *
> - */
> -#if defined(GCC_VERSION) && GCC_VERSION < 50000
> -#define UPD_CONSTR ""
> -#else
> #define UPD_CONSTR "<>"
> -#endif
There is no point in keeping UPD_CONSTR if it becomes invariant. You
should just replace all instances of UPD_CONSTR with <> and drop
UPD_CONSTR completely.
Christophe
^ permalink raw reply
* Re: [PATCH v2 3/5] signal: Add unsafe_copy_siginfo_to_user()
From: Eric W. Biederman @ 2021-09-11 15:58 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <7caf5127-36fc-7c77-00f1-7be82d6f26e0@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> On 9/8/21 6:17 PM, Eric W. Biederman wrote:
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>
>>> Le 02/09/2021 à 20:43, Eric W. Biederman a écrit :
>>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>>>
>>>>> In the same spirit as commit fb05121fd6a2 ("signal: Add
>>>>> unsafe_get_compat_sigset()"), implement an 'unsafe' version of
>>>>> copy_siginfo_to_user() in order to use it within user access blocks.
>>>>>
>>>>> For that, also add an 'unsafe' version of clear_user().
>>>>
>>>> Looking at your use cases you need the 32bit compat version of this
>>>> as well.
>>>>
>>>> The 32bit compat version is too complicated to become a macro, so I
>>>> don't think you can make this work correctly for the 32bit compat case.
>>>
>>> When looking into patch 5/5 that you nacked, I think you missed the fact that we
>>> keep using copy_siginfo_to_user32() as it for the 32 bit compat case.
>>
>> I did. My mistake.
>>
>> However that mistake was so easy I think it mirrors the comments others
>> have made that this looks like a maintenance hazard.
>>
>> Is improving the performance of 32bit kernels interesting?
>
> Yes it is, and that's what this series do.
>
>> Is improving the performance of 32bit compat support interesting?
>
> For me this is a corner case, so I left it aside for now.
>
>>
>> If performance one or either of those cases is interesting it looks like
>> we already have copy_siginfo_to_external32 the factor you would need
>> to build unsafe_copy_siginfo_to_user32.
>
> I'm not sure I understand your saying here. What do you expect me to
> do with copy_siginfo_to_external32() ?
Implement unsafe_copy_siginfo_to_user32.
> copy_siginfo_to_user32() is for compat only.
>
> Native 32 bits powerpc use copy_siginfo_to_user()
What you implemented doubles the number of test cases necessary to
compile test the 32bit ppc signal code, and makes the code noticeably
harder to follow.
Having a unsafe_copy_to_siginfo_to_user32 at least would allow the
number of test cases to remain the same as the current code.
>> So I am not going to say impossible but please make something
>> maintainable. I unified all of the compat 32bit siginfo logic because
>> it simply did not get enough love and attention when it was implemented
>> per architecture.
>
> Yes, and ? I didn't do any modification to the compat case, so what
> you did remains.
You undid the unification between the 32bit code and the 32bit compat
code.
>> In general I think that concern applies to this case as well. We really
>> need an implementation that shares as much burden as possible with other
>> architectures.
>
> I think yes, that's the reason why I made a generic
> unsafe_copy_siginfo_to_user() and didn't make a powerpc dedicated
> change.
>
> Once this is merged any other architecture can use
> unsafe_copy_siginfo_to_user().
>
> Did I miss something ?
Not dealing with the compat case and making the code signal stack frame
code noticeably more complicated.
If this optimization profitably applies to other architectures we need
to figure out how to implement unsafe_copy_siginfo_to_user32 or risk
making them all much worse to maintain.
Eric
^ permalink raw reply
* Re: [PATCH 0/2] powerpc/perf: Add instruction and data address registers to extended regs
From: Arnaldo Carvalho de Melo @ 2021-09-11 19:09 UTC (permalink / raw)
To: Athira Rajeev
Cc: Madhavan Srinivasan, rnsastry, kajoljain, Jiri Olsa, linuxppc-dev
In-Reply-To: <BAA2B1EE-36BA-495A-B507-EC3C996199D8@linux.vnet.ibm.com>
Em Mon, Sep 06, 2021 at 08:13:13AM +0530, Athira Rajeev escreveu:
>
>
> > On 02-Sep-2021, at 1:04 PM, kajoljain <kjain@linux.ibm.com> wrote:
> >
> >
> >
> > On 6/20/21 8:15 PM, Athira Rajeev wrote:
> >> Patch set adds PMU registers namely Sampled Instruction Address Register
> >> (SIAR) and Sampled Data Address Register (SDAR) as part of extended regs
> >> in PowerPC. These registers provides the instruction/data address and
> >> adding these to extended regs helps in debug purposes.
> >>
> >> Patch 1/2 adds SIAR and SDAR as part of the extended regs mask.
> >> Patch 2/2 includes perf tools side changes to add the SPRs to
> >> sample_reg_mask to use with -I? option.
> >>
> >> Athira Rajeev (2):
> >> powerpc/perf: Expose instruction and data address registers as part of
> >> extended regs
> >> tools/perf: Add perf tools support to expose instruction and data
> >> address registers as part of extended regs
> >>
> >
> > Patchset looks good to me.
> >
> > Reviewed-By: kajol Jain<kjain@linux.ibm.com>
>
> Hi Arnaldo,
>
> Requesting for your review on this patchset.
So, this touches the kernel, usually I get a patchkit when the kernel
bits landed, is that the case by now?
- Arnaldo
>
> Thanks
> Athira
> >
> > Thanks,
> > Kajol Jain
> >
> >> arch/powerpc/include/uapi/asm/perf_regs.h | 12 +++++++-----
> >> arch/powerpc/perf/perf_regs.c | 4 ++++
> >> tools/arch/powerpc/include/uapi/asm/perf_regs.h | 12 +++++++-----
> >> tools/perf/arch/powerpc/include/perf_regs.h | 2 ++
> >> tools/perf/arch/powerpc/util/perf_regs.c | 2 ++
> >> 5 files changed, 22 insertions(+), 10 deletions(-)
--
- Arnaldo
^ permalink raw reply
* Re: [PATCH 1/1] selftests/powerpc: Add memmove_64 test
From: Ritesh Harjani @ 2021-09-13 2:57 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Aneesh Kumar K . V, linuxppc-dev, Vaibhav Jain
In-Reply-To: <87sfybl5f9.fsf@mpe.ellerman.id.au>
On 21/09/11 09:26PM, Michael Ellerman wrote:
> Ritesh Harjani <riteshh@linux.ibm.com> writes:
> > While debugging an issue, we wanted to check whether the arch specific
> > kernel memmove implementation is correct. This selftest could help test that.
> >
> > Suggested-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> > ---
> > tools/testing/selftests/powerpc/Makefile | 1 +
> > .../selftests/powerpc/memmoveloop/.gitignore | 2 +
> > .../selftests/powerpc/memmoveloop/Makefile | 19 +++++++
> > .../powerpc/memmoveloop/asm/asm-compat.h | 0
> > .../powerpc/memmoveloop/asm/export.h | 4 ++
> > .../powerpc/memmoveloop/asm/feature-fixups.h | 0
> > .../selftests/powerpc/memmoveloop/asm/kasan.h | 0
> > .../powerpc/memmoveloop/asm/ppc_asm.h | 39 +++++++++++++
> > .../powerpc/memmoveloop/asm/processor.h | 0
> > .../selftests/powerpc/memmoveloop/mem_64.S | 1 +
> > .../selftests/powerpc/memmoveloop/memcpy_64.S | 1 +
> > .../selftests/powerpc/memmoveloop/stubs.S | 8 +++
> > .../selftests/powerpc/memmoveloop/validate.c | 56 +++++++++++++++++++
> > 13 files changed, 131 insertions(+)
> > create mode 100644 tools/testing/selftests/powerpc/memmoveloop/.gitignore
> > create mode 100644 tools/testing/selftests/powerpc/memmoveloop/Makefile
> > create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/asm-compat.h
> > create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/export.h
> > create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/feature-fixups.h
> > create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/kasan.h
> > create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/ppc_asm.h
> > create mode 100644 tools/testing/selftests/powerpc/memmoveloop/asm/processor.h
> > create mode 120000 tools/testing/selftests/powerpc/memmoveloop/mem_64.S
> > create mode 120000 tools/testing/selftests/powerpc/memmoveloop/memcpy_64.S
> > create mode 100644 tools/testing/selftests/powerpc/memmoveloop/stubs.S
> > create mode 100644 tools/testing/selftests/powerpc/memmoveloop/validate.c
>
> You've copied a lot of tools/testing/selftests/powerpc/copyloops
>
> I'd be happier if you integrated the memmove test into that existing
> code. I realise memmove is not technically a copy loop, but it's close
> enough.
Sure.
>
> Did you try that and hit some roadblock?
Nope, let me try that and send it in v2.
-ritesh
>
> cheers
>
>
> > diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
> > index 0830e63818c1..d110b3e5cbcd 100644
> > --- a/tools/testing/selftests/powerpc/Makefile
> > +++ b/tools/testing/selftests/powerpc/Makefile
> > @@ -16,6 +16,7 @@ export CFLAGS
> > SUB_DIRS = alignment \
> > benchmarks \
> > cache_shape \
> > + memmoveloop \
> > copyloops \
> > dscr \
> > mm \
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/.gitignore b/tools/testing/selftests/powerpc/memmoveloop/.gitignore
> > new file mode 100644
> > index 000000000000..56c1426013d5
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/memmoveloop/.gitignore
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +memmove_64
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/Makefile b/tools/testing/selftests/powerpc/memmoveloop/Makefile
> > new file mode 100644
> > index 000000000000..d58d8c100099
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/memmoveloop/Makefile
> > @@ -0,0 +1,19 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +CFLAGS += -m64
> > +CFLAGS += -I$(CURDIR)
> > +CFLAGS += -D SELFTEST
> > +CFLAGS += -maltivec
> > +
> > +ASFLAGS = $(CFLAGS) -Wa,-mpower4
> > +
> > +TEST_GEN_PROGS := memmove_64
> > +EXTRA_SOURCES := validate.c ../harness.c stubs.S
> > +CPPFLAGS += -D TEST_MEMMOVE=test_memmove
> > +
> > +top_srcdir = ../../../../..
> > +include ../../lib.mk
> > +
> > +$(OUTPUT)/memmove_64: mem_64.S memcpy_64.S $(EXTRA_SOURCES)
> > + $(CC) $(CPPFLAGS) $(CFLAGS) \
> > + -D TEST_MEMMOVE=test_memmove \
> > + -o $@ $^
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/asm-compat.h b/tools/testing/selftests/powerpc/memmoveloop/asm/asm-compat.h
> > new file mode 100644
> > index 000000000000..e69de29bb2d1
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/export.h b/tools/testing/selftests/powerpc/memmoveloop/asm/export.h
> > new file mode 100644
> > index 000000000000..e6b80d5fbd14
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/memmoveloop/asm/export.h
> > @@ -0,0 +1,4 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#define EXPORT_SYMBOL(x)
> > +#define EXPORT_SYMBOL_GPL(x)
> > +#define EXPORT_SYMBOL_KASAN(x)
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/feature-fixups.h b/tools/testing/selftests/powerpc/memmoveloop/asm/feature-fixups.h
> > new file mode 100644
> > index 000000000000..e69de29bb2d1
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/kasan.h b/tools/testing/selftests/powerpc/memmoveloop/asm/kasan.h
> > new file mode 100644
> > index 000000000000..e69de29bb2d1
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/ppc_asm.h b/tools/testing/selftests/powerpc/memmoveloop/asm/ppc_asm.h
> > new file mode 100644
> > index 000000000000..117005c56e19
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/memmoveloop/asm/ppc_asm.h
> > @@ -0,0 +1,39 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __SELFTESTS_POWERPC_PPC_ASM_H
> > +#define __SELFTESTS_POWERPC_PPC_ASM_H
> > +#include <ppc-asm.h>
> > +
> > +#define CONFIG_ALTIVEC
> > +
> > +#define r1 1
> > +
> > +#define R14 r14
> > +#define R15 r15
> > +#define R16 r16
> > +#define R17 r17
> > +#define R18 r18
> > +#define R19 r19
> > +#define R20 r20
> > +#define R21 r21
> > +#define R22 r22
> > +#define R29 r29
> > +#define R30 r30
> > +#define R31 r31
> > +
> > +#define STACKFRAMESIZE 256
> > +#define STK_REG(i) (112 + ((i)-14)*8)
> > +
> > +#define _GLOBAL(A) FUNC_START(test_ ## A)
> > +#define _GLOBAL_TOC(A) _GLOBAL(A)
> > +#define _GLOBAL_TOC_KASAN(A) _GLOBAL(A)
> > +#define _GLOBAL_KASAN(A) _GLOBAL(A)
> > +
> > +#define PPC_MTOCRF(A, B) mtocrf A, B
> > +
> > +#define BEGIN_FTR_SECTION
> > +#define FTR_SECTION_ELSE
> > +#define ALT_FTR_SECTION_END_IFCLR(x)
> > +#define ALT_FTR_SECTION_END(x, y)
> > +#define END_FTR_SECTION_IFCLR(x)
> > +
> > +#endif /* __SELFTESTS_POWERPC_PPC_ASM_H */
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/asm/processor.h b/tools/testing/selftests/powerpc/memmoveloop/asm/processor.h
> > new file mode 100644
> > index 000000000000..e69de29bb2d1
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/mem_64.S b/tools/testing/selftests/powerpc/memmoveloop/mem_64.S
> > new file mode 120000
> > index 000000000000..db254c9a5f5c
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/memmoveloop/mem_64.S
> > @@ -0,0 +1 @@
> > +../../../../../arch/powerpc/lib/mem_64.S
> > \ No newline at end of file
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/memcpy_64.S b/tools/testing/selftests/powerpc/memmoveloop/memcpy_64.S
> > new file mode 120000
> > index 000000000000..cce33fb6f9d8
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/memmoveloop/memcpy_64.S
> > @@ -0,0 +1 @@
> > +../../../../../arch/powerpc/lib/memcpy_64.S
> > \ No newline at end of file
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/stubs.S b/tools/testing/selftests/powerpc/memmoveloop/stubs.S
> > new file mode 100644
> > index 000000000000..d9baa832fa49
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/memmoveloop/stubs.S
> > @@ -0,0 +1,8 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#include <asm/ppc_asm.h>
> > +
> > +FUNC_START(memcpy)
> > + b test_memcpy
> > +
> > +FUNC_START(backwards_memcpy)
> > + b test_backwards_memcpy
> > diff --git a/tools/testing/selftests/powerpc/memmoveloop/validate.c b/tools/testing/selftests/powerpc/memmoveloop/validate.c
> > new file mode 100644
> > index 000000000000..52f7d32bb3fe
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/memmoveloop/validate.c
> > @@ -0,0 +1,56 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <malloc.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <assert.h>
> > +#include "utils.h"
> > +
> > +void *TEST_MEMMOVE(const void *s1, const void *s2, size_t n);
> > +
> > +#define BUF_LEN 65536
> > +#define MAX_OFFSET 512
> > +
> > +size_t max(size_t a, size_t b)
> > +{
> > + if (a >= b) return a;
> > + return b;
> > +}
> > +
> > +static int testcase_run(void)
> > +{
> > + size_t i, src_off, dst_off, len;
> > +
> > + char *usermap = memalign(BUF_LEN, BUF_LEN);
> > + char *kernelmap = memalign(BUF_LEN, BUF_LEN);
> > +
> > + assert(usermap != NULL);
> > + assert(kernelmap != NULL);
> > +
> > + memset(usermap, 0, BUF_LEN);
> > + memset(kernelmap, 0, BUF_LEN);
> > +
> > + for (i = 0; i < BUF_LEN; i++) {
> > + usermap[i] = i & 0xff;
> > + kernelmap[i] = i & 0xff;
> > + }
> > +
> > + for (src_off = 0; src_off < MAX_OFFSET; src_off++) {
> > + for (dst_off = 0; dst_off < MAX_OFFSET; dst_off++) {
> > + for (len = 1; len < MAX_OFFSET - max(src_off, dst_off); len++) {
> > +
> > + memmove(usermap + dst_off, usermap + src_off, len);
> > + TEST_MEMMOVE(kernelmap + dst_off, kernelmap + src_off, len);
> > + if (memcmp(usermap, kernelmap, MAX_OFFSET) != 0) {
> > + printf("memmove failed at %ld %ld %ld\n", src_off, dst_off, len);
> > + abort();
> > + }
> > + }
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > +int main(void)
> > +{
> > + return test_harness(testcase_run, "memmove");
> > +}
> > --
> > 2.31.1
^ permalink raw reply
* [PATCHv2] selftests/powerpc/copyloops: Add memmove_64 test
From: Ritesh Harjani @ 2021-09-13 6:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Vaibhav Jain, Ritesh Harjani, Aneesh Kumar K . V
While debugging an issue, we wanted to check whether the arch specific
kernel memmove implementation is correct.
This selftest could help test that.
Suggested-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
v1 -> v2: Integrated memmove_64 test within copyloops tests
[v1]: https://patchwork.ozlabs.org/patch/1518082
.../selftests/powerpc/copyloops/.gitignore | 1 +
.../selftests/powerpc/copyloops/Makefile | 9 ++-
.../selftests/powerpc/copyloops/asm/ppc_asm.h | 1 +
.../selftests/powerpc/copyloops/mem_64.S | 1 +
.../powerpc/copyloops/memcpy_stubs.S | 8 +++
.../powerpc/copyloops/memmove_validate.c | 58 +++++++++++++++++++
6 files changed, 77 insertions(+), 1 deletion(-)
create mode 120000 tools/testing/selftests/powerpc/copyloops/mem_64.S
create mode 100644 tools/testing/selftests/powerpc/copyloops/memcpy_stubs.S
create mode 100644 tools/testing/selftests/powerpc/copyloops/memmove_validate.c
diff --git a/tools/testing/selftests/powerpc/copyloops/.gitignore b/tools/testing/selftests/powerpc/copyloops/.gitignore
index 994b11af765c..7283e8b07b75 100644
--- a/tools/testing/selftests/powerpc/copyloops/.gitignore
+++ b/tools/testing/selftests/powerpc/copyloops/.gitignore
@@ -13,3 +13,4 @@ copyuser_64_exc_t0
copyuser_64_exc_t1
copyuser_64_exc_t2
copy_mc_64
+memmove_64
diff --git a/tools/testing/selftests/powerpc/copyloops/Makefile b/tools/testing/selftests/powerpc/copyloops/Makefile
index 3095b1f1c02b..77594e697f2f 100644
--- a/tools/testing/selftests/powerpc/copyloops/Makefile
+++ b/tools/testing/selftests/powerpc/copyloops/Makefile
@@ -13,7 +13,8 @@ TEST_GEN_PROGS := copyuser_64_t0 copyuser_64_t1 copyuser_64_t2 \
copyuser_p7_t0 copyuser_p7_t1 \
memcpy_64_t0 memcpy_64_t1 memcpy_64_t2 \
memcpy_p7_t0 memcpy_p7_t1 copy_mc_64 \
- copyuser_64_exc_t0 copyuser_64_exc_t1 copyuser_64_exc_t2
+ copyuser_64_exc_t0 copyuser_64_exc_t1 copyuser_64_exc_t2 \
+ memmove_64
EXTRA_SOURCES := validate.c ../harness.c stubs.S
@@ -56,3 +57,9 @@ $(OUTPUT)/copyuser_64_exc_t%: copyuser_64.S exc_validate.c ../harness.c \
-D COPY_LOOP=test___copy_tofrom_user_base \
-D SELFTEST_CASE=$(subst copyuser_64_exc_t,,$(notdir $@)) \
-o $@ $^
+
+$(OUTPUT)/memmove_64: mem_64.S memcpy_64.S memmove_validate.c ../harness.c \
+ memcpy_stubs.S
+ $(CC) $(CPPFLAGS) $(CFLAGS) \
+ -D TEST_MEMMOVE=test_memmove \
+ -o $@ $^
diff --git a/tools/testing/selftests/powerpc/copyloops/asm/ppc_asm.h b/tools/testing/selftests/powerpc/copyloops/asm/ppc_asm.h
index 58c1cef3e399..003e1b3d9300 100644
--- a/tools/testing/selftests/powerpc/copyloops/asm/ppc_asm.h
+++ b/tools/testing/selftests/powerpc/copyloops/asm/ppc_asm.h
@@ -26,6 +26,7 @@
#define _GLOBAL(A) FUNC_START(test_ ## A)
#define _GLOBAL_TOC(A) _GLOBAL(A)
#define _GLOBAL_TOC_KASAN(A) _GLOBAL(A)
+#define _GLOBAL_KASAN(A) _GLOBAL(A)
#define PPC_MTOCRF(A, B) mtocrf A, B
diff --git a/tools/testing/selftests/powerpc/copyloops/mem_64.S b/tools/testing/selftests/powerpc/copyloops/mem_64.S
new file mode 120000
index 000000000000..db254c9a5f5c
--- /dev/null
+++ b/tools/testing/selftests/powerpc/copyloops/mem_64.S
@@ -0,0 +1 @@
+../../../../../arch/powerpc/lib/mem_64.S
\ No newline at end of file
diff --git a/tools/testing/selftests/powerpc/copyloops/memcpy_stubs.S b/tools/testing/selftests/powerpc/copyloops/memcpy_stubs.S
new file mode 100644
index 000000000000..d9baa832fa49
--- /dev/null
+++ b/tools/testing/selftests/powerpc/copyloops/memcpy_stubs.S
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <asm/ppc_asm.h>
+
+FUNC_START(memcpy)
+ b test_memcpy
+
+FUNC_START(backwards_memcpy)
+ b test_backwards_memcpy
diff --git a/tools/testing/selftests/powerpc/copyloops/memmove_validate.c b/tools/testing/selftests/powerpc/copyloops/memmove_validate.c
new file mode 100644
index 000000000000..1a23218b5757
--- /dev/null
+++ b/tools/testing/selftests/powerpc/copyloops/memmove_validate.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <malloc.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+#include "utils.h"
+
+void *TEST_MEMMOVE(const void *s1, const void *s2, size_t n);
+
+#define BUF_LEN 65536
+#define MAX_OFFSET 512
+
+size_t max(size_t a, size_t b)
+{
+ if (a >= b)
+ return a;
+ return b;
+}
+
+static int testcase_run(void)
+{
+ size_t i, src_off, dst_off, len;
+
+ char *usermap = memalign(BUF_LEN, BUF_LEN);
+ char *kernelmap = memalign(BUF_LEN, BUF_LEN);
+
+ assert(usermap != NULL);
+ assert(kernelmap != NULL);
+
+ memset(usermap, 0, BUF_LEN);
+ memset(kernelmap, 0, BUF_LEN);
+
+ for (i = 0; i < BUF_LEN; i++) {
+ usermap[i] = i & 0xff;
+ kernelmap[i] = i & 0xff;
+ }
+
+ for (src_off = 0; src_off < MAX_OFFSET; src_off++) {
+ for (dst_off = 0; dst_off < MAX_OFFSET; dst_off++) {
+ for (len = 1; len < MAX_OFFSET - max(src_off, dst_off); len++) {
+
+ memmove(usermap + dst_off, usermap + src_off, len);
+ TEST_MEMMOVE(kernelmap + dst_off, kernelmap + src_off, len);
+ if (memcmp(usermap, kernelmap, MAX_OFFSET) != 0) {
+ printf("memmove failed at %ld %ld %ld\n",
+ src_off, dst_off, len);
+ abort();
+ }
+ }
+ }
+ }
+ return 0;
+}
+
+int main(void)
+{
+ return test_harness(testcase_run, "memmove");
+}
--
2.31.1
^ permalink raw reply related
* [Bug 213837] "Kernel panic - not syncing: corrupted stack end detected inside scheduler" at building via distcc on a G5
From: bugzilla-daemon @ 2021-09-13 7:16 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-213837-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=213837
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |michael@ellerman.id.au
Component|Other |PPC-64
Hardware|All |PPC-64
Assignee|akpm@linux-foundation.org |platform_ppc-64@kernel-bugs
| |.osdl.org
Product|Memory Management |Platform Specific/Hardware
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.
^ permalink raw reply
* [Bug 213837] "Kernel panic - not syncing: corrupted stack end detected inside scheduler" at building via distcc on a G5
From: bugzilla-daemon @ 2021-09-13 7:16 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-213837-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=213837
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |NEEDINFO
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.
^ permalink raw reply
* Re: [PATCH v3 1/2] powerpc/64s: system call scv tabort fix for corrupt irq soft-mask state
From: kernel test robot @ 2021-09-13 7:51 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Eirik Fuller, kbuild-all, Nicholas Piggin
In-Reply-To: <202109040154.k1W0uBhG-lkp@intel.com>
[-- Attachment #1: Type: text/plain, Size: 4421 bytes --]
Please kindly note that this is a powerpc32 build.
Hi Nicholas,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.14 next-20210903]
[cannot apply to powerpc/next scottwood/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64s-system-call-scv-tabort-fix-for-corrupt-irq-soft-mask-state/20210903-205907
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a9c9a6f741cdaa2fa9ba24a790db8d07295761e3
:::::: branch date: 5 hours ago
:::::: commit date: 5 hours ago
config: powerpc-randconfig-s032-20210903 (attached as .config)
compiler: powerpc-linux-gcc (GCC) 11.2.0
reproduce:
cd linux
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O make.cross
chmod +x make.cross
# https://github.com/0day-ci/linux/commit/3510c8c0951ec7ac98da8d6931df7499ca6c881e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Nicholas-Piggin/powerpc-64s-system-call-scv-tabort-fix-for-corrupt-irq-soft-mask-state/20210903-205907
git checkout 3510c8c0951ec7ac98da8d6931df7499ca6c881e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 ./make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from arch/powerpc/include/asm/processor.h:11,
from arch/powerpc/include/asm/thread_info.h:40,
from include/linux/thread_info.h:60,
from arch/powerpc/include/asm/ptrace.h:298,
from arch/powerpc/include/asm/hw_irq.h:12,
from arch/powerpc/include/asm/irqflags.h:12,
from include/linux/irqflags.h:16,
from include/asm-generic/cmpxchg-local.h:6,
from arch/powerpc/include/asm/cmpxchg.h:526,
from arch/powerpc/include/asm/atomic.h:11,
from include/linux/atomic.h:7,
from include/linux/rcupdate.h:25,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from include/linux/context_tracking.h:5,
from arch/powerpc/kernel/interrupt.c:3:
arch/powerpc/kernel/interrupt.c: In function 'system_call_exception':
>> arch/powerpc/include/asm/reg.h:66:29: error: left shift count >= width of type [-Werror=shift-count-overflow]
66 | #define __MASK(X) (1UL<<(X))
| ^~
arch/powerpc/include/asm/reg.h:1378:61: note: in definition of macro 'mtmsr'
1378 | : "r" ((unsigned long)(v)) \
| ^
arch/powerpc/include/asm/reg.h:115:25: note: in expansion of macro '__MASK'
115 | #define MSR_TM __MASK(MSR_TM_LG) /* Transactional Mem Available */
| ^~~~~~
arch/powerpc/kernel/interrupt.c:153:33: note: in expansion of macro 'MSR_TM'
153 | mtmsr(mfmsr() | MSR_TM);
| ^~~~~~
cc1: all warnings being treated as errors
vim +66 arch/powerpc/include/asm/reg.h
14cf11af6cf608 include/asm-powerpc/reg.h Paul Mackerras 2005-09-26 62
9f04b9e327c495 include/asm-powerpc/reg.h Paul Mackerras 2005-10-10 63 #ifdef __ASSEMBLY__
9f04b9e327c495 include/asm-powerpc/reg.h Paul Mackerras 2005-10-10 64 #define __MASK(X) (1<<(X))
9f04b9e327c495 include/asm-powerpc/reg.h Paul Mackerras 2005-10-10 65 #else
9f04b9e327c495 include/asm-powerpc/reg.h Paul Mackerras 2005-10-10 @66 #define __MASK(X) (1UL<<(X))
9f04b9e327c495 include/asm-powerpc/reg.h Paul Mackerras 2005-10-10 67 #endif
9f04b9e327c495 include/asm-powerpc/reg.h Paul Mackerras 2005-10-10 68
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26576 bytes --]
^ permalink raw reply
* [Bug 206669] Little-endian kernel crashing on POWER8 on heavy big-endian PowerKVM load
From: bugzilla-daemon @ 2021-09-13 8:52 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206669-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206669
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |NEEDINFO
CC| |michael@ellerman.id.au
--- Comment #15 from Michael Ellerman (michael@ellerman.id.au) ---
After a day and a half I have managed to get BE debian installed in a VM :}
You said "running the glibc testsuite" was enough to trigger it. Do you mean
from the glibc git tree? I can't get upstream, or the debian packaged glibc
sources to build.
Both fail building with:
../include/setjmp.h:42:3: error: static assertion failed: "size of jmp_buf !=
656"
42 | _Static_assert (sizeof (type) == size, \
| ^~~~~~~~~~~~~~
I guess I'm doing something wrong.
Any pointers on what your setup is?
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
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