Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
Hi,
On 06-04-18 16:08, Luis R. Rodriguez 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.
Ok, so I've made a ROM dump of one of the tablets which I have with
the touchscreen firmware embedded in the EFI code using flashrom,
then I ran UEFIExtract on it, to get all the separate files.
Then I wrote a little test.sh script using hexdump piped to grep to
find the magic prefix, here is the result of running this on all
files UEFIExtract generated:
[hans@shalem chuwi-vi8-plus-cwi519-bios.bin.dump]$ find -type f -exec ./test.sh
'{}' \;
0x0c403136B0 07 00 00 E4 07 00 00
found in ./2 BIOS region/6 8C8CE578-8A3D-4F1C-9935-896185C32DD3/31 I2cHid/1
EE4E5898-3914-4259-9D6E-DC7BD79403CF/0 PE32 image section/body.bin
0x0be03040B0 07 00 00 E4 07 00 00
found in ./2 BIOS region/5 8C8CE578-8A3D-4F1C-9935-896185C32DD3/31 I2cHid/1
EE4E5898-3914-4259-9D6E-DC7BD79403CF/0 PE32 image section/body.bin
With the version found at offset 0xbe0 of the "5
8C8CE578-8A3D-4F1C-9935-896185C32DD3"
section matching what we find in the efi_boot_services_code while running.
As the I2cHid name suggests this is embedded in the driver (which is a PE
executable), not in a separate file:
[hans@shalem chuwi-vi8-plus-cwi519-bios.bin.dump]$ file './2 BIOS region/5
8C8CE578-8A3D-4F1C-9935-896185C32DD3/31 I2cHid/1
EE4E5898-3914-4259-9D6E-DC7BD79403CF/0 PE32 image section/body.bin'
./2 BIOS region/5 8C8CE578-8A3D-4F1C-9935-896185C32DD3/31 I2cHid/1
EE4E5898-3914-4259-9D6E-DC7BD79403CF/0 PE32 image section/body.bin: MS-DOS
executable
So using the EFI_FIRMWARE_VOLUME_PROTOCOL.ReadFile() is not really
going to help here, since this is not in a separate file which we
can consume in its entirety, we still need to scan for the prefix
and do e.g. a CRC check to make sure we've actually got what we
expect, at which point simply scanning all of efi_boot_services_code
is a lot simpler and less error prone.
Using an implementation specific EFI protocol for this means calling
into EFI code and potentially triggering various bugs in there, breaking
boot. This is esp. likely to happen since this is a protocol which is
not used outside of the EFI ROMs internal code so there are likely
little ABI guarantees making this approach extra error prone.
Just scanning the efi_boot_services_code OTOH is quite safe TODO.
As for the overhead of scanning the efi_boot_services_code, we are
only doing this on a dmi match, so on most machines there is no
overhead other then the dmi check. On machines where there is
a dmi match, the price (I guess about 30 ms or so for the scan)
is well worth the result of having the touchscreen OOTB.
Regard,
Hans
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On Fri, Apr 06, 2018 at 02:08:32PM +, Luis R. Rodriguez wrote: > How do you get the GUIDs for each driver BTW? They're used as filenames when extracting a Firmware Volume with UEFIExtract. E.g. let's say I'm looking for the EFI driver containing the UCS-2 string "ThunderboltDROM" in the MacBookPro9,1 Firmware Volume: $ UEFIExtract MBP91_00D3_B0C_LOCKED.scap $ grep -rl T.h.u.n.d.e.r.b.o.l.t.D.R.O.M MBP91_00D3_B0C_LOCKED.scap.dump MBP91_00D3_B0C_LOCKED.scap.dump/0 UEFI image/0 7A9354D9-0468-444A-81CE-0BF617D890DF/4 283FA2EE-532C-484D-9383-9F93B36F0B7E/0 7A9354D9-0468-444A-81CE-0BF617D890DF/0 77AD7FDB-DF2A-4302-8898-C72E4CDBD0F4/0 Compressed section/0 FC1BCDB0-7D31-49AA-936A-A4600D9DD083/0 Volume image section/0 7A9354D9-0468-444A-81CE-0BF617D890DF/97 9D1A8B60-6CB0-11DE-8E91-0002A5D5C51B/0 Compressed section/0 FC1BCDB0-7D31-49AA-936A-A4600D9DD083/0 PE32 image section/body.bin That file can then be examined with a hex editor, disassembler, etc. Since Hans knows the 8 byte signature of the file he's looking for, he'd just use: $ grep -rl '\x01\x23\x34\x56\x78\x9a\xbc\xde' ... (The \x syntax works with BSD grep shipping with macOS, but not with GNU grep. pcregrep should work, or grep -P, though the latter has been unreliable for me.) Pretty trivial obviously, the problem is how do you get the Firmware Volume? Apple ships firmware blobs for all supported machines as part of their OS updates, but if the vendor never provides updates (as Hans wrote), your only chance is to dump the ROM. On Macs the ROM is at physical address 0xffe0. The rEFIt bootloader contains a "fvdump" tool which can be used to dump the Firmware Volume at this address to the ESP: https://github.com/yep/refit/blob/master/refit/dumpfv/dumpfv.c The tool dumps only 2 MByte, but contempary Macs use considerably larger firmware blobs (8 MByte+). A quick Google search turns up these alternative addresses where the ROM may be located: https://github.com/tianocore/edk2/blob/master/OvmfPkg/README "The base address for the 1MB image in QEMU physical memory is 0xfff0. The base address for the 2MB image is 0xffe0. The base address for the 4MB image is 0xffc0." > 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. An advantage of the approach I'm suggesting is that it also catches firmware even if the EFI driver wasn't loaded. There may even be firmware for devices that aren't present, vendors ship lots of surprising stuff on Firmware Volumes. (I've found a FireWire driver on the 12" MacBook8,1 volume, even though the machine has neither a FireWire port, nor a Thunderbolt port that you could connect a FireWire adapter to. They were probably just too lazy to constrain the firmware contents to what's actually needed on a specific machine.) On Fri, Apr 06, 2018 at 10:16:44AM -0400, Peter Jones wrote: > That said, EFI_FIRMWARE_VOLUME_PROTOCOL is part of the > PI spec, not the UEFI spec. It's not required in order to implement UEFI, > and some implementations don't use it. Even when the implementation > does include PI, there's no guarantee PI protocols are exposed after the > "End of DXE" event (though edk2 will expose this, I think). Thanks for pointing that out, I wasn't aware of it. On Fri, Apr 06, 2018 at 04:28:49PM +0200, Ard Biesheuvel wrote: > > 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. Retrieval of Apple device properties is done using a custom Apple protocol, it doesn't involve the EFI_FIRMWARE_VOLUME_PROTOCOL. What I meant to say is, the EFI stub already stashes PCI ROMs and Apple device properties in setup_data payloads for consumption by the kernel proper, so extending that pattern to retrieval of device firmware (using EFI_FIRMWARE_VOLUME_PROTOCOL) seems kind of natural. Thanks, Lukas
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On 6 April 2018 at 16:14, Ard Biesheuvel wrote: > On 6 April 2018 at 16:08, Luis R. Rodriguez 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.
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
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! Okay, that's a fair point that I'd missed reading your first mail. It's worth a shot. That said, EFI_FIRMWARE_VOLUME_PROTOCOL is part of the PI spec, not the UEFI spec. It's not required in order to implement UEFI, and some implementations don't use it. Even when the implementation does include PI, there's no guarantee PI protocols are exposed after the "End of DXE" event (though edk2 will expose this, I think). > Parsing the firmware volume is only necessary to find out the GUIDs > of the files you're looking for. You only do that *once*. > > I habitually extract Apple's EFI Firmware Volumes and found the > existing tools always did a sufficiently good job to get the > information I need (such as UEFIExtract, which I've linked to, > and which is part of the UEFITool suite, which you've linked to > once more). Yeah, it's probably worth implementing your way and using it when we can. > On Wed, Apr 04, 2018 at 10:25:05PM +0200, Hans de Goede wrote: > > Lukas, thank you for your suggestions on this, but I doubt that these > > devices use the Firmware Volume stuff. > > If they're using EFI, they're using a Firmware Volume and you should > be able to use the Firmware Volume Protocol to read files from it. This is not actually the case. There is more than one implementation of UEFI, and they do not all implement PEI/DXE/etc from the PI spec. For two examples, there are still vendors that ship EDK-I implementations on some platforms, and current u-boot can be built to export a UEFI API. (It's a subset, but so is every other implementation.) > If that protocol wasn't supported, the existing EFI drivers wouldn't > be able to function as they couldn't load files from the volume. This is not correct. Not all UEFI implementations implement *any* of PI. Also, AFAIK, nothing we use in Linux so far directly depends on anything in PI. > > What you are describing sounds like significantly more work then > > the vendor just embedding the firmware as a char firmware[] in their > > EFI mouse driver. > > In such cases, read the EFI mouse driver from the firmware volume and > extract the firmware from the offset you've pre-determined. That's > never going to change since the devices never receive updates, as > you're saying. So basically you'll have a struct with GUID, offset, > length, and the name under which the firmware is made available to > Linux drivers. No, he's saying that it's really easy to implement a driver with the device firmware in a char [] in the code, so cheap ODMs who supply a UEFI driver with their hardware part are very, very likely to have done that instead of providing two things to go into the FV - a UEFI driver *and* a separate image for the device firmware. This also cuts out a point of failure between the OEM and the ODM. > > That combined with Peter's worries about difficulties parsing the > > Firmware Volume stuff, makes me believe that it is best to just > > stick with my current approach as Peter suggests. > > I don't know why you insist on a hackish solution (your own words) > even though a cleaner solution is suggested, but fortunately it's > not for me to decide what goes in and what doesn't. ;-) It already works... -- Peter
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On 6 April 2018 at 16:08, Luis R. Rodriguez 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.
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
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. Luis
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
Hi, On 03-04-18 20:47, Luis R. Rodriguez wrote: In your next patches please Cc the folks I added for future review as well. We don't have a mailing list for the firmware API so I just tend to Cc who I think should help review when needed. Hmm, quite a long Cc list, but ok. On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote: This is not part of the standard. There has been a long(ish) standing issue with us not being able to get re-distribute permission for the firmware for some touchscreen controllers found on cheap x86 devices. Which means that we cannot put it in Linux firmware. BTW do these cheap x86 devices have hw signing support? Not sure what you mean with hw signing support here, they support UEFI secure-boot and have a fTPM. Just curious thinking long term here. Because of it is not-standard then perhaps wen can partner later up with a vendor to this properly and actually support hw firmware singing. Dave Olsthoorn (in the Cc) noticed that the touchscreen did work in the refind bootload UI, so the EFI code must have a copy of the firmware. :) I asked Peter Jones for suggestions how to extract this during boot and he suggested seeing if there was a copy of the firmware in the EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is. Sneaky Pete, nice. So essentially we're reverse engineering support for this. Yes. Anyway please mention that this is not part of standard in the documentation, and we've just found out in practice some vendors are doing this. That would avoid having people ask later. Ok, I will add some docs for v2 of the patch. My patch to add support for this contains a table of device-model (dmi strings), firmware header (first 64 bits), length and crc32 and then if we boot on a device-model which is in the table the code scans the EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and caches the firmware for later use by request-firmware. Neat, best to add proper docs for this. Ok. So I just do a brute-force search for the firmware, this really is hack, nothing standard about it I'm afraid. But it works on 4 different x86 tablets I have and makes the touchscreen work OOTB on them, so I believe it is a worthwhile hack to have. Absolutely, just not to shove an entire fallback firmware path to all users. Ok. What mechanism would have in place to ensure that a driver which expects firmware to be on EFI data to be already available prior to its driver's call to initialize? See above, this still runs before start_kernel() calls rest_init() which is where any normal init calls (and driver probing) happens so still early enough for any users I can think of. The firmware API is even used to load microcode, but that relies on built-in firmware support. That code needs to be refactored to be a proper citizen of the firmware API, right now its just a hack. Reason for asking all these details was to ensure we document the restrictions correctly so that expecations are set correctly for callers prior to rest_init(). Please be sure to document the limitations. Ok. This means we rely on the EFI_BOOT_SERVICES_CODE not being free-ed until efi_free_boot_services() is called, which means that this will only work on x86, if we ever want this on ARM we should make ARM delay the freeing of the EFI_BOOT_SERVICES_* memory-segments too. Why not do that as well with your patch? That requires making significant changes to the early bringup code on ARM, x86 keeps EFI_BOOT_SERVICES_* memory-segments around until near the end of start_kernel() because freeing them earlier triggers bugs in some x86 EFI implementations, ARM EFI implementations do not have these bugs, so they free them almost directly at boot. Changing this really falls outside the scope of this patch. Sure but did you poke ARM folks about it? Maybe they can do it? And if this becomes a common practice, perhaps they can do it with actual firmware signing instead of a CRC. Not sure how hard it is to exploit EFI_BOOT_SERVICES_CODE... but it may help UEFI folks with a nice warm fuzzy to start doing this right later instead of propagating what seems to be a cheap hack. If things are compromised before the kernel boots no amount of checks are going to help us, an attacker can then just boot an entirely different kernel, rather then attacking the system through a backdoored firmware, so a CRC should be fine. Either way I'm happy to switch to a crypto hash, but the crypto subsystem is not initialized yet at this point AFAICT, so using a CRC is easier. Note this commit also modifies efi_mem_desc_lookup() to not skip EFI_BOOT_SERVICES_CODE memory-segments, so that efi_mem_reserve() works on such segments. Reported-by: Dave Olsthoorn Suggested-by: Peter Jones Signed-off-by: Hans de Goede --- drivers/base/firmware_class.c| 29 +++ drivers/firmware/efi/Makefile| 1 + drivers/firmware/efi/efi.c | 1 + dri
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
Hi,
On 03-04-18 21:53, Peter Jones wrote:
On Sat, Mar 31, 2018 at 02:19:44PM +0200, Hans de Goede wrote:
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index fddc5f706fd2..1a5ea950f58f 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -455,6 +455,7 @@ int __init efi_mem_desc_lookup(u64 phys_addr,
efi_memory_desc_t *out_md)
u64 end;
if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
+ md->type != EFI_BOOT_SERVICES_CODE &&
md->type != EFI_BOOT_SERVICES_DATA &&
md->type != EFI_RUNTIME_SERVICES_DATA) {
continue;
Might be worth adding a comment here to ensure nobody comes along later
and adds something like EFI_BOOT_LOADER_DATA or other stuff that's
allocated later here. I don't want to accidentally patch our way into
having the ability to stumble across a firmware blob somebody dumped
into the middle of a grub config file, especially since you only need to
collide crc32 (within the same length) to pre-alias a match.
As discussed elsewhere in the thread, I'm going to switch to doing a
kmemdup on the found firmware, so this chunk will go away :)
...
+static int __init efi_check_md_for_embedded_firmware(
+ efi_memory_desc_t *md, const struct embedded_fw_desc *desc)
+{
...
+ if (found_fw_count >= MAX_EMBEDDED_FIRMWARES) {
+ pr_err("Error already have %d embedded firmwares\n",
+ MAX_EMBEDDED_FIRMWARES);
+ return -ENOSPC;
+ }
Doesn't seem like this needs to be pr_err(); after all we have already
found a valid match, so the firmware vendor has done something
moderately stupid, but we have a firmware that will probably work. Of
course it still needs to return != 0, but pr_warn() or even pr_info()
seems more reasonable.
We break from the search loop as soon as a firmware is found, this can
only trigger if someone adds a second firmware to the dmi data and then
does not update MAX_EMBEDDED_FIRMWARES...
But mcgrof wants me to switch to a linked list here, so this is going
away too.
Aside from those nits, looks good to me.
Reviewed-by: Peter Jones
Thanks, but v2 is going to have so much changes that I don't feel
comfortable bringing this forward to v2.
Regards,
Hans
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
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*. I habitually extract Apple's EFI Firmware Volumes and found the existing tools always did a sufficiently good job to get the information I need (such as UEFIExtract, which I've linked to, and which is part of the UEFITool suite, which you've linked to once more). On Wed, Apr 04, 2018 at 10:25:05PM +0200, Hans de Goede wrote: > Lukas, thank you for your suggestions on this, but I doubt that these > devices use the Firmware Volume stuff. If they're using EFI, they're using a Firmware Volume and you should be able to use the Firmware Volume Protocol to read files from it. If that protocol wasn't supported, the existing EFI drivers wouldn't be able to function as they couldn't load files from the volume. > What you are describing sounds like significantly more work then > the vendor just embedding the firmware as a char firmware[] in their > EFI mouse driver. In such cases, read the EFI mouse driver from the firmware volume and extract the firmware from the offset you've pre-determined. That's never going to change since the devices never receive updates, as you're saying. So basically you'll have a struct with GUID, offset, length, and the name under which the firmware is made available to Linux drivers. > That combined with Peter's worries about difficulties parsing the > Firmware Volume stuff, makes me believe that it is best to just > stick with my current approach as Peter suggests. I don't know why you insist on a hackish solution (your own words) even though a cleaner solution is suggested, but fortunately it's not for me to decide what goes in and what doesn't. ;-) Thanks, Lukas
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On 4 April 2018 at 22:25, Hans de Goede wrote: > HI, > > > On 04-04-18 19:18, Peter Jones wrote: >> >> On Tue, Apr 03, 2018 at 06:58:48PM +, Luis R. Rodriguez wrote: >>> >>> On Tue, Apr 03, 2018 at 08:07:11PM +0200, Lukas Wunner wrote: On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote: > > I asked Peter Jones for suggestions how to extract this during boot and > he suggested seeing if there was a copy of the firmware in the > EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is. > > My patch to add support for this contains a table of device-model (dmi > strings), firmware header (first 64 bits), length and crc32 and then if > we boot on a device-model which is in the table the code scans the > EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and > caches the firmware for later use by request-firmware. > > So I just do a brute-force search for the firmware, this really is > hack, > nothing standard about it I'm afraid. But it works on 4 different x86 > tablets I have and makes the touchscreen work OOTB on them, so I > believe > it is a worthwhile hack to have. The EFI Firmware Volume contains a kind of filesystem with files identified by GUIDs. Those files include EFI drivers, ACPI tables, DMI data and so on. It is actually quite common for vendors to also include device firmware on the Firmware Volume. Apple is doing this to ship firmware updates e.g. for the GMUX controller found on dual GPU MacBook Pros. If they want to update the controller's firmware, they include it in a BIOS update, and an EFI driver checks on boot if the firmware update for the controller is necessary and if so, flashes it. The firmware files you're looking for are almost certainly included on the Firmware Volume as individual files. >>> >>> >>> What Hans implemented seems to have been for a specific x86 hack, best if >>> we >>> confirm if indeed they are present on the Firmware Volume. >> >> >> 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. >> >> [0] g...@github.com:LongSoft/UEFITool.git >> [1] g...@github.com:theopolis/uefi-firmware-parser.git >> Rather than scraping the EFI memory for firmware, I think it would be cleaner and more elegant if you just retrieve the files you're interested in from the Firmware Volume. We're doing something similar with Apple EFI properties, see 58c5475aba67 and c9cc3aaa0281. Basically what you need to do to implement this approach is: * Determine the GUIDs used by vendors for the files you're interested in. Either dump the Firmware Volume or take an EFI update as shipped by the vendor, then feed it to UEFIExtract: https://github.com/LongSoft/UEFITool * 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. * Once the kernel has booted, make the files you've retrieved available to device drivers as firmware blobs. >>> >>> >>> Happen to know if devices using Firmware Volumes also sign their firmware >>> and if hw checks the firmware at load time? >> >> >> It varies on a per-device basis, of course. Most new Intel machines as >> of Haswell *should* be verifying their system firmware via Boot Guard, >> which both checks an RSA signature and measures the firmware into the >> TPM, but as with everything of this nature, there are certainly vendors >> that screw it up. (I think AMD has something similar, but I'm really not >> sure.) > > > Lukas, thank you for your suggestions on this, but I doubt that these > devices use the Firmware Volume stuff. > Aren't Firmware Volumes a PI thing rather than a UEFI thing? > These are really cheap x86 Windows 10 tablets, everything about them is > simply hacked together by the manufacturer till it boots Windows10 and > then it is shipped to the customer without receiving any update > afterwards ever. > > What you are describing sounds like significantly more work then > the ven
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
HI, On 04-04-18 19:18, Peter Jones wrote: On Tue, Apr 03, 2018 at 06:58:48PM +, Luis R. Rodriguez wrote: On Tue, Apr 03, 2018 at 08:07:11PM +0200, Lukas Wunner wrote: On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote: I asked Peter Jones for suggestions how to extract this during boot and he suggested seeing if there was a copy of the firmware in the EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is. My patch to add support for this contains a table of device-model (dmi strings), firmware header (first 64 bits), length and crc32 and then if we boot on a device-model which is in the table the code scans the EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and caches the firmware for later use by request-firmware. So I just do a brute-force search for the firmware, this really is hack, nothing standard about it I'm afraid. But it works on 4 different x86 tablets I have and makes the touchscreen work OOTB on them, so I believe it is a worthwhile hack to have. The EFI Firmware Volume contains a kind of filesystem with files identified by GUIDs. Those files include EFI drivers, ACPI tables, DMI data and so on. It is actually quite common for vendors to also include device firmware on the Firmware Volume. Apple is doing this to ship firmware updates e.g. for the GMUX controller found on dual GPU MacBook Pros. If they want to update the controller's firmware, they include it in a BIOS update, and an EFI driver checks on boot if the firmware update for the controller is necessary and if so, flashes it. The firmware files you're looking for are almost certainly included on the Firmware Volume as individual files. What Hans implemented seems to have been for a specific x86 hack, best if we confirm if indeed they are present on the Firmware Volume. 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. [0] g...@github.com:LongSoft/UEFITool.git [1] g...@github.com:theopolis/uefi-firmware-parser.git Rather than scraping the EFI memory for firmware, I think it would be cleaner and more elegant if you just retrieve the files you're interested in from the Firmware Volume. We're doing something similar with Apple EFI properties, see 58c5475aba67 and c9cc3aaa0281. Basically what you need to do to implement this approach is: * Determine the GUIDs used by vendors for the files you're interested in. Either dump the Firmware Volume or take an EFI update as shipped by the vendor, then feed it to UEFIExtract: https://github.com/LongSoft/UEFITool * 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. * Once the kernel has booted, make the files you've retrieved available to device drivers as firmware blobs. Happen to know if devices using Firmware Volumes also sign their firmware and if hw checks the firmware at load time? It varies on a per-device basis, of course. Most new Intel machines as of Haswell *should* be verifying their system firmware via Boot Guard, which both checks an RSA signature and measures the firmware into the TPM, but as with everything of this nature, there are certainly vendors that screw it up. (I think AMD has something similar, but I'm really not sure.) Lukas, thank you for your suggestions on this, but I doubt that these devices use the Firmware Volume stuff. These are really cheap x86 Windows 10 tablets, everything about them is simply hacked together by the manufacturer till it boots Windows10 and then it is shipped to the customer without receiving any update afterwards ever. What you are describing sounds like significantly more work then the vendor just embedding the firmware as a char firmware[] in their EFI mouse driver. That combined with Peter's worries about difficulties parsing the Firmware Volume stuff, makes me believe that it is best to just stick with my current approach as Peter suggests. Regards, Hans
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On Tue, Apr 03, 2018 at 06:58:48PM +, Luis R. Rodriguez wrote: > On Tue, Apr 03, 2018 at 08:07:11PM +0200, Lukas Wunner wrote: > > On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote: > > > I asked Peter Jones for suggestions how to extract this during boot and > > > he suggested seeing if there was a copy of the firmware in the > > > EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is. > > > > > > My patch to add support for this contains a table of device-model (dmi > > > strings), firmware header (first 64 bits), length and crc32 and then if > > > we boot on a device-model which is in the table the code scans the > > > EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and > > > caches the firmware for later use by request-firmware. > > > > > > So I just do a brute-force search for the firmware, this really is hack, > > > nothing standard about it I'm afraid. But it works on 4 different x86 > > > tablets I have and makes the touchscreen work OOTB on them, so I believe > > > it is a worthwhile hack to have. > > > > The EFI Firmware Volume contains a kind of filesystem with files > > identified by GUIDs. Those files include EFI drivers, ACPI tables, > > DMI data and so on. It is actually quite common for vendors to > > also include device firmware on the Firmware Volume. Apple is doing > > this to ship firmware updates e.g. for the GMUX controller found on > > dual GPU MacBook Pros. If they want to update the controller's > > firmware, they include it in a BIOS update, and an EFI driver checks > > on boot if the firmware update for the controller is necessary and > > if so, flashes it. > > > > The firmware files you're looking for are almost certainly included > > on the Firmware Volume as individual files. > > What Hans implemented seems to have been for a specific x86 hack, best if we > confirm if indeed they are present on the Firmware Volume. 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. [0] g...@github.com:LongSoft/UEFITool.git [1] g...@github.com:theopolis/uefi-firmware-parser.git > > Rather than scraping > > the EFI memory for firmware, I think it would be cleaner and more > > elegant if you just retrieve the files you're interested in from > > the Firmware Volume. > > > > We're doing something similar with Apple EFI properties, see > > 58c5475aba67 and c9cc3aaa0281. > > > > Basically what you need to do to implement this approach is: > > > > * Determine the GUIDs used by vendors for the files you're interested > > in. Either dump the Firmware Volume or take an EFI update as > > shipped by the vendor, then feed it to UEFIExtract: > > https://github.com/LongSoft/UEFITool > > > > * 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. > > > > * Once the kernel has booted, make the files you've retrieved > > available to device drivers as firmware blobs. > > Happen to know if devices using Firmware Volumes also sign their firmware > and if hw checks the firmware at load time? It varies on a per-device basis, of course. Most new Intel machines as of Haswell *should* be verifying their system firmware via Boot Guard, which both checks an RSA signature and measures the firmware into the TPM, but as with everything of this nature, there are certainly vendors that screw it up. (I think AMD has something similar, but I'm really not sure.) -- Peter
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On Sat, Mar 31, 2018 at 02:19:44PM +0200, Hans de Goede wrote:
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index fddc5f706fd2..1a5ea950f58f 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -455,6 +455,7 @@ int __init efi_mem_desc_lookup(u64 phys_addr,
> efi_memory_desc_t *out_md)
> u64 end;
>
> if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
> + md->type != EFI_BOOT_SERVICES_CODE &&
> md->type != EFI_BOOT_SERVICES_DATA &&
> md->type != EFI_RUNTIME_SERVICES_DATA) {
> continue;
Might be worth adding a comment here to ensure nobody comes along later
and adds something like EFI_BOOT_LOADER_DATA or other stuff that's
allocated later here. I don't want to accidentally patch our way into
having the ability to stumble across a firmware blob somebody dumped
into the middle of a grub config file, especially since you only need to
collide crc32 (within the same length) to pre-alias a match.
...
> +static int __init efi_check_md_for_embedded_firmware(
> + efi_memory_desc_t *md, const struct embedded_fw_desc *desc)
> +{
...
> + if (found_fw_count >= MAX_EMBEDDED_FIRMWARES) {
> + pr_err("Error already have %d embedded firmwares\n",
> +MAX_EMBEDDED_FIRMWARES);
> + return -ENOSPC;
> + }
Doesn't seem like this needs to be pr_err(); after all we have already
found a valid match, so the firmware vendor has done something
moderately stupid, but we have a firmware that will probably work. Of
course it still needs to return != 0, but pr_warn() or even pr_info()
seems more reasonable.
Aside from those nits, looks good to me.
Reviewed-by: Peter Jones
--
Peter
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On Tue, Apr 03, 2018 at 08:07:11PM +0200, Lukas Wunner wrote: > On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote: > > I asked Peter Jones for suggestions how to extract this during boot and > > he suggested seeing if there was a copy of the firmware in the > > EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is. > > > > My patch to add support for this contains a table of device-model (dmi > > strings), firmware header (first 64 bits), length and crc32 and then if > > we boot on a device-model which is in the table the code scans the > > EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and > > caches the firmware for later use by request-firmware. > > > > So I just do a brute-force search for the firmware, this really is hack, > > nothing standard about it I'm afraid. But it works on 4 different x86 > > tablets I have and makes the touchscreen work OOTB on them, so I believe > > it is a worthwhile hack to have. > > The EFI Firmware Volume contains a kind of filesystem with files > identified by GUIDs. Those files include EFI drivers, ACPI tables, > DMI data and so on. It is actually quite common for vendors to > also include device firmware on the Firmware Volume. Apple is doing > this to ship firmware updates e.g. for the GMUX controller found on > dual GPU MacBook Pros. If they want to update the controller's > firmware, they include it in a BIOS update, and an EFI driver checks > on boot if the firmware update for the controller is necessary and > if so, flashes it. > > The firmware files you're looking for are almost certainly included > on the Firmware Volume as individual files. What Hans implemented seems to have been for a specific x86 hack, best if we confirm if indeed they are present on the Firmware Volume. > Rather than scraping > the EFI memory for firmware, I think it would be cleaner and more > elegant if you just retrieve the files you're interested in from > the Firmware Volume. > > We're doing something similar with Apple EFI properties, see > 58c5475aba67 and c9cc3aaa0281. > > Basically what you need to do to implement this approach is: > > * Determine the GUIDs used by vendors for the files you're interested > in. Either dump the Firmware Volume or take an EFI update as > shipped by the vendor, then feed it to UEFIExtract: > https://github.com/LongSoft/UEFITool > > * 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. > > * Once the kernel has booted, make the files you've retrieved > available to device drivers as firmware blobs. Happen to know if devices using Firmware Volumes also sign their firmware and if hw checks the firmware at load time? Luis > The end result is mostly the same as what you're doing here, > and you'll also have a similar array of structs, but instead > of hardcoding 8-byte signatures of firmware files, you'll be > using GUIDs. I envision lots of interesting use cases for > a generic Firmware Volume file retrieval mechanism. What you > need to keep in mind though is that this approach only works > if the kernel is booted via the EFI stub. > > Thanks, > > Lukas > -- Do not panic
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
In your next patches please Cc the folks I added for future review as well. We don't have a mailing list for the firmware API so I just tend to Cc who I think should help review when needed. On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote: > Hi Luis, > > Thank you for the review. > > On 03-04-18 01:23, Luis R. Rodriguez wrote: > > On Sat, Mar 31, 2018 at 02:19:44PM +0200, Hans de Goede wrote: > > > Just like with PCI options ROMs, which we save in the setup_efi_pci* > > > functions from arch/x86/boot/compressed/eboot.c, the EFI code / ROM itself > > > sometimes may contain data which is useful/necessary for peripheral > > > drivers > > > to have access to. > > > > > > Specifically the EFI code may contain an embedded copy of firmware which > > > needs to be (re)loaded into the peripheral. Normally such firmware would > > > be > > > part of linux-firmware, but in some cases this is not feasible, for 2 > > > reasons: > > > > > > 1) The firmware is customized for a specific use-case of the chipset / use > > > with a specific hardware model, so we cannot have a single firmware file > > > for the chipset. E.g. touchscreen controller firmwares are compiled > > > specifically for the hardware model they are used with, as they are > > > calibrated for a specific model digitizer. > > > > Some devices have OTP and use this sort of calibration data, > > Right, I'm not sure it really is OTP and not flash, but many touchscreen > controllers do come with their firmware embedded into the controller, It varies, sometimes firmware has default fluff calibration data as well which can be used if the device does not have specific calibration data. > but not all unfortunately. Indeed. > > I was unaware of > > the use of EFI to stash firmware. Good to know, but can you also provide > > references to what part of what standard should be followed for it in > > documentation? > > This is not part of the standard. There has been a long(ish) standing issue > with us not being able to get re-distribute permission for the firmware for > some touchscreen controllers found on cheap x86 devices. Which means that > we cannot put it in Linux firmware. BTW do these cheap x86 devices have hw signing support? Just curious thinking long term here. Because of it is not-standard then perhaps wen can partner later up with a vendor to this properly and actually support hw firmware singing. > Dave Olsthoorn (in the Cc) noticed that the touchscreen did work in the > refind bootload UI, so the EFI code must have a copy of the firmware. :) > I asked Peter Jones for suggestions how to extract this during boot and > he suggested seeing if there was a copy of the firmware in the > EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is. Sneaky Pete, nice. So essentially we're reverse engineering support for this. Anyway please mention that this is not part of standard in the documentation, and we've just found out in practice some vendors are doing this. That would avoid having people ask later. > My patch to add support for this contains a table of device-model (dmi > strings), firmware header (first 64 bits), length and crc32 and then if > we boot on a device-model which is in the table the code scans the > EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and > caches the firmware for later use by request-firmware. Neat, best to add proper docs for this. > So I just do a brute-force search for the firmware, this really is hack, > nothing standard about it I'm afraid. But it works on 4 different x86 > tablets I have and makes the touchscreen work OOTB on them, so I believe > it is a worthwhile hack to have. Absolutely, just not to shove an entire fallback firmware path to all users. > > > 2) Despite repeated attempts we have failed to get permission to > > > redistribute the firmware. This is especially a problem with customized > > > firmwares, these get created by the chip vendor for a specific ODM and the > > > copyright may partially belong with the ODM, so the chip vendor cannot > > > give a blanket permission to distribute these. > > > > > > This commit adds support for finding peripheral firmware embedded in the > > > EFI code and making this available to peripheral drivers through the > > > standard firmware loading mechanism. > > > > Neat. > > > > > Note we check the EFI_BOOT_SERVICES_CODE for embedded firmware pretty > > > late in the init sequence, > > > > This also creates a technical limitation on use for the API that users > > should be aware of. Its important to document such limitation. > > I don't think this is a problem for any normal drivers, when I say pretty > late I mean late in init/main.c: start_kernel(), so still before any normal > drivers load. > > The first idea was to scan for the firmware at the same time we check for > things as the ACPI BGRT logo stuff, but as mentioned that requires using > early_mmap() which does not work for the amount of memory we
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote: > I asked Peter Jones for suggestions how to extract this during boot and > he suggested seeing if there was a copy of the firmware in the > EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is. > > My patch to add support for this contains a table of device-model (dmi > strings), firmware header (first 64 bits), length and crc32 and then if > we boot on a device-model which is in the table the code scans the > EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and > caches the firmware for later use by request-firmware. > > So I just do a brute-force search for the firmware, this really is hack, > nothing standard about it I'm afraid. But it works on 4 different x86 > tablets I have and makes the touchscreen work OOTB on them, so I believe > it is a worthwhile hack to have. The EFI Firmware Volume contains a kind of filesystem with files identified by GUIDs. Those files include EFI drivers, ACPI tables, DMI data and so on. It is actually quite common for vendors to also include device firmware on the Firmware Volume. Apple is doing this to ship firmware updates e.g. for the GMUX controller found on dual GPU MacBook Pros. If they want to update the controller's firmware, they include it in a BIOS update, and an EFI driver checks on boot if the firmware update for the controller is necessary and if so, flashes it. The firmware files you're looking for are almost certainly included on the Firmware Volume as individual files. Rather than scraping the EFI memory for firmware, I think it would be cleaner and more elegant if you just retrieve the files you're interested in from the Firmware Volume. We're doing something similar with Apple EFI properties, see 58c5475aba67 and c9cc3aaa0281. Basically what you need to do to implement this approach is: * Determine the GUIDs used by vendors for the files you're interested in. Either dump the Firmware Volume or take an EFI update as shipped by the vendor, then feed it to UEFIExtract: https://github.com/LongSoft/UEFITool * 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. * Once the kernel has booted, make the files you've retrieved available to device drivers as firmware blobs. The end result is mostly the same as what you're doing here, and you'll also have a similar array of structs, but instead of hardcoding 8-byte signatures of firmware files, you'll be using GUIDs. I envision lots of interesting use cases for a generic Firmware Volume file retrieval mechanism. What you need to keep in mind though is that this approach only works if the kernel is booted via the EFI stub. Thanks, Lukas
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
Hi Luis, Thank you for the review. On 03-04-18 01:23, Luis R. Rodriguez wrote: On Sat, Mar 31, 2018 at 02:19:44PM +0200, Hans de Goede wrote: Just like with PCI options ROMs, which we save in the setup_efi_pci* functions from arch/x86/boot/compressed/eboot.c, the EFI code / ROM itself sometimes may contain data which is useful/necessary for peripheral drivers to have access to. Specifically the EFI code may contain an embedded copy of firmware which needs to be (re)loaded into the peripheral. Normally such firmware would be part of linux-firmware, but in some cases this is not feasible, for 2 reasons: 1) The firmware is customized for a specific use-case of the chipset / use with a specific hardware model, so we cannot have a single firmware file for the chipset. E.g. touchscreen controller firmwares are compiled specifically for the hardware model they are used with, as they are calibrated for a specific model digitizer. Some devices have OTP and use this sort of calibration data, Right, I'm not sure it really is OTP and not flash, but many touchscreen controllers do come with their firmware embedded into the controller, but not all unfortunately. I was unaware of the use of EFI to stash firmware. Good to know, but can you also provide references to what part of what standard should be followed for it in documentation? This is not part of the standard. There has been a long(ish) standing issue with us not being able to get re-distribute permission for the firmware for some touchscreen controllers found on cheap x86 devices. Which means that we cannot put it in Linux firmware. Dave Olsthoorn (in the Cc) noticed that the touchscreen did work in the refind bootload UI, so the EFI code must have a copy of the firmware. I asked Peter Jones for suggestions how to extract this during boot and he suggested seeing if there was a copy of the firmware in the EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is. My patch to add support for this contains a table of device-model (dmi strings), firmware header (first 64 bits), length and crc32 and then if we boot on a device-model which is in the table the code scans the EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and caches the firmware for later use by request-firmware. So I just do a brute-force search for the firmware, this really is hack, nothing standard about it I'm afraid. But it works on 4 different x86 tablets I have and makes the touchscreen work OOTB on them, so I believe it is a worthwhile hack to have. 2) Despite repeated attempts we have failed to get permission to redistribute the firmware. This is especially a problem with customized firmwares, these get created by the chip vendor for a specific ODM and the copyright may partially belong with the ODM, so the chip vendor cannot give a blanket permission to distribute these. This commit adds support for finding peripheral firmware embedded in the EFI code and making this available to peripheral drivers through the standard firmware loading mechanism. Neat. Note we check the EFI_BOOT_SERVICES_CODE for embedded firmware pretty late in the init sequence, This also creates a technical limitation on use for the API that users should be aware of. Its important to document such limitation. I don't think this is a problem for any normal drivers, when I say pretty late I mean late in init/main.c: start_kernel(), so still before any normal drivers load. The first idea was to scan for the firmware at the same time we check for things as the ACPI BGRT logo stuff, but as mentioned that requires using early_mmap() which does not work for the amount of memory we want to map. Also if we can address the limitation that would be even better. For instance, on what part of the driver is the call to request firmware being made? Note that we support async probe now, so if the call was done on probe, it may be wise to use async probe, however, can we be *certain* that the EFI firmware would have been parsed and ready by then? Please check. It just may be the case. Or, if we use late_initcall() would that suffice on the driver, if they used a request firmware call on init or probe? As said I think we still do it early enough for any driver use, when I wrote "late in the init sequence" I should have probably written something else, like "near the end of start_kernel() instead of from setup_arch()" this is on purpose because the typical EFI_BOOT_SERVICES_CODE memory-segment is too large for early_memremap(). To be clear you neede to use memremap() Yes. What mechanism would have in place to ensure that a driver which expects firmware to be on EFI data to be already available prior to its driver's call to initialize? See above, this still runs before start_kernel() calls rest_init() which is where any normal init calls (and driver probing) happens so still early enough for any users I can think of. I think my poorly worded commit message is causing a b
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On Sat, Mar 31, 2018 at 02:19:44PM +0200, Hans de Goede wrote:
> Just like with PCI options ROMs, which we save in the setup_efi_pci*
> functions from arch/x86/boot/compressed/eboot.c, the EFI code / ROM itself
> sometimes may contain data which is useful/necessary for peripheral drivers
> to have access to.
>
> Specifically the EFI code may contain an embedded copy of firmware which
> needs to be (re)loaded into the peripheral. Normally such firmware would be
> part of linux-firmware, but in some cases this is not feasible, for 2
> reasons:
>
> 1) The firmware is customized for a specific use-case of the chipset / use
> with a specific hardware model, so we cannot have a single firmware file
> for the chipset. E.g. touchscreen controller firmwares are compiled
> specifically for the hardware model they are used with, as they are
> calibrated for a specific model digitizer.
Some devices have OTP and use this sort of calibration data, I was unaware of
the use of EFI to stash firmware. Good to know, but can you also provide
references to what part of what standard should be followed for it in
documentation?
> 2) Despite repeated attempts we have failed to get permission to
> redistribute the firmware. This is especially a problem with customized
> firmwares, these get created by the chip vendor for a specific ODM and the
> copyright may partially belong with the ODM, so the chip vendor cannot
> give a blanket permission to distribute these.
>
> This commit adds support for finding peripheral firmware embedded in the
> EFI code and making this available to peripheral drivers through the
> standard firmware loading mechanism.
Neat.
> Note we check the EFI_BOOT_SERVICES_CODE for embedded firmware pretty
> late in the init sequence,
This also creates a technical limitation on use for the API that users
should be aware of. Its important to document such limitation.
Also if we can address the limitation that would be even better.
For instance, on what part of the driver is the call to request firmware
being made? Note that we support async probe now, so if the call was done
on probe, it may be wise to use async probe, however, can we be *certain*
that the EFI firmware would have been parsed and ready by then? Please
check. It just may be the case.
Or, if we use late_initcall() would that suffice on the driver, if they
used a request firmware call on init or probe?
> this is on purpose because the typical
> EFI_BOOT_SERVICES_CODE memory-segment is too large for early_memremap().
To be clear you neede to use memremap()
What mechanism would have in place to ensure that a driver which expects
firmware to be on EFI data to be already available prior to its driver's
call to initialize?
You seem to say its this consumes about about 25 MiB now, and for now you
have made this a debug thing only? How have these size requirements changed
over time? Has EFI_BOOT_SERVICES_CODE grown over time? How much? Do we
expect it will blow up later?
> This means we rely on the EFI_BOOT_SERVICES_CODE not being free-ed until
> efi_free_boot_services() is called, which means that this will only work
> on x86, if we ever want this on ARM we should make ARM delay the freeing
> of the EFI_BOOT_SERVICES_* memory-segments too.
Why not do that as well with your patch?
> Note this commit also modifies efi_mem_desc_lookup() to not skip
> EFI_BOOT_SERVICES_CODE memory-segments, so that efi_mem_reserve() works
> on such segments.
>
> Reported-by: Dave Olsthoorn
> Suggested-by: Peter Jones
> Signed-off-by: Hans de Goede
> ---
> drivers/base/firmware_class.c| 29 +++
> drivers/firmware/efi/Makefile| 1 +
> drivers/firmware/efi/efi.c | 1 +
> drivers/firmware/efi/embedded-firmware.c | 232 +++
> include/linux/efi.h | 2 +
> init/main.c | 1 +
> 6 files changed, 266 insertions(+)
> create mode 100644 drivers/firmware/efi/embedded-firmware.c
>
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 7dd36ace6152..b1e7b3de1975 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -12,6 +12,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -1207,6 +1208,32 @@ static inline void unregister_sysfs_loader(void)
>
> #endif /* CONFIG_FW_LOADER_USER_HELPER */
>
> +#ifdef CONFIG_EFI
> +static int
> +fw_get_efi_embedded_fw(struct device *dev, struct fw_priv *fw_priv, int ret)
> +{
> + size_t size;
> + int rc;
> +
> + rc = efi_get_embedded_fw(fw_priv->fw_name, &fw_priv->data, &size,
> + fw_priv->data ? fw_priv->allocated_size : 0);
> + if (rc == 0) {
> + dev_dbg(dev, "using efi-embedded fw %s\n", fw_priv->fw_name);
> + fw_priv->size = size;
> + fw_state_done(fw_priv);
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +#else
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
Hi Hans, I love your patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.16-rc7] [cannot apply to next-20180329] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Hans-de-Goede/efi-Export-boot-services-code-and-data-as-debugfs-blobs/20180401-062627 config: ia64-allnoconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 All errors (new ones prefixed by >>): drivers/firmware/efi/embedded-firmware.o: In function `efi_check_for_embedded_firmwares': >> embedded-firmware.c:(.init.text+0x202): undefined reference to `crc32_le' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
Hi Hans,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc7]
[cannot apply to next-20180329]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Hans-de-Goede/efi-Export-boot-services-code-and-data-as-debugfs-blobs/20180401-062627
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All error/warnings (new ones prefixed by >>):
drivers/firmware/efi/embedded-firmware.c: In function
'efi_check_md_for_embedded_firmware':
>> drivers/firmware/efi/embedded-firmware.c:125:8: error: implicit declaration
>> of function 'memremap'; did you mean 'memset_p'?
>> [-Werror=implicit-function-declaration]
mem = memremap(md->phys_addr, size, MEMREMAP_WB);
^~~~
memset_p
>> drivers/firmware/efi/embedded-firmware.c:125:38: error: 'MEMREMAP_WB'
>> undeclared (first use in this function)
mem = memremap(md->phys_addr, size, MEMREMAP_WB);
^~~
drivers/firmware/efi/embedded-firmware.c:125:38: note: each undeclared
identifier is reported only once for each function it appears in
>> drivers/firmware/efi/embedded-firmware.c:142:2: error: implicit declaration
>> of function 'memunmap'; did you mean 'memcmp'?
>> [-Werror=implicit-function-declaration]
memunmap(mem);
^~~~
memcmp
drivers/firmware/efi/embedded-firmware.c: In function 'efi_get_embedded_fw':
>> drivers/firmware/efi/embedded-firmware.c:222:9: error: implicit declaration
>> of function 'vmalloc'; did you mean 'd_alloc'?
>> [-Werror=implicit-function-declaration]
buf = vmalloc(fw->length);
^~~
d_alloc
>> drivers/firmware/efi/embedded-firmware.c:222:7: warning: assignment makes
>> pointer from integer without a cast [-Wint-conversion]
buf = vmalloc(fw->length);
^
cc1: some warnings being treated as errors
vim +125 drivers/firmware/efi/embedded-firmware.c
109
110 /*
111 * Note the efi_check_for_embedded_firmwares() code currently makes the
112 * following 2 assumptions. This may needs to be revisited if embedded
firmware
113 * is found where this is not true:
114 * 1) The firmware is only found in EFI_BOOT_SERVICES_CODE memory
segments
115 * 2) The firmware always starts at an offset which is a multiple of 8
bytes
116 */
117 static int __init efi_check_md_for_embedded_firmware(
118 efi_memory_desc_t *md, const struct embedded_fw_desc *desc)
119 {
120 u64 i, size;
121 u32 crc;
122 u8 *mem;
123
124 size = md->num_pages << EFI_PAGE_SHIFT;
> 125 mem = memremap(md->phys_addr, size, MEMREMAP_WB);
126 if (!mem) {
127 pr_err("Error mapping EFI mem at %#llx\n",
md->phys_addr);
128 return -ENOMEM;
129 }
130
131 size -= desc->length;
132 for (i = 0; i < size; i += 8) {
133 if (*((u64 *)(mem + i)) != *((u64 *)desc->prefix))
134 continue;
135
136 /* Seed with ~0, invert to match crc32 userspace
utility */
137 crc = ~crc32(~0, mem + i, desc->length);
138 if (crc == desc->crc)
139 break;
140 }
141
> 142 memunmap(mem);
143
144 if (i >= size)
145 return -ENOENT;
146
147 pr_info("Found EFI embedded fw '%s' crc %08x\n", desc->name,
desc->crc);
148
149 if (found_fw_count >= MAX_EMBEDDED_FIRMWARES) {
150 pr_err("Error already have %d embedded firmwares\n",
151 MAX_EMBEDDED_FIRMWARES);
152 return -ENOSPC;
153 }
154
155 found_fw[found_fw_count].data =
156 memremap(md->phys_addr + i, desc->length, MEMREMAP_WB);
157 if (!found_fw[found_fw_count].data) {
158 pr_err("Error mapping embedded firmware\n");
159 return -ENOMEM;
160 }
161
162 found_fw[found_fw_count].name = desc->name;
163 found_fw[found_fw_count].length = desc->length;
164 found_fw_count++;
165
166 /* Note md points to *unmapped* memory after this!!! */
167 efi_mem_reserve(md->phys_addr + i, desc->length);
168 return 0;
169 }
170
171 void __init efi_check_for_embedded_firmwares(void)
172 {
[PATCH 2/2] efi: Add embedded peripheral firmware support
Just like with PCI options ROMs, which we save in the setup_efi_pci*
functions from arch/x86/boot/compressed/eboot.c, the EFI code / ROM itself
sometimes may contain data which is useful/necessary for peripheral drivers
to have access to.
Specifically the EFI code may contain an embedded copy of firmware which
needs to be (re)loaded into the peripheral. Normally such firmware would be
part of linux-firmware, but in some cases this is not feasible, for 2
reasons:
1) The firmware is customized for a specific use-case of the chipset / use
with a specific hardware model, so we cannot have a single firmware file
for the chipset. E.g. touchscreen controller firmwares are compiled
specifically for the hardware model they are used with, as they are
calibrated for a specific model digitizer.
2) Despite repeated attempts we have failed to get permission to
redistribute the firmware. This is especially a problem with customized
firmwares, these get created by the chip vendor for a specific ODM and the
copyright may partially belong with the ODM, so the chip vendor cannot
give a blanket permission to distribute these.
This commit adds support for finding peripheral firmware embedded in the
EFI code and making this available to peripheral drivers through the
standard firmware loading mechanism.
Note we check the EFI_BOOT_SERVICES_CODE for embedded firmware pretty
late in the init sequence, this is on purpose because the typical
EFI_BOOT_SERVICES_CODE memory-segment is too large for early_memremap().
This means we rely on the EFI_BOOT_SERVICES_CODE not being free-ed until
efi_free_boot_services() is called, which means that this will only work
on x86, if we ever want this on ARM we should make ARM delay the freeing
of the EFI_BOOT_SERVICES_* memory-segments too.
Note this commit also modifies efi_mem_desc_lookup() to not skip
EFI_BOOT_SERVICES_CODE memory-segments, so that efi_mem_reserve() works
on such segments.
Reported-by: Dave Olsthoorn
Suggested-by: Peter Jones
Signed-off-by: Hans de Goede
---
drivers/base/firmware_class.c| 29 +++
drivers/firmware/efi/Makefile| 1 +
drivers/firmware/efi/efi.c | 1 +
drivers/firmware/efi/embedded-firmware.c | 232 +++
include/linux/efi.h | 2 +
init/main.c | 1 +
6 files changed, 266 insertions(+)
create mode 100644 drivers/firmware/efi/embedded-firmware.c
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 7dd36ace6152..b1e7b3de1975 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -12,6 +12,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1207,6 +1208,32 @@ static inline void unregister_sysfs_loader(void)
#endif /* CONFIG_FW_LOADER_USER_HELPER */
+#ifdef CONFIG_EFI
+static int
+fw_get_efi_embedded_fw(struct device *dev, struct fw_priv *fw_priv, int ret)
+{
+ size_t size;
+ int rc;
+
+ rc = efi_get_embedded_fw(fw_priv->fw_name, &fw_priv->data, &size,
+fw_priv->data ? fw_priv->allocated_size : 0);
+ if (rc == 0) {
+ dev_dbg(dev, "using efi-embedded fw %s\n", fw_priv->fw_name);
+ fw_priv->size = size;
+ fw_state_done(fw_priv);
+ ret = 0;
+ }
+
+ return ret;
+}
+#else
+static inline int
+fw_get_efi_embedded_fw(struct device *dev, struct fw_priv *fw_priv, int ret)
+{
+ return ret;
+}
+#endif
+
/* prepare firmware and firmware_buf structs;
* return 0 if a firmware is already assigned, 1 if need to load one,
* or a negative error code
@@ -1296,6 +1323,8 @@ _request_firmware(const struct firmware **firmware_p,
const char *name,
goto out;
ret = fw_get_filesystem_firmware(device, fw->priv);
+ if (ret)
+ ret = fw_get_efi_embedded_fw(device, fw->priv, ret);
if (ret) {
if (!(opt_flags & FW_OPT_NO_WARN))
dev_warn(device,
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index cb805374f4bc..cb946f7d0181 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -13,6 +13,7 @@ KASAN_SANITIZE_runtime-wrappers.o := n
obj-$(CONFIG_ACPI_BGRT)+= efi-bgrt.o
obj-$(CONFIG_EFI) += efi.o vars.o reboot.o memattr.o tpm.o
obj-$(CONFIG_EFI) += capsule.o memmap.o
+obj-$(CONFIG_EFI) += embedded-firmware.o
obj-$(CONFIG_EFI_VARS) += efivars.o
obj-$(CONFIG_EFI_ESRT) += esrt.o
obj-$(CONFIG_EFI_VARS_PSTORE) += efi-pstore.o
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index fddc5f706fd2..1a5ea950f58f 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -455,6 +455,7 @@ int __init efi_mem_desc_lookup(u64 phys_addr,
efi_memory_desc_t *out_md)
