diff mbox series

[v6,2/7] fpga: sec-mgr: enable secure updates

Message ID 20201106010905.11935-3-russell.h.weight@intel.com (mailing list archive)
State Superseded, archived
Headers show
Series FPGA Security Manager Class Driver | expand

Commit Message

Russ Weight Nov. 6, 2020, 1:09 a.m. UTC
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 <russell.h.weight@intel.com>
Reviewed-by: Tom Rix <trix@redhat.com>
---
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(+)

Comments

Martin Hundebøll Nov. 19, 2020, 9:28 a.m. UTC | #1
Hi Russ,

Just stumbled upon the below when preparing to upstream some Silicom changes

On 06/11/2020 02.09, Russ Weight wrote:

<snip>

> 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
> +};

<snip>

> +
> +/* 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
>   };

Shouldn't this enum and the fpga_sec_err above be indexed starting from 
0, to make comparison with FPGA_SEC_ERR_MAX and

FPGA_SEC_PROG_MAX correct?

// Martin
Martin Hundebøll Nov. 26, 2020, 2:02 p.m. UTC | #2
Hi Russ,

I found another thing while testing this...

On 06/11/2020 02.09, Russ Weight wrote:

<snip>

> +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 = kstrndup(buf, count - 1, GFP_KERNEL);

The `count - 1` is meant to remove a trailing newline, but opae-sdk 
writes the filename without newline, so better do it conditionally...

> +	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,
> +};

Thanks,
Martin
Russ Weight Nov. 30, 2020, 11:54 p.m. UTC | #3
Thanks Martin. I'll work on a fix for this.

- Russ

On 11/26/20 6:02 AM, Martin Hundebøll wrote:
> Hi Russ,
>
> I found another thing while testing this...
>
> On 06/11/2020 02.09, Russ Weight wrote:
>
> <snip>
>
>> +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 = kstrndup(buf, count - 1, GFP_KERNEL);
>
> The `count - 1` is meant to remove a trailing newline, but opae-sdk writes the filename without newline, so better do it conditionally...
>
>> +    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,
>> +};
>
> Thanks,
> Martin
Martin Hundebøll Dec. 1, 2020, 8:47 a.m. UTC | #4
Hi Russ,

On 01/12/2020 00.54, Russ Weight wrote:
> Thanks Martin. I'll work on a fix for this.

Attached is my in-house fix.

// Martin

> On 11/26/20 6:02 AM, Martin Hundebøll wrote:
>> Hi Russ,
>>
>> I found another thing while testing this...
>>
>> On 06/11/2020 02.09, Russ Weight wrote:
>>
>> <snip>
>>
>>> +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 = kstrndup(buf, count - 1, GFP_KERNEL);
>>
>> The `count - 1` is meant to remove a trailing newline, but opae-sdk writes the filename without newline, so better do it conditionally...
>>
>>> +    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,
>>> +};
>>
>> Thanks,
>> Martin
>
Russ Weight Dec. 1, 2020, 11:30 p.m. UTC | #5
On 12/1/20 12:47 AM, Martin Hundebøll wrote:
> Hi Russ,
>
> On 01/12/2020 00.54, Russ Weight wrote:
>> Thanks Martin. I'll work on a fix for this.
>
> Attached is my in-house fix.
>
> // Martin
>
>> On 11/26/20 6:02 AM, Martin Hundebøll wrote:
>>> Hi Russ,
>>>
>>> I found another thing while testing this...
>>>
>>> On 06/11/2020 02.09, Russ Weight wrote:
>>>
>>> <snip>
>>>
>>>> +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 = kstrndup(buf, count - 1, GFP_KERNEL);
>>>
>>> The `count - 1` is meant to remove a trailing newline, but opae-sdk writes the filename without newline, so better do it conditionally...

After looking at how kstrndup() is used elsewhere, and after
doing some experimentation, I think the best fix may be to just
remove the "- 1":

    smgr->filename = kstrndup(buf, count, GFP_KERNEL);

The code shouldn't have assumed a "\n", and I don't think the
kernel should be required to do white-space cleanup.

Does this fix seem OK to you?

- Russ
>>>
>>>> +    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,
>>>> +};
>>>
>>> Thanks,
>>> Martin
>>
Martin Hundebøll Dec. 2, 2020, 1:40 p.m. UTC | #6
Hi Russ,

On 02/12/2020 00.30, Russ Weight wrote:
> 
> On 12/1/20 12:47 AM, Martin Hundebøll wrote:
>> Hi Russ,
>>
>> On 01/12/2020 00.54, Russ Weight wrote:
>>> Thanks Martin. I'll work on a fix for this.
>> Attached is my in-house fix.
>>
>> // Martin
>>
>>> On 11/26/20 6:02 AM, Martin Hundebøll wrote:
>>>> Hi Russ,
>>>>
>>>> I found another thing while testing this...
>>>>
>>>> On 06/11/2020 02.09, Russ Weight wrote:
>>>>
>>>> <snip>
>>>>
>>>>> +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 = kstrndup(buf, count - 1, GFP_KERNEL);
>>>> The `count - 1` is meant to remove a trailing newline, but opae-sdk writes the filename without newline, so better do it conditionally...
> After looking at how kstrndup() is used elsewhere, and after
> doing some experimentation, I think the best fix may be to just
> remove the "- 1":
> 
>      smgr->filename = kstrndup(buf, count, GFP_KERNEL);
> 
> The code shouldn't have assumed a "\n", and I don't think the
> kernel should be required to do white-space cleanup.
> 
> Does this fix seem OK to you?

Since we always use opae-sdk to write the filename, we wouldn't even notice.

While your'e at it, kerneldoc in mm/util.c suggests to use kmemdup_nul() 
when the size is known.

// Martin
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
index ecda22a3ff3b..b50c662fea14 100644
--- a/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
+++ b/Documentation/ABI/testing/sysfs-class-fpga-sec-mgr
@@ -3,3 +3,16 @@  Date:		Oct 2020
 KernelVersion:  5.11
 Contact:	Russ Weight <russell.h.weight@intel.com>
 Description:	Name of low level fpga security manager driver.
+
+What: 		/sys/class/fpga_sec_mgr/fpga_secX/update/filename
+Date:		Oct 2020
+KernelVersion:  5.11
+Contact:	Russ Weight <russell.h.weight@intel.com>
+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..77548937aac9 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 = kstrndup(buf, count - 1, 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;
 };