[PATCH 11/12] i386: Fix race in multiprocessor ktss

[Damien Zammit]

---
 i386/i386/io_perm.c |  5 ---
 i386/i386/pcb.c     | 79 ++++++++++++++++++++++++---------------------
 2 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/i386/i386/io_perm.c b/i386/i386/io_perm.c
index 6db60f73..d9c646fe 100644
--- a/i386/i386/io_perm.c
+++ b/i386/i386/io_perm.c
@@ -266,9 +266,7 @@ i386_io_perm_modify (task_t target_task, io_perm_t io_perm, 
boolean_t enable)

   if (!iopb)
     {
-      simple_unlock (&target_task->machine.iopb_lock);
       iopb = (unsigned char *) kmem_cache_alloc (&machine_task_iopb_cache);
-      simple_lock (&target_task->machine.iopb_lock);
       if (target_task->machine.iopb)
        {
          if (iopb)
@@ -308,9 +306,6 @@ i386_io_perm_modify (task_t target_task, io_perm_t io_perm, 
boolean_t enable)
       target_task->machine.iopb_size = iopb_size;
     }

-#if NCPUS>1
-#warning SMP support missing (notify all CPUs running threads in that of the 
I/O bitmap change).
-#endif
   if (target_task == current_task())
     update_ktss_iopb (iopb, target_task->machine.iopb_size);

diff --git a/i386/i386/pcb.c b/i386/i386/pcb.c
index 5ac487b7..4ff7309c 100644
--- a/i386/i386/pcb.c
+++ b/i386/i386/pcb.c
@@ -48,6 +48,7 @@
 #include <i386/user_ldt.h>
 #include <i386/db_interface.h>
 #include <i386/fpu.h>
+#include <i386/spl.h>
 #include "eflags.h"
 #include "gdt.h"
 #include "ldt.h"
@@ -122,8 +123,8 @@ vm_offset_t stack_detach(thread_t thread)
 #define        curr_gdt(mycpu)         (mp_gdt[mycpu])
 #define        curr_ktss(mycpu)        (mp_ktss[mycpu])
 #else
-#define        curr_gdt(mycpu)         ((void)(mycpu), gdt)
-#define        curr_ktss(mycpu)        ((void)(mycpu), (struct task_tss 
*)&ktss)
+#define        curr_gdt(mycpu)         (gdt)
+#define        curr_ktss(mycpu)        ((struct task_tss *)&ktss)
 #endif

 #define        gdt_desc_p(mycpu,sel) \
@@ -131,6 +132,8 @@ vm_offset_t stack_detach(thread_t thread)

 void switch_ktss(pcb_t pcb)
 {
+       spl_t s = splhigh();
+
        int                     mycpu = cpu_number();
     {
        vm_offset_t             pcb_stack_top;
@@ -222,21 +225,20 @@ void switch_ktss(pcb_t pcb)
         pcb->ims.user_gdt, sizeof pcb->ims.user_gdt);
 #endif /* MACH_PV_DESCRIPTORS */

-       db_load_context(pcb);
+       db_set_debug_state(pcb, &pcb->ims.ids)

        /*
         * Load the floating-point context, if necessary.
         */
        fpu_load_context(pcb);

+       splx(s);
 }

-/* If NEW_IOPB is not null, the SIZE denotes the number of bytes in
-   the new bitmap.  Expects iopb_lock to be held.  */
-void
-update_ktss_iopb (unsigned char *new_iopb, io_port_t size)
+static void
+update_ktss_iopb_per_cpu (unsigned char *new_iopb, io_port_t size, int cpu)
 {
-  struct task_tss *tss = curr_ktss (cpu_number ());
+  struct task_tss *tss = curr_ktss (cpu);

   if (new_iopb && size > 0)
     {
@@ -249,6 +251,24 @@ update_ktss_iopb (unsigned char *new_iopb, io_port_t size)
     tss->tss.io_bit_map_offset = IOPB_INVAL;
 }

+/* If NEW_IOPB is not null, the SIZE denotes the number of bytes in
+   the new bitmap.  Expects iopb_lock to be held per TASK.  */
+void
+update_ktss_iopb (unsigned char *new_iopb, io_port_t size)
+{
+#if NCPUS > 1
+  int slot;
+
+  for (slot = 0; slot < NCPUS; slot++)
+    {
+      if (machine_slot[slot].is_cpu)
+       update_ktss_iopb_per_cpu (new_iopb, size, slot);
+    }
+#else
+  update_ktss_iopb_per_cpu (new_iopb, size, cpu_number());
+#endif
+}
+
 /*
  *     stack_handoff:
  *
@@ -259,8 +279,12 @@ void stack_handoff(
        thread_t        old,
        thread_t        new)
 {
-       int             mycpu = cpu_number();
        vm_offset_t     stack;
+       task_t old_task, new_task;
+       int mycpu;
+       spl_t s;
+
+       mycpu = cpu_number();

        /*
         *      Save FP registers if in use.
@@ -270,8 +294,6 @@ void stack_handoff(
        /*
         *      Switch address maps if switching tasks.
         */
-    {
-       task_t old_task, new_task;

        if ((old_task = old->task) != (new_task = new->task)) {
                PMAP_DEACTIVATE_USER(vm_map_pmap(old_task->map),
@@ -279,20 +301,12 @@ void stack_handoff(
                PMAP_ACTIVATE_USER(vm_map_pmap(new_task->map),
                                   new, mycpu);

+               s = splhigh();
                simple_lock (&new_task->machine.iopb_lock);
-#if NCPUS>1
-#warning SMP support missing (avoid races with io_perm_modify).
-#else
-               /* This optimization only works on a single processor
-                  machine, where old_task's iopb can not change while
-                  we are switching.  */
-               if (old_task->machine.iopb || new_task->machine.iopb)
-#endif
-                 update_ktss_iopb (new_task->machine.iopb,
-                                   new_task->machine.iopb_size);
+               update_ktss_iopb (new_task->machine.iopb, 
new_task->machine.iopb_size);
                simple_unlock (&new_task->machine.iopb_lock);
+               splx(s);
        }
-    }

        /*
         *      Load the rest of the user state for the new thread
@@ -335,6 +349,10 @@ thread_t switch_context(
        void            (*continuation)(),
        thread_t        new)
 {
+       task_t old_task, new_task;
+       int     mycpu = cpu_number();
+       spl_t s;
+
        /*
         *      Save FP registers if in use.
         */
@@ -343,9 +361,6 @@ thread_t switch_context(
        /*
         *      Switch address maps if switching tasks.
         */
-    {
-       task_t old_task, new_task;
-       int     mycpu = cpu_number();

        if ((old_task = old->task) != (new_task = new->task)) {
                PMAP_DEACTIVATE_USER(vm_map_pmap(old_task->map),
@@ -353,20 +368,12 @@ thread_t switch_context(
                PMAP_ACTIVATE_USER(vm_map_pmap(new_task->map),
                                   new, mycpu);

+               s = splhigh();
                simple_lock (&new_task->machine.iopb_lock);
-#if NCPUS>1
-#warning SMP support missing (avoid races with io_perm_modify).
-#else
-               /* This optimization only works on a single processor
-                  machine, where old_task's iopb can not change while
-                  we are switching.  */
-               if (old_task->machine.iopb || new_task->machine.iopb)
-#endif
-                 update_ktss_iopb (new_task->machine.iopb,
-                                   new_task->machine.iopb_size);
+               update_ktss_iopb (new_task->machine.iopb, 
new_task->machine.iopb_size);
                simple_unlock (&new_task->machine.iopb_lock);
+               splx(s);
        }
-    }

        /*
         *      Load the rest of the user state for the new thread
--
2.34.1