All of lore.kernel.org
 help / color / mirror / Atom feed
From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
To: rjw@rjwysocki.net, lenb@kernel.org, viresh.kumar@linaro.org
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	axboe@kernel.dk,
	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Subject: [PATCH] cpufreq: intel_pstate: Fix for HWP interrupt before driver is ready
Date: Fri,  3 Sep 2021 22:37:03 -0700	[thread overview]
Message-ID: <20210904053703.581297-1-srinivas.pandruvada@linux.intel.com> (raw)

In Lenovo X1 gen9 laptop, HWP interrupts arrive before driver is ready
to handle on that CPU. Basically didn't even allocated memory for per
cpu data structure and not even started interrupt enable process on that
CPU. So interrupt handler observes a NULL pointer to schedule work.

This interrupt was probably for SMM, but since it is redirected to
OS by OSC call, OS receives it, but not ready to handle. That redirection
of interrupt to OS was also done to solve one SMM crash on Yoga 260 for
HWP interrupt a while back.

To solve this the HWP interrupt handler should ignore such request if the
driver is not ready. This will require some flag to wait till the driver
setup a workqueue to handle on a CPU. We can't simply assume cpudata to
be NULL and avoid processing as it may not be NULL but data structure is
not in consistent state.

So created a cpumask which sets the CPU on which interrupt was setup. If
not setup, simply clear the interrupt status and return. Since the
similar issue can happen during S3 resume, clear the bit during offline.

Since interrupt timing may be before HWP is enabled, use safe MSR read
writes as before the change for HWP interrupt.

Fixes: d0e936adbd22 ("cpufreq: intel_pstate: Process HWP Guaranteed change notification")
Reported-and-tested-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/intel_pstate.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index b4ffe6c8a0d0..5ac86bfa1080 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -298,6 +298,8 @@ static bool hwp_boost __read_mostly;
 
 static struct cpufreq_driver *intel_pstate_driver __read_mostly;
 
+static cpumask_t hwp_intr_enable_mask;
+
 #ifdef CONFIG_ACPI
 static bool acpi_ppc;
 #endif
@@ -1067,11 +1069,15 @@ static void intel_pstate_hwp_set(unsigned int cpu)
 	wrmsrl_on_cpu(cpu, MSR_HWP_REQUEST, value);
 }
 
+static void intel_pstate_disable_hwp_interrupt(struct cpudata *cpudata);
+
 static void intel_pstate_hwp_offline(struct cpudata *cpu)
 {
 	u64 value = READ_ONCE(cpu->hwp_req_cached);
 	int min_perf;
 
+	intel_pstate_disable_hwp_interrupt(cpu);
+
 	if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
 		/*
 		 * In case the EPP has been set to "performance" by the
@@ -1645,20 +1651,35 @@ void notify_hwp_interrupt(void)
 	if (!hwp_active || !boot_cpu_has(X86_FEATURE_HWP_NOTIFY))
 		return;
 
-	rdmsrl(MSR_HWP_STATUS, value);
+	rdmsrl_safe(MSR_HWP_STATUS, &value);
 	if (!(value & 0x01))
 		return;
 
+	if (!cpumask_test_cpu(this_cpu, &hwp_intr_enable_mask)) {
+		wrmsrl_safe(MSR_HWP_STATUS, 0);
+		return;
+	}
+
 	cpudata = all_cpu_data[this_cpu];
 	schedule_delayed_work_on(this_cpu, &cpudata->hwp_notify_work, msecs_to_jiffies(10));
 }
 
+static void intel_pstate_disable_hwp_interrupt(struct cpudata *cpudata)
+{
+
+	if (cpumask_test_and_clear_cpu(cpudata->cpu, &hwp_intr_enable_mask)) {
+		wrmsrl_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x00);
+		cancel_delayed_work_sync(&cpudata->hwp_notify_work);
+	}
+}
+
 static void intel_pstate_enable_hwp_interrupt(struct cpudata *cpudata)
 {
 	/* Enable HWP notification interrupt for guaranteed performance change */
 	if (boot_cpu_has(X86_FEATURE_HWP_NOTIFY)) {
 		INIT_DELAYED_WORK(&cpudata->hwp_notify_work, intel_pstate_notify_work);
 		wrmsrl_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x01);
+		cpumask_set_cpu(cpudata->cpu, &hwp_intr_enable_mask);
 	}
 }
 
-- 
2.31.1


             reply	other threads:[~2021-09-04  5:37 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-04  5:37 Srinivas Pandruvada [this message]
2021-09-06 16:17 ` [PATCH] cpufreq: intel_pstate: Fix for HWP interrupt before driver is ready Rafael J. Wysocki
2021-09-06 16:43   ` Jens Axboe
2021-09-06 16:54     ` Srinivas Pandruvada
2021-09-06 16:58       ` Rafael J. Wysocki
2021-09-06 17:23         ` Srinivas Pandruvada
2021-09-06 17:54           ` Rafael J. Wysocki
2021-09-06 18:14             ` Srinivas Pandruvada
2021-09-06 18:25               ` Rafael J. Wysocki
2021-09-06 19:56                 ` Srinivas Pandruvada
2021-09-07 13:41                   ` Rafael J. Wysocki
2021-09-06 16:53   ` Srinivas Pandruvada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210904053703.581297-1-srinivas.pandruvada@linux.intel.com \
    --to=srinivas.pandruvada@linux.intel.com \
    --cc=axboe@kernel.dk \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.