From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Marchand Subject: Re: [RFC] igb_uio: deprecate iomem and ioport mapping Date: Fri, 9 Sep 2016 11:06:54 +0200 Message-ID: References: <1472696197-37614-1-git-send-email-jianfeng.tan@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: "dev@dpdk.org" , Ferruh Yigit , Stephen Hemminger To: Jianfeng Tan Return-path: Received: from mail-wm0-f46.google.com (mail-wm0-f46.google.com [74.125.82.46]) by dpdk.org (Postfix) with ESMTP id 421EA58C5 for ; Fri, 9 Sep 2016 11:07:15 +0200 (CEST) Received: by mail-wm0-f46.google.com with SMTP id w12so20036592wmf.0 for ; Fri, 09 Sep 2016 02:07:15 -0700 (PDT) In-Reply-To: <1472696197-37614-1-git-send-email-jianfeng.tan@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hello Jianfeng, On Thu, Sep 1, 2016 at 4:16 AM, Jianfeng Tan wrote: > Previously in igb_uio, iomem is mapped, and both ioport and io mem > are recorded into uio framework, which is duplicated and makes the > code too complex. > > For iomem, DPDK user space code never opens or reads files under > /sys/pci/bus/devices/xxxx:xx:xx.x/uio/uioY/maps/. Instead, > /sys/pci/bus/devices/xxxx:xx:xx.x/resourceY are used to map device > memory. > > For ioport, non-x86 platforms cannot read from files under > /sys/pci/bus/devices/xxxx:xx:xx.x/uio/uioY/portio/ directly, because > non-x86 platforms need to map port region for access in user space, > see non-x86 version pci_uio_ioport_map(). x86 platforms can use the > the same way as uio_pci_generic. > > This patch deprecates iomem and ioport mapping in igb_uio kernel > module, and adjusts the iomem implementation in both igb_uio and > uio_pci_generic: > - for x86 platform, get ports info from /proc/ioports; > - for non-x86 platform, map and get ports info by pci_uio_ioport_map(). > > Note: this will affect those applications who are using files under > /sys/pci/bus/devices/xxxx:xx:xx.x/uio/uioY/maps/ and > /sys/pci/bus/devices/xxxx:xx:xx.x/uio/uioY/portio/. > > Signed-off-by: Jianfeng Tan > --- > lib/librte_eal/linuxapp/eal/eal_pci.c | 4 - > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 56 +------------- > lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 119 ++---------------------------- > 3 files changed, 9 insertions(+), 170 deletions(-) [snip] > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > index 1786b75..28d09ed 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c [snip] > - /* FIXME only for primary process ? */ > - if (dev->intr_handle.type == RTE_INTR_HANDLE_UNKNOWN) { > - > - snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num); > - dev->intr_handle.fd = open(filename, O_RDWR); > - if (dev->intr_handle.fd < 0) { > - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", > - filename, strerror(errno)); > - return -1; > - } > - dev->intr_handle.type = RTE_INTR_HANDLE_UIO; > - } The only potential issue I can see removing this is that if a device has no iomem resource, then its interrupt fd will never be initialised. I can see no problem at the moment, so let's go with this. If such a problem arises later, we can separate this from the resource mapping stuff (with a new api ?). The rest looks good to me. As Ferruh said, this should go through deprecation notices then go in 17.02. -- David Marchand