| 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: <D7T7XHTX1SS2.NTHM4WBUN8EC@gmail.com>
Date: Sat, 15 Feb 2025 13:07:50 -0500
From: "Kurt Borja" <kuurtb@...il.com>
To: "Armin Wolf" <W_Armin@....de>, <jlee@...e.com>,
<basak.sb2006@...il.com>, <rayanmargham4@...il.com>
Cc: <hdegoede@...hat.com>, <ilpo.jarvinen@...ux.intel.com>,
<platform-driver-x86@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 1/3] platform/x86: acer-wmi: Fix setting of fan
behavior
On Sat Feb 15, 2025 at 12:40 PM -05, Armin Wolf wrote:
> Am 15.02.25 um 02:30 schrieb Kurt Borja:
>
>> Hi Armin,
>>
>> On Fri Feb 14, 2025 at 5:13 PM -05, Armin Wolf wrote:
>>> After studying the linuwu_sense driver
>>> (https://github.com/0x7375646F/Linuwu-Sense) i was able to understand
>>> the meaning of the SetGamingFanBehavior() WMI method:
>>>
>>> - the first 16-bit are a bitmap of all fans affected by a fan behavior
>>> change request.
>>>
>>> - the next 8 bits contain four fan mode fields (2-bit), each being
>>> associated with a bit inside the fan bitmap.
>>>
>>> There are three fan modes: auto, turbo and custom.
>>>
>>> Use this newfound knowledge to fix the turbo fan handling by setting
>>> the correct bits before calling SetGamingFanBehavior(). Also check
>>> the result of the WMI method call and return an error should the ACPI
>>> firmware signal failure.
>>>
>>> Signed-off-by: Armin Wolf <W_Armin@....de>
>>> ---
>>> drivers/platform/x86/acer-wmi.c | 75 +++++++++++++++++++++++----------
>>> 1 file changed, 52 insertions(+), 23 deletions(-)
>>>
>>> --
>>> 2.39.5
>>>
>>> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
>>> index 69336bd778ee..f20a882e3650 100644
>>> --- a/drivers/platform/x86/acer-wmi.c
>>> +++ b/drivers/platform/x86/acer-wmi.c
>>> @@ -68,10 +68,19 @@ MODULE_LICENSE("GPL");
>>> #define ACER_WMID_SET_GAMING_LED_METHODID 2
>>> #define ACER_WMID_GET_GAMING_LED_METHODID 4
>>> #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
>>> -#define ACER_WMID_SET_GAMING_FAN_BEHAVIOR 14
>>> +#define ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID 14
>>> #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
>>> #define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
>>>
>>> +#define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0)
>>> +#define ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK GENMASK_ULL(23, 16)
>>> +
>>> +#define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0)
>>> +#define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3)
>>> +
>>> +#define ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK GENMASK(1, 0)
>>> +#define ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK GENMASK(7, 6)
>>> +
>>> #define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
>>> #define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
>>> #define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
>>> @@ -121,6 +130,12 @@ enum acer_wmi_predator_v4_sensor_id {
>>> ACER_WMID_SENSOR_GPU_TEMPERATURE = 0x0A,
>>> };
>>>
>>> +enum acer_wmi_gaming_fan_mode {
>>> + ACER_WMID_FAN_MODE_AUTO = 0x01,
>>> + ACER_WMID_FAN_MODE_TURBO = 0x02,
>>> + ACER_WMID_FAN_MODE_CUSTOM = 0x03,
>>> +};
>>> +
>>> enum acer_wmi_predator_v4_oc {
>>> ACER_WMID_OC_NORMAL = 0x0000,
>>> ACER_WMID_OC_TURBO = 0x0002,
>>> @@ -1565,9 +1580,6 @@ static acpi_status WMID_gaming_set_u64(u64 value, u32 cap)
>>> case ACER_CAP_TURBO_LED:
>>> method_id = ACER_WMID_SET_GAMING_LED_METHODID;
>>> break;
>>> - case ACER_CAP_TURBO_FAN:
>>> - method_id = ACER_WMID_SET_GAMING_FAN_BEHAVIOR;
>>> - break;
>>> default:
>>> return AE_BAD_PARAMETER;
>>> }
>>> @@ -1618,25 +1630,42 @@ static int WMID_gaming_get_sys_info(u32 command, u64 *out)
>>> return 0;
>>> }
>>>
>>> +static int WMID_gaming_set_fan_behavior(u16 fan_bitmap, u8 mode_bitmap)
>>> +{
>>> + acpi_status status;
>>> + u64 input = 0;
>>> + u64 result;
>>> +
>>> + input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_ID_MASK, fan_bitmap);
>>> + input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK, mode_bitmap);
>>> +
>>> + status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID, input,
>>> + &result);
>>> + if (ACPI_FAILURE(status))
>>> + return -EIO;
>>> +
>>> + /* TODO: Proper error handling */
>>> + pr_notice("Fan behavior return status: %llu\n", result);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static void WMID_gaming_set_fan_mode(u8 fan_mode)
>>> {
>>> - /* fan_mode = 1 is used for auto, fan_mode = 2 used for turbo*/
>>> - u64 gpu_fan_config1 = 0, gpu_fan_config2 = 0;
>>> - int i;
>>> -
>>> - if (quirks->cpu_fans > 0)
>>> - gpu_fan_config2 |= 1;
>>> - for (i = 0; i < (quirks->cpu_fans + quirks->gpu_fans); ++i)
>>> - gpu_fan_config2 |= 1 << (i + 1);
>> This was not replicated bellow. Just to be sure, are there no fans at
>> BIT(1) and BIT(2)?
>
> AFAIK the Acer OEM software support the following fans:
>
> - CPU (BIT(0))
> - GPU 1 (BIT(3))
> - GPU 2 (BIT(4), but untested)
>
> The other bits seem to be unused.
Interesting.
I ask because quirks->cpu_fans + quirks->gpu_fans is currently always 2,
so the line two `-` lines I referenced make the following equivalent
operation:
fan_bitmap |= BIT(1);
mode_bitmap |= FIELD_PREP(GENMASK(3, 2), fan_mode);
fan_bitmap |= BIT(2);
mode_bitmap |= FIELD_PREP(GENMASK(5, 4), fan_mode);
So if any model has fans at BIT(1), BIT(2) this may cause regressions if
the behavior is not mimicked.
Am I missing something?
Anyway, your explaination for how this method works makes a lot of
sense, so it is weird that the original author of this is summing the
number of fans and setting this bits in the first place.
--
~ Kurt
>
> Thanks,
> Armin Wolf
>
>>> - for (i = 0; i < quirks->gpu_fans; ++i)
>>> - gpu_fan_config2 |= 1 << (i + 3);
>>> - if (quirks->cpu_fans > 0)
>>> - gpu_fan_config1 |= fan_mode;
>>> - for (i = 0; i < (quirks->cpu_fans + quirks->gpu_fans); ++i)
>>> - gpu_fan_config1 |= fan_mode << (2 * i + 2);
>> Same as above.
>>
Powered by blists - more mailing lists