xen/24996-x86-cpuidle-array-overrun.patch

# HG changeset patch
# User Eric Chanudet <eric.chanudet@eu.citrix.com>
# Date 1331222672 -3600
# Node ID 396801f25e922bdf1db5fd644435f46407586524
# Parent  66de4220113e937811529b12ea7f6427c0848630
XENPF_set_processor_pminfo XEN_PM_CX overflows states array

Calling XENPF_set_processor_pminfo with XEN_PM_CX could cause states
array in "struct acpi_processor_power" to exceed its limit.

The array used to be reset (by function cpuidle_init_cpu()) for each
hypercall. The patch puts it back that way and adds an assertion to
make it clear in case that happens again.

Signed-off-by: Eric Chanudet <eric.chanudet@eu.citrix.com>

- convert assertion to printk() & bail
- eliminate struct acpi_processor_cx's valid member (not read anymore)
- further adjustments to one-time-only vs each-time operations in
  cpuidle_init_cpu()
- don't use ACPI_STATE_Cn as array index anymore

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>

--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -70,10 +70,8 @@ static void lapic_timer_nop(void) { }
 static void (*lapic_timer_off)(void);
 static void (*lapic_timer_on)(void);
 
-static uint64_t (*get_tick)(void);
-static uint64_t (*ticks_elapsed)(uint64_t t1, uint64_t t2);
-static uint64_t (*tick_to_ns)(uint64_t ticks);
-static uint64_t (*ns_to_tick)(uint64_t ticks);
+static uint64_t (*__read_mostly tick_to_ns)(uint64_t) = acpi_pm_tick_to_ns;
+static uint64_t (*__read_mostly ns_to_tick)(uint64_t) = ns_to_acpi_pm_tick;
 
 extern void (*pm_idle) (void);
 extern void (*dead_idle) (void);
@@ -224,6 +222,10 @@ static uint64_t acpi_pm_ticks_elapsed(ui
         return ((0xFFFFFFFF - t1) + t2 +1);
 }
 
+static uint64_t (*__read_mostly get_tick)(void) = get_acpi_pm_tick;
+static uint64_t (*__read_mostly ticks_elapsed)(uint64_t, uint64_t)
+    = acpi_pm_ticks_elapsed;
+
 #define MWAIT_ECX_INTERRUPT_BREAK   (0x1)
 
 /*
@@ -609,7 +611,16 @@ static int cpuidle_init_cpu(int cpu)
     acpi_power = processor_powers[cpu];
     if ( !acpi_power )
     {
-        int i;
+        unsigned int i;
+
+        if ( cpu == 0 && boot_cpu_has(X86_FEATURE_NONSTOP_TSC) )
+        {
+            get_tick = get_stime_tick;
+            ticks_elapsed = stime_ticks_elapsed;
+            tick_to_ns = stime_tick_to_ns;
+            ns_to_tick = ns_to_stime_tick;
+        }
+
         acpi_power = xmalloc(struct acpi_processor_power);
         if ( !acpi_power )
             return -ENOMEM;
@@ -617,36 +628,15 @@ static int cpuidle_init_cpu(int cpu)
 
         for ( i = 0; i < ACPI_PROCESSOR_MAX_POWER; i++ )
             acpi_power->states[i].idx = i;
-     
-        acpi_power->states[ACPI_STATE_C1].type = ACPI_STATE_C1;
-        acpi_power->states[ACPI_STATE_C1].entry_method = ACPI_CSTATE_EM_HALT;
-     
-        acpi_power->states[ACPI_STATE_C0].valid = 1;
-        acpi_power->states[ACPI_STATE_C1].valid = 1;
-     
-        acpi_power->count = 2;
-        acpi_power->safe_state = &acpi_power->states[ACPI_STATE_C1];
+
         acpi_power->cpu = cpu;
         processor_powers[cpu] = acpi_power;
     }
 
-    if ( cpu == 0 )
-    {
-        if ( boot_cpu_has(X86_FEATURE_NONSTOP_TSC) )
-        {
-            get_tick = get_stime_tick;
-            ticks_elapsed = stime_ticks_elapsed;
-            tick_to_ns = stime_tick_to_ns;
-            ns_to_tick = ns_to_stime_tick;
-        }
-        else
-        {
-            get_tick = get_acpi_pm_tick;
-            ticks_elapsed = acpi_pm_ticks_elapsed;
-            tick_to_ns = acpi_pm_tick_to_ns;
-            ns_to_tick = ns_to_acpi_pm_tick;
-        }
-    }
+    acpi_power->count = 2;
+    acpi_power->states[1].type = ACPI_STATE_C1;
+    acpi_power->states[1].entry_method = ACPI_CSTATE_EM_HALT;
+    acpi_power->safe_state = &acpi_power->states[1];
 
     return 0;
 }
@@ -863,17 +853,25 @@ static void set_cx(
     if ( check_cx(acpi_power, xen_cx) != 0 )
         return;
 
-    if ( xen_cx->type == ACPI_STATE_C1 )
+    switch ( xen_cx->type )
+    {
+    case ACPI_STATE_C1:
         cx = &acpi_power->states[1];
-    else
-        cx = &acpi_power->states[acpi_power->count];
-
-    if ( !cx->valid )
-        acpi_power->count++;
+        break;
+    default:
+        if ( acpi_power->count >= ACPI_PROCESSOR_MAX_POWER )
+        {
+    case ACPI_STATE_C0:
+            printk(XENLOG_WARNING "CPU%u: C%d data ignored\n",
+                   acpi_power->cpu, xen_cx->type);
+            return;
+        }
+        cx = &acpi_power->states[acpi_power->count++];
+        cx->type = xen_cx->type;
+        break;
+    }
 
-    cx->valid    = 1;
-    cx->type     = xen_cx->type;
-    cx->address  = xen_cx->reg.address;
+    cx->address = xen_cx->reg.address;
 
     switch ( xen_cx->reg.space_id )
     {
--- a/xen/include/xen/cpuidle.h
+++ b/xen/include/xen/cpuidle.h
@@ -40,7 +40,6 @@
 struct acpi_processor_cx
 {
     u8 idx;
-    u8 valid;
     u8 type;
     u32 address;
     u8 entry_method; /* ACPI_CSTATE_EM_xxx */