| Message ID | 20210929230025.68961-3-russell.h.weight@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | FPGA Image Load (previously Security Manager) | expand |
On 9/29/21 4:00 PM, Russ Weight wrote: > Extend the FPGA Image Load framework to include IOCTL support > (FPGA_IMAGE_LOAD_WRITE) for initiating an upload of an image to a device. > The IOCTL will return immediately, and the update will begin in the > context of a kernel worker thread. > > Signed-off-by: Russ Weight <russell.h.weight@intel.com> > --- > v17: > - Minor documentation cleanup and references to cancellation are moved > to a later patch. > - We no longer send a "just in case" cancel on HW errors. The cleanup() > op can and should be used to manage the HW state on an error. > - Removed the FPGA_IMAGE_ERR_NONE macro in favor of returning zero. The > constant name made sense when we were using enum data types, but is no > longer necessary. > - Changed the syntax of the write_blk() function and renamed it to write(). > It now returns a positive size or a negative error number. > - Changed the prepare() and write() ops to pass in elements of the > fpga_image_load data structure instead of having the op functions access > them directly via imgld. > - Removed the default block size. The lower-level driver must choose a > block size. > - Changed work queue from system_unbound_wq to system_long_wq. > - Removed calls to cond_resched(). The lower-level driver must manage > any scheduler issues. > - Use the cdev_set_parent() macro. > v16: > - Shift from "Driver" terminology to "Framework" in comments and > documentation > - Rename lops to ops for structure member and local variables > - Change the write_blk() definition to pass in *blk_size (a pointer to > a default block size of WRITE_BLOCK_SIZE=0x4000) and max_size (the > the maximum block size to stay within the limit of the data buffer). > The write_blk() op may use the default *blk_size or modify it to a > more optimal number for the given device, subject to the max_size limit. > - All enum values for progress and errors are changed to macros, because > they are included in the uapi header. This is done to maintain consistency > amongst the DFL related IOCTL header files. All references to the enum > types are changed to u32. > - Bail out early in fpga_image_do_load() if imgld->driver_unload is true. > - Add a call to cond_resched() in the write_blk() loop to ensure that > we yield to higher priority tasks during long data transfers. > - Switch to the system_unbound_wq to enable calls to cond_resched(). > - Switch from test_and_set_bit() to atomic_cmpxchg() to manage > imgld->opened. > - Change fpga_image_load_release() to block until the image upload is > complete. > - Remove the completion object, imgld->update_done, in favor of calling > flush_work(&imgld->work); > v15: > - Compare to previous patch: > [PATCH v14 2/6] fpga: sec-mgr: enable secure updates > - Changed file, symbol, and config names to reflect the new driver name > - Removed update/filename sysfs file and added the FPGA_IMAGE_LOAD_WRITE > IOCTL for writing the image data to the FPGA card. The driver no longer > uses the request_firmware framework. > - Fixed some error return values in fpga_image_load_register() > - Removed signed-off/reviewed-by tags > v14: > - Added MAINTAINERS reference for > Documentation/ABI/testing/sysfs-class-fpga-sec-mgr > - Updated ABI documentation date and kernel version > - Updated copyright to 2021 > v13: > - Change "if (count == 0 || " to "if (!count || " > - Improve error message: "Attempt to register without all required ops\n" > v12: > - Updated Date and KernelVersion fields in ABI documentation > - Removed size parameter from write_blk() op - it is now up to > the lower-level driver to determine the appropriate size and > to update smgr->remaining_size accordingly. > v11: > - Fixed a spelling error in a comment > - Initialize smgr->err_code and smgr->progress explicitly in > fpga_sec_mgr_create() instead of accepting the default 0 value. > 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 > --- > Documentation/fpga/fpga-image-load.rst | 25 ++- > MAINTAINERS | 1 + > drivers/fpga/fpga-image-load.c | 217 ++++++++++++++++++++++++- > include/linux/fpga/fpga-image-load.h | 27 +++ > include/uapi/linux/fpga-image-load.h | 53 ++++++ > 5 files changed, 321 insertions(+), 2 deletions(-) > create mode 100644 include/uapi/linux/fpga-image-load.h > > diff --git a/Documentation/fpga/fpga-image-load.rst b/Documentation/fpga/fpga-image-load.rst > index dda9283ef1c7..f6b0e1624a05 100644 > --- a/Documentation/fpga/fpga-image-load.rst > +++ b/Documentation/fpga/fpga-image-load.rst > @@ -7,4 +7,27 @@ FPGA Image Load Framework > The FPGA Image Load framework provides a common API for user-space > tools to manage image uploads to FPGA devices. Device drivers that > instantiate the FPGA Image Load framework will interact with the > -target device to transfer and authenticate the image data. > +target device to transfer and authenticate the image data. Image uploads > +are processed in the context of a kernel worker thread. > + > +User API > +======== > + > +open > +---- > + > +An fpga_image_load device is opened exclusively to control an image upload. > +The device must remain open throughout the duration of the image upload. > +An attempt to close the device while an upload is in progress will block > +until the image upload is complete. > + > +ioctl > +----- > + > +FPGA_IMAGE_LOAD_WRITE: > + > +Start an image upload with the provided image buffer. This IOCTL returns > +immediately after starting a kernel worker thread to process the image > +upload which could take as long as 40 minutes depending on the actual > +device being updated. This is an exclusive operation; an attempt to start > +concurrent image uploads for the same device will fail with EBUSY. > diff --git a/MAINTAINERS b/MAINTAINERS > index a99622eebbff..e3b5c555ecbd 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7415,6 +7415,7 @@ S: Maintained > F: Documentation/fpga/fpga-image-load.rst > F: drivers/fpga/fpga-image-load.c > F: include/linux/fpga/fpga-image-load.h > +F: include/uapi/linux/fpga-image-load.h > > FPU EMULATOR > M: Bill Metzenthen <billm@melbpc.org.au> > diff --git a/drivers/fpga/fpga-image-load.c b/drivers/fpga/fpga-image-load.c > index 799d18444f7c..4c2571e3b26f 100644 > --- a/drivers/fpga/fpga-image-load.c > +++ b/drivers/fpga/fpga-image-load.c > @@ -5,18 +5,190 @@ > * Copyright (C) 2019-2021 Intel Corporation, Inc. > */ > > +#include <linux/delay.h> > #include <linux/fpga/fpga-image-load.h> > +#include <linux/fs.h> > +#include <linux/kernel.h> > #include <linux/module.h> > #include <linux/slab.h> > +#include <linux/uaccess.h> > #include <linux/vmalloc.h> > > #define IMAGE_LOAD_XA_LIMIT XA_LIMIT(0, INT_MAX) > static DEFINE_XARRAY_ALLOC(fpga_image_load_xa); > > static struct class *fpga_image_load_class; > +static dev_t fpga_image_devt; > > #define to_image_load(d) container_of(d, struct fpga_image_load, dev) > > +static void fpga_image_prog_complete(struct fpga_image_load *imgld) > +{ > + mutex_lock(&imgld->lock); > + imgld->progress = FPGA_IMAGE_PROG_IDLE; > + mutex_unlock(&imgld->lock); > +} > + > +static void fpga_image_do_load(struct work_struct *work) > +{ > + struct fpga_image_load *imgld; > + s32 ret, offset = 0; > + > + imgld = container_of(work, struct fpga_image_load, work); > + > + if (imgld->driver_unload) { > + imgld->err_code = FPGA_IMAGE_ERR_CANCELED; > + goto idle_exit; > + } > + > + get_device(&imgld->dev); > + if (!try_module_get(imgld->dev.parent->driver->owner)) { > + imgld->err_code = FPGA_IMAGE_ERR_BUSY; > + goto putdev_exit; > + } > + > + imgld->progress = FPGA_IMAGE_PROG_PREPARING; > + ret = imgld->ops->prepare(imgld, imgld->data, imgld->remaining_size); > + if (ret) { > + imgld->err_code = ret; > + goto modput_exit; > + } > + > + imgld->progress = FPGA_IMAGE_PROG_WRITING; > + while (imgld->remaining_size) { > + ret = imgld->ops->write(imgld, imgld->data, offset, > + imgld->remaining_size); > + if (ret <= 0) { > + if (!ret) { > + dev_warn(&imgld->dev, > + "write-op wrote zero data\n"); > + ret = -FPGA_IMAGE_ERR_RW_ERROR; > + } > + imgld->err_code = -ret; > + goto done; > + } > + > + imgld->remaining_size -= ret; > + offset += ret; > + } > + > + imgld->progress = FPGA_IMAGE_PROG_PROGRAMMING; > + ret = imgld->ops->poll_complete(imgld); > + if (ret) > + imgld->err_code = ret; > + > +done: > + if (imgld->ops->cleanup) > + imgld->ops->cleanup(imgld); > + > +modput_exit: > + module_put(imgld->dev.parent->driver->owner); > + > +putdev_exit: > + put_device(&imgld->dev); > + > +idle_exit: > + /* > + * Note: imgld->remaining_size is left unmodified here to provide > + * additional information on errors. It will be reinitialized when > + * the next image load begins. > + */ > + vfree(imgld->data); > + imgld->data = NULL; > + fpga_image_prog_complete(imgld); > +} > + > +static int fpga_image_load_ioctl_write(struct fpga_image_load *imgld, > + unsigned long arg) > +{ > + struct fpga_image_write wb; > + unsigned long minsz; > + u8 *buf; > + > + if (imgld->driver_unload || imgld->progress != FPGA_IMAGE_PROG_IDLE) > + return -EBUSY; > + > + minsz = offsetofend(struct fpga_image_write, buf); > + if (copy_from_user(&wb, (void __user *)arg, minsz)) > + return -EFAULT; > + > + if (wb.flags) > + return -EINVAL; FYI: Static analysis flagged that wb.size is not being validated before being used by vzalloc. It is a 32-bit unsigned value, so size is probably not a concern. If there are size or alignment constraints, they belong to the lower-level driver and should be checked there. I propose to add a null check here: + if (!wb.size) + return -EINVAL; + I'll await more comments before I re-roll the patch set. - Russ > + > + buf = vzalloc(wb.size); > + if (!buf) > + return -ENOMEM; > + > + if (copy_from_user(buf, u64_to_user_ptr(wb.buf), wb.size)) { > + vfree(buf); > + return -EFAULT; > + } > + > + imgld->data = buf; > + imgld->remaining_size = wb.size; > + imgld->err_code = 0; > + imgld->progress = FPGA_IMAGE_PROG_STARTING; > + queue_work(system_long_wq, &imgld->work); > + > + return 0; > +} > + > +static long fpga_image_load_ioctl(struct file *filp, unsigned int cmd, > + unsigned long arg) > +{ > + struct fpga_image_load *imgld = filp->private_data; > + int ret = -ENOTTY; > + > + switch (cmd) { > + case FPGA_IMAGE_LOAD_WRITE: > + mutex_lock(&imgld->lock); > + ret = fpga_image_load_ioctl_write(imgld, arg); > + mutex_unlock(&imgld->lock); > + break; > + } > + > + return ret; > +} > + > +static int fpga_image_load_open(struct inode *inode, struct file *filp) > +{ > + struct fpga_image_load *imgld = container_of(inode->i_cdev, > + struct fpga_image_load, cdev); > + > + if (atomic_cmpxchg(&imgld->opened, 0, 1)) > + return -EBUSY; > + > + filp->private_data = imgld; > + > + return 0; > +} > + > +static int fpga_image_load_release(struct inode *inode, struct file *filp) > +{ > + struct fpga_image_load *imgld = filp->private_data; > + > + mutex_lock(&imgld->lock); > + if (imgld->progress == FPGA_IMAGE_PROG_IDLE) { > + mutex_unlock(&imgld->lock); > + goto close_exit; > + } > + > + mutex_unlock(&imgld->lock); > + flush_work(&imgld->work); > + > +close_exit: > + atomic_set(&imgld->opened, 0); > + > + return 0; > +} > + > +static const struct file_operations fpga_image_load_fops = { > + .owner = THIS_MODULE, > + .open = fpga_image_load_open, > + .release = fpga_image_load_release, > + .unlocked_ioctl = fpga_image_load_ioctl, > +}; > + > /** > * fpga_image_load_register - create and register an FPGA Image Load Device > * > @@ -35,6 +207,11 @@ fpga_image_load_register(struct device *parent, > struct fpga_image_load *imgld; > int ret; > > + if (!ops || !ops->prepare || !ops->write || !ops->poll_complete) { > + dev_err(parent, "Attempt to register without all required ops\n"); > + return ERR_PTR(-ENOMEM); > + } > + > imgld = kzalloc(sizeof(*imgld), GFP_KERNEL); > if (!imgld) > return ERR_PTR(-ENOMEM); > @@ -48,9 +225,13 @@ fpga_image_load_register(struct device *parent, > > imgld->priv = priv; > imgld->ops = ops; > + imgld->err_code = 0; > + imgld->progress = FPGA_IMAGE_PROG_IDLE; > + INIT_WORK(&imgld->work, fpga_image_do_load); > > imgld->dev.class = fpga_image_load_class; > imgld->dev.parent = parent; > + imgld->dev.devt = MKDEV(MAJOR(fpga_image_devt), imgld->dev.id); > > ret = dev_set_name(&imgld->dev, "fpga_image_load%d", imgld->dev.id); > if (ret) { > @@ -65,6 +246,16 @@ fpga_image_load_register(struct device *parent, > return ERR_PTR(ret); > } > > + cdev_init(&imgld->cdev, &fpga_image_load_fops); > + imgld->cdev.owner = parent->driver->owner; > + cdev_set_parent(&imgld->cdev, &imgld->dev.kobj); > + > + ret = cdev_add(&imgld->cdev, imgld->dev.devt, 1); > + if (ret) { > + put_device(&imgld->dev); > + return ERR_PTR(ret); > + } > + > return imgld; > > error_device: > @@ -83,10 +274,23 @@ EXPORT_SYMBOL_GPL(fpga_image_load_register); > * @imgld: pointer to struct fpga_image_load > * > * This function is intended for use in the parent driver's remove() > - * function. > + * function. The driver_unload flag prevents new updates from starting > + * once the unregister process has begun. > */ > void fpga_image_load_unregister(struct fpga_image_load *imgld) > { > + mutex_lock(&imgld->lock); > + imgld->driver_unload = true; > + if (imgld->progress == FPGA_IMAGE_PROG_IDLE) { > + mutex_unlock(&imgld->lock); > + goto unregister; > + } > + > + mutex_unlock(&imgld->lock); > + flush_work(&imgld->work); > + > +unregister: > + cdev_del(&imgld->cdev); > device_unregister(&imgld->dev); > } > EXPORT_SYMBOL_GPL(fpga_image_load_unregister); > @@ -101,19 +305,30 @@ static void fpga_image_load_dev_release(struct device *dev) > > static int __init fpga_image_load_class_init(void) > { > + int ret; > pr_info("FPGA Image Load Framework\n"); > > fpga_image_load_class = class_create(THIS_MODULE, "fpga_image_load"); > if (IS_ERR(fpga_image_load_class)) > return PTR_ERR(fpga_image_load_class); > > + ret = alloc_chrdev_region(&fpga_image_devt, 0, MINORMASK, > + "fpga_image_load"); > + if (ret) > + goto exit_destroy_class; > + > fpga_image_load_class->dev_release = fpga_image_load_dev_release; > > return 0; > + > +exit_destroy_class: > + class_destroy(fpga_image_load_class); > + return ret; > } > > static void __exit fpga_image_load_class_exit(void) > { > + unregister_chrdev_region(fpga_image_devt, MINORMASK); > class_destroy(fpga_image_load_class); > WARN_ON(!xa_empty(&fpga_image_load_xa)); > } > diff --git a/include/linux/fpga/fpga-image-load.h b/include/linux/fpga/fpga-image-load.h > index 8b051c82ef5f..7b445f489644 100644 > --- a/include/linux/fpga/fpga-image-load.h > +++ b/include/linux/fpga/fpga-image-load.h > @@ -7,22 +7,49 @@ > #ifndef _LINUX_FPGA_IMAGE_LOAD_H > #define _LINUX_FPGA_IMAGE_LOAD_H > > +#include <linux/cdev.h> > #include <linux/device.h> > #include <linux/mutex.h> > #include <linux/types.h> > +#include <uapi/linux/fpga-image-load.h> > > struct fpga_image_load; > > /** > * struct fpga_image_load_ops - device specific operations > + * @prepare: Required: Prepare secure update > + * @write: Required: The write() op receives the remaining > + * size to be written and must return the actual > + * size written or a negative error code. The write() > + * op will be called repeatedly until all data is > + * written. > + * @poll_complete: Required: Check for the completion of the > + * HW authentication/programming process. > + * @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_image_load_ops { > + u32 (*prepare)(struct fpga_image_load *imgld, const u8 *data, u32 size); > + s32 (*write)(struct fpga_image_load *imgld, const u8 *data, > + u32 offset, u32 size); > + u32 (*poll_complete)(struct fpga_image_load *imgld); > + void (*cleanup)(struct fpga_image_load *imgld); > }; > > struct fpga_image_load { > struct device dev; > + struct cdev cdev; > const struct fpga_image_load_ops *ops; > struct mutex lock; /* protect data structure contents */ > + atomic_t opened; > + struct work_struct work; > + const u8 *data; /* pointer to update data */ > + u32 remaining_size; /* size remaining to transfer */ > + u32 progress; > + u32 err_code; /* image load error code */ > + bool driver_unload; > void *priv; > }; > > diff --git a/include/uapi/linux/fpga-image-load.h b/include/uapi/linux/fpga-image-load.h > new file mode 100644 > index 000000000000..20eae3bb10d8 > --- /dev/null > +++ b/include/uapi/linux/fpga-image-load.h > @@ -0,0 +1,53 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +/* > + * Header File for FPGA Image Load User API > + * > + * Copyright (C) 2019-2021 Intel Corporation, Inc. > + * > + */ > + > +#ifndef _UAPI_LINUX_FPGA_IMAGE_LOAD_H > +#define _UAPI_LINUX_FPGA_IMAGE_LOAD_H > + > +#include <linux/types.h> > +#include <linux/ioctl.h> > + > +#define FPGA_IMAGE_LOAD_MAGIC 0xB9 > + > +/* Image load progress codes */ > +#define FPGA_IMAGE_PROG_IDLE 0 > +#define FPGA_IMAGE_PROG_STARTING 1 > +#define FPGA_IMAGE_PROG_PREPARING 2 > +#define FPGA_IMAGE_PROG_WRITING 3 > +#define FPGA_IMAGE_PROG_PROGRAMMING 4 > +#define FPGA_IMAGE_PROG_MAX 5 > + > +/* Image error progress codes */ > +#define FPGA_IMAGE_ERR_HW_ERROR 1 > +#define FPGA_IMAGE_ERR_TIMEOUT 2 > +#define FPGA_IMAGE_ERR_CANCELED 3 > +#define FPGA_IMAGE_ERR_BUSY 4 > +#define FPGA_IMAGE_ERR_INVALID_SIZE 5 > +#define FPGA_IMAGE_ERR_RW_ERROR 6 > +#define FPGA_IMAGE_ERR_WEAROUT 7 > +#define FPGA_IMAGE_ERR_MAX 8 > + > +/** > + * FPGA_IMAGE_LOAD_WRITE - _IOW(FPGA_IMAGE_LOAD_MAGIC, 0, > + * struct fpga_image_write) > + * > + * Upload a data buffer to the target device. The user must provide the > + * data buffer and size. > + * > + * Return: 0 on success, -errno on failure. > + */ > +struct fpga_image_write { > + /* Input */ > + __u32 flags; /* Zero for now */ > + __u32 size; /* Data size (in bytes) to be written */ > + __u64 buf; /* User space address of source data */ > +}; > + > +#define FPGA_IMAGE_LOAD_WRITE _IOW(FPGA_IMAGE_LOAD_MAGIC, 0, struct fpga_image_write) > + > +#endif /* _UAPI_LINUX_FPGA_IMAGE_LOAD_H */
diff --git a/Documentation/fpga/fpga-image-load.rst b/Documentation/fpga/fpga-image-load.rst index dda9283ef1c7..f6b0e1624a05 100644 --- a/Documentation/fpga/fpga-image-load.rst +++ b/Documentation/fpga/fpga-image-load.rst @@ -7,4 +7,27 @@ FPGA Image Load Framework The FPGA Image Load framework provides a common API for user-space tools to manage image uploads to FPGA devices. Device drivers that instantiate the FPGA Image Load framework will interact with the -target device to transfer and authenticate the image data. +target device to transfer and authenticate the image data. Image uploads +are processed in the context of a kernel worker thread. + +User API +======== + +open +---- + +An fpga_image_load device is opened exclusively to control an image upload. +The device must remain open throughout the duration of the image upload. +An attempt to close the device while an upload is in progress will block +until the image upload is complete. + +ioctl +----- + +FPGA_IMAGE_LOAD_WRITE: + +Start an image upload with the provided image buffer. This IOCTL returns +immediately after starting a kernel worker thread to process the image +upload which could take as long as 40 minutes depending on the actual +device being updated. This is an exclusive operation; an attempt to start +concurrent image uploads for the same device will fail with EBUSY. diff --git a/MAINTAINERS b/MAINTAINERS index a99622eebbff..e3b5c555ecbd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7415,6 +7415,7 @@ S: Maintained F: Documentation/fpga/fpga-image-load.rst F: drivers/fpga/fpga-image-load.c F: include/linux/fpga/fpga-image-load.h +F: include/uapi/linux/fpga-image-load.h FPU EMULATOR M: Bill Metzenthen <billm@melbpc.org.au> diff --git a/drivers/fpga/fpga-image-load.c b/drivers/fpga/fpga-image-load.c index 799d18444f7c..4c2571e3b26f 100644 --- a/drivers/fpga/fpga-image-load.c +++ b/drivers/fpga/fpga-image-load.c @@ -5,18 +5,190 @@ * Copyright (C) 2019-2021 Intel Corporation, Inc. */ +#include <linux/delay.h> #include <linux/fpga/fpga-image-load.h> +#include <linux/fs.h> +#include <linux/kernel.h> #include <linux/module.h> #include <linux/slab.h> +#include <linux/uaccess.h> #include <linux/vmalloc.h> #define IMAGE_LOAD_XA_LIMIT XA_LIMIT(0, INT_MAX) static DEFINE_XARRAY_ALLOC(fpga_image_load_xa); static struct class *fpga_image_load_class; +static dev_t fpga_image_devt; #define to_image_load(d) container_of(d, struct fpga_image_load, dev) +static void fpga_image_prog_complete(struct fpga_image_load *imgld) +{ + mutex_lock(&imgld->lock); + imgld->progress = FPGA_IMAGE_PROG_IDLE; + mutex_unlock(&imgld->lock); +} + +static void fpga_image_do_load(struct work_struct *work) +{ + struct fpga_image_load *imgld; + s32 ret, offset = 0; + + imgld = container_of(work, struct fpga_image_load, work); + + if (imgld->driver_unload) { + imgld->err_code = FPGA_IMAGE_ERR_CANCELED; + goto idle_exit; + } + + get_device(&imgld->dev); + if (!try_module_get(imgld->dev.parent->driver->owner)) { + imgld->err_code = FPGA_IMAGE_ERR_BUSY; + goto putdev_exit; + } + + imgld->progress = FPGA_IMAGE_PROG_PREPARING; + ret = imgld->ops->prepare(imgld, imgld->data, imgld->remaining_size); + if (ret) { + imgld->err_code = ret; + goto modput_exit; + } + + imgld->progress = FPGA_IMAGE_PROG_WRITING; + while (imgld->remaining_size) { + ret = imgld->ops->write(imgld, imgld->data, offset, + imgld->remaining_size); + if (ret <= 0) { + if (!ret) { + dev_warn(&imgld->dev, + "write-op wrote zero data\n"); + ret = -FPGA_IMAGE_ERR_RW_ERROR; + } + imgld->err_code = -ret; + goto done; + } + + imgld->remaining_size -= ret; + offset += ret; + } + + imgld->progress = FPGA_IMAGE_PROG_PROGRAMMING; + ret = imgld->ops->poll_complete(imgld); + if (ret) + imgld->err_code = ret; + +done: + if (imgld->ops->cleanup) + imgld->ops->cleanup(imgld); + +modput_exit: + module_put(imgld->dev.parent->driver->owner); + +putdev_exit: + put_device(&imgld->dev); + +idle_exit: + /* + * Note: imgld->remaining_size is left unmodified here to provide + * additional information on errors. It will be reinitialized when + * the next image load begins. + */ + vfree(imgld->data); + imgld->data = NULL; + fpga_image_prog_complete(imgld); +} + +static int fpga_image_load_ioctl_write(struct fpga_image_load *imgld, + unsigned long arg) +{ + struct fpga_image_write wb; + unsigned long minsz; + u8 *buf; + + if (imgld->driver_unload || imgld->progress != FPGA_IMAGE_PROG_IDLE) + return -EBUSY; + + minsz = offsetofend(struct fpga_image_write, buf); + if (copy_from_user(&wb, (void __user *)arg, minsz)) + return -EFAULT; + + if (wb.flags) + return -EINVAL; + + buf = vzalloc(wb.size); + if (!buf) + return -ENOMEM; + + if (copy_from_user(buf, u64_to_user_ptr(wb.buf), wb.size)) { + vfree(buf); + return -EFAULT; + } + + imgld->data = buf; + imgld->remaining_size = wb.size; + imgld->err_code = 0; + imgld->progress = FPGA_IMAGE_PROG_STARTING; + queue_work(system_long_wq, &imgld->work); + + return 0; +} + +static long fpga_image_load_ioctl(struct file *filp, unsigned int cmd, + unsigned long arg) +{ + struct fpga_image_load *imgld = filp->private_data; + int ret = -ENOTTY; + + switch (cmd) { + case FPGA_IMAGE_LOAD_WRITE: + mutex_lock(&imgld->lock); + ret = fpga_image_load_ioctl_write(imgld, arg); + mutex_unlock(&imgld->lock); + break; + } + + return ret; +} + +static int fpga_image_load_open(struct inode *inode, struct file *filp) +{ + struct fpga_image_load *imgld = container_of(inode->i_cdev, + struct fpga_image_load, cdev); + + if (atomic_cmpxchg(&imgld->opened, 0, 1)) + return -EBUSY; + + filp->private_data = imgld; + + return 0; +} + +static int fpga_image_load_release(struct inode *inode, struct file *filp) +{ + struct fpga_image_load *imgld = filp->private_data; + + mutex_lock(&imgld->lock); + if (imgld->progress == FPGA_IMAGE_PROG_IDLE) { + mutex_unlock(&imgld->lock); + goto close_exit; + } + + mutex_unlock(&imgld->lock); + flush_work(&imgld->work); + +close_exit: + atomic_set(&imgld->opened, 0); + + return 0; +} + +static const struct file_operations fpga_image_load_fops = { + .owner = THIS_MODULE, + .open = fpga_image_load_open, + .release = fpga_image_load_release, + .unlocked_ioctl = fpga_image_load_ioctl, +}; + /** * fpga_image_load_register - create and register an FPGA Image Load Device * @@ -35,6 +207,11 @@ fpga_image_load_register(struct device *parent, struct fpga_image_load *imgld; int ret; + if (!ops || !ops->prepare || !ops->write || !ops->poll_complete) { + dev_err(parent, "Attempt to register without all required ops\n"); + return ERR_PTR(-ENOMEM); + } + imgld = kzalloc(sizeof(*imgld), GFP_KERNEL); if (!imgld) return ERR_PTR(-ENOMEM); @@ -48,9 +225,13 @@ fpga_image_load_register(struct device *parent, imgld->priv = priv; imgld->ops = ops; + imgld->err_code = 0; + imgld->progress = FPGA_IMAGE_PROG_IDLE; + INIT_WORK(&imgld->work, fpga_image_do_load); imgld->dev.class = fpga_image_load_class; imgld->dev.parent = parent; + imgld->dev.devt = MKDEV(MAJOR(fpga_image_devt), imgld->dev.id); ret = dev_set_name(&imgld->dev, "fpga_image_load%d", imgld->dev.id); if (ret) { @@ -65,6 +246,16 @@ fpga_image_load_register(struct device *parent, return ERR_PTR(ret); } + cdev_init(&imgld->cdev, &fpga_image_load_fops); + imgld->cdev.owner = parent->driver->owner; + cdev_set_parent(&imgld->cdev, &imgld->dev.kobj); + + ret = cdev_add(&imgld->cdev, imgld->dev.devt, 1); + if (ret) { + put_device(&imgld->dev); + return ERR_PTR(ret); + } + return imgld; error_device: @@ -83,10 +274,23 @@ EXPORT_SYMBOL_GPL(fpga_image_load_register); * @imgld: pointer to struct fpga_image_load * * This function is intended for use in the parent driver's remove() - * function. + * function. The driver_unload flag prevents new updates from starting + * once the unregister process has begun. */ void fpga_image_load_unregister(struct fpga_image_load *imgld) { + mutex_lock(&imgld->lock); + imgld->driver_unload = true; + if (imgld->progress == FPGA_IMAGE_PROG_IDLE) { + mutex_unlock(&imgld->lock); + goto unregister; + } + + mutex_unlock(&imgld->lock); + flush_work(&imgld->work); + +unregister: + cdev_del(&imgld->cdev); device_unregister(&imgld->dev); } EXPORT_SYMBOL_GPL(fpga_image_load_unregister); @@ -101,19 +305,30 @@ static void fpga_image_load_dev_release(struct device *dev) static int __init fpga_image_load_class_init(void) { + int ret; pr_info("FPGA Image Load Framework\n"); fpga_image_load_class = class_create(THIS_MODULE, "fpga_image_load"); if (IS_ERR(fpga_image_load_class)) return PTR_ERR(fpga_image_load_class); + ret = alloc_chrdev_region(&fpga_image_devt, 0, MINORMASK, + "fpga_image_load"); + if (ret) + goto exit_destroy_class; + fpga_image_load_class->dev_release = fpga_image_load_dev_release; return 0; + +exit_destroy_class: + class_destroy(fpga_image_load_class); + return ret; } static void __exit fpga_image_load_class_exit(void) { + unregister_chrdev_region(fpga_image_devt, MINORMASK); class_destroy(fpga_image_load_class); WARN_ON(!xa_empty(&fpga_image_load_xa)); } diff --git a/include/linux/fpga/fpga-image-load.h b/include/linux/fpga/fpga-image-load.h index 8b051c82ef5f..7b445f489644 100644 --- a/include/linux/fpga/fpga-image-load.h +++ b/include/linux/fpga/fpga-image-load.h @@ -7,22 +7,49 @@ #ifndef _LINUX_FPGA_IMAGE_LOAD_H #define _LINUX_FPGA_IMAGE_LOAD_H +#include <linux/cdev.h> #include <linux/device.h> #include <linux/mutex.h> #include <linux/types.h> +#include <uapi/linux/fpga-image-load.h> struct fpga_image_load; /** * struct fpga_image_load_ops - device specific operations + * @prepare: Required: Prepare secure update + * @write: Required: The write() op receives the remaining + * size to be written and must return the actual + * size written or a negative error code. The write() + * op will be called repeatedly until all data is + * written. + * @poll_complete: Required: Check for the completion of the + * HW authentication/programming process. + * @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_image_load_ops { + u32 (*prepare)(struct fpga_image_load *imgld, const u8 *data, u32 size); + s32 (*write)(struct fpga_image_load *imgld, const u8 *data, + u32 offset, u32 size); + u32 (*poll_complete)(struct fpga_image_load *imgld); + void (*cleanup)(struct fpga_image_load *imgld); }; struct fpga_image_load { struct device dev; + struct cdev cdev; const struct fpga_image_load_ops *ops; struct mutex lock; /* protect data structure contents */ + atomic_t opened; + struct work_struct work; + const u8 *data; /* pointer to update data */ + u32 remaining_size; /* size remaining to transfer */ + u32 progress; + u32 err_code; /* image load error code */ + bool driver_unload; void *priv; }; diff --git a/include/uapi/linux/fpga-image-load.h b/include/uapi/linux/fpga-image-load.h new file mode 100644 index 000000000000..20eae3bb10d8 --- /dev/null +++ b/include/uapi/linux/fpga-image-load.h @@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* + * Header File for FPGA Image Load User API + * + * Copyright (C) 2019-2021 Intel Corporation, Inc. + * + */ + +#ifndef _UAPI_LINUX_FPGA_IMAGE_LOAD_H +#define _UAPI_LINUX_FPGA_IMAGE_LOAD_H + +#include <linux/types.h> +#include <linux/ioctl.h> + +#define FPGA_IMAGE_LOAD_MAGIC 0xB9 + +/* Image load progress codes */ +#define FPGA_IMAGE_PROG_IDLE 0 +#define FPGA_IMAGE_PROG_STARTING 1 +#define FPGA_IMAGE_PROG_PREPARING 2 +#define FPGA_IMAGE_PROG_WRITING 3 +#define FPGA_IMAGE_PROG_PROGRAMMING 4 +#define FPGA_IMAGE_PROG_MAX 5 + +/* Image error progress codes */ +#define FPGA_IMAGE_ERR_HW_ERROR 1 +#define FPGA_IMAGE_ERR_TIMEOUT 2 +#define FPGA_IMAGE_ERR_CANCELED 3 +#define FPGA_IMAGE_ERR_BUSY 4 +#define FPGA_IMAGE_ERR_INVALID_SIZE 5 +#define FPGA_IMAGE_ERR_RW_ERROR 6 +#define FPGA_IMAGE_ERR_WEAROUT 7 +#define FPGA_IMAGE_ERR_MAX 8 + +/** + * FPGA_IMAGE_LOAD_WRITE - _IOW(FPGA_IMAGE_LOAD_MAGIC, 0, + * struct fpga_image_write) + * + * Upload a data buffer to the target device. The user must provide the + * data buffer and size. + * + * Return: 0 on success, -errno on failure. + */ +struct fpga_image_write { + /* Input */ + __u32 flags; /* Zero for now */ + __u32 size; /* Data size (in bytes) to be written */ + __u64 buf; /* User space address of source data */ +}; + +#define FPGA_IMAGE_LOAD_WRITE _IOW(FPGA_IMAGE_LOAD_MAGIC, 0, struct fpga_image_write) + +#endif /* _UAPI_LINUX_FPGA_IMAGE_LOAD_H */
Extend the FPGA Image Load framework to include IOCTL support (FPGA_IMAGE_LOAD_WRITE) for initiating an upload of an image to a device. The IOCTL will return immediately, and the update will begin in the context of a kernel worker thread. Signed-off-by: Russ Weight <russell.h.weight@intel.com> --- v17: - Minor documentation cleanup and references to cancellation are moved to a later patch. - We no longer send a "just in case" cancel on HW errors. The cleanup() op can and should be used to manage the HW state on an error. - Removed the FPGA_IMAGE_ERR_NONE macro in favor of returning zero. The constant name made sense when we were using enum data types, but is no longer necessary. - Changed the syntax of the write_blk() function and renamed it to write(). It now returns a positive size or a negative error number. - Changed the prepare() and write() ops to pass in elements of the fpga_image_load data structure instead of having the op functions access them directly via imgld. - Removed the default block size. The lower-level driver must choose a block size. - Changed work queue from system_unbound_wq to system_long_wq. - Removed calls to cond_resched(). The lower-level driver must manage any scheduler issues. - Use the cdev_set_parent() macro. v16: - Shift from "Driver" terminology to "Framework" in comments and documentation - Rename lops to ops for structure member and local variables - Change the write_blk() definition to pass in *blk_size (a pointer to a default block size of WRITE_BLOCK_SIZE=0x4000) and max_size (the the maximum block size to stay within the limit of the data buffer). The write_blk() op may use the default *blk_size or modify it to a more optimal number for the given device, subject to the max_size limit. - All enum values for progress and errors are changed to macros, because they are included in the uapi header. This is done to maintain consistency amongst the DFL related IOCTL header files. All references to the enum types are changed to u32. - Bail out early in fpga_image_do_load() if imgld->driver_unload is true. - Add a call to cond_resched() in the write_blk() loop to ensure that we yield to higher priority tasks during long data transfers. - Switch to the system_unbound_wq to enable calls to cond_resched(). - Switch from test_and_set_bit() to atomic_cmpxchg() to manage imgld->opened. - Change fpga_image_load_release() to block until the image upload is complete. - Remove the completion object, imgld->update_done, in favor of calling flush_work(&imgld->work); v15: - Compare to previous patch: [PATCH v14 2/6] fpga: sec-mgr: enable secure updates - Changed file, symbol, and config names to reflect the new driver name - Removed update/filename sysfs file and added the FPGA_IMAGE_LOAD_WRITE IOCTL for writing the image data to the FPGA card. The driver no longer uses the request_firmware framework. - Fixed some error return values in fpga_image_load_register() - Removed signed-off/reviewed-by tags v14: - Added MAINTAINERS reference for Documentation/ABI/testing/sysfs-class-fpga-sec-mgr - Updated ABI documentation date and kernel version - Updated copyright to 2021 v13: - Change "if (count == 0 || " to "if (!count || " - Improve error message: "Attempt to register without all required ops\n" v12: - Updated Date and KernelVersion fields in ABI documentation - Removed size parameter from write_blk() op - it is now up to the lower-level driver to determine the appropriate size and to update smgr->remaining_size accordingly. v11: - Fixed a spelling error in a comment - Initialize smgr->err_code and smgr->progress explicitly in fpga_sec_mgr_create() instead of accepting the default 0 value. 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 --- Documentation/fpga/fpga-image-load.rst | 25 ++- MAINTAINERS | 1 + drivers/fpga/fpga-image-load.c | 217 ++++++++++++++++++++++++- include/linux/fpga/fpga-image-load.h | 27 +++ include/uapi/linux/fpga-image-load.h | 53 ++++++ 5 files changed, 321 insertions(+), 2 deletions(-) create mode 100644 include/uapi/linux/fpga-image-load.h