diff mbox

[dpdk-dev,v2,01/11] eal/linux: vfio map misc intr to vector zero

Message ID 1446182873-28814-2-git-send-email-cunming.liang@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Cunming Liang Oct. 30, 2015, 5:27 a.m. UTC 
  During VFIO_DEVICE_SET_IRQS, the previous order is {Q0_fd, ... Qn_fd, misc_fd}.
The vector number of misc is indeterminable which is ugly to some NIC(e.g. i40e, fm10k).
The patch adjusts the order in {misc_fd, Q0_fd, ... Qn_fd}, always reserve the first vector to misc interrupt.

Signed-off-by: Cunming Liang <cunming.liang@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_interrupts.c           | 18 ++++++++++++------
 .../linuxapp/eal/include/exec-env/rte_interrupts.h     |  3 +++
 2 files changed, 15 insertions(+), 6 deletions(-)

Comments

He, Shaopeng Oct. 30, 2015, 7:11 a.m. UTC | #1 
> -----Original Message-----
> From: Liang, Cunming
> Sent: Friday, October 30, 2015 1:28 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin; He, Shaopeng; Wu, Jingjing; Liang, Cunming
> Subject: [PATCH v2 01/11] eal/linux: vfio map misc intr to vector zero
> 
> During VFIO_DEVICE_SET_IRQS, the previous order is {Q0_fd, ... Qn_fd,
> misc_fd}.
> The vector number of misc is indeterminable which is ugly to some NIC(e.g.
> i40e, fm10k).
> The patch adjusts the order in {misc_fd, Q0_fd, ... Qn_fd}, always reserve
> the first vector to misc interrupt.
> 
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
Acked-by : Shaopeng He <shaopeng.he@intel.com>
Zhang, Helin Oct. 30, 2015, 7:33 a.m. UTC | #2 
> -----Original Message-----
> From: Liang, Cunming
> Sent: Friday, October 30, 2015 1:28 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin; He, Shaopeng; Wu, Jingjing; Liang, Cunming
> Subject: [PATCH v2 01/11] eal/linux: vfio map misc intr to vector zero
> 
> During VFIO_DEVICE_SET_IRQS, the previous order is {Q0_fd, ... Qn_fd, misc_fd}.
> The vector number of misc is indeterminable which is ugly to some NIC(e.g. i40e,
> fm10k).
> The patch adjusts the order in {misc_fd, Q0_fd, ... Qn_fd}, always reserve the
> first vector to misc interrupt.
> 
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
Cunming Liang Oct. 30, 2015, 2:22 p.m. UTC | #3 
Hi David,

May I ask your help to review the patch related to eal ?

Thanks,
Steve
> -----Original Message-----
> From: Liang, Cunming
> Sent: Friday, October 30, 2015 1:28 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin; He, Shaopeng; Wu, Jingjing; Liang, Cunming
> Subject: [PATCH v2 01/11] eal/linux: vfio map misc intr to vector zero
> 
> During VFIO_DEVICE_SET_IRQS, the previous order is {Q0_fd, ... Qn_fd, misc_fd}.
> The vector number of misc is indeterminable which is ugly to some NIC(e.g. i40e,
> fm10k).
> The patch adjusts the order in {misc_fd, Q0_fd, ... Qn_fd}, always reserve the first
> vector to misc interrupt.
> 
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> ---
>  lib/librte_eal/linuxapp/eal/eal_interrupts.c           | 18
> ++++++++++++------
>  .../linuxapp/eal/include/exec-env/rte_interrupts.h     |  3 +++
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> index 078318c..8e76a7a 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> @@ -300,8 +300,10 @@ vfio_enable_msix(struct rte_intr_handle *intr_handle)
> {
>  	irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
>  	irq_set->start = 0;
>  	fd_ptr = (int *) &irq_set->data;
> -	memcpy(fd_ptr, intr_handle->efds, sizeof(intr_handle->efds));
> -	fd_ptr[intr_handle->max_intr - 1] = intr_handle->fd;
> +	fd_ptr[MISC_VEC_ID] = intr_handle->fd;
> +	/* follow up with misc(0) interrupt */
> +	memcpy(&fd_ptr[RX_VEC_START], intr_handle->efds,
> +		sizeof(*intr_handle->efds) * intr_handle->nb_efd);
> 
>  	ret = ioctl(intr_handle->vfio_dev_fd, VFIO_DEVICE_SET_IRQS, irq_set);
> 
> @@ -1068,10 +1070,13 @@ rte_intr_rx_ctl(struct rte_intr_handle *intr_handle,
> int epfd,
>  	struct rte_epoll_event *rev;
>  	struct rte_epoll_data *epdata;
>  	int epfd_op;
> +	unsigned int efd_idx;
>  	int rc = 0;
> 
> +	efd_idx = (vec >= RX_VEC_START) ? (vec - RX_VEC_START) : vec;
> +
>  	if (!intr_handle || intr_handle->nb_efd == 0 ||
> -	    vec >= intr_handle->nb_efd) {
> +	    efd_idx >= intr_handle->nb_efd) {
>  		RTE_LOG(ERR, EAL, "Wrong intr vector number.\n");
>  		return -EPERM;
>  	}
> @@ -1079,7 +1084,7 @@ rte_intr_rx_ctl(struct rte_intr_handle *intr_handle,
> int epfd,
>  	switch (op) {
>  	case RTE_INTR_EVENT_ADD:
>  		epfd_op = EPOLL_CTL_ADD;
> -		rev = &intr_handle->elist[vec];
> +		rev = &intr_handle->elist[efd_idx];
>  		if (rev->status != RTE_EPOLL_INVALID) {
>  			RTE_LOG(INFO, EAL, "Event already been added.\n");
>  			return -EEXIST;
> @@ -1091,7 +1096,8 @@ rte_intr_rx_ctl(struct rte_intr_handle *intr_handle,
> int epfd,
>  		epdata->data   = data;
>  		epdata->cb_fun = (rte_intr_event_cb_t)eal_intr_proc_rxtx_intr;
>  		epdata->cb_arg = (void *)intr_handle;
> -		rc = rte_epoll_ctl(epfd, epfd_op, intr_handle->efds[vec], rev);
> +		rc = rte_epoll_ctl(epfd, epfd_op,
> +				   intr_handle->efds[efd_idx], rev);
>  		if (!rc)
>  			RTE_LOG(DEBUG, EAL,
>  				"efd %d associated with vec %d added on epfd %d"
> @@ -1101,7 +1107,7 @@ rte_intr_rx_ctl(struct rte_intr_handle *intr_handle,
> int epfd,
>  		break;
>  	case RTE_INTR_EVENT_DEL:
>  		epfd_op = EPOLL_CTL_DEL;
> -		rev = &intr_handle->elist[vec];
> +		rev = &intr_handle->elist[efd_idx];
>  		if (rev->status == RTE_EPOLL_INVALID) {
>  			RTE_LOG(INFO, EAL, "Event does not exist.\n");
>  			return -EPERM;
> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> index 45071b7..b8fd318 100644
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> @@ -77,6 +77,9 @@ struct rte_epoll_event {
>  	struct rte_epoll_data epdata;
>  };
> 
> +#define MISC_VEC_ID                (0)
> +#define RX_VEC_START               (MISC_VEC_ID + 1)
> +
>  /** Handle for interrupts. */
>  struct rte_intr_handle {
>  	union {
> --
> 2.4.3
David Marchand Nov. 2, 2015, 3:53 p.m. UTC | #4 
Hello,

On Fri, Oct 30, 2015 at 6:27 AM, Cunming Liang <cunming.liang@intel.com>
wrote:
[snip]

> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> index 45071b7..b8fd318 100644
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> @@ -77,6 +77,9 @@ struct rte_epoll_event {
>         struct rte_epoll_data epdata;
>  };
>
> +#define MISC_VEC_ID                (0)
>

"misc" is not really descriptive ...


> +#define RX_VEC_START               (MISC_VEC_ID + 1)
> +
>

Please, prefix these macros properly.
Else, when looking at the driver code, this kind of macros seems to be
local to the driver.
Cunming Liang Nov. 4, 2015, 1:17 a.m. UTC | #5 
Hi David,

On 11/2/2015 11:53 PM, David Marchand wrote:
> Hello,
>
> On Fri, Oct 30, 2015 at 6:27 AM, Cunming Liang <cunming.liang@intel.com>
> wrote:
> [snip]
>
>> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
>> b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
>> index 45071b7..b8fd318 100644
>> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
>> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
>> @@ -77,6 +77,9 @@ struct rte_epoll_event {
>>          struct rte_epoll_data epdata;
>>   };
>>
>> +#define MISC_VEC_ID                (0)
>>
> "misc" is not really descriptive ...
>
>
>> +#define RX_VEC_START               (MISC_VEC_ID + 1)
>> +
>>
> Please, prefix these macros properly.
> Else, when looking at the driver code, this kind of macros seems to be
> local to the driver.
>
>
That makes sense, will fix it in v3. Thanks. /Steve
Cunming Liang Nov. 4, 2015, 6:07 a.m. UTC | #6 
v3 change:
  - rename MISC_VEC_ID and RX_VEC_START
  - add bsdapp dummy
  - split cleanup and fix patches
  - merge doc update along with code change

v2 change:
  - rework to depend on one previous patch
    patch http://dpdk.org/dev/patchwork/patch/7504/
  - always set DIS_AUTOMASK_* bit in PF to avoid ENA flag auto-clear

This patch series contains four major parts.

1. always reserve vector zero for misc cause in vfio mapping
2. add api to declare the capability of multiple interrupt vector support
3. fix the rx interrupt compatible issue with mbox in ixgbe/igb IOV-PF
4. add rx interrupt support in i40e PF and VF


Cunming Liang (13):
  eal: vfio map misc intr to vector zero
  ixgbe: reserve intr vector zero for misc cause
  igb: reserve intr vector zero for misc cause
  eal/linux: not allow to enable zero intr efd
  ixgbe: fix efd_enable with zero number
  igb: fix efd_enable with zero number
  eal: add intr api to report multi-vector capability
  ixgbe: fix rx intr compatible issue with PF mbox
  ixgbe: fix unnecessary intr_vec free in dev_close
  ixgbevf: cleanup unnecessary interrupt handler
  igb: fix rx intr compatible issue with PF mbox
  i40e: add rx interrupt support
  i40evf: add rx interrupt support

 doc/guides/rel_notes/release_2_2.rst               |   5 +
 drivers/net/e1000/e1000_ethdev.h                   |   3 +
 drivers/net/e1000/igb_ethdev.c                     |  63 +++-
 drivers/net/i40e/i40e_ethdev.c                     | 379 +++++++++++++++++----
 drivers/net/i40e/i40e_ethdev.h                     |  20 ++
 drivers/net/i40e/i40e_ethdev_vf.c                  | 144 +++++++-
 drivers/net/i40e/i40e_pf.c                         |   7 +-
 drivers/net/ixgbe/ixgbe_ethdev.c                   | 144 +++-----
 drivers/net/ixgbe/ixgbe_ethdev.h                   |   3 +
 lib/librte_eal/bsdapp/eal/eal_interrupts.c         |   7 +
 .../bsdapp/eal/include/exec-env/rte_interrupts.h   |  16 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map      |   7 +
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       |  36 +-
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |  15 +-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map    |   7 +
 15 files changed, 654 insertions(+), 202 deletions(-)
Cunming Liang Nov. 4, 2015, 8:45 a.m. UTC | #7 
v4 change:
  - remove redundancy condition check on PF

v3 changes:
  - rename MISC_VEC_ID and RX_VEC_START
  - add bsdapp dummy
  - split cleanup and fix patches
  - merge doc update along with code change

v2 changes:
  - rework to depend on one previous patch
    patch http://dpdk.org/dev/patchwork/patch/7504/
  - always set DIS_AUTOMASK_* bit in PF to avoid ENA flag auto-clear

This patch series contains four major parts.

1. always reserve vector zero for misc cause in vfio mapping
2. add api to declare the capability of multiple interrupt vector support
3. fix the rx interrupt compatible issue with mbox in ixgbe/igb IOV-PF
4. add rx interrupt support in i40e PF and VF

Cunming Liang (13):
  eal: vfio map misc intr to vector zero
  ixgbe: reserve intr vector zero for misc cause
  igb: reserve intr vector zero for misc cause
  eal/linux: not allow to enable zero intr efd
  ixgbe: fix efd_enable with zero number
  igb: fix efd_enable with zero number
  eal: add intr api to report multi-vector capability
  ixgbe: fix rx intr compatible issue with PF mbox
  ixgbe: fix unnecessary intr_vec free in dev_close
  ixgbevf: cleanup unnecessary interrupt handler
  igb: fix rx intr compatible issue with PF mbox
  i40e: add rx interrupt support
  i40evf: add rx interrupt support

 doc/guides/rel_notes/release_2_2.rst               |   5 +
 drivers/net/e1000/e1000_ethdev.h                   |   3 +
 drivers/net/e1000/igb_ethdev.c                     |  62 +++-
 drivers/net/i40e/i40e_ethdev.c                     | 378 +++++++++++++++++----
 drivers/net/i40e/i40e_ethdev.h                     |  20 ++
 drivers/net/i40e/i40e_ethdev_vf.c                  | 144 +++++++-
 drivers/net/i40e/i40e_pf.c                         |   7 +-
 drivers/net/ixgbe/ixgbe_ethdev.c                   | 143 +++-----
 drivers/net/ixgbe/ixgbe_ethdev.h                   |   3 +
 lib/librte_eal/bsdapp/eal/eal_interrupts.c         |   7 +
 .../bsdapp/eal/include/exec-env/rte_interrupts.h   |  16 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map      |   7 +
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       |  36 +-
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |  15 +-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map    |   7 +
 15 files changed, 651 insertions(+), 202 deletions(-)
Thomas Monjalon Nov. 4, 2015, 2:29 p.m. UTC | #8 
2015-11-04 16:45, Cunming Liang:
> v4 change:
>   - remove redundancy condition check on PF
> 
> v3 changes:
>   - rename MISC_VEC_ID and RX_VEC_START
>   - add bsdapp dummy
>   - split cleanup and fix patches
>   - merge doc update along with code change
> 
> v2 changes:
>   - rework to depend on one previous patch
>     patch http://dpdk.org/dev/patchwork/patch/7504/
>   - always set DIS_AUTOMASK_* bit in PF to avoid ENA flag auto-clear
> 
> This patch series contains four major parts.
> 
> 1. always reserve vector zero for misc cause in vfio mapping
> 2. add api to declare the capability of multiple interrupt vector support
> 3. fix the rx interrupt compatible issue with mbox in ixgbe/igb IOV-PF
> 4. add rx interrupt support in i40e PF and VF

Applied, thanks
diff mbox

Patch

diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 078318c..8e76a7a 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -300,8 +300,10 @@  vfio_enable_msix(struct rte_intr_handle *intr_handle) {
 	irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
 	irq_set->start = 0;
 	fd_ptr = (int *) &irq_set->data;
-	memcpy(fd_ptr, intr_handle->efds, sizeof(intr_handle->efds));
-	fd_ptr[intr_handle->max_intr - 1] = intr_handle->fd;
+	fd_ptr[MISC_VEC_ID] = intr_handle->fd;
+	/* follow up with misc(0) interrupt */
+	memcpy(&fd_ptr[RX_VEC_START], intr_handle->efds,
+		sizeof(*intr_handle->efds) * intr_handle->nb_efd);
 
 	ret = ioctl(intr_handle->vfio_dev_fd, VFIO_DEVICE_SET_IRQS, irq_set);
 
@@ -1068,10 +1070,13 @@  rte_intr_rx_ctl(struct rte_intr_handle *intr_handle, int epfd,
 	struct rte_epoll_event *rev;
 	struct rte_epoll_data *epdata;
 	int epfd_op;
+	unsigned int efd_idx;
 	int rc = 0;
 
+	efd_idx = (vec >= RX_VEC_START) ? (vec - RX_VEC_START) : vec;
+
 	if (!intr_handle || intr_handle->nb_efd == 0 ||
-	    vec >= intr_handle->nb_efd) {
+	    efd_idx >= intr_handle->nb_efd) {
 		RTE_LOG(ERR, EAL, "Wrong intr vector number.\n");
 		return -EPERM;
 	}
@@ -1079,7 +1084,7 @@  rte_intr_rx_ctl(struct rte_intr_handle *intr_handle, int epfd,
 	switch (op) {
 	case RTE_INTR_EVENT_ADD:
 		epfd_op = EPOLL_CTL_ADD;
-		rev = &intr_handle->elist[vec];
+		rev = &intr_handle->elist[efd_idx];
 		if (rev->status != RTE_EPOLL_INVALID) {
 			RTE_LOG(INFO, EAL, "Event already been added.\n");
 			return -EEXIST;
@@ -1091,7 +1096,8 @@  rte_intr_rx_ctl(struct rte_intr_handle *intr_handle, int epfd,
 		epdata->data   = data;
 		epdata->cb_fun = (rte_intr_event_cb_t)eal_intr_proc_rxtx_intr;
 		epdata->cb_arg = (void *)intr_handle;
-		rc = rte_epoll_ctl(epfd, epfd_op, intr_handle->efds[vec], rev);
+		rc = rte_epoll_ctl(epfd, epfd_op,
+				   intr_handle->efds[efd_idx], rev);
 		if (!rc)
 			RTE_LOG(DEBUG, EAL,
 				"efd %d associated with vec %d added on epfd %d"
@@ -1101,7 +1107,7 @@  rte_intr_rx_ctl(struct rte_intr_handle *intr_handle, int epfd,
 		break;
 	case RTE_INTR_EVENT_DEL:
 		epfd_op = EPOLL_CTL_DEL;
-		rev = &intr_handle->elist[vec];
+		rev = &intr_handle->elist[efd_idx];
 		if (rev->status == RTE_EPOLL_INVALID) {
 			RTE_LOG(INFO, EAL, "Event does not exist.\n");
 			return -EPERM;
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
index 45071b7..b8fd318 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
@@ -77,6 +77,9 @@  struct rte_epoll_event {
 	struct rte_epoll_data epdata;
 };
 
+#define MISC_VEC_ID                (0)
+#define RX_VEC_START               (MISC_VEC_ID + 1)
+
 /** Handle for interrupts. */
 struct rte_intr_handle {
 	union {