[dpdk-dev] [PATCH 3/5] common/dpaax: add library for PA VA translation table

[Shreyansh Jain]

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 +
>>
>> <snip>
>>
>>> +    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.

[...]