* RFC: Integrating secure update functions into the FPGA Manager @ 2021-04-07 23:56 Russ Weight 2021-04-29 4:10 ` Moritz Fischer 0 siblings, 1 reply; 4+ messages in thread From: Russ Weight @ 2021-04-07 23:56 UTC (permalink / raw) To: Moritz Fischer, Xu, Yilun, Wu, Hao, Tom Rix; +Cc: linux-fpga Hi Moritz, Please see below my analysis of the effort to integrate secure functions into the FPGA Manager. I have recommendations interspersed below. The bottom line is that it seems like there is very little opportunity to share code. I could try to rewrite the fpga_mgr_load() related functions to accommodate worker threads, cancellation, and progress updates, but I'm not sure there is enough value-add to justify the the additional complexity, especially for the scatter-gather versions. The other options is to just port the secure-manager functions into the FPGA Manager. - Russ ======================================================================= This is a comparison of the FPGA Security Manager patch-set with the current FPGA Manager. Recommendations are provided on how to the extend the FPGA Manager to support the Security Manager functions. Purpose of the FPGA Security Manager Class Driver ================================================= The security manager provides a common user API (via sysfs) for transferring an opaque image to the card BMC for validation and disposition with a completion time of up to 40 minutes. A separate trigger function (image_load) can be used to activate a previously transferred image (e.g. FPGA Static Region or BMC image). Note that secure updates have no notion of Regions, Bridges or FPGA running state. A successful merge of the Security Manager with the FPGA Manager would include an extension of the FPGA Manager API to provide sysfs nodes to support secure updates. The FPGA Security Manager API ============================= Image Update (transfer data to Card BMC for validation & storage) WO: filename, cancel RO: name, status, error, hw_errinfo, remaining_size Image Load (trigger BMC, FPGA, or FW load from FLASH) RO: available_images # Images that can be loaded/activated WO: image_load: # Trigger an image load Merging Security Manager and FPGA Manager functions (organized by sysfs-node) ============================================================================= The "name" sysfs node has essentially the same purpose, so no change is required. The sec-mgr "status" is similar to fpga-mgr "state" The sec-mgr "error" is simliar to fpga-mgr "status" All others are unique to the security manager. * RECOMMENDATION: The secure-update sysfs files should be grouped together * in "secure-update" sysfs directory to clearly associate them with the * secure update process and separate them from the other driver functions. The sec-mgr sysfs status file has some similarity to the fpga-mgr sysfs state file: Sec-Mgr status FPGA-Mgr state -------------- -------------- FPGA_MGR_STATE_UNKNOWN FPGA_MGR_STATE_POWER_OFF FPGA_MGR_STATE_POWER_UP FPGA_MGR_STATE_RESET FPGA_SEC_PROG_IDLE FPGA_SEC_PROG_READING FPGA_MGR_STATE_FIRMWARE_REQ FPGA_MGR_STATE_FIRMWARE_REQ_ERR FPGA_SEC_PROG_PREPARING FPGA_MGR_STATE_WRITE_INIT FPGA_MGR_STATE_WRITE_INIT_ERR FPGA_SEC_PROG_WRITING FPGA_MGR_STATE_WRITE FPGA_MGR_STATE_WRITE_ERR FPGA_SEC_PROG_PROGRAMMING FPGA_MGR_STATE_WRITE_COMPLETE FPGA_MGR_STATE_WRITE_COMPLETE_ERR FPGA_MGR_STATE_OPERATING FPGA_SEC_PROG_IDLE The sec-mgr sysfs error file seems to be similar in purpose to the fpga-mgr sysfs status file, but there is no overlap in the actual error codes. * RECOMMENDATION: there is not enough similarity between the status/state and * error/status nodes to try to share them. If all of the other secure-update * related nodes are grouped together in a secure-update directory, it would * probably create confusion to try to share the state and status files that * are in a different location. sysfs: filename =============== In the current security manager patches, writing the name of an image file to "filename" is roughly equivalent to creating a worker thread and having it call fpga_mgr_load() with info->firmware_name set to the image file name. The update sequences are similar: Sec-Mgr ops flow FPGA-Mgr ops flow ================ ================= prepare() write_init() # Loop on 16KB blocks write() write_blk() poll_complete() write_complete() In the worst case, we have seen n3000 FPGA image updates take up to 40 minutes. The remaining_size is updated between each 16KB block to allow users to monitor the progress of the data transfer. The sec-mgr also checks for a cancel flag between each 16KB write and between each stage of the update. The fpga_mgr has a "buffer" flow and a "scatter-gather" flow. The sec-mgr flow is always started via sysfs and uses request_firmware(), so it only needs one (buffer) flow. * RECOMMENDATION: keep the update functions separate. Although they are * similar, the fpga-mgr updates are simpler and more readable as they * are. The sec-mgr update is done via kernel worker thread and is a little * more complicated with support for maintaining the remaining size and * with support for cancellation. There currently no need for a scatter-gather * secure update implementation since secure updates use request_firmware. Note that the sec-mgr supports additional ops: cancel() : send cancel-request to lower-level driver cleanup() : optional: called if and only if the prepare op succeeds get_hw_errinfo() : optional: provides device-specific 64-bit error info sysfs: cancel, status, error, hw_errinfo, remaining_size ======================================================== * RECOMMENDATION: There are no equivalent features in fpga-mgr. Port * these each from the security manager to the FPGA Manager. sysfs: available_images, load_image =================================== available_images: RO: list device-specific keywords that can be used to trigger an image to be loaded/activated. eg. fpga_factory, fpga_user1, fpga_user2, bmc_factory. load_image: WO: trigger the load/activation of an image from FLASH * RECOMMENDATION: Nothing similar in fpga-mgr. Port functionality to the * FPGA Manager Conclusion ========== The purpose of the secure-update support is to provide a common user API for loading an opaque image to a device with completion times of up to 40 minutes. The FPGA Manager can be extended to include the sysfs nodes required for a secure update. There is very little opportunity for shared code between the current FPGA Manager and the secure update support. For the most part, this would be an addition of new functionality. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFC: Integrating secure update functions into the FPGA Manager 2021-04-07 23:56 RFC: Integrating secure update functions into the FPGA Manager Russ Weight @ 2021-04-29 4:10 ` Moritz Fischer 2021-04-30 15:44 ` Russ Weight 0 siblings, 1 reply; 4+ messages in thread From: Moritz Fischer @ 2021-04-29 4:10 UTC (permalink / raw) To: Russ Weight; +Cc: Moritz Fischer, Xu, Yilun, Wu, Hao, Tom Rix, linux-fpga Hi Russ, On Wed, Apr 07, 2021 at 04:56:29PM -0700, Russ Weight wrote: > Hi Moritz, > > Please see below my analysis of the effort to integrate secure functions > into the FPGA Manager. I have recommendations interspersed below. The > bottom line is that it seems like there is very little opportunity to > share code. I could try to rewrite the fpga_mgr_load() related functions > to accommodate worker threads, cancellation, and progress updates, but I'm > not sure there is enough value-add to justify the the additional complexity, > especially for the scatter-gather versions. The other options is to just > port the secure-manager functions into the FPGA Manager. > > - Russ > > ======================================================================= > > This is a comparison of the FPGA Security Manager patch-set with the > current FPGA Manager. Recommendations are provided on how to the extend > the FPGA Manager to support the Security Manager functions. > > Purpose of the FPGA Security Manager Class Driver > ================================================= > > The security manager provides a common user API (via sysfs) for transferring > an opaque image to the card BMC for validation and disposition with a > completion time of up to 40 minutes. A separate trigger function (image_load) > can be used to activate a previously transferred image (e.g. FPGA Static > Region or BMC image). > > Note that secure updates have no notion of Regions, Bridges or FPGA > running state. Not needed. > > A successful merge of the Security Manager with the FPGA Manager would include > an extension of the FPGA Manager API to provide sysfs nodes to support secure > updates. Yes. > > The FPGA Security Manager API > ============================= > > Image Update (transfer data to Card BMC for validation & storage) > WO: filename, cancel > RO: name, status, error, hw_errinfo, remaining_size > > Image Load (trigger BMC, FPGA, or FW load from FLASH) > RO: available_images # Images that can be loaded/activated > WO: image_load: # Trigger an image load > > > Merging Security Manager and FPGA Manager functions (organized by sysfs-node) > ============================================================================= > > The "name" sysfs node has essentially the same purpose, so no change is > required. > > The sec-mgr "status" is similar to fpga-mgr "state" > The sec-mgr "error" is simliar to fpga-mgr "status" > > All others are unique to the security manager. > > * RECOMMENDATION: The secure-update sysfs files should be grouped together > * in "secure-update" sysfs directory to clearly associate them with the > * secure update process and separate them from the other driver functions. SGTM. > > The sec-mgr sysfs status file has some similarity to the fpga-mgr sysfs > state file: > > Sec-Mgr status FPGA-Mgr state > -------------- -------------- > FPGA_MGR_STATE_UNKNOWN > FPGA_MGR_STATE_POWER_OFF > FPGA_MGR_STATE_POWER_UP > FPGA_MGR_STATE_RESET > FPGA_SEC_PROG_IDLE > FPGA_SEC_PROG_READING FPGA_MGR_STATE_FIRMWARE_REQ > FPGA_MGR_STATE_FIRMWARE_REQ_ERR > FPGA_SEC_PROG_PREPARING FPGA_MGR_STATE_WRITE_INIT > FPGA_MGR_STATE_WRITE_INIT_ERR > FPGA_SEC_PROG_WRITING FPGA_MGR_STATE_WRITE > FPGA_MGR_STATE_WRITE_ERR > FPGA_SEC_PROG_PROGRAMMING FPGA_MGR_STATE_WRITE_COMPLETE > FPGA_MGR_STATE_WRITE_COMPLETE_ERR > FPGA_MGR_STATE_OPERATING > FPGA_SEC_PROG_IDLE > > The sec-mgr sysfs error file seems to be similar in purpose to the > fpga-mgr sysfs status file, but there is no overlap in the actual > error codes. > > * RECOMMENDATION: there is not enough similarity between the status/state and > * error/status nodes to try to share them. If all of the other secure-update > * related nodes are grouped together in a secure-update directory, it would > * probably create confusion to try to share the state and status files that > * are in a different location. Yes. > > sysfs: filename > =============== > > In the current security manager patches, writing the name of an image file > to "filename" is roughly equivalent to creating a worker thread and having it > call fpga_mgr_load() with info->firmware_name set to the image file name. > > The update sequences are similar: > > Sec-Mgr ops flow FPGA-Mgr ops flow > ================ ================= > prepare() write_init() > # Loop on 16KB blocks write() > write_blk() > poll_complete() write_complete() > > In the worst case, we have seen n3000 FPGA image updates take up to 40 > minutes. The remaining_size is updated between each 16KB block to > allow users to monitor the progress of the data transfer. The sec-mgr > also checks for a cancel flag between each 16KB write and between > each stage of the update. > > The fpga_mgr has a "buffer" flow and a "scatter-gather" flow. The sec-mgr > flow is always started via sysfs and uses request_firmware(), so it only > needs one (buffer) flow. > > * RECOMMENDATION: keep the update functions separate. Although they are > * similar, the fpga-mgr updates are simpler and more readable as they > * are. The sec-mgr update is done via kernel worker thread and is a little > * more complicated with support for maintaining the remaining size and > * with support for cancellation. There currently no need for a scatter-gather > * secure update implementation since secure updates use request_firmware. > > Note that the sec-mgr supports additional ops: > cancel() : send cancel-request to lower-level driver > cleanup() : optional: called if and only if the prepare op succeeds > get_hw_errinfo() : optional: provides device-specific 64-bit error info > > sysfs: cancel, status, error, hw_errinfo, remaining_size > ======================================================== > > * RECOMMENDATION: There are no equivalent features in fpga-mgr. Port > * these each from the security manager to the FPGA Manager. SGTM. > > sysfs: available_images, load_image > =================================== > > available_images: RO: list device-specific keywords that can be used > to trigger an image to be loaded/activated. eg. > fpga_factory, fpga_user1, fpga_user2, bmc_factory. > > load_image: WO: trigger the load/activation of an image from FLASH > > * RECOMMENDATION: Nothing similar in fpga-mgr. Port functionality to the > * FPGA Manager > > Conclusion > ========== > > The purpose of the secure-update support is to provide a common user API > for loading an opaque image to a device with completion times of up to 40 > minutes. The FPGA Manager can be extended to include the sysfs nodes > required for a secure update. There is very little opportunity for shared > code between the current FPGA Manager and the secure update support. For > the most part, this would be an addition of new functionality. Sounds good to me overall, sorry for the late response. - Moritz ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFC: Integrating secure update functions into the FPGA Manager 2021-04-29 4:10 ` Moritz Fischer @ 2021-04-30 15:44 ` Russ Weight 2021-05-02 18:41 ` Moritz Fischer 0 siblings, 1 reply; 4+ messages in thread From: Russ Weight @ 2021-04-30 15:44 UTC (permalink / raw) To: Moritz Fischer; +Cc: Xu, Yilun, Wu, Hao, Tom Rix, linux-fpga Moritz, On 4/28/21 9:10 PM, Moritz Fischer wrote: > Hi Russ, > > On Wed, Apr 07, 2021 at 04:56:29PM -0700, Russ Weight wrote: >> Hi Moritz, >> >> Please see below my analysis of the effort to integrate secure functions >> into the FPGA Manager. I have recommendations interspersed below. The >> bottom line is that it seems like there is very little opportunity to >> share code. I could try to rewrite the fpga_mgr_load() related functions >> to accommodate worker threads, cancellation, and progress updates, but I'm >> not sure there is enough value-add to justify the the additional complexity, >> especially for the scatter-gather versions. The other options is to just >> port the secure-manager functions into the FPGA Manager. >> >> - Russ >> >> ======================================================================= >> >> This is a comparison of the FPGA Security Manager patch-set with the >> current FPGA Manager. Recommendations are provided on how to the extend >> the FPGA Manager to support the Security Manager functions. >> >> Purpose of the FPGA Security Manager Class Driver >> ================================================= >> >> The security manager provides a common user API (via sysfs) for transferring >> an opaque image to the card BMC for validation and disposition with a >> completion time of up to 40 minutes. A separate trigger function (image_load) >> can be used to activate a previously transferred image (e.g. FPGA Static >> Region or BMC image). >> >> Note that secure updates have no notion of Regions, Bridges or FPGA >> running state. > Not needed. >> A successful merge of the Security Manager with the FPGA Manager would include >> an extension of the FPGA Manager API to provide sysfs nodes to support secure >> updates. > Yes. >> The FPGA Security Manager API >> ============================= >> >> Image Update (transfer data to Card BMC for validation & storage) >> WO: filename, cancel >> RO: name, status, error, hw_errinfo, remaining_size >> >> Image Load (trigger BMC, FPGA, or FW load from FLASH) >> RO: available_images # Images that can be loaded/activated >> WO: image_load: # Trigger an image load >> >> >> Merging Security Manager and FPGA Manager functions (organized by sysfs-node) >> ============================================================================= >> >> The "name" sysfs node has essentially the same purpose, so no change is >> required. >> >> The sec-mgr "status" is similar to fpga-mgr "state" >> The sec-mgr "error" is simliar to fpga-mgr "status" >> >> All others are unique to the security manager. >> >> * RECOMMENDATION: The secure-update sysfs files should be grouped together >> * in "secure-update" sysfs directory to clearly associate them with the >> * secure update process and separate them from the other driver functions. > SGTM. >> The sec-mgr sysfs status file has some similarity to the fpga-mgr sysfs >> state file: >> >> Sec-Mgr status FPGA-Mgr state >> -------------- -------------- >> FPGA_MGR_STATE_UNKNOWN >> FPGA_MGR_STATE_POWER_OFF >> FPGA_MGR_STATE_POWER_UP >> FPGA_MGR_STATE_RESET >> FPGA_SEC_PROG_IDLE >> FPGA_SEC_PROG_READING FPGA_MGR_STATE_FIRMWARE_REQ >> FPGA_MGR_STATE_FIRMWARE_REQ_ERR >> FPGA_SEC_PROG_PREPARING FPGA_MGR_STATE_WRITE_INIT >> FPGA_MGR_STATE_WRITE_INIT_ERR >> FPGA_SEC_PROG_WRITING FPGA_MGR_STATE_WRITE >> FPGA_MGR_STATE_WRITE_ERR >> FPGA_SEC_PROG_PROGRAMMING FPGA_MGR_STATE_WRITE_COMPLETE >> FPGA_MGR_STATE_WRITE_COMPLETE_ERR >> FPGA_MGR_STATE_OPERATING >> FPGA_SEC_PROG_IDLE >> >> The sec-mgr sysfs error file seems to be similar in purpose to the >> fpga-mgr sysfs status file, but there is no overlap in the actual >> error codes. >> >> * RECOMMENDATION: there is not enough similarity between the status/state and >> * error/status nodes to try to share them. If all of the other secure-update >> * related nodes are grouped together in a secure-update directory, it would >> * probably create confusion to try to share the state and status files that >> * are in a different location. > Yes. >> sysfs: filename >> =============== >> >> In the current security manager patches, writing the name of an image file >> to "filename" is roughly equivalent to creating a worker thread and having it >> call fpga_mgr_load() with info->firmware_name set to the image file name. >> >> The update sequences are similar: >> >> Sec-Mgr ops flow FPGA-Mgr ops flow >> ================ ================= >> prepare() write_init() >> # Loop on 16KB blocks write() >> write_blk() >> poll_complete() write_complete() >> >> In the worst case, we have seen n3000 FPGA image updates take up to 40 >> minutes. The remaining_size is updated between each 16KB block to >> allow users to monitor the progress of the data transfer. The sec-mgr >> also checks for a cancel flag between each 16KB write and between >> each stage of the update. >> >> The fpga_mgr has a "buffer" flow and a "scatter-gather" flow. The sec-mgr >> flow is always started via sysfs and uses request_firmware(), so it only >> needs one (buffer) flow. >> >> * RECOMMENDATION: keep the update functions separate. Although they are >> * similar, the fpga-mgr updates are simpler and more readable as they >> * are. The sec-mgr update is done via kernel worker thread and is a little >> * more complicated with support for maintaining the remaining size and >> * with support for cancellation. There currently no need for a scatter-gather >> * secure update implementation since secure updates use request_firmware. >> >> Note that the sec-mgr supports additional ops: >> cancel() : send cancel-request to lower-level driver >> cleanup() : optional: called if and only if the prepare op succeeds >> get_hw_errinfo() : optional: provides device-specific 64-bit error info >> >> sysfs: cancel, status, error, hw_errinfo, remaining_size >> ======================================================== >> >> * RECOMMENDATION: There are no equivalent features in fpga-mgr. Port >> * these each from the security manager to the FPGA Manager. > SGTM. >> sysfs: available_images, load_image >> =================================== >> >> available_images: RO: list device-specific keywords that can be used >> to trigger an image to be loaded/activated. eg. >> fpga_factory, fpga_user1, fpga_user2, bmc_factory. >> >> load_image: WO: trigger the load/activation of an image from FLASH >> >> * RECOMMENDATION: Nothing similar in fpga-mgr. Port functionality to the >> * FPGA Manager >> >> Conclusion >> ========== >> >> The purpose of the secure-update support is to provide a common user API >> for loading an opaque image to a device with completion times of up to 40 >> minutes. The FPGA Manager can be extended to include the sysfs nodes >> required for a secure update. There is very little opportunity for shared >> code between the current FPGA Manager and the secure update support. For >> the most part, this would be an addition of new functionality. > Sounds good to me overall, sorry for the late response. Thanks for the feedback Mortiz. Have you had a chance to look at the follow-up patch that I sent out? Between the cover letter and the RFC patch itself it gives a better idea of what a merged version of the driver would look like and raises some questions about how tightly the new functionality should be integrated with the fpga manager. https://marc.info/?l=linux-fpga&m=161841884401312&w=2 https://marc.info/?l=linux-fpga&m=161884993123556&w=2 Thanks, - Russ > > - Moritz ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFC: Integrating secure update functions into the FPGA Manager 2021-04-30 15:44 ` Russ Weight @ 2021-05-02 18:41 ` Moritz Fischer 0 siblings, 0 replies; 4+ messages in thread From: Moritz Fischer @ 2021-05-02 18:41 UTC (permalink / raw) To: Russ Weight; +Cc: Moritz Fischer, Xu, Yilun, Wu, Hao, Tom Rix, linux-fpga Hi Russ, Richard On Fri, Apr 30, 2021 at 08:44:18AM -0700, Russ Weight wrote: > Moritz, > > > On 4/28/21 9:10 PM, Moritz Fischer wrote: > > Hi Russ, > > > > On Wed, Apr 07, 2021 at 04:56:29PM -0700, Russ Weight wrote: > >> Hi Moritz, > >> > >> Please see below my analysis of the effort to integrate secure functions > >> into the FPGA Manager. I have recommendations interspersed below. The > >> bottom line is that it seems like there is very little opportunity to > >> share code. I could try to rewrite the fpga_mgr_load() related functions > >> to accommodate worker threads, cancellation, and progress updates, but I'm > >> not sure there is enough value-add to justify the the additional complexity, > >> especially for the scatter-gather versions. The other options is to just > >> port the secure-manager functions into the FPGA Manager. > >> > >> - Russ > >> > >> ======================================================================= > >> > >> This is a comparison of the FPGA Security Manager patch-set with the > >> current FPGA Manager. Recommendations are provided on how to the extend > >> the FPGA Manager to support the Security Manager functions. > >> > >> Purpose of the FPGA Security Manager Class Driver > >> ================================================= > >> > >> The security manager provides a common user API (via sysfs) for transferring > >> an opaque image to the card BMC for validation and disposition with a > >> completion time of up to 40 minutes. A separate trigger function (image_load) > >> can be used to activate a previously transferred image (e.g. FPGA Static > >> Region or BMC image). > >> > >> Note that secure updates have no notion of Regions, Bridges or FPGA > >> running state. > > Not needed. > >> A successful merge of the Security Manager with the FPGA Manager would include > >> an extension of the FPGA Manager API to provide sysfs nodes to support secure > >> updates. > > Yes. > >> The FPGA Security Manager API > >> ============================= > >> > >> Image Update (transfer data to Card BMC for validation & storage) > >> WO: filename, cancel > >> RO: name, status, error, hw_errinfo, remaining_size > >> > >> Image Load (trigger BMC, FPGA, or FW load from FLASH) > >> RO: available_images # Images that can be loaded/activated > >> WO: image_load: # Trigger an image load > >> > >> > >> Merging Security Manager and FPGA Manager functions (organized by sysfs-node) > >> ============================================================================= > >> > >> The "name" sysfs node has essentially the same purpose, so no change is > >> required. > >> > >> The sec-mgr "status" is similar to fpga-mgr "state" > >> The sec-mgr "error" is simliar to fpga-mgr "status" > >> > >> All others are unique to the security manager. > >> > >> * RECOMMENDATION: The secure-update sysfs files should be grouped together > >> * in "secure-update" sysfs directory to clearly associate them with the > >> * secure update process and separate them from the other driver functions. > > SGTM. > >> The sec-mgr sysfs status file has some similarity to the fpga-mgr sysfs > >> state file: > >> > >> Sec-Mgr status FPGA-Mgr state > >> -------------- -------------- > >> FPGA_MGR_STATE_UNKNOWN > >> FPGA_MGR_STATE_POWER_OFF > >> FPGA_MGR_STATE_POWER_UP > >> FPGA_MGR_STATE_RESET > >> FPGA_SEC_PROG_IDLE > >> FPGA_SEC_PROG_READING FPGA_MGR_STATE_FIRMWARE_REQ > >> FPGA_MGR_STATE_FIRMWARE_REQ_ERR > >> FPGA_SEC_PROG_PREPARING FPGA_MGR_STATE_WRITE_INIT > >> FPGA_MGR_STATE_WRITE_INIT_ERR > >> FPGA_SEC_PROG_WRITING FPGA_MGR_STATE_WRITE > >> FPGA_MGR_STATE_WRITE_ERR > >> FPGA_SEC_PROG_PROGRAMMING FPGA_MGR_STATE_WRITE_COMPLETE > >> FPGA_MGR_STATE_WRITE_COMPLETE_ERR > >> FPGA_MGR_STATE_OPERATING > >> FPGA_SEC_PROG_IDLE > >> > >> The sec-mgr sysfs error file seems to be similar in purpose to the > >> fpga-mgr sysfs status file, but there is no overlap in the actual > >> error codes. > >> > >> * RECOMMENDATION: there is not enough similarity between the status/state and > >> * error/status nodes to try to share them. If all of the other secure-update > >> * related nodes are grouped together in a secure-update directory, it would > >> * probably create confusion to try to share the state and status files that > >> * are in a different location. > > Yes. > >> sysfs: filename > >> =============== > >> > >> In the current security manager patches, writing the name of an image file > >> to "filename" is roughly equivalent to creating a worker thread and having it > >> call fpga_mgr_load() with info->firmware_name set to the image file name. > >> > >> The update sequences are similar: > >> > >> Sec-Mgr ops flow FPGA-Mgr ops flow > >> ================ ================= > >> prepare() write_init() > >> # Loop on 16KB blocks write() > >> write_blk() > >> poll_complete() write_complete() > >> > >> In the worst case, we have seen n3000 FPGA image updates take up to 40 > >> minutes. The remaining_size is updated between each 16KB block to > >> allow users to monitor the progress of the data transfer. The sec-mgr > >> also checks for a cancel flag between each 16KB write and between > >> each stage of the update. > >> > >> The fpga_mgr has a "buffer" flow and a "scatter-gather" flow. The sec-mgr > >> flow is always started via sysfs and uses request_firmware(), so it only > >> needs one (buffer) flow. > >> > >> * RECOMMENDATION: keep the update functions separate. Although they are > >> * similar, the fpga-mgr updates are simpler and more readable as they > >> * are. The sec-mgr update is done via kernel worker thread and is a little > >> * more complicated with support for maintaining the remaining size and > >> * with support for cancellation. There currently no need for a scatter-gather > >> * secure update implementation since secure updates use request_firmware. > >> > >> Note that the sec-mgr supports additional ops: > >> cancel() : send cancel-request to lower-level driver > >> cleanup() : optional: called if and only if the prepare op succeeds > >> get_hw_errinfo() : optional: provides device-specific 64-bit error info > >> > >> sysfs: cancel, status, error, hw_errinfo, remaining_size > >> ======================================================== > >> > >> * RECOMMENDATION: There are no equivalent features in fpga-mgr. Port > >> * these each from the security manager to the FPGA Manager. > > SGTM. > >> sysfs: available_images, load_image > >> =================================== > >> > >> available_images: RO: list device-specific keywords that can be used > >> to trigger an image to be loaded/activated. eg. > >> fpga_factory, fpga_user1, fpga_user2, bmc_factory. > >> > >> load_image: WO: trigger the load/activation of an image from FLASH > >> > >> * RECOMMENDATION: Nothing similar in fpga-mgr. Port functionality to the > >> * FPGA Manager > >> > >> Conclusion > >> ========== > >> > >> The purpose of the secure-update support is to provide a common user API > >> for loading an opaque image to a device with completion times of up to 40 > >> minutes. The FPGA Manager can be extended to include the sysfs nodes > >> required for a secure update. There is very little opportunity for shared > >> code between the current FPGA Manager and the secure update support. For > >> the most part, this would be an addition of new functionality. > > Sounds good to me overall, sorry for the late response. > > Thanks for the feedback Mortiz. Have you had a chance to look at the follow-up > patch that I sent out? Between the cover letter and the RFC patch itself it > gives a better idea of what a merged version of the driver would look like > and raises some questions about how tightly the new functionality should be > integrated with the fpga manager. > > https://marc.info/?l=linux-fpga&m=161841884401312&w=2 > https://marc.info/?l=linux-fpga&m=161884993123556&w=2 > > Thanks, > - Russ > > > > - Moritz > Yeah, took another look. It does complicate the FPGA Manager code somewhat. Especially the unregister path. I'm starting to think maybe your original proposal made more sense... Can you resend your original patchset updated for 5.14 (docs) and let's get this merged this cycle. Sorry for the back and forth. Regarding Richard's issue, can't we just have a sysfs entry that you cat the filename into and it'll lete you know if the authentication passes. Like echo 'foo.bin' > /sys/class/fpga-mgr/verify_image And then have a corresponding status? I'm not sure the overlay path adds anything since we would not add nodes to the overlay? - Moritz ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-05-02 18:41 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-07 23:56 RFC: Integrating secure update functions into the FPGA Manager Russ Weight 2021-04-29 4:10 ` Moritz Fischer 2021-04-30 15:44 ` Russ Weight 2021-05-02 18:41 ` Moritz Fischer
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.