From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id CD1491B12B for ; Tue, 25 Sep 2018 15:28:30 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Sep 2018 06:28:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,302,1534834800"; d="scan'208";a="265560160" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.55]) ([10.237.220.55]) by fmsmga005.fm.intel.com with ESMTP; 25 Sep 2018 06:28:21 -0700 To: Shreyansh Jain , ferruh.yigit@intel.com Cc: dev@dpdk.org References: <20180925125423.7505-1-shreyansh.jain@nxp.com> <20180925125423.7505-4-shreyansh.jain@nxp.com> From: "Burakov, Anatoly" Message-ID: Date: Tue, 25 Sep 2018 14:28:20 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180925125423.7505-4-shreyansh.jain@nxp.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 3/5] common/dpaax: add library for PA VA translation table 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: , X-List-Received-Date: Tue, 25 Sep 2018 13:28:31 -0000 On 25-Sep-18 1:54 PM, Shreyansh Jain wrote: > A common library, valid for dpaaX drivers, which is used to maintain > a local copy of PA->VA translations. > > In case of physical addressing mode (one of the option for FSLMC, and > only option for DPAA bus), the addresses of descriptors Rx'd are > physical. These need to be converted into equivalent VA for rte_mbuf > and other similar calls. > > Using the rte_mem_virt2iova or rte_mem_virt2phy is expensive. This > library is an attempt to reduce the overall cost associated with > this translation. > > A small table is maintained, containing continuous entries > representing a continguous physical range. Each of these entries > stores the equivalent VA, which is fed during mempool creation, or > memory allocation/deallocation callbacks. > > Signed-off-by: Shreyansh Jain > --- Hi Shreyansh, So, basically, you're reimplementing old DPDK's memory view (storing VA's in a PA-centric way). Makes sense :) I should caution you that right now, external memory allocator implementation does *not* trigger any callbacks for newly added memory. So, anything coming from external memory will not be reflected in your table, unless it happens to be already there before dpaax_iova_table_populate() gets called. This patchset makes a good argument for why perhaps it should trigger callbacks. Thoughts? Also, a couple of nitpicks below. > config/common_base | 5 + > config/common_linuxapp | 5 + > drivers/common/Makefile | 4 + > drivers/common/dpaax/Makefile | 31 ++ > drivers/common/dpaax/dpaax_iova_table.c | 509 ++++++++++++++++++ > drivers/common/dpaax/dpaax_iova_table.h | 104 ++++ > drivers/common/dpaax/dpaax_logs.h | 39 ++ > drivers/common/dpaax/meson.build | 12 + > + DPAAX_DEBUG("Add: Found slot at (%"PRIu64")[(%zu)] for vaddr:(%p)," > + " phy(%"PRIu64"), len(%zu)", entry[i].start, e_offset, > + vaddr, paddr, length); > + return 0; > +} > + > +int > +dpaax_iova_table_del(phys_addr_t paddr, size_t len __rte_unused) len is not unused. > +{ > + int found = 0; > + unsigned int i; > + size_t e_offset; > + struct dpaax_iovat_element *entry; > + phys_addr_t align_paddr; > + > + align_paddr = paddr & DPAAX_MEM_SPLIT_MASK; > + > + /* Check if paddr is available in table */ > +static inline void * > +dpaax_iova_table_get_va(phys_addr_t paddr) { > + unsigned int i = 0, index; > + void *vaddr = 0; > + phys_addr_t paddr_align = paddr & DPAAX_MEM_SPLIT_MASK; > + size_t offset = paddr & DPAAX_MEM_SPLIT_MASK_OFF; > + struct dpaax_iovat_element *entry; > + > + entry = dpaax_iova_table_p->entries; > + > + do { > + if (unlikely(i > dpaax_iova_table_p->count)) > + break; > + > + if (paddr_align < entry[i].start) { > + /* Incorrect paddr; Not in memory range */ > + return 0; NULL? -- Thanks, Anatoly