From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id F2682A0613 for ; Tue, 24 Sep 2019 18:04:39 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C0B4D2C5E; Tue, 24 Sep 2019 18:04:34 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 19AB12E8F for ; Tue, 24 Sep 2019 18:04:32 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Sep 2019 09:04:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,544,1559545200"; d="scan'208";a="193486730" Received: from yexl-server.sh.intel.com (HELO localhost) ([10.67.117.17]) by orsmga006.jf.intel.com with ESMTP; 24 Sep 2019 09:04:30 -0700 Date: Wed, 25 Sep 2019 00:02:22 +0800 From: Ye Xiaolong To: Andy Pei Cc: dev@dpdk.org, rosen.xu@intel.com, tianfei.zhang@intel.com, qi.z.zhang@intel.com, david.lomartire@intel.com, ferruh.yigit@intel.com Message-ID: <20190924160222.GC67866@intel.com> References: <1568881185-89233-2-git-send-email-andy.pei@intel.com> <1568883774-92149-1-git-send-email-andy.pei@intel.com> <1568883774-92149-3-git-send-email-andy.pei@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1568883774-92149-3-git-send-email-andy.pei@intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH v6 02/17] raw/ifpga/base: add irq support 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 09/19, Andy Pei wrote: >From: Tianfei zhang > >Add irq support for ifpga FME globle error, port error and uint unit. s/globle/global >We implmented this feature by vfio interrupt mechanism. > >Signed-off-by: Tianfei zhang >Signed-off-by: Andy Pei >--- > drivers/raw/ifpga/base/ifpga_feature_dev.c | 60 ++++++++++++++++++++++++++++++ > drivers/raw/ifpga/base/ifpga_fme_error.c | 22 +++++++++++ > drivers/raw/ifpga/base/ifpga_port.c | 20 ++++++++++ > drivers/raw/ifpga/base/ifpga_port_error.c | 21 +++++++++++ > 4 files changed, 123 insertions(+) > >diff --git a/drivers/raw/ifpga/base/ifpga_feature_dev.c b/drivers/raw/ifpga/base/ifpga_feature_dev.c >index 63c8bcc..f0fb242 100644 >--- a/drivers/raw/ifpga/base/ifpga_feature_dev.c >+++ b/drivers/raw/ifpga/base/ifpga_feature_dev.c >@@ -3,6 +3,7 @@ > */ > > #include >+#include > > #include "ifpga_feature_dev.h" > >@@ -331,3 +332,62 @@ int port_hw_init(struct ifpga_port_hw *port) > port_hw_uinit(port); > return ret; > } >+ >+/* >+ * FIXME: we should get msix vec count during pci enumeration instead of >+ * below hardcode value. >+ */ >+#define FPGA_MSIX_VEC_COUNT 20 So what is preventing us from getting msix vec count from pci enumeration? >+/* irq set buffer length for interrupt */ >+#define MSIX_IRQ_SET_BUF_LEN (sizeof(struct vfio_irq_set) + \ >+ sizeof(int) * FPGA_MSIX_VEC_COUNT) >+ >+/* only support msix for now*/ >+static int vfio_msix_enable_block(s32 vfio_dev_fd, unsigned int vec_start, >+ unsigned int count, s32 *fds) DPDK convention is put the function return type in a separate line. >+{ >+ char irq_set_buf[MSIX_IRQ_SET_BUF_LEN]; >+ struct vfio_irq_set *irq_set; >+ int len, ret; >+ int *fd_ptr; >+ >+ len = sizeof(irq_set_buf); >+ >+ irq_set = (struct vfio_irq_set *)irq_set_buf; >+ irq_set->argsz = len; >+ irq_set->count = count; >+ irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | >+ VFIO_IRQ_SET_ACTION_TRIGGER; >+ irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX; >+ irq_set->start = vec_start; >+ >+ fd_ptr = (int *)&irq_set->data; >+ opae_memcpy(fd_ptr, fds, sizeof(int) * count); >+ >+ ret = ioctl(vfio_dev_fd, VFIO_DEVICE_SET_IRQS, irq_set); >+ if (ret) >+ printf("Error enabling MSI-X interrupts\n"); >+ >+ return ret; >+} >+ >+int fpga_msix_set_block(struct ifpga_feature *feature, unsigned int start, >+ unsigned int count, s32 *fds) Ditto. >+{ >+ struct feature_irq_ctx *ctx = feature->ctx; >+ unsigned int i; >+ int ret; >+ >+ if (start >= feature->ctx_num || start + count > feature->ctx_num) >+ return -EINVAL; >+ >+ /* assume that each feature has continuous vector space in msix*/ >+ ret = vfio_msix_enable_block(feature->vfio_dev_fd, >+ ctx[start].idx, count, fds); >+ if (!ret) { >+ for (i = 0; i < count; i++) >+ ctx[i].eventfd = fds[i]; >+ } >+ >+ return ret; >+} >diff --git a/drivers/raw/ifpga/base/ifpga_fme_error.c b/drivers/raw/ifpga/base/ifpga_fme_error.c >index 3794564..068f52c 100644 >--- a/drivers/raw/ifpga/base/ifpga_fme_error.c >+++ b/drivers/raw/ifpga/base/ifpga_fme_error.c >@@ -373,9 +373,31 @@ static int fme_global_error_set_prop(struct ifpga_feature *feature, > return -ENOENT; > } > >+static int fme_global_err_set_irq(struct ifpga_feature *feature, void *irq_set) Ditto. >+{ >+ struct fpga_fme_err_irq_set *err_irq_set = >+ (struct fpga_fme_err_irq_set *)irq_set; Cast is not needed for void *. >+ struct ifpga_fme_hw *fme; >+ int ret; >+ >+ fme = (struct ifpga_fme_hw *)feature->parent; Ditto. >+ >+ spinlock_lock(&fme->lock); >+ if (!(fme->capability & FPGA_FME_CAP_ERR_IRQ)) { >+ spinlock_unlock(&fme->lock); >+ return -ENODEV; >+ } >+ >+ ret = fpga_msix_set_block(feature, 0, 1, &err_irq_set->evtfd); >+ spinlock_unlock(&fme->lock); >+ >+ return ret; >+} >+ > struct ifpga_feature_ops fme_global_err_ops = { > .init = fme_global_error_init, > .uinit = fme_global_error_uinit, > .get_prop = fme_global_error_get_prop, > .set_prop = fme_global_error_set_prop, >+ .set_irq = fme_global_err_set_irq, > }; >diff --git a/drivers/raw/ifpga/base/ifpga_port.c b/drivers/raw/ifpga/base/ifpga_port.c >index 6c41164..56b04a6 100644 >--- a/drivers/raw/ifpga/base/ifpga_port.c >+++ b/drivers/raw/ifpga/base/ifpga_port.c >@@ -384,9 +384,29 @@ static void port_uint_uinit(struct ifpga_feature *feature) > dev_info(NULL, "PORT UINT UInit.\n"); > } > >+static int port_uint_set_irq(struct ifpga_feature *feature, void *irq_set) Ditto. >+{ >+ struct fpga_uafu_irq_set *uafu_irq_set = irq_set; >+ struct ifpga_port_hw *port = feature->parent; >+ int ret; >+ >+ spinlock_lock(&port->lock); >+ if (!(port->capability & FPGA_PORT_CAP_UAFU_IRQ)) { >+ spinlock_unlock(&port->lock); >+ return -ENODEV; >+ } >+ >+ ret = fpga_msix_set_block(feature, uafu_irq_set->start, >+ uafu_irq_set->count, uafu_irq_set->evtfds); >+ spinlock_unlock(&port->lock); >+ >+ return ret; >+} >+ > struct ifpga_feature_ops ifpga_rawdev_port_uint_ops = { > .init = port_uint_init, > .uinit = port_uint_uinit, >+ .set_irq = port_uint_set_irq, > }; > > static int port_afu_init(struct ifpga_feature *feature) >diff --git a/drivers/raw/ifpga/base/ifpga_port_error.c b/drivers/raw/ifpga/base/ifpga_port_error.c >index 138284e..8aef7d7 100644 >--- a/drivers/raw/ifpga/base/ifpga_port_error.c >+++ b/drivers/raw/ifpga/base/ifpga_port_error.c >@@ -136,9 +136,30 @@ static int port_error_set_prop(struct ifpga_feature *feature, > return -ENOENT; > } > >+static int port_error_set_irq(struct ifpga_feature *feature, void *irq_set) Ditto. >+{ >+ struct fpga_port_err_irq_set *err_irq_set = irq_set; >+ struct ifpga_port_hw *port; >+ int ret; >+ >+ port = feature->parent; >+ >+ spinlock_lock(&port->lock); >+ if (!(port->capability & FPGA_PORT_CAP_ERR_IRQ)) { >+ spinlock_unlock(&port->lock); >+ return -ENODEV; >+ } >+ >+ ret = fpga_msix_set_block(feature, 0, 1, &err_irq_set->evtfd); >+ spinlock_unlock(&port->lock); >+ >+ return ret; >+} >+ Above 3 new functions have a lot of similarity, better to extract out common function to reduce duplication. Thanks, Xiaolong > struct ifpga_feature_ops ifpga_rawdev_port_error_ops = { > .init = port_error_init, > .uinit = port_error_uinit, > .get_prop = port_error_get_prop, > .set_prop = port_error_set_prop, >+ .set_irq = port_error_set_irq, > }; >-- >1.8.3.1 >