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"
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;
-- 
2.53.0

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;
>

On Wed, Apr 01, 2026 at 02:23:31PM +0300, Ilpo Järvinen wrote:
>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?
>

Yes, for my board (and for the several recent additions in Victus S list)
the second field in the fan table header happened to match the number of
entries in the table, making the previous implementation appear correct.

However on board 8D87 (and potentially others), the second field contains
a larger value which causes the following check to fail:

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))

After reverse engineering the Windows implementation, I found that the
official software ignores this second field entirely. Instead it uses
the "stop-at-null" approach implemented in this patch.

>-- 
> i.