linux-kernel - Re: [PATCH v1] ACPI: PM: s2idle: Add module parameter for LPS0 constraints checking

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] [day] [month] [year] [list]

Message-ID: <8c458422-9fa7-44fd-90a7-b66bb905bdc6@kernel.org>
Date: Tue, 13 Jan 2026 16:06:41 -0600
From: "Mario Limonciello (AMD) (kernel.org)" <superm1@...nel.org>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Linux ACPI <linux-acpi@...r.kernel.org>,
 LKML <linux-kernel@...r.kernel.org>, Linux PM <linux-pm@...r.kernel.org>,
 Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Subject: Re: [PATCH v1] ACPI: PM: s2idle: Add module parameter for LPS0
 constraints checking



On 1/13/2026 4:05 PM, Rafael J. Wysocki wrote:
> On Tue, Jan 13, 2026 at 11:00 PM Mario Limonciello
> <mario.limonciello@....com> wrote:
>> On 1/13/2026 3:57 PM, Rafael J. Wysocki wrote:
>>> On Tue, Jan 13, 2026 at 10:48 PM Mario Limonciello
>>> <mario.limonciello@....com> wrote:
>>>> On 1/13/2026 7:36 AM, Rafael J. Wysocki wrote:
>>>>> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>>>>>
>>>>> Commit 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when
>>>>> needed") attempted to avoid useless evaluation of LPS0 _DSM Function 1
>>>>> in lps0_device_attach() because pm_debug_messages_on might never be set
>>>>> (and that is the case on production systems most of the time), but it
>>>>> turns out that LPS0 _DSM Function 1 is generally problematic on some
>>>>> platforms and causes suspend issues to occur when pm_debug_messages_on
>>>>> is set now.
>>>>
>>>> Any ideas why it's causing problems?  AML doing something it shouldn't?
>>>
>>> It's not a clear AML bug AFAICS.  Rather, it seems to have
>>> dependencies on something that is not ready when it is evaluated, so
>>> an ordering issue or similar.
>>
>> Ah I see.
>>
>>>
>>>>>
>>>>> In Linux, LPS0 _DSM Function 1 is only useful for diagnostics and only
>>>>> in the cases when the system does not reach the deepest platform idle
>>>>> state during suspend-to-idle for some reason.  If such diagnostics is
>>>>> not necessary, evaluating it is a loss of time, so using it along with
>>>>> the other pm_debug_messages_on diagnostics is questionable because the
>>>>> latter is expected to be suitable for collecting debug information even
>>>>> during production use of system suspend.
>>>>>
>>>>> For this reason, add a module parameter called check_lps0_constraints
>>>>> to control whether or not the list of LPS0 constraints will be checked
>>>>> in acpi_s2idle_prepare_late_lps0() and so whether or not to evaluate
>>>>> LPS0 _DSM Function 1 (once) in acpi_s2idle_begin_lps0().
>>>>>
>>>>> Fixes: 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when needed")
>>>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>>>>> ---
>>>>>     drivers/acpi/x86/s2idle.c |    8 ++++++--
>>>>>     1 file changed, 6 insertions(+), 2 deletions(-)
>>>>>
>>>>> --- a/drivers/acpi/x86/s2idle.c
>>>>> +++ b/drivers/acpi/x86/s2idle.c
>>>>> @@ -28,6 +28,10 @@ static bool sleep_no_lps0 __read_mostly;
>>>>>     module_param(sleep_no_lps0, bool, 0644);
>>>>>     MODULE_PARM_DESC(sleep_no_lps0, "Do not use the special LPS0 device interface");
>>>>>
>>>>> +static bool check_lps0_constraints __read_mostly;
>>>>> +module_param(check_lps0_constraints, bool, 0644);
>>>>> +MODULE_PARM_DESC(check_lps0_constraints, "Check LPS0 device constraints");
>>>>
>>>> I'm personally not really a fan of another module parameter for
>>>> debugging.  I know some other subsystem maintainers are very anti-module
>>>> parameters too.
>>>>
>>>> I did like having /sys/power/pm_debug_messages able to be a one stop
>>>> shop for debugging messages at runtime.
>>>
>>> Well, this is not just debug messages, rather a whole debug facility
>>> enabled via pm_debug_messages, which is kind of confusing.
>>
>> Good point.
>>
>>>
>>>> So I had another idea I wanted to throw around.  What if instead we had
>>>> a file /sys/kernel/debug/x86/lps0_constraints?
>>>
>>> Then you cannot use this without debugfs.
>>
>> I mean; yeah.  But it really is debugging and most distros have debugfs
>> enabled by default these days don't they?
> 
> Some of them don't.
> 
> That's one of the reasons why tracing has its own pseudo-filesystem
> now, so it is a real obstacle.
> 
>>>
>>>> If the file is never accessed never evaluate constraints.  If you read
>>>> it once then you can get a dump of all the current constraints and any
>>>> future suspends during that boot will also include constraints in the
>>>> logs (IE call lpi_check_constraints()).
>>>
>>> So if it is not in debugfs, it would need to be in sysfs and then I
>>> don't see much difference between it and a module param, honestly.
>>>
>>> I actually prefer the latter because it uses an existing infra.
>>
>> I agree if debugfs is decided to be out sysfs and module parameter are
>> equal footing and then this patch makes sense.
> 
> I think that debugfs cannot be relied on to be always present.

OK, I think the patch is fine as is then.

Reviewed-by: Mario Limonciello (AMD) <superm1@...nel.org>