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: <r3c2of4wx7kpaova47dqo23vsxv5p3kscw6rmxuxwi6opx2bua@dabdghtinnaz>
Date: Tue, 29 Oct 2024 21:02:28 -0300
From: Kurt Borja <kuurtb@...il.com>
To: Armin Wolf <W_Armin@....de>
Cc: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>, 
	Hans de Goede <hdegoede@...hat.com>, LKML <linux-kernel@...r.kernel.org>, 
	platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v10 3/5] alienware-wmi: added platform profile support

On Wed, Oct 30, 2024 at 12:11:51AM +0100, Armin Wolf wrote:
> Am 29.10.24 um 18:06 schrieb Ilpo Järvinen:
> 
> > On Tue, 29 Oct 2024, Kurt Borja wrote:
> > 
> > > On Tue, Oct 29, 2024 at 05:44:05PM +0200, Ilpo Järvinen wrote:
> > > > On Tue, 29 Oct 2024, Kurt Borja wrote:
> > > > 
> > > > > Implements platform profile support for Dell laptops with new WMAX thermal
> > > > > interface, present on some Alienware X-Series, Alienware M-Series and
> > > > > Dell's G-Series laptops. This interface is suspected to be used by
> > > > > Alienware Command Center (AWCC), which is not available for linux
> > > > > systems, to manage thermal profiles.
> > > > > 
> > > > > This implementation makes use of three WMI methods, namely
> > > > > THERMAL_CONTROL, THERMAL_INFORMATION and GAME_SHIFT_STATUS, which take
> > > > > u32 as input and output arguments. Each method has a set of supported
> > > > > operations specified in their respective enums.
> > > > > 
> > > > > Not all models with WMAX WMI interface support these methods. Because of
> > > > > this, models have to manually declare support through new quirks
> > > > > `thermal` for THERMAL_CONTROL and THERMAL_INFORMATION and `gmode` for
> > > > > GAME_SHIFT_STATUS.
> > > > > 
> > > > > Wrappers written for these methods support multiple operations.
> > > > > 
> > > > > THERMAL_CONTROL switches thermal modes through operation
> > > > > ACTIVATE_PROFILE. Available thermal codes are auto-detected at runtime
> > > > > and matched against a list of known thermal codes:
> > > > > 
> > > > > Thermal Table "User Selectable Thermal Tables" (USTT):
> > > > > 	BALANCED			0xA0
> > > > > 	BALANCED_PERFORMANCE		0xA1
> > > > > 	COOL				0xA2
> > > > > 	QUIET				0xA3
> > > > > 	PERFORMANCE			0xA4
> > > > > 	LOW_POWER			0xA5
> > > > > 
> > > > > Thermal Table Basic:
> > > > > 	QUIET				0x96
> > > > > 	BALANCED			0x97
> > > > > 	BALANCED_PERFORMANCE		0x98
> > > > > 	PERFORMANCE			0x99
> > > > > 
> > > > > Devices are known to implement only one of these tables without mixing
> > > > > their thermal codes.
> > > > > 
> > > > > The fact that the least significant digit of every thermal code is
> > > > > consecutive of one another is exploited to efficiently match codes
> > > > > through arrays.
> > > > > 
> > > > > Autodetection of available codes is done through operation LIST_IDS of
> > > > > method THERMAL_INFORMATION. This operation lists fan IDs, CPU sensor ID,
> > > > > GPU sensor ID and available thermal profile codes, *in that order*. As
> > > > > number of fans and thermal codes is very model dependent, almost every
> > > > > ID is scanned and matched based on conditions found on
> > > > > is_wmax_thermal_code(). The known upper bound for the number of IDs is
> > > > > 13, corresponding to a device that have 4 fans, 2 sensors and 7 thermal
> > > > > codes.
> > > > > 
> > > > > Additionally G-Series laptops have a key called G-key, which (with AWCC
> > > > > proprietary driver) switches the thermal mode to an special mode named
> > > > > GMODE with code 0xAB and changes Game Shift Status to 1. Game Shift is a
> > > > > mode the manufacturer claims, increases gaming performance.
> > > > > 
> > > > > GAME_SHIFT_STATUS method is used to mimic this behavior when selecting
> > > > > PLATFORM_PROFILE_PERFORMANCE option.
> > > > > 
> > > > > All of these profiles are known to only change fan speed profiles,
> > > > > although there are untested claims that some of them also change power
> > > > > profiles.
> > > > > 
> > > > > Activating a thermal mode with method THERMAL_CONTROL may cause short
> > > > > hangs. This is a known problem present on every platform.
> > > > > 
> > > > > Signed-off-by: Kurt Borja <kuurtb@...il.com>
> > > > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> > > > > ---
> > > > > v10:
> > > > >   - Corrected THERMAL_MODE_BASIC_BALANCED -> THERMAL_MODE_BASIC_QUIET in
> > > > >     is_wmax_thermal_code() logic
> > > > >   - `thermal` and `gmode` quirks now have to be manually selected,
> > > > >     because not all Dell devices posses the new WMI thermal methods.
> > > > >   - Because of the above reason, errors in create_thermal_profile are now
> > > > >     propagated
> > > > > v9:
> > > > >   - Bool comparisons are now coherent with their type
> > > > > v8:
> > > > >   - Fixed alignment in wmax_mode_to_platform_profile[]
> > > > >   - Quirk thermal and gmode changed from u8 -> bool
> > > > >   - Autodetected quirk entries are not initialized
> > > > >   - is_wmax_thermal_code refactored to increase readibility
> > > > >   - is_wmax_thermal_code now covers all possibilities
> > > > >   - Better commit message
> > > > > v7:
> > > > >   - Method operations are now clearly listed as separate enums
> > > > >   - wmax_thermal_modes are now listed without codes in order to support
> > > > >     autodetection, as well as getting and setting thermal profiles
> > > > >     cleanly through arrays
> > > > >   - Added wmax_mode_to_platform_profile[]
> > > > >   - Added struct wmax_u32_args to replace bit mask approach of
> > > > >     constructing arguments for wmax methods
> > > > >   - create_thermal_profile now autodetects available thermal codes
> > > > >     through operation 0x03 of THERMAL_INFORMATION method. These are
> > > > >     codes are stored in supported_thermal_profiles[]
> > > > >   - thermal_profile_get now uses wmax_mode_to_platform_profile[] instead of
> > > > >     switch-case approach
> > > > >   - thermal_profile_set now uses supported_thermal_profiles[] instead of
> > > > >     switch-case approach
> > > > >   - When gmode is autodetected, thermal_profile_set also sets Game Shift
> > > > >     status accordingly
> > > > > v6:
> > > > >   - Fixed alignment on some function definitions
> > > > >   - Fixed braces on if statment
> > > > >   - Removed quirk thermal_ustt
> > > > >   - Now quirk thermal can take values defined in enum WMAX_THERMAL_TABLE.
> > > > >   - Proper removal of thermal_profile
> > > > > ---
> > > > >   drivers/platform/x86/dell/Kconfig         |   1 +
> > > > >   drivers/platform/x86/dell/alienware-wmi.c | 306 ++++++++++++++++++++++
> > > > >   2 files changed, 307 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> > > > > index 68a49788a..b06d634cd 100644
> > > > > --- a/drivers/platform/x86/dell/Kconfig
> > > > > +++ b/drivers/platform/x86/dell/Kconfig
> > > > > @@ -21,6 +21,7 @@ config ALIENWARE_WMI
> > > > >   	depends on LEDS_CLASS
> > > > >   	depends on NEW_LEDS
> > > > >   	depends on ACPI_WMI
> > > > > +	select ACPI_PLATFORM_PROFILE
> > > > >   	help
> > > > >   	 This is a driver for controlling Alienware BIOS driven
> > > > >   	 features.  It exposes an interface for controlling the AlienFX
> > > > > diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
> > > > > index b27f3b64c..1d62c2ce7 100644
> > > > > --- a/drivers/platform/x86/dell/alienware-wmi.c
> > > > > +++ b/drivers/platform/x86/dell/alienware-wmi.c
> > > > > @@ -8,8 +8,11 @@
> > > > >   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > > 
> > > > >   #include <linux/acpi.h>
> > > > > +#include <linux/bitfield.h>
> > > > > +#include <linux/bits.h>
> > > > >   #include <linux/module.h>
> > > > >   #include <linux/platform_device.h>
> > > > > +#include <linux/platform_profile.h>
> > > > >   #include <linux/dmi.h>
> > > > >   #include <linux/leds.h>
> > > > > 
> > > > > @@ -25,6 +28,13 @@
> > > > >   #define WMAX_METHOD_AMPLIFIER_CABLE	0x6
> > > > >   #define WMAX_METHOD_DEEP_SLEEP_CONTROL	0x0B
> > > > >   #define WMAX_METHOD_DEEP_SLEEP_STATUS	0x0C
> > > > > +#define WMAX_METHOD_THERMAL_INFORMATION	0x14
> > > > > +#define WMAX_METHOD_THERMAL_CONTROL	0x15
> > > > > +#define WMAX_METHOD_GAME_SHIFT_STATUS	0x25
> > > > > +
> > > > > +#define WMAX_THERMAL_MODE_GMODE		0xAB
> > > > > +
> > > > > +#define WMAX_FAILURE_CODE		0xFFFFFFFF
> > > > > 
> > > > >   MODULE_AUTHOR("Mario Limonciello <mario.limonciello@...look.com>");
> > > > >   MODULE_DESCRIPTION("Alienware special feature control");
> > > > > @@ -49,11 +59,59 @@ enum WMAX_CONTROL_STATES {
> > > > >   	WMAX_SUSPEND = 3,
> > > > >   };
> > > > > 
> > > > > +enum WMAX_THERMAL_INFORMATION_OPERATIONS {
> > > > > +	WMAX_OPERATION_LIST_IDS			= 0x03,
> > > > > +	WMAX_OPERATION_CURRENT_PROFILE		= 0x0B,
> > > > > +};
> > > > > +
> > > > > +enum WMAX_THERMAL_CONTROL_OPERATIONS {
> > > > > +	WMAX_OPERATION_ACTIVATE_PROFILE		= 0x01,
> > > > > +};
> > > > > +
> > > > > +enum WMAX_GAME_SHIFT_STATUS_OPERATIONS {
> > > > > +	WMAX_OPERATION_TOGGLE_GAME_SHIFT	= 0x01,
> > > > > +	WMAX_OPERATION_GET_GAME_SHIFT_STATUS	= 0x02,
> > > > > +};
> > > > > +
> > > > > +enum WMAX_THERMAL_TABLES {
> > > > > +	WMAX_THERMAL_TABLE_BASIC		= 0x90,
> > > > > +	WMAX_THERMAL_TABLE_USTT			= 0xA0,
> > > > > +};
> > > > > +
> > > > > +enum wmax_thermal_mode {
> > > > > +	THERMAL_MODE_USTT_BALANCED,
> > > > > +	THERMAL_MODE_USTT_BALANCED_PERFORMANCE,
> > > > > +	THERMAL_MODE_USTT_COOL,
> > > > > +	THERMAL_MODE_USTT_QUIET,
> > > > > +	THERMAL_MODE_USTT_PERFORMANCE,
> > > > > +	THERMAL_MODE_USTT_LOW_POWER,
> > > > > +	THERMAL_MODE_BASIC_QUIET,
> > > > > +	THERMAL_MODE_BASIC_BALANCED,
> > > > > +	THERMAL_MODE_BASIC_BALANCED_PERFORMANCE,
> > > > > +	THERMAL_MODE_BASIC_PERFORMANCE,
> > > > > +	THERMAL_MODE_LAST,
> > > > > +};
> > > > > +
> > > > > +static const enum platform_profile_option wmax_mode_to_platform_profile[THERMAL_MODE_LAST] = {
> > > > > +	[THERMAL_MODE_USTT_BALANCED]			= PLATFORM_PROFILE_BALANCED,
> > > > > +	[THERMAL_MODE_USTT_BALANCED_PERFORMANCE]	= PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> > > > > +	[THERMAL_MODE_USTT_COOL]			= PLATFORM_PROFILE_COOL,
> > > > > +	[THERMAL_MODE_USTT_QUIET]			= PLATFORM_PROFILE_QUIET,
> > > > > +	[THERMAL_MODE_USTT_PERFORMANCE]			= PLATFORM_PROFILE_PERFORMANCE,
> > > > > +	[THERMAL_MODE_USTT_LOW_POWER]			= PLATFORM_PROFILE_LOW_POWER,
> > > > > +	[THERMAL_MODE_BASIC_QUIET]			= PLATFORM_PROFILE_QUIET,
> > > > > +	[THERMAL_MODE_BASIC_BALANCED]			= PLATFORM_PROFILE_BALANCED,
> > > > > +	[THERMAL_MODE_BASIC_BALANCED_PERFORMANCE]	= PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> > > > > +	[THERMAL_MODE_BASIC_PERFORMANCE]		= PLATFORM_PROFILE_PERFORMANCE,
> > > > > +};
> > > > > +
> > > > >   struct quirk_entry {
> > > > >   	u8 num_zones;
> > > > >   	u8 hdmi_mux;
> > > > >   	u8 amplifier;
> > > > >   	u8 deepslp;
> > > > > +	bool thermal;
> > > > > +	bool gmode;
> > > > >   };
> > > > > 
> > > > >   static struct quirk_entry *quirks;
> > > > > @@ -64,6 +122,8 @@ static struct quirk_entry quirk_inspiron5675 = {
> > > > >   	.hdmi_mux = 0,
> > > > >   	.amplifier = 0,
> > > > >   	.deepslp = 0,
> > > > > +	.thermal = false,
> > > > > +	.gmode = false,
> > > > >   };
> > > > > 
> > > > >   static struct quirk_entry quirk_unknown = {
> > > > > @@ -71,6 +131,8 @@ static struct quirk_entry quirk_unknown = {
> > > > >   	.hdmi_mux = 0,
> > > > >   	.amplifier = 0,
> > > > >   	.deepslp = 0,
> > > > > +	.thermal = false,
> > > > > +	.gmode = false,
> > > > >   };
> > > > > 
> > > > >   static struct quirk_entry quirk_x51_r1_r2 = {
> > > > > @@ -78,6 +140,8 @@ static struct quirk_entry quirk_x51_r1_r2 = {
> > > > >   	.hdmi_mux = 0,
> > > > >   	.amplifier = 0,
> > > > >   	.deepslp = 0,
> > > > > +	.thermal = false,
> > > > > +	.gmode = false,
> > > > >   };
> > > > > 
> > > > >   static struct quirk_entry quirk_x51_r3 = {
> > > > > @@ -85,6 +149,8 @@ static struct quirk_entry quirk_x51_r3 = {
> > > > >   	.hdmi_mux = 0,
> > > > >   	.amplifier = 1,
> > > > >   	.deepslp = 0,
> > > > > +	.thermal = false,
> > > > > +	.gmode = false,
> > > > >   };
> > > > > 
> > > > >   static struct quirk_entry quirk_asm100 = {
> > > > > @@ -92,6 +158,8 @@ static struct quirk_entry quirk_asm100 = {
> > > > >   	.hdmi_mux = 1,
> > > > >   	.amplifier = 0,
> > > > >   	.deepslp = 0,
> > > > > +	.thermal = false,
> > > > > +	.gmode = false,
> > > > >   };
> > > > > 
> > > > >   static struct quirk_entry quirk_asm200 = {
> > > > > @@ -99,6 +167,8 @@ static struct quirk_entry quirk_asm200 = {
> > > > >   	.hdmi_mux = 1,
> > > > >   	.amplifier = 0,
> > > > >   	.deepslp = 1,
> > > > > +	.thermal = false,
> > > > > +	.gmode = false,
> > > > >   };
> > > > > 
> > > > >   static struct quirk_entry quirk_asm201 = {
> > > > > @@ -106,6 +176,17 @@ static struct quirk_entry quirk_asm201 = {
> > > > >   	.hdmi_mux = 1,
> > > > >   	.amplifier = 1,
> > > > >   	.deepslp = 1,
> > > > > +	.thermal = false,
> > > > > +	.gmode = false,
> > > > > +};
> > > > > +
> > > > > +static struct quirk_entry quirk_x_series = {
> > > > > +	.num_zones = 2,
> > > > > +	.hdmi_mux = 0,
> > > > > +	.amplifier = 0,
> > > > > +	.deepslp = 0,
> > > > > +	.thermal = true,
> > > > > +	.gmode = false,
> > > > >   };
> > > > So now gmode is always false unless the module parameter from patch 4 is
> > > > given?
> > > G-Series laptops can also register a quirk_entry with gmode set in the
> > > future. I can do it ahead of time, but I don't have a G-Series laptop to
> > > test it.
> > Understood. I'm fine with this in the current form if it's fine for Armin
> > too.
> 
> I am fine with this, we can add the necessary quirks later should someone have access
> to the necessary hardware.

I'm glad to hear that! Thank you both. 

Kurt

> 
> Reviewed-by: Armin Wolf <W_Armin@....de>
> 
> > 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ