From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A6E98A0C41; Thu, 24 Jun 2021 08:49:58 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 21A2040141; Thu, 24 Jun 2021 08:49:58 +0200 (CEST) Received: from mail-il1-f182.google.com (mail-il1-f182.google.com [209.85.166.182]) by mails.dpdk.org (Postfix) with ESMTP id 73D944003C for ; Thu, 24 Jun 2021 08:49:56 +0200 (CEST) Received: by mail-il1-f182.google.com with SMTP id z1so5112494ils.0 for ; Wed, 23 Jun 2021 23:49:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=shS6HskGeMviLHPK8cMjH/0tv/Mjolozn1BhM6LGWfQ=; b=vNnZ4h+HB3cjDdBSLEHJTNUMi8lap5VJS4rMQtO1eEF4MgjSpiUKXoFtEIKDiqSWLl 6Grne7qkL3RMPa8zmRFNADV+zYOcE9O13yWxcR+tm/YmBin0D59q0M88v3N8SZTIn+PK q1YXabn+L1nw6QsDtCXuT8DtNerpbMWk83WiXUDnNFDE1rmnAww53nh7ZDCDsdam35Sn VmJAlCO3/iaTnIOooPopukuWDCsdw3eKeAQ2TqamT86v5m9h6uB/jAkQjsWnc46n2wXY /pLan54mG7zxGRHDDcjCl0CCIifWla24sCtXfbGBdqYo/uT5GR2efwDOhu299RKdtw1M si9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=shS6HskGeMviLHPK8cMjH/0tv/Mjolozn1BhM6LGWfQ=; b=kGX8UX2tDvLKLewV8UqJCZyk+HMu7bmkVHCvnzqHAENVYH9d0+yqaTnnxhy2jbVZSC 9qczczGyiHBQeZf5CT0HMjAWgYnVIwBaFcDIRAAkcCBIPW6ZrpPopVbNLUE7HqvT4VMI HBbOSNC6yTF1M1sAIx/CKkbOPkmIC0gNyJ3GON8o95jwqqGECrG+l89oY6sCyxncCxuk TVjNjfxaXYeb34qGUNEQDVaw/aBa3tDtKQOtWehkFAhlhwfV7fCrsiytNMX9AZDifykN ysimyK5zdW9HfrLOo282fTMCbzB8B0GZapPiuuRFH72DmCRVgi7XaX4uKOv2JpVglfjL PjsQ== X-Gm-Message-State: AOAM532OTW1AFWkzrl/T3TNplzzFsl3xWt1G85f950fPF06tFR33vh71 pQ2rUWEJAWyEkHhImTslqzLvE/mt6TcxpZC3snQ= X-Google-Smtp-Source: ABdhPJzDu7g5Rm5T8uHO2pwVlrJYucGJbPpM6Vg+NxdzxB/fFnmjksYS955DFGO8SzTuBOuioAMs9vREk18Hc0cdzh0= X-Received: by 2002:a92:1310:: with SMTP id 16mr2688567ilt.60.1624517395619; Wed, 23 Jun 2021 23:49:55 -0700 (PDT) MIME-Version: 1.0 References: <3cb0bd01-2b0d-cf96-d173-920947466041@huawei.com> In-Reply-To: From: Jerin Jacob Date: Thu, 24 Jun 2021 12:19:29 +0530 Message-ID: To: Bruce Richardson Cc: fengchengwen , =?UTF-8?Q?Morten_Br=C3=B8rup?= , Thomas Monjalon , Ferruh Yigit , dpdk-dev , Nipun Gupta , Hemant Agrawal , Maxime Coquelin , Honnappa Nagarahalli , Jerin Jacob , David Marchand , Satananda Burla , Prasun Kapoor Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [RFC PATCH] dmadev: introduce DMA device library X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 Wed, Jun 23, 2021 at 7:50 PM Bruce Richardson wrote: > > On Wed, Jun 23, 2021 at 05:10:22PM +0530, Jerin Jacob wrote: > > On Wed, Jun 23, 2021 at 3:07 PM Bruce Richardson > > wrote: > > > > > > On Wed, Jun 23, 2021 at 12:51:07PM +0530, Jerin Jacob wrote: > > > > On Wed, Jun 23, 2021 at 9:00 AM fengchengwen wrote: > > > > > > > > > > > > > > > > > Currently, it is hard to define generic dma descriptor, I think the well-defined > > > > > APIs is feasible. > > > > > > > > I would like to understand why not feasible? if we move the > > > > preparation to the slow path. > > > > > > > > i.e > > > > > > > > struct rte_dmadev_desc defines all the "attributes" of all DMA devices available > > > > using capability. I believe with the scheme, we can scale and > > > > incorporate all features of > > > > all DMA HW without any performance impact. > > > > > > > > something like: > > > > > > > > struct rte_dmadev_desc { > > > > /* Attributes all DMA transfer available for all HW under capability. */ > > > > channel or port; > > > > ops ; // copy, fill etc.. > > > > /* impemention opqueue memory as zero length array, > > > > rte_dmadev_desc_prep() update this memory with HW specific information > > > > */ > > > > uint8_t impl_opq[]; > > > > } > > > > > > > > // allocate the memory for dma decriptor > > > > struct rte_dmadev_desc *rte_dmadev_desc_alloc(devid); > > > > // Convert DPDK specific descriptors to HW specific descriptors in slowpath */ > > > > rte_dmadev_desc_prep(devid, struct rte_dmadev_desc *desc); > > > > // Free dma descriptor memory > > > > rte_dmadev_desc_free(devid, struct rte_dmadev_desc *desc ) > > > > > > > > The above calls in slow path. > > > > > > > > Only below call in fastpath. > > > > // Here desc can be NULL(in case you don't need any specific attribute > > > > attached to transfer, if needed, it can be an object which is gone > > > > through rte_dmadev_desc_prep()) > > > > rte_dmadev_enq(devid, struct rte_dmadev_desc *desc, void *src, void > > > > *dest, unsigned int len, cookie) > > > > > > > > > > The trouble here is the performance penalty due to building up and tearing > > > down structures and passing those structures into functions via function > > > pointer. With the APIs for enqueue/dequeue that have been discussed here, > > > all parameters will be passed in registers, and then each driver can do a > > > write of the actual hardware descriptor straight to cache/memory from > > > registers. With the scheme you propose above, the register contains a > > > pointer to the data which must then be loaded into the CPU before being > > > written out again. This increases our offload cost. > > > > See below. > > > > > > > > However, assuming that the desc_prep call is just for slowpath or > > > initialization time, I'd be ok to have the functions take an extra > > > hw-specific parameter for each call prepared with tx_prep. It would still > > > allow all other parameters to be passed in registers. How much data are you > > > looking to store in this desc struct? It can't all be represented as flags, > > > for example? > > > > There is around 128bit of metadata for octeontx2. New HW may > > completely different metata > > http://code.dpdk.org/dpdk/v21.05/source/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.h#L149 > > > > I see following issue with flags scheme: > > > > - We need to start populate in fastpath, Since it based on capabality, > > application needs to have > > different versions of fastpath code > > - Not future proof, Not easy add other stuff as needed when new HW > > comes with new > > transfer attributes. > > > > > > Understood. Would the "tx_prep" (or perhaps op_prep) function you proposed > solve that problem, if it were passed (along with flags) to the > enqueue_copy function? i.e. would the below work for you, and if so, what > parameters would you see passed to the prep function? Below prototype loooks good to me. But we need make sure we need to encode the items the "flags" needs to supported by all PMDs aka it should be generic. I.e application should not have the capability check in fastpath for flags. > > metad = rte_dma_op_prep(dev_id, ....) > > rte_dma_enqueue_copy(dev_id, src, dst, len, flags, metad) > > > > > > > > As for the individual APIs, we could do a generic "enqueue" API, which > > > takes the op as a parameter, I prefer having each operation as a separate > > > function, in order to increase the readability of the code and to reduce > > > > Only issue I see, all application needs have two path for doing the stuff, > > one with _prep() and separate function() and drivers need to support both. > > > If prep is not called per-op, we could always mandate it be called before > the actual enqueue functions, even if some drivers ignore the argument. Thats works. > > > > the number of parameters needed per function i.e. thereby saving registers > > > needing to be used and potentially making the function calls and offload > > > > My worry is, struct rte_dmadev can hold only function pointers for <= > > 8 fastpath functions for 64B cache line. > > When you say new op, say fill, need a new function, What will be the > > change wrt HW > > driver point of view? Is it updating HW descriptor with op as _fill_ > > vs _copy_? something beyond that? > > Well, from a user view-point each operation takes different parameters, so > for a fill operation, you have a destination address, but instead of a > source address for copy you have pattern for fill. OK. > > Internally, for those two ops, the only different in input and descriptor > writing is indeed in the op flag, and no additional context or metadata is > needed for a copy other than source, address and length (+ plus maybe some > flags e.g. for caching behaviour or the like), so having the extra prep > function adds no value for us, and loading data from a prebuilt structure > just adds more IO overhead. Therefore, I'd ok to add it for enabling other > hardware, but only in such a way as it doesn't impact the offload cost. > > If we want to in future look at adding more advanced or complex > capabilities, I'm ok for adding a general "enqueue_op" function which takes > multiple op types, but for very simple ops like copy, where we need to keep > the offload cost down to a minimum, having fastpath specific copy functions > makes a lot of sense to me. OK. > > > If it is about, HW descriptor update, then _prep() can do all work, > > just driver need to copy desc to > > to HW. > > > > I believe upto to 6 arguments passed over registers in x86(it is 8 in > > arm64). if so, > > the desc pointer(already populated in HW descriptor format by _prep()) > > is in register, and > > would be simple 64bit/128bit copy from desc pointer to HW memory on > > driver enq(). I dont see > > any overhead on that, On other side, we if keep adding arguments, it > > will spill out > > to stack. > > > For a copy operation, we should never need more than 6 arguments - see > proposal above which has 6 including a set of flags and arbitrary void * > pointer for extensibility. If anything more complex than that is needed, > the generic "enqueue_op" function can be used instead. Let's fast-path the > common, simple case, since that what is most likely to be used most! OK. > > /Bruce