On 9/10/21 1:52 AM, Xu Yilun wrote:
> On Wed, Sep 08, 2021 at 07:18:45PM -0700, Russ Weight wrote:
>> Extend the FPGA Image Load class driver to include a status sysfs node that
>> can be viewed to determine from the command line if an image load is in
>> progress. Status will be one of: idle, starting, preparing, writing, or
>> programming.
> The FPGA_IMAGE_LOAD_STATUS ioctl already provides the progress info.
> Why we need 2 user interfaces for the same information?
Updates on Vista Creek can take up to 40 minutes. I thought it might
be helpful to have a simple way, from the command line, to verify
whether or not there is an image upload in progress. Do you agree? Or
do you think this is unnecessary? Should I remove it? Or save it for
a later patch (after the main patches have been accepted)?
- Russ
>
> Thanks,
> Yilun
>
>> Signed-off-by: Russ Weight <russell.h.weight@xxxxxxxxx>
>> ---
>> v15:
>> - Compare to previous patch:
>> [PATCH v14 3/6] fpga: sec-mgr: expose sec-mgr update status
>> - Changed file, symbol, and config names to reflect the new driver name
>> - Removed signed-off/reviewed-by tags
>> v14:
>> - Updated ABI documentation date and kernel version
>> v13:
>> - No change
>> v12:
>> - Updated Date and KernelVersion fields in ABI documentation
>> - Changed syntax of sec_mgr_prog_str[] array definition from:
>> "idle", /* FPGA_SEC_PROG_IDLE */
>> to:
>> [FPGA_SEC_PROG_IDLE] = "idle",
>> v11:
>> - No change
>> 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 status_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 a local variable to read progress once in status_show()
>> - Use dev_err to report invalid progress status
>> v2:
>> - Bumped documentation date and version
>> - Changed progress state "read_file" to "reading"
>> ---
>> .../ABI/testing/sysfs-class-fpga-image-load | 7 ++++
>> MAINTAINERS | 1 +
>> drivers/fpga/fpga-image-load.c | 33 +++++++++++++++++++
>> 3 files changed, 41 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-class-fpga-image-load
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-fpga-image-load b/Documentation/ABI/testing/sysfs-class-fpga-image-load
>> new file mode 100644
>> index 000000000000..6c04a49f01cc
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-fpga-image-load
>> @@ -0,0 +1,7 @@
>> +What: /sys/class/fpga_image_load/fpga_imageX/status
>> +Date: Aug 2021
>> +KernelVersion: 5.15
>> +Contact: Russ Weight <russell.h.weight@xxxxxxxxx>
>> +Description: Read-only. Returns a string describing the current status
>> + of an FPGA image upload. The string will be one of the
>> + following: idle, starting, preparing, writing, programming.
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 637bc003ca81..e3fbc1bde9bc 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7362,6 +7362,7 @@ FPGA SECURITY MANAGER DRIVERS
>> M: Russ Weight <russell.h.weight@xxxxxxxxx>
>> L: linux-fpga@xxxxxxxxxxxxxxx
>> S: Maintained
>> +F: Documentation/ABI/testing/sysfs-class-fpga-image-load
>> F: Documentation/fpga/fpga-image-load.rst
>> F: drivers/fpga/fpga-image-load.c
>> F: include/linux/fpga/fpga-image-load.h
>> diff --git a/drivers/fpga/fpga-image-load.c b/drivers/fpga/fpga-image-load.c
>> index 99a47b21c995..6ec0a39f07b3 100644
>> --- a/drivers/fpga/fpga-image-load.c
>> +++ b/drivers/fpga/fpga-image-load.c
>> @@ -236,6 +236,38 @@ static const struct file_operations fpga_image_load_fops = {
>> .unlocked_ioctl = fpga_image_load_ioctl,
>> };
>>
>> +static const char * const image_load_prog_str[] = {
>> + [FPGA_IMAGE_PROG_IDLE] = "idle",
>> + [FPGA_IMAGE_PROG_STARTING] = "starting",
>> + [FPGA_IMAGE_PROG_PREPARING] = "preparing",
>> + [FPGA_IMAGE_PROG_WRITING] = "writing",
>> + [FPGA_IMAGE_PROG_PROGRAMMING] = "programming"
>> +};
>> +
>> +static ssize_t
>> +status_show(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> + struct fpga_image_load *imgld = to_image_load(dev);
>> + const char *status = "unknown-status";
>> + enum fpga_image_prog progress;
>> +
>> + progress = imgld->progress;
>> + if (progress < FPGA_IMAGE_PROG_MAX)
>> + status = image_load_prog_str[progress];
>> + else
>> + dev_err(dev, "Invalid status during secure update: %d\n",
>> + progress);
>> +
>> + return sysfs_emit(buf, "%s\n", status);
>> +}
>> +static DEVICE_ATTR_RO(status);
>> +
>> +static struct attribute *fpga_image_load_attrs[] = {
>> + &dev_attr_status.attr,
>> + NULL,
>> +};
>> +ATTRIBUTE_GROUPS(fpga_image_load);
>> +
>> /**
>> * fpga_image_load_register - create and register an FPGA Image Load Device
>> *
>> @@ -373,6 +405,7 @@ static int __init fpga_image_load_class_init(void)
>> if (ret)
>> goto exit_destroy_class;
>>
>> + fpga_image_load_class->dev_groups = fpga_image_load_groups;
>> fpga_image_load_class->dev_release = fpga_image_load_dev_release;
>>
>> return 0;
>> --
>> 2.25.1
