> -----Original Message-----
> From: McDaniel, Timothy <timothy.mcdan...@intel.com>
> Sent: Friday, September 11, 2020 3:26 PM
> Cc: dev@dpdk.org; Carrillo, Erik G <erik.g.carri...@intel.com>; Eads, Gage
> <gage.e...@intel.com>; Van Haaren, Harry <harry.van.haa...@intel.com>;
> jer...@marvell.com
> Subject: [PATCH 11/22] event/dlb2: add port setup
>
> Configure the load balanded (ldb) or directed (dir) port.
> The consumer queue (CQ) and producer port (PP) are also
> set up here.
>
> Signed-off-by: Timothy McDaniel <timothy.mcdan...@intel.com>
> ---
> drivers/event/dlb2/dlb2.c | 527 +++++++++++++++++
> drivers/event/dlb2/dlb2_iface.c | 9 +
> drivers/event/dlb2/dlb2_iface.h | 8 +
> drivers/event/dlb2/pf/base/dlb2_resource.c | 921
> +++++++++++++++++++++++++++++
> drivers/event/dlb2/pf/dlb2_main.c | 28 +
> drivers/event/dlb2/pf/dlb2_pf.c | 179 ++++++
> 6 files changed, 1672 insertions(+)
>
> diff --git a/drivers/event/dlb2/dlb2.c b/drivers/event/dlb2/dlb2.c
> index 366e194..a4c8833 100644
> --- a/drivers/event/dlb2/dlb2.c
> +++ b/drivers/event/dlb2/dlb2.c
> @@ -1043,6 +1043,532 @@ dlb2_eventdev_queue_setup(struct rte_eventdev
> *dev,
> return ret;
> }
>
> +static int
> +dlb2_init_consume_qe(struct dlb2_port *qm_port, char *mz_name)
> +{
> + struct dlb2_cq_pop_qe *qe;
> +
> + qe = rte_malloc(mz_name,
> + DLB2_NUM_QES_PER_CACHE_LINE *
> + sizeof(struct dlb2_cq_pop_qe),
> + RTE_CACHE_LINE_SIZE);
> +
> + if (qe == NULL) {
> + DLB2_LOG_ERR("dlb2: no memory for consume_qe\n");
> + return -ENOMEM;
> + }
> + qm_port->consume_qe = qe;
> +
> + memset(qe, 0, DLB2_NUM_QES_PER_CACHE_LINE *
> + sizeof(struct dlb2_cq_pop_qe));
You can use rte_zmalloc() instead (applies to other init_*_qe functions below
too), and
no need to zero-init the fields again after the memset.
> +
> + qe->qe_valid = 0;
> + qe->qe_frag = 0;
> + qe->qe_comp = 0;
> + qe->cq_token = 1;
> + /* Tokens value is 0-based; i.e. '0' returns 1 token, '1' returns 2,
> + * and so on.
> + */
> + qe->tokens = 0; /* set at run time */
> + qe->meas_lat = 0;
> + qe->no_dec = 0;
> + /* Completion IDs are disabled */
> + qe->cmp_id = 0;
> +
> + return 0;
> +}
> +
> +static int
> +dlb2_init_int_arm_qe(struct dlb2_port *qm_port, char *mz_name)
> +{
> + struct dlb2_enqueue_qe *qe;
> +
> + qe = rte_malloc(mz_name,
> + DLB2_NUM_QES_PER_CACHE_LINE *
> + sizeof(struct dlb2_enqueue_qe),
> + RTE_CACHE_LINE_SIZE);
> +
> + if (qe == NULL) {
> + DLB2_LOG_ERR("dlb2: no memory for complete_qe\n");
> + return -ENOMEM;
> + }
> + qm_port->int_arm_qe = qe;
> +
> + memset(qe, 0, DLB2_NUM_QES_PER_CACHE_LINE *
> + sizeof(struct dlb2_enqueue_qe));
> +
> + /* V2 - INT ARM is CQ_TOKEN + FRAG */
> + qe->qe_valid = 0;
> + qe->qe_frag = 1;
> + qe->qe_comp = 0;
> + qe->cq_token = 1;
> + qe->meas_lat = 0;
> + qe->no_dec = 0;
> + /* Completion IDs are disabled */
> + qe->cmp_id = 0;
> +
> + return 0;
> +}
> +
> +static int
> +dlb2_init_qe_mem(struct dlb2_port *qm_port, char *mz_name)
> +{
> + int ret, sz;
> +
> + sz = DLB2_NUM_QES_PER_CACHE_LINE * sizeof(struct
> dlb2_enqueue_qe);
> +
> + qm_port->qe4 = rte_malloc(mz_name, sz, RTE_CACHE_LINE_SIZE);
> +
> + if (qm_port->qe4 == NULL) {
> + DLB2_LOG_ERR("dlb2: no qe4 memory\n");
> + ret = -ENOMEM;
> + goto error_exit;
> + }
> +
> + memset(qm_port->qe4, 0, sz);
> +
> + ret = dlb2_init_int_arm_qe(qm_port, mz_name);
> + if (ret < 0) {
> + DLB2_LOG_ERR("dlb2: dlb2_init_int_arm_qe ret=%d\n",
> + ret);
This can fit on one line
> + goto error_exit;
> + }
> +
> + ret = dlb2_init_consume_qe(qm_port, mz_name);
> + if (ret < 0) {
> + DLB2_LOG_ERR("dlb2: dlb2_init_consume_qe ret=%d\n",
> + ret);
This can fit on one line
> + goto error_exit;
> + }
> +
> + return 0;
> +
> +error_exit:
> +
> + dlb2_free_qe_mem(qm_port);
> +
> + return ret;
> +}
> +
> +static int
> +dlb2_hw_create_ldb_port(struct dlb2_eventdev *dlb2,
> + struct dlb2_eventdev_port *ev_port,
> + uint32_t dequeue_depth,
> + uint32_t enqueue_depth)
> +{
> + struct dlb2_hw_dev *handle = &dlb2->qm_instance;
> + struct dlb2_create_ldb_port_args cfg = {0};
> + int ret;
> + struct dlb2_port *qm_port = NULL;
> + char mz_name[RTE_MEMZONE_NAMESIZE];
> + uint32_t qm_port_id;
> + uint16_t ldb_credit_high_watermark;
> + uint16_t dir_credit_high_watermark;
> +
> + if (handle == NULL)
> + return -EINVAL;
> +
> + if (dequeue_depth < DLB2_MIN_CQ_DEPTH ||
> + dequeue_depth > DLB2_MAX_INPUT_QUEUE_DEPTH) {
> + DLB2_LOG_ERR("dlb2: invalid dequeue_depth, must be %d-
> %d\n",
> + DLB2_MIN_CQ_DEPTH,
> DLB2_MAX_INPUT_QUEUE_DEPTH);
> + return -EINVAL;
> + }
> +
> + if (enqueue_depth < DLB2_MIN_ENQUEUE_DEPTH) {
> + DLB2_LOG_ERR("dlb2: invalid enqueue_depth, must be at least
> %d\n",
> + DLB2_MIN_ENQUEUE_DEPTH);
> + return -EINVAL;
> + }
The dequeue and enqueue depth checks are suspiciously inconsistent -- only the
dequeue
depth is compared against an upper bound. I suspect the enqueue upper bound
check is missing
because it's already checked in both dlb2_eventdev_port_setup() and
rte_event_port_setup()...if that's the case, can the dequeue depth max check be
dropped as well?
> +
> + rte_spinlock_lock(&handle->resource_lock);
> +
> + /* TODO - additional parameter validation */
Leftover TODO
> + /* We round up to the next power of 2 if necessary */
> + cfg.cq_depth = rte_align32pow2(dequeue_depth);
> + cfg.cq_depth_threshold = 1;
> +
> + cfg.cq_history_list_size =
> DLB2_NUM_HIST_LIST_ENTRIES_PER_LDB_PORT;
> +
> + if (handle->cos_id == DLB2_COS_DEFAULT)
> + cfg.cos_id = 0;
> + else
> + cfg.cos_id = handle->cos_id;
> +
> + cfg.cos_strict = 0;
> +
> + /* User controls the LDB high watermark via enqueue depth. The DIR
> high
> + * watermark is equal, unless the directed credit pool is too small.
> + */
> + ldb_credit_high_watermark = enqueue_depth;
> +
> + /* If there are no directed ports, the kernel driver will ignore this
> + * port's directed credit settings. Don't use enqueue_depth if it would
> + * require more directed credits than are available.
> + */
> + dir_credit_high_watermark =
> + RTE_MIN(enqueue_depth,
> + handle->cfg.num_dir_credits / dlb2->num_ports);
> +
> + /* Per QM values */
> +
> + /* DEBUG
> + * DLB2_LOG_ERR("create ldb port - grp=%d, devId=%d\n",
> + * handle->cfg.domain_id, handle->device_id);
> + */
Leftover debug code
> +
> + ret = dlb2_iface_ldb_port_create(handle, &cfg, dlb2->poll_mode);
> + if (ret < 0) {
> + DLB2_LOG_ERR("dlb2: dlb2_ldb_port_create error, ret=%d
> (driver status: %s)\n",
> + ret, dlb2_error_strings[cfg.response.status]);
> + goto error_exit;
> + }
> +
> + qm_port_id = cfg.response.id;
> +
> + DLB2_LOG_DBG("dlb2: ev_port %d uses qm LB port %d <<<<<\n",
> + ev_port->id, qm_port_id);
> +
> + qm_port = &ev_port->qm_port;
> + qm_port->ev_port = ev_port; /* back ptr */
> + qm_port->dlb2 = dlb2; /* back ptr */
> + /*
> + * Allocate and init local qe struct(s).
> + * Note: MOVDIR64 requires the enqueue QE (qe4) to be aligned.
> + */
> +
> + snprintf(mz_name, sizeof(mz_name), "%s_ldb_port%d",
> + handle->device_name,
> + ev_port->id);
> +
I see handle->device_name getting read here, in dlb2_hw_create_dir_port(),
and also in dlb2_eventdev_dump(), but I don't see it written anywhere?
> + ret = dlb2_init_qe_mem(qm_port, mz_name);
> + if (ret < 0) {
> + DLB2_LOG_ERR("dlb2: init_qe_mem failed, ret=%d\n", ret);
> + goto error_exit;
> + }
> +
> + qm_port->id = qm_port_id;
> +
> + qm_port->cached_ldb_credits = 0;
> + qm_port->cached_dir_credits = 0;
> + /* CQs with depth < 8 use an 8-entry queue, but withhold credits so
> + * the effective depth is smaller.
> + */
> + qm_port->cq_depth = cfg.cq_depth <= 8 ? 8 : cfg.cq_depth;
> + qm_port->cq_idx = 0;
> + qm_port->cq_idx_unmasked = 0;
> +
> + if (dlb2->poll_mode == DLB2_CQ_POLL_MODE_SPARSE)
> + qm_port->cq_depth_mask = (qm_port->cq_depth * 4) - 1;
> + else
> + qm_port->cq_depth_mask = qm_port->cq_depth - 1;
> +
> + qm_port->gen_bit_shift = __builtin_popcount(qm_port-
> >cq_depth_mask);
> + /* starting value of gen bit - it toggles at wrap time */
> + qm_port->gen_bit = 1;
> +
> + qm_port->int_armed = false;
> +
> + /* Save off for later use in info and lookup APIs. */
> + qm_port->qid_mappings = &dlb2->qm_ldb_to_ev_queue_id[0];
> +
> + qm_port->dequeue_depth = dequeue_depth;
> + qm_port->token_pop_thresh = dequeue_depth;
> +
> + qm_port->owed_tokens = 0;
> + qm_port->issued_releases = 0;
> +
> + /* Save config message too. */
> + rte_memcpy(&qm_port->cfg.ldb, &cfg, sizeof(cfg));
I know qm_port->cfg.ldb and cfg are the same type, but just for safety in case
that
ever changes in the future...probably better to use sizeof() on the destination
rather
than the source.
> +
> + /* update state */
> + qm_port->state = PORT_STARTED; /* enabled at create time */
> + qm_port->config_state = DLB2_CONFIGURED;
> +
> + qm_port->dir_credits = dir_credit_high_watermark;
> + qm_port->ldb_credits = ldb_credit_high_watermark;
> + qm_port->credit_pool[DLB2_DIR_QUEUE] = &dlb2->dir_credit_pool;
> + qm_port->credit_pool[DLB2_LDB_QUEUE] = &dlb2->ldb_credit_pool;
> +
> + DLB2_LOG_DBG("dlb2: created ldb port %d, depth = %d, ldb credits=%d,
> dir credits=%d\n",
> + qm_port_id,
> + dequeue_depth,
> + qm_port->ldb_credits,
> + qm_port->dir_credits);
> +
> + rte_spinlock_unlock(&handle->resource_lock);
> +
> + return 0;
> +
> +error_exit:
> +
> + if (qm_port)
> + dlb2_free_qe_mem(qm_port);
> +
> + rte_spinlock_unlock(&handle->resource_lock);
> +
> + DLB2_LOG_ERR("dlb2: create ldb port failed!\n");
> +
> + return ret;
> +}
> +
> +static void
> +dlb2_port_link_teardown(struct dlb2_eventdev *dlb2,
> + struct dlb2_eventdev_port *ev_port)
> +{
> + struct dlb2_eventdev_queue *ev_queue;
> + int i;
> +
> + for (i = 0; i < DLB2_MAX_NUM_QIDS_PER_LDB_CQ; i++) {
> + if (!ev_port->link[i].valid)
> + continue;
> +
> + ev_queue = &dlb2->ev_queues[ev_port->link[i].queue_id];
> +
> + ev_port->link[i].valid = false;
> + ev_port->num_links--;
> + ev_queue->num_links--;
> + }
> +}
> +
> +static int
> +dlb2_hw_create_dir_port(struct dlb2_eventdev *dlb2,
> + struct dlb2_eventdev_port *ev_port,
> + uint32_t dequeue_depth,
> + uint32_t enqueue_depth)
> +{
> + struct dlb2_hw_dev *handle = &dlb2->qm_instance;
> + struct dlb2_create_dir_port_args cfg = {0};
> + int ret;
> + struct dlb2_port *qm_port = NULL;
> + char mz_name[RTE_MEMZONE_NAMESIZE];
> + uint32_t qm_port_id;
> + uint16_t ldb_credit_high_watermark;
> + uint16_t dir_credit_high_watermark;
> +
> + if (dlb2 == NULL || handle == NULL)
> + return -EINVAL;
> +
> + if (dequeue_depth < DLB2_MIN_CQ_DEPTH ||
> + dequeue_depth > DLB2_MAX_INPUT_QUEUE_DEPTH) {
> + DLB2_LOG_ERR("dlb2: invalid dequeue_depth, must be %d-
> %d\n",
> + DLB2_MIN_CQ_DEPTH,
> DLB2_MAX_INPUT_QUEUE_DEPTH);
> + return -EINVAL;
> + }
Enqueue depth check needed?
> +
> + rte_spinlock_lock(&handle->resource_lock);
> +
> + /* Directed queues are configured at link time. */
> + cfg.queue_id = -1;
> +
> + /* We round up to the next power of 2 if necessary */
> + cfg.cq_depth = rte_align32pow2(dequeue_depth);
> + cfg.cq_depth_threshold = 1;
> +
> + /* User controls the LDB high watermark via enqueue depth. The DIR
> high
> + * watermark is equal, unless the directed credit pool is too small.
> + */
> + ldb_credit_high_watermark = enqueue_depth;
> +
> + /* Don't use enqueue_depth if it would require more directed credits
> + * than are available.
> + */
> + dir_credit_high_watermark =
> + RTE_MIN(enqueue_depth,
> + handle->cfg.num_dir_credits / dlb2->num_ports);
> +
> + /* Per QM values */
> +
> + ret = dlb2_iface_dir_port_create(handle, &cfg, dlb2->poll_mode);
> + if (ret < 0) {
> + DLB2_LOG_ERR("dlb2: dlb2_dir_port_create error, ret=%d (driver
> status: %s)\n",
> + ret, dlb2_error_strings[cfg.response.status]);
> + goto error_exit;
> + }
> +
> + qm_port_id = cfg.response.id;
> +
> + DLB2_LOG_DBG("dlb2: ev_port %d uses qm DIR port %d <<<<<\n",
> + ev_port->id, qm_port_id);
> +
> + qm_port = &ev_port->qm_port;
> + qm_port->ev_port = ev_port; /* back ptr */
> + qm_port->dlb2 = dlb2; /* back ptr */
> +
> + /*
> + * Init local qe struct(s).
> + * Note: MOVDIR64 requires the enqueue QE to be aligned
> + */
> +
> + snprintf(mz_name, sizeof(mz_name), "%s_dir_port%d",
> + handle->device_name,
(See device_name comment above)
> + ev_port->id);
> +
> + ret = dlb2_init_qe_mem(qm_port, mz_name);
> +
> + if (ret < 0) {
> + DLB2_LOG_ERR("dlb2: init_qe_mem failed, ret=%d\n", ret);
> + goto error_exit;
> + }
> +
> + qm_port->id = qm_port_id;
> +
> + qm_port->cached_ldb_credits = 0;
> + qm_port->cached_dir_credits = 0;
> + /* CQs with depth < 8 use an 8-entry queue, but withhold credits so
> + * the effective depth is smaller.
> + */
> + qm_port->cq_depth = cfg.cq_depth <= 8 ? 8 : cfg.cq_depth;
> + qm_port->cq_idx = 0;
> + qm_port->cq_idx_unmasked = 0;
> +
> + if (dlb2->poll_mode == DLB2_CQ_POLL_MODE_SPARSE)
> + qm_port->cq_depth_mask = (cfg.cq_depth * 4) - 1;
> + else
> + qm_port->cq_depth_mask = cfg.cq_depth - 1;
> +
> + qm_port->gen_bit_shift = __builtin_popcount(qm_port-
> >cq_depth_mask);
> + /* starting value of gen bit - it toggles at wrap time */
> + qm_port->gen_bit = 1;
> +
> + qm_port->int_armed = false;
> +
> + /* Save off for later use in info and lookup APIs. */
> + qm_port->qid_mappings = &dlb2->qm_dir_to_ev_queue_id[0];
> +
> + qm_port->dequeue_depth = dequeue_depth;
> +
> + /* Directed ports are auto-pop, by default. */
> + qm_port->token_pop_mode = AUTO_POP;
> + qm_port->owed_tokens = 0;
> + qm_port->issued_releases = 0;
> +
> + /* Save config message too. */
> + rte_memcpy(&qm_port->cfg.dir, &cfg, sizeof(cfg));
(See sizeof() comment above)
> +
> + /* update state */
> + qm_port->state = PORT_STARTED; /* enabled at create time */
> + qm_port->config_state = DLB2_CONFIGURED;
> +
> + qm_port->dir_credits = dir_credit_high_watermark;
> + qm_port->ldb_credits = ldb_credit_high_watermark;
> + qm_port->credit_pool[DLB2_DIR_QUEUE] = &dlb2->dir_credit_pool;
> + qm_port->credit_pool[DLB2_LDB_QUEUE] = &dlb2->ldb_credit_pool;
> +
> + DLB2_LOG_DBG("dlb2: created dir port %d, depth = %d cr=%d,%d\n",
> + qm_port_id,
> + dequeue_depth,
> + dir_credit_high_watermark,
> + ldb_credit_high_watermark);
> +
> + rte_spinlock_unlock(&handle->resource_lock);
> +
> + return 0;
> +
> +error_exit:
> +
> + if (qm_port)
> + dlb2_free_qe_mem(qm_port);
> +
> + rte_spinlock_unlock(&handle->resource_lock);
> +
> + DLB2_LOG_ERR("dlb2: create dir port failed!\n");
> +
> + return ret;
> +}
> +
> +static int
> +dlb2_eventdev_port_setup(struct rte_eventdev *dev,
> + uint8_t ev_port_id,
> + const struct rte_event_port_conf *port_conf)
> +{
> + struct dlb2_eventdev *dlb2;
> + struct dlb2_eventdev_port *ev_port;
> + int ret;
> +
> + if (dev == NULL || port_conf == NULL) {
> + DLB2_LOG_ERR("Null parameter\n");
> + return -EINVAL;
> + }
> +
> + dlb2 = dlb2_pmd_priv(dev);
> +
> + if (ev_port_id >= DLB2_MAX_NUM_PORTS)
> + return -EINVAL;
> +
> + if (port_conf->dequeue_depth >
> + evdev_dlb2_default_info.max_event_port_dequeue_depth ||
> + port_conf->enqueue_depth >
> + evdev_dlb2_default_info.max_event_port_enqueue_depth)
> + return -EINVAL;
> +
> + ev_port = &dlb2->ev_ports[ev_port_id];
> + /* configured? */
> + if (ev_port->setup_done) {
> + DLB2_LOG_ERR("evport %d is already configured\n",
> ev_port_id);
> + return -EINVAL;
> + }
> +
> + /* The reserved token interrupt arming scheme requires that one or
> more
> + * CQ tokens be reserved by the PMD. This limits the amount of CQ space
> + * usable by the DLB, so in order to give an *effective* CQ depth equal
> + * to the user-requested value, we double CQ depth and reserve half of
> + * its tokens. If the user requests the max CQ depth (256) then we
> + * cannot double it, so we reserve one token and give an effective
> + * depth of 255 entries.
> + */
I don't think this comment applies to the 2.0 device.
> +
> + ev_port->qm_port.is_directed = port_conf->event_port_cfg &
> + RTE_EVENT_PORT_CFG_SINGLE_LINK;
> +
> + if (!ev_port->qm_port.is_directed) {
> + ret = dlb2_hw_create_ldb_port(dlb2,
> + ev_port,
> + port_conf->dequeue_depth,
> + port_conf->enqueue_depth);
> + if (ret < 0) {
> + DLB2_LOG_ERR("Failed to create the lB port ve
> portId=%d\n",
> + ev_port_id);
> +
> + return ret;
> + }
> + } else {
> + ret = dlb2_hw_create_dir_port(dlb2,
> + ev_port,
> + port_conf->dequeue_depth,
> + port_conf->enqueue_depth);
> + if (ret < 0) {
> + DLB2_LOG_ERR("Failed to create the DIR port\n");
> + return ret;
> + }
> + }
> +
> + /* Save off port config for reconfig */
> + dlb2->ev_ports[ev_port_id].conf = *port_conf;
Nit: 'ev_port' is assigned to &dlb2->ev_ports[ev_port_id] above, use it here
and below?
> +
> + dlb2->ev_ports[ev_port_id].id = ev_port_id;
> + dlb2->ev_ports[ev_port_id].enq_configured = true;
> + dlb2->ev_ports[ev_port_id].setup_done = true;
> + dlb2->ev_ports[ev_port_id].inflight_max =
> + port_conf->new_event_threshold;
> + dlb2->ev_ports[ev_port_id].implicit_release =
> + !(port_conf->event_port_cfg &
> + RTE_EVENT_PORT_CFG_DISABLE_IMPL_REL);
> + dlb2->ev_ports[ev_port_id].outstanding_releases = 0;
> + dlb2->ev_ports[ev_port_id].inflight_credits = 0;
> + dlb2->ev_ports[ev_port_id].credit_update_quanta =
> + RTE_LIBRTE_PMD_DLB2_SW_CREDIT_QUANTA;
> + dlb2->ev_ports[ev_port_id].dlb2 = dlb2; /* reverse link */
> +
> + /* Tear down pre-existing port->queue links */
> + if (dlb2->run_state == DLB2_RUN_STATE_STOPPED)
> + dlb2_port_link_teardown(dlb2, &dlb2->ev_ports[ev_port_id]);
> +
> + dev->data->ports[ev_port_id] = &dlb2->ev_ports[ev_port_id];
> +
> + return 0;
> +}
[...]
> diff --git a/drivers/event/dlb2/pf/dlb2_pf.c b/drivers/event/dlb2/pf/dlb2_pf.c
> index dea70e6..a6824b1 100644
> --- a/drivers/event/dlb2/pf/dlb2_pf.c
> +++ b/drivers/event/dlb2/pf/dlb2_pf.c
> @@ -234,6 +234,183 @@ dlb2_pf_set_sn_allocation(struct dlb2_hw_dev
> *handle,
> return ret;
> }
>
> +static void *
> +dlb2_alloc_coherent_aligned(uintptr_t *phys, size_t size, int align)
> +{
> + const struct rte_memzone *mz;
> + char mz_name[RTE_MEMZONE_NAMESIZE];
> + uint32_t core_id = rte_lcore_id();
> + unsigned int socket_id;
> +
> + snprintf(mz_name, sizeof(mz_name) - 1, "%lx",
> + (unsigned long)rte_get_timer_cycles());
For debug purposes, it would be better if this name can trace the mz back to
this driver. How about something like event_dlb2_pf_name + ldb/dir_port + port
ID?
> + if (core_id == (unsigned int)LCORE_ID_ANY)
> + core_id = rte_get_master_lcore();
> + socket_id = rte_lcore_to_socket_id(core_id);
Should this use the socket ID devarg (and perhaps fall back to the core's
socket if unspecified)?
> + mz = rte_memzone_reserve_aligned(mz_name, size, socket_id,
> + RTE_MEMZONE_IOVA_CONTIG, align);
> + if (!mz) {
> + DLB2_LOG_DBG("Unable to allocate DMA memory of size %zu
> bytes - %s\n",
> + size, rte_strerror(rte_errno));
> + *phys = 0;
> + return NULL;
> + }
> + *phys = mz->iova;
> + return mz->addr;
> +}
> +
> +static int
> +dlb2_pf_ldb_port_create(struct dlb2_hw_dev *handle,
> + struct dlb2_create_ldb_port_args *cfg,
> + enum dlb2_cq_poll_modes poll_mode)
> +{
> + struct dlb2_dev *dlb2_dev = (struct dlb2_dev *)handle->pf_dev;
> + struct dlb2_cmd_response response = {0};
> + struct dlb2_port_memory port_memory;
> + int ret, cq_alloc_depth;
> + uint8_t *port_base;
> + int alloc_sz, qe_sz;
> + phys_addr_t cq_base;
> + phys_addr_t pp_base;
> + int is_dir = false;
> +
> + DLB2_INFO(dev->dlb2_device, "Entering %s()\n", __func__);
> +
> + if (poll_mode == DLB2_CQ_POLL_MODE_STD)
> + qe_sz = sizeof(struct dlb2_dequeue_qe);
> + else
> + qe_sz = RTE_CACHE_LINE_SIZE;
> +
> + /* Calculate the port memory required, and round up to the nearest
> + * cache line.
> + */
> + cq_alloc_depth = RTE_MAX(cfg->cq_depth,
> DLB2_MIN_HARDWARE_CQ_DEPTH);
> + alloc_sz = cq_alloc_depth * qe_sz;
> + alloc_sz = RTE_CACHE_LINE_ROUNDUP(alloc_sz);
> +
> + port_base = dlb2_alloc_coherent_aligned(&cq_base,
> + alloc_sz,
> + PAGE_SIZE);
This can fit on one line
> + if (port_base == NULL)
> + return -ENOMEM;
> +
> + /* Lock the page in memory */
> + ret = rte_mem_lock_page(port_base);
> + if (ret < 0) {
> + DLB2_LOG_ERR("dlb2 pf pmd could not lock page for device
> i/o\n");
> + goto create_port_err;
> + }
> +
> +
> + memset(port_base, 0, alloc_sz);
> +
> + ret = dlb2_pf_create_ldb_port(&dlb2_dev->hw,
> + handle->domain_id,
> + cfg,
> + cq_base,
> + &response);
> + if (ret)
> + goto create_port_err;
> +
> + pp_base = (uintptr_t)dlb2_dev->hw.func_kva + PP_BASE(is_dir);
> + dlb2_port[response.id][DLB2_LDB_PORT].pp_addr =
> + (void *)(uintptr_t)(pp_base + (PAGE_SIZE * response.id));
If pp_base is defined as a uintptr_t, I think you can avoid some of the explicit
casts.
> +
> + dlb2_port[response.id][DLB2_LDB_PORT].cq_base =
> + (void *)(uintptr_t)(port_base);
Since port_base is a uint8_t*, the uintptr_t cast shouldn't be necessary
> + memset(&port_memory, 0, sizeof(port_memory));
> + dlb2_list_init_head(&port_memory.list);
> +
> + /* Fill out the per-port memory tracking structure */
> + dlb2_dev->ldb_port_pages[response.id].valid = true;
> + dlb2_list_splice(&port_memory.list,
> + &dlb2_dev->ldb_port_pages[response.id].list);
Does the list serve any purpose? Looks like port_memory is zero-initialized,
then it becomes
the sole entry on a per-port list.
> +
> + cfg->response = response;
> +
> + DLB2_INFO(dev->dlb2_device, "Exiting %s() with ret=%d\n",
> + __func__, ret);
> +
> +create_port_err:
Need to free the memzone in this case.
> +
> + return ret;
> +}
> +
> +static int
> +dlb2_pf_dir_port_create(struct dlb2_hw_dev *handle,
> + struct dlb2_create_dir_port_args *cfg,
> + enum dlb2_cq_poll_modes poll_mode)
> +{
> + struct dlb2_dev *dlb2_dev = (struct dlb2_dev *)handle->pf_dev;
> + struct dlb2_cmd_response response = {0};
> + struct dlb2_port_memory port_memory;
> + int ret;
> + uint8_t *port_base;
> + int alloc_sz, qe_sz;
> + phys_addr_t cq_base;
> + phys_addr_t pp_base;
> + int is_dir = true;
> +
> + DLB2_INFO(dev->dlb2_device, "Entering %s()\n", __func__);
> +
> + if (poll_mode == DLB2_CQ_POLL_MODE_STD)
> + qe_sz = sizeof(struct dlb2_dequeue_qe);
> + else
> + qe_sz = RTE_CACHE_LINE_SIZE;
> +
> + /* Calculate the port memory required, and round up to the nearest
> + * cache line.
> + */
> + alloc_sz = cfg->cq_depth * qe_sz;
> + alloc_sz = RTE_CACHE_LINE_ROUNDUP(alloc_sz);
> +
> + port_base = dlb2_alloc_coherent_aligned(&cq_base,
> + alloc_sz,
> + PAGE_SIZE);
This can fit on one line
> + if (port_base == NULL)
> + return -ENOMEM;
> +
> + /* Lock the page in memory */
> + ret = rte_mem_lock_page(port_base);
> + if (ret < 0) {
> + DLB2_LOG_ERR("dlb2 pf pmd could not lock page for device
> i/o\n");
> + goto create_port_err;
> + }
> +
> + memset(port_base, 0, alloc_sz);
> +
> + ret = dlb2_pf_create_dir_port(&dlb2_dev->hw,
> + handle->domain_id,
> + cfg,
> + cq_base,
> + &response);
> + if (ret)
> + goto create_port_err;
> +
> + pp_base = (uintptr_t)dlb2_dev->hw.func_kva + PP_BASE(is_dir);
(See uintptr_t comment above)
> + dlb2_port[response.id][DLB2_DIR_PORT].pp_addr =
> + (void *)(uintptr_t)(pp_base + (PAGE_SIZE * response.id));
> +
> + dlb2_port[response.id][DLB2_DIR_PORT].cq_base =
> + (void *)(uintptr_t)(port_base);
(See port_base comment above)
> + memset(&port_memory, 0, sizeof(port_memory));
> + dlb2_list_init_head(&port_memory.list);
> +
> + /* Fill out the per-port memory tracking structure */
> + dlb2_dev->dir_port_pages[response.id].valid = true;
> + dlb2_list_splice(&port_memory.list,
> + &dlb2_dev->dir_port_pages[response.id].list);
> +
(See list comment above)
> + cfg->response = response;
> +
> + DLB2_INFO(dev->dlb2_device, "Exiting %s() with ret=%d\n",
> + __func__, ret);
> +
> +create_port_err:
Need to free the memzone in this case
> +
> + return ret;
> +}
> +
> static void
> dlb2_pf_iface_fn_ptrs_init(void)
> {
> @@ -247,6 +424,8 @@ dlb2_pf_iface_fn_ptrs_init(void)
> dlb2_iface_get_cq_poll_mode = dlb2_pf_get_cq_poll_mode;
> dlb2_iface_sched_domain_create = dlb2_pf_sched_domain_create;
> dlb2_iface_ldb_queue_create = dlb2_pf_ldb_queue_create;
> + dlb2_iface_ldb_port_create = dlb2_pf_ldb_port_create;
> + dlb2_iface_dir_port_create = dlb2_pf_dir_port_create;
> dlb2_iface_get_sn_allocation = dlb2_pf_get_sn_allocation;
> dlb2_iface_set_sn_allocation = dlb2_pf_set_sn_allocation;
> dlb2_iface_get_sn_occupancy = dlb2_pf_get_sn_occupancy;
> --
> 2.6.4
I don't see the port memzones getting freed anywhere, e.g. if the event device
is reset.
Looks like a possible memory leak.
Thanks,
Gage
