[PATCH 01/10] uadk/v1/drv: hisi_zip_udrv - fix unmap buffer for lz77_zstd stream mode
In uadk v1, the library will alloc a ctx_buffer used for storing stream information. On hisilicon platform, the hardware will output some data to this buffer. So the ctx_buffer should be map to PA/IOVA. But here the lz77_zstd forgot to unmap this buffer on error branch. What's more, if the user use the wd_bmm ops, the unmap is a void function. Signed-off-by: Yang Shen <shenyang39@huawei.com> --- v1/drv/hisi_zip_udrv.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/v1/drv/hisi_zip_udrv.c b/v1/drv/hisi_zip_udrv.c index f820b7f..470acb9 100644 --- a/v1/drv/hisi_zip_udrv.c +++ b/v1/drv/hisi_zip_udrv.c @@ -512,6 +512,9 @@ static int fill_zip_addr_lz77_zstd(void *ssqe, unmap_phy_seq: drv_iova_unmap(q, zstd_out->literal, (void *)phy_lit, zstd_out->lit_sz); unmap_phy_lit: + if (msg->stream_mode == WCRYPTO_COMP_STATEFUL) + drv_iova_unmap(q, msg->ctx_buf, (void *)addr.ctxbuf_addr - CTX_BUFFER_OFFSET, + MAX_CTX_RSV_SIZE); drv_iova_unmap(q, msg->src, (void *)addr.source_addr, msg->in_size); return -WD_ENOMEM; } -- 2.33.0
Some region is reserved for driver data in ctx_buf. So there is a offset between the hardware ctx_buf and the original ctx_buf. The unmapped address should be the original ctx_buf rather than hardware ctx_buf. Signed-off-by: Yang Shen <shenyang39@huawei.com> --- v1/drv/hisi_zip_udrv.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/v1/drv/hisi_zip_udrv.c b/v1/drv/hisi_zip_udrv.c index 470acb9..dfd1fb3 100644 --- a/v1/drv/hisi_zip_udrv.c +++ b/v1/drv/hisi_zip_udrv.c @@ -126,6 +126,7 @@ static int qm_fill_zip_sqe_get_phy_addr(struct hisi_zip_sqe_addr *addr, WD_ERR("Get zip in buf dma address fail!\n"); return -WD_ENOMEM; } + if (!(is_lz77 && msg->data_fmt == WD_SGL_BUF)) { phy_out = (uintptr_t)drv_iova_map(q, msg->dst, msg->avail_out); if (!phy_out) { @@ -296,8 +297,8 @@ int qm_parse_zip_sqe(void *hw_msg, const struct qm_queue_info *info, phy_out = DMA_ADDR(sqe->dest_addr_h, sqe->dest_addr_l); drv_iova_unmap(q, recv_msg->dst, (void *)phy_out, recv_msg->avail_out); if (recv_msg->ctx_buf) { - phy_ctxbuf = DMA_ADDR(sqe->stream_ctx_addr_h, - sqe->stream_ctx_addr_l); + phy_ctxbuf = DMA_ADDR(sqe->stream_ctx_addr_h, sqe->stream_ctx_addr_l) - + CTX_BUFFER_OFFSET; drv_iova_unmap(q, recv_msg->ctx_buf, (void *)phy_ctxbuf, MAX_CTX_RSV_SIZE); } @@ -727,8 +728,8 @@ int qm_parse_zip_sqe_v3(void *hw_msg, const struct qm_queue_info *info, phy_out = DMA_ADDR(sqe->dest_addr_h, sqe->dest_addr_l); drv_iova_unmap(q, recv_msg->dst, (void *)phy_out, recv_msg->avail_out); if (recv_msg->ctx_buf) { - phy_ctxbuf = DMA_ADDR(sqe->stream_ctx_addr_h, - sqe->stream_ctx_addr_l); + phy_ctxbuf = DMA_ADDR(sqe->stream_ctx_addr_h, sqe->stream_ctx_addr_l) - + CTX_BUFFER_OFFSET; drv_iova_unmap(q, recv_msg->ctx_buf, (void *)phy_ctxbuf, MAX_CTX_RSV_SIZE); } -- 2.33.0
Move the resource release into lock. Signed-off-by: Yang Shen <shenyang39@huawei.com> --- v1/wd_comp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v1/wd_comp.c b/v1/wd_comp.c index f898c1d..fb9e413 100644 --- a/v1/wd_comp.c +++ b/v1/wd_comp.c @@ -192,8 +192,8 @@ free_ctx_buf: free(ctx); free_ctx_id: wd_free_id(qinfo->ctx_id, WD_MAX_CTX_NUM, ctx_id, WD_MAX_CTX_NUM); - wd_spinlock(&qinfo->qlock); qinfo->ctx_num--; + wd_spinlock(&qinfo->qlock); unlock: wd_unspinlock(&qinfo->qlock); return NULL; -- 2.33.0
Add checks for some necessary parameters of dynamically registered drivers. Signed-off-by: Yang Shen <shenyang39@huawei.com> --- wd_alg.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/wd_alg.c b/wd_alg.c index 8d88316..3b111c8 100644 --- a/wd_alg.c +++ b/wd_alg.c @@ -139,6 +139,11 @@ int wd_alg_driver_register(struct wd_alg_driver *drv) return -WD_EINVAL; } + if (!drv->init || !drv->exit || !drv->send || !drv->recv) { + WD_ERR("invalid: driver's parameter is NULL!\n"); + return -WD_EINVAL; + } + new_alg = calloc(1, sizeof(struct wd_alg_list)); if (!new_alg) { WD_ERR("failed to alloc alg driver memory!\n"); -- 2.33.0
Update some comments. Signed-off-by: Yang Shen <shenyang39@huawei.com> --- include/wd_comp.h | 20 ++++++++++---------- wd_comp.c | 3 +-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/include/wd_comp.h b/include/wd_comp.h index 45b0d0b..45994ff 100644 --- a/include/wd_comp.h +++ b/include/wd_comp.h @@ -39,12 +39,12 @@ enum wd_comp_level { WD_COMP_L7, /* Compression level 7 */ WD_COMP_L8, /* Compression level 8 */ WD_COMP_L9, /* Compression level 9 */ - WD_COMP_L10, /* Compression level 10 */ - WD_COMP_L11, /* Compression level 11 */ - WD_COMP_L12, /* Compression level 12 */ - WD_COMP_L13, /* Compression level 13 */ - WD_COMP_L14, /* Compression level 14 */ - WD_COMP_L15, /* Compression level 15 */ + WD_COMP_L10, /* Compression level 10 */ + WD_COMP_L11, /* Compression level 11 */ + WD_COMP_L12, /* Compression level 12 */ + WD_COMP_L13, /* Compression level 13 */ + WD_COMP_L14, /* Compression level 14 */ + WD_COMP_L15, /* Compression level 15 */ }; enum wd_comp_winsz_type { @@ -72,10 +72,10 @@ struct wd_comp_req { __u32 dst_len; wd_alg_comp_cb_t *cb; void *cb_param; - enum wd_comp_op_type op_type; /* Denoted by wd_comp_op_type */ - enum wd_buff_type data_fmt; /* Denoted by wd_buff_type */ - __u32 last; - __u32 status; + enum wd_comp_op_type op_type; /* denoted by wd_comp_op_type */ + enum wd_buff_type data_fmt; /* denoted by wd_buff_type */ + __u32 last; /* flag of last block for stream mode */ + __u32 status; /* request error code */ void *priv; }; diff --git a/wd_comp.c b/wd_comp.c index 21c9928..6e71d35 100644 --- a/wd_comp.c +++ b/wd_comp.c @@ -494,7 +494,6 @@ static void fill_comp_msg(struct wd_comp_sess *sess, struct wd_comp_msg *msg, msg->win_sz = sess->win_sz; msg->avail_out = req->dst_len; - /* if is last 1: flush end; other: sync flush */ msg->req.last = 1; } @@ -722,7 +721,7 @@ static int append_store_block(struct wd_comp_sess *sess, memcpy(req->dst, store_block, blocksize); req->dst_len = blocksize; checksum = (__u32) cpu_to_be32(checksum); - /* if zlib, ADLER32 */ + /* if zlib, ADLER32 */ memcpy(req->dst + blocksize, &checksum, sizeof(checksum)); req->dst_len += sizeof(checksum); } else if (sess->alg_type == WD_GZIP) { -- 2.33.0
From: Weili Qian <qianweili@huawei.com> When request ctx busy, the ctx needs to be applied again instead of performing subsequent operations. In addition, add ctx checks, if ctx is 0, return -WD_ENOMEM. Signed-off-by: Weili Qian <qianweili@huawei.com> --- wd_util.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/wd_util.c b/wd_util.c index a9640c3..8af0261 100644 --- a/wd_util.c +++ b/wd_util.c @@ -2418,6 +2418,7 @@ static int wd_init_ctx_set(struct wd_init_attrs *attrs, struct uacce_dev_list *l struct wd_ctx_nums ctx_nums = attrs->ctx_params->ctx_set_num[op_type]; __u32 ctx_set_num = ctx_nums.sync_ctx_num + ctx_nums.async_ctx_num; struct wd_ctx_config *ctx_config = attrs->ctx_config; + __u32 count = idx + ctx_set_num; struct uacce_dev *dev; __u32 i; @@ -2429,13 +2430,21 @@ static int wd_init_ctx_set(struct wd_init_attrs *attrs, struct uacce_dev_list *l if (WD_IS_ERR(dev)) return WD_PTR_ERR(dev); - for (i = idx; i < idx + ctx_set_num; i++) { + for (i = idx; i < count; i++) { ctx_config->ctxs[i].ctx = wd_request_ctx(dev); if (errno == WD_EBUSY) { dev = wd_find_dev_by_numa(list, numa_id); if (WD_IS_ERR(dev)) return WD_PTR_ERR(dev); + i--; + continue; + } else if (!ctx_config->ctxs[i].ctx) { + /* + * wd_release_ctx_set will release ctx in + * caller wd_init_ctx_and_sched. + */ + return -WD_ENOMEM; } ctx_config->ctxs[i].op_type = op_type; ctx_config->ctxs[i].ctx_mode = -- 2.33.0
In wd_init_ctx_set, it will try to get the ctx from the most idle device. But requesting ctx from devices is an exclusive behavior. And there is no lock to protect against multi-process contention. In the worst situation, a process will always get 'EBUSY' from wd_request_ctx. So here add a retry count to avoid a infinite loop. Signed-off-by: Yang Shen <shenyang39@huawei.com> --- wd_util.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/wd_util.c b/wd_util.c index 8af0261..bfa3af0 100644 --- a/wd_util.c +++ b/wd_util.c @@ -2420,7 +2420,7 @@ static int wd_init_ctx_set(struct wd_init_attrs *attrs, struct uacce_dev_list *l struct wd_ctx_config *ctx_config = attrs->ctx_config; __u32 count = idx + ctx_set_num; struct uacce_dev *dev; - __u32 i; + __u32 i, cnt = 0; /* If the ctx set number is 0, the initialization is skipped. */ if (!ctx_set_num) @@ -2437,6 +2437,12 @@ static int wd_init_ctx_set(struct wd_init_attrs *attrs, struct uacce_dev_list *l if (WD_IS_ERR(dev)) return WD_PTR_ERR(dev); + if (cnt++ > WD_INIT_RETRY_TIMES) { + WD_ERR("failed to request enough ctx due to timeout!\n"); + return -WD_ETIMEDOUT; + } + + /* self-decrease i to eliminate self-increase on next loop */ i--; continue; } else if (!ctx_config->ctxs[i].ctx) { -- 2.33.0
UADK supports registration of log to the rsyslog service. So it will read the rsyslog.conf to user's log level. Here use '%[^\n ] ' to divided the lines in file into words to analysis. But there is a problem if the first line in the file is '\n'. The fscanf will return 0 and always read the first line, which makes a infinite loop. Add a whitespace before '%[^\n ] ' to filter the unintended '\n'. Signed-off-by: Yang Shen <shenyang39@huawei.com> --- wd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wd.c b/wd.c index ddde38d..e88c993 100644 --- a/wd.c +++ b/wd.c @@ -79,7 +79,7 @@ static void wd_parse_log_level(void) goto close_file; } - while (fscanf(in_file, "%[^\n ] ", file_contents) != EOF) { + while (fscanf(in_file, " %[^\n ] ", file_contents) != EOF) { if (!strcmp("local5.debug", file_contents)) log_debug = true; else if (!strcmp("local5.info", file_contents)) -- 2.33.0
Add the check on req.status to show users invalid requests. Signed-off-by: Yang Shen <shenyang39@huawei.com> --- wd_zlibwrapper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wd_zlibwrapper.c b/wd_zlibwrapper.c index 7189b7f..afa8ee7 100644 --- a/wd_zlibwrapper.c +++ b/wd_zlibwrapper.c @@ -236,8 +236,8 @@ static int wd_zlib_do_request(z_streamp strm, int flush, enum wd_comp_op_type ty req.last = (flush == Z_FINISH) ? 1 : 0; ret = wd_do_comp_strm(h_sess, &req); - if (unlikely(ret)) { - WD_ERR("failed to do compress(%d)!\n", ret); + if (unlikely(ret || req.status == WD_IN_EPARA)) { + WD_ERR("failed to do compress, ret = %d, req.status = %u!\n", ret, req.status); return Z_STREAM_ERROR; } -- 2.33.0
The HiSilicon drivers support copy data between the pbuffer and sgl. And it supports copying data from the specified domain of sgl. There is a bug when the driver calculate the sgl address. It adds the next sgl node length rather than the origin one. So if the sgl lengths are not equal, the target node will be wrong. Therefore, the driver should add the origin sgl length and then fetch the next node. Signed-off-by: Yang Shen <shenyang39@huawei.com> --- drv/hisi_qm_udrv.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drv/hisi_qm_udrv.c b/drv/hisi_qm_udrv.c index d86a692..13db3f0 100644 --- a/drv/hisi_qm_udrv.c +++ b/drv/hisi_qm_udrv.c @@ -972,20 +972,19 @@ void hisi_qm_sgl_copy(void *pbuff, void *hw_sgl, __u32 offset, __u32 size, __u8 direct) { struct hisi_sgl *tmp = hw_sgl; + int begin_sge = 0, i; __u32 sge_offset = 0; __u32 len = 0; - int begin_sge = 0; - int i; if (!pbuff || !size || !tmp) return; while (len + tmp->entry_size_in_sgl <= offset) { + len += tmp->entry_size_in_sgl; + tmp = (struct hisi_sgl *)tmp->next_dma; if (!tmp) return; - - len += tmp->entry_size_in_sgl; } /* find the start sge position and start offset */ -- 2.33.0
participants (1)
-
Yang Shen