From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 81001A0583; Thu, 19 Mar 2020 10:36:42 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 88A301C025; Thu, 19 Mar 2020 10:36:41 +0100 (CET) Received: from mail-lj1-f193.google.com (mail-lj1-f193.google.com [209.85.208.193]) by dpdk.org (Postfix) with ESMTP id B94221515 for ; Thu, 19 Mar 2020 10:36:39 +0100 (CET) Received: by mail-lj1-f193.google.com with SMTP id u12so1678884ljo.2 for ; Thu, 19 Mar 2020 02:36:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=gpTCRuqRtamfKI4LEW9uPO+FYawNv928moQZ2h8bW7s=; b=CNOkfBv5yC1WFBqKKYZ5Y6z1I1n4lLvnCPA6tCFdH4vXnBJBVoDkXaRP5klZ+updU1 mqcZQKV/+8IQYSq0A+f2h1JgW+g60M8fy1WXiPgRrfXH5NM82qYxb3QFN2vn2mxR7acn rDNw/8GHR9Lxskla1/Tpro7ngidU/2GBPQChlhZ5dXqkNJY6qfNrQiR8sI8Xj6CoBwLN Vt/abu4hbXWLNVbf5lZy49+Gj1vF8t62qEweNybpdgSBoiU8PiQATSwNDONUNcFnDrph K3VNb26ydradYuZhEe774Pf/wtcDTJT6uEO3rdFA9AamDsdwdkwqFfIFsy9ghmEFZBiF rEkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=gpTCRuqRtamfKI4LEW9uPO+FYawNv928moQZ2h8bW7s=; b=Xltsuk0b0oUNiB3RGpW3DihQ5rQPr+GF6KzkI5VMppPZCoBwgH5jcs8ndKYjgnTVea kECsP7HLmF5j5yR6uAgyeMDhUQVxI/i6MpdluJoNwmffHCqLzNWhixjl/10vZ6R9Kn2W bH0hDXawJlWEegQG9sGQkO/TUBS7J+KlI4mvXAlFsC0PwQh9Dz1yN5M3owKyq9IU0M8q /Pdzs1yRgp3C+wsJ2Ik2EJjFTC267+06/SDR+ISb5t5M1KKb1VEBomITB2nkvqGXhVfK fDiccu/ZILlPP5HZ/PFrSBy+9lDsS1IPXS2osWAvL53a0jQ552OKs1o/eQlh+Qhcnid+ 1AbA== X-Gm-Message-State: ANhLgQ0X8MLbHrLdUm46Dbe/B7WDXuOPkCjVMmIic0T+I5jcJ3K4DR9q F2Q5vV/ug7e/uGN7M0wMFatbpLcstHxbWg== X-Google-Smtp-Source: ADFU+vvYtWQ6GVCF7sQVlsxJIaa3S1b+Hqmwpi6PJb+wBTyENsh0xhoUSyg1GzKr5H16KyPperaILA== X-Received: by 2002:a2e:9a96:: with SMTP id p22mr1551179lji.258.1584610597319; Thu, 19 Mar 2020 02:36:37 -0700 (PDT) Received: from [10.0.0.72] (31-172-191-173.noc.fibertech.net.pl. [31.172.191.173]) by smtp.gmail.com with ESMTPSA id r4sm1243173ljg.36.2020.03.19.02.36.36 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 19 Mar 2020 02:36:36 -0700 (PDT) To: dev@dpdk.org References: <20200306163524.1650-1-pbhagavatula@marvell.com> From: Andrzej Ostruszka Message-ID: Date: Thu, 19 Mar 2020 10:36:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <20200306163524.1650-1-pbhagavatula@marvell.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 1/2] mempool/octeontx2: add devargs to lock ctx in cache X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 3/6/20 5:35 PM, pbhagavatula@marvell.com wrote: > From: Pavan Nikhilesh > > Add device arguments to lock NPA aura and pool contexts in NDC cache. > The device args take hexadecimal bitmask where each bit represent the > corresponding aura/pool id. > Example: > -w 0002:02:00.0,npa_lock_mask=0xf // Lock first 4 aura/pool ctx > > Signed-off-by: Pavan Nikhilesh [...] > +- ``Lock NPA contexts in NDC`` > + > + Lock NPA aura and pool contexts in NDC cache. > + The device args take hexadecimal bitmask where each bit represent the > + corresponding aura/pool id. > + > + For example:: > + -w 0002:0e:00.0,npa_lock_mask=0xf I think you need to make a paragraph break (empty line) after "::" in order to have this example treated as "literal block" (same as max_pool above - not visible in diff). At least it looks so when I build doc with "ninja doc" and check the result in browser. > diff --git a/doc/guides/mempool/octeontx2.rst b/doc/guides/mempool/octeontx2.rst > index 2c9a0953b..c594934d8 100644 > --- a/doc/guides/mempool/octeontx2.rst > +++ b/doc/guides/mempool/octeontx2.rst > @@ -61,6 +61,15 @@ Runtime Config Options > provide ``max_pools`` parameter to the first PCIe device probed by the given > application. > > +- ``Lock NPA contexts in NDC`` > + > + Lock NPA aura and pool contexts in NDC cache. > + The device args take hexadecimal bitmask where each bit represent the > + corresponding aura/pool id. > + > + For example:: > + -w 0002:02:00.0,npa_lock_mask=0xf Ditto. > diff --git a/doc/guides/nics/octeontx2.rst b/doc/guides/nics/octeontx2.rst > index 60187ec72..819d09e11 100644 > --- a/doc/guides/nics/octeontx2.rst > +++ b/doc/guides/nics/octeontx2.rst > @@ -213,6 +213,15 @@ Runtime Config Options > parameters to all the PCIe devices if application requires to configure on > all the ethdev ports. > > +- ``Lock NPA contexts in NDC`` > + > + Lock NPA aura and pool contexts in NDC cache. > + The device args take hexadecimal bitmask where each bit represent the > + corresponding aura/pool id. > + > + For example:: > + -w 0002:02:00.0,npa_lock_mask=0xf Ditto - make that general comment (you might also want to fix other places - not only those introduced). [...] > diff --git a/drivers/common/octeontx2/otx2_common.c b/drivers/common/octeontx2/otx2_common.c > index 1a257cf07..684bb3a0f 100644 > --- a/drivers/common/octeontx2/otx2_common.c > +++ b/drivers/common/octeontx2/otx2_common.c > @@ -169,6 +169,41 @@ int otx2_npa_lf_obj_ref(void) > return cnt ? 0 : -EINVAL; > } > > +static int > +parse_npa_lock_mask(const char *key, const char *value, void *extra_args) > +{ > + RTE_SET_USED(key); > + uint64_t val; > + > + val = strtoull(value, NULL, 16); > + > + *(uint64_t *)extra_args = val; > + > + return 0; > +} > + > +#define OTX2_NPA_LOCK_MASK "npa_lock_mask" > +/* > + * @internal > + * Parse common device arguments > + */ > +void otx2_parse_common_devargs(struct rte_kvargs *kvlist) > +{ > + > + struct otx2_idev_cfg *idev; > + uint64_t npa_lock_mask; Missing initialization of 'npa_lock_mask' - when user does not supply this devarg then no callback is called and you copy this to idev (below). > + > + idev = otx2_intra_dev_get_cfg(); > + > + if (idev == NULL) > + return; > + > + rte_kvargs_process(kvlist, OTX2_NPA_LOCK_MASK, > + &parse_npa_lock_mask, &npa_lock_mask); > + > + idev->npa_lock_mask = npa_lock_mask; > +} [...] > diff --git a/drivers/mempool/octeontx2/otx2_mempool_ops.c b/drivers/mempool/octeontx2/otx2_mempool_ops.c > index ac2d61861..5075b027a 100644 > --- a/drivers/mempool/octeontx2/otx2_mempool_ops.c > +++ b/drivers/mempool/octeontx2/otx2_mempool_ops.c > @@ -348,6 +348,7 @@ npa_lf_aura_pool_init(struct otx2_mbox *mbox, uint32_t aura_id, > struct npa_aq_enq_req *aura_init_req, *pool_init_req; > struct npa_aq_enq_rsp *aura_init_rsp, *pool_init_rsp; > struct otx2_mbox_dev *mdev = &mbox->dev[0]; > + struct otx2_idev_cfg *idev; > int rc, off; > > aura_init_req = otx2_mbox_alloc_msg_npa_aq_enq(mbox); > @@ -379,6 +380,46 @@ npa_lf_aura_pool_init(struct otx2_mbox *mbox, uint32_t aura_id, > return 0; > else > return NPA_LF_ERR_AURA_POOL_INIT; > + > + idev = otx2_intra_dev_get_cfg(); > + if (idev == NULL) > + return 0; Is this not an error? > + > + if (!(idev->npa_lock_mask & BIT_ULL(aura_id))) > + return 0; > + > + aura_init_req = otx2_mbox_alloc_msg_npa_aq_enq(mbox); > + aura_init_req->aura_id = aura_id; > + aura_init_req->ctype = NPA_AQ_CTYPE_AURA; > + aura_init_req->op = NPA_AQ_INSTOP_LOCK; > + > + pool_init_req = otx2_mbox_alloc_msg_npa_aq_enq(mbox); > + if (!pool_init_req) { > + /* The shared memory buffer can be full. > + * Flush it and retry > + */ > + otx2_mbox_msg_send(mbox, 0); > + rc = otx2_mbox_wait_for_rsp(mbox, 0); > + if (rc < 0) { > + otx2_err("Failed to LOCK AURA context"); > + return 0; Same here and below - if these are not errors then maybe do not log them as such. If they are errors then we should probably signal them via return value ("return rc;"). > + } > + > + pool_init_req = otx2_mbox_alloc_msg_npa_aq_enq(mbox); > + if (!pool_init_req) { > + otx2_err("Failed to LOCK POOL context"); > + return 0; See above. > + } > + } > + pool_init_req->aura_id = aura_id; > + pool_init_req->ctype = NPA_AQ_CTYPE_POOL; > + pool_init_req->op = NPA_AQ_INSTOP_LOCK; > + > + rc = otx2_mbox_process(mbox); > + if (rc < 0) > + otx2_err("Failed to lock POOL ctx to NDC"); See above. > + > + return > } > > static int > @@ -390,6 +431,7 @@ npa_lf_aura_pool_fini(struct otx2_mbox *mbox, > struct npa_aq_enq_rsp *aura_rsp, *pool_rsp; > struct otx2_mbox_dev *mdev = &mbox->dev[0]; > struct ndc_sync_op *ndc_req; > + struct otx2_idev_cfg *idev; > int rc, off; > > /* Procedure for disabling an aura/pool */ > @@ -434,6 +476,32 @@ npa_lf_aura_pool_fini(struct otx2_mbox *mbox, > otx2_err("Error on NDC-NPA LF sync, rc %d", rc); > return NPA_LF_ERR_AURA_POOL_FINI; > } > + > + idev = otx2_intra_dev_get_cfg(); > + if (idev == NULL) > + return 0; > + > + if (!(idev->npa_lock_mask & BIT_ULL(aura_id))) > + return 0; Same comments here and below as for *pool_init above. > + > + aura_req = otx2_mbox_alloc_msg_npa_aq_enq(mbox); > + aura_req->aura_id = aura_id; > + aura_req->ctype = NPA_AQ_CTYPE_AURA; > + aura_req->op = NPA_AQ_INSTOP_UNLOCK; > + > + rc = otx2_mbox_process(mbox); > + if (rc < 0) > + otx2_err("Failed to unlock AURA ctx to NDC"); > + > + pool_req = otx2_mbox_alloc_msg_npa_aq_enq(mbox); > + pool_req->aura_id = aura_id; > + pool_req->ctype = NPA_AQ_CTYPE_POOL; > + pool_req->op = NPA_AQ_INSTOP_UNLOCK; > + > + rc = otx2_mbox_process(mbox); > + if (rc < 0) > + otx2_err("Failed to unlock POOL ctx to NDC"); > + > return 0; > } With regards Andrzej Ostruszka