Message-Id: <1466121559-22363-1-git-send-email-mcgrof@kernel.org>
Date:	Thu, 16 Jun 2016 16:59:11 -0700
From:	"Luis R. Rodriguez" <mcgrof@...nel.org>
To:	ming.lei@...onical.com, akpm@...ux-foundation.org, mmarek@...e.com,
	gregkh@...uxfoundation.org, bp@...en8.de, chunkeey@...glemail.com
Cc:	linux-kernel@...r.kernel.org, markivx@...eaurora.org,
	stephen.boyd@...aro.org, zohar@...ux.vnet.ibm.com,
	broonie@...nel.org, tiwai@...e.de, johannes@...solutions.net,
	hauke@...ke-m.de, jwboyer@...oraproject.org,
	dmitry.torokhov@...il.com, dwmw2@...radead.org, jslaby@...e.com,
	torvalds@...ux-foundation.org, luto@...capital.net,
	fengguang.wu@...el.com, rpurdie@...ys.net, ki@...sung.com,
	Abhay_Salunke@...l.com, Julia.Lawall@...6.fr,
	Gilles.Muller@...6.fr, nicolas.palix@...g.fr, teg@...m.no,
	dhowells@...hat.com, keescook@...omium.org, tj@...nel.org,
	daniel.vetter@...ll.ch, corbet@....net,
	"Luis R. Rodriguez" <mcgrof@...nel.org>
Subject: [PATCH v2 0/8] firmware: add new sysdata API

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

Powered by blists - more mailing lists