On 5/12/20 5:29 AM, Liu, Yong wrote:
>
>
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coque...@redhat.com>
>> Sent: Tuesday, May 12, 2020 3:50 AM
>> To: Yigit, Ferruh <ferruh.yi...@intel.com>; Wang, Zhihong
>> <zhihong.w...@intel.com>; Ye, Xiaolong <xiaolong...@intel.com>; Liu,
>> Yong <yong....@intel.com>
>> Cc: dev@dpdk.org; Thomas Monjalon <tho...@monjalon.net>; David
>> Marchand <david.march...@redhat.com>; Richardson, Bruce
>> <bruce.richard...@intel.com>; Nicolau, Radu <radu.nico...@intel.com>;
>> Luca Boccassi <bl...@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.coque...@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.coque...@redhat.com>
>>> Signed-off-by: Ferruh Yigit <ferruh.yi...@intel.com>
>>> ---
>>> Cc: Bruce Richardson <bruce.richard...@intel.com>
>>> Cc: Radu Nicolau <radu.nico...@intel.com>
>>> Cc: Luca Boccassi <bl...@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.coque...@redhat.com>
>>
>> Maxime
>
