[Xen-changelog] [xen-unstable] x2apic: Clean up send_IPI_mask_x2apic() and add a memory barrier.

[Xen patchbot-unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>]

# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1222333384 -3600
# Node ID 50170dc8649cb43630f067eb9b2db3ce3f9c6db4
# Parent  b53b02976633c20ff5c462cf109d9a3f41cbc2ce
x2apic: Clean up send_IPI_mask_x2apic() and add a memory barrier.

The barrier is required to prevent the WRMSR from executing before the
processor has written synchronising data to be received by remote
CPUs.

At the same time remove needless wmb() from on_selected_cpus(). We now
assume send_IPI_mask() is a sufficient compiler and CPU memory
barrier.

Original patch by Kevin Tian <kevin.tian@xxxxxxxxx>
Signed-off-by: Keir Fraser <keir.fraser@xxxxxxxxxx>
---
 xen/arch/x86/genapic/x2apic.c |   23 ++++++++++++++---------
 xen/arch/x86/smp.c            |    1 -
 2 files changed, 14 insertions(+), 10 deletions(-)

diff -r b53b02976633 -r 50170dc8649c xen/arch/x86/genapic/x2apic.c
--- a/xen/arch/x86/genapic/x2apic.c     Wed Sep 24 12:47:33 2008 +0100
+++ b/xen/arch/x86/genapic/x2apic.c     Thu Sep 25 10:03:04 2008 +0100
@@ -58,21 +58,26 @@ unsigned int cpu_mask_to_apicid_x2apic(c
 
 void send_IPI_mask_x2apic(cpumask_t cpumask, int vector)
 {
-    unsigned int query_cpu;
-    u32 cfg, dest;
+    unsigned int cpu, cfg;
     unsigned long flags;
 
-    ASSERT(cpus_subset(cpumask, cpu_online_map));
-    ASSERT(!cpus_empty(cpumask));
+    /*
+     * Ensure that any synchronisation data written in program order by this
+     * CPU is seen by notified remote CPUs. The WRMSR contained within
+     * apic_icr_write() can otherwise be executed early.
+     * 
+     * The reason mb() is sufficient here is subtle: the register arguments
+     * to WRMSR must depend on a memory read executed after the barrier. This
+     * is guaranteed by cpu_physical_id(), which reads from a global array (and
+     * so cannot be hoisted above the barrier even by a clever compiler).
+     */
+    mb();
 
     local_irq_save(flags);
 
     cfg = APIC_DM_FIXED | 0 /* no shorthand */ | APIC_DEST_PHYSICAL | vector;
-    for_each_cpu_mask(query_cpu, cpumask)
-    {
-        dest =  cpu_physical_id(query_cpu);
-        apic_icr_write(cfg, dest);
-    }
+    for_each_cpu_mask ( cpu, cpumask )
+        apic_wrmsr(APIC_ICR, cfg, cpu_physical_id(cpu));
 
     local_irq_restore(flags);
 }
diff -r b53b02976633 -r 50170dc8649c xen/arch/x86/smp.c
--- a/xen/arch/x86/smp.c        Wed Sep 24 12:47:33 2008 +0100
+++ b/xen/arch/x86/smp.c        Thu Sep 25 10:03:04 2008 +0100
@@ -280,7 +280,6 @@ int on_selected_cpus(
     spin_lock(&call_lock);
 
     call_data = &data;
-    wmb();
 
     send_IPI_mask(selected, CALL_FUNCTION_VECTOR);
 

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