From patchwork Thu Nov 30 23:54:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 10085847 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D3A1D602B5 for ; Thu, 30 Nov 2017 23:54:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C31012A431 for ; Thu, 30 Nov 2017 23:54:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B77202A434; Thu, 30 Nov 2017 23:54:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 549CE2A431 for ; Thu, 30 Nov 2017 23:54:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752427AbdK3Xy3 (ORCPT ); Thu, 30 Nov 2017 18:54:29 -0500 Received: from mx2.suse.de ([195.135.220.15]:36451 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752014AbdK3Xy2 (ORCPT ); Thu, 30 Nov 2017 18:54:28 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 1A153AF7D; Thu, 30 Nov 2017 23:54:26 +0000 (UTC) Date: Fri, 1 Dec 2017 00:54:25 +0100 From: "Luis R. Rodriguez" To: "Luis R. Rodriguez" Cc: Greg KH , akpm@linux-foundation.org, keescook@chromium.org, mfuzzey@parkeon.com, zohar@linux.vnet.ibm.com, dhowells@redhat.com, pali.rohar@gmail.com, tiwai@suse.de, arend.vanspriel@broadcom.com, zajec5@gmail.com, nbroeking@me.com, markivx@codeaurora.org, stephen.boyd@linaro.org, broonie@kernel.org, dmitry.torokhov@gmail.com, dwmw2@infradead.org, torvalds@linux-foundation.org, Abhay_Salunke@dell.com, bjorn.andersson@linaro.org, jewalt@lgsinnovations.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v2 19/23] firmware: add debug facility to emulate forcing sysfs fallback Message-ID: <20171130235425.GO729@wotan.suse.de> References: <20171120182409.27348-1-mcgrof@kernel.org> <20171120182409.27348-20-mcgrof@kernel.org> <20171129102804.GA12916@kroah.com> <20171130203516.GN729@wotan.suse.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20171130203516.GN729@wotan.suse.de> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Nov 30, 2017 at 09:35:16PM +0100, Luis R. Rodriguez wrote: > On Wed, Nov 29, 2017 at 11:28:04AM +0100, Greg KH wrote: > > On Mon, Nov 20, 2017 at 10:24:05AM -0800, Luis R. Rodriguez wrote: > > > diff --git a/drivers/base/firmware_debug.c b/drivers/base/firmware_debug.c > > > new file mode 100644 > > > index 000000000000..f2817eb6f480 > > > --- /dev/null > > > +++ b/drivers/base/firmware_debug.c > > > @@ -0,0 +1,34 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* Firmware dubugging interface */ > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > + > > > +#include > > > +#include "firmware_debug.h" > > > + > > > +struct firmware_debug fw_debug; > > > + > > > +static struct dentry *debugfs_firmware; > > > + > > > +int __init register_fw_debugfs(void) > > > +{ > > > + debugfs_firmware = debugfs_create_dir("firmware", NULL); > > > + if (!debugfs_firmware) > > > + return -ENOMEM; > > > > You never need to check the return value of a debugfs call, you should > > not care about it, nor do anything different in your code. The value > > returned can always be passed back into any other debugfs call when > > needed. > > Neat, so all uses as in the above are wrong eh? You know, I'm wondering if it just makes sense to go straight into making CONFIG_FW_LOADER_USER_HELPER_FALLBACK nothing but a setting a bool on a config to true. Thoughts, preferences? Luis diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader.c index 43b97a8137f7..d3f2aabfc41d 100644 --- a/drivers/base/firmware_loader.c +++ b/drivers/base/firmware_loader.c @@ -117,6 +117,12 @@ struct fw_name_devm { const char *name; }; +struct firmware_config { + bool force_sysfs_fallback; +}; + +static struct firmware_config fw_config; + static inline struct fw_priv *to_fw_priv(struct kref *ref) { return container_of(ref, struct fw_priv, ref); @@ -1151,18 +1157,25 @@ static int fw_load_from_user_helper(struct firmware *firmware, } #ifdef CONFIG_FW_LOADER_USER_HELPER_FALLBACK -static bool fw_force_sysfs_fallback(unsigned int opt_flags) +static void __init fw_config_init(void) { - return true; + fw_config.force_sysfs_fallback = true; } + #else +static void __init fw_config_init(void) +{ +} +#endif + static bool fw_force_sysfs_fallback(unsigned int opt_flags) { + if (fw_config.force_sysfs_fallback) + return true; if (!(opt_flags & FW_OPT_USERHELPER)) return false; return true; } -#endif static bool fw_run_sysfs_fallback(unsigned int opt_flags) { @@ -1911,6 +1924,7 @@ static int __init firmware_class_init(void) int ret; /* No need to unfold these on exit */ + fw_config_init(); fw_cache_init(); ret = register_fw_pm_ops(); After which we can add two generic syfs firmware knobs to help do the same as I did for debugfs, only we actually support it as proper API. Thoughts? For instance for changing to force the usermode helper: diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader.c index d3f2aabfc41d..659db28f5c02 100644 --- a/drivers/base/firmware_loader.c +++ b/drivers/base/firmware_loader.c @@ -41,6 +41,9 @@ MODULE_AUTHOR("Manuel Estrada Sainz"); MODULE_DESCRIPTION("Multi purpose firmware loading support"); MODULE_LICENSE("GPL"); +static unsigned int zero; +static unsigned int one = 1; + enum fw_status { FW_STATUS_UNKNOWN, FW_STATUS_LOADING, @@ -1919,6 +1922,19 @@ static struct notifier_block fw_shutdown_nb = { .notifier_call = fw_shutdown_notify, }; +struct ctl_table firmware_config_table[] = { + { + .procname = "force_sysfs_fallback", + .data = &fw_config.force_sysfs_fallback, + .maxlen = sizeof(unsigned int), + .mode = 0644, + .proc_handler = proc_douintvec_minmax, + .extra1 = &zero, + .extra2 = &one, + }, + { } +}; + static int __init firmware_class_init(void) { int ret; diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 557d46728577..202442f3c58c 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -251,6 +251,10 @@ extern struct ctl_table random_table[]; extern struct ctl_table epoll_table[]; #endif +#ifdef CONFIG_FW_LOADER +extern struct ctl_table firmware_config_table[]; +#endif + #ifdef HAVE_ARCH_PICK_MMAP_LAYOUT int sysctl_legacy_va_layout; #endif @@ -746,6 +750,13 @@ static struct ctl_table kern_table[] = { .mode = 0555, .child = usermodehelper_table, }, +#ifdef CONFIG_FW_LOADER + { + .procname = "firmware_config", + .mode = 0555, + .child = firmware_config_table, + }, +#endif { .procname = "overflowuid", .data = &overflowuid,