Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5477c090-1d8d-f5a6-a204-45cb5acb6e20@redhat.com>
Date:   Fri, 10 Jun 2022 21:50:07 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH v3 1/1] Resolve WMI query failures on some devices
Content-Language: en-US
To:     Andy Shevchenko <andy.shevchenko@gmail.com>,
        Jorge Lopez <jorgealtxwork@gmail.com>
Cc:     balalic.enver@gmail.com,
        Platform Driver <platform-driver-x86@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Mark Gross <markgross@kernel.org>
References: <20220608212923.8585-1-jorge.lopez2@hp.com>
 <20220608212923.8585-2-jorge.lopez2@hp.com>
 <CAHp75VfPoranKhsHJo=8jJXXhdvqn4-x2E2UJa-Yxb_=tO3QDg@mail.gmail.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <CAHp75VfPoranKhsHJo=8jJXXhdvqn4-x2E2UJa-Yxb_=tO3QDg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 6/9/22 18:29, Andy Shevchenko wrote:
> On Wed, Jun 8, 2022 at 11:32 PM Jorge Lopez <jorgealtxwork@gmail.com> 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.
> 
> Perfect!
> 
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Also needs a Fixes tag:
> 
> Fixes: 4b4967cbd268 ("platform/x86: hp-wmi: Changing bios_args.data to
> be dynamically allocated")
> 
> (I believe Hans can help you and add it on the fly)

Right, I've just merged this into my review-hans branch,
with the Fixes tag added. I'll also include this in my
first fixes pull-req for the 5.19 cycle.

Regards,

Hans


> 
>> Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
>>
>> ---
>> Based on the latest platform-drivers-x86.git/for-next
>> ---
>>  drivers/platform/x86/hp-wmi.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
>> index 0e9a25b56e0e..d3540dd62d06 100644
>> --- a/drivers/platform/x86/hp-wmi.c
>> +++ b/drivers/platform/x86/hp-wmi.c
>> @@ -290,14 +290,16 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
>>         struct bios_return *bios_return;
>>         union acpi_object *obj = NULL;
>>         struct bios_args *args = NULL;
>> -       int mid, actual_outsize, ret;
>> +       int mid, actual_insize, actual_outsize;
>>         size_t bios_args_size;
>> +       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
>>
> 
>