Hi,
On 3/7/22 23:12, Limonciello, Mario wrote:
> [Public]
>
>
>
>> -----Original Message-----
>> From: Jorge Lopez <jorgealtxwork@xxxxxxxxx>
>> Sent: Monday, March 7, 2022 16:10
>> To: platform-driver-x86@xxxxxxxxxxxxxxx
>> Subject: [PATCH v4 3/3] Changing bios_args.data to be dynamically allocated
>>
>> The purpose of this patch is to remove 128 bytes buffer limitation
>> imposed in bios_args structure.
>>
>> A limiting factor discovered during this investigation was the struct
>> bios_args.data size restriction. The data member size limits all possible
>> WMI commands to those requiring buffer size of 128 bytes or less.
>> Several WMI commands and queries require a buffer size larger than 128
>> bytes hence limiting current and feature supported by the driver.
>> It is for this reason, struct bios_args.data changed and is dynamically
>> allocated. hp_wmi_perform_query function changed to handle the memory
>> allocation and release of any required buffer size.
>>
>> All 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@xxxxxx>
>>
>> ---
>> Based on the latest platform-drivers-x86.git/for-next
>
> Was this meant as "changes from v3->v4"? Or it's just a comment?
>
>> ---
>
> No need for double --. Just put everything commentary below the one --.
The second cut-line gets added by git format-patch, there is nothing
which can be done about that.
Regards,
Hans
>
>> drivers/platform/x86/hp-wmi.c | 59 ++++++++++++++++++++++-------------
>> 1 file changed, 38 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
>> index a0aba7db8a1c..a04723fdea60 100644
>> --- a/drivers/platform/x86/hp-wmi.c
>> +++ b/drivers/platform/x86/hp-wmi.c
>> @@ -82,12 +82,17 @@ enum hp_wmi_event_ids {
>> HPWMI_BATTERY_CHARGE_PERIOD = 0x10,
>> };
>>
>> +/**
>> + * struct bios_args buffer is dynamically allocated. New WMI command
>> types
>> + * were introduced that exceeds 128-byte data size. Changes to handle
>> + * the data size allocation scheme were kept in hp_wmi_perform_qurey
>> function.
>> + */
>> struct bios_args {
>> u32 signature;
>> u32 command;
>> u32 commandtype;
>> u32 datasize;
>> - u8 data[128];
>> + u8 data[0];
>> };
>>
>> enum hp_wmi_commandtype {
>> @@ -268,34 +273,39 @@ static int hp_wmi_perform_query(int query, enum
>> hp_wmi_command command,
>> int mid;
>> struct bios_return *bios_return;
>> int actual_outsize;
>> - union acpi_object *obj;
>> - struct bios_args args = {
>> - .signature = 0x55434553,
>> - .command = command,
>> - .commandtype = query,
>> - .datasize = insize,
>> - .data = { 0 },
>> - };
>> - struct acpi_buffer input = { sizeof(struct bios_args), &args };
>> + union acpi_object *obj = NULL;
>> + size_t bios_args_size = sizeof(struct bios_args) + insize;
>> + struct bios_args *args = NULL;
>> + struct acpi_buffer input;
>> struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>> int ret = 0;
>>
>> - mid = encode_outsize_for_pvsz(outsize);
>> - if (WARN_ON(mid < 0))
>> - return mid;
>> + args = kmalloc(bios_args_size, GFP_KERNEL);
>> + if (!args)
>> + return -ENOMEM;
>>
>> - if (WARN_ON(insize > sizeof(args.data)))
>> - return -EINVAL;
>> - memcpy(&args.data[0], buffer, insize);
>> + input.length = bios_args_size;
>> + input.pointer = args;
>>
>> - wmi_evaluate_method(HPWMI_BIOS_GUID, 0, mid, &input,
>> &output);
>> + mid = encode_outsize_for_pvsz(outsize);
>> + if (WARN_ON(mid < 0)) {
>> + ret = mid;
>> + goto out_free;
>> + }
>>
>> - obj = output.pointer;
>> + memcpy(args->data, buffer, insize);
>>
>> - if (!obj)
>> - return -EINVAL;
>> + args->signature = 0x55434553;
>> + args->command = command;
>> + args->commandtype = query;
>> + args->datasize = insize;
>>
>> - if (obj->type != ACPI_TYPE_BUFFER) {
>> + ret = wmi_evaluate_method(HPWMI_BIOS_GUID, 0, mid, &input,
>> &output);
>> + if (ret)
>> + goto out_free;
>> +
>> + obj = output.pointer;
>> + if (!obj) {
>> ret = -EINVAL;
>> goto out_free;
>> }
>> @@ -310,6 +320,12 @@ static int hp_wmi_perform_query(int query, enum
>> hp_wmi_command command,
>> goto out_free;
>> }
>>
>> + if (obj->type != ACPI_TYPE_BUFFER) {
>> + pr_warn("query 0x%x returned an invalid object 0x%x\n",
>> query, ret);
>> + ret = -EINVAL;
>> + goto out_free;
>> + }
>> +
>> /* Ignore output data of zero size */
>> if (!outsize)
>> goto out_free;
>> @@ -320,6 +336,7 @@ static int hp_wmi_perform_query(int query, enum
>> hp_wmi_command command,
>>
>> out_free:
>> kfree(obj);
>> + kfree(args);
>> return ret;
>> }
>>
>> --
>> 2.25.1
>
