Re: [PATCH 1/4] platform/x86: hp-wmi: fix fan table parsing

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Wed, 1 Apr 2026, Krishna Chomal wrote:

> For Victus S devices, the BIOS fan table header was being incorrectly
> parsed as:
> struct {
> 	u8 unknown;
> 	u8 num_entries;
> }
> 
> The first field should be num_fans and the second should be unknown. It
> is pure coincidence that interpreting an "unknown" field as "num_entries"

So this coincidence is that both fields happened to contain the same 
value? Or (same or) smaller?

-- 
 i.

> worked on multiple device, however for board 8D87 (in an upcoming patch),
> this assumption fails, and the hp-wmi driver fails to load.
> 
> We fix this by correcting the header definition and compensating for
> num_entries by parsing each entry of the fan table until an all-NULL row
> is obtained, mirroring the behavior of OMEN Gaming Hub on Windows.
> 
> Fixes: 46be1453e6e6 ("platform/x86: hp-wmi: add manual fan control for Victus S models")
> Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 41 +++++++++++++++++++++++++-------
>  1 file changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 304d9ac63c8a..d630b7f6e146 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -433,14 +433,14 @@ struct hp_wmi_hwmon_priv {
>  };
>  
>  struct victus_s_fan_table_header {
> +	u8 num_fans;
>  	u8 unknown;
> -	u8 num_entries;
>  } __packed;
>  
>  struct victus_s_fan_table_entry {
>  	u8 cpu_rpm;
>  	u8 gpu_rpm;
> -	u8 unknown;
> +	u8 noise_db;
>  } __packed;
>  
>  struct victus_s_fan_table {
> @@ -2514,7 +2514,9 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
>  	u8 fan_data[128] = { 0 };
>  	struct victus_s_fan_table *fan_table;
>  	u8 min_rpm, max_rpm, gpu_delta;
> -	int ret;
> +	u8 cpu_rpm, gpu_rpm, noise_db;
> +	int ret, num_entries, i;
> +	size_t header_size, entry_size;
>  
>  	/* Default behaviour on hwmon init is automatic mode */
>  	priv->mode = PWM_MODE_AUTO;
> @@ -2529,13 +2531,36 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
>  		return ret;
>  
>  	fan_table = (struct victus_s_fan_table *)fan_data;
> -	if (fan_table->header.num_entries == 0 ||
> -	    sizeof(struct victus_s_fan_table_header) +
> -	    sizeof(struct victus_s_fan_table_entry) * fan_table->header.num_entries > sizeof(fan_data))
> +	if (fan_table->header.num_fans == 0)
> +		return -EINVAL;
> +
> +	header_size = sizeof(struct victus_s_fan_table_header);
> +	entry_size = sizeof(struct victus_s_fan_table_entry);
> +	num_entries = (sizeof(fan_data) - header_size) / entry_size;
> +	min_rpm = U8_MAX;
> +	max_rpm = 0;
> +
> +	for (i = 0 ; i < num_entries ; i++) {
> +		cpu_rpm = fan_table->entries[i].cpu_rpm;
> +		gpu_rpm = fan_table->entries[i].gpu_rpm;
> +		noise_db = fan_table->entries[i].noise_db;
> +
> +		/*
> +		 * On some devices, the fan table is truncated with an all-zero row,
> +		 * hence we stop parsing here.
> +		 */
> +		if (cpu_rpm == 0 && gpu_rpm == 0 && noise_db == 0)
> +			break;
> +
> +		if (cpu_rpm < min_rpm)
> +			min_rpm = cpu_rpm;
> +		if (cpu_rpm > max_rpm)
> +			max_rpm = cpu_rpm;
> +	}
> +
> +	if (min_rpm == U8_MAX || max_rpm == 0)
>  		return -EINVAL;
>  
> -	min_rpm = fan_table->entries[0].cpu_rpm;
> -	max_rpm = fan_table->entries[fan_table->header.num_entries - 1].cpu_rpm;
>  	gpu_delta = fan_table->entries[0].gpu_rpm - fan_table->entries[0].cpu_rpm;
>  	priv->min_rpm = min_rpm;
>  	priv->max_rpm = max_rpm;
>