[dpdk-dev,v2] virtio: Use cpuflag for vector api
Commit Message
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
- v1: This is a rework of patch [1].
Note: This patch will let non-x86 arch to use virtio pmd.
[1] http://dpdk.org/dev/patchwork/patch/10429/
config/defconfig_arm-armv7a-linuxapp-gcc | 1 -
config/defconfig_arm64-armv8a-linuxapp-gcc | 1 -
drivers/net/virtio/virtio_rxtx.c | 16 +++++++++++++++-
drivers/net/virtio/virtio_rxtx.h | 2 ++
drivers/net/virtio/virtio_rxtx_simple.c | 11 ++++++++++-
5 files changed, 27 insertions(+), 4 deletions(-)
Comments
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.
--yliu
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].
[1] http://dpdk.org/dev/patchwork/patch/10429/
> --yliu
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
- 1st patch: let non-x86 arch use virtio pmd driver in non-vec
- 2nd patch: enable virtio arm support
- 3rd patch: update virtio for arm feature entry in release guide.
Thanks.
Santosh Shukla (3):
virtio: use vector rx/tx for ssse cpuflag only
config: enable virtio for armv7/v8
guide/release: add virtio for arm feature info
config/defconfig_arm-armv7a-linuxapp-gcc | 1 -
config/defconfig_arm64-armv8a-linuxapp-gcc | 1 -
doc/guides/rel_notes/release_16_04.rst | 5 +++++
drivers/net/virtio/Makefile | 3 +++
drivers/net/virtio/virtio_rxtx.c | 16 +++++++++++++++-
drivers/net/virtio/virtio_rxtx.h | 2 ++
6 files changed, 25 insertions(+), 3 deletions(-)
On Tue, Mar 1, 2016 at 12:02 PM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> 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?
>
Sent a new patch series. Incorporated comments in this thread
http://dpdk.org/dev/patchwork/patch/10945/
http://dpdk.org/dev/patchwork/patch/10946/
> 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
>
Posted a patch just now:
http://dpdk.org/dev/patchwork/patch/10947/
> --yliu
>
On Tue, Mar 01, 2016 at 03:32:17PM +0530, Santosh Shukla wrote:
> - 1st patch: let non-x86 arch use virtio pmd driver in non-vec
> - 2nd patch: enable virtio arm support
> - 3rd patch: update virtio for arm feature entry in release guide.
Series looks good to me:
Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
However, FYI, Thomas would like to see that the release note is
added __inside__ the patch that acutally enables it, but not in
another standalone patch. In another word, you should squash
patch 3 to patch 2.
I'm wondering Thomas could do that for you this time while applying
your pathces. But, this is just a kind remind, and you should not
do that next time.
--yliu
On Wed, Mar 2, 2016 at 2:02 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> On Tue, Mar 01, 2016 at 03:32:17PM +0530, Santosh Shukla wrote:
>> - 1st patch: let non-x86 arch use virtio pmd driver in non-vec
>> - 2nd patch: enable virtio arm support
>> - 3rd patch: update virtio for arm feature entry in release guide.
>
> Series looks good to me:
>
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>
> However, FYI, Thomas would like to see that the release note is
> added __inside__ the patch that acutally enables it, but not in
> another standalone patch. In another word, you should squash
> patch 3 to patch 2.
>
> I'm wondering Thomas could do that for you this time while applying
> your pathces. But, this is just a kind remind, and you should not
> do that next time.
>
Thanks!,
Thomas, Can you please take care?
> --yliu
2016-03-02 14:11, Santosh Shukla:
> On Wed, Mar 2, 2016 at 2:02 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> > On Tue, Mar 01, 2016 at 03:32:17PM +0530, Santosh Shukla wrote:
> >> - 1st patch: let non-x86 arch use virtio pmd driver in non-vec
> >> - 2nd patch: enable virtio arm support
> >> - 3rd patch: update virtio for arm feature entry in release guide.
> >
> > Series looks good to me:
> >
> > Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> >
> > However, FYI, Thomas would like to see that the release note is
> > added __inside__ the patch that acutally enables it, but not in
> > another standalone patch. In another word, you should squash
> > patch 3 to patch 2.
> >
> > I'm wondering Thomas could do that for you this time while applying
> > your pathces. But, this is just a kind remind, and you should not
> > do that next time.
> >
>
> Thanks!,
>
> Thomas, Can you please take care?
I've squashed and reworded a bit the release note.
Applied, thanks.
On Thu, Mar 3, 2016 at 6:56 PM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 2016-03-02 14:11, Santosh Shukla:
>> On Wed, Mar 2, 2016 at 2:02 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
>> > On Tue, Mar 01, 2016 at 03:32:17PM +0530, Santosh Shukla wrote:
>> >> - 1st patch: let non-x86 arch use virtio pmd driver in non-vec
>> >> - 2nd patch: enable virtio arm support
>> >> - 3rd patch: update virtio for arm feature entry in release guide.
>> >
>> > Series looks good to me:
>> >
>> > Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>> >
>> > However, FYI, Thomas would like to see that the release note is
>> > added __inside__ the patch that acutally enables it, but not in
>> > another standalone patch. In another word, you should squash
>> > patch 3 to patch 2.
>> >
>> > I'm wondering Thomas could do that for you this time while applying
>> > your pathces. But, this is just a kind remind, and you should not
>> > do that next time.
>> >
>>
>> Thanks!,
>>
>> Thomas, Can you please take care?
>
> I've squashed and reworded a bit the release note.
> Applied, thanks.
Thanks you!
@@ -70,7 +70,6 @@ CONFIG_RTE_LIBRTE_I40E_PMD=n
CONFIG_RTE_LIBRTE_IXGBE_PMD=n
CONFIG_RTE_LIBRTE_MLX4_PMD=n
CONFIG_RTE_LIBRTE_MPIPE_PMD=n
-CONFIG_RTE_LIBRTE_VIRTIO_PMD=n
CONFIG_RTE_LIBRTE_VMXNET3_PMD=n
CONFIG_RTE_LIBRTE_PMD_XENVIRT=n
CONFIG_RTE_LIBRTE_PMD_BNX2X=n
@@ -44,7 +44,6 @@ CONFIG_RTE_TOOLCHAIN="gcc"
CONFIG_RTE_TOOLCHAIN_GCC=y
CONFIG_RTE_IXGBE_INC_VECTOR=n
-CONFIG_RTE_LIBRTE_VIRTIO_PMD=n
CONFIG_RTE_LIBRTE_IVSHMEM=n
CONFIG_RTE_LIBRTE_FM10K_PMD=n
CONFIG_RTE_LIBRTE_I40E_PMD=n
@@ -67,7 +67,9 @@
#define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
ETH_TXQ_FLAGS_NOOFFLOADS)
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
static int use_simple_rxtx;
+#endif
static void
vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx)
@@ -307,12 +309,13 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
nbufs = 0;
error = ENOSPC;
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
if (use_simple_rxtx)
for (i = 0; i < vq->vq_nentries; i++) {
vq->vq_ring.avail->ring[i] = i;
vq->vq_ring.desc[i].flags = VRING_DESC_F_WRITE;
}
-
+#endif
memset(&vq->fake_mbuf, 0, sizeof(vq->fake_mbuf));
for (i = 0; i < RTE_PMD_VIRTIO_RX_MAX_BURST; i++)
vq->sw_ring[vq->vq_nentries + i] = &vq->fake_mbuf;
@@ -325,9 +328,11 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
/******************************************
* Enqueue allocated buffers *
*******************************************/
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
if (use_simple_rxtx)
error = virtqueue_enqueue_recv_refill_simple(vq, m);
else
+#endif
error = virtqueue_enqueue_recv_refill(vq, m);
if (error) {
rte_pktmbuf_free(m);
@@ -340,6 +345,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
PMD_INIT_LOG(DEBUG, "Allocated %d bufs", nbufs);
} else if (queue_type == VTNET_TQ) {
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
if (use_simple_rxtx) {
int mid_idx = vq->vq_nentries >> 1;
for (i = 0; i < mid_idx; i++) {
@@ -357,6 +363,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
for (i = mid_idx; i < vq->vq_nentries; i++)
vq->vq_ring.avail->ring[i] = i;
}
+#endif
}
}
@@ -423,7 +430,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
dev->data->rx_queues[queue_idx] = vq;
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
virtio_rxq_vec_setup(vq);
+#endif
return 0;
}
@@ -449,7 +458,10 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
const struct rte_eth_txconf *tx_conf)
{
uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
+
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
struct virtio_hw *hw = dev->data->dev_private;
+#endif
struct virtqueue *vq;
uint16_t tx_free_thresh;
int ret;
@@ -462,6 +474,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
return -EINVAL;
}
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
/* Use simple rx/tx func if single segment and no offloads */
if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS &&
!vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
@@ -470,6 +483,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
dev->rx_pkt_burst = virtio_recv_pkts_vec;
use_simple_rxtx = 1;
}
+#endif
ret = virtio_dev_queue_setup(dev, VTNET_TQ, queue_idx, vtpci_queue_idx,
nb_desc, socket_id, &vq);
@@ -33,7 +33,9 @@
#define RTE_PMD_VIRTIO_RX_MAX_BURST 64
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
int virtio_rxq_vec_setup(struct virtqueue *rxq);
int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
struct rte_mbuf *m);
+#endif
@@ -37,7 +37,9 @@
#include <string.h>
#include <errno.h>
-#include <tmmintrin.h>
+#ifdef __SSE3__
+#include <rte_vect.h>
+#endif
#include <rte_cycles.h>
#include <rte_memory.h>
@@ -66,6 +68,7 @@
#pragma GCC diagnostic ignored "-Wcast-qual"
#endif
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
int __attribute__((cold))
virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
struct rte_mbuf *cookie)
@@ -90,6 +93,7 @@ virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
return 0;
}
+#endif
static inline void
virtio_rxq_rearm_vec(struct virtqueue *rxvq)
@@ -130,6 +134,7 @@ virtio_rxq_rearm_vec(struct virtqueue *rxvq)
vq_update_avail_idx(rxvq);
}
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
/* virtio vPMD receive routine, only accept(nb_pkts >= RTE_VIRTIO_DESC_PER_LOOP)
*
* This routine is for non-mergeable RX, one desc for each guest buffer.
@@ -291,6 +296,7 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
rxvq->packets += nb_pkts_received;
return nb_pkts_received;
}
+#endif
#define VIRTIO_TX_FREE_THRESH 32
#define VIRTIO_TX_MAX_FREE_BUF_SZ 32
@@ -398,6 +404,7 @@ virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts,
return nb_pkts;
}
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
int __attribute__((cold))
virtio_rxq_vec_setup(struct virtqueue *rxq)
{
@@ -416,3 +423,5 @@ virtio_rxq_vec_setup(struct virtqueue *rxq)
return 0;
}
+#endif
+