[GIT PATCH 0/14] xen: events: cleanups + ween off nr_irqs

[GIT PATCH 0/14] xen: events: cleanups + ween off nr_irqs

[Ian Campbell]

The following series makes a few cleanups to the Xen IRQ infrastructure.
The most important thing is that it removes the need to know about
nr_irqs and in particular the reliance on nr_irqs being static.

Apart from being generally a good thing this is needed because in 2.6.39
nr_irqs will be able to grow dynamically, specifically e7bcecb7b1d2
"genirq: Make nr_irqs runtime expandable" from tip/core/irq is targeted
at 2.6.39.

Dynamically growing nr_irqs also allows us to remove the workaround
which eats into GSI space if a dynamic IRQ cannot be allocated.

There is no ideal sequencing of this series vs e7bcecb7b1d2 (most should
have gone in before, but the penultimate patch really needed to be
simultaneous) so I haven't bothered to try and pull anything from tip
into this branch -- it should all be resolved during the merge window
and bisection won't be too broken since the "eat into GSI space"
workaround only appears to be needed on a small number of older
platforms (qemu being the main exception).

I have tested:
      * Domain 0 on real h/w and under qemu
      * PV guest, including migration and passthrough of both VF and PF.
      * PVHVM guest, including migration and passthrough of both VF and
        PF.

The git pull is a branch on top of konrad/stable/irq.cleanup. However
there is an interaction with konrad/devel/xen-pciback-0.4.driver (the
addition of the domid parameter) so for convenience I have also produced
an irq-pciback branch at the same location which has
konrad/devel/xen-pciback-0.4.driver merged into this branch.

Note that this series obsoletes an older patcho f mine "xen: events:
mark cpu_evtchn_mask_p as __refdata" by virtue of removing the code in
question...

The following changes since commit c5ae07bb307b658c8458f29ca77d237aec0f9327:
  Ian Campbell (1):
        xen: events: remove dom0 specific xen_create_msi_irq

are available in the git repository at:

  git://xenbits.xen.org/people/ianc/linux-2.6.git irq

Ian Campbell (14):
      xen: events: separate two unrelated halves of if condition
      xen: events: fix xen_map_pirq_gsi error return
      xen: events: simplify comment
      xen: events: remove unused public functions
      xen: events: rename restore_cpu_pirqs -> restore_pirqs
      xen: events: refactor GSI pirq bindings functions
      xen: events: use per-cpu variable for cpu_evtchn_mask
      xen: events: turn irq_info constructors into initialiser functions
      xen: events: push setup of irq<->{evtchn,ipi,virq,pirq} maps into irq_info init functions
      xen: events: maintain a list of Xen interrupts
      xen: events: dynamically allocate irq info structures
      xen: events: remove use of nr_irqs as upper bound on number of pirqs
      xen: events: do not workaround too-small nr_irqs
      xen: events: propagate irq allocation failure instead of panicking

 arch/x86/pci/xen.c   |   40 ++++--
 drivers/xen/events.c |  357 ++++++++++++++++++++++++++------------------------
 include/xen/events.h |   24 ++--
 3 files changed, 225 insertions(+), 196 deletions(-)

[PATCH 01/14] xen: events: separate two unrelated halves of if condition

[Ian Campbell]

Clarifies which bit the comment applies to.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 6befe62..684b095 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -1143,10 +1143,14 @@ static int rebind_irq_to_cpu(unsigned irq, unsigned tcpu)
 	struct evtchn_bind_vcpu bind_vcpu;
 	int evtchn = evtchn_from_irq(irq);
 
-	/* events delivered via platform PCI interrupts are always
-	 * routed to vcpu 0 */
-	if (!VALID_EVTCHN(evtchn) ||
-		(xen_hvm_domain() && !xen_have_vector_callback))
+	if (!VALID_EVTCHN(evtchn))
+		return -1;
+
+	/*
+	 * Events delivered via platform PCI interrupts are always
+	 * routed to vcpu 0 and hence cannot be rebound.
+	 */
+	if (xen_hvm_domain() && !xen_have_vector_callback)
 		return -1;
 
 	/* Send future instances of this interrupt to other vcpu. */
-- 
1.5.6.5

[PATCH 02/14] xen: events: fix xen_map_pirq_gsi error return

[Ian Campbell]

Fix initial value of irq so that first goto out (if pirq or gsi
arguments are too large) actually returns an error.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 684b095..2ce95a6 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -596,7 +596,7 @@ int xen_allocate_pirq(unsigned gsi, int shareable, char *name)
  */
 int xen_map_pirq_gsi(unsigned pirq, unsigned gsi, int shareable, char *name)
 {
-	int irq = 0;
+	int irq = -1;
 	struct physdev_irq irq_op;
 
 	spin_lock(&irq_mapping_update_lock);
-- 
1.5.6.5

[PATCH 03/14] xen: events: simplify comment

[Ian Campbell]

It is never valid assume any particular relationship between a Xen
PIRQ number and and Linux IRQ number so there is no need to hedge when
saying so.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |   10 +++-------
 include/xen/events.h |    4 +---
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 2ce95a6..a9c154d 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -583,13 +583,9 @@ int xen_allocate_pirq(unsigned gsi, int shareable, char *name)
 	return xen_map_pirq_gsi(gsi, gsi, shareable, name);
 }
 
-/* xen_map_pirq_gsi might allocate irqs from the top down, as a
- * consequence don't assume that the irq number returned has a low value
- * or can be used as a pirq number unless you know otherwise.
- *
- * One notable exception is when xen_map_pirq_gsi is called passing an
- * hardware gsi as argument, in that case the irq number returned
- * matches the gsi number passed as second argument.
+/*
+ * Do not make any assumptions regarding the relationship between the
+ * IRQ number returned here and the Xen pirq argument.
  *
  * Note: We don't assign an event channel until the irq actually started
  * up.  Return an existing irq if we've already got one for the gsi.
diff --git a/include/xen/events.h b/include/xen/events.h
index 962da2c..f6fed94 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -68,9 +68,7 @@ int xen_set_callback_via(uint64_t via);
 void xen_evtchn_do_upcall(struct pt_regs *regs);
 void xen_hvm_evtchn_do_upcall(void);
 
-/* Allocate an irq for a physical interrupt, given a gsi.  "Legacy"
- * GSIs are identity mapped; others are dynamically allocated as
- * usual. */
+/* Allocate an irq for a physical interrupt, given a gsi. */
 int xen_allocate_pirq(unsigned gsi, int shareable, char *name);
 int xen_map_pirq_gsi(unsigned pirq, unsigned gsi, int shareable, char *name);
 
-- 
1.5.6.5

[PATCH 04/14] xen: events: remove unused public functions

[Ian Campbell]

I was unable to find any user of these functions in either the
functionality pending for 2.6.39 or the xen/next-2.6.32 branch of
xen.git

An exception to this was xen_gsi_from_irq which did appear to be used
in xen/next-2.6.32's pciback. However in the 2.6.39 version of pciback
xen_pirq_from_irq is, correctly AFAICT, used instead.

Only a minority of functions in events.h use "extern" so drop it from
those places for consistency.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |   20 --------------------
 include/xen/events.h |   12 +++---------
 2 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index a9c154d..d52defd 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -222,16 +222,6 @@ static unsigned gsi_from_irq(unsigned irq)
 	return info->u.pirq.gsi;
 }
 
-static unsigned vector_from_irq(unsigned irq)
-{
-	struct irq_info *info = info_for_irq(irq);
-
-	BUG_ON(info == NULL);
-	BUG_ON(info->type != IRQT_PIRQ);
-
-	return info->u.pirq.vector;
-}
-
 static enum xen_irq_type type_from_irq(unsigned irq)
 {
 	return info_for_irq(irq)->type;
@@ -716,16 +706,6 @@ out:
 	return rc;
 }
 
-int xen_vector_from_irq(unsigned irq)
-{
-	return vector_from_irq(irq);
-}
-
-int xen_gsi_from_irq(unsigned irq)
-{
-	return gsi_from_irq(irq);
-}
-
 int xen_irq_from_pirq(unsigned pirq)
 {
 	return pirq_to_irq[pirq];
diff --git a/include/xen/events.h b/include/xen/events.h
index f6fed94..99a64f0 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -41,9 +41,9 @@ static inline void notify_remote_via_evtchn(int port)
 	(void)HYPERVISOR_event_channel_op(EVTCHNOP_send, &send);
 }
 
-extern void notify_remote_via_irq(int irq);
+void notify_remote_via_irq(int irq);
 
-extern void xen_irq_resume(void);
+void xen_irq_resume(void);
 
 /* Clear an irq's pending state, in preparation for polling on it */
 void xen_clear_irq_pending(int irq);
@@ -62,7 +62,7 @@ void xen_poll_irq_timeout(int irq, u64 timeout);
 unsigned irq_from_evtchn(unsigned int evtchn);
 
 /* Xen HVM evtchn vector callback */
-extern void xen_hvm_callback_vector(void);
+void xen_hvm_callback_vector(void);
 extern int xen_have_vector_callback;
 int xen_set_callback_via(uint64_t via);
 void xen_evtchn_do_upcall(struct pt_regs *regs);
@@ -81,12 +81,6 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
 /* De-allocates the above mentioned physical interrupt. */
 int xen_destroy_irq(int irq);
 
-/* Return vector allocated to pirq */
-int xen_vector_from_irq(unsigned pirq);
-
-/* Return gsi allocated to pirq */
-int xen_gsi_from_irq(unsigned pirq);
-
 /* Return irq from pirq */
 int xen_irq_from_pirq(unsigned pirq);
 
-- 
1.5.6.5

[PATCH 05/14] xen: events: rename restore_cpu_pirqs -> restore_pirqs

[Ian Campbell]

There is nothing per-cpu about this function.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index d52defd..e828456 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -1213,7 +1213,7 @@ static int retrigger_dynirq(struct irq_data *data)
 	return ret;
 }
 
-static void restore_cpu_pirqs(void)
+static void restore_pirqs(void)
 {
 	int pirq, rc, irq, gsi;
 	struct physdev_map_pirq map_irq;
@@ -1375,7 +1375,7 @@ void xen_irq_resume(void)
 		restore_cpu_ipis(cpu);
 	}
 
-	restore_cpu_pirqs();
+	restore_pirqs();
 }
 
 static struct irq_chip xen_dynamic_chip __read_mostly = {
-- 
1.5.6.5

[PATCH 06/14] xen: events: refactor GSI pirq bindings functions

[Ian Campbell]

Following the example set by xen_allocate_pirq_msi and
xen_bind_pirq_msi_to_irq:

xen_allocate_pirq becomes xen_allocate_pirq_gsi and now only allocates
a pirq number and does not bind it.

xen_map_pirq_gsi becomes xen_bind_pirq_gsi_to_irq and binds an
existing pirq.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 arch/x86/pci/xen.c   |   40 ++++++++++++++++++++++++++++------------
 drivers/xen/events.c |    7 ++++---
 include/xen/events.h |   10 +++++++---
 3 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 8c4085a..b0fb79d 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -50,7 +50,7 @@ static int acpi_register_gsi_xen_hvm(struct device *dev, u32 gsi,
 		name = "ioapic-level";
 	}
 
-	irq = xen_map_pirq_gsi(map_irq.pirq, gsi, shareable, name);
+	irq = xen_bind_pirq_gsi_to_irq(gsi, map_irq.pirq, shareable, name);
 
 	printk(KERN_DEBUG "xen: --> irq=%d, pirq=%d\n", irq, map_irq.pirq);
 
@@ -237,6 +237,7 @@ static int xen_pcifront_enable_irq(struct pci_dev *dev)
 {
 	int rc;
 	int share = 1;
+	int pirq;
 	u8 gsi;
 
 	rc = pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &gsi);
@@ -246,13 +247,21 @@ static int xen_pcifront_enable_irq(struct pci_dev *dev)
 		return rc;
 	}
 
+	rc = xen_allocate_pirq_gsi(gsi);
+	if (rc < 0) {
+		dev_warn(&dev->dev, "Xen PCI: failed to allocate a PIRQ for GSI%d: %d\n",
+			 gsi, rc);
+		return rc;
+	}
+	pirq = rc;
+
 	if (gsi < NR_IRQS_LEGACY)
 		share = 0;
 
-	rc = xen_allocate_pirq(gsi, share, "pcifront");
+	rc = xen_bind_pirq_gsi_to_irq(gsi, pirq, share, "pcifront");
 	if (rc < 0) {
-		dev_warn(&dev->dev, "Xen PCI: failed to register GSI%d: %d\n",
-			 gsi, rc);
+		dev_warn(&dev->dev, "Xen PCI: failed to bind GSI%d (PIRQ%d) to IRQ: %d\n",
+			 gsi, pirq, rc);
 		return rc;
 	}
 
@@ -309,7 +318,7 @@ int __init pci_xen_hvm_init(void)
 #ifdef CONFIG_XEN_DOM0
 static int xen_register_pirq(u32 gsi, int triggering)
 {
-	int rc, irq;
+	int rc, pirq, irq = -1;
 	struct physdev_map_pirq map_irq;
 	int shareable = 0;
 	char *name;
@@ -325,17 +334,20 @@ static int xen_register_pirq(u32 gsi, int triggering)
 		name = "ioapic-level";
 	}
 
-	irq = xen_allocate_pirq(gsi, shareable, name);
-
-	printk(KERN_DEBUG "xen: --> irq=%d\n", irq);
+	pirq = xen_allocate_pirq_gsi(gsi);
+	if (pirq < 0)
+		goto out;
 
+	irq = xen_bind_pirq_gsi_to_irq(gsi, pirq, shareable, name);
 	if (irq < 0)
 		goto out;
 
+	printk(KERN_DEBUG "xen: --> pirq=%d -> irq=%d\n", pirq, irq);
+
 	map_irq.domid = DOMID_SELF;
 	map_irq.type = MAP_PIRQ_TYPE_GSI;
 	map_irq.index = gsi;
-	map_irq.pirq = irq;
+	map_irq.pirq = pirq;
 
 	rc = HYPERVISOR_physdev_op(PHYSDEVOP_map_pirq, &map_irq);
 	if (rc) {
@@ -422,13 +434,17 @@ static int __init pci_xen_initial_domain(void)
 
 void __init xen_setup_pirqs(void)
 {
-	int irq;
+	int pirq, irq;
 
 	pci_xen_initial_domain();
 
 	if (0 == nr_ioapics) {
-		for (irq = 0; irq < NR_IRQS_LEGACY; irq++)
-			xen_allocate_pirq(irq, 0, "xt-pic");
+		for (irq = 0; irq < NR_IRQS_LEGACY; irq++) {
+			pirq = xen_allocate_pirq_gsi(irq);
+			if (pirq < 0)
+				break;
+			irq = xen_bind_pirq_gsi_to_irq(irq, pirq, 0, "xt-pic");
+		}
 		return;
 	}
 
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index e828456..a40b2a1 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -568,9 +568,9 @@ static int find_irq_by_gsi(unsigned gsi)
 	return -1;
 }
 
-int xen_allocate_pirq(unsigned gsi, int shareable, char *name)
+int xen_allocate_pirq_gsi(unsigned gsi)
 {
-	return xen_map_pirq_gsi(gsi, gsi, shareable, name);
+	return gsi;
 }
 
 /*
@@ -580,7 +580,8 @@ int xen_allocate_pirq(unsigned gsi, int shareable, char *name)
  * Note: We don't assign an event channel until the irq actually started
  * up.  Return an existing irq if we've already got one for the gsi.
  */
-int xen_map_pirq_gsi(unsigned pirq, unsigned gsi, int shareable, char *name)
+int xen_bind_pirq_gsi_to_irq(unsigned gsi,
+			     unsigned pirq, int shareable, char *name)
 {
 	int irq = -1;
 	struct physdev_irq irq_op;
diff --git a/include/xen/events.h b/include/xen/events.h
index 99a64f0..32ebeba 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -68,12 +68,16 @@ int xen_set_callback_via(uint64_t via);
 void xen_evtchn_do_upcall(struct pt_regs *regs);
 void xen_hvm_evtchn_do_upcall(void);
 
-/* Allocate an irq for a physical interrupt, given a gsi. */
-int xen_allocate_pirq(unsigned gsi, int shareable, char *name);
-int xen_map_pirq_gsi(unsigned pirq, unsigned gsi, int shareable, char *name);
+/* Allocate a pirq for a physical interrupt, given a gsi. */
+int xen_allocate_pirq_gsi(unsigned gsi);
+/* Bind a pirq for a physical interrupt to an irq. */
+int xen_bind_pirq_gsi_to_irq(unsigned gsi,
+			     unsigned pirq, int shareable, char *name);
 
 #ifdef CONFIG_PCI_MSI
+/* Allocate a pirq for a MSI style physical interrupt. */
 int xen_allocate_pirq_msi(struct pci_dev *dev, struct msi_desc *msidesc);
+/* Bind an PSI pirq to an irq. */
 int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
 			     int pirq, int vector, const char *name);
 #endif
-- 
1.5.6.5

Re: [Xen-devel] [PATCH 06/14] xen: events: refactor GSI pirq bindings functions

[Konrad Rzeszutek Wilk]

On Wed, Mar 09, 2011 at 05:41:18PM +0000, Ian Campbell wrote:
> Following the example set by xen_allocate_pirq_msi and
> xen_bind_pirq_msi_to_irq:
> 
> xen_allocate_pirq becomes xen_allocate_pirq_gsi and now only allocates
> a pirq number and does not bind it.
> 
> xen_map_pirq_gsi becomes xen_bind_pirq_gsi_to_irq and binds an
> existing pirq.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  arch/x86/pci/xen.c   |   40 ++++++++++++++++++++++++++++------------
>  drivers/xen/events.c |    7 ++++---
>  include/xen/events.h |   10 +++++++---
>  3 files changed, 39 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index 8c4085a..b0fb79d 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -50,7 +50,7 @@ static int acpi_register_gsi_xen_hvm(struct device *dev, u32 gsi,
>  		name = "ioapic-level";
>  	}
>  
> -	irq = xen_map_pirq_gsi(map_irq.pirq, gsi, shareable, name);
> +	irq = xen_bind_pirq_gsi_to_irq(gsi, map_irq.pirq, shareable, name);
>  
>  	printk(KERN_DEBUG "xen: --> irq=%d, pirq=%d\n", irq, map_irq.pirq);
>  
> @@ -237,6 +237,7 @@ static int xen_pcifront_enable_irq(struct pci_dev *dev)
>  {
>  	int rc;
>  	int share = 1;
> +	int pirq;
>  	u8 gsi;
>  
>  	rc = pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &gsi);
> @@ -246,13 +247,21 @@ static int xen_pcifront_enable_irq(struct pci_dev *dev)
>  		return rc;
>  	}
>  
> +	rc = xen_allocate_pirq_gsi(gsi);
> +	if (rc < 0) {
> +		dev_warn(&dev->dev, "Xen PCI: failed to allocate a PIRQ for GSI%d: %d\n",
> +			 gsi, rc);
> +		return rc;
> +	}
> +	pirq = rc;
> +
>  	if (gsi < NR_IRQS_LEGACY)
>  		share = 0;
>  
> -	rc = xen_allocate_pirq(gsi, share, "pcifront");
> +	rc = xen_bind_pirq_gsi_to_irq(gsi, pirq, share, "pcifront");
>  	if (rc < 0) {
> -		dev_warn(&dev->dev, "Xen PCI: failed to register GSI%d: %d\n",
> -			 gsi, rc);
> +		dev_warn(&dev->dev, "Xen PCI: failed to bind GSI%d (PIRQ%d) to IRQ: %d\n",
> +			 gsi, pirq, rc);
>  		return rc;
>  	}
>  
> @@ -309,7 +318,7 @@ int __init pci_xen_hvm_init(void)
>  #ifdef CONFIG_XEN_DOM0
>  static int xen_register_pirq(u32 gsi, int triggering)
>  {
> -	int rc, irq;
> +	int rc, pirq, irq = -1;
>  	struct physdev_map_pirq map_irq;
>  	int shareable = 0;
>  	char *name;
> @@ -325,17 +334,20 @@ static int xen_register_pirq(u32 gsi, int triggering)
>  		name = "ioapic-level";
>  	}
>  
> -	irq = xen_allocate_pirq(gsi, shareable, name);
> -
> -	printk(KERN_DEBUG "xen: --> irq=%d\n", irq);
> +	pirq = xen_allocate_pirq_gsi(gsi);
> +	if (pirq < 0)
> +		goto out;
>  
> +	irq = xen_bind_pirq_gsi_to_irq(gsi, pirq, shareable, name);
>  	if (irq < 0)
>  		goto out;
>  
> +	printk(KERN_DEBUG "xen: --> pirq=%d -> irq=%d\n", pirq, irq);
> +
>  	map_irq.domid = DOMID_SELF;
>  	map_irq.type = MAP_PIRQ_TYPE_GSI;
>  	map_irq.index = gsi;
> -	map_irq.pirq = irq;
> +	map_irq.pirq = pirq;
>  
>  	rc = HYPERVISOR_physdev_op(PHYSDEVOP_map_pirq, &map_irq);
>  	if (rc) {
> @@ -422,13 +434,17 @@ static int __init pci_xen_initial_domain(void)
>  
>  void __init xen_setup_pirqs(void)
>  {
> -	int irq;
> +	int pirq, irq;
>  
>  	pci_xen_initial_domain();
>  
>  	if (0 == nr_ioapics) {
> -		for (irq = 0; irq < NR_IRQS_LEGACY; irq++)
> -			xen_allocate_pirq(irq, 0, "xt-pic");
> +		for (irq = 0; irq < NR_IRQS_LEGACY; irq++) {
> +			pirq = xen_allocate_pirq_gsi(irq);
> +			if (pirq < 0)
> +				break;

Would it make sense to print a warning here?

Re: [Xen-devel] [PATCH 06/14] xen: events: refactor GSI pirq bindings functions

[Ian Campbell]

On Thu, 2011-03-10 at 04:00 +0000, Konrad Rzeszutek Wilk wrote:
> 	if (0 == nr_ioapics) {
> > -		for (irq = 0; irq < NR_IRQS_LEGACY; irq++)
> > -			xen_allocate_pirq(irq, 0, "xt-pic");
> > +		for (irq = 0; irq < NR_IRQS_LEGACY; irq++) {
> > +			pirq = xen_allocate_pirq_gsi(irq);
> > +			if (pirq < 0)
> > +				break;
> 
> Would it make sense to print a warning here?

I was almost tempted by a BUG_ON but:

8<-------------------------------------------------

>From b5f92c9d914988cd29c45a84cde462a8588467b6 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Thu, 10 Mar 2011 08:36:55 +0000
Subject: [PATCH] xen: irq: warn if we cannot allocate a PIRQ for the legacy IRQs in dom0

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 arch/x86/pci/xen.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 3ee5f4a..ca58a73 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -452,7 +452,8 @@ void __init xen_setup_pirqs(void)
 	if (0 == nr_ioapics) {
 		for (irq = 0; irq < NR_IRQS_LEGACY; irq++) {
 			pirq = xen_allocate_pirq_gsi(irq);
-			if (pirq < 0)
+			if (WARN(pirq < 0,
+				 "Could not allocate PIRQ for legacy interrupt\n"))
 				break;
 			irq = xen_bind_pirq_gsi_to_irq(irq, pirq, 0, "xt-pic");
 		}
-- 
1.5.6.5

[PATCH 07/14] xen: events: use per-cpu variable for cpu_evtchn_mask

[Ian Campbell]

I can't see any reason why it isn't already.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |   28 ++++++++--------------------
 1 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index a40b2a1..2dffa43 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -107,19 +107,9 @@ static struct irq_info *irq_info;
 static int *pirq_to_irq;
 
 static int *evtchn_to_irq;
-struct cpu_evtchn_s {
-	unsigned long bits[NR_EVENT_CHANNELS/BITS_PER_LONG];
-};
-
-static __initdata struct cpu_evtchn_s init_evtchn_mask = {
-	.bits[0 ... (NR_EVENT_CHANNELS/BITS_PER_LONG)-1] = ~0ul,
-};
-static struct cpu_evtchn_s *cpu_evtchn_mask_p = &init_evtchn_mask;
 
-static inline unsigned long *cpu_evtchn_mask(int cpu)
-{
-	return cpu_evtchn_mask_p[cpu].bits;
-}
+static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
+		      cpu_evtchn_mask);
 
 /* Xen will never allocate port zero for any purpose. */
 #define VALID_EVTCHN(chn)	((chn) != 0)
@@ -257,7 +247,7 @@ static inline unsigned long active_evtchns(unsigned int cpu,
 					   unsigned int idx)
 {
 	return (sh->evtchn_pending[idx] &
-		cpu_evtchn_mask(cpu)[idx] &
+		per_cpu(cpu_evtchn_mask, cpu)[idx] &
 		~sh->evtchn_mask[idx]);
 }
 
@@ -270,8 +260,8 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
 	cpumask_copy(irq_to_desc(irq)->irq_data.affinity, cpumask_of(cpu));
 #endif
 
-	clear_bit(chn, cpu_evtchn_mask(cpu_from_irq(irq)));
-	set_bit(chn, cpu_evtchn_mask(cpu));
+	clear_bit(chn, per_cpu(cpu_evtchn_mask, cpu_from_irq(irq)));
+	set_bit(chn, per_cpu(cpu_evtchn_mask, cpu));
 
 	irq_info[irq].cpu = cpu;
 }
@@ -289,8 +279,8 @@ static void init_evtchn_cpu_bindings(void)
 #endif
 
 	for_each_possible_cpu(i)
-		memset(cpu_evtchn_mask(i),
-		       (i == 0) ? ~0 : 0, sizeof(struct cpu_evtchn_s));
+		memset(per_cpu(cpu_evtchn_mask, i),
+		       (i == 0) ? ~0 : 0, sizeof(*per_cpu(cpu_evtchn_mask, i)));
 
 }
 
@@ -925,7 +915,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
 {
 	struct shared_info *sh = HYPERVISOR_shared_info;
 	int cpu = smp_processor_id();
-	unsigned long *cpu_evtchn = cpu_evtchn_mask(cpu);
+	unsigned long *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
 	int i;
 	unsigned long flags;
 	static DEFINE_SPINLOCK(debug_lock);
@@ -1462,8 +1452,6 @@ void __init xen_init_IRQ(void)
 {
 	int i;
 
-	cpu_evtchn_mask_p = kcalloc(nr_cpu_ids, sizeof(struct cpu_evtchn_s),
-				    GFP_KERNEL);
 	irq_info = kcalloc(nr_irqs, sizeof(*irq_info), GFP_KERNEL);
 
 	/* We are using nr_irqs as the maximum number of pirq available but
-- 
1.5.6.5

[PATCH 08/14] xen: events: turn irq_info constructors into initialiser functions

[Ian Campbell]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |  104 +++++++++++++++++++++++++++++++------------------
 1 files changed, 66 insertions(+), 38 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 2dffa43..72725fa 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -118,46 +118,76 @@ static struct irq_chip xen_dynamic_chip;
 static struct irq_chip xen_percpu_chip;
 static struct irq_chip xen_pirq_chip;
 
-/* Constructor for packed IRQ information. */
-static struct irq_info mk_unbound_info(void)
+/* Get info for IRQ */
+static struct irq_info *info_for_irq(unsigned irq)
 {
-	return (struct irq_info) { .type = IRQT_UNBOUND };
+	return &irq_info[irq];
 }
 
-static struct irq_info mk_evtchn_info(unsigned short evtchn)
+/* Constructors for packed IRQ information. */
+static void xen_irq_info_common_init(struct irq_info *info,
+				     enum xen_irq_type type,
+				     unsigned short evtchn,
+				     unsigned short cpu)
 {
-	return (struct irq_info) { .type = IRQT_EVTCHN, .evtchn = evtchn,
-			.cpu = 0 };
+
+	BUG_ON(info->type != IRQT_UNBOUND && info->type != type);
+
+	info->type = type;
+	info->evtchn = evtchn;
+	info->cpu = cpu;
 }
 
-static struct irq_info mk_ipi_info(unsigned short evtchn, enum ipi_vector ipi)
+static void xen_irq_info_evtchn_init(unsigned irq,
+				     unsigned short evtchn)
 {
-	return (struct irq_info) { .type = IRQT_IPI, .evtchn = evtchn,
-			.cpu = 0, .u.ipi = ipi };
+	struct irq_info *info = info_for_irq(irq);
+
+	xen_irq_info_common_init(info, IRQT_EVTCHN, evtchn, 0);
 }
 
-static struct irq_info mk_virq_info(unsigned short evtchn, unsigned short virq)
+static void xen_irq_info_ipi_init(unsigned irq,
+				  unsigned short evtchn,
+				  enum ipi_vector ipi)
 {
-	return (struct irq_info) { .type = IRQT_VIRQ, .evtchn = evtchn,
-			.cpu = 0, .u.virq = virq };
+	struct irq_info *info = info_for_irq(irq);
+
+	xen_irq_info_common_init(info, IRQT_IPI, evtchn, 0);
+
+	info->u.ipi = ipi;
 }
 
-static struct irq_info mk_pirq_info(unsigned short evtchn, unsigned short pirq,
-				    unsigned short gsi, unsigned short vector)
+static void xen_irq_info_virq_init(unsigned irq,
+				   unsigned short evtchn,
+				   unsigned short virq)
 {
-	return (struct irq_info) { .type = IRQT_PIRQ, .evtchn = evtchn,
-			.cpu = 0,
-			.u.pirq = { .pirq = pirq, .gsi = gsi, .vector = vector } };
+	struct irq_info *info = info_for_irq(irq);
+
+	xen_irq_info_common_init(info, IRQT_VIRQ, evtchn, 0);
+
+	info->u.virq = virq;
 }
 
-/*
- * Accessors for packed IRQ information.
- */
-static struct irq_info *info_for_irq(unsigned irq)
+static void xen_irq_info_pirq_init(unsigned irq,
+				   unsigned short evtchn,
+				   unsigned short pirq,
+				   unsigned short gsi,
+				   unsigned short vector,
+				   unsigned char flags)
 {
-	return &irq_info[irq];
+	struct irq_info *info = info_for_irq(irq);
+
+	xen_irq_info_common_init(info, IRQT_PIRQ, evtchn, 0);
+
+	info->u.pirq.pirq = pirq;
+	info->u.pirq.gsi = gsi;
+	info->u.pirq.vector = vector;
+	info->u.pirq.flags = flags;
 }
 
+/*
+ * Accessors for packed IRQ information.
+ */
 static unsigned int evtchn_from_irq(unsigned irq)
 {
 	if (unlikely(WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n", irq)))
@@ -414,6 +444,8 @@ static int xen_allocate_irq_gsi(unsigned gsi)
 
 static void xen_free_irq(unsigned irq)
 {
+	irq_info[irq].type = IRQT_UNBOUND;
+
 	/* Legacy IRQ descriptors are managed by the arch. */
 	if (irq < NR_IRQS_LEGACY)
 		return;
@@ -610,8 +642,8 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
 		goto out;
 	}
 
-	irq_info[irq] = mk_pirq_info(0, pirq, gsi, irq_op.vector);
-	irq_info[irq].u.pirq.flags |= shareable ? PIRQ_SHAREABLE : 0;
+	xen_irq_info_pirq_init(irq, 0, pirq, gsi, irq_op.vector,
+			       shareable ? PIRQ_SHAREABLE : 0);
 	pirq_to_irq[pirq] = irq;
 
 out:
@@ -649,7 +681,7 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
 	set_irq_chip_and_handler_name(irq, &xen_pirq_chip,
 				      handle_level_irq, name);
 
-	irq_info[irq] = mk_pirq_info(0, pirq, 0, vector);
+	xen_irq_info_pirq_init(irq, 0, pirq, 0, vector, 0);
 	pirq_to_irq[pirq] = irq;
 	ret = set_irq_msi(irq, msidesc);
 	if (ret < 0)
@@ -688,8 +720,6 @@ int xen_destroy_irq(int irq)
 	}
 	pirq_to_irq[info->u.pirq.pirq] = -1;
 
-	irq_info[irq] = mk_unbound_info();
-
 	xen_free_irq(irq);
 
 out:
@@ -717,7 +747,7 @@ int bind_evtchn_to_irq(unsigned int evtchn)
 					      handle_fasteoi_irq, "event");
 
 		evtchn_to_irq[evtchn] = irq;
-		irq_info[irq] = mk_evtchn_info(evtchn);
+		xen_irq_info_evtchn_init(irq, evtchn);
 	}
 
 	spin_unlock(&irq_mapping_update_lock);
@@ -750,7 +780,7 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
 		evtchn = bind_ipi.port;
 
 		evtchn_to_irq[evtchn] = irq;
-		irq_info[irq] = mk_ipi_info(evtchn, ipi);
+		xen_irq_info_ipi_init(irq, evtchn, ipi);
 		per_cpu(ipi_to_irq, cpu)[ipi] = irq;
 
 		bind_evtchn_to_cpu(evtchn, cpu);
@@ -785,7 +815,7 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
 		evtchn = bind_virq.port;
 
 		evtchn_to_irq[evtchn] = irq;
-		irq_info[irq] = mk_virq_info(evtchn, virq);
+		xen_irq_info_virq_init(irq, evtchn, virq);
 
 		per_cpu(virq_to_irq, cpu)[virq] = irq;
 
@@ -828,11 +858,9 @@ static void unbind_from_irq(unsigned int irq)
 		evtchn_to_irq[evtchn] = -1;
 	}
 
-	if (irq_info[irq].type != IRQT_UNBOUND) {
-		irq_info[irq] = mk_unbound_info();
+	BUG_ON(irq_info[irq].type == IRQT_UNBOUND);
 
-		xen_free_irq(irq);
-	}
+	xen_free_irq(irq);
 
 	spin_unlock(&irq_mapping_update_lock);
 }
@@ -1093,7 +1121,7 @@ void rebind_evtchn_irq(int evtchn, int irq)
 	BUG_ON(info->type == IRQT_UNBOUND);
 
 	evtchn_to_irq[evtchn] = irq;
-	irq_info[irq] = mk_evtchn_info(evtchn);
+	xen_irq_info_evtchn_init(irq, evtchn);
 
 	spin_unlock(&irq_mapping_update_lock);
 
@@ -1229,7 +1257,7 @@ static void restore_pirqs(void)
 		if (rc) {
 			printk(KERN_WARNING "xen map irq failed gsi=%d irq=%d pirq=%d rc=%d\n",
 					gsi, irq, pirq, rc);
-			irq_info[irq] = mk_unbound_info();
+			xen_free_irq(irq);
 			pirq_to_irq[pirq] = -1;
 			continue;
 		}
@@ -1261,7 +1289,7 @@ static void restore_cpu_virqs(unsigned int cpu)
 
 		/* Record the new mapping. */
 		evtchn_to_irq[evtchn] = irq;
-		irq_info[irq] = mk_virq_info(evtchn, virq);
+		xen_irq_info_virq_init(irq, evtchn, virq);
 		bind_evtchn_to_cpu(evtchn, cpu);
 	}
 }
@@ -1286,7 +1314,7 @@ static void restore_cpu_ipis(unsigned int cpu)
 
 		/* Record the new mapping. */
 		evtchn_to_irq[evtchn] = irq;
-		irq_info[irq] = mk_ipi_info(evtchn, ipi);
+		xen_irq_info_ipi_init(irq, evtchn, ipi);
 		bind_evtchn_to_cpu(evtchn, cpu);
 	}
 }
-- 
1.5.6.5

[PATCH 09/14] xen: events: push setup of irq<->{evtchn,ipi,virq,pirq} maps into irq_info init functions

[Ian Campbell]

Encapsulate setup of XXX_to_irq array in the relevant
xen_irq_info_*_init function.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |   42 +++++++++++++++++++++---------------------
 1 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 72725fa..cf372d4 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -126,6 +126,7 @@ static struct irq_info *info_for_irq(unsigned irq)
 
 /* Constructors for packed IRQ information. */
 static void xen_irq_info_common_init(struct irq_info *info,
+				     unsigned irq,
 				     enum xen_irq_type type,
 				     unsigned short evtchn,
 				     unsigned short cpu)
@@ -136,6 +137,8 @@ static void xen_irq_info_common_init(struct irq_info *info,
 	info->type = type;
 	info->evtchn = evtchn;
 	info->cpu = cpu;
+
+	evtchn_to_irq[evtchn] = irq;
 }
 
 static void xen_irq_info_evtchn_init(unsigned irq,
@@ -143,29 +146,35 @@ static void xen_irq_info_evtchn_init(unsigned irq,
 {
 	struct irq_info *info = info_for_irq(irq);
 
-	xen_irq_info_common_init(info, IRQT_EVTCHN, evtchn, 0);
+	xen_irq_info_common_init(info, irq, IRQT_EVTCHN, evtchn, 0);
 }
 
-static void xen_irq_info_ipi_init(unsigned irq,
+static void xen_irq_info_ipi_init(unsigned cpu,
+				  unsigned irq,
 				  unsigned short evtchn,
 				  enum ipi_vector ipi)
 {
 	struct irq_info *info = info_for_irq(irq);
 
-	xen_irq_info_common_init(info, IRQT_IPI, evtchn, 0);
+	xen_irq_info_common_init(info, irq, IRQT_IPI, evtchn, 0);
 
 	info->u.ipi = ipi;
+
+	per_cpu(ipi_to_irq, cpu)[ipi] = irq;
 }
 
-static void xen_irq_info_virq_init(unsigned irq,
+static void xen_irq_info_virq_init(unsigned cpu,
+				   unsigned irq,
 				   unsigned short evtchn,
 				   unsigned short virq)
 {
 	struct irq_info *info = info_for_irq(irq);
 
-	xen_irq_info_common_init(info, IRQT_VIRQ, evtchn, 0);
+	xen_irq_info_common_init(info, irq, IRQT_VIRQ, evtchn, 0);
 
 	info->u.virq = virq;
+
+	per_cpu(virq_to_irq, cpu)[virq] = irq;
 }
 
 static void xen_irq_info_pirq_init(unsigned irq,
@@ -177,12 +186,14 @@ static void xen_irq_info_pirq_init(unsigned irq,
 {
 	struct irq_info *info = info_for_irq(irq);
 
-	xen_irq_info_common_init(info, IRQT_PIRQ, evtchn, 0);
+	xen_irq_info_common_init(info, irq, IRQT_PIRQ, evtchn, 0);
 
 	info->u.pirq.pirq = pirq;
 	info->u.pirq.gsi = gsi;
 	info->u.pirq.vector = vector;
 	info->u.pirq.flags = flags;
+
+	pirq_to_irq[pirq] = irq;
 }
 
 /*
@@ -644,7 +655,6 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
 
 	xen_irq_info_pirq_init(irq, 0, pirq, gsi, irq_op.vector,
 			       shareable ? PIRQ_SHAREABLE : 0);
-	pirq_to_irq[pirq] = irq;
 
 out:
 	spin_unlock(&irq_mapping_update_lock);
@@ -682,7 +692,6 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
 				      handle_level_irq, name);
 
 	xen_irq_info_pirq_init(irq, 0, pirq, 0, vector, 0);
-	pirq_to_irq[pirq] = irq;
 	ret = set_irq_msi(irq, msidesc);
 	if (ret < 0)
 		goto error_irq;
@@ -746,7 +755,6 @@ int bind_evtchn_to_irq(unsigned int evtchn)
 		set_irq_chip_and_handler_name(irq, &xen_dynamic_chip,
 					      handle_fasteoi_irq, "event");
 
-		evtchn_to_irq[evtchn] = irq;
 		xen_irq_info_evtchn_init(irq, evtchn);
 	}
 
@@ -779,9 +787,7 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
 			BUG();
 		evtchn = bind_ipi.port;
 
-		evtchn_to_irq[evtchn] = irq;
-		xen_irq_info_ipi_init(irq, evtchn, ipi);
-		per_cpu(ipi_to_irq, cpu)[ipi] = irq;
+		xen_irq_info_ipi_init(cpu, irq, evtchn, ipi);
 
 		bind_evtchn_to_cpu(evtchn, cpu);
 	}
@@ -814,10 +820,7 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
 			BUG();
 		evtchn = bind_virq.port;
 
-		evtchn_to_irq[evtchn] = irq;
-		xen_irq_info_virq_init(irq, evtchn, virq);
-
-		per_cpu(virq_to_irq, cpu)[virq] = irq;
+		xen_irq_info_virq_init(cpu, irq, evtchn, virq);
 
 		bind_evtchn_to_cpu(evtchn, cpu);
 	}
@@ -1120,7 +1123,6 @@ void rebind_evtchn_irq(int evtchn, int irq)
 	   so there should be a proper type */
 	BUG_ON(info->type == IRQT_UNBOUND);
 
-	evtchn_to_irq[evtchn] = irq;
 	xen_irq_info_evtchn_init(irq, evtchn);
 
 	spin_unlock(&irq_mapping_update_lock);
@@ -1288,8 +1290,7 @@ static void restore_cpu_virqs(unsigned int cpu)
 		evtchn = bind_virq.port;
 
 		/* Record the new mapping. */
-		evtchn_to_irq[evtchn] = irq;
-		xen_irq_info_virq_init(irq, evtchn, virq);
+		xen_irq_info_virq_init(cpu, irq, evtchn, virq);
 		bind_evtchn_to_cpu(evtchn, cpu);
 	}
 }
@@ -1313,8 +1314,7 @@ static void restore_cpu_ipis(unsigned int cpu)
 		evtchn = bind_ipi.port;
 
 		/* Record the new mapping. */
-		evtchn_to_irq[evtchn] = irq;
-		xen_irq_info_ipi_init(irq, evtchn, ipi);
+		xen_irq_info_ipi_init(cpu, irq, evtchn, ipi);
 		bind_evtchn_to_cpu(evtchn, cpu);
 	}
 }
-- 
1.5.6.5

Re: [PATCH 09/14] xen: events: push setup of irq<->{evtchn,ipi,virq,pirq} maps into irq_info init functions

[Konrad Rzeszutek Wilk]

On Wed, Mar 09, 2011 at 05:41:21PM +0000, Ian Campbell wrote:
> Encapsulate setup of XXX_to_irq array in the relevant
> xen_irq_info_*_init function.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  drivers/xen/events.c |   42 +++++++++++++++++++++---------------------
>  1 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 72725fa..cf372d4 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -126,6 +126,7 @@ static struct irq_info *info_for_irq(unsigned irq)
>  
>  /* Constructors for packed IRQ information. */
>  static void xen_irq_info_common_init(struct irq_info *info,
> +				     unsigned irq,
>  				     enum xen_irq_type type,
>  				     unsigned short evtchn,
>  				     unsigned short cpu)
> @@ -136,6 +137,8 @@ static void xen_irq_info_common_init(struct irq_info *info,
>  	info->type = type;
>  	info->evtchn = evtchn;
>  	info->cpu = cpu;
> +
> +	evtchn_to_irq[evtchn] = irq;

Is there any case where this would lead to an over-write? Should we
have an
  WARN_ON(evtchn_to_irq[evtchn] != -1)

just to check?
>  }
>  
>  static void xen_irq_info_evtchn_init(unsigned irq,
> @@ -143,29 +146,35 @@ static void xen_irq_info_evtchn_init(unsigned irq,
>  {
>  	struct irq_info *info = info_for_irq(irq);
>  
> -	xen_irq_info_common_init(info, IRQT_EVTCHN, evtchn, 0);
> +	xen_irq_info_common_init(info, irq, IRQT_EVTCHN, evtchn, 0);
>  }
>  
> -static void xen_irq_info_ipi_init(unsigned irq,
> +static void xen_irq_info_ipi_init(unsigned cpu,
> +				  unsigned irq,
>  				  unsigned short evtchn,
>  				  enum ipi_vector ipi)
>  {
>  	struct irq_info *info = info_for_irq(irq);
>  
> -	xen_irq_info_common_init(info, IRQT_IPI, evtchn, 0);
> +	xen_irq_info_common_init(info, irq, IRQT_IPI, evtchn, 0);
>  
>  	info->u.ipi = ipi;
> +
> +	per_cpu(ipi_to_irq, cpu)[ipi] = irq;

Ditto. Should we do a check first to see if we are overwritting anything
but the default value of -1?
>  }
>  
> -static void xen_irq_info_virq_init(unsigned irq,
> +static void xen_irq_info_virq_init(unsigned cpu,
> +				   unsigned irq,
>  				   unsigned short evtchn,
>  				   unsigned short virq)
>  {
>  	struct irq_info *info = info_for_irq(irq);
>  
> -	xen_irq_info_common_init(info, IRQT_VIRQ, evtchn, 0);
> +	xen_irq_info_common_init(info, irq, IRQT_VIRQ, evtchn, 0);
>  
>  	info->u.virq = virq;
> +
> +	per_cpu(virq_to_irq, cpu)[virq] = irq;
>  }
>  
>  static void xen_irq_info_pirq_init(unsigned irq,
> @@ -177,12 +186,14 @@ static void xen_irq_info_pirq_init(unsigned irq,
>  {
>  	struct irq_info *info = info_for_irq(irq);
>  
> -	xen_irq_info_common_init(info, IRQT_PIRQ, evtchn, 0);
> +	xen_irq_info_common_init(info, irq, IRQT_PIRQ, evtchn, 0);
>  
>  	info->u.pirq.pirq = pirq;
>  	info->u.pirq.gsi = gsi;
>  	info->u.pirq.vector = vector;
>  	info->u.pirq.flags = flags;
> +
> +	pirq_to_irq[pirq] = irq;
>  }
>  
>  /*
> @@ -644,7 +655,6 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
>  
>  	xen_irq_info_pirq_init(irq, 0, pirq, gsi, irq_op.vector,
>  			       shareable ? PIRQ_SHAREABLE : 0);
> -	pirq_to_irq[pirq] = irq;
>  
>  out:
>  	spin_unlock(&irq_mapping_update_lock);
> @@ -682,7 +692,6 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
>  				      handle_level_irq, name);
>  
>  	xen_irq_info_pirq_init(irq, 0, pirq, 0, vector, 0);
> -	pirq_to_irq[pirq] = irq;
>  	ret = set_irq_msi(irq, msidesc);
>  	if (ret < 0)
>  		goto error_irq;
> @@ -746,7 +755,6 @@ int bind_evtchn_to_irq(unsigned int evtchn)
>  		set_irq_chip_and_handler_name(irq, &xen_dynamic_chip,
>  					      handle_fasteoi_irq, "event");
>  
> -		evtchn_to_irq[evtchn] = irq;
>  		xen_irq_info_evtchn_init(irq, evtchn);
>  	}
>  
> @@ -779,9 +787,7 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
>  			BUG();
>  		evtchn = bind_ipi.port;
>  
> -		evtchn_to_irq[evtchn] = irq;
> -		xen_irq_info_ipi_init(irq, evtchn, ipi);
> -		per_cpu(ipi_to_irq, cpu)[ipi] = irq;
> +		xen_irq_info_ipi_init(cpu, irq, evtchn, ipi);
>  
>  		bind_evtchn_to_cpu(evtchn, cpu);
>  	}
> @@ -814,10 +820,7 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
>  			BUG();
>  		evtchn = bind_virq.port;
>  
> -		evtchn_to_irq[evtchn] = irq;
> -		xen_irq_info_virq_init(irq, evtchn, virq);
> -
> -		per_cpu(virq_to_irq, cpu)[virq] = irq;
> +		xen_irq_info_virq_init(cpu, irq, evtchn, virq);
>  
>  		bind_evtchn_to_cpu(evtchn, cpu);
>  	}
> @@ -1120,7 +1123,6 @@ void rebind_evtchn_irq(int evtchn, int irq)
>  	   so there should be a proper type */
>  	BUG_ON(info->type == IRQT_UNBOUND);
>  
> -	evtchn_to_irq[evtchn] = irq;
>  	xen_irq_info_evtchn_init(irq, evtchn);
>  
>  	spin_unlock(&irq_mapping_update_lock);
> @@ -1288,8 +1290,7 @@ static void restore_cpu_virqs(unsigned int cpu)
>  		evtchn = bind_virq.port;
>  
>  		/* Record the new mapping. */
> -		evtchn_to_irq[evtchn] = irq;
> -		xen_irq_info_virq_init(irq, evtchn, virq);
> +		xen_irq_info_virq_init(cpu, irq, evtchn, virq);
>  		bind_evtchn_to_cpu(evtchn, cpu);
>  	}
>  }
> @@ -1313,8 +1314,7 @@ static void restore_cpu_ipis(unsigned int cpu)
>  		evtchn = bind_ipi.port;
>  
>  		/* Record the new mapping. */
> -		evtchn_to_irq[evtchn] = irq;
> -		xen_irq_info_ipi_init(irq, evtchn, ipi);
> +		xen_irq_info_ipi_init(cpu, irq, evtchn, ipi);
>  		bind_evtchn_to_cpu(evtchn, cpu);
>  	}
>  }
> -- 
> 1.5.6.5

Re: [PATCH 09/14] xen: events: push setup of irq<->{evtchn,ipi,virq,pirq} maps into irq_info init functions

[Ian Campbell]

On Thu, 2011-03-10 at 04:56 +0000, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 09, 2011 at 05:41:21PM +0000, Ian Campbell wrote:
> > Encapsulate setup of XXX_to_irq array in the relevant
> > xen_irq_info_*_init function.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  drivers/xen/events.c |   42 +++++++++++++++++++++---------------------
> >  1 files changed, 21 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > index 72725fa..cf372d4 100644
> > --- a/drivers/xen/events.c
> > +++ b/drivers/xen/events.c
> > @@ -126,6 +126,7 @@ static struct irq_info *info_for_irq(unsigned irq)
> >  
> >  /* Constructors for packed IRQ information. */
> >  static void xen_irq_info_common_init(struct irq_info *info,
> > +				     unsigned irq,
> >  				     enum xen_irq_type type,
> >  				     unsigned short evtchn,
> >  				     unsigned short cpu)
> > @@ -136,6 +137,8 @@ static void xen_irq_info_common_init(struct irq_info *info,
> >  	info->type = type;
> >  	info->evtchn = evtchn;
> >  	info->cpu = cpu;
> > +
> > +	evtchn_to_irq[evtchn] = irq;
> 
> Is there any case where this would lead to an over-write? Should we
> have an
>   WARN_ON(evtchn_to_irq[evtchn] != -1)
> 
> just to check?

This patch is pure code motion so there should be no more need for a
check than there was before.

I don't think it can happen, the callers are either running in a context
where the evtchn mapping has just been zapped (e.g. resume) or they
include an explicit check of evtchn_to_irq before getting this far (e.g.
bind_evtchn_to_irq and friends).

In any case, not much further down the series this switches to
initialising a recently allocated irq_info structure so we wouldn't
catch any errors this way.

Ian.

> >  }
> >  
> >  static void xen_irq_info_evtchn_init(unsigned irq,
> > @@ -143,29 +146,35 @@ static void xen_irq_info_evtchn_init(unsigned irq,
> >  {
> >  	struct irq_info *info = info_for_irq(irq);
> >  
> > -	xen_irq_info_common_init(info, IRQT_EVTCHN, evtchn, 0);
> > +	xen_irq_info_common_init(info, irq, IRQT_EVTCHN, evtchn, 0);
> >  }
> >  
> > -static void xen_irq_info_ipi_init(unsigned irq,
> > +static void xen_irq_info_ipi_init(unsigned cpu,
> > +				  unsigned irq,
> >  				  unsigned short evtchn,
> >  				  enum ipi_vector ipi)
> >  {
> >  	struct irq_info *info = info_for_irq(irq);
> >  
> > -	xen_irq_info_common_init(info, IRQT_IPI, evtchn, 0);
> > +	xen_irq_info_common_init(info, irq, IRQT_IPI, evtchn, 0);
> >  
> >  	info->u.ipi = ipi;
> > +
> > +	per_cpu(ipi_to_irq, cpu)[ipi] = irq;
> 
> Ditto. Should we do a check first to see if we are overwritting anything
> but the default value of -1?
> >  }
> >  
> > -static void xen_irq_info_virq_init(unsigned irq,
> > +static void xen_irq_info_virq_init(unsigned cpu,
> > +				   unsigned irq,
> >  				   unsigned short evtchn,
> >  				   unsigned short virq)
> >  {
> >  	struct irq_info *info = info_for_irq(irq);
> >  
> > -	xen_irq_info_common_init(info, IRQT_VIRQ, evtchn, 0);
> > +	xen_irq_info_common_init(info, irq, IRQT_VIRQ, evtchn, 0);
> >  
> >  	info->u.virq = virq;
> > +
> > +	per_cpu(virq_to_irq, cpu)[virq] = irq;
> >  }
> >  
> >  static void xen_irq_info_pirq_init(unsigned irq,
> > @@ -177,12 +186,14 @@ static void xen_irq_info_pirq_init(unsigned irq,
> >  {
> >  	struct irq_info *info = info_for_irq(irq);
> >  
> > -	xen_irq_info_common_init(info, IRQT_PIRQ, evtchn, 0);
> > +	xen_irq_info_common_init(info, irq, IRQT_PIRQ, evtchn, 0);
> >  
> >  	info->u.pirq.pirq = pirq;
> >  	info->u.pirq.gsi = gsi;
> >  	info->u.pirq.vector = vector;
> >  	info->u.pirq.flags = flags;
> > +
> > +	pirq_to_irq[pirq] = irq;
> >  }
> >  
> >  /*
> > @@ -644,7 +655,6 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
> >  
> >  	xen_irq_info_pirq_init(irq, 0, pirq, gsi, irq_op.vector,
> >  			       shareable ? PIRQ_SHAREABLE : 0);
> > -	pirq_to_irq[pirq] = irq;
> >  
> >  out:
> >  	spin_unlock(&irq_mapping_update_lock);
> > @@ -682,7 +692,6 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
> >  				      handle_level_irq, name);
> >  
> >  	xen_irq_info_pirq_init(irq, 0, pirq, 0, vector, 0);
> > -	pirq_to_irq[pirq] = irq;
> >  	ret = set_irq_msi(irq, msidesc);
> >  	if (ret < 0)
> >  		goto error_irq;
> > @@ -746,7 +755,6 @@ int bind_evtchn_to_irq(unsigned int evtchn)
> >  		set_irq_chip_and_handler_name(irq, &xen_dynamic_chip,
> >  					      handle_fasteoi_irq, "event");
> >  
> > -		evtchn_to_irq[evtchn] = irq;
> >  		xen_irq_info_evtchn_init(irq, evtchn);
> >  	}
> >  
> > @@ -779,9 +787,7 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
> >  			BUG();
> >  		evtchn = bind_ipi.port;
> >  
> > -		evtchn_to_irq[evtchn] = irq;
> > -		xen_irq_info_ipi_init(irq, evtchn, ipi);
> > -		per_cpu(ipi_to_irq, cpu)[ipi] = irq;
> > +		xen_irq_info_ipi_init(cpu, irq, evtchn, ipi);
> >  
> >  		bind_evtchn_to_cpu(evtchn, cpu);
> >  	}
> > @@ -814,10 +820,7 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
> >  			BUG();
> >  		evtchn = bind_virq.port;
> >  
> > -		evtchn_to_irq[evtchn] = irq;
> > -		xen_irq_info_virq_init(irq, evtchn, virq);
> > -
> > -		per_cpu(virq_to_irq, cpu)[virq] = irq;
> > +		xen_irq_info_virq_init(cpu, irq, evtchn, virq);
> >  
> >  		bind_evtchn_to_cpu(evtchn, cpu);
> >  	}
> > @@ -1120,7 +1123,6 @@ void rebind_evtchn_irq(int evtchn, int irq)
> >  	   so there should be a proper type */
> >  	BUG_ON(info->type == IRQT_UNBOUND);
> >  
> > -	evtchn_to_irq[evtchn] = irq;
> >  	xen_irq_info_evtchn_init(irq, evtchn);
> >  
> >  	spin_unlock(&irq_mapping_update_lock);
> > @@ -1288,8 +1290,7 @@ static void restore_cpu_virqs(unsigned int cpu)
> >  		evtchn = bind_virq.port;
> >  
> >  		/* Record the new mapping. */
> > -		evtchn_to_irq[evtchn] = irq;
> > -		xen_irq_info_virq_init(irq, evtchn, virq);
> > +		xen_irq_info_virq_init(cpu, irq, evtchn, virq);
> >  		bind_evtchn_to_cpu(evtchn, cpu);
> >  	}
> >  }
> > @@ -1313,8 +1314,7 @@ static void restore_cpu_ipis(unsigned int cpu)
> >  		evtchn = bind_ipi.port;
> >  
> >  		/* Record the new mapping. */
> > -		evtchn_to_irq[evtchn] = irq;
> > -		xen_irq_info_ipi_init(irq, evtchn, ipi);
> > +		xen_irq_info_ipi_init(cpu, irq, evtchn, ipi);
> >  		bind_evtchn_to_cpu(evtchn, cpu);
> >  	}
> >  }
> > -- 
> > 1.5.6.5

[PATCH 10/14] xen: events: maintain a list of Xen interrupts

[Ian Campbell]

In a PVHVM kernel not all interrupts are Xen interrupts (APIC interrupts can also be present).

Currently we get away with walking over all interrupts because the
lookup in the irq_info array simply returns IRQT_UNBOUND and we ignore
it. However this array will be going away in a future patch so we need
to manually track which interrupts have been allocated by the Xen
events infrastructure.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |   59 +++++++++++++++++++++++++++++++++++++------------
 1 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index cf372d4..e119989 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -56,6 +56,8 @@
  */
 static DEFINE_SPINLOCK(irq_mapping_update_lock);
 
+static LIST_HEAD(xen_irq_list_head);
+
 /* IRQ <-> VIRQ mapping. */
 static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... NR_VIRQS-1] = -1};
 
@@ -85,7 +87,9 @@ enum xen_irq_type {
  */
 struct irq_info
 {
+	struct list_head list;
 	enum xen_irq_type type;	/* type */
+	unsigned irq;
 	unsigned short evtchn;	/* event channel */
 	unsigned short cpu;	/* cpu bound */
 
@@ -135,6 +139,7 @@ static void xen_irq_info_common_init(struct irq_info *info,
 	BUG_ON(info->type != IRQT_UNBOUND && info->type != type);
 
 	info->type = type;
+	info->irq = irq;
 	info->evtchn = evtchn;
 	info->cpu = cpu;
 
@@ -311,10 +316,11 @@ static void init_evtchn_cpu_bindings(void)
 {
 	int i;
 #ifdef CONFIG_SMP
-	struct irq_desc *desc;
+	struct irq_info *info;
 
 	/* By default all event channels notify CPU#0. */
-	for_each_irq_desc(i, desc) {
+	list_for_each_entry(info, &xen_irq_list_head, list) {
+		struct irq_desc *desc = irq_to_desc(info->irq);
 		cpumask_copy(desc->irq_data.affinity, cpumask_of(0));
 	}
 #endif
@@ -397,6 +403,21 @@ static void unmask_evtchn(int port)
 	put_cpu();
 }
 
+static void xen_irq_init(unsigned irq)
+{
+	struct irq_info *info;
+	struct irq_desc *desc = irq_to_desc(irq);
+
+	/* By default all event channels notify CPU#0. */
+	cpumask_copy(desc->irq_data.affinity, cpumask_of(0));
+
+	info = &irq_info[irq];
+
+	info->type = IRQT_UNBOUND;
+
+	list_add_tail(&info->list, &xen_irq_list_head);
+}
+
 static int xen_allocate_irq_dynamic(void)
 {
 	int first = 0;
@@ -426,6 +447,8 @@ retry:
 	if (irq < 0)
 		panic("No available IRQ to bind to: increase nr_irqs!\n");
 
+	xen_irq_init(irq);
+
 	return irq;
 }
 
@@ -444,18 +467,25 @@ static int xen_allocate_irq_gsi(unsigned gsi)
 
 	/* Legacy IRQ descriptors are already allocated by the arch. */
 	if (gsi < NR_IRQS_LEGACY)
-		return gsi;
+		irq = gsi;
+	else
+		irq = irq_alloc_desc_at(gsi, -1);
 
-	irq = irq_alloc_desc_at(gsi, -1);
 	if (irq < 0)
 		panic("Unable to allocate to IRQ%d (%d)\n", gsi, irq);
 
+	xen_irq_init(irq);
+
 	return irq;
 }
 
 static void xen_free_irq(unsigned irq)
 {
-	irq_info[irq].type = IRQT_UNBOUND;
+	struct irq_info *info = &irq_info[irq];
+
+	info->type = IRQT_UNBOUND;
+
+	list_del(&info->list);
 
 	/* Legacy IRQ descriptors are managed by the arch. */
 	if (irq < NR_IRQS_LEGACY)
@@ -586,16 +616,14 @@ static void ack_pirq(struct irq_data *data)
 
 static int find_irq_by_gsi(unsigned gsi)
 {
-	int irq;
-
-	for (irq = 0; irq < nr_irqs; irq++) {
-		struct irq_info *info = info_for_irq(irq);
+	struct irq_info *info;
 
-		if (info == NULL || info->type != IRQT_PIRQ)
+	list_for_each_entry(info, &xen_irq_list_head, list) {
+		if (info->type != IRQT_PIRQ)
 			continue;
 
-		if (gsi_from_irq(irq) == gsi)
-			return irq;
+		if (info->u.pirq.gsi == gsi)
+			return info->irq;
 	}
 
 	return -1;
@@ -1374,7 +1402,8 @@ void xen_poll_irq(int irq)
 
 void xen_irq_resume(void)
 {
-	unsigned int cpu, irq, evtchn;
+	unsigned int cpu, evtchn;
+	struct irq_info *info;
 
 	init_evtchn_cpu_bindings();
 
@@ -1383,8 +1412,8 @@ void xen_irq_resume(void)
 		mask_evtchn(evtchn);
 
 	/* No IRQ <-> event-channel mappings. */
-	for (irq = 0; irq < nr_irqs; irq++)
-		irq_info[irq].evtchn = 0; /* zap event-channel binding */
+	list_for_each_entry(info, &xen_irq_list_head, list)
+		info->evtchn = 0; /* zap event-channel binding */
 
 	for (evtchn = 0; evtchn < NR_EVENT_CHANNELS; evtchn++)
 		evtchn_to_irq[evtchn] = -1;
-- 
1.5.6.5

Re: [Xen-devel] [PATCH 10/14] xen: events: maintain a list of Xen interrupts

[Konrad Rzeszutek Wilk]

On Wed, Mar 09, 2011 at 05:41:22PM +0000, Ian Campbell wrote:
> In a PVHVM kernel not all interrupts are Xen interrupts (APIC interrupts can also be present).
> 
> Currently we get away with walking over all interrupts because the
> lookup in the irq_info array simply returns IRQT_UNBOUND and we ignore
> it. However this array will be going away in a future patch so we need
> to manually track which interrupts have been allocated by the Xen
> events infrastructure.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  drivers/xen/events.c |   59 +++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 44 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index cf372d4..e119989 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -56,6 +56,8 @@
>   */
>  static DEFINE_SPINLOCK(irq_mapping_update_lock);
>  
> +static LIST_HEAD(xen_irq_list_head);
> +
>  /* IRQ <-> VIRQ mapping. */
>  static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... NR_VIRQS-1] = -1};
>  
> @@ -85,7 +87,9 @@ enum xen_irq_type {
>   */
>  struct irq_info
>  {
> +	struct list_head list;
>  	enum xen_irq_type type;	/* type */
> +	unsigned irq;
>  	unsigned short evtchn;	/* event channel */
>  	unsigned short cpu;	/* cpu bound */
>  
> @@ -135,6 +139,7 @@ static void xen_irq_info_common_init(struct irq_info *info,
>  	BUG_ON(info->type != IRQT_UNBOUND && info->type != type);
>  
>  	info->type = type;
> +	info->irq = irq;
>  	info->evtchn = evtchn;
>  	info->cpu = cpu;
>  
> @@ -311,10 +316,11 @@ static void init_evtchn_cpu_bindings(void)
>  {
>  	int i;
>  #ifdef CONFIG_SMP
> -	struct irq_desc *desc;
> +	struct irq_info *info;
>  
>  	/* By default all event channels notify CPU#0. */
> -	for_each_irq_desc(i, desc) {
> +	list_for_each_entry(info, &xen_irq_list_head, list) {
> +		struct irq_desc *desc = irq_to_desc(info->irq);
>  		cpumask_copy(desc->irq_data.affinity, cpumask_of(0));
>  	}
>  #endif
> @@ -397,6 +403,21 @@ static void unmask_evtchn(int port)
>  	put_cpu();
>  }
>  
> +static void xen_irq_init(unsigned irq)
> +{
> +	struct irq_info *info;
> +	struct irq_desc *desc = irq_to_desc(irq);
> +
> +	/* By default all event channels notify CPU#0. */
> +	cpumask_copy(desc->irq_data.affinity, cpumask_of(0));
> +
> +	info = &irq_info[irq];
> +
> +	info->type = IRQT_UNBOUND;
> +
> +	list_add_tail(&info->list, &xen_irq_list_head);

Should we use some form of spinlock lock? Just in case
there are two drivers that are being unloaded?

Re: [Xen-devel] [PATCH 10/14] xen: events: maintain a list of Xen interrupts

[Ian Campbell]

On Thu, 2011-03-10 at 05:22 +0000, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 09, 2011 at 05:41:22PM +0000, Ian Campbell wrote:
> > In a PVHVM kernel not all interrupts are Xen interrupts (APIC interrupts can also be present).
> > 
> > Currently we get away with walking over all interrupts because the
> > lookup in the irq_info array simply returns IRQT_UNBOUND and we ignore
> > it. However this array will be going away in a future patch so we need
> > to manually track which interrupts have been allocated by the Xen
> > events infrastructure.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  drivers/xen/events.c |   59 +++++++++++++++++++++++++++++++++++++------------
> >  1 files changed, 44 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > index cf372d4..e119989 100644
> > --- a/drivers/xen/events.c
> > +++ b/drivers/xen/events.c
> > @@ -56,6 +56,8 @@
> >   */
> >  static DEFINE_SPINLOCK(irq_mapping_update_lock);
> >  
> > +static LIST_HEAD(xen_irq_list_head);
> > +
> >  /* IRQ <-> VIRQ mapping. */
> >  static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... NR_VIRQS-1] = -1};
> >  
> > @@ -85,7 +87,9 @@ enum xen_irq_type {
> >   */
> >  struct irq_info
> >  {
> > +	struct list_head list;
> >  	enum xen_irq_type type;	/* type */
> > +	unsigned irq;
> >  	unsigned short evtchn;	/* event channel */
> >  	unsigned short cpu;	/* cpu bound */
> >  
> > @@ -135,6 +139,7 @@ static void xen_irq_info_common_init(struct irq_info *info,
> >  	BUG_ON(info->type != IRQT_UNBOUND && info->type != type);
> >  
> >  	info->type = type;
> > +	info->irq = irq;
> >  	info->evtchn = evtchn;
> >  	info->cpu = cpu;
> >  
> > @@ -311,10 +316,11 @@ static void init_evtchn_cpu_bindings(void)
> >  {
> >  	int i;
> >  #ifdef CONFIG_SMP
> > -	struct irq_desc *desc;
> > +	struct irq_info *info;
> >  
> >  	/* By default all event channels notify CPU#0. */
> > -	for_each_irq_desc(i, desc) {
> > +	list_for_each_entry(info, &xen_irq_list_head, list) {
> > +		struct irq_desc *desc = irq_to_desc(info->irq);
> >  		cpumask_copy(desc->irq_data.affinity, cpumask_of(0));
> >  	}
> >  #endif
> > @@ -397,6 +403,21 @@ static void unmask_evtchn(int port)
> >  	put_cpu();
> >  }
> >  
> > +static void xen_irq_init(unsigned irq)
> > +{
> > +	struct irq_info *info;
> > +	struct irq_desc *desc = irq_to_desc(irq);
> > +
> > +	/* By default all event channels notify CPU#0. */
> > +	cpumask_copy(desc->irq_data.affinity, cpumask_of(0));
> > +
> > +	info = &irq_info[irq];
> > +
> > +	info->type = IRQT_UNBOUND;
> > +
> > +	list_add_tail(&info->list, &xen_irq_list_head);
> 
> Should we use some form of spinlock lock? Just in case
> there are two drivers that are being unloaded?

The callers are xen_allocate_irq_dynamic and xen_allocate_irq_gsi both
of which expect to be protected by irq_mapping_update_lock already.

[PATCH 11/14] xen: events: dynamically allocate irq info structures

[Ian Campbell]

Removes nr_irq sized array allocation at start of day.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |   31 ++++++++++++++++---------------
 1 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index e119989..002283e 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -107,7 +107,6 @@ struct irq_info
 #define PIRQ_NEEDS_EOI	(1 << 0)
 #define PIRQ_SHAREABLE	(1 << 1)
 
-static struct irq_info *irq_info;
 static int *pirq_to_irq;
 
 static int *evtchn_to_irq;
@@ -125,7 +124,7 @@ static struct irq_chip xen_pirq_chip;
 /* Get info for IRQ */
 static struct irq_info *info_for_irq(unsigned irq)
 {
-	return &irq_info[irq];
+	return get_irq_data(irq);
 }
 
 /* Constructors for packed IRQ information. */
@@ -309,7 +308,7 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
 	clear_bit(chn, per_cpu(cpu_evtchn_mask, cpu_from_irq(irq)));
 	set_bit(chn, per_cpu(cpu_evtchn_mask, cpu));
 
-	irq_info[irq].cpu = cpu;
+	info_for_irq(irq)->cpu = cpu;
 }
 
 static void init_evtchn_cpu_bindings(void)
@@ -328,7 +327,6 @@ static void init_evtchn_cpu_bindings(void)
 	for_each_possible_cpu(i)
 		memset(per_cpu(cpu_evtchn_mask, i),
 		       (i == 0) ? ~0 : 0, sizeof(*per_cpu(cpu_evtchn_mask, i)));
-
 }
 
 static inline void clear_evtchn(int port)
@@ -411,10 +409,14 @@ static void xen_irq_init(unsigned irq)
 	/* By default all event channels notify CPU#0. */
 	cpumask_copy(desc->irq_data.affinity, cpumask_of(0));
 
-	info = &irq_info[irq];
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (info == NULL)
+		panic("Unable to allocate metadata for IRQ%d\n", irq);
 
 	info->type = IRQT_UNBOUND;
 
+	set_irq_data(irq, info);
+
 	list_add_tail(&info->list, &xen_irq_list_head);
 }
 
@@ -481,12 +483,14 @@ static int xen_allocate_irq_gsi(unsigned gsi)
 
 static void xen_free_irq(unsigned irq)
 {
-	struct irq_info *info = &irq_info[irq];
-
-	info->type = IRQT_UNBOUND;
+	struct irq_info *info = get_irq_data(irq);
 
 	list_del(&info->list);
 
+	set_irq_data(irq, NULL);
+
+	kfree(info);
+
 	/* Legacy IRQ descriptors are managed by the arch. */
 	if (irq < NR_IRQS_LEGACY)
 		return;
@@ -649,10 +653,9 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
 
 	spin_lock(&irq_mapping_update_lock);
 
-	if ((pirq > nr_irqs) || (gsi > nr_irqs)) {
-		printk(KERN_WARNING "xen_map_pirq_gsi: %s %s is incorrect!\n",
-			pirq > nr_irqs ? "pirq" :"",
-			gsi > nr_irqs ? "gsi" : "");
+	if (pirq > nr_irqs) {
+		printk(KERN_WARNING "xen_map_pirq_gsi: pirq %d > nr_irqs %d!\n",
+		       pirq, nr_irqs);
 		goto out;
 	}
 
@@ -889,7 +892,7 @@ static void unbind_from_irq(unsigned int irq)
 		evtchn_to_irq[evtchn] = -1;
 	}
 
-	BUG_ON(irq_info[irq].type == IRQT_UNBOUND);
+	BUG_ON(info_for_irq(irq)->type == IRQT_UNBOUND);
 
 	xen_free_irq(irq);
 
@@ -1509,8 +1512,6 @@ void __init xen_init_IRQ(void)
 {
 	int i;
 
-	irq_info = kcalloc(nr_irqs, sizeof(*irq_info), GFP_KERNEL);
-
 	/* We are using nr_irqs as the maximum number of pirq available but
 	 * that number is actually chosen by Xen and we don't know exactly
 	 * what it is. Be careful choosing high pirq numbers. */
-- 
1.5.6.5

Re: [PATCH 11/14] xen: events: dynamically allocate irq info structures

[Konrad Rzeszutek Wilk]

> @@ -649,10 +653,9 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
>  
>  	spin_lock(&irq_mapping_update_lock);
>  
> -	if ((pirq > nr_irqs) || (gsi > nr_irqs)) {
> -		printk(KERN_WARNING "xen_map_pirq_gsi: %s %s is incorrect!\n",
> -			pirq > nr_irqs ? "pirq" :"",
> -			gsi > nr_irqs ? "gsi" : "");
> +	if (pirq > nr_irqs) {
> +		printk(KERN_WARNING "xen_map_pirq_gsi: pirq %d > nr_irqs %d!\n",
> +		       pirq, nr_irqs);

Looks like this belongs to another patch?

Re: [PATCH 11/14] xen: events: dynamically allocate irq info structures

[Ian Campbell]

On Thu, 2011-03-10 at 05:27 +0000, Konrad Rzeszutek Wilk wrote:
> > @@ -649,10 +653,9 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
> >  
> >  	spin_lock(&irq_mapping_update_lock);
> >  
> > -	if ((pirq > nr_irqs) || (gsi > nr_irqs)) {
> > -		printk(KERN_WARNING "xen_map_pirq_gsi: %s %s is incorrect!\n",
> > -			pirq > nr_irqs ? "pirq" :"",
> > -			gsi > nr_irqs ? "gsi" : "");
> > +	if (pirq > nr_irqs) {
> > +		printk(KERN_WARNING "xen_map_pirq_gsi: pirq %d > nr_irqs %d!\n",
> > +		       pirq, nr_irqs);
> 
> Looks like this belongs to another patch?

To be honest I'm not entirely sure what that check was protecting
anyway. Possibly it comes from a time when the GSI<->IRQ was 1-1 and
prevented us from spilling off the end of the irq_info array.

It may be that it is safe to have gsi > nr_irqs in an earlier patch in
this series (possibly "xen: events: maintain a list of Xen interrupts")
or even in one of my earlier series which switched to using the core
interrupt allocation logic and/or removed the 1-1 mapping above
nr_irqs_gsi in certain cases.

Anyway, this is the first patch where I'm pretty sure it is safe to
allow GSI > nr_irqs since there are no nr_irqs based limitations left
apart from the pirq one.

Ian.

[PATCH 12/14] xen: events: remove use of nr_irqs as upper bound on number of pirqs

[Ian Campbell]

There isn't really much relationship between the two, other than
nr_irqs often being the larger of the two.

Allows us to remove a nr_irqs sized array, the only users of this
array are MSI setup and restore, neither of which are particularly
performance critical.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |   59 +++++++++++++++++++++----------------------------
 1 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 002283e..b07f5bb 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -107,8 +107,6 @@ struct irq_info
 #define PIRQ_NEEDS_EOI	(1 << 0)
 #define PIRQ_SHAREABLE	(1 << 1)
 
-static int *pirq_to_irq;
-
 static int *evtchn_to_irq;
 
 static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
@@ -196,8 +194,6 @@ static void xen_irq_info_pirq_init(unsigned irq,
 	info->u.pirq.gsi = gsi;
 	info->u.pirq.vector = vector;
 	info->u.pirq.flags = flags;
-
-	pirq_to_irq[pirq] = irq;
 }
 
 /*
@@ -247,16 +243,6 @@ static unsigned pirq_from_irq(unsigned irq)
 	return info->u.pirq.pirq;
 }
 
-static unsigned gsi_from_irq(unsigned irq)
-{
-	struct irq_info *info = info_for_irq(irq);
-
-	BUG_ON(info == NULL);
-	BUG_ON(info->type != IRQT_PIRQ);
-
-	return info->u.pirq.gsi;
-}
-
 static enum xen_irq_type type_from_irq(unsigned irq)
 {
 	return info_for_irq(irq)->type;
@@ -653,12 +639,6 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
 
 	spin_lock(&irq_mapping_update_lock);
 
-	if (pirq > nr_irqs) {
-		printk(KERN_WARNING "xen_map_pirq_gsi: pirq %d > nr_irqs %d!\n",
-		       pirq, nr_irqs);
-		goto out;
-	}
-
 	irq = find_irq_by_gsi(gsi);
 	if (irq != -1) {
 		printk(KERN_INFO "xen_map_pirq_gsi: returning irq %d for gsi %u\n",
@@ -758,7 +738,6 @@ int xen_destroy_irq(int irq)
 			goto out;
 		}
 	}
-	pirq_to_irq[info->u.pirq.pirq] = -1;
 
 	xen_free_irq(irq);
 
@@ -769,7 +748,24 @@ out:
 
 int xen_irq_from_pirq(unsigned pirq)
 {
-	return pirq_to_irq[pirq];
+	int irq;
+
+	struct irq_info *info;
+
+	spin_lock(&irq_mapping_update_lock);
+
+	list_for_each_entry(info, &xen_irq_list_head, list) {
+		if (info == NULL || info->type != IRQT_PIRQ)
+			continue;
+		irq = info->irq;
+		if (info->u.pirq.pirq == pirq)
+			goto out;
+	}
+	irq = -1;
+out:
+	spin_lock(&irq_mapping_update_lock);
+
+	return -1;
 }
 
 int bind_evtchn_to_irq(unsigned int evtchn)
@@ -1269,15 +1265,18 @@ static void restore_pirqs(void)
 {
 	int pirq, rc, irq, gsi;
 	struct physdev_map_pirq map_irq;
+	struct irq_info *info;
 
-	for (pirq = 0; pirq < nr_irqs; pirq++) {
-		irq = pirq_to_irq[pirq];
-		if (irq == -1)
+	list_for_each_entry(info, &xen_irq_list_head, list) {
+		if (info->type != IRQT_PIRQ)
 			continue;
 
+		pirq = info->u.pirq.pirq;
+		gsi = info->u.pirq.gsi;
+		irq = info->irq;
+
 		/* save/restore of PT devices doesn't work, so at this point the
 		 * only devices present are GSI based emulated devices */
-		gsi = gsi_from_irq(irq);
 		if (!gsi)
 			continue;
 
@@ -1291,7 +1290,6 @@ static void restore_pirqs(void)
 			printk(KERN_WARNING "xen map irq failed gsi=%d irq=%d pirq=%d rc=%d\n",
 					gsi, irq, pirq, rc);
 			xen_free_irq(irq);
-			pirq_to_irq[pirq] = -1;
 			continue;
 		}
 
@@ -1512,13 +1510,6 @@ void __init xen_init_IRQ(void)
 {
 	int i;
 
-	/* We are using nr_irqs as the maximum number of pirq available but
-	 * that number is actually chosen by Xen and we don't know exactly
-	 * what it is. Be careful choosing high pirq numbers. */
-	pirq_to_irq = kcalloc(nr_irqs, sizeof(*pirq_to_irq), GFP_KERNEL);
-	for (i = 0; i < nr_irqs; i++)
-		pirq_to_irq[i] = -1;
-
 	evtchn_to_irq = kcalloc(NR_EVENT_CHANNELS, sizeof(*evtchn_to_irq),
 				    GFP_KERNEL);
 	for (i = 0; i < NR_EVENT_CHANNELS; i++)
-- 
1.5.6.5

Re: [PATCH 12/14] xen: events: remove use of nr_irqs as upper bound on number of pirqs

[Konrad Rzeszutek Wilk]

>  int xen_irq_from_pirq(unsigned pirq)
>  {
> -	return pirq_to_irq[pirq];
> +	int irq;
> +
> +	struct irq_info *info;
> +
> +	spin_lock(&irq_mapping_update_lock);
> +
> +	list_for_each_entry(info, &xen_irq_list_head, list) {
> +		if (info == NULL || info->type != IRQT_PIRQ)
> +			continue;
> +		irq = info->irq;
> +		if (info->u.pirq.pirq == pirq)
> +			goto out;
> +	}
> +	irq = -1;
> +out:
> +	spin_lock(&irq_mapping_update_lock);
> +
> +	return -1;

Shouldn't this be:

  return irq

?

How come you are using the spin_lock here, but not
in other places when iterating over the xen_irq_list_head?

Re: [PATCH 12/14] xen: events: remove use of nr_irqs as upper bound on number of pirqs

[Ian Campbell]

On Thu, 2011-03-10 at 05:33 +0000, Konrad Rzeszutek Wilk wrote:
> >  int xen_irq_from_pirq(unsigned pirq)
> >  {
> > -	return pirq_to_irq[pirq];
> > +	int irq;
> > +
> > +	struct irq_info *info;
> > +
> > +	spin_lock(&irq_mapping_update_lock);
> > +
> > +	list_for_each_entry(info, &xen_irq_list_head, list) {
> > +		if (info == NULL || info->type != IRQT_PIRQ)
> > +			continue;
> > +		irq = info->irq;
> > +		if (info->u.pirq.pirq == pirq)
> > +			goto out;
> > +	}
> > +	irq = -1;
> > +out:
> > +	spin_lock(&irq_mapping_update_lock);
> > +
> > +	return -1;
> 
> Shouldn't this be:
> 
>   return irq

Yes. The impact of not doing so is that xen_hvm_setup_msi_irqs would
allocate a fresh PIRQ each time an MSI was reused, instead of resuing
the old one -- i.e. it's a leak. I retested PVHVM PCI hotplug with the
following patch.

> How come you are using the spin_lock here, but not
> in other places when iterating over the xen_irq_list_head?

Those other places already hold the lock in their caller (or are known
to be single threaded -- e.g. resume). Callers of xen_irq_from_pirq do
not hold the lock.

Ian.

8<------------------------

>From 9b1686b874c4893a3b014e400185940dcba43676 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Thu, 10 Mar 2011 08:54:06 +0000
Subject: [PATCH] xen: events: fix return value from xen_irq_from_pirq

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 77c2b43..dc5b575 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -765,7 +765,7 @@ int xen_irq_from_pirq(unsigned pirq)
 out:
 	spin_lock(&irq_mapping_update_lock);
 
-	return -1;
+	return irq;
 }
 
 
-- 
1.5.6.5

[PATCH 13/14] xen: events: do not workaround too-small nr_irqs

[Ian Campbell]

This workaround was somewhat useful prior to the introduction of the
core irq allocator and 026c9d2d0d75 "xen: events: allocate GSIs and
dynamic IRQs from separate IRQ ranges." but should be unnecessary now.

If nr_irqs turns out to be too small under Xen then we should increase
nr_irqs rather than working around the core allocator in this way.

In my configuration NR_IRQS ends up being 2304 with nr_irq_gsi 272
which is sufficient.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
---
 drivers/xen/events.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index b07f5bb..51c6a5b 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -423,15 +423,8 @@ static int xen_allocate_irq_dynamic(void)
 		first = get_nr_irqs_gsi();
 #endif
 
-retry:
 	irq = irq_alloc_desc_from(first, -1);
 
-	if (irq == -ENOMEM && first > NR_IRQS_LEGACY) {
-		printk(KERN_ERR "Out of dynamic IRQ space and eating into GSI space. You should increase nr_irqs\n");
-		first = max(NR_IRQS_LEGACY, first - NR_IRQS_LEGACY);
-		goto retry;
-	}
-
 	if (irq < 0)
 		panic("No available IRQ to bind to: increase nr_irqs!\n");
 
-- 
1.5.6.5

Re: [Xen-devel] [PATCH 13/14] xen: events: do not workaround too-small nr_irqs

[Konrad Rzeszutek Wilk]

On Wed, Mar 09, 2011 at 05:41:25PM +0000, Ian Campbell wrote:
> This workaround was somewhat useful prior to the introduction of the
> core irq allocator and 026c9d2d0d75 "xen: events: allocate GSIs and
> dynamic IRQs from separate IRQ ranges." but should be unnecessary now.

OK, so you tested this under QEMU with Xen + Dom0? A simple one CPU
config was what we had trouble with.
> 
> If nr_irqs turns out to be too small under Xen then we should increase
> nr_irqs rather than working around the core allocator in this way.
> 
> In my configuration NR_IRQS ends up being 2304 with nr_irq_gsi 272
> which is sufficient.

Re: [Xen-devel] [PATCH 13/14] xen: events: do not workaround too-small nr_irqs

[Ian Campbell]

On Thu, 2011-03-10 at 05:40 +0000, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 09, 2011 at 05:41:25PM +0000, Ian Campbell wrote:
> > This workaround was somewhat useful prior to the introduction of the
> > core irq allocator and 026c9d2d0d75 "xen: events: allocate GSIs and
> > dynamic IRQs from separate IRQ ranges." but should be unnecessary now.
> 
> OK, so you tested this under QEMU with Xen + Dom0? A simple one CPU
> config was what we had trouble with.

Yes, I tested with:
qemu-system-x86_64 -m 256 -vnc 0.0.0.0:1 -k en-gb -serial stdio -boot nc \
	-usb -usbdevice tablet \
	-net nic,vlan=0,macaddr=00:16:3e:3f:73:b6,model=e1000 \
	-net tap,vlan=0,ifname=tapQEMU.0 \
	-hda /dev/VG/debian-HVM-1

Booting a recent Xen hypervisor + this kernel.

The comment should really have been updated to reflect the importance of
e7bcecb7b1d2 rather than 026c9d2d0d75. I'll update to:

        With the introduction of e7bcecb7b1d2 "genirq: Make nr_irqs
        runtime expandable" nr_irqs can grow as necessary to accommodate
        our allocation requests.

Ian.

> > 
> > If nr_irqs turns out to be too small under Xen then we should increase
> > nr_irqs rather than working around the core allocator in this way.
> > 
> > In my configuration NR_IRQS ends up being 2304 with nr_irq_gsi 272
> > which is sufficient.

[PATCH 14/14] xen: events: propagate irq allocation failure instead of panicking

[Ian Campbell]

Running out of IRQs need not be fatal to the machine as a whole.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
---
 drivers/xen/events.c |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 51c6a5b..c6f2a2e 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -406,7 +406,7 @@ static void xen_irq_init(unsigned irq)
 	list_add_tail(&info->list, &xen_irq_list_head);
 }
 
-static int xen_allocate_irq_dynamic(void)
+static int __must_check xen_allocate_irq_dynamic(void)
 {
 	int first = 0;
 	int irq;
@@ -425,15 +425,12 @@ static int xen_allocate_irq_dynamic(void)
 
 	irq = irq_alloc_desc_from(first, -1);
 
-	if (irq < 0)
-		panic("No available IRQ to bind to: increase nr_irqs!\n");
-
 	xen_irq_init(irq);
 
 	return irq;
 }
 
-static int xen_allocate_irq_gsi(unsigned gsi)
+static int __must_check xen_allocate_irq_gsi(unsigned gsi)
 {
 	int irq;
 
@@ -452,9 +449,6 @@ static int xen_allocate_irq_gsi(unsigned gsi)
 	else
 		irq = irq_alloc_desc_at(gsi, -1);
 
-	if (irq < 0)
-		panic("Unable to allocate to IRQ%d (%d)\n", gsi, irq);
-
 	xen_irq_init(irq);
 
 	return irq;
@@ -640,6 +634,8 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
 	}
 
 	irq = xen_allocate_irq_gsi(gsi);
+	if (irq < 0)
+		goto out;
 
 	set_irq_chip_and_handler_name(irq, &xen_pirq_chip,
 				      handle_level_irq, name);
@@ -771,6 +767,8 @@ int bind_evtchn_to_irq(unsigned int evtchn)
 
 	if (irq == -1) {
 		irq = xen_allocate_irq_dynamic();
+		if (irq == -1)
+			goto out;
 
 		set_irq_chip_and_handler_name(irq, &xen_dynamic_chip,
 					      handle_fasteoi_irq, "event");
@@ -778,6 +776,7 @@ int bind_evtchn_to_irq(unsigned int evtchn)
 		xen_irq_info_evtchn_init(irq, evtchn);
 	}
 
+out:
 	spin_unlock(&irq_mapping_update_lock);
 
 	return irq;
@@ -829,6 +828,8 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
 
 	if (irq == -1) {
 		irq = xen_allocate_irq_dynamic();
+		if (irq == -1)
+			goto out;
 
 		set_irq_chip_and_handler_name(irq, &xen_percpu_chip,
 					      handle_percpu_irq, "virq");
@@ -845,6 +846,7 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
 		bind_evtchn_to_cpu(evtchn, cpu);
 	}
 
+out:
 	spin_unlock(&irq_mapping_update_lock);
 
 	return irq;
@@ -897,6 +899,8 @@ int bind_evtchn_to_irqhandler(unsigned int evtchn,
 	int retval;
 
 	irq = bind_evtchn_to_irq(evtchn);
+	if (irq < 0)
+		return irq;
 	retval = request_irq(irq, handler, irqflags, devname, dev_id);
 	if (retval != 0) {
 		unbind_from_irq(irq);
@@ -915,6 +919,8 @@ int bind_virq_to_irqhandler(unsigned int virq, unsigned int cpu,
 	int retval;
 
 	irq = bind_virq_to_irq(virq, cpu);
+	if (irq < 0)
+		return irq;
 	retval = request_irq(irq, handler, irqflags, devname, dev_id);
 	if (retval != 0) {
 		unbind_from_irq(irq);
-- 
1.5.6.5

Re: [PATCH 14/14] xen: events: propagate irq allocation failure instead of panicking

[Konrad Rzeszutek Wilk]

On Wed, Mar 09, 2011 at 05:41:26PM +0000, Ian Campbell wrote:
> Running out of IRQs need not be fatal to the machine as a whole.

Do the backends/frontends deal with this appropiately?
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> ---
>  drivers/xen/events.c |   22 ++++++++++++++--------
>  1 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 51c6a5b..c6f2a2e 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -406,7 +406,7 @@ static void xen_irq_init(unsigned irq)
>  	list_add_tail(&info->list, &xen_irq_list_head);
>  }
>  
> -static int xen_allocate_irq_dynamic(void)
> +static int __must_check xen_allocate_irq_dynamic(void)

What is the '__must_check' for?

Re: [PATCH 14/14] xen: events: propagate irq allocation failure instead of panicking

[Ian Campbell]

On Thu, 2011-03-10 at 05:38 +0000, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 09, 2011 at 05:41:26PM +0000, Ian Campbell wrote:
> > Running out of IRQs need not be fatal to the machine as a whole.
> 
> Do the backends/frontends deal with this appropiately?
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> > ---
> >  drivers/xen/events.c |   22 ++++++++++++++--------
> >  1 files changed, 14 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > index 51c6a5b..c6f2a2e 100644
> > --- a/drivers/xen/events.c
> > +++ b/drivers/xen/events.c
> > @@ -406,7 +406,7 @@ static void xen_irq_init(unsigned irq)
> >  	list_add_tail(&info->list, &xen_irq_list_head);
> >  }
> >  
> > -static int xen_allocate_irq_dynamic(void)
> > +static int __must_check xen_allocate_irq_dynamic(void)
> 
> What is the '__must_check' for?

It makes gcc warn if callers don't check the return code. Perhaps
overkill for so few callers but it made it easy to be sure I'd
propagated the error code.

Ian.

[GIT PATCH v2 0/14] xen: events: cleanups + ween off nr_irqs

[Ian Campbell]

Changes since last time:
      * correct return value of xen_irq_from_pirq
      * WARN if a pirq cannot be allocated for a legacy IRQ
      * Updated checking comment of "xen: events: do not workaround
        too-small nr_irqs"

The following series makes a few cleanups to the Xen IRQ infrastructure.
The most important thing is that it removes the need to know about
nr_irqs and in particular the reliance on nr_irqs being static.

Apart from being generally a good thing this is needed because in 2.6.39
nr_irqs will be able to grow dynamically, specifically e7bcecb7b1d2
"genirq: Make nr_irqs runtime expandable" from tip/core/irq is targeted
at 2.6.39.

Dynamically growing nr_irqs also allows us to remove the workaround
which eats into GSI space if a dynamic IRQ cannot be allocated.

There is no ideal sequencing of this series vs e7bcecb7b1d2 (most should
have gone in before, but the penultimate patch really needed to be
simultaneous) so I haven't bothered to try and pull anything from tip
into this branch -- it should all be resolved during the merge window
and bisection won't be too broken since the "eat into GSI space"
workaround only appears to be needed on a small number of older
platforms (qemu being the main exception).

I have tested:
      * Domain 0 on real h/w and under qemu
      * PV guest, including migration and passthrough of both VF and PF.
      * PVHVM guest, including migration and passthrough of both VF and
        PF.

The git pull is a branch on top of konrad/stable/irq.cleanup. However
there is an interaction with konrad/devel/xen-pciback-0.4.driver (the
addition of the domid parameter) so for convenience I have also produced
an irq-pciback branch at the same location which has
konrad/devel/xen-pciback-0.4.driver merged into this branch.

Note that this series obsoletes an older patcho f mine "xen: events:
mark cpu_evtchn_mask_p as __refdata" by virtue of removing the code in
question...

The following changes since commit c5ae07bb307b658c8458f29ca77d237aec0f9327:
  Ian Campbell (1):
        xen: events: remove dom0 specific xen_create_msi_irq

are available in the git repository at:

  git://xenbits.xen.org/people/ianc/linux-2.6.git irq

Ian Campbell (14):
      xen: events: separate two unrelated halves of if condition
      xen: events: fix xen_map_pirq_gsi error return
      xen: events: simplify comment
      xen: events: remove unused public functions
      xen: events: rename restore_cpu_pirqs -> restore_pirqs
      xen: events: refactor GSI pirq bindings functions
      xen: events: use per-cpu variable for cpu_evtchn_mask
      xen: events: turn irq_info constructors into initialiser functions
      xen: events: push setup of irq<->{evtchn,ipi,virq,pirq} maps into irq_info init functions
      xen: events: maintain a list of Xen interrupts
      xen: events: dynamically allocate irq info structures
      xen: events: remove use of nr_irqs as upper bound on number of pirqs
      xen: events: do not workaround too-small nr_irqs
      xen: events: propagate irq allocation failure instead of panicking

 arch/x86/pci/xen.c   |   41 ++++--
 drivers/xen/events.c |  357 ++++++++++++++++++++++++++------------------------
 include/xen/events.h |   24 ++--
 3 files changed, 226 insertions(+), 196 deletions(-)

[PATCH 09/14] xen: events: push setup of irq<->{evtchn,ipi,virq,pirq} maps into irq_info init functions

[Ian Campbell]

Encapsulate setup of XXX_to_irq array in the relevant
xen_irq_info_*_init function.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/events.c |   42 +++++++++++++++++++++---------------------
 1 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 72725fa..cf372d4 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -126,6 +126,7 @@ static struct irq_info *info_for_irq(unsigned irq)
 
 /* Constructors for packed IRQ information. */
 static void xen_irq_info_common_init(struct irq_info *info,
+				     unsigned irq,
 				     enum xen_irq_type type,
 				     unsigned short evtchn,
 				     unsigned short cpu)
@@ -136,6 +137,8 @@ static void xen_irq_info_common_init(struct irq_info *info,
 	info->type = type;
 	info->evtchn = evtchn;
 	info->cpu = cpu;
+
+	evtchn_to_irq[evtchn] = irq;
 }
 
 static void xen_irq_info_evtchn_init(unsigned irq,
@@ -143,29 +146,35 @@ static void xen_irq_info_evtchn_init(unsigned irq,
 {
 	struct irq_info *info = info_for_irq(irq);
 
-	xen_irq_info_common_init(info, IRQT_EVTCHN, evtchn, 0);
+	xen_irq_info_common_init(info, irq, IRQT_EVTCHN, evtchn, 0);
 }
 
-static void xen_irq_info_ipi_init(unsigned irq,
+static void xen_irq_info_ipi_init(unsigned cpu,
+				  unsigned irq,
 				  unsigned short evtchn,
 				  enum ipi_vector ipi)
 {
 	struct irq_info *info = info_for_irq(irq);
 
-	xen_irq_info_common_init(info, IRQT_IPI, evtchn, 0);
+	xen_irq_info_common_init(info, irq, IRQT_IPI, evtchn, 0);
 
 	info->u.ipi = ipi;
+
+	per_cpu(ipi_to_irq, cpu)[ipi] = irq;
 }
 
-static void xen_irq_info_virq_init(unsigned irq,
+static void xen_irq_info_virq_init(unsigned cpu,
+				   unsigned irq,
 				   unsigned short evtchn,
 				   unsigned short virq)
 {
 	struct irq_info *info = info_for_irq(irq);
 
-	xen_irq_info_common_init(info, IRQT_VIRQ, evtchn, 0);
+	xen_irq_info_common_init(info, irq, IRQT_VIRQ, evtchn, 0);
 
 	info->u.virq = virq;
+
+	per_cpu(virq_to_irq, cpu)[virq] = irq;
 }
 
 static void xen_irq_info_pirq_init(unsigned irq,
@@ -177,12 +186,14 @@ static void xen_irq_info_pirq_init(unsigned irq,
 {
 	struct irq_info *info = info_for_irq(irq);
 
-	xen_irq_info_common_init(info, IRQT_PIRQ, evtchn, 0);
+	xen_irq_info_common_init(info, irq, IRQT_PIRQ, evtchn, 0);
 
 	info->u.pirq.pirq = pirq;
 	info->u.pirq.gsi = gsi;
 	info->u.pirq.vector = vector;
 	info->u.pirq.flags = flags;
+
+	pirq_to_irq[pirq] = irq;
 }
 
 /*
@@ -644,7 +655,6 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
 
 	xen_irq_info_pirq_init(irq, 0, pirq, gsi, irq_op.vector,
 			       shareable ? PIRQ_SHAREABLE : 0);
-	pirq_to_irq[pirq] = irq;
 
 out:
 	spin_unlock(&irq_mapping_update_lock);
@@ -682,7 +692,6 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
 				      handle_level_irq, name);
 
 	xen_irq_info_pirq_init(irq, 0, pirq, 0, vector, 0);
-	pirq_to_irq[pirq] = irq;
 	ret = set_irq_msi(irq, msidesc);
 	if (ret < 0)
 		goto error_irq;
@@ -746,7 +755,6 @@ int bind_evtchn_to_irq(unsigned int evtchn)
 		set_irq_chip_and_handler_name(irq, &xen_dynamic_chip,
 					      handle_fasteoi_irq, "event");
 
-		evtchn_to_irq[evtchn] = irq;
 		xen_irq_info_evtchn_init(irq, evtchn);
 	}
 
@@ -779,9 +787,7 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
 			BUG();
 		evtchn = bind_ipi.port;
 
-		evtchn_to_irq[evtchn] = irq;
-		xen_irq_info_ipi_init(irq, evtchn, ipi);
-		per_cpu(ipi_to_irq, cpu)[ipi] = irq;
+		xen_irq_info_ipi_init(cpu, irq, evtchn, ipi);
 
 		bind_evtchn_to_cpu(evtchn, cpu);
 	}
@@ -814,10 +820,7 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
 			BUG();
 		evtchn = bind_virq.port;
 
-		evtchn_to_irq[evtchn] = irq;
-		xen_irq_info_virq_init(irq, evtchn, virq);
-
-		per_cpu(virq_to_irq, cpu)[virq] = irq;
+		xen_irq_info_virq_init(cpu, irq, evtchn, virq);
 
 		bind_evtchn_to_cpu(evtchn, cpu);
 	}
@@ -1120,7 +1123,6 @@ void rebind_evtchn_irq(int evtchn, int irq)
 	   so there should be a proper type */
 	BUG_ON(info->type == IRQT_UNBOUND);
 
-	evtchn_to_irq[evtchn] = irq;
 	xen_irq_info_evtchn_init(irq, evtchn);
 
 	spin_unlock(&irq_mapping_update_lock);
@@ -1288,8 +1290,7 @@ static void restore_cpu_virqs(unsigned int cpu)
 		evtchn = bind_virq.port;
 
 		/* Record the new mapping. */
-		evtchn_to_irq[evtchn] = irq;
-		xen_irq_info_virq_init(irq, evtchn, virq);
+		xen_irq_info_virq_init(cpu, irq, evtchn, virq);
 		bind_evtchn_to_cpu(evtchn, cpu);
 	}
 }
@@ -1313,8 +1314,7 @@ static void restore_cpu_ipis(unsigned int cpu)
 		evtchn = bind_ipi.port;
 
 		/* Record the new mapping. */
-		evtchn_to_irq[evtchn] = irq;
-		xen_irq_info_ipi_init(irq, evtchn, ipi);
+		xen_irq_info_ipi_init(cpu, irq, evtchn, ipi);
 		bind_evtchn_to_cpu(evtchn, cpu);
 	}
 }
-- 
1.5.6.5