Palacios Public Git Repository

To checkout Palacios execute

  git clone http://v3vee.org/palacios/palacios.web/palacios.git
This will give you the master branch. You probably want the devel branch or one of the release branches. To switch to the devel branch, simply execute
  cd palacios
  git checkout --track -b devel origin/devel
The other branches are similar.


separated virtual cores from physical cores
[palacios-OLD.git] / palacios / src / devices / apic.c
index 427ec79..5a2856a 100644 (file)
 #include <palacios/vmm_types.h>
 
 
+#include <palacios/vmm_queue.h>
+#include <palacios/vmm_lock.h>
+
+/* The locking in this file is nasty.
+ * There are 3 different locking approaches that are taken, depending on the APIC operation
+ * 1. Queue locks. Actual irq insertions are done via queueing irq ops at the dest apic. 
+ *    The destination apic's core is responsible for draining the queue, and actually 
+ *    setting the vector table. 
+ * 2. State locks. This is a standard lock taken when internal apic state is read/written. 
+ *    When an irq's destination is determined this lock is taken to examine the apic's 
+ *    addressability. 
+ * 3. VM barrier lock. This is taken when actual VM core state is changed (via SIPI). 
+ */
+
 
 
-#ifndef CONFIG_DEBUG_APIC
+#ifndef V3_CONFIG_DEBUG_APIC
 #undef PrintDebug
 #define PrintDebug(fmt, args...)
-#endif
-
+#else
 
-#ifdef CONFIG_DEBUG_APIC
 static char * shorthand_str[] = { 
     "(no shorthand)",
     "(self)",
@@ -54,12 +66,8 @@ static char * deliverymode_str[] = {
     "(Start Up)",
     "(ExtInt)",
 };
-#endif
-
-// Temporary removal of locking
-#define v3_lock(p) p=p
-#define v3_unlock(p) p=p
 
+#endif
 
 typedef enum { APIC_TMR_INT, APIC_THERM_INT, APIC_PERF_INT, 
               APIC_LINT0_INT, APIC_LINT1_INT, APIC_ERR_INT } apic_irq_type_t;
@@ -231,12 +239,12 @@ struct apic_state {
 
     struct v3_timer * timer;
 
+    v3_lock_t state_lock;
+    struct v3_queue irq_queue;
+
     uint32_t eoi;
 
-    v3_lock_t  lock;
 
-    // debug
-    uint8_t in_icr;   
 };
 
 
@@ -244,8 +252,7 @@ struct apic_state {
 
 struct apic_dev_state {
     int num_apics;
-    //    v3_lock_t ipi_lock;   // acquired by route_ipi - only one IPI active at a time
-
+  
     struct apic_state apics[0];
 } __attribute__((packed));
 
@@ -313,70 +320,65 @@ static void init_apic_state(struct apic_state * apic, uint32_t id) {
     apic->ext_apic_ctrl.val = 0x00000000;
     apic->spec_eoi.val = 0x00000000;
 
-    v3_lock_init(&(apic->lock));
 
-    //debug
-    apic->in_icr=0;
+    v3_init_queue(&(apic->irq_queue));
+
+
 }
 
 
 
 
-// MSR handler - locks apic itself
+
 static int read_apic_msr(struct guest_info * core, uint_t msr, v3_msr_t * dst, void * priv_data) {
     struct apic_dev_state * apic_dev = (struct apic_dev_state *)priv_data;
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]);
+    struct apic_state * apic = &(apic_dev->apics[core->vcpu_id]);
+
+    PrintDebug("apic %u: core %u: MSR read\n", apic->lapic_id.val, core->vcpu_id);
 
-    PrintDebug("apic %u: core %u: MSR read\n", apic->lapic_id.val, core->cpu_id);
-    v3_lock(apic->lock);
     dst->value = apic->base_addr;
-    v3_unlock(apic->lock);
+
     return 0;
 }
 
-// MSR handler - locks apic itself
+
 static int write_apic_msr(struct guest_info * core, uint_t msr, v3_msr_t src, void * priv_data) {
     struct apic_dev_state * apic_dev = (struct apic_dev_state *)priv_data;
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]);
-    struct v3_mem_region * old_reg = v3_get_mem_region(core->vm_info, core->cpu_id, apic->base_addr);
+    struct apic_state * apic = &(apic_dev->apics[core->vcpu_id]);
+    struct v3_mem_region * old_reg = v3_get_mem_region(core->vm_info, core->vcpu_id, apic->base_addr);
 
 
-    PrintDebug("apic %u: core %u: MSR write\n", apic->lapic_id.val, core->cpu_id);
+    PrintDebug("apic %u: core %u: MSR write\n", apic->lapic_id.val, core->vcpu_id);
 
     if (old_reg == NULL) {
        // uh oh...
        PrintError("apic %u: core %u: APIC Base address region does not exit...\n",
-                  apic->lapic_id.val, core->cpu_id);
+                  apic->lapic_id.val, core->vcpu_id);
        return -1;
     }
     
-    v3_lock(apic->lock);
+
 
     v3_delete_mem_region(core->vm_info, old_reg);
 
     apic->base_addr = src.value;
 
-    if (v3_hook_full_mem(core->vm_info, core->cpu_id, apic->base_addr, 
+    if (v3_hook_full_mem(core->vm_info, core->vcpu_id, apic->base_addr, 
                         apic->base_addr + PAGE_SIZE_4KB, 
                         apic_read, apic_write, apic_dev) == -1) {
        PrintError("apic %u: core %u: Could not hook new APIC Base address\n",
-                  apic->lapic_id.val, core->cpu_id);
-       v3_unlock(apic->lock);
+                  apic->lapic_id.val, core->vcpu_id);
+
        return -1;
     }
 
-    v3_unlock(apic->lock);
+
     return 0;
 }
 
 
 // irq_num is the bit offset into a 256 bit buffer...
-// return values
-//    -1 = error
-//     0 = OK, no interrupt needed now
-//     1 = OK, interrupt needed now
-// the caller is expeced to have locked the apic
-static int activate_apic_irq_nolock(struct apic_state * apic, uint32_t irq_num) {
+static int activate_apic_irq(struct apic_state * apic, uint32_t irq_num) {
     int major_offset = (irq_num & ~0x00000007) >> 3;
     int minor_offset = irq_num & 0x00000007;
     uint8_t * req_location = apic->int_req_reg + major_offset;
@@ -384,14 +386,14 @@ static int activate_apic_irq_nolock(struct apic_state * apic, uint32_t irq_num)
     uint8_t flag = 0x1 << minor_offset;
 
 
-    if (irq_num <= 15 || irq_num>255) {
+    if (irq_num <= 15 || irq_num > 255) {
        PrintError("apic %u: core %d: Attempting to raise an invalid interrupt: %d\n", 
-                  apic->lapic_id.val, apic->core->cpu_id, irq_num);
+                  apic->lapic_id.val, apic->core->vcpu_id, irq_num);
        return -1;
     }
 
 
-    PrintDebug("apic %u: core %d: Raising APIC IRQ %d\n", apic->lapic_id.val, apic->core->cpu_id, irq_num);
+    PrintDebug("apic %u: core %d: Raising APIC IRQ %d\n", apic->lapic_id.val, apic->core->vcpu_id, irq_num);
 
     if (*req_location & flag) {
        PrintDebug("Interrupt %d  coallescing\n", irq_num);
@@ -400,23 +402,17 @@ static int activate_apic_irq_nolock(struct apic_state * apic, uint32_t irq_num)
 
     if (*en_location & flag) {
        *req_location |= flag;
-
-       if (apic->in_icr) { 
-           PrintError("apic %u: core %d: activate_apic_irq_nolock to deliver irq 0x%x when in_icr=1\n",  apic->lapic_id.val, apic->core->cpu_id, irq_num);
-           //      return 0;
-       }
-
        return 1;
     } else {
        PrintDebug("apic %u: core %d: Interrupt  not enabled... %.2x\n", 
-                  apic->lapic_id.val, apic->core->cpu_id,*en_location);
-       return 0;
+                  apic->lapic_id.val, apic->core->vcpu_id, *en_location);
     }
 
+    return 0;
 }
 
 
-// Caller is expected to have locked the apic
+
 static int get_highest_isr(struct apic_state * apic) {
     int i = 0, j = 0;
 
@@ -438,7 +434,7 @@ static int get_highest_isr(struct apic_state * apic) {
 }
  
 
-// Caller is expected to have locked the apic
+
 static int get_highest_irr(struct apic_state * apic) {
     int i = 0, j = 0;
 
@@ -461,7 +457,7 @@ static int get_highest_irr(struct apic_state * apic) {
  
 
 
-// Caller is expected to have locked the apic
+
 static int apic_do_eoi(struct apic_state * apic) {
     int isr_irq = get_highest_isr(apic);
 
@@ -475,7 +471,7 @@ static int apic_do_eoi(struct apic_state * apic) {
        
        *svc_location &= ~flag;
 
-#ifdef CONFIG_CRAY_XT
+#ifdef V3_CONFIG_CRAY_XT
        
        if ((isr_irq == 238) || 
            (isr_irq == 239)) {
@@ -493,8 +489,8 @@ static int apic_do_eoi(struct apic_state * apic) {
     return 0;
 }
  
-// Caller is expected to have locked the apic
-static int activate_internal_irq_nolock(struct apic_state * apic, apic_irq_type_t int_type) {
+
+static int activate_internal_irq(struct apic_state * apic, apic_irq_type_t int_type) {
     uint32_t vec_num = 0;
     uint32_t del_mode = 0;
     int masked = 0;
@@ -544,7 +540,7 @@ static int activate_internal_irq_nolock(struct apic_state * apic, apic_irq_type_
 
     if (del_mode == APIC_FIXED_DELIVERY) {
        //PrintDebug("Activating internal APIC IRQ %d\n", vec_num);
-       return activate_apic_irq_nolock(apic, vec_num);
+       return activate_apic_irq(apic, vec_num);
     } else {
        PrintError("apic %u: core ?: Unhandled Delivery Mode\n", apic->lapic_id.val);
        return -1;
@@ -552,7 +548,7 @@ static int activate_internal_irq_nolock(struct apic_state * apic, apic_irq_type_
 }
 
 
-// Caller is expected to have locked the destination apic
+
 static inline int should_deliver_cluster_ipi(struct guest_info * dst_core, 
                                             struct apic_state * dst_apic, uint8_t mda) {
 
@@ -560,26 +556,25 @@ static inline int should_deliver_cluster_ipi(struct guest_info * dst_core,
          ((mda & 0x0f) & (dst_apic->log_dst.dst_log_id & 0x0f)) ) {  /*  I am in the set)        */
 
        PrintDebug("apic %u core %u: accepting clustered IRQ (mda 0x%x == log_dst 0x%x)\n",
-                  dst_apic->lapic_id.val, dst_core->cpu_id, mda, 
+                  dst_apic->lapic_id.val, dst_core->vcpu_id, mda, 
                   dst_apic->log_dst.dst_log_id);
        
        return 1;
     } else {
        PrintDebug("apic %u core %u: rejecting clustered IRQ (mda 0x%x != log_dst 0x%x)\n",
-                  dst_apic->lapic_id.val, dst_core->cpu_id, mda, 
+                  dst_apic->lapic_id.val, dst_core->vcpu_id, mda, 
                   dst_apic->log_dst.dst_log_id);
        return 0;
     }
 }
 
-// Caller is expected to have locked the destiation apic
 static inline int should_deliver_flat_ipi(struct guest_info * dst_core,
                                          struct apic_state * dst_apic, uint8_t mda) {
 
     if (dst_apic->log_dst.dst_log_id & mda) {  // I am in the set 
 
        PrintDebug("apic %u core %u: accepting flat IRQ (mda 0x%x == log_dst 0x%x)\n",
-                  dst_apic->lapic_id.val, dst_core->cpu_id, mda, 
+                  dst_apic->lapic_id.val, dst_core->vcpu_id, mda, 
                   dst_apic->log_dst.dst_log_id);
 
        return 1;
@@ -587,14 +582,14 @@ static inline int should_deliver_flat_ipi(struct guest_info * dst_core,
   } else {
 
        PrintDebug("apic %u core %u: rejecting flat IRQ (mda 0x%x != log_dst 0x%x)\n",
-                  dst_apic->lapic_id.val, dst_core->cpu_id, mda, 
+                  dst_apic->lapic_id.val, dst_core->vcpu_id, mda, 
                   dst_apic->log_dst.dst_log_id);
        return 0;
   }
 }
 
 
-// Caller is expected to have locked the destiation apic
+
 static int should_deliver_ipi(struct guest_info * dst_core, 
                              struct apic_state * dst_apic, uint8_t mda) {
 
@@ -619,12 +614,12 @@ static int should_deliver_ipi(struct guest_info * dst_core,
 
     } else {
        PrintError("apic %u core %u: invalid destination format register value 0x%x for logical mode delivery.\n", 
-                  dst_apic->lapic_id.val, dst_core->cpu_id, dst_apic->dst_fmt.model);
+                  dst_apic->lapic_id.val, dst_core->vcpu_id, dst_apic->dst_fmt.model);
        return -1;
     }
 }
 
-// Caller is expected to have locked the destination apic
+
 // Only the src_apic pointer is used
 static int deliver_ipi(struct apic_state * src_apic, 
                       struct apic_state * dst_apic, 
@@ -632,51 +627,43 @@ static int deliver_ipi(struct apic_state * src_apic,
 
 
     struct guest_info * dst_core = dst_apic->core;
-    int do_xcall;
+
 
     switch (del_mode) {
 
        case APIC_FIXED_DELIVERY:  
-       case APIC_LOWEST_DELIVERY:
+       case APIC_LOWEST_DELIVERY: {
            // lowest priority - 
            // caller needs to have decided which apic to deliver to!
 
-           PrintDebug("delivering IRQ %d to core %u\n", vector, dst_core->cpu_id); 
+           int do_xcall;
+
+           PrintDebug("delivering IRQ %d to core %u\n", vector, dst_core->vcpu_id); 
 
-           do_xcall=activate_apic_irq_nolock(dst_apic, vector);
+           do_xcall = activate_apic_irq(dst_apic, vector);
            
-           if (do_xcall<0) { 
-               PrintError("Failed to activate apic irq!\n");
-               return -1;
-           }
 
-           if (do_xcall && (dst_apic != src_apic)) { 
-               // Assume core # is same as logical processor for now
-               // TODO FIX THIS FIX THIS
-               // THERE SHOULD BE:  guestapicid->virtualapicid map,
-               //                   cpu_id->logical processor map
-               //     host maitains logical proc->phsysical proc
+
+           if (dst_apic != src_apic) { 
                PrintDebug(" non-local core with new interrupt, forcing it to exit now\n"); 
 
-#ifdef CONFIG_MULTITHREAD_OS
-               v3_interrupt_cpu(dst_core->vm_info, dst_core->cpu_id, 0);
-#else
-               V3_ASSERT(0);
+#ifdef V3_CONFIG_MULTITHREAD_OS
+               v3_interrupt_cpu(dst_core->vm_info, dst_core->pcpu_id, 0);
 #endif
            }
 
            break;
-
+       }
        case APIC_INIT_DELIVERY: { 
 
-           PrintDebug(" INIT delivery to core %u\n", dst_core->cpu_id);
+           PrintDebug(" INIT delivery to core %u\n", dst_core->vcpu_id);
 
            // TODO: any APIC reset on dest core (shouldn't be needed, but not sure...)
 
            // Sanity check
            if (dst_apic->ipi_state != INIT_ST) { 
                PrintError(" Warning: core %u is not in INIT state (mode = %d), ignored (assuming this is the deassert)\n",
-                          dst_core->cpu_id, dst_apic->ipi_state);
+                          dst_core->vcpu_id, dst_apic->ipi_state);
                // Only a warning, since INIT INIT SIPI is common
                break;
            }
@@ -699,7 +686,7 @@ static int deliver_ipi(struct apic_state * src_apic,
            // Sanity check
            if (dst_apic->ipi_state != SIPI) { 
                PrintError(" core %u is not in SIPI state (mode = %d), ignored!\n",
-                          dst_core->cpu_id, dst_apic->ipi_state);
+                          dst_core->vcpu_id, dst_apic->ipi_state);
                break;
            }
 
@@ -719,7 +706,7 @@ static int deliver_ipi(struct apic_state * src_apic,
            dst_core->segments.cs.base = vector << 12;
 
            PrintDebug(" SIPI delivery (0x%x -> 0x%x:0x0) to core %u\n",
-                      vector, dst_core->segments.cs.selector, dst_core->cpu_id);
+                      vector, dst_core->segments.cs.selector, dst_core->vcpu_id);
            // Maybe need to adjust the APIC?
            
            // We transition the target core to SIPI state
@@ -749,14 +736,14 @@ static struct apic_state * find_physical_apic(struct apic_dev_state *apic_dev, s
 {
     int i;
     
-    if (icr->dst >0 && icr->dst < apic_dev->num_apics) { 
+    if ( (icr->dst > 0) && (icr->dst < apic_dev->num_apics) ) { 
        // see if it simply is the core id
        if (apic_dev->apics[icr->dst].lapic_id.val == icr->dst) { 
            return &(apic_dev->apics[icr->dst]);
        }
     }
 
-    for (i=0;i<apic_dev->num_apics;i++) { 
+    for (i = 0; i < apic_dev->num_apics; i++) { 
        if (apic_dev->apics[i].lapic_id.val == icr->dst) { 
            return &(apic_dev->apics[i]);
        }
@@ -766,28 +753,13 @@ static struct apic_state * find_physical_apic(struct apic_dev_state *apic_dev, s
 
 }
 
-// route_ipi is responsible for all locking
-// the assumption is that you enter with no locks
-// there is a global lock for the icc bus, so only
-// one route_ipi progresses at any time
-// destination apics are locked as needed
-// if multiple apic locks are acquired at any point,
-// this is done in the order of the array, so no
-// deadlock should be possible
+
 static int route_ipi(struct apic_dev_state * apic_dev,
                     struct apic_state * src_apic, 
                     struct int_cmd_reg * icr) {
     struct apic_state * dest_apic = NULL;
 
 
-    //v3_lock(apic_dev->ipi_lock);  // this may not be needed
-    // now I know only one IPI is being routed, this one
-    // also, I do not have any apic locks
-    // I need to acquire locks on pairs of src/dest apics
-    // and I will do that using the total order
-    // given by their cores
-
-
     PrintDebug("apic: IPI %s %u from apic %p to %s %s %u (icr=0x%llx)\n",
               deliverymode_str[icr->del_mode], 
               icr->vec, 
@@ -797,147 +769,97 @@ static int route_ipi(struct apic_dev_state * apic_dev,
               icr->dst,
               icr->val);
 
-#if 0
-        if (icr->vec!=48) { 
-           V3_Print("apic: IPI %s %u from apic %p to %s %s %u (icr=0x%llx)\n",
-                   deliverymode_str[icr->del_mode], 
-                   icr->vec, 
-                   src_apic,          
-                   (icr->dst_mode == 0) ? "(physical)" : "(logical)", 
-                   shorthand_str[icr->dst_shorthand], 
-                   icr->dst,
-                   icr->val);
-       }
-
-#endif
-
 
     switch (icr->dst_shorthand) {
 
        case APIC_SHORTHAND_NONE:  // no shorthand
            if (icr->dst_mode == APIC_DEST_PHYSICAL) { 
 
-               dest_apic=find_physical_apic(apic_dev,icr);
+               dest_apic = find_physical_apic(apic_dev, icr);
                
-               if (dest_apic==NULL) { 
+               if (dest_apic == NULL) { 
                    PrintError("apic: Attempted send to unregistered apic id=%u\n", icr->dst);
-                   goto route_ipi_out_bad;
+                   return -1;
                }
 
-
-       
-               //V3_Print("apic: phsyical destination of %u (apic %u at 0x%p)\n", icr->dst,dest_apic->lapic_id.val,dest_apic);
-
-               v3_lock(dest_apic->lock);
-
                if (deliver_ipi(src_apic, dest_apic, 
                                icr->vec, icr->del_mode) == -1) {
                    PrintError("apic: Could not deliver IPI\n");
-                   v3_unlock(dest_apic->lock);
-                   goto route_ipi_out_bad;
+                   return -1;
                }
 
-               v3_unlock(dest_apic->lock);
 
-               //V3_Print("apic: done\n");
+               PrintDebug("apic: done\n");
 
            } else if (icr->dst_mode == APIC_DEST_LOGICAL) {
                
-               if (icr->del_mode!=APIC_LOWEST_DELIVERY ) { 
+               if (icr->del_mode != APIC_LOWEST_DELIVERY) { 
+                   int i;
+                   uint8_t mda = icr->dst;
+
                    // logical, but not lowest priority
                    // we immediately trigger
                    // fixed, smi, reserved, nmi, init, sipi, etc
-                   int i;
-                   
-                   uint8_t mda = icr->dst;
+
                    
                    for (i = 0; i < apic_dev->num_apics; i++) { 
+                       int del_flag = 0;
                        
                        dest_apic = &(apic_dev->apics[i]);
                        
-                       v3_lock(dest_apic->lock);
-                       
-                       int del_flag = should_deliver_ipi(dest_apic->core, dest_apic, mda);
+                       del_flag = should_deliver_ipi(dest_apic->core, dest_apic, mda);
                        
                        if (del_flag == -1) {
+
                            PrintError("apic: Error checking delivery mode\n");
-                           v3_unlock(dest_apic->lock);
-                           goto route_ipi_out_bad;
+                           return -1;
                        } else if (del_flag == 1) {
+
                            if (deliver_ipi(src_apic, dest_apic, 
                                            icr->vec, icr->del_mode) == -1) {
                                PrintError("apic: Error: Could not deliver IPI\n");
-                               v3_unlock(dest_apic->lock);
-                               goto route_ipi_out_bad;
+                               return -1;
                            }
                        }
-                       
-                       v3_unlock(dest_apic->lock);
                    }
                } else {  //APIC_LOWEST_DELIVERY
-                   // logical, lowest priority
-                   // scan, keeping a lock on the current best, then trigger
                    int i;
-                   int have_cur_lock;
                    struct apic_state * cur_best_apic = NULL;
-                   
                    uint8_t mda = icr->dst;
-                   
-                   have_cur_lock=0;
-
-
-                   // Note that even if there are multiple concurrent 
-                   // copies of this loop executing, they are all 
-                   // locking in the same order
+                  
+                   // logical, lowest priority
 
                    for (i = 0; i < apic_dev->num_apics; i++) { 
-                       
+                       int del_flag = 0;
+
                        dest_apic = &(apic_dev->apics[i]);
                        
-                       v3_lock(dest_apic->lock);
-                       have_cur_lock=1;
-                       
-                       int del_flag = should_deliver_ipi(dest_apic->core, dest_apic, mda);
+                       del_flag = should_deliver_ipi(dest_apic->core, dest_apic, mda);
                        
                        if (del_flag == -1) {
                            PrintError("apic: Error checking delivery mode\n");
-                           v3_unlock(dest_apic->lock);
-                           if (cur_best_apic && cur_best_apic!=dest_apic) { 
-                               v3_unlock(cur_best_apic->lock);
-                           }
-                           goto route_ipi_out_bad;
+
+                           return -1;
                        } else if (del_flag == 1) {
                            // update priority for lowest priority scan
-                           if (!cur_best_apic) {
-                               cur_best_apic=dest_apic;  // note we leave it locked
-                               have_cur_lock=0;          // we will unlock as cur_best_apic
+                           if (cur_best_apic == 0) {
+                               cur_best_apic = dest_apic;  
                            } else if (dest_apic->task_prio.val < cur_best_apic->task_prio.val) {
-                               // we now unlock the current best one and then switch
-                               // so in the end we have a lock on the new cur_best_apic
-                               v3_unlock(cur_best_apic->lock);
-                               cur_best_apic=dest_apic;
-                               have_cur_lock=0; // will unlock as cur_best_apic
+                               cur_best_apic = dest_apic;
                            } 
-                       }
-                       if (have_cur_lock) {
-                           v3_unlock(dest_apic->lock);
-                       }
-                       
+                       }                       
                    }
+
                    // now we will deliver to the best one if it exists
-                   // and it is locked
                    if (!cur_best_apic) { 
                        PrintDebug("apic: lowest priority deliver, but no destinations!\n");
                    } else {
                        if (deliver_ipi(src_apic, cur_best_apic, 
                                        icr->vec, icr->del_mode) == -1) {
                            PrintError("apic: Error: Could not deliver IPI\n");
-                           v3_unlock(cur_best_apic->lock);
-                           goto route_ipi_out_bad;
-                       } else {
-                           v3_unlock(cur_best_apic->lock);
+                           return -1;
                        }
-                           //V3_Print("apic: logical, lowest priority delivery to apic %u\n",cur_best_apic->lapic_id.val);
+                       //V3_Print("apic: logical, lowest priority delivery to apic %u\n",cur_best_apic->lapic_id.val);
                    }
                }
            }
@@ -946,90 +868,73 @@ static int route_ipi(struct apic_dev_state * apic_dev,
            
        case APIC_SHORTHAND_SELF:  // self
 
-           /* I assume I am already locked! */
-
            if (src_apic == NULL) {    /* this is not an apic, but it's trying to send to itself??? */
                PrintError("apic: Sending IPI to self from generic IPI sender\n");
                break;
            }
 
-           v3_lock(src_apic->lock);
+
 
            if (icr->dst_mode == APIC_DEST_PHYSICAL)  {  /* physical delivery */
                if (deliver_ipi(src_apic, src_apic, icr->vec, icr->del_mode) == -1) {
                    PrintError("apic: Could not deliver IPI to self (physical)\n");
-                   v3_unlock(src_apic->lock);
-                   goto route_ipi_out_bad;
+                   return -1;
                }
            } else if (icr->dst_mode == APIC_DEST_LOGICAL) {  /* logical delivery */
                PrintError("apic: use of logical delivery in self (untested)\n");
+
                if (deliver_ipi(src_apic, src_apic, icr->vec, icr->del_mode) == -1) {
                    PrintError("apic: Could not deliver IPI to self (logical)\n");
-                   v3_unlock(src_apic->lock);
-                   goto route_ipi_out_bad;
+                   return -1;
                }
            }
-           v3_unlock(src_apic->lock);
+
            break;
            
        case APIC_SHORTHAND_ALL: 
        case APIC_SHORTHAND_ALL_BUT_ME: { /* all and all-but-me */
            /* assuming that logical verus physical doesn't matter
               although it is odd that both are used */
-
            int i;
 
            for (i = 0; i < apic_dev->num_apics; i++) { 
                dest_apic = &(apic_dev->apics[i]);
                
-               
                if ((dest_apic != src_apic) || (icr->dst_shorthand == APIC_SHORTHAND_ALL)) { 
-                   v3_lock(dest_apic->lock);
                    if (deliver_ipi(src_apic, dest_apic, icr->vec, icr->del_mode) == -1) {
                        PrintError("apic: Error: Could not deliver IPI\n");
-                       v3_unlock(dest_apic->lock);
-                       goto route_ipi_out_bad;
+                       return -1;
                    }
-                   v3_unlock(dest_apic->lock);
                }
-               
            }
-       }
+
            break;
+       }
        default:
            PrintError("apic: Error routing IPI, invalid Mode (%d)\n", icr->dst_shorthand);
-           goto route_ipi_out_bad;
+           return -1;
     }
-
-
-    // route_ipi_out_good:
-    //v3_unlock(apic_dev->ipi_lock);  
     return 0;
-
- route_ipi_out_bad:
-    //v3_unlock(apic_dev->ipi_lock);  
-    return -1;
 }
 
 
 // External function, expected to acquire lock on apic
 static int apic_read(struct guest_info * core, addr_t guest_addr, void * dst, uint_t length, void * priv_data) {
     struct apic_dev_state * apic_dev = (struct apic_dev_state *)(priv_data);
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]);
+    struct apic_state * apic = &(apic_dev->apics[core->vcpu_id]);
     addr_t reg_addr  = guest_addr - apic->base_addr;
     struct apic_msr * msr = (struct apic_msr *)&(apic->base_addr_msr.value);
     uint32_t val = 0;
 
-    v3_lock(apic->lock);
 
     PrintDebug("apic %u: core %u: at %p: Read apic address space (%p)\n",
-              apic->lapic_id.val, core->cpu_id, apic, (void *)guest_addr);
+              apic->lapic_id.val, core->vcpu_id, apic, (void *)guest_addr);
 
     if (msr->apic_enable == 0) {
        PrintError("apic %u: core %u: Read from APIC address space with disabled APIC, apic msr=0x%llx\n",
-                  apic->lapic_id.val, core->cpu_id, apic->base_addr_msr.value);
-
-       goto apic_read_out_bad;
+                  apic->lapic_id.val, core->vcpu_id, apic->base_addr_msr.value);
+       return -1;
     }
 
 
@@ -1241,8 +1146,8 @@ static int apic_read(struct guest_info * core, addr_t guest_addr, void * dst, ui
 
        default:
            PrintError("apic %u: core %u: Read from Unhandled APIC Register: %x (getting zero)\n", 
-                      apic->lapic_id.val, core->cpu_id, (uint32_t)reg_addr);
-           goto apic_read_out_bad;
+                      apic->lapic_id.val, core->vcpu_id, (uint32_t)reg_addr);
+           return -1;
     }
 
 
@@ -1264,21 +1169,14 @@ static int apic_read(struct guest_info * core, addr_t guest_addr, void * dst, ui
 
     } else {
        PrintError("apic %u: core %u: Invalid apic read length (%d)\n", 
-                  apic->lapic_id.val, core->cpu_id, length);
-       goto apic_read_out_bad;
+                  apic->lapic_id.val, core->vcpu_id, length);
+       return -1;
     }
 
     PrintDebug("apic %u: core %u: Read finished (val=%x)\n", 
-              apic->lapic_id.val, core->cpu_id, *(uint32_t *)dst);
-
+              apic->lapic_id.val, core->vcpu_id, *(uint32_t *)dst);
 
-    // apic_read_out_good:
-    v3_unlock(apic->lock);
     return length;
-
- apic_read_out_bad:
-    v3_unlock(apic->lock);
-    return -1;
 }
 
 
@@ -1287,31 +1185,28 @@ static int apic_read(struct guest_info * core, addr_t guest_addr, void * dst, ui
  */
 static int apic_write(struct guest_info * core, addr_t guest_addr, void * src, uint_t length, void * priv_data) {
     struct apic_dev_state * apic_dev = (struct apic_dev_state *)(priv_data);
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]); 
+    struct apic_state * apic = &(apic_dev->apics[core->vcpu_id]); 
     addr_t reg_addr  = guest_addr - apic->base_addr;
     struct apic_msr * msr = (struct apic_msr *)&(apic->base_addr_msr.value);
     uint32_t op_val = *(uint32_t *)src;
 
-
-    v3_lock(apic->lock);
-
     PrintDebug("apic %u: core %u: at %p and priv_data is at %p\n",
-              apic->lapic_id.val, core->cpu_id, apic, priv_data);
+              apic->lapic_id.val, core->vcpu_id, apic, priv_data);
 
     PrintDebug("apic %u: core %u: write to address space (%p) (val=%x)\n", 
-              apic->lapic_id.val, core->cpu_id, (void *)guest_addr, *(uint32_t *)src);
+              apic->lapic_id.val, core->vcpu_id, (void *)guest_addr, *(uint32_t *)src);
 
     if (msr->apic_enable == 0) {
        PrintError("apic %u: core %u: Write to APIC address space with disabled APIC, apic msr=0x%llx\n",
-                  apic->lapic_id.val, core->cpu_id, apic->base_addr_msr.value);
-       goto apic_write_out_bad;
+                  apic->lapic_id.val, core->vcpu_id, apic->base_addr_msr.value);
+       return -1;
     }
 
 
     if (length != 4) {
        PrintError("apic %u: core %u: Invalid apic write length (%d)\n", 
-                  apic->lapic_id.val, length, core->cpu_id);
-       goto apic_write_out_bad;
+                  apic->lapic_id.val, length, core->vcpu_id);
+       return -1;
     }
 
     switch (reg_addr) {
@@ -1346,15 +1241,14 @@ static int apic_write(struct guest_info * core, addr_t guest_addr, void * src, u
        case EXT_APIC_FEATURE_OFFSET:
 
            PrintError("apic %u: core %u: Attempting to write to read only register %p (error)\n", 
-                      apic->lapic_id.val, core->cpu_id, (void *)reg_addr);
-           //  goto apic_write_out_bad;
+                      apic->lapic_id.val, core->vcpu_id, (void *)reg_addr);
 
            break;
 
            // Data registers
        case APIC_ID_OFFSET:
            //V3_Print("apic %u: core %u: my id is being changed to %u\n", 
-           //       apic->lapic_id.val, core->cpu_id, op_val);
+           //       apic->lapic_id.val, core->vcpu_id, op_val);
 
            apic->lapic_id.val = op_val;
            break;
@@ -1363,7 +1257,7 @@ static int apic_write(struct guest_info * core, addr_t guest_addr, void * src, u
            break;
        case LDR_OFFSET:
            PrintDebug("apic %u: core %u: setting log_dst.val to 0x%x\n",
-                      apic->lapic_id.val, core->cpu_id, op_val);
+                      apic->lapic_id.val, core->vcpu_id, op_val);
            apic->log_dst.val = op_val;
            break;
        case DFR_OFFSET:
@@ -1447,84 +1341,67 @@ static int apic_write(struct guest_info * core, addr_t guest_addr, void * src, u
 
            // Action Registers
        case EOI_OFFSET:
-           // do eoi (we already have the lock)
+           // do eoi 
            apic_do_eoi(apic);
            break;
 
        case INT_CMD_LO_OFFSET: {
-           // execute command (we already have the lock)
+           // execute command 
 
            struct int_cmd_reg tmp_icr;
 
            apic->int_cmd.lo = op_val;
 
-           tmp_icr=apic->int_cmd;
+           tmp_icr = apic->int_cmd;
 
            //      V3_Print("apic %u: core %u: sending cmd 0x%llx to apic %u\n", 
-           //       apic->lapic_id.val, core->cpu_id,
+           //       apic->lapic_id.val, core->vcpu_id,
            //       apic->int_cmd.val, apic->int_cmd.dst);
 
-           apic->in_icr=0;
-
-           v3_unlock(apic->lock);
-
-           // route_ipi is responsible for locking apics, so we go in unlocked)
            if (route_ipi(apic_dev, apic, &tmp_icr) == -1) { 
                PrintError("IPI Routing failure\n");
-               goto apic_write_out_bad;
+               return -1;
            }
 
-           //      v3_lock(apic->lock);  // expected for leaving this function
-
-       }
            break;
-
+       }
        case INT_CMD_HI_OFFSET: {
-           // already have the lock
-           if (apic->in_icr) { 
-               PrintError("apic %u: core %u: writing command high=0x%x while in_icr=1\n", apic->lapic_id.val, core->cpu_id,apic->int_cmd.hi);
-           }
-
            apic->int_cmd.hi = op_val;
-           //V3_Print("apic %u: core %u: writing command high=0x%x\n", apic->lapic_id.val, core->cpu_id,apic->int_cmd.hi);
-           apic->in_icr=1;
-       }
-           break;
-
+           V3_Print("apic %u: core %u: writing command high=0x%x\n", apic->lapic_id.val, core->vcpu_id,apic->int_cmd.hi);
 
+           break;
+       }
        // Unhandled Registers
        case EXT_APIC_CMD_OFFSET:
        case SEOI_OFFSET:
        default:
            PrintError("apic %u: core %u: Write to Unhandled APIC Register: %x (ignored)\n", 
-                      apic->lapic_id.val, core->cpu_id, (uint32_t)reg_addr);
+                      apic->lapic_id.val, core->vcpu_id, (uint32_t)reg_addr);
 
-           goto apic_write_out_bad;
+           return -1;
     }
 
-    PrintDebug("apic %u: core %u: Write finished\n", apic->lapic_id.val, core->cpu_id);
+    PrintDebug("apic %u: core %u: Write finished\n", apic->lapic_id.val, core->vcpu_id);
 
-    // apic_write_out_good:
-    v3_unlock(apic->lock);
     return length;
 
- apic_write_out_bad:
-    v3_unlock(apic->lock);
-    return -1;
 }
 
 
 
 /* Interrupt Controller Functions */
 
-// internally used, expects caller to lock
-static int apic_intr_pending_nolock(struct guest_info * core, void * private_data) {
+
+static int apic_intr_pending(struct guest_info * core, void * private_data) {
     struct apic_dev_state * apic_dev = (struct apic_dev_state *)(private_data);
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]); 
+    struct apic_state * apic = &(apic_dev->apics[core->vcpu_id]); 
+
+    // drain irq QUEUE
+
     int req_irq = get_highest_irr(apic);
     int svc_irq = get_highest_isr(apic);
 
-    //    PrintDebug("apic %u: core %u: req_irq=%d, svc_irq=%d\n",apic->lapic_id.val,info->cpu_id,req_irq,svc_irq);
+    //    PrintDebug("apic %u: core %u: req_irq=%d, svc_irq=%d\n",apic->lapic_id.val,info->vcpu_id,req_irq,svc_irq);
 
     if ((req_irq >= 0) && 
        (req_irq > svc_irq)) {
@@ -1534,25 +1411,11 @@ static int apic_intr_pending_nolock(struct guest_info * core, void * private_dat
     return 0;
 }
 
-// externally visible, so must lock itself
-static int apic_intr_pending(struct guest_info * core, void * private_data) {
-    struct apic_dev_state * apic_dev = (struct apic_dev_state *)(private_data);
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]); 
-    int rc;
 
-    v3_lock(apic->lock);
-    
-    rc=apic_intr_pending_nolock(core,private_data);
-    
-    v3_unlock(apic->lock);
 
-    return rc;
-}
-
-// Internal - no lock
-static int apic_get_intr_number_nolock(struct guest_info * core, void * private_data) {
+static int apic_get_intr_number(struct guest_info * core, void * private_data) {
     struct apic_dev_state * apic_dev = (struct apic_dev_state *)(private_data);
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]); 
+    struct apic_state * apic = &(apic_dev->apics[core->vcpu_id]); 
     int req_irq = get_highest_irr(apic);
     int svc_irq = get_highest_isr(apic);
 
@@ -1566,25 +1429,7 @@ static int apic_get_intr_number_nolock(struct guest_info * core, void * private_
 }
 
 
-// Externally visible, so must lock itself
-static int apic_get_intr_number(struct guest_info * core, void * private_data) {
-    struct apic_dev_state * apic_dev = (struct apic_dev_state *)(private_data);
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]); 
-    int rc;
 
-    v3_lock(apic->lock);
-    
-    rc=apic_get_intr_number_nolock(core,private_data);
-
-    v3_unlock(apic->lock);
-
-    return rc;
-}
-
-
-//
-// Here there is no source APIC, so there is no need to lock it
-// Furthermore, the expectation is that route_ipi will lock the destiation apic
 int v3_apic_send_ipi(struct v3_vm_info * vm, struct v3_gen_ipi * ipi, void * dev_data) {
     struct apic_dev_state * apic_dev = (struct apic_dev_state *)
        (((struct vm_device *)dev_data)->private_data);
@@ -1600,7 +1445,7 @@ int v3_apic_send_ipi(struct v3_vm_info * vm, struct v3_gen_ipi * ipi, void * dev
     tmp_icr.dst_shorthand = ipi->dst_shorthand;
     tmp_icr.dst = ipi->dst;
 
-    // route_ipi is responsible for locking the destination apic
+
     return route_ipi(apic_dev, NULL, &tmp_icr);
 }
 
@@ -1613,33 +1458,30 @@ int v3_apic_raise_intr(struct v3_vm_info * vm, uint32_t irq, uint32_t dst, void
 
     PrintDebug("apic %u core ?: raising interrupt IRQ %u (dst = %u).\n", apic->lapic_id.val, irq, dst); 
 
-    v3_lock(apic->lock);
-
-    do_xcall=activate_apic_irq_nolock(apic, irq);
+    do_xcall = activate_apic_irq(apic, irq);
 
-    if (do_xcall<0) { 
+    if (do_xcall < 0) { 
        PrintError("Failed to activate apic irq\n");
-       v3_unlock(apic->lock);
        return -1;
     }
     
-    if (do_xcall>0 && (V3_Get_CPU() != dst)) {
-#ifdef CONFIG_MULTITHREAD_OS
+    if (do_xcall > 0 && (V3_Get_CPU() != dst)) {
+#ifdef V3_CONFIG_MULTITHREAD_OS
        v3_interrupt_cpu(vm, dst, 0);
 #else
        V3_ASSERT(0);
 #endif
 
     }
-    v3_unlock(apic->lock);
+
     return 0;
 }
 
 
-// internal - caller must lock
-static int apic_begin_irq_nolock(struct guest_info * core, void * private_data, int irq) {
+
+static int apic_begin_irq(struct guest_info * core, void * private_data, int irq) {
     struct apic_dev_state * apic_dev = (struct apic_dev_state *)(private_data);
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]); 
+    struct apic_state * apic = &(apic_dev->apics[core->vcpu_id]); 
     int major_offset = (irq & ~0x00000007) >> 3;
     int minor_offset = irq & 0x00000007;
     uint8_t *req_location = apic->int_req_reg + major_offset;
@@ -1654,36 +1496,23 @@ static int apic_begin_irq_nolock(struct guest_info * core, void * private_data,
     } else {
        // do nothing... 
        //PrintDebug("apic %u: core %u: begin irq for %d ignored since I don't own it\n",
-       //         apic->lapic_id.val, core->cpu_id, irq);
+       //         apic->lapic_id.val, core->vcpu_id, irq);
     }
 
     return 0;
 }
 
-// Since this is called, externally, it should lock the apic
-static int apic_begin_irq(struct guest_info * core, void * private_data, int irq) {
-    struct apic_dev_state * apic_dev = (struct apic_dev_state *)(private_data);
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]); 
-    int rc;
-
-    v3_lock(apic->lock);
-
-    rc=apic_begin_irq_nolock(core,private_data,irq);
-
-    v3_unlock(apic->lock);
 
-    return rc;
-}
 
 
 
 /* Timer Functions */
-// Caller will lock the apic
-static void apic_update_time_nolock(struct guest_info * core, 
+
+static void apic_update_time(struct guest_info * core, 
                             uint64_t cpu_cycles, uint64_t cpu_freq, 
                             void * priv_data) {
     struct apic_dev_state * apic_dev = (struct apic_dev_state *)(priv_data);
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]); 
+    struct apic_state * apic = &(apic_dev->apics[core->vcpu_id]); 
 
     // The 32 bit GCC runtime is a pile of shit
 #ifdef __V3_64BIT__
@@ -1702,7 +1531,7 @@ static void apic_update_time_nolock(struct guest_info * core,
     if ((apic->tmr_init_cnt == 0) || 
        ( (apic->tmr_vec_tbl.tmr_mode == APIC_TMR_ONESHOT) &&
          (apic->tmr_cur_cnt == 0))) {
-       //PrintDebug("apic %u: core %u: APIC timer not yet initialized\n",apic->lapic_id.val,info->cpu_id);
+       //PrintDebug("apic %u: core %u: APIC timer not yet initialized\n",apic->lapic_id.val,info->vcpu_id);
        return;
     }
 
@@ -1734,7 +1563,7 @@ static void apic_update_time_nolock(struct guest_info * core,
            break;
        default:
            PrintError("apic %u: core %u: Invalid Timer Divider configuration\n",
-                      apic->lapic_id.val, core->cpu_id);
+                      apic->lapic_id.val, core->vcpu_id);
            return;
     }
 
@@ -1749,18 +1578,18 @@ static void apic_update_time_nolock(struct guest_info * core,
 
        // raise irq
        PrintDebug("apic %u: core %u: Raising APIC Timer interrupt (periodic=%d) (icnt=%d) (div=%d)\n",
-                  apic->lapic_id.val, core->cpu_id,
+                  apic->lapic_id.val, core->vcpu_id,
                   apic->tmr_vec_tbl.tmr_mode, apic->tmr_init_cnt, shift_num);
 
-       if (apic_intr_pending_nolock(core, priv_data)) {
+       if (apic_intr_pending(core, priv_data)) {
            PrintDebug("apic %u: core %u: Overriding pending IRQ %d\n", 
-                      apic->lapic_id.val, core->cpu_id, 
+                      apic->lapic_id.val, core->vcpu_id, 
                       apic_get_intr_number(core, priv_data));
        }
 
-       if (activate_internal_irq_nolock(apic, APIC_TMR_INT) == -1) {
+       if (activate_internal_irq(apic, APIC_TMR_INT) == -1) {
            PrintError("apic %u: core %u: Could not raise Timer interrupt\n",
-                      apic->lapic_id.val, core->cpu_id);
+                      apic->lapic_id.val, core->vcpu_id);
        }
     
        if (apic->tmr_vec_tbl.tmr_mode == APIC_TMR_PERIODIC) {
@@ -1773,21 +1602,6 @@ static void apic_update_time_nolock(struct guest_info * core,
 }
 
 
-static void apic_update_time(struct guest_info * core, 
-                            uint64_t cpu_cycles, uint64_t cpu_freq, 
-                            void * priv_data) {
-    struct apic_dev_state * apic_dev = (struct apic_dev_state *)(priv_data);
-    struct apic_state * apic = &(apic_dev->apics[core->cpu_id]); 
-
-    v3_lock(apic->lock);
-
-    apic_update_time_nolock(core,cpu_cycles,cpu_freq,priv_data);
-
-    v3_unlock(apic->lock);
-    
-    return;
-}
-
 static struct intr_ctrl_ops intr_ops = {
     .intr_pending = apic_intr_pending,
     .get_intr_number = apic_get_intr_number,
@@ -1847,8 +1661,6 @@ static int apic_init(struct v3_vm_info * vm, v3_cfg_tree_t * cfg) {
 
     apic_dev->num_apics = vm->num_cores;
 
-    //v3_lock_init(&(apic_dev->ipi_lock));
-
     struct vm_device * dev = v3_add_device(vm, dev_id, &dev_ops, apic_dev);
 
     if (dev == NULL) {
@@ -1876,12 +1688,12 @@ static int apic_init(struct v3_vm_info * vm, v3_cfg_tree_t * cfg) {
            return -1;
        }
 
-       v3_hook_full_mem(vm, core->cpu_id, apic->base_addr, apic->base_addr + PAGE_SIZE_4KB, apic_read, apic_write, apic_dev);
+       v3_hook_full_mem(vm, core->vcpu_id, apic->base_addr, apic->base_addr + PAGE_SIZE_4KB, apic_read, apic_write, apic_dev);
 
        PrintDebug("apic %u: (setup device): done, my id is %u\n", i, apic->lapic_id.val);
     }
 
-#ifdef CONFIG_DEBUG_APIC
+#ifdef V3_CONFIG_DEBUG_APIC
     for (i = 0; i < vm->num_cores; i++) {
        struct apic_state * apic = &(apic_dev->apics[i]);
        PrintDebug("apic: sanity check: apic %u (at %p) has id %u and msr value %llx and core at %p\n",