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

Andy Lutomirski
2015-04-30T17:55:47Z

On Thu, Apr 30, 2015 at 2:17 AM, Kweh, Hock Leong
<hock.leong.kweh@intel.com> wrote:
>> -----Original Message-----
>> From: Andy Lutomirski [mailto:luto@amacapital.net]
>> Sent: Thursday, April 30, 2015 2:41 AM
>>
>> On Wed, Apr 29, 2015 at 4:23 AM, Kweh, Hock Leong
>> <hock.leong.kweh@intel.com> wrote:
>> >
>> > Dear communities,
>> >
>> > 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
>>
>> If you do this, there's no need for the misc device.
>
> I do this so that in the future when someone want to implement the
> Ioctl(), he or she can base on this and expand it.
>
>>
>> > - 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
>>
>> Make sure the mutex operation is killable, then, and maybe even
>> interruptable.
>
> Okay.
>
>>
>> > - 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.
>> >
>>
>> If it works (and cat really does fail reliably), then it seems okay to me.
>>
>> However, since I like pulling increasing numbers of my hats, someone should
>> verify that the common embedded cat implementations are also okay with
>> this. For example, I haven't yet found any code in busybox's cat
>> implementation that closes stdout.
>>
>> Given that the main targets of this (for now, at least) are embedded, this
>> might be a problem.
>>
>
> I think we shouldn't focus on the cat implementation for the close issue.
>
> My understanding about this action:
> cat file.bin > /sys/..../capsule_loader
> It is actually the ">" (IO redirection) who perform the open write & close
> to this "/sys/..../capsule_loader" file note and not the "cat" do it.
> So, I think your answer can be found at Shell source code.

The shell opens capsule_loader and then execs the command. If you type:

(cat file.bin) >/sys/.../captule_loader, then the command is a
subshell and the subshell will close the file. (cat might also close
it, but there will be two references.)

If you type:

cat file.bin >/sys/.../capsule_loader

then the shell doesn't retain a reference to the file at all.

--Andy