| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4aee9c31-1fd0-609a-00d5-198f092051a8@linaro.org>
Date: Mon, 13 Jun 2022 17:17:20 +0800
From: Zhangfei Gao <zhangfei.gao@...aro.org>
To: Kai Ye <yekai13@...wei.com>, gregkh@...uxfoundation.org,
linux-accelerators@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
linuxarm@...wei.com, wangzhou1@...ilicon.com
Subject: Re: [PATCH 1/2] uacce: supports device isolation feature
On 2022/6/11 下午3:08, Kai Ye via Linux-accelerators wrote:
> UACCE add the hardware error isolation API. Users can configure
> the error frequency threshold by this vfs node. This API interface
> certainly supports the configuration of user protocol strategy. Then
> parse it inside the device driver. UACCE only reports the device
> isolate state. When the error frequency is exceeded, the device
> will be isolated. The isolation strategy should be defined in each
> driver module.
>
> Signed-off-by: Kai Ye <yekai13@...wei.com>
> Reviewed-by: Zhou Wang <wangzhou1@...ilicon.com>
> ---
> drivers/misc/uacce/uacce.c | 76 ++++++++++++++++++++++++++++++++++++++
> include/linux/uacce.h | 21 ++++++++++-
> 2 files changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
> index b6219c6bfb48..26c07f6f2f7c 100644
> --- a/drivers/misc/uacce/uacce.c
> +++ b/drivers/misc/uacce/uacce.c
> @@ -12,6 +12,38 @@ static dev_t uacce_devt;
> static DEFINE_MUTEX(uacce_mutex);
> static DEFINE_XARRAY_ALLOC(uacce_xa);
>
> +static int cdev_get(struct device *dev, void *data)
> +{
> + struct uacce_device *uacce;
> + struct device **t_dev = data;
> +
> + uacce = container_of(dev, struct uacce_device, dev);
> + if (uacce->parent == *t_dev) {
> + *t_dev = dev;
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * dev_to_uacce - Get structure uacce device from its parent device
> + * @dev: the device
> + */
> +struct uacce_device *dev_to_uacce(struct device *dev)
> +{
> + struct device **tdev = &dev;
> + int ret;
> +
> + ret = class_for_each_device(uacce_class, NULL, tdev, cdev_get);
> + if (ret) {
> + dev = *tdev;
> + return container_of(dev, struct uacce_device, dev);
> + }
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(dev_to_uacce);
Not find dev_to_uacce user.
> +
> static int uacce_start_queue(struct uacce_queue *q)
> {
> int ret = 0;
> @@ -54,6 +86,7 @@ static int uacce_put_queue(struct uacce_queue *q)
> uacce->ops->put_queue(q);
>
> q->state = UACCE_Q_ZOMBIE;
> + atomic_dec(&uacce->ref);
> out:
> mutex_unlock(&uacce_mutex);
>
> @@ -155,6 +188,7 @@ static int uacce_fops_open(struct inode *inode, struct file *filep)
> goto out_with_bond;
> }
>
> + atomic_inc(&uacce->ref);
Use uacce->ref is strange here, also no dec in release.
How about hidden this in driver itself, like via qm->state, to make
uacce itslef clean.
> init_waitqueue_head(&q->wait);
> filep->private_data = q;
> uacce->inode = inode;
> @@ -346,12 +380,52 @@ 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);
> +
> + 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);
> +
> + return sysfs_emit(buf, "%s\n", uacce->isolate_strategy);
> +}
> +
> +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);
> + int ret;
> +
> + if (atomic_read(&uacce->ref))
> + return -EBUSY;
> +
> + if (!buf || sizeof(buf) > UACCE_MAX_ISOLATE_STRATEGY_LEN)
> + return -EINVAL;
> +
> + memcpy(uacce->isolate_strategy, buf, strlen(buf));
> +
> + ret = uacce->ops->isolate_strategy_write(uacce, buf);
> + if (ret)
> + return -EINVAL;
> +
> + return 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);
Need update Documentation/ABI/testing/sysfs-driver-uacce as well.
isolate_strategy is confusing, what strategy can be chosen, need doc.
Thanks
>
> static struct attribute *uacce_dev_attrs[] = {
> &dev_attr_api.attr,
> @@ -360,6 +434,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..30b1595b3f85 100644
> --- a/include/linux/uacce.h
> +++ b/include/linux/uacce.h
> @@ -8,6 +8,7 @@
> #define UACCE_NAME "uacce"
> #define UACCE_MAX_REGION 2
> #define UACCE_MAX_NAME_SIZE 64
> +#define UACCE_MAX_ISOLATE_STRATEGY_LEN 256
>
> struct uacce_queue;
> struct uacce_device;
> @@ -30,6 +31,8 @@ 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_store: stored the isolate strategy to the device
> */
> struct uacce_ops {
> int (*get_available_instances)(struct uacce_device *uacce);
> @@ -43,6 +46,8 @@ 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, const char *buf);
> };
>
> /**
> @@ -57,6 +62,12 @@ struct uacce_interface {
> const struct uacce_ops *ops;
> };
>
> +enum uacce_dev_state {
> + UACCE_DEV_ERR = -1,
> + UACCE_DEV_NORMAL,
> + UACCE_DEV_ISOLATE,
> +};
> +
> enum uacce_q_state {
> UACCE_Q_ZOMBIE = 0,
> UACCE_Q_INIT,
> @@ -99,6 +110,7 @@ struct uacce_queue {
> * @dev: dev of the uacce
> * @priv: private pointer of the uacce
> * @queues: list of queues
> + * @ref: reference of the uacce
> * @queues_lock: lock for queues list
> * @inode: core vfs
> */
> @@ -114,9 +126,11 @@ struct uacce_device {
> struct cdev *cdev;
> struct device dev;
> void *priv;
> + atomic_t ref;
> struct list_head queues;
> struct mutex queues_lock;
> struct inode *inode;
> + char isolate_strategy[UACCE_MAX_ISOLATE_STRATEGY_LEN];
> };
>
> #if IS_ENABLED(CONFIG_UACCE)
> @@ -125,7 +139,7 @@ struct uacce_device *uacce_alloc(struct device *parent,
> struct uacce_interface *interface);
> int uacce_register(struct uacce_device *uacce);
> void uacce_remove(struct uacce_device *uacce);
> -
> +struct uacce_device *dev_to_uacce(struct device *dev);
> #else /* CONFIG_UACCE */
>
> static inline
> @@ -142,6 +156,11 @@ static inline int uacce_register(struct uacce_device *uacce)
>
> static inline void uacce_remove(struct uacce_device *uacce) {}
>
> +static inline struct uacce_device *dev_to_uacce(struct device *dev)
> +{
> + return NULL;
> +}
> +
> #endif /* CONFIG_UACCE */
>
> #endif /* _LINUX_UACCE_H */
Powered by blists - more mailing lists