linux-kernel - Re: [PATCH] intel_idle: introduce 'use_acpi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b50ade64-21ea-4420-a7ec-5e17ea7f6af7@redhat.com>
Date: Tue, 4 Feb 2025 08:04:27 -0500
From: Prarit Bhargava <prarit@...hat.com>
To: David Arcari <darcari@...hat.com>, dedekind1@...il.com,
 linux-pm@...r.kernel.org
Cc: Jonathan Corbet <corbet@....net>,
 Jacob Pan <jacob.jun.pan@...ux.intel.com>, Len Brown <lenb@...nel.org>,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter

On 2/4/25 7:52 AM, David Arcari wrote:
> 
> Hi Artem,
> 
> On 2/4/25 7:23 AM, Artem Bityutskiy wrote:
>> Hi David,
>>
>> On Tue, 2025-01-28 at 09:11 -0500, David Arcari wrote:
>>
>>> +The ``use_acpi_cst`` module parameter (recognized by ``intel_idle`` 
>>> if the
>>> +kernel has been configured with ACPI support) can be set to make the 
>>> driver
>>> +ignore the per cpu idle states in lieu of ACPI idle states. 
>>> ``use_acpi_cst``
>>> +has no effect if ``no_acpi`` is set).
>>
>> With this change, there will be three parameters:
>>
>> * no_acpi
>> * use_acpi
>> * use_acpi_cst
>>
>> I would like to make the naming as intuitive as possible. We do not 
>> rename the
>> first 2, but for the 3rd one, I think "force_acpi" would be a better 
>> name. Or
>> perhaps "no_native"?
> 
> The problem with force_acpi is it is very similar to force_use_acpi 
> which is what intel_idle.c uses internally:
> 

Given that @Artem is suggesting we change the names of things -- perhaps 
we should fix "force_use_acpi" as well?

P.

> drivers/idle/intel_idle.c:module_param_named(use_acpi, force_use_acpi, 
> bool, 0444);
> 
> That said, I am not attached to the 'use_acpi_cst' parameter name.
> 
>>
>> * no_acpi - Do not use ACPI at all. Only native mode is available, no 
>> ACPI mode.
>> * use_acpi - No-op in ACPI mode, consult ACPI tables for C-states on/off
>>    status in native mode.
>> * force_acpi (or no_native?) - Work only in ACPI mode, no native mode 
>> available
>>    (ignore all custom tables).
>>
>> Additionally, I think we should enhance the documentation for 
>> 'no_acpi' and
>> 'use_acpi' while we're at it. Otherwise, it is hard to distinguish 
>> between these
>> three options. Would you consider another patch that improves the 
>> documentation
>> for 'no_acpi' and 'use_acpi', and then adds the third parameter?
> 
> I'm happy to resubmit. I guess I could use 'no_native' for the new 
> parameter and then update the documentation as you suggest above.
> 
> Does that work?
> 
>>
>> Thanks, Artem!
>>
> 
> Best,
> -DA
>