[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 1/3] x86: Allow limiting the max C-state sub-state

On 06/26/2014 02:38 PM, Jan Beulich wrote:
On 25.06.14 at 17:52, <ross.lagerwall@xxxxxxxxxx> wrote:
On 06/25/2014 01:37 PM, Jan Beulich wrote:
On 23.06.14 at 13:09, <ross.lagerwall@xxxxxxxxxx> wrote:
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -330,7 +330,9 @@ static void mwait_idle(void)
            (next_state = cpuidle_current_governor->select(power)) > 0) {
                do {
                        cx = &power->states[next_state];
-               } while (cx->type > max_cstate && --next_state);
+               } while ((cx->type > max_cstate || (cx->type == max_cstate &&
+                         MWAIT_HINT2SUBSTATE(cx->address) > max_csubstate)) &&
+                        --next_state);

In the context of the above comment it then is questionable
whether here (and similarly in acpi_processor_idle()) using the
MWAIT parameter value for the comparison here is really
suitable: If you look at hsw_cstates[] and atom_cstates[] you'll
see that there we have states with just a single non-zero sub-
state (which the logic here would exclude in certain cases when
one would expect it to be permitted).

When would one expect them to be permitted that this logic would exclude?

C7s-HSW has a C-state of 4 and a sub-state of 2.  If you set max_cstate
= 4, then no C-state > 4 will be selected.
Similarly, if you select max_csubstate = 2, then no sub C-state > 2 will
be selected (if max_cstate = 4). This seems congruous to me.

Actually I think I got it the wrong way round: Taking your example, if
one sets max_csubstate = 1, one may expect the 1st (not necessarily
the one numbered "1") to still be permitted (much like we also don't tie
max_cstate to actual values to be passed to MWAIT). Just look at how
much more interesting the sub-states get with the ports from recent
Linux that I posted earlier today.

AFAICT, max_cstate _is_ tied to the actual values passed to MWAIT. For example, take Haswell's C9-HSW state: it has an MWAIT flag value of 0x50 which gets assigned as cx->type = 6 and subsequently compared with max_cstate.

max_csubstate is implemented in the same way as max_cstate. However, given the states in the Bay Trail patch (specifically C6N-BYT and C6S-BYT), this is clearly insufficient. How do you think max_csubstate should limit the substate? Should it be something like a max_csubstate = 2 permits the first and second substates?

Ross Lagerwall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.