public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: gregkh@linuxfoundation.org
Cc: wagi@monom.org, dwmw2@infradead.org, rafal@milecki.pl,
	arend.vanspriel@broadcom.com, rjw@rjwysocki.net,
	yi1.li@linux.intel.com, atull@opensource.altera.com,
	moritz.fischer@ettus.com, pmladek@suse.com,
	johannes.berg@intel.com, emmanuel.grumbach@intel.com,
	luciano.coelho@intel.com, kvalo@codeaurora.org, luto@kernel.org,
	torvalds@linux-foundation.org, keescook@chromium.org,
	takahiro.akashi@linaro.org, dhowells@redhat.com,
	pjones@redhat.com, hdegoede@redhat.com,
	linux-kernel@vger.kernel.org,
	"Luis R. Rodriguez" <mcgrof@kernel.org>
Subject: [PATCH v2 4/6] firmware: add sanity check on shutdown/suspend
Date: Tue,  2 May 2017 01:31:05 -0700	[thread overview]
Message-ID: <20170502083107.23418-5-mcgrof@kernel.org> (raw)
In-Reply-To: <20170502083107.23418-1-mcgrof@kernel.org>

The firmware API should not be used after we go to suspend
and after we reboot/halt. The suspend/resume case is a bit
complex, so this documents that so things are clearer.

We want to know about users of the API in incorrect places so
that their callers are corrected, so this also adds a warn
for those cases.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 .../driver-api/firmware/request_firmware.rst       | 11 +++
 drivers/base/firmware_class.c                      | 99 ++++++++++++++++++++++
 2 files changed, 110 insertions(+)

diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
index cc0aea880824..1c2c4967cd43 100644
--- a/Documentation/driver-api/firmware/request_firmware.rst
+++ b/Documentation/driver-api/firmware/request_firmware.rst
@@ -44,6 +44,17 @@ request_firmware_nowait
 .. kernel-doc:: drivers/base/firmware_class.c
    :functions: request_firmware_nowait
 
+Considerations for suspend and resume
+=====================================
+
+During suspend and resume only the built-in firmware and the firmware cache
+elements of the firmware API can be used. This is managed by fw_pm_notify().
+
+fw_pm_notify
+------------
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: fw_pm_notify
+
 request firmware API expected driver use
 ========================================
 
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index dd9b7f3d0927..a272c8662212 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -260,6 +260,38 @@ static int fw_cache_piggyback_on_request(const char *name);
  * guarding for corner cases a global lock should be OK */
 static DEFINE_MUTEX(fw_lock);
 
+static bool __enable_firmware = false;
+
+static void enable_firmware(void)
+{
+	mutex_lock(&fw_lock);
+	__enable_firmware = true;
+	mutex_unlock(&fw_lock);
+}
+
+static void disable_firmware(void)
+{
+	mutex_lock(&fw_lock);
+	__enable_firmware = false;
+	mutex_unlock(&fw_lock);
+}
+
+/*
+ * When disabled only the built-in firmware and the firmware cache will be
+ * used to look for firmware.
+ */
+static bool firmware_enabled(void)
+{
+	bool enabled = false;
+
+	mutex_lock(&fw_lock);
+	if (__enable_firmware)
+		enabled = true;
+	mutex_unlock(&fw_lock);
+
+	return enabled;
+}
+
 static struct firmware_cache fw_cache;
 
 static struct firmware_buf *__allocate_fw_buf(const char *fw_name,
@@ -1163,6 +1195,12 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 	if (ret <= 0) /* error or already assigned */
 		goto out;
 
+	if (!firmware_enabled()) {
+		WARN(1, "firmware request while host is not available\n");
+		ret = -EHOSTDOWN;
+		goto out;
+	}
+
 	ret = 0;
 	timeout = firmware_loading_timeout();
 	if (opt_flags & FW_OPT_NOWAIT) {
@@ -1695,6 +1733,62 @@ static void device_uncache_fw_images_delay(unsigned long delay)
 			   msecs_to_jiffies(delay));
 }
 
+/**
+ * fw_pm_notify - notifier for suspend/resume
+ * @notify_block: unused
+ * @mode: mode we are switching to
+ * @unused: unused
+ *
+ * Used to modify the firmware_class state as we move in between states.
+ * The firmware_class implements a firmware cache to enable device driver
+ * to fetch firmware upon resume before the root filesystem is ready. We
+ * disable API calls which do not use the built-in firmware or the firmware
+ * cache when we know these calls will not work.
+ *
+ * The inner logic behind all this is a bit complex so it is worth summarizing
+ * the kernel's own suspend/resume process with context and focus on how this
+ * can impact the firmware API.
+ *
+ * First a review on how we go to suspend::
+ *
+ *	pm_suspend() --> enter_state() -->
+ *	sys_sync()
+ *	suspend_prepare() -->
+ *		__pm_notifier_call_chain(PM_SUSPEND_PREPARE, ...);
+ *		suspend_freeze_processes() -->
+ *			freeze_processes() -->
+ *				__usermodehelper_set_disable_depth(UMH_DISABLED);
+ *				freeze all tasks ...
+ *			freeze_kernel_threads()
+ *	suspend_devices_and_enter() -->
+ *		dpm_suspend_start() -->
+ *				dpm_prepare()
+ *				dpm_suspend()
+ *		suspend_enter()  -->
+ *			platform_suspend_prepare()
+ *			dpm_suspend_late()
+ *			freeze_enter()
+ *			syscore_suspend()
+ *
+ * When we resume we bail out of a loop from suspend_devices_and_enter() and
+ * unwind back out to the caller enter_state() where we were before as follows::
+ *
+ * 	enter_state() -->
+ *	suspend_devices_and_enter() --> (bail from loop)
+ *		dpm_resume_end() -->
+ *			dpm_resume()
+ *			dpm_complete()
+ *	suspend_finish() -->
+ *		suspend_thaw_processes() -->
+ *			thaw_processes() -->
+ *				__usermodehelper_set_disable_depth(UMH_FREEZING);
+ *				thaw_workqueues();
+ *				thaw all processes ...
+ *				usermodehelper_enable();
+ *		pm_notifier_call_chain(PM_POST_SUSPEND);
+ *
+ * fw_pm_notify() works through pm_notifier_call_chain().
+ */
 static int fw_pm_notify(struct notifier_block *notify_block,
 			unsigned long mode, void *unused)
 {
@@ -1708,6 +1802,7 @@ static int fw_pm_notify(struct notifier_block *notify_block,
 		 */
 		kill_pending_fw_fallback_reqs(true);
 		device_cache_fw_images();
+		disable_firmware();
 		break;
 
 	case PM_POST_SUSPEND:
@@ -1720,6 +1815,7 @@ static int fw_pm_notify(struct notifier_block *notify_block,
 		mutex_lock(&fw_lock);
 		fw_cache.state = FW_LOADER_NO_CACHE;
 		mutex_unlock(&fw_lock);
+		enable_firmware();
 
 		device_uncache_fw_images_delay(10 * MSEC_PER_SEC);
 		break;
@@ -1768,6 +1864,7 @@ static void __init fw_cache_init(void)
 static int fw_shutdown_notify(struct notifier_block *unused1,
 			      unsigned long unused2, void *unused3)
 {
+	disable_firmware();
 	/*
 	 * Kill all pending fallback requests to avoid both stalling shutdown,
 	 * and avoid a deadlock with the usermode_lock.
@@ -1783,6 +1880,7 @@ static struct notifier_block fw_shutdown_nb = {
 
 static int __init firmware_class_init(void)
 {
+	enable_firmware();
 	fw_cache_init();
 	register_reboot_notifier(&fw_shutdown_nb);
 #ifdef CONFIG_FW_LOADER_USER_HELPER
@@ -1794,6 +1892,7 @@ static int __init firmware_class_init(void)
 
 static void __exit firmware_class_exit(void)
 {
+	disable_firmware();
 #ifdef CONFIG_PM_SLEEP
 	unregister_syscore_ops(&fw_syscore_ops);
 	unregister_pm_notifier(&fw_cache.pm_notify);
-- 
2.11.0

  parent reply	other threads:[~2017-05-02  8:31 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-30  3:24 [PATCH 0/5] firmware: move UMH locks onto fallback code Luis R. Rodriguez
2017-03-30  3:24 ` [PATCH 1/5] firmware: share fw fallback killing on reboot/suspend Luis R. Rodriguez
2017-04-06  6:38   ` Coelho, Luciano
2017-04-27  1:56     ` Luis R. Rodriguez
2017-03-30  3:24 ` [PATCH 2/5] firmware: always enable the reboot notifier Luis R. Rodriguez
2017-03-30  3:24 ` [PATCH 3/5] firmware: add sanity check on shutdown/suspend Luis R. Rodriguez
2017-03-30  3:24 ` [PATCH 4/5] firmware: move assign_firmware_buf() further up Luis R. Rodriguez
2017-03-30  3:24 ` [PATCH 5/5] firmware: move umh try locks into the umh code Luis R. Rodriguez
2017-05-02  8:31 ` [PATCH v2 0/6] firmware: move UMH locks onto fallback code Luis R. Rodriguez
2017-05-02  8:31   ` [PATCH v2 1/6] firmware: move kill_requests_without_uevent() up above Luis R. Rodriguez
2017-05-02  8:31   ` [PATCH v2 2/6] firmware: share fw fallback killing on reboot/suspend Luis R. Rodriguez
2017-05-02  8:31   ` [PATCH v2 3/6] firmware: always enable the reboot notifier Luis R. Rodriguez
2017-05-02  8:31   ` Luis R. Rodriguez [this message]
2017-05-02  8:31   ` [PATCH v2 5/6] firmware: move assign_firmware_buf() further up Luis R. Rodriguez
2017-05-02  8:31   ` [PATCH v2 6/6] firmware: move umh try locks into the umh code Luis R. Rodriguez

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=20170502083107.23418-5-mcgrof@kernel.org \
    --to=mcgrof@kernel.org \
    --cc=arend.vanspriel@broadcom.com \
    --cc=atull@opensource.altera.com \
    --cc=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=emmanuel.grumbach@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=johannes.berg@intel.com \
    --cc=keescook@chromium.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luciano.coelho@intel.com \
    --cc=luto@kernel.org \
    --cc=moritz.fischer@ettus.com \
    --cc=pjones@redhat.com \
    --cc=pmladek@suse.com \
    --cc=rafal@milecki.pl \
    --cc=rjw@rjwysocki.net \
    --cc=takahiro.akashi@linaro.org \
    --cc=torvalds@linux-foundation.org \
    --cc=wagi@monom.org \
    --cc=yi1.li@linux.intel.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox