[dpdk-dev] [RFC PATCH] vfio: avoid re-installing irq handler

[dpdk-dev] [RFC PATCH] vfio: avoid re-installing irq handler

[Hyong Youb Kim]

A rough patch for the approach mentioned earlier. It is only for
discussion.
http://mails.dpdk.org/archives/dev/2019-July/138113.html

To try this out, first revert the following then apply.
commit 89aac60e0be9 ("vfio: fix interrupts race condition")

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
---
 .../common/include/rte_eal_interrupts.h       |  1 +
 lib/librte_eal/linux/eal/eal_interrupts.c     | 68 ++++++++++++++++++-
 2 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
index b370c0d26..d509967d3 100644
--- a/lib/librte_eal/common/include/rte_eal_interrupts.h
+++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
@@ -76,6 +76,7 @@ struct rte_intr_handle {
 	enum rte_intr_handle_type type;  /**< handle type */
 	uint32_t max_intr;             /**< max interrupt requested */
 	uint32_t nb_efd;               /**< number of available efd(event fd) */
+	uint8_t vfio_efd_assigned;
 	uint8_t efd_counter_size;      /**< size of efd counter, used for vdev */
 	int efds[RTE_MAX_RXTX_INTR_VEC_ID];  /**< intr vectors/efds mapping */
 	struct rte_epoll_event elist[RTE_MAX_RXTX_INTR_VEC_ID];
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 79ad5e8d7..5b03388dd 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -107,9 +107,21 @@ static pthread_t intr_thread;
 #define MSIX_IRQ_SET_BUF_LEN (sizeof(struct vfio_irq_set) + \
 			      sizeof(int) * (RTE_MAX_RXTX_INTR_VEC_ID + 1))
 
-/* enable legacy (INTx) interrupts */
+#ifndef __INTEL_COMPILER
+#pragma GCC diagnostic ignored "-Wcast-qual"
+#endif
+
+static void
+set_vfio_efd_assigned(const struct rte_intr_handle *intr_handle, uint8_t v) {
+	((struct rte_intr_handle *)intr_handle)->vfio_efd_assigned = v;
+}
+
+#ifndef __INTEL_COMPILER
+#pragma GCC diagnostic error "-Wcast-qual"
+#endif
+
 static int
-vfio_enable_intx(const struct rte_intr_handle *intr_handle) {
+vfio_assign_efd_intx(const struct rte_intr_handle *intr_handle) {
 	struct vfio_irq_set *irq_set;
 	char irq_set_buf[IRQ_SET_BUF_LEN];
 	int len, ret;
@@ -134,8 +146,36 @@ vfio_enable_intx(const struct rte_intr_handle *intr_handle) {
 						intr_handle->fd);
 		return -1;
 	}
+	/*
+	 * In the kernel, irq has been set up, its handler installed (request_irq),
+	 * and efd is assigned to it.  Remember this so we can avoid
+	 * re-doing irq setup later.
+	 */
+	set_vfio_efd_assigned(intr_handle, 1);
+	return 0;
+}
+
+/* enable legacy (INTx) interrupts */
+static int
+vfio_enable_intx(const struct rte_intr_handle *intr_handle) {
+	struct vfio_irq_set *irq_set;
+	char irq_set_buf[IRQ_SET_BUF_LEN];
+	int len, ret;
+
+	/*
+	 * Assign efd to irq only if it's not been done, to avoid
+	 * re-installing irq handler in the kernel.
+	 */
+	if (!intr_handle->vfio_efd_assigned) {
+		ret = vfio_assign_efd_intx(intr_handle);
+		if (ret)
+			return ret;
+	}
+
+	len = sizeof(irq_set_buf);
 
 	/* unmask INTx after enabling */
+	irq_set = (struct vfio_irq_set *) irq_set_buf;
 	memset(irq_set, 0, len);
 	len = sizeof(struct vfio_irq_set);
 	irq_set->argsz = len;
@@ -194,6 +234,12 @@ vfio_disable_intx(const struct rte_intr_handle *intr_handle) {
 			"Error disabling INTx interrupts for fd %d\n", intr_handle->fd);
 		return -1;
 	}
+	/*
+	 * Irq handler's been removed in the kernel. The next time we
+	 * enable intr_handle, we need to assign efd to irq. Remember
+	 * that.
+	 */
+	set_vfio_efd_assigned(intr_handle, 0);
 	return 0;
 }
 
@@ -205,6 +251,13 @@ vfio_enable_msi(const struct rte_intr_handle *intr_handle) {
 	struct vfio_irq_set *irq_set;
 	int *fd_ptr;
 
+	/*
+	 * vfio does not support MSI masking/unmaking. So if irq has
+	 * been set up previously, nothing to do.
+	 */
+	if (intr_handle->vfio_efd_assigned)
+		return 0;
+
 	len = sizeof(irq_set_buf);
 
 	irq_set = (struct vfio_irq_set *) irq_set_buf;
@@ -223,6 +276,7 @@ vfio_enable_msi(const struct rte_intr_handle *intr_handle) {
 						intr_handle->fd);
 		return -1;
 	}
+	set_vfio_efd_assigned(intr_handle, 1);
 	return 0;
 }
 
@@ -248,6 +302,7 @@ vfio_disable_msi(const struct rte_intr_handle *intr_handle) {
 		RTE_LOG(ERR, EAL,
 			"Error disabling MSI interrupts for fd %d\n", intr_handle->fd);
 
+	set_vfio_efd_assigned(intr_handle, 0);
 	return ret;
 }
 
@@ -259,6 +314,13 @@ vfio_enable_msix(const struct rte_intr_handle *intr_handle) {
 	struct vfio_irq_set *irq_set;
 	int *fd_ptr;
 
+	/*
+	 * vfio does not support MSI-X masking/unmaking. So if irq has
+	 * been set up previously, nothing to do.
+	 */
+	if (intr_handle->vfio_efd_assigned)
+		return 0;
+
 	len = sizeof(irq_set_buf);
 
 	irq_set = (struct vfio_irq_set *) irq_set_buf;
@@ -284,6 +346,7 @@ vfio_enable_msix(const struct rte_intr_handle *intr_handle) {
 		return -1;
 	}
 
+	set_vfio_efd_assigned(intr_handle, 1);
 	return 0;
 }
 
@@ -309,6 +372,7 @@ vfio_disable_msix(const struct rte_intr_handle *intr_handle) {
 		RTE_LOG(ERR, EAL,
 			"Error disabling MSI-X interrupts for fd %d\n", intr_handle->fd);
 
+	set_vfio_efd_assigned(intr_handle, 0);
 	return ret;
 }
 
-- 
2.22.0

Re: [dpdk-dev] [RFC PATCH] vfio: avoid re-installing irq handler

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Hyong Youb Kim (hyonkim) <hyonkim@cisco.com>
> Sent: Tuesday, July 16, 2019 1:19 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; David Marchand
> <david.marchand@redhat.com>; Thomas Monjalon
> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Alejandro
> Lucero <alejandro.lucero@netronome.com>; Anatoly Burakov
> <anatoly.burakov@intel.com>
> Cc: dev@dpdk.org; John Daley (johndale) <johndale@cisco.com>; Shahed
> Shaikh <shshaikh@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>
> Subject: RE: [RFC PATCH] vfio: avoid re-installing irq handler
> 
> > -----Original Message-----
> > From: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
>  [...]
> > > > > A rough patch for the approach mentioned earlier. It is only for
> > > discussion.
> > > > > http://mails.dpdk.org/archives/dev/2019-July/138113.html
> > > > >
> > > > > To try this out, first revert the following then apply.
> > > > > commit 89aac60e0be9 ("vfio: fix interrupts race condition")
> > > >
> > > > Yes. This patch has to be to reverted. It changes the existing
> > > > interrupt behavior and does not address the MSIX case as well.
> > > >
> > > > I think, The clean fix would be to introduce rte_intr_mask() and
> > > > rte_intr_unmask() by abstracting the INTX and MSIX differences And
> > > > let qede driver call it as needed.
> > > >
> > > > Thoughts?
> > >
> > > Hi,
> >
> > Hi Hyong,
> >
> > >
> > > You are proposing these?
> > > - Add rte_intr_mask_intx, rte_intr_unmask_intx.
> > >   No APIs for masking MSI/MSI-X as vfio-pci does not support that.
> > > - Modify PMD irq handlers to use rte_intr_unmask_intx as necessary.
> >
> > No, introduce the rte_intr_mask() and rte_intr_unmask().
> > For MSIX + Linux VFIO, That API can return -ENOSUP as Linux VFIO+MSIX
> > is not supporting.
> > Another platform/eal may support it.
> >
> 
> These generic names would invite people to use API, only to see it fail, since
> it only works with INTx..

It works for all non VFIO MSIx case. VFIO MSIx case it NOP(Yes, No need to return error in that case)


> 
> > Mask and unmask is operation is known to all IRQ controllers.
> > So, IMO, As far as abstraction is concerned it will be good fit.
> >
> > > That might be too intrusive. And too much work for the sake of INTx..
> > > Anyone really using/needing INTx these days? :-)
> >
> > Yup. Mask needs to called only for only qede INTx. Looks like qede Has
> > MSIX and INTX separate handler. So this mask can go to qede INTx
> >
> > >
> > > The following drivers call rte_intr_enable from their irq handlers.
> > > So with explicit rte_intr_unmask_intx, all these would need to do
> > > "if using intx, unmask"?
> > >
> > > atlantic, avp, axgbe, bnx2x, e1000, fm10k, ice, ixgbe, nfp, qede,
> > > sfc,
> > > vmxnet3
> >
> > No change on these PMDs.
> >
> 
> Why is that?
> 
> These drivers potentially have the same "lost" interrupt issue mentioned in
> the original redhat bz (qede + MSI). I *think* this observation led David to
> address them all through vfio changes, rather than fixing qede alone.
> 
> You want to introduce unmask API and use it only for qede in this cycle, and
> ask respective maintainers to fix their drivers in 19.11?

Changing the rte_intr_enable() to rte_intr_unmask() is  trivial on the places
Where existing drivers enable as unmask.

If we understand it correctly:

In case of non VFIO MSIX(INTx) and UIO
-------------------------------------------------
AK1) Kernel receives interrupt
AK2) Kernel _mask_ the interrupt
AK3) Kernel notify the use space

On usersapce:
AU1) Driver specific interrupt handler invoked
AU2) Handle the driver specific interrupt
AU3) Call rte_intr_enable(), it will intern call  VFIO_IRQ_SET_ACTION_UNMASK using VFIO_DEVICE_SET_IRQS to unmask the interrupt.

In case of  VFIO MSIX(INTx)
------------------------------------
BK1) Kernel receives interrupt.
BK2) Kernel notify the use space

On usersapce:
BU1) Driver specific interrupt handler invoked
BU2) Handle the driver specific interrupt
BU3) Call rte_intr_enable(), it will intern call  VFIO_IRQ_SET_ACTION_TRIGGER using VFIO_DEVICE_SET_IRQS to unmask the interrupt.

VFIO_IRQ_SET_ACTION_TRIGGER: is the nasty one, it will free the existing interrupt handler and request new handler etc.
Which is the source of the actual race conditional problem. 

Ideally BU3 can be just NOP. Since we need to keep the same interrupt handler for both UIO and MSIX(I *think*)
DPDK tends to use rte_intr_enable() which can work for AU3/BU3 as well.

So we need,
A light weight primitive, which unmask the AK2 incase of VFIO INTx by not overriding 
The meaning of normal rte_intr_enable() which suppose not use for MSIX interrupt in action due to racy behavior of VFIO_IRQ_SET_ACTION_TRIGGER

Replacing AU3 and BU3 as rte_intr_unmask() would fix problem. Where rte_intr_unmask() for BU3 is just NOP.

Re: [dpdk-dev] [RFC PATCH] vfio: avoid re-installing irq handler

[Hyong Youb Kim (hyonkim)]

> -----Original Message-----
> From: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
 [...]
> > > > A rough patch for the approach mentioned earlier. It is only for
> > discussion.
> > > > http://mails.dpdk.org/archives/dev/2019-July/138113.html
> > > >
> > > > To try this out, first revert the following then apply.
> > > > commit 89aac60e0be9 ("vfio: fix interrupts race condition")
> > >
> > > Yes. This patch has to be to reverted. It changes the existing
> > > interrupt behavior and does not address the MSIX case as well.
> > >
> > > I think, The clean fix would be to introduce rte_intr_mask() and
> > > rte_intr_unmask() by abstracting the INTX and MSIX differences And let
> > > qede driver call it as needed.
> > >
> > > Thoughts?
> >
> > Hi,
> 
> Hi Hyong,
> 
> >
> > You are proposing these?
> > - Add rte_intr_mask_intx, rte_intr_unmask_intx.
> >   No APIs for masking MSI/MSI-X as vfio-pci does not support that.
> > - Modify PMD irq handlers to use rte_intr_unmask_intx as necessary.
> 
> No, introduce the rte_intr_mask() and rte_intr_unmask().
> For MSIX + Linux VFIO, That API can return -ENOSUP as Linux VFIO+MSIX is
> not supporting.
> Another platform/eal may support it.
> 

These generic names would invite people to use API, only to see it
fail, since it only works with INTx..

> Mask and unmask is operation is known to all IRQ controllers.
> So, IMO, As far as abstraction is concerned it will be good fit.
> 
> > That might be too intrusive. And too much work for the sake of INTx..
> > Anyone really using/needing INTx these days? :-)
> 
> Yup. Mask needs to called only for only qede INTx. Looks like qede
> Has MSIX and INTX separate handler. So this mask can go to qede INTx
> 
> >
> > The following drivers call rte_intr_enable from their irq handlers. So with
> > explicit rte_intr_unmask_intx, all these would need to do "if using intx,
> > unmask"?
> >
> > atlantic, avp, axgbe, bnx2x, e1000, fm10k, ice, ixgbe, nfp, qede, sfc,
> > vmxnet3
> 
> No change on these PMDs.
> 

Why is that?

These drivers potentially have the same "lost" interrupt issue
mentioned in the original redhat bz (qede + MSI). I *think* this
observation led David to address them all through vfio changes, rather
than fixing qede alone.

You want to introduce unmask API and use it only for qede in this
cycle, and ask respective maintainers to fix their drivers in 19.11?

> > And nfp seems to rely on rte_intr_enable to re-install irq handler to
> unmask
> > a vector in MSI-X Table?
> >
> >         if (hw->ctrl & NFP_NET_CFG_CTRL_MSIXAUTO) {
> >                 /* If MSI-X auto-masking is used, clear the entry */
> >                 rte_wmb();
> >                 rte_intr_enable(&pci_dev->intr_handle);
> >
> > With David's patch and mine, this handler would have to first
> > rte_intr_disable() and then enable, if such unmasking is really necessary..
> >
> > As for the semantics of rte_intr_enable/disable, I am ok as is.
> > - "enable": put things in a state where NIC can send an interrupt, and
> >   PMD/app gets a callback.
> >   Whether this involves unmasking for INTx is hidden.
> > - "disable": put things in a state where NIC cannot send an interrupt.
> 
> It looks OK to me. My only thought was, Since mask and unmask
> is a common irq controller operation. We may not need to add
> A lot of common code(Introducing a state) to hide unmask INTx.
> More over as you said, There is may only handful of devices uses INTX.
> 
> IMO, mask and unmask API is good fit as eal abstraction.
> But Using a separate API or hide inside eal to solve this problem is good
> question.
> May be more thoughts from another quys will be good.
> 
> We will try to send a version with mask/unmask API to see the changes
> required.
> 
> >
> > Regardless of vfio changes, we should probably remove rte_intr_enable
> > from qede_interrupt_handler (the MSI/MSI-X interrupt handler), to make
> > usage/intention clear..
> 
> Yes. Anyway this change is required.
> 

That change fixes the immediate problem (redhat bz) that started all
this discussion. And allow us to kick the can down the road, for
potential issues in other PMDs :-) Not suggesting we do, but it
becomes an option..

Thanks.
-Hyong

Re: [dpdk-dev] [RFC PATCH] vfio: avoid re-installing irq handler

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Hyong Youb Kim (hyonkim) <hyonkim@cisco.com>
> Sent: Tuesday, July 16, 2019 11:28 AM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; David Marchand
> <david.marchand@redhat.com>; Thomas Monjalon
> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Alejandro
> Lucero <alejandro.lucero@netronome.com>; Anatoly Burakov
> <anatoly.burakov@intel.com>
> Cc: dev@dpdk.org; John Daley (johndale) <johndale@cisco.com>; Shahed
> Shaikh <shshaikh@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>
> Subject: RE: [RFC PATCH] vfio: avoid re-installing irq handler
> 
> > > A rough patch for the approach mentioned earlier. It is only for
> discussion.
> > > http://mails.dpdk.org/archives/dev/2019-July/138113.html
> > >
> > > To try this out, first revert the following then apply.
> > > commit 89aac60e0be9 ("vfio: fix interrupts race condition")
> >
> > Yes. This patch has to be to reverted. It changes the existing
> > interrupt behavior and does not address the MSIX case as well.
> >
> > I think, The clean fix would be to introduce rte_intr_mask() and
> > rte_intr_unmask() by abstracting the INTX and MSIX differences And let
> > qede driver call it as needed.
> >
> > Thoughts?
> 
> Hi,

Hi Hyong,

> 
> You are proposing these?
> - Add rte_intr_mask_intx, rte_intr_unmask_intx.
>   No APIs for masking MSI/MSI-X as vfio-pci does not support that.
> - Modify PMD irq handlers to use rte_intr_unmask_intx as necessary.

No, introduce the rte_intr_mask() and rte_intr_unmask().
For MSIX + Linux VFIO, That API can return -ENOSUP as Linux VFIO+MSIX is not supporting.
Another platform/eal may support it.

Mask and unmask is operation is known to all IRQ controllers.
So, IMO, As far as abstraction is concerned it will be good fit.

> That might be too intrusive. And too much work for the sake of INTx..
> Anyone really using/needing INTx these days? :-)

Yup. Mask needs to called only for only qede INTx. Looks like qede
Has MSIX and INTX separate handler. So this mask can go to qede INTx

> 
> The following drivers call rte_intr_enable from their irq handlers. So with
> explicit rte_intr_unmask_intx, all these would need to do "if using intx,
> unmask"?
> 
> atlantic, avp, axgbe, bnx2x, e1000, fm10k, ice, ixgbe, nfp, qede, sfc,
> vmxnet3

No change on these PMDs.

> And nfp seems to rely on rte_intr_enable to re-install irq handler to unmask
> a vector in MSI-X Table?
> 
>         if (hw->ctrl & NFP_NET_CFG_CTRL_MSIXAUTO) {
>                 /* If MSI-X auto-masking is used, clear the entry */
>                 rte_wmb();
>                 rte_intr_enable(&pci_dev->intr_handle);
> 
> With David's patch and mine, this handler would have to first
> rte_intr_disable() and then enable, if such unmasking is really necessary..
> 
> As for the semantics of rte_intr_enable/disable, I am ok as is.
> - "enable": put things in a state where NIC can send an interrupt, and
>   PMD/app gets a callback.
>   Whether this involves unmasking for INTx is hidden.
> - "disable": put things in a state where NIC cannot send an interrupt.

It looks OK to me. My only thought was, Since mask and unmask
is a common irq controller operation. We may not need to add
A lot of common code(Introducing a state) to hide unmask INTx.
More over as you said, There is may only handful of devices uses INTX.

IMO, mask and unmask API is good fit as eal abstraction.
But Using a separate API or hide inside eal to solve this problem is good question.
May be more thoughts from another quys will be good.

We will try to send a version with mask/unmask API to see the changes required.

> 
> Regardless of vfio changes, we should probably remove rte_intr_enable
> from qede_interrupt_handler (the MSI/MSI-X interrupt handler), to make
> usage/intention clear..

Yes. Anyway this change is required.

> 
> Thanks.
> -Hyong

Re: [dpdk-dev] [RFC PATCH] vfio: avoid re-installing irq handler

[Hyong Youb Kim (hyonkim)]

> -----Original Message-----
> From: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Sent: Tuesday, July 16, 2019 1:51 AM
> To: Hyong Youb Kim (hyonkim) <hyonkim@cisco.com>; David Marchand
> <david.marchand@redhat.com>; Thomas Monjalon
> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; John Daley (johndale) <johndale@cisco.com>; Shahed
> Shaikh <shshaikh@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>
> Subject: RE: [RFC PATCH] vfio: avoid re-installing irq handler
> 
> > -----Original Message-----
> > From: Hyong Youb Kim <hyonkim@cisco.com>
> > Sent: Monday, July 15, 2019 9:28 PM
> > To: David Marchand <david.marchand@redhat.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> > Ferruh Yigit <ferruh.yigit@intel.com>
> > Cc: dev@dpdk.org; John Daley <johndale@cisco.com>; Hyong Youb Kim
> > <hyonkim@cisco.com>
> > Subject: [EXT] [RFC PATCH] vfio: avoid re-installing irq handler
> >
> > A rough patch for the approach mentioned earlier. It is only for discussion.
> > http://mails.dpdk.org/archives/dev/2019-July/138113.html
> >
> > To try this out, first revert the following then apply.
> > commit 89aac60e0be9 ("vfio: fix interrupts race condition")
> 
> Yes. This patch has to be to reverted. It changes the existing interrupt
> behavior and does not address the MSIX case as well.
> 
> I think, The clean fix would be to introduce rte_intr_mask() and
> rte_intr_unmask() by abstracting the INTX and MSIX differences
> And let qede driver call it as needed.
> 
> Thoughts?

Hi,

You are proposing these?
- Add rte_intr_mask_intx, rte_intr_unmask_intx.
  No APIs for masking MSI/MSI-X as vfio-pci does not support that.
- Modify PMD irq handlers to use rte_intr_unmask_intx as necessary.

That might be too intrusive. And too much work for the sake of
INTx.. Anyone really using/needing INTx these days? :-)

The following drivers call rte_intr_enable from their irq handlers. So
with explicit rte_intr_unmask_intx, all these would need to do "if
using intx, unmask"?

atlantic, avp, axgbe, bnx2x, e1000, fm10k, ice, ixgbe, nfp, qede, sfc,
vmxnet3

And nfp seems to rely on rte_intr_enable to re-install irq handler to
unmask a vector in MSI-X Table?

        if (hw->ctrl & NFP_NET_CFG_CTRL_MSIXAUTO) {
                /* If MSI-X auto-masking is used, clear the entry */
                rte_wmb();
                rte_intr_enable(&pci_dev->intr_handle);

With David's patch and mine, this handler would have to first
rte_intr_disable() and then enable, if such unmasking is really
necessary..

As for the semantics of rte_intr_enable/disable, I am ok as is.
- "enable": put things in a state where NIC can send an interrupt, and
  PMD/app gets a callback.
  Whether this involves unmasking for INTx is hidden.
- "disable": put things in a state where NIC cannot send an interrupt.

Regardless of vfio changes, we should probably remove rte_intr_enable
from qede_interrupt_handler (the MSI/MSI-X interrupt handler), to make
usage/intention clear..

Thanks.
-Hyong

Re: [dpdk-dev] [RFC PATCH] vfio: avoid re-installing irq handler

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Hyong Youb Kim <hyonkim@cisco.com>
> Sent: Monday, July 15, 2019 9:28 PM
> To: David Marchand <david.marchand@redhat.com>; Thomas Monjalon
> <thomas@monjalon.net>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; John Daley <johndale@cisco.com>; Hyong Youb Kim
> <hyonkim@cisco.com>
> Subject: [EXT] [RFC PATCH] vfio: avoid re-installing irq handler
> 
> A rough patch for the approach mentioned earlier. It is only for discussion.
> http://mails.dpdk.org/archives/dev/2019-July/138113.html
> 
> To try this out, first revert the following then apply.
> commit 89aac60e0be9 ("vfio: fix interrupts race condition")

Yes. This patch has to be to reverted. It changes the existing interrupt behavior and does not address the MSIX case as well.

I think, The clean fix would be to introduce rte_intr_mask() and rte_intr_unmask() by abstracting the INTX and MSIX differences
And let qede driver call it as needed.

Thoughts?