| 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
| ||
|
Message-ID:
<BY5PR19MB392256C65661E76FC292C0889AE52@BY5PR19MB3922.namprd19.prod.outlook.com>
Date: Wed, 8 May 2024 14:24:46 +0000
From: "Shen, Yijun" <Yijun.Shen@...l.com>
To: Lyndon Sanche <lsanche@...deno.ca>
CC: "mario.limonciello@....com" <mario.limonciello@....com>,
"pali@...nel.org"
<pali@...nel.org>,
"W_Armin@....de" <W_Armin@....de>,
"srinivas.pandruvada@...ux.intel.com" <srinivas.pandruvada@...ux.intel.com>,
"ilpo.jarvinen@...ux.intel.com" <ilpo.jarvinen@...ux.intel.com>,
"lkp@...el.com" <lkp@...el.com>, Hans de Goede <hdegoede@...hat.com>,
Matthew
Garrett <mjg59@...f.ucam.org>,
Jonathan Corbet <corbet@....net>,
Heiner
Kallweit <hkallweit1@...il.com>,
Vegard Nossum <vegard.nossum@...cle.com>,
"platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Dell Client
Kernel <Dell.Client.Kernel@...l.com>
Subject: RE: [PATCH v5] platform/x86: dell-laptop: Implement platform_profile
Hi Lyndon,
Thanks for working on this patch.
Internal Use - Confidential
> -----Original Message-----
> From: Lyndon Sanche <lsanche@...deno.ca>
> Sent: Thursday, May 2, 2024 5:58 AM
> To: lsanche@...deno.ca
> Cc: mario.limonciello@....com; pali@...nel.org; W_Armin@....de;
> srinivas.pandruvada@...ux.intel.com; ilpo.jarvinen@...ux.intel.com;
> lkp@...el.com; Hans de Goede <hdegoede@...hat.com>; Matthew Garrett
> <mjg59@...f.ucam.org>; Jonathan Corbet <corbet@....net>; Heiner Kallweit
> <hkallweit1@...il.com>; Vegard Nossum <vegard.nossum@...cle.com>;
> platform-driver-x86@...r.kernel.org; linux-kernel@...r.kernel.org; Dell Client
> Kernel <Dell.Client.Kernel@...l.com>
> Subject: [PATCH v5] platform/x86: dell-laptop: Implement platform_profile
>
>
> [EXTERNAL EMAIL]
>
> Some Dell laptops support configuration of preset fan modes through smbios
> tables.
>
> If the platform supports these fan modes, set up platform_profile to change
> these modes. If not supported, skip enabling platform_profile.
>
> Signed-off-by: Lyndon Sanche <lsanche@...deno.ca>
> ---
> v5:
> - Fix indent in smbios-thermal-ctl comment
> - Remove linux/wmi.h include
> - Add 'select ACPI_PLATFORM_PROFILE' to Dell KConfig
> v4:
> - Make thermal_init and thermal_cleanup static
> - Rearrange order of added includes, did not edit current includes
> - Include bits.h
> - Switch comment style
> - Return error if platform_profile registering failed
> - Add thermal calls to call_blacklist
> - Align defines with tabs
> - Correct separation of function and error handling
> - Propagate error codes up
> v3:
> - Convert smbios-thermal-ctl docs to multiline comment and wrap
> - Change thermal_mode_bits enum to directly be BIT() values
> - Convert related code to use this
> - Use FIELD_GET/PREP and GENNMASK for getting/setting thermal modes
> - Correct offset for getting current ACC mode, setting offset
> unchanged
> - Check if thermal_handler is allocated before freeing and
> unregistering platform_profile
> v2:
> - Wrap smbios-thermal-ctl comment
> - Return proper error code when invalid state returned
> - Simplify platform_profile_get returns
> - Propogate ENOMEM error
> ---
Dell side has an initial testing with this patch on some laptops, it looks good. While changing the platform profile:
1. The corresponding USTT option in BIOS will be changed.
2. thermald will not be impacted. The related PSVT and ITMT will be loaded.
Some Dell DTs does not have the USTT, Dell'll have a check if nothing is broken.
Additional, with this patch, follow behavior is found:
1. For example, the platform profile is quiet.
2. Reboot the system and change the USTT to performance.
3. Boot to desktop, the platform profile is "quiet", the USTT will be changed back to "quiet".
This looks like not a proper user experience. The platform profile should honor the BIOS setting, aka, the platform profile should be switched to "performance".
> drivers/platform/x86/dell/Kconfig | 1 +
> drivers/platform/x86/dell/dell-laptop.c | 238 +++++++++++++++++++
> drivers/platform/x86/dell/dell-smbios-base.c | 1 +
> drivers/platform/x86/dell/dell-smbios.h | 1 +
> 4 files changed, 241 insertions(+)
>
> diff --git a/drivers/platform/x86/dell/Kconfig
> b/drivers/platform/x86/dell/Kconfig
> index bd9f445974cc..5195ad59b44d 100644
> --- a/drivers/platform/x86/dell/Kconfig
> +++ b/drivers/platform/x86/dell/Kconfig
> @@ -57,6 +57,7 @@ config DELL_LAPTOP
> select POWER_SUPPLY
> select LEDS_CLASS
> select NEW_LEDS
> + select ACPI_PLATFORM_PROFILE
> help
> This driver adds support for rfkill and backlight control to Dell
> laptops (except for some models covered by the Compal driver).
> diff --git a/drivers/platform/x86/dell/dell-laptop.c
> b/drivers/platform/x86/dell/dell-laptop.c
> index 42f7de2b4522..dc530a4f5047 100644
> --- a/drivers/platform/x86/dell/dell-laptop.c
> +++ b/drivers/platform/x86/dell/dell-laptop.c
> @@ -27,6 +27,9 @@
> #include <linux/i8042.h>
> #include <linux/debugfs.h>
> #include <linux/seq_file.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/platform_profile.h>
> #include <acpi/video.h>
> #include "dell-rbtn.h"
> #include "dell-smbios.h"
> @@ -95,6 +98,7 @@ static struct backlight_device *dell_backlight_device;
> static struct rfkill *wifi_rfkill; static struct rfkill *bluetooth_rfkill; static struct
> rfkill *wwan_rfkill;
> +static struct platform_profile_handler *thermal_handler;
> static bool force_rfkill;
> static bool micmute_led_registered;
> static bool mute_led_registered;
> @@ -2199,6 +2203,232 @@ static int mute_led_set(struct led_classdev
> *led_cdev,
> return 0;
> }
>
> +/* Derived from smbios-thermal-ctl
> + *
> + * cbClass 17
> + * cbSelect 19
> + * User Selectable Thermal Tables(USTT)
> + * cbArg1 determines the function to be performed
> + * cbArg1 0x0 = Get Thermal Information
> + * cbRES1 Standard return codes (0, -1, -2)
> + * cbRES2, byte 0 Bitmap of supported thermal modes. A mode is
> supported if
> + * its bit is set to 1
> + * Bit 0 Balanced
> + * Bit 1 Cool Bottom
> + * Bit 2 Quiet
> + * Bit 3 Performance
> + * cbRES2, byte 1 Bitmap of supported Active Acoustic Controller (AAC)
> modes.
> + * Each mode corresponds to the supported thermal modes in
> + * byte 0. A mode is supported if its bit is set to 1.
> + * Bit 0 AAC (Balanced)
> + * Bit 1 AAC (Cool Bottom
> + * Bit 2 AAC (Quiet)
> + * Bit 3 AAC (Performance)
> + * cbRes3, byte 0 Current Thermal Mode
> + * Bit 0 Balanced
> + * Bit 1 Cool Bottom
> + * Bit 2 Quiet
> + * Bit 3 Performanc
> + * cbRes3, byte 1 AAC Configuration type
> + * 0 Global (AAC enable/disable applies to all supported USTT
> modes)
> + * 1 USTT mode specific
> + * cbRes3, byte 2 Current Active Acoustic Controller (AAC) Mode
> + * If AAC Configuration Type is Global,
> + * 0 AAC mode disabled
> + * 1 AAC mode enabled
> + * If AAC Configuration Type is USTT mode specific (multiple bits may be
> set),
> + * Bit 0 AAC (Balanced)
> + * Bit 1 AAC (Cool Bottom
> + * Bit 2 AAC (Quiet)
> + * Bit 3 AAC (Performance)
> + * cbRes3, byte 3 Current Fan Failure Mode
> + * Bit 0 Minimal Fan Failure (at least one fan has failed, one fan working)
> + * Bit 1 Catastrophic Fan Failure (all fans have failed)
> + *
> + * cbArg1 0x1 (Set Thermal Information), both desired thermal mode and
> + * desired AAC mode shall be applied
> + * cbArg2, byte 0 Desired Thermal Mode to set
> + * (only one bit may be set for this parameter)
> + * Bit 0 Balanced
> + * Bit 1 Cool Bottom
> + * Bit 2 Quiet
> + * Bit 3 Performance
> + * cbArg2, byte 1 Desired Active Acoustic Controller (AAC) Mode to set
> + * If AAC Configuration Type is Global,
> + * 0 AAC mode disabled
> + * 1 AAC mode enabled
> + * If AAC Configuration Type is USTT mode specific
> + * (multiple bits may be set for this parameter),
> + * Bit 0 AAC (Balanced)
> + * Bit 1 AAC (Cool Bottom
> + * Bit 2 AAC (Quiet)
> + * Bit 3 AAC (Performance)
> + */
> +
> +#define DELL_ACC_GET_FIELD GENMASK(19, 16)
> +#define DELL_ACC_SET_FIELD GENMASK(11, 8)
> +#define DELL_THERMAL_SUPPORTED GENMASK(3, 0)
> +
> +enum thermal_mode_bits {
> + DELL_BALANCED = BIT(0),
> + DELL_COOL_BOTTOM = BIT(1),
> + DELL_QUIET = BIT(2),
> + DELL_PERFORMANCE = BIT(3),
> +};
> +
> +static int thermal_get_mode(void)
> +{
> + struct calling_interface_buffer buffer;
> + int state;
> + int ret;
> +
> + dell_fill_request(&buffer, 0x0, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO,
> SELECT_THERMAL_MANAGEMENT);
> + if (ret)
> + return ret;
> + state = buffer.output[2];
> + if (state & DELL_BALANCED)
> + return DELL_BALANCED;
> + else if (state & DELL_COOL_BOTTOM)
> + return DELL_COOL_BOTTOM;
> + else if (state & DELL_QUIET)
> + return DELL_QUIET;
> + else if (state & DELL_PERFORMANCE)
> + return DELL_PERFORMANCE;
> + else
> + return -ENXIO;
> +}
> +
> +static int thermal_get_supported_modes(int *supported_bits) {
> + struct calling_interface_buffer buffer;
> + int ret;
> +
> + dell_fill_request(&buffer, 0x0, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO,
> SELECT_THERMAL_MANAGEMENT);
> + if (ret)
> + return ret;
> + *supported_bits = FIELD_GET(DELL_THERMAL_SUPPORTED,
> buffer.output[1]);
> + return 0;
> +}
> +
> +static int thermal_get_acc_mode(int *acc_mode) {
> + struct calling_interface_buffer buffer;
> + int ret;
> +
> + dell_fill_request(&buffer, 0x0, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO,
> SELECT_THERMAL_MANAGEMENT);
> + if (ret)
> + return ret;
> + *acc_mode = FIELD_GET(DELL_ACC_GET_FIELD, buffer.output[3]);
> + return 0;
> +}
> +
> +static int thermal_set_mode(enum thermal_mode_bits state) {
> + struct calling_interface_buffer buffer;
> + int ret;
> + int acc_mode;
> +
> + ret = thermal_get_acc_mode(&acc_mode);
> + if (ret)
> + return ret;
> +
> + dell_fill_request(&buffer, 0x1, FIELD_PREP(DELL_ACC_SET_FIELD,
> acc_mode) | state, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO,
> SELECT_THERMAL_MANAGEMENT);
> + return ret;
> +}
> +
> +static int thermal_platform_profile_set(struct platform_profile_handler
> *pprof,
> + enum platform_profile_option
> profile) {
> + switch (profile) {
> + case PLATFORM_PROFILE_BALANCED:
> + return thermal_set_mode(DELL_BALANCED);
> + case PLATFORM_PROFILE_PERFORMANCE:
> + return thermal_set_mode(DELL_PERFORMANCE);
> + case PLATFORM_PROFILE_QUIET:
> + return thermal_set_mode(DELL_QUIET);
> + case PLATFORM_PROFILE_COOL:
> + return thermal_set_mode(DELL_COOL_BOTTOM);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int thermal_platform_profile_get(struct platform_profile_handler
> *pprof,
> + enum platform_profile_option
> *profile) {
> + int ret;
> +
> + ret = thermal_get_mode();
> + if (ret < 0)
> + return ret;
> +
> + switch (ret) {
> + case DELL_BALANCED:
> + *profile = PLATFORM_PROFILE_BALANCED;
> + break;
> + case DELL_PERFORMANCE:
> + *profile = PLATFORM_PROFILE_PERFORMANCE;
> + break;
> + case DELL_COOL_BOTTOM:
> + *profile = PLATFORM_PROFILE_COOL;
> + break;
> + case DELL_QUIET:
> + *profile = PLATFORM_PROFILE_QUIET;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int thermal_init(void)
> +{
> + int ret;
> + int supported_modes;
> +
> + /* If thermal modes not supported, exit without error */
> + ret = thermal_get_supported_modes(&supported_modes);
> + if (ret < 0)
> + return ret;
> + if (!supported_modes)
> + return 0;
> +
> + thermal_handler = kzalloc(sizeof(*thermal_handler), GFP_KERNEL);
> + if (!thermal_handler)
> + return -ENOMEM;
> + thermal_handler->profile_get = thermal_platform_profile_get;
> + thermal_handler->profile_set = thermal_platform_profile_set;
> +
> + if (supported_modes & DELL_QUIET)
> + set_bit(PLATFORM_PROFILE_QUIET, thermal_handler-
> >choices);
> + if (supported_modes & DELL_COOL_BOTTOM)
> + set_bit(PLATFORM_PROFILE_COOL, thermal_handler-
> >choices);
> + if (supported_modes & DELL_BALANCED)
> + set_bit(PLATFORM_PROFILE_BALANCED, thermal_handler-
> >choices);
> + if (supported_modes & DELL_PERFORMANCE)
> + set_bit(PLATFORM_PROFILE_PERFORMANCE,
> thermal_handler->choices);
> +
> + /* Clean up if failed */
> + ret = platform_profile_register(thermal_handler);
> + if (ret)
> + kfree(thermal_handler);
> +
> + return ret;
> +}
> +
> +static void thermal_cleanup(void)
> +{
> + if (thermal_handler) {
> + platform_profile_remove();
> + kfree(thermal_handler);
> + }
> +}
> +
> static struct led_classdev mute_led_cdev = {
> .name = "platform::mute",
> .max_brightness = 1,
> @@ -2238,6 +2468,11 @@ static int __init dell_init(void)
> goto fail_rfkill;
> }
>
> + /* Do not fail module if thermal modes not supported, just skip */
> + ret = thermal_init();
> + if (ret)
> + goto fail_thermal;
> +
> if (quirks && quirks->touchpad_led)
> touchpad_led_init(&platform_device->dev);
>
> @@ -2317,6 +2552,8 @@ static int __init dell_init(void)
> led_classdev_unregister(&mute_led_cdev);
> fail_led:
> dell_cleanup_rfkill();
> +fail_thermal:
> + thermal_cleanup();
> fail_rfkill:
> platform_device_del(platform_device);
> fail_platform_device2:
> @@ -2344,6 +2581,7 @@ static void __exit dell_exit(void)
> platform_device_unregister(platform_device);
> platform_driver_unregister(&platform_driver);
> }
> + thermal_cleanup();
> }
>
> /* dell-rbtn.c driver export functions which will not work correctly (and could
> diff --git a/drivers/platform/x86/dell/dell-smbios-base.c
> b/drivers/platform/x86/dell/dell-smbios-base.c
> index e61bfaf8b5c4..5bc2e394dd1c 100644
> --- a/drivers/platform/x86/dell/dell-smbios-base.c
> +++ b/drivers/platform/x86/dell/dell-smbios-base.c
> @@ -71,6 +71,7 @@ static struct smbios_call call_blacklist[] = {
> /* handled by kernel: dell-laptop */
> {0x0000, CLASS_INFO, SELECT_RFKILL},
> {0x0000, CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT},
> + {0x0000, CLASS_INFO, SELECT_THERMAL_MANAGEMENT},
> };
>
> struct token_range {
> diff --git a/drivers/platform/x86/dell/dell-smbios.h
> b/drivers/platform/x86/dell/dell-smbios.h
> index eb341bf000c6..585d042f1779 100644
> --- a/drivers/platform/x86/dell/dell-smbios.h
> +++ b/drivers/platform/x86/dell/dell-smbios.h
> @@ -19,6 +19,7 @@
> /* Classes and selects used only in kernel drivers */ #define
> CLASS_KBD_BACKLIGHT 4 #define SELECT_KBD_BACKLIGHT 11
> +#define SELECT_THERMAL_MANAGEMENT 19
>
> /* Tokens used in kernel drivers, any of these
> * should be filtered from userspace access
> --
> 2.42.0
Powered by blists - more mailing lists