Page MenuHomeFreeBSD

firmware: load binary firmware files
ClosedPublic

Authored by imp on Jan 23 2024, 5:24 AM.
Tags
None
Referenced Files
F103818095: D43555.id135230.diff
Fri, Nov 29, 8:30 PM
Unknown Object (File)
Thu, Nov 21, 8:58 PM
Unknown Object (File)
Tue, Nov 19, 6:58 AM
Unknown Object (File)
Tue, Nov 12, 8:14 PM
Unknown Object (File)
Tue, Nov 12, 4:53 PM
Unknown Object (File)
Wed, Nov 6, 4:28 PM
Unknown Object (File)
Wed, Nov 6, 4:26 PM
Unknown Object (File)
Wed, Nov 6, 4:25 PM
Subscribers

Details

Summary

When we can't find a .ko module to satisfy the firmware request, try
harder by looking for a file to read in directly. We compuse this file's
name by appening the imagename to the firmware path (currently
hard-wired at /boot/firmware). Allow this file to be unload when
firmware_put() releases the last reference, but we don't need to do the
indirection and dance we need to do when unloading the .ko that will
unregister the firmware.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
sys/kern/subr_firmware.c
145

FYI even for drm it's enough, most firmware are <500KiB so if you want to lower this value it's ok on my side.
EDIT: Wifi firmware seems to be 1.5MiB max (checked intel and brcm ones)

273

Why .fw ?

Make this work, generally,
But also don't change the names we try to load by adding / trimming .fw
That was a brain artifact of my trying to be safe or the link_firmware.c
path that can't really be taken.

sys/kern/subr_firmware.c
145

Maybe I'll default this to 2MB or 4MB then.

273

Stupid reasons... I'll kill it.

I had another review that did this via a linker (link_firmware.c). And for that I'd only link in .fw files as a safety measure (to make sure my linker didn't accidentally consume a .ko that's loaded). But that method fell part for other reasons... leaving just a couple of .fw floating in the ether in the version.

With the latest, kldload if_rtw88 gives me

rtw880: <rtw_8822be> port 0x3000-0x30ff mem 0x56200000-0x5620ffff at device 0.0 on pci1
rtw8822b_fw.bin: could not load /boot/firmware/rtw8822b_fw.bin either
rtw88/rtw8822b_fw.bin: Loaded using /boot/firmware/rtw88/rtw8822b_fw.bin
rtw880: successfully loaded firmware image 'rtw88/rtw8822b_fw.bin'
rtw880: Firmware version 27.2.0, H2C version 13
wlan0: Ethernet address: 28:3a:4d:45:95:9f

the 'either' is because sometimes we warn about not loading the .ko. and maybe the debugging
is too much...

sys/kern/subr_firmware.c
145

A default of 4MB look sane to me.

sys/kern/subr_firmware.c
145

-rw-r--r-- 1 bz staff 3963336 Jun 12 2023 ./ath11kfw/ath11k/QCN9074/hw1.0/amss.bin?h=20230515

Just about if this doesn't get any bigger... Maybe we should scroll through linux-firmware.git and see about file sizes... okay, we lost...

-rw-r--r-- 1 bz staff 6308684 Dec 28 20:00 ./ath11k/WCN6855/hw2.0/board-2.bin

Not sure if we still support Netronome NICs but some of their firmware is also lager ...

The largets file if I am not wrong seems to be:
-rw-r--r-- 1 bz staff 38061600 Nov 23 23:57 ./nvidia/ga102/gsp/gsp-535.113.01.bin

257

Can we please not call this "fallback" but name it by what it is?

558

If you want to do this we should keep the hierarchy, indeed.

sys/kern/subr_firmware.c
145

Let's go to 8MB then.
For nvidia iirc it's only used for nouveau which we don't (and I expect never will) support

rework to allow unloading and handle various corner cases found in testing.

double space after full stop.

imp marked 4 inline comments as done.

8m cap

imp marked 2 inline comments as done.Jan 24 2024, 7:26 PM

I claim I've addressed all the comments :).
If I've missed something, please comment again. It was unintentional.

sys/kern/subr_firmware.c
257

I'll think and cope in next pass. Maybe try_raw_file? try_direct_file?

558

I've lost the context for this comment... however, guessing I think it's addressed by the changes to not strip paths.

imp marked an inline comment as done.Jan 24 2024, 7:34 PM
imp added inline comments.
sys/kern/subr_firmware.c
309

Maybe we should register this as 'fn' with the full path too? I can make arguments both ways... Convince me one is a better choice :)

314

Maybe this should be behind bootverbose?

sys/kern/subr_firmware.c
257

I think I've settled with 'try_native_file' and use the term native in preference to 'raw' in other places.

imp retitled this revision from firmware: load raw firmware files to firmware: load native firmware files.Jan 26 2024, 4:57 AM
imp edited the summary of this revision. (Show Details)

register full filenmae rather than just imagename.

imp marked an inline comment as done.Jan 26 2024, 5:14 AM

Few comments but looks ok.
(BTW I prefered the 'raw' name compared to 'native' but don't much care too tbh :P)

sys/kern/subr_firmware.c
260

Is this still "XXX" ?

296

As a non native english speaker "longer" seems wrong and "larger" seems correct but I don't know which one is correct english :)

303

Maybe tweak this comment as we might load firmware that execute code even if they are less popular than back then in the "good old days"

314

For linuxkpi drivers we will have two printfs (the other one comes from linuxkpi firmware functions) but for native FreeBSD driver I guess we will not so it's good to see that a firmware was loaded without boot verbose I think.

This revision is now accepted and ready to land.Jan 26 2024, 5:54 PM

I'm not sure that native is exactly right either. "Raw" is relative to the "cooked" version in .ko files.
As long as we're consistent, I'd be happy with either of the names.

sys/kern/subr_firmware.c
260

The comment was cut and pasted from elsewhere in this file. That's a good question...
I also wonder if this can be called from a context where curthread is NULL since this is only called in a taskqueue context....

296

"larger" likely is more correct now that I read this.... "longer" might also be correct, but it's not as good a fit.

But "Firmware of %ld bytes is too big. Max %ld.\n" is likely even better.

303

This is about making the pages backing 'data' read-only. Currently, firmware is build with it as const, which makes it read-only so this would be a difference.

In D43555#994920, @bz wrote:

Maybe it's plain files?

Yes. I thought of "ordinary", "regular" and "normal", but so far "plain" is the one I like the most.

sys/kern/subr_firmware.c
129

loading director?

254

FEATURE REQUEST: This should probably come from kenv at some point only with this as a default here. And then it can probably be multiple places.

279

bootverbose?

309

That's probably related to the manual unload question. Driver code likely will load foo/bar.bin and want to unload foo/bar.bin and not /boot/firmware/foo/bar.bin but if kldload support was there the latter should be able to find the former but we can probably do that by trying to strip a prefix so I would suggest "no full path".

314

Do what the .ko path does. From memory -- unless there is an error things are only printed with bootverbose?
Also for the "loaded" case we put the name in '' and print size information. Would be nice to have :)

347

I would pass in flags btw. Will save us some trouble one day in the future.
Also would allow us (if not bootverb above) to say "We'll print this ourselves" for the firmware loaded to avoid double-output.

531

Do you want to change that flag to FW_NATIVE then as well?

imp retitled this revision from firmware: load native firmware files to firmware: load binary firmware files.Feb 5 2024, 3:38 AM
imp marked 6 inline comments as done.Feb 5 2024, 4:06 AM

I've updated to binary, per an IRC conversation. I've created a draft to cover the bigger issues and will work towards that. Anybody reviewing this is welcome to comment on the draft..
I'll be letting this cook while I implement the extras requested there.
But extra features will be extra commits.

sys/kern/subr_firmware.c
254

Yes. This will eventually be firmware_path, come in via the boot loader, and have a sensible set of defaults. That will be a future commit in this list.

257

After talking on IRC, and working on things. We've settled on binary here as the name.
https://hackmd.io/5pTesZVOQAKlMLFYiZjTgw
has the current draft...

260

I think this is no longer XXX.

279

ah, yes. done.

309

We use the same lookup code for both, so this works.

314

I'll do bootverbose...

347

I'll pass in the flags... If we need to rework it's easy enough.

531

I've renamed it to FW_BINARY with the shift of the name to binary.

imp marked 4 inline comments as done.

Rename to binary, plus review comments
I claim all the comments are addressed, but if I'm in error let me know

This revision now requires review to proceed.Feb 5 2024, 4:12 AM
sys/kern/subr_firmware.c
257

Well I'll add my comment here; there's some of text files to be loaded as well as firmware; mostly settings and calibrations and stuff.

sys/kern/subr_firmware.c
299

s/bug/big/

sys/kern/subr_firmware.c
257

Are those text files supposed to be read by the kernel or uploaded as a blob to the chip ?
(I supposed that you're talking about this kind of stuff: https://git.kernel.org/pub/scm/linux/kernel/git/afaerber/linux-firmware.git/tree/brcm/brcmfmac43430-sdio.AP6212.txt)

sys/kern/subr_firmware.c
257

For example, yes.

A few typos in the commit log:

  • s/compuse/compose/
  • s/appening/appending/
  • s/imagename/image name/?
  • s/hard-wired at/hard-wired to/?
  • s/be unload/be unloaded/
sys/kern/subr_firmware.c
257

cxgbe has text config files it also currently loads via firmware_get(), but I think just treating those as binary files is fine

294–295
531
This revision is now accepted and ready to land.Feb 7 2024, 11:24 PM
imp marked 4 inline comments as done.Feb 8 2024, 12:47 AM
imp added inline comments.
sys/kern/subr_firmware.c
257

I'll invoke 'binary' ftp mode. You can transfer ascii and non-ascii files with them too.

531

yea, I don't like this style... but it is our style...

This revision was automatically updated to reflect the committed changes.
imp marked an inline comment as done.