RE: [EXTERNAL] [PATCH v22 07/13] compress/zsda: add zsda compressdev dev ops

[Akhil Goyal <gakhil@marvell.com>]

> add zsda compressdev dev interface implementation.
> 
> Signed-off-by: Hanxiao Li <li.hanxiao@zte.com.cn>
> ---
>  drivers/common/zsda/meson.build       |   1 +
>  drivers/common/zsda/zsda_qp_common.c  |  57 ++++++++++
>  drivers/common/zsda/zsda_qp_common.h  |  36 +++++++
>  drivers/compress/zsda/zsda_comp_pmd.c | 144 +++++++++++++++++++++++++-
>  drivers/compress/zsda/zsda_comp_pmd.h |   5 +
>  5 files changed, 238 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/common/zsda/zsda_qp_common.c
> 
> diff --git a/drivers/common/zsda/meson.build
> b/drivers/common/zsda/meson.build
> index 6ee2a68f4b..6e6d5ab006 100644
> --- a/drivers/common/zsda/meson.build
> +++ b/drivers/common/zsda/meson.build
> @@ -12,6 +12,7 @@ sources += files(
>          'zsda_device.c',
>          'zsda_logs.c',
>          'zsda_qp.c',
> +        'zsda_qp_common.c',
>          )
> 
>  zsda_compress = true
> diff --git a/drivers/common/zsda/zsda_qp_common.c
> b/drivers/common/zsda/zsda_qp_common.c
> new file mode 100644
> index 0000000000..9c7152eb24
> --- /dev/null
> +++ b/drivers/common/zsda/zsda_qp_common.c
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2024 ZTE Corporation
> + */
> +
> +#include "zsda_qp_common.h"
> +
> +static void
> +zsda_queue_delete(const struct zsda_queue *queue)
> +{
> +    const struct rte_memzone *mz;
> +
> +    if (queue == NULL) {
> +        ZSDA_LOG(DEBUG, "Invalid queue");
> +        return;
> +    }
> +
> +    mz = rte_memzone_lookup(queue->memz_name);
> +    if (mz != NULL) {
> +        memset(queue->base_addr, 0x0,
> +               (uint16_t)(queue->queue_size * queue->msg_size));
> +        rte_memzone_free(mz);
> +    } else
> +        ZSDA_LOG(DEBUG, "queue %s doesn't exist", queue->memz_name);
> +}
> +
> +int
> +zsda_queue_pair_release(struct zsda_qp **qp_addr)
> +{
> +    struct zsda_qp *qp = *qp_addr;
> +    uint32_t i;
> +    enum zsda_service_type type;
> +
> +    if (qp == NULL) {
> +        ZSDA_LOG(DEBUG, "qp already freed");
> +        return 0;
> +    }
> +
> +    for (type = 0; type < ZSDA_SERVICE_INVALID; type++) {
> +        if (!qp->srv[type].used)
> +            continue;
> +
> +        zsda_queue_delete(&(qp->srv[type].tx_q));
> +        zsda_queue_delete(&(qp->srv[type].rx_q));
> +        qp->srv[type].used = false;
> +        for (i = 0; i < qp->srv[type].nb_descriptors; i++)
> +            rte_mempool_put(qp->srv[type].op_cookie_pool,
> +                    qp->srv[type].op_cookies[i]);
> +
> +        rte_mempool_free(qp->srv[type].op_cookie_pool);
> +        rte_free(qp->srv[type].op_cookies);
> +    }
> +
> +    rte_free(qp);
> +    *qp_addr = NULL;
> +
> +    return ZSDA_SUCCESS;
> +}
> diff --git a/drivers/common/zsda/zsda_qp_common.h
> b/drivers/common/zsda/zsda_qp_common.h
> index 722fd730b2..1522f9605e 100644
> --- a/drivers/common/zsda/zsda_qp_common.h
> +++ b/drivers/common/zsda/zsda_qp_common.h
> @@ -68,4 +68,40 @@ struct zsda_admin_resp_qcfg {
>      uint8_t data[14];
>  } __rte_packed;
> 
> +struct zsda_queue {
> +    char memz_name[RTE_MEMZONE_NAMESIZE];
> +    uint8_t *io_addr;
> +    uint8_t *base_addr;       /* Base address */
> +    rte_iova_t base_phys_addr; /* Queue physical address */
> +    uint16_t head;           /* Shadow copy of the head */
> +    uint16_t tail;           /* Shadow copy of the tail */
> +    uint16_t modulo_mask;
> +    uint16_t msg_size;
> +    uint16_t queue_size;
> +    uint16_t cycle_size;
> +    uint16_t pushed_wqe;
> +
> +    uint8_t hw_queue_number;
> +    uint32_t csr_head; /* last written head value */
> +    uint32_t csr_tail; /* last written tail value */
> +
> +    uint8_t valid;
> +    uint16_t sid;
> +};
> +
> +struct qp_srv {
> +    bool used;
> +    struct zsda_queue tx_q;
> +    struct zsda_queue rx_q;
> +    struct rte_mempool *op_cookie_pool;
> +    void **op_cookies;
> +    uint16_t nb_descriptors;
> +};
> +
> +struct zsda_qp {
> +    struct qp_srv srv[ZSDA_MAX_SERVICES];
> +};
> +
> +int zsda_queue_pair_release(struct zsda_qp **qp_addr);
> +
>  #endif /* _ZSDA_QP_COMMON_H_ */
> diff --git a/drivers/compress/zsda/zsda_comp_pmd.c
> b/drivers/compress/zsda/zsda_comp_pmd.c
> index d1c33f448c..3bdcd66f6a 100644
> --- a/drivers/compress/zsda/zsda_comp_pmd.c
> +++ b/drivers/compress/zsda/zsda_comp_pmd.c
> @@ -8,13 +8,145 @@
>  #include "zsda_qp_common.h"
>  #include "zsda_comp_pmd.h"
> 
> +static int
> +zsda_comp_xform_size(void)
> +{
> +    return RTE_ALIGN_CEIL(sizeof(struct zsda_comp_xform), 8);
> +}
> +
> +static struct rte_mempool *
> +zsda_comp_create_xform_pool(struct zsda_comp_dev_private *comp_dev,
> +                struct rte_compressdev_config *config,
> +                uint32_t num_elements)

Can you rename the API to zsda_comp_xform_pool_create?
There is a convention followed in most of the APIs to have the verb in the end.

> +{
> +    char xform_pool_name[RTE_MEMPOOL_NAMESIZE];
> +    struct rte_mempool *mp;
> +
> +    snprintf(xform_pool_name, RTE_MEMPOOL_NAMESIZE, "%s_xforms",
> +         comp_dev->zsda_pci_dev->name);
> +
> +    ZSDA_LOG(DEBUG, "xformpool: %s", xform_pool_name);
> +    mp = rte_mempool_lookup(xform_pool_name);
> +
> +    if (mp != NULL) {
> +        ZSDA_LOG(DEBUG, "xformpool already created");
> +        if (mp->size != num_elements) {
> +            ZSDA_LOG(DEBUG, "xformpool wrong size - delete it");
> +            rte_mempool_free(mp);
> +            mp = NULL;
> +            comp_dev->xformpool = NULL;
> +        }
> +    } else {
> +        mp = rte_mempool_create(xform_pool_name, num_elements,
> +                    zsda_comp_xform_size(), 0, 0, NULL,
> +                    NULL, NULL, NULL, config->socket_id, 0);
> +        if (mp == NULL) {
> +            ZSDA_LOG(ERR, "Failed! mp is NULL");
> +            return NULL;
> +        }
> +    }
> +
> +    return mp;
> +}
> +
> +static int
> +zsda_comp_dev_config(struct rte_compressdev *dev,
> +             struct rte_compressdev_config *config)
> +{
> +    struct zsda_comp_dev_private *comp_dev = dev->data->dev_private;
> +
> +    if (config->max_nb_priv_xforms) {
> +        comp_dev->xformpool = zsda_comp_create_xform_pool(
> +            comp_dev, config, config->max_nb_priv_xforms);
> +        if (comp_dev->xformpool == NULL)
> +            return -ENOMEM;
> +    } else
> +        comp_dev->xformpool = NULL;
> +
> +    return ZSDA_SUCCESS;
> +}
> +
> +static int
> +zsda_comp_dev_start(struct rte_compressdev *dev)
> +{
> +    struct zsda_comp_dev_private *comp_dev = dev->data->dev_private;
> +    int ret;
> +
> +    ret = zsda_queue_start(comp_dev->zsda_pci_dev->pci_dev);
> +
> +    if (ret)
> +        ZSDA_LOG(ERR, "Failed! zsda_queue_start.");
> +
> +    return ret;
> +}
> +
> +static void
> +zsda_comp_dev_stop(struct rte_compressdev *dev)
> +{
> +    struct zsda_comp_dev_private *comp_dev = dev->data->dev_private;
> +
> +    zsda_queue_stop(comp_dev->zsda_pci_dev->pci_dev);
> +}
> +
> +static int
> +zsda_comp_qp_release(struct rte_compressdev *dev, uint16_t queue_pair_id)
> +{
> +    return zsda_queue_pair_release(
> +        (struct zsda_qp **)&(dev->data->queue_pairs[queue_pair_id]));
> +}
> +
> +static int
> +zsda_comp_dev_close(struct rte_compressdev *dev)
> +{
> +    uint16_t i;
> +    struct zsda_comp_dev_private *comp_dev = dev->data->dev_private;
> +
> +    for (i = 0; i < dev->data->nb_queue_pairs; i++)
> +        zsda_comp_qp_release(dev, i);

I do not see qp_setup being called in dev_config but it is released in dev_close.
Can you move the qp_setup and qp_release calls in same patch?

> +
> +    rte_mempool_free(comp_dev->xformpool);
> +    comp_dev->xformpool = NULL;
> +
> +    return ZSDA_SUCCESS;
> +}
> +
> +static uint16_t
> +zsda_comp_max_nb_qps(void)
> +{
> +    uint16_t comp = zsda_nb_qps.encomp;
> +    uint16_t decomp = zsda_nb_qps.decomp;
> +    uint16_t min = 0;
> +
> +    if ((comp == MAX_QPS_ON_FUNCTION) ||
> +        (decomp == MAX_QPS_ON_FUNCTION))
> +        min = MAX_QPS_ON_FUNCTION;
> +    else
> +        min = (comp < decomp) ? comp : decomp;
> +    if (min == 0)
> +        return MAX_QPS_ON_FUNCTION;
> +    return min;
> +}
> +
> +static void
> +zsda_comp_dev_info_get(struct rte_compressdev *dev,
> +               struct rte_compressdev_info *info)
> +{
> +    struct zsda_comp_dev_private *comp_dev = dev->data->dev_private;
> +
> +    if (info != NULL) {
> +        info->max_nb_queue_pairs = zsda_comp_max_nb_qps();
> +        info->feature_flags = dev->feature_flags;
> +        info->capabilities = comp_dev->zsda_dev_capabilities;
> +    }
> +}
> +
>  static struct rte_compressdev_ops compress_zsda_ops = {
> 
> -    .dev_configure = NULL,
> -    .dev_start = NULL,
> -    .dev_stop = NULL,
> -    .dev_close = NULL,
> -    .dev_infos_get = NULL,
> +    .dev_configure = zsda_comp_dev_config,
> +    .dev_start = zsda_comp_dev_start,
> +    .dev_stop = zsda_comp_dev_stop,
> +    .dev_close = zsda_comp_dev_close,
> +    .dev_infos_get = zsda_comp_dev_info_get,
> 
>      .stats_get = NULL,
>      .stats_reset = NULL,
> @@ -121,6 +253,8 @@ zsda_comp_dev_destroy(struct zsda_pci_device
> *zsda_pci_dev)
>      if (rte_eal_process_type() == RTE_PROC_PRIMARY)
>          rte_memzone_free(zsda_pci_dev->comp_dev->capa_mz);
> 
> +    zsda_comp_dev_close(comp_dev->compressdev);
> +
>      rte_compressdev_pmd_destroy(comp_dev->compressdev);
>      zsda_pci_dev->comp_dev = NULL;
> 
> diff --git a/drivers/compress/zsda/zsda_comp_pmd.h
> b/drivers/compress/zsda/zsda_comp_pmd.h
> index c6ef57af8e..8d76ef2bcc 100644
> --- a/drivers/compress/zsda/zsda_comp_pmd.h
> +++ b/drivers/compress/zsda/zsda_comp_pmd.h
> @@ -13,6 +13,11 @@
>  /**< ZSDA Compression PMD driver name */
>  #define COMPRESSDEV_NAME_ZSDA_PMD compress_zsda
> 
> +struct zsda_comp_xform {
> +    enum rte_comp_xform_type type;
> +    enum rte_comp_checksum_type checksum_type;
> +};
> +
>  int zsda_comp_dev_create(struct zsda_pci_device *zsda_pci_dev);
> 
>  int zsda_comp_dev_destroy(struct zsda_pci_device *zsda_pci_dev);
> --
> 2.27.0