From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 07A70AA37 for ; Tue, 16 Feb 2016 04:06:05 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP; 15 Feb 2016 19:05:45 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,453,1449561600"; d="scan'208";a="746988181" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by orsmga003.jf.intel.com with ESMTP; 15 Feb 2016 19:05:46 -0800 Date: Tue, 16 Feb 2016 11:05:48 +0800 From: Yuanhan Liu To: Santosh Shukla , "Xie, Huawei" Message-ID: <20160216030548.GE21426@yliu-dev.sh.intel.com> References: <1454853068-14621-1-git-send-email-sshukla@mvista.com> <1454853068-14621-3-git-send-email-sshukla@mvista.com> <2395124.Wzh8l6ZlGf@xps13> <20160215105743.GB21426@yliu-dev.sh.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dpdk Subject: Re: [dpdk-dev] [PATCH v7 2/4] virtio: Introduce config RTE_VIRTIO_INC_VECTOR X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Feb 2016 03:06:06 -0000 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 > 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 wrote: > >> > On Mon, Feb 8, 2016 at 2:55 AM, Thomas Monjalon > >> > 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