Message ID | 1453203972-24855-11-git-send-email-sshukla@mvista.com (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Thomas Monjalon |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 3B4EC91B8; Tue, 19 Jan 2016 12:47:10 +0100 (CET) Received: from mail-pf0-f179.google.com (mail-pf0-f179.google.com [209.85.192.179]) by dpdk.org (Postfix) with ESMTP id B1BED8E8F for <dev@dpdk.org>; Tue, 19 Jan 2016 12:47:07 +0100 (CET) Received: by mail-pf0-f179.google.com with SMTP id e65so174780935pfe.0 for <dev@dpdk.org>; Tue, 19 Jan 2016 03:47:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mvista-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=KyOkoathv1A+jNkyQqKcO8aKUu2zDSzDph0IyKhDijM=; b=GrLBQ6+xgZCo3giQitqbSPKyTcOuMVnoTKg0TzwkgaYyYexMcOnztMuXJAmhuGONaJ RR9HVRg1K4/jyHMK1xvSEprnnkrHGL6taMM20nFgCWLFIikrSoaM5sm2us5d7DaYPYZH oCPj7yuag9mhkX1WHP4qx4kL473qR+Z8RfYJTvKAiHZTkdomGdzhRB/o6UVFWoEel/8x N0Cg8whV58jX8Sa/Emm24pQMnnGF1gMZSUzIILwnSVBQk/8JumNWjxDkJyMSDPL5qnsQ wGlfHMbMgZ2C7eemN4zz8p1QbRJlZXfjHJGhBybTP//8vB8yQtiFog54CMIeZbvzO+F3 HO4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=KyOkoathv1A+jNkyQqKcO8aKUu2zDSzDph0IyKhDijM=; b=XmOmgBrLHT2LOMQbNY29r89IssTLHoNqC8QGto41DNPAuAZQSDtlf6QPHzArOpKnU1 Rk6gzTPFlNn+bzIdO+F2ny2/MM2Zd9VKgeoi0m5Hh+pXOebFJMrT7XLB5JCS6yR0+CRl K1CtPZFiqSlGj0GWntVvQ+V0Gt2wSXYI41frFdPfrBNb19TeFqqw+CglzLdZw40nKLTl QsONFyWcbQMzZr/qOZ7uN89pa8+um6vwcai0jf69zCxn3bn15m5Qzhj8XsVTPCoSW61x I7iC5N1IY+ginUb/qQUhdn4gzv2Yi3tCM27AejDXldtxRPnO8erLO3gecgymc7q1Jvzu VY4A== X-Gm-Message-State: ALoCoQmfyXWKTXvb2FAfhqoCsmhEVjjYaTBcKpLpsc2so2DCOzkCr9pOKx3yT0I+HAglr25MIRrfnYoiQzFcEGTvqr/UfKNvuA== X-Received: by 10.98.65.90 with SMTP id o87mr43098278pfa.115.1453204027165; Tue, 19 Jan 2016 03:47:07 -0800 (PST) Received: from santosh-Latitude-E5530-non-vPro.mvista.com ([111.93.218.67]) by smtp.gmail.com with ESMTPSA id 75sm41014170pfj.20.2016.01.19.03.47.03 (version=TLS1_1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 19 Jan 2016 03:47:06 -0800 (PST) From: Santosh Shukla <sshukla@mvista.com> To: dev@dpdk.org Date: Tue, 19 Jan 2016 17:16:11 +0530 Message-Id: <1453203972-24855-11-git-send-email-sshukla@mvista.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1453203972-24855-1-git-send-email-sshukla@mvista.com> References: <1453203972-24855-1-git-send-email-sshukla@mvista.com> Subject: [dpdk-dev] [PATCH v5 10/11] virtio: pci: add dummy func definition for in/outb for non-x86 arch X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Santosh Shukla
Jan. 19, 2016, 11:46 a.m. UTC
For non-x86 arch, Compiler will throw build error for in/out apis. Including
dummy api function so to pass build.
Note that: For virtio to work for non-x86 arch - RTE_EAL_VFIO is the only
supported method. RTE_EAL_IGB_UIO is not supported for non-x86 arch.
So, Virtio support for arch and supported interface by that arch:
ARCH IGB_UIO VFIO
x86 Y Y
ARM64 N/A Y
PPC_64 N/A Y (Not tested but likely should work, as vfio is
arch independent)
Note: Applicable for virtio spec 0.95
Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
drivers/net/virtio/virtio_pci.h | 46 +++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
Comments
Ping? On Tue, Jan 19, 2016 at 5:16 PM, Santosh Shukla <sshukla@mvista.com> wrote: > For non-x86 arch, Compiler will throw build error for in/out apis. Including > dummy api function so to pass build. > > Note that: For virtio to work for non-x86 arch - RTE_EAL_VFIO is the only > supported method. RTE_EAL_IGB_UIO is not supported for non-x86 arch. > > So, Virtio support for arch and supported interface by that arch: > > ARCH IGB_UIO VFIO > x86 Y Y > ARM64 N/A Y > PPC_64 N/A Y (Not tested but likely should work, as vfio is > arch independent) > > Note: Applicable for virtio spec 0.95 > > Signed-off-by: Santosh Shukla <sshukla@mvista.com> > --- > drivers/net/virtio/virtio_pci.h | 46 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h > index f550d22..b88f9ec 100644 > --- a/drivers/net/virtio/virtio_pci.h > +++ b/drivers/net/virtio/virtio_pci.h > @@ -46,6 +46,7 @@ > #endif > > #include <rte_ethdev.h> > +#include "virtio_logs.h" > > struct virtqueue; > > @@ -320,6 +321,51 @@ outl_p(unsigned int data, unsigned int port) > } > #endif > > +#if !defined(RTE_ARCH_X86_64) && !defined(RTE_ARCH_I686) && \ > + defined(RTE_EXEC_ENV_LINUXAPP) > +static inline uint8_t > +inb(unsigned long addr __rte_unused) > +{ > + PMD_INIT_LOG(ERR, "inb() not supported for this RTE_ARCH\n"); > + return 0; > +} > + > +static inline uint16_t > +inw(unsigned long addr __rte_unused) > +{ > + PMD_INIT_LOG(ERR, "inw() not supported for this RTE_ARCH\n"); > + return 0; > +} > + > +static inline uint32_t > +inl(unsigned long addr __rte_unused) > +{ > + PMD_INIT_LOG(ERR, "in() not supported for this RTE_ARCH\n"); > + return 0; > +} > + > +static inline void > +outb_p(unsigned char data __rte_unused, unsigned int port __rte_unused) > +{ > + PMD_INIT_LOG(ERR, "outb_p() not supported for this RTE_ARCH\n"); > + return; > +} > + > +static inline void > +outw_p(unsigned short data __rte_unused, unsigned int port __rte_unused) > +{ > + PMD_INIT_LOG(ERR, "outw_p() not supported for this RTE_ARCH\n"); > + return; > +} > + > +static inline void > +outl_p(unsigned int data __rte_unused, unsigned int port __rte_unused) > +{ > + PMD_INIT_LOG(ERR, "outl_p() not supported for this RTE_ARCH\n"); > + return; > +} > +#endif > + > static inline int > vtpci_with_feature(struct virtio_hw *hw, uint64_t bit) > { > -- > 1.7.9.5 >
On Tue, Jan 19, 2016 at 05:16:11PM +0530, Santosh Shukla wrote: > For non-x86 arch, Compiler will throw build error for in/out apis. Including > dummy api function so to pass build. > > Note that: For virtio to work for non-x86 arch - RTE_EAL_VFIO is the only > supported method. RTE_EAL_IGB_UIO is not supported for non-x86 arch. > > So, Virtio support for arch and supported interface by that arch: > > ARCH IGB_UIO VFIO > x86 Y Y > ARM64 N/A Y > PPC_64 N/A Y (Not tested but likely should work, as vfio is > arch independent) > > Note: Applicable for virtio spec 0.95 > > Signed-off-by: Santosh Shukla <sshukla@mvista.com> > --- > drivers/net/virtio/virtio_pci.h | 46 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h > index f550d22..b88f9ec 100644 > --- a/drivers/net/virtio/virtio_pci.h > +++ b/drivers/net/virtio/virtio_pci.h > @@ -46,6 +46,7 @@ > #endif > > #include <rte_ethdev.h> > +#include "virtio_logs.h" > > struct virtqueue; > > @@ -320,6 +321,51 @@ outl_p(unsigned int data, unsigned int port) > } > #endif > > +#if !defined(RTE_ARCH_X86_64) && !defined(RTE_ARCH_I686) && \ > + defined(RTE_EXEC_ENV_LINUXAPP) > +static inline uint8_t > +inb(unsigned long addr __rte_unused) > +{ > + PMD_INIT_LOG(ERR, "inb() not supported for this RTE_ARCH\n"); > + return 0; > +} The whole port read/write stuff is getting messy here: we not only have to care the FreeBSD and Linux difference, but also the x86 and non-x86 difference. And you just added yet another vfio layer. First of all, they are not belong here (virtio_pci.h). A new place like virtio_io.h sounds much better to me. Therefore, I'd suggest you to put all those stuff there, like the one I have just cooked up: #ifndef __VIRTIO_IO_H__ #if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686) #ifdef __FreeBSD__ #include <sys/types.h> #include <machine/cpufunc.h> #define outb_p outb #define outw_p outw #define outl_p outl #else #include <sys/io.h> #endif #else /* ! X86 */ static inline uint8_t inb(unsigned long addr __rte_unused) { PMD_INIT_LOG(ERR, "inb() not supported for this RTE_ARCH\n"); return 0; } .... #endif /* X86 */ /* And put the vfio io port read/write here */ #endif /* __VIRTIO_IO_H__ */ Note that you may need squash patch 4 (build fix for sys/io.h ...) here. They both resolve one thing: to make it build on non-x86 platforms. Another minor note is that while you are trying this way, I'd suggest you to make a patch to introduce virtio_io.h, and then make another patch to fix build errors on non-x86 platforms. Another generic comment about this patchset is that it VERY okay to include several components change in one set, but putting them in order helps review a lot. Say, this patch set has dependence on VFIO stuff, therefore, it'd be much better __IF__ you can put all VFIO related patches first, and then virtio related patches follows, but not in an interleaved way you did. If, for somereason, you can't do that, you should at least try to minimise the chance of interleave. --yliu
On Fri, Jan 29, 2016 at 12:31 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote: > On Tue, Jan 19, 2016 at 05:16:11PM +0530, Santosh Shukla wrote: >> For non-x86 arch, Compiler will throw build error for in/out apis. Including >> dummy api function so to pass build. >> >> Note that: For virtio to work for non-x86 arch - RTE_EAL_VFIO is the only >> supported method. RTE_EAL_IGB_UIO is not supported for non-x86 arch. >> >> So, Virtio support for arch and supported interface by that arch: >> >> ARCH IGB_UIO VFIO >> x86 Y Y >> ARM64 N/A Y >> PPC_64 N/A Y (Not tested but likely should work, as vfio is >> arch independent) >> >> Note: Applicable for virtio spec 0.95 >> >> Signed-off-by: Santosh Shukla <sshukla@mvista.com> >> --- >> drivers/net/virtio/virtio_pci.h | 46 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 46 insertions(+) >> >> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h >> index f550d22..b88f9ec 100644 >> --- a/drivers/net/virtio/virtio_pci.h >> +++ b/drivers/net/virtio/virtio_pci.h >> @@ -46,6 +46,7 @@ >> #endif >> >> #include <rte_ethdev.h> >> +#include "virtio_logs.h" >> >> struct virtqueue; >> >> @@ -320,6 +321,51 @@ outl_p(unsigned int data, unsigned int port) >> } >> #endif >> >> +#if !defined(RTE_ARCH_X86_64) && !defined(RTE_ARCH_I686) && \ >> + defined(RTE_EXEC_ENV_LINUXAPP) >> +static inline uint8_t >> +inb(unsigned long addr __rte_unused) >> +{ >> + PMD_INIT_LOG(ERR, "inb() not supported for this RTE_ARCH\n"); >> + return 0; >> +} > > The whole port read/write stuff is getting messy here: we not only have > to care the FreeBSD and Linux difference, but also the x86 and non-x86 > difference. And you just added yet another vfio layer. > > First of all, they are not belong here (virtio_pci.h). A new place like > virtio_io.h sounds much better to me. Therefore, I'd suggest you to > put all those stuff there, like the one I have just cooked up: > > > #ifndef __VIRTIO_IO_H__ > > #if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686) > > #ifdef __FreeBSD__ > #include <sys/types.h> > #include <machine/cpufunc.h> > > #define outb_p outb > #define outw_p outw > #define outl_p outl > #else > #include <sys/io.h> > #endif > > #else /* ! X86 */ > > static inline uint8_t > inb(unsigned long addr __rte_unused) > { > PMD_INIT_LOG(ERR, "inb() not supported for this RTE_ARCH\n"); > return 0; > } > > > .... > #endif /* X86 */ > > > /* And put the vfio io port read/write here */ > > #endif /* __VIRTIO_IO_H__ */ > > Note that you may need squash patch 4 (build fix for sys/io.h ...) > here. They both resolve one thing: to make it build on non-x86 platforms. > > Another minor note is that while you are trying this way, I'd suggest > you to make a patch to introduce virtio_io.h, and then make another > patch to fix build errors on non-x86 platforms. > Ok. > Another generic comment about this patchset is that it VERY okay to > include several components change in one set, but putting them in > order helps review a lot. > > Say, this patch set has dependence on VFIO stuff, therefore, it'd be > much better __IF__ you can put all VFIO related patches first, and > then virtio related patches follows, but not in an interleaved way > you did. If, for somereason, you can't do that, you should at least > try to minimise the chance of interleave. > I agree that, but this patch series dependent on other patches including virtio 1.0 and then vfio-noiommu, its was difficult for me to keep topic-wise sanity in patch series. V6 will take care patch ordering. Thanks > --yliu
On Fri, Jan 29, 2016 at 01:01:02PM +0530, Santosh Shukla wrote: > > Another generic comment about this patchset is that it VERY okay to > > include several components change in one set, but putting them in > > order helps review a lot. > > > > Say, this patch set has dependence on VFIO stuff, therefore, it'd be > > much better __IF__ you can put all VFIO related patches first, and > > then virtio related patches follows, but not in an interleaved way > > you did. If, for somereason, you can't do that, you should at least > > try to minimise the chance of interleave. > > > > I agree that, but this patch series dependent on other patches > including virtio 1.0 and then vfio-noiommu, its was difficult for me > to keep topic-wise sanity in patch series. That would not be an issue to me: just apply the dependence patches first, and build your patches on top of that. You just need mention the dependence info in your cover-letter. --yliu > > V6 will take care patch ordering. Thanks Thanks! --yliu
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h index f550d22..b88f9ec 100644 --- a/drivers/net/virtio/virtio_pci.h +++ b/drivers/net/virtio/virtio_pci.h @@ -46,6 +46,7 @@ #endif #include <rte_ethdev.h> +#include "virtio_logs.h" struct virtqueue; @@ -320,6 +321,51 @@ outl_p(unsigned int data, unsigned int port) } #endif +#if !defined(RTE_ARCH_X86_64) && !defined(RTE_ARCH_I686) && \ + defined(RTE_EXEC_ENV_LINUXAPP) +static inline uint8_t +inb(unsigned long addr __rte_unused) +{ + PMD_INIT_LOG(ERR, "inb() not supported for this RTE_ARCH\n"); + return 0; +} + +static inline uint16_t +inw(unsigned long addr __rte_unused) +{ + PMD_INIT_LOG(ERR, "inw() not supported for this RTE_ARCH\n"); + return 0; +} + +static inline uint32_t +inl(unsigned long addr __rte_unused) +{ + PMD_INIT_LOG(ERR, "in() not supported for this RTE_ARCH\n"); + return 0; +} + +static inline void +outb_p(unsigned char data __rte_unused, unsigned int port __rte_unused) +{ + PMD_INIT_LOG(ERR, "outb_p() not supported for this RTE_ARCH\n"); + return; +} + +static inline void +outw_p(unsigned short data __rte_unused, unsigned int port __rte_unused) +{ + PMD_INIT_LOG(ERR, "outw_p() not supported for this RTE_ARCH\n"); + return; +} + +static inline void +outl_p(unsigned int data __rte_unused, unsigned int port __rte_unused) +{ + PMD_INIT_LOG(ERR, "outl_p() not supported for this RTE_ARCH\n"); + return; +} +#endif + static inline int vtpci_with_feature(struct virtio_hw *hw, uint64_t bit) {