From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f173.google.com (mail-pd0-f173.google.com [209.85.192.173]) by dpdk.org (Postfix) with ESMTP id E8E79B36B for ; Fri, 18 Jul 2014 18:14:20 +0200 (CEST) Received: by mail-pd0-f173.google.com with SMTP id w10so5285673pde.18 for ; Fri, 18 Jul 2014 09:15:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:user-agent:date:from:to:cc:subject :references:mime-version:content-type:content-disposition; bh=+J9VZ4l4yLcKf9C0NB0KqoatsgLo0BO0cTW4JBU66uo=; b=iYTIwEZZSx/6Sa529eRXjxqSH37lyqZMTipj9jfWcEPbgERBMyzOxTafx8LGTMvTh/ CtLpvESt6Q4uKiqUbc9QOk3mfyri0WWeuf4cLHxmnsa5zpFp6zG/BM9XcMjfysuku2R7 /geNd5O8m6L8+QTlj9SnqgggKFgaUW4yKHYul3qi+1YvkY3SmyeOAsq8HIF1nsNvuR1L RCRVp8vWPU8Bu/1Z7Xo7c1/25/Za8h24fu7UKBj156oPj2Q90VAcPaJkHkngpJvq9mTH g0BvrxEnuIrdJLJAldhvWlh1Kdpv5z3D3g+n5idkrWTJHCyBFf0AcTqH/0FJ5NEurGEr QXAQ== X-Gm-Message-State: ALoCoQksvluhCR/XupNppBzJy/DqR3alNz6y8v58XfyaXcaGtxp8gEgAdhlhg2xG7zMPkoYmYGvv X-Received: by 10.66.163.98 with SMTP id yh2mr6614148pab.104.1405700121407; Fri, 18 Jul 2014 09:15:21 -0700 (PDT) Received: from localhost (static-50-53-66-61.bvtn.or.frontiernet.net. [50.53.66.61]) by mx.google.com with ESMTPSA id xc1sm24553932pab.39.2014.07.18.09.15.20 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Jul 2014 09:15:20 -0700 (PDT) Message-Id: <20140718161523.307218192@networkplumber.org> User-Agent: quilt/0.63-1 Date: Fri, 18 Jul 2014 09:14:54 -0700 From: Stephen Hemminger To: Thomas Monjalon References: <20140718161447.020882834@networkplumber.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Disposition: inline; filename=igb_uio-irq-mgmt.patch Cc: dev@dpdk.org Subject: [dpdk-dev] [PATCH 07/10] igb_uio: fix IRQ mode handling 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: Fri, 18 Jul 2014 16:14:21 -0000 This pach reworks how IRQ mode handling is done. The biggest code change is to use the standard INTX management code that exists in more recent kernels (and provide backport version). This also fixes the pci_lock code which was broken, since it was not protecting against config access, and was doing trylock. Make this driver behave like other Linux drivers. Start at MSI-X and degrade to less desireable modes automatically if the desired type is not available. This patch also makes MSI mode work, previously the mode was there but it would never work. Signed-off-by: Stephen Hemminger --- lib/librte_eal/common/include/rte_pci_dev_feature_defs.h | 3 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 243 +++++++-------- 2 files changed, 119 insertions(+), 127 deletions(-) --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c 2014-07-18 08:43:13.252721981 -0700 +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c 2014-07-18 08:47:06.813542050 -0700 @@ -52,7 +52,6 @@ struct rte_uio_pci_dev { struct uio_info info; struct pci_dev *pdev; - spinlock_t lock; /* spinlock for accessing PCI config space or msix data in multi tasks/isr */ enum rte_intr_mode mode; }; @@ -220,36 +219,67 @@ static const struct attribute_group dev_ .attrs = dev_attrs, }; -static inline int -pci_lock(struct pci_dev * pdev) -{ - /* Some function names changes between 3.2.0 and 3.3.0... */ + #if LINUX_VERSION_CODE < KERNEL_VERSION(3,3,0) - pci_block_user_cfg_access(pdev); - return 1; -#else - return pci_cfg_access_trylock(pdev); -#endif +/* Check if INTX works to control irq's. + * Set's INTX_DISABLE flag and reads it back + */ +static bool pci_intx_mask_supported(struct pci_dev *dev) +{ + bool mask_supported = false; + uint16_t orig, new + + pci_block_user_cfg_access(dev); + pci_read_config_word(pdev, PCI_COMMAND, &orig); + pci_write_config_word(dev, PCI_COMMAND, + orig ^ PCI_COMMAND_INTX_DISABLE); + pci_read_config_word(dev, PCI_COMMAND, &new); + + if ((new ^ orig) & ~PCI_COMMAND_INTX_DISABLE) { + dev_err(&dev->dev, "Command register changed from " + "0x%x to 0x%x: driver or hardware bug?\n", orig, new); + } else if ((new ^ orig) & PCI_COMMAND_INTX_DISABLE) { + mask_supported = true; + pci_write_config_word(dev, PCI_COMMAND, orig); + } + pci_unblock_user_cfg_access(dev); } -static inline void -pci_unlock(struct pci_dev * pdev) +static bool pci_check_and_mask_intx(struct pci_dev *pdev) { - /* Some function names changes between 3.2.0 and 3.3.0... */ -#if LINUX_VERSION_CODE < KERNEL_VERSION(3,3,0) - pci_unblock_user_cfg_access(pdev); -#else - pci_cfg_access_unlock(pdev); -#endif + bool pending; + uint32_t status; + + pci_block_user_cfg_access(dev); + pci_read_config_dword(pdev, PCI_COMMAND, &status); + + /* interrupt is not ours, goes to out */ + pending = (((status >> 16) & PCI_STATUS_INTERRUPT) != 0); + if (pending) { + uint16_t old, new; + + old = status; + if (state != 0) + new = old & (~PCI_COMMAND_INTX_DISABLE); + else + new = old | PCI_COMMAND_INTX_DISABLE; + + if (old != new) + pci_write_config_word(pdev, PCI_COMMAND, new); + } + pci_unblock_user_cfg_access(dev); + + return pending; } +#endif -/** +/* * It masks the msix on/off of generating MSI-X messages. */ -static int +static void igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state) { - uint32_t mask_bits = desc->masked; + u32 mask_bits = desc->masked; unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE + PCI_MSIX_ENTRY_VECTOR_CTRL; @@ -263,48 +293,25 @@ igbuio_msix_mask_irq(struct msi_desc *de readl(desc->mask_base); desc->masked = mask_bits; } - - return 0; } -/** - * This function sets/clears the masks for generating LSC interrupts. - * - * @param info - * The pointer to struct uio_info. - * @param on - * The on/off flag of masking LSC. - * @return - * -On success, zero value. - * -On failure, a negative value. - */ -static int -igbuio_set_interrupt_mask(struct rte_uio_pci_dev *udev, int32_t state) -{ - struct pci_dev *pdev = udev->pdev; - - if (udev->mode == RTE_INTR_MODE_MSIX) { - struct msi_desc *desc; - list_for_each_entry(desc, &pdev->msi_list, list) { - igbuio_msix_mask_irq(desc, state); - } - } else if (udev->mode == RTE_INTR_MODE_LEGACY) { - uint32_t status; - uint16_t old, new; +static void +igbuio_msi_mask_irq(struct irq_data *data, u32 enable) +{ + struct msi_desc *desc = irq_data_get_msi(data); + u32 mask_bits = desc->masked; + unsigned offset = data->irq - desc->dev->irq; + u32 mask = 1 << offset; + u32 flag = enable << offset; - pci_read_config_dword(pdev, PCI_COMMAND, &status); - old = status; - if (state != 0) - new = old & (~PCI_COMMAND_INTX_DISABLE); - else - new = old | PCI_COMMAND_INTX_DISABLE; + mask_bits &= ~mask; + mask_bits |= flag; - if (old != new) - pci_write_config_word(pdev, PCI_COMMAND, new); + if (desc->msi_attrib.maskbit && mask_bits != desc->masked) { + pci_write_config_dword(desc->dev, desc->mask_pos, mask_bits); + desc->masked = mask_bits; } - - return 0; } /** @@ -323,20 +330,23 @@ igbuio_set_interrupt_mask(struct rte_uio static int igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state) { - unsigned long flags; struct rte_uio_pci_dev *udev = igbuio_get_uio_pci_dev(info); struct pci_dev *pdev = udev->pdev; - spin_lock_irqsave(&udev->lock, flags); - if (!pci_lock(pdev)) { - spin_unlock_irqrestore(&udev->lock, flags); - return -1; - } + pci_cfg_access_lock(pdev); + if (udev->mode == RTE_INTR_MODE_LEGACY) + pci_intx(pdev, !!irq_state); + else if (udev->mode == RTE_INTR_MODE_MSI) { + struct irq_data *data = irq_get_irq_data(pdev->irq); - igbuio_set_interrupt_mask(udev, irq_state); + igbuio_msi_mask_irq(data, !!irq_state); + } else if (udev->mode == RTE_INTR_MODE_MSIX) { + struct msi_desc *desc; - pci_unlock(pdev); - spin_unlock_irqrestore(&udev->lock, flags); + list_for_each_entry(desc, &pdev->msi_list, list) + igbuio_msix_mask_irq(desc, irq_state); + } + pci_cfg_access_unlock(pdev); return 0; } @@ -348,37 +358,15 @@ igbuio_pci_irqcontrol(struct uio_info *i static irqreturn_t igbuio_pci_irqhandler(int irq, struct uio_info *info) { - irqreturn_t ret = IRQ_NONE; - unsigned long flags; struct rte_uio_pci_dev *udev = igbuio_get_uio_pci_dev(info); - struct pci_dev *pdev = udev->pdev; - uint32_t cmd_status_dword; - uint16_t status; - spin_lock_irqsave(&udev->lock, flags); - /* block userspace PCI config reads/writes */ - if (!pci_lock(pdev)) - goto spin_unlock; - - /* for legacy mode, interrupt maybe shared */ - if (udev->mode == RTE_INTR_MODE_LEGACY) { - pci_read_config_dword(pdev, PCI_COMMAND, &cmd_status_dword); - status = cmd_status_dword >> 16; - /* interrupt is not ours, goes to out */ - if (!(status & PCI_STATUS_INTERRUPT)) - goto done; - } - - igbuio_set_interrupt_mask(udev, 0); - ret = IRQ_HANDLED; -done: - /* unblock userspace PCI config reads/writes */ - pci_unlock(pdev); -spin_unlock: - spin_unlock_irqrestore(&udev->lock, flags); - pr_info("irq 0x%x %s\n", irq, (ret == IRQ_HANDLED) ? "handled" : "not handled"); + /* Legacy mode need to mask in hardware */ + if (udev->mode == RTE_INTR_MODE_LEGACY && + !pci_check_and_mask_intx(udev->pdev)) + return IRQ_NONE; - return ret; + /* Message signal mode, no share IRQ and automasked */ + return IRQ_HANDLED; } #ifdef CONFIG_XEN_DOM0 @@ -582,6 +570,7 @@ igbuio_pci_probe(struct pci_dev *dev, co udev->info.version = "0.1"; udev->info.handler = igbuio_pci_irqhandler; udev->info.irqcontrol = igbuio_pci_irqcontrol; + udev->info.irq = dev->irq; #ifdef CONFIG_XEN_DOM0 /* check if the driver run on Xen Dom0 */ if (xen_initial_domain()) @@ -589,38 +578,48 @@ igbuio_pci_probe(struct pci_dev *dev, co #endif udev->info.priv = udev; udev->pdev = dev; - udev->mode = RTE_INTR_MODE_LEGACY; - spin_lock_init(&udev->lock); - /* check if it need to try msix first */ - if (igbuio_intr_mode_preferred == RTE_INTR_MODE_MSIX) { + switch (igbuio_intr_mode_preferred) { + case RTE_INTR_MODE_NONE: + udev->info.irq = 0; + break; + + case RTE_INTR_MODE_MSIX: + /* Only 1 msi-x vector needed */ msix_entry.entry = 0; if (pci_enable_msix(dev, &msix_entry, 1) == 0) { + dev_dbg(&dev->dev, "using MSI-X"); + udev->info.irq = msix_entry.vector; udev->mode = RTE_INTR_MODE_MSIX; + break; } - else { - pci_disable_msix(udev->pdev); - pr_info("fail to enable pci msix, or not enough msix entries\n"); - } - } - switch (udev->mode) { - case RTE_INTR_MODE_MSIX: - udev->info.irq_flags = 0; - udev->info.irq = msix_entry.vector; - break; + /* fall back to MSI */ case RTE_INTR_MODE_MSI: - break; + if (pci_enable_msi(dev) == 0) { + dev_dbg(&dev->dev, "using MSI"); + udev->info.irq = dev->irq; + udev->mode = RTE_INTR_MODE_MSI; + break; + } + /* fall back to INTX */ case RTE_INTR_MODE_LEGACY: - udev->info.irq_flags = IRQF_SHARED; - udev->info.irq = dev->irq; + if (pci_intx_mask_supported(dev)) { + dev_dbg(&dev->dev, "using INTX"); + udev->info.irq_flags = IRQF_SHARED; + udev->mode = RTE_INTR_MODE_LEGACY; + } else { + dev_err(&dev->dev, "PCI INTX mask not supported\n"); + err = -EIO; + goto fail_release_iomem; + } break; default: - break; + dev_err(&dev->dev, "invalid IRQ mode %u", + igbuio_intr_mode_preferred); + err = -EINVAL; + goto fail_release_iomem; } - pci_set_drvdata(dev, udev); - igbuio_pci_irqcontrol(&udev->info, 0); - err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp); if (err != 0) goto fail_release_iomem; @@ -630,7 +629,10 @@ igbuio_pci_probe(struct pci_dev *dev, co if (err != 0) goto fail_remove_group; - printk(KERN_INFO "uio device registered with irq %lx\n", udev->info.irq); + pci_set_drvdata(dev, udev); + + dev_info(&dev->dev, "uio device registered with irq %lx\n", + udev->info.irq); return 0; @@ -640,6 +642,8 @@ fail_release_iomem: igbuio_pci_release_iomem(&udev->info); if (udev->mode == RTE_INTR_MODE_MSIX) pci_disable_msix(udev->pdev); + if (udev->mode == RTE_INTR_MODE_MSI) + pci_disable_msi(udev->pdev); pci_release_regions(dev); fail_disable: pci_disable_device(dev); --- a/lib/librte_eal/common/include/rte_pci_dev_feature_defs.h 2014-07-18 08:43:13.252721981 -0700 +++ b/lib/librte_eal/common/include/rte_pci_dev_feature_defs.h 2014-07-18 08:43:13.252721981 -0700 @@ -39,8 +39,7 @@ enum rte_intr_mode { RTE_INTR_MODE_NONE = 0, RTE_INTR_MODE_LEGACY, RTE_INTR_MODE_MSI, - RTE_INTR_MODE_MSIX, - RTE_INTR_MODE_MAX + RTE_INTR_MODE_MSIX }; #endif /* _RTE_PCI_DEV_DEFS_H_ */