Re: [PATCH 2/2] efi: Add embedded peripheral firmware support

[Ard Biesheuvel]

On 6 April 2018 at 16:14, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:
> On 6 April 2018 at 16:08, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> 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

... or maybe not: I thought Lukas alluded to that in this thread, but
I can't actually find any traces of that in the code so I must have
misunderstood.

, but I think it makes sense
> to make an exception there, given that Mac UEFI firmware is 'special'
> already.