On Wed, Jun 8, 2022 at 8:06 AM Jorge Lopez <jorgealtxwork@xxxxxxxxx> wrote:
>
> WMI queries fail on some devices where the ACPI method HWMC
> unconditionally attempts to create Fields beyond the buffer
> if the buffer is too small, this breaks essential features
> such as power profiles:
>
> CreateByteField (Arg1, 0x10, D008)
> CreateByteField (Arg1, 0x11, D009)
> CreateByteField (Arg1, 0x12, D010)
> CreateDWordField (Arg1, 0x10, D032)
> CreateField (Arg1, 0x80, 0x0400, D128)
>
> In cases where args->data had zero length, ACPI BIOS Error
> (bug): AE_AML_BUFFER_LIMIT, Field [D008] at bit
> offset/length 128/8 exceeds size of target Buffer (128 bits)
> (20211217/dsopcode-198) was obtained.
>
> ACPI BIOS Error (bug): AE_AML_BUFFER_LIMIT, Field [D009] at bit
> offset/length 136/8 exceeds size of target Buffer (136bits)
> (20211217/dsopcode-198)
>
> The original code created a buffer size of 128 bytes regardless if
> the WMI call required a smaller buffer or not. This particular
> behavior occurs in older BIOS and reproduced in OMEN laptops. Newer
> BIOS handles buffer sizes properly and meets the latest specification
> requirements. This is the reason why testing with a dynamically
> allocated buffer did not uncover any failures with the test systems at
> hand.
>
> This patch was tested on several OMEN, Elite, and Zbooks. It was
> confirmed the patch resolves HPWMI_FAN GET/SET calls in an OMEN
> Laptop 15-ek0xxx. No problems were reported when testing on several Elite
> and Zbooks notebooks.
I am in general fine with the change, only a nit-pick below.
Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> Signed-off-by: Jorge Lopez <jorge.lopez2@xxxxxx>
>
> ---
> Based on the latest platform-drivers-x86.git/for-next
> ---
> drivers/platform/x86/hp-wmi.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
> index 0e9a25b56e0e..7bcfa07cc6ab 100644
> --- a/drivers/platform/x86/hp-wmi.c
> +++ b/drivers/platform/x86/hp-wmi.c
> @@ -292,12 +292,14 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
> struct bios_args *args = NULL;
> int mid, actual_outsize, ret;
> size_t bios_args_size;
> + int actual_insize;
We already have above similar variables, I would either put like this
int mid, actual_inzise, actual_outsize, ret;
or this (my personal preference):
int mid, actual_insize, actual_outsize;
...
int ret;
> mid = encode_outsize_for_pvsz(outsize);
> if (WARN_ON(mid < 0))
> return mid;
>
> - bios_args_size = struct_size(args, data, insize);
> + actual_insize = max(insize, 128);
> + bios_args_size = struct_size(args, data, actual_insize);
> args = kmalloc(bios_args_size, GFP_KERNEL);
> if (!args)
> return -ENOMEM;
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
