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: <4cbf5396-37e5-5e66-e26f-c43186e285da@linux.intel.com>
Date: Mon, 12 Aug 2024 15:19:23 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Armin Wolf <W_Armin@....de>
cc: matan@...alib.org, ghostwind@...il.com, 
    Hans de Goede <hdegoede@...hat.com>, platform-driver-x86@...r.kernel.org, 
    LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] platform/x86: lg-laptop: Add operation region support

On Fri, 9 Aug 2024, Armin Wolf wrote:

> The LEGX0820 ACPI device is expected to provide a custom operation
> region:
> 
> 	OperationRegion (XIN1, 0x8F, Zero, 0x04B0)
>         Field (XIN1, AnyAcc, Lock, Preserve)
>         {
>             DMSG,   8,
>             HDAP,   8,
>             Offset (0x03),
>             AFNM,   8,
>             Offset (0x10),
>             P80B,   8,
>             P81B,   8,
>             P82B,   8,
>             P83B,   8,
>             P84B,   8,
>             P85B,   8,
>             P86B,   8,
>             P87B,   8,
>             Offset (0x20),
>             DTTM,   8,
>             TMP1,   8,
>             LTP1,   8,
>             HTP1,   8,
>             TMP2,   8,
>             LTP2,   8,
>             HTP2,   8,
>             Offset (0x3E8),
>             PMSG,   1600
>         }
> 
> The PMSG field is used by AML code to log debug messages when DMSG is
> true. Since those debug messages are already logged using the standard
> ACPI Debug object, we set DMSG unconditionally to 0x00 and ignore any
> writes to PMSG.
> 
> The TMPx, LTPx, HTPx and AFNM fields are used to inform the driver when
> the temperature/(presumably) trip points/fan mode changes. This only
> happens when the DTTM flag is set.
> 
> Unfortunately we have to implement support for this operation region
> because the AML codes uses code constructs like this one:
> 
> 	If (((\_SB.XINI.PLAV != Zero) && (\_SB.XINI.DTTM != Zero)))
> 
> The PLAV field gets set to 1 when the driver registers its address space
> handler, so by default XIN1 should not be accessed.
> 
> However ACPI does not use short-circuit evaluation when evaluating
> logical conditions. This causes the DTTM field to be accessed even
> when PLAV is 0, which results in an ACPI error.
> Since this check happens inside various thermal-related ACPI control
> methods, various thermal zone become unusable since any attempts to

attempt

> read their temperature results in an ACPI error.
> 
> Fix this by providing support for this operation region. I suspect
> that the problem does not happen under Windows (which seemingly does
> not use short-circuit evaluation either) because the necessary driver
> comes preinstalled with the machine.
> 
> Tested-by: Chris <ghostwind@...il.com>
> Signed-off-by: Armin Wolf <W_Armin@....de>
> ---
>  drivers/platform/x86/lg-laptop.c | 128 +++++++++++++++++++++++++++++++
>  1 file changed, 128 insertions(+)
> 
> diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c
> index 9c7857842caf..6310a23f808c 100644
> --- a/drivers/platform/x86/lg-laptop.c
> +++ b/drivers/platform/x86/lg-laptop.c
> @@ -8,6 +8,8 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
>  #include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/dev_printk.h>
>  #include <linux/dmi.h>
>  #include <linux/input.h>
>  #include <linux/input/sparse-keymap.h>
> @@ -31,6 +33,25 @@ MODULE_AUTHOR("Matan Ziv-Av");
>  MODULE_DESCRIPTION("LG WMI Hotkey Driver");
>  MODULE_LICENSE("GPL");
> 
> +static bool fw_debug;
> +module_param(fw_debug, bool, 0);
> +MODULE_PARM_DESC(fw_debug, "Enable printing of firmware debug messages");
> +
> +#define LG_ADDRESS_SPACE_ID			0x8F
> +
> +#define LG_ADDRESS_SPACE_DEBUG_FLAG_ADR		0x00
> +#define LG_ADDRESS_SPACE_DEBUG_MSG_START_ADR	0x3E8
> +#define LG_ADDRESS_SPACE_DEBUG_MSG_END_ADR	0x5E8
> +
> +#define LG_ADDRESS_SPACE_DTTM_FLAG_ADR		0x20
> +#define LG_ADDRESS_SPACE_FAN_MODE_ADR		0x03

Any particular reason why there are not in numerical order?

> +#define LG_ADDRESS_SPACE_CPU_TEMP_ADR		0x21
> +#define LG_ADDRESS_SPACE_CPU_TRIP_LOW_ADR	0x22
> +#define LG_ADDRESS_SPACE_CPU_TRIP_HIGH_ADR	0x23
> +#define LG_ADDRESS_SPACE_MB_TEMP_ADR		0x24
> +#define LG_ADDRESS_SPACE_MB_TRIP_LOW_ADR	0x25
> +#define LG_ADDRESS_SPACE_MB_TRIP_HIGH_ADR	0x26
> +
>  #define WMI_EVENT_GUID0	"E4FB94F9-7F2B-4173-AD1A-CD1D95086248"
>  #define WMI_EVENT_GUID1	"023B133E-49D1-4E10-B313-698220140DC2"
>  #define WMI_EVENT_GUID2	"37BE1AC0-C3F2-4B1F-BFBE-8FDEAF2814D6"
> @@ -646,6 +667,101 @@ static struct platform_driver pf_driver = {
>  	}
>  };
> 
> +static acpi_status lg_laptop_address_space_write(struct device *dev, acpi_physical_address address,
> +						 size_t size, u64 *value)

Why is this write function taking u64 _pointer_?

> +{
> +	u8 byte;
> +
> +	/* Ignore any debug messages */
> +	if (address >= LG_ADDRESS_SPACE_DEBUG_MSG_START_ADR &&
> +	    address <= LG_ADDRESS_SPACE_DEBUG_MSG_END_ADR)
> +		return AE_OK;
> +
> +	if (size != sizeof(byte))
> +		return AE_BAD_PARAMETER;
> +
> +	byte = *value & 0xFF;
> +
> +	switch (address) {
> +	case LG_ADDRESS_SPACE_FAN_MODE_ADR:
> +		dev_dbg(dev, "Fan mode set to mode %u\n", byte);
> +		return AE_OK;
> +	case LG_ADDRESS_SPACE_CPU_TEMP_ADR:
> +		dev_dbg(dev, "CPU temperature is %u °C\n", byte);
> +		return AE_OK;
> +	case LG_ADDRESS_SPACE_CPU_TRIP_LOW_ADR:
> +		dev_dbg(dev, "CPU lower trip point set to %u °C\n", byte);
> +		return AE_OK;
> +	case LG_ADDRESS_SPACE_CPU_TRIP_HIGH_ADR:
> +		dev_dbg(dev, "CPU higher trip point set to %u °C\n", byte);
> +		return AE_OK;
> +	case LG_ADDRESS_SPACE_MB_TEMP_ADR:
> +		dev_dbg(dev, "Motherboard temperature is %u °C\n", byte);
> +		return AE_OK;
> +	case LG_ADDRESS_SPACE_MB_TRIP_LOW_ADR:
> +		dev_dbg(dev, "Motherboard lower trip point set to %u °C\n", byte);
> +		return AE_OK;
> +	case LG_ADDRESS_SPACE_MB_TRIP_HIGH_ADR:
> +		dev_dbg(dev, "Motherboard higher trip point set to %u °C\n", byte);
> +		return AE_OK;
> +	default:
> +		dev_notice_ratelimited(dev, "Ignoring write to unknown opregion address %llu\n",
> +				       address);
> +		return AE_OK;
> +	}
> +}
> +
> +static acpi_status lg_laptop_address_space_read(struct device *dev, acpi_physical_address address,
> +						size_t size, u64 *value)
> +{
> +	if (size != 1)
> +		return AE_BAD_PARAMETER;
> +
> +	switch (address) {
> +	case LG_ADDRESS_SPACE_DEBUG_FLAG_ADR:
> +		/* Debug messages are already printed using the standard ACPI Debug object */
> +		*value = 0x00;
> +		return AE_OK;
> +	case LG_ADDRESS_SPACE_DTTM_FLAG_ADR:
> +		*value = fw_debug;
> +		return AE_OK;
> +	default:
> +		dev_notice_ratelimited(dev, "Attempt to read unknown opregion address %llu\n",
> +				       address);
> +		return AE_BAD_PARAMETER;
> +	}
> +}
> +
> +static acpi_status lg_laptop_address_space_handler(u32 function, acpi_physical_address address,
> +						   u32 bits, u64 *value, void *handler_context,
> +						   void *region_context)
> +{
> +	struct device *dev = handler_context;
> +	size_t size;
> +
> +	if (bits % 8)
> +		return AE_BAD_PARAMETER;
> +
> +	size = bits / 8;

BITS_PER_BYTE x2

-- 
 i.

> +
> +	switch (function) {
> +	case ACPI_READ:
> +		return lg_laptop_address_space_read(dev, address, size, value);
> +	case ACPI_WRITE:
> +		return lg_laptop_address_space_write(dev, address, size, value);
> +	default:
> +		return AE_BAD_PARAMETER;
> +	}
> +}
> +
> +static void lg_laptop_remove_address_space_handler(void *data)
> +{
> +	struct acpi_device *device = data;
> +
> +	acpi_remove_address_space_handler(device->handle, LG_ADDRESS_SPACE_ID,
> +					  &lg_laptop_address_space_handler);
> +}
> +
>  static int acpi_add(struct acpi_device *device)
>  {
>  	struct platform_device_info pdev_info = {
> @@ -653,6 +769,7 @@ static int acpi_add(struct acpi_device *device)
>  		.name = PLATFORM_NAME,
>  		.id = PLATFORM_DEVID_NONE,
>  	};
> +	acpi_status status;
>  	int ret;
>  	const char *product;
>  	int year = 2017;
> @@ -660,6 +777,17 @@ static int acpi_add(struct acpi_device *device)
>  	if (pf_device)
>  		return 0;
> 
> +	status = acpi_install_address_space_handler(device->handle, LG_ADDRESS_SPACE_ID,
> +						    &lg_laptop_address_space_handler,
> +						    NULL, &device->dev);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	ret = devm_add_action_or_reset(&device->dev, lg_laptop_remove_address_space_handler,
> +				       device);
> +	if (ret < 0)
> +		return ret;
> +
>  	ret = platform_driver_register(&pf_driver);
>  	if (ret)
>  		return ret;
> --
> 2.39.2
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ