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.


Fix to VNET bug when vnet_host_hooks is not properly initiated
[palacios-OLD.git] / palacios / src / devices / apic.c
index c6441fb..5a2856a 100644 (file)
@@ -11,7 +11,8 @@
  * Copyright (c) 2008, The V3VEE Project <http://www.v3vee.org> 
  * All rights reserved.
  *
- * Author: Jack Lange <jarusl@cs.northwestern.edu>
+ * Authors: Jack Lange <jarusl@cs.northwestern.edu>
+ *          Peter Dinda <pdinda@northwestern.edu> (SMP)
  *
  * This is free software.  You are permitted to use,
  * redistribute, and modify it as specified in the file "V3VEE_LICENSE".
 #include <palacios/vmm_types.h>
 
 
-#ifndef CONFIG_DEBUG_APIC
+#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 V3_CONFIG_DEBUG_APIC
 #undef PrintDebug
 #define PrintDebug(fmt, args...)
-#endif
+#else
 
-#ifdef CONFIG_DEBUG_APIC
-static char *shorthand_str[] = { 
+static char * shorthand_str[] = { 
     "(no shorthand)",
     "(self)",
     "(all)",
     "(all-but-me)",
 };
 
-static char *deliverymode_str[] = { 
+static char * deliverymode_str[] = { 
     "(fixed)",
     "(lowest priority)",
     "(SMI)",
@@ -50,17 +66,29 @@ static char *deliverymode_str[] = {
     "(Start Up)",
     "(ExtInt)",
 };
+
 #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;
 
 #define APIC_FIXED_DELIVERY  0x0
+#define APIC_LOWEST_DELIVERY 0x1
 #define APIC_SMI_DELIVERY    0x2
+#define APIC_RES1_DELIVERY   0x3
 #define APIC_NMI_DELIVERY    0x4
 #define APIC_INIT_DELIVERY   0x5
+#define APIC_SIPI_DELIVERY   0x6
 #define APIC_EXTINT_DELIVERY 0x7
 
+#define APIC_SHORTHAND_NONE        0x0
+#define APIC_SHORTHAND_SELF        0x1
+#define APIC_SHORTHAND_ALL         0x2
+#define APIC_SHORTHAND_ALL_BUT_ME  0x3
+
+#define APIC_DEST_PHYSICAL    0x0
+#define APIC_DEST_LOGICAL     0x1
+
 
 #define BASE_ADDR_MSR     0x0000001B
 #define DEFAULT_BASE_ADDR 0xfee00000
@@ -135,10 +163,6 @@ typedef enum { APIC_TMR_INT, APIC_THERM_INT, APIC_PERF_INT,
 #define EXT_INT_LOC_VEC_TBL_OFFSET2       0x520   // 0x500 - 0x530
 #define EXT_INT_LOC_VEC_TBL_OFFSET3       0x530   // 0x500 - 0x530
 
-
-
-
-
 struct apic_msr {
     union {
        uint64_t value;
@@ -147,15 +171,17 @@ struct apic_msr {
            uint8_t bootstrap_cpu : 1;
            uint8_t rsvd2         : 2;
            uint8_t apic_enable   : 1;
-           uint64_t base_addr   : 40;
-           uint32_t rsvd3         : 12;
+           uint64_t base_addr    : 40;
+           uint32_t rsvd3        : 12;
        } __attribute__((packed));
     } __attribute__((packed));
 } __attribute__((packed));
 
 
 
-typedef enum {INIT, SIPI, STARTED} ipi_state_t; 
+typedef enum {INIT_ST, 
+             SIPI, 
+             STARTED} ipi_state_t; 
 
 struct apic_dev_state;
 
@@ -209,9 +235,16 @@ struct apic_state {
 
     struct guest_info * core;
 
+    void * controller_handle;
+
+    struct v3_timer * timer;
+
+    v3_lock_t state_lock;
+    struct v3_queue irq_queue;
+
     uint32_t eoi;
 
-    v3_lock_t  lock;
+
 };
 
 
@@ -219,15 +252,18 @@ struct apic_state {
 
 struct apic_dev_state {
     int num_apics;
-
+  
     struct apic_state apics[0];
 } __attribute__((packed));
 
 
 
+
+
 static int apic_read(struct guest_info * core, addr_t guest_addr, void * dst, uint_t length, void * priv_data);
 static int apic_write(struct guest_info * core, addr_t guest_addr, void * src, uint_t length, void * priv_data);
 
+// No lcoking done
 static void init_apic_state(struct apic_state * apic, uint32_t id) {
     apic->base_addr = DEFAULT_BASE_ADDR;
 
@@ -259,7 +295,7 @@ static void init_apic_state(struct apic_state * apic, uint32_t id) {
 
     apic->lapic_id.val = id;
     
-    apic->ipi_state = INIT;
+    apic->ipi_state = INIT_ST;
 
     // The P6 has 6 LVT entries, so we set the value to (6-1)...
     apic->apic_ver.val = 0x80050010;
@@ -284,53 +320,59 @@ 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));
+
+    v3_init_queue(&(apic->irq_queue));
+
+
 }
 
 
 
 
+
 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;
 }
 
 
 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, apic->base_addr + PAGE_SIZE_4KB, apic_read, apic_write, apic_dev) == -1) {
+    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;
 }
 
@@ -344,25 +386,26 @@ static int activate_apic_irq(struct apic_state * apic, uint32_t irq_num) {
     uint8_t flag = 0x1 << minor_offset;
 
 
-
-    if (irq_num <= 15) {
-       PrintError("apic %u: core ?: Attempting to raise an invalid interrupt: %d\n", apic->lapic_id.val,irq_num);
+    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->vcpu_id, irq_num);
        return -1;
     }
 
 
-    PrintDebug("apic %u: core ?: Raising APIC IRQ %d\n", apic->lapic_id.val, 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) {
-       //V3_Print("Interrupts coallescing\n");
+       PrintDebug("Interrupt %d  coallescing\n", irq_num);
+       return 0;
     }
 
     if (*en_location & flag) {
        *req_location |= flag;
+       return 1;
     } else {
-       PrintDebug("apic %u: core ?: Interrupt  not enabled... %.2x\n", 
-                  apic->lapic_id.val, *en_location);
-       return 0;
+       PrintDebug("apic %u: core %d: Interrupt  not enabled... %.2x\n", 
+                  apic->lapic_id.val, apic->core->vcpu_id, *en_location);
     }
 
     return 0;
@@ -428,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)) {
@@ -509,17 +552,17 @@ static int activate_internal_irq(struct apic_state * apic, apic_irq_type_t int_t
 static inline int should_deliver_cluster_ipi(struct guest_info * dst_core, 
                                             struct apic_state * dst_apic, uint8_t mda) {
 
-    if         ( ((mda & 0xf0) == (dst_apic->log_dst.dst_log_id & 0xf0)) &&     // (I am in the cluster and
-         ((mda & 0x0f) & (dst_apic->log_dst.dst_log_id & 0x0f)) ) {  //  I am in the set)
+    if         ( ((mda & 0xf0) == (dst_apic->log_dst.dst_log_id & 0xf0)) &&  /* (I am in the cluster and */
+         ((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;
     }
@@ -531,14 +574,17 @@ static inline int should_deliver_flat_ipi(struct guest_info * dst_core,
     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;
+
+       return 1;
+
   } 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;
+       return 0;
   }
 }
 
@@ -547,60 +593,77 @@ static inline int should_deliver_flat_ipi(struct guest_info * dst_core,
 static int should_deliver_ipi(struct guest_info * dst_core, 
                              struct apic_state * dst_apic, uint8_t mda) {
 
-    if (mda == 0xff) {
-       // always deliver broadcast
-       return 1;
-    }
 
     if (dst_apic->dst_fmt.model == 0xf) {
-       return should_deliver_cluster_ipi(dst_core, dst_apic, mda);
-    } else if (dst_apic->dst_fmt.model == 0x0) {
+
+       if (mda == 0xff) {
+           /* always deliver broadcast */
+           return 1;
+       }
+
        return should_deliver_flat_ipi(dst_core, dst_apic, mda);
+
+    } else if (dst_apic->dst_fmt.model == 0x0) {
+
+       if (mda == 0xff) {
+           /*  always deliver broadcast */
+           return 1;
+       }
+
+       return should_deliver_cluster_ipi(dst_core, dst_apic, mda);
+
     } 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;
     }
 }
 
 
-static int deliver_ipi(struct guest_info * core, 
-                      struct apic_state * src_apic, 
+// Only the src_apic pointer is used
+static int deliver_ipi(struct apic_state * src_apic, 
                       struct apic_state * dst_apic, 
                       uint32_t vector, uint8_t del_mode) {
 
+
     struct guest_info * dst_core = dst_apic->core;
 
+
     switch (del_mode) {
 
-       case 0:  //fixed
-       case 1: // lowest priority
-           PrintDebug(" delivering IRQ to core %u\n", dst_core->cpu_id); 
+       case APIC_FIXED_DELIVERY:  
+       case APIC_LOWEST_DELIVERY: {
+           // lowest priority - 
+           // caller needs to have decided which apic to deliver to!
+
+           int do_xcall;
+
+           PrintDebug("delivering IRQ %d to core %u\n", vector, dst_core->vcpu_id); 
+
+           do_xcall = activate_apic_irq(dst_apic, vector);
+           
 
-           activate_apic_irq(dst_apic, vector);
 
            if (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
-               PrintDebug(" non-local core, forcing it to exit\n"); 
-
-               v3_interrupt_cpu(core->vm_info, dst_core->cpu_id, 0);
+               PrintDebug(" non-local core with new interrupt, forcing it to exit now\n"); 
+
+#ifdef V3_CONFIG_MULTITHREAD_OS
+               v3_interrupt_cpu(dst_core->vm_info, dst_core->pcpu_id, 0);
+#endif
            }
 
            break;
-       case 5: { //INIT
+       }
+       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) { 
-               PrintError(" Warning: core %u is not in INIT state (mode = %d), ignored\n",
-                          dst_core->cpu_id, dst_core->cpu_mode);
+           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->vcpu_id, dst_apic->ipi_state);
                // Only a warning, since INIT INIT SIPI is common
                break;
            }
@@ -618,12 +681,12 @@ static int deliver_ipi(struct guest_info * core,
 
            break;                                                      
        }
-       case 6: { //SIPI
+       case APIC_SIPI_DELIVERY: { 
 
            // 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_core->cpu_mode);
+                          dst_core->vcpu_id, dst_apic->ipi_state);
                break;
            }
 
@@ -643,60 +706,61 @@ static int deliver_ipi(struct guest_info * core,
            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
            dst_core->core_run_state = CORE_RUNNING;  // note: locking should not be needed here
            dst_apic->ipi_state = STARTED;
-
+           
            // As with INIT, we should not need to do anything else
-
+           
            PrintDebug(" SIPI delivery done\n");
-
+           
            break;                                                      
        }
-       case 2: // SMI                  
-       case 3: // reserved                                             
-       case 4: // NMI                                  
-       case 7: // ExtInt
+       case APIC_SMI_DELIVERY: 
+       case APIC_RES1_DELIVERY: // reserved                                            
+       case APIC_NMI_DELIVERY:
+       case APIC_EXTINT_DELIVERY: // ExtInt
        default:
            PrintError("IPI %d delivery is unsupported\n", del_mode); 
            return -1;
     }
-
+    
     return 0;
-
+    
 }
 
+static struct apic_state * find_physical_apic(struct apic_dev_state *apic_dev, struct int_cmd_reg *icr)
+{
+    int i;
+    
+    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]);
+       }
+    }
 
-static int route_ipi(struct guest_info * core, struct apic_dev_state * apic_dev,
-                    struct apic_state * src_apic,  uint32_t icr_val) {
-    struct int_cmd_reg * icr = (struct int_cmd_reg *)&icr_val;
-    struct apic_state * dest_apic = NULL;
-
-    PrintDebug("route_ipi: src_apic=%p, icr_data=%x", 
-              src_apic, icr_val);
-
-
-    // initial sanity checks
-    if (src_apic == NULL) { 
-       PrintError("route_ipi: Apparently sending from unregistered apic id=%d\n", 
-                  src_apic->core->cpu_id);
-       return -1;
+    for (i = 0; i < apic_dev->num_apics; i++) { 
+       if (apic_dev->apics[i].lapic_id.val == icr->dst) { 
+           return &(apic_dev->apics[i]);
+       }
     }
+    
+    return NULL;
 
+}
 
-    if ((icr->dst_mode == 0) && (icr->dst >= apic_dev->num_apics)) { 
-       PrintError("route_ipi: Attempted send to unregistered apic id=%u\n", 
-                  icr->dst);
-       return -1;
-    }
 
-    dest_apic =  &(apic_dev->apics[icr->dst]);
+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;
 
 
-    PrintDebug("route_ipi: IPI %s %u from apic %p to %s %s %u (icr=0x%llx)\n",
+    PrintDebug("apic: IPI %s %u from apic %p to %s %s %u (icr=0x%llx)\n",
               deliverymode_str[icr->del_mode], 
               icr->vec, 
               src_apic,               
@@ -708,99 +772,168 @@ static int route_ipi(struct guest_info * core, struct apic_dev_state * apic_dev,
 
     switch (icr->dst_shorthand) {
 
-       case 0:  // no shorthand
-           if (icr->dst_mode == 0) { 
-               // physical delivery
+       case APIC_SHORTHAND_NONE:  // no shorthand
+           if (icr->dst_mode == APIC_DEST_PHYSICAL) { 
+
+               dest_apic = find_physical_apic(apic_dev, icr);
+               
+               if (dest_apic == NULL) { 
+                   PrintError("apic: Attempted send to unregistered apic id=%u\n", icr->dst);
+                   return -1;
+               }
 
-               if (deliver_ipi(core, src_apic, dest_apic, 
+               if (deliver_ipi(src_apic, dest_apic, 
                                icr->vec, icr->del_mode) == -1) {
-                   PrintError("Error: Could not deliver IPI\n");
+                   PrintError("apic: Could not deliver IPI\n");
                    return -1;
                }
 
-           } else {
-               // logical delivery
-               int i;
-               uint8_t mda = icr->dst;
-
-               for (i = 0; i < apic_dev->num_apics; i++) { 
-                    dest_apic = &(apic_dev->apics[i]);
-                    int del_flag = should_deliver_ipi(dest_apic->core, dest_apic, mda);
-                    
-                    if (del_flag == -1) {
-                        PrintError("Error checking delivery mode\n");
-                        return -1;
-                    } else if (del_flag == 1) {
-                       if (deliver_ipi(core, src_apic, dest_apic, 
+
+               PrintDebug("apic: done\n");
+
+           } else if (icr->dst_mode == APIC_DEST_LOGICAL) {
+               
+               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
+
+                   
+                   for (i = 0; i < apic_dev->num_apics; i++) { 
+                       int del_flag = 0;
+                       
+                       dest_apic = &(apic_dev->apics[i]);
+                       
+                       del_flag = should_deliver_ipi(dest_apic->core, dest_apic, mda);
+                       
+                       if (del_flag == -1) {
+
+                           PrintError("apic: Error checking delivery mode\n");
+                           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");
+                               return -1;
+                           }
+                       }
+                   }
+               } else {  //APIC_LOWEST_DELIVERY
+                   int i;
+                   struct apic_state * cur_best_apic = NULL;
+                   uint8_t mda = icr->dst;
+                  
+                   // logical, lowest priority
+
+                   for (i = 0; i < apic_dev->num_apics; i++) { 
+                       int del_flag = 0;
+
+                       dest_apic = &(apic_dev->apics[i]);
+                       
+                       del_flag = should_deliver_ipi(dest_apic->core, dest_apic, mda);
+                       
+                       if (del_flag == -1) {
+                           PrintError("apic: Error checking delivery mode\n");
+
+                           return -1;
+                       } else if (del_flag == 1) {
+                           // update priority for lowest priority scan
+                           if (cur_best_apic == 0) {
+                               cur_best_apic = dest_apic;  
+                           } else if (dest_apic->task_prio.val < cur_best_apic->task_prio.val) {
+                               cur_best_apic = dest_apic;
+                           } 
+                       }                       
+                   }
+
+                   // now we will deliver to the best one if it exists
+                   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("Error: Could not deliver IPI\n");
+                           PrintError("apic: Error: Could not deliver IPI\n");
                            return -1;
                        }
+                       //V3_Print("apic: logical, lowest priority delivery to apic %u\n",cur_best_apic->lapic_id.val);
                    }
                }
            }
-           
+
            break;
            
-       case 1:  // self
+       case APIC_SHORTHAND_SELF:  // self
+
+           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;
+           }
+
 
-           if (icr->dst_mode == 0) { 
-               if (deliver_ipi(core, src_apic, src_apic, icr->vec, icr->del_mode) == -1) {
-                   PrintError("Could not deliver IPI\n");
+
+           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");
+                   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");
                    return -1;
                }
-           } else {
-               // logical delivery
-               PrintError("icc_bus: use of logical delivery in self is not yet supported.\n");
-               return -1;
            }
+
            break;
            
-       case 2: 
-       case 3: { // all and all-but-me
-           // assuming that logical verus physical doesn't matter
-           // although it is odd that both are used
+       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 == 2)) { 
-                   if (deliver_ipi(core, src_apic, dest_apic, icr->vec, icr->del_mode) == -1) {
-                       PrintError("Error: Could not deliver IPI\n");
+               
+               if ((dest_apic != src_apic) || (icr->dst_shorthand == APIC_SHORTHAND_ALL)) { 
+                   if (deliver_ipi(src_apic, dest_apic, icr->vec, icr->del_mode) == -1) {
+                       PrintError("apic: Error: Could not deliver IPI\n");
                        return -1;
                    }
                }
-           }   
+           }
 
            break;
        }
        default:
-           PrintError("Error routing IPI, invalid Mode (%d)\n", icr->dst_shorthand);
+           PrintError("apic: Error routing IPI, invalid Mode (%d)\n", icr->dst_shorthand);
            return -1;
     }
-    
-
     return 0;
 }
 
 
-
+// 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;
 
 
     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);
-
+                  apic->lapic_id.val, core->vcpu_id, apic->base_addr_msr.value);
        return -1;
     }
 
@@ -1013,7 +1146,7 @@ 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);
+                      apic->lapic_id.val, core->vcpu_id, (uint32_t)reg_addr);
            return -1;
     }
 
@@ -1036,12 +1169,12 @@ 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);
+                  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);
 
     return length;
 }
@@ -1052,27 +1185,27 @@ 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;
 
     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("Write to address space (%p) (val=%x)\n", 
-              (void *)guest_addr, *(uint32_t *)src);
+    PrintDebug("apic %u: core %u: write to address space (%p) (val=%x)\n", 
+              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);
+                  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);
+                  apic->lapic_id.val, length, core->vcpu_id);
        return -1;
     }
 
@@ -1108,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);
-           //  return -1;
+                      apic->lapic_id.val, core->vcpu_id, (void *)reg_addr);
 
            break;
 
            // Data registers
        case APIC_ID_OFFSET:
-           PrintDebug("apic %u: core %u: my id is being changed to %u\n", 
-                      apic->lapic_id.val, core->cpu_id, op_val);
+           //V3_Print("apic %u: core %u: my id is being changed to %u\n", 
+           //       apic->lapic_id.val, core->vcpu_id, op_val);
 
            apic->lapic_id.val = op_val;
            break;
@@ -1125,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:
@@ -1209,56 +1341,67 @@ static int apic_write(struct guest_info * core, addr_t guest_addr, void * src, u
 
            // Action Registers
        case EOI_OFFSET:
-           // do eoi
+           // do eoi 
            apic_do_eoi(apic);
            break;
 
-       case INT_CMD_LO_OFFSET:
+       case INT_CMD_LO_OFFSET: {
+           // execute command 
+
+           struct int_cmd_reg tmp_icr;
+
            apic->int_cmd.lo = op_val;
 
-           PrintDebug("apic %u: core %u: sending cmd 0x%llx to apic %u\n", 
-                      apic->lapic_id.val, core->cpu_id,
-                      apic->int_cmd.val, apic->int_cmd.dst);
+           tmp_icr = apic->int_cmd;
+
+           //      V3_Print("apic %u: core %u: sending cmd 0x%llx to apic %u\n", 
+           //       apic->lapic_id.val, core->vcpu_id,
+           //       apic->int_cmd.val, apic->int_cmd.dst);
 
-           if (route_ipi(core, apic_dev, apic, apic->int_cmd.val) == -1) { 
+           if (route_ipi(apic_dev, apic, &tmp_icr) == -1) { 
                PrintError("IPI Routing failure\n");
                return -1;
            }
 
            break;
-
-       case INT_CMD_HI_OFFSET:
+       }
+       case INT_CMD_HI_OFFSET: {
            apic->int_cmd.hi = op_val;
-           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);
 
            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);
 
     return length;
+
 }
 
 
 
 /* Interrupt Controller Functions */
 
-// returns 1 if an interrupt is pending, 0 otherwise
+
 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)) {
@@ -1268,9 +1411,11 @@ static int apic_intr_pending(struct guest_info * core, void * private_data) {
     return 0;
 }
 
+
+
 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);
 
@@ -1284,17 +1429,49 @@ static int apic_get_intr_number(struct guest_info * core, void * private_data) {
 }
 
 
-int v3_apic_raise_intr(struct v3_vm_info * vm, struct vm_device * dev, 
-                      uint32_t irq, uint32_t dst) {
-    struct apic_dev_state * apic_dev = (struct apic_dev_state *)(dev->private_data);
+
+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);
+    struct int_cmd_reg tmp_icr;
+
+    // zero out all the fields
+    tmp_icr.val = 0;
+
+    tmp_icr.vec = ipi->vector;
+    tmp_icr.del_mode = ipi->mode;
+    tmp_icr.dst_mode = ipi->logical;
+    tmp_icr.trig_mode = ipi->trigger_mode;
+    tmp_icr.dst_shorthand = ipi->dst_shorthand;
+    tmp_icr.dst = ipi->dst;
+
+
+    return route_ipi(apic_dev, NULL, &tmp_icr);
+}
+
+
+int v3_apic_raise_intr(struct v3_vm_info * vm, uint32_t irq, uint32_t dst, void * dev_data) {
+    struct apic_dev_state * apic_dev = (struct apic_dev_state *)
+       (((struct vm_device*)dev_data)->private_data);
     struct apic_state * apic = &(apic_dev->apics[dst]); 
+    int do_xcall;
 
     PrintDebug("apic %u core ?: raising interrupt IRQ %u (dst = %u).\n", apic->lapic_id.val, irq, dst); 
 
-    activate_apic_irq(apic, irq);
+    do_xcall = activate_apic_irq(apic, irq);
 
-    if (V3_Get_CPU() != dst) {
+    if (do_xcall < 0) { 
+       PrintError("Failed to activate apic irq\n");
+       return -1;
+    }
+    
+    if (do_xcall > 0 && (V3_Get_CPU() != dst)) {
+#ifdef V3_CONFIG_MULTITHREAD_OS
        v3_interrupt_cpu(vm, dst, 0);
+#else
+       V3_ASSERT(0);
+#endif
+
     }
 
     return 0;
@@ -1304,11 +1481,11 @@ int v3_apic_raise_intr(struct v3_vm_info * vm, struct vm_device * dev,
 
 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;
-    uint8_t * svc_location = apic->int_svc_reg + major_offset;
+    uint8_t *req_location = apic->int_req_reg + major_offset;
+    uint8_t *svc_location = apic->int_svc_reg + major_offset;
     uint8_t flag = 0x01 << minor_offset;
 
     if (*req_location & flag) {
@@ -1319,7 +1496,7 @@ static int apic_begin_irq(struct guest_info * core, void * private_data, int irq
     } 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;
@@ -1328,12 +1505,14 @@ static int apic_begin_irq(struct guest_info * core, void * private_data, int irq
 
 
 
+
 /* Timer Functions */
+
 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__
@@ -1352,7 +1531,7 @@ static void apic_update_time(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;
     }
 
@@ -1384,7 +1563,7 @@ static void apic_update_time(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;
     }
 
@@ -1399,18 +1578,18 @@ static void apic_update_time(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(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(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) {
@@ -1419,7 +1598,7 @@ static void apic_update_time(struct guest_info * core,
        }
     }
 
-
+    return;
 }
 
 
@@ -1430,36 +1609,46 @@ static struct intr_ctrl_ops intr_ops = {
 };
 
 
-static struct vm_timer_ops timer_ops = {
+static struct v3_timer_ops timer_ops = {
     .update_timer = apic_update_time,
 };
 
 
 
 
-static int apic_free(struct vm_device * dev) {
+static int apic_free(struct apic_dev_state * apic_dev) {
+    int i = 0;
+    struct v3_vm_info * vm = NULL;
+
+    for (i = 0; i < apic_dev->num_apics; i++) {
+       struct apic_state * apic = &(apic_dev->apics[i]);
+       struct guest_info * core = apic->core;
+       
+       vm = core->vm_info;
+
+       v3_remove_intr_controller(core, apic->controller_handle);
+
+       if (apic->timer) {
+           v3_remove_timer(core, apic->timer);
+       }
 
-    /* TODO: This should crosscall to force an unhook on each CPU */
+       // unhook memory
 
-    //   struct apic_state * apic = (struct apic_state *)dev->private_data;
+    }
 
-    v3_unhook_msr(dev->vm, BASE_ADDR_MSR);
+    v3_unhook_msr(vm, BASE_ADDR_MSR);
 
+    V3_Free(apic_dev);
     return 0;
 }
 
 
 static struct v3_device_ops dev_ops = {
-    .free = apic_free,
-    .reset = NULL,
-    .start = NULL,
-    .stop = NULL,
+    .free = (int (*)(void *))apic_free,
 };
 
 
 
-
-
 static int apic_init(struct v3_vm_info * vm, v3_cfg_tree_t * cfg) {
     char * dev_id = v3_cfg_val(cfg, "ID");
     struct apic_dev_state * apic_dev = NULL;
@@ -1472,10 +1661,11 @@ static int apic_init(struct v3_vm_info * vm, v3_cfg_tree_t * cfg) {
 
     apic_dev->num_apics = vm->num_cores;
 
-    struct vm_device * dev = v3_allocate_device(dev_id, &dev_ops, apic_dev);
+    struct vm_device * dev = v3_add_device(vm, dev_id, &dev_ops, apic_dev);
 
-    if (v3_attach_device(vm, dev) == -1) {
+    if (dev == NULL) {
        PrintError("apic: Could not attach device %s\n", dev_id);
+       V3_Free(apic_dev);
        return -1;
     }
 
@@ -1488,20 +1678,26 @@ static int apic_init(struct v3_vm_info * vm, v3_cfg_tree_t * cfg) {
 
        init_apic_state(apic, i);
 
-       v3_register_intr_controller(core, &intr_ops, apic_dev);
+       apic->controller_handle = v3_register_intr_controller(core, &intr_ops, apic_dev);
+
+       apic->timer = v3_add_timer(core, &timer_ops, apic_dev);
 
-       v3_add_timer(core, &timer_ops, apic_dev);
+       if (apic->timer == NULL) {
+           PrintError("APIC: Failed to attach timer to core %d\n", i);
+           v3_remove_device(dev);
+           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\n",
-                  i, apic, apic->lapic_id.val, apic->base_addr_msr.value);
+       PrintDebug("apic: sanity check: apic %u (at %p) has id %u and msr value %llx and core at %p\n",
+                  i, apic, apic->lapic_id.val, apic->base_addr_msr.value,apic->core);
     }
 #endif