Date: Wed, 29 Apr 2015 17:54:29 -0300
From: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
To: Borislav Petkov <bp@alien8.de>
Cc: Daniel J Blueman <daniel@numascale.com>,
        Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Quentin Casasnovas <quentin.casasnovas@oracle.com>,
        Steffen Persvold <sp@numascale.com>, linux-kernel@vger.kernel.org,
        x86@kernel.org
Subject: Re: [PATCH] x86: Unbreak early processor microcode loading
Message-ID: <20150429205428.GC14218@khazad-dum.debian.net>
References: <20150318184249.GC3249@pd.tnic>
 <1426750223.3578.2@cpanel21.proisp.no>
 <20150319092703.GA10860@pd.tnic>
 <20150429182357.GB14218@khazad-dum.debian.net>
 <20150429184338.GF5498@pd.tnic>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150429184338.GF5498@pd.tnic>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4833
Lines: 100

On Wed, 29 Apr 2015, Borislav Petkov wrote:
> On Wed, Apr 29, 2015 at 03:23:57PM -0300, Henrique de Moraes Holschuh wrote:
> > That would obviously work, but people often have other firmware
> > built-in, so the likehood of CONFIG_EXTRA_FIRMWARE_DIR pointing to the
> > root of a linux-firmware work tree or to "/lib/firmware" is not low at
> > all.
> > 
> > In fact, it is natural to expect that CONFIG_EXTRA_FIRMWARE should point
> > to something that will result in the same filenames as the kernel would
> > want to use for regular firmware loading.  The current text of the
> > Kconfig help for CONFIG_EXTRA_FIRMWARE even says so.
> 
> You mean that:
> 
> 	  "These files should exist under
>           the directory specified by the EXTRA_FIRMWARE_DIR option, which is
>           by default the firmware subdirectory of the kernel source tree.
> 
>           For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
>           the usb8388.bin file into the firmware directory, and build the kernel.
>           Then any request_firmware("usb8388.bin") will be satisfied internally
>           without needing to call out to userspace."
> 
> So people with lotsa firmware should put it all under one directory and
> all should just work.

That's one possible reading, yes.  Only, it will break for some drivers.

The full Kconfig text of the two most-relevant kconfig defines are:

config EXTRA_FIRMWARE
        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 option is a string and takes the (space-separated) names of
        the firmware files -- the same names that appear in
        MODULE_FIRMWARE() and request_firmware() in the source. These
        files should exist under the directory specified by the
        EXTRA_FIRMWARE_DIR option, which is by default the firmware
        subdirectory of the kernel source tree.

        For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin",
        copy the usb8388.bin file into the firmware directory, and build
        the kernel.  Then any request_firmware("usb8388.bin") will be
        satisfied internally without needing to call out to userspace.

        WARNING: If you include additional firmware files into your
        binary kernel image that are not available under the terms of
        the GPL, then it may be a violation of the GPL to distribute the
        resulting image since it combines both GPL and non-GPL work. You
        should consult a lawyer of your own before distributing such an
        image.

config EXTRA_FIRMWARE_DIR
        This option controls the directory in which the kernel build
        system looks for the firmware files listed in the EXTRA_FIRMWARE
        option.  The default is firmware/ in the kernel source tree, but
        by changing this option you can point it elsewhere, such as
        /lib/firmware/ or some other directory containing the firmware
        files.

Note the explicit mention of /lib/firmware as a possibility for
CONFIG_EXTRA_FIRMWARE_DIR, as well as the second paragraph in the
CONFIG_EXTRA_FIRMWARE help text.

This help text needs some love...

Things only work for the drivers that do something like
MODULE_FIRMWARE("foo/bar.fw") when you consider that by "names of the
firmware files" the help text actually means "relative path to the
firmware files".

For the drivers do MODULE_FIRMWARE("foo/bar.fw"), should you place the
firmware file directly in the root of EXTRA_FIRMWARE_DIR (built-in case)
or in the root of the firmware search path for the non-builtin case
(i.e., if drop the "foo/"), it is not going to be found by
request_firmware().

That's why IMHO the least-suprise path would be for builtin microcode to
be searched with the same relative path as it would for late loading.
Because that's how regular drivers that do request_firmware() behave.

FWIW, my preference would be for the early microcode core to look for
builtin firmware using the same names used for the early initramfs
(including the kernel/x86/microcode/ prefix), since it is doing an early
update.  But that would certainly require documentation.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/