From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 4864D9A8A for ; Mon, 25 May 2015 08:01:30 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 24 May 2015 23:01:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,489,1427785200"; d="scan'208";a="715119682" Received: from shwdeisgchi017.ccr.corp.intel.com (HELO [10.239.66.47]) ([10.239.66.47]) by fmsmga001.fm.intel.com with ESMTP; 24 May 2015 23:01:15 -0700 Message-ID: <5562BAAA.5050301@intel.com> Date: Mon, 25 May 2015 14:01:14 +0800 From: "Liang, Cunming" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Stephen Hemminger , cumming.lian@intel.com References: <1431970814-25951-1-git-send-email-stephen@networkplumber.org> <1431970814-25951-5-git-send-email-stephen@networkplumber.org> In-Reply-To: <1431970814-25951-5-git-send-email-stephen@networkplumber.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 4/5] uio: new driver with MSI-X support 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: Mon, 25 May 2015 06:01:32 -0000 On 5/19/2015 1:40 AM, Stephen Hemminger wrote: > + > +/* set the mapping between vector # and existing eventfd. */ > +static int set_irq_eventfd(struct uio_msi_pci_dev *udev, u32 vec, int fd) > +{ > + struct uio_msi_irq_ctx *ctx; > + struct eventfd_ctx *trigger; > + int irq, err; > + > + if (vec >= udev->num_vectors) { > + dev_notice(&udev->pdev->dev, "vec %u >= num_vec %u\n", > + vec, udev->num_vectors); > + return -ERANGE; > + } > + > + irq = udev->msix[vec].vector; > + > + /* Clearup existing irq mapping */ > + ctx = &udev->ctx[vec]; > + if (ctx->trigger) { > + free_irq(irq, ctx->trigger); > + eventfd_ctx_put(ctx->trigger); > + ctx->trigger = NULL; > + } > + > + /* Passing -1 is used to disable interrupt */ > + if (fd < 0) > + return 0; > + > + One unnecessary blank line here. > + trigger = eventfd_ctx_fdget(fd); > + if (IS_ERR(trigger)) { > + err = PTR_ERR(trigger); > + dev_notice(&udev->pdev->dev, > + "eventfd ctx get failed: %d\n", err); > + return err; > + } > + > + err = request_irq(irq, uio_msi_irqhandler, 0, ctx->name, trigger); > + if (err) { > + dev_notice(&udev->pdev->dev, > + "request irq failed: %d\n", err); > + eventfd_ctx_put(trigger); > + return err; > + } > + > + dev_dbg(&udev->pdev->dev, "map vector %u to fd %d trigger %p\n", > + vec, fd, trigger); > + ctx->trigger = trigger; > + return 0; > +} > + > +static int > +uio_msi_ioctl(struct uio_info *info, unsigned int cmd, unsigned long arg) > +{ > + struct uio_msi_pci_dev *udev > + = container_of(info, struct uio_msi_pci_dev, info); > + struct uio_msi_irq_set hdr; > + int err; > + > + switch (cmd) { > + case UIO_MSI_IRQ_SET: > + if (copy_from_user(&hdr, (void __user *)arg, sizeof(hdr))) > + return -EFAULT; > + > + mutex_lock(&udev->mutex); > + err = set_irq_eventfd(udev, hdr.vec, hdr.fd); > + mutex_unlock(&udev->mutex); > + break; > + default: > + err = -EOPNOTSUPP; > + } > + return err; > +} "uio_msi_irq_set" defines in single pattern. Compare with the bulk set in "vfio_irq_set", it requires additional syscall during uio_msix_enable(). > + > +static int uio_msi_probe(struct pci_dev *pdev, const struct pci_device_id *id) > +{ > + struct uio_msi_pci_dev *udev; > + int i, err, vectors; > + > + udev = kzalloc(sizeof(struct uio_msi_pci_dev), GFP_KERNEL); > + if (!udev) > + return -ENOMEM; > + > + err = pci_enable_device(pdev); > + if (err != 0) { > + dev_err(&pdev->dev, "cannot enable PCI device\n"); > + goto fail_free; > + } > + > + vectors = pci_msix_vec_count(pdev); > + if (vectors < 0) { > + dev_err(&pdev->dev, "device does not support MSI-X\n"); > + err = -EINVAL; > + goto fail_disable; > + } pci_msix_vec_count() is available since v3.14, it requires a compatible check. In order to support older version, probably a function 'uio_msix_vec_count()' is necessary. I've one overall question, is there a special reason not enhance igb_uio but define a new uio_msi? And the looks like the piece could be add into igb_uio or uio_pci_generic as well. Do you have plan for the latter? Thanks.