[PATCH v5 4/4] Changing bios_args.data to be dynamically allocated

From: Jorge Lopez <jorgealtxwork@gmail.com>
To: platform-driver-x86@vger.kernel.org
Subject: [PATCH v5 4/4] Changing bios_args.data to be dynamically allocated
Date: Thu, 10 Mar 2022 15:08:53 -0600	[thread overview]
Message-ID: <20220310210853.28367-5-jorge.lopez2@hp.com> (raw)
In-Reply-To: <20220310210853.28367-1-jorge.lopez2@hp.com>

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@hp.com>

---
Based on the latest platform-drivers-x86.git/for-next
---
 drivers/platform/x86/hp-wmi.c | 64 +++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 22 deletions(-)

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index e76bd4bef6b5..cc5c4f637328 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[];
 };
 
 enum hp_wmi_commandtype {
@@ -268,34 +273,40 @@ 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;
+	struct bios_args *args = NULL;
+	size_t bios_args_size = struct_size(args, data, insize);
+	
+	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, flex_array_size(args, data, 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,9 +321,17 @@ 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)
+	if (!outsize) {
+		ret = 0;
 		goto out_free;
+	}
 
 	actual_outsize = min(outsize, (int)(obj->buffer.length - sizeof(*bios_return)));
 	memcpy(buffer, obj->buffer.pointer + sizeof(*bios_return), actual_outsize);
@@ -320,6 +339,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

