Re: [dpdk-dev] [PATCH v2] virtio: Use cpuflag for vector api

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

On Tue, Mar 01, 2016 at 11:38:55AM +0530, Santosh Shukla wrote:
> On Tue, Mar 1, 2016 at 11:29 AM, Yuanhan Liu
> <yuanhan.liu@linux.intel.com> wrote:
> > On Mon, Feb 29, 2016 at 06:28:10PM +0530, Santosh Shukla wrote:
> >> Check cpuflag macro before using vectored api.
> >> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
> >> - Also wrap other vectored freind api ie..
> >> 1) virtqueue_enqueue_recv_refill_simple
> >> 2) virtio_rxq_vec_setup
> >>
> >> - removed VIRTIO_PMD=n from armv7/v8 config.
> >>
> >> todo:
> >> 1) Move virtio_recv_pkts_vec() implementation to
> >>    drivers/virtio/virtio_vec_<arch>.h file.
> >> 2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
> >>    files to provide vectored/non-vectored rx/tx apis.
> >>
> >> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> >> ---
> >> - v2: Removed VIRTIO_PMD=n from arm v7/v8
> >
> > Firstly, I would not suggest you to send another new version, while there
> > still was discussions ongoing on old version.
> >
> > And, you should not mix the ARM stuff here; this patch should only do
> > what the patch title tells. In generic, don't do two or more things in
> > one patch.
> >
> 
> w/o v2 patch, old version wont build for armv7/v8. Clubbing both in
> v2, inspired from v7 virtio INC_VEC review comment/feedback [1].

Thinking it this way, that build won't work for ARM, with or without
this patch. And this patch just fix a build error for platforms that
doesn't has vec instructions (which could include old x86 platforms).

So, the right way to go is to separate the ARM stuff to another
standalone patch, claiming that we now supports ARM.

Makes sense to you?

BTW, is this the last piece of code to make virtio for ARM work?
I maybe wrong, but I remembered you have few more patches for virtio
in old versions. (Yeah, I'm aware of that the EAL parts have been
merged)

Anyway, here is a remind: don't forget to update release note:

    doc/guides/rel_notes/release_16_04.rst

	--yliu