diff mbox series

vfio: retry creating sPAPR DMA window

Message ID 20190607022830.2044-1-tyos@jp.ibm.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series vfio: retry creating sPAPR DMA window |

Checks

Context Check Description
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Takeshi Yoshimura June 7, 2019, 2:28 a.m. UTC 
  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(-)

Comments

Thomas Monjalon July 4, 2019, 4:01 p.m. UTC | #1 
Please, any review or ack for this patch?

07/06/2019 04:28, Takeshi Yoshimura:
> 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(-)
Burakov, Anatoly July 5, 2019, 8:15 a.m. UTC | #2 
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>
> ---

This doesn't affect any code outside of sPAPR and looks sane, so

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Thomas Monjalon July 7, 2019, 9:21 p.m. UTC | #3 
05/07/2019 10:15, 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>
> > ---
> 
> This doesn't affect any code outside of sPAPR and looks sane, so
> 
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks
David Christensen July 8, 2019, 4:47 p.m. UTC | #4 
> Please, any review or ack for this patch?
> 
> 07/06/2019 04:28, Takeshi Yoshimura:
>> 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(-)
> 

I've shared my comments with Takeshi and I don't think the patch is 
ready in it's current state.  It appears to modify x86 functionality and 
I'm not comfortable with signing off on that since it's in an anrea I'm 
not familiar with.

Dave
Thomas Monjalon July 8, 2019, 5:45 p.m. UTC | #5 
08/07/2019 18:47, David Christensen:
> > Please, any review or ack for this patch?
> > 
> > 07/06/2019 04:28, Takeshi Yoshimura:
> >> 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>
> 
> I've shared my comments with Takeshi and I don't think the patch is 
> ready in it's current state.  It appears to modify x86 functionality and 
> I'm not comfortable with signing off on that since it's in an anrea I'm 
> not familiar with.

It is a function specific to sPAPR.
So there is no risk for x86 or Arm.
As you didn't share your comments publicly,
this patch has been merged in 19.08-rc1.
Burakov, Anatoly July 9, 2019, 10:22 a.m. UTC | #6 
On 08-Jul-19 6:45 PM, Thomas Monjalon wrote:
> 08/07/2019 18:47, David Christensen:
>>> Please, any review or ack for this patch?
>>>
>>> 07/06/2019 04:28, Takeshi Yoshimura:
>>>> 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>
>>
>> I've shared my comments with Takeshi and I don't think the patch is
>> ready in it's current state.  It appears to modify x86 functionality and
>> I'm not comfortable with signing off on that since it's in an anrea I'm
>> not familiar with.
> 
> It is a function specific to sPAPR.
> So there is no risk for x86 or Arm.
> As you didn't share your comments publicly,
> this patch has been merged in 19.08-rc1.
> 

I have a suspicion that David has confused this patch with another one, 
which did indeed touch a lot of code (but still didn't touch x86 AFAIR).
Burakov, Anatoly July 10, 2019, 10:32 a.m. UTC | #7 
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?
Thomas Monjalon July 10, 2019, 12:17 p.m. UTC | #8 
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.
Burakov, Anatoly July 10, 2019, 12:35 p.m. UTC | #9 
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.

[1] https://ubuntu.com/about/release-cycle
Thomas Monjalon July 10, 2019, 12:39 p.m. UTC | #10 
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.
Burakov, Anatoly July 10, 2019, 4:01 p.m. UTC | #11 
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.
David Christensen July 11, 2019, 12:50 a.m. UTC | #12 
>>>>> @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.

Just to confirm, these failures are on x86 builds, is that correct? 
Power 9 systems aren't supported by these older distros, Linux kernel 
4.6 was the first kernel with Power 9 support.

Dave
Takeshi Yoshimura July 11, 2019, 2:18 a.m. UTC | #13 
-----"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
diff mbox series

Patch

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;
+
+			while (pgsizes != 0) {
+				create->page_shift = 31 - __builtin_clz(pgsizes);
+				create->levels = levels;
+				ret = ioctl(vfio_container_fd,
+					VFIO_IOMMU_SPAPR_TCE_CREATE, create);
+				if (!ret)
+					break;
+				pgsizes &= ~(1 << create->page_shift);
+			}
+			if (!ret)
+				break;
+		}
+		if (ret) {
+			RTE_LOG(ERR, EAL, "  cannot create new DMA window, "
+					"error %i (%s)\n", errno, strerror(errno));
+			return -1;
+		}
 	}
 
 	if (create->start_addr != 0) {