Re: [dpdk-dev] [PATCH] vfio: retry creating sPAPR DMA window

["Takeshi T Yoshimura" <TYOS@jp.ibm.com>]

-----"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote: -----

>To: Thomas Monjalon <thomas@monjalon.net>
>From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
>Date: 07/10/2019 09:01AM
>Cc: Takeshi Yoshimura <tyos@jp.ibm.com>, dev@dpdk.org, David
>Christensen <drc@linux.vnet.ibm.com>
>Subject: [EXTERNAL] Re: [dpdk-dev] [PATCH] vfio: retry creating sPAPR
>DMA window
>
>On 10-Jul-19 1:39 PM, Thomas Monjalon wrote:
>> 10/07/2019 14:35, Burakov, Anatoly:
>>> On 10-Jul-19 1:17 PM, Thomas Monjalon wrote:
>>>> 10/07/2019 12:32, Burakov, Anatoly:
>>>>> On 07-Jun-19 3:28 AM, Takeshi Yoshimura wrote:
>>>>>> sPAPR allows only page_shift from VFIO_IOMMU_SPAPR_TCE_GET_INFO
>ioctl.
>>>>>> However, Linux 4.17 or before returns incorrect page_shift for
>Power9.
>>>>>> I added the code for retrying creation of sPAPR DMA window.
>>>>>>
>>>>>> Signed-off-by: Takeshi Yoshimura <tyos@jp.ibm.com>
>>>>>> ---
>>>>>>     lib/librte_eal/linux/eal/eal_vfio.c | 26
>+++++++++++++++++++++++---
>>>>>>     1 file changed, 23 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/lib/librte_eal/linux/eal/eal_vfio.c
>b/lib/librte_eal/linux/eal/eal_vfio.c
>>>>>> index 6892a2c14..f16c5c3c0 100644
>>>>>> --- a/lib/librte_eal/linux/eal/eal_vfio.c
>>>>>> +++ b/lib/librte_eal/linux/eal/eal_vfio.c
>>>>>> @@ -1448,9 +1448,29 @@ vfio_spapr_create_new_dma_window(int
>vfio_container_fd,
>>>>>>     	/* create new DMA window */
>>>>>>     	ret = ioctl(vfio_container_fd,
>VFIO_IOMMU_SPAPR_TCE_CREATE, create);
>>>>>>     	if (ret) {
>>>>>> -		RTE_LOG(ERR, EAL, "  cannot create new DMA window, "
>>>>>> -				"error %i (%s)\n", errno, strerror(errno));
>>>>>> -		return -1;
>>>>>> +		/* try possible page_shift and levels for workaround */
>>>>>> +		uint32_t levels;
>>>>>> +
>>>>>> +		for (levels = 1; levels <= info.ddw.levels; levels++) {
>>>>>> +			uint32_t pgsizes = info.ddw.pgsizes;
>>>>>
>>>>> +CC POWER maintainer from MAINTAINERS file.
>>>>>
>>>>> This is failing compilation on some older distros (Fedora 20,
>Ubuntu 14.04):
>>>>>
>>>>>
>>>>> /root/dpdk/lib/librte_eal/linux/eal/eal_vfio.c: In function
>>>>> ‘vfio_spapr_create_new_dma_window’:
>>>>> /root/dpdk/lib/librte_eal/linux/eal/eal_vfio.c:1451:34: error:
>‘struct
>>>>> vfio_iommu_spapr_tce_info’ has no member named ‘ddw’
>>>>> for (levels = 1; levels <= info.ddw.levels; levels++) {
>>>>> ^
>>>>> /root/dpdk/lib/librte_eal/linux/eal/eal_vfio.c:1452:27: error:
>‘struct
>>>>> vfio_iommu_spapr_tce_info’ has no member named ‘ddw’
>>>>> uint32_t pgsizes = info.ddw.pgsizes;
>>>>> ^
>>>>> /root/dpdk/lib/librte_eal/linux/eal/eal_vfio.c: At top level:
>>>>> cc1: error: unrecognized command line option
>>>>> "-Wno-address-of-packed-member" [-Werror]
>>>>> cc1: all warnings being treated as errors
>>>>> make[7]: *** [eal_vfio.o] Error 1
>>>>> make[7]: *** Waiting for unfinished jobs....
>>>>> make[6]: *** [eal] Error 2
>>>>> make[5]: *** [linux] Error 2
>>>>> make[4]: *** [librte_eal] Error 2
>>>>> make[3]: *** [lib] Error 2
>>>>> make[2]: *** [all] Error 2
>>>>> make[1]: *** [pre_install] Error 2
>>>>> make: *** [install] Error 2
>>>>>
>>>>>
>>>>> Looking at the eal_vfio.h we are handling case of missing sPAPR
>info/ddw
>>>>> structures, but this isn't simply a case of a missing struct;
>rather it
>>>>> is an issue of differing definitions between what recent kernels
>have,
>>>>> and what older kernels had.
>>>>>
>>>>> @Thomas Do we still support these older distros?
>>>>
>>>> If the kernel is not maintained, we don't support it.
>>>> For most distros, Linux < 3.16 is not maintained.
>>>
>>> This is on kernel 3.13 (Ubuntu 14.04 LTS which according to
>Canonical's
>>> published schedule [1] has stopped receiving even maintenance
>updates,
>>> and is only updated for select customers). It was never an LTS
>kernel to
>>> begin with, i think.
>> 
>> So we can ignore this issue.
>> 
>
>It seems that it's also reproducible on Ubuntu 14.04 with kernel
>3.19. 
>Which is not an upstream LTS kernel, but it's >3.16.
>
>It also looks like it may be an issue for us specifically, so i would
>
>suggest reverting the patch for rc2 if the submitter/maintainers
>don't 
>come back and fix it.
>
>-- 
>Thanks,
>Anatoly
>
>

Hi Anatoly,
Thank you for your report and analysis. This can be avoided by adding #ifdef VFIO_IOMMU_SPAPR_INFO_DDW.
I will quickly submit the fix soon after this email.

Regards,
Takeshi