| Message ID | 20200904235305.6254-12-russell.h.weight@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Intel FPGA Security Manager Class Driver | expand |
On 9/4/20 4:53 PM, Russ Weight wrote: > Extend the Intel 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 <russell.h.weight@intel.com> > Reviewed-by: Wu Hao <hao.wu@intel.com> > --- > .../ABI/testing/sysfs-class-ifpga-sec-mgr | 14 +++++++ > drivers/fpga/ifpga-sec-mgr.c | 38 +++++++++++++++++++ > include/linux/fpga/ifpga-sec-mgr.h | 5 +++ > 3 files changed, 57 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-class-ifpga-sec-mgr b/Documentation/ABI/testing/sysfs-class-ifpga-sec-mgr > index 762a7dee9453..20bde1abb5e4 100644 > --- a/Documentation/ABI/testing/sysfs-class-ifpga-sec-mgr > +++ b/Documentation/ABI/testing/sysfs-class-ifpga-sec-mgr > @@ -135,3 +135,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/ifpga_sec_mgr/ifpga_secX/update/hw_errinfo > +Date: Sep 2020 > +KernelVersion: 5.10 > +Contact: Russ Weight <russell.h.weight@intel.com> > +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/ifpga-sec-mgr.c b/drivers/fpga/ifpga-sec-mgr.c > index afd97c135ebe..6944396eff80 100644 > --- a/drivers/fpga/ifpga-sec-mgr.c > +++ b/drivers/fpga/ifpga-sec-mgr.c > @@ -152,10 +152,17 @@ static void set_error(struct ifpga_sec_mgr *imgr, enum ifpga_sec_err err_code) > imgr->err_code = err_code; > } > > +static void set_hw_errinfo(struct ifpga_sec_mgr *imgr) > +{ > + if (imgr->iops->get_hw_errinfo) > + imgr->hw_errinfo = imgr->iops->get_hw_errinfo(imgr); > +} > + > static void ifpga_sec_dev_error(struct ifpga_sec_mgr *imgr, > enum ifpga_sec_err err_code) > { > set_error(imgr, err_code); > + set_hw_errinfo(imgr); > imgr->iops->cancel(imgr); > } > > @@ -348,6 +355,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 ifpga_sec_mgr *imgr = to_sec_mgr(dev); > + int ret; > + > + mutex_lock(&imgr->lock); > + if (imgr->progress != IFPGA_SEC_PROG_IDLE) > + ret = -EBUSY; > + else > + ret = sprintf(buf, "0x%llx\n", imgr->hw_errinfo); > + mutex_unlock(&imgr->lock); > + > + return ret; > +} > +static DEVICE_ATTR_RO(hw_errinfo); > + > static ssize_t remaining_size_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > @@ -382,6 +406,7 @@ static ssize_t filename_store(struct device *dev, struct device_attribute *attr, > imgr->filename[strlen(imgr->filename) - 1] = '\0'; > > imgr->err_code = IFPGA_SEC_ERR_NONE; > + imgr->hw_errinfo = 0; > imgr->request_cancel = false; > imgr->progress = IFPGA_SEC_PROG_READ_FILE; > reinit_completion(&imgr->update_done); > @@ -416,18 +441,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 ifpga_sec_mgr *imgr = to_sec_mgr(kobj_to_dev(kobj)); > + > + if (attr == &dev_attr_hw_errinfo.attr && !imgr->iops->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/ifpga-sec-mgr.h b/include/linux/fpga/ifpga-sec-mgr.h > index f51ed663a723..3be8d8da078a 100644 > --- a/include/linux/fpga/ifpga-sec-mgr.h > +++ b/include/linux/fpga/ifpga-sec-mgr.h > @@ -135,6 +135,9 @@ enum ifpga_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 ifpga_sec_mgr_ops { > sysfs_cnt_hndlr_t user_flash_count; > @@ -158,6 +161,7 @@ struct ifpga_sec_mgr_ops { > enum ifpga_sec_err (*poll_complete)(struct ifpga_sec_mgr *imgr); > void (*cleanup)(struct ifpga_sec_mgr *imgr); > enum ifpga_sec_err (*cancel)(struct ifpga_sec_mgr *imgr); > + u64 (*get_hw_errinfo)(struct ifpga_sec_mgr *imgr); > }; > > /* Update progress codes */ > @@ -183,6 +187,7 @@ struct ifpga_sec_mgr { > enum ifpga_sec_prog progress; > enum ifpga_sec_prog err_state; /* progress state at time of failure */ > enum ifpga_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; This looks fine. Reviewed-by: Tom Rix <trix@redhat.com>
diff --git a/Documentation/ABI/testing/sysfs-class-ifpga-sec-mgr b/Documentation/ABI/testing/sysfs-class-ifpga-sec-mgr index 762a7dee9453..20bde1abb5e4 100644 --- a/Documentation/ABI/testing/sysfs-class-ifpga-sec-mgr +++ b/Documentation/ABI/testing/sysfs-class-ifpga-sec-mgr @@ -135,3 +135,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/ifpga_sec_mgr/ifpga_secX/update/hw_errinfo +Date: Sep 2020 +KernelVersion: 5.10 +Contact: Russ Weight <russell.h.weight@intel.com> +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/ifpga-sec-mgr.c b/drivers/fpga/ifpga-sec-mgr.c index afd97c135ebe..6944396eff80 100644 --- a/drivers/fpga/ifpga-sec-mgr.c +++ b/drivers/fpga/ifpga-sec-mgr.c @@ -152,10 +152,17 @@ static void set_error(struct ifpga_sec_mgr *imgr, enum ifpga_sec_err err_code) imgr->err_code = err_code; } +static void set_hw_errinfo(struct ifpga_sec_mgr *imgr) +{ + if (imgr->iops->get_hw_errinfo) + imgr->hw_errinfo = imgr->iops->get_hw_errinfo(imgr); +} + static void ifpga_sec_dev_error(struct ifpga_sec_mgr *imgr, enum ifpga_sec_err err_code) { set_error(imgr, err_code); + set_hw_errinfo(imgr); imgr->iops->cancel(imgr); } @@ -348,6 +355,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 ifpga_sec_mgr *imgr = to_sec_mgr(dev); + int ret; + + mutex_lock(&imgr->lock); + if (imgr->progress != IFPGA_SEC_PROG_IDLE) + ret = -EBUSY; + else + ret = sprintf(buf, "0x%llx\n", imgr->hw_errinfo); + mutex_unlock(&imgr->lock); + + return ret; +} +static DEVICE_ATTR_RO(hw_errinfo); + static ssize_t remaining_size_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -382,6 +406,7 @@ static ssize_t filename_store(struct device *dev, struct device_attribute *attr, imgr->filename[strlen(imgr->filename) - 1] = '\0'; imgr->err_code = IFPGA_SEC_ERR_NONE; + imgr->hw_errinfo = 0; imgr->request_cancel = false; imgr->progress = IFPGA_SEC_PROG_READ_FILE; reinit_completion(&imgr->update_done); @@ -416,18 +441,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 ifpga_sec_mgr *imgr = to_sec_mgr(kobj_to_dev(kobj)); + + if (attr == &dev_attr_hw_errinfo.attr && !imgr->iops->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/ifpga-sec-mgr.h b/include/linux/fpga/ifpga-sec-mgr.h index f51ed663a723..3be8d8da078a 100644 --- a/include/linux/fpga/ifpga-sec-mgr.h +++ b/include/linux/fpga/ifpga-sec-mgr.h @@ -135,6 +135,9 @@ enum ifpga_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 ifpga_sec_mgr_ops { sysfs_cnt_hndlr_t user_flash_count; @@ -158,6 +161,7 @@ struct ifpga_sec_mgr_ops { enum ifpga_sec_err (*poll_complete)(struct ifpga_sec_mgr *imgr); void (*cleanup)(struct ifpga_sec_mgr *imgr); enum ifpga_sec_err (*cancel)(struct ifpga_sec_mgr *imgr); + u64 (*get_hw_errinfo)(struct ifpga_sec_mgr *imgr); }; /* Update progress codes */ @@ -183,6 +187,7 @@ struct ifpga_sec_mgr { enum ifpga_sec_prog progress; enum ifpga_sec_prog err_state; /* progress state at time of failure */ enum ifpga_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;