| Message ID | 20220708070820.43958-2-yekai13@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: hisilicon - supports device isolation feature | expand |
On Fri, Jul 08, 2022 at 03:08:18PM +0800, Kai Ye wrote: > UACCE adds the hardware error isolation API. Users can configure > the isolation frequency by this sysfs node. UACCE reports the device > isolate state to the user space. If the AER error frequency exceeds > the value of setting for a certain period of time, the device will be > isolated. > > Signed-off-by: Kai Ye <yekai13@huawei.com> > --- > drivers/misc/uacce/uacce.c | 55 ++++++++++++++++++++++++++++++++++++++ > include/linux/uacce.h | 11 ++++++++ > 2 files changed, 66 insertions(+) > > diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c > index 281c54003edc..d07b5f1f0596 100644 > --- a/drivers/misc/uacce/uacce.c > +++ b/drivers/misc/uacce/uacce.c > @@ -7,6 +7,8 @@ > #include <linux/slab.h> > #include <linux/uacce.h> > > +#define MAX_ERR_ISOLATE_COUNT 65535 > + > static struct class *uacce_class; > static dev_t uacce_devt; > static DEFINE_MUTEX(uacce_mutex); > @@ -339,12 +341,63 @@ static ssize_t region_dus_size_show(struct device *dev, > uacce->qf_pg_num[UACCE_QFRT_DUS] << PAGE_SHIFT); > } > > +static ssize_t isolate_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct uacce_device *uacce = to_uacce_device(dev); > + > + if (!uacce->ops->get_isolate_state) > + return -ENODEV; If there is no callback, why is this sysfs even created at all? Please do not create it if it can not be accessed. Use the is_visable() callback for the group to do this. > + > + return sysfs_emit(buf, "%d\n", uacce->ops->get_isolate_state(uacce)); > +} > + > +static ssize_t isolate_strategy_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct uacce_device *uacce = to_uacce_device(dev); > + u32 val; > + > + if (!uacce->ops->isolate_strategy_read) > + return -ENODEV; Same here, don't have a sysfs file that does nothing. > + > + val = uacce->ops->isolate_strategy_read(uacce); > + if (val > MAX_ERR_ISOLATE_COUNT) > + return -EINVAL; > + > + return sysfs_emit(buf, "%u\n", val); > +} > + > +static ssize_t isolate_strategy_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct uacce_device *uacce = to_uacce_device(dev); > + unsigned long val; > + int ret; > + > + if (!uacce->ops->isolate_strategy_write) > + return -ENODEV; Same here. thanks, greg k-h
On 2022/7/8 15:28, Greg KH wrote: > On Fri, Jul 08, 2022 at 03:08:18PM +0800, Kai Ye wrote: >> UACCE adds the hardware error isolation API. Users can configure >> the isolation frequency by this sysfs node. UACCE reports the device >> isolate state to the user space. If the AER error frequency exceeds >> the value of setting for a certain period of time, the device will be >> isolated. >> >> Signed-off-by: Kai Ye <yekai13@huawei.com> >> --- >> drivers/misc/uacce/uacce.c | 55 ++++++++++++++++++++++++++++++++++++++ >> include/linux/uacce.h | 11 ++++++++ >> 2 files changed, 66 insertions(+) >> >> diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c >> index 281c54003edc..d07b5f1f0596 100644 >> --- a/drivers/misc/uacce/uacce.c >> +++ b/drivers/misc/uacce/uacce.c >> @@ -7,6 +7,8 @@ >> #include <linux/slab.h> >> #include <linux/uacce.h> >> >> +#define MAX_ERR_ISOLATE_COUNT 65535 >> + >> static struct class *uacce_class; >> static dev_t uacce_devt; >> static DEFINE_MUTEX(uacce_mutex); >> @@ -339,12 +341,63 @@ static ssize_t region_dus_size_show(struct device *dev, >> uacce->qf_pg_num[UACCE_QFRT_DUS] << PAGE_SHIFT); >> } >> >> +static ssize_t isolate_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct uacce_device *uacce = to_uacce_device(dev); >> + >> + if (!uacce->ops->get_isolate_state) >> + return -ENODEV; > > If there is no callback, why is this sysfs even created at all? Please > do not create it if it can not be accessed. > > Use the is_visable() callback for the group to do this. > If is_visable() is used as the judgment, all uacce device nodes cannot be registered if there is no callback by test. >> + >> + return sysfs_emit(buf, "%d\n", uacce->ops->get_isolate_state(uacce)); >> +} >> + >> +static ssize_t isolate_strategy_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct uacce_device *uacce = to_uacce_device(dev); >> + u32 val; >> + >> + if (!uacce->ops->isolate_strategy_read) >> + return -ENODEV; > > Same here, don't have a sysfs file that does nothing. > >> + >> + val = uacce->ops->isolate_strategy_read(uacce); >> + if (val > MAX_ERR_ISOLATE_COUNT) >> + return -EINVAL; >> + >> + return sysfs_emit(buf, "%u\n", val); >> +} >> + >> +static ssize_t isolate_strategy_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct uacce_device *uacce = to_uacce_device(dev); >> + unsigned long val; >> + int ret; >> + >> + if (!uacce->ops->isolate_strategy_write) >> + return -ENODEV; > > Same here. > > thanks, > > greg k-h > . >
On Fri, Jul 08, 2022 at 05:33:42PM +0800, yekai(A) wrote: > > > On 2022/7/8 15:28, Greg KH wrote: > > On Fri, Jul 08, 2022 at 03:08:18PM +0800, Kai Ye wrote: > > > UACCE adds the hardware error isolation API. Users can configure > > > the isolation frequency by this sysfs node. UACCE reports the device > > > isolate state to the user space. If the AER error frequency exceeds > > > the value of setting for a certain period of time, the device will be > > > isolated. > > > > > > Signed-off-by: Kai Ye <yekai13@huawei.com> > > > --- > > > drivers/misc/uacce/uacce.c | 55 ++++++++++++++++++++++++++++++++++++++ > > > include/linux/uacce.h | 11 ++++++++ > > > 2 files changed, 66 insertions(+) > > > > > > diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c > > > index 281c54003edc..d07b5f1f0596 100644 > > > --- a/drivers/misc/uacce/uacce.c > > > +++ b/drivers/misc/uacce/uacce.c > > > @@ -7,6 +7,8 @@ > > > #include <linux/slab.h> > > > #include <linux/uacce.h> > > > > > > +#define MAX_ERR_ISOLATE_COUNT 65535 > > > + > > > static struct class *uacce_class; > > > static dev_t uacce_devt; > > > static DEFINE_MUTEX(uacce_mutex); > > > @@ -339,12 +341,63 @@ static ssize_t region_dus_size_show(struct device *dev, > > > uacce->qf_pg_num[UACCE_QFRT_DUS] << PAGE_SHIFT); > > > } > > > > > > +static ssize_t isolate_show(struct device *dev, > > > + struct device_attribute *attr, char *buf) > > > +{ > > > + struct uacce_device *uacce = to_uacce_device(dev); > > > + > > > + if (!uacce->ops->get_isolate_state) > > > + return -ENODEV; > > > > If there is no callback, why is this sysfs even created at all? Please > > do not create it if it can not be accessed. > > > > Use the is_visable() callback for the group to do this. > > > > If is_visable() is used as the judgment, all uacce device nodes cannot be > registered if there is no callback by test. I am sorry, I do not understand. That callback is to be used for this type of thing, and works for lots of driver subsystems. Why exactly will it not work here as well? thanks, greg k-h
diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c index 281c54003edc..d07b5f1f0596 100644 --- a/drivers/misc/uacce/uacce.c +++ b/drivers/misc/uacce/uacce.c @@ -7,6 +7,8 @@ #include <linux/slab.h> #include <linux/uacce.h> +#define MAX_ERR_ISOLATE_COUNT 65535 + static struct class *uacce_class; static dev_t uacce_devt; static DEFINE_MUTEX(uacce_mutex); @@ -339,12 +341,63 @@ static ssize_t region_dus_size_show(struct device *dev, uacce->qf_pg_num[UACCE_QFRT_DUS] << PAGE_SHIFT); } +static ssize_t isolate_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct uacce_device *uacce = to_uacce_device(dev); + + if (!uacce->ops->get_isolate_state) + return -ENODEV; + + return sysfs_emit(buf, "%d\n", uacce->ops->get_isolate_state(uacce)); +} + +static ssize_t isolate_strategy_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct uacce_device *uacce = to_uacce_device(dev); + u32 val; + + if (!uacce->ops->isolate_strategy_read) + return -ENODEV; + + val = uacce->ops->isolate_strategy_read(uacce); + if (val > MAX_ERR_ISOLATE_COUNT) + return -EINVAL; + + return sysfs_emit(buf, "%u\n", val); +} + +static ssize_t isolate_strategy_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct uacce_device *uacce = to_uacce_device(dev); + unsigned long val; + int ret; + + if (!uacce->ops->isolate_strategy_write) + return -ENODEV; + + if (kstrtoul(buf, 0, &val) < 0) + return -EINVAL; + + if (val > MAX_ERR_ISOLATE_COUNT) + return -EINVAL; + + ret = uacce->ops->isolate_strategy_write(uacce, val); + + return ret ? ret : count; +} + static DEVICE_ATTR_RO(api); static DEVICE_ATTR_RO(flags); static DEVICE_ATTR_RO(available_instances); static DEVICE_ATTR_RO(algorithms); static DEVICE_ATTR_RO(region_mmio_size); static DEVICE_ATTR_RO(region_dus_size); +static DEVICE_ATTR_RO(isolate); +static DEVICE_ATTR_RW(isolate_strategy); static struct attribute *uacce_dev_attrs[] = { &dev_attr_api.attr, @@ -353,6 +406,8 @@ static struct attribute *uacce_dev_attrs[] = { &dev_attr_algorithms.attr, &dev_attr_region_mmio_size.attr, &dev_attr_region_dus_size.attr, + &dev_attr_isolate.attr, + &dev_attr_isolate_strategy.attr, NULL, }; diff --git a/include/linux/uacce.h b/include/linux/uacce.h index 48e319f40275..69e8f238d80c 100644 --- a/include/linux/uacce.h +++ b/include/linux/uacce.h @@ -30,6 +30,9 @@ struct uacce_qfile_region { * @is_q_updated: check whether the task is finished * @mmap: mmap addresses of queue to user space * @ioctl: ioctl for user space users of the queue + * @get_isolate_state: get the device state after set the isolate strategy + * @isolate_strategy_write: stored the isolate strategy to the device + * @isolate_strategy_read: read the isolate strategy value from the device */ struct uacce_ops { int (*get_available_instances)(struct uacce_device *uacce); @@ -43,6 +46,9 @@ struct uacce_ops { struct uacce_qfile_region *qfr); long (*ioctl)(struct uacce_queue *q, unsigned int cmd, unsigned long arg); + enum uacce_dev_state (*get_isolate_state)(struct uacce_device *uacce); + int (*isolate_strategy_write)(struct uacce_device *uacce, u32 freq); + u32 (*isolate_strategy_read)(struct uacce_device *uacce); }; /** @@ -57,6 +63,11 @@ struct uacce_interface { const struct uacce_ops *ops; }; +enum uacce_dev_state { + UACCE_DEV_NORMAL, + UACCE_DEV_ISOLATE, +}; + enum uacce_q_state { UACCE_Q_ZOMBIE = 0, UACCE_Q_INIT,
UACCE adds the hardware error isolation API. Users can configure the isolation frequency by this sysfs node. UACCE reports the device isolate state to the user space. If the AER error frequency exceeds the value of setting for a certain period of time, the device will be isolated. Signed-off-by: Kai Ye <yekai13@huawei.com> --- drivers/misc/uacce/uacce.c | 55 ++++++++++++++++++++++++++++++++++++++ include/linux/uacce.h | 11 ++++++++ 2 files changed, 66 insertions(+)