Messages in this thread | | | From | "Luis R. Rodriguez" <> | Subject | [PATCH v2 0/8] firmware: add new sysdata API | Date | Thu, 16 Jun 2016 16:59:11 -0700 |
| |
I started working on extending the firmware API to in the long run help add firmware signing, that was the orignal purpose of the sysdata API, now it also provides a good outlet for us for how to compartamentalize the old usermode helper, since we cannot remove it.
The core stuff that we wished to share is now mergd, which consisted of adding a common core file loader. Mimi merged that work, so now its time to respin this series. I decided to tackle adding a test driver as well, which lets you muck at will with the API, even letting you run things in parallel, so a lot of test can be written now in userspace.
Perhaps the biggest change since last iteration is is both the use of async_schedule*(), a test driver, and of course a large set of SmPL grammar to help users convert if they so wish. Since we now have grammar to help hunt down explicit usermode helper users [0] and annotations to whitelist these callers and we've deteremined that we only have TWO drivers still left explictly calling out for the usermode helper we may want to consider if we can just sweep out the usermode helper underneath all other calls, however that does a huge disservice to any Linux kernel built with CONFIG_FW_LOADER_USER_HELPER_FALLBACK. Most distributions disable this now though.
Converting drivers one by one is a large task, I don't recommend it. I'd like to instead recommend we convert over users over that we know we can see a benefit for and for which we know won't break old userspace. One can know if one will not break userspace if one is certain old userspace does not exist that requires a usermode helper for the driver. One example of a benefit of using the sysdata API is for instance -- letting the sysdata API deal with free'ing your sysdata; or if you're adding a new driver, you not wanting to add your own completion / wait stuff.
The use of the old firmware API API varies, there is a good split between drivers that need to keep the firmware and drivers that just request it for an immediate use. For the driver that need to keep the firmware (in sysdata lingo this is the ones that use the descriptors with SYSDATA_KEEP_SYNC() or SYSDATA_KEEP_ASYNC()) I'd recommend instead we seriously consider extending the sysdata API with using devm wrappers so that free'ing can also be skipped there. Another example future extension to the API is daisy-chained requests, there's a few drivers that do this, and having a simple API that manages this would provide a a huge cleanup and probably fix quite a bit of odd bugs.
This series depends on 2 other series, the coccicheck enhancements [2] and the firmware SmPL grammar extensions [3]. If you wish you can get the changes from my linux-next tree as well [4]. Please do note that all these series are based on linux-next tag next-20160616, and I noticed that Andrew Morton had picked up a patch by Stephen Boyd and Vikram Mulukutla to add yet-another new old firmware API. This then applies on top of that as its merged on linux-next, however my recommendation would be to revert that patch and re-write the patch under the sysdata API to help take advantage of a simple API and stop creating new symbols for minor tweaks.
The SmPL transformation patches are provided as-is, but help is obviously welcomed to enhance them, its why they are not in scripts/coccinelle/ -- what I recommend is to leave it as is as it deals with the cases that use the firmware locally and therefore do not require to keep it (SYSDATA_KEEP_SYNC() or SYSDATA_KEEP_ASYNC()), we add devm support later, and then if user want to help clean up their driver they can then use the form that does not require releaesing the firmware at all on their end. The way we envision you'd use the transformation patches is you'd use them against each driver one at a time, not on the entire kernel, however using it against the entire kernel is also possible but note that currently the async cookies are not added for you when that is done.
FWIW running it against linux-next next-20160616 on a 32-core system takes about 3 minutes using glimpse, also ~3 minutes with gitgrep and produces the following diffstat:
148 files changed, 5676 insertions(+), 4504 deletions(-)
A lot of this is because the sync case can now deal with freeing your firmware for you, as such a callback is needed. Running it on my 4-core laptop using gitgrep (default if you change my coccicheck series if you're on git) takes ~11 minutes on the entire kernel (not recommended), and about ~4 seconds against a random 802.11 driver (drivers/net/wireless/ti/wl1251/). If you'd like to be very careful with the transformations you could merge an series of avoids declarations, by reverting a patch I dropped [5], we discareded this given that we expect driver developers only to use this to later manually inspect the code changes by hand carefully. Feedback on the results of the transformations is welcomed and appreciated.
The two drivers I picked to transform here were simply random to help demo the transformations.
[0] https://lkml.kernel.org/r/1466117661-22075-4-git-send-email-mcgrof@kernel.org [1] https://lkml.kernel.org/r/1466117661-22075-5-git-send-email-mcgrof@kernel.org [2] https://lkml.kernel.org/r/1466116292-21843-1-git-send-email-mcgrof@kernel.org [3] https://lkml.kernel.org/r/1466117661-22075-1-git-send-email-mcgrof@kernel.org [4] https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20160616-sysdata-v2 [5] http://drvbp1.linux-foundation.org/~mcgrof/2016/06/16/remove-avoids.patch
Luis R. Rodriguez (8): firmware: add new extensible firmware API - sysdata_file_request*() lib/test_firmware.c: use late_initcall() selftests: firmware: only modprobe if driver is missing selftests: firmware: send expected errors to /dev/null test: add new sysdata_file_request*() loader tester Documentation/firmware_class: add sysdata API converter SmPL patch x86/microcode: convert to use sysdata API p54: convert to sysdata API
.../firmware_class/0001-convert-sysdata-sync.cocci | 1154 ++++++++++++++++++++ .../0002-convert-sysdata-async.cocci | 259 +++++ .../0003-convert-sysdata-generic.cocci | 43 + Documentation/firmware_class/convert-sysdata.sh | 13 + Documentation/firmware_class/system_data.txt | 138 +++ MAINTAINERS | 3 +- arch/x86/kernel/cpu/microcode/amd.c | 56 +- drivers/base/firmware_class.c | 327 ++++++ drivers/net/wireless/intersil/p54/eeprom.c | 2 +- drivers/net/wireless/intersil/p54/fwio.c | 5 +- drivers/net/wireless/intersil/p54/led.c | 2 +- drivers/net/wireless/intersil/p54/main.c | 2 +- drivers/net/wireless/intersil/p54/p54.h | 3 +- drivers/net/wireless/intersil/p54/p54pci.c | 26 +- drivers/net/wireless/intersil/p54/p54pci.h | 4 +- drivers/net/wireless/intersil/p54/p54spi.c | 81 +- drivers/net/wireless/intersil/p54/p54spi.h | 2 +- drivers/net/wireless/intersil/p54/p54usb.c | 18 +- drivers/net/wireless/intersil/p54/p54usb.h | 4 +- drivers/net/wireless/intersil/p54/txrx.c | 2 +- include/linux/sysdata.h | 244 +++++ lib/Kconfig.debug | 12 + lib/Makefile | 1 + lib/test_firmware.c | 2 +- lib/test_sysdata.c | 1046 ++++++++++++++++++ tools/testing/selftests/firmware/fw_filesystem.sh | 17 +- tools/testing/selftests/firmware/sysdata.sh | 633 +++++++++++ 27 files changed, 4013 insertions(+), 86 deletions(-) create mode 100644 Documentation/firmware_class/0001-convert-sysdata-sync.cocci create mode 100644 Documentation/firmware_class/0002-convert-sysdata-async.cocci create mode 100644 Documentation/firmware_class/0003-convert-sysdata-generic.cocci create mode 100755 Documentation/firmware_class/convert-sysdata.sh create mode 100644 Documentation/firmware_class/system_data.txt create mode 100644 include/linux/sysdata.h create mode 100644 lib/test_sysdata.c create mode 100755 tools/testing/selftests/firmware/sysdata.sh
-- 2.8.2
| |