linux-kernel - Re: [PATCH v1 03/10] platform/x86: msi-wmi-platform: Add quirk system

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: <70b63140-1e68-4db2-acb0-896636926dfd@gmx.de>
Date: Tue, 13 May 2025 22:43:52 +0200
From: Armin Wolf <W_Armin@....de>
To: Antheas Kapenekakis <lkml@...heas.dev>,
 platform-driver-x86@...r.kernel.org
Cc: Jonathan Corbet <corbet@....net>, Hans de Goede <hdegoede@...hat.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 Jean Delvare <jdelvare@...e.com>, Guenter Roeck <linux@...ck-us.net>,
 Kurt Borja <kuurtb@...il.com>, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v1 03/10] platform/x86: msi-wmi-platform: Add quirk system

Am 11.05.25 um 22:44 schrieb Antheas Kapenekakis:

> MSI uses the WMI interface as a passthrough for writes to the EC
> and uses a board name match and a quirk table from userspace on
> Windows. Therefore, there is no auto-detection functionality and
> we have to fallback to a quirk table.
>
> Introduce it here, prior to starting to add features to it.
>
> Signed-off-by: Antheas Kapenekakis <lkml@...heas.dev>
> ---
>   drivers/platform/x86/msi-wmi-platform.c | 45 +++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
>
> diff --git a/drivers/platform/x86/msi-wmi-platform.c b/drivers/platform/x86/msi-wmi-platform.c
> index f0d1b8e1a2fec..408d42ab19e20 100644
> --- a/drivers/platform/x86/msi-wmi-platform.c
> +++ b/drivers/platform/x86/msi-wmi-platform.c
> @@ -14,6 +14,7 @@
>   #include <linux/debugfs.h>
>   #include <linux/device.h>
>   #include <linux/device/driver.h>
> +#include <linux/dmi.h>
>   #include <linux/errno.h>
>   #include <linux/hwmon.h>
>   #include <linux/kernel.h>
> @@ -79,8 +80,12 @@ enum msi_wmi_platform_method {
>   	MSI_PLATFORM_GET_WMI		= 0x1d,
>   };
>   
> +struct msi_wmi_platform_quirk {
> +};
> +
>   struct msi_wmi_platform_data {
>   	struct wmi_device *wdev;
> +	struct msi_wmi_platform_quirk *quirks;
>   	struct mutex wmi_lock;	/* Necessary when calling WMI methods */
>   };
>   
> @@ -124,6 +129,39 @@ static const char * const msi_wmi_platform_debugfs_names[] = {
>   	"get_wmi"
>   };
>   
> +static struct msi_wmi_platform_quirk quirk_default = {};
> +static struct msi_wmi_platform_quirk quirk_gen1 = {
> +};
> +static struct msi_wmi_platform_quirk quirk_gen2 = {
> +};
> +
> +static const struct dmi_system_id msi_quirks[] = {
> +	{
> +		.ident = "MSI Claw (gen 1)",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
> +			DMI_MATCH(DMI_BOARD_NAME, "MS-1T41"),
> +		},
> +		.driver_data = &quirk_gen1,
> +	},
> +	{
> +		.ident = "MSI Claw AI+ 7",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
> +			DMI_MATCH(DMI_BOARD_NAME, "MS-1T42"),
> +		},
> +		.driver_data = &quirk_gen2,
> +	},
> +	{
> +		.ident = "MSI Claw AI+ 8",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
> +			DMI_MATCH(DMI_BOARD_NAME, "MS-1T52"),
> +		},
> +		.driver_data = &quirk_gen2,
> +	},
> +};

I would prefer if we rely on the model code reported by the embedded controller instead of
the SMBIOS board name.

The model code is contained inside the firmware string returned by the Get_EC() WMI method,
see https://github.com/BeardOverflow/msi-ec/blob/main/docs/device_support_guide.md (section "Additional Info") for details.
This way support for devices sharing a model code can be implemented more efficiently.

I suggest we extract this model code inside msi_wmi_platform_ec_init() and perform the quirk matching there.

Thanks,
Armin Wolf

> +
>   static int msi_wmi_platform_parse_buffer(union acpi_object *obj, u8 *output, size_t length)
>   {
>   	if (obj->type != ACPI_TYPE_BUFFER)
> @@ -413,6 +451,7 @@ static int msi_wmi_platform_init(struct msi_wmi_platform_data *data)
>   static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
>   {
>   	struct msi_wmi_platform_data *data;
> +	const struct dmi_system_id *dmi_id;
>   	int ret;
>   
>   	data = devm_kzalloc(&wdev->dev, sizeof(*data), GFP_KERNEL);
> @@ -422,6 +461,12 @@ static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
>   	data->wdev = wdev;
>   	dev_set_drvdata(&wdev->dev, data);
>   
> +	dmi_id = dmi_first_match(msi_quirks);
> +	if (dmi_id)
> +		data->quirks = dmi_id->driver_data;
> +	else
> +		data->quirks = &quirk_default;
> +
>   	ret = devm_mutex_init(&wdev->dev, &data->wmi_lock);
>   	if (ret < 0)
>   		return ret;