* In-Band Firmware Update @ 2018-07-23 18:13 Patrick Venture 2018-07-23 22:18 ` Vernon Mauery 0 siblings, 1 reply; 18+ messages in thread From: Patrick Venture @ 2018-07-23 18:13 UTC (permalink / raw) To: OpenBMC Maillist I've started to implement the host-tool outside of google3, and started splitting up the OEM handler that corresponds with it. However, firstly, I've submitted the design for the protocol for review, please let me know if you're interested and I'll add you to the review. IIRC, there was at least one interested party outside of us. Patrick ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-07-23 18:13 In-Band Firmware Update Patrick Venture @ 2018-07-23 22:18 ` Vernon Mauery 2018-07-24 0:13 ` Sai Dasari 0 siblings, 1 reply; 18+ messages in thread From: Vernon Mauery @ 2018-07-23 22:18 UTC (permalink / raw) To: Patrick Venture; +Cc: OpenBMC Maillist On 23-Jul-2018 11:13 AM, Patrick Venture wrote: >I've started to implement the host-tool outside of google3, and >started splitting up the OEM handler that corresponds with it. >However, firstly, I've submitted the design for the protocol for >review, please let me know if you're interested and I'll add you to >the review. IIRC, there was at least one interested party outside of >us. I am interested in coming up with a common (OpenBMC OEM level) set of Firmware NetFn commands that will allow all users of OpenBMC to be able to use common, open-source utilities to do firmware updates. If they are IPMI commands, this would include in-band (with KCS/BT for command/control and USB for transfer) or out-of-band (over RMCP+). Rather than use the OEM NetFn, for firmware updates, we should be using the Firmware NetFn. The entire Firmware NetFn is considered to be OEM per the IPMI spec. I would propose that we simply provide a common implementation for OpenBMC (as a provider library, of course, so it could be replaced if a downstream OEM doesn't want it). --Vernon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-07-23 22:18 ` Vernon Mauery @ 2018-07-24 0:13 ` Sai Dasari 2018-07-24 19:02 ` Bakshi, Sachit 2018-07-24 20:38 ` Matt Spinler 0 siblings, 2 replies; 18+ messages in thread From: Sai Dasari @ 2018-07-24 0:13 UTC (permalink / raw) To: Vernon Mauery, Patrick Venture; +Cc: OpenBMC Maillist On 7/23/18, 3:19 PM, "openbmc on behalf of Vernon Mauery" <openbmc-bounces+sdasari=fb.com@lists.ozlabs.org on behalf of vernon.mauery@linux.intel.com> wrote: On 23-Jul-2018 11:13 AM, Patrick Venture wrote: >I've started to implement the host-tool outside of google3, and >started splitting up the OEM handler that corresponds with it. >However, firstly, I've submitted the design for the protocol for >review, please let me know if you're interested and I'll add you to >the review. IIRC, there was at least one interested party outside of >us. I am interested in coming up with a common (OpenBMC OEM level) set of Firmware NetFn commands that will allow all users of OpenBMC to be able to use common, open-source utilities to do firmware updates. If they are IPMI commands, this would include in-band (with KCS/BT for command/control and USB for transfer) or out-of-band (over RMCP+). Rather than use the OEM NetFn, for firmware updates, we should be using the Firmware NetFn. The entire Firmware NetFn is considered to be OEM per the IPMI spec. I would propose that we simply provide a common implementation for OpenBMC (as a provider library, of course, so it could be replaced if a downstream OEM doesn't want it). Any thoughts on reusing/leveraging the PICMG's hpm spec @ https://www.picmg.org/openstandards/hardware-platform-management/ . One of the benefit would be the standard 'ipmitool' has native support for the update and changes are limited to BMC f/w. On a downside, the firmware binary has to be repackaged as .hpm format for this protocol to do some preparation steps as it support multiple f/w components in a single package. --Vernon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-07-24 0:13 ` Sai Dasari @ 2018-07-24 19:02 ` Bakshi, Sachit 2018-07-24 20:38 ` Matt Spinler 1 sibling, 0 replies; 18+ messages in thread From: Bakshi, Sachit @ 2018-07-24 19:02 UTC (permalink / raw) To: Sai Dasari, Vernon Mauery, Patrick Venture; +Cc: OpenBMC Maillist > > > On 7/23/18, 3:19 PM, "openbmc on behalf of Vernon Mauery" <openbmc-bounces+sdasari=fb.com@lists.ozlabs.org on behalf of vernon.mauery@linux.intel.com> wrote: > > On 23-Jul-2018 11:13 AM, Patrick Venture wrote: > >I've started to implement the host-tool outside of google3, and > >started splitting up the OEM handler that corresponds with it. > >However, firstly, I've submitted the design for the protocol for > >review, please let me know if you're interested and I'll add you to > >the review. IIRC, there was at least one interested party outside of > >us. > > I am interested in coming up with a common (OpenBMC OEM level) set of > Firmware NetFn commands that will allow all users of OpenBMC to be able > to use common, open-source utilities to do firmware updates. If they are > IPMI commands, this would include in-band (with KCS/BT for > command/control and USB for transfer) or out-of-band (over RMCP+). This sounds good. I'd advocate for some agreement around commands we support. Our BMC updates components besides itself, and we support multiple transport protocols. Therefore we have commands defined to initiate an update, transfer the image, check the status, get errors, start the update, etc. I'm sure others have similar support in their implementations. > > Rather than use the OEM NetFn, for firmware updates, we should be using > the Firmware NetFn. The entire Firmware NetFn is considered to be OEM > per the IPMI spec. I would propose that we simply provide a common > implementation for OpenBMC (as a provider library, of course, so it > could be replaced if a downstream OEM doesn't want it). Agreed. > Any thoughts on reusing/leveraging the PICMG's hpm spec @ https://www.picmg.org/openstandards/hardware-platform-management/ . Thanks for sharing this, I was not familiar with it. I'll look into it, but I know we sign our images to secure our system, not sure if HPM format will work for things like that, just something to consider. > One of the benefit would be the standard 'ipmitool' has native support for the update and changes are limited to BMC f/w. > On a downside, the firmware binary has to be repackaged as .hpm format for this protocol to do some preparation steps as it support multiple f/w components in a single package. > > --Vernon > > Thanks, Sachit ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-07-24 0:13 ` Sai Dasari 2018-07-24 19:02 ` Bakshi, Sachit @ 2018-07-24 20:38 ` Matt Spinler 2018-07-27 19:01 ` Patrick Venture 2018-08-06 21:59 ` Ed Tanous 1 sibling, 2 replies; 18+ messages in thread From: Matt Spinler @ 2018-07-24 20:38 UTC (permalink / raw) To: Sai Dasari; +Cc: Vernon Mauery, Patrick Venture, OpenBMC Maillist, openbmc On 2018-07-23 19:13, Sai Dasari wrote: > Any thoughts on reusing/leveraging the PICMG's hpm spec @ > https://www.picmg.org/openstandards/hardware-platform-management/ . > One of the benefit would be the standard 'ipmitool' has native support > for the update and changes are limited to BMC f/w. > On a downside, the firmware binary has to be repackaged as .hpm format > for this protocol to do some preparation steps as it support multiple > f/w components in a single package. This was brought up a few months ago and decided against: https://lists.ozlabs.org/pipermail/openbmc/2017-November/009938.html We are going to investigate using the DFU protocol, as that also has host side tools already available. > > --Vernon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-07-24 20:38 ` Matt Spinler @ 2018-07-27 19:01 ` Patrick Venture 2018-08-06 16:51 ` Patrick Venture 2018-08-06 21:59 ` Ed Tanous 1 sibling, 1 reply; 18+ messages in thread From: Patrick Venture @ 2018-07-27 19:01 UTC (permalink / raw) To: Matt Spinler; +Cc: Sai Dasari, Vernon Mauery, OpenBMC Maillist, openbmc Sorry, I haven't been on this email label recently -- accidentally fell behind. In parallel to these efforts, I've started upstreaming a design: https://gerrit.openbmc-project.xyz/11588 -- feel free to make comments. I'm going to implement the design (it's already implemented, but now I'm expanding it to address incoming comments). Patrick On Tue, Jul 24, 2018 at 1:38 PM, Matt Spinler <mspinler@linux.vnet.ibm.com> wrote: > On 2018-07-23 19:13, Sai Dasari wrote: > >> Any thoughts on reusing/leveraging the PICMG's hpm spec @ >> https://www.picmg.org/openstandards/hardware-platform-management/ . >> One of the benefit would be the standard 'ipmitool' has native support >> for the update and changes are limited to BMC f/w. >> On a downside, the firmware binary has to be repackaged as .hpm format >> for this protocol to do some preparation steps as it support multiple >> f/w components in a single package. > > > This was brought up a few months ago and decided against: > https://lists.ozlabs.org/pipermail/openbmc/2017-November/009938.html > > We are going to investigate using the DFU protocol, as that > also has host side tools already available. > > >> >> --Vernon > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-07-27 19:01 ` Patrick Venture @ 2018-08-06 16:51 ` Patrick Venture 2018-08-06 17:26 ` Patrick Venture 0 siblings, 1 reply; 18+ messages in thread From: Patrick Venture @ 2018-08-06 16:51 UTC (permalink / raw) To: Matt Spinler; +Cc: Sai Dasari, Vernon Mauery, OpenBMC Maillist, openbmc In an effort to do the OEM IPMI firmware update as full open source and to enable testing more thoroughly, I'm writing it anew and staging the patches upstream. https://gerrit.openbmc-project.xyz/#/c/openbmc/phosphor-ipmi-flash/+/11772/ is the current top of the stack of patches, but that'll change a bit this week as I implement more of the design. If you're very interested, and not on the patches, please let me know and I will add you to future ones. That said, other than next week, when I'll be out-of-office, I'm hoping to keep the patches going in a brisk pace, so rapid feedback is appreciated. Once the block-transfer interface portion is finished, I'll push up the host-side tool (which I also get to write anew). Patrick On Fri, Jul 27, 2018 at 12:01 PM, Patrick Venture <venture@google.com> wrote: > Sorry, I haven't been on this email label recently -- accidentally fell behind. > > In parallel to these efforts, I've started upstreaming a design: > https://gerrit.openbmc-project.xyz/11588 -- feel free to make > comments. I'm going to implement the design (it's already > implemented, but now I'm expanding it to address incoming comments). > > Patrick > > On Tue, Jul 24, 2018 at 1:38 PM, Matt Spinler > <mspinler@linux.vnet.ibm.com> wrote: >> On 2018-07-23 19:13, Sai Dasari wrote: >> >>> Any thoughts on reusing/leveraging the PICMG's hpm spec @ >>> https://www.picmg.org/openstandards/hardware-platform-management/ . >>> One of the benefit would be the standard 'ipmitool' has native support >>> for the update and changes are limited to BMC f/w. >>> On a downside, the firmware binary has to be repackaged as .hpm format >>> for this protocol to do some preparation steps as it support multiple >>> f/w components in a single package. >> >> >> This was brought up a few months ago and decided against: >> https://lists.ozlabs.org/pipermail/openbmc/2017-November/009938.html >> >> We are going to investigate using the DFU protocol, as that >> also has host side tools already available. >> >> >>> >>> --Vernon >> >> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-08-06 16:51 ` Patrick Venture @ 2018-08-06 17:26 ` Patrick Venture 2018-08-06 18:36 ` Patrick Venture 0 siblings, 1 reply; 18+ messages in thread From: Patrick Venture @ 2018-08-06 17:26 UTC (permalink / raw) To: Matt Spinler, Robert Lippert, Brad Bishop, Nancy Yuen Cc: Sai Dasari, Vernon Mauery, OpenBMC Maillist, openbmc We've been working on another interface lately that allows arbitrary BLOB read and write over IPMI. We're using it to talk to a driver on a future software stack that's in the works. It's a generic approach to reading and writing bytes that could be tailored to any purpose. One simply implements a handler in the interface. It was suggested that instead of sending the current in-band code upstream (albeit rewritten for better testing) that I instead send this upstream and then write the update mechanism into that interface. The interface defines some primitives: - BmcBlobGetCount: returns the number of blobs known presently - BmcBlobEnumerate: returns the blob's name for an index into the count (index 0 might return "/tmp/bmc-image" or something) - BmcBlobOpen: opens the blob and returns a session for future actions - BmcBlobRead: returns bytes - BmcBlobWrite: writes bytes - BmcBlocCommit: any action the handler wants that basically means, do thing. - BmcBlobClose: closes the file - BmcBlobDelete: deletes the blob, not always possible. - BmcBlobStat: returns blob metadata - BmcBlobSessionStat: returns metadata specific to the session It doesn't immediately define actions such as "abort", however, "close without commit" might be equivalent. It also wasn't designed to support OOB data, so it doesn't have a native packet that indicates there are bytes for Write() but they live elsewhere, such as in a P2A 64-KiB block or an LPC memory map region. I could likely find a way to easily handle those cases, possibly by passing information into the Open command, and reading back from the Stat() command. Open takes a path to open, and it asks each handler if they recognize the path, so one could have "/tmp/bmc-image/p2a/{addr}" or something similar as an easy method to configure that session. OR "/tmp/..tgz" to use the UBI handler... I've only pushed ~14 patches for the upstream OEM IPMI handler, and I could port those to route through the blob interface rather straightforwardly since it's nearly a 1:1 match. The BMC IPMI Blobs source was written by me following TDD as closely as possible. The design has been under review for its use case for a while, and is stable. Regardless if we want to use it for this, I'd also be willing to upstream our ipmi-blobs handler infrastructure. It has presently one limitation, and that is that it doesn't support run-time loading of handlers, how ipmid supports finding IPMI libraries. It is fully contained with the handlers, and they're enabled by configuration flags in the build. This limitation could be removed if desired. Patrick On Mon, Aug 6, 2018 at 9:51 AM, Patrick Venture <venture@google.com> wrote: > In an effort to do the OEM IPMI firmware update as full open source > and to enable testing more thoroughly, I'm writing it anew and staging > the patches upstream. > https://gerrit.openbmc-project.xyz/#/c/openbmc/phosphor-ipmi-flash/+/11772/ > is the current top of the stack of patches, but that'll change a bit > this week as I implement more of the design. > > If you're very interested, and not on the patches, please let me know > and I will add you to future ones. That said, other than next week, > when I'll be out-of-office, I'm hoping to keep the patches going in a > brisk pace, so rapid feedback is appreciated. Once the block-transfer > interface portion is finished, I'll push up the host-side tool (which > I also get to write anew). > > Patrick > > On Fri, Jul 27, 2018 at 12:01 PM, Patrick Venture <venture@google.com> wrote: >> Sorry, I haven't been on this email label recently -- accidentally fell behind. >> >> In parallel to these efforts, I've started upstreaming a design: >> https://gerrit.openbmc-project.xyz/11588 -- feel free to make >> comments. I'm going to implement the design (it's already >> implemented, but now I'm expanding it to address incoming comments). >> >> Patrick >> >> On Tue, Jul 24, 2018 at 1:38 PM, Matt Spinler >> <mspinler@linux.vnet.ibm.com> wrote: >>> On 2018-07-23 19:13, Sai Dasari wrote: >>> >>>> Any thoughts on reusing/leveraging the PICMG's hpm spec @ >>>> https://www.picmg.org/openstandards/hardware-platform-management/ . >>>> One of the benefit would be the standard 'ipmitool' has native support >>>> for the update and changes are limited to BMC f/w. >>>> On a downside, the firmware binary has to be repackaged as .hpm format >>>> for this protocol to do some preparation steps as it support multiple >>>> f/w components in a single package. >>> >>> >>> This was brought up a few months ago and decided against: >>> https://lists.ozlabs.org/pipermail/openbmc/2017-November/009938.html >>> >>> We are going to investigate using the DFU protocol, as that >>> also has host side tools already available. >>> >>> >>>> >>>> --Vernon >>> >>> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-08-06 17:26 ` Patrick Venture @ 2018-08-06 18:36 ` Patrick Venture 2018-08-06 22:20 ` Ed Tanous 0 siblings, 1 reply; 18+ messages in thread From: Patrick Venture @ 2018-08-06 18:36 UTC (permalink / raw) To: Matt Spinler, Robert Lippert, Brad Bishop, Nancy Yuen, Tanous, Ed Cc: Sai Dasari, Vernon Mauery, OpenBMC Maillist, openbmc +Ed On Mon, Aug 6, 2018 at 10:26 AM, Patrick Venture <venture@google.com> wrote: > We've been working on another interface lately that allows arbitrary > BLOB read and write over IPMI. We're using it to talk to a driver on > a future software stack that's in the works. It's a generic approach > to reading and writing bytes that could be tailored to any purpose. > One simply implements a handler in the interface. It was suggested > that instead of sending the current in-band code upstream (albeit > rewritten for better testing) that I instead send this upstream and > then write the update mechanism into that interface. > > The interface defines some primitives: > - BmcBlobGetCount: returns the number of blobs known presently > - BmcBlobEnumerate: returns the blob's name for an index into the > count (index 0 might return "/tmp/bmc-image" or something) > - BmcBlobOpen: opens the blob and returns a session for future actions > - BmcBlobRead: returns bytes > - BmcBlobWrite: writes bytes > - BmcBlocCommit: any action the handler wants that basically means, do thing. > - BmcBlobClose: closes the file > - BmcBlobDelete: deletes the blob, not always possible. > - BmcBlobStat: returns blob metadata > - BmcBlobSessionStat: returns metadata specific to the session > > It doesn't immediately define actions such as "abort", however, "close > without commit" might be equivalent. It also wasn't designed to > support OOB data, so it doesn't have a native packet that indicates > there are bytes for Write() but they live elsewhere, such as in a P2A > 64-KiB block or an LPC memory map region. > > I could likely find a way to easily handle those cases, possibly by > passing information into the Open command, and reading back from the > Stat() command. Open takes a path to open, and it asks each handler > if they recognize the path, so one could have > "/tmp/bmc-image/p2a/{addr}" or something similar as an easy method to > configure that session. OR "/tmp/..tgz" to use the UBI handler... > > I've only pushed ~14 patches for the upstream OEM IPMI handler, and I > could port those to route through the blob interface rather > straightforwardly since it's nearly a 1:1 match. > > The BMC IPMI Blobs source was written by me following TDD as closely > as possible. The design has been under review for its use case for a > while, and is stable. > > Regardless if we want to use it for this, I'd also be willing to > upstream our ipmi-blobs handler infrastructure. It has presently one > limitation, and that is that it doesn't support run-time loading of > handlers, how ipmid supports finding IPMI libraries. It is fully > contained with the handlers, and they're enabled by configuration > flags in the build. This limitation could be removed if desired. > > Patrick > > > On Mon, Aug 6, 2018 at 9:51 AM, Patrick Venture <venture@google.com> wrote: >> In an effort to do the OEM IPMI firmware update as full open source >> and to enable testing more thoroughly, I'm writing it anew and staging >> the patches upstream. >> https://gerrit.openbmc-project.xyz/#/c/openbmc/phosphor-ipmi-flash/+/11772/ >> is the current top of the stack of patches, but that'll change a bit >> this week as I implement more of the design. >> >> If you're very interested, and not on the patches, please let me know >> and I will add you to future ones. That said, other than next week, >> when I'll be out-of-office, I'm hoping to keep the patches going in a >> brisk pace, so rapid feedback is appreciated. Once the block-transfer >> interface portion is finished, I'll push up the host-side tool (which >> I also get to write anew). >> >> Patrick >> >> On Fri, Jul 27, 2018 at 12:01 PM, Patrick Venture <venture@google.com> wrote: >>> Sorry, I haven't been on this email label recently -- accidentally fell behind. >>> >>> In parallel to these efforts, I've started upstreaming a design: >>> https://gerrit.openbmc-project.xyz/11588 -- feel free to make >>> comments. I'm going to implement the design (it's already >>> implemented, but now I'm expanding it to address incoming comments). >>> >>> Patrick >>> >>> On Tue, Jul 24, 2018 at 1:38 PM, Matt Spinler >>> <mspinler@linux.vnet.ibm.com> wrote: >>>> On 2018-07-23 19:13, Sai Dasari wrote: >>>> >>>>> Any thoughts on reusing/leveraging the PICMG's hpm spec @ >>>>> https://www.picmg.org/openstandards/hardware-platform-management/ . >>>>> One of the benefit would be the standard 'ipmitool' has native support >>>>> for the update and changes are limited to BMC f/w. >>>>> On a downside, the firmware binary has to be repackaged as .hpm format >>>>> for this protocol to do some preparation steps as it support multiple >>>>> f/w components in a single package. >>>> >>>> >>>> This was brought up a few months ago and decided against: >>>> https://lists.ozlabs.org/pipermail/openbmc/2017-November/009938.html >>>> >>>> We are going to investigate using the DFU protocol, as that >>>> also has host side tools already available. >>>> >>>> >>>>> >>>>> --Vernon >>>> >>>> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-08-06 18:36 ` Patrick Venture @ 2018-08-06 22:20 ` Ed Tanous 2018-08-06 23:04 ` Andrew Jeffery 2018-08-07 15:11 ` Patrick Venture 0 siblings, 2 replies; 18+ messages in thread From: Ed Tanous @ 2018-08-06 22:20 UTC (permalink / raw) To: Patrick Venture, Matt Spinler, Robert Lippert, Brad Bishop, Nancy Yuen Cc: Sai Dasari, Vernon Mauery, OpenBMC Maillist, openbmc >> The interface defines some primitives: >> - BmcBlobGetCount: returns the number of blobs known presently Is this blobs per object? Blobs total for the system? >> - BmcBlobEnumerate: returns the blob's name for an index into the >> count (index 0 might return "/tmp/bmc-image" or something) Could this be a well known name rather than an index? Blob 0 is a lot less descriptive than Blob: bmc-image. >> - BmcBlobOpen: opens the blob and returns a session for future actions I'm not really following what blob open would do. What arguments does it take? Would this be on a higher level collection type interface? is BmcBlobReset or BmcBlobClear a better name for the action it's performing? >> - BmcBlobRead: returns bytes >> - BmcBlobWrite: writes bytes Should we put any restrictions on write/read sizes, or leave it up to each implementation to enforce? I'm kind of leaning toward the later, although it makes the interface harder to use. I'm thinking about in band updatable power supplies that can only accept writes of a very particular block sizes (I think it's 16 bytes at a time). >> - BmcBlocCommit: any action the handler wants that basically means, do thing. >> - BmcBlobClose: closes the file What is the difference between commit and close? Close seems like a possible implementation of commit, but not necessarily required if Blob doesn't represent a file. >> - BmcBlobDelete: deletes the blob, not always possible. >> - BmcBlobStat: returns blob metadata What would this return? >> - BmcBlobSessionStat: returns metadata specific to the session ? I don't see any other mention of session. I feel like I'm missing part of the picture. Overall, this looks really interesting. We have a relatively similar interface for shipping SMBIOS and MDR data from BIOS that we had to invent to get those fields to work. I'm not sure if they map 100%, but we could likely get it to work with a little massaging to our stuff. As a general purpose interface, it seems very useful, especially in the context of firmware update, which could be implemented in any number of interfaces. I'd be interested in separating out the "file like" nature from the interface, and keep it as just a "block level" update, that may be a file, or may be something else. I'm also interested in the actual arguments, as well as the distinction between a Blob and a BlobSession that I'm not really understanding. >> >> It doesn't immediately define actions such as "abort", however, "close >> without commit" might be equivalent. I kind of like this about the interface. Some updates can't really be reverted cleanly, so leaving it up to the implementation to decide (possibly based on a timeout or something) seems kinda nice to have. >> Regardless if we want to use it for this, I'd also be willing to >> upstream our ipmi-blobs handler infrastructure. It has presently one >> limitation, and that is that it doesn't support run-time loading of >> handlers, how ipmid supports finding IPMI libraries. It is fully >> contained with the handlers, and they're enabled by configuration >> flags in the build. This limitation could be removed if desired. From the looks of Vernons IPMI handler rewrite document, runtime loading of handler libraries might be out anyway, so I don't think this is a non-starter, especially given the timeframes. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-08-06 22:20 ` Ed Tanous @ 2018-08-06 23:04 ` Andrew Jeffery 2018-08-07 15:14 ` Patrick Venture 2018-08-07 15:11 ` Patrick Venture 1 sibling, 1 reply; 18+ messages in thread From: Andrew Jeffery @ 2018-08-06 23:04 UTC (permalink / raw) To: Ed Tanous, Patrick Venture, Matt Spinler, Robert Lippert, Brad Bishop, Nancy Yuen Cc: Vernon Mauery, OpenBMC Maillist, openbmc > I'd be interested in separating out the "file like" nature from the > interface, and keep it as just a "block level" update, that may be a > file, or may be something else. > A slight tangent, but I think it's worth talking about here: On OpenPOWER systems we developed a control protocol for the host to manipulate the LPC FW mapping. v3 of the protocol included support for mapping "arbitrary" stuff into the FW space with a command to list what devices were available. It seems very similar to what Patrick has outlined, though operates at the block level as Ed mused about above. Typically this mechanism is used for the host to access its firmware which - again on OpenPOWER designs - sits on a flash device owned by the BMC. The control protocol was run over the LPC mailbox available on ASPEED BMCs. However, I am currently developing a PoC IPMI replacement for the mailbox interface, and intended to use much the same command-set and argument layout as we have with the mailbox. As mentioned above it aligns pretty well with what Patrick is proposing. So our use-case has less of an update focus, but at the end of the day if you can perform arbitrary writes it roughly amounts to the same thing. Should we expand the scope to cover my use-case here? I'm fine if the answer is "no", but with the similarities it seemed necessary to bring up. Cheers, Andrew PS: The bits - Repository: https://github.com/openbmc/mboxbridge Protocol documentation: https://github.com/openbmc/mboxbridge/blob/master/Documentation/mbox_protocol.md Implementation notes: https://github.com/openbmc/mboxbridge/blob/master/Documentation/mboxd.md ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-08-06 23:04 ` Andrew Jeffery @ 2018-08-07 15:14 ` Patrick Venture 2018-08-07 16:41 ` Patrick Venture 0 siblings, 1 reply; 18+ messages in thread From: Patrick Venture @ 2018-08-07 15:14 UTC (permalink / raw) To: Andrew Jeffery Cc: Ed Tanous, Matt Spinler, Robert Lippert, Brad Bishop, Nancy Yuen, Vernon Mauery, OpenBMC Maillist, openbmc On Mon, Aug 6, 2018 at 4:04 PM, Andrew Jeffery <andrew@aj.id.au> wrote: >> I'd be interested in separating out the "file like" nature from the >> interface, and keep it as just a "block level" update, that may be a >> file, or may be something else. >> > > A slight tangent, but I think it's worth talking about here: > > On OpenPOWER systems we developed a control protocol for the > host to manipulate the LPC FW mapping. v3 of the protocol included > support for mapping "arbitrary" stuff into the FW space with a > command to list what devices were available. It seems very similar to > what Patrick has outlined, though operates at the block level as Ed > mused about above. i'm somewhat familiar with the mailbox daemon and v3 protocols insomuch as we were using them on our openpower system before transitioning to phosphor-ipmi-flash (for consistency across platform). > > Typically this mechanism is used for the host to access its firmware > which - again on OpenPOWER designs - sits on a flash device owned > by the BMC. > > The control protocol was run over the LPC mailbox available on > ASPEED BMCs. However, I am currently developing a PoC IPMI > replacement for the mailbox interface, and intended to use much > the same command-set and argument layout as we have with the > mailbox. As mentioned above it aligns pretty well with what Patrick > is proposing. > > So our use-case has less of an update focus, but at the end of the > day if you can perform arbitrary writes it roughly amounts to the > same thing. Should we expand the scope to cover my use-case here? > I'm fine if the answer is "no", but with the similarities it seemed > necessary to bring up. The explicit out-of-band-data flow-control messages in phosphor-ipmi-flash fit the semantics to an extent. We actually use one of the packets in the design https://github.com/openbmc/phosphor-ipmi-flash/blob/master/docs/flash_update.md#flashmapregionlpc-13 to provide information used for the ioctl: bool LpcMapperAspeed::MapWindow(uint32_t *windowOffset, uint32_t *windowSize, uint32_t address, uint32_t length) { static const uint32_t MASK_64K = 0xFFFFU; const uint32_t offset = address & MASK_64K; if (offset + length > regionSize) { fprintf(stderr, "requested window size %" PRIu32 ", offset %#" PRIx32 " is too large for mem region at %#" PRIXPTR " of size %zu\n", length, offset, physAddr, regionSize); *windowSize = regionSize - offset; return false; } *windowOffset = offset; *windowSize = length; struct aspeed_lpc_ctrl_mapping map = { .window_type = ASPEED_LPC_CTRL_WINDOW_MEMORY, .window_id = 0, .flags = 0, .addr = address & ~MASK_64K, .offset = 0, .size = __ALIGN_KERNEL_MASK(offset + length, MASK_64K), }; printf("requesting Aspeed LPC window at %#" PRIx32 " of size %" PRIu32 "\n", map.addr, map.size); static const char lpcControlPath[] = "/dev/aspeed-lpc-ctrl"; const auto lpcControlFd = open(lpcControlPath, O_RDWR); if (FileClosed == lpcControlFd) { fprintf(stderr, "cannot open Aspeed LPC kernel control dev \"%s\"\n", lpcControlPath); return false; } if (ioctl(lpcControlFd, ASPEED_LPC_CTRL_IOCTL_MAP, &map) == -1) { fprintf(stderr, "Failed to ioctl Aspeed LPC map with error %s\n", strerror(errno)); return false; } return true; } The above code is going into phosphor-ipmi-flash once I finish rewriting the IPMI BT interface support. > > Cheers, > > Andrew > > PS: The bits - > Repository: https://github.com/openbmc/mboxbridge > Protocol documentation: https://github.com/openbmc/mboxbridge/blob/master/Documentation/mbox_protocol.md > Implementation notes: https://github.com/openbmc/mboxbridge/blob/master/Documentation/mboxd.md ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-08-07 15:14 ` Patrick Venture @ 2018-08-07 16:41 ` Patrick Venture 2018-08-07 18:48 ` Patrick Venture 0 siblings, 1 reply; 18+ messages in thread From: Patrick Venture @ 2018-08-07 16:41 UTC (permalink / raw) To: Andrew Jeffery Cc: Ed Tanous, Matt Spinler, Robert Lippert, Brad Bishop, Nancy Yuen, Vernon Mauery, OpenBMC Maillist, openbmc On Tue, Aug 7, 2018 at 8:14 AM, Patrick Venture <venture@google.com> wrote: > On Mon, Aug 6, 2018 at 4:04 PM, Andrew Jeffery <andrew@aj.id.au> wrote: >>> I'd be interested in separating out the "file like" nature from the >>> interface, and keep it as just a "block level" update, that may be a >>> file, or may be something else. >>> >> >> A slight tangent, but I think it's worth talking about here: >> >> On OpenPOWER systems we developed a control protocol for the >> host to manipulate the LPC FW mapping. v3 of the protocol included >> support for mapping "arbitrary" stuff into the FW space with a >> command to list what devices were available. It seems very similar to >> what Patrick has outlined, though operates at the block level as Ed >> mused about above. > > i'm somewhat familiar with the mailbox daemon and v3 protocols > insomuch as we were using them on our openpower system before > transitioning to phosphor-ipmi-flash (for consistency across > platform). > >> >> Typically this mechanism is used for the host to access its firmware >> which - again on OpenPOWER designs - sits on a flash device owned >> by the BMC. >> >> The control protocol was run over the LPC mailbox available on >> ASPEED BMCs. However, I am currently developing a PoC IPMI >> replacement for the mailbox interface, and intended to use much >> the same command-set and argument layout as we have with the >> mailbox. As mentioned above it aligns pretty well with what Patrick >> is proposing. >> >> So our use-case has less of an update focus, but at the end of the >> day if you can perform arbitrary writes it roughly amounts to the >> same thing. Should we expand the scope to cover my use-case here? >> I'm fine if the answer is "no", but with the similarities it seemed >> necessary to bring up. > > The explicit out-of-band-data flow-control messages in > phosphor-ipmi-flash fit the semantics to an extent. We actually use > one of the packets in the design > https://github.com/openbmc/phosphor-ipmi-flash/blob/master/docs/flash_update.md#flashmapregionlpc-13 > to provide information used for the ioctl: > > bool LpcMapperAspeed::MapWindow(uint32_t *windowOffset, uint32_t *windowSize, > uint32_t address, uint32_t length) > { > static const uint32_t MASK_64K = 0xFFFFU; > const uint32_t offset = address & MASK_64K; > if (offset + length > regionSize) > { > fprintf(stderr, "requested window size %" PRIu32 ", offset %#" PRIx32 > " is too large for mem region at %#" PRIXPTR " of size %zu\n", > length, offset, physAddr, regionSize); > *windowSize = regionSize - offset; > return false; > } > *windowOffset = offset; > *windowSize = length; > struct aspeed_lpc_ctrl_mapping map = > { > .window_type = ASPEED_LPC_CTRL_WINDOW_MEMORY, > .window_id = 0, > .flags = 0, > .addr = address & ~MASK_64K, > .offset = 0, > .size = __ALIGN_KERNEL_MASK(offset + length, MASK_64K), > }; > printf("requesting Aspeed LPC window at %#" PRIx32 " of size %" PRIu32 "\n", > map.addr, map.size); > static const char lpcControlPath[] = "/dev/aspeed-lpc-ctrl"; > const auto lpcControlFd = open(lpcControlPath, O_RDWR); > if (FileClosed == lpcControlFd) > { > fprintf(stderr, "cannot open Aspeed LPC kernel control dev \"%s\"\n", > lpcControlPath); > return false; > } > if (ioctl(lpcControlFd, ASPEED_LPC_CTRL_IOCTL_MAP, &map) == -1) > { > fprintf(stderr, "Failed to ioctl Aspeed LPC map with error %s\n", > strerror(errno)); > return false; > } > return true; > } > > The above code is going into phosphor-ipmi-flash once I finish > rewriting the IPMI BT interface support. So, if there is a desire to see the generic blob ipmi interface up -- I can start upstreaming that. I can continue phosphor-ipmi-flash for a flash-only approach, or I can move that code into the blobs interface. Matt and Vernon have been reviewing that code, which has been great as it has great momentum! The transition to ipmi-blobs will be very similar code as far as the handler is concerned, however, the ipmi portion, which is what I've nearly finished for phosphor-ipmi-flash will be tossed as the blob has its own IPMI command details. I don't mind reworking things, and wanted to also draw attention and thanks to Matt and Vernon for reviewing phosphor-ipmi-flash. > >> >> Cheers, >> >> Andrew >> >> PS: The bits - >> Repository: https://github.com/openbmc/mboxbridge >> Protocol documentation: https://github.com/openbmc/mboxbridge/blob/master/Documentation/mbox_protocol.md >> Implementation notes: https://github.com/openbmc/mboxbridge/blob/master/Documentation/mboxd.md ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-08-07 16:41 ` Patrick Venture @ 2018-08-07 18:48 ` Patrick Venture 0 siblings, 0 replies; 18+ messages in thread From: Patrick Venture @ 2018-08-07 18:48 UTC (permalink / raw) To: Andrew Jeffery Cc: Ed Tanous, Matt Spinler, Robert Lippert, Brad Bishop, Nancy Yuen, Vernon Mauery, OpenBMC Maillist, openbmc On Tue, Aug 7, 2018 at 9:41 AM, Patrick Venture <venture@google.com> wrote: > On Tue, Aug 7, 2018 at 8:14 AM, Patrick Venture <venture@google.com> wrote: >> On Mon, Aug 6, 2018 at 4:04 PM, Andrew Jeffery <andrew@aj.id.au> wrote: >>>> I'd be interested in separating out the "file like" nature from the >>>> interface, and keep it as just a "block level" update, that may be a >>>> file, or may be something else. >>>> >>> >>> A slight tangent, but I think it's worth talking about here: >>> >>> On OpenPOWER systems we developed a control protocol for the >>> host to manipulate the LPC FW mapping. v3 of the protocol included >>> support for mapping "arbitrary" stuff into the FW space with a >>> command to list what devices were available. It seems very similar to >>> what Patrick has outlined, though operates at the block level as Ed >>> mused about above. >> >> i'm somewhat familiar with the mailbox daemon and v3 protocols >> insomuch as we were using them on our openpower system before >> transitioning to phosphor-ipmi-flash (for consistency across >> platform). >> >>> >>> Typically this mechanism is used for the host to access its firmware >>> which - again on OpenPOWER designs - sits on a flash device owned >>> by the BMC. >>> >>> The control protocol was run over the LPC mailbox available on >>> ASPEED BMCs. However, I am currently developing a PoC IPMI >>> replacement for the mailbox interface, and intended to use much >>> the same command-set and argument layout as we have with the >>> mailbox. As mentioned above it aligns pretty well with what Patrick >>> is proposing. >>> >>> So our use-case has less of an update focus, but at the end of the >>> day if you can perform arbitrary writes it roughly amounts to the >>> same thing. Should we expand the scope to cover my use-case here? >>> I'm fine if the answer is "no", but with the similarities it seemed >>> necessary to bring up. >> >> The explicit out-of-band-data flow-control messages in >> phosphor-ipmi-flash fit the semantics to an extent. We actually use >> one of the packets in the design >> https://github.com/openbmc/phosphor-ipmi-flash/blob/master/docs/flash_update.md#flashmapregionlpc-13 >> to provide information used for the ioctl: >> >> bool LpcMapperAspeed::MapWindow(uint32_t *windowOffset, uint32_t *windowSize, >> uint32_t address, uint32_t length) >> { >> static const uint32_t MASK_64K = 0xFFFFU; >> const uint32_t offset = address & MASK_64K; >> if (offset + length > regionSize) >> { >> fprintf(stderr, "requested window size %" PRIu32 ", offset %#" PRIx32 >> " is too large for mem region at %#" PRIXPTR " of size %zu\n", >> length, offset, physAddr, regionSize); >> *windowSize = regionSize - offset; >> return false; >> } >> *windowOffset = offset; >> *windowSize = length; >> struct aspeed_lpc_ctrl_mapping map = >> { >> .window_type = ASPEED_LPC_CTRL_WINDOW_MEMORY, >> .window_id = 0, >> .flags = 0, >> .addr = address & ~MASK_64K, >> .offset = 0, >> .size = __ALIGN_KERNEL_MASK(offset + length, MASK_64K), >> }; >> printf("requesting Aspeed LPC window at %#" PRIx32 " of size %" PRIu32 "\n", >> map.addr, map.size); >> static const char lpcControlPath[] = "/dev/aspeed-lpc-ctrl"; >> const auto lpcControlFd = open(lpcControlPath, O_RDWR); >> if (FileClosed == lpcControlFd) >> { >> fprintf(stderr, "cannot open Aspeed LPC kernel control dev \"%s\"\n", >> lpcControlPath); >> return false; >> } >> if (ioctl(lpcControlFd, ASPEED_LPC_CTRL_IOCTL_MAP, &map) == -1) >> { >> fprintf(stderr, "Failed to ioctl Aspeed LPC map with error %s\n", >> strerror(errno)); >> return false; >> } >> return true; >> } >> >> The above code is going into phosphor-ipmi-flash once I finish >> rewriting the IPMI BT interface support. > > So, if there is a desire to see the generic blob ipmi interface up -- > I can start upstreaming that. I can continue phosphor-ipmi-flash for > a flash-only approach, or I can move that code into the blobs > interface. Matt and Vernon have been reviewing that code, which has > been great as it has great momentum! The transition to ipmi-blobs > will be very similar code as far as the handler is concerned, however, > the ipmi portion, which is what I've nearly finished for > phosphor-ipmi-flash will be tossed as the blob has its own IPMI > command details. I don't mind reworking things, and wanted to also > draw attention and thanks to Matt and Vernon for reviewing > phosphor-ipmi-flash. > I'm going to convert the current blobs protocol to markdown and send it for comment. >> >>> >>> Cheers, >>> >>> Andrew >>> >>> PS: The bits - >>> Repository: https://github.com/openbmc/mboxbridge >>> Protocol documentation: https://github.com/openbmc/mboxbridge/blob/master/Documentation/mbox_protocol.md >>> Implementation notes: https://github.com/openbmc/mboxbridge/blob/master/Documentation/mboxd.md ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-08-06 22:20 ` Ed Tanous 2018-08-06 23:04 ` Andrew Jeffery @ 2018-08-07 15:11 ` Patrick Venture 1 sibling, 0 replies; 18+ messages in thread From: Patrick Venture @ 2018-08-07 15:11 UTC (permalink / raw) To: Ed Tanous Cc: Matt Spinler, Robert Lippert, Brad Bishop, Nancy Yuen, Sai Dasari, Vernon Mauery, OpenBMC Maillist, openbmc On Mon, Aug 6, 2018 at 3:20 PM, Ed Tanous <ed.tanous@intel.com> wrote: >>> The interface defines some primitives: >>> - BmcBlobGetCount: returns the number of blobs known presently > > Is this blobs per object? Blobs total for the system? Blobs for the system, there's the notion of quasi statically named blobs, folder blobs, dynamically named blobs. Given that each handler is asked, what blobs do you currently know? It returns a list which is cached and then provided during enumerate. So you could have 3 handlers registered: The first returns: "/tmp/pvfs/" for its list the second: "/blob/monkey123" the third: "" empty list If the one providing "/tmp/pvfs/" is known to you (you know how to talk to it, you could actually provide "/tmp/pvfs/1" to open() to open a "file" in that "folder." Then future calls to list blobs can include /1 in the list. But it's a flat hierarchy and it's all really a first-match -- The various calls that take a blob_id string ask each handler (until one claims it) if they handle that blob_id. As it's currently implemented downstream. > >>> - BmcBlobEnumerate: returns the blob's name for an index into the >>> count (index 0 might return "/tmp/bmc-image" or something) > > Could this be a well known name rather than an index? Blob 0 is a lot less > descriptive than Blob: bmc-image. So the index is so you can access something in the cached list. The value it returns is the string. > >>> - BmcBlobOpen: opens the blob and returns a session for future actions > > I'm not really following what blob open would do. What arguments does it > take? Would this be on a higher level collection type interface? is > BmcBlobReset or BmcBlobClear a better name for the action it's performing? Open associates a session with a blob_id for future calls, such as write. It has real file semantics in that way. Future calls that are associated with a session require you to provide the session_id. It takes the blob_id, and flags. We're considering extending it to also take a small arbitrary payload, but haven't decided whether that change is useful. struct BmcBlobOpenTx { uint16_t crc16; uint16_t flags; char blob_id[]; /* Must correspond to a valid blob. */ }; Every payload in this design has a crc16. > > >>> - BmcBlobRead: returns bytes >>> - BmcBlobWrite: writes bytes > > Should we put any restrictions on write/read sizes, or leave it up to each > implementation to enforce? I'm kind of leaning toward the later, although > it makes the interface harder to use. struct BmcBlobWriteTx { uint16_t crc16; uint16_t session_id; /* Returned from BmcBlobOpen. */ uint32_t offset; /* The byte sequence start, 0-based. */ uint8_t data[]; }; struct BmcBlobReadTx { uint16_t crc16; uint16_t session_id; /* Returned from BmcBlobOpen. */ uint32_t offset; /* The byte sequence start, 0-based. */ uint32_t requested_size; /* The number of bytes requested for reading. */ }; Those are the control structures for read and write, which I feel may clear up how it's presently designed. There's no limitation on sizes beyond 32-bits for the requested_size. > I'm thinking about in band updatable power supplies that can only accept > writes of a very particular block sizes (I think it's 16 bytes at a time). That's do-able. One could also cache the entire write and then handle it in the commit() step. We have a handler right now that has a 1KiB buffer that you're really reading and writing into that then pushes the buffer when you call commit(). > >>> - BmcBlocCommit: any action the handler wants that basically means, do >>> thing. >>> - BmcBlobClose: closes the file > > What is the difference between commit and close? Close seems like a > possible implementation of commit, but not necessarily required if Blob > doesn't represent a file. Commit is a primitive that implies you want some action to be taken. Our initial and only current use-case of this interface buffers the write data and then on commit() sends the payload to another utility that does X with it. If we route the firmware update through this interface, then conceivably, commit() can be implemented in that handler to do image verification, etc. Close() just closes our your session. There's also an internal expiration mechanism that'll expire a session and the semantics are presently the same. However, each handler could conceivably do their own thing. > >>> - BmcBlobDelete: deletes the blob, not always possible. >>> - BmcBlobStat: returns blob metadata > > What would this return? Presently: struct BmcBlobStatRx { uint16_t crc16; uint16_t blob_state; uint32_t size; /* Size in bytes of the blob. */ uint8_t metadata_len; uint8_t metadata[]; /* Optional blob-specific metadata. */ }; We have it implemented as the above, however, if the metadata specific data doesn't fit in one command, there's not an explicit way to indicate this, etc. So that's something that is still in discussion on the design. Right now, there's no use-case for that metadata blob, it was included for future use. Also, don't mind the style in the structs, the way I've implemented it follows the openbmc style (so that'll at least reduce X comments when pushing upstream :D ) > >>> - BmcBlobSessionStat: returns metadata specific to the session > > ? I don't see any other mention of session. I feel like I'm missing part of > the picture. I thought I mentioned sessions w.r.t to opening of files. If not, my mistake. In this event, you're requesting session metadata. Consider that you have three open sessions to a blob, depending on the implementation they might have different information or the same. The blob-level stat() might actually not make sense, and then you'd not support it. The design basically lets you implement the interface however you need, and any primitive you don't want can just return false, and fail happily. There's no requirement that one implements read() on a write-only object. > > Overall, this looks really interesting. We have a relatively similar > interface for shipping SMBIOS and MDR data from BIOS that we had to invent > to get those fields to work. I'm not sure if they map 100%, but we could > likely get it to work with a little massaging to our stuff. As a general > purpose interface, it seems very useful, especially in the context of > firmware update, which could be implemented in any number of interfaces. > > I'd be interested in separating out the "file like" nature from the > interface, and keep it as just a "block level" update, that may be a file, > or may be something else. I'm not sure I follow the difference. One could open "/tmp/blocks" and then write a block, commit it, write the next block. Or one could open "/blobs/block/1" and then "/blobs/block/2" and so on. > > I'm also interested in the actual arguments, as well as the distinction > between a Blob and a BlobSession that I'm not really understanding. > >>> >>> It doesn't immediately define actions such as "abort", however, "close >>> without commit" might be equivalent. > > I kind of like this about the interface. Some updates can't really be > reverted cleanly, so leaving it up to the implementation to decide (possibly > based on a timeout or something) seems kinda nice to have. > >>> Regardless if we want to use it for this, I'd also be willing to >>> upstream our ipmi-blobs handler infrastructure. It has presently one >>> limitation, and that is that it doesn't support run-time loading of >>> handlers, how ipmid supports finding IPMI libraries. It is fully >>> contained with the handlers, and they're enabled by configuration >>> flags in the build. This limitation could be removed if desired. > > > From the looks of Vernons IPMI handler rewrite document, runtime loading of > handler libraries might be out anyway, so I don't think this is a > non-starter, especially given the timeframes. Ok. I'm working with people presently to see if we can extend the interface in a couple ways to make out-of-band data easier. I'd prefer to not just hack it into the interface. For instance, the write API call: struct BmcBlobWriteTx { uint16_t crc16; uint16_t session_id; /* Returned from BmcBlobOpen. */ uint32_t offset; /* The byte sequence start, 0-based. */ uint8_t data[]; }; Conceivably if your handler is expecting the data over the LPC memory buffer, the data field there could be a structure that provides the information instead of just byte data. However, I would prefer something more explicit. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-07-24 20:38 ` Matt Spinler 2018-07-27 19:01 ` Patrick Venture @ 2018-08-06 21:59 ` Ed Tanous 2018-08-08 1:32 ` Jeremy Kerr 1 sibling, 1 reply; 18+ messages in thread From: Ed Tanous @ 2018-08-06 21:59 UTC (permalink / raw) To: openbmc > > We are going to investigate using the DFU protocol, as that > also has host side tools already available. > DFU doesn't completely solve the issue though, does it? Presumably for security reasons you can't have the DFU device exposed to the host all the time. If you did, I'm sure the penetration testers would hit it hard. Assuming that leaving it available all the time is a non-starter, don't you need some command to activate the interface to allow the upload? Assuming I'm not missing something there (I probably am) doesn't it make more sense to just expose a USB mass storage device when the "start" command is sent, as opposed to implementing the full DFU protocol? It seems like that would require no utilities (aside from a simple nsh/bash script) and be very easy to replicate. Is there any more details on this approach? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-08-06 21:59 ` Ed Tanous @ 2018-08-08 1:32 ` Jeremy Kerr 2018-08-08 21:42 ` Vernon Mauery 0 siblings, 1 reply; 18+ messages in thread From: Jeremy Kerr @ 2018-08-08 1:32 UTC (permalink / raw) To: Ed Tanous, openbmc Hi Ed, >> We are going to investigate using the DFU protocol, as that >> also has host side tools already available. > > DFU doesn't completely solve the issue though, does it? Presumably for > security reasons you can't have the DFU device exposed to the host all > the time. If you did, I'm sure the penetration testers would hit it > hard. Assuming that leaving it available all the time is a non-starter, > don't you need some command to activate the interface to allow the upload? Yes, we'd have an endpoint exposed all the time (the run-time descriptor set). This changes into the DFU mode descriptor set at the start of the upgrade process, when the host-side tools issue a DFU_DETACH. Does hiding the descriptor get us anything, if it can be enabled from the host anyway? The run-time descriptor is super simple. > Assuming I'm not missing something there (I probably am) doesn't it make > more sense to just expose a USB mass storage device when the "start" > command is sent, as opposed to implementing the full DFU protocol? It > seems like that would require no utilities (aside from a simple nsh/bash > script) and be very easy to replicate. Just that this requires more custom stuff host-side; raw IPMI commands for the start and stop (unless we can hook into a SCSI eject event for the stop perhaps). If we can use existing tools that already support DFU, that's less stuff that needs to be provided on the host side. That said, it does look like the current DFU spec isn't going to allow fast enough transfers for a BMC firmware image in reasonable times, so we may have to modify those *anyway*, so most of the benefit there may be lost... A USB-mass-storage-based solution could be okay, as long as we have a solid specification for what gets written to the device (a filesystem containing the image? raw image on the device?), secure parsing for that data (particularly if the BMC is reading a filesystem) and a protocol between BMC and host so that we can implement proper handover. If we can use standard tools for that, it might be a good way to go. Cheers, Jeremy ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: In-Band Firmware Update 2018-08-08 1:32 ` Jeremy Kerr @ 2018-08-08 21:42 ` Vernon Mauery 0 siblings, 0 replies; 18+ messages in thread From: Vernon Mauery @ 2018-08-08 21:42 UTC (permalink / raw) To: Jeremy Kerr; +Cc: Ed Tanous, openbmc On 08-Aug-2018 09:32 AM, Jeremy Kerr wrote: >Hi Ed, > >>>We are going to investigate using the DFU protocol, as that >>>also has host side tools already available. >> >>DFU doesn't completely solve the issue though, does it? Presumably >>for security reasons you can't have the DFU device exposed to the >>host all the time. If you did, I'm sure the penetration testers >>would hit it hard. Assuming that leaving it available all the time >>is a non-starter, don't you need some command to activate the >>interface to allow the upload? > >Yes, we'd have an endpoint exposed all the time (the run-time descriptor >set). This changes into the DFU mode descriptor set at the start of the >upgrade process, when the host-side tools issue a DFU_DETACH. > >Does hiding the descriptor get us anything, if it can be enabled from >the host anyway? The run-time descriptor is super simple. I am not saying security by obscurity is good, but there is a difference between an unlocked door and an open one. I am sure our security team would not want the firmware update descriptor present at all times. >>Assuming I'm not missing something there (I probably am) doesn't it >>make more sense to just expose a USB mass storage device when the >>"start" command is sent, as opposed to implementing the full DFU >>protocol? It seems like that would require no utilities (aside from >>a simple nsh/bash script) and be very easy to replicate. > >Just that this requires more custom stuff host-side; raw IPMI commands >for the start and stop (unless we can hook into a SCSI eject event for >the stop perhaps). If we can use existing tools that already support >DFU, that's less stuff that needs to be provided on the host side. > >That said, it does look like the current DFU spec isn't going to allow >fast enough transfers for a BMC firmware image in reasonable times, so >we may have to modify those *anyway*, so most of the benefit there may >be lost... Speed is a big thing. With firmware image sizes as big as they are, we cannot afford slow transfers. >A USB-mass-storage-based solution could be okay, as long as we have a >solid specification for what gets written to the device (a filesystem >containing the image? raw image on the device?), secure parsing for that >data (particularly if the BMC is reading a filesystem) and a protocol >between BMC and host so that we can implement proper handover. If we can >use standard tools for that, it might be a good way to go. If the BMC exports an empty FAT filesystem, just about anything can read that. It is easy to parse on the inbound and can easily be done with a loop-mount. In this case though, it would be nice to have some sort of control that tells the BMC when to "plug in" and "eject" the device to/from the host for synchronization mechanisms. The BMC could then look at the volume and attempt to use each file as a firmware volume to prepare for activation. --Vernon ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-08-08 21:42 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-07-23 18:13 In-Band Firmware Update Patrick Venture 2018-07-23 22:18 ` Vernon Mauery 2018-07-24 0:13 ` Sai Dasari 2018-07-24 19:02 ` Bakshi, Sachit 2018-07-24 20:38 ` Matt Spinler 2018-07-27 19:01 ` Patrick Venture 2018-08-06 16:51 ` Patrick Venture 2018-08-06 17:26 ` Patrick Venture 2018-08-06 18:36 ` Patrick Venture 2018-08-06 22:20 ` Ed Tanous 2018-08-06 23:04 ` Andrew Jeffery 2018-08-07 15:14 ` Patrick Venture 2018-08-07 16:41 ` Patrick Venture 2018-08-07 18:48 ` Patrick Venture 2018-08-07 15:11 ` Patrick Venture 2018-08-06 21:59 ` Ed Tanous 2018-08-08 1:32 ` Jeremy Kerr 2018-08-08 21:42 ` Vernon Mauery
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.