Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <9c50a737-3e3d-71ea-2722-914cd89c9a07@amd.com>
Date:   Mon, 23 May 2022 10:32:48 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH v3 1/3] x86: Use HLT in default_idle when idle=nomwait
 cmdline arg is passed
Content-Language: en-US
To:     Zhang Rui <rui.zhang@intel.com>, linux-kernel@vger.kernel.org
Cc:     Lewis.Carroll@amd.com, Mario.Limonciello@amd.com,
        gautham.shenoy@amd.com, Ananth.Narayan@amd.com, bharata@amd.com,
        len.brown@intel.com, x86@kernel.org, tglx@linutronix.de,
        mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
        hpa@zytor.com, peterz@infradead.org, chang.seok.bae@intel.com,
        keescook@chromium.org, metze@samba.org, zhengqi.arch@bytedance.com,
        mark.rutland@arm.com, puwen@hygon.cn, rafael.j.wysocki@intel.com,
        andrew.cooper3@citrix.com, jing2.liu@intel.com,
        jmattson@google.com, pawan.kumar.gupta@linux.intel.com
References: <cover.fba143c82098dffab6bbf0a2f3c4be8bae07ccf1.1652176835.git-series.wyes.karny@amd.com>
 <1b24cadcf1d86a91352baa13905218015b9d4fed.1652176835.git-series.wyes.karny@amd.com>
 <a2fba1bf7cf0833372a825f626c12502a44a660e.camel@intel.com>
From:   Wyes Karny <wyes.karny@amd.com>
In-Reply-To: <a2fba1bf7cf0833372a825f626c12502a44a660e.camel@intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bE1sQUZlNDArb082Ny9uck12STVweVkrVzJPZHVxSGRkMndKa0VNY2YwNUpx?=
 =?utf-8?B?S3l4cWRMZDdZeHZjYTFOUG8waEtTT0hYaVpRSnM3dmVnaXJVNjJINmlzREVI?=
 =?utf-8?B?bHpMWit0YVlyYTFEZ3RnQkJsS20rN1Z2WEQ0aVgzdVJlOTZleFZGNEFYZGZ2?=
 =?utf-8?B?VmNCWm1hSXVKTk9tem1CdzAxRVM5N1luQ0F3ZThtOWM0eHFnOTV1YXhzcHFi?=
 =?utf-8?B?WGxhS2IrNnU1WWhkd2JRbjMyYWxsMGk4dCtDRzB1blBxcks4R0FsODZVd3RX?=
 =?utf-8?B?blRqNnFjeE9DTk5odDRvQXBwR29KV0Vpa1BtNGlCVkV1WDFxNnBmUHB1OGZw?=
 =?utf-8?B?SUxKM3ZNblhiMzFuNlFuU05ZL1VKdlZBbnBiaUFVSXRxYkxPTnhza3pwc25z?=
 =?utf-8?B?SEp4VUtwOStLQU1CN0tnUSs3SXhEVXkwOWdTUURrSys1ZVJiaGMzdFJLY0Rt?=
 =?utf-8?B?MGZqY0lWMjFzVS83bmFGa0NyZHZWTXB6M3FEVWxHZGdER2Rpc21EdG91eXJR?=
 =?utf-8?B?WXNCWFpxZG9OTExObzY5MzBicDFpRnVQZXZLdzJkRWRKWEszZjVOM3hSN0xV?=
 =?utf-8?B?NTZ6aTR1ajVybzVudFhZa3gvQzBqZ3h0RmZ0eFpLWUFOaXNPYTZnV3NjSDVM?=
 =?utf-8?B?T01Lc0FoRTFMM1U1dXRPZjZFL29xTmtrKytEMkVBdTZabk9HSlpUam9KTENv?=
 =?utf-8?B?WTMyY0VQR0RpZ3F3dzQ3M01kRjhQZWZVdjdrYmVWdXhWQlJiL09oeXdHVjVY?=
 =?utf-8?B?R0NsYnpLZHl0SDVmNmRDUXNGN015RnVuTFZrZ0dFVUp1eFhRUVQrWExTWElX?=
 =?utf-8?B?ZnVZeDN0TUxKMHVWNjJQeFE4eGV0dlNUNDFHQmRVRldZTnhGSmEwNEpib2gx?=
 =?utf-8?B?Q3dlSFNtTVBXa1UzUjJxNW5wOGhnd0duT0Q3VVZ6dm1LN2lFUUkxQStkTGh5?=
 =?utf-8?B?eE5ZV1BFTHlSaHRpK2kzS00wYlhITkxOYnpmSHM3Zy93VUtCRGdjMENvYlQ1?=
 =?utf-8?B?KzZMUitsSGdQT0dXQURvRnptb3N5MHRiMXJ0YVpYWUpEbmpYeFdDN2NRWFFP?=
 =?utf-8?B?K3FQODMyblB0S2JrZ2poMytnOHQ5dHBRc3A3d2VFeHNBSFJlU0pDWFdIVTkr?=
 =?utf-8?B?MTdadFZxYVZPTXh1WHVmZVBkdTRhaEV4Y2pjOHFkZ3FyR0t0ZEozNHJNbS9R?=
 =?utf-8?B?UFYyM3BFalZLVUY1OU9MRGdhZ3lSWU10SWIvZ2ttUEhEVUxiTy9JZVAyT3RN?=
 =?utf-8?B?UmVFZVNTT2llVCtmOFdrZk9VNnRabTBnQzc0TE9UdjhLbTl0RzB5eklCNjQw?=
 =?utf-8?B?eTVrNG54RVNlR21PaHIvUWRyUjhSTUpsSXEvdG1ueCtZOTBMV1JmSWdvZG8r?=
 =?utf-8?B?RXg1SkI2NkJKNzJiZGo4V2xvdVhHMXNPZ3p5WU1IbFUzM1doZ3FYU1hBWVBq?=
 =?utf-8?B?L2FKeGhvaFNOSFdSazY5NytEV1cvd3pwV3I4K1pNV3FCSnFheldlaTNiYW8v?=
 =?utf-8?B?MVNVbisvY0JMUTQzaGZMVWhkR2twdjkxMXNZTU93WUdnMFdUbFM4a0d6d2FY?=
 =?utf-8?B?VTU3ODc2dG5NQ3FuZkduVG9PcU12YStQR2xQWkJ1QVlTdy95eGdsQi9id1RX?=
 =?utf-8?B?WERLV2lZdjZVaCtIQmVjL3JySUU4VmFyWTAvUjFMbDJsS3F4TUR0U3V1QVBz?=
 =?utf-8?B?M1pBWTRuMVpyWmNHZnFUNnpKRVhqRzV5YU5BSFAyRUFuY0pjazFFWkFxOHZ3?=
 =?utf-8?B?V3RuVndSUWJWTDJHOExmQS9xQ3lzUEkxSUJJSXNCUVBPMEJ1b1k3clFhVE9E?=
 =?utf-8?B?MDFLVEpyMFF5Wk9tTnRXRHBlQVpoZGJ6bnUwdk45WUxMdlJvSHk3Szd0UlRV?=
 =?utf-8?B?UDZSNGUrOWlsRGh0eUlVM21kRGtBY1ZnaXRmMjJZM2hSVTYwVUNQd05NSVds?=
 =?utf-8?B?UlVJVTFlWFZKcnNHcnYwWi9qbjRDM2hFZmdUZ29ISWozMkZpdTQwWlRHa0pF?=
 =?utf-8?B?Vk9OdTFobkZoSWJXczdVN2lDRVd2WngzUXFpakpsck1vaEJENlBpaWJGSzBy?=
 =?utf-8?B?RUVFek9YZVZ4cjdHZGg3WXpNU0QvNnJtb0NtV2lIbTU4cDBoR2dZQTF5ZmFa?=
 =?utf-8?B?UjNUdm9ieEo2OWdxMThuL3d2WHBlUXl5dmZGc1M3Y1doZGNXMlpWVlFOQVFv?=
 =?utf-8?B?NzJHdnJhYlUwY0Yya0dONWVnZjNPTDc4WWRuY1ZaT0xVakx5Tk51amZIaXNr?=
 =?utf-8?B?RHZGME5XM3M1dmg3OE02dXlFZWl3QWVtY1hrK2RramgxNk5UT2hRdXU5V3NL?=
 =?utf-8?B?RWMwWW1BQ3UvRHpQN2NzTE5HTUYzTVdrUFFlcHhwKy9yYTFtSGp5Zz09?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 971e6d98-a403-43a3-3713-08da3c79869b
X-MS-Exchange-CrossTenant-AuthSource: BY5PR12MB3876.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2022 05:03:07.8218
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 6zWZADcrrBp1U0QI3PXCZSpoAaS/5UNcHkvX2S9itFrmHR6dmb/uMsNFoTD5gI+S26h3poAxjFP1iKTyWSF6UQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB5879
X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hello Rui,

On 5/20/2022 9:08 PM, Zhang Rui wrote:
> Hi, Wyes,
> 
> On Tue, 2022-05-10 at 15:48 +0530, Wyes Karny wrote:
>> When kernel is booted with idle=nomwait do not use MWAIT as the
>> default idle state.
>>
>> If the user boots the kernel with idle=nomwait, it is a clear
>> direction to not use mwait as the default idle state.
>> However, the current code does not take this into consideration
>> while selecting the default idle state on x86.
>>
>> This patch fixes it by checking for the idle=nomwait boot option in
>> prefer_mwait_c1_over_halt().
>>
>> Also update the documentation around idle=nomwait appropriately.
> 
> I think we also need to update the comment in idle_setup() as well. 

Agreed. Will update. Thanks!

> 
>         } else if (!strcmp(str, "nomwait")) {
>                 /*
>         
>          * If the boot option of "idle=nomwait" is added,
>               
>    * it means that mwait will be disabled for CPU C2/C3
>                 
>  * states. In such case it won't touch the variable
>                  *
> of boot_option_idle_override.
>                  */
>                 boot_op
> tion_idle_override = IDLE_NOMWAIT;
>         } else
> 
> thanks,
> rui
>>
>> Signed-off-by: Wyes Karny <wyes.karny@amd.com>
>> ---
>> Changes in v3:
>> - Update documentation around idle=mwait
>>
>>  Documentation/admin-guide/pm/cpuidle.rst | 15 +++++++++------
>>  arch/x86/kernel/process.c                |  4 ++++
>>  2 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/pm/cpuidle.rst
>> b/Documentation/admin-guide/pm/cpuidle.rst
>> index aec2cd2aaea7..19754beb5a4e 100644
>> --- a/Documentation/admin-guide/pm/cpuidle.rst
>> +++ b/Documentation/admin-guide/pm/cpuidle.rst
>> @@ -612,8 +612,8 @@ the ``menu`` governor to be used on the systems
>> that use the ``ladder`` governor
>>  by default this way, for example.
>>  
>>  The other kernel command line parameters controlling CPU idle time
>> management
>> -described below are only relevant for the *x86* architecture and
>> some of
>> -them affect Intel processors only.
>> +described below are only relevant for the *x86* architecture and
>> references
>> +to ``intel_idle`` affect Intel processors only.
>>  
>>  The *x86* architecture support code recognizes three kernel command
>> line
>>  options related to CPU idle time management: ``idle=poll``,
>> ``idle=halt``,
>> @@ -635,10 +635,13 @@ idle, so it very well may hurt single-thread
>> computations performance as well as
>>  energy-efficiency.  Thus using it for performance reasons may not be
>> a good idea
>>  at all.]
>>  
>> -The ``idle=nomwait`` option disables the ``intel_idle`` driver and
>> causes
>> -``acpi_idle`` to be used (as long as all of the information needed
>> by it is
>> -there in the system's ACPI tables), but it is not allowed to use the
>> -``MWAIT`` instruction of the CPUs to ask the hardware to enter idle
>> states.
>> +The ``idle=nomwait`` option prevents the use of ``MWAIT``
>> instruction of
>> +the CPU to enter idle states. When this option is used, the
>> ``acpi_idle``
>> +driver will use the ``HLT`` instruction instead of ``MWAIT``. On
>> systems
>> +running Intel processors, this option disables the ``intel_idle``
>> driver
>> +and forces the use of the ``acpi_idle`` driver instead. Note that in
>> either
>> +case, ``acpi_idle`` driver will function only if all the information
>> needed
>> +by it is in the system's ACPI tables.
>>  
>>  In addition to the architecture-level kernel command line options
>> affecting CPU
>>  idle time management, there are parameters affecting individual
>> ``CPUIdle``
>> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
>> index b370767f5b19..49b915d1b7b4 100644
>> --- a/arch/x86/kernel/process.c
>> +++ b/arch/x86/kernel/process.c
>> @@ -824,6 +824,10 @@ static void amd_e400_idle(void)
>>   */
>>  static int prefer_mwait_c1_over_halt(const struct cpuinfo_x86 *c)
>>  {
>> +	/* User has disallowed the use of MWAIT. Fallback to HALT */
>> +	if (boot_option_idle_override == IDLE_NOMWAIT)
>> +		return 0;
>> +
>>  	if (c->x86_vendor != X86_VENDOR_INTEL)
>>  		return 0;
>>  
> 

Thanks,
Wyes