[PATCH 0 of 8] Allow building xen with clang/llvm

[PATCH 0 of 8] Allow building xen with clang/llvm

[Tim Deegan]

This series of patches allows Xen builds with clang/llvm as well as
GCC.  It only fixes Xen, not tools &c, and only 64-bit, so far. 

Building Xen with clang isn't useful in itself (and indeed produces 
a hypervisor that can be up to 10% slower for a default -O2 build);
rather it's a stepping-stone to doing full link-time optimizations
on the LLVM bitcode of the whole hypervisor.  I will post a second 
series to enable that, but it's not so clean as this one. 

I think all the changes in this series are reasonable and won't 
inconvenience any normal users, but obviously they're meant for
the xen-unstable tree only. 

Cheers,

Tim.

[PATCH 1 of 8] x86: make spinlock's 16-bit asm operand explicitly 16-bit

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 95 bytes --]

This is needed to compile xen with clang.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>



[-- Attachment #2: xen-llvm.hg-1.patch --]
[-- Type: text/x-patch, Size: 788 bytes --]

# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1299496871 0
# Node ID c574ec6f7c8633397804aee7c96f428779688ce9
# Parent  bbc03993d31f5b9c56c44b068cdd00f08971c21c
x86: make spinlock's 16-bit asm operand explicitly 16-bit.
This is needed to compile xen with clang.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r bbc03993d31f -r c574ec6f7c86 xen/include/asm-x86/spinlock.h
--- a/xen/include/asm-x86/spinlock.h	Sat Mar 05 16:02:33 2011 +0000
+++ b/xen/include/asm-x86/spinlock.h	Mon Mar 07 11:21:11 2011 +0000
@@ -27,7 +27,7 @@ static always_inline int _raw_spin_trylo
     asm volatile (
         "xchgw %w0,%1"
         :"=r" (oldval), "=m" (lock->lock)
-        :"0" (0) : "memory" );
+        :"0" ((s16)0) : "memory" );
     return (oldval > 0);
 }
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 2 of 8] x86: add explicit size suffixes to some assembly instructions

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 95 bytes --]

This is needed to compile xen with clang.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>



[-- Attachment #2: xen-llvm.hg-2.patch --]
[-- Type: text/x-patch, Size: 4789 bytes --]

# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1299496871 0
# Node ID b7eaf2bcdca1307bf4eca8aac353d0f26076633b
# Parent  c574ec6f7c8633397804aee7c96f428779688ce9
x86: add explicit size suffixes to some assembly instructions.
This is needed to compile xen with clang.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r c574ec6f7c86 -r b7eaf2bcdca1 xen/arch/x86/acpi/suspend.c
--- a/xen/arch/x86/acpi/suspend.c	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/arch/x86/acpi/suspend.c	Mon Mar 07 11:21:11 2011 +0000
@@ -28,7 +28,7 @@ void save_rest_processor_state(void)
 
 #if defined(CONFIG_X86_64)
     asm volatile (
-        "mov %%ds,(%0); mov %%es,2(%0); mov %%fs,4(%0); mov %%gs,6(%0)"
+        "movw %%ds,(%0); movw %%es,2(%0); movw %%fs,4(%0); movw %%gs,6(%0)"
         : : "r" (saved_segs) : "memory" );
     rdmsrl(MSR_FS_BASE, saved_fs_base);
     rdmsrl(MSR_GS_BASE, saved_gs_base);
@@ -75,7 +75,7 @@ void restore_rest_processor_state(void)
     if ( !is_idle_vcpu(curr) )
     {
         asm volatile (
-            "mov (%0),%%ds; mov 2(%0),%%es; mov 4(%0),%%fs"
+            "movw (%0),%%ds; movw 2(%0),%%es; movw 4(%0),%%fs"
             : : "r" (saved_segs) : "memory" );
         do_set_segment_base(SEGBASE_GS_USER_SEL, saved_segs[3]);
     }
diff -r c574ec6f7c86 -r b7eaf2bcdca1 xen/arch/x86/x86_emulate/x86_emulate.c
--- a/xen/arch/x86/x86_emulate/x86_emulate.c	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c	Mon Mar 07 11:21:11 2011 +0000
@@ -2678,7 +2678,7 @@ x86_emulate(
                 emulate_fpu_insn_memsrc("fiaddl", src.val);
                 break;
             case 1: /* fimul m64i */
-                emulate_fpu_insn_memsrc("fimul", src.val);
+                emulate_fpu_insn_memsrc("fimuls", src.val);
                 break;
             case 2: /* ficom m64i */
                 emulate_fpu_insn_memsrc("ficoml", src.val);
@@ -2897,28 +2897,28 @@ x86_emulate(
             switch ( modrm_reg & 7 )
             {
             case 0: /* fiadd m16i */
-                emulate_fpu_insn_memsrc("fiadd", src.val);
+                emulate_fpu_insn_memsrc("fiadds", src.val);
                 break;
             case 1: /* fimul m16i */
-                emulate_fpu_insn_memsrc("fimul", src.val);
+                emulate_fpu_insn_memsrc("fimuls", src.val);
                 break;
             case 2: /* ficom m16i */
-                emulate_fpu_insn_memsrc("ficom", src.val);
+                emulate_fpu_insn_memsrc("ficoms", src.val);
                 break;
             case 3: /* ficomp m16i */
-                emulate_fpu_insn_memsrc("ficomp", src.val);
+                emulate_fpu_insn_memsrc("ficomps", src.val);
                 break;
             case 4: /* fisub m16i */
-                emulate_fpu_insn_memsrc("fisub", src.val);
+                emulate_fpu_insn_memsrc("fisubs", src.val);
                 break;
             case 5: /* fisubr m16i */
-                emulate_fpu_insn_memsrc("fisubr", src.val);
+                emulate_fpu_insn_memsrc("fisubrs", src.val);
                 break;
             case 6: /* fidiv m16i */
-                emulate_fpu_insn_memsrc("fidiv", src.val);
+                emulate_fpu_insn_memsrc("fidivs", src.val);
                 break;
             case 7: /* fidivr m16i */
-                emulate_fpu_insn_memsrc("fidivr", src.val);
+                emulate_fpu_insn_memsrc("fidivrs", src.val);
                 break;
             default:
                 goto cannot_emulate;
@@ -2950,25 +2950,25 @@ x86_emulate(
                 if ( (rc = ops->read(src.mem.seg, src.mem.off, &src.val,
                                      src.bytes, ctxt)) != 0 )
                     goto done;
-                emulate_fpu_insn_memsrc("fild", src.val);
+                emulate_fpu_insn_memsrc("filds", src.val);
                 break;
             case 1: /* fisttp m16i */
                 ea.bytes = 2;
                 dst = ea;
                 dst.type = OP_MEM;
-                emulate_fpu_insn_memdst("fisttp", dst.val);
+                emulate_fpu_insn_memdst("fisttps", dst.val);
                 break;
             case 2: /* fist m16i */
                 ea.bytes = 2;
                 dst = ea;
                 dst.type = OP_MEM;
-                emulate_fpu_insn_memdst("fist", dst.val);
+                emulate_fpu_insn_memdst("fists", dst.val);
                 break;
             case 3: /* fistp m16i */
                 ea.bytes = 2;
                 dst = ea;
                 dst.type = OP_MEM;
-                emulate_fpu_insn_memdst("fistp", dst.val);
+                emulate_fpu_insn_memdst("fistps", dst.val);
                 break;
             case 4: /* fbld m80dec */
                 ea.bytes = 10;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 3 of 8] x86: redefine a few empty macros as explicit nops

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 95 bytes --]

This is needed to compile xen with clang.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>



[-- Attachment #2: xen-llvm.hg-3.patch --]
[-- Type: text/x-patch, Size: 1196 bytes --]

# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1299496871 0
# Node ID 692a97ea255e8150f088cb9e71f32fd9f9825e06
# Parent  b7eaf2bcdca1307bf4eca8aac353d0f26076633b
x86: redefine a few empty macros as explicit nops.
This is needed to compile xen with clang.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r b7eaf2bcdca1 -r 692a97ea255e xen/include/asm-x86/apic.h
--- a/xen/include/asm-x86/apic.h	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/include/asm-x86/apic.h	Mon Mar 07 11:21:11 2011 +0000
@@ -6,7 +6,7 @@
 #include <asm/fixmap.h>
 #include <asm/msr.h>
 
-#define Dprintk(x...)
+#define Dprintk(x...) do {} while (0)
 
 /*
  * Debugging macros
diff -r b7eaf2bcdca1 -r 692a97ea255e xen/include/asm-x86/hvm/support.h
--- a/xen/include/asm-x86/hvm/support.h	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/include/asm-x86/hvm/support.h	Mon Mar 07 11:21:11 2011 +0000
@@ -61,7 +61,7 @@ extern unsigned int opt_hvm_debug_level;
                    ## _a);                                                    \
     } while (0)
 #else
-#define HVM_DBG_LOG(level, _f, _a...)
+#define HVM_DBG_LOG(level, _f, _a...) do {} while (0)
 #endif
 
 extern unsigned long hvm_io_bitmap[];

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 4 of 8] xen: adjust cpumask initializers to suit clang's incomplete gccisms

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 95 bytes --]

This is needed to compile xen with clang.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>



[-- Attachment #2: xen-llvm.hg-4.patch --]
[-- Type: text/x-patch, Size: 1526 bytes --]

# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1299496871 0
# Node ID fed25f206a372891b7a0f1368592e25eda479788
# Parent  692a97ea255e8150f088cb9e71f32fd9f9825e06
xen: adjust cpumask initializers to suit clang's incomplete gccisms.
This is needed to compile xen with clang.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r 692a97ea255e -r fed25f206a37 xen/arch/x86/io_apic.c
--- a/xen/arch/x86/io_apic.c	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/arch/x86/io_apic.c	Mon Mar 07 11:21:11 2011 +0000
@@ -1907,6 +1907,7 @@ static void __init check_timer(void)
     int apic1, pin1, apic2, pin2;
     int vector, ret;
     unsigned long flags;
+    cpumask_t mask_all = CPU_MASK_ALL;
 
     local_irq_save(flags);
 
@@ -1917,7 +1918,7 @@ static void __init check_timer(void)
     vector = FIRST_HIPRIORITY_VECTOR;
     clear_irq_vector(0);
 
-    if ((ret = bind_irq_vector(0, vector, (cpumask_t)CPU_MASK_ALL)))
+    if ((ret = bind_irq_vector(0, vector, mask_all)))
         printk(KERN_ERR"..IRQ0 is not set correctly with ioapic!!!, err:%d\n", ret);
     
     irq_desc[0].depth  = 0;
diff -r 692a97ea255e -r fed25f206a37 xen/include/xen/cpumask.h
--- a/xen/include/xen/cpumask.h	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/include/xen/cpumask.h	Mon Mar 07 11:21:11 2011 +0000
@@ -296,7 +296,7 @@ static inline const cpumask_t *cpumask_o
 
 #define CPU_MASK_NONE							\
 /*(cpumask_t)*/ { {							\
-	[0 ... BITS_TO_LONGS(NR_CPUS)-1] =  0UL				\
+	0UL								\
 } }
 
 #define CPU_MASK_CPU0							\

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 5 of 8] credit2: remove two nested functions, replacing them with static ones

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 150 bytes --]

and passing the outer scope's variables in explicitly.
This is needed to compile xen with clang.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>



[-- Attachment #2: xen-llvm.hg-5.patch --]
[-- Type: text/x-patch, Size: 11205 bytes --]

# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1299496871 0
# Node ID c72c074d0e084dcabb5314ac71e3b7730898c764
# Parent  fed25f206a372891b7a0f1368592e25eda479788
credit2: remove two nested functions, replacing them with static ones
and passing the outer scope's variables in explicitly.
This is needed to compile xen with clang.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r fed25f206a37 -r c72c074d0e08 xen/common/sched_credit2.c
--- a/xen/common/sched_credit2.c	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/common/sched_credit2.c	Mon Mar 07 11:21:11 2011 +0000
@@ -1109,86 +1109,96 @@ out_up:
     return new_cpu;
 }
 
+/* Working state of the load-balancing algorithm */
+typedef struct {
+    /* NB: Modified by consider() */
+    s_time_t load_delta;
+    struct csched_vcpu * best_push_svc, *best_pull_svc;
+    /* NB: Read by consider() */
+    struct csched_runqueue_data *lrqd;
+    struct csched_runqueue_data *orqd;                  
+} balance_state_t;
+
+static void consider(balance_state_t *st, 
+                     struct csched_vcpu *push_svc,
+                     struct csched_vcpu *pull_svc)
+{
+    s_time_t l_load, o_load, delta;
+
+    l_load = st->lrqd->b_avgload;
+    o_load = st->orqd->b_avgload;
+    if ( push_svc )
+    {
+        /* What happens to the load on both if we push? */
+        l_load -= push_svc->avgload;
+        o_load += push_svc->avgload;
+    }
+    if ( pull_svc )
+    {
+        /* What happens to the load on both if we pull? */
+        l_load += pull_svc->avgload;
+        o_load -= pull_svc->avgload;
+    }
+
+    delta = l_load - o_load;
+    if ( delta < 0 )
+        delta = -delta;
+
+    if ( delta < st->load_delta )
+    {
+        st->load_delta = delta;
+        st->best_push_svc=push_svc;
+        st->best_pull_svc=pull_svc;
+    }
+}
+
+
+void migrate(const struct scheduler *ops,
+             struct csched_vcpu *svc, 
+             struct csched_runqueue_data *trqd, 
+             s_time_t now)
+{
+    if ( test_bit(__CSFLAG_scheduled, &svc->flags) )
+    {
+        d2printk("d%dv%d %d-%d a\n", svc->vcpu->domain->domain_id, svc->vcpu->vcpu_id,
+                 svc->rqd->id, trqd->id);
+        /* It's running; mark it to migrate. */
+        svc->migrate_rqd = trqd;
+        set_bit(_VPF_migrating, &svc->vcpu->pause_flags);
+        set_bit(__CSFLAG_runq_migrate_request, &svc->flags);
+    }
+    else
+    {
+        int on_runq=0;
+        /* It's not running; just move it */
+        d2printk("d%dv%d %d-%d i\n", svc->vcpu->domain->domain_id, svc->vcpu->vcpu_id,
+                 svc->rqd->id, trqd->id);
+        if ( __vcpu_on_runq(svc) )
+        {
+            __runq_remove(svc);
+            update_load(ops, svc->rqd, svc, -1, now);
+            on_runq=1;
+        }
+        __runq_deassign(svc);
+        svc->vcpu->processor = first_cpu(trqd->active);
+        __runq_assign(svc, trqd);
+        if ( on_runq )
+        {
+            update_load(ops, svc->rqd, svc, 1, now);
+            runq_insert(ops, svc->vcpu->processor, svc);
+            runq_tickle(ops, svc->vcpu->processor, svc, now);
+        }
+    }
+}
+
+
 static void balance_load(const struct scheduler *ops, int cpu, s_time_t now)
 {
     struct csched_private *prv = CSCHED_PRIV(ops);
     int i, max_delta_rqi = -1;
     struct list_head *push_iter, *pull_iter;
 
-    /* NB: Modified by consider() */
-    s_time_t load_delta;
-    struct csched_vcpu * best_push_svc=NULL, *best_pull_svc=NULL;
-    /* NB: Read by consider() */
-    struct csched_runqueue_data *lrqd;
-    struct csched_runqueue_data *orqd;
-    
-    void consider(struct csched_vcpu *push_svc,
-                  struct csched_vcpu *pull_svc)
-    {
-        s_time_t l_load, o_load, delta;
-
-        l_load = lrqd->b_avgload;
-        o_load = orqd->b_avgload;
-        if ( push_svc )
-        {
-            /* What happens to the load on both if we push? */
-            l_load -= push_svc->avgload;
-            o_load += push_svc->avgload;
-        }
-        if ( pull_svc )
-        {
-            /* What happens to the load on both if we pull? */
-            l_load += pull_svc->avgload;
-            o_load -= pull_svc->avgload;
-        }
-        
-        delta = l_load - o_load;
-        if ( delta < 0 )
-            delta = -delta;
-
-        if ( delta < load_delta )
-        {
-            load_delta = delta;
-            best_push_svc=push_svc;
-            best_pull_svc=pull_svc;
-        }
-    }
-
-    void migrate(struct csched_vcpu *svc, struct csched_runqueue_data *trqd)
-    {
-        if ( test_bit(__CSFLAG_scheduled, &svc->flags) )
-        {
-            d2printk("d%dv%d %d-%d a\n", svc->vcpu->domain->domain_id, svc->vcpu->vcpu_id,
-                     svc->rqd->id, trqd->id);
-            /* It's running; mark it to migrate. */
-            svc->migrate_rqd = trqd;
-            set_bit(_VPF_migrating, &svc->vcpu->pause_flags);
-            set_bit(__CSFLAG_runq_migrate_request, &svc->flags);
-        }
-        else
-        {
-            int on_runq=0;
-            /* It's not running; just move it */
-            d2printk("d%dv%d %d-%d i\n", svc->vcpu->domain->domain_id, svc->vcpu->vcpu_id,
-                     svc->rqd->id, trqd->id);
-            if ( __vcpu_on_runq(svc) )
-            {
-                __runq_remove(svc);
-                update_load(ops, svc->rqd, svc, -1, now);
-                on_runq=1;
-            }
-            __runq_deassign(svc);
-            svc->vcpu->processor = first_cpu(trqd->active);
-            __runq_assign(svc, trqd);
-            if ( on_runq )
-            {
-                update_load(ops, svc->rqd, svc, 1, now);
-                runq_insert(ops, svc->vcpu->processor, svc);
-                runq_tickle(ops, svc->vcpu->processor, svc, now);
-            }
-        }
-    }
-                  
+    balance_state_t st = { .best_push_svc = NULL, .best_pull_svc = NULL };
     
     /*
      * Basic algorithm: Push, pull, or swap.
@@ -1200,39 +1210,39 @@ static void balance_load(const struct sc
     /* Locking:
      * - pcpu schedule lock should be already locked
      */
-    lrqd = RQD(ops, cpu);
+    st.lrqd = RQD(ops, cpu);
 
-    __update_runq_load(ops, lrqd, 0, now);
+    __update_runq_load(ops, st.lrqd, 0, now);
 
 retry:
     if ( !spin_trylock(&prv->lock) )
         return;
 
-    load_delta = 0;
+    st.load_delta = 0;
 
     for_each_cpu_mask(i, prv->active_queues)
     {
         s_time_t delta;
         
-        orqd = prv->rqd + i;
+        st.orqd = prv->rqd + i;
 
-        if ( orqd == lrqd
-             || !spin_trylock(&orqd->lock) )
+        if ( st.orqd == st.lrqd
+             || !spin_trylock(&st.orqd->lock) )
             continue;
 
-        __update_runq_load(ops, orqd, 0, now);
+        __update_runq_load(ops, st.orqd, 0, now);
     
-        delta = lrqd->b_avgload - orqd->b_avgload;
+        delta = st.lrqd->b_avgload - st.orqd->b_avgload;
         if ( delta < 0 )
             delta = -delta;
 
-        if ( delta > load_delta )
+        if ( delta > st.load_delta )
         {
-            load_delta = delta;
+            st.load_delta = delta;
             max_delta_rqi = i;
         }
 
-        spin_unlock(&orqd->lock);
+        spin_unlock(&st.orqd->lock);
     }
 
     /* Minimize holding the big lock */
@@ -1245,23 +1255,23 @@ retry:
         int cpus_max;
 
         
-        load_max = lrqd->b_avgload;
-        if ( orqd->b_avgload > load_max )
-            load_max = orqd->b_avgload;
+        load_max = st.lrqd->b_avgload;
+        if ( st.orqd->b_avgload > load_max )
+            load_max = st.orqd->b_avgload;
 
-        cpus_max=cpus_weight(lrqd->active);
-        if ( cpus_weight(orqd->active) > cpus_max )
-            cpus_max = cpus_weight(orqd->active);
+        cpus_max=cpus_weight(st.lrqd->active);
+        if ( cpus_weight(st.orqd->active) > cpus_max )
+            cpus_max = cpus_weight(st.orqd->active);
 
         /* If we're under 100% capacaty, only shift if load difference
          * is > 1.  otherwise, shift if under 12.5% */
         if ( load_max < (1ULL<<(prv->load_window_shift))*cpus_max )
         {
-            if ( load_delta < (1ULL<<(prv->load_window_shift+opt_underload_balance_tolerance) ) )
+            if ( st.load_delta < (1ULL<<(prv->load_window_shift+opt_underload_balance_tolerance) ) )
                  goto out;
         }
         else
-            if ( load_delta < (1ULL<<(prv->load_window_shift+opt_overload_balance_tolerance)) )
+            if ( st.load_delta < (1ULL<<(prv->load_window_shift+opt_overload_balance_tolerance)) )
                 goto out;
     }
              
@@ -1269,19 +1279,19 @@ retry:
      * meantime, try the process over again.  This can't deadlock
      * because if it doesn't get any other rqd locks, it will simply
      * give up and return. */
-    orqd = prv->rqd + max_delta_rqi;
-    if ( !spin_trylock(&orqd->lock) )
+    st.orqd = prv->rqd + max_delta_rqi;
+    if ( !spin_trylock(&st.orqd->lock) )
         goto retry;
 
     /* Make sure the runqueue hasn't been deactivated since we released prv->lock */
-    if ( unlikely(orqd->id < 0) )
+    if ( unlikely(st.orqd->id < 0) )
         goto out_up;
 
     /* Look for "swap" which gives the best load average
      * FIXME: O(n^2)! */
 
     /* Reuse load delta (as we're trying to minimize it) */
-    list_for_each( push_iter, &lrqd->svc )
+    list_for_each( push_iter, &st.lrqd->svc )
     {
         int inner_load_updated = 0;
         struct csched_vcpu * push_svc = list_entry(push_iter, struct csched_vcpu, rqd_elem);
@@ -1292,7 +1302,7 @@ retry:
         if ( test_bit(__CSFLAG_runq_migrate_request, &push_svc->flags) )
             continue;
 
-        list_for_each( pull_iter, &orqd->svc )
+        list_for_each( pull_iter, &st.orqd->svc )
         {
             struct csched_vcpu * pull_svc = list_entry(pull_iter, struct csched_vcpu, rqd_elem);
             
@@ -1305,16 +1315,16 @@ retry:
             if ( test_bit(__CSFLAG_runq_migrate_request, &pull_svc->flags) )
                 continue;
 
-            consider(push_svc, pull_svc);
+            consider(&st, push_svc, pull_svc);
         }
 
         inner_load_updated = 1;
 
         /* Consider push only */
-        consider(push_svc, NULL);
+        consider(&st, push_svc, NULL);
     }
 
-    list_for_each( pull_iter, &orqd->svc )
+    list_for_each( pull_iter, &st.orqd->svc )
     {
         struct csched_vcpu * pull_svc = list_entry(pull_iter, struct csched_vcpu, rqd_elem);
         
@@ -1323,17 +1333,17 @@ retry:
             continue;
 
         /* Consider pull only */
-        consider(NULL, pull_svc);
+        consider(&st, NULL, pull_svc);
     }
 
     /* OK, now we have some candidates; do the moving */
-    if ( best_push_svc )
-        migrate(best_push_svc, orqd);
-    if ( best_pull_svc )
-        migrate(best_pull_svc, lrqd);
+    if ( st.best_push_svc )
+        migrate(ops, st.best_push_svc, st.orqd, now);
+    if ( st.best_pull_svc )
+        migrate(ops, st.best_pull_svc, st.lrqd, now);
 
 out_up:
-    spin_unlock(&orqd->lock);
+    spin_unlock(&st.orqd->lock);
 
 out:
     return;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 204 bytes --]

(dump_execution_state()) as its own bug-trap.

This is needed to compile xen with clang, which can't handle using a
function name in an asm immediate.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>



[-- Attachment #2: xen-llvm.hg-6.patch --]
[-- Type: text/x-patch, Size: 3045 bytes --]

# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1299496871 0
# Node ID 04783b003d5c5691930af6baa3614389247ddda1
# Parent  c72c074d0e084dcabb5314ac71e3b7730898c764
Xen: remove run_in_exception_handler() and recode its only caller
(dump_execution_state()) as its own bug-trap.

This is needed to compile xen with clang, which can't handle using a
function name in an asm immediate.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r c72c074d0e08 -r 04783b003d5c xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/arch/x86/traps.c	Mon Mar 07 11:21:11 2011 +0000
@@ -907,6 +907,15 @@ asmlinkage void do_invalid_op(struct cpu
         goto die;
     eip += sizeof(bug);
 
+    id = bug.id & 3;
+
+    if ( id == BUGFRAME_state )
+    {
+        show_execution_state(regs);
+        regs->eip = (unsigned long)eip;
+        return;
+    }
+
     /* Decode first pointer argument. */
     if ( !is_kernel(eip) ||
          __copy_from_user(&bug_str, eip, sizeof(bug_str)) ||
@@ -917,16 +926,6 @@ asmlinkage void do_invalid_op(struct cpu
         goto die;
     eip += sizeof(bug_str);
 
-    id = bug.id & 3;
-
-    if ( id == BUGFRAME_run_fn )
-    {
-        void (*fn)(struct cpu_user_regs *) = (void *)p;
-        (*fn)(regs);
-        regs->eip = (unsigned long)eip;
-        return;
-    }
-
     /* WARN, BUG or ASSERT: decode the filename pointer and line number. */
     filename = p;
     lineno = bug.id >> 2;
diff -r c72c074d0e08 -r 04783b003d5c xen/include/asm-x86/bug.h
--- a/xen/include/asm-x86/bug.h	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/include/asm-x86/bug.h	Mon Mar 07 11:21:11 2011 +0000
@@ -13,16 +13,15 @@ struct bug_frame {
     unsigned short id; /* BUGFRAME_??? */
 } __attribute__((packed));
 
-#define BUGFRAME_run_fn 0
+#define BUGFRAME_state  0
 #define BUGFRAME_warn   1
 #define BUGFRAME_bug    2
 #define BUGFRAME_assert 3
 
-#define run_in_exception_handler(fn)               \
+#define dump_execution_state()                     \
     asm volatile (                                 \
-        "ud2 ; ret %0" BUG_STR(1)                  \
-        : : "i" (BUGFRAME_run_fn),                 \
-            "i" (fn) )
+        "ud2 ; ret %0"                             \
+        : : "i" (BUGFRAME_state) )
 
 #define WARN()                                     \
     asm volatile (                                 \
diff -r c72c074d0e08 -r 04783b003d5c xen/include/asm-x86/processor.h
--- a/xen/include/asm-x86/processor.h	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/include/asm-x86/processor.h	Mon Mar 07 11:21:11 2011 +0000
@@ -542,7 +542,6 @@ void show_stack(struct cpu_user_regs *re
 void show_stack_overflow(unsigned int cpu, unsigned long esp);
 void show_registers(struct cpu_user_regs *regs);
 void show_execution_state(struct cpu_user_regs *regs);
-#define dump_execution_state() run_in_exception_handler(show_execution_state)
 void show_page_walk(unsigned long addr);
 asmlinkage void fatal_trap(int trapnr, struct cpu_user_regs *regs);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 7 of 8] x86: redefine REX64_PREFIX for clang, which doesn't like 'rex64/'

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 52 bytes --]

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>



[-- Attachment #2: xen-llvm.hg-7.patch --]
[-- Type: text/x-patch, Size: 732 bytes --]

# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1299496871 0
# Node ID 804007170cf03ea832022e2d589507a3bc0505dc
# Parent  04783b003d5c5691930af6baa3614389247ddda1
x86: redefine REX64_PREFIX for clang, which doesn't like 'rex64/'.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r 04783b003d5c -r 804007170cf0 xen/include/asm-x86/x86_64/asm_defns.h
--- a/xen/include/asm-x86/x86_64/asm_defns.h	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/include/asm-x86/x86_64/asm_defns.h	Mon Mar 07 11:21:11 2011 +0000
@@ -84,6 +84,8 @@ 1:      addq  $8,%rsp;
 
 #ifdef __sun__
 #define REX64_PREFIX "rex64\\"
+#elif defined(__clang__)
+#define REX64_PREFIX ".byte 0x48; "
 #else
 #define REX64_PREFIX "rex64/"
 #endif

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 240 bytes --]

Tested with svn snapshot of clang and llvm from 17 February 2011.
Only x86_64 hypervisor builds (make dist-xen clang=y) are supported
and I haven't even begun to look at cross-compiling.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>



[-- Attachment #2: xen-llvm.hg-8.patch --]
[-- Type: text/x-patch, Size: 3737 bytes --]

# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1299496871 0
# Node ID d91e6a8d36ad3dbf89f42264334baec0cb37063f
# Parent  804007170cf03ea832022e2d589507a3bc0505dc
xen: add "clang=y" option to build Xen with clang/llvm instead of gcc.

Tested with svn snapshot of clang and llvm from 17 February 2011.
Only x86_64 hypervisor builds (make dist-xen clang=y) are supported
and I haven't even begun to look at cross-compiling.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r 804007170cf0 -r d91e6a8d36ad Config.mk
--- a/Config.mk	Mon Mar 07 11:21:11 2011 +0000
+++ b/Config.mk	Mon Mar 07 11:21:11 2011 +0000
@@ -148,6 +148,13 @@ CFLAGS += -Wall -Wstrict-prototypes
 # result of any casted expression causes a warning.
 CFLAGS += -Wno-unused-value
 
+ifeq ($(clang),y)
+# Clang complains about macros that expand to 'if ( ( foo == bar ) ) ...'
+CFLAGS += -Wno-parentheses
+# And is over-zealous with the printf format lint
+CFLAGS += -Wno-format
+endif
+
 $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
 $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
 
diff -r 804007170cf0 -r d91e6a8d36ad config/StdGNU.mk
--- a/config/StdGNU.mk	Mon Mar 07 11:21:11 2011 +0000
+++ b/config/StdGNU.mk	Mon Mar 07 11:21:11 2011 +0000
@@ -1,6 +1,11 @@
 AS         = $(CROSS_COMPILE)as
+ifeq ($(clang),y)
+LD         = $(CROSS_COMPILE)gold
+CC         = $(CROSS_COMPILE)clang
+else
 LD         = $(CROSS_COMPILE)ld
 CC         = $(CROSS_COMPILE)gcc
+endif
 CPP        = $(CC) -E
 AR         = $(CROSS_COMPILE)ar
 RANLIB     = $(CROSS_COMPILE)ranlib
@@ -69,5 +74,8 @@ ifneq ($(debug),y)
 CFLAGS += -O2 -fomit-frame-pointer
 else
 # Less than -O1 produces bad code and large stack frames
-CFLAGS += -O1 -fno-omit-frame-pointer -fno-optimize-sibling-calls
+CFLAGS += -O1 -fno-omit-frame-pointer
+ifneq ($(clang),y)
+CFLAGS += -fno-optimize-sibling-calls
 endif
+endif
diff -r 804007170cf0 -r d91e6a8d36ad xen/Makefile
--- a/xen/Makefile	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/Makefile	Mon Mar 07 11:21:11 2011 +0000
@@ -88,7 +88,7 @@ include/xen/compile.h: include/xen/compi
 	    -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
 	    -e 's/@@domain@@/$(XEN_DOMAIN)/g' \
 	    -e 's/@@hostname@@/$(shell hostname)/g' \
-	    -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) -v 2>&1 | tail -1)!g' \
+	    -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) -v 2>&1 | grep version | tail -1)!g' \
 	    -e 's/@@version@@/$(XEN_VERSION)/g' \
 	    -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
 	    -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
diff -r 804007170cf0 -r d91e6a8d36ad xen/Rules.mk
--- a/xen/Rules.mk	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/Rules.mk	Mon Mar 07 11:21:11 2011 +0000
@@ -62,6 +62,9 @@ endif
 
 AFLAGS-y                += -D__ASSEMBLY__
 
+# Clang's built-in assembler can't handle .code16/.code32/.code64 yet
+AFLAGS-$(clang)         += -no-integrated-as
+
 ALL_OBJS := $(ALL_OBJS-y)
 
 # Get gcc to generate the dependencies for us.
diff -r 804007170cf0 -r d91e6a8d36ad xen/arch/x86/Rules.mk
--- a/xen/arch/x86/Rules.mk	Mon Mar 07 11:21:11 2011 +0000
+++ b/xen/arch/x86/Rules.mk	Mon Mar 07 11:21:11 2011 +0000
@@ -12,9 +12,12 @@ xenoprof := y
 supervisor_mode_kernel ?= n
 
 # Solaris grabs stdarg.h and friends from the system include directory.
+# Clang likewise.
 ifneq ($(XEN_OS),SunOS)
+ifneq ($(clang),y)
 CFLAGS += -nostdinc
 endif
+endif
 
 CFLAGS += -fno-builtin -fno-common -Wredundant-decls
 CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
@@ -49,5 +52,7 @@ x86_32 := n
 x86_64 := y
 endif
 
+ifneq ($(clang),y)
 # Require GCC v3.4+ (to avoid issues with alignment constraints in Xen headers)
 $(call cc-ver-check,CC,0x030400,"Xen requires at least gcc-3.4")
+endif

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 0 of 8] Allow building xen with clang/llvm

[Keir Fraser]

These and the first LTO one are all fine by me.

Acked-by: Keir Fraser <keir@xen.org>


On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:

> This series of patches allows Xen builds with clang/llvm as well as
> GCC.  It only fixes Xen, not tools &c, and only 64-bit, so far.
> 
> Building Xen with clang isn't useful in itself (and indeed produces
> a hypervisor that can be up to 10% slower for a default -O2 build);
> rather it's a stepping-stone to doing full link-time optimizations
> on the LLVM bitcode of the whole hypervisor.  I will post a second
> series to enable that, but it's not so clean as this one.
> 
> I think all the changes in this series are reasonable and won't
> inconvenience any normal users, but obviously they're meant for
> the xen-unstable tree only.
> 
> Cheers,
> 
> Tim.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc

[Ian Campbell]

On Mon, 2011-03-07 at 11:26 +0000, Tim Deegan wrote:
> # HG changeset patch
> # User Tim Deegan <Tim.Deegan@citrix.com>
> # Date 1299496871 0
> # Node ID d91e6a8d36ad3dbf89f42264334baec0cb37063f
> # Parent  804007170cf03ea832022e2d589507a3bc0505dc
> xen: add "clang=y" option to build Xen with clang/llvm instead of gcc.
> 
> Tested with svn snapshot of clang and llvm from 17 February 2011.
> Only x86_64 hypervisor builds (make dist-xen clang=y) are supported
> and I haven't even begun to look at cross-compiling.
> 
> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
> 
> diff -r 804007170cf0 -r d91e6a8d36ad Config.mk
> --- a/Config.mk Mon Mar 07 11:21:11 2011 +0000
> +++ b/Config.mk Mon Mar 07 11:21:11 2011 +0000
> @@ -148,6 +148,13 @@ CFLAGS += -Wall -Wstrict-prototypes
>  # result of any casted expression causes a warning.
>  CFLAGS += -Wno-unused-value
>  
> +ifeq ($(clang),y)
> +# Clang complains about macros that expand to 'if ( ( foo == bar )
> ) ...'
> +CFLAGS += -Wno-parentheses
> +# And is over-zealous with the printf format lint
> +CFLAGS += -Wno-format
> +endif

Is it worth arranging for "gcc := y" when clang is not enabled? Then a
whole bunch of this sort of thing devolves into the 
	CFLAGS-$(a-particular-cc) += -Wfoo
pattern.

> +
>  $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
>  $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
>  
> diff -r 804007170cf0 -r d91e6a8d36ad config/StdGNU.mk
> --- a/config/StdGNU.mk  Mon Mar 07 11:21:11 2011 +0000
> +++ b/config/StdGNU.mk  Mon Mar 07 11:21:11 2011 +0000
> @@ -1,6 +1,11 @@
>  AS         = $(CROSS_COMPILE)as
> +ifeq ($(clang),y)
> +LD         = $(CROSS_COMPILE)gold
> +CC         = $(CROSS_COMPILE)clang
> +else
>  LD         = $(CROSS_COMPILE)ld
>  CC         = $(CROSS_COMPILE)gcc
> +endif
>  CPP        = $(CC) -E
>  AR         = $(CROSS_COMPILE)ar
>  RANLIB     = $(CROSS_COMPILE)ranlib

LD-$(clang) = ...
LD-$(gcc)   = ...

LD := $(LD-y)

?

> @@ -69,5 +74,8 @@ ifneq ($(debug),y)
>  CFLAGS += -O2 -fomit-frame-pointer
>  else
>  # Less than -O1 produces bad code and large stack frames
> -CFLAGS += -O1 -fno-omit-frame-pointer -fno-optimize-sibling-calls
> +CFLAGS += -O1 -fno-omit-frame-pointer
> +ifneq ($(clang),y)
> +CFLAGS += -fno-optimize-sibling-calls

CFLAGS-$(gcc) += ...

etc etc

Ian.

Re: [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller

[Keir Fraser]

On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:

> (dump_execution_state()) as its own bug-trap.
> 
> This is needed to compile xen with clang, which can't handle using a
> function name in an asm immediate.

Actually run_in_exception_handler() does have another user, in ns16550.c.
Although non-essential, it makes the 'd' debug key much more useful when
running the UART in polled mode.

So I suggest we keep run_in_exception_handler but modify it to pass the
function pointer in (say) rAX. I think we won't easily be able to use
BUG_STR() logic but r_i_e_h is only used (directly or indirectly) in a few
places so the BUG_STR optimisation is unimportant. The only other
disadvantage is that rAX is less interesting in the state dump, but any
value the function pointer displaces can still be found in the stack dump,
albeit with likely a little extra effort.

Sound good?

 -- Keir 

> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller

[Keir Fraser]

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote:

> On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:
> 
>> (dump_execution_state()) as its own bug-trap.
>> 
>> This is needed to compile xen with clang, which can't handle using a
>> function name in an asm immediate.
> 
> Actually run_in_exception_handler() does have another user, in ns16550.c.
> Although non-essential, it makes the 'd' debug key much more useful when
> running the UART in polled mode.
> 
> So I suggest we keep run_in_exception_handler but modify it to pass the
> function pointer in (say) rAX.

Like the attached patch (against latest tip).

 -- Keir

> I think we won't easily be able to use
> BUG_STR() logic but r_i_e_h is only used (directly or indirectly) in a few
> places so the BUG_STR optimisation is unimportant. The only other
> disadvantage is that rAX is less interesting in the state dump, but any
> value the function pointer displaces can still be found in the stack dump,
> albeit with likely a little extra effort.
> 
> Sound good?
> 
>  -- Keir 
> 
>> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
> 
> 


[-- Attachment #2: 00-reintroduce-run-in-exc-handler --]
[-- Type: application/octet-stream, Size: 1884 bytes --]

diff -r 076b63b74cf6 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c	Mon Mar 07 11:34:09 2011 +0000
+++ b/xen/arch/x86/traps.c	Mon Mar 07 15:14:22 2011 +0000
@@ -909,9 +909,10 @@
 
     id = bug.id & 3;
 
-    if ( id == BUGFRAME_state )
+    if ( id == BUGFRAME_run_fn )
     {
-        show_execution_state(regs);
+        void (*fn)(struct cpu_user_regs *) = (void *)regs->eax;
+        (*fn)(regs);
         regs->eip = (unsigned long)eip;
         return;
     }
diff -r 076b63b74cf6 xen/include/asm-x86/bug.h
--- a/xen/include/asm-x86/bug.h	Mon Mar 07 11:34:09 2011 +0000
+++ b/xen/include/asm-x86/bug.h	Mon Mar 07 15:14:22 2011 +0000
@@ -13,15 +13,16 @@
     unsigned short id; /* BUGFRAME_??? */
 } __attribute__((packed));
 
-#define BUGFRAME_state  0
+#define BUGFRAME_run_fn 0
 #define BUGFRAME_warn   1
 #define BUGFRAME_bug    2
 #define BUGFRAME_assert 3
 
-#define dump_execution_state()                     \
+#define run_in_exception_handler(fn)               \
     asm volatile (                                 \
         "ud2 ; ret %0"                             \
-        : : "i" (BUGFRAME_state) )
+        : : "i" (BUGFRAME_run_fn),                 \
+            "a" (fn) )
 
 #define WARN()                                     \
     asm volatile (                                 \
diff -r 076b63b74cf6 xen/include/asm-x86/processor.h
--- a/xen/include/asm-x86/processor.h	Mon Mar 07 11:34:09 2011 +0000
+++ b/xen/include/asm-x86/processor.h	Mon Mar 07 15:14:22 2011 +0000
@@ -542,6 +542,7 @@
 void show_stack_overflow(unsigned int cpu, unsigned long esp);
 void show_registers(struct cpu_user_regs *regs);
 void show_execution_state(struct cpu_user_regs *regs);
+#define dump_execution_state() run_in_exception_handler(show_execution_state)
 void show_page_walk(unsigned long addr);
 asmlinkage void fatal_trap(int trapnr, struct cpu_user_regs *regs);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 5 of 8] credit2: remove two nested functions, replacing them with static ones

[George Dunlap]

Yeah, I kind of knew those nested functions were skanky when I wrote them. :-)

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

On Mon, Mar 7, 2011 at 11:26 AM, Tim Deegan <Tim.Deegan@citrix.com> wrote:
> and passing the outer scope's variables in explicitly.
> This is needed to compile xen with clang.
>
> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>

Re: [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller

[Tim Deegan]

At 15:15 +0000 on 07 Mar (1299510944), Keir Fraser wrote:
> On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote:
> 
> > On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:
> > 
> >> (dump_execution_state()) as its own bug-trap.
> >> 
> >> This is needed to compile xen with clang, which can't handle using a
> >> function name in an asm immediate.
> > 
> > Actually run_in_exception_handler() does have another user, in ns16550.c.
> > Although non-essential, it makes the 'd' debug key much more useful when
> > running the UART in polled mode.
> > 
> > So I suggest we keep run_in_exception_handler but modify it to pass the
> > function pointer in (say) rAX.
> 
> Like the attached patch (against latest tip).

Sorry, I had missed that other user.  I'll see if I can find a way to
make clang use the function address directly; if not, I'd be inclined to 
have dump_execution_state have its own ID anyway, to keep %rax valid(er)
in BUG()s.

Tim.

> > I think we won't easily be able to use
> > BUG_STR() logic but r_i_e_h is only used (directly or indirectly) in a few
> > places so the BUG_STR optimisation is unimportant. The only other
> > disadvantage is that rAX is less interesting in the state dump, but any
> > value the function pointer displaces can still be found in the stack dump,
> > albeit with likely a little extra effort.
> > 
> > Sound good?
> > 
> >  -- Keir 
> > 
> >> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
> >> 
> >> 
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xensource.com
> >> http://lists.xensource.com/xen-devel
> > 
> > 
> 



-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller

[Keir Fraser]

On 07/03/2011 15:38, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:

> At 15:15 +0000 on 07 Mar (1299510944), Keir Fraser wrote:
>> On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote:
>> 
>>> On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:
>>> 
>>>> (dump_execution_state()) as its own bug-trap.
>>>> 
>>>> This is needed to compile xen with clang, which can't handle using a
>>>> function name in an asm immediate.
>>> 
>>> Actually run_in_exception_handler() does have another user, in ns16550.c.
>>> Although non-essential, it makes the 'd' debug key much more useful when
>>> running the UART in polled mode.
>>> 
>>> So I suggest we keep run_in_exception_handler but modify it to pass the
>>> function pointer in (say) rAX.
>> 
>> Like the attached patch (against latest tip).
> 
> Sorry, I had missed that other user.  I'll see if I can find a way to
> make clang use the function address directly; if not, I'd be inclined to
> have dump_execution_state have its own ID anyway, to keep %rax valid(er)
> in BUG()s.

But actually dump_execution_state() isn't used for BUGs and WARNs and
ASSERTs. In fact it's practically dead on x86 -- it's used in one rather
unlikely ACPI error function, and also in __bug/__warn which are never used
on x86. And that's it. Actually the user of run_in_exception_handler() in
ns16550.c is really the only one we care about!

 -- Keir

> Tim.
> 
>>> I think we won't easily be able to use
>>> BUG_STR() logic but r_i_e_h is only used (directly or indirectly) in a few
>>> places so the BUG_STR optimisation is unimportant. The only other
>>> disadvantage is that rAX is less interesting in the state dump, but any
>>> value the function pointer displaces can still be found in the stack dump,
>>> albeit with likely a little extra effort.
>>> 
>>> Sound good?
>>> 
>>>  -- Keir 
>>> 
>>>> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
>>>> 
>>>> 
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xensource.com
>>>> http://lists.xensource.com/xen-devel
>>> 
>>> 
>> 
> 
> 

Re: [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller

[Keir Fraser]

On 07/03/2011 15:44, "Keir Fraser" <keir.xen@gmail.com> wrote:

>> 
>> Sorry, I had missed that other user.  I'll see if I can find a way to
>> make clang use the function address directly; if not, I'd be inclined to
>> have dump_execution_state have its own ID anyway, to keep %rax valid(er)
>> in BUG()s.
> 
> But actually dump_execution_state() isn't used for BUGs and WARNs and
> ASSERTs. In fact it's practically dead on x86 -- it's used in one rather
> unlikely ACPI error function, and also in __bug/__warn which are never used
> on x86. And that's it. Actually the user of run_in_exception_handler() in
> ns16550.c is really the only one we care about!

And furthermore, the GPRs dumped from the 'd' key via a UART in polled mode
are always uninteresting. Because the backtrace will start from ns16550_poll
and out from there through Xen's timer subsystem. The fact we clobber rAX in
r_i_e_h() would not make things worse. :-)

So if you can't find a way to force a function address through clang then my
patch would be a perfectly fine alternative.

 -- Keir

>  -- Keir
> 
>> Tim.
>> 
>>>> I think we won't easily be able to use
>>>> BUG_STR() logic but r_i_e_h is only used (directly or indirectly) in a few
>>>> places so the BUG_STR optimisation is unimportant. The only other
>>>> disadvantage is that rAX is less interesting in the state dump, but any
>>>> value the function pointer displaces can still be found in the stack dump,
>>>> albeit with likely a little extra effort.
>>>> 
>>>> Sound good?
>>>> 
>>>>  -- Keir 
>>>> 
>>>>> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
>>>>> 
>>>>> 
>>>>> _______________________________________________
>>>>> Xen-devel mailing list
>>>>> Xen-devel@lists.xensource.com
>>>>> http://lists.xensource.com/xen-devel
>>>> 
>>>> 
>>> 
>> 
>> 
> 
> 

Re: [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller

[Tim Deegan]

At 15:38 +0000 on 07 Mar (1299512321), Tim Deegan wrote:
> At 15:15 +0000 on 07 Mar (1299510944), Keir Fraser wrote:
> > On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote:
> > > On 07/03/2011 11:26, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:
> > >> This is needed to compile xen with clang, which can't handle using a
> > >> function name in an asm immediate.
> > > 
> > > Actually run_in_exception_handler() does have another user, in ns16550.c.
> > > Although non-essential, it makes the 'd' debug key much more useful when
> > > running the UART in polled mode.
> > > 
> > > So I suggest we keep run_in_exception_handler but modify it to pass the
> > > function pointer in (say) rAX.
> > 
> > Like the attached patch (against latest tip).
> 
> Sorry, I had missed that other user.  I'll see if I can find a way to
> make clang use the function address directly; if not, I'd be inclined to 
> have dump_execution_state have its own ID anyway, to keep %rax valid(er)
> in BUG()s.

Turns out to be very straightforward: another level of indirection makes
the parser happy.  If it's OK with you, I'll revert 22987:3147f2d1c6fb
and apply this instead:

diff -r 3147f2d1c6fb xen/include/asm-x86/bug.h
--- a/xen/include/asm-x86/bug.h	Mon Mar 07 15:47:59 2011 +0000
+++ b/xen/include/asm-x86/bug.h	Mon Mar 07 15:48:32 2011 +0000
@@ -22,7 +22,7 @@ struct bug_frame {
     asm volatile (                                 \
         "ud2 ; ret %0" BUG_STR(1)                  \
         : : "i" (BUGFRAME_run_fn),                 \
-            "i" (fn) )
+            "i" (&(fn)) )
 
 #define WARN()                                     \
     asm volatile (                                 \

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller

[Keir Fraser]

On 07/03/2011 15:56, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:

> At 15:38 +0000 on 07 Mar (1299512321), Tim Deegan wrote:
>> At 15:15 +0000 on 07 Mar (1299510944), Keir Fraser wrote:
>>> On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote:
>>> 
>>> Like the attached patch (against latest tip).
>> 
>> Sorry, I had missed that other user.  I'll see if I can find a way to
>> make clang use the function address directly; if not, I'd be inclined to
>> have dump_execution_state have its own ID anyway, to keep %rax valid(er)
>> in BUG()s.
> 
> Turns out to be very straightforward: another level of indirection makes
> the parser happy.  If it's OK with you, I'll revert 22987:3147f2d1c6fb
> and apply this instead:

If you've successfully both build- and run-tested it with gcc then it's fine
with me. It needs testing as it's a moderately skanky construct in the first
place.

 -- Keir

> diff -r 3147f2d1c6fb xen/include/asm-x86/bug.h
> --- a/xen/include/asm-x86/bug.h Mon Mar 07 15:47:59 2011 +0000
> +++ b/xen/include/asm-x86/bug.h Mon Mar 07 15:48:32 2011 +0000
> @@ -22,7 +22,7 @@ struct bug_frame {
>      asm volatile (                                 \
>          "ud2 ; ret %0" BUG_STR(1)                  \
>          : : "i" (BUGFRAME_run_fn),                 \
> -            "i" (fn) )
> +            "i" (&(fn)) )
>  
>  #define WARN()                                     \
>      asm volatile (                                 \
> 
> Cheers,
> 
> Tim.

Re: [PATCH 6 of 8] Xen: remove run_in_exception_handler() and recode its only caller

[Tim Deegan]

At 16:00 +0000 on 07 Mar (1299513606), Keir Fraser wrote:
> On 07/03/2011 15:56, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:
> 
> > At 15:38 +0000 on 07 Mar (1299512321), Tim Deegan wrote:
> >> At 15:15 +0000 on 07 Mar (1299510944), Keir Fraser wrote:
> >>> On 07/03/2011 15:05, "Keir Fraser" <keir.xen@gmail.com> wrote:
> >>> 
> >>> Like the attached patch (against latest tip).
> >> 
> >> Sorry, I had missed that other user.  I'll see if I can find a way to
> >> make clang use the function address directly; if not, I'd be inclined to
> >> have dump_execution_state have its own ID anyway, to keep %rax valid(er)
> >> in BUG()s.
> > 
> > Turns out to be very straightforward: another level of indirection makes
> > the parser happy.  If it's OK with you, I'll revert 22987:3147f2d1c6fb
> > and apply this instead:
> 
> If you've successfully both build- and run-tested it with gcc then it's fine
> with me. It needs testing as it's a moderately skanky construct in the first
> place.

I have, and at least with 64bit gcc 4.4.5 dump_execution_state() still
works fine. :)

Tim.

>  -- Keir
> 
> > diff -r 3147f2d1c6fb xen/include/asm-x86/bug.h
> > --- a/xen/include/asm-x86/bug.h Mon Mar 07 15:47:59 2011 +0000
> > +++ b/xen/include/asm-x86/bug.h Mon Mar 07 15:48:32 2011 +0000
> > @@ -22,7 +22,7 @@ struct bug_frame {
> >      asm volatile (                                 \
> >          "ud2 ; ret %0" BUG_STR(1)                  \
> >          : : "i" (BUGFRAME_run_fn),                 \
> > -            "i" (fn) )
> > +            "i" (&(fn)) )
> >  
> >  #define WARN()                                     \
> >      asm volatile (                                 \
> > 
> > Cheers,
> > 
> > Tim.
> 
> 

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

At 14:54 +0000 on 07 Mar (1299509669), Ian Campbell wrote:
> Is it worth arranging for "gcc := y" when clang is not enabled? Then a
> whole bunch of this sort of thing devolves into the 
> 	CFLAGS-$(a-particular-cc) += -Wfoo
> pattern.

Something like the attached?  It tidies up four such ifeqs, at the cost
of one new one to define $(gcc). 
(4 files changed, 17 insertions(+), 16 deletions(-))

> > @@ -1,6 +1,11 @@
> >  AS         = $(CROSS_COMPILE)as
> > +ifeq ($(clang),y)
> > +LD         = $(CROSS_COMPILE)gold
> > +CC         = $(CROSS_COMPILE)clang
> > +else
> >  LD         = $(CROSS_COMPILE)ld
> >  CC         = $(CROSS_COMPILE)gcc
> > +endif
> >  CPP        = $(CC) -E
> >  AR         = $(CROSS_COMPILE)ar
> >  RANLIB     = $(CROSS_COMPILE)ranlib
> 
> LD-$(clang) = ...
> LD-$(gcc)   = ...
> 
> LD := $(LD-y)

I tried that but it looks about as bad, and actually has more
repetition. 

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[-- Attachment #2: makeism --]
[-- Type: text/plain, Size: 2839 bytes --]

diff -r ca556b7bad5c Config.mk
--- a/Config.mk	Mon Mar 07 16:07:06 2011 +0000
+++ b/Config.mk	Mon Mar 07 16:26:43 2011 +0000
@@ -23,6 +23,15 @@ DESTDIR     ?= /
 # Allow phony attribute to be listed as dependency rather than fake target
 .PHONY: .phony
 
+# Use Clang/LLVM instead of GCC?
+clang ?= n
+ifeq ($(clang),n)
+gcc := y
+else
+gcc := n
+endif
+
+
 include $(XEN_ROOT)/config/$(XEN_OS).mk
 include $(XEN_ROOT)/config/$(XEN_TARGET_ARCH).mk
 
@@ -148,12 +157,9 @@ CFLAGS += -Wall -Wstrict-prototypes
 # result of any casted expression causes a warning.
 CFLAGS += -Wno-unused-value
 
-ifeq ($(clang),y)
 # Clang complains about macros that expand to 'if ( ( foo == bar ) ) ...'
-CFLAGS += -Wno-parentheses
-# And is over-zealous with the printf format lint
-CFLAGS += -Wno-format
-endif
+# and is over-zealous with the printf format lint
+CFLAGS-$(clang) += -Wno-parentheses -Wno-format
 
 $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
 $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
diff -r ca556b7bad5c config/StdGNU.mk
--- a/config/StdGNU.mk	Mon Mar 07 16:07:06 2011 +0000
+++ b/config/StdGNU.mk	Mon Mar 07 16:26:43 2011 +0000
@@ -76,14 +76,10 @@ CFLAGS += -O2 -fomit-frame-pointer
 else
 # Less than -O1 produces bad code and large stack frames
 CFLAGS += -O1 -fno-omit-frame-pointer
-ifneq ($(clang),y)
-CFLAGS += -fno-optimize-sibling-calls
-endif
+CFLAGS-$(gcc) += -fno-optimize-sibling-calls
 endif
 
 ifeq ($(lto),y)
 CFLAGS += -flto
-ifeq ($(clang),y)
-LDFLAGS += -plugin LLVMgold.so
+LDFLAGS-$(clang) += -plugin LLVMgold.so
 endif
-endif
diff -r ca556b7bad5c xen/Rules.mk
--- a/xen/Rules.mk	Mon Mar 07 16:07:06 2011 +0000
+++ b/xen/Rules.mk	Mon Mar 07 16:26:43 2011 +0000
@@ -9,7 +9,6 @@ perfc_arrays  ?= n
 lock_profile  ?= n
 crash_debug   ?= n
 frame_pointer ?= n
-clang         ?= n
 lto           ?= n
 
 XEN_ROOT=$(BASEDIR)/..
@@ -83,6 +82,8 @@ AFLAGS += $(AFLAGS-y) $(filter-out -std=
 # LDFLAGS are only passed directly to $(LD)
 LDFLAGS += $(LDFLAGS_DIRECT)
 
+LDFLAGS += $(LDFLAGS-y)
+
 include Makefile
 
 # Ensure each subdirectory has exactly one trailing slash.
diff -r ca556b7bad5c xen/arch/x86/Rules.mk
--- a/xen/arch/x86/Rules.mk	Mon Mar 07 16:07:06 2011 +0000
+++ b/xen/arch/x86/Rules.mk	Mon Mar 07 16:26:43 2011 +0000
@@ -14,9 +14,7 @@ supervisor_mode_kernel ?= n
 # Solaris grabs stdarg.h and friends from the system include directory.
 # Clang likewise.
 ifneq ($(XEN_OS),SunOS)
-ifneq ($(clang),y)
-CFLAGS += -nostdinc
-endif
+CFLAGS-$(gcc) += -nostdinc
 endif
 
 CFLAGS += -fno-builtin -fno-common -Wredundant-decls
@@ -52,7 +50,7 @@ x86_32 := n
 x86_64 := y
 endif
 
-ifneq ($(clang),y)
+ifeq ($(gcc),y)
 # Require GCC v3.4+ (to avoid issues with alignment constraints in Xen headers)
 $(call cc-ver-check,CC,0x030400,"Xen requires at least gcc-3.4")
 endif

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc

[Keir Fraser]

On 07/03/2011 16:29, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:

> At 14:54 +0000 on 07 Mar (1299509669), Ian Campbell wrote:
>> Is it worth arranging for "gcc := y" when clang is not enabled? Then a
>> whole bunch of this sort of thing devolves into the
>> CFLAGS-$(a-particular-cc) += -Wfoo
>> pattern.
> 
> Something like the attached?  It tidies up four such ifeqs, at the cost
> of one new one to define $(gcc).
> (4 files changed, 17 insertions(+), 16 deletions(-))

This looks like a definite improvement in readability, to me. I'd like it to
be applied.

 -- Keir

>>> @@ -1,6 +1,11 @@
>>>  AS         = $(CROSS_COMPILE)as
>>> +ifeq ($(clang),y)
>>> +LD         = $(CROSS_COMPILE)gold
>>> +CC         = $(CROSS_COMPILE)clang
>>> +else
>>>  LD         = $(CROSS_COMPILE)ld
>>>  CC         = $(CROSS_COMPILE)gcc
>>> +endif
>>>  CPP        = $(CC) -E
>>>  AR         = $(CROSS_COMPILE)ar
>>>  RANLIB     = $(CROSS_COMPILE)ranlib
>> 
>> LD-$(clang) = ...
>> LD-$(gcc)   = ...
>> 
>> LD := $(LD-y)
> 
> I tried that but it looks about as bad, and actually has more
> repetition. 
> 
> Tim.

Re: [PATCH 2 of 8] x86: add explicit size suffixes to some assembly instructions

[Jan Beulich]

>>> On 07.03.11 at 12:26, Tim Deegan <Tim.Deegan@citrix.com> wrote:
>--- a/xen/arch/x86/x86_emulate/x86_emulate.c	Mon Mar 07 11:21:11 2011 +0000
>+++ b/xen/arch/x86/x86_emulate/x86_emulate.c	Mon Mar 07 11:21:11 2011 +0000
>@@ -2678,7 +2678,7 @@ x86_emulate(
>                 emulate_fpu_insn_memsrc("fiaddl", src.val);
>                 break;
>             case 1: /* fimul m64i */
>-                emulate_fpu_insn_memsrc("fimul", src.val);
>+                emulate_fpu_insn_memsrc("fimuls", src.val);

fimull.

>                 break;
>             case 2: /* ficom m64i */
>                 emulate_fpu_insn_memsrc("ficoml", src.val);

(Side note: The comments in this section are all wrong - they
should say "m32i"; there are no 64-bit integer operations
other than loads and stores.)

Jan

Re: [PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc

[Ian Campbell]

On Mon, 2011-03-07 at 16:29 +0000, Tim Deegan wrote:
> At 14:54 +0000 on 07 Mar (1299509669), Ian Campbell wrote:
> > Is it worth arranging for "gcc := y" when clang is not enabled? Then a
> > whole bunch of this sort of thing devolves into the 
> > 	CFLAGS-$(a-particular-cc) += -Wfoo
> > pattern.
> 
> Something like the attached?  It tidies up four such ifeqs, at the cost
> of one new one to define $(gcc). 

I think that one is worth it to clean up all the others -- at least all
the nastiness will be contained to 1 place. Imagine a far distant future
where we also support e.g. icc or something...

> (4 files changed, 17 insertions(+), 16 deletions(-))
> 
> > > @@ -1,6 +1,11 @@
> > >  AS         = $(CROSS_COMPILE)as
> > > +ifeq ($(clang),y)
> > > +LD         = $(CROSS_COMPILE)gold
> > > +CC         = $(CROSS_COMPILE)clang
> > > +else
> > >  LD         = $(CROSS_COMPILE)ld
> > >  CC         = $(CROSS_COMPILE)gcc
> > > +endif
> > >  CPP        = $(CC) -E
> > >  AR         = $(CROSS_COMPILE)ar
> > >  RANLIB     = $(CROSS_COMPILE)ranlib
> > 
> > LD-$(clang) = ...
> > LD-$(gcc)   = ...
> > 
> > LD := $(LD-y)
> 
> I tried that but it looks about as bad, and actually has more
> repetition. 

Sure.

Ian.

Re: [PATCH 8 of 8] xen: add "clang=y" option to build Xen with clang/llvm instead of gcc

[Tim Deegan]

At 17:01 +0000 on 07 Mar (1299517318), Keir Fraser wrote:
> On 07/03/2011 16:29, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:
> 
> > At 14:54 +0000 on 07 Mar (1299509669), Ian Campbell wrote:
> >> Is it worth arranging for "gcc := y" when clang is not enabled? Then a
> >> whole bunch of this sort of thing devolves into the
> >> CFLAGS-$(a-particular-cc) += -Wfoo
> >> pattern.
> > 
> > Something like the attached?  It tidies up four such ifeqs, at the cost
> > of one new one to define $(gcc).
> > (4 files changed, 17 insertions(+), 16 deletions(-))
> 
> This looks like a definite improvement in readability, to me. I'd like it to
> be applied.

Done (I'm interpreting this as an Acked-by:).

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH 2 of 8] x86: add explicit size suffixes to some assembly instructions

[Tim Deegan]

At 09:27 +0000 on 08 Mar (1299576427), Jan Beulich wrote:
> >>> On 07.03.11 at 12:26, Tim Deegan <Tim.Deegan@citrix.com> wrote:
> >--- a/xen/arch/x86/x86_emulate/x86_emulate.c	Mon Mar 07 11:21:11 2011 +0000
> >+++ b/xen/arch/x86/x86_emulate/x86_emulate.c	Mon Mar 07 11:21:11 2011 +0000
> >@@ -2678,7 +2678,7 @@ x86_emulate(
> >                 emulate_fpu_insn_memsrc("fiaddl", src.val);
> >                 break;
> >             case 1: /* fimul m64i */
> >-                emulate_fpu_insn_memsrc("fimul", src.val);
> >+                emulate_fpu_insn_memsrc("fimuls", src.val);
> 
> fimull.

fimuls is what the previous code generates, but it does look like that
was wrong.  I'll fix it, thanks.

Tim.

> >                 break;
> >             case 2: /* ficom m64i */
> >                 emulate_fpu_insn_memsrc("ficoml", src.val);
> 
> (Side note: The comments in this section are all wrong - they
> should say "m32i"; there are no 64-bit integer operations
> other than loads and stores.)



-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH 2 of 8] x86: add explicit size suffixes to some assembly instructions

[Jan Beulich]

>>> On 08.03.11 at 11:44, Tim Deegan <Tim.Deegan@citrix.com> wrote:
> At 09:27 +0000 on 08 Mar (1299576427), Jan Beulich wrote:
>> >>> On 07.03.11 at 12:26, Tim Deegan <Tim.Deegan@citrix.com> wrote:
>> >--- a/xen/arch/x86/x86_emulate/x86_emulate.c	Mon Mar 07 11:21:11 2011 +0000
>> >+++ b/xen/arch/x86/x86_emulate/x86_emulate.c	Mon Mar 07 11:21:11 2011 +0000
>> >@@ -2678,7 +2678,7 @@ x86_emulate(
>> >                 emulate_fpu_insn_memsrc("fiaddl", src.val);
>> >                 break;
>> >             case 1: /* fimul m64i */
>> >-                emulate_fpu_insn_memsrc("fimul", src.val);
>> >+                emulate_fpu_insn_memsrc("fimuls", src.val);
>> 
>> fimull.
> 
> fimuls is what the previous code generates, but it does look like that
> was wrong.  I'll fix it, thanks.

And perhaps that fix by itself should go into 4.1/4.0 too.

Jan

Re: [PATCH 2 of 8] x86: add explicit size suffixes to some assembly instructions

[Keir Fraser]

On 08/03/2011 11:01, "Jan Beulich" <JBeulich@novell.com> wrote:

>>>> On 08.03.11 at 11:44, Tim Deegan <Tim.Deegan@citrix.com> wrote:
>> At 09:27 +0000 on 08 Mar (1299576427), Jan Beulich wrote:
>>>>>> On 07.03.11 at 12:26, Tim Deegan <Tim.Deegan@citrix.com> wrote:
>>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c Mon Mar 07 11:21:11 2011 +0000
>>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c Mon Mar 07 11:21:11 2011 +0000
>>>> @@ -2678,7 +2678,7 @@ x86_emulate(
>>>>                 emulate_fpu_insn_memsrc("fiaddl", src.val);
>>>>                 break;
>>>>             case 1: /* fimul m64i */
>>>> -                emulate_fpu_insn_memsrc("fimul", src.val);
>>>> +                emulate_fpu_insn_memsrc("fimuls", src.val);
>>> 
>>> fimull.
>> 
>> fimuls is what the previous code generates, but it does look like that
>> was wrong.  I'll fix it, thanks.
> 
> And perhaps that fix by itself should go into 4.1/4.0 too.

Done. Also I fixed the comments to read 'm32i' and also noted and fixed
src.bytes=ea.bytes=4 rather than 8.

 -- Keir

> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel