x86: fixes early core_id() calls
authorBarret Rhoden <brho@cs.berkeley.edu>
Wed, 17 Jul 2013 21:21:28 +0000 (14:21 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 19 Jul 2013 23:56:06 +0000 (16:56 -0700)
Ever since we set up minimal paging in assembly, we were unable to access the
LAPIC (and therefore coreid) until the LAPIC was mapped in during vm_init().
This prevented the newer panic and warn from working, and totally killed
spinlock debugging.

Previously, we had a full "virtual" KERNBASE mapping (in 32 bit) with
segmentation.  I think those attempted accesses of the unmapped LAPIC were just
silentely returning 0, which is the 'right' answer at that stage of bootup.
Either way, now that paging is on, unmapped LAPIC accesses would page fault and
trigger a reboot since the IDT isn't set up yet either.

Any callers of core_id() that can happen before vm_init() need to use
core_id_early().  I'm not overly concerned about code reachable via the
monitor, since if you got there, it's by a panic and we should already be
working on that problem.  If this is too much of a burden, we can just put the
bool check in core_id(), and take that potential cache miss on every call.

kern/arch/riscv/arch.h
kern/arch/x86/apic.c
kern/arch/x86/apic.h
kern/arch/x86/arch.h
kern/arch/x86/pmap32.c
kern/arch/x86/pmap64.c
kern/src/atomic.c
kern/src/init.c
kern/src/monitor.c

index 3d257ab..cd0aee7 100644 (file)
@@ -142,6 +142,11 @@ static __inline int core_id(void)
        return get_os_coreid(hw_core_id());
 }
 
+static __inline int core_id_early(void)
+{
+       return core_id();
+}
+
 static __inline void cache_flush(void)
 {
 }
index 5a9d83e..e71d74a 100644 (file)
@@ -19,6 +19,7 @@
 #include <bitmask.h>
 
 system_timing_t RO system_timing = {0, 0, 0xffff, 0};
+bool core_id_ready = FALSE;
 
 /* * Remaps the Programmable Interrupt Controller to use IRQs 32-47
  * http://wiki.osdev.org/PIC
index 9bd89fb..0c46a72 100644 (file)
@@ -123,6 +123,21 @@ typedef struct system_timing {
 
 extern system_timing_t system_timing;
 
+/* Tracks whether it is safe to execute core_id() or not.  If we're using the
+ * LAPIC, we need to have the LAPIC mapped into VM.  vm_init() sets this to
+ * TRUE.
+ *
+ * If we're using rdtscp, if the instruction is supported, we can call core_id()
+ * without rebooting.  cpuinfo should have panic'd if we're running on a machine
+ * that doesn't support rdtscp, before vm_init().
+ *
+ * If we're using something else (like segmentation), then that will need to get
+ * set up before vm_init(), at least for core 0.
+ *
+ * Note that core_id() will return 0 (or possibly another wrong answer) on cores
+ * other than core 0 when it is called before smp_boot completes. */
+extern bool core_id_ready;
+
 void pic_remap(void);
 void pic_mask_irq(uint8_t irq);
 void pic_unmask_irq(uint8_t irq);
index 3252d1b..07045f6 100644 (file)
@@ -30,6 +30,7 @@ static inline int get_hw_coreid(uint32_t coreid) __attribute__((always_inline));
 static inline int hw_core_id(void) __attribute__((always_inline));
 static inline int get_os_coreid(int hw_coreid) __attribute__((always_inline));
 static inline int core_id(void) __attribute__((always_inline));
+static inline int core_id_early(void) __attribute__((always_inline));
 static inline void cache_flush(void) __attribute__((always_inline));
 static inline void reboot(void)
               __attribute__((always_inline)) __attribute__((noreturn));
@@ -172,6 +173,13 @@ static inline int core_id(void)
        return get_os_coreid(hw_core_id());
 }
 
+static inline int core_id_early(void)
+{
+       if (!core_id_ready)
+               return 0;
+       return core_id();
+}
+
 static inline void cache_flush(void)
 {
        wbinvd();
index 481d9a5..58d432a 100644 (file)
@@ -276,6 +276,7 @@ vm_init(void)
 
        // Flush the TLB for good measure, to kill the pgdir[0] mapping.
        tlb_flush_global();
+       core_id_ready = TRUE;
 }
 
 void x86_cleanup_bootmem(void)
index 7820da1..a0b4e79 100644 (file)
@@ -451,6 +451,11 @@ void vm_init(void)
        gdt_pd = (pseudodesc_t) {sizeof(segdesc_t) * SEG_COUNT - 1,
                                 (uintptr_t)gdt};
        asm volatile("lgdt %0" : : "m"(gdt_pd));
+       /* LAPIC is mapped, if we're using that.  if we're using rdtscp or some
+        * other non-LAPIC method, we can (probably) start using it right away.  we
+        * may get 0 back on other cores, if smp_boot hasn't completed, though
+        * that's no different than before this is TRUE. */
+       core_id_ready = TRUE;
 }
 
 void x86_cleanup_bootmem(void)
index 099605c..7582d7d 100644 (file)
@@ -28,7 +28,7 @@ static void decrease_lock_depth(uint32_t coreid)
 #ifdef CONFIG_SPINLOCK_DEBUG
 void spin_lock(spinlock_t *lock)
 {
-       uint32_t coreid = core_id();
+       uint32_t coreid = core_id_early();
        struct per_cpu_info *pcpui = &per_cpu_info[coreid];
        /* Short circuit our lock checking, so we can print or do other things to
         * announce the failure that require locks. */
index 63fa4c4..d410325 100644 (file)
@@ -108,17 +108,12 @@ void _panic(const char *file, int line, const char *fmt,...)
 {
        va_list ap;
        struct per_cpu_info *pcpui;
-       #if 0
-       /* Debug panic, in case we panic before core_id is available */
-       printk("Kernel panic at %s:%d\n", file, line);
-       while (1)
-               cpu_relax();
-       #endif
        /* We're panicing, possibly in a place that can't handle the lock checker */
-       pcpui = &per_cpu_info[core_id()];
+       pcpui = &per_cpu_info[core_id_early()];
        pcpui->__lock_depth_disabled++;
        va_start(ap, fmt);
-       cprintf("kernel panic at %s:%d, from core %d: ", file, line, core_id());
+       printk("kernel panic at %s:%d, from core %d: ", file, line,
+              core_id_early());
        vcprintf(fmt, ap);
        cprintf("\n");
        va_end(ap);
@@ -138,7 +133,8 @@ void _warn(const char *file, int line, const char *fmt,...)
        va_list ap;
 
        va_start(ap, fmt);
-       cprintf("kernel warning at %s:%d: ", file, line);
+       printk("kernel warning at %s:%d, from core %d: ", file, line,
+              core_id_early());
        vcprintf(fmt, ap);
        cprintf("\n");
        va_end(ap);
index bd2f8ea..463ace6 100644 (file)
@@ -762,19 +762,20 @@ void monitor(struct hw_trapframe *hw_tf)
        #define MON_CMD_LENGTH 256
        char buf[MON_CMD_LENGTH];
        int cnt;
+       int coreid = core_id_early();
 
        /* they are always disabled, since we have this irqsave lock */
        if (irq_is_enabled())
-               printk("Entering Nanwan's Dungeon on Core %d (Ints on):\n", core_id());
+               printk("Entering Nanwan's Dungeon on Core %d (Ints on):\n", coreid);
        else
-               printk("Entering Nanwan's Dungeon on Core %d (Ints off):\n", core_id());
+               printk("Entering Nanwan's Dungeon on Core %d (Ints off):\n", coreid);
        printk("Type 'help' for a list of commands.\n");
 
        if (hw_tf != NULL)
                print_trapframe(hw_tf);
 
        while (1) {
-               cnt = readline(buf, MON_CMD_LENGTH, "ROS(Core %d)> ", core_id());
+               cnt = readline(buf, MON_CMD_LENGTH, "ROS(Core %d)> ", coreid);
                if (cnt > 0) {
                        buf[cnt] = 0;
                        if (runcmd(buf, hw_tf) < 0)