lkml.org 
[lkml]   [2020]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v6 1/7] fpga: sec-mgr: fpga security manager class driver
Hi Russ,

On Thu, Nov 19, 2020 at 06:39:44PM -0800, Russ Weight wrote:
>
>
> On 11/15/20 3:03 PM, Moritz Fischer wrote:
> > Hi Russ,
> >
> > On Thu, Nov 05, 2020 at 05:08:59PM -0800, Russ Weight wrote:
> >> Create the FPGA Security Manager class driver. The security
> >> manager provides interfaces to manage secure updates for the
> >> FPGA and BMC images that are stored in FLASH. The driver can
> >> also be used to update root entry hashes and to cancel code
> >> signing keys. The image type is encoded in the image file
> >> and is decoded by the HW/FW secure update engine.
> >>
> >> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> >> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> >> Reviewed-by: Tom Rix <trix@redhat.com>
> >> ---
> >> v6:
> >> - Removed sysfs support and documentation for the display of the
> >> flash count, root entry hashes, and code-signing-key cancelation
> >> vectors.
> >> v5:
> >> - Added the devm_fpga_sec_mgr_unregister() function, following recent
> >> changes to the fpga_manager() implementation.
> >> - Changed some *_show() functions to use sysfs_emit() instead of sprintf(
> >> v4:
> >> - Changed from "Intel FPGA Security Manager" to FPGA Security Manager"
> >> and removed unnecessary references to "Intel".
> >> - Changed: iops -> sops, imgr -> smgr, IFPGA_ -> FPGA_, ifpga_ to fpga_
> >> v3:
> >> - Modified sysfs handler check in check_sysfs_handler() to make
> >> it more readable.
> >> v2:
> >> - Bumped documentation dates and versions
> >> - Added Documentation/fpga/ifpga-sec-mgr.rst
> >> - Removed references to bmc_flash_count & smbus_flash_count (not supported)
> >> - Split ifpga_sec_mgr_register() into create() and register() functions
> >> - Added devm_ifpga_sec_mgr_create()
> >> - Removed typedefs for imgr ops
> >> ---
> >> .../ABI/testing/sysfs-class-fpga-sec-mgr | 5 +
> >> Documentation/fpga/fpga-sec-mgr.rst | 44 +++
> >> Documentation/fpga/index.rst | 1 +
> >> MAINTAINERS | 9 +
> >> drivers/fpga/Kconfig | 9 +
> >> drivers/fpga/Makefile | 3 +
> >> drivers/fpga/fpga-sec-mgr.c | 296 ++++++++++++++++++
> >> include/linux/fpga/fpga-sec-mgr.h | 44 +++
> >> 8 files changed, 411 insertions(+)
> >> create mode 100644 Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
> >> create mode 100644 Documentation/fpga/fpga-sec-mgr.rst
> >> create mode 100644 drivers/fpga/fpga-sec-mgr.c
> >> create mode 100644 include/linux/fpga/fpga-sec-mgr.h
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
> >> new file mode 100644
> >> index 000000000000..ecda22a3ff3b
> >> --- /dev/null
> >> +++ b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
> >> @@ -0,0 +1,5 @@
> >> +What: /sys/class/fpga_sec_mgr/fpga_secX/name
> >> +Date: Oct 2020
> >> +KernelVersion: 5.11
> >> +Contact: Russ Weight <russell.h.weight@intel.com>
> >> +Description: Name of low level fpga security manager driver.
> >> diff --git a/Documentation/fpga/fpga-sec-mgr.rst b/Documentation/fpga/fpga-sec-mgr.rst
> >> new file mode 100644
> >> index 000000000000..26dac599ead7
> >> --- /dev/null
> >> +++ b/Documentation/fpga/fpga-sec-mgr.rst
> >> @@ -0,0 +1,44 @@
> >> +.. SPDX-License-Identifier: GPL-2.0
> >> +
> >> +========================================
> >> +FPGA Security Manager Class Driver
> >> +========================================
> >> +
> >> +The FPGA Security Manager class driver provides a common
> >> +API for user-space tools to manage updates for secure FPGA
> >> +devices. Device drivers that instantiate the Security
> >> +Manager class driver will interact with a HW secure update
> >> +engine in order to transfer new FPGA and BMC images to FLASH so
> >> +that they will be automatically loaded when the FPGA card reboots.
> >> +
> >> +A significant difference between the FPGA Manager and the FPGA
> >> +Security Manager is that the FPGA Manager does a live update (Partial
> >> +Reconfiguration) to a device, whereas the FPGA Security Manager
> >> +updates the FLASH images for the Static Region and the BMC so that
> >> +they will be loaded the next time the FPGA card boots. Security is
> >> +enforced by hardware and firmware. The security manager interacts
> >> +with the firmware to initiate an update, pass in the necessary data,
> >> +and collect status on the update.
> > I've always wondered if we could've made this a functionality of an FPGA
> > manager 'non-volatile' node or something.
> >
> > I guess there might be cases where you can only do either of them, i.e.
> > only update flash or only update at runtime.
>
> Today, in light of Richard Gong's recent patch set, I took another look at
> the fpga manager, trying to determine what changes would need to be made in
> the fpga manager order to support secure updates. These are my observations:
>
> (1) For the devices that I am working on, the lower-level drivers are
>     completely different for PR vs image updates to flash. As a result,
>     if we used the fpga-mgr, we would need to create different instances
>     of the fpga-mgr, one for PR and one for secure updates - each supported
>     by a different low-level driver.

I was mostly thinking about adding a somewhat similar API to the FPGA
manager (close to what you're doing), but as I said it was a suggestion.
>
> (2) For secure updates, our worst case time is 40 minutes. I doubt that it
>     will ever be longer than that, but we need to support that case. For this
>     length of time, we feel that it is important to show some indication
>     of progress to the user during the update. To handle this, we
>     are using a write_blk() function to break up the writes so that the
>     class driver can provide updates during the data transfer (e.g. this
>     much is left to transfer). We have also "backgrounded" the kernel
>     process by spawning a kernel worker thread to do the update. The user,
>     or user-space code, can monitor the progress by polling for status
>     through sysfs.
>
> (3) Also, because of the long updates, it seems necessary to provide a way
>     to cancel the update. For example, if the user accidentally specifies the
>     wrong image file, 40 minutes is too long to wait before they are able
>     to try again. We have provided a way to signal the worker thread to
>     abort when possible.
>
> (4) Another observation is that we are using the same secure update mechanism
>     to program new root-entry-hashes and to cancel code-signing keys. The
>     image type is encoded in the file header. The payload is opaque to host
>     software, so this isn't an issue - just an observation.
>    
> So is it worth adding these additional features to the fpga-mgr? Or is it
> better to keep them separate? To me they seem different enough, that I think
> it would be cleaner to keep them separate.

Yeah, I think that's fine. Thanks for taking another look, though.

Cheers,
Moritz

\
 
 \ /
  Last update: 2020-11-20 07:28    [W:0.100 / U:2.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site