>>> On 24.11.14 at 10:08, <sfl...@ihonk.com> wrote:
> On Nov 24, 2014, at 00:45, Jan Beulich <jbeul...@suse.com> wrote:
> 
>>>>> On 23.11.14 at 02:28, <sfl...@ihonk.com> wrote:
>>> With mwait-idle=0:
>>> 
>>> (XEN) 'c' pressed -> printing ACPI Cx structures
>>> (XEN) ==cpu0==
>>> (XEN) active state:             C0
>>> (XEN) max_cstate:               C7
>>> (XEN) states:
>>> (XEN)     C1:   type[C1] latency[001] usage[00000000] method[  FFH] 
>>> duration[0]
>>> (XEN)     C2:   type[C0] latency[000] usage[00000000] method[ NONE] 
>>> duration[0]
>>> (XEN)     C3:   type[C3] latency[064] usage[00000000] method[  FFH] 
>>> duration[0]
>>> (XEN)     C4:   type[C3] latency[096] usage[00000000] method[  FFH] 
>>> duration[0]
>>> (XEN)    *C0:   usage[00000000] duration[46930624784]
>>> (XEN) PC2[0] PC3[0] PC6[0] PC7[0]
>>> (XEN) CC3[0] CC6[0] CC7[0]
>>> [...]
>> 
>> Very interesting - the hypervisor has C-state information, but never
>> entered any of them. That certainly explains the difference between
>> using/not using the ,wait-idle driver, but puts us back to there being
>> a more general issue with C-state use on this CPU model. Possibly
>> related to C2 having entry method "NONE", but then again I can't
>> see how such a state could get entered into the table the first place:
>> set_cx() bails upon check_cx() returning an error, and hence its
>> switch()'s default statement should never be reached. Plus even if an
>> array entry was set to "NONE", it should simply be ignored when
>> looking for a state to enter. I'll probably need to put together a
>> debugging patch to figure out what's going on here.
> 
> Okay, happy to give it a go whenever you have the time to put something 
> together.

While putting this together I found the reason for the strange
C2: line, and the attached debugging patch already has the fix
for it (which I'll also submit separately, and hence you may need
to drop that specific hunk should you end up applying it on a tree
which already has that fix). You'll need to again run with
"mwait-idle=0", and it's the boot messages along with the 'c'
debug key output that's of interest.

Thanks, Jan

--- unstable.orig/xen/arch/x86/acpi/cpu_idle.c
+++ unstable/xen/arch/x86/acpi/cpu_idle.c
@@ -58,7 +58,7 @@
 #include <xen/notifier.h>
 #include <xen/cpu.h>
 
-/*#define DEBUG_PM_CX*/
+#define DEBUG_PM_CX
 
 #define GET_HW_RES_IN_NS(msr, val) \
     do { rdmsrl(msr, val); val = tsc_ticks2ns(val); } while( 0 )
@@ -238,6 +238,9 @@ static char* acpi_cstate_method_name[] =
     "HALT"
 };
 
+struct reasons { unsigned long max, pwr, urg, nxt; };//temp
+static DEFINE_PER_CPU(struct reasons, reasons);//temp
+
 static void print_acpi_power(uint32_t cpu, struct acpi_processor_power *power)
 {
     uint32_t i, idle_usage = 0;
@@ -273,6 +276,8 @@ static void print_acpi_power(uint32_t cp
     printk((last_state_idx == 0) ? "   *" : "    ");
     printk("C0:\tusage[%08d] duration[%"PRId64"]\n",
            idle_usage, NOW() - idle_res);
+printk("max=%lx pwr=%lx urg=%lx nxt=%lx\n",//temp
+       per_cpu(reasons.max, cpu), per_cpu(reasons.pwr, cpu), 
per_cpu(reasons.urg, cpu), per_cpu(reasons.nxt, cpu));
 
     print_hw_residencies(cpu);
 }
@@ -501,6 +506,7 @@ static void acpi_processor_idle(void)
     u32 exp = 0, pred = 0;
     u32 irq_traced[4] = { 0 };
 
+next_state = 1;//temp
     if ( max_cstate > 0 && power && !sched_has_urgent_vcpu() &&
          (next_state = cpuidle_current_governor->select(power)) > 0 )
     {
@@ -519,6 +525,10 @@ static void acpi_processor_idle(void)
     }
     if ( !cx )
     {
+this_cpu(reasons.max) += max_cstate <= 0;//temp
+this_cpu(reasons.pwr) += !power;//temp
+this_cpu(reasons.urg) += !!sched_has_urgent_vcpu();//temp
+this_cpu(reasons.nxt) += next_state <= 0;//temp
         if ( pm_idle_save )
             pm_idle_save();
         else
@@ -1007,6 +1017,7 @@ static void set_cx(
         cx->entry_method = ACPI_CSTATE_EM_SYSIO;
         break;
     default:
+printk("CPU%u: C%u space %x?\n", acpi_power->cpu, xen_cx->type, 
xen_cx->reg.space_id);//temp
         cx->entry_method = ACPI_CSTATE_EM_NONE;
         break;
     }
@@ -1015,7 +1026,7 @@ static void set_cx(
     cx->target_residency = cx->latency * latency_factor;
 
     smp_wmb();
-    acpi_power->count++;
+    acpi_power->count += (cx->type != ACPI_STATE_C1);
     if ( cx->type == ACPI_STATE_C1 || cx->type == ACPI_STATE_C2 )
         acpi_power->safe_state = cx;
 }
@@ -1141,6 +1152,7 @@ long set_cx_pminfo(uint32_t cpu, struct 
 
     /* FIXME: C-state dependency is not supported by far */
 
+printk("CPU%u: %pS, %pS\n", cpu, pm_idle_save, pm_idle);//temp
     if ( cpu_id == 0 )
     {
         if ( pm_idle_save == NULL )
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Reply via email to