| 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: <9ba4a500-9d88-4630-bd94-99f07dd51abe@amd.com>
Date: Sat, 11 May 2024 10:16:56 -0500
From: "Limonciello, Mario" <mario.limonciello@....com>
To: Lyndon Sanche <lsanche@...deno.ca>
Cc: pali@...nel.org, W_Armin@....de, srinivas.pandruvada@...ux.intel.com,
ilpo.jarvinen@...ux.intel.com, lkp@...el.com, hdegoede@...hat.com,
Yijun.Shen@...l.com, Matthew Garrett <mjg59@...f.ucam.org>,
Heiner Kallweit <hkallweit1@...il.com>, Randy Dunlap
<rdunlap@...radead.org>, Jonathan Corbet <corbet@....net>,
Vegard Nossum <vegard.nossum@...cle.com>,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
Dell.Client.Kernel@...l.com
Subject: Re: [PATCH v6 2/2] platform/x86: dell-laptop: Implement
platform_profile
On 5/10/2024 9:36 PM, Lyndon Sanche wrote:
> 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>
> ---
> drivers/platform/x86/dell/Kconfig | 2 +
> drivers/platform/x86/dell/dell-laptop.c | 242 +++++++++++++++++++
> drivers/platform/x86/dell/dell-smbios-base.c | 1 +
> drivers/platform/x86/dell/dell-smbios.h | 1 +
> 4 files changed, 246 insertions(+)
>
> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> index bd9f445974cc..26679f22733c 100644
> --- a/drivers/platform/x86/dell/Kconfig
> +++ b/drivers/platform/x86/dell/Kconfig
> @@ -47,6 +47,7 @@ config DCDBAS
> config DELL_LAPTOP
> tristate "Dell Laptop Extras"
> default m
> + depends on ACPI
> depends on DMI
> depends on BACKLIGHT_CLASS_DEVICE
> depends on ACPI_VIDEO || ACPI_VIDEO = n
> @@ -57,6 +58,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..07f54f1cbac5 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,236 @@ 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 commands not supported, exit without error */
> + if (!dell_laptop_check_supported_cmds(CLASS_INFO))
> + return 0;
> +
> + /* 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 +2472,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 +2556,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 +2585,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 6ae09d7f76fb..387fa5618f7a 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},
> };
So when Alex checked on v5 that this doesn't load on workstations, it
has made me realize that doing this will block the interface totally
even on workstations.
So I think there are a few ways to go to handle this:
1) Rename dell-laptop to dell-client or dell-pc and let dell-laptop load
on more form factors. This would require some internal handling in the
module for which features make sense for different form factors.
2) Add a new module just for the thermal handling and put all this code
into it instead.
I don't have a strong opinion, but I do think one of them should be done
to ensure there aren't problems on workstations losing access to thermal
control.
>
> struct token_range {
> diff --git a/drivers/platform/x86/dell/dell-smbios.h b/drivers/platform/x86/dell/dell-smbios.h
> index 63116671eada..5d7178df80c6 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
Powered by blists - more mailing lists