* RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface @ 2015-03-02 10:59 Kweh, Hock Leong 2015-03-02 12:29 ` Matt Fleming 0 siblings, 1 reply; 37+ messages in thread From: Kweh, Hock Leong @ 2015-03-02 10:59 UTC (permalink / raw) To: Borislav Petkov Cc: Andy Lutomirski, Sam Protsenko, Matt Fleming, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2121 bytes --] > -----Original Message----- > From: Borislav Petkov [mailto:bp@alien8.de] > Sent: Wednesday, February 25, 2015 8:49 PM > > On Wed, Feb 25, 2015 at 12:38:20PM +0000, Kweh, Hock Leong wrote: > > The reason we use this interface for efi capsule is that efi capsule > > support multi binaries to be uploaded and each binary file name > > can be different. > > So you can write the file path to a second file and reload then, once > per file. Thanks for the suggestion. Did some exploration on this approach and would like to continue the discussion together with this suggested design. Ideal condition: - one file note is enough for load binary and status return (capsule_load) - load steps would be as simple as just: echo [binary file name] > capsule_load - status return in the same command action. If failed, the echo will fail with the failing status code. Trade off: - does not have the run time flexibility to load any firmware binaries at different different path as firmware class only support one custom path inputted during boot time/load time. Unless to add new export function for firmware class. - if any typo on user level or file path not found, firmware class falls back to user helper interface. User is required to open another terminal to performs: -> echo 1 > loading -> cat capsule.bin > data -> echo 0 > loading Or wait for timeout. - User has the capability to configure the firmware_class time out value. If the infinite value is set, the only method to break the infinite waiting by manually perform "echo -1 > loading" on the other terminal. Are you guys okay with these trade off? Or you guys still okay with the previous 2 design idea? > > Alternatively, you can combine all the files into a cpio archive like > we do with the microcode loader too, and let the kernel parse the cpio > archive. I believe this will make the implementation complicated compare to above. Regards, Wilson ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-02 10:59 Re: [PATCH v2 3/3] efi: Capsule update with user helper interface Kweh, Hock Leong @ 2015-03-02 12:29 ` Matt Fleming 2015-03-03 5:56 ` Kweh, Hock Leong 0 siblings, 1 reply; 37+ messages in thread From: Matt Fleming @ 2015-03-02 12:29 UTC (permalink / raw) To: Kweh, Hock Leong Cc: Borislav Petkov, Andy Lutomirski, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Mon, 02 Mar, at 10:59:00AM, Kweh Hock Leong wrote: > > -----Original Message----- > > From: Borislav Petkov [mailto:bp@alien8.de] > > Sent: Wednesday, February 25, 2015 8:49 PM > > > > On Wed, Feb 25, 2015 at 12:38:20PM +0000, Kweh, Hock Leong wrote: > > > The reason we use this interface for efi capsule is that efi capsule > > > support multi binaries to be uploaded and each binary file name > > > can be different. > > > > So you can write the file path to a second file and reload then, once > > per file. > > Thanks for the suggestion. Did some exploration on this approach and > would like to continue the discussion together with this suggested design. > > Ideal condition: > - one file note is enough for load binary and status return (capsule_load) > - load steps would be as simple as just: > echo [binary file name] > capsule_load > - status return in the same command action. If failed, the echo will fail > with the failing status code. > > Trade off: > - does not have the run time flexibility to load any firmware binaries at > different different path as firmware class only support one custom > path inputted during boot time/load time. Unless to add new export > function for firmware class. Could you elaborate here? I don't understand this point. > - if any typo on user level or file path not found, firmware class falls back > to user helper interface. User is required to open another terminal to > performs: > -> echo 1 > loading > -> cat capsule.bin > data > -> echo 0 > loading > Or wait for timeout. Not if you use request_firmware_direct(), right? request_firmware_direct() explicitly does not invoke the user helper. > - User has the capability to configure the firmware_class time out value. > If the infinite value is set, the only method to break the infinite waiting > by manually perform "echo -1 > loading" on the other terminal. I'm not sure this is a problem if we use request_firmware_direct(). > Are you guys okay with these trade off? > Or you guys still okay with the previous 2 design idea? I quite like the design that Borislav is proposing. Things become a lot simpler when we stop using request_firmware_nowait(). The next question is whether we want to batch passing capsules to the firmware. The UpdateCapsule() EFI runtime service allows you to chain capsule blobs together and pass a scatter-gather list. If we do want to batch uploading then we need the equivalent of the 'reload' file from the microcode loader to signal to the kernel that the userland process has finished uploading capsules, and the kernel can now send them to the firmware as one chunk. Also, Andy previously raised the point about multiple userland processes passing capsules to the firmware simultaneously and the races that occur, so we'd need a way to make that atomic. I'd much prefer to send one capsule at a time to the firmware, because it just makes this whole thing simpler. Wilson, I'm really looking for your input here with how capsule uploading works on Quark. If we can just repeatedly call UpdateCapsule() with one capsule file at a time, that's fine. If we absolutely must bundle multiple capsule files together then we probably need to provide some atomicity. Thoughts? > > Alternatively, you can combine all the files into a cpio archive like > > we do with the microcode loader too, and let the kernel parse the cpio > > archive. > > I believe this will make the implementation complicated compare to above. Agreed. The capsule files we're sending to the firmware (via this interface) already contain all the information we need to stitch multiple binaries together - there's really no reason to bundle things any further. -- Matt Fleming, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 37+ messages in thread
* RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-02 12:29 ` Matt Fleming @ 2015-03-03 5:56 ` Kweh, Hock Leong 2015-03-03 20:37 ` Andy Lutomirski 0 siblings, 1 reply; 37+ messages in thread From: Kweh, Hock Leong @ 2015-03-03 5:56 UTC (permalink / raw) To: Matt Fleming Cc: Borislav Petkov, Andy Lutomirski, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi > -----Original Message----- > From: Matt Fleming [mailto:matt@console-pimps.org] > Sent: Monday, March 02, 2015 8:30 PM > > On Mon, 02 Mar, at 10:59:00AM, Kweh Hock Leong wrote: > > > -----Original Message----- > > > From: Borislav Petkov [mailto:bp@alien8.de] > > > Sent: Wednesday, February 25, 2015 8:49 PM > > > > > > On Wed, Feb 25, 2015 at 12:38:20PM +0000, Kweh, Hock Leong wrote: > > > > The reason we use this interface for efi capsule is that efi capsule > > > > support multi binaries to be uploaded and each binary file name > > > > can be different. > > > > > > So you can write the file path to a second file and reload then, once > > > per file. > > > > Thanks for the suggestion. Did some exploration on this approach and > > would like to continue the discussion together with this suggested design. > > > > Ideal condition: > > - one file note is enough for load binary and status return (capsule_load) > > - load steps would be as simple as just: > > echo [binary file name] > capsule_load > > - status return in the same command action. If failed, the echo will fail > > with the failing status code. > > > > Trade off: > > - does not have the run time flexibility to load any firmware binaries at > > different different path as firmware class only support one custom > > path inputted during boot time/load time. Unless to add new export > > function for firmware class. > > Could you elaborate here? I don't understand this point. Just to call out that using firmware class auto locate binary feature is limited to locations: - "/lib/firmware/updates/" UTS_RELEASE, - "/lib/firmware/updates", - "/lib/firmware/" UTS_RELEASE, - "/lib/firmware" and one custom path which inputted during firmware_class module load time or kernel boot up time. It is just not like the user helper interface which allow load the binary at any path/location. This really is not a big deal. User should cope with it. > > > - if any typo on user level or file path not found, firmware class falls back > > to user helper interface. User is required to open another terminal to > > performs: > > -> echo 1 > loading > > -> cat capsule.bin > data > > -> echo 0 > loading > > Or wait for timeout. > > Not if you use request_firmware_direct(), right? > request_firmware_direct() explicitly does not invoke the user helper. > > > - User has the capability to configure the firmware_class time out value. > > If the infinite value is set, the only method to break the infinite waiting > > by manually perform "echo -1 > loading" on the other terminal. > > I'm not sure this is a problem if we use request_firmware_direct(). Oops... I miss out this function. Will rework using this function. > > > Are you guys okay with these trade off? > > Or you guys still okay with the previous 2 design idea? > > I quite like the design that Borislav is proposing. Things become a lot > simpler when we stop using request_firmware_nowait(). > > The next question is whether we want to batch passing capsules to the > firmware. The UpdateCapsule() EFI runtime service allows you to chain > capsule blobs together and pass a scatter-gather list. > > If we do want to batch uploading then we need the equivalent of the > 'reload' file from the microcode loader to signal to the kernel that the > userland process has finished uploading capsules, and the kernel can now > send them to the firmware as one chunk. > > Also, Andy previously raised the point about multiple userland processes > passing capsules to the firmware simultaneously and the races that > occur, so we'd need a way to make that atomic. > > I'd much prefer to send one capsule at a time to the firmware, because > it just makes this whole thing simpler. > > Wilson, I'm really looking for your input here with how capsule > uploading works on Quark. If we can just repeatedly call UpdateCapsule() > with one capsule file at a time, that's fine. If we absolutely must > bundle multiple capsule files together then we probably need to provide > some atomicity. > > Thoughts? Quark does not need batch uploading. Even I tested multiple capsules on Quark by doing repeatedly calling UpdateCapsule() is still working for Quark. So, Quark does not require this bundling. > > > > Alternatively, you can combine all the files into a cpio archive like > > > we do with the microcode loader too, and let the kernel parse the cpio > > > archive. > > > > I believe this will make the implementation complicated compare to above. > > Agreed. The capsule files we're sending to the firmware (via this > interface) already contain all the information we need to stitch > multiple binaries together - there's really no reason to bundle things > any further. > > -- > Matt Fleming, Intel Open Source Technology Center Hi Greg, Ming Lei, Sam & Andy, Do you guys have any others concern on Borislav proposed approach? Thanks. Regards, Wilson ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-03 5:56 ` Kweh, Hock Leong @ 2015-03-03 20:37 ` Andy Lutomirski 2015-03-03 20:49 ` Borislav Petkov 2015-03-05 9:18 ` Kweh, Hock Leong 0 siblings, 2 replies; 37+ messages in thread From: Andy Lutomirski @ 2015-03-03 20:37 UTC (permalink / raw) To: Kweh, Hock Leong Cc: Matt Fleming, Borislav Petkov, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Mon, Mar 2, 2015 at 9:56 PM, Kweh, Hock Leong <hock.leong.kweh@intel.com> wrote: >> -----Original Message----- >> From: Matt Fleming [mailto:matt@console-pimps.org] >> Sent: Monday, March 02, 2015 8:30 PM >> >> On Mon, 02 Mar, at 10:59:00AM, Kweh Hock Leong wrote: >> > > -----Original Message----- >> > > From: Borislav Petkov [mailto:bp@alien8.de] >> > > Sent: Wednesday, February 25, 2015 8:49 PM >> > > >> > > On Wed, Feb 25, 2015 at 12:38:20PM +0000, Kweh, Hock Leong wrote: >> > > > The reason we use this interface for efi capsule is that efi capsule >> > > > support multi binaries to be uploaded and each binary file name >> > > > can be different. >> > > >> > > So you can write the file path to a second file and reload then, once >> > > per file. >> > >> > Thanks for the suggestion. Did some exploration on this approach and >> > would like to continue the discussion together with this suggested design. >> > >> > Ideal condition: >> > - one file note is enough for load binary and status return (capsule_load) >> > - load steps would be as simple as just: >> > echo [binary file name] > capsule_load >> > - status return in the same command action. If failed, the echo will fail >> > with the failing status code. >> > >> > Trade off: >> > - does not have the run time flexibility to load any firmware binaries at >> > different different path as firmware class only support one custom >> > path inputted during boot time/load time. Unless to add new export >> > function for firmware class. >> >> Could you elaborate here? I don't understand this point. > > Just to call out that using firmware class auto locate binary feature is limited > to locations: > - "/lib/firmware/updates/" UTS_RELEASE, > - "/lib/firmware/updates", > - "/lib/firmware/" UTS_RELEASE, > - "/lib/firmware" > and one custom path which inputted during firmware_class module load > time or kernel boot up time. > > It is just not like the user helper interface which allow load the binary at > any path/location. > > This really is not a big deal. User should cope with it. No, it's a big deal, and the user should not cope. The user *should not* be required to have write access to anything in /lib to install a UEFI capsule that they download from their motherboard vendor's website. /lib belongs to the distro, and UEFI capsules do not belong to the distro. In this regard, UEFI capsules are completely unlike your wireless card firmware, your cpu microcode, etc. Imagine systems using NFS root, Atomic-style systems (e.g. ostree), systems that boot off squashfs, etc. They should still be able to load capsules. The basic user interface that should work is: # uefi-load-capsule /path/to/capsule or: # uefi-load-capsule - </path/to/capsule I don't really care how uefi-load-capsule is implemented, as long as it's straightforward, because people will screw it up if it isn't straightforward. Why is it so hard to have a file in sysfs that you write the capsule to using *cat* (not echo) and that will return an error code if cat fails? Is it because you don't know where the end of the capsule is? if so, ioctl is designed for exactly this purpose. TBH, I find this thread kind of ridiculous. The problem that you're trying to solve is extremely simple, the functionality that userspace needs is trivial, and all of these complex proposals for how it should work are an artifact of the fact that the kernel-internal interfaces you're using for it are not well suited to the problem at hand. --Andy ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-03 20:37 ` Andy Lutomirski @ 2015-03-03 20:49 ` Borislav Petkov 2015-03-03 21:56 ` Andy Lutomirski 2015-03-05 9:18 ` Kweh, Hock Leong 1 sibling, 1 reply; 37+ messages in thread From: Borislav Petkov @ 2015-03-03 20:49 UTC (permalink / raw) To: Andy Lutomirski Cc: Kweh, Hock Leong, Matt Fleming, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Tue, Mar 03, 2015 at 12:37:54PM -0800, Andy Lutomirski wrote: > The user *should not* be required to have write access to anything in > /lib to install a UEFI capsule that they download from their > motherboard vendor's website. /lib belongs to the distro, and UEFI > capsules do not belong to the distro. In this regard, UEFI capsules > are completely unlike your wireless card firmware, your cpu microcode, > etc. Oh oh but but, if an UEFI capsule can brick the system, a normal user would be able to brick that system then. I think we should forbid that. I agree with the rest of your note that a simple cat <fw_blob> > /sys/... should be enough. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-03 20:49 ` Borislav Petkov @ 2015-03-03 21:56 ` Andy Lutomirski 0 siblings, 0 replies; 37+ messages in thread From: Andy Lutomirski @ 2015-03-03 21:56 UTC (permalink / raw) To: Borislav Petkov Cc: Matt Fleming, Ong Boon Leong, linux-efi, Greg Kroah-Hartman, Sam Protsenko, Kweh, Hock Leong, LKML, Ming Lei On Mar 3, 2015 12:51 PM, "Borislav Petkov" <bp@alien8.de> wrote: > > On Tue, Mar 03, 2015 at 12:37:54PM -0800, Andy Lutomirski wrote: > > The user *should not* be required to have write access to anything in > > /lib to install a UEFI capsule that they download from their > > motherboard vendor's website. /lib belongs to the distro, and UEFI > > capsules do not belong to the distro. In this regard, UEFI capsules > > are completely unlike your wireless card firmware, your cpu microcode, > > etc. > > Oh oh but but, if an UEFI capsule can brick the system, a normal user > would be able to brick that system then. I think we should forbid that. Absolutely. That's why I said # uefi-load-capsule and not $ uefi-load-capsule :) > > I agree with the rest of your note that a simple > > cat <fw_blob> > /sys/... > > should be enough. > > -- > Regards/Gruss, > Boris. > > ECO tip #101: Trim your mails when you reply. > -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-03 20:37 ` Andy Lutomirski 2015-03-03 20:49 ` Borislav Petkov @ 2015-03-05 9:18 ` Kweh, Hock Leong 2015-03-05 23:08 ` Andy Lutomirski 1 sibling, 1 reply; 37+ messages in thread From: Kweh, Hock Leong @ 2015-03-05 9:18 UTC (permalink / raw) To: Andy Lutomirski Cc: Matt Fleming, Borislav Petkov, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2817 bytes --] > -----Original Message----- > From: Andy Lutomirski [mailto:luto@amacapital.net] > Sent: Wednesday, March 04, 2015 4:38 AM > > On Mon, Mar 2, 2015 at 9:56 PM, Kweh, Hock Leong > <hock.leong.kweh@intel.com> wrote: > > > > Just to call out that using firmware class auto locate binary feature is limited > > to locations: > > - "/lib/firmware/updates/" UTS_RELEASE, > > - "/lib/firmware/updates", > > - "/lib/firmware/" UTS_RELEASE, > > - "/lib/firmware" > > and one custom path which inputted during firmware_class module load > > time or kernel boot up time. > > > > It is just not like the user helper interface which allow load the binary at > > any path/location. > > > > This really is not a big deal. User should cope with it. > > No, it's a big deal, and the user should not cope. > > The user *should not* be required to have write access to anything in > /lib to install a UEFI capsule that they download from their > motherboard vendor's website. /lib belongs to the distro, and UEFI > capsules do not belong to the distro. In this regard, UEFI capsules > are completely unlike your wireless card firmware, your cpu microcode, > etc. > > Imagine systems using NFS root, Atomic-style systems (e.g. ostree), > systems that boot off squashfs, etc. They should still be able to > load capsules. The basic user interface that should work is: > > # uefi-load-capsule /path/to/capsule > > or: > > # uefi-load-capsule - </path/to/capsule > > I don't really care how uefi-load-capsule is implemented, as long as > it's straightforward, because people will screw it up if it isn't > straightforward. > > Why is it so hard to have a file in sysfs that you write the capsule > to using *cat* (not echo) and that will return an error code if cat > fails? Is it because you don't know where the end of the capsule is? > if so, ioctl is designed for exactly this purpose. > > TBH, I find this thread kind of ridiculous. The problem that you're > trying to solve is extremely simple, the functionality that userspace > needs is trivial, and all of these complex proposals for how it should > work are an artifact of the fact that the kernel-internal interfaces > you're using for it are not well suited to the problem at hand. > > --Andy Sorry, I may not catch your point correctly. Are you trying to tell that a "normal" user can perform efi capsule update. But a "normal" user does not have the right to install or copy the capsule binary into "/lib/firmware/". So, there is a need to make this capsule module to allow uploading the capsule binary at any path or location other than "/lib/firmware/". Is this what you mean? Regards, Wilson ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ permalink raw reply [flat|nested] 37+ messages in thread
* RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-05 9:18 ` Kweh, Hock Leong @ 2015-03-05 23:08 ` Andy Lutomirski 2015-03-06 8:13 ` Borislav Petkov 2015-03-06 12:20 ` Re: [PATCH v2 3/3] efi: Capsule update with user helper interface Kweh, Hock Leong 0 siblings, 2 replies; 37+ messages in thread From: Andy Lutomirski @ 2015-03-05 23:08 UTC (permalink / raw) To: Kweh Hock Leong Cc: Borislav Petkov, Matt Fleming, Ong, Boon Leong, linux-efi, Greg Kroah-Hartman, Sam Protsenko, LKML, Ming Lei On Mar 5, 2015 1:19 AM, "Kweh, Hock Leong" <hock.leong.kweh@intel.com> wrote: > > > -----Original Message----- > > From: Andy Lutomirski [mailto:luto@amacapital.net] > > Sent: Wednesday, March 04, 2015 4:38 AM > > > > On Mon, Mar 2, 2015 at 9:56 PM, Kweh, Hock Leong > > <hock.leong.kweh@intel.com> wrote: > > > > > > Just to call out that using firmware class auto locate binary feature is limited > > > to locations: > > > - "/lib/firmware/updates/" UTS_RELEASE, > > > - "/lib/firmware/updates", > > > - "/lib/firmware/" UTS_RELEASE, > > > - "/lib/firmware" > > > and one custom path which inputted during firmware_class module load > > > time or kernel boot up time. > > > > > > It is just not like the user helper interface which allow load the binary at > > > any path/location. > > > > > > This really is not a big deal. User should cope with it. > > > > No, it's a big deal, and the user should not cope. > > > > The user *should not* be required to have write access to anything in > > /lib to install a UEFI capsule that they download from their > > motherboard vendor's website. /lib belongs to the distro, and UEFI > > capsules do not belong to the distro. In this regard, UEFI capsules > > are completely unlike your wireless card firmware, your cpu microcode, > > etc. > > > > Imagine systems using NFS root, Atomic-style systems (e.g. ostree), > > systems that boot off squashfs, etc. They should still be able to > > load capsules. The basic user interface that should work is: > > > > # uefi-load-capsule /path/to/capsule > > > > or: > > > > # uefi-load-capsule - </path/to/capsule > > > > I don't really care how uefi-load-capsule is implemented, as long as > > it's straightforward, because people will screw it up if it isn't > > straightforward. > > > > Why is it so hard to have a file in sysfs that you write the capsule > > to using *cat* (not echo) and that will return an error code if cat > > fails? Is it because you don't know where the end of the capsule is? > > if so, ioctl is designed for exactly this purpose. > > > > TBH, I find this thread kind of ridiculous. The problem that you're > > trying to solve is extremely simple, the functionality that userspace > > needs is trivial, and all of these complex proposals for how it should > > work are an artifact of the fact that the kernel-internal interfaces > > you're using for it are not well suited to the problem at hand. > > > > --Andy > > Sorry, I may not catch your point correctly. Are you trying to tell that > a "normal" user can perform efi capsule update. But a "normal" user > does not have the right to install or copy the capsule binary into > "/lib/firmware/". So, there is a need to make this capsule module to > allow uploading the capsule binary at any path or location other than > "/lib/firmware/". > > Is this what you mean? No. Only root should be able to load capsules, but even root may not be able to write to /lib. --Andy > > > Regards, > Wilson ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-05 23:08 ` Andy Lutomirski @ 2015-03-06 8:13 ` Borislav Petkov 2015-03-06 11:41 ` Kweh, Hock Leong 2015-03-06 12:20 ` Re: [PATCH v2 3/3] efi: Capsule update with user helper interface Kweh, Hock Leong 1 sibling, 1 reply; 37+ messages in thread From: Borislav Petkov @ 2015-03-06 8:13 UTC (permalink / raw) To: Andy Lutomirski Cc: Kweh Hock Leong, Matt Fleming, Ong, Boon Leong, linux-efi, Greg Kroah-Hartman, Sam Protsenko, LKML, Ming Lei On Thu, Mar 05, 2015 at 03:08:42PM -0800, Andy Lutomirski wrote: > No. Only root should be able to load capsules, but even root may not > be able to write to /lib. So basically what we want to do is: # cat /any/path/to/efi/capsule/accessible/to/root/efi_capsule.img > /sys/firmware/efi/update Now it can't get any simpler than that and you get error codes too by failing the cat if the update fails. Mind you, I'm using '#' and not '$' as a shell prompt :-) -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-06 8:13 ` Borislav Petkov @ 2015-03-06 11:41 ` Kweh, Hock Leong 2015-03-06 14:47 ` Borislav Petkov 0 siblings, 1 reply; 37+ messages in thread From: Kweh, Hock Leong @ 2015-03-06 11:41 UTC (permalink / raw) To: Borislav Petkov, Andy Lutomirski Cc: Matt Fleming, Ong, Boon Leong, linux-efi, Greg Kroah-Hartman, Sam Protsenko, LKML, Ming Lei [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1151 bytes --] > -----Original Message----- > From: Borislav Petkov [mailto:bp@alien8.de] > Sent: Friday, March 06, 2015 4:14 PM > > On Thu, Mar 05, 2015 at 03:08:42PM -0800, Andy Lutomirski wrote: > > No. Only root should be able to load capsules, but even root may not > > be able to write to /lib. > > So basically what we want to do is: > > # cat /any/path/to/efi/capsule/accessible/to/root/efi_capsule.img > > /sys/firmware/efi/update > > Now it can't get any simpler than that and you get error codes too by > failing the cat if the update fails. > > Mind you, I'm using '#' and not '$' as a shell prompt :-) > > -- > Regards/Gruss, > Boris. > > ECO tip #101: Trim your mails when you reply. > -- I believe you guys are more paying attention to the PATH and whether doing: # cat /any/path/capsule.bin > /sys/devices/platform/efi_capsule/capsule_load or doing: # echo "/any/path/capsule.bin" > /sys/devices/platform/efi_capsule/capsule_load is not a big issue right? Regards, Wilson ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-06 11:41 ` Kweh, Hock Leong @ 2015-03-06 14:47 ` Borislav Petkov 2015-03-09 21:23 ` fwupdate Borislav Petkov 0 siblings, 1 reply; 37+ messages in thread From: Borislav Petkov @ 2015-03-06 14:47 UTC (permalink / raw) To: Kweh, Hock Leong Cc: Andy Lutomirski, Matt Fleming, Ong, Boon Leong, linux-efi, Greg Kroah-Hartman, Sam Protsenko, LKML, Ming Lei On Fri, Mar 06, 2015 at 11:41:57AM +0000, Kweh, Hock Leong wrote: > # cat /any/path/capsule.bin > /sys/devices/platform/efi_capsule/capsule_load This is straight-forward and clean. > or doing: > # echo "/any/path/capsule.bin" > /sys/devices/platform/efi_capsule/capsule_load This is strange and clumsy. So do the first one please. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* fwupdate 2015-03-06 14:47 ` Borislav Petkov @ 2015-03-09 21:23 ` Borislav Petkov 2015-03-10 1:54 ` fwupdate Roy Franz 0 siblings, 1 reply; 37+ messages in thread From: Borislav Petkov @ 2015-03-09 21:23 UTC (permalink / raw) To: Kweh, Hock Leong Cc: Andy Lutomirski, Matt Fleming, Ong, Boon Leong, linux-efi, Greg Kroah-Hartman, Sam Protsenko, LKML, Ming Lei, Peter Jones + pjones. So reportedly, there is already a capsule-loading thing which doesn't need the kernel at all: https://github.com/rhinstaller/fwupdate So why are we even wasting energy with this discussion here? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: fwupdate 2015-03-09 21:23 ` fwupdate Borislav Petkov @ 2015-03-10 1:54 ` Roy Franz 2015-03-10 14:56 ` fwupdate Peter Jones 0 siblings, 1 reply; 37+ messages in thread From: Roy Franz @ 2015-03-10 1:54 UTC (permalink / raw) To: Borislav Petkov Cc: Kweh, Hock Leong, Andy Lutomirski, Matt Fleming, Ong, Boon Leong, linux-efi, Greg Kroah-Hartman, Sam Protsenko, LKML, Ming Lei, Peter Jones On Mon, Mar 9, 2015 at 2:23 PM, Borislav Petkov <bp@alien8.de> wrote: > + pjones. > > So reportedly, there is already a capsule-loading thing which doesn't > need the kernel at all: > > https://github.com/rhinstaller/fwupdate > > So why are we even wasting energy with this discussion here? > > -- > Regards/Gruss, > Boris. The network boot case can be taken care of as Peter described (ie taking network device paths, instead of just file paths), so this should work for those cases as well. There would be some extra setup, as for this to work the EFI firmware update program would need to be run at boot (via the BootNext variable), but I don't think this is unreasonable. It looks like GnuEFI now supports Aarch64, so building the firmware update utility shouldn't be a problem. Peter - have you you tried building this on Aarch64 yet? If not I'll give it a try. I think there is some value in using runtime update capsule calls, as that will help make sure the feature works in the firmware. I think with arm64 servers in an early stage of development, we can influence the firmware support of various features by using them. I don't know that this is a sufficient reason, but if runtime capsules are never used then there is a good chance that there will be many broken implementations. Roy ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: fwupdate 2015-03-10 1:54 ` fwupdate Roy Franz @ 2015-03-10 14:56 ` Peter Jones 2015-03-10 15:27 ` fwupdate Peter Jones 0 siblings, 1 reply; 37+ messages in thread From: Peter Jones @ 2015-03-10 14:56 UTC (permalink / raw) To: Roy Franz Cc: Borislav Petkov, Kweh, Hock Leong, Andy Lutomirski, Matt Fleming, Ong, Boon Leong, linux-efi, Greg Kroah-Hartman, Sam Protsenko, LKML, Ming Lei On Mon, Mar 09, 2015 at 06:54:12PM -0700, Roy Franz wrote: > On Mon, Mar 9, 2015 at 2:23 PM, Borislav Petkov <bp@alien8.de> wrote: > > + pjones. > > > > So reportedly, there is already a capsule-loading thing which doesn't > > need the kernel at all: > > > > https://github.com/rhinstaller/fwupdate > > > > So why are we even wasting energy with this discussion here? > > > > -- > > Regards/Gruss, > > Boris. > > The network boot case can be taken care of as Peter described (ie > taking network device paths, instead of just file paths), so > this should work for those cases as well. There would be some extra > setup, as for this to work the EFI firmware update program > would need to be run at boot (via the BootNext variable), but I don't > think this is unreasonable. > It looks like GnuEFI now supports Aarch64, so building the firmware > update utility shouldn't be a problem. Peter - have you you tried > building > this on Aarch64 yet? If not I'll give it a try. I tried a very early version of the code on an Aarch64 machine where I also wrote the firmware feature, and it basically seemed to work, modulo my buggy firmware ;) I haven't tried it recently, but I do have all the right makefile goo in there to build it reasonably, and I don't know of any reason it wouldn't work. That said, I haven't sent my patch to add the capsule headers to gnu-efi yet, so you won't get very far - I'll make sure and send those this week, hopefully today. Also note that fwupdate is still a very active work in progress; it's not /quite/ ready for general purpose deployment even in a "trying it out" phase, but I'm hoping to get to that point this week or next - in particular the code on github right now assumes an HD() device path with a specific partition guid. That is, it has literally worked on two machines ever. Yes, it's a thing we intend on supporting, but it's not /there/ yet. > I think there is some value in using runtime update capsule calls, as > that will help make sure the feature works in the firmware. I think > with arm64 servers in an early stage of development, we can influence > the firmware support of various features by using them. I don't know > that this is a sufficient reason, but if runtime capsules are never > used then there is a good chance that there will be many broken > implementations. That's certainly potentially valid, but it doesn't necessarily yield something we (the OS vendor deploying features to customers) can actually depend on. Usually firmware features like this become generally usable when there's some market maker causing hardware vendors to have a large interest in making sure the feature works. That means most often the carrot being dangled is MS Logo certification marketing dollars, and the testing all follows from the situations they require to work. So far, it appears MS has been using this only from Boot Services, so that's what works. It looks to me like that's probably likely to remain true. One reason is peripheral cards. My expectation (though possibly unfounded) is that MS will require peripheral cards to use this same update mechanism for option ROMs, and even if they don't I know some vendors are working on it. To do so, the card's UEFI driver (supplied on the option ROM) will be using EFI's Firmware Management Protocol (FMP) to participate in the capsule update system. The 2.5 spec drafts that include FMP and UpdateCapsule() support for FMP state that capsule updates for FMP aren't required to be available during runtime - it expressly calls out the case where CAPSULE_FLAGS_PERSIST_ACROSS_RESET is used. (I don't think I'm allowed to quote language from drafts in public, but if you have a copy, I'm talking about the last paragraph of 22.2.1) In my mind this means most implementations are going to always try to do updates from before ExitBootServices() for those option types - anything else and you're just in an error case where you have to do that anyway. So again, I'm not dead-set against having a kernel implementation, but I think its use cases are severely constrained, and most of the time most implementations will want to do this before the kernel loads. The best case for doing it in the kernel is embedded devices where a) there's no persistent storage except the flash part you're updating anyway, and b) the vendor nominally controls both the firmware and the deployed OS. -- Peter, heading back to the code. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: fwupdate 2015-03-10 14:56 ` fwupdate Peter Jones @ 2015-03-10 15:27 ` Peter Jones 0 siblings, 0 replies; 37+ messages in thread From: Peter Jones @ 2015-03-10 15:27 UTC (permalink / raw) To: Roy Franz Cc: Borislav Petkov, Kweh, Hock Leong, Andy Lutomirski, Matt Fleming, Ong, Boon Leong, linux-efi, Greg Kroah-Hartman, Sam Protsenko, LKML, Ming Lei On Tue, Mar 10, 2015 at 10:56:32AM -0400, Peter Jones wrote: > That said, I haven't sent my patch to add the capsule headers to gnu-efi > yet, so you won't get very far - I'll make sure and send those this > week, hopefully today. Slight correction, I did actually do that - it's in the current upstream repo. -- Peter ^ permalink raw reply [flat|nested] 37+ messages in thread
* RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-05 23:08 ` Andy Lutomirski 2015-03-06 8:13 ` Borislav Petkov @ 2015-03-06 12:20 ` Kweh, Hock Leong 2015-03-06 19:05 ` Andy Lutomirski 1 sibling, 1 reply; 37+ messages in thread From: Kweh, Hock Leong @ 2015-03-06 12:20 UTC (permalink / raw) To: Andy Lutomirski Cc: Borislav Petkov, Matt Fleming, Ong, Boon Leong, linux-efi, Greg Kroah-Hartman, Sam Protsenko, LKML, Ming Lei [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3223 bytes --] > -----Original Message----- > From: Andy Lutomirski [mailto:luto@amacapital.net] > Sent: Friday, March 06, 2015 7:09 AM > > On Mar 5, 2015 1:19 AM, "Kweh, Hock Leong" <hock.leong.kweh@intel.com> > wrote: > > > > > > This really is not a big deal. User should cope with it. > > > > > > No, it's a big deal, and the user should not cope. > > > > > > The user *should not* be required to have write access to anything in > > > /lib to install a UEFI capsule that they download from their > > > motherboard vendor's website. /lib belongs to the distro, and UEFI > > > capsules do not belong to the distro. In this regard, UEFI capsules > > > are completely unlike your wireless card firmware, your cpu microcode, > > > etc. > > > > > > Imagine systems using NFS root, Atomic-style systems (e.g. ostree), > > > systems that boot off squashfs, etc. They should still be able to > > > load capsules. The basic user interface that should work is: > > > > > > # uefi-load-capsule /path/to/capsule > > > > > > or: > > > > > > # uefi-load-capsule - </path/to/capsule > > > > > > I don't really care how uefi-load-capsule is implemented, as long as > > > it's straightforward, because people will screw it up if it isn't > > > straightforward. > > > > > > Why is it so hard to have a file in sysfs that you write the capsule > > > to using *cat* (not echo) and that will return an error code if cat > > > fails? Is it because you don't know where the end of the capsule is? > > > if so, ioctl is designed for exactly this purpose. > > > > > > TBH, I find this thread kind of ridiculous. The problem that you're > > > trying to solve is extremely simple, the functionality that userspace > > > needs is trivial, and all of these complex proposals for how it should > > > work are an artifact of the fact that the kernel-internal interfaces > > > you're using for it are not well suited to the problem at hand. > > > > > > --Andy > > > > Sorry, I may not catch your point correctly. Are you trying to tell that > > a "normal" user can perform efi capsule update. But a "normal" user > > does not have the right to install or copy the capsule binary into > > "/lib/firmware/". So, there is a need to make this capsule module to > > allow uploading the capsule binary at any path or location other than > > "/lib/firmware/". > > > > Is this what you mean? > > No. Only root should be able to load capsules, but even root may not > be able to write to /lib. > > --Andy > Okay, I accept that only root user can perform the load capsule. It is new to me that root user may not have the access right to "/lib/firmware". But I remember you do mention that CPU micro code and wifi firmware they are different and able to write in /lib/firmware. I am curious why efi capsule binary have such a restriction. What is preventing it being write to that location? I even went to check out the FHS: http://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard I do not find any restriction calling out for that. Would you mind to educate me on that? Thanks. Regards, Wilson ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ permalink raw reply [flat|nested] 37+ messages in thread
* RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-06 12:20 ` Re: [PATCH v2 3/3] efi: Capsule update with user helper interface Kweh, Hock Leong @ 2015-03-06 19:05 ` Andy Lutomirski 0 siblings, 0 replies; 37+ messages in thread From: Andy Lutomirski @ 2015-03-06 19:05 UTC (permalink / raw) To: Kweh Hock Leong Cc: Borislav Petkov, Matt Fleming, Ong, Boon Leong, linux-efi, Sam Protsenko, Greg Kroah-Hartman, LKML, Ming Lei On Mar 6, 2015 4:20 AM, "Kweh, Hock Leong" <hock.leong.kweh@intel.com> wrote: > > > -----Original Message----- > > From: Andy Lutomirski [mailto:luto@amacapital.net] > > Sent: Friday, March 06, 2015 7:09 AM > > > > On Mar 5, 2015 1:19 AM, "Kweh, Hock Leong" <hock.leong.kweh@intel.com> > > wrote: > > > > > > > > This really is not a big deal. User should cope with it. > > > > > > > > No, it's a big deal, and the user should not cope. > > > > > > > > The user *should not* be required to have write access to anything in > > > > /lib to install a UEFI capsule that they download from their > > > > motherboard vendor's website. /lib belongs to the distro, and UEFI > > > > capsules do not belong to the distro. In this regard, UEFI capsules > > > > are completely unlike your wireless card firmware, your cpu microcode, > > > > etc. > > > > > > > > Imagine systems using NFS root, Atomic-style systems (e.g. ostree), > > > > systems that boot off squashfs, etc. They should still be able to > > > > load capsules. The basic user interface that should work is: > > > > > > > > # uefi-load-capsule /path/to/capsule > > > > > > > > or: > > > > > > > > # uefi-load-capsule - </path/to/capsule > > > > > > > > I don't really care how uefi-load-capsule is implemented, as long as > > > > it's straightforward, because people will screw it up if it isn't > > > > straightforward. > > > > > > > > Why is it so hard to have a file in sysfs that you write the capsule > > > > to using *cat* (not echo) and that will return an error code if cat > > > > fails? Is it because you don't know where the end of the capsule is? > > > > if so, ioctl is designed for exactly this purpose. > > > > > > > > TBH, I find this thread kind of ridiculous. The problem that you're > > > > trying to solve is extremely simple, the functionality that userspace > > > > needs is trivial, and all of these complex proposals for how it should > > > > work are an artifact of the fact that the kernel-internal interfaces > > > > you're using for it are not well suited to the problem at hand. > > > > > > > > --Andy > > > > > > Sorry, I may not catch your point correctly. Are you trying to tell that > > > a "normal" user can perform efi capsule update. But a "normal" user > > > does not have the right to install or copy the capsule binary into > > > "/lib/firmware/". So, there is a need to make this capsule module to > > > allow uploading the capsule binary at any path or location other than > > > "/lib/firmware/". > > > > > > Is this what you mean? > > > > No. Only root should be able to load capsules, but even root may not > > be able to write to /lib. > > > > --Andy > > > > Okay, I accept that only root user can perform the load capsule. It is new > to me that root user may not have the access right to "/lib/firmware". > > But I remember you do mention that CPU micro code and wifi firmware > they are different and able to write in /lib/firmware. I am curious why > efi capsule binary have such a restriction. What is preventing it being > write to that location? > It has to do with where the firmware comes from. When someone prepares Linux fs image, they put user code, a kernel (usually), all modules that might be needed, and all firmware that's likely to be needed into /. This might come from the distro or a company-wide image. If a normal firmware firmware file needs an update, it's just like updating a driver or a library -- / will be updated by whatever mechanism is in use. Nonvolatile firmware is different. The update isn't needed on subsequent boots, and it could be much less generic than a driver. For example, a capsule could contain a boot splash screen image that says "this is computer #27." Updating the system image to do this makes little sense. Instead it'll be a one-time thing done by root. --Andy > I even went to check out the FHS: > http://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard > I do not find any restriction calling out for that. > > Would you mind to educate me on that? > Thanks. > > > Regards, > Wilson > ^ permalink raw reply [flat|nested] 37+ messages in thread
[parent not found: <F54AEECA5E2B9541821D670476DAE19C2B8AC95C@PGSMSX102.gar.corp.intel.com>]
* RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface [not found] <F54AEECA5E2B9541821D670476DAE19C2B8AC95C@PGSMSX102.gar.corp.intel.com> @ 2015-02-24 12:49 ` Kweh, Hock Leong 2015-02-25 11:47 ` Borislav Petkov 2015-03-06 21:39 ` Peter Jones 0 siblings, 2 replies; 37+ messages in thread From: Kweh, Hock Leong @ 2015-02-24 12:49 UTC (permalink / raw) To: Andy Lutomirski, Sam Protsenko, Matt Fleming, Ming Lei, Greg Kroah-Hartman Cc: Kweh, Hock Leong, Ong, Boon Leong, LKML, linux-efi [-- Attachment #1: Type: text/plain, Size: 5650 bytes --] > -----Original Message----- > From: Kweh, Hock Leong > Sent: Tuesday, February 24, 2015 6:54 PM > > In callbackfn_efi_capsule, you call request_firmware_nowait. When that > callback is invoked, I think that the /sys/class/firmware/efi-capsule-file > directory doesn't exist at all. > If the callback takes longer than it takes your script to make it through a full > iteration, then it will try uploading the second capsule before the firmware > class directory is there, so it will fail. > > But I just realized that your script has a loop below to handle that. > It's this: > > oldtime=$(date +%S) > oldtime=$(((time + 2) % 60)) > until [ -f /sys/class/firmware/efi-capsule-file/loading ] > do > newtime=$(date +%S) > if [ $newtime -eq $oldtime ] > then > break > fi > done > > Aside from the fact that this loop itself is racy (it may loop forever if > something goes wrong in the kernel, since $newtime -eq $oldtime may > never happen), it should help, if you're lucky. But there's another bug. > > > Here's the race: > > User: > echo 1 > /sys/class/firmware/efi-capsule-file/loading > cat capsule1 > /sys/class/firmware/efi-capsule-file/data > echo 0 > /sys/class/firmware/efi-capsule-file/loading > Kernel: Be a little slow here due to preemption or whatever. > > User: > -f /sys/class/firmware/efi-capsule-file/loading returns true capsules_loaded > == 0 Assume failure, incorrectly > > Kernel: catch up and increment capsules_loaded. > > If these patches get applied, then I think that the protocol needs to be > documented in Documentation/ABI. It should say something like: > > To upload an EFI capsule, do this: > > Write 1 to /sys/class/firmware/efi-capsule-file/loading > Write the capsule to /sys/class/firmware/efi-capsule-file/data > Write 0 to /sys/class/firmware/efi-capsule-file/loading > > Make sure that /sys/class/firmware/efi-capsule-file disappears and comes > back, perhaps by cd-ing there and waiting for all the files in the directory to > go away. > > Then, and only then, read capsules_loaded to detect success. > > > Once you've written that doc, please seriously consider whether this > interface is justifiable. I think it sucks. > > --Andy Hi All, After some internal discussion and re-design prototyping & testing on this efi capsule interface kernel module, I would like to start a discussion here on the new idea and wish to get input for the implementation and eventually upstream. This new idea will expose 2 read-only file notes from the efi capsule kernel module - "capsule_ticket" and "capsule_report". The "capsule_ticket" will let user to obtain a NON-ZERO number and then perform a mutex lock. The obtained number will be used later for "capsule_report" status checking. Unlock mutex is done after "echo 0 > loading" at the end of callback function. So the process steps basically look like this: 1.) cat capsule_ticket =======> acquire a number and lock mutex then expose firmware_class user helper interface as well as start timer for timeout counting 2.) repeat step 1 if obtained a "0" number 3.) echo 1 > loading 4.) cat bin > data 5.) echo 0 > loading =======> stop the timeout counting then unlock mutex at the end of callback routine 6.) cat capsule_report =======> grep the number acquired from beginning for checking succeeded/failed The ticket numbering is starting from 1 to 999 and then rolls over from 1 to 999 again. The "capsule_report" will show the whole list of the acquired entries and will be limited to 128 entries which is capped within one page buffer. The format of this "capsule_report" output will look like: "Capsule $num uploads successful/failed/aborted" There is a 2mins time out (internal) for each of the number acquired. After that, the interface will be closed/disappeared. I have attached a SAMPLE script and kernel module code in case you are not able to understand my description above. I believe this would really take care of the multi-capsule update concurrently issue and also asynchronous status reporting issue. Besides, this will indirectly take care the module unload issue with "rmmod" and not "rmmod -f". What do you guys think? ---------------------------------------------------------------------------------------------- There is another idea during internal discussion. The 2nd idea would require some changes to drivers/base/firmware_class.c user helper interface for the mutex locking as well as status return. Mutex lock will be performed while doing the 'echo 1 > loading' session and status return will be performed after the 'echo 0 > loading'. The mutex unlock will be done at 'echo 0 > loading' too or may be at the status reading session for avoid asynchronous reporting. This will likely changed the original firmware class user helper interface design behavior. But this approach could save the 2 file notes from the 1st approach. Which of the approach do you guys prefer? Thanks. Regards, Wilson [-- Attachment #2: newcapsule.txt --] [-- Type: text/plain, Size: 947 bytes --] #!/bin/sh for arg in "$@" do if [ -f $arg ] then retry=0 until [ ! $num = '0' ] do sleep 1 num=$(cat /sys/devices/platform/efi_capsule_user_helper/capsule_ticket) if [ $retry -le 130 ] then retry=$((retry + 1)) else echo "Failed to acquire capsule ticket" exit 1 fi done oldtime=$(date +%S) oldtime=$(((time + 2) % 60)) until [ -f /sys/class/firmware/efi-capsule-file/loading ] do newtime=$(date +%S) if [ $newtime -eq $oldtime ] then echo "Failed to expose user helper interface" exit 1 fi done echo 1 > /sys/class/firmware/efi-capsule-file/loading cat $arg > /sys/class/firmware/efi-capsule-file/data echo 0 > /sys/class/firmware/efi-capsule-file/loading until [ -n "$result" ] do result=$(cat /sys/devices/platform/efi_capsule_user_helper/capsule_report | grep $num) done echo $result " (" $arg ")" else echo "File $arg not found !!" fi done exit 0 [-- Attachment #3: 0002-efi-Capsule-update-with-user-helper-interface.patch --] [-- Type: application/octet-stream, Size: 12646 bytes --] From 7395877e6895231255baab6838d8b92e44f0342b Mon Sep 17 00:00:00 2001 Message-Id: <7395877e6895231255baab6838d8b92e44f0342b.1424244086.git.hock.leong.kweh@intel.com> In-Reply-To: <90701184a919bb58c8c71cd5d9d2e28cc722e5d1.1424244086.git.hock.leong.kweh@intel.com> References: <90701184a919bb58c8c71cd5d9d2e28cc722e5d1.1424244086.git.hock.leong.kweh@intel.com> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com> Date: Tue, 2 Sep 2014 22:10:42 +0800 Subject: [PATCH 2/2] efi: Capsule update with user helper interface Introducing a kernel module to expose user helper interface for user to upload capsule binaries. This module leverage the request_firmware_nowait() to expose an interface to user. Example steps to load the capsule binary: 1.) num=$(cat /sys/devices/platform/efi_capsule_user_helper/capsule_ticket) 2.) continue to acquire the number if the number is '0' 3.) echo 1 > /sys/class/firmware/efi-capsule-file/loading 4.) cat capsule.bin > /sys/class/firmware/efi-capsule-file/data 5.) echo 0 > /sys/class/firmware/efi-capsule-file/loading 6.) cat /sys/devices/platform/efi_capsule_user_helper/capsule_report | grep $num Whereas, this module does not allow the capsule binaries to be obtained from the request_firmware library search path. If any capsule binary loaded from firmware seach path, the module will abort the capsule storing function. Besides the request_firmware user helper interface, this module also exposes two read only file notes to interact with user. - 'capsule_ticket' allows user to acquire a number and enabling firmware_class user helper interface for uploading the capsule binary. The ticket number is starting from 001 to 999. If obtained a number of '0' means someone is performing the loading. Please wait until the loading is done and obtained the number in the correct range then perform to the next steps. A 2 minutes time out is implemented for each loading activity. - 'capsule_report' allows user to verify the status of the capsule loading. User can verify by using the number acquired at the 1st step with 'capsule_ticket'. This design cater the preventing of multi capsules concurrent loading problem as well as the status reporting synchronization. Cc: Matt Fleming <matt.fleming@intel.com> Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com> --- drivers/firmware/efi/Kconfig | 13 + drivers/firmware/efi/Makefile | 1 + drivers/firmware/efi/efi-capsule-user-helper.c | 316 ++++++++++++++++++++++++ 3 files changed, 330 insertions(+) create mode 100644 drivers/firmware/efi/efi-capsule-user-helper.c diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index f712d47..7dc814e 100644 --- a/drivers/firmware/efi/Kconfig +++ b/drivers/firmware/efi/Kconfig @@ -60,6 +60,19 @@ config EFI_RUNTIME_WRAPPERS config EFI_ARMSTUB bool +config EFI_CAPSULE_USER_HELPER + tristate "EFI capsule user mode helper" + depends on EFI + select FW_LOADER + select FW_LOADER_USER_HELPER + help + This option exposes the user mode helper interface for user to load + EFI capsule binary and update the EFI firmware after system reboot. + This feature does not support auto locating capsule binaries at the + firmware lib search path. + + If unsure, say N. + endmenu config UEFI_CPER diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile index 698846e..63f6910 100644 --- a/drivers/firmware/efi/Makefile +++ b/drivers/firmware/efi/Makefile @@ -8,3 +8,4 @@ obj-$(CONFIG_UEFI_CPER) += cper.o obj-$(CONFIG_EFI_RUNTIME_MAP) += runtime-map.o obj-$(CONFIG_EFI_RUNTIME_WRAPPERS) += runtime-wrappers.o obj-$(CONFIG_EFI_STUB) += libstub/ +obj-$(CONFIG_EFI_CAPSULE_USER_HELPER) += efi-capsule-user-helper.o diff --git a/drivers/firmware/efi/efi-capsule-user-helper.c b/drivers/firmware/efi/efi-capsule-user-helper.c new file mode 100644 index 0000000..05838f5 --- /dev/null +++ b/drivers/firmware/efi/efi-capsule-user-helper.c @@ -0,0 +1,316 @@ +/* + * EFI capsule user mode helper interface driver. + * + * Copyright 2015 Intel Corporation + * + * This file is part of the Linux kernel, and is made available under + * the terms of the GNU General Public License version 2. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/delay.h> +#include <linux/highmem.h> +#include <linux/slab.h> +#include <linux/mutex.h> +#include <linux/reboot.h> +#include <linux/efi.h> +#include <linux/firmware.h> +#include <linux/workqueue.h> + +#define CAPSULE_NAME "efi-capsule-file" +#define DEV_NAME "efi_capsule_user_helper" +#define STRING_INTEGER_MAX_LENGTH 13 +#define MAX_SYSFS_BUF_LENGTH 4096 +#define CAPSULE_USER_HELPER_TIMEOUT 120 +#define CAPSULE_REPORT_MAX_STR_SIZE 32 +#define CAPSULE_RESULT_PASS "successful" +#define CAPSULE_RESULT_FAIL "failed " +#define CAPSULE_RESULT_EXIT "aborted " + +static DEFINE_MUTEX(user_helper_lock); +static int capsule_count; +static struct platform_device *efi_capsule_pdev; +static struct delayed_work timeout_work; +static unsigned char *report_buf; +static int report_buf_head_off, report_buf_write_off; + +/* + * This function will store the capsule binary and pass it to + * efi_capsule_update() API in capsule.c + */ +static int efi_capsule_store(const struct firmware *fw) +{ + int i; + int ret; + int count = fw->size; + int copy_size = (fw->size > PAGE_SIZE) ? PAGE_SIZE : fw->size; + int pages_needed = ALIGN(fw->size, PAGE_SIZE) >> PAGE_SHIFT; + struct page **pages; + void *page_data; + efi_capsule_header_t *capsule = NULL; + + pages = kmalloc_array(pages_needed, sizeof(void *), GFP_KERNEL); + if (!pages) { + pr_err("%s: kmalloc_array() failed\n", __func__); + return -ENOMEM; + } + + for (i = 0; i < pages_needed; i++) { + pages[i] = alloc_page(GFP_KERNEL); + if (!pages[i]) { + pr_err("%s: alloc_page() failed\n", __func__); + --i; + ret = -ENOMEM; + goto failed; + } + } + + for (i = 0; i < pages_needed; i++) { + page_data = kmap(pages[i]); + memcpy(page_data, fw->data + (i * PAGE_SIZE), copy_size); + + if (i == 0) + capsule = (efi_capsule_header_t *)page_data; + else + kunmap(pages[i]); + + count -= copy_size; + if (count < PAGE_SIZE) + copy_size = count; + } + + ret = efi_capsule_update(capsule, pages); + if (ret) { + pr_err("%s: efi_capsule_update() failed\n", __func__); + --i; + goto failed; + } + kunmap(pages[0]); + + /* + * we cannot free the pages here due to reboot need that data + * maintained. + */ + return 0; + +failed: + while (i >= 0) + __free_page(pages[i--]); + kfree(pages); + return ret; +} + +/* + * This callback function will be called by request_firmware_nowait() when + * user has loaded the capsule binary or aborted user helper interface + */ +static void callbackfn_efi_capsule(const struct firmware *fw, void *context) +{ + int ret; + + if (fw) { + /* + * Binary which is not getting from user helper interface, + * the fw->pages is equal to NULL + */ + if (!fw->pages) { + pr_err("%s: ERROR: Capsule binary '%s' at %s\n", + __func__, CAPSULE_NAME, + "firmware lib search path are not supported"); + pr_err("user helper interface disabled\n"); + snprintf(report_buf + report_buf_write_off, + CAPSULE_REPORT_MAX_STR_SIZE, + "Capsule %03d uploads %s\n", + capsule_count, + CAPSULE_RESULT_FAIL); + } else { + ret = efi_capsule_store(fw); + if (!ret) { + snprintf(report_buf + report_buf_write_off, + CAPSULE_REPORT_MAX_STR_SIZE, + "Capsule %03d uploads %s\n", + capsule_count, + CAPSULE_RESULT_PASS); + } else { + pr_err("%s: %s %03d, return error code = %d\n", + __func__, + "Failed to store capsule binary", + capsule_count, ret); + snprintf(report_buf + report_buf_write_off, + CAPSULE_REPORT_MAX_STR_SIZE, + "Capsule %03d uploads %s\n", + capsule_count, + CAPSULE_RESULT_FAIL); + } + } + release_firmware(fw); + } else { + snprintf(report_buf + report_buf_write_off, + CAPSULE_REPORT_MAX_STR_SIZE, + "Capsule %03d uploads %s\n", + capsule_count, + CAPSULE_RESULT_EXIT); + } + + report_buf_write_off += CAPSULE_REPORT_MAX_STR_SIZE; + report_buf_write_off %= PAGE_SIZE; + + cancel_delayed_work_sync(&timeout_work); + mutex_unlock(&user_helper_lock); +} + +static void capsule_user_helper_timeout_work(struct work_struct *work) +{ + request_firmware_abort(CAPSULE_NAME); +} + +static ssize_t capsule_ticket_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int ret; + + if (mutex_trylock(&user_helper_lock)) { + /* + * Use request_firmware_nowait() to expose an user helper + * interface for obtaining the capsule binary from user space + */ + ret = request_firmware_nowait(THIS_MODULE, FW_ACTION_NOHOTPLUG, + CAPSULE_NAME, + &efi_capsule_pdev->dev, + GFP_KERNEL, NULL, + callbackfn_efi_capsule); + if (ret) { + pr_err("%s: request_firmware_nowait() failed\n", + __func__); + mutex_unlock(&user_helper_lock); + return snprintf(buf, STRING_INTEGER_MAX_LENGTH, "0\n"); + } + + queue_delayed_work(system_power_efficient_wq, + &timeout_work, + CAPSULE_USER_HELPER_TIMEOUT * HZ); + + ++capsule_count; + capsule_count %= 1000 ? : ++capsule_count; + + return snprintf(buf, STRING_INTEGER_MAX_LENGTH, "%03d\n", + capsule_count); + } else { + return snprintf(buf, STRING_INTEGER_MAX_LENGTH, "0\n"); + } +} + +static DEVICE_ATTR_RO(capsule_ticket); + +static ssize_t capsule_report_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int length = capsule_count * CAPSULE_REPORT_MAX_STR_SIZE; + + length = length > PAGE_SIZE ? PAGE_SIZE : length; + memcpy(buf, report_buf + report_buf_head_off, length); + return length; +} + +static DEVICE_ATTR_RO(capsule_report); + +/* reboot notifier for avoid deadlock with usermode_lock */ +static int capsule_shutdown_notify(struct notifier_block *nb, + unsigned long sys_state, + void *reboot_cmd) +{ + request_firmware_abort(CAPSULE_NAME); + return NOTIFY_DONE; +} + +static struct notifier_block capsule_shutdown_nb = { + .notifier_call = capsule_shutdown_notify, + /* + * In order to reboot properly, it is required to ensure the priority + * here is higher than firmware_class fw_shutdown_nb priority + */ + .priority = 1, +}; + +static int __init efi_capsule_user_helper_init(void) +{ + int ret; + + efi_capsule_pdev = platform_device_register_simple(DEV_NAME, + PLATFORM_DEVID_NONE, + NULL, 0); + if (IS_ERR(efi_capsule_pdev)) { + pr_err("%s: platform_device_register_simple() failed\n", + __func__); + return PTR_ERR(efi_capsule_pdev); + } + + report_buf = devm_kzalloc(&efi_capsule_pdev->dev, PAGE_SIZE, + GFP_KERNEL); + if (!report_buf) { + ret = -ENOMEM; + pr_err("%s: devm_kzalloc() failed\n", __func__); + goto out; + } + + /* + * create this file node for user to acquire a ticket number and get + * ready to upload capsule binaries + */ + ret = device_create_file(&efi_capsule_pdev->dev, + &dev_attr_capsule_ticket); + if (ret) { + pr_err("%s: device_create_file() failed\n", __func__); + goto out; + } + + /* + * create this file node for user to check the capsule binaries upload + * status + */ + ret = device_create_file(&efi_capsule_pdev->dev, + &dev_attr_capsule_report); + if (ret) { + pr_err("%s: device_create_file() failed\n", __func__); + goto out; + } + + INIT_DELAYED_WORK(&timeout_work, capsule_user_helper_timeout_work); + register_reboot_notifier(&capsule_shutdown_nb); + return 0; + +out: + platform_device_unregister(efi_capsule_pdev); + return ret; +} +module_init(efi_capsule_user_helper_init); + +/* + * To remove this kernel module, just perform: + * rmmod efi_capsule_user_helper.ko + * + * rmmod -f efi_capsule_user_helper.ko is NOT recommended and NOT supported + * by this module design. Any force unload may cause the system crash. + */ +static void __exit efi_capsule_user_helper_exit(void) +{ + unregister_reboot_notifier(&capsule_shutdown_nb); + + // request_firmware_abort(CAPSULE_NAME); + // /* + // * synchronization is needed to make sure request_firmware is fully + // * aborted + // */ + // while (efi_capsule_pdev->dev.kobj.kref.refcount.counter > 3) + // msleep(20); /* avoid busy waiting for cooperative kernel */ + + platform_device_unregister(efi_capsule_pdev); +} +module_exit(efi_capsule_user_helper_exit); + +MODULE_DESCRIPTION("EFI Capsule user helper binary load utility"); +MODULE_LICENSE("GPL v2"); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-02-24 12:49 ` Kweh, Hock Leong @ 2015-02-25 11:47 ` Borislav Petkov 2015-02-25 12:38 ` Kweh, Hock Leong 2015-02-26 15:30 ` Andy Lutomirski 2015-03-06 21:39 ` Peter Jones 1 sibling, 2 replies; 37+ messages in thread From: Borislav Petkov @ 2015-02-25 11:47 UTC (permalink / raw) To: Kweh, Hock Leong Cc: Andy Lutomirski, Sam Protsenko, Matt Fleming, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Tue, Feb 24, 2015 at 12:49:09PM +0000, Kweh, Hock Leong wrote: > So the process steps basically look like this: > 1.) cat capsule_ticket =======> acquire a number and lock mutex then > expose firmware_class user helper > interface as well as start timer for timeout > counting > 2.) repeat step 1 if obtained a "0" number > 3.) echo 1 > loading > 4.) cat bin > data > 5.) echo 0 > loading =======> stop the timeout counting then unlock > mutex at the end of callback routine > 6.) cat capsule_report =======> grep the number acquired from beginning > for checking succeeded/failed So this sounds pretty overengineered for no reason, or maybe I'm missing the reason. If I had to give an example from the microcode loader, what we do there is put the microcode in /lib/firmware/... and do echo 1 > /sys/devices/system/cpu/microcode/reload which goes and calls reload_store() in arch/x86/kernel/cpu/microcode/core.c which grabs a mutex, disables CPU hotplug, etc, etc... And this mechanism is as simple as it can get. Maybe capsules can be loaded like that too? Error code can be propagated too, if needed, of course. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-02-25 11:47 ` Borislav Petkov @ 2015-02-25 12:38 ` Kweh, Hock Leong 2015-02-25 12:49 ` Borislav Petkov 2015-02-26 15:30 ` Andy Lutomirski 1 sibling, 1 reply; 37+ messages in thread From: Kweh, Hock Leong @ 2015-02-25 12:38 UTC (permalink / raw) To: Borislav Petkov Cc: Andy Lutomirski, Sam Protsenko, Matt Fleming, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1201 bytes --] > -----Original Message----- > From: Borislav Petkov [mailto:bp@alien8.de] > Sent: Wednesday, February 25, 2015 7:48 PM > > On Tue, Feb 24, 2015 at 12:49:09PM +0000, Kweh, Hock Leong wrote: > > So this sounds pretty overengineered for no reason, or maybe I'm missing > the reason. > > If I had to give an example from the microcode loader, what we do there > is put the microcode in /lib/firmware/... and do > > echo 1 > /sys/devices/system/cpu/microcode/reload > > which goes and calls reload_store() in > arch/x86/kernel/cpu/microcode/core.c which grabs a mutex, disables CPU > hotplug, etc, etc... > > And this mechanism is as simple as it can get. Maybe capsules can be > loaded like that too? > > Error code can be propagated too, if needed, of course. > > -- > Regards/Gruss, > Boris. > > ECO tip #101: Trim your mails when you reply. > -- Hi Boris, The reason we use this interface for efi capsule is that efi capsule support multi binaries to be uploaded and each binary file name can be different. Regards, Wilson ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-02-25 12:38 ` Kweh, Hock Leong @ 2015-02-25 12:49 ` Borislav Petkov 0 siblings, 0 replies; 37+ messages in thread From: Borislav Petkov @ 2015-02-25 12:49 UTC (permalink / raw) To: Kweh, Hock Leong Cc: Andy Lutomirski, Sam Protsenko, Matt Fleming, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Wed, Feb 25, 2015 at 12:38:20PM +0000, Kweh, Hock Leong wrote: > The reason we use this interface for efi capsule is that efi capsule > support multi binaries to be uploaded and each binary file name > can be different. So you can write the file path to a second file and reload then, once per file. Alternatively, you can combine all the files into a cpio archive like we do with the microcode loader too, and let the kernel parse the cpio archive. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-02-25 11:47 ` Borislav Petkov 2015-02-25 12:38 ` Kweh, Hock Leong @ 2015-02-26 15:30 ` Andy Lutomirski 2015-02-26 15:54 ` Borislav Petkov 1 sibling, 1 reply; 37+ messages in thread From: Andy Lutomirski @ 2015-02-26 15:30 UTC (permalink / raw) To: Borislav Petkov Cc: Kweh, Hock Leong, Sam Protsenko, Matt Fleming, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Wed, Feb 25, 2015 at 3:47 AM, Borislav Petkov <bp@alien8.de> wrote: > On Tue, Feb 24, 2015 at 12:49:09PM +0000, Kweh, Hock Leong wrote: >> So the process steps basically look like this: >> 1.) cat capsule_ticket =======> acquire a number and lock mutex then >> expose firmware_class user helper >> interface as well as start timer for timeout >> counting >> 2.) repeat step 1 if obtained a "0" number >> 3.) echo 1 > loading >> 4.) cat bin > data >> 5.) echo 0 > loading =======> stop the timeout counting then unlock >> mutex at the end of callback routine >> 6.) cat capsule_report =======> grep the number acquired from beginning >> for checking succeeded/failed > > So this sounds pretty overengineered for no reason, or maybe I'm missing > the reason. > > If I had to give an example from the microcode loader, what we do there > is put the microcode in /lib/firmware/... and do > > echo 1 > /sys/devices/system/cpu/microcode/reload > > which goes and calls reload_store() in > arch/x86/kernel/cpu/microcode/core.c which grabs a mutex, disables CPU > hotplug, etc, etc... > > And this mechanism is as simple as it can get. Maybe capsules can be > loaded like that too? > > Error code can be propagated too, if needed, of course. How can the error code be propagated? Would that echo command fail in case of error? --Andy ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-02-26 15:30 ` Andy Lutomirski @ 2015-02-26 15:54 ` Borislav Petkov 2015-03-02 11:24 ` Matt Fleming 0 siblings, 1 reply; 37+ messages in thread From: Borislav Petkov @ 2015-02-26 15:54 UTC (permalink / raw) To: Andy Lutomirski Cc: Kweh, Hock Leong, Sam Protsenko, Matt Fleming, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Thu, Feb 26, 2015 at 07:30:54AM -0800, Andy Lutomirski wrote: > How can the error code be propagated? Would that echo command fail in > case of error? Yeah, either that or we can put the error code in the sysfs file which userspace can cat. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-02-26 15:54 ` Borislav Petkov @ 2015-03-02 11:24 ` Matt Fleming 0 siblings, 0 replies; 37+ messages in thread From: Matt Fleming @ 2015-03-02 11:24 UTC (permalink / raw) To: Borislav Petkov Cc: Andy Lutomirski, Kweh, Hock Leong, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Thu, 26 Feb, at 04:54:58PM, Borislav Petkov wrote: > On Thu, Feb 26, 2015 at 07:30:54AM -0800, Andy Lutomirski wrote: > > How can the error code be propagated? Would that echo command fail in > > case of error? > > Yeah, either that or we can put the error code in the sysfs file which > userspace can cat. FWIW, I'd prefer to make the echo command fail in case of capsule error. -- Matt Fleming, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-02-24 12:49 ` Kweh, Hock Leong 2015-02-25 11:47 ` Borislav Petkov @ 2015-03-06 21:39 ` Peter Jones 2015-03-06 21:49 ` Roy Franz 2015-03-10 12:26 ` Matt Fleming 1 sibling, 2 replies; 37+ messages in thread From: Peter Jones @ 2015-03-06 21:39 UTC (permalink / raw) To: Kweh, Hock Leong Cc: Andy Lutomirski, Sam Protsenko, Matt Fleming, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Tue, Feb 24, 2015 at 12:49:09PM +0000, Kweh, Hock Leong wrote: > Hi All, > > After some internal discussion and re-design prototyping & testing on > this efi capsule interface kernel module, I would like to start a discussion > here on the new idea and wish to get input for the implementation and > eventually upstream. So do we actually *need* a kernel interface to UpdateCapsule? We've already got an implementation in progress where we use my ESRT patch to figure out what firmwares we can update, and we schedule them and do the update during boot up before the normal bootloader is run. That means never having to call UpdateCapsule() after ExitBootServices() or SetVirtualAddressMap() have been called, and only doing it across reboots that UEFI is performing itself. It also means never having to do it with multiple CPUs running. I've posted several revisions of the ESRT patch to linux-efi , and right now we're just waiting on the UEFI 2.5 spec to be finalized before I send a final copy to Matt. The command line tool and the code to apply the firmwares during boot are at https://github.com/rhinstaller/fwupdate and there's a GNOME-based UI in progress at https://github.com/hughsie/fwupd (yes we apparently stink at naming things.) I'm not sure how making this go through the kernel will make things better - it introduces a lot more things that can go wrong, and the only benefit is one reboot where BootNext is set - which actually is relatively fast even slow-POSTing machines. After that the UpdateCapsule+reboot to apply is *extremely* fast, because applying capsule updates have to happen very very early in the boot-up sequence. (That early processing is /effectively/ a requirement, since it has to happen before the block write locking on the SPI part is done.) So even on the machine that takes quite some time to POST, the time that would be saved saved is less than 1% or so of the total update time. An early version of my code worked to update a machine I got from your employer, FWIW - right now we're adding API and fixing up implementation bits I made specifically to that one proof-of-concept scenario, and there's some API parts that are in UEFI 2.5 draft releases that we're not quite handling yet. However, there's code there that's already been checked in which has successfully applied system firmware updates without having a kernel interface to UpdateCapsule(). So again: do we really need or want to do this? -- Peter ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-06 21:39 ` Peter Jones @ 2015-03-06 21:49 ` Roy Franz 2015-03-06 22:17 ` Peter Jones 2015-03-10 12:26 ` Matt Fleming 1 sibling, 1 reply; 37+ messages in thread From: Roy Franz @ 2015-03-06 21:49 UTC (permalink / raw) To: Peter Jones Cc: Kweh, Hock Leong, Andy Lutomirski, Sam Protsenko, Matt Fleming, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Fri, Mar 6, 2015 at 1:39 PM, Peter Jones <pjones@redhat.com> wrote: > On Tue, Feb 24, 2015 at 12:49:09PM +0000, Kweh, Hock Leong wrote: >> Hi All, >> >> After some internal discussion and re-design prototyping & testing on >> this efi capsule interface kernel module, I would like to start a discussion >> here on the new idea and wish to get input for the implementation and >> eventually upstream. > > So do we actually *need* a kernel interface to UpdateCapsule? We've > already got an implementation in progress where we use my ESRT patch to > figure out what firmwares we can update, and we schedule them and do the > update during boot up before the normal bootloader is run. That means > never having to call UpdateCapsule() after ExitBootServices() or > SetVirtualAddressMap() have been called, and only doing it across > reboots that UEFI is performing itself. It also means never having to > do it with multiple CPUs running. So this does it by writing the capsules to the EFI system partition, and having them processed from there during the next boot? How would this work on diskless systems? (or boot-disk-less systems) What are the use cases for capsules that don't require a reboot? I'm not really clear uses for those, but the kernel interface could be better for those, as it would allow processing without a reboot. Roy > > I've posted several revisions of the ESRT patch to linux-efi , and right > now we're just waiting on the UEFI 2.5 spec to be finalized before I > send a final copy to Matt. The command line tool and the code to apply > the firmwares during boot are at https://github.com/rhinstaller/fwupdate > and there's a GNOME-based UI in progress at > https://github.com/hughsie/fwupd (yes we apparently stink at naming > things.) > > I'm not sure how making this go through the kernel will make things > better - it introduces a lot more things that can go wrong, and the only > benefit is one reboot where BootNext is set - which actually is > relatively fast even slow-POSTing machines. After that the > UpdateCapsule+reboot to apply is *extremely* fast, because > applying capsule updates have to happen very very early in the boot-up > sequence. (That early processing is /effectively/ a requirement, > since it has to happen before the block write locking on the SPI part is > done.) So even on the machine that takes quite some time to POST, the > time that would be saved saved is less than 1% or so of the total update > time. > > An early version of my code worked to update a machine I got from your > employer, FWIW - right now we're adding API and fixing up implementation > bits I made specifically to that one proof-of-concept scenario, and > there's some API parts that are in UEFI 2.5 draft releases that we're > not quite handling yet. However, there's code there that's already been > checked in which has successfully applied system firmware updates > without having a kernel interface to UpdateCapsule(). > > So again: do we really need or want to do this? > > -- > Peter > -- > To unsubscribe from this list: send the line "unsubscribe linux-efi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-06 21:49 ` Roy Franz @ 2015-03-06 22:17 ` Peter Jones 0 siblings, 0 replies; 37+ messages in thread From: Peter Jones @ 2015-03-06 22:17 UTC (permalink / raw) To: Roy Franz Cc: Kweh, Hock Leong, Andy Lutomirski, Sam Protsenko, Matt Fleming, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Fri, Mar 06, 2015 at 01:49:20PM -0800, Roy Franz wrote: > On Fri, Mar 6, 2015 at 1:39 PM, Peter Jones <pjones@redhat.com> wrote: > > On Tue, Feb 24, 2015 at 12:49:09PM +0000, Kweh, Hock Leong wrote: > >> Hi All, > >> > >> After some internal discussion and re-design prototyping & testing on > >> this efi capsule interface kernel module, I would like to start a discussion > >> here on the new idea and wish to get input for the implementation and > >> eventually upstream. > > > > So do we actually *need* a kernel interface to UpdateCapsule? We've > > already got an implementation in progress where we use my ESRT patch to > > figure out what firmwares we can update, and we schedule them and do the > > update during boot up before the normal bootloader is run. That means > > never having to call UpdateCapsule() after ExitBootServices() or > > SetVirtualAddressMap() have been called, and only doing it across > > reboots that UEFI is performing itself. It also means never having to > > do it with multiple CPUs running. > > So this does it by writing the capsules to the EFI system partition, and having > them processed from there during the next boot? > How would this work on diskless systems? (or boot-disk-less systems) One of the things I'm currently writing is making the file we load be specified correctly by UEFI device paths - and that'll include the ability to get it from devices presented by the network drivers. On currently extant test machines that includes tftp support, just like netbooting. On UEFI 2.5 machines, where ESRT is introduced so that we actually know something about the capsules we can apply updates for, it also includes http support. Admittedly that means when you're doing deployment you'll have to have some process for putting the images someplace, but it can be the same device you're net-booting from. Just one example. If we're talking about systems that are booting entirely out of their own firmware volume, there's definitely some legitimate argument that doing this without an ESP could be valuable - so yeah, a kernel API in that case might be worthwhile. That said, the extra complexity combined with the fact that it's running after ExitBootServices() and SetVirtualAddressMap() means I'll probably avoid using it from the userland code except on machines where there are no other options. My faith that we're going to see a lot of machines where that's well tested without our address maps looking exactly like Windows's isn't very high, and we've repeatedly run into a lot of pain with that in the past. That's not the only thing that has to be exactly right, either - for instance there's no guarantee it'll work if you use the ACPI reset vector instead of the EFI runtime services ResetSystem(EfiResetWarm) call. Right now we use the ACPI method preferentially because of SetVirtualAddressMap() not working as documented on so *many* platforms. That means what happens upon reboot when we do UpdateCapsule() is undefined behavior. Of course I'm likely not the only person who will ever implement tools in userland to orchestrate firmware updates, either :) > What are the use cases for capsules that don't require a reboot? I'm > not really clear uses for those, but the kernel interface could be > better for those, as it would allow processing without a reboot. I'm really not sure if we know the answer to that yet. Things like USB devices most often won't ever be registered with firmware's FMP anyway, so they won't have capsules exposed, and they'll use more traditional USB commands to do firmware updates - stuff like hughsie's ColorHug device are in that category, and he's already supporting that with fwupd. Things like peripheral card option ROMs are potentially in that category, though I'm a little skeptical of the idea that I actually want to update the firmware on my video or network card with the kernel already running, and that when I do I'm not going to want to reboot immediately to make sure it worked right anyway. There are almost certainly lots of cases I haven't thought of, though. If we want this interface for those cases, I think we should also be enumerating the cases we think it's supporting, as well, even if only in broad strokes. I don't think we need to say "support this specific device's updates", but keeping track of the basic model of the update we're supporting would go a long way to establishing the value of supporting all the complexity. -- Peter ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-06 21:39 ` Peter Jones 2015-03-06 21:49 ` Roy Franz @ 2015-03-10 12:26 ` Matt Fleming 2015-03-10 15:21 ` Peter Jones 1 sibling, 1 reply; 37+ messages in thread From: Matt Fleming @ 2015-03-10 12:26 UTC (permalink / raw) To: Peter Jones Cc: Kweh, Hock Leong, Andy Lutomirski, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Fri, 06 Mar, at 04:39:12PM, Peter Jones wrote: > > So again: do we really need or want to do this? One thing that we totally lose the ability to do is use the capsule interface for things *other* than firmware updates, e.g. https://lkml.org/lkml/2013/10/16/327 Also, requiring embedded or custom OS to include fwupdate into their existing boot solutions is a bit heavy handed when literally all they want to do is cat a binary blob to a sysfs file. I don't see why we can't have both solutions. Another thing is that ESRT isn't going to be supported by every platform. So, for the sysfs interface, let's not allow loading from /lib. Let's not require a userland tool. Let's just do, # echo /path/to/my/awesome/capsule.bin > /sys/../capsule and be done with it. Hmmm? -- Matt Fleming, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-10 12:26 ` Matt Fleming @ 2015-03-10 15:21 ` Peter Jones 2015-03-10 15:26 ` Andy Lutomirski 0 siblings, 1 reply; 37+ messages in thread From: Peter Jones @ 2015-03-10 15:21 UTC (permalink / raw) To: Matt Fleming Cc: Kweh, Hock Leong, Andy Lutomirski, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Tue, Mar 10, 2015 at 12:26:52PM +0000, Matt Fleming wrote: > On Fri, 06 Mar, at 04:39:12PM, Peter Jones wrote: > > > > So again: do we really need or want to do this? > > One thing that we totally lose the ability to do is use the capsule > interface for things *other* than firmware updates, e.g. > > https://lkml.org/lkml/2013/10/16/327 > > Also, requiring embedded or custom OS to include fwupdate into their > existing boot solutions is a bit heavy handed when literally all they > want to do is cat a binary blob to a sysfs file. > > I don't see why we can't have both solutions. Yeah - we clearly need a kernel interface for some embedded devices, and it'd be better for every vendor to not implement it themselves. > Another thing is that ESRT isn't going to be supported by every > platform. Yeah - though I think you're *mostly* talking about the same platforms as above. > So, for the sysfs interface, let's not allow loading from /lib. Let's > not require a userland tool. Let's just do, > > # echo /path/to/my/awesome/capsule.bin > /sys/../capsule > > and be done with it. > > Hmmm? I assume you're implying a) the capsule header with the guid is embedded in the .bin there already, and b) one contiguous write(2) with error reporting coming through something like vars.c's efi_status_to_err()? If so, yes, I prefer this API. -- Peter ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-10 15:21 ` Peter Jones @ 2015-03-10 15:26 ` Andy Lutomirski 2015-03-10 15:40 ` Peter Jones 0 siblings, 1 reply; 37+ messages in thread From: Andy Lutomirski @ 2015-03-10 15:26 UTC (permalink / raw) To: Peter Jones Cc: Matt Fleming, Kweh, Hock Leong, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Tue, Mar 10, 2015 at 8:21 AM, Peter Jones <pjones@redhat.com> wrote: > On Tue, Mar 10, 2015 at 12:26:52PM +0000, Matt Fleming wrote: >> On Fri, 06 Mar, at 04:39:12PM, Peter Jones wrote: >> > >> > So again: do we really need or want to do this? >> >> One thing that we totally lose the ability to do is use the capsule >> interface for things *other* than firmware updates, e.g. >> >> https://lkml.org/lkml/2013/10/16/327 >> >> Also, requiring embedded or custom OS to include fwupdate into their >> existing boot solutions is a bit heavy handed when literally all they >> want to do is cat a binary blob to a sysfs file. >> >> I don't see why we can't have both solutions. > > Yeah - we clearly need a kernel interface for some embedded devices, and > it'd be better for every vendor to not implement it themselves. > >> Another thing is that ESRT isn't going to be supported by every >> platform. > > Yeah - though I think you're *mostly* talking about the same platforms > as above. > >> So, for the sysfs interface, let's not allow loading from /lib. Let's >> not require a userland tool. Let's just do, >> >> # echo /path/to/my/awesome/capsule.bin > /sys/../capsule > >> >> and be done with it. >> >> Hmmm? > > I assume you're implying a) the capsule header with the guid is embedded > in the .bin there already, and b) one contiguous write(2) with error > reporting coming through something like vars.c's efi_status_to_err()? > > If so, yes, I prefer this API. > Is using a char device really so bad? I have a "simple_char" that makes this really easy that's pending review. --Andy > -- > Peter -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-10 15:26 ` Andy Lutomirski @ 2015-03-10 15:40 ` Peter Jones 2015-03-10 15:51 ` Andy Lutomirski 0 siblings, 1 reply; 37+ messages in thread From: Peter Jones @ 2015-03-10 15:40 UTC (permalink / raw) To: Andy Lutomirski Cc: Matt Fleming, Kweh, Hock Leong, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi > >> So, for the sysfs interface, let's not allow loading from /lib. Let's > >> not require a userland tool. Let's just do, > >> > >> # echo /path/to/my/awesome/capsule.bin > /sys/../capsule > > > >> > >> and be done with it. > >> > >> Hmmm? > > > > I assume you're implying a) the capsule header with the guid is embedded > > in the .bin there already, and b) one contiguous write(2) with error > > reporting coming through something like vars.c's efi_status_to_err()? > > > > If so, yes, I prefer this API. > > > > Is using a char device really so bad? I have a "simple_char" that > makes this really easy that's pending review. As long as there's straightforward propagation of the EFI_STATUS return from UpdateCapsule() back, sysfs file vs char device makes very little difference to me. Either way it's open(), write(), close(). Using the runtime firmware upload interface designed for wifi and scsi devices is the part I don't really like. -- Peter ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-10 15:40 ` Peter Jones @ 2015-03-10 15:51 ` Andy Lutomirski 2015-03-10 17:26 ` Peter Jones 2015-03-12 22:47 ` Matt Fleming 0 siblings, 2 replies; 37+ messages in thread From: Andy Lutomirski @ 2015-03-10 15:51 UTC (permalink / raw) To: Peter Jones Cc: Matt Fleming, Kweh, Hock Leong, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Tue, Mar 10, 2015 at 8:40 AM, Peter Jones <pjones@redhat.com> wrote: > >> >> So, for the sysfs interface, let's not allow loading from /lib. Let's >> >> not require a userland tool. Let's just do, >> >> >> >> # echo /path/to/my/awesome/capsule.bin > /sys/../capsule >> > >> >> >> >> and be done with it. >> >> >> >> Hmmm? >> > >> > I assume you're implying a) the capsule header with the guid is embedded >> > in the .bin there already, and b) one contiguous write(2) with error >> > reporting coming through something like vars.c's efi_status_to_err()? >> > >> > If so, yes, I prefer this API. >> > >> >> Is using a char device really so bad? I have a "simple_char" that >> makes this really easy that's pending review. > > As long as there's straightforward propagation of the EFI_STATUS return > from UpdateCapsule() back, sysfs file vs char device makes very little > difference to me. Either way it's open(), write(), close(). Using the > runtime firmware upload interface designed for wifi and scsi devices is > the part I don't really like. > I'm not 100% happy with write(2) (which is all we have in sysfs) for two reasons: 1. If we write a file name, eww. That's more complicated, requires temporary files, has annoying mount namespace issues, etc. 2. If we write the full contents, we need to do it in a single call to write. That means that we can't use cat, which mostly defeats the purpose. In fact, using cat could be actively harmful. --Andy ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-10 15:51 ` Andy Lutomirski @ 2015-03-10 17:26 ` Peter Jones 2015-03-10 17:31 ` Andy Lutomirski 2015-03-12 22:47 ` Matt Fleming 1 sibling, 1 reply; 37+ messages in thread From: Peter Jones @ 2015-03-10 17:26 UTC (permalink / raw) To: Andy Lutomirski Cc: Matt Fleming, Kweh, Hock Leong, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Tue, Mar 10, 2015 at 08:51:59AM -0700, Andy Lutomirski wrote: > On Tue, Mar 10, 2015 at 8:40 AM, Peter Jones <pjones@redhat.com> wrote: > > > >> >> So, for the sysfs interface, let's not allow loading from /lib. Let's > >> >> not require a userland tool. Let's just do, > >> >> > >> >> # echo /path/to/my/awesome/capsule.bin > /sys/../capsule > >> > > >> >> > >> >> and be done with it. > >> >> > >> >> Hmmm? > >> > > >> > I assume you're implying a) the capsule header with the guid is embedded > >> > in the .bin there already, and b) one contiguous write(2) with error > >> > reporting coming through something like vars.c's efi_status_to_err()? > >> > > >> > If so, yes, I prefer this API. > >> > > >> > >> Is using a char device really so bad? I have a "simple_char" that > >> makes this really easy that's pending review. > > > > As long as there's straightforward propagation of the EFI_STATUS return > > from UpdateCapsule() back, sysfs file vs char device makes very little > > difference to me. Either way it's open(), write(), close(). Using the > > runtime firmware upload interface designed for wifi and scsi devices is > > the part I don't really like. > > > > I'm not 100% happy with write(2) (which is all we have in sysfs) for > two reasons: > > 1. If we write a file name, eww. That's more complicated, requires > temporary files, has annoying mount namespace issues, etc. > > 2. If we write the full contents, we need to do it in a single call to > write. That means that we can't use cat, which mostly defeats the > purpose. In fact, using cat could be actively harmful. So if what we wind up with is: fd = open("/sys/.../capsule", O_RDWR); write(fd, buf, size/N); ... write(fd, buf + M*size/N, size/N); close(fd); You're suggesting the error code would post on close()? My worry about that is that I imagine a lot less code in the wild checks the error code on close() than on write() - though gnu cat does do so on both. But there are other questions still - will it post on fdatasync()? On fsync()? -- Peter ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-10 17:26 ` Peter Jones @ 2015-03-10 17:31 ` Andy Lutomirski 0 siblings, 0 replies; 37+ messages in thread From: Andy Lutomirski @ 2015-03-10 17:31 UTC (permalink / raw) To: Peter Jones Cc: Matt Fleming, Kweh, Hock Leong, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Tue, Mar 10, 2015 at 10:26 AM, Peter Jones <pjones@redhat.com> wrote: > On Tue, Mar 10, 2015 at 08:51:59AM -0700, Andy Lutomirski wrote: >> On Tue, Mar 10, 2015 at 8:40 AM, Peter Jones <pjones@redhat.com> wrote: >> > >> >> >> So, for the sysfs interface, let's not allow loading from /lib. Let's >> >> >> not require a userland tool. Let's just do, >> >> >> >> >> >> # echo /path/to/my/awesome/capsule.bin > /sys/../capsule >> >> > >> >> >> >> >> >> and be done with it. >> >> >> >> >> >> Hmmm? >> >> > >> >> > I assume you're implying a) the capsule header with the guid is embedded >> >> > in the .bin there already, and b) one contiguous write(2) with error >> >> > reporting coming through something like vars.c's efi_status_to_err()? >> >> > >> >> > If so, yes, I prefer this API. >> >> > >> >> >> >> Is using a char device really so bad? I have a "simple_char" that >> >> makes this really easy that's pending review. >> > >> > As long as there's straightforward propagation of the EFI_STATUS return >> > from UpdateCapsule() back, sysfs file vs char device makes very little >> > difference to me. Either way it's open(), write(), close(). Using the >> > runtime firmware upload interface designed for wifi and scsi devices is >> > the part I don't really like. >> > >> >> I'm not 100% happy with write(2) (which is all we have in sysfs) for >> two reasons: >> >> 1. If we write a file name, eww. That's more complicated, requires >> temporary files, has annoying mount namespace issues, etc. >> >> 2. If we write the full contents, we need to do it in a single call to >> write. That means that we can't use cat, which mostly defeats the >> purpose. In fact, using cat could be actively harmful. > > So if what we wind up with is: > > fd = open("/sys/.../capsule", O_RDWR); > write(fd, buf, size/N); > ... > write(fd, buf + M*size/N, size/N); > close(fd); > > You're suggesting the error code would post on close()? My worry about > that is that I imagine a lot less code in the wild checks the error code > on close() than on write() - though gnu cat does do so on both. But > there are other questions still - will it post on fdatasync()? On > fsync()? Cat, for example, doesn't check any of the above, which is why I don't like this approach. --Andy ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-10 15:51 ` Andy Lutomirski 2015-03-10 17:26 ` Peter Jones @ 2015-03-12 22:47 ` Matt Fleming 2015-03-13 14:42 ` Greg Kroah-Hartman 1 sibling, 1 reply; 37+ messages in thread From: Matt Fleming @ 2015-03-12 22:47 UTC (permalink / raw) To: Andy Lutomirski Cc: Peter Jones, Kweh, Hock Leong, Sam Protsenko, Ming Lei, Greg Kroah-Hartman, Ong, Boon Leong, LKML, linux-efi On Tue, 10 Mar, at 08:51:59AM, Andy Lutomirski wrote: > > I'm not 100% happy with write(2) (which is all we have in sysfs) for > two reasons: > > 1. If we write a file name, eww. That's more complicated, requires > temporary files, has annoying mount namespace issues, etc. > > 2. If we write the full contents, we need to do it in a single call to > write. That means that we can't use cat, which mostly defeats the > purpose. In fact, using cat could be actively harmful. At this point I'd really like Greg to chime in. In principal, I'm not stricly opposed to using a simple char device provided that it's not essentially a copy and paste of code from drivers/base/firmware_class.c. Greg? -- Matt Fleming, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-12 22:47 ` Matt Fleming @ 2015-03-13 14:42 ` Greg Kroah-Hartman 2015-03-16 15:35 ` Andy Lutomirski 0 siblings, 1 reply; 37+ messages in thread From: Greg Kroah-Hartman @ 2015-03-13 14:42 UTC (permalink / raw) To: Matt Fleming Cc: Andy Lutomirski, Peter Jones, Kweh, Hock Leong, Sam Protsenko, Ming Lei, Ong, Boon Leong, LKML, linux-efi On Thu, Mar 12, 2015 at 10:47:54PM +0000, Matt Fleming wrote: > On Tue, 10 Mar, at 08:51:59AM, Andy Lutomirski wrote: > > > > I'm not 100% happy with write(2) (which is all we have in sysfs) for > > two reasons: > > > > 1. If we write a file name, eww. That's more complicated, requires > > temporary files, has annoying mount namespace issues, etc. > > > > 2. If we write the full contents, we need to do it in a single call to > > write. That means that we can't use cat, which mostly defeats the > > purpose. In fact, using cat could be actively harmful. > > At this point I'd really like Greg to chime in. > > In principal, I'm not stricly opposed to using a simple char device > provided that it's not essentially a copy and paste of code from > drivers/base/firmware_class.c. > > Greg? Yes, I don't want a character driver here for this if at all possible. Just stick with the firmware download code, it's there and should work "as-is" for your stuff. thanks, greg k-h ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface 2015-03-13 14:42 ` Greg Kroah-Hartman @ 2015-03-16 15:35 ` Andy Lutomirski 0 siblings, 0 replies; 37+ messages in thread From: Andy Lutomirski @ 2015-03-16 15:35 UTC (permalink / raw) To: Greg KH Cc: Matt Fleming, Ong Boon Leong, linux-efi, Sam Protsenko, Kweh, Hock Leong, LKML, Peter Jones, Ming Lei On Mar 13, 2015 7:42 AM, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> wrote: > > On Thu, Mar 12, 2015 at 10:47:54PM +0000, Matt Fleming wrote: > > On Tue, 10 Mar, at 08:51:59AM, Andy Lutomirski wrote: > > > > > > I'm not 100% happy with write(2) (which is all we have in sysfs) for > > > two reasons: > > > > > > 1. If we write a file name, eww. That's more complicated, requires > > > temporary files, has annoying mount namespace issues, etc. > > > > > > 2. If we write the full contents, we need to do it in a single call to > > > write. That means that we can't use cat, which mostly defeats the > > > purpose. In fact, using cat could be actively harmful. > > > > At this point I'd really like Greg to chime in. > > > > In principal, I'm not stricly opposed to using a simple char device > > provided that it's not essentially a copy and paste of code from > > drivers/base/firmware_class.c. > > > > Greg? > > Yes, I don't want a character driver here for this if at all possible. > Just stick with the firmware download code, it's there and should work > "as-is" for your stuff. Given the rest of this interminable discussion, it seems pretty clear to me that the firmware download code doesn't work as is for this use case. It will sort of work with lots of changes (to locking, synchronicity, error reporting, enumeration, etc), but I think that the total complexity of doing that will far exceed the complexity if either a new chardev or some straightforward sysfs interface. We don't have ioctls in sysfs, though, and adding that sounds worse than a new character driver, so... --Andy > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2015-03-16 15:35 UTC | newest] Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-02 10:59 Re: [PATCH v2 3/3] efi: Capsule update with user helper interface Kweh, Hock Leong 2015-03-02 12:29 ` Matt Fleming 2015-03-03 5:56 ` Kweh, Hock Leong 2015-03-03 20:37 ` Andy Lutomirski 2015-03-03 20:49 ` Borislav Petkov 2015-03-03 21:56 ` Andy Lutomirski 2015-03-05 9:18 ` Kweh, Hock Leong 2015-03-05 23:08 ` Andy Lutomirski 2015-03-06 8:13 ` Borislav Petkov 2015-03-06 11:41 ` Kweh, Hock Leong 2015-03-06 14:47 ` Borislav Petkov 2015-03-09 21:23 ` fwupdate Borislav Petkov 2015-03-10 1:54 ` fwupdate Roy Franz 2015-03-10 14:56 ` fwupdate Peter Jones 2015-03-10 15:27 ` fwupdate Peter Jones 2015-03-06 12:20 ` Re: [PATCH v2 3/3] efi: Capsule update with user helper interface Kweh, Hock Leong 2015-03-06 19:05 ` Andy Lutomirski [not found] <F54AEECA5E2B9541821D670476DAE19C2B8AC95C@PGSMSX102.gar.corp.intel.com> 2015-02-24 12:49 ` Kweh, Hock Leong 2015-02-25 11:47 ` Borislav Petkov 2015-02-25 12:38 ` Kweh, Hock Leong 2015-02-25 12:49 ` Borislav Petkov 2015-02-26 15:30 ` Andy Lutomirski 2015-02-26 15:54 ` Borislav Petkov 2015-03-02 11:24 ` Matt Fleming 2015-03-06 21:39 ` Peter Jones 2015-03-06 21:49 ` Roy Franz 2015-03-06 22:17 ` Peter Jones 2015-03-10 12:26 ` Matt Fleming 2015-03-10 15:21 ` Peter Jones 2015-03-10 15:26 ` Andy Lutomirski 2015-03-10 15:40 ` Peter Jones 2015-03-10 15:51 ` Andy Lutomirski 2015-03-10 17:26 ` Peter Jones 2015-03-10 17:31 ` Andy Lutomirski 2015-03-12 22:47 ` Matt Fleming 2015-03-13 14:42 ` Greg Kroah-Hartman 2015-03-16 15:35 ` Andy Lutomirski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).