Re: dell_rbu: Select CONFIG_FW_LOADER_USER_HELPER explicitly

[Ming Lei]

On Mon, Jul 8, 2013 at 5:05 PM, Takashi Iwai <tiwai@xxxxxxx> wrote:
> At Sat, 6 Jul 2013 15:30:03 -0700,
> Greg KH wrote:
>>
>> On Sat, Jul 06, 2013 at 06:14:01PM -0400, Dave Jones wrote:
>> > On Tue, Jul 02, 2013 at 07:27:49PM +0000, Linux Kernel wrote:
>> >  > Gitweb:     http://git.kernel.org/linus/;a=commit;h=d05c39ea67f5786a549ac9d672d2951992b658c6
>> >  > Commit:     d05c39ea67f5786a549ac9d672d2951992b658c6
>> >  > Parent:     a3b2c8c7aa1ca860edcf0b0afa371d9eb2269c3c
>> >  > Author:     Takashi Iwai <tiwai@xxxxxxx>
>> >  > AuthorDate: Wed May 22 18:28:37 2013 +0200
>> >  > Committer:  Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> >  > CommitDate: Mon Jun 3 13:57:29 2013 -0700
>> >  >
>> >  >     dell_rbu: Select CONFIG_FW_LOADER_USER_HELPER explicitly
>> >  >
>> >  >     The usermode helper is mandatory for this driver.
>> >  >
>> >  >     Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
>> >  >     Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> >  > ---
>> >  >  drivers/firmware/Kconfig |    1 +
>> >  >  1 files changed, 1 insertions(+), 0 deletions(-)
>> >  >
>> >  > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
>> >  > index 9387630..0747872 100644
>> >  > --- a/drivers/firmware/Kconfig
>> >  > +++ b/drivers/firmware/Kconfig
>> >  > @@ -64,6 +64,7 @@ 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)
>> >
>> >
>> > This is pretty crappy. Now every distro kernel has to have that enabled,
>> > which screws up for eg, the x86 microcode driver. (It takes 1 minute per cpu
>> > when this is enabled).
>>
>> I'll let you and Takashi battle this one out, for some reason I thought
>> he added it _because_ a distro kernel needed it...
>
> The reason is that dell_rbu driver requires it.  Without the kconfig
> option, this driver won't work at all.  So, it's a right fix for
> dell_rbu.
>
> AFAIK, the consensus in the kernel side is that this too long fw
> loading time is basically a regression of user-space (udev or
> whatever).  There is no change in the kernel behavior.  The problem
> must exist even with the older kernels.
>
> But, looking at the development, we can't expect that udev will be
> fixed soon, and this breakage persists already way too long.  Maybe a
> better solution is to kill the fallback to udev for normal f/w loading
> (i.e. for distro kernels).
>
> The patch below is an untested quick hack.  It adds a new Kconfig and
> a new function request_firmware_via_user_helper().  Distro kernels may
> set CONFIG_FW_LOADER_USER_HELPER_FALLBACK=n for avoiding 60 seconds

I understand your proposed approach is basically equal to unset DELL_RBU,
don't I?

Actually if DELL_RBU is set, FW_LOADER_USER_HELPER is still set, and
it won't avoid the fallback to loading from userspace.

> stall for non-existing firmware file access -- as distributions know
> that the firmware files should be placed in the right path.
>
> Thoughts?

I suggest not to introduce new config options in firmware loader any more,
and the current options are too many already and it is very easy to trigger
build problem, and introduce extra complexity to users at the same time.

About Dave's problem, I think distribution may not trigger it since
cpu microcode
should exist, so I am wondering if Dave can disable DELL_RBU to work around
the problem in his system? Or fake a one byte microcode file to fool the driver?

Also looks the problem should be handled inside x86 microcode driver because
the usage is unique in x86 microcode driver. Other drivers either need one
firmware or stop to work without a firmware, but this driver can work
well no matter
the firmware loading is satisfied or not.

Or could we force dell rbu to use uevent based loading now?

thanks
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/