diff mbox series

[v2] net/virtio: fix AVX512 datapath selection

Message ID 20200511184857.614820-1-ferruh.yigit@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers show
Series [v2] net/virtio: fix AVX512 datapath selection | expand

Checks

Context Check Description
ci/iol-testing fail Testing issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation fail Compilation issues
ci/travis-robot warning Travis build: failed
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Ferruh Yigit May 11, 2020, 6:48 p.m. UTC
From: Maxime Coquelin <maxime.coquelin@redhat.com>

The AVX512 packed ring datapath selection was only done
at build time, but it should also be checked at runtime
that the CPU supports it.

This patch add a CPU flags check so that non-vectorized
path is selected at runtime if AVX512 is not supported.

Also in meson build enable vectorization only for relevant file, not for
all driver.

Fixes: ccb10995c2ad ("net/virtio: add election for vectorized path")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Bruce Richardson <bruce.richardson@intel.com>
Cc: Radu Nicolau <radu.nicolau@intel.com>
Cc: Luca Boccassi <bluca@debian.org>

For meson I mainly adapted implementation from other driver, not able to
test or verify myself.
---
 drivers/net/virtio/meson.build     | 9 +++++++--
 drivers/net/virtio/virtio_ethdev.c | 6 ++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

Comments

Maxime Coquelin May 11, 2020, 7:49 p.m. UTC | #1
On 5/11/20 8:48 PM, Ferruh Yigit wrote:
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> The AVX512 packed ring datapath selection was only done
> at build time, but it should also be checked at runtime
> that the CPU supports it.
> 
> This patch add a CPU flags check so that non-vectorized
> path is selected at runtime if AVX512 is not supported.
> 
> Also in meson build enable vectorization only for relevant file, not for
> all driver.
> 
> Fixes: ccb10995c2ad ("net/virtio: add election for vectorized path")
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Bruce Richardson <bruce.richardson@intel.com>
> Cc: Radu Nicolau <radu.nicolau@intel.com>
> Cc: Luca Boccassi <bluca@debian.org>
> 
> For meson I mainly adapted implementation from other driver, not able to
> test or verify myself.
> ---
>  drivers/net/virtio/meson.build     | 9 +++++++--
>  drivers/net/virtio/virtio_ethdev.c | 6 ++++--
>  2 files changed, 11 insertions(+), 4 deletions(-)

Thanks Ferruh, I cannot test either right now but it looks good to me:

In case you're waiting for it:
Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Maxime
Ferruh Yigit May 11, 2020, 10:21 p.m. UTC | #2
On 5/11/2020 8:49 PM, Maxime Coquelin wrote:
> 
> 
> On 5/11/20 8:48 PM, Ferruh Yigit wrote:
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
>> The AVX512 packed ring datapath selection was only done
>> at build time, but it should also be checked at runtime
>> that the CPU supports it.
>>
>> This patch add a CPU flags check so that non-vectorized
>> path is selected at runtime if AVX512 is not supported.
>>
>> Also in meson build enable vectorization only for relevant file, not for
>> all driver.
>>
>> Fixes: ccb10995c2ad ("net/virtio: add election for vectorized path")
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> Cc: Bruce Richardson <bruce.richardson@intel.com>
>> Cc: Radu Nicolau <radu.nicolau@intel.com>
>> Cc: Luca Boccassi <bluca@debian.org>
>>
>> For meson I mainly adapted implementation from other driver, not able to
>> test or verify myself.
>> ---
>>  drivers/net/virtio/meson.build     | 9 +++++++--
>>  drivers/net/virtio/virtio_ethdev.c | 6 ++++--
>>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> Thanks Ferruh, I cannot test either right now but it looks good to me:
> 
> In case you're waiting for it:
> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 

Applied to dpdk-next-net/master, thanks.
Marvin Liu May 12, 2020, 3:29 a.m. UTC | #3
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, May 12, 2020 3:50 AM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Wang, Zhihong
> <zhihong.wang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>; Liu,
> Yong <yong.liu@intel.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; David
> Marchand <david.marchand@redhat.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>;
> Luca Boccassi <bluca@debian.org>
> Subject: Re: [PATCH v2] net/virtio: fix AVX512 datapath selection
> 
> 
> 
> On 5/11/20 8:48 PM, Ferruh Yigit wrote:
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >
> > The AVX512 packed ring datapath selection was only done
> > at build time, but it should also be checked at runtime
> > that the CPU supports it.
> >
> > This patch add a CPU flags check so that non-vectorized
> > path is selected at runtime if AVX512 is not supported.
> >
> > Also in meson build enable vectorization only for relevant file, not for
> > all driver.
> >
> > Fixes: ccb10995c2ad ("net/virtio: add election for vectorized path")
> >
> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > Cc: Radu Nicolau <radu.nicolau@intel.com>
> > Cc: Luca Boccassi <bluca@debian.org>
> >
> > For meson I mainly adapted implementation from other driver, not able
> to
> > test or verify myself.
> > ---
> >  drivers/net/virtio/meson.build     | 9 +++++++--
> >  drivers/net/virtio/virtio_ethdev.c | 6 ++++--
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> Thanks Ferruh, I cannot test either right now but it looks good to me:
> 

Hi Maxime & Ferruh,
IMHO, meson build update is the essential part for fixing unexpected AVX512 instructions. 
Change in virtio_ethdev may cause building issues on ppc and arm platform.  Is it convenient to revert that change?

Regards,
Marvin

> In case you're waiting for it:
> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Maxime
Maxime Coquelin May 12, 2020, 8:36 a.m. UTC | #4
On 5/12/20 5:29 AM, Liu, Yong wrote:
> 
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Tuesday, May 12, 2020 3:50 AM
>> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Wang, Zhihong
>> <zhihong.wang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>; Liu,
>> Yong <yong.liu@intel.com>
>> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; David
>> Marchand <david.marchand@redhat.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>;
>> Luca Boccassi <bluca@debian.org>
>> Subject: Re: [PATCH v2] net/virtio: fix AVX512 datapath selection
>>
>>
>>
>> On 5/11/20 8:48 PM, Ferruh Yigit wrote:
>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>
>>> The AVX512 packed ring datapath selection was only done
>>> at build time, but it should also be checked at runtime
>>> that the CPU supports it.
>>>
>>> This patch add a CPU flags check so that non-vectorized
>>> path is selected at runtime if AVX512 is not supported.
>>>
>>> Also in meson build enable vectorization only for relevant file, not for
>>> all driver.
>>>
>>> Fixes: ccb10995c2ad ("net/virtio: add election for vectorized path")
>>>
>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>> ---
>>> Cc: Bruce Richardson <bruce.richardson@intel.com>
>>> Cc: Radu Nicolau <radu.nicolau@intel.com>
>>> Cc: Luca Boccassi <bluca@debian.org>
>>>
>>> For meson I mainly adapted implementation from other driver, not able
>> to
>>> test or verify myself.
>>> ---
>>>  drivers/net/virtio/meson.build     | 9 +++++++--
>>>  drivers/net/virtio/virtio_ethdev.c | 6 ++++--
>>>  2 files changed, 11 insertions(+), 4 deletions(-)
>>
>> Thanks Ferruh, I cannot test either right now but it looks good to me:
>>
> 
> Hi Maxime & Ferruh,
> IMHO, meson build update is the essential part for fixing unexpected AVX512 instructions. 
> Change in virtio_ethdev may cause building issues on ppc and arm platform.  Is it convenient to revert that change?

As replied to v1:

With a bit more of context, we can see that it only affects packed ring
when CC_AVX512_SUPPORT is set. So it does break neither split ring nor
ARM/PPC:

	if (vectorized) {
		if (!vtpci_packed_queue(hw)) {
			hw->use_vec_rx = 1;
		} else {
#if !defined(CC_AVX512_SUPPORT)
			PMD_DRV_LOG(INFO,
				"building environment do not support packed ring vectorized");
#else
			if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F)) {
				hw->use_vec_rx = 1;
				hw->use_vec_tx = 1;
			}
#endif
		}
	}

So IMO, no revert has to be done.

> Regards,
> Marvin
> 
>> In case you're waiting for it:
>> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
>> Maxime
>
Marvin Liu May 12, 2020, 8:46 a.m. UTC | #5
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, May 12, 2020 4:36 PM
> To: Liu, Yong <yong.liu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; David
> Marchand <david.marchand@redhat.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>;
> Luca Boccassi <bluca@debian.org>; Wang, Zhihong
> <zhihong.wang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
> Subject: Re: [PATCH v2] net/virtio: fix AVX512 datapath selection
> 
> 
> 
> On 5/12/20 5:29 AM, Liu, Yong wrote:
> >
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: Tuesday, May 12, 2020 3:50 AM
> >> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Wang, Zhihong
> >> <zhihong.wang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>; Liu,
> >> Yong <yong.liu@intel.com>
> >> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; David
> >> Marchand <david.marchand@redhat.com>; Richardson, Bruce
> >> <bruce.richardson@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>;
> >> Luca Boccassi <bluca@debian.org>
> >> Subject: Re: [PATCH v2] net/virtio: fix AVX512 datapath selection
> >>
> >>
> >>
> >> On 5/11/20 8:48 PM, Ferruh Yigit wrote:
> >>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>>
> >>> The AVX512 packed ring datapath selection was only done
> >>> at build time, but it should also be checked at runtime
> >>> that the CPU supports it.
> >>>
> >>> This patch add a CPU flags check so that non-vectorized
> >>> path is selected at runtime if AVX512 is not supported.
> >>>
> >>> Also in meson build enable vectorization only for relevant file, not for
> >>> all driver.
> >>>
> >>> Fixes: ccb10995c2ad ("net/virtio: add election for vectorized path")
> >>>
> >>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>> ---
> >>> Cc: Bruce Richardson <bruce.richardson@intel.com>
> >>> Cc: Radu Nicolau <radu.nicolau@intel.com>
> >>> Cc: Luca Boccassi <bluca@debian.org>
> >>>
> >>> For meson I mainly adapted implementation from other driver, not
> able
> >> to
> >>> test or verify myself.
> >>> ---
> >>>  drivers/net/virtio/meson.build     | 9 +++++++--
> >>>  drivers/net/virtio/virtio_ethdev.c | 6 ++++--
> >>>  2 files changed, 11 insertions(+), 4 deletions(-)
> >>
> >> Thanks Ferruh, I cannot test either right now but it looks good to me:
> >>
> >
> > Hi Maxime & Ferruh,
> > IMHO, meson build update is the essential part for fixing unexpected
> AVX512 instructions.
> > Change in virtio_ethdev may cause building issues on ppc and arm
> platform.  Is it convenient to revert that change?
> 
> As replied to v1:
> 
> With a bit more of context, we can see that it only affects packed ring
> when CC_AVX512_SUPPORT is set. So it does break neither split ring nor
> ARM/PPC:
> 
> 	if (vectorized) {
> 		if (!vtpci_packed_queue(hw)) {
> 			hw->use_vec_rx = 1;
> 		} else {
> #if !defined(CC_AVX512_SUPPORT)
> 			PMD_DRV_LOG(INFO,
> 				"building environment do not support
> packed ring vectorized");
> #else
> 			if
> (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F)) {
> 				hw->use_vec_rx = 1;
> 				hw->use_vec_tx = 1;
> 			}
> #endif
> 		}
> 	}
> 
> So IMO, no revert has to be done.
> 

Ok, I messed it with my previous building fix.  It will be no harm for  this double check. 

> > Regards,
> > Marvin
> >
> >> In case you're waiting for it:
> >> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>
> >> Maxime
> >
Maxime Coquelin May 12, 2020, 10:04 a.m. UTC | #6
On 5/12/20 10:46 AM, Liu, Yong wrote:
> 
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Tuesday, May 12, 2020 4:36 PM
>> To: Liu, Yong <yong.liu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
>> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; David
>> Marchand <david.marchand@redhat.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>;
>> Luca Boccassi <bluca@debian.org>; Wang, Zhihong
>> <zhihong.wang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
>> Subject: Re: [PATCH v2] net/virtio: fix AVX512 datapath selection
>>
>>
>>
>> On 5/12/20 5:29 AM, Liu, Yong wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Sent: Tuesday, May 12, 2020 3:50 AM
>>>> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Wang, Zhihong
>>>> <zhihong.wang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>; Liu,
>>>> Yong <yong.liu@intel.com>
>>>> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; David
>>>> Marchand <david.marchand@redhat.com>; Richardson, Bruce
>>>> <bruce.richardson@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>;
>>>> Luca Boccassi <bluca@debian.org>
>>>> Subject: Re: [PATCH v2] net/virtio: fix AVX512 datapath selection
>>>>
>>>>
>>>>
>>>> On 5/11/20 8:48 PM, Ferruh Yigit wrote:
>>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>>
>>>>> The AVX512 packed ring datapath selection was only done
>>>>> at build time, but it should also be checked at runtime
>>>>> that the CPU supports it.
>>>>>
>>>>> This patch add a CPU flags check so that non-vectorized
>>>>> path is selected at runtime if AVX512 is not supported.
>>>>>
>>>>> Also in meson build enable vectorization only for relevant file, not for
>>>>> all driver.
>>>>>
>>>>> Fixes: ccb10995c2ad ("net/virtio: add election for vectorized path")
>>>>>
>>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>> ---
>>>>> Cc: Bruce Richardson <bruce.richardson@intel.com>
>>>>> Cc: Radu Nicolau <radu.nicolau@intel.com>
>>>>> Cc: Luca Boccassi <bluca@debian.org>
>>>>>
>>>>> For meson I mainly adapted implementation from other driver, not
>> able
>>>> to
>>>>> test or verify myself.
>>>>> ---
>>>>>  drivers/net/virtio/meson.build     | 9 +++++++--
>>>>>  drivers/net/virtio/virtio_ethdev.c | 6 ++++--
>>>>>  2 files changed, 11 insertions(+), 4 deletions(-)
>>>>
>>>> Thanks Ferruh, I cannot test either right now but it looks good to me:
>>>>
>>>
>>> Hi Maxime & Ferruh,
>>> IMHO, meson build update is the essential part for fixing unexpected
>> AVX512 instructions.
>>> Change in virtio_ethdev may cause building issues on ppc and arm
>> platform.  Is it convenient to revert that change?
>>
>> As replied to v1:
>>
>> With a bit more of context, we can see that it only affects packed ring
>> when CC_AVX512_SUPPORT is set. So it does break neither split ring nor
>> ARM/PPC:
>>
>> 	if (vectorized) {
>> 		if (!vtpci_packed_queue(hw)) {
>> 			hw->use_vec_rx = 1;
>> 		} else {
>> #if !defined(CC_AVX512_SUPPORT)
>> 			PMD_DRV_LOG(INFO,
>> 				"building environment do not support
>> packed ring vectorized");
>> #else
>> 			if
>> (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F)) {
>> 				hw->use_vec_rx = 1;
>> 				hw->use_vec_tx = 1;
>> 			}
>> #endif
>> 		}
>> 	}
>>
>> So IMO, no revert has to be done.
>>
> 
> Ok, I messed it with my previous building fix.  It will be no harm for  this double check. 

While it does not break, I agree for the unnecessary double-check.
You can send a clean-up patch to remove this part in -rc3.

Thanks,
Maxime

>>> Regards,
>>> Marvin
>>>
>>>> In case you're waiting for it:
>>>> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>
>>>> Maxime
>>>
>
Marvin Liu May 12, 2020, 1:19 p.m. UTC | #7
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, May 12, 2020 6:04 PM
> To: Liu, Yong <yong.liu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; David
> Marchand <david.marchand@redhat.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>;
> Luca Boccassi <bluca@debian.org>; Wang, Zhihong
> <zhihong.wang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
> Subject: Re: [PATCH v2] net/virtio: fix AVX512 datapath selection
> 
> 
> 
> On 5/12/20 10:46 AM, Liu, Yong wrote:
> >
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: Tuesday, May 12, 2020 4:36 PM
> >> To: Liu, Yong <yong.liu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> >> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; David
> >> Marchand <david.marchand@redhat.com>; Richardson, Bruce
> >> <bruce.richardson@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>;
> >> Luca Boccassi <bluca@debian.org>; Wang, Zhihong
> >> <zhihong.wang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
> >> Subject: Re: [PATCH v2] net/virtio: fix AVX512 datapath selection
> >>
> >>
> >>
> >> On 5/12/20 5:29 AM, Liu, Yong wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>>> Sent: Tuesday, May 12, 2020 3:50 AM
> >>>> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Wang, Zhihong
> >>>> <zhihong.wang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>;
> Liu,
> >>>> Yong <yong.liu@intel.com>
> >>>> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; David
> >>>> Marchand <david.marchand@redhat.com>; Richardson, Bruce
> >>>> <bruce.richardson@intel.com>; Nicolau, Radu
> <radu.nicolau@intel.com>;
> >>>> Luca Boccassi <bluca@debian.org>
> >>>> Subject: Re: [PATCH v2] net/virtio: fix AVX512 datapath selection
> >>>>
> >>>>
> >>>>
> >>>> On 5/11/20 8:48 PM, Ferruh Yigit wrote:
> >>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>>>>
> >>>>> The AVX512 packed ring datapath selection was only done
> >>>>> at build time, but it should also be checked at runtime
> >>>>> that the CPU supports it.
> >>>>>
> >>>>> This patch add a CPU flags check so that non-vectorized
> >>>>> path is selected at runtime if AVX512 is not supported.
> >>>>>
> >>>>> Also in meson build enable vectorization only for relevant file, not
> for
> >>>>> all driver.
> >>>>>
> >>>>> Fixes: ccb10995c2ad ("net/virtio: add election for vectorized path")
> >>>>>
> >>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>>> ---
> >>>>> Cc: Bruce Richardson <bruce.richardson@intel.com>
> >>>>> Cc: Radu Nicolau <radu.nicolau@intel.com>
> >>>>> Cc: Luca Boccassi <bluca@debian.org>
> >>>>>
> >>>>> For meson I mainly adapted implementation from other driver, not
> >> able
> >>>> to
> >>>>> test or verify myself.
> >>>>> ---
> >>>>>  drivers/net/virtio/meson.build     | 9 +++++++--
> >>>>>  drivers/net/virtio/virtio_ethdev.c | 6 ++++--
> >>>>>  2 files changed, 11 insertions(+), 4 deletions(-)
> >>>>
> >>>> Thanks Ferruh, I cannot test either right now but it looks good to me:
> >>>>
> >>>
> >>> Hi Maxime & Ferruh,
> >>> IMHO, meson build update is the essential part for fixing unexpected
> >> AVX512 instructions.
> >>> Change in virtio_ethdev may cause building issues on ppc and arm
> >> platform.  Is it convenient to revert that change?
> >>
> >> As replied to v1:
> >>
> >> With a bit more of context, we can see that it only affects packed ring
> >> when CC_AVX512_SUPPORT is set. So it does break neither split ring nor
> >> ARM/PPC:
> >>
> >> 	if (vectorized) {
> >> 		if (!vtpci_packed_queue(hw)) {
> >> 			hw->use_vec_rx = 1;
> >> 		} else {
> >> #if !defined(CC_AVX512_SUPPORT)
> >> 			PMD_DRV_LOG(INFO,
> >> 				"building environment do not support
> >> packed ring vectorized");
> >> #else
> >> 			if
> >> (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F)) {
> >> 				hw->use_vec_rx = 1;
> >> 				hw->use_vec_tx = 1;
> >> 			}
> >> #endif
> >> 		}
> >> 	}
> >>
> >> So IMO, no revert has to be done.
> >>
> >
> > Ok, I messed it with my previous building fix.  It will be no harm for  this
> double check.
> 
> While it does not break, I agree for the unnecessary double-check.
> You can send a clean-up patch to remove this part in -rc3.
> 

Thanks a lot, have sent clean-up patch.

> Thanks,
> Maxime
> 
> >>> Regards,
> >>> Marvin
> >>>
> >>>> In case you're waiting for it:
> >>>> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>>>
> >>>> Maxime
> >>>
> >
diff mbox series

Patch

diff --git a/drivers/net/virtio/meson.build b/drivers/net/virtio/meson.build
index 5cc529f6aa..3fd6051f4b 100644
--- a/drivers/net/virtio/meson.build
+++ b/drivers/net/virtio/meson.build
@@ -11,8 +11,14 @@  deps += ['kvargs', 'bus_pci']
 if arch_subdir == 'x86'
 	if not machine_args.contains('-mno-avx512f')
 		if cc.has_argument('-mavx512f') and cc.has_argument('-mavx512vl') and cc.has_argument('-mavx512bw')
-			cflags += ['-mavx512f', '-mavx512bw', '-mavx512vl']
 			cflags += ['-DCC_AVX512_SUPPORT']
+			virtio_avx512_lib = static_library('virtio_avx512_lib',
+					      'virtio_rxtx_packed_avx.c',
+					      dependencies: [static_rte_ethdev,
+						static_rte_kvargs, static_rte_bus_pci],
+					      include_directories: includes,
+					      c_args: [cflags, '-mavx512f', '-mavx512bw', '-mavx512vl'])
+			objs += virtio_avx512_lib.extract_objects('virtio_rxtx_packed_avx.c')
 			if (toolchain == 'gcc' and cc.version().version_compare('>=8.3.0'))
 				cflags += '-DVHOST_GCC_UNROLL_PRAGMA'
 			elif (toolchain == 'clang' and cc.version().version_compare('>=3.7.0'))
@@ -20,7 +26,6 @@  if arch_subdir == 'x86'
 			elif (toolchain == 'icc' and cc.version().version_compare('>=16.0.0'))
 				cflags += '-DVHOST_ICC_UNROLL_PRAGMA'
 			endif
-			sources += files('virtio_rxtx_packed_avx.c')
 		endif
 	endif
 	sources += files('virtio_rxtx_simple_sse.c')
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 312871cb48..49ccef12c7 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1965,8 +1965,10 @@  eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 			PMD_DRV_LOG(INFO,
 				"building environment do not support packed ring vectorized");
 #else
-			hw->use_vec_rx = 1;
-			hw->use_vec_tx = 1;
+			if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F)) {
+				hw->use_vec_rx = 1;
+				hw->use_vec_tx = 1;
+			}
 #endif
 		}
 	}