[V7] x86/cpuidle: get accurate C0 value with xenpm tool

[V7] x86/cpuidle: get accurate C0 value with xenpm tool

[Huaitong Han]

When checking the ACPI funciton of C-status, after 100 seconds sleep,
the sampling value of C0 status from the xenpm tool decreases.
Because C0=NOW()-C1-C2-C3-C4, when NOW() value is during idle time,
NOW() value is bigger than last C-status update time, and C0 value
is also bigger than ture value. if margin of the second error cannot
make up for margin of the first error, the value of C0 would decrease.

Signed-off-by: Huaitong Han <huaitong.han@intel.com>

---
ChangeLog:
V7:
delete uint64_t;
delete extern variable and keep variable static.

V6:
Add blank lines.
delete res_ticks[] and a wrong comment.

V5:
Ticks clock souce may be acpi_pm, so use common funciton "ticks_elapsed".
Taking every "tick_to_ns" outside spin_lock.
Spliting the "for" loop.

V4:
delete pointless initializers and hard tabs.

V3:
1.Don't use tick_to_ns inside lock in print_acpi_power.
2.Use 08 padding in printk.
3.Merge two "for" circulation into one for coding style.

V2:
C0 = last_cx_update_time-C1-C2-C3-C4, but last_cx_update_time is not now,
so the C0 value is stale, NOW-last_update_time should be calculated.
C[current_cx_stat]+=NOW-last_update_time, so the CX value is fresh.

V1:
Initial patch
---

diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index e639c99..39b5e4d 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -252,11 +252,33 @@ static char* acpi_cstate_method_name[] =
     "HALT"
 };
 
+static uint64_t get_stime_tick(void) { return (uint64_t)NOW(); }
+static uint64_t stime_ticks_elapsed(uint64_t t1, uint64_t t2) { return t2 - t1; }
+static uint64_t stime_tick_to_ns(uint64_t ticks) { return ticks; }
+
+static uint64_t get_acpi_pm_tick(void) { return (uint64_t)inl(pmtmr_ioport); }
+static uint64_t acpi_pm_ticks_elapsed(uint64_t t1, uint64_t t2)
+{
+    if ( t2 >= t1 )
+        return (t2 - t1);
+    else if ( !(acpi_gbl_FADT.flags & ACPI_FADT_32BIT_TIMER) )
+        return (((0x00FFFFFF - t1) + t2 + 1) & 0x00FFFFFF);
+    else
+        return ((0xFFFFFFFF - t1) + t2 +1);
+}
+
+uint64_t (*__read_mostly cpuidle_get_tick)(void) = get_acpi_pm_tick;
+static uint64_t (*__read_mostly ticks_elapsed)(uint64_t, uint64_t)
+    = acpi_pm_ticks_elapsed;
+
+
 static void print_acpi_power(uint32_t cpu, struct acpi_processor_power *power)
 {
-    uint32_t i, idle_usage = 0;
-    uint64_t res, idle_res = 0;
-    u32 usage;
+    uint64_t idle_res = 0, idle_usage = 0;
+    uint64_t last_state_update_tick, current_tick, current_stime;
+    uint64_t usage[ACPI_PROCESSOR_MAX_POWER] = { 0 };
+    uint64_t res_tick[ACPI_PROCESSOR_MAX_POWER] = { 0 };
+    unsigned int i;
     u8 last_state_idx;
 
     printk("==cpu%d==\n", cpu);
@@ -264,28 +286,37 @@ static void print_acpi_power(uint32_t cpu, struct acpi_processor_power *power)
     printk("active state:\t\tC%d\n", last_state_idx);
     printk("max_cstate:\t\tC%d\n", max_cstate);
     printk("states:\n");
-    
+
+    spin_lock_irq(&power->stat_lock);
+    current_tick = cpuidle_get_tick();
+    current_stime = NOW();
     for ( i = 1; i < power->count; i++ )
     {
-        spin_lock_irq(&power->stat_lock);	
-        res = tick_to_ns(power->states[i].time);
-        usage = power->states[i].usage;
-        spin_unlock_irq(&power->stat_lock);
+        res_tick[i] = power->states[i].time;
+        usage[i] = power->states[i].usage;
+    }
+    last_state_update_tick = power->last_state_update_tick;
+    spin_unlock_irq(&power->stat_lock);
 
-        idle_usage += usage;
-        idle_res += res;
+    res_tick[last_state_idx] += ticks_elapsed(last_state_update_tick, current_tick);
+    usage[last_state_idx]++;
+
+    for ( i = 1; i < power->count; i++ )
+    {
+        idle_usage += usage[i];
+        idle_res += tick_to_ns(res_tick[i]);
 
         printk((last_state_idx == i) ? "   *" : "    ");
         printk("C%d:\t", i);
         printk("type[C%d] ", power->states[i].type);
         printk("latency[%03d] ", power->states[i].latency);
-        printk("usage[%08d] ", usage);
+        printk("usage[%08"PRIu64"] ", usage[i]);
         printk("method[%5s] ", acpi_cstate_method_name[power->states[i].entry_method]);
-        printk("duration[%"PRId64"]\n", res);
+        printk("duration[%"PRIu64"]\n", tick_to_ns(res_tick[i]));
     }
     printk((last_state_idx == 0) ? "   *" : "    ");
-    printk("C0:\tusage[%08d] duration[%"PRId64"]\n",
-           idle_usage, NOW() - idle_res);
+    printk("C0:\tusage[%08"PRIu64"] duration[%"PRIu64"]\n",
+           usage[0] + idle_usage, current_stime - idle_res);
 
     print_hw_residencies(cpu);
 }
@@ -313,24 +344,6 @@ static int __init cpu_idle_key_init(void)
 }
 __initcall(cpu_idle_key_init);
 
-static uint64_t get_stime_tick(void) { return (uint64_t)NOW(); }
-static uint64_t stime_ticks_elapsed(uint64_t t1, uint64_t t2) { return t2 - t1; }
-static uint64_t stime_tick_to_ns(uint64_t ticks) { return ticks; }
-
-static uint64_t get_acpi_pm_tick(void) { return (uint64_t)inl(pmtmr_ioport); }
-static uint64_t acpi_pm_ticks_elapsed(uint64_t t1, uint64_t t2)
-{
-    if ( t2 >= t1 )
-        return (t2 - t1);
-    else if ( !(acpi_gbl_FADT.flags & ACPI_FADT_32BIT_TIMER) )
-        return (((0x00FFFFFF - t1) + t2 + 1) & 0x00FFFFFF);
-    else
-        return ((0xFFFFFFFF - t1) + t2 +1);
-}
-
-uint64_t (*__read_mostly cpuidle_get_tick)(void) = get_acpi_pm_tick;
-static uint64_t (*__read_mostly ticks_elapsed)(uint64_t, uint64_t)
-    = acpi_pm_ticks_elapsed;
 
 /*
  * The bit is set iff cpu use monitor/mwait to enter C state
@@ -486,6 +499,17 @@ bool_t errata_c6_eoi_workaround(void)
     return (fix_needed && cpu_has_pending_apic_eoi());
 }
 
+void update_last_cx_stat(struct acpi_processor_power *power,
+                         struct acpi_processor_cx *cx, uint64_t ticks)
+{
+    ASSERT(!local_irq_is_enabled());
+
+    spin_lock(&power->stat_lock);
+    power->last_state = cx;
+    power->last_state_update_tick = ticks;
+    spin_unlock(&power->stat_lock);
+}
+
 void update_idle_stats(struct acpi_processor_power *power,
                        struct acpi_processor_cx *cx,
                        uint64_t before, uint64_t after)
@@ -501,6 +525,8 @@ void update_idle_stats(struct acpi_processor_power *power,
         power->last_residency = tick_to_ns(sleep_ticks) / 1000UL;
         cx->time += sleep_ticks;
     }
+    power->last_state = &power->states[0];
+    power->last_state_update_tick = after;
 
     spin_unlock(&power->stat_lock);
 }
@@ -557,7 +583,6 @@ static void acpi_processor_idle(void)
     if ( (cx->type == ACPI_STATE_C3) && errata_c6_eoi_workaround() )
         cx = power->safe_state;
 
-    power->last_state = cx;
 
     /*
      * Sleep:
@@ -574,6 +599,9 @@ static void acpi_processor_idle(void)
             t1 = cpuidle_get_tick();
             /* Trace cpu idle entry */
             TRACE_4D(TRC_PM_IDLE_ENTRY, cx->idx, t1, exp, pred);
+
+            update_last_cx_stat(power, cx, t1);
+
             /* Invoke C2 */
             acpi_idle_do_entry(cx);
             /* Get end time (ticks) */
@@ -603,6 +631,8 @@ static void acpi_processor_idle(void)
         /* Trace cpu idle entry */
         TRACE_4D(TRC_PM_IDLE_ENTRY, cx->idx, t1, exp, pred);
 
+        update_last_cx_stat(power, cx, t1);
+
         /*
          * disable bus master
          * bm_check implies we need ARB_DIS
@@ -1172,7 +1202,9 @@ int pmstat_get_cx_stat(uint32_t cpuid, struct pm_cx_stat *stat)
 {
     struct acpi_processor_power *power = processor_powers[cpuid];
     uint64_t idle_usage = 0, idle_res = 0;
-    uint64_t usage[ACPI_PROCESSOR_MAX_POWER], res[ACPI_PROCESSOR_MAX_POWER];
+    uint64_t last_state_update_tick, current_stime, current_tick;
+    uint64_t usage[ACPI_PROCESSOR_MAX_POWER] = { 0 };
+    uint64_t res[ACPI_PROCESSOR_MAX_POWER] = { 0 };
     unsigned int i, nr, nr_pc = 0, nr_cc = 0;
 
     if ( power == NULL )
@@ -1185,7 +1217,6 @@ int pmstat_get_cx_stat(uint32_t cpuid, struct pm_cx_stat *stat)
         return 0;
     }
 
-    stat->last = power->last_state ? power->last_state->idx : 0;
     stat->idle_time = get_cpu_idle_time(cpuid);
     nr = min(stat->nr, power->count);
 
@@ -1193,9 +1224,12 @@ int pmstat_get_cx_stat(uint32_t cpuid, struct pm_cx_stat *stat)
     if ( pm_idle_save == NULL )
     {
         stat->nr = 2;
+        stat->last = power->last_state ? power->last_state->idx : 0;
 
         usage[1] = idle_usage = 1;
         res[1] = idle_res = stat->idle_time;
+
+        current_stime = NOW();
     }
     else
     {
@@ -1203,13 +1237,24 @@ int pmstat_get_cx_stat(uint32_t cpuid, struct pm_cx_stat *stat)
 
         stat->nr = power->count;
 
+        spin_lock_irq(&power->stat_lock);
+        current_tick = cpuidle_get_tick();
+        current_stime = NOW();
         for ( i = 1; i < nr; i++ )
         {
-            spin_lock_irq(&power->stat_lock);
             usage[i] = power->states[i].usage;
-            res[i] = tick_to_ns(power->states[i].time);
-            spin_unlock_irq(&power->stat_lock);
+            res[i] = power->states[i].time;
+        }
+        last_state_update_tick = power->last_state_update_tick;
+        stat->last = power->last_state ? power->last_state->idx : 0;
+        spin_unlock_irq(&power->stat_lock);
 
+        usage[stat->last]++;
+        res[stat->last] += ticks_elapsed(last_state_update_tick, current_tick);
+
+        for ( i = 1; i < nr; i++ )
+        {
+            res[i] = tick_to_ns(res[i]);
             idle_usage += usage[i];
             idle_res += res[i];
         }
@@ -1242,8 +1287,8 @@ int pmstat_get_cx_stat(uint32_t cpuid, struct pm_cx_stat *stat)
 #undef PUT_xC
     }
 
-    usage[0] = idle_usage;
-    res[0] = NOW() - idle_res;
+    usage[0] += idle_usage;
+    res[0] = current_stime - idle_res;
 
     if ( copy_to_guest(stat->triggers, usage, nr) ||
          copy_to_guest(stat->residencies, res, nr) )
diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
index 6dd5822..13bf4ce 100644
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -536,7 +536,6 @@ static void mwait_idle(void)
 		return;
 	}
 
-	power->last_state = cx;
 	eax = cx->address;
 	cstate = ((eax >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1;
 
@@ -555,6 +554,8 @@ static void mwait_idle(void)
 	before = cpuidle_get_tick();
 	TRACE_4D(TRC_PM_IDLE_ENTRY, cx->type, before, exp, pred);
 
+	update_last_cx_stat(power, cx, before);
+
 	if (cpu_is_haltable(cpu))
 		mwait_idle_with_hints(eax, MWAIT_ECX_INTERRUPT_BREAK);
 
@@ -565,15 +566,13 @@ static void mwait_idle(void)
 	TRACE_6D(TRC_PM_IDLE_EXIT, cx->type, after,
 		irq_traced[0], irq_traced[1], irq_traced[2], irq_traced[3]);
 
+	/* Now back in C0. */
 	update_idle_stats(power, cx, before, after);
 	local_irq_enable();
 
 	if (!(lapic_timer_reliable_states & (1 << cstate)))
 		lapic_timer_on();
 
-	/* Now back in C0. */
-	power->last_state = &power->states[0];
-
 	sched_tick_resume();
 	cpufreq_dbs_timer_resume();
 
diff --git a/xen/include/asm-x86/cpuidle.h b/xen/include/asm-x86/cpuidle.h
index 4d70677..46e614b 100644
--- a/xen/include/asm-x86/cpuidle.h
+++ b/xen/include/asm-x86/cpuidle.h
@@ -23,6 +23,8 @@ void acpi_dead_idle(void);
 void trace_exit_reason(u32 *irq_traced);
 void update_idle_stats(struct acpi_processor_power *,
                        struct acpi_processor_cx *, uint64_t, uint64_t);
+void update_last_cx_stat(struct acpi_processor_power *,
+                         struct acpi_processor_cx *, uint64_t);
 
 /*
  * vcpu is urgent if vcpu is polling event channel
diff --git a/xen/include/xen/cpuidle.h b/xen/include/xen/cpuidle.h
index b7b9e8c..342f4fe 100644
--- a/xen/include/xen/cpuidle.h
+++ b/xen/include/xen/cpuidle.h
@@ -66,6 +66,7 @@ struct acpi_processor_power
     struct acpi_processor_cx *last_state;
     struct acpi_processor_cx *safe_state;
     void *gdata; /* governor specific data */
+    u64 last_state_update_tick;
     u32 last_residency;
     u32 count;
     spinlock_t stat_lock;
-- 
1.9.1

Re: [V7] x86/cpuidle: get accurate C0 value with xenpm tool

[Andrew Cooper]

On 21/05/15 10:39, Huaitong Han wrote:
> When checking the ACPI funciton of C-status, after 100 seconds sleep,
> the sampling value of C0 status from the xenpm tool decreases.
> Because C0=NOW()-C1-C2-C3-C4, when NOW() value is during idle time,
> NOW() value is bigger than last C-status update time, and C0 value
> is also bigger than ture value. if margin of the second error cannot
> make up for margin of the first error, the value of C0 would decrease.
>
> Signed-off-by: Huaitong Han <huaitong.han@intel.com>
>
>  static void print_acpi_power(uint32_t cpu, struct acpi_processor_power *power)
>  {
> -    uint32_t i, idle_usage = 0;
> -    uint64_t res, idle_res = 0;
> -    u32 usage;
> +    uint64_t idle_res = 0, idle_usage = 0;
> +    uint64_t last_state_update_tick, current_tick, current_stime;
> +    uint64_t usage[ACPI_PROCESSOR_MAX_POWER] = { 0 };
> +    uint64_t res_tick[ACPI_PROCESSOR_MAX_POWER] = { 0 };
> +    unsigned int i;
>      u8 last_state_idx;
>  
>      printk("==cpu%d==\n", cpu);
> @@ -264,28 +286,37 @@ static void print_acpi_power(uint32_t cpu, struct acpi_processor_power *power)
>      printk("active state:\t\tC%d\n", last_state_idx);
>      printk("max_cstate:\t\tC%d\n", max_cstate);
>      printk("states:\n");
> -    
> +
> +    spin_lock_irq(&power->stat_lock);
> +    current_tick = cpuidle_get_tick();
> +    current_stime = NOW();
>      for ( i = 1; i < power->count; i++ )
>      {
> -        spin_lock_irq(&power->stat_lock);	
> -        res = tick_to_ns(power->states[i].time);
> -        usage = power->states[i].usage;
> -        spin_unlock_irq(&power->stat_lock);
> +        res_tick[i] = power->states[i].time;
> +        usage[i] = power->states[i].usage;
> +    }
> +    last_state_update_tick = power->last_state_update_tick;
> +    spin_unlock_irq(&power->stat_lock);
>  
> -        idle_usage += usage;
> -        idle_res += res;
> +    res_tick[last_state_idx] += ticks_elapsed(last_state_update_tick, current_tick);
> +    usage[last_state_idx]++;

These two arrays get used with last_state_idx as -1, which underflows to
256.

Please see about fixing, or we can see about reverting if you can't get
to it quickly.

~Andrew

[PATCH] x86/cpuidle: prevent out of bounds array access

[Jan Beulich]

... resulting from fbeef5570c ("x86/cpuidle: get accurate C0 value with
xenpm tool").

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -279,7 +279,7 @@ static void print_acpi_power(
     uint64_t usage[ACPI_PROCESSOR_MAX_POWER] = { 0 };
     uint64_t res_tick[ACPI_PROCESSOR_MAX_POWER] = { 0 };
     unsigned int i;
-    u8 last_state_idx;
+    signed int last_state_idx;
 
     printk("==cpu%d==\n", cpu);
     last_state_idx = power->last_state ? power->last_state->idx : -1;
@@ -298,8 +298,12 @@ static void print_acpi_power(
     last_state_update_tick = power->last_state_update_tick;
     spin_unlock_irq(&power->stat_lock);
 
-    res_tick[last_state_idx] += ticks_elapsed(last_state_update_tick, current_tick);
-    usage[last_state_idx]++;
+    if ( last_state_idx >= 0 )
+    {
+        res_tick[last_state_idx] += ticks_elapsed(last_state_update_tick,
+                                                  current_tick);
+        usage[last_state_idx]++;
+    }
 
     for ( i = 1; i < power->count; i++ )
     {

Re: [PATCH] x86/cpuidle: prevent out of bounds array access

[Andrew Cooper]

On 22/05/15 13:48, Jan Beulich wrote:
> ... resulting from fbeef5570c ("x86/cpuidle: get accurate C0 value with
> xenpm tool").
>
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

This appears to fix the issue.

However, looking at the other cases which play the same games, 0 is used
in preference to -1, given a zero last_state.  It would seem logical to
follow suit here (although it is just a debugkey so I am not overly fussed).

~Andrew

>
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -279,7 +279,7 @@ static void print_acpi_power(
>      uint64_t usage[ACPI_PROCESSOR_MAX_POWER] = { 0 };
>      uint64_t res_tick[ACPI_PROCESSOR_MAX_POWER] = { 0 };
>      unsigned int i;
> -    u8 last_state_idx;
> +    signed int last_state_idx;
>  
>      printk("==cpu%d==\n", cpu);
>      last_state_idx = power->last_state ? power->last_state->idx : -1;
> @@ -298,8 +298,12 @@ static void print_acpi_power(
>      last_state_update_tick = power->last_state_update_tick;
>      spin_unlock_irq(&power->stat_lock);
>  
> -    res_tick[last_state_idx] += ticks_elapsed(last_state_update_tick, current_tick);
> -    usage[last_state_idx]++;
> +    if ( last_state_idx >= 0 )
> +    {
> +        res_tick[last_state_idx] += ticks_elapsed(last_state_update_tick,
> +                                                  current_tick);
> +        usage[last_state_idx]++;
> +    }
>  
>      for ( i = 1; i < power->count; i++ )
>      {
>
>
>

Re: [PATCH] x86/cpuidle: prevent out of bounds array access

[Jan Beulich]

>>> On 22.05.15 at 14:57, <andrew.cooper3@citrix.com> wrote:
> On 22/05/15 13:48, Jan Beulich wrote:
>> ... resulting from fbeef5570c ("x86/cpuidle: get accurate C0 value with
>> xenpm tool").
>>
>> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> This appears to fix the issue.
> 
> However, looking at the other cases which play the same games, 0 is used
> in preference to -1, given a zero last_state.  It would seem logical to
> follow suit here (although it is just a debugkey so I am not overly fussed).

I have to admit that within that function (print_acpi_power()) I don't
see any "other cases which play the same games". Or are you
referring to pmstat_get_cx_stat()? While we shouldn't screw up
stat->last there, I think I agree that we also should account things
to C0 when we really don't know where things belong. V2 getting
ready...

Jan

[PATCH v2] x86/cpuidle: prevent out of bounds array access

[Jan Beulich]

... resulting from fbeef5570c ("x86/cpuidle: get accurate C0 value with
xenpm tool"). For consistency also no longer account an unknown state
to C0 in pmstat_get_cx_stat().

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -279,7 +279,7 @@ static void print_acpi_power(
     uint64_t usage[ACPI_PROCESSOR_MAX_POWER] = { 0 };
     uint64_t res_tick[ACPI_PROCESSOR_MAX_POWER] = { 0 };
     unsigned int i;
-    u8 last_state_idx;
+    signed int last_state_idx;
 
     printk("==cpu%d==\n", cpu);
     last_state_idx = power->last_state ? power->last_state->idx : -1;
@@ -298,8 +298,12 @@ static void print_acpi_power(
     last_state_update_tick = power->last_state_update_tick;
     spin_unlock_irq(&power->stat_lock);
 
-    res_tick[last_state_idx] += ticks_elapsed(last_state_update_tick, current_tick);
-    usage[last_state_idx]++;
+    if ( last_state_idx >= 0 )
+    {
+        res_tick[last_state_idx] += ticks_elapsed(last_state_update_tick,
+                                                  current_tick);
+        usage[last_state_idx]++;
+    }
 
     for ( i = 1; i < power->count; i++ )
     {
@@ -1233,6 +1237,7 @@ int pmstat_get_cx_stat(uint32_t
     else
     {
         struct hw_residencies hw_res;
+        signed int last_state_idx;
 
         stat->nr = power->count;
 
@@ -1245,11 +1250,18 @@ int pmstat_get_cx_stat(uint32_t
             res[i] = power->states[i].time;
         }
         last_state_update_tick = power->last_state_update_tick;
-        stat->last = power->last_state ? power->last_state->idx : 0;
+        last_state_idx = power->last_state ? power->last_state->idx : -1;
         spin_unlock_irq(&power->stat_lock);

-        usage[stat->last]++;
-        res[stat->last] += ticks_elapsed(last_state_update_tick, current_tick);
+        if ( last_state_idx >= 0 )
+        {
+            usage[last_state_idx]++;
+            res[last_state_idx] += ticks_elapsed(last_state_update_tick,
+                                                 current_tick);
+            stat->last = last_state_idx;
+        }
+        else
+            stat->last = 0;
 
         for ( i = 1; i < nr; i++ )
         {

Re: [PATCH v2] x86/cpuidle: prevent out of bounds array access

[Andrew Cooper]

On 22/05/15 14:55, Jan Beulich wrote:
> ... resulting from fbeef5570c ("x86/cpuidle: get accurate C0 value with
> xenpm tool"). For consistency also no longer account an unknown state
> to C0 in pmstat_get_cx_stat().
>
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Looks plausible.  Reviewed-by: Andrew Cooper <andrew.cooper3@citirx.com>

>
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -279,7 +279,7 @@ static void print_acpi_power(
>      uint64_t usage[ACPI_PROCESSOR_MAX_POWER] = { 0 };
>      uint64_t res_tick[ACPI_PROCESSOR_MAX_POWER] = { 0 };
>      unsigned int i;
> -    u8 last_state_idx;
> +    signed int last_state_idx;
>  
>      printk("==cpu%d==\n", cpu);
>      last_state_idx = power->last_state ? power->last_state->idx : -1;
> @@ -298,8 +298,12 @@ static void print_acpi_power(
>      last_state_update_tick = power->last_state_update_tick;
>      spin_unlock_irq(&power->stat_lock);
>  
> -    res_tick[last_state_idx] += ticks_elapsed(last_state_update_tick, current_tick);
> -    usage[last_state_idx]++;
> +    if ( last_state_idx >= 0 )
> +    {
> +        res_tick[last_state_idx] += ticks_elapsed(last_state_update_tick,
> +                                                  current_tick);
> +        usage[last_state_idx]++;
> +    }
>  
>      for ( i = 1; i < power->count; i++ )
>      {
> @@ -1233,6 +1237,7 @@ int pmstat_get_cx_stat(uint32_t
>      else
>      {
>          struct hw_residencies hw_res;
> +        signed int last_state_idx;
>  
>          stat->nr = power->count;
>  
> @@ -1245,11 +1250,18 @@ int pmstat_get_cx_stat(uint32_t
>              res[i] = power->states[i].time;
>          }
>          last_state_update_tick = power->last_state_update_tick;
> -        stat->last = power->last_state ? power->last_state->idx : 0;
> +        last_state_idx = power->last_state ? power->last_state->idx : -1;
>          spin_unlock_irq(&power->stat_lock);
>
> -        usage[stat->last]++;
> -        res[stat->last] += ticks_elapsed(last_state_update_tick, current_tick);
> +        if ( last_state_idx >= 0 )
> +        {
> +            usage[last_state_idx]++;
> +            res[last_state_idx] += ticks_elapsed(last_state_update_tick,
> +                                                 current_tick);
> +            stat->last = last_state_idx;
> +        }
> +        else
> +            stat->last = 0;
>  
>          for ( i = 1; i < nr; i++ )
>          {
>
>
>