From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 598AA1B6DB; Tue, 17 Oct 2017 22:14:42 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga105.fm.intel.com with ESMTP; 17 Oct 2017 13:14:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,392,1503385200"; d="scan'208";a="324361334" Received: from silpixa00372839.ir.intel.com (HELO silpixa00372839.ger.corp.intel.com) ([10.237.222.154]) by fmsmga004.fm.intel.com with ESMTP; 17 Oct 2017 13:14:38 -0700 From: Ferruh Yigit To: Thomas Monjalon , Ferruh Yigit Cc: dev@dpdk.org, Jianfeng Tan , Jingjing Wu , Shijith Thotton , Gregory Etelson , Harish Patil , George Prekas , stable@dpdk.org Date: Tue, 17 Oct 2017 21:14:36 +0100 Message-Id: <20171017201436.65270-1-ferruh.yigit@intel.com> X-Mailer: git-send-email 2.13.6 Subject: [dpdk-dev] [PATCH] igb_uio: revert open and release operations 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: Tue, 17 Oct 2017 20:14:43 -0000 This reverts commit 6b9ed026a8704b9e5ee5da7997617ef7cc82e114. This reverts commit 5f6ff30dc5075c49069d684bab229aef7ff0fdc3. This reverts commit b58eedfc7dd57eef6d12e2c654a52c834f36084a. There were bug reports about terminated application may leave device in undesired state: http://dpdk.org/ml/archives/dev/2016-November/049745.html http://dpdk.org/ml/archives/dev/2016-November/050932.html And a proposal to fix: http://dpdk.org/ml/archives/dev/2016-December/051844.html Later another proposal triggered the discussion: http://dpdk.org/ml/archives/dev/2017-May/066317.html Finally a fix patch pushed into v17.08: Commit: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of device file") Later a regression report sent related to the pushed patch: http://dpdk.org/ml/archives/dev/2017-September/075236.html And a fix for regression integrated into v17.11-rc1: http://dpdk.org/ml/archives/dev/2017-October/079166.html Commit: 5f6ff30dc507 ("igb_uio: fix interrupt enablement after FLR in VM") Commit: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17") Even after the fix qede PMD reported to be broken: http://dpdk.org/ml/archives/dev/2017-October/079359.html So this patch reverts original fix and related commits. The related igb_uio code part turns back to v17.05 base. Cc: Jianfeng Tan Cc: Jingjing Wu Cc: Shijith Thotton Cc: Gregory Etelson Cc: Harish Patil Cc: George Prekas Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of device file") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit --- It would be nice to solve this issue in LTS release, but being close to the release and the error report without details makes it hard to work more on this issue. Thanks everyone who spent effort for this, hopefully we can continue to work on next release cycle. Jingjing, there is a i40e commit, was part of igb_uio fix patchset, is it generic, or needs to be reverted with this patch? Commit: 8cacf78469a7 ("net/i40e: fix VF initialization error") --- lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 201 +++++++++++------------------- 1 file changed, 76 insertions(+), 125 deletions(-) diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c index f7ef82554..786df68a2 100644 --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c @@ -49,6 +49,7 @@ struct rte_uio_pci_dev { static char *intr_mode; static enum rte_intr_mode igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX; + /* sriov sysfs */ static ssize_t show_max_vfs(struct device *dev, struct device_attribute *attr, @@ -207,22 +208,80 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state) * If yes, disable it here and will be enable later. */ static irqreturn_t -igbuio_pci_irqhandler(int irq, void *dev_id) +igbuio_pci_irqhandler(int irq, struct uio_info *info) { - struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id; - struct uio_info *info = &udev->info; + struct rte_uio_pci_dev *udev = info->priv; /* Legacy mode need to mask in hardware */ if (udev->mode == RTE_INTR_MODE_LEGACY && !pci_check_and_mask_intx(udev->pdev)) return IRQ_NONE; - uio_event_notify(info); - /* Message signal mode, no share IRQ and automasked */ return IRQ_HANDLED; } +/* Remap pci resources described by bar #pci_bar in uio resource n. */ +static int +igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info, + int n, int pci_bar, const char *name) +{ + unsigned long addr, len; + void *internal_addr; + + if (n >= ARRAY_SIZE(info->mem)) + return -EINVAL; + + addr = pci_resource_start(dev, pci_bar); + len = pci_resource_len(dev, pci_bar); + if (addr == 0 || len == 0) + return -1; + internal_addr = ioremap(addr, len); + if (internal_addr == NULL) + return -1; + info->mem[n].name = name; + info->mem[n].addr = addr; + info->mem[n].internal_addr = internal_addr; + info->mem[n].size = len; + info->mem[n].memtype = UIO_MEM_PHYS; + return 0; +} + +/* Get pci port io resources described by bar #pci_bar in uio resource n. */ +static int +igbuio_pci_setup_ioport(struct pci_dev *dev, struct uio_info *info, + int n, int pci_bar, const char *name) +{ + unsigned long addr, len; + + if (n >= ARRAY_SIZE(info->port)) + return -EINVAL; + + addr = pci_resource_start(dev, pci_bar); + len = pci_resource_len(dev, pci_bar); + if (addr == 0 || len == 0) + return -EINVAL; + + info->port[n].name = name; + info->port[n].start = addr; + info->port[n].size = len; + info->port[n].porttype = UIO_PORT_X86; + + return 0; +} + +/* Unmap previously ioremap'd resources */ +static void +igbuio_pci_release_iomem(struct uio_info *info) +{ + int i; + + for (i = 0; i < MAX_UIO_MAPS; i++) { + if (info->mem[i].internal_addr) + iounmap(info->mem[i].internal_addr); + } +} + static int igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev) { @@ -252,7 +311,6 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev) break; } #endif - /* fall back to MSI */ case RTE_INTR_MODE_MSI: #ifndef HAVE_ALLOC_IRQ_VECTORS @@ -291,28 +349,15 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev) default: dev_err(&udev->pdev->dev, "invalid IRQ mode %u", igbuio_intr_mode_preferred); - udev->info.irq = UIO_IRQ_NONE; err = -EINVAL; } - if (udev->info.irq != UIO_IRQ_NONE) - err = request_irq(udev->info.irq, igbuio_pci_irqhandler, - udev->info.irq_flags, udev->info.name, - udev); - dev_info(&udev->pdev->dev, "uio device registered with irq %lx\n", - udev->info.irq); - return err; } static void igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev) { - if (udev->info.irq) { - free_irq(udev->info.irq, udev); - udev->info.irq = 0; - } - #ifndef HAVE_ALLOC_IRQ_VECTORS if (udev->mode == RTE_INTR_MODE_MSIX) pci_disable_msix(udev->pdev); @@ -325,109 +370,6 @@ igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev) #endif } - -/** - * This gets called while opening uio device file. - */ -static int -igbuio_pci_open(struct uio_info *info, struct inode *inode) -{ - struct rte_uio_pci_dev *udev = info->priv; - struct pci_dev *dev = udev->pdev; - int err; - - pci_reset_function(dev); - - /* set bus master, which was cleared by the reset function */ - pci_set_master(dev); - - /* enable interrupts */ - err = igbuio_pci_enable_interrupts(udev); - if (err) { - dev_err(&dev->dev, "Enable interrupt fails\n"); - return err; - } - return 0; -} - -static int -igbuio_pci_release(struct uio_info *info, struct inode *inode) -{ - struct rte_uio_pci_dev *udev = info->priv; - struct pci_dev *dev = udev->pdev; - - /* disable interrupts */ - igbuio_pci_disable_interrupts(udev); - - /* stop the device from further DMA */ - pci_clear_master(dev); - - pci_reset_function(dev); - - return 0; -} - -/* Remap pci resources described by bar #pci_bar in uio resource n. */ -static int -igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info, - int n, int pci_bar, const char *name) -{ - unsigned long addr, len; - void *internal_addr; - - if (n >= ARRAY_SIZE(info->mem)) - return -EINVAL; - - addr = pci_resource_start(dev, pci_bar); - len = pci_resource_len(dev, pci_bar); - if (addr == 0 || len == 0) - return -1; - internal_addr = ioremap(addr, len); - if (internal_addr == NULL) - return -1; - info->mem[n].name = name; - info->mem[n].addr = addr; - info->mem[n].internal_addr = internal_addr; - info->mem[n].size = len; - info->mem[n].memtype = UIO_MEM_PHYS; - return 0; -} - -/* Get pci port io resources described by bar #pci_bar in uio resource n. */ -static int -igbuio_pci_setup_ioport(struct pci_dev *dev, struct uio_info *info, - int n, int pci_bar, const char *name) -{ - unsigned long addr, len; - - if (n >= ARRAY_SIZE(info->port)) - return -EINVAL; - - addr = pci_resource_start(dev, pci_bar); - len = pci_resource_len(dev, pci_bar); - if (addr == 0 || len == 0) - return -EINVAL; - - info->port[n].name = name; - info->port[n].start = addr; - info->port[n].size = len; - info->port[n].porttype = UIO_PORT_X86; - - return 0; -} - -/* Unmap previously ioremap'd resources */ -static void -igbuio_pci_release_iomem(struct uio_info *info) -{ - int i; - - for (i = 0; i < MAX_UIO_MAPS; i++) { - if (info->mem[i].internal_addr) - iounmap(info->mem[i].internal_addr); - } -} - static int igbuio_setup_bars(struct pci_dev *dev, struct uio_info *info) { @@ -518,16 +460,19 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) /* fill uio infos */ udev->info.name = "igb_uio"; udev->info.version = "0.1"; + udev->info.handler = igbuio_pci_irqhandler; udev->info.irqcontrol = igbuio_pci_irqcontrol; - udev->info.open = igbuio_pci_open; - udev->info.release = igbuio_pci_release; udev->info.priv = udev; udev->pdev = dev; - err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp); + err = igbuio_pci_enable_interrupts(udev); if (err != 0) goto fail_release_iomem; + err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp); + if (err != 0) + goto fail_disable_interrupts; + /* register uio driver */ err = uio_register_device(&dev->dev, &udev->info); if (err != 0) @@ -535,6 +480,9 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) pci_set_drvdata(dev, udev); + dev_info(&dev->dev, "uio device registered with irq %lx\n", + udev->info.irq); + /* * Doing a harmless dma mapping for attaching the device to * the iommu identity mapping if kernel boots with iommu=pt. @@ -560,6 +508,8 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) fail_remove_group: sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp); +fail_disable_interrupts: + igbuio_pci_disable_interrupts(udev); fail_release_iomem: igbuio_pci_release_iomem(&udev->info); pci_disable_device(dev); @@ -576,6 +526,7 @@ igbuio_pci_remove(struct pci_dev *dev) sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp); uio_unregister_device(&udev->info); + igbuio_pci_disable_interrupts(udev); igbuio_pci_release_iomem(&udev->info); pci_disable_device(dev); pci_set_drvdata(dev, NULL); -- 2.13.6