Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 04-Nov-19 5:34 PM, Damjan Marion (damarion) wrote:
> 
> 
>> On 4 Nov 2019, at 18:27, Burakov, Anatoly <anatoly.burakov@intel.com> wrote:
>>
>> On 04-Nov-19 1:57 PM, Damjan Marion (damarion) wrote:
>>>> On 25 Oct 2019, at 15:02, Damjan Marion (damarion) <damarion@cisco.com <mailto:damarion@cisco.com>> wrote:
>>>>
>>>>
>>>>
>>>>> On 25 Oct 2019, at 14:23, Burakov, Anatoly <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>> wrote:
>>>>>
>>>>> On 25-Oct-19 12:13 PM, Damjan Marion (damarion) wrote:
>>>>>>> On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net <mailto:thomas@monjalon.net>> wrote:
>>>>>>>
>>>>>>> 24/10/2019 21:09, David Marchand:
>>>>>>>> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
>>>>>>>> <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>> wrote:
>>>>>>>>>
>>>>>>>>> The rte_vfio_dma_map/unmap API's have been marked as deprecated in
>>>>>>>>> release 19.05. Remove them.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Notes:
>>>>>>>>>     Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
>>>>>>>>>     it wasn't marked as __rte_deprecated in code. Should we still remove it?
>>>>>>>>
>>>>>>>> I can see that vpp is still using this api.
>>>>>>>> I would prefer we get some ack from their side.
>>>>>>>>
>>>>>>>> Shahaf?
>>>>>>>> Ray?
>>>>>>>>
>>>>>>>> Do you guys have contact with VPP devs?
>>>>>>>
>>>>>>> +Cc Damjan
>>>>>> Thanks for looping me in. If I remember correctly that was used only to get mlx PMDs working.
>>>>>> We can remove that calls but then mlx PMDs will stop working unless there is alternative solution.
>>>>>>  From my perspective it is not big issue as we already have native rdma based mlx support, but i would expect that other people will complain.
>>>>>> Is there alternative way to tell DPDK about DMA mapping?
>>>>>
>>>>> The rte_vfio_container_dma_map(VFIO_DEFAULT_CONTAINER, ...) is the exact equivalent of the functions being removed. Also, rte_dev_dma_map() is supposed to be the more general DMA mapping API that works with VFIO and with any other bus/device-specific DMA mapping.
>>>>>
>>>>> So yes, a simple search and replace for "rte_vfio_dma_(un)?map(" to "rte_vfio_container_dma_(un)?map(VFIO_DEFAULT_CONTAINER, " should trigger exactly the same behavior.
>>>>
>>>> Done, will be merged after it passes verify jobs…
>>>>
>>>> https://gerrit.fd.io/r/c/vpp/+/22982
>>> I just got report that this patch breaks some tests. Is it RTE_VFIO_DEFAULT_CONTAINER_FD right value to use here?
>>> Maybe i wrongly assumed that when you said VFIO_DEFAULT_CONTAINER, you meant RTE_VFIO_DEFAULT_CONTAINER_FD…
>>> —
>>> Damjan
>> Yes, i think i can see the bug. Can you rerun the failing test after applying the following patch?
>>
>> diff --git a/lib/librte_eal/linux/eal/eal_vfio.c b/lib/librte_eal/linux/eal/eal_vfio.c
>> index d9541b1220..d7887388f9 100644
>> --- a/lib/librte_eal/linux/eal/eal_vfio.c
>> +++ b/lib/librte_eal/linux/eal/eal_vfio.c
>> @@ -412,6 +412,9 @@ get_vfio_cfg_by_container_fd(int container_fd)
>> {
>> 	int i;
>>
>> +	if (container_fd == RTE_VFIO_DEFAULT_CONTAINER_FD)
>> +		return default_vfio_cfg;
>> +
>> 	for (i = 0; i < VFIO_MAX_CONTAINERS; i++) {
>> 		if (vfio_cfgs[i].vfio_container_fd == container_fd)
>> 			return &vfio_cfgs[i];
>>
>>
>> The problem seems to be that we're looking at actual fd, whereas the RTE_VFIO_DEFAULT_CONTAINER_FD value is -1, which will not match anything in that list.
> 
> That was exactly my reading, but I didn’t want to rush into conclusion. Will ask guys to test…
> 

This should make it easier to test:

http://patches.dpdk.org/patch/62390/

-- 
Thanks,
Anatoly