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 04064A0548; Thu, 15 Jul 2021 12:05:57 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 71E104014D; Thu, 15 Jul 2021 12:05:57 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id E8C5440143 for ; Thu, 15 Jul 2021 12:05:54 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10045"; a="210503550" X-IronPort-AV: E=Sophos;i="5.84,240,1620716400"; d="scan'208";a="210503550" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Jul 2021 03:05:29 -0700 X-IronPort-AV: E=Sophos;i="5.84,240,1620716400"; d="scan'208";a="489192367" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.5.22]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 15 Jul 2021 03:05:24 -0700 Date: Thu, 15 Jul 2021 11:05:19 +0100 From: Bruce Richardson To: Jerin Jacob Cc: Chengwen Feng , Thomas Monjalon , Ferruh Yigit , Jerin Jacob , Andrew Rybchenko , dpdk-dev , Morten =?iso-8859-1?Q?Br=F8rup?= , Nipun Gupta , Hemant Agrawal , Maxime Coquelin , Honnappa Nagarahalli , David Marchand , Satananda Burla , Prasun Kapoor , "Ananyev, Konstantin" Message-ID: References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com> <1626179263-14645-1-git-send-email-fengchengwen@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v3] 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 Thu, Jul 15, 2021 at 11:03:08AM +0100, Bruce Richardson wrote: > On Thu, Jul 15, 2021 at 03:00:01PM +0530, Jerin Jacob wrote: > > On Thu, Jul 15, 2021 at 2:33 PM Bruce Richardson > > wrote: > > > > > > On Thu, Jul 15, 2021 at 12:40:01PM +0530, Jerin Jacob wrote: > > > > ) > > > > a > > > > > > > > On Tue, Jul 13, 2021 at 6:01 PM Chengwen Feng wrote: > > > > > > > > > > This patch introduce 'dmadevice' which is a generic type of DMA > > > > > device. > > > > > > > > > > The APIs of dmadev library exposes some generic operations which can > > > > > enable configuration and I/O with the DMA devices. > > > > > > > > > > Signed-off-by: Chengwen Feng > > > > > > > > Thanks for v3. Seems like all major items as covered. Some more > > > > comments below inline. > > > > > > > > I would suggest v4 to split the patch like (so that we can review and > > > > ack each patch) > > > > 1) Only public header file with Doxygen inclusion, (There is a lot of > > > > Doxygen syntax issue in the patch) > > > > 2) 1 or more patches for implementation. > > > > > > > > > > One additional follow-up comment on flags below. > > > > > > /Bruce > > > > > > > > > > > > diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h > > > > > new file mode 100644 > > > > > index 0000000..f6cc4e5 > > > > > > > > + enum rte_dmadev_port_type port_type; > > > > missing doxgen comment for this. > > > > > + union { > > > > > + /** For PCIE port > > > > > + * > > > > > + * The following model show SoC's PCIE module connects to > > > > > + * multiple PCIE hosts and multiple endpoints. The PCIE module > > > > > + * has an integrate DMA controller. > > > > > + * If the DMA wants to access the memory of host A, it can be > > > > > + * initiated by PF1 in core0, or by VF0 of PF0 in core0. > > > > > + * > > > > > > > + /** The pasid filed in TLP packet */ > > > > > + uint64_t pasid : 20; > > > > > + /** The attributes filed in TLP packet */ > > > > > + uint64_t attr : 3; > > > > > + /** The processing hint filed in TLP packet */ > > > > > + uint64_t ph : 2; > > > > > + /** The steering tag filed in TLP packet */ > > > > > + uint64_t st : 16; > > > > > > > > We don't support a few attributes like passid, ph, st. Do we need > > > > the capability of this? or ignore this. In either case, please update the doc. > > > > > > > > We also support additional flags for allocating LLC flag. > > > > This is a hint to DMA engine that the cache blocks should be allocated > > > > in the LLC (if they were not already). > > > > When the MEM pointer is a destination in DMA operation, the referenced > > > > cache blocks are allocated into the cache as part of completing the > > > > DMA (when not already present in the LLC) > > > > this is helpful if software has to access the data right after dma is completed. > > > > > > > > Could you add bit or flag for the same? > > > > > > > > > > I wonder if this is the best location for such a flag for LLC vs memory > > > writes. It would also apply to memory-to-memory transactions, not just for > > > those done to PCI devices. > > > > Ack. it can be used for MEM to MEM > > > > > As well as that, I think any flag should default > > > to "on" rather than "off" since writing to cache rather than DRAM is > > > generally the desired behaviour, I would think. > > > > I think, keeping it is "allocate in LLC" on all transfer will not be good. > > As large transters polute the LLC and dataplane may not touch the complete > > data only header. Also in device copy, Adding it LLC there is an > > additional cost unline MEM-MEM. > > > > So IMO, better to add the flag to allow to allocate to LLC as a HINT. > > > > > Should it be a per-operation flag, rather than per context? > > > > Yes. better it be per-operation as it is the hint. > > > Ok. Let's define a new per-op flag for LLC allocation, and keep default > (without flag) as no-alloc. [Apologies for self-reply] Let's also be clear in the documentation for the flag that this is a HINT, and that drivers may not follow this. That way we don't need to add a capability flag for it, or to return error from a function which doesn't support it, etc. etc.