From: Russell Currey <rus...@russell.cc>

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 <rus...@russell.cc>
[mpe: Rework to avoid needing a definition in paca.h and lppaca.h]
Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
Link: https://msgid.link/20230808005317.20374-1-rus...@russell.cc
---
 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(-)

v2: mpe, rework based on the preceeding header untangling patches.

diff --git a/arch/powerpc/include/asm/lppaca.h 
b/arch/powerpc/include/asm/lppaca.h
index b6a63fa0965f..61ec2447dabf 100644
--- a/arch/powerpc/include/asm/lppaca.h
+++ b/arch/powerpc/include/asm/lppaca.h
@@ -23,6 +23,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,
@@ -105,14 +106,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 27fb656bd6ba..f2cb62148f36 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -640,16 +640,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 8acc70509520..1c151d77e74b 100644
--- a/arch/powerpc/platforms/pseries/lparcfg.c
+++ b/arch/powerpc/platforms/pseries/lparcfg.c
@@ -206,7 +206,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);
@@ -560,7 +560,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 bb0a9aeb50f9..ecea85c74c43 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -849,7 +849,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 a7d33f3ee01e..14db9b7d985d 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -414,13 +414,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.41.0

Reply via email to