linux-kernel - RE: [PATCH AUTOSEL 5.14 10/25] ACPI: PM: s2idle: Run both AMD and Microsoft methods if both are supported

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]
Date:   Tue, 21 Sep 2021 14:57:26 +0000
From:   "Limonciello, Mario" <Mario.Limonciello@....com>
To:     Sasha Levin <sashal@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
CC:     Maxwell Beck <max@....one>,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: RE: [PATCH AUTOSEL 5.14 10/25] ACPI: PM: s2idle: Run both AMD and
 Microsoft methods if both are supported

[Public]

> -----Original Message-----
> From: Sasha Levin <sashal@...nel.org>
> Sent: Monday, September 13, 2021 17:33
> To: linux-kernel@...r.kernel.org; stable@...r.kernel.org
> Cc: Limonciello, Mario <Mario.Limonciello@....com>; Maxwell Beck
> <max@....one>; Rafael J . Wysocki <rafael.j.wysocki@...el.com>; Sasha Levin
> <sashal@...nel.org>; linux-acpi@...r.kernel.org
> Subject: [PATCH AUTOSEL 5.14 10/25] ACPI: PM: s2idle: Run both AMD and
> Microsoft methods if both are supported
> 
> From: Mario Limonciello <mario.limonciello@....com>
> 
> [ Upstream commit fa209644a7124b3f4cf811ced55daef49ae39ac6 ]
> 
> It was reported that on "HP ENVY x360" that power LED does not come
> back, certain keys like brightness controls do not work, and the fan
> never spins up, even under load on 5.14 final.
> 
> In analysis of the SSDT it's clear that the Microsoft UUID doesn't
> provide functional support, but rather the AMD UUID should be
> supporting this system.
> 
> Because this is a gap in the expected logic, we checked back with
> internal team.  The conclusion was that on Windows AMD uPEP *does*
> run even when Microsoft UUID present, but most OEM systems have
> adopted value of "0x3" for supported functions and hence nothing
> runs.
> 
> Henceforth add support for running both Microsoft and AMD methods.
> This approach will also allow the same logic on Intel systems if
> desired at a future time as well by pulling the evaluation of
> `lps0_dsm_func_mask_microsoft` out of the `if` block for
> `acpi_s2idle_vendor_amd`.
> 
> Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.fr
> eedesktop.org%2Fdrm%2Famd%2Fuploads%2F9fbcd7ec3a385cc6949c9bacf45d
> c41b%2Facpi-
> f.20.bin&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7Ce1f8dfc3d
> bfb45fc44ef08d97706917f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C
> 0%7C637671692363481559%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sda
> ta=wP0oz8OMnby9PA4MFrbY1ZAT%2FKv1jctTyXl%2BDteNHqY%3D&amp;reserv
> ed=0
> BugLink:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.fr
> eedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1691&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7
> Ce1f8dfc3dbfb45fc44ef08d97706917f%7C3dd8961fe4884e608e11a82d994e183
> d%7C0%7C0%7C637671692363481559%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C100
> 0&amp;sdata=MVhVz%2BYBTdwgvkkSRRFsL5QdfDLPgTzoMBjD4dsFfMA%3D&a
> mp;reserved=0
> Reported-by: Maxwell Beck <max@....one>
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> [ rjw: Edits of the new comments ]
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
>  drivers/acpi/x86/s2idle.c | 67 +++++++++++++++++++++++----------------
>  1 file changed, 39 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 3a308461246a..bd92b549fd5a 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -449,25 +449,30 @@ int acpi_s2idle_prepare_late(void)
>  	if (pm_debug_messages_on)
>  		lpi_check_constraints();
> 
> -	if (lps0_dsm_func_mask_microsoft > 0) {
> +	/* Screen off */
> +	if (lps0_dsm_func_mask > 0)
> +		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> +					ACPI_LPS0_SCREEN_OFF_AMD :
> +					ACPI_LPS0_SCREEN_OFF,
> +					lps0_dsm_func_mask, lps0_dsm_guid);
> +
> +	if (lps0_dsm_func_mask_microsoft > 0)
>  		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
>  				lps0_dsm_func_mask_microsoft,
> lps0_dsm_guid_microsoft);
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY,
> -				lps0_dsm_func_mask_microsoft,
> lps0_dsm_guid_microsoft);
> +
> +	/* LPS0 entry */
> +	if (lps0_dsm_func_mask > 0)
> +		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> +					ACPI_LPS0_ENTRY_AMD :
> +					ACPI_LPS0_ENTRY,
> +					lps0_dsm_func_mask, lps0_dsm_guid);
> +	if (lps0_dsm_func_mask_microsoft > 0) {
>  		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
>  				lps0_dsm_func_mask_microsoft,
> lps0_dsm_guid_microsoft);
> -	} else if (acpi_s2idle_vendor_amd()) {
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD,
> -				lps0_dsm_func_mask, lps0_dsm_guid);
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
> -				lps0_dsm_func_mask, lps0_dsm_guid);
> -	} else {
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
> -				lps0_dsm_func_mask, lps0_dsm_guid);
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
> -				lps0_dsm_func_mask, lps0_dsm_guid);
> +		/* modern standby entry */
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY,
> +				lps0_dsm_func_mask_microsoft,
> lps0_dsm_guid_microsoft);
>  	}
> -
>  	return 0;
>  }
> 
> @@ -476,24 +481,30 @@ void acpi_s2idle_restore_early(void)
>  	if (!lps0_device_handle || sleep_no_lps0)
>  		return;
> 
> -	if (lps0_dsm_func_mask_microsoft > 0) {
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
> -				lps0_dsm_func_mask_microsoft,
> lps0_dsm_guid_microsoft);
> +	/* Modern standby exit */
> +	if (lps0_dsm_func_mask_microsoft > 0)
>  		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT,
>  				lps0_dsm_func_mask_microsoft,
> lps0_dsm_guid_microsoft);
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
> -				lps0_dsm_func_mask_microsoft,
> lps0_dsm_guid_microsoft);
> -	} else if (acpi_s2idle_vendor_amd()) {
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD,
> -				lps0_dsm_func_mask, lps0_dsm_guid);
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD,
> -				lps0_dsm_func_mask, lps0_dsm_guid);
> -	} else {
> +
> +	/* LPS0 exit */
> +	if (lps0_dsm_func_mask > 0)
> +		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> +					ACPI_LPS0_EXIT_AMD :
> +					ACPI_LPS0_EXIT,
> +					lps0_dsm_func_mask, lps0_dsm_guid);
> +	if (lps0_dsm_func_mask_microsoft > 0)
>  		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
> -				lps0_dsm_func_mask, lps0_dsm_guid);
> +				lps0_dsm_func_mask_microsoft,
> lps0_dsm_guid_microsoft);
> +
> +	/* Screen on */
> +	if (lps0_dsm_func_mask_microsoft > 0)
>  		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
> -				lps0_dsm_func_mask, lps0_dsm_guid);
> -	}
> +				lps0_dsm_func_mask_microsoft,
> lps0_dsm_guid_microsoft);
> +	if (lps0_dsm_func_mask > 0)
> +		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> +					ACPI_LPS0_SCREEN_ON_AMD :
> +					ACPI_LPS0_SCREEN_ON,
> +					lps0_dsm_func_mask, lps0_dsm_guid);
>  }
> 
>  static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {
> --
> 2.30.2

I noticed this didn't get picked up automatically for 5.14.6, so as the submitter of the
original patch here is an explicit:

Acked-by: Mario Limonciello <mario.limonciello@....com>