2021-03-09 00:37:46

by Russ Weight

[permalink] [raw]
Subject: [PATCH v10 0/7] 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 FPGA 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.

The n3000bmc-secure driver is the first driver to use the FPGA
Security Manager. This driver was previously submitted in the same
patch set, but has been split out into a separate patch set starting
with V2. Future devices will also make use of this common API for
secure updates.

In addition to managing secure updates of the FPGA and BMC images,
the FPGA Security Manager update process may also be used to
program root entry hashes and cancellation keys for the FPGA static
region, the FPGA partial reconfiguration region, and the BMC.
The image files are self-describing, and contain a header describing
the image type.

Secure updates make use of the request_firmware framework, which
requires that image files are accessible under /lib/firmware. A request
for a secure update returns immediately, while the update itself
proceeds in the context of a kernel worker thread. Sysfs files provide
a means for monitoring the progress of a secure update and for
retrieving error information in the event of a failure.

The API includes a "name" sysfs file to export the name of the parent
driver. It also includes an "update" sub-directory containing files that
that can be used to instantiate and monitor a secure update.

Changelog v9 -> v10:
- Rebased to 5.12-rc2 next
- Updated Date and KernelVersion in ABI documentation

Changelog v8 -> v9:
- Rebased patches for 5.11-rc2
- Updated Date and KernelVersion in ABI documentation

Changelog v7 -> v8:
- Fixed grammatical error in Documentation/fpga/fpga-sec-mgr.rst

Changelog v6 -> v7:
- Changed dates in documentation file to December 2020
- Changed filename_store() to use kmemdup_nul() instead of
kstrndup() and changed the count to not assume a line-return.

Changelog v5 -> v6:
- Removed sysfs support and documentation for the display of the
flash count, root entry hashes, and code-signing-key cancelation
vectors from the class driver. This information can vary by device
and will instead be displayed by the device-specific parent driver.

Changelog v4 -> v5:
- Added the devm_fpga_sec_mgr_unregister() function, following recent
changes to the fpga_manager() implementation.
- Changed most of the *_show() functions to use sysfs_emit()
instead of sprintf(
- When checking the return values for functions of type enum
fpga_sec_err err_code, test for FPGA_SEC_ERR_NONE instead of 0

Changelog v3 -> v4:
- This driver is generic enough that it could be used for non Intel
FPGA devices. 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_
Note that this also affects some filenames.

Changelog v2 -> v3:
- Use dev_err() to report invalid progress in sec_progress()
- Use dev_err() to report invalid error code in sec_error()
- Modified sysfs handler check in check_sysfs_handler() to make
it more readable.
- Removed unnecessary "goto done"
- Added a comment to explain imgr->driver_unload in
ifpga_sec_mgr_unregister()

Changelog v1 -> v2:
- Separated out the MAX10 BMC Security Engine to be submitted in
a separate patch-set.
- Bumped documentation dates and versions
- Split ifpga_sec_mgr_register() into create() and register() functions
- Added devm_ifpga_sec_mgr_create()
- Added Documentation/fpga/ifpga-sec-mgr.rst
- Changed progress state "read_file" to "reading"
- Added sec_error() function (similar to sec_progress())
- Removed references to bmc_flash_count & smbus_flash_count (not supported)
- Removed typedefs for imgr ops
- Removed explicit value assignments in enums
- Other minor code cleanup per review comments

Russ Weight (7):
fpga: sec-mgr: fpga security manager class driver
fpga: sec-mgr: enable secure updates
fpga: sec-mgr: expose sec-mgr update status
fpga: sec-mgr: expose sec-mgr update errors
fpga: sec-mgr: expose sec-mgr update size
fpga: sec-mgr: enable cancel of secure update
fpga: sec-mgr: expose hardware error info

.../ABI/testing/sysfs-class-fpga-sec-mgr | 81 +++
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 | 652 ++++++++++++++++++
include/linux/fpga/fpga-sec-mgr.h | 100 +++
8 files changed, 899 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

--
2.25.1


2021-03-09 00:37:46

by Russ Weight

[permalink] [raw]
Subject: [PATCH v10 4/7] fpga: sec-mgr: expose sec-mgr update errors

Extend the FPGA Security Manager class driver to include
an update/error sysfs node that can be read for error
information when a secure update fails.

Signed-off-by: Russ Weight <[email protected]>
Reviewed-by: Tom Rix <[email protected]>
---
v10:
- Rebased to 5.12-rc2 next
- Updated Date and KernelVersion in ABI documentation
v9:
- Updated Date and KernelVersion in ABI documentation
v8:
- No change
v7:
- Changed Date in documentation file to December 2020
v6:
- No change
v5:
- Use new function sysfs_emit() in the error_show() function
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:
- Use dev_err() for invalid error code in sec_error()
v2:
- Bumped documentation date and version
- Added warning to sec_progress() for invalid progress status
- Added sec_error() function (similar to sec_progress())
---
.../ABI/testing/sysfs-class-fpga-sec-mgr | 17 ++++
drivers/fpga/fpga-sec-mgr.c | 83 ++++++++++++++++---
include/linux/fpga/fpga-sec-mgr.h | 1 +
3 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
index 64420c101626..77738577b2c3 100644
--- a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
+++ b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
@@ -27,3 +27,20 @@ Description: Read-only. Returns a string describing the current
programming. Userspace code can poll on this file,
as it will be signaled by sysfs_notify() on each
state change.
+
+What: /sys/class/fpga_sec_mgr/fpga_secX/update/error
+Date: April 2021
+KernelVersion: 5.13
+Contact: Russ Weight <[email protected]>
+Description: Read-only. Returns a string describing the failure
+ of a secure update. This string will be in the form
+ of <STATUS>:<ERROR>, where <STATUS> will be one of
+ the status strings described for the status sysfs
+ file and <ERROR> will be one of the following:
+ hw-error, timeout, user-abort, device-busy,
+ invalid-file-size, read-write-error, flash-wearout,
+ file-read-error. The error sysfs file is only
+ meaningful when the secure update engine is in the
+ idle state. If this file is read while a secure
+ update is in progress, then the read will fail with
+ EBUSY.
diff --git a/drivers/fpga/fpga-sec-mgr.c b/drivers/fpga/fpga-sec-mgr.c
index dd60014b7511..3bd89598cccd 100644
--- a/drivers/fpga/fpga-sec-mgr.c
+++ b/drivers/fpga/fpga-sec-mgr.c
@@ -32,10 +32,16 @@ static void update_progress(struct fpga_sec_mgr *smgr,
sysfs_notify(&smgr->dev.kobj, "update", "status");
}

+static void set_error(struct fpga_sec_mgr *smgr, enum fpga_sec_err err_code)
+{
+ smgr->err_state = smgr->progress;
+ smgr->err_code = err_code;
+}
+
static void fpga_sec_dev_error(struct fpga_sec_mgr *smgr,
enum fpga_sec_err err_code)
{
- smgr->err_code = err_code;
+ set_error(smgr, err_code);
smgr->sops->cancel(smgr);
}

@@ -58,7 +64,7 @@ static void fpga_sec_mgr_update(struct work_struct *work)

get_device(&smgr->dev);
if (request_firmware(&fw, smgr->filename, &smgr->dev)) {
- smgr->err_code = FPGA_SEC_ERR_FILE_READ;
+ set_error(smgr, FPGA_SEC_ERR_FILE_READ);
goto idle_exit;
}

@@ -66,7 +72,7 @@ static void fpga_sec_mgr_update(struct work_struct *work)
smgr->remaining_size = fw->size;

if (!try_module_get(smgr->dev.parent->driver->owner)) {
- smgr->err_code = FPGA_SEC_ERR_BUSY;
+ set_error(smgr, FPGA_SEC_ERR_BUSY);
goto release_fw_exit;
}

@@ -128,24 +134,76 @@ static const char * const sec_mgr_prog_str[] = {
"programming" /* FPGA_SEC_PROG_PROGRAMMING */
};

-static ssize_t
-status_show(struct device *dev, struct device_attribute *attr, char *buf)
+static const char * const sec_mgr_err_str[] = {
+ "none", /* FPGA_SEC_ERR_NONE */
+ "hw-error", /* FPGA_SEC_ERR_HW_ERROR */
+ "timeout", /* FPGA_SEC_ERR_TIMEOUT */
+ "user-abort", /* FPGA_SEC_ERR_CANCELED */
+ "device-busy", /* FPGA_SEC_ERR_BUSY */
+ "invalid-file-size", /* FPGA_SEC_ERR_INVALID_SIZE */
+ "read-write-error", /* FPGA_SEC_ERR_RW_ERROR */
+ "flash-wearout", /* FPGA_SEC_ERR_WEAROUT */
+ "file-read-error" /* FPGA_SEC_ERR_FILE_READ */
+};
+
+static const char *sec_progress(struct device *dev, enum fpga_sec_prog prog)
{
- struct fpga_sec_mgr *smgr = to_sec_mgr(dev);
const char *status = "unknown-status";
- enum fpga_sec_prog progress;

- progress = smgr->progress;
- if (progress < FPGA_SEC_PROG_MAX)
- status = sec_mgr_prog_str[progress];
+ if (prog < FPGA_SEC_PROG_MAX)
+ status = sec_mgr_prog_str[prog];
else
dev_err(dev, "Invalid status during secure update: %d\n",
- progress);
+ prog);
+
+ return status;
+}
+
+static const char *sec_error(struct device *dev, enum fpga_sec_err err_code)
+{
+ const char *error = "unknown-error";
+
+ if (err_code < FPGA_SEC_ERR_MAX)
+ error = sec_mgr_err_str[err_code];
+ else
+ dev_err(dev, "Invalid error code during secure update: %d\n",
+ err_code);
+
+ return error;
+}
+
+static ssize_t
+status_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct fpga_sec_mgr *smgr = to_sec_mgr(dev);

- return sysfs_emit(buf, "%s\n", status);
+ return sysfs_emit(buf, "%s\n", sec_progress(dev, smgr->progress));
}
static DEVICE_ATTR_RO(status);

+static ssize_t
+error_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct fpga_sec_mgr *smgr = to_sec_mgr(dev);
+ int ret;
+
+ mutex_lock(&smgr->lock);
+
+ if (smgr->progress != FPGA_SEC_PROG_IDLE)
+ ret = -EBUSY;
+ else if (!smgr->err_code)
+ ret = 0;
+ else
+ ret = sysfs_emit(buf, "%s:%s\n",
+ sec_progress(dev, smgr->err_state),
+ sec_error(dev, smgr->err_code));
+
+ mutex_unlock(&smgr->lock);
+
+ return ret;
+}
+static DEVICE_ATTR_RO(error);
+
static ssize_t filename_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
@@ -181,6 +239,7 @@ static DEVICE_ATTR_WO(filename);
static struct attribute *sec_mgr_update_attrs[] = {
&dev_attr_filename.attr,
&dev_attr_status.attr,
+ &dev_attr_error.attr,
NULL,
};

diff --git a/include/linux/fpga/fpga-sec-mgr.h b/include/linux/fpga/fpga-sec-mgr.h
index e03de72134d6..78f8dc51a508 100644
--- a/include/linux/fpga/fpga-sec-mgr.h
+++ b/include/linux/fpga/fpga-sec-mgr.h
@@ -71,6 +71,7 @@ struct fpga_sec_mgr {
const u8 *data; /* pointer to update data */
u32 remaining_size; /* size remaining to transfer */
enum fpga_sec_prog progress;
+ enum fpga_sec_prog err_state; /* progress state at time of failure */
enum fpga_sec_err err_code; /* security manager error code */
bool driver_unload;
void *priv;
--
2.25.1

2021-03-09 00:37:47

by Russ Weight

[permalink] [raw]
Subject: [PATCH v10 2/7] fpga: sec-mgr: enable secure updates

Extend the FPGA Security Manager class driver to
include an update/filename sysfs node that can be used
to initiate a secure update. The filename of a secure
update file (BMC image, FPGA image, Root Entry Hash image,
or Code Signing Key cancellation image) can be written to
this sysfs entry to cause a secure update to occur.

The write of the filename will return immediately, and the
update will begin in the context of a kernel worker thread.
This tool utilizes the request_firmware framework, which
requires that the image file reside under /lib/firmware.

Signed-off-by: Russ Weight <[email protected]>
Reviewed-by: Tom Rix <[email protected]>
---
v10:
- Rebased to 5.12-rc2 next
- Updated Date and KernelVersion in ABI documentation
v9:
- Updated Date and KernelVersion in ABI documentation
v8:
- No change
v7:
- Changed Date in documentation file to December 2020
- Changed filename_store() to use kmemdup_nul() instead of
kstrndup() and changed the count to not assume a line-return.
v6:
- Changed "security update" to "secure update" in commit message
v5:
- When checking the return values for functions of type enum
fpga_sec_err err_code, test for FPGA_SEC_ERR_NONE instead of 0
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:
- Removed unnecessary "goto done"
- Added a comment to explain imgr->driver_unload in
ifpga_sec_mgr_unregister()
v2:
- Bumped documentation date and version
- Removed explicit value assignments in enums
- Other minor code cleanup per review comments
---
.../ABI/testing/sysfs-class-fpga-sec-mgr | 13 ++
drivers/fpga/fpga-sec-mgr.c | 164 ++++++++++++++++++
include/linux/fpga/fpga-sec-mgr.h | 49 ++++++
3 files changed, 226 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
index 0dda4604c178..4c0bdfd25553 100644
--- a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
+++ b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
@@ -3,3 +3,16 @@ Date: April 2021
KernelVersion: 5.13
Contact: Russ Weight <[email protected]>
Description: Name of low level fpga security manager driver.
+
+What: /sys/class/fpga_sec_mgr/fpga_secX/update/filename
+Date: April 2021
+KernelVersion: 5.13
+Contact: Russ Weight <[email protected]>
+Description: Write only. Write the filename of an image
+ file to this sysfs file to initiate a secure
+ update. The file must have an appropriate header
+ which, among other things, identifies the target
+ for the update. This mechanism is used to update
+ BMC images, BMC firmware, Static Region images,
+ and Root Entry Hashes, and to cancel Code Signing
+ Keys (CSK).
diff --git a/drivers/fpga/fpga-sec-mgr.c b/drivers/fpga/fpga-sec-mgr.c
index 468379e0c825..7347ba1ba73e 100644
--- a/drivers/fpga/fpga-sec-mgr.c
+++ b/drivers/fpga/fpga-sec-mgr.c
@@ -5,8 +5,11 @@
* Copyright (C) 2019-2020 Intel Corporation, Inc.
*/

+#include <linux/delay.h>
+#include <linux/firmware.h>
#include <linux/fpga/fpga-sec-mgr.h>
#include <linux/idr.h>
+#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
@@ -18,8 +21,140 @@ struct fpga_sec_mgr_devres {
struct fpga_sec_mgr *smgr;
};

+#define WRITE_BLOCK_SIZE 0x4000 /* Update remaining_size every 0x4000 bytes */
+
#define to_sec_mgr(d) container_of(d, struct fpga_sec_mgr, dev)

+static void fpga_sec_dev_error(struct fpga_sec_mgr *smgr,
+ enum fpga_sec_err err_code)
+{
+ smgr->err_code = err_code;
+ smgr->sops->cancel(smgr);
+}
+
+static void progress_complete(struct fpga_sec_mgr *smgr)
+{
+ mutex_lock(&smgr->lock);
+ smgr->progress = FPGA_SEC_PROG_IDLE;
+ complete_all(&smgr->update_done);
+ mutex_unlock(&smgr->lock);
+}
+
+static void fpga_sec_mgr_update(struct work_struct *work)
+{
+ u32 size, blk_size, offset = 0;
+ struct fpga_sec_mgr *smgr;
+ const struct firmware *fw;
+ enum fpga_sec_err ret;
+
+ smgr = container_of(work, struct fpga_sec_mgr, work);
+
+ get_device(&smgr->dev);
+ if (request_firmware(&fw, smgr->filename, &smgr->dev)) {
+ smgr->err_code = FPGA_SEC_ERR_FILE_READ;
+ goto idle_exit;
+ }
+
+ smgr->data = fw->data;
+ smgr->remaining_size = fw->size;
+
+ if (!try_module_get(smgr->dev.parent->driver->owner)) {
+ smgr->err_code = FPGA_SEC_ERR_BUSY;
+ goto release_fw_exit;
+ }
+
+ smgr->progress = FPGA_SEC_PROG_PREPARING;
+ ret = smgr->sops->prepare(smgr);
+ if (ret != FPGA_SEC_ERR_NONE) {
+ fpga_sec_dev_error(smgr, ret);
+ goto modput_exit;
+ }
+
+ smgr->progress = FPGA_SEC_PROG_WRITING;
+ size = smgr->remaining_size;
+ while (size) {
+ blk_size = min_t(u32, size, WRITE_BLOCK_SIZE);
+ size -= blk_size;
+ ret = smgr->sops->write_blk(smgr, offset, blk_size);
+ if (ret != FPGA_SEC_ERR_NONE) {
+ fpga_sec_dev_error(smgr, ret);
+ goto done;
+ }
+
+ smgr->remaining_size = size;
+ offset += blk_size;
+ }
+
+ smgr->progress = FPGA_SEC_PROG_PROGRAMMING;
+ ret = smgr->sops->poll_complete(smgr);
+ if (ret != FPGA_SEC_ERR_NONE)
+ fpga_sec_dev_error(smgr, ret);
+
+done:
+ if (smgr->sops->cleanup)
+ smgr->sops->cleanup(smgr);
+
+modput_exit:
+ module_put(smgr->dev.parent->driver->owner);
+
+release_fw_exit:
+ smgr->data = NULL;
+ release_firmware(fw);
+
+idle_exit:
+ /*
+ * Note: smgr->remaining_size is left unmodified here to
+ * provide additional information on errors. It will be
+ * reinitialized when the next secure update begins.
+ */
+ kfree(smgr->filename);
+ smgr->filename = NULL;
+ put_device(&smgr->dev);
+ progress_complete(smgr);
+}
+
+static ssize_t filename_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fpga_sec_mgr *smgr = to_sec_mgr(dev);
+ int ret = count;
+
+ if (count == 0 || count >= PATH_MAX)
+ return -EINVAL;
+
+ mutex_lock(&smgr->lock);
+ if (smgr->driver_unload || smgr->progress != FPGA_SEC_PROG_IDLE) {
+ ret = -EBUSY;
+ goto unlock_exit;
+ }
+
+ smgr->filename = kmemdup_nul(buf, count, GFP_KERNEL);
+ if (!smgr->filename) {
+ ret = -ENOMEM;
+ goto unlock_exit;
+ }
+
+ smgr->err_code = FPGA_SEC_ERR_NONE;
+ smgr->progress = FPGA_SEC_PROG_READING;
+ reinit_completion(&smgr->update_done);
+ schedule_work(&smgr->work);
+
+unlock_exit:
+ mutex_unlock(&smgr->lock);
+ return ret;
+}
+static DEVICE_ATTR_WO(filename);
+
+static struct attribute *sec_mgr_update_attrs[] = {
+ &dev_attr_filename.attr,
+ NULL,
+};
+
+static struct attribute_group sec_mgr_update_attr_group = {
+ .name = "update",
+ .attrs = sec_mgr_update_attrs,
+};
+
static ssize_t name_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -40,6 +175,7 @@ static struct attribute_group sec_mgr_attr_group = {

static const struct attribute_group *fpga_sec_mgr_attr_groups[] = {
&sec_mgr_attr_group,
+ &sec_mgr_update_attr_group,
NULL,
};

@@ -65,6 +201,12 @@ fpga_sec_mgr_create(struct device *dev, const char *name,
struct fpga_sec_mgr *smgr;
int id, ret;

+ if (!sops || !sops->cancel || !sops->prepare ||
+ !sops->write_blk || !sops->poll_complete) {
+ dev_err(dev, "Attempt to register without required ops\n");
+ return NULL;
+ }
+
if (!name || !strlen(name)) {
dev_err(dev, "Attempt to register with no name!\n");
return NULL;
@@ -83,6 +225,8 @@ fpga_sec_mgr_create(struct device *dev, const char *name,
smgr->name = name;
smgr->priv = priv;
smgr->sops = sops;
+ init_completion(&smgr->update_done);
+ INIT_WORK(&smgr->work, fpga_sec_mgr_update);

device_initialize(&smgr->dev);
smgr->dev.class = fpga_sec_mgr_class;
@@ -200,11 +344,31 @@ EXPORT_SYMBOL_GPL(fpga_sec_mgr_register);
*
* This function is intended for use in an FPGA security manager
* driver's remove() function.
+ *
+ * For some devices, once the secure update has begun authentication
+ * the hardware cannot be signaled to stop, and the driver will not
+ * exit until the hardware signals completion. This could be 30+
+ * minutes of waiting. The driver_unload flag enableds a force-unload
+ * of the driver (e.g. modprobe -r) by signaling the parent driver to
+ * exit even if the hardware update is incomplete. The driver_unload
+ * flag also prevents new updates from starting once the unregister
+ * process has begun.
*/
void fpga_sec_mgr_unregister(struct fpga_sec_mgr *smgr)
{
dev_info(&smgr->dev, "%s %s\n", __func__, smgr->name);

+ mutex_lock(&smgr->lock);
+ smgr->driver_unload = true;
+ if (smgr->progress == FPGA_SEC_PROG_IDLE) {
+ mutex_unlock(&smgr->lock);
+ goto unregister;
+ }
+
+ mutex_unlock(&smgr->lock);
+ wait_for_completion(&smgr->update_done);
+
+unregister:
device_unregister(&smgr->dev);
}
EXPORT_SYMBOL_GPL(fpga_sec_mgr_unregister);
diff --git a/include/linux/fpga/fpga-sec-mgr.h b/include/linux/fpga/fpga-sec-mgr.h
index f85665b79b9d..e03de72134d6 100644
--- a/include/linux/fpga/fpga-sec-mgr.h
+++ b/include/linux/fpga/fpga-sec-mgr.h
@@ -7,16 +7,57 @@
#ifndef _LINUX_FPGA_SEC_MGR_H
#define _LINUX_FPGA_SEC_MGR_H

+#include <linux/completion.h>
#include <linux/device.h>
#include <linux/mutex.h>
#include <linux/types.h>

struct fpga_sec_mgr;

+enum fpga_sec_err {
+ FPGA_SEC_ERR_NONE,
+ FPGA_SEC_ERR_HW_ERROR,
+ FPGA_SEC_ERR_TIMEOUT,
+ FPGA_SEC_ERR_CANCELED,
+ FPGA_SEC_ERR_BUSY,
+ FPGA_SEC_ERR_INVALID_SIZE,
+ FPGA_SEC_ERR_RW_ERROR,
+ FPGA_SEC_ERR_WEAROUT,
+ FPGA_SEC_ERR_FILE_READ,
+ FPGA_SEC_ERR_MAX
+};
+
/**
* struct fpga_sec_mgr_ops - device specific operations
+ * @prepare: Required: Prepare secure update
+ * @write_blk: Required: Write a block of data
+ * @poll_complete: Required: Check for the completion of the
+ * HW authentication/programming process. This
+ * function should check for smgr->driver_unload
+ * and abort with FPGA_SEC_ERR_CANCELED when true.
+ * @cancel: Required: Signal HW to cancel update
+ * @cleanup: Optional: Complements the prepare()
+ * function and is called at the completion
+ * of the update, whether success or failure,
+ * if the prepare function succeeded.
*/
struct fpga_sec_mgr_ops {
+ enum fpga_sec_err (*prepare)(struct fpga_sec_mgr *smgr);
+ enum fpga_sec_err (*write_blk)(struct fpga_sec_mgr *smgr,
+ u32 offset, u32 size);
+ enum fpga_sec_err (*poll_complete)(struct fpga_sec_mgr *smgr);
+ enum fpga_sec_err (*cancel)(struct fpga_sec_mgr *smgr);
+ void (*cleanup)(struct fpga_sec_mgr *smgr);
+};
+
+/* Update progress codes */
+enum fpga_sec_prog {
+ FPGA_SEC_PROG_IDLE,
+ FPGA_SEC_PROG_READING,
+ FPGA_SEC_PROG_PREPARING,
+ FPGA_SEC_PROG_WRITING,
+ FPGA_SEC_PROG_PROGRAMMING,
+ FPGA_SEC_PROG_MAX
};

struct fpga_sec_mgr {
@@ -24,6 +65,14 @@ struct fpga_sec_mgr {
struct device dev;
const struct fpga_sec_mgr_ops *sops;
struct mutex lock; /* protect data structure contents */
+ struct work_struct work;
+ struct completion update_done;
+ char *filename;
+ const u8 *data; /* pointer to update data */
+ u32 remaining_size; /* size remaining to transfer */
+ enum fpga_sec_prog progress;
+ enum fpga_sec_err err_code; /* security manager error code */
+ bool driver_unload;
void *priv;
};

--
2.25.1

2021-03-09 00:37:53

by Russ Weight

[permalink] [raw]
Subject: [PATCH v10 7/7] fpga: sec-mgr: expose hardware error info

Extend the FPGA Security Manager class driver to include
an optional update/hw_errinfo sysfs node that can be used
to retrieve 64 bits of device specific error information
following a secure update failure.

The underlying driver must provide a get_hw_errinfo() callback
function to enable this feature. This data is treated as
opaque by the class driver. It is left to user-space software
or support personnel to interpret this data.

Signed-off-by: Russ Weight <[email protected]>
Reviewed-by: Tom Rix <[email protected]>
---
v10:
- Rebased to 5.12-rc2 next
- Updated Date and KernelVersion in ABI documentation
v9:
- Updated Date and KernelVersion in ABI documentation
v8:
- No change
v7:
- Changed Date in documentation file to December 2020
v6:
- No change
v5:
- No change
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:
- No change
v2:
- Bumped documentation date and version
---
.../ABI/testing/sysfs-class-fpga-sec-mgr | 14 +++++++
drivers/fpga/fpga-sec-mgr.c | 38 +++++++++++++++++++
include/linux/fpga/fpga-sec-mgr.h | 5 +++
3 files changed, 57 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
index fba3066e2b69..ff1458701def 100644
--- a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
+++ b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
@@ -65,3 +65,17 @@ Description: Read-only. Returns a string describing the failure
idle state. If this file is read while a secure
update is in progress, then the read will fail with
EBUSY.
+
+What: /sys/class/fpga_sec_mgr/fpga_secX/update/hw_errinfo
+Date: April 2021
+KernelVersion: 5.13
+Contact: Russ Weight <[email protected]>
+Description: Read-only. Returns a 64 bit error value providing
+ hardware specific information that may be useful in
+ debugging errors that occur during FPGA image updates.
+ This file is only visible if the underlying device
+ supports it. The hw_errinfo value is only accessible
+ when the secure update engine is in the idle state.
+ If this file is read while a secure update is in
+ progress, then the read will fail with EBUSY.
+ Format: "0x%llx".
diff --git a/drivers/fpga/fpga-sec-mgr.c b/drivers/fpga/fpga-sec-mgr.c
index d354fe2ab582..aebe06d9a54a 100644
--- a/drivers/fpga/fpga-sec-mgr.c
+++ b/drivers/fpga/fpga-sec-mgr.c
@@ -38,10 +38,17 @@ static void set_error(struct fpga_sec_mgr *smgr, enum fpga_sec_err err_code)
smgr->err_code = err_code;
}

+static void set_hw_errinfo(struct fpga_sec_mgr *smgr)
+{
+ if (smgr->sops->get_hw_errinfo)
+ smgr->hw_errinfo = smgr->sops->get_hw_errinfo(smgr);
+}
+
static void fpga_sec_dev_error(struct fpga_sec_mgr *smgr,
enum fpga_sec_err err_code)
{
set_error(smgr, err_code);
+ set_hw_errinfo(smgr);
smgr->sops->cancel(smgr);
}

@@ -227,6 +234,23 @@ error_show(struct device *dev, struct device_attribute *attr, char *buf)
}
static DEVICE_ATTR_RO(error);

+static ssize_t
+hw_errinfo_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct fpga_sec_mgr *smgr = to_sec_mgr(dev);
+ int ret;
+
+ mutex_lock(&smgr->lock);
+ if (smgr->progress != FPGA_SEC_PROG_IDLE)
+ ret = -EBUSY;
+ else
+ ret = sysfs_emit(buf, "0x%llx\n", smgr->hw_errinfo);
+ mutex_unlock(&smgr->lock);
+
+ return ret;
+}
+static DEVICE_ATTR_RO(hw_errinfo);
+
static ssize_t remaining_size_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -258,6 +282,7 @@ static ssize_t filename_store(struct device *dev, struct device_attribute *attr,
}

smgr->err_code = FPGA_SEC_ERR_NONE;
+ smgr->hw_errinfo = 0;
smgr->request_cancel = false;
smgr->progress = FPGA_SEC_PROG_READING;
reinit_completion(&smgr->update_done);
@@ -292,18 +317,31 @@ static ssize_t cancel_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(cancel);

+static umode_t
+sec_mgr_update_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+ struct fpga_sec_mgr *smgr = to_sec_mgr(kobj_to_dev(kobj));
+
+ if (attr == &dev_attr_hw_errinfo.attr && !smgr->sops->get_hw_errinfo)
+ return 0;
+
+ return attr->mode;
+}
+
static struct attribute *sec_mgr_update_attrs[] = {
&dev_attr_filename.attr,
&dev_attr_cancel.attr,
&dev_attr_status.attr,
&dev_attr_error.attr,
&dev_attr_remaining_size.attr,
+ &dev_attr_hw_errinfo.attr,
NULL,
};

static struct attribute_group sec_mgr_update_attr_group = {
.name = "update",
.attrs = sec_mgr_update_attrs,
+ .is_visible = sec_mgr_update_visible,
};

static ssize_t name_show(struct device *dev,
diff --git a/include/linux/fpga/fpga-sec-mgr.h b/include/linux/fpga/fpga-sec-mgr.h
index e63e6437f394..04235fab8667 100644
--- a/include/linux/fpga/fpga-sec-mgr.h
+++ b/include/linux/fpga/fpga-sec-mgr.h
@@ -40,6 +40,9 @@ enum fpga_sec_err {
* function and is called at the completion
* of the update, whether success or failure,
* if the prepare function succeeded.
+ * @get_hw_errinfo: Optional: Return u64 hw specific error info.
+ * The software err_code may used to determine
+ * whether the hw error info is applicable.
*/
struct fpga_sec_mgr_ops {
enum fpga_sec_err (*prepare)(struct fpga_sec_mgr *smgr);
@@ -48,6 +51,7 @@ struct fpga_sec_mgr_ops {
enum fpga_sec_err (*poll_complete)(struct fpga_sec_mgr *smgr);
enum fpga_sec_err (*cancel)(struct fpga_sec_mgr *smgr);
void (*cleanup)(struct fpga_sec_mgr *smgr);
+ u64 (*get_hw_errinfo)(struct fpga_sec_mgr *smgr);
};

/* Update progress codes */
@@ -73,6 +77,7 @@ struct fpga_sec_mgr {
enum fpga_sec_prog progress;
enum fpga_sec_prog err_state; /* progress state at time of failure */
enum fpga_sec_err err_code; /* security manager error code */
+ u64 hw_errinfo; /* 64 bits of HW specific error info */
bool request_cancel;
bool driver_unload;
void *priv;
--
2.25.1

2021-03-09 00:37:58

by Russ Weight

[permalink] [raw]
Subject: [PATCH v10 5/7] fpga: sec-mgr: expose sec-mgr update size

Extend the FPGA Security Manager class driver to include
an update/remaining_size sysfs node that can be read to
determine how much data remains to be transferred to the
secure update engine. This file can be used to monitor
progress during the "writing" phase of an update.

Signed-off-by: Russ Weight <[email protected]>
Reviewed-by: Tom Rix <[email protected]>
---
v10:
- Rebased to 5.12-rc2 next
- Updated Date and KernelVersion in ABI documentation
v9:
- Updated Date and KernelVersion in ABI documentation
v8:
- No change
v7:
- Changed Date in documentation file to December 2020
v6:
- No change
v5:
- Use new function sysfs_emit() in the remaining_size_show() function
v4:
- Changed from "Intel FPGA Security Manager" to FPGA Security Manager"
and removed unnecessary references to "Intel".
- Changed: imgr -> smgr, ifpga_ to fpga_
v3:
- No change
v2:
- Bumped documentation date and version
---
Documentation/ABI/testing/sysfs-class-fpga-sec-mgr | 11 +++++++++++
drivers/fpga/fpga-sec-mgr.c | 10 ++++++++++
2 files changed, 21 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
index 77738577b2c3..2e1f60edf546 100644
--- a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
+++ b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
@@ -28,6 +28,17 @@ Description: Read-only. Returns a string describing the current
as it will be signaled by sysfs_notify() on each
state change.

+What: /sys/class/fpga_sec_mgr/fpga_secX/update/remaining_size
+Date: April 2021
+KernelVersion: 5.13
+Contact: Russ Weight <[email protected]>
+Description: Read-only. Returns the size of data that remains to
+ be written to the secure update engine. The size
+ value is initialized to the full size of the file
+ image and the value is updated periodically during
+ the "writing" phase of the update.
+ Format: "%u".
+
What: /sys/class/fpga_sec_mgr/fpga_secX/update/error
Date: April 2021
KernelVersion: 5.13
diff --git a/drivers/fpga/fpga-sec-mgr.c b/drivers/fpga/fpga-sec-mgr.c
index 3bd89598cccd..951020942991 100644
--- a/drivers/fpga/fpga-sec-mgr.c
+++ b/drivers/fpga/fpga-sec-mgr.c
@@ -204,6 +204,15 @@ error_show(struct device *dev, struct device_attribute *attr, char *buf)
}
static DEVICE_ATTR_RO(error);

+static ssize_t remaining_size_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fpga_sec_mgr *smgr = to_sec_mgr(dev);
+
+ return sysfs_emit(buf, "%u\n", smgr->remaining_size);
+}
+static DEVICE_ATTR_RO(remaining_size);
+
static ssize_t filename_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
@@ -240,6 +249,7 @@ static struct attribute *sec_mgr_update_attrs[] = {
&dev_attr_filename.attr,
&dev_attr_status.attr,
&dev_attr_error.attr,
+ &dev_attr_remaining_size.attr,
NULL,
};

--
2.25.1

2021-03-09 00:39:47

by Russ Weight

[permalink] [raw]
Subject: [PATCH v10 6/7] fpga: sec-mgr: enable cancel of secure update

Extend the FPGA Security Manager class driver to include
an update/cancel sysfs file that can be written to request
that an update be canceled. The write may return EBUSY if
the update has progressed to the point that it cannot be
canceled by software or ENODEV if there is no update in
progress.

Signed-off-by: Russ Weight <[email protected]>
Reviewed-by: Tom Rix <[email protected]>
---
v10:
- Rebased to 5.12-rc2 next
- Updated Date and KernelVersion in ABI documentation
v9:
- Updated Date and KernelVersion in ABI documentation
v8:
- No change
v7:
- Changed Date in documentation file to December 2020
v6:
- No change
v5:
- No change
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:
- No change
v2:
- Bumped documentation date and version
- Minor code cleanup per review comments
---
.../ABI/testing/sysfs-class-fpga-sec-mgr | 10 ++++
drivers/fpga/fpga-sec-mgr.c | 59 +++++++++++++++++--
include/linux/fpga/fpga-sec-mgr.h | 1 +
3 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
index 2e1f60edf546..fba3066e2b69 100644
--- a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
+++ b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
@@ -17,6 +17,16 @@ Description: Write only. Write the filename of an image
and Root Entry Hashes, and to cancel Code Signing
Keys (CSK).

+What: /sys/class/fpga_sec_mgr/fpga_secX/update/cancel
+Date: April 2021
+KernelVersion: 5.13
+Contact: Russ Weight <[email protected]>
+Description: Write-only. Write a "1" to this file to request
+ that a current update be canceled. This request
+ will be rejected (EBUSY) if the programming phase
+ has already started or (ENODEV) if there is no
+ update in progress.
+
What: /sys/class/fpga_sec_mgr/fpga_secX/update/status
Date: April 2021
KernelVersion: 5.13
diff --git a/drivers/fpga/fpga-sec-mgr.c b/drivers/fpga/fpga-sec-mgr.c
index 951020942991..d354fe2ab582 100644
--- a/drivers/fpga/fpga-sec-mgr.c
+++ b/drivers/fpga/fpga-sec-mgr.c
@@ -45,6 +45,23 @@ static void fpga_sec_dev_error(struct fpga_sec_mgr *smgr,
smgr->sops->cancel(smgr);
}

+static int progress_transition(struct fpga_sec_mgr *smgr,
+ enum fpga_sec_prog new_progress)
+{
+ int ret = 0;
+
+ mutex_lock(&smgr->lock);
+ if (smgr->request_cancel) {
+ set_error(smgr, FPGA_SEC_ERR_CANCELED);
+ smgr->sops->cancel(smgr);
+ ret = -ECANCELED;
+ } else {
+ update_progress(smgr, new_progress);
+ }
+ mutex_unlock(&smgr->lock);
+ return ret;
+}
+
static void progress_complete(struct fpga_sec_mgr *smgr)
{
mutex_lock(&smgr->lock);
@@ -76,16 +93,20 @@ static void fpga_sec_mgr_update(struct work_struct *work)
goto release_fw_exit;
}

- update_progress(smgr, FPGA_SEC_PROG_PREPARING);
+ if (progress_transition(smgr, FPGA_SEC_PROG_PREPARING))
+ goto modput_exit;
+
ret = smgr->sops->prepare(smgr);
if (ret != FPGA_SEC_ERR_NONE) {
fpga_sec_dev_error(smgr, ret);
goto modput_exit;
}

- update_progress(smgr, FPGA_SEC_PROG_WRITING);
+ if (progress_transition(smgr, FPGA_SEC_PROG_WRITING))
+ goto done;
+
size = smgr->remaining_size;
- while (size) {
+ while (size && !smgr->request_cancel) {
blk_size = min_t(u32, size, WRITE_BLOCK_SIZE);
size -= blk_size;
ret = smgr->sops->write_blk(smgr, offset, blk_size);
@@ -98,7 +119,9 @@ static void fpga_sec_mgr_update(struct work_struct *work)
offset += blk_size;
}

- update_progress(smgr, FPGA_SEC_PROG_PROGRAMMING);
+ if (progress_transition(smgr, FPGA_SEC_PROG_PROGRAMMING))
+ goto done;
+
ret = smgr->sops->poll_complete(smgr);
if (ret != FPGA_SEC_ERR_NONE)
fpga_sec_dev_error(smgr, ret);
@@ -235,6 +258,7 @@ static ssize_t filename_store(struct device *dev, struct device_attribute *attr,
}

smgr->err_code = FPGA_SEC_ERR_NONE;
+ smgr->request_cancel = false;
smgr->progress = FPGA_SEC_PROG_READING;
reinit_completion(&smgr->update_done);
schedule_work(&smgr->work);
@@ -245,8 +269,32 @@ static ssize_t filename_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(filename);

+static ssize_t cancel_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fpga_sec_mgr *smgr = to_sec_mgr(dev);
+ bool cancel;
+ int ret = count;
+
+ if (kstrtobool(buf, &cancel) || !cancel)
+ return -EINVAL;
+
+ mutex_lock(&smgr->lock);
+ if (smgr->progress == FPGA_SEC_PROG_PROGRAMMING)
+ ret = -EBUSY;
+ else if (smgr->progress == FPGA_SEC_PROG_IDLE)
+ ret = -ENODEV;
+ else
+ smgr->request_cancel = true;
+ mutex_unlock(&smgr->lock);
+
+ return ret;
+}
+static DEVICE_ATTR_WO(cancel);
+
static struct attribute *sec_mgr_update_attrs[] = {
&dev_attr_filename.attr,
+ &dev_attr_cancel.attr,
&dev_attr_status.attr,
&dev_attr_error.attr,
&dev_attr_remaining_size.attr,
@@ -468,6 +516,9 @@ void fpga_sec_mgr_unregister(struct fpga_sec_mgr *smgr)
goto unregister;
}

+ if (smgr->progress != FPGA_SEC_PROG_PROGRAMMING)
+ smgr->request_cancel = true;
+
mutex_unlock(&smgr->lock);
wait_for_completion(&smgr->update_done);

diff --git a/include/linux/fpga/fpga-sec-mgr.h b/include/linux/fpga/fpga-sec-mgr.h
index 78f8dc51a508..e63e6437f394 100644
--- a/include/linux/fpga/fpga-sec-mgr.h
+++ b/include/linux/fpga/fpga-sec-mgr.h
@@ -73,6 +73,7 @@ struct fpga_sec_mgr {
enum fpga_sec_prog progress;
enum fpga_sec_prog err_state; /* progress state at time of failure */
enum fpga_sec_err err_code; /* security manager error code */
+ bool request_cancel;
bool driver_unload;
void *priv;
};
--
2.25.1

2021-03-09 16:06:56

by Tom Rix

[permalink] [raw]
Subject: Re: [PATCH v10 0/7] FPGA Security Manager Class Driver

Moritz,

This and the next patchset apply to today's char-misc-next.

However they conflicts with other in flight linux-fpga patchsets.

Since I believe these patchsets came first, I think they should have preference.

This feature of updating is needed for the basic operation of the fpga.

Tom

On 3/8/21 4:35 PM, Russ Weight wrote:
> 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 FPGA 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.
>
> The n3000bmc-secure driver is the first driver to use the FPGA
> Security Manager. This driver was previously submitted in the same
> patch set, but has been split out into a separate patch set starting
> with V2. Future devices will also make use of this common API for
> secure updates.
>
> In addition to managing secure updates of the FPGA and BMC images,
> the FPGA Security Manager update process may also be used to
> program root entry hashes and cancellation keys for the FPGA static
> region, the FPGA partial reconfiguration region, and the BMC.
> The image files are self-describing, and contain a header describing
> the image type.
>
> Secure updates make use of the request_firmware framework, which
> requires that image files are accessible under /lib/firmware. A request
> for a secure update returns immediately, while the update itself
> proceeds in the context of a kernel worker thread. Sysfs files provide
> a means for monitoring the progress of a secure update and for
> retrieving error information in the event of a failure.
>
> The API includes a "name" sysfs file to export the name of the parent
> driver. It also includes an "update" sub-directory containing files that
> that can be used to instantiate and monitor a secure update.
>
> Changelog v9 -> v10:
> - Rebased to 5.12-rc2 next
> - Updated Date and KernelVersion in ABI documentation
>
> Changelog v8 -> v9:
> - Rebased patches for 5.11-rc2
> - Updated Date and KernelVersion in ABI documentation
>
> Changelog v7 -> v8:
> - Fixed grammatical error in Documentation/fpga/fpga-sec-mgr.rst
>
> Changelog v6 -> v7:
> - Changed dates in documentation file to December 2020
> - Changed filename_store() to use kmemdup_nul() instead of
> kstrndup() and changed the count to not assume a line-return.
>
> Changelog v5 -> v6:
> - Removed sysfs support and documentation for the display of the
> flash count, root entry hashes, and code-signing-key cancelation
> vectors from the class driver. This information can vary by device
> and will instead be displayed by the device-specific parent driver.
>
> Changelog v4 -> v5:
> - Added the devm_fpga_sec_mgr_unregister() function, following recent
> changes to the fpga_manager() implementation.
> - Changed most of the *_show() functions to use sysfs_emit()
> instead of sprintf(
> - When checking the return values for functions of type enum
> fpga_sec_err err_code, test for FPGA_SEC_ERR_NONE instead of 0
>
> Changelog v3 -> v4:
> - This driver is generic enough that it could be used for non Intel
> FPGA devices. 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_
> Note that this also affects some filenames.
>
> Changelog v2 -> v3:
> - Use dev_err() to report invalid progress in sec_progress()
> - Use dev_err() to report invalid error code in sec_error()
> - Modified sysfs handler check in check_sysfs_handler() to make
> it more readable.
> - Removed unnecessary "goto done"
> - Added a comment to explain imgr->driver_unload in
> ifpga_sec_mgr_unregister()
>
> Changelog v1 -> v2:
> - Separated out the MAX10 BMC Security Engine to be submitted in
> a separate patch-set.
> - Bumped documentation dates and versions
> - Split ifpga_sec_mgr_register() into create() and register() functions
> - Added devm_ifpga_sec_mgr_create()
> - Added Documentation/fpga/ifpga-sec-mgr.rst
> - Changed progress state "read_file" to "reading"
> - Added sec_error() function (similar to sec_progress())
> - Removed references to bmc_flash_count & smbus_flash_count (not supported)
> - Removed typedefs for imgr ops
> - Removed explicit value assignments in enums
> - Other minor code cleanup per review comments
>
> Russ Weight (7):
> fpga: sec-mgr: fpga security manager class driver
> fpga: sec-mgr: enable secure updates
> fpga: sec-mgr: expose sec-mgr update status
> fpga: sec-mgr: expose sec-mgr update errors
> fpga: sec-mgr: expose sec-mgr update size
> fpga: sec-mgr: enable cancel of secure update
> fpga: sec-mgr: expose hardware error info
>
> .../ABI/testing/sysfs-class-fpga-sec-mgr | 81 +++
> 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 | 652 ++++++++++++++++++
> include/linux/fpga/fpga-sec-mgr.h | 100 +++
> 8 files changed, 899 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
>

2021-03-10 02:52:57

by Moritz Fischer

[permalink] [raw]
Subject: Re: [PATCH v10 0/7] FPGA Security Manager Class Driver

Hi Tom,
On Tue, Mar 09, 2021 at 08:03:09AM -0800, Tom Rix wrote:
> Moritz,
>
> This and the next patchset apply to today's char-misc-next.
>
> However they conflicts with other in flight linux-fpga patchsets.
>
> Since I believe these patchsets came first, I think they should have preference.

I'm not sure what the ask here is, do you expect me to back out the
applied patches?

Conflicts will happen, it's part of working with git.

> This feature of updating is needed for the basic operation of the fpga.
>
> Tom
>
> On 3/8/21 4:35 PM, Russ Weight wrote:
> > 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 FPGA 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.
> >
> > The n3000bmc-secure driver is the first driver to use the FPGA
> > Security Manager. This driver was previously submitted in the same
> > patch set, but has been split out into a separate patch set starting
> > with V2. Future devices will also make use of this common API for
> > secure updates.
> >
> > In addition to managing secure updates of the FPGA and BMC images,
> > the FPGA Security Manager update process may also be used to
> > program root entry hashes and cancellation keys for the FPGA static
> > region, the FPGA partial reconfiguration region, and the BMC.
> > The image files are self-describing, and contain a header describing
> > the image type.
> >
> > Secure updates make use of the request_firmware framework, which
> > requires that image files are accessible under /lib/firmware. A request
> > for a secure update returns immediately, while the update itself
> > proceeds in the context of a kernel worker thread. Sysfs files provide
> > a means for monitoring the progress of a secure update and for
> > retrieving error information in the event of a failure.
> >
> > The API includes a "name" sysfs file to export the name of the parent
> > driver. It also includes an "update" sub-directory containing files that
> > that can be used to instantiate and monitor a secure update.
> >
> > Changelog v9 -> v10:
> > - Rebased to 5.12-rc2 next
> > - Updated Date and KernelVersion in ABI documentation
> >
> > Changelog v8 -> v9:
> > - Rebased patches for 5.11-rc2
> > - Updated Date and KernelVersion in ABI documentation
> >
> > Changelog v7 -> v8:
> > - Fixed grammatical error in Documentation/fpga/fpga-sec-mgr.rst
> >
> > Changelog v6 -> v7:
> > - Changed dates in documentation file to December 2020
> > - Changed filename_store() to use kmemdup_nul() instead of
> > kstrndup() and changed the count to not assume a line-return.
> >
> > Changelog v5 -> v6:
> > - Removed sysfs support and documentation for the display of the
> > flash count, root entry hashes, and code-signing-key cancelation
> > vectors from the class driver. This information can vary by device
> > and will instead be displayed by the device-specific parent driver.
> >
> > Changelog v4 -> v5:
> > - Added the devm_fpga_sec_mgr_unregister() function, following recent
> > changes to the fpga_manager() implementation.
> > - Changed most of the *_show() functions to use sysfs_emit()
> > instead of sprintf(
> > - When checking the return values for functions of type enum
> > fpga_sec_err err_code, test for FPGA_SEC_ERR_NONE instead of 0
> >
> > Changelog v3 -> v4:
> > - This driver is generic enough that it could be used for non Intel
> > FPGA devices. 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_
> > Note that this also affects some filenames.
> >
> > Changelog v2 -> v3:
> > - Use dev_err() to report invalid progress in sec_progress()
> > - Use dev_err() to report invalid error code in sec_error()
> > - Modified sysfs handler check in check_sysfs_handler() to make
> > it more readable.
> > - Removed unnecessary "goto done"
> > - Added a comment to explain imgr->driver_unload in
> > ifpga_sec_mgr_unregister()
> >
> > Changelog v1 -> v2:
> > - Separated out the MAX10 BMC Security Engine to be submitted in
> > a separate patch-set.
> > - Bumped documentation dates and versions
> > - Split ifpga_sec_mgr_register() into create() and register() functions
> > - Added devm_ifpga_sec_mgr_create()
> > - Added Documentation/fpga/ifpga-sec-mgr.rst
> > - Changed progress state "read_file" to "reading"
> > - Added sec_error() function (similar to sec_progress())
> > - Removed references to bmc_flash_count & smbus_flash_count (not supported)
> > - Removed typedefs for imgr ops
> > - Removed explicit value assignments in enums
> > - Other minor code cleanup per review comments
> >
> > Russ Weight (7):
> > fpga: sec-mgr: fpga security manager class driver
> > fpga: sec-mgr: enable secure updates
> > fpga: sec-mgr: expose sec-mgr update status
> > fpga: sec-mgr: expose sec-mgr update errors
> > fpga: sec-mgr: expose sec-mgr update size
> > fpga: sec-mgr: enable cancel of secure update
> > fpga: sec-mgr: expose hardware error info
> >
> > .../ABI/testing/sysfs-class-fpga-sec-mgr | 81 +++
> > 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 | 652 ++++++++++++++++++
> > include/linux/fpga/fpga-sec-mgr.h | 100 +++
> > 8 files changed, 899 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
> >
>

2021-03-10 15:44:56

by Tom Rix

[permalink] [raw]
Subject: Re: [PATCH v10 0/7] FPGA Security Manager Class Driver


On 3/9/21 6:51 PM, Moritz Fischer wrote:
> Hi Tom,
> On Tue, Mar 09, 2021 at 08:03:09AM -0800, Tom Rix wrote:
>> Moritz,
>>
>> This and the next patchset apply to today's char-misc-next.
>>
>> However they conflicts with other in flight linux-fpga patchsets.
>>
>> Since I believe these patchsets came first, I think they should have preference.
> I'm not sure what the ask here is, do you expect me to back out the
> applied patches?
>
> Conflicts will happen, it's part of working with git.

When I say in-flight, I mean yet to be accepted patches.

I track these here

https://github.com/trixirt/linux-fpga/tree/fpga-testing


I am sure everyone wants their patch in 5.13, I am plugging for these because they are oldest and most useful.

This is an old patchset, from the changelog in this patchset it has been rebased for the last 2 releases without change.

AFAIK, the dfl fpga's bitstream can not be updated with the mainline kernel without these patches.

Since updating is why you would use an fpga, this is an important feature.


While I am plugging for 5.13

A fix for dfl port enable logic

https://lore.kernel.org/linux-fpga/[email protected]/

A patchset with a fix for stable

https://lore.kernel.org/linux-fpga/[email protected]/

Tom

>> This feature of updating is needed for the basic operation of the fpga.
>>
>> Tom
>>
>> On 3/8/21 4:35 PM, Russ Weight wrote:
>>> 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 FPGA 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.
>>>
>>> The n3000bmc-secure driver is the first driver to use the FPGA
>>> Security Manager. This driver was previously submitted in the same
>>> patch set, but has been split out into a separate patch set starting
>>> with V2. Future devices will also make use of this common API for
>>> secure updates.
>>>
>>> In addition to managing secure updates of the FPGA and BMC images,
>>> the FPGA Security Manager update process may also be used to
>>> program root entry hashes and cancellation keys for the FPGA static
>>> region, the FPGA partial reconfiguration region, and the BMC.
>>> The image files are self-describing, and contain a header describing
>>> the image type.
>>>
>>> Secure updates make use of the request_firmware framework, which
>>> requires that image files are accessible under /lib/firmware. A request
>>> for a secure update returns immediately, while the update itself
>>> proceeds in the context of a kernel worker thread. Sysfs files provide
>>> a means for monitoring the progress of a secure update and for
>>> retrieving error information in the event of a failure.
>>>
>>> The API includes a "name" sysfs file to export the name of the parent
>>> driver. It also includes an "update" sub-directory containing files that
>>> that can be used to instantiate and monitor a secure update.
>>>
>>> Changelog v9 -> v10:
>>> - Rebased to 5.12-rc2 next
>>> - Updated Date and KernelVersion in ABI documentation
>>>
>>> Changelog v8 -> v9:
>>> - Rebased patches for 5.11-rc2
>>> - Updated Date and KernelVersion in ABI documentation
>>>
>>> Changelog v7 -> v8:
>>> - Fixed grammatical error in Documentation/fpga/fpga-sec-mgr.rst
>>>
>>> Changelog v6 -> v7:
>>> - Changed dates in documentation file to December 2020
>>> - Changed filename_store() to use kmemdup_nul() instead of
>>> kstrndup() and changed the count to not assume a line-return.
>>>
>>> Changelog v5 -> v6:
>>> - Removed sysfs support and documentation for the display of the
>>> flash count, root entry hashes, and code-signing-key cancelation
>>> vectors from the class driver. This information can vary by device
>>> and will instead be displayed by the device-specific parent driver.
>>>
>>> Changelog v4 -> v5:
>>> - Added the devm_fpga_sec_mgr_unregister() function, following recent
>>> changes to the fpga_manager() implementation.
>>> - Changed most of the *_show() functions to use sysfs_emit()
>>> instead of sprintf(
>>> - When checking the return values for functions of type enum
>>> fpga_sec_err err_code, test for FPGA_SEC_ERR_NONE instead of 0
>>>
>>> Changelog v3 -> v4:
>>> - This driver is generic enough that it could be used for non Intel
>>> FPGA devices. 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_
>>> Note that this also affects some filenames.
>>>
>>> Changelog v2 -> v3:
>>> - Use dev_err() to report invalid progress in sec_progress()
>>> - Use dev_err() to report invalid error code in sec_error()
>>> - Modified sysfs handler check in check_sysfs_handler() to make
>>> it more readable.
>>> - Removed unnecessary "goto done"
>>> - Added a comment to explain imgr->driver_unload in
>>> ifpga_sec_mgr_unregister()
>>>
>>> Changelog v1 -> v2:
>>> - Separated out the MAX10 BMC Security Engine to be submitted in
>>> a separate patch-set.
>>> - Bumped documentation dates and versions
>>> - Split ifpga_sec_mgr_register() into create() and register() functions
>>> - Added devm_ifpga_sec_mgr_create()
>>> - Added Documentation/fpga/ifpga-sec-mgr.rst
>>> - Changed progress state "read_file" to "reading"
>>> - Added sec_error() function (similar to sec_progress())
>>> - Removed references to bmc_flash_count & smbus_flash_count (not supported)
>>> - Removed typedefs for imgr ops
>>> - Removed explicit value assignments in enums
>>> - Other minor code cleanup per review comments
>>>
>>> Russ Weight (7):
>>> fpga: sec-mgr: fpga security manager class driver
>>> fpga: sec-mgr: enable secure updates
>>> fpga: sec-mgr: expose sec-mgr update status
>>> fpga: sec-mgr: expose sec-mgr update errors
>>> fpga: sec-mgr: expose sec-mgr update size
>>> fpga: sec-mgr: enable cancel of secure update
>>> fpga: sec-mgr: expose hardware error info
>>>
>>> .../ABI/testing/sysfs-class-fpga-sec-mgr | 81 +++
>>> 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 | 652 ++++++++++++++++++
>>> include/linux/fpga/fpga-sec-mgr.h | 100 +++
>>> 8 files changed, 899 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
>>>