* [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper
@ 2016-05-16 18:27 Mario Limonciello
2016-05-16 18:27 ` [PATCH v2 2/3] dell_rbu: Update documentation Mario Limonciello
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Mario Limonciello @ 2016-05-16 18:27 UTC (permalink / raw)
To: ming.lei; +Cc: LKML, Mario Limonciello
when loading firmware dell_rbu previously would allow a userspace
application to craft the payload after dell_rbu was loaded and
abuse the udev userspace API.
Instead require the payload to be crafted and placed in
/lib/firmware/dell_rbu ahead of time.
This adjusts dell_rbu to immediately load the firmware from
/lib/firmware/dell_rbu when "init" is passed into image_type using the
kernel helper.
Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
---
drivers/firmware/Kconfig | 1 -
drivers/firmware/dell_rbu.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 6664f11..85afe59 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -86,7 +86,6 @@ config DELL_RBU
tristate "BIOS update support for DELL systems via sysfs"
depends on X86
select FW_LOADER
- select FW_LOADER_USER_HELPER
help
Say m if you want to have the option of updating the BIOS for your
DELL system. Note you need a Dell OpenManage or Dell Update package (DUP)
diff --git a/drivers/firmware/dell_rbu.c b/drivers/firmware/dell_rbu.c
index 2f452f1..77b2a77 100644
--- a/drivers/firmware/dell_rbu.c
+++ b/drivers/firmware/dell_rbu.c
@@ -620,7 +620,7 @@ static ssize_t write_rbu_image_type(struct file *filp, struct kobject *kobj,
if (!rbu_data.entry_created) {
spin_unlock(&rbu_data.lock);
req_firm_rc = request_firmware_nowait(THIS_MODULE,
- FW_ACTION_NOHOTPLUG, "dell_rbu",
+ FW_ACTION_HOTPLUG, "dell_rbu",
&rbu_device->dev, GFP_KERNEL, &context,
callbackfn_rbu);
if (req_firm_rc) {
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] dell_rbu: Update documentation
2016-05-16 18:27 [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper Mario Limonciello
@ 2016-05-16 18:27 ` Mario Limonciello
2016-05-16 18:27 ` [PATCH v2 3/3] firmware_class: drop support for FW_LOADER_USER_HELPER_FALLBACK Mario Limonciello
2016-06-01 15:35 ` [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper Mario_Limonciello
2 siblings, 0 replies; 6+ messages in thread
From: Mario Limonciello @ 2016-05-16 18:27 UTC (permalink / raw)
To: ming.lei; +Cc: LKML, Mario Limonciello
Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
---
Documentation/dell_rbu.txt | 24 +++---------------------
1 file changed, 3 insertions(+), 21 deletions(-)
diff --git a/Documentation/dell_rbu.txt b/Documentation/dell_rbu.txt
index d262e22..b2714e6 100644
--- a/Documentation/dell_rbu.txt
+++ b/Documentation/dell_rbu.txt
@@ -31,8 +31,6 @@ The user should not unload the rbu driver after downloading the BIOS image
or updating.
The driver load creates the following directories under the /sys file system.
-/sys/class/firmware/dell_rbu/loading
-/sys/class/firmware/dell_rbu/data
/sys/devices/platform/dell_rbu/image_type
/sys/devices/platform/dell_rbu/data
/sys/devices/platform/dell_rbu/packet_size
@@ -60,7 +58,7 @@ added together should match the specified packet_size. This makes one
packet, the user needs to create more such packets out of the entire BIOS
image file and then arrange all these packets back to back in to one single
file.
-This file is then copied to /sys/class/firmware/dell_rbu/data.
+This file is then copied to /lib/firmware/dell_rbu.
Once this file gets to the driver, the driver extracts packet_size data from
the file and spreads it across the physical memory in contiguous packet_sized
space.
@@ -70,29 +68,13 @@ In monolithic update the user simply get the BIOS image (.hdr file) and copies
to the data file as is without any change to the BIOS image itself.
Do the steps below to download the BIOS image.
-1) echo 1 > /sys/class/firmware/dell_rbu/loading
-2) cp bios_image.hdr /sys/class/firmware/dell_rbu/data
-3) echo 0 > /sys/class/firmware/dell_rbu/loading
-
-The /sys/class/firmware/dell_rbu/ entries will remain till the following is
-done.
-echo -1 > /sys/class/firmware/dell_rbu/loading
-Until this step is completed the driver cannot be unloaded.
-Also echoing either mono, packet or init in to image_type will free up the
-memory allocated by the driver.
-
-If a user by accident executes steps 1 and 3 above without executing step 2;
-it will make the /sys/class/firmware/dell_rbu/ entries disappear.
-The entries can be recreated by doing the following
-echo init > /sys/devices/platform/dell_rbu/image_type
-NOTE: echoing init in image_type does not change it original value.
+1) Prepare BIOS image and place in /lib/firmware/dell_rbu
+2) echo "init" > /sys/devices/platform/dell_rbu/packet_type
Also the driver provides /sys/devices/platform/dell_rbu/data readonly file to
read back the image downloaded.
NOTE:
-This driver requires a patch for firmware_class.c which has the modified
-request_firmware_nowait function.
Also after updating the BIOS image a user mode application needs to execute
code which sends the BIOS update request to the BIOS. So on the next reboot
the BIOS knows about the new image downloaded and it updates itself.
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] firmware_class: drop support for FW_LOADER_USER_HELPER_FALLBACK
2016-05-16 18:27 [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper Mario Limonciello
2016-05-16 18:27 ` [PATCH v2 2/3] dell_rbu: Update documentation Mario Limonciello
@ 2016-05-16 18:27 ` Mario Limonciello
2016-06-01 15:35 ` [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper Mario_Limonciello
2 siblings, 0 replies; 6+ messages in thread
From: Mario Limonciello @ 2016-05-16 18:27 UTC (permalink / raw)
To: ming.lei; +Cc: LKML, Mario Limonciello
The last consumer of this is dell_rbu, and it no longer needs this
due to userspace changes in how updates are passed to the OS.
Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
---
drivers/base/Kconfig | 14 --------------
drivers/base/firmware_class.c | 9 ++-------
2 files changed, 2 insertions(+), 21 deletions(-)
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 98504ec..55e6ed1 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -151,20 +151,6 @@ config EXTRA_FIRMWARE_DIR
config FW_LOADER_USER_HELPER
bool
-config FW_LOADER_USER_HELPER_FALLBACK
- bool "Fallback user-helper invocation for firmware loading"
- depends on FW_LOADER
- select 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.
-
- If you are unsure about this, say N here.
-
config WANT_DEV_COREDUMP
bool
help
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 773fc30..3da4f31 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -106,11 +106,6 @@ static inline long firmware_loading_timeout(void)
#else
#define FW_OPT_USERHELPER 0
#endif
-#ifdef CONFIG_FW_LOADER_USER_HELPER_FALLBACK
-#define FW_OPT_FALLBACK FW_OPT_USERHELPER
-#else
-#define FW_OPT_FALLBACK 0
-#endif
#define FW_OPT_NO_WARN (1U << 3)
struct firmware_cache {
@@ -1185,7 +1180,7 @@ request_firmware(const struct firmware **firmware_p, const char *name,
/* Need to pin this module until return */
__module_get(THIS_MODULE);
ret = _request_firmware(firmware_p, name, device,
- FW_OPT_UEVENT | FW_OPT_FALLBACK);
+ FW_OPT_UEVENT);
module_put(THIS_MODULE);
return ret;
}
@@ -1301,7 +1296,7 @@ request_firmware_nowait(
fw_work->device = device;
fw_work->context = context;
fw_work->cont = cont;
- fw_work->opt_flags = FW_OPT_NOWAIT | FW_OPT_FALLBACK |
+ fw_work->opt_flags = FW_OPT_NOWAIT |
(uevent ? FW_OPT_UEVENT : FW_OPT_USERHELPER);
if (!try_module_get(module)) {
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper
2016-05-16 18:27 [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper Mario Limonciello
2016-05-16 18:27 ` [PATCH v2 2/3] dell_rbu: Update documentation Mario Limonciello
2016-05-16 18:27 ` [PATCH v2 3/3] firmware_class: drop support for FW_LOADER_USER_HELPER_FALLBACK Mario Limonciello
@ 2016-06-01 15:35 ` Mario_Limonciello
2016-06-02 2:24 ` Ming Lei
2 siblings, 1 reply; 6+ messages in thread
From: Mario_Limonciello @ 2016-06-01 15:35 UTC (permalink / raw)
To: ming.lei; +Cc: linux-kernel
> -----Original Message-----
> From: Limonciello, Mario
> Sent: Monday, May 16, 2016 1:28 PM
> To: ming.lei@canonical.com
> Cc: LKML <linux-kernel@vger.kernel.org>; Limonciello, Mario
> <Mario_Limonciello@Dell.com>
> Subject: [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper
>
> when loading firmware dell_rbu previously would allow a userspace
> application to craft the payload after dell_rbu was loaded and abuse the
> udev userspace API.
>
> Instead require the payload to be crafted and placed in
> /lib/firmware/dell_rbu ahead of time.
>
> This adjusts dell_rbu to immediately load the firmware from
> /lib/firmware/dell_rbu when "init" is passed into image_type using the
> kernel helper.
>
> Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
> ---
> drivers/firmware/Kconfig | 1 -
> drivers/firmware/dell_rbu.c | 2 +-
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index
> 6664f11..85afe59 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -86,7 +86,6 @@ config DELL_RBU
> tristate "BIOS update support for DELL systems via sysfs"
> depends on X86
> select FW_LOADER
> - select FW_LOADER_USER_HELPER
> help
> Say m if you want to have the option of updating the BIOS for your
> DELL system. Note you need a Dell OpenManage or Dell Update
> package (DUP) diff --git a/drivers/firmware/dell_rbu.c
> b/drivers/firmware/dell_rbu.c index 2f452f1..77b2a77 100644
> --- a/drivers/firmware/dell_rbu.c
> +++ b/drivers/firmware/dell_rbu.c
> @@ -620,7 +620,7 @@ static ssize_t write_rbu_image_type(struct file *filp,
> struct kobject *kobj,
> if (!rbu_data.entry_created) {
> spin_unlock(&rbu_data.lock);
> req_firm_rc =
> request_firmware_nowait(THIS_MODULE,
> - FW_ACTION_NOHOTPLUG, "dell_rbu",
> + FW_ACTION_HOTPLUG, "dell_rbu",
> &rbu_device->dev, GFP_KERNEL, &context,
> callbackfn_rbu);
> if (req_firm_rc) {
> --
> 2.7.4
Hi Ming,
Could you comment on these patches? There isn't currently a subsystem maintainer for dell_rbu, so I think you are the most qualified to pull these in. They have the best implications for you since you can drop all that fallback code now too.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper
2016-06-01 15:35 ` [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper Mario_Limonciello
@ 2016-06-02 2:24 ` Ming Lei
2016-06-02 2:33 ` Mario_Limonciello
0 siblings, 1 reply; 6+ messages in thread
From: Ming Lei @ 2016-06-02 2:24 UTC (permalink / raw)
To: Mario Limonciello; +Cc: Linux Kernel Mailing List
On Wed, Jun 1, 2016 at 11:35 PM, <Mario_Limonciello@dell.com> wrote:
>> -----Original Message-----
>> From: Limonciello, Mario
>> Sent: Monday, May 16, 2016 1:28 PM
>> To: ming.lei@canonical.com
>> Cc: LKML <linux-kernel@vger.kernel.org>; Limonciello, Mario
>> <Mario_Limonciello@Dell.com>
>> Subject: [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper
>>
>> when loading firmware dell_rbu previously would allow a userspace
>> application to craft the payload after dell_rbu was loaded and abuse the
>> udev userspace API.
>>
>> Instead require the payload to be crafted and placed in
>> /lib/firmware/dell_rbu ahead of time.
>>
>> This adjusts dell_rbu to immediately load the firmware from
>> /lib/firmware/dell_rbu when "init" is passed into image_type using the
>> kernel helper.
>>
>> Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
>> ---
>> drivers/firmware/Kconfig | 1 -
>> drivers/firmware/dell_rbu.c | 2 +-
>> 2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index
>> 6664f11..85afe59 100644
>> --- a/drivers/firmware/Kconfig
>> +++ b/drivers/firmware/Kconfig
>> @@ -86,7 +86,6 @@ config DELL_RBU
>> tristate "BIOS update support for DELL systems via sysfs"
>> depends on X86
>> select FW_LOADER
>> - select FW_LOADER_USER_HELPER
>> help
>> Say m if you want to have the option of updating the BIOS for your
>> DELL system. Note you need a Dell OpenManage or Dell Update
>> package (DUP) diff --git a/drivers/firmware/dell_rbu.c
>> b/drivers/firmware/dell_rbu.c index 2f452f1..77b2a77 100644
>> --- a/drivers/firmware/dell_rbu.c
>> +++ b/drivers/firmware/dell_rbu.c
>> @@ -620,7 +620,7 @@ static ssize_t write_rbu_image_type(struct file *filp,
>> struct kobject *kobj,
>> if (!rbu_data.entry_created) {
>> spin_unlock(&rbu_data.lock);
>> req_firm_rc =
>> request_firmware_nowait(THIS_MODULE,
>> - FW_ACTION_NOHOTPLUG, "dell_rbu",
>> + FW_ACTION_HOTPLUG, "dell_rbu",
>> &rbu_device->dev, GFP_KERNEL, &context,
>> callbackfn_rbu);
>> if (req_firm_rc) {
>> --
>> 2.7.4
>
> Hi Ming,
>
> Could you comment on these patches? There isn't currently a subsystem maintainer for dell_rbu, so I think you are the most qualified to pull these in. They have the best implications for you since you can drop all that fallback code now too.
Hi Mario,
I am fine for the 1st two patches.
But IMO it isn't good to drop support for
FW_LOADER_USER_HELPER_FALLBACK inside kernel now because udev
isn't used everywhere, and not every distribution always put the firmware files
under the default builtin path.
Thanks,
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper
2016-06-02 2:24 ` Ming Lei
@ 2016-06-02 2:33 ` Mario_Limonciello
0 siblings, 0 replies; 6+ messages in thread
From: Mario_Limonciello @ 2016-06-02 2:33 UTC (permalink / raw)
To: ming.lei; +Cc: linux-kernel
> -----Original Message-----
> From: Ming Lei [mailto:ming.lei@canonical.com]
> Sent: Wednesday, June 1, 2016 9:24 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper
>
> On Wed, Jun 1, 2016 at 11:35 PM, <Mario_Limonciello@dell.com> wrote:
> >> -----Original Message-----
> >> From: Limonciello, Mario
> >> Sent: Monday, May 16, 2016 1:28 PM
> >> To: ming.lei@canonical.com
> >> Cc: LKML <linux-kernel@vger.kernel.org>; Limonciello, Mario
> >> <Mario_Limonciello@Dell.com>
> >> Subject: [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper
> >>
> >> when loading firmware dell_rbu previously would allow a userspace
> >> application to craft the payload after dell_rbu was loaded and abuse
> >> the udev userspace API.
> >>
> >> Instead require the payload to be crafted and placed in
> >> /lib/firmware/dell_rbu ahead of time.
> >>
> >> This adjusts dell_rbu to immediately load the firmware from
> >> /lib/firmware/dell_rbu when "init" is passed into image_type using
> >> the kernel helper.
> >>
> >> Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
> >> ---
> >> drivers/firmware/Kconfig | 1 -
> >> drivers/firmware/dell_rbu.c | 2 +-
> >> 2 files changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> >> index
> >> 6664f11..85afe59 100644
> >> --- a/drivers/firmware/Kconfig
> >> +++ b/drivers/firmware/Kconfig
> >> @@ -86,7 +86,6 @@ config DELL_RBU
> >> tristate "BIOS update support for DELL systems via sysfs"
> >> depends on X86
> >> select FW_LOADER
> >> - select FW_LOADER_USER_HELPER
> >> help
> >> Say m if you want to have the option of updating the BIOS for your
> >> DELL system. Note you need a Dell OpenManage or Dell Update
> >> package (DUP) diff --git a/drivers/firmware/dell_rbu.c
> >> b/drivers/firmware/dell_rbu.c index 2f452f1..77b2a77 100644
> >> --- a/drivers/firmware/dell_rbu.c
> >> +++ b/drivers/firmware/dell_rbu.c
> >> @@ -620,7 +620,7 @@ static ssize_t write_rbu_image_type(struct file
> >> *filp, struct kobject *kobj,
> >> if (!rbu_data.entry_created) {
> >> spin_unlock(&rbu_data.lock);
> >> req_firm_rc =
> >> request_firmware_nowait(THIS_MODULE,
> >> - FW_ACTION_NOHOTPLUG, "dell_rbu",
> >> + FW_ACTION_HOTPLUG, "dell_rbu",
> >> &rbu_device->dev, GFP_KERNEL, &context,
> >> callbackfn_rbu);
> >> if (req_firm_rc) {
> >> --
> >> 2.7.4
> >
> > Hi Ming,
> >
> > Could you comment on these patches? There isn't currently a subsystem
> maintainer for dell_rbu, so I think you are the most qualified to pull these in.
> They have the best implications for you since you can drop all that fallback code
> now too.
>
> Hi Mario,
>
> I am fine for the 1st two patches.
>
> But IMO it isn't good to drop support for FW_LOADER_USER_HELPER_FALLBACK
> inside kernel now because udev isn't used everywhere, and not every distribution
> always put the firmware files under the default builtin path.
>
> Thanks,
Ming,
OK that's fine to me.
I just thought it would simplify your code, didn't realize anything there were distros still putting firmware in odd places or not using udev.
If you could include the first two patches in your tree for next kernel I would appreciate.
Thanks,
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-02 2:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-16 18:27 [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper Mario Limonciello
2016-05-16 18:27 ` [PATCH v2 2/3] dell_rbu: Update documentation Mario Limonciello
2016-05-16 18:27 ` [PATCH v2 3/3] firmware_class: drop support for FW_LOADER_USER_HELPER_FALLBACK Mario Limonciello
2016-06-01 15:35 ` [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper Mario_Limonciello
2016-06-02 2:24 ` Ming Lei
2016-06-02 2:33 ` Mario_Limonciello
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).