| Message ID | b5b7560a-2418-9df7-2062-3ba1f6f0152f@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/5] x86/cpuidle: switch to uniform meaning of "max_cstate=" | expand |
On Wed, Jul 03, 2019 at 01:03:02PM +0000, Jan Beulich wrote: > From: Ross Lagerwall <ross.lagerwall@citrix.com> > > 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 > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > v2: Explicitly log "unlimited". Pass NULL in the 2nd simple_strtoul() > invocation. > > --- a/docs/misc/xen-command-line.pandoc > +++ b/docs/misc/xen-command-line.pandoc > @@ -1374,9 +1374,11 @@ Specify the maximum number of CPUs that > This option is ignored in **pv-shim** mode. > > ### max_cstate (x86) > -> `= <integer>` > +> `= <integer>[,<integer>]` > > -Specify the deepest C-state CPUs are permitted to be placed in. > +Specify the deepest C-state CPUs are permitted to be placed in, and > +optionally the maximum sub C-state to be used used. The latter only applies > +to the highest permitted C-state. > > ### max_gsi_irqs (x86) > > `= <integer>` > --- a/xen/arch/x86/acpi/cpu_idle.c > +++ b/xen/arch/x86/acpi/cpu_idle.c > @@ -104,7 +104,17 @@ bool lapic_timer_init(void) > > void (*__read_mostly pm_idle_save)(void); > unsigned int max_cstate __read_mostly = UINT_MAX; > -integer_param("max_cstate", max_cstate); > +unsigned int max_csubstate __read_mostly = UINT_MAX; > + > +static int __init parse_cstate(const char *s) > +{ > + max_cstate = simple_strtoul(s, &s, 0); > + if ( *s == ',' ) > + max_csubstate = simple_strtoul(s + 1, NULL, 0); > + return 0; > +} > +custom_param("max_cstate", parse_cstate); > + > static bool __read_mostly local_apic_timer_c2_ok; > boolean_param("lapic_timer_c2_ok", local_apic_timer_c2_ok); > > @@ -347,7 +357,13 @@ static void dump_cx(unsigned char key) > > printk("'%c' pressed -> printing ACPI Cx structures\n", key); > if ( max_cstate < UINT_MAX ) > + { > printk("max state: C%u\n", max_cstate); > + if ( max_csubstate < UINT_MAX ) > + printk("max sub-state: %u\n", max_csubstate); > + else > + printk("max sub-state: unlimited\n"); > + } > else > printk("max state: unlimited\n"); > for_each_present_cpu ( cpu ) > @@ -592,7 +608,13 @@ static void acpi_processor_idle(void) > > do { > cx = &power->states[next_state]; > - } while ( cx->type > max_state && --next_state ); > + } while ( (cx->type > max_state || > + cx->entry_method == ACPI_CSTATE_EM_NONE || > + (cx->entry_method == ACPI_CSTATE_EM_FFH && > + cx->type == max_cstate && > + (cx->address & MWAIT_SUBSTATE_MASK) > max_csubstate)) && > + --next_state ); > + cx = &power->states[next_state]; Is the line above a stray addition? It is at least not properly aligned AFAICT. Thanks, Roger.
On 16.07.2019 16:48, Roger Pau Monné wrote: > On Wed, Jul 03, 2019 at 01:03:02PM +0000, Jan Beulich wrote: >> @@ -592,7 +608,13 @@ static void acpi_processor_idle(void) >> >> do { >> cx = &power->states[next_state]; >> - } while ( cx->type > max_state && --next_state ); >> + } while ( (cx->type > max_state || >> + cx->entry_method == ACPI_CSTATE_EM_NONE || >> + (cx->entry_method == ACPI_CSTATE_EM_FFH && >> + cx->type == max_cstate && >> + (cx->address & MWAIT_SUBSTATE_MASK) > max_csubstate)) && >> + --next_state ); >> + cx = &power->states[next_state]; > > Is the line above a stray addition? It is at least not properly > aligned AFAICT. Oh, yes, that's a re-basing mistake. Thanks for spotting. Jan
On 03/07/2019 14:03, Jan Beulich wrote: > From: Ross Lagerwall <ross.lagerwall@citrix.com> > > 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 > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, subject to the correction Roger noticed.
--- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -1374,9 +1374,11 @@ Specify the maximum number of CPUs that This option is ignored in **pv-shim** mode. ### max_cstate (x86) -> `= <integer>` +> `= <integer>[,<integer>]` -Specify the deepest C-state CPUs are permitted to be placed in. +Specify the deepest C-state CPUs are permitted to be placed in, and +optionally the maximum sub C-state to be used used. The latter only applies +to the highest permitted C-state. ### max_gsi_irqs (x86) > `= <integer>` --- a/xen/arch/x86/acpi/cpu_idle.c +++ b/xen/arch/x86/acpi/cpu_idle.c @@ -104,7 +104,17 @@ bool lapic_timer_init(void) void (*__read_mostly pm_idle_save)(void); unsigned int max_cstate __read_mostly = UINT_MAX; -integer_param("max_cstate", max_cstate); +unsigned int max_csubstate __read_mostly = UINT_MAX; + +static int __init parse_cstate(const char *s) +{ + max_cstate = simple_strtoul(s, &s, 0); + if ( *s == ',' ) + max_csubstate = simple_strtoul(s + 1, NULL, 0); + return 0; +} +custom_param("max_cstate", parse_cstate); + static bool __read_mostly local_apic_timer_c2_ok; boolean_param("lapic_timer_c2_ok", local_apic_timer_c2_ok); @@ -347,7 +357,13 @@ static void dump_cx(unsigned char key) printk("'%c' pressed -> printing ACPI Cx structures\n", key); if ( max_cstate < UINT_MAX ) + { printk("max state: C%u\n", max_cstate); + if ( max_csubstate < UINT_MAX ) + printk("max sub-state: %u\n", max_csubstate); + else + printk("max sub-state: unlimited\n"); + } else printk("max state: unlimited\n"); for_each_present_cpu ( cpu ) @@ -592,7 +608,13 @@ static void acpi_processor_idle(void) do { cx = &power->states[next_state]; - } while ( cx->type > max_state && --next_state ); + } while ( (cx->type > max_state || + cx->entry_method == ACPI_CSTATE_EM_NONE || + (cx->entry_method == ACPI_CSTATE_EM_FFH && + cx->type == max_cstate && + (cx->address & MWAIT_SUBSTATE_MASK) > max_csubstate)) && + --next_state ); + cx = &power->states[next_state]; if ( next_state ) { if ( cx->type == ACPI_STATE_C3 && power->flags.bm_check && --- a/xen/arch/x86/cpu/mwait-idle.c +++ b/xen/arch/x86/cpu/mwait-idle.c @@ -731,7 +731,9 @@ static void mwait_idle(void) do { cx = &power->states[next_state]; - } while (cx->type > max_state && --next_state); + } while ((cx->type > max_state || (cx->type == max_cstate && + MWAIT_HINT2SUBSTATE(cx->address) > max_csubstate)) && + --next_state); if (!next_state) cx = NULL; else if (tb_init_done) --- a/xen/include/xen/acpi.h +++ b/xen/include/xen/acpi.h @@ -141,13 +141,21 @@ void acpi_unregister_gsi (u32 gsi); #ifdef CONFIG_ACPI_CSTATE /* - * Set highest legal C-state - * 0: C0 okay, but not C1 - * 1: C1 okay, but not C2 - * 2: C2 okay, but not C3 etc. + * max_cstate sets the highest legal C-state. + * max_cstate = 0: C0 okay, but not C1 + * max_cstate = 1: C1 okay, but not C2 + * max_cstate = 2: C2 okay, but not C3 etc. + + * max_csubstate sets the highest legal C-state sub-state. Only applies to the + * highest legal C-state. + * 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 */ extern unsigned int max_cstate; +extern unsigned int max_csubstate; static inline unsigned int acpi_get_cstate_limit(void) {