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

[Eric Chanudet <eric.chanudet@xxxxxxxxxxxxx>]

On Wed, Mar 07, 2012 at 10:07:55AM -0500, Jan Beulich wrote:
> Is there any reason to move all of these assignments, rather than
> just the assignment of ->count and maybe ->states[ACPI_STATE_C1]?
> Everything else should not get modified anywhere else.

I see no reason indeed, this is how it was before everything went up in
the if() statement.

> To be on the safe side, you should probably also check for
> ACPI_STATE_C0 (and ignore its data), otherwise (other than before
> your patch) ->count would get incremented for this too. So I'd
> suggest converting the if() here to a switch().

Done.

Here is the refreshed patch.

diff -r 8964c223836c xen/arch/x86/acpi/cpu_idle.c
--- a/xen/arch/x86/acpi/cpu_idle.c      Tue Mar 06 15:51:33 2012 +0100
+++ b/xen/arch/x86/acpi/cpu_idle.c      Wed Mar 07 17:39:34 2012 +0000
@@ -645,12 +645,12 @@
      
         acpi_power->states[ACPI_STATE_C0].valid = 1;
         acpi_power->states[ACPI_STATE_C1].valid = 1;
+        acpi_power->cpu = cpu;
+        processor_powers[cpu] = acpi_power;
+    }
      
         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 )
     {
@@ -885,16 +885,20 @@
     if ( check_cx(acpi_power, xen_cx) != 0 )
         return;
 
-    if ( xen_cx->type == ACPI_STATE_C1 )
+    switch (xen_cx->type)
+    {
+    case ACPI_STATE_C0:
+        return;
+    case ACPI_STATE_C1:
         cx = &acpi_power->states[1];
-    else
-        cx = &acpi_power->states[acpi_power->count];
+        break;
+    default:
+        ASSERT(acpi_power->count < ACPI_PROCESSOR_MAX_POWER);
+        cx = &acpi_power->states[acpi_power->count++];
+        cx->valid = 1;
+        cx->type = xen_cx->type;
+    }
 
-    if ( !cx->valid )
-        acpi_power->count++;
-
-    cx->valid    = 1;
-    cx->type     = xen_cx->type;
     cx->address  = xen_cx->reg.address;
 
     switch ( xen_cx->reg.space_id )

-- 
Eric Chanudet

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