| Message ID | 5CE68FA70200007800231B3E@prv1-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: CPU idle management adjustments | expand |
On 23/05/2019 13:18, 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> Is this only useful in practice for limiting C1E ? > > --- a/docs/misc/xen-command-line.pandoc > +++ b/docs/misc/xen-command-line.pandoc > @@ -1369,9 +1369,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, &s, 0); You can pass NULL for endp, seeing as it isn't used. ~Andrew
>>> On 10.06.19 at 18:43, <andrew.cooper3@citrix.com> wrote: > On 23/05/2019 13:18, 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> > > Is this only useful in practice for limiting C1E ? This may have been the original primary goal, but if you look at the tables in mwait-idle.c you'll find further sub-states. >> --- 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, &s, 0); > > You can pass NULL for endp, seeing as it isn't used. Ah yes. Jan
--- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -1369,9 +1369,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, &s, 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,11 @@ 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); + } for_each_present_cpu ( cpu ) { struct acpi_processor_power *power = processor_powers[cpu]; @@ -590,7 +604,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) {