arm: SMP boot

arm: implement udelay()

Signed-off-by Tim Deegan <[email protected]>

diff -r 4a7c14209131 -r ec051056db2b xen/arch/arm/time.c

xen/arch/arm/time.c +3 -0

+    } while ( get_s_time() - deadline < 0 );
+}
+
 /*
  * Local variables:
  * mode: C

Quoted 4 lines · Tim Deegan

arm: implement udelay()

Signed-off-by Tim Deegan <[email protected]>

[...]

Quoted 7 lines

diff -r 4a7c14209131 -r ec051056db2b xen/arch/arm/time.c

xen/arch/arm/time.c +0 -0

@@ -171,6 +171,16 @@ void __cpuinit init_timer_interrupt(void
     request_irq(30, timer_interrupt, 0, "phytimer", NULL);
 }
 

xen/arch/arm/time.c void __cpuinit init_timer_interrupt(void · line 180

Quoted 7 lines

xen/arch/arm/time.c +7 -0

+/* Wait a set number of microseconds */
+void udelay(unsigned long usecs)
+{
+    s_time_t deadline = get_s_time() + 1000 * (s_time_t) usecs;
+    do {
+        dsb();
+        isb();

What are these barriers for?

# HG changeset patch
# User Tim Deegan <[email protected]>
# Date 1330018799 0
# Node ID 437ad1207a175c9ad376871f3f4c075dbcd5b6e6
# Parent ec051056db2b6d37344629e2f01d17240099d5ec
arm: Move some GIC distributor init out of the per-CPU init function

Signed-off-by Tim Deegan <[email protected]>

diff -r ec051056db2b -r 437ad1207a17 xen/arch/arm/gic.c

xen/arch/arm/gic.c +0 -1

@@ -216,14 +216,6 @@ static void __init gic_dist_init(void)
     for ( i = 32; i < gic.lines; i += 32 )
         GICD[GICD_ICENABLER + i / 32] = ~0ul;
 
-    /* Turn on the distributor */

xen/arch/arm/gic.c +6 -0

@@ -231,6 +223,12 @@ static void __cpuinit gic_cpu_init(void)
     for (i = 0; i < 32; i += 4)
         GICD[GICD_IPRIORITYR + i / 4] = 0xa0a0a0a0;
 
+    /* Turn on the distributor */
+    GICD[GICD_CTLR] = GICD_CTL_ENABLE;
+}
+
+static void __cpuinit gic_cpu_init(void)
+{
     /* Local settings: interface controller */
     GICC[GICC_PMR] = 0xff;                /* Don't mask by priority */
     GICC[GICC_BPR] = 0;                   /* Finest granularity of priority */

Quoted 16 lines · Tim Deegan

# HG changeset patch
# User Tim Deegan <[email protected]>
# Date 1330018799 0
# Node ID 437ad1207a175c9ad376871f3f4c075dbcd5b6e6
# Parent ec051056db2b6d37344629e2f01d17240099d5ec
arm: Move some GIC distributor init out of the per-CPU init function

Signed-off-by Tim Deegan <[email protected]>

diff -r ec051056db2b -r 437ad1207a17 xen/arch/arm/gic.c

xen/arch/arm/gic.c +0 -0

@@ -216,14 +216,6 @@ static void __init gic_dist_init(void)
     for ( i = 32; i < gic.lines; i += 32 )
         GICD[GICD_ICENABLER + i / 32] = ~0ul;
 

xen/arch/arm/gic.c static void __init gic_dist_init(void) · line 221

Quoted 11 lines

xen/arch/arm/gic.c +0 -8

-    /* Turn on the distributor */
-    GICD[GICD_CTLR] = GICD_CTL_ENABLE;
-}
-
-static void __cpuinit gic_cpu_init(void)
-{
-    int i;
-
     /* Disable all PPI and enable all SGI */
     GICD[GICD_ICENABLER] = 0xffff0000; /* Disable all PPI */
     GICD[GICD_ISENABLER] = 0x0000ffff; /* Enable all SGI */

PPIs and SGIs are per physical-CPU and therefore, I think, the GICD registers of various sorts which refer to the first 32 interrupts are per physical-CPU as well. IOW moving these from gic_cpu_init to gic_dist_init is wrong?

Quoted 17 lines · Ian Campbell

Quoted 16 lines · Tim Deegan

# HG changeset patch
# User Tim Deegan <[email protected]>
# Date 1330018799 0
# Node ID 437ad1207a175c9ad376871f3f4c075dbcd5b6e6
# Parent ec051056db2b6d37344629e2f01d17240099d5ec
arm: Move some GIC distributor init out of the per-CPU init function

Signed-off-by Tim Deegan <[email protected]>

diff -r ec051056db2b -r 437ad1207a17 xen/arch/arm/gic.c

xen/arch/arm/gic.c +0 -0

@@ -216,14 +216,6 @@ static void __init gic_dist_init(void)
     for ( i = 32; i < gic.lines; i += 32 )
         GICD[GICD_ICENABLER + i / 32] = ~0ul;
 

Quoted 16 lines

xen/arch/arm/gic.c static void __init gic_dist_init(void) · line 221

Quoted 11 lines

xen/arch/arm/gic.c +0 -8

-    /* Turn on the distributor */
-    GICD[GICD_CTLR] = GICD_CTL_ENABLE;
-}
-
-static void __cpuinit gic_cpu_init(void)
-{
-    int i;
-
     /* Disable all PPI and enable all SGI */
     GICD[GICD_ICENABLER] = 0xffff0000; /* Disable all PPI */
     GICD[GICD_ISENABLER] = 0x0000ffff; /* Enable all SGI */

PPIs and SGIs are per physical-CPU and therefore, I think, the GICD registers of various sorts which refer to the first 32 interrupts are per physical-CPU as well. IOW moving these from gic_cpu_init to gic_dist_init is wrong?

Yep, you're right ISENABLER0 and ICENABLER0 are banked.

I'll replace this with a comment explaining that.

On Thu, 2012-02-23 at 17:40 +0000, Tim Deegan wrote:
[...]

xen/arch/arm/head.S paging: · line 280

Quoted 8 lines

xen/arch/arm/head.S +8 -0

+	/* Signal the next non-boot CPU to come and join us here */
+	ldr   r0, =boot_gate         /* VA of gate */
+	add   r0, r0, r10            /* PA of gate */
+	mov   r1, #0                 /* (0 == unlocked) */
+	str   r1, [r0]
+	dsb
+	isb
+	sev

Here we have released the next CPU from the holding pen...

[...]

xen/arch/arm/head.S paging: · line 300

Quoted 6 lines

xen/arch/arm/head.S +6 -0

+	/* Non-boot CPUs report that they've got this far */
+	ldr   r0, =ready_cpus
+	ldr   r1, [r0]               /* Read count of ready CPUs */
+	add   r1, r1, #1             /* ++ */
+	str   r1, [r0]               /* Writeback */
+	dsb

... and here we do a non-atomic update of a shared variable.

What prevents the following CPU from catching us up and conflicting here?

Would we be better signalling the next CPU after the increment instead?

# HG changeset patch
# User Tim Deegan <[email protected]>
# Date 1330018799 0
# Node ID 3b1d7a91a2596baca178e8b5610b3cbc299fa5ea # Parent 1e9c6bd7cc99d1af0107aa927ee2ba03721449b7
arm: per-cpu areas

Signed-off-by Tim Deegan <[email protected]>

diff -r 1e9c6bd7cc99 -r 3b1d7a91a259 xen/arch/arm/Makefile

xen/arch/arm/Makefile +1 -0

@@ -13,6 +13,7 @@ obj-y += irq.o
 obj-y += kernel.o
 obj-y += mm.o
 obj-y += p2m.o
+obj-y += percpu.o
 obj-y += guestcopy.o
 obj-y += setup.o
 obj-y += time.o

diff -r 1e9c6bd7cc99 -r 3b1d7a91a259 xen/arch/arm/dummy.S

xen/arch/arm/dummy.S +0 -1

@@ -14,7 +14,6 @@ DUMMY(per_cpu__cpu_core_mask);
 DUMMY(per_cpu__cpu_sibling_mask);
 DUMMY(node_online_map);
 DUMMY(smp_send_state_dump);
-DUMMY(__per_cpu_offset);
 
 /* PIRQ support */
 DUMMY(alloc_pirq_struct);

diff -r 1e9c6bd7cc99 -r 3b1d7a91a259 xen/arch/arm/percpu.c

xen/arch/arm/percpu.c new file +85 -0

@@ -0,0 +1,85 @@
+#include <xen/config.h>
+#include <xen/percpu.h>
+#include <xen/cpu.h>
+#include <xen/init.h>
+#include <xen/mm.h>
+#include <xen/rcupdate.h>
+
+unsigned long __per_cpu_offset[NR_CPUS];
+#define INVALID_PERCPU_AREA (-(long)__per_cpu_start)
+#define PERCPU_ORDER (get_order_from_bytes(__per_cpu_data_end-__per_cpu_start))
+
+void __init percpu_init_areas(void)
+{
+    unsigned int cpu;
+    for ( cpu = 1; cpu < NR_CPUS; cpu++ )
+        __per_cpu_offset[cpu] = INVALID_PERCPU_AREA;
+}
+
+static int init_percpu_area(unsigned int cpu)
+{
+    char *p;
+    if ( __per_cpu_offset[cpu] != INVALID_PERCPU_AREA )
+        return -EBUSY;
+    if ( (p = alloc_xenheap_pages(PERCPU_ORDER, 0)) == NULL )
+        return -ENOMEM;
+    memset(p, 0, __per_cpu_data_end - __per_cpu_start);
+    __per_cpu_offset[cpu] = p - __per_cpu_start;
+    return 0;
+}
+
+struct free_info {
+    unsigned int cpu;
+    struct rcu_head rcu;
+};
+static DEFINE_PER_CPU(struct free_info, free_info);
+
+static void _free_percpu_area(struct rcu_head *head)
+{
+    struct free_info *info = container_of(head, struct free_info, rcu);
+    unsigned int cpu = info->cpu;
+    char *p = __per_cpu_start + __per_cpu_offset[cpu];
+    free_xenheap_pages(p, PERCPU_ORDER);
+    __per_cpu_offset[cpu] = INVALID_PERCPU_AREA;
+}
+
+static void free_percpu_area(unsigned int cpu)
+{
+    struct free_info *info = &per_cpu(free_info, cpu);
+    info->cpu = cpu;
+    call_rcu(&info->rcu, _free_percpu_area);
+}
+
+static int cpu_percpu_callback(
+    struct notifier_block *nfb, unsigned long action, void *hcpu)
+{
+    unsigned int cpu = (unsigned long)hcpu;
+    int rc = 0;
+
+    switch ( action )
+    {
+    case CPU_UP_PREPARE:
+        rc = init_percpu_area(cpu);
+        break;
+    case CPU_UP_CANCELED:
+    case CPU_DEAD:
+        free_percpu_area(cpu);
+        break;
+    default:
+        break;
+    }
+
+    return !rc ? NOTIFY_DONE : notifier_from_errno(rc);
+}
+
+static struct notifier_block cpu_percpu_nfb = {
+    .notifier_call = cpu_percpu_callback,
+    .priority = 100 /* highest priority */
+};
+
+static int __init percpu_presmp_init(void)
+{
+    register_cpu_notifier(&cpu_percpu_nfb);
+    return 0;
+}
+presmp_initcall(percpu_presmp_init);

diff -r 1e9c6bd7cc99 -r 3b1d7a91a259 xen/include/asm-arm/percpu.h

xen/include/asm-arm/percpu.h +5 -2

@@ -12,8 +12,11 @@ void percpu_init_areas(void);
     __attribute__((__section__(".bss.percpu" #suffix)))         \
     __typeof__(type) per_cpu_##name
 
-#define per_cpu(var, cpu) ((&per_cpu__##var)[cpu?0:0])
-#define __get_cpu_var(var) per_cpu__##var
+
+#define per_cpu(var, cpu)  \
+    (*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset[cpu]))
+#define __get_cpu_var(var) \
+    (*RELOC_HIDE(&per_cpu__##var, get_cpu_info()->per_cpu_offset))
 
 #define DECLARE_PER_CPU(type, name) extern __typeof__(type) per_cpu__##name
 

# HG changeset patch
# User Tim Deegan <[email protected]>
# Date 1330018799 0
# Node ID fd78d23ba19de4129e21cdc7303848b105057227 # Parent 3b1d7a91a2596baca178e8b5610b3cbc299fa5ea
arm: start plumbing in SMP bringup in C

Still a noop, but no longer just a dummy symbol.

Signed-off-by Tim Deegan <[email protected]>

diff -r 3b1d7a91a259 -r fd78d23ba19d xen/arch/arm/dummy.S

xen/arch/arm/dummy.S +0 -3

@@ -7,9 +7,6 @@ x:	.word 0xe7f000f0 /* Undefined instruc
 x:	mov pc, lr
 
 /* SMP support */
-DUMMY(__cpu_die);
-DUMMY(__cpu_disable);
-DUMMY(__cpu_up);
 DUMMY(per_cpu__cpu_core_mask);
 DUMMY(per_cpu__cpu_sibling_mask);
 DUMMY(node_online_map);

diff -r 3b1d7a91a259 -r fd78d23ba19d xen/arch/arm/setup.c

xen/arch/arm/setup.c +2 -5

@@ -38,9 +38,6 @@
 #include <asm/setup.h>
 #include "gic.h"
 
-/* maxcpus: maximum number of CPUs to activate. */
-static unsigned int __initdata max_cpus = NR_CPUS;
-
 /* Xen stack for bringing up the first CPU. */
 unsigned char __initdata init_stack[STACK_SIZE] __attribute__((__aligned__(STACK_SIZE)));
 

@@ -206,7 +203,7 @@ void __init start_xen(unsigned long boot
     cpumask_set_cpu(smp_processor_id(), &cpu_online_map);
     cpumask_set_cpu(smp_processor_id(), &cpu_present_map);
 
-    smp_prepare_cpus(max_cpus);
+    smp_prepare_cpus(cpus);
 
     init_xen_time();
 

@@ -253,7 +250,7 @@ void __init start_xen(unsigned long boot
 
     for_each_present_cpu ( i )
     {
-        if ( (num_online_cpus() < max_cpus) && !cpu_online(i) )
+        if ( (num_online_cpus() < cpus) && !cpu_online(i) )
         {
             int ret = cpu_up(i);
             if ( ret != 0 )

diff -r 3b1d7a91a259 -r fd78d23ba19d xen/arch/arm/smpboot.c

xen/arch/arm/smpboot.c +33 -8

@@ -19,6 +19,7 @@
 #include <xen/cpumask.h>
 #include <xen/smp.h>
 #include <xen/init.h>
+#include <xen/errno.h>
 
 cpumask_t cpu_online_map;
 EXPORT_SYMBOL(cpu_online_map);

@@ -30,16 +31,40 @@ EXPORT_SYMBOL(cpu_possible_map);
 void __init
 smp_prepare_cpus (unsigned int max_cpus)
 {
-        set_processor_id(0); /* needed early, for smp_processor_id() */
+    int i;
+    set_processor_id(0); /* needed early, for smp_processor_id() */
 
-        cpumask_clear(&cpu_online_map);
-        cpumask_clear(&cpu_present_map);
-        cpumask_clear(&cpu_possible_map);
-        cpumask_set_cpu(0, &cpu_online_map);
-        cpumask_set_cpu(0, &cpu_present_map);
-        cpumask_set_cpu(0, &cpu_possible_map);
-        return;
+    cpumask_clear(&cpu_online_map);
+    cpumask_set_cpu(0, &cpu_online_map);
+
+    cpumask_clear(&cpu_possible_map);
+    for ( i = 0; i < max_cpus; i++ )
+        cpumask_set_cpu(i, &cpu_possible_map);
+    cpumask_copy(&cpu_present_map, &cpu_possible_map);
 }
+
+/* Bring up a non-boot CPU */
+int __cpu_up(unsigned int cpu)
+{
+    /* Not yet... */
+    return -ENODEV;
+}
+
+/* Shut down the current CPU */
+void __cpu_disable(void)
+{
+    /* TODO: take down timers, GIC, &c. */
+    BUG();
+}
+
+/* Wait for a remote CPU to die */
+void __cpu_die(unsigned int cpu)
+{
+    /* TODO: interlock with __cpu_disable */
+    BUG();
+}
+
+
 /*
  * Local variables:
  * mode: C

# HG changeset patch
# User Tim Deegan <[email protected]>
# Date 1330018799 0
# Node ID d35b52e5fde829dfbaf3da73e0716d004faded2f # Parent fd78d23ba19de4129e21cdc7303848b105057227
arm: Boot secondary CPUs into C

Signed-off-by Tim Deegan <[email protected]>

# HG changeset patch
# User Tim Deegan <[email protected]>
# Date 1330018799 0
# Node ID a78bc9b8421492e0545c6d52c7a32b9de9737d61 # Parent d35b52e5fde829dfbaf3da73e0716d004faded2f
arm: SMP CPU shutdown

For completeness, also implelent the CPU shutdown path.

Signed-off-by TIm Deegan <[email protected]>

Quoted 8 lines · Tim Deegan

# HG changeset patch
# User Tim Deegan <[email protected]>
# Date 1330018799 0
# Node ID a78bc9b8421492e0545c6d52c7a32b9de9737d61 # Parent d35b52e5fde829dfbaf3da73e0716d004faded2f
arm: SMP CPU shutdown

For completeness, also implelent the CPU shutdown path.

Does something free the init_stack for a CPU as it goes down?

Alternatively the bring up path could reuse it if the CPU came back but we don't seem to do that either.

Quoted 1 line

Signed-off-by TIm Deegan <[email protected]>

Typo here.

[...]

xen/arch/arm/smpboot.c void __cpuinit start_secondary(unsigned · line 119

Quoted 8 lines

xen/arch/arm/smpboot.c +8 -0

+    /* It's now safe to remove this processor from the online map */
+    cpumask_clear_cpu(cpu, &cpu_online_map);
+
+    if ( cpu_disable_scheduler(cpu) )
+        BUG();
+    mb();
+
+    /* Return to caller; eventually the IPI mecahnism will unwind and the 

mechanism

xen/arch/arm/smpboot.c void __cpuinit start_secondary(unsigned · line 126

Quoted 7 lines

xen/arch/arm/smpboot.c +7 -0

+     * scheduler will drop to the idle loop, which will call stop_cpu(). */
+}
+
+void stop_cpu(void)
+{
+    local_irq_disable();
+    cpu_is_dead = 1;

Do we lock this variable? What stops multiple CPUs coming down at once?

# HG changeset patch
# User Tim Deegan <[email protected]>
# Date 1330018799 0
# Node ID 8a2d38ab67ccaf8637e223feb0d0678433974e93 # Parent a78bc9b8421492e0545c6d52c7a32b9de9737d61
arm: Shutdown and reboot

Reboot runes grabbed from linux's SP810 reset function. Doesn't seem to work on the model, though.

Signed-off-by Tim Deegan <[email protected]>

diff -r a78bc9b84214 -r 8a2d38ab67cc xen/arch/arm/shutdown.c

xen/arch/arm/shutdown.c +50 -5

@@ -1,18 +1,63 @@
 #include <xen/config.h>
+#include <xen/console.h>
+#include <xen/cpu.h>
+#include <xen/delay.h>
 #include <xen/lib.h>
+#include <xen/mm.h>
+#include <xen/smp.h>
+
+static void raw_machine_reset(void)
+{
+#ifdef SP810_ADDRESS
+    /* Use the SP810 system controller to force a reset */
+    volatile uint32_t *sp810;
+    set_fixmap(FIXMAP_MISC, SP810_ADDRESS >> PAGE_SHIFT, DEV_SHARED);
+    sp810 = ((uint32_t *)
+             (FIXMAP_ADDR(FIXMAP_MISC) + (SP810_ADDRESS & ~PAGE_MASK)));
+    sp810[0] = 0x3; /* switch to slow mode */
+    dsb(); isb();
+    sp810[1] = 0x1; /* writing any value to SCSYSSTAT reg will reset system */
+    dsb(); isb();
+    clear_fixmap(FIXMAP_MISC);
+#endif
+}
+
+static void halt_this_cpu(void *arg)
+{
+    __cpu_disable();
+    stop_cpu();
+}
 
 void machine_halt(void)
 {
-        /* TODO: halt */
-        while(1) ;
+    watchdog_disable();
+    console_start_sync();
+    local_irq_enable();
+    smp_call_function(halt_this_cpu, NULL, 0);
+    halt_this_cpu(NULL);
 }
 
 void machine_restart(unsigned int delay_millisecs)
 {
-        /* TODO: restart */
-        printk("Cannot restart yet\n");
-        while(1);
+    int timeout = 10;
+
+    local_irq_enable();
+    smp_call_function(halt_this_cpu, NULL, 0);
+    local_irq_disable();
+
+    mdelay(delay_millisecs);
+
+    /* Wait at most another 10ms for all other CPUs to go offline. */
+    while ( (num_online_cpus() > 1) && (timeout-- > 0) )
+        mdelay(1);
+
+    while ( 1 )
+    {
+        raw_machine_reset();
+        mdelay(100);
+    }
 }
+
 /*
  * Local variables:
  * mode: C

diff -r a78bc9b84214 -r 8a2d38ab67cc xen/include/asm-arm/config.h

xen/include/asm-arm/config.h +3 -0

@@ -119,6 +119,9 @@ extern unsigned long frametable_virt_end
 #define GIC_CR_OFFSET 0x2000
 #define GIC_HR_OFFSET 0x4000 /* Guess work http://lists.infradead.org/pipermail/linux-arm-kernel/2011-September/064219.html */
 #define GIC_VR_OFFSET 0x6000 /* Virtual Machine CPU interface) */
+/* Board-specific: base address of system controller */
+#define SP810_ADDRESS 0x1C020000
+
 
 #endif /* __ARM_CONFIG_H__ */
 /*

This patch series implements SMP boot for arch/arm, as far as getting all CPUs up and running the idle loop.

The first few patches are semi-independent cleanups and improvements, most of which are needed for the SMP patches later.

This patch series implements SMP boot for arch/arm, as far as getting all CPUs up and running the idle loop.

Quoted 2 lines · Tim Deegan

This patch series implements SMP boot for arch/arm, as far as getting all CPUs up and running the idle loop.

What additional steps are needed when building the model to get an SMP system?

Ideally this would eventually be documented in
http://wiki.xen.org/wiki/Xen_ARMv7_with_Virtualization_Extensions/FastModels but it is probably a bit premature for that right now.

I think it is sufficient to do the following after opening the project ("sgcanvas <SGPROJ>") and before clicking the "Build" button.

* Right click the header of the "cluster" in the diagram (the top bit of the box which says "cluster" in it)
* Select "Object Properties" from the context menu
* In the "Parameters" tab update the "num_cores" variable, e.g. to "0x4"
* Click build and continue as usual.