From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh Jain Subject: Re: [PATCH 3/5] common/dpaax: add library for PA VA translation table Date: Tue, 9 Oct 2018 16:15:45 +0530 Message-ID: <2d2e9008-fb5b-3ecd-2d2c-e86250f5d363@nxp.com> References: <20180925125423.7505-1-shreyansh.jain@nxp.com> <20180925125423.7505-4-shreyansh.jain@nxp.com> <894130a9-017c-348d-31f8-c4c23f517f25@nxp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: ferruh.yigit@intel.com, dev@dpdk.org To: "Burakov, Anatoly" Return-path: Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-eopbgr80075.outbound.protection.outlook.com [40.107.8.75]) by dpdk.org (Postfix) with ESMTP id B151F1B4C9 for ; Tue, 9 Oct 2018 12:46:16 +0200 (CEST) In-Reply-To: <894130a9-017c-348d-31f8-c4c23f517f25@nxp.com> Content-Language: en-US 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 Tuesday 25 September 2018 07:09 PM, Shreyansh Jain wrote: > Hello Anatoly, > > On Tuesday 25 September 2018 06:58 PM, Burakov, Anatoly wrote: >> 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. >>> [...] > >> >> Also, a couple of nitpicks below. >> >>>   cosnfig/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. > > I will fix this. > Actually, this function itself is useless - more for symmetry reason. > Callers would be either simply updating the table, or ignoring it > completely. But, yes, this is indeed wrong that I set that unused. > Actually, I was wrong in my first reply. In case of dpaax_iova_table_del(), len is indeed redundant. This is because the mapping is for a complete page (min of 2MB size), even if the request is for lesser length. So, removal of a single entry (of fixed size) would be done. In fact, while on this, I think deleting a PA->VA entry itself is incorrect (not just useless). A single entry (~2MB equivalent) can represent multiple users (working on a rte_malloc'd area, for example). So, effectively, its always an update - not an add or del. I will send updated series with this change. [...]