Messages in this thread | | | Subject | Re: [RFC PATCH 1/5] firmware: Create firmware upload framework | From | Russ Weight <> | Date | Wed, 17 Nov 2021 12:02:05 -0800 |
| |
On 11/17/21 10:54 AM, Greg KH wrote: > On Wed, Nov 17, 2021 at 10:47:38AM -0800, Russ Weight wrote: >> >> On 11/17/21 10:18 AM, Greg KH wrote: >>> On Wed, Nov 17, 2021 at 10:00:54AM -0800, Russ Weight wrote: >>>> On 11/17/21 7:15 AM, Greg KH wrote: >>>>> On Wed, Nov 10, 2021 at 05:13:41PM -0800, Russ Weight wrote: >>>>>> The Firmware Upload class driver provides a common API for uploading >>>>>> firmware files to devices. >>>>> That is exactly what the existing firmware api in the kernel is supposed >>>>> to be accomplishing. >>>>> >>>>> If it is not doing what you need it to do, then you need to document the >>>>> heck out of why it is not, and why you need a different api for this. I >>>>> do not see that here in this changelog at all :( >>>> This is part of the documentation included later in this patch. I can add >>>> this to the changelog. >>>> >>>> +Some devices load firmware from on-board FLASH when the card initializes. >>>> +These cards do not require the request_firmware framework to load the >>>> +firmware when the card boots, but they to require a utility to allow >>>> +users to update the FLASH contents. >>> There's no requirement that request_firmware only be done at boot time, >>> why not use it at any point in time? >> Calling request_firmware() is not restricted to boot time. But it requires >> a firmware filename under /lib/firmware > Not really, there are many locations it can be in. See the different > configuration options we have. > > But why would you want firmware in another location? My current use case is for a user to upload a new, signed FPGA image to an FPGA card. The card BMC authenticates and stores the data in FLASH. From the perspective of the card, the image in FLASH is analogous to a firmware file for another device being stored in /lib/firmware. For the FPGA images, there is no real value to also storing the files in /lib/firmware (or anywhere else on the system).
> >> , and the convention is to specify the >> filename in the kernel config. > That is not a requirement at all. > >> I don't see any support for a user to provide a filename at run time >> to be uploaded to a device, and that is the use case that I want to >> support. > If that's the only difference here, please work with the existing > framework to add that tiny thing (i.e. pass in a name) to the current > framework. Do not create a whole new one please. I think another fundamental difference is that the request_firmware() API is a driver API for the device driver to do a data "pull". The firmware-upload API is a user API for doing a data "push" (prepare(), write(), poll_complete()) to the lower-level driver.
I did look at the backup option of the request_firmware() framework for writing image data via sysfs. That's a possibility, but I thought that we would be abusing the intent. I can look more at that option...
> >>>> When you say "existing firmware api", I'm thinking request_firmware, which >>>> requires that driver names be specified in the kernel config and wants to >>>> load firmware automatically during device initialization. >>> It can be used at any time, why do you think it's restricted to init >>> time? >>> >>> And I do not understand your issue with driver names. >> Sorry - I meant so say "firmware file names" >> >> In an earlier iteration of this patchset, you pointed out that allowing a user >> to provide a filename for request_firmware() to use was a security issue. > It is crazy, don't you think? > >> The use case that I am targeting is to allow a user to provide an image file >> to a device at run time. > That's not a limitation of the existing firmware layer. > > thanks, > > greg k-h
| |