From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f175.google.com (mail-pf0-f175.google.com [209.85.192.175]) by dpdk.org (Postfix) with ESMTP id A329E2716 for ; Wed, 16 Dec 2015 15:20:52 +0100 (CET) Received: by mail-pf0-f175.google.com with SMTP id 68so12811418pfc.1 for ; Wed, 16 Dec 2015 06:20:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mvista-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=dSMfgnvCJQ0Km+TeedT94nA6WnyRgRfl4uMgIgLRfVs=; b=zA6uJ0/eVVDBdLJzP+qZJiLBmfE9JGPl/1Vsng0NcBhhq3lVbR6SJa8jFl3dkbaOyK v6Yc+zBdpSFEEbkxTZIgfb/hN6ST7NpZ74wN1P9tLQQ2l07LZyeq2IKUc5VZYdmn31md NOh8vW/+L3yufTTBP6Hq1RVnO55A+Wz4I4/hBIvc5+luuf+lfIPXbYRj1FO5A+Mn0/u0 2MgIkztIddxURrWSqI1oP5ZW3j5Sal3ZmmVDDfXwnu96s93PLKrz5MetzEXHTUJ8u984 i4XURa9i9O7F6okAdYH1ie/vCg6UZOCJZaIKCooB6NPLRDtCE9QcoqTLoDmZ0lD61QIK cUeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=dSMfgnvCJQ0Km+TeedT94nA6WnyRgRfl4uMgIgLRfVs=; b=iq1U5LZSOzpWCH9CDJAIByZ1/A+mrTXRu5v7hC7huzwzciX2iGnZOsXxEUo5Ero/SC IzmQru6nXB12dOHNLY6mGDKDwILrbTfR8fijYUkKxcR84RwZ3u31rS53DKLuP74VSTgi qcTtJcBac6EUqS4Fv+SoRhsYeIrl5UIWaRn34GOMCYeqvlNi8zPqBthq4ti/4wIVX+4n h04Jn0irlIKgVOHPfA9xP/JOPVx32ah/vloerWKKnXoOxf7XRuBZs3BTMqCZP+iyvrKa 9XzgVTjzB+jLaycF+4jAKIIkq7dJjxll1PUfVhODyaYTqF5KmOSDK1FQeUk+F5Zy/VdS 46ZA== X-Gm-Message-State: ALoCoQlZpNZrZ27tMGvGYvAVzm0pybVXrV5akKWX+/pmSb+rCrHEg1S3/MmKoaoviIw0KG9AWA5MPiDdH0Y1YsQwiRKJ206QAuw3qUdzW4qIcg0TBbezuN4= MIME-Version: 1.0 X-Received: by 10.98.0.16 with SMTP id 16mr5675308pfa.5.1450275651916; Wed, 16 Dec 2015 06:20:51 -0800 (PST) Received: by 10.66.13.233 with HTTP; Wed, 16 Dec 2015 06:20:51 -0800 (PST) In-Reply-To: <20151216132940.GT29571@yliu-dev.sh.intel.com> References: <1450098032-21198-1-git-send-email-sshukla@mvista.com> <1450098032-21198-12-git-send-email-sshukla@mvista.com> <20151216132940.GT29571@yliu-dev.sh.intel.com> Date: Wed, 16 Dec 2015 19:50:51 +0530 Message-ID: From: Santosh Shukla To: Yuanhan Liu Content-Type: text/plain; charset=UTF-8 Cc: dev@dpdk.org, Rakesh Krishnamurthy , Rizwan Ansari Subject: Re: [dpdk-dev] [ [PATCH v2] 11/13] virtio_ioport: armv7/v8: mmap virtio iopci bar region 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: Wed, 16 Dec 2015 14:20:53 -0000 On Wed, Dec 16, 2015 at 6:59 PM, Yuanhan Liu wrote: > On Mon, Dec 14, 2015 at 06:30:30PM +0530, Santosh Shukla wrote: >> Introducing module to mmap iopci bar region. Applicable for linuxapp for non-x86 >> archs, Tested for arm64/ThunderX platform for linux. For that adding two global >> api. >> - virtio_ioport_init >> - virtio_ioport_unmap >> >> Signed-off-by: Santosh Shukla >> Signed-off-by: Rizwan Ansari >> Signed-off-by: Rakesh Krishnamurthy >> --- >> drivers/net/virtio/Makefile | 1 + >> drivers/net/virtio/virtio_ioport.c | 163 ++++++++++++++++++++++++++++++++++++ >> drivers/net/virtio/virtio_ioport.h | 42 ++++++++++ >> 3 files changed, 206 insertions(+) >> create mode 100644 drivers/net/virtio/virtio_ioport.c >> create mode 100644 drivers/net/virtio/virtio_ioport.h >> >> diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile >> index 25a842d..5cba6d3 100644 >> --- a/drivers/net/virtio/Makefile >> +++ b/drivers/net/virtio/Makefile >> @@ -50,6 +50,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtqueue.c >> SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_pci.c >> SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx.c >> SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_ethdev.c >> +SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_ioport.c >> SRCS-$(CONFIG_RTE_VIRTIO_INC_VECTOR) += virtio_rxtx_simple.c >> >> # this lib depends upon: >> diff --git a/drivers/net/virtio/virtio_ioport.c b/drivers/net/virtio/virtio_ioport.c >> new file mode 100644 >> index 0000000..ffeb8e9 >> --- /dev/null >> +++ b/drivers/net/virtio/virtio_ioport.c >> @@ -0,0 +1,163 @@ >> +/* >> + * BSD LICENSE >> + * >> + * Copyright(c) 2015 Cavium Networks. All rights reserved. >> + * All rights reserved. >> + * >> + * Redistribution and use in source and binary forms, with or without >> + * modification, are permitted provided that the following conditions >> + * are met: >> + * >> + * * Redistributions of source code must retain the above copyright >> + * notice, this list of conditions and the following disclaimer. >> + * * Redistributions in binary form must reproduce the above copyright >> + * notice, this list of conditions and the following disclaimer in >> + * the documentation and/or other materials provided with the >> + * distribution. >> + * * Neither the name of Intel Corporation nor the names of its >> + * contributors may be used to endorse or promote products derived >> + * from this software without specific prior written permission. >> + * >> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS >> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT >> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR >> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT >> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, >> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT >> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, >> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY >> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT >> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE >> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >> + * >> + */ >> + >> +#include "virtio_ioport.h" >> + >> +#if defined(RTE_EXEC_ENV_LINUXAPP) && (defined(RTE_ARCH_ARM) || \ >> + defined(RTE_ARCH_ARM64)) >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include "virtio_logs.h" >> + >> +/* start address of first pci_iobar slot (user-space virtual-addres) */ >> +void *ioport_map; > > You still forgot "static"? > I misunderstood last comment, sorry we'll do. >> +/** >> + * ioport map count, >> + * Use-case: virtio-net-pci. > > How about removing above two lines; it's quite meaningless here, but > instead a bit redundant. > ok. >> + * Keeps track of number of virtio-net-pci device mapped/unmapped. Max device >> + * support by linux kernel is 31, so ioport_map_cnt can not be greater than 31. >> + */ >> +static int ioport_map_cnt; >> + >> +static int >> +virtio_map_ioport(void **resource_addr) >> +{ >> + int fd; >> + int ret = 0; >> + >> + /* avoid -Werror=unused-parameter, keep compiler happy */ >> + (void)resource_addr; > > Using __rte_unused is more elegant. > ok. >> + fd = open(VIRT_IOPORT_DEV, O_RDWR); >> + if (fd < 0) { >> + PMD_INIT_LOG(ERR, "device file %s open error: %d\n", >> + DEV_NAME, fd); >> + ret = -1; >> + goto out; >> + } >> + >> + ioport_map = mmap(NULL, PCI_VIRT_IOPORT_SIZE, >> + PROT_EXEC | PROT_WRITE | PROT_READ, MAP_SHARED, fd, 0); >> + >> + if (ioport_map == MAP_FAILED) { >> + PMD_INIT_LOG(ERR, "mmap: failed to map bar Address=%p\n", >> + *resource_addr); >> + ret = -ENOMEM; >> + goto out1; >> + } >> + >> + PMD_INIT_LOG(INFO, "First pci_iobar mapped at %p\n", ioport_map); >> + >> +out1: >> + close(fd); >> +out: >> + return ret; >> +} >> + >> +static int >> +virtio_set_ioport_addr(void **resource_addr, unsigned long offset) >> +{ >> + int ret = 0; >> + >> + if (ioport_map_cnt >= PCI_VIRT_IOPORT_MAX) { >> + ret = -1; >> + PMD_INIT_LOG(ERR, >> + "ioport_map_cnt(%d) greater than" >> + "PCI_VIRT_IOPORT_MAX(%d)\n", >> + ioport_map_cnt, PCI_VIRT_IOPORT_MAX); >> + return ret; >> + } >> + *resource_addr = (void *)((char *)ioport_map + (ioport_map_cnt)*offset); > > Redundant (), and the void * cast seems to be unnecessary. > (void *) is unnecessary, but couldn't get the redundant() part? >> + ioport_map_cnt++; >> + >> + PMD_INIT_LOG(DEBUG, "pci.resource_addr %p ioport_map_cnt %d\n", >> + *resource_addr, ioport_map_cnt); >> + return ret; >> +} >> + >> +int virtio_ioport_init(struct rte_pci_resource *mem_resource) >> +{ >> + int ret = 0; >> + >> + /** >> + * Map the all IOBAR entry from /proc/ioport to 4k page_size only once. >> + * Later virtio_set_ioport_addr() func will update correct bar_addr for >> + * each ioport (i.e..pci_dev->mem_resource[0].addr) >> + */ >> + if (!ioport_map) { >> + ret = virtio_map_ioport(&mem_resource->addr); >> + if (ret < 0) >> + return ret; >> + PMD_INIT_LOG(INFO, "ioport_map %p\n", ioport_map); >> + } >> + >> + ret = virtio_set_ioport_addr(&mem_resource->addr, mem_resource->len); >> + if (ret < 0) >> + return ret; >> + >> + PMD_INIT_LOG(INFO, "resource_addr %p resource_len :%ld\n", >> + mem_resource->addr, (unsigned long)mem_resource->len); >> + return ret; >> +} >> + >> +void virtio_ioport_unmap(void) >> +{ >> + /* unmap ioport memory */ > > Redundant comment. > ok. >> + ioport_map_cnt--; >> + if (!ioport_map_cnt) >> + munmap(ioport_map, PCI_VIRT_IOPORT_SIZE); >> + >> + PMD_INIT_LOG(DEBUG, "unmapping ioport_mem %d\n", ioport_map_cnt); >> +} >> + >> +#else /* !LINUXAPP && !ARM/64 */ >> + >> +int virtio_ioport_init(struct rte_pci_resource *mem_resource) >> +{ >> + (void)mem_resource; > > ditto. > Is it redundant comment or your suggesting to use : r / (void) / __rte_unused? >> + return 0; >> +} >> + >> +void virtio_ioport_unmap(void) >> +{ >> + return; >> +} >> + >> +#endif /* LINUXAPP, ARM/64 */ >> diff --git a/drivers/net/virtio/virtio_ioport.h b/drivers/net/virtio/virtio_ioport.h >> new file mode 100644 >> index 0000000..bf79551 >> --- /dev/null >> +++ b/drivers/net/virtio/virtio_ioport.h >> @@ -0,0 +1,42 @@ >> +/* >> + * BSD LICENSE >> + * >> + * Copyright(c) 2015 Cavium Networks. All rights reserved. >> + * All rights reserved. >> + * >> + * Redistribution and use in source and binary forms, with or without >> + * modification, are permitted provided that the following conditions >> + * are met: >> + * >> + * * Redistributions of source code must retain the above copyright >> + * notice, this list of conditions and the following disclaimer. >> + * * Redistributions in binary form must reproduce the above copyright >> + * notice, this list of conditions and the following disclaimer in >> + * the documentation and/or other materials provided with the >> + * distribution. >> + * * Neither the name of Intel Corporation nor the names of its >> + * contributors may be used to endorse or promote products derived >> + * from this software without specific prior written permission. >> + * >> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS >> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT >> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR >> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT >> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, >> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT >> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, >> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY >> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT >> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE >> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >> + * >> + */ >> +#ifndef _VIRTIO_IOPORT_H_ >> +#define _VIRTIO_IOPORT_H_ >> + >> +#include >> + >> +int virtio_ioport_init(struct rte_pci_resource *mem_resource); >> +void virtio_ioport_unmap(void); >> + >> +#endif /* _VIRTIO_IOPORT_H_ */ >> -- >> 1.7.9.5