[Xen-devel] [PATCH] Re: XENPF_set_processor_pminfo XEN_PM_CX overflows states array

["Jan Beulich" <JBeulich@xxxxxxxx>]

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@xxxxxxxxxxxxx>

- 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@xxxxxxxx>

--- 2012-03-06.orig/xen/arch/x86/acpi/cpu_idle.c        2012-03-06 
13:35:15.000000000 +0100
+++ 2012-03-06/xen/arch/x86/acpi/cpu_idle.c     2012-03-08 12:36:24.000000000 
+0100
@@ -73,10 +73,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;
 
 static void (*pm_idle_save) (void) __read_mostly;
 unsigned int max_cstate __read_mostly = ACPI_PROCESSOR_MAX_POWER - 1;
@@ -234,6 +232,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)
 
 /*
@@ -632,43 +634,31 @@ 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 = xzalloc(struct acpi_processor_power);
         if ( !acpi_power )
             return -ENOMEM;
 
         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;
 }
@@ -885,17 +875,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 )
     {
--- 2012-03-06.orig/xen/include/xen/cpuidle.h   2011-06-16 09:21:02.000000000 
+0200
+++ 2012-03-06/xen/include/xen/cpuidle.h        2012-03-08 12:34:26.000000000 
+0100
@@ -40,7 +40,6 @@
 struct acpi_processor_cx
 {
     u8 idx;
-    u8 valid;
     u8 type;
     u32 address;
     u8 entry_method; /* ACPI_CSTATE_EM_xxx */



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel