Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware

Andy Lutomirski
2015-04-29T21:37:04Z

On Wed, Apr 29, 2015 at 2:35 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Wed, 2015-04-29 at 11:23 +0000, Kweh, Hock Leong wrote:
>> I agree with James. Due to different people may have different needs. But
>> from our side, we would just like to have a simple interface for us to upload
>> the efi capsule and perform update. We do not have any use case or need
>> to get info from QueryCapsuleUpdate(). Let me give a suggestion here:
>> please allow me to focus on deliver this simple loading interface and
>> upstream it. Then later whoever has the actual use case or needs on the ioctl
>> implementation, he or she could enhance base on this simple loading interface.
>> What do you guys think?
>>
>> Let me summarize the latest design idea:
>> - No longer leverage on firmware class but use misc device
>> - Do not use platform device but use device_create()
>> - User just need to perform "cat file.bin > /sys/.../capsule_loader" in the shell
>> - File operation functions include: open(), read(), write() and flush()
>> - Perform mutex lock in open() then release the mutex in flush() for avoiding
>> race condition / concurrent loading
>> - Perform the capsule update and error return at flush() function
>>
>> Is there anything I missed? Any one still have concern with this idea?
>> Thanks for providing the ideas as well as the review.
>
> I think that's pretty much it.
>
> Why don't you let me construct a straw man patch. It's going to be a
> bit controversial because it involves adding flush operations to sysfs
> and kernfs, slicing apart firmware_class.c to extract the transaction
> handling stuff and creating an new efi update capsule file which makes
> use of it.
>
> Once we have code, we at least have something more concrete to argue
> over.

Would it be worth checking whether busybox is also okay with it first?
(Sorry to be a naysayer.)

It would be a shame if we do all this to keep the userspace footprint
light and then it doesn't work for non-coreutils userspace.

--Andy