6.1-stable review patch. If anyone has any objections, please let me know.
------------------
From: Russell Currey <ruscur@xxxxxxxxxx>
[ Upstream commit eac030b22ea12cdfcbb2e941c21c03964403c63f ]
lppaca_shared_proc() takes a pointer to the lppaca which is typically
accessed through get_lppaca(). With DEBUG_PREEMPT enabled, this leads
to checking if preemption is enabled, for example:
BUG: using smp_processor_id() in preemptible [00000000] code: grep/10693
caller is lparcfg_data+0x408/0x19a0
CPU: 4 PID: 10693 Comm: grep Not tainted 6.5.0-rc3 #2
Call Trace:
dump_stack_lvl+0x154/0x200 (unreliable)
check_preemption_disabled+0x214/0x220
lparcfg_data+0x408/0x19a0
...
This isn't actually a problem however, as it does not matter which
lppaca is accessed, the shared proc state will be the same.
vcpudispatch_stats_procfs_init() already works around this by disabling
preemption, but the lparcfg code does not, erroring any time
/proc/powerpc/lparcfg is accessed with DEBUG_PREEMPT enabled.
Instead of disabling preemption on the caller side, rework
lppaca_shared_proc() to not take a pointer and instead directly access
the lppaca, bypassing any potential preemption checks.
Fixes: f13c13a00512 ("powerpc: Stop using non-architected shared_proc field in lppaca")
Signed-off-by: Russell Currey <ruscur@xxxxxxxxxx>
[mpe: Rework to avoid needing a definition in paca.h and lppaca.h]
Signed-off-by: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
Link: https://msgid.link/20230823055317.751786-4-mpe@xxxxxxxxxxxxxx
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
arch/powerpc/include/asm/lppaca.h | 11 +++++++++--
arch/powerpc/platforms/pseries/lpar.c | 10 +---------
arch/powerpc/platforms/pseries/lparcfg.c | 4 ++--
arch/powerpc/platforms/pseries/setup.c | 2 +-
drivers/cpuidle/cpuidle-pseries.c | 8 +-------
5 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/include/asm/lppaca.h b/arch/powerpc/include/asm/lppaca.h
index fe278172e9d42..ee1488d38fdc1 100644
--- a/arch/powerpc/include/asm/lppaca.h
+++ b/arch/powerpc/include/asm/lppaca.h
@@ -45,6 +45,7 @@
#include <asm/types.h>
#include <asm/mmu.h>
#include <asm/firmware.h>
+#include <asm/paca.h>
/*
* The lppaca is the "virtual processor area" registered with the hypervisor,
@@ -127,14 +128,20 @@ struct lppaca {
*/
#define LPPACA_OLD_SHARED_PROC 2
-static inline bool lppaca_shared_proc(struct lppaca *l)
+#ifdef CONFIG_PPC_PSERIES
+/*
+ * All CPUs should have the same shared proc value, so directly access the PACA
+ * to avoid false positives from DEBUG_PREEMPT.
+ */
+static inline bool lppaca_shared_proc(void)
{
+ struct lppaca *l = local_paca->lppaca_ptr;
+
if (!firmware_has_feature(FW_FEATURE_SPLPAR))
return false;
return !!(l->__old_status & LPPACA_OLD_SHARED_PROC);
}
-#ifdef CONFIG_PPC_PSERIES
#define get_lppaca() (get_paca()->lppaca_ptr)
#endif
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index 97ef6499e5019..2c2812a87d470 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -638,16 +638,8 @@ static const struct proc_ops vcpudispatch_stats_freq_proc_ops = {
static int __init vcpudispatch_stats_procfs_init(void)
{
- /*
- * Avoid smp_processor_id while preemptible. All CPUs should have
- * the same value for lppaca_shared_proc.
- */
- preempt_disable();
- if (!lppaca_shared_proc(get_lppaca())) {
- preempt_enable();
+ if (!lppaca_shared_proc())
return 0;
- }
- preempt_enable();
if (!proc_create("powerpc/vcpudispatch_stats", 0600, NULL,
&vcpudispatch_stats_proc_ops))
diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
index 63fd925ccbb83..ca10a3682c46e 100644
--- a/arch/powerpc/platforms/pseries/lparcfg.c
+++ b/arch/powerpc/platforms/pseries/lparcfg.c
@@ -205,7 +205,7 @@ static void parse_ppp_data(struct seq_file *m)
ppp_data.active_system_procs);
/* pool related entries are appropriate for shared configs */
- if (lppaca_shared_proc(get_lppaca())) {
+ if (lppaca_shared_proc()) {
unsigned long pool_idle_time, pool_procs;
seq_printf(m, "pool=%d\n", ppp_data.pool_num);
@@ -616,7 +616,7 @@ static int pseries_lparcfg_data(struct seq_file *m, void *v)
partition_potential_processors);
seq_printf(m, "shared_processor_mode=%d\n",
- lppaca_shared_proc(get_lppaca()));
+ lppaca_shared_proc());
#ifdef CONFIG_PPC_64S_HASH_MMU
if (!radix_enabled())
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 8ef3270515a9b..a0701dbdb1348 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -846,7 +846,7 @@ static void __init pSeries_setup_arch(void)
if (firmware_has_feature(FW_FEATURE_LPAR)) {
vpa_init(boot_cpuid);
- if (lppaca_shared_proc(get_lppaca())) {
+ if (lppaca_shared_proc()) {
static_branch_enable(&shared_processor);
pv_spinlocks_init();
#ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index 7e7ab5597d7ac..0590001db6532 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -410,13 +410,7 @@ static int __init pseries_idle_probe(void)
return -ENODEV;
if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
- /*
- * Use local_paca instead of get_lppaca() since
- * preemption is not disabled, and it is not required in
- * fact, since lppaca_ptr does not need to be the value
- * associated to the current CPU, it can be from any CPU.
- */
- if (lppaca_shared_proc(local_paca->lppaca_ptr)) {
+ if (lppaca_shared_proc()) {
cpuidle_state_table = shared_states;
max_idle_state = ARRAY_SIZE(shared_states);
} else {
--
2.40.1
