From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by dpdk.org (Postfix) with ESMTP id BBA731B2C7 for ; Mon, 30 Oct 2017 10:17:49 +0100 (CET) Received: by mail-wm0-f65.google.com with SMTP id t139so14893504wmt.1 for ; Mon, 30 Oct 2017 02:17:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=2QY8/v5fSz29ryXEpMrOIZDLJsSWqX+eAylFqbblucg=; b=IvRu3ppkUdcTvSEfRaekDkuWj3OGDfSxLHkZz7b4BMu0jchJkn+oKfJcCQxAxXwiia /Vkr/ZV9t1hCKGPIGwGulkd0fFPx4woDitpiYEco5XVaCvgx44nrIuWo5AQoNajUfnfV HJQwTdwErj8/WZaIhu39VFCSj/0u8hbgD2f+6+cSTt4vQAl5JIPIS7JgE1o9mp+PJ0Xq kYui8YFLTi46DVAaJIUvHc/606h6rqhpNEgwXtIkIXi/cfOut4TcZgC/vkDpaYxljD77 zNanHF5BFaldIjcYBD3oZli+NYgJVdaeUK2P7SJFK86+dN+5sN8s6aA42fLHf/o2hOje MUYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=2QY8/v5fSz29ryXEpMrOIZDLJsSWqX+eAylFqbblucg=; b=sQqSLIZp2V4WQ1i89+vXC6FL+nsXYhFfdWHj5lepN6JLgiWW2z+YGxzVDsrhJWsAM7 SAwfsBK0u52fg6OgvlGKo5QCXf29VVYWYAo5Op1Jl/Owm210rn5cWoOtG81bY7DkwFBt TmEbDEaHqIFCw1nq9GvFfjjn9QJXYVO8kAaAs8hNOx2dbWUatOg7actC2e0gJYkAcXfa ml++tcQRwlSUIc98BHRuh7wEiqIIlEmjO/iZaGvh9OuDbVjxn3QemZUC+nTFz4bi4TMT OSANE0Qd4WY/MHjJPK8HnhUh4Q0hYwcIU9+ykaKwL6DINhYg8p3awsEq982+oeYO4Gxz 4FwQ== X-Gm-Message-State: AMCzsaU3tCAC1U/tSAp9Zk7q3oJ+7OvAs1f9WXCq+P4Rdogc0V/55V6/ yVOtS0oyjP9pITZh1wcr0JcdQM2e X-Google-Smtp-Source: ABhQp+RiQESfAFpnP2ggZwSW5kE6GYXVqX+oeizmkmmZFcEgLTDu6olZgC70I7pMNBIp9ll1d5NJFw== X-Received: by 10.80.186.29 with SMTP id g29mr11244901edc.206.1509355069323; Mon, 30 Oct 2017 02:17:49 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id a39sm10659479eda.79.2017.10.30.02.17.48 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 30 Oct 2017 02:17:48 -0700 (PDT) Date: Mon, 30 Oct 2017 10:17:36 +0100 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Ferruh Yigit Cc: Jerin Jacob , dev@dpdk.org, thomas@monjalon.net Message-ID: <20171030091736.GH10890@bidouze.vm.6wind.com> References: <20171028062053.6615-1-jerin.jacob@caviumnetworks.com> <20171030080654.GF10890@bidouze.vm.6wind.com> <6e9922c5-9659-a42a-4213-700ebe1e5714@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6e9922c5-9659-a42a-4213-700ebe1e5714@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] bus/pci: fix vfio mode X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Oct 2017 09:17:49 -0000 Hi Ferruh, On Mon, Oct 30, 2017 at 02:00:43AM -0700, Ferruh Yigit wrote: > On 10/30/2017 1:06 AM, Gaëtan Rivet wrote: > > Hi Jerin, > > > > On Sat, Oct 28, 2017 at 11:50:52AM +0530, Jerin Jacob wrote: > >> The definition of VFIO_PRESENT is "eal_vfio.h", Fail to > >> include eal_vfio.h will result in disabling vfio. > >> > >> Fixes: 279b581c897d ("vfio: expose functions") > >> > > > > Thanks for the fix, sorry for VFIO. > > I tried to let go of VFIO_PRESENT in the PCI patchset, unfortunately I did > > not do a good-enough job. > > > > Instead of reinstating the dependency on the private eal_vfio.h header, > > I'd suggest replacing all VFIO_PRESENT references within the PCI bus by > > RTE_EAL_VFIO, and make the pci_vfio.c compilation depend on it within > > the linux Makefile. Something like: > > > > ---8<--- > > > > grep -rl VFIO_PRESENT drivers/bus/pci/linux/ |while read -r file > > do sed -i 's;VFIO_PRESENT;RTE_EAL_VFIO;' $file > > done > > VFIO_PRESENT is the combination of the if user enabled VFIO and if Linux kernel > supports it. > > Why not add same check and VFIO_PRESENT definition to rte_vfio.h: > > --- a/lib/librte_eal/common/include/rte_vfio.h > +++ b/lib/librte_eal/common/include/rte_vfio.h > @@ -34,7 +34,13 @@ > #ifndef _RTE_VFIO_H_ > #define _RTE_VFIO_H_ > > +#if !defined(VFIO_PRESENT) && defined(RTE_EAL_VFIO) > +#include > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 6, 0) > #include > +#define VFIO_PRESENT > +#endif /* >= 3.6.0 */ > +#endif > + > +#ifdef VFIO_PRESENT > > ... Need to wrap here in case VFIO disabled ... > > #endif > It would work indeed. I mostly dislike having whole compilation units disabled silently on a linux version check. I think that if someone wanted to support kernels < 3.6, they ought to do the work of disabling RTE_EAL_VFIO. A build error could be thrown to help those users toward the right solution, but I think that the meaning of having this option enabled should be enforced: if it is enabled, it is compiled. If dependencies are not met, then the option should be disabled. > > > > patch -p1 <<___HERE > > diff --git a/drivers/bus/pci/linux/Makefile b/drivers/bus/pci/linux/Makefile > > index 77c5f97..b5b9c54 100644 > > --- a/drivers/bus/pci/linux/Makefile > > +++ b/drivers/bus/pci/linux/Makefile > > @@ -31,6 +31,8 @@ > > > > SRCS += pci.c > > SRCS += pci_uio.c > > +ifeq (\$(CONFIG_RTE_EAL_VFIO),y) > > SRCS += pci_vfio.c > > +endif > > +1 > > Also ? > +ifeq ($(CONFIG_RTE_EAL_VFIO),y) > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_vfio.c > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_vfio_mp_sync.c > +endif > > Build shouldn't broken when VFIO disabled form config > > > > > CFLAGS += -D_GNU_SOURCE > > ___HERE > > > > --->8--- > > > > Do you think it could work? If so, I think it would be preferable > > to re-including eal_vfio.h. Private EAL headers are bound to be removed > > from other subsystems at some point. I tried to start with VFIO, others > > should follow. > > > >> Cc: Gaetan Rivet > >> > >> Signed-off-by: Jerin Jacob > >> --- > >> drivers/bus/pci/linux/pci.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c > >> index cdf810693..d0ce0207a 100644 > >> --- a/drivers/bus/pci/linux/pci.c > >> +++ b/drivers/bus/pci/linux/pci.c > >> @@ -46,6 +46,7 @@ > >> > >> #include "eal_private.h" > >> #include "eal_filesystem.h" > >> +#include "eal_vfio.h" > >> > >> #include "private.h" > >> #include "pci_init.h" > >> -- > >> 2.14.3 > >> > > > -- Gaëtan Rivet 6WIND