Re: [PATCH v5 3/4] Fix 0x05 error code reported by several WMI calls

From: Hans de Goede <hdegoede@redhat.com>
To: Jorge Lopez <jorgealtxwork@gmail.com>,
	platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH v5 3/4] Fix 0x05 error code reported by several WMI calls
Date: Mon, 14 Mar 2022 11:44:44 +0100	[thread overview]
Message-ID: <c9448644-917a-76c1-b9c8-ef3aa08c57bd@redhat.com> (raw)
In-Reply-To: <20220310210853.28367-4-jorge.lopez2@hp.com>

Hi,

On 3/10/22 22:08, Jorge Lopez wrote:
> Several WMI queries leverage hp_wmi_read_int function to read their
> data. hp_wmi_read_int function was corrected in a previous patch.
> Now, this function invokes hp_wmi_perform_query with input parameter
> of size zero and the output buffer of size 4.
> 
> WMI commands calling hp_wmi_perform_query with input buffer size value
> of zero are listed below.
> 
> HPWMI_DISPLAY_QUERY
> HPWMI_HDDTEMP_QUERY
> HPWMI_ALS_QUERY
> HPWMI_HARDWARE_QUERY
> HPWMI_WIRELESS_QUERY
> HPWMI_BIOS_QUERY
> HPWMI_FEATURE_QUERY
> HPWMI_HOTKEY_QUERY
> HPWMI_FEATURE2_QUERY
> HPWMI_WIRELESS2_QUERY
> HPWMI_POSTCODEERROR_QUERY
> HPWMI_THERMAL_PROFILE_QUERY
> HPWMI_FAN_SPEED_MAX_GET_QUERY
> 
> Invoking those WMI commands with an input buffer size greater
> than zero will cause error 0x05 to be returned.
> 
> All WMI commands executed by the driver were reviewed and changes
> were made to ensure the expected input and output buffer size match
> the WMI specification.
> 
> Changes were validated on a HP ZBook Workstation notebook,
> HP EliteBook x360, and HP EliteBook 850 G8.  Additional
> validation was included in the test process to ensure no other
> commands were incorrectly handled.
> 
> Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans

> 
> ---
> Based on the latest platform-drivers-x86.git/for-next
> ---
>  drivers/platform/x86/hp-wmi.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
> index e9aa05c26a40..e76bd4bef6b5 100644
> --- a/drivers/platform/x86/hp-wmi.c
> +++ b/drivers/platform/x86/hp-wmi.c
> @@ -329,7 +329,7 @@ static int hp_wmi_get_fan_speed(int fan)
>  	char fan_data[4] = { fan, 0, 0, 0 };
>  
>  	int ret = hp_wmi_perform_query(HPWMI_FAN_SPEED_GET_QUERY, HPWMI_GM,
> -				       &fan_data, sizeof(fan_data),
> +				       &fan_data, sizeof(char),
>  				       sizeof(fan_data));
>  
>  	if (ret != 0)
> @@ -399,7 +399,7 @@ static int omen_thermal_profile_set(int mode)
>  		return -EINVAL;
>  
>  	ret = hp_wmi_perform_query(HPWMI_SET_PERFORMANCE_MODE, HPWMI_GM,
> -				   &buffer, sizeof(buffer), sizeof(buffer));
> +				   &buffer, sizeof(buffer), 0);
>  
>  	if (ret)
>  		return ret < 0 ? ret : -EINVAL;
> @@ -436,7 +436,7 @@ static int hp_wmi_fan_speed_max_set(int enabled)
>  	int ret;
>  
>  	ret = hp_wmi_perform_query(HPWMI_FAN_SPEED_MAX_SET_QUERY, HPWMI_GM,
> -				   &enabled, sizeof(enabled), sizeof(enabled));
> +				   &enabled, sizeof(enabled), 0);
>  
>  	if (ret)
>  		return ret < 0 ? ret : -EINVAL;
> @@ -449,7 +449,7 @@ static int hp_wmi_fan_speed_max_get(void)
>  	int val = 0, ret;
>  
>  	ret = hp_wmi_perform_query(HPWMI_FAN_SPEED_MAX_GET_QUERY, HPWMI_GM,
> -				   &val, sizeof(val), sizeof(val));
> +				   &val, 0, sizeof(val));
>  
>  	if (ret)
>  		return ret < 0 ? ret : -EINVAL;
> @@ -461,7 +461,7 @@ static int __init hp_wmi_bios_2008_later(void)
>  {
>  	int state = 0;
>  	int ret = hp_wmi_perform_query(HPWMI_FEATURE_QUERY, HPWMI_READ, &state,
> -				       sizeof(state), sizeof(state));
> +				       0, sizeof(state));
>  	if (!ret)
>  		return 1;
>  
> @@ -472,7 +472,7 @@ static int __init hp_wmi_bios_2009_later(void)
>  {
>  	u8 state[128];
>  	int ret = hp_wmi_perform_query(HPWMI_FEATURE2_QUERY, HPWMI_READ, &state,
> -				       sizeof(state), sizeof(state));
> +				       0, sizeof(state));
>  	if (!ret)
>  		return 1;
>  
> @@ -550,7 +550,7 @@ static int hp_wmi_rfkill2_refresh(void)
>  	int err, i;
>  
>  	err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_READ, &state,
> -				   sizeof(state), sizeof(state));
> +				   0, sizeof(state));
>  	if (err)
>  		return err;
>  
> @@ -639,7 +639,7 @@ static ssize_t als_store(struct device *dev, struct device_attribute *attr,
>  		return ret;
>  
>  	ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, HPWMI_WRITE, &tmp,
> -				       sizeof(tmp), sizeof(tmp));
> +				       sizeof(tmp), 0);
>  	if (ret)
>  		return ret < 0 ? ret : -EINVAL;
>  
> @@ -660,9 +660,9 @@ static ssize_t postcode_store(struct device *dev, struct device_attribute *attr,
>  	if (clear == false)
>  		return -EINVAL;
>  
> -	/* Clear the POST error code. It is kept until until cleared. */
> +	/* Clear the POST error code. It is kept until cleared. */
>  	ret = hp_wmi_perform_query(HPWMI_POSTCODEERROR_QUERY, HPWMI_WRITE, &tmp,
> -				       sizeof(tmp), sizeof(tmp));
> +				       sizeof(tmp), 0);
>  	if (ret)
>  		return ret < 0 ? ret : -EINVAL;
>  
> @@ -952,7 +952,7 @@ static int __init hp_wmi_rfkill2_setup(struct platform_device *device)
>  	int err, i;
>  
>  	err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_READ, &state,
> -				   sizeof(state), sizeof(state));
> +				   0, sizeof(state));
>  	if (err)
>  		return err < 0 ? err : -EINVAL;
>  