All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jorge Lopez <jorgealtxwork@gmail.com>
To: platform-driver-x86@vger.kernel.org
Subject: [PATCH v2 3/3] Changing bios_args.data to be dynamically allocated
Date: Fri, 18 Feb 2022 10:09:07 -0600	[thread overview]
Message-ID: <20220218160907.3422-4-jorge.lopez2@hp.com> (raw)
In-Reply-To: <20220218160907.3422-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 ZBook Workstation notebook. Additional
validation was included to ensure no other commands were failing or
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 | 67 +++++++++++++++++++++++------------
 1 file changed, 44 insertions(+), 23 deletions(-)

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index de715687021a..11c8e9b6e64a 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -83,12 +83,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 {
@@ -258,39 +263,47 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
 	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 };
+	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 in_free;
+	}
 
-	obj = output.pointer;
+	/* Avoid unnecessary copy to the data buffer if input buffer size is zero */
+	if (insize > 0)
+		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);
+
+	obj = output.pointer;
+	if (!obj) {
+		pr_warn("query 0x%x returned a null obj 0x%x\n", query, ret);
 		ret = -EINVAL;
-		goto out_free;
+		goto in_free;
 	}
 
-	bios_return = (struct bios_return *)obj->buffer.pointer;
-	ret = bios_return->return_code;
+	if (!ret && obj->type == ACPI_TYPE_BUFFER) {
+		bios_return = (struct bios_return *)obj->buffer.pointer;
+		ret = bios_return->return_code;
+	}
 
 	if (ret) {
 		if (ret != HPWMI_RET_UNKNOWN_COMMAND &&
@@ -299,6 +312,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;
@@ -309,6 +328,8 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
 
 out_free:
 	kfree(obj);
+in_free:
+	kfree(args);
 	return ret;
 }
 
-- 
2.25.1


  parent reply	other threads:[~2022-02-18 16:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-18 16:09 [PATCH v2 0/3] Fix SW_TABLET_MODE detection method Jorge Lopez
2022-02-18 16:09 ` [PATCH v2 1/3] " Jorge Lopez
2022-02-22 16:13   ` Hans de Goede
2022-02-18 16:09 ` [PATCH v2 2/3] Fix 0x05 error code reported by several WMI calls Jorge Lopez
2022-02-22 16:22   ` Hans de Goede
2022-02-18 16:09 ` Jorge Lopez [this message]
2022-02-22 17:14   ` [PATCH v2 3/3] Changing bios_args.data to be dynamically allocated Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220218160907.3422-4-jorge.lopez2@hp.com \
    --to=jorgealtxwork@gmail.com \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.