[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen/arm: irq: Initialize the per-CPU IRQs while preparing the CPU



Hi Stefano,

On 15/06/2022 01:32, Stefano Stabellini wrote:
On Tue, 14 Jun 2022, Julien Grall wrote:
From: Julien Grall <jgrall@xxxxxxxxxx>

Commit 5047cd1d5dea "xen/common: Use enhanced ASSERT_ALLOC_CONTEXT in
xmalloc()" extended the checks in _xmalloc() to catch any use of the
helpers from context with interrupts disabled.

Unfortunately, the rule is not followed when initializing the per-CPU
IRQs:

(XEN) Xen call trace:
(XEN)    [<002389f4>] _xmalloc+0xfc/0x314 (PC)
(XEN)    [<00000000>] 00000000 (LR)
(XEN)    [<0021a7c4>] init_one_irq_desc+0x48/0xd0
(XEN)    [<002807a8>] irq.c#init_local_irq_data+0x48/0xa4
(XEN)    [<00280834>] init_secondary_IRQ+0x10/0x2c
(XEN)    [<00288fa4>] start_secondary+0x194/0x274
(XEN)    [<40010170>] 40010170
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 2:
(XEN) Assertion '!in_irq() && (local_irq_is_enabled() || num_online_cpus() <= 
1)' failed at common/xmalloc_tlsf.c:601
(XEN) ****************************************

This is happening because zalloc_cpumask_var() may allocate memory
if NR_CPUS is > 2 * sizeof(unsigned long).

Avoid the problem by allocate the per-CPU IRQs while preparing the
CPU.

This also has the benefit to remove a BUG_ON() in the secondary CPU
code.

Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
---
  xen/arch/arm/include/asm/irq.h |  1 -
  xen/arch/arm/irq.c             | 35 +++++++++++++++++++++++++++-------
  xen/arch/arm/smpboot.c         |  2 --
  3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/include/asm/irq.h b/xen/arch/arm/include/asm/irq.h
index e45d57459899..245f49dcbac5 100644
--- a/xen/arch/arm/include/asm/irq.h
+++ b/xen/arch/arm/include/asm/irq.h
@@ -73,7 +73,6 @@ static inline bool is_lpi(unsigned int irq)
  bool is_assignable_irq(unsigned int irq);
void init_IRQ(void);
-void init_secondary_IRQ(void);
int route_irq_to_guest(struct domain *d, unsigned int virq,
                         unsigned int irq, const char *devname);
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index b761d90c4063..56bdcb95335d 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -17,6 +17,7 @@
   * GNU General Public License for more details.
   */
+#include <xen/cpu.h>
  #include <xen/lib.h>
  #include <xen/spinlock.h>
  #include <xen/irq.h>
@@ -100,7 +101,7 @@ static int __init init_irq_data(void)
      return 0;
  }
-static int init_local_irq_data(void)
+static int init_local_irq_data(unsigned int cpu)
  {
      int irq;
@@ -108,7 +109,7 @@ static int init_local_irq_data(void) for ( irq = 0; irq < NR_LOCAL_IRQS; irq++ )
      {
-        struct irq_desc *desc = irq_to_desc(irq);
+        struct irq_desc *desc = &per_cpu(local_irq_desc, cpu)[irq];
          int rc = init_one_irq_desc(desc);
if ( rc )
@@ -131,6 +132,29 @@ static int init_local_irq_data(void)
      return 0;
  }
+static int cpu_callback(struct notifier_block *nfb, unsigned long action,
+                        void *hcpu)
+{
+    unsigned long cpu = (unsigned long)hcpu;

unsigned int cpu ?

Hmmm... We seem to have a mix in the code base. I am OK to switch to unsigned int.


The rest looks good
Can this be converted to an ack or review tag?

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.