From: Kees Cook <keescook@chromium.org>
To: "Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: "Greg KH" <gregkh@linuxfoundation.org>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Josh Triplett" <josh@joshtriplett.org>,
maco@android.com, "Andy Gross" <andy.gross@linaro.org>,
"David Brown" <david.brown@linaro.org>,
bjorn.andersson@linaro.org, teg@jklm.no, wagi@monom.org,
"Hans de Goede" <hdegoede@redhat.com>,
"Andres Rodriguez" <andresx7@gmail.com>,
"Mimi Zohar" <zohar@linux.vnet.ibm.com>,
"Jakub Kicinski" <kubakici@wp.pl>,
"Shuah Khan" <shuah@kernel.org>,
"Martin Fuzzey" <mfuzzey@parkeon.com>,
"David Howells" <dhowells@redhat.com>,
pali.rohar@gmail.com, "Takashi Iwai" <tiwai@suse.de>,
"Kalle Valo" <kvalo@codeaurora.org>,
"Arend van Spriel" <arend.vanspriel@broadcom.com>,
"Rafał Miłecki" <zajec5@gmail.com>,
"Nicolas Broeking" <nbroeking@me.com>,
"Vikram Mulukutla" <markivx@codeaurora.org>,
"Mark Brown" <broonie@kernel.org>,
"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
"David Woodhouse" <dwmw2@infradead.org>,
"Linus Torvalds" <torvalds@linux-foundation.org>,
Abhay_Salunke@dell.com, jewalt@lgsinnovations.com,
oneukum@suse.com,
"Bitterblue of Monsea" <cantabile.desu@gmail.com>,
"Alexei Starovoitov" <ast@fb.com>, hare <hare@suse.com>,
"James E.J. Bottomley" <jejb@linux.vnet.ibm.com>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
khc@pm.waw.pl, "David S. Miller" <davem@davemloft.net>,
"Arve Hjønnevåg" <arve@android.com>,
tkjos@android.com, "Jonathan Corbet" <corbet@lwn.net>,
mchehab+samsung@kernel.org, LKML <linux-kernel@vger.kernel.org>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
linux-scsi@vger.kernel.org,
linux-wireless <linux-wireless@vger.kernel.org>,
"Network Development" <netdev@vger.kernel.org>
Subject: Re: [PATCH v6 05/13] firmware_loader: enhance Kconfig documentation over FW_LOADER
Date: Tue, 8 May 2018 15:42:33 -0700 [thread overview]
Message-ID: <CAGXu5jJXMxx2Xna_M_g1wsS59XnYKqHAGdxhzDtk3PhnLL-gQQ@mail.gmail.com> (raw)
In-Reply-To: <20180508181247.19431-6-mcgrof@kernel.org>
On Tue, May 8, 2018 at 11:12 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> If you try to read FW_LOADER today it speaks of old riddles and
> unless you have been following development closely you will loose
Typo: lose
> track of what is what. Even the documentation for PREVENT_FIRMWARE_BUILD
> is a bit fuzzy and how it fits into this big picture.
>
> Give the FW_LOADER kconfig documentation some love with more up to
> date developments and recommendations. While at it, wrap the FW_LOADER
> code into its own menu to compartamentalize and make it clearer which
Typo: compartmentalize
> components really are part of the FW_LOADER. This should also make
> it easier to later move these kconfig entries into the firmware_loader/
> directory later.
>
> This also now recommends using firmwared [0] for folks left needing a uevent
> handler in userspace for the sysfs firmware fallback mechanis given udev's
> uevent firmware mechanism was ripped out a while ago.
>
> [0] https://github.com/teg/firmwared
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
> drivers/base/Kconfig | 165 ++++++++++++++++++++++++++++++++++---------
> 1 file changed, 131 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 29b0eb452b3a..a4fe86caecca 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -70,39 +70,64 @@ config STANDALONE
> If unsure, say Y.
>
> config PREVENT_FIRMWARE_BUILD
> - bool "Prevent firmware from being built"
> + bool "Disable drivers features which enable custom firmware building"
> default y
> help
> - Say yes to avoid building firmware. Firmware is usually shipped
> - with the driver and only when updating the firmware should a
> - rebuild be made.
> - If unsure, say Y here.
> + Say yes to disable driver features which enable building a custom
> + driver firmwar at kernel build time. These drivers do not use the
Typo: firmware
> + kernel firmware API to load firmware (CONFIG_FW_LOADER), instead they
> + use their own custom loading mechanism. The required firmware is
> + usually shipped with the driver, building the driver firmware
> + should only be needed if you have an updated firmware source.
> +
> + Firmware should not be being built as part of kernel, these days
> + you should always prevent this and say Y here. There are only two
> + old drivers which enable building of its firmware at kernel build
> + time:
> +
> + o CONFIG_WANXL through CONFIG_WANXL_BUILD_FIRMWARE
> + o CONFIG_SCSI_AIC79XX through CONFIG_AIC79XX_BUILD_FIRMWARE
> +
> +menu "Firmware loader"
>
> config FW_LOADER
> - tristate "Userspace firmware loading support" if EXPERT
> + tristate "Firmware loading facility" if EXPERT
> default y
> ---help---
> - This option is provided for the case where none of the in-tree modules
> - require userspace firmware loading support, but a module built
> - out-of-tree does.
> + This enables the firmware loading facility in the kernel. The kernel
> + will first look for built-in firmware, if it has any. Next, it will
> + look for the requested firmware in a series of filesystem paths:
> +
> + o firmware_class path module parameter or kernel boot param
> + o /lib/firmware/updates/UTS_RELEASE
> + o /lib/firmware/updates
> + o /lib/firmware/UTS_RELEASE
> + o /lib/firmware
> +
> + Enabling this feature only increases your kernel image by about
> + 828 bytes, enable this option unless you are certain you don't
> + need firmware.
> +
> + You typically want this built-in (=y) but you can also enable this
> + as a module, in which case the firmware_class module will be built.
> + You also want to be sure to enable this built-in if you are going to
> + enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
> +
> +if FW_LOADER
>
> config EXTRA_FIRMWARE
> - string "External firmware blobs to build into the kernel binary"
> - depends on FW_LOADER
> + string "Build these firmware blobs into the kernel binary"
Maybe "Build named firmware blobs ..." "these" took me a while to figure out.
> help
> - Various drivers in the kernel source tree may require firmware,
> - which is generally available in your distribution's linux-firmware
> - package.
> + Device drivers which require firmware can typically deal with
> + having the kernel load firmware from the various supported
> + /lib/firmware/ paths. This option enables you to build into the
> + kernel firmware files. Built-in firmware searches are preceeded
Typo: preceded
> + over firmware lookups using your filesystem over the supported
> + /lib/firmware paths documented on CONFIG_FW_LOADER.
>
> - The linux-firmware package should install firmware into
> - /lib/firmware/ on your system, so they can be loaded by userspace
> - helpers on request.
> -
> - This option allows firmware to be built into the kernel for the case
> - where the user either cannot or doesn't want to provide it from
> - userspace at runtime (for example, when the firmware in question is
> - required for accessing the boot device, and the user doesn't want to
> - use an initrd).
> + This may be useful for testing or if the firmware is required early on
> + in boot and cannot rely on the firmware being placed in an initrd or
> + initramfs.
>
> This option is a string and takes the (space-separated) names of the
> firmware files -- the same names that appear in MODULE_FIRMWARE()
> @@ -113,7 +138,7 @@ config EXTRA_FIRMWARE
> For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
> the usb8388.bin file into /lib/firmware, and build the kernel. Then
> any request_firmware("usb8388.bin") will be satisfied internally
> - without needing to call out to userspace.
> + inside the kernel without ever looking at your filesystem at runtime.
>
> WARNING: If you include additional firmware files into your binary
> kernel image that are not available under the terms of the GPL,
> @@ -130,22 +155,94 @@ config EXTRA_FIRMWARE_DIR
> looks for the firmware files listed in the EXTRA_FIRMWARE option.
>
> config FW_LOADER_USER_HELPER
> - bool
> + bool "Enable the firmware sysfs fallback mechanism"
> + help
> + This option enables a sysfs loading facility to enable firmware
> + loading to the kernel through userspace as a fallback mechanism
> + if and only if the kernel's direct filesystem lookup for the
> + firmware failed using the different /lib/firmware/ paths, or the
> + path specified in the firmware_class path module parameter, or the
> + firmware_class path kernel boot parameter if the firmware_class is
> + built-in. For details on how to work with the sysfs fallback mechanism
> + refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
> +
> + The direct filesystem lookup for firwmare is always used first now.
Typo: firmware
> +
> + If the kernel's direct filesystem lookup for firware fails to find
> + the requested firmware a sysfs fallback loading facility is made
> + available and userspace is informed about this through uevents.
> + The uevent can be supressed if the driver explicitly requested it,
> + this is known as the driver using the custom fallback mechanism.
> + If the custom fallback mechanism is used userspace must always
> + acknowledge failure to find firmware as the timeout for the fallback
> + mechanism is disabled, and failed requests will linger forever.
> +
> + This used to be the default firmware loading facility, and udev used
> + to listen for uvents to load firmware for the kernel. The firmware
> + loading facility functionality in udev has been removed, as such it
> + can no longer be relied upon as a fallback mechanism. Linux no longer
> + relies on or uses a fallback mechanism in userspace. If you need to
> + rely on one refer to the permissively licensed firmwared:
Typo: firmware
> +
> + https://github.com/teg/firmwared
> +
> + Since this was the default firmware loading facility at one point,
> + old userspace may exist which relies upon it, and as such this
> + mechanism can never be removed from the kernel.
> +
> + You should only enable this functionality if you are certain you
> + require a fallback mechanism and have a userspace mechanism ready to
> + load firmware in case it is not found. One main reason for this may
> + be if you have drivers which require firmware built-in and for
> + whatever reason cannot place the required firmware in initramfs.
> + Another reason kernels may have this feature enabled is to support a
> + driver which explicitly relies on this fallback mechanism. Only two
> + drivers need this today:
> +
> + o CONFIG_LEDS_LP55XX_COMMON
> + o CONFIG_DELL_RBU
> +
> + Outside of supporting the above drivers, another reason for needing
> + this may be that your firmware resides outside of the paths the kernel
> + looks for and cannot possibily be specified using the firmware_class
> + path module parameter or kernel firmware_class path boot parameter
> + if firmware_class is built-in.
> +
> + A modern use case may be to temporarily mount a custom partition
> + during provisioning which is only accessible to userspace, and then
> + to use it to look for and fetch the required firmware. Such type of
> + driver functionality may not even ever be desirable upstream by
> + vendors, and as such is only required to be supported as an interface
> + for provisioning. Since udev's firmware loading facility has been
> + removed you can use firmwared or a fork of it to customize how you
> + want to load firmware based on uevents issued.
> +
> + Enabling this option will increase your kernel image size by about
> + 13436 bytes.
> +
> + If you are unsure about this, say N here, unless you are Linux
> + distribution and need to support the above two drivers, or you are
> + certain you need to support some really custom firmware loading
> + facility in userspace.
>
> config FW_LOADER_USER_HELPER_FALLBACK
> - bool "Fallback user-helper invocation for firmware loading"
> - depends on FW_LOADER
> - select FW_LOADER_USER_HELPER
> + bool "Force the firmware sysfs fallback mechanism when possible"
> + depends on FW_LOADER_USER_HELPER
> help
> - This option enables / disables the invocation of user-helper
> - (e.g. udev) for loading firmware files as a fallback after the
> - direct file loading in kernel fails. The user-mode helper is
> - no longer required unless you have a special firmware file that
> - resides in a non-standard path. Moreover, the udev support has
> - been deprecated upstream.
> + Enabling this option forces a sysfs userspace fallback mechanism
> + to be used for all firmware requests which explicitly do not disable a
> + a fallback mechanism. Firmware calls which do prohibit a fallback
> + mechanism is request_firmware_direct(). This option is kept for
> + backward compatibility purposes given this precise mechanism can also
> + be enabled by setting the proc sysctl value to true:
> +
> + /proc/sys/kernel/firmware_config/force_sysfs_fallback
>
> If you are unsure about this, say N here.
>
> +endif # FW_LOADER
> +endmenu
> +
> config WANT_DEV_COREDUMP
> bool
> help
> --
> 2.17.0
>
-Kees
--
Kees Cook
Pixel Security
next prev parent reply other threads:[~2018-05-08 22:42 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-08 18:12 [PATCH v6 00/13] firmware_loader changes for v4.18 Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 01/13] firmware: wrap FW_OPT_* into an enum Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 02/13] firmware: use () to terminate kernel-doc function names Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 03/13] firmware: rename fw_sysfs_fallback to firmware_fallback_sysfs() Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 04/13] firmware_loader: document firmware_sysfs_fallback() Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 05/13] firmware_loader: enhance Kconfig documentation over FW_LOADER Luis R. Rodriguez
2018-05-08 22:42 ` Kees Cook [this message]
2018-05-09 20:55 ` Luis R. Rodriguez
2018-05-09 23:01 ` Kees Cook
2018-05-08 18:12 ` [PATCH v6 06/13] firmware_loader: move kconfig FW_LOADER entries to its own file Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 07/13] firmware_loader: make firmware_fallback_sysfs() print more useful Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 08/13] firmware: add firmware_request_nowarn() - load firmware without warnings Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 09/13] ath10k: use firmware_request_nowarn() to load firmware Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 10/13] ath10k: re-enable the firmware fallback mechanism for testmode Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 11/13] Documentation: fix few typos and clarifications for the firmware loader Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 12/13] Documentation: remove stale firmware API reference Luis R. Rodriguez
2018-05-09 15:12 ` Mauro Carvalho Chehab
2018-05-09 19:43 ` Luis R. Rodriguez
2018-05-08 18:12 ` [PATCH v6 13/13] Documentation: clarify firmware_class provenance and why we can't rename the module Luis R. Rodriguez
2018-05-08 18:53 ` Andres Rodriguez
2018-05-09 13:21 ` Mauro Carvalho Chehab
2018-05-08 22:45 ` [PATCH v6 00/13] firmware_loader changes for v4.18 Kees Cook
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=CAGXu5jJXMxx2Xna_M_g1wsS59XnYKqHAGdxhzDtk3PhnLL-gQQ@mail.gmail.com \
--to=keescook@chromium.org \
--cc=Abhay_Salunke@dell.com \
--cc=akpm@linux-foundation.org \
--cc=andresx7@gmail.com \
--cc=andy.gross@linaro.org \
--cc=arend.vanspriel@broadcom.com \
--cc=arve@android.com \
--cc=ast@fb.com \
--cc=bjorn.andersson@linaro.org \
--cc=broonie@kernel.org \
--cc=cantabile.desu@gmail.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=david.brown@linaro.org \
--cc=dhowells@redhat.com \
--cc=dmitry.torokhov@gmail.com \
--cc=dwmw2@infradead.org \
--cc=gregkh@linuxfoundation.org \
--cc=hare@suse.com \
--cc=hdegoede@redhat.com \
--cc=jejb@linux.vnet.ibm.com \
--cc=jewalt@lgsinnovations.com \
--cc=josh@joshtriplett.org \
--cc=khc@pm.waw.pl \
--cc=kubakici@wp.pl \
--cc=kvalo@codeaurora.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=maco@android.com \
--cc=markivx@codeaurora.org \
--cc=martin.petersen@oracle.com \
--cc=mcgrof@kernel.org \
--cc=mchehab+samsung@kernel.org \
--cc=mfuzzey@parkeon.com \
--cc=nbroeking@me.com \
--cc=netdev@vger.kernel.org \
--cc=oneukum@suse.com \
--cc=pali.rohar@gmail.com \
--cc=shuah@kernel.org \
--cc=teg@jklm.no \
--cc=tiwai@suse.de \
--cc=tkjos@android.com \
--cc=torvalds@linux-foundation.org \
--cc=wagi@monom.org \
--cc=zajec5@gmail.com \
--cc=zohar@linux.vnet.ibm.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;
as well as URLs for NNTP newsgroup(s).