On Mon, Feb 15, 2016 at 04:48:36PM +0530, Santosh Shukla wrote:
> Hi Yuanhan,
>
> On Mon, Feb 15, 2016 at 4:27 PM, Yuanhan Liu
> <yuanhan.liu at linux.intel.com> wrote:
> > On Mon, Feb 15, 2016 at 03:22:11PM +0530, Santosh Shukla wrote:
> >> Hi Yuanhan,
> >>
> >> I guess you are back from vacation.
> >>
> >> Can you pl. review this patch, Except this patch, rest of patches
> >> received ack-by:
> >
> > I had a quick glimpse of the comments from Thomas: he made a good point.
> > I will have a deeper thought tomorrow, to see what I can do to fix it.
> >
>
> I agree to what Thomas pointed out about runtime mode switch (vectored
> vs non-vectored). I have a proposal in my mind and Like to know you
> opinion:
>
> - need for apis like is_arch_support_vec().
>
> if (is_arch_support_vec())
> simpple_xxxx = 1 /* Switch code path to vector mode */
> else
> simple_xxxx = 0 /* Switch code path to non-vector mode */
>
> That api should reside to arch file. i.e.. arch like i686/arm{for
> implementation not exist so say no supported} will return 0 and for
> x86_64 = 1
I was thinking that Thomas meant to something like below (like what
we did at rte_memcpy.h):
#ifdef RTE_MACHINE_CPUFLAG_SSE (or whatever)
/* with vec here */
#else
/* without vec here */
#endif
I mean, you have to bypass the build first; otherwise, you can't
go that further to runtime, right?
Huawei, since it's your patch introduced such issue, mind to fix
it?
--yliu
>
> Does this make sense?
>
> Thanks
> > --yliu
> >>
> >> Thanks
> >>
> >> On Mon, Feb 8, 2016 at 11:15 AM, Santosh Shukla <sshukla at mvista.com>
> >> wrote:
> >> > On Mon, Feb 8, 2016 at 2:55 AM, Thomas Monjalon
> >> > <thomas.monjalon at 6wind.com> wrote:
> >> >> 2016-02-07 19:21, Santosh Shukla:
> >> >>> - virtio_recv_pkts_vec and other virtio vector friend apis are written
> >> >>> for
> >> >>> sse/avx instructions. For arm64 in particular, virtio vector
> >> >>> implementation
> >> >>> does not exist(todo).
> >> >>>
> >> >>> So virtio pmd driver wont build for targets like i686, arm64. By
> >> >>> making
> >> >>> RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and
> >> >>> will work
> >> >>> in non-vectored virtio mode.
> >> >>>
> >> >>> Disabling RTE_VIRTIO_INC_VECTOR config for :
> >> >>>
> >> >>> - i686 arch as i686 target config says:
> >> >>> config/defconfig_i686-native-linuxapp-gcc says "Vectorized PMD is not
> >> >>> supported on 32-bit".
> >> >>>
> >> >>> - armv7/v8 arch.
> >> >>
> >> >> Yes it can be useful to disable vector optimizations, but it should done
> >> >> at runtime, not a compilation option. I know it is already wrongly
> >> >> configured
> >> >> at compilation for other drivers, we should fix them.
> >> >>
> >> >
> >> > Can't we consider this separate topic. My intent is virtio works for arm.
> >> >
> >> >> Here, you want to avoid SSE/AVX code on ARM. So we should just add the
> >> >> appropriate ifdefs. Adding a compilation option does not prevent from
> >> >> enabling
> >> >> it on ARM or old x86 which do not support these instructions.
> >> >>
> >> >
> >> > By disabling VIRTIO_INC_VEC, compiler wont build
> >> > virtio_recv_pkts_vec(), so wont generate SSE/AVX code. Adding ifdef
> >> > for other arch example arm, is next step. Vector instruction for arm
> >> > are not fully supported, Its a todolist (Pl. refer my early v1/2
> >> > cover-letter), We'll add that after virtio functionally works for arm.
> >> >
> >> >> Please virtio maintainers, we need to fix this code. Thanks
