[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 3/4] mwait-idle: add 'preferred_cstates' module argument
On 27.04.2022 17:06, Roger Pau Monné wrote: > On Wed, Apr 27, 2022 at 03:41:24PM +0200, Jan Beulich wrote: >> On 27.04.2022 14:45, Roger Pau Monné wrote: >>> On Tue, Apr 26, 2022 at 12:05:28PM +0200, Jan Beulich wrote: >>>> --- unstable.orig/xen/arch/x86/cpu/mwait-idle.c >>>> +++ unstable/xen/arch/x86/cpu/mwait-idle.c >>>> @@ -82,6 +82,18 @@ boolean_param("mwait-idle", opt_mwait_id >>>> >>>> static unsigned int mwait_substates; >>>> >>>> +/* >>>> + * Some platforms come with mutually exclusive C-states, so that if one is >>>> + * enabled, the other C-states must not be used. Example: C1 and C1E on >>>> + * Sapphire Rapids platform. This parameter allows for selecting the >>>> + * preferred C-states among the groups of mutually exclusive C-states - >>>> the >>>> + * selected C-states will be registered, the other C-states from the >>>> mutually >>>> + * exclusive group won't be registered. If the platform has no mutually >>>> + * exclusive C-states, this parameter has no effect. >>>> + */ >>>> +static unsigned int __ro_after_init preferred_states_mask; >>>> +integer_param("preferred-cstates", preferred_states_mask); >>>> + >>>> #define LAPIC_TIMER_ALWAYS_RELIABLE 0xFFFFFFFF >>>> /* Reliable LAPIC Timer States, bit 1 for C1 etc. Default to only C1. */ >>>> static unsigned int lapic_timer_reliable_states = (1 << 1); >>>> @@ -96,6 +108,7 @@ struct idle_cpu { >>>> unsigned long auto_demotion_disable_flags; >>>> bool byt_auto_demotion_disable_flag; >>>> bool disable_promotion_to_c1e; >>>> + bool enable_promotion_to_c1e; >>> >>> I'm confused by those fields, shouldn't we just have: >>> promotion_to_c1e = true | false? >>> >>> As one field is the negation of the other: >>> enable_promotion_to_c1e = !disable_promotion_to_c1e >>> >>> I know this is code from Linux, but would like to understand why two >>> fields are needed. >> >> This really is a tristate; Linux is now changing their global variable >> to an enum, but we don't have an equivalent of that global variable. > > So it would be: leave default, disable C1E promotion, enable C1E > promotion. > > And Linux is leaving the {disable,enable}_promotion_to_c1e in > idle_cpu? Iirc they only have disable_promotion_to_c1e there (as a struct field) and keep it, but they convert the similarly named file-scope variable to a tristate. > I guess there's not much we can do unless we want to diverge from > upstream. We've diverged some from Linux here already - as said, for example we don't have their file-scope variable. I could convert our struct field to an enum, but that would be larger code churn for (I think) little gain. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |