x86: remove misconceptions about "sti" and halting
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 8 Dec 2015 16:41:32 +0000 (11:41 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 10 Dec 2015 16:26:39 +0000 (11:26 -0500)
We need to atomically enable interrupts and halt.  Unbeknownst to
past-barret, sti doesn't take effect until after the *next* instruction.
The Good Book definitely says it too.

On a similar note, enabling and disabling IRQs in proc_restartcore() did
nothing.  It had a TODO to remove it anyways.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/arch/x86/arch.h
kern/arch/x86/trap.c
kern/src/process.c

index 9865bb2..aeb9cb5 100644 (file)
@@ -130,8 +130,8 @@ static inline void cpu_relax(void)
        __cpu_relax();
 }
 
-/* This doesn't atomically enable interrupts and then halt, like we want, so
- * x86 needs to use a custom helper in the irq handler in trap.c. */
+/* This atomically enables interrupts and halts.  sti does not take effect until
+ * after the *next* instruction */
 static inline void cpu_halt(void)
 {
        asm volatile("sti; hlt" : : : "memory");
index 0314348..952eb3e 100644 (file)
@@ -430,24 +430,6 @@ static void set_current_ctx_sw(struct per_cpu_info *pcpui,
        pcpui->cur_ctx = &pcpui->actual_ctx;
 }
 
-/* If the interrupt interrupted a halt, we advance past it.  Made to work with
- * x86's custom cpu_halt() in arch/arch.h.  Note this nearly never gets called.
- * I needed to insert exactly one 'nop' in cpu_halt() (that isn't there now) to
- * get the interrupt to trip on the hlt, o/w the hlt will execute before the
- * interrupt arrives (even with a pending interrupt that should hit right after
- * an interrupt_enable (sti)).  This was on the i7. */
-static void abort_halt(struct hw_trapframe *hw_tf)
-{
-       /* Don't care about user TFs.  Incidentally, dereferencing user EIPs is
-        * reading userspace memory, which can be dangerous.  It can page fault,
-        * like immediately after a fork (which doesn't populate the pages). */
-       if (!in_kernel(hw_tf))
-               return;
-       /* the halt instruction in is 0xf4, and it's size is 1 byte */
-       if (*(uint8_t*)x86_get_ip_hw(hw_tf) == 0xf4)
-               x86_advance_ip(hw_tf, 1);
-}
-
 void trap(struct hw_trapframe *hw_tf)
 {
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
@@ -502,8 +484,6 @@ void handle_irq(struct hw_trapframe *hw_tf)
        if (!in_irq_ctx(pcpui))
                __set_cpu_state(pcpui, CPU_STATE_IRQ);
        inc_irq_depth(pcpui);
-       /* Coupled with cpu_halt() and smp_idle() */
-       abort_halt(hw_tf);
        //if (core_id())
        if (hw_tf->tf_trapno != IdtLAPIC_TIMER) /* timer irq */
        if (hw_tf->tf_trapno != I_KERNEL_MSG)
index 17f5180..1e7f103 100644 (file)
@@ -767,16 +767,8 @@ void __proc_startcore(struct proc *p, struct user_context *ctx)
 void proc_restartcore(void)
 {
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
+
        assert(!pcpui->cur_kthread->sysc);
-       /* TODO: can probably remove this enable_irq.  it was an optimization for
-        * RKMs */
-       /* Try and get any interrupts before we pop back to userspace.  If we didn't
-        * do this, we'd just get them in userspace, but this might save us some
-        * effort/overhead. */
-       enable_irq();
-       /* Need ints disabled when we return from PRKM (race on missing
-        * messages/IPIs) */
-       disable_irq();
        process_routine_kmsg();
        /* If there is no owning process, just idle, since we don't know what to do.
         * This could be because the process had been restarted a long time ago and