RFC: Integrating secure update functions into the FPGA Manager

* 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