[PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Feng Wu]

This patch adds the registration/unregistration of an
irq_bypass_producer for MSI/MSIx on vfio pci devices.

Signed-off-by: Feng Wu <feng.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
v8:
- Merge "[PATCH v7 08/17] vfio: Select IRQ_BYPASS_MANAGER for vfio PCI devices"
  into this patch.

v6:
- Make the add_consumer and del_consumer callbacks static
- Remove pointless INIT_LIST_HEAD to 'vdev->ctx[vector].producer.node)'
- Use dev_info instead of WARN_ON() when irq_bypass_register_producer fails
- Remove optional dummy callbacks for irq producer

 drivers/vfio/pci/Kconfig            | 1 +
 drivers/vfio/pci/vfio_pci_intrs.c   | 9 +++++++++
 drivers/vfio/pci/vfio_pci_private.h | 2 ++
 3 files changed, 12 insertions(+)

diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
index 579d83b..02912f1 100644
--- a/drivers/vfio/pci/Kconfig
+++ b/drivers/vfio/pci/Kconfig
@@ -2,6 +2,7 @@ config VFIO_PCI
 	tristate "VFIO support for PCI devices"
 	depends on VFIO && PCI && EVENTFD
 	select VFIO_VIRQFD
+	select IRQ_BYPASS_MANAGER
 	help
 	  Support for the PCI VFIO bus driver.  This is required to make
 	  use of PCI drivers using the VFIO framework.
diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
index 1f577b4..c65299d 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -319,6 +319,7 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
 
 	if (vdev->ctx[vector].trigger) {
 		free_irq(irq, vdev->ctx[vector].trigger);
+		irq_bypass_unregister_producer(&vdev->ctx[vector].producer);
 		kfree(vdev->ctx[vector].name);
 		eventfd_ctx_put(vdev->ctx[vector].trigger);
 		vdev->ctx[vector].trigger = NULL;
@@ -360,6 +361,14 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
 		return ret;
 	}
 
+	vdev->ctx[vector].producer.token = trigger;
+	vdev->ctx[vector].producer.irq = irq;
+	ret = irq_bypass_register_producer(&vdev->ctx[vector].producer);
+	if (unlikely(ret))
+		dev_info(&pdev->dev,
+		"irq bypass producer (token %p) registeration fails: %d\n",
+		vdev->ctx[vector].producer.token, ret);
+
 	vdev->ctx[vector].trigger = trigger;
 
 	return 0;
diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
index ae0e1b4..0e7394f 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -13,6 +13,7 @@
 
 #include <linux/mutex.h>
 #include <linux/pci.h>
+#include <linux/irqbypass.h>
 
 #ifndef VFIO_PCI_PRIVATE_H
 #define VFIO_PCI_PRIVATE_H
@@ -29,6 +30,7 @@ struct vfio_pci_irq_ctx {
 	struct virqfd		*mask;
 	char			*name;
 	bool			masked;
+	struct irq_bypass_producer	producer;
 };
 
 struct vfio_pci_device {
-- 
2.1.0

Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Alex Williamson]

On Fri, 2015-09-18 at 22:29 +0800, Feng Wu wrote:
> This patch adds the registration/unregistration of an
> irq_bypass_producer for MSI/MSIx on vfio pci devices.
> 
> Signed-off-by: Feng Wu <feng.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

On nit, Paolo could you please fix the spelling of "registration" in the
dev_info, otherwise:

Acked-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>


> ---
> v8:
> - Merge "[PATCH v7 08/17] vfio: Select IRQ_BYPASS_MANAGER for vfio PCI devices"
>   into this patch.
> 
> v6:
> - Make the add_consumer and del_consumer callbacks static
> - Remove pointless INIT_LIST_HEAD to 'vdev->ctx[vector].producer.node)'
> - Use dev_info instead of WARN_ON() when irq_bypass_register_producer fails
> - Remove optional dummy callbacks for irq producer
> 
>  drivers/vfio/pci/Kconfig            | 1 +
>  drivers/vfio/pci/vfio_pci_intrs.c   | 9 +++++++++
>  drivers/vfio/pci/vfio_pci_private.h | 2 ++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> index 579d83b..02912f1 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
> @@ -2,6 +2,7 @@ config VFIO_PCI
>  	tristate "VFIO support for PCI devices"
>  	depends on VFIO && PCI && EVENTFD
>  	select VFIO_VIRQFD
> +	select IRQ_BYPASS_MANAGER
>  	help
>  	  Support for the PCI VFIO bus driver.  This is required to make
>  	  use of PCI drivers using the VFIO framework.
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
> index 1f577b4..c65299d 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -319,6 +319,7 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
>  
>  	if (vdev->ctx[vector].trigger) {
>  		free_irq(irq, vdev->ctx[vector].trigger);
> +		irq_bypass_unregister_producer(&vdev->ctx[vector].producer);
>  		kfree(vdev->ctx[vector].name);
>  		eventfd_ctx_put(vdev->ctx[vector].trigger);
>  		vdev->ctx[vector].trigger = NULL;
> @@ -360,6 +361,14 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
>  		return ret;
>  	}
>  
> +	vdev->ctx[vector].producer.token = trigger;
> +	vdev->ctx[vector].producer.irq = irq;
> +	ret = irq_bypass_register_producer(&vdev->ctx[vector].producer);
> +	if (unlikely(ret))
> +		dev_info(&pdev->dev,
> +		"irq bypass producer (token %p) registeration fails: %d\n",
> +		vdev->ctx[vector].producer.token, ret);
> +
>  	vdev->ctx[vector].trigger = trigger;
>  
>  	return 0;
> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
> index ae0e1b4..0e7394f 100644
> --- a/drivers/vfio/pci/vfio_pci_private.h
> +++ b/drivers/vfio/pci/vfio_pci_private.h
> @@ -13,6 +13,7 @@
>  
>  #include <linux/mutex.h>
>  #include <linux/pci.h>
> +#include <linux/irqbypass.h>
>  
>  #ifndef VFIO_PCI_PRIVATE_H
>  #define VFIO_PCI_PRIVATE_H
> @@ -29,6 +30,7 @@ struct vfio_pci_irq_ctx {
>  	struct virqfd		*mask;
>  	char			*name;
>  	bool			masked;
> +	struct irq_bypass_producer	producer;
>  };
>  
>  struct vfio_pci_device {

RE: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Wu, Feng]

Hi Paolo & Alex,

I find that there is a build error in the following two cases:
- KVM is configured as 'M' and VFIO as 'Y'
The reason is the build of irqbypass manager is triggered in
arch/x86/kvm/Makefile, and VFIO is built before KVM, hence
it cannot find the symbols in irqbypass manager.

- Disable KVM and enable VFIO in .config
The reason is similar with the above one, the irqbypass manager
is not built since KVM is not configured.

I think the point is that we cannot trigger the build of irqbypass
manager inside KVM or VFIO, we need trigger the build at a high
level and it should be built before VFIO and KVM. Any ideas?

Thanks,
Feng

> -----Original Message-----
> From: Wu, Feng
> Sent: Friday, September 18, 2015 10:30 PM
> To: pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org;
> mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> Cc: eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Wu, Feng
> Subject: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer
> 
> This patch adds the registration/unregistration of an
> irq_bypass_producer for MSI/MSIx on vfio pci devices.
> 
> Signed-off-by: Feng Wu <feng.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> v8:
> - Merge "[PATCH v7 08/17] vfio: Select IRQ_BYPASS_MANAGER for vfio PCI
> devices"
>   into this patch.
> 
> v6:
> - Make the add_consumer and del_consumer callbacks static
> - Remove pointless INIT_LIST_HEAD to 'vdev->ctx[vector].producer.node)'
> - Use dev_info instead of WARN_ON() when irq_bypass_register_producer fails
> - Remove optional dummy callbacks for irq producer
> 
>  drivers/vfio/pci/Kconfig            | 1 +
>  drivers/vfio/pci/vfio_pci_intrs.c   | 9 +++++++++
>  drivers/vfio/pci/vfio_pci_private.h | 2 ++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> index 579d83b..02912f1 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
> @@ -2,6 +2,7 @@ config VFIO_PCI
>  	tristate "VFIO support for PCI devices"
>  	depends on VFIO && PCI && EVENTFD
>  	select VFIO_VIRQFD
> +	select IRQ_BYPASS_MANAGER
>  	help
>  	  Support for the PCI VFIO bus driver.  This is required to make
>  	  use of PCI drivers using the VFIO framework.
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
> index 1f577b4..c65299d 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -319,6 +319,7 @@ static int vfio_msi_set_vector_signal(struct
> vfio_pci_device *vdev,
> 
>  	if (vdev->ctx[vector].trigger) {
>  		free_irq(irq, vdev->ctx[vector].trigger);
> +		irq_bypass_unregister_producer(&vdev->ctx[vector].producer);
>  		kfree(vdev->ctx[vector].name);
>  		eventfd_ctx_put(vdev->ctx[vector].trigger);
>  		vdev->ctx[vector].trigger = NULL;
> @@ -360,6 +361,14 @@ static int vfio_msi_set_vector_signal(struct
> vfio_pci_device *vdev,
>  		return ret;
>  	}
> 
> +	vdev->ctx[vector].producer.token = trigger;
> +	vdev->ctx[vector].producer.irq = irq;
> +	ret = irq_bypass_register_producer(&vdev->ctx[vector].producer);
> +	if (unlikely(ret))
> +		dev_info(&pdev->dev,
> +		"irq bypass producer (token %p) registeration fails: %d\n",
> +		vdev->ctx[vector].producer.token, ret);
> +
>  	vdev->ctx[vector].trigger = trigger;
> 
>  	return 0;
> diff --git a/drivers/vfio/pci/vfio_pci_private.h
> b/drivers/vfio/pci/vfio_pci_private.h
> index ae0e1b4..0e7394f 100644
> --- a/drivers/vfio/pci/vfio_pci_private.h
> +++ b/drivers/vfio/pci/vfio_pci_private.h
> @@ -13,6 +13,7 @@
> 
>  #include <linux/mutex.h>
>  #include <linux/pci.h>
> +#include <linux/irqbypass.h>
> 
>  #ifndef VFIO_PCI_PRIVATE_H
>  #define VFIO_PCI_PRIVATE_H
> @@ -29,6 +30,7 @@ struct vfio_pci_irq_ctx {
>  	struct virqfd		*mask;
>  	char			*name;
>  	bool			masked;
> +	struct irq_bypass_producer	producer;
>  };
> 
>  struct vfio_pci_device {
> --
> 2.1.0

Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Paolo Bonzini]

On 21/09/2015 10:56, Wu, Feng wrote:
> Hi Paolo & Alex,
> 
> I find that there is a build error in the following two cases:
> - KVM is configured as 'M' and VFIO as 'Y'
> The reason is the build of irqbypass manager is triggered in
> arch/x86/kvm/Makefile, and VFIO is built before KVM, hence
> it cannot find the symbols in irqbypass manager.
> 
> - Disable KVM and enable VFIO in .config
> The reason is similar with the above one, the irqbypass manager
> is not built since KVM is not configured.
> 
> I think the point is that we cannot trigger the build of irqbypass
> manager inside KVM or VFIO, we need trigger the build at a high
> level and it should be built before VFIO and KVM. Any ideas?

We can add virt/Makefile and build virt/lib/ directly, not through
arch/x86/kvm.

Paolo

RE: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Wu, Feng]

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
> Sent: Monday, September 21, 2015 5:32 PM
> To: Wu, Feng; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org;
> mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> Cc: eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer
> 
> 
> 
> On 21/09/2015 10:56, Wu, Feng wrote:
> > Hi Paolo & Alex,
> >
> > I find that there is a build error in the following two cases:
> > - KVM is configured as 'M' and VFIO as 'Y'
> > The reason is the build of irqbypass manager is triggered in
> > arch/x86/kvm/Makefile, and VFIO is built before KVM, hence
> > it cannot find the symbols in irqbypass manager.
> >
> > - Disable KVM and enable VFIO in .config
> > The reason is similar with the above one, the irqbypass manager
> > is not built since KVM is not configured.
> >
> > I think the point is that we cannot trigger the build of irqbypass
> > manager inside KVM or VFIO, we need trigger the build at a high
> > level and it should be built before VFIO and KVM. Any ideas?
> 
> We can add virt/Makefile and build virt/lib/ directly, not through
> arch/x86/kvm.

Yes, that can solve the build error. Should I send a new version?

Thanks,
Feng

> 
> Paolo

Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Paolo Bonzini]

On 21/09/2015 13:35, Wu, Feng wrote:
>>> > > I think the point is that we cannot trigger the build of irqbypass
>>> > > manager inside KVM or VFIO, we need trigger the build at a high
>>> > > level and it should be built before VFIO and KVM. Any ideas?
>> > 
>> > We can add virt/Makefile and build virt/lib/ directly, not through
>> > arch/x86/kvm.
> Yes, that can solve the build error. Should I send a new version?

You can send a separate patch on top of this v9.

Paolo

RE: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Wu, Feng]

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
> Sent: Monday, September 21, 2015 8:07 PM
> To: Wu, Feng; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org;
> mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> Cc: eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer
> 
> 
> 
> On 21/09/2015 13:35, Wu, Feng wrote:
> >>> > > I think the point is that we cannot trigger the build of irqbypass
> >>> > > manager inside KVM or VFIO, we need trigger the build at a high
> >>> > > level and it should be built before VFIO and KVM. Any ideas?
> >> >
> >> > We can add virt/Makefile and build virt/lib/ directly, not through
> >> > arch/x86/kvm.
> > Yes, that can solve the build error. Should I send a new version?
> 
> You can send a separate patch on top of this v9.

Sure, will do this soon!

Thanks,
Feng

> 
> Paolo

RE: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Wu, Feng]

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
> Sent: Monday, September 21, 2015 5:32 PM
> To: Wu, Feng; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org;
> mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> Cc: eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer
> 
> 
> 
> On 21/09/2015 10:56, Wu, Feng wrote:
> > Hi Paolo & Alex,
> >
> > I find that there is a build error in the following two cases:
> > - KVM is configured as 'M' and VFIO as 'Y'
> > The reason is the build of irqbypass manager is triggered in
> > arch/x86/kvm/Makefile, and VFIO is built before KVM, hence
> > it cannot find the symbols in irqbypass manager.
> >
> > - Disable KVM and enable VFIO in .config
> > The reason is similar with the above one, the irqbypass manager
> > is not built since KVM is not configured.
> >
> > I think the point is that we cannot trigger the build of irqbypass
> > manager inside KVM or VFIO, we need trigger the build at a high
> > level and it should be built before VFIO and KVM. Any ideas?
> 
> We can add virt/Makefile and build virt/lib/ directly, not through
> arch/x86/kvm.

Thinking about this more, does that mean we need to add the virt directory
in the top Makefile in Linux tree?

Thanks,
Feng

> 
> Paolo

Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Paolo Bonzini]

On 21/09/2015 14:53, Wu, Feng wrote:
>>> > > I think the point is that we cannot trigger the build of irqbypass
>>> > > manager inside KVM or VFIO, we need trigger the build at a high
>>> > > level and it should be built before VFIO and KVM. Any ideas?
>> > 
>> > We can add virt/Makefile and build virt/lib/ directly, not through
>> > arch/x86/kvm.
> Thinking about this more, does that mean we need to add the virt directory
> in the top Makefile in Linux tree?

Yes, it does.

Paolo

Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Eric Auger]

Hi,
On 09/21/2015 03:02 PM, Paolo Bonzini wrote:
> 
> 
> On 21/09/2015 14:53, Wu, Feng wrote:
>>>>>> I think the point is that we cannot trigger the build of irqbypass
>>>>>> manager inside KVM or VFIO, we need trigger the build at a high
>>>>>> level and it should be built before VFIO and KVM. Any ideas?
>>>>
>>>> We can add virt/Makefile and build virt/lib/ directly, not through
>>>> arch/x86/kvm.
>> Thinking about this more, does that mean we need to add the virt directory
>> in the top Makefile in Linux tree?
> 
> Yes, it does.
So I understand this will replace patches 2 & 3 then and will fix the
arm64 issue then.

Thanks

Eric

> 
> Paolo
> 

RE: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Wu, Feng]

> -----Original Message-----
> From: linux-kernel-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> [mailto:linux-kernel-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Eric Auger
> Sent: Tuesday, September 22, 2015 3:46 AM
> To: Paolo Bonzini; Wu, Feng; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org;
> mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> Cc: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org;
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer
> 
> Hi,
> On 09/21/2015 03:02 PM, Paolo Bonzini wrote:
> >
> >
> > On 21/09/2015 14:53, Wu, Feng wrote:
> >>>>>> I think the point is that we cannot trigger the build of irqbypass
> >>>>>> manager inside KVM or VFIO, we need trigger the build at a high
> >>>>>> level and it should be built before VFIO and KVM. Any ideas?
> >>>>
> >>>> We can add virt/Makefile and build virt/lib/ directly, not through
> >>>> arch/x86/kvm.
> >> Thinking about this more, does that mean we need to add the virt directory
> >> in the top Makefile in Linux tree?
> >
> > Yes, it does.
> So I understand this will replace patches 2 & 3 then and will fix the
> arm64 issue then.

I just sent a patch to fix this build error. BTW, from the reply of Paolo, seems
he dropped patch 3 in this series, maybe he think it doesn't have much
relationship with other patches, so maybe you could include it in your series
when forwarded irq work gets ready.

Thanks,
Feng

> 
> Thanks
> 
> Eric
> 
> >
> > Paolo
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Eric Auger]

Hi Feng,
On 09/22/2015 11:01 AM, Wu, Feng wrote:
> 
> 
>> -----Original Message-----
>> From: linux-kernel-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> [mailto:linux-kernel-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Eric Auger
>> Sent: Tuesday, September 22, 2015 3:46 AM
>> To: Paolo Bonzini; Wu, Feng; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org;
>> mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
>> Cc: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org;
>> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Subject: Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer
>>
>> Hi,
>> On 09/21/2015 03:02 PM, Paolo Bonzini wrote:
>>>
>>>
>>> On 21/09/2015 14:53, Wu, Feng wrote:
>>>>>>>> I think the point is that we cannot trigger the build of irqbypass
>>>>>>>> manager inside KVM or VFIO, we need trigger the build at a high
>>>>>>>> level and it should be built before VFIO and KVM. Any ideas?
>>>>>>
>>>>>> We can add virt/Makefile and build virt/lib/ directly, not through
>>>>>> arch/x86/kvm.
>>>> Thinking about this more, does that mean we need to add the virt directory
>>>> in the top Makefile in Linux tree?
>>>
>>> Yes, it does.
>> So I understand this will replace patches 2 & 3 then and will fix the
>> arm64 issue then.
> 
> I just sent a patch to fix this build error. BTW, from the reply of Paolo, seems
> he dropped patch 3 in this series, maybe he think it doesn't have much
> relationship with other patches, so maybe you could include it in your series
> when forwarded irq work gets ready.
OK no problem. I moved that patch (modified according to the new
compilation scheme) in the irq forwarding series.

Thanks

Eric
> 
> Thanks,
> Feng
> 
>>
>> Thanks
>>
>> Eric
>>
>>>
>>> Paolo
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Alex Williamson]

On Fri, 18 Sep 2015 22:29:50 +0800
Feng Wu <feng.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

 @@ -360,6 +361,14 @@ static int vfio_msi_set_vector_signal(struct
 vfio_pci_device *vdev,
>  		return ret;
>  	}
>  
> +	vdev->ctx[vector].producer.token = trigger;
> +	vdev->ctx[vector].producer.irq = irq;
> +	ret = irq_bypass_register_producer(&vdev->ctx[vector].producer);
> +	if (unlikely(ret))
> +		dev_info(&pdev->dev,
> +		"irq bypass producer (token %p) registeration fails: %d\n",
> +		vdev->ctx[vector].producer.token, ret);
> +
>  	vdev->ctx[vector].trigger = trigger;
>  
>  	return 0;

Digging back into the IRQ producer/consumer thing, I'm not sure how we
should be handling a failure here, but it turns out that what we have
is pretty sub-optimal.  Any sort of testing on AMD hits this dev_info
because kvm_arch_irq_bypass_add_producer() returns -EINVAL without
kvm_x86_ops->update_pi_irte which is only implemented for vmx.  Clearly
we don't want to spew confusing error messages for a feature that does
not exist.

The easiest option is to simply make this error silent, but should
registering a producer/consumer really fail due to a mismatch on the
other end or should the __connect sequence fail silently, which both
ends would know about (if they care) due to the add/del handshake
between them?  Perhaps for now we simply need a stable suitable fix to
silence the dev_info above, but longer term, registration shouldn't
fail for mismatches like this.  Thoughts?  Thanks,

Alex

RE: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Wu, Feng]

> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
> Sent: Wednesday, April 27, 2016 4:08 AM
> To: Wu, Feng <feng.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org; mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs/ROKNJybVBZg@public.gmane.org
> foundation.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer
> 
> On Fri, 18 Sep 2015 22:29:50 +0800
> Feng Wu <feng.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> 
>  @@ -360,6 +361,14 @@ static int vfio_msi_set_vector_signal(struct
>  vfio_pci_device *vdev,
> >  		return ret;
> >  	}
> >
> > +	vdev->ctx[vector].producer.token = trigger;
> > +	vdev->ctx[vector].producer.irq = irq;
> > +	ret = irq_bypass_register_producer(&vdev->ctx[vector].producer);
> > +	if (unlikely(ret))
> > +		dev_info(&pdev->dev,
> > +		"irq bypass producer (token %p) registeration fails: %d\n",
> > +		vdev->ctx[vector].producer.token, ret);
> > +
> >  	vdev->ctx[vector].trigger = trigger;
> >
> >  	return 0;
> 
> Digging back into the IRQ producer/consumer thing, I'm not sure how we
> should be handling a failure here, but it turns out that what we have
> is pretty sub-optimal.  Any sort of testing on AMD hits this dev_info
> because kvm_arch_irq_bypass_add_producer() returns -EINVAL without
> kvm_x86_ops->update_pi_irte which is only implemented for vmx.  Clearly
> we don't want to spew confusing error messages for a feature that does
> not exist.
> 
> The easiest option is to simply make this error silent, but should
> registering a producer/consumer really fail due to a mismatch on the
> other end or should the __connect sequence fail silently, which both
> ends would know about (if they care) due to the add/del handshake
> between them?  Perhaps for now we simply need a stable suitable fix to
> silence the dev_info above, but longer term, registration shouldn't
> fail for mismatches like this.  Thoughts?  Thanks,

Can we just return 0 when kvm_x86_ops->update_pi_irte is NULL in
kvm_arch_irq_bypass_add_producer?

Thanks,
Feng

> 
> Alex

Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Eric Auger]

Hi Alex,
On 04/26/2016 10:08 PM, Alex Williamson wrote:
> On Fri, 18 Sep 2015 22:29:50 +0800
> Feng Wu <feng.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> 
>  @@ -360,6 +361,14 @@ static int vfio_msi_set_vector_signal(struct
>  vfio_pci_device *vdev,
>>  		return ret;
>>  	}
>>  
>> +	vdev->ctx[vector].producer.token = trigger;
>> +	vdev->ctx[vector].producer.irq = irq;
>> +	ret = irq_bypass_register_producer(&vdev->ctx[vector].producer);
>> +	if (unlikely(ret))
>> +		dev_info(&pdev->dev,
>> +		"irq bypass producer (token %p) registeration fails: %d\n",
>> +		vdev->ctx[vector].producer.token, ret);
>> +
>>  	vdev->ctx[vector].trigger = trigger;
>>  
>>  	return 0;
> 
> Digging back into the IRQ producer/consumer thing, I'm not sure how we
> should be handling a failure here, but it turns out that what we have
> is pretty sub-optimal.  Any sort of testing on AMD hits this dev_info
> because kvm_arch_irq_bypass_add_producer() returns -EINVAL without
> kvm_x86_ops->update_pi_irte which is only implemented for vmx.  Clearly
> we don't want to spew confusing error messages for a feature that does
> not exist.
> 
> The easiest option is to simply make this error silent, but should
> registering a producer/consumer really fail due to a mismatch on the
> other end or should the __connect sequence fail silently, which both
> ends would know about (if they care) due to the add/del handshake
> between them?  Perhaps for now we simply need a stable suitable fix to
> silence the dev_info above, but longer term, registration shouldn't
> fail for mismatches like this.  Thoughts?  Thanks,

Regarding the ARM IRQ forwarding use case, I think it is OK to fail
silently. We would fall back to the irqfd standard mechanism. Anyway
this series still is waiting for ARM new-vgic dependency to be resolved,
as discussed with Christoffer and Marc.

Best Regards

Eric
> 
> Alex
> 

Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer

[Alex Williamson]

On Wed, 27 Apr 2016 01:32:32 +0000
"Wu, Feng" <feng.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
> > Sent: Wednesday, April 27, 2016 4:08 AM
> > To: Wu, Feng <feng.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Cc: pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org; mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> > eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs/ROKNJybVBZg@public.gmane.org
> > foundation.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Subject: Re: [PATCH v9 12/18] vfio: Register/unregister irq_bypass_producer
> > 
> > On Fri, 18 Sep 2015 22:29:50 +0800
> > Feng Wu <feng.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> > 
> >  @@ -360,6 +361,14 @@ static int vfio_msi_set_vector_signal(struct
> >  vfio_pci_device *vdev,  
> > >  		return ret;
> > >  	}
> > >
> > > +	vdev->ctx[vector].producer.token = trigger;
> > > +	vdev->ctx[vector].producer.irq = irq;
> > > +	ret = irq_bypass_register_producer(&vdev->ctx[vector].producer);
> > > +	if (unlikely(ret))
> > > +		dev_info(&pdev->dev,
> > > +		"irq bypass producer (token %p) registeration fails: %d\n",
> > > +		vdev->ctx[vector].producer.token, ret);
> > > +
> > >  	vdev->ctx[vector].trigger = trigger;
> > >
> > >  	return 0;  
> > 
> > Digging back into the IRQ producer/consumer thing, I'm not sure how we
> > should be handling a failure here, but it turns out that what we have
> > is pretty sub-optimal.  Any sort of testing on AMD hits this dev_info
> > because kvm_arch_irq_bypass_add_producer() returns -EINVAL without
> > kvm_x86_ops->update_pi_irte which is only implemented for vmx.  Clearly
> > we don't want to spew confusing error messages for a feature that does
> > not exist.
> > 
> > The easiest option is to simply make this error silent, but should
> > registering a producer/consumer really fail due to a mismatch on the
> > other end or should the __connect sequence fail silently, which both
> > ends would know about (if they care) due to the add/del handshake
> > between them?  Perhaps for now we simply need a stable suitable fix to
> > silence the dev_info above, but longer term, registration shouldn't
> > fail for mismatches like this.  Thoughts?  Thanks,  
> 
> Can we just return 0 when kvm_x86_ops->update_pi_irte is NULL in
> kvm_arch_irq_bypass_add_producer?

Yeah, that may be the best way to go, only return error for actual
failures, not for simple lack of a bypass mechanism.  This is
consistent with what update_pi_irte does when running on hardware
or configurations without PI.  Thanks,

Alex