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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.06.14 at 13:09, <ross.lagerwall@xxxxxxxxxx> wrote:
> Allow limiting the max C-state sub-state by appending to the max_cstate
> command-line parameter. E.g. max_cstate=1,0
> The limit only applies to the highest legal C-state. For example:
>  max_cstate = 1, max_csubstate = 0 ==> C0, C1 okay, but not C1E
>  max_cstate = 1, max_csubstate = 1 ==> C0, C1 and C1E okay, but not C2
>  max_cstate = 2, max_csubstate = 0 ==> C0, C1, C1E, C2 okay, but not C3
>  max_cstate = 2, max_csubstate = 1 ==> C0, C1, C1E, C2 okay, but not C3

While from an abstract perspective this looks okay to me now, I'm
afraid the description, which is also being put into the header file, is
possibly misleading: Neither is the first sub-state of C1 necessarily
C1E, nor is it excluded that C2 and higher also have sub-states (yet
the last of the examples sort of suggests that).

> --- 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).

Jan


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