Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20180406140832.GF30543@wotan.suse.de>
References: <78ae4d18-8964-5748-a69f-0017d0dca5f7@redhat.com>
 <20180404171835.xvllgcqirl3b5gd5@redhat.com> <20180405054349.GA25653@wunner.de>
 <20180406140832.GF30543@wotan.suse.de>
From:   Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Fri, 6 Apr 2018 16:14:03 +0200
Message-ID: <CAKv+Gu8R2YfjWjSQkyvLv78C-ixFqYDaZ7qwNje53_S9yTJ83g@mail.gmail.com>
Subject: Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
To:     "Luis R. Rodriguez" <mcgrof@kernel.org>
Cc:     Lukas Wunner <lukas@wunner.de>, Peter Jones <pjones@redhat.com>,
        Hans de Goede <hdegoede@redhat.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Kalle Valo <kvalo@codeaurora.org>,
        Arend Van Spriel <arend.vanspriel@broadcom.com>,
        Ingo Molnar <mingo@redhat.com>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Dave Olsthoorn <dave@bewaar.me>,
        "the arch/x86 maintainers" <x86@kernel.org>,
        linux-efi@vger.kernel.org, Will Deacon <will.deacon@arm.com>,
        Andy Lutomirski <luto@kernel.org>,
        Matt Fleming <matt@codeblueprint.co.uk>,
        David Howells <dhowells@redhat.com>,
        Mimi Zohar <zohar@linux.vnet.ibm.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Matthew Garrett <mjg59@srcf.ucam.org>,
        One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Martin Fuzzey <mfuzzey@parkeon.com>,
        Kees Cook <keescook@chromium.org>,
        Nicolas Broeking <nbroeking@me.com>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Torsten Duwe <duwe@suse.de>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 6 April 2018 at 16:08, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Thu, Apr 05, 2018 at 07:43:49AM +0200, Lukas Wunner wrote:
>> On Wed, Apr 04, 2018 at 01:18:36PM -0400, Peter Jones wrote:
>> > > On Tue, Apr 03, 2018 at 08:07:11PM +0200, Lukas Wunner wrote:
>> > > > * Add the EFI Firmware Volume Protocol to include/linux/efi.h:
>> > > >   https://www.intel.com/content/dam/doc/reference-guide/efi-firmware-file-volume-specification.pdf
>> > > >
>> > > > * Amend arch/x86/boot/compressed/eboot.c to read the files with the
>> > > >   GUIDs you're interested in into memory and pass the files to the
>> > > >   kernel as setup_data payloads.
>> >
>> > To be honest, I'm a bit skeptical about the firmware volume approach.
>> > Tools like UEFITool[0] and uefi-firmware-parser[1] have existed for
>> > years, still don't seem to reliably parse firmware images I see in the
>> > wild, and have a fairly regular need for fixes.  These are tools
>> > maintained by smart people who are making a real effort, and it still
>> > looks pretty hard to do a good job that applies across a lot of
>> > platforms.
>> >
>> > So I'd rather use Hans's existing patches, at least for now, and if
>> > someone is interested in hacking on making an efi firmware volume parser
>> > for the kernel, switch them to that when such a thing is ready.
>>
>> Hello?  As I've written in the above-quoted e-mail the kernel should
>> read the files using EFI_FIRMWARE_VOLUME_PROTOCOL.ReadFile().
>>
>> *Not* by parsing the firmware volume!
>>
>> Parsing the firmware volume is only necessary to find out the GUIDs
>> of the files you're looking for.  You only do that *once*.
>
> How do you get the GUIDs for each driver BTW?
>
> Hans, I do believe we should *try* this approach at the very least.
>
> Why not?
>
> Otherwise it would be wise to provide a technical reason for why
> we'd choose one custom mechanism which would only serve a few tablets
> over a mechanism which could serve more devices.
>

Because EFI_FIRMWARE_VOLUME_PROTOCOL is not part of the UEFI spec but
of the PI spec, and so we will be adding dependencies on
implementation details of the firmware. I am aware we may already have
done so for the Apple properties support, but I think it makes sense
to make an exception there, given that Mac UEFI firmware is 'special'
already.