[Xen-devel] [PATCH RFC 37/44] x86/misc: Move some IPI parameters off the stack

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

With percpu stacks, it will not be safe to pass stack pointers.  The logic in
machine_restart(), time_calibration() and set_mtrr() is singleton, so switch
to using static variables.

The set_mtrr_data is protected under the mtrr_mutex, which requires
mtrr_ap_init() and mtrr_aps_sync_end() to hold the mutex around calls to
set_mtrr().

time_calibration() runs exclusively out of a timer on cpu0 so is safe, while
machine_restart() doesn't have any concurrency to be worried about.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
 xen/arch/x86/cpu/mtrr/main.c | 27 +++++++++++++++++----------
 xen/arch/x86/shutdown.c      |  8 ++++++--
 xen/arch/x86/time.c          |  7 +++++--
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/cpu/mtrr/main.c b/xen/arch/x86/cpu/mtrr/main.c
index 56f71a6..d8ae9bf 100644
--- a/xen/arch/x86/cpu/mtrr/main.c
+++ b/xen/arch/x86/cpu/mtrr/main.c
@@ -59,9 +59,6 @@ u64 __read_mostly size_and_mask;
 
 const struct mtrr_ops *__read_mostly mtrr_if = NULL;
 
-static void set_mtrr(unsigned int reg, unsigned long base,
-                    unsigned long size, mtrr_type type);
-
 static const char *const mtrr_strings[MTRR_NUM_TYPES] =
 {
     "uncachable",               /* 0 */
@@ -211,21 +208,27 @@ static inline int types_compatible(mtrr_type type1, 
mtrr_type type2) {
 static void set_mtrr(unsigned int reg, unsigned long base,
                     unsigned long size, mtrr_type type)
 {
+       /* Can't pass a stack pointer to an IPI. */
+       static struct set_mtrr_data data;
+
        cpumask_t allbutself;
        unsigned int nr_cpus;
-       struct set_mtrr_data data;
        unsigned long flags;
 
+       ASSERT(spin_is_locked(&mtrr_mutex));
+
        cpumask_andnot(&allbutself, &cpu_online_map,
                       cpumask_of(smp_processor_id()));
        nr_cpus = cpumask_weight(&allbutself);
 
-       data.smp_reg = reg;
-       data.smp_base = base;
-       data.smp_size = size;
-       data.smp_type = type;
-       atomic_set(&data.count, nr_cpus);
-       atomic_set(&data.gate,0);
+       data = (struct set_mtrr_data){
+               .smp_reg  = reg,
+               .smp_base = base,
+               .smp_size = size,
+               .smp_type = type,
+               .count    = ATOMIC_INIT(nr_cpus),
+               .gate     = ATOMIC_INIT(0),
+       };
 
        /* Start the ball rolling on other CPUs */
        on_selected_cpus(&allbutself, ipi_handler, &data, 0);
@@ -593,7 +596,9 @@ void mtrr_ap_init(void)
         * 2.cpu hotadd time. We let mtrr_add/del_page hold cpuhotplug lock to
         * prevent mtrr entry changes
         */
+       mutex_lock(&mtrr_mutex);
        set_mtrr(~0U, 0, 0, 0);
+       mutex_unlock(&mtrr_mutex);
 }
 
 /**
@@ -621,7 +626,9 @@ void mtrr_aps_sync_end(void)
 {
        if (!use_intel())
                return;
+       mutex_lock(&mtrr_mutex);
        set_mtrr(~0U, 0, 0, 0);
+       mutex_unlock(&mtrr_mutex);
        hold_mtrr_updates_on_aps = 0;
 }
 
diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index a87aa60..29317d9 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -536,9 +536,13 @@ void machine_restart(unsigned int delay_millisecs)
         /* Ensure we are the boot CPU. */
         if ( get_apic_id() != boot_cpu_physical_apicid )
         {
+            /* Can't pass a stack pointer to an IPI. */
+            static unsigned int delay;
+
+            delay = delay_millisecs;
+
             /* Send IPI to the boot CPU (logical cpu 0). */
-            on_selected_cpus(cpumask_of(0), __machine_restart,
-                             &delay_millisecs, 0);
+            on_selected_cpus(cpumask_of(0), __machine_restart, &delay, 0);
             for ( ; ; )
                 halt();
         }
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 2a87950..390cf0c 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1446,8 +1446,11 @@ static void (*time_calibration_rendezvous_fn)(void *) =
 
 static void time_calibration(void *unused)
 {
-    struct calibration_rendezvous r = {
-        .semaphore = ATOMIC_INIT(0)
+    /* Can't pass a stack pointer to an IPI. */
+    static struct calibration_rendezvous r;
+
+    r = (struct calibration_rendezvous){
+        .semaphore = ATOMIC_INIT(0),
     };
 
     if ( clocksource_is_tsc() )
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel