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

Re: [PATCH 3/4] mwait-idle: add 'preferred_cstates' module argument


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 27 Apr 2022 17:25:35 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=mcw5RbpZhysFabc4fPyBliDEyDRG8Lfq8DNGKuZXeNU=; b=W6nqK0E60e0RNXm8Fkk5cja9cq1+N1W3GEXVHFesfMXoUsP6+1cg8AESz6z0qKAeUZMr0rQxBnm67mZ7I5JzUoFx8Rlmhk4Y098YY1jtA7TBPl5PwRa+stdR1TwVACwpe4o4DgSGs98eZfuSM0WFOUCJKER2kkfQXDtzbVMoklyzaMSqmNdDnTbNduYQvuIV8yHzO0HFAk6qCUydgYdFBJQ1BqsBuETyqpqobOvPA7UE7+6IWCTPsqSIhQLc/g5BW8KxBdTwWEH0+6oDa/2eB1blApa13ZmsUVbclOjvtDtWaOIa+nTxLnctYcxx1Y+Fp9kTNbx9JukUi55phuC9iw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=WazJxgv3RiFhRNMkTNVFnU+Z8rwE/r06XYyt+GCF4brwzmnz2q56hCdMUJeGVGohZc4ZZUUStJpLEs1uBykSB25/QU6rYgUM7oxNMmRmCcd2MXZKSpoUyVWGLWl5Eh5GiEJIKTAOIsgq8jdVW7oTpX8yq4P3mgVFwT/p7oqbHRMdy7Jloon4z6ttx8s4CaN00SpesTYNhisQLdSWzyAJJbeW7jiJTMoHuih8w8ZouMLiXziXUVPYoxQVHiD537a+gJfWcSGDv2sG1N+qKkA1Y1vKOSQTmP242kAPZdQFstUyN6vP4GF4u/0NVuHiZfJU73vWrm6qwF2vyCx3aUbM4g==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Wed, 27 Apr 2022 15:25:48 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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




 


Rackspace

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