[PATCH 0/4] ACPI: Serialize EC queries, UCSI and ideapad methods

[PATCH 0/4] ACPI: Serialize EC queries, UCSI and ideapad methods

[Ali Ahmet Memiş]

This goes on top of [1] and uses its exclusive helpers for the other
users of the shared EC mutex in bko221065: the _Qxx methods, the UCSI
_DSM and the ideapad-laptop methods, as discussed in [2].

Patches 3 and 4 depend on patch 1 and on [1], so it is probably easiest
to take the whole set through the ACPI tree.

The ideapad part is only build-tested, so it would be great if someone
with one of the bko221065 machines could try both series together.

[1] https://lore.kernel.org/all/20261014132703.93355-1-aliamemis@disroot.org/
[2] https://lore.kernel.org/all/8be1a245-6061-4f86-9223-0da18d5e423a@oss.qualcomm.com/

Ali Ahmet Memiş (4):
  ACPI: utils: Add acpi_evaluate_dsm_exclusive()
  ACPI: EC: Evaluate _Qxx methods exclusively
  usb: typec: ucsi: acpi: Evaluate _DSM exclusively
  platform/x86: ideapad-laptop: Evaluate ACPI methods exclusively

 drivers/acpi/ec.c                            |  7 ++-
 drivers/acpi/utils.c                         | 54 +++++++++++++-------
 drivers/platform/x86/lenovo/ideapad-laptop.c | 21 ++++++--
 drivers/usb/typec/ucsi/ucsi_acpi.c           |  8 ++-
 include/acpi/acpi_bus.h                      |  3 ++
 include/linux/acpi.h                         |  8 +++
 6 files changed, 76 insertions(+), 25 deletions(-)


base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
prerequisite-patch-id: 4bd123d6caded06beb9a977d502370df046f82aa
prerequisite-patch-id: 19de145660fffa3651defed4e6d536cb78418940
prerequisite-patch-id: 01d8992be9ebc2b28dd3986fe06269eb2fe959f5
-- 
2.55.0

[PATCH 1/4] ACPI: utils: Add acpi_evaluate_dsm_exclusive()

[Ali Ahmet Memiş]

Add an exclusive variant of acpi_evaluate_dsm() for _DSM methods that
are known to break when interleaved with other AML.

Signed-off-by: Ali Ahmet Memiş <aliamemis@disroot.org>
---
 drivers/acpi/utils.c    | 54 +++++++++++++++++++++++++++--------------
 include/acpi/acpi_bus.h |  3 +++
 include/linux/acpi.h    |  8 ++++++
 3 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index f28680424f14..32689c901564 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -799,23 +799,9 @@ acpi_status acpi_evaluate_reg(acpi_handle handle, u8 space_id, u32 function)
 }
 EXPORT_SYMBOL(acpi_evaluate_reg);
 
-/**
- * acpi_evaluate_dsm - evaluate device's _DSM method
- * @handle: ACPI device handle
- * @guid: GUID of requested functions, should be 16 bytes
- * @rev: revision number of requested function
- * @func: requested function number
- * @argv4: the function specific parameter
- *
- * Evaluate device's _DSM method with specified GUID, revision id and
- * function number. Caller needs to free the returned object.
- *
- * Though ACPI defines the fourth parameter for _DSM should be a package,
- * some old BIOSes do expect a buffer or an integer etc.
- */
-union acpi_object *
-acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 func,
-		  union acpi_object *argv4)
+static union acpi_object *
+__acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 func,
+		    union acpi_object *argv4, bool exclusive)
 {
 	acpi_status ret;
 	struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL};
@@ -840,7 +826,10 @@ acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 func,
 		params[3].package.elements = NULL;
 	}
 
-	ret = acpi_evaluate_object(handle, "_DSM", &input, &buf);
+	if (exclusive)
+		ret = acpi_evaluate_object_exclusive(handle, "_DSM", &input, &buf);
+	else
+		ret = acpi_evaluate_object(handle, "_DSM", &input, &buf);
 	if (ACPI_SUCCESS(ret))
 		return (union acpi_object *)buf.pointer;
 
@@ -851,8 +840,37 @@ acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 func,
 
 	return NULL;
 }
+
+/**
+ * acpi_evaluate_dsm - evaluate device's _DSM method
+ * @handle: ACPI device handle
+ * @guid: GUID of requested functions, should be 16 bytes
+ * @rev: revision number of requested function
+ * @func: requested function number
+ * @argv4: the function specific parameter
+ *
+ * Evaluate device's _DSM method with specified GUID, revision id and
+ * function number. Caller needs to free the returned object.
+ *
+ * Though ACPI defines the fourth parameter for _DSM should be a package,
+ * some old BIOSes do expect a buffer or an integer etc.
+ */
+union acpi_object *
+acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 func,
+		  union acpi_object *argv4)
+{
+	return __acpi_evaluate_dsm(handle, guid, rev, func, argv4, false);
+}
 EXPORT_SYMBOL(acpi_evaluate_dsm);
 
+union acpi_object *
+acpi_evaluate_dsm_exclusive(acpi_handle handle, const guid_t *guid, u64 rev,
+			    u64 func, union acpi_object *argv4)
+{
+	return __acpi_evaluate_dsm(handle, guid, rev, func, argv4, true);
+}
+EXPORT_SYMBOL_GPL(acpi_evaluate_dsm_exclusive);
+
 /**
  * acpi_check_dsm - check if _DSM method supports requested functions.
  * @handle: ACPI device handle
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e7a671dba624..e9859fdf76e8 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -66,6 +66,9 @@ bool acpi_dock_match(acpi_handle handle);
 bool acpi_check_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 funcs);
 union acpi_object *acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid,
 			u64 rev, u64 func, union acpi_object *argv4);
+union acpi_object *acpi_evaluate_dsm_exclusive(acpi_handle handle,
+			const guid_t *guid, u64 rev, u64 func,
+			union acpi_object *argv4);
 #ifdef CONFIG_ACPI
 bool
 acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index ddacac812094..6a798d5bde5c 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1042,6 +1042,14 @@ static inline union acpi_object *acpi_evaluate_dsm(acpi_handle handle,
 	return NULL;
 }
 
+static inline union acpi_object *acpi_evaluate_dsm_exclusive(acpi_handle handle,
+							     const guid_t *guid,
+							     u64 rev, u64 func,
+							     union acpi_object *argv4)
+{
+	return NULL;
+}
+
 static inline union acpi_object *acpi_evaluate_dsm_typed(acpi_handle handle,
 							 const guid_t *guid,
 							 u64 rev, u64 func,
-- 
2.55.0

[PATCH 2/4] ACPI: EC: Evaluate _Qxx methods exclusively

[Ali Ahmet Memiş]

On some Lenovo IdeaPad and Yoga models, _Q0A, _Q0B and the UCSI
notification query _Q16 take the same AML mutex as the battery, UCSI and
ideapad methods. Making those exclusive does not help as long as the
_Qxx methods still run interleaved with them.

Evaluate _Qxx with acpi_evaluate_object_exclusive().

Link: https://bugzilla.kernel.org/show_bug.cgi?id=221065
Signed-off-by: Ali Ahmet Memiş <aliamemis@disroot.org>
---
 drivers/acpi/ec.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index a89f10256dbb..6cec5a319ec2 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1154,10 +1154,15 @@ static void acpi_ec_event_processor(struct work_struct *work)
 
 	ec_dbg_evt("Query(0x%02x) started", handler->query_bit);
 
+	/*
+	 * Evaluate _Qxx exclusively, as on some laptops they take the same
+	 * AML mutex as the battery, UCSI and platform methods, which then
+	 * time out waiting for it.
+	 */
 	if (handler->func)
 		handler->func(handler->data);
 	else if (handler->handle)
-		acpi_evaluate_object(handler->handle, NULL, NULL, NULL);
+		acpi_evaluate_object_exclusive(handler->handle, NULL, NULL, NULL);
 
 	ec_dbg_evt("Query(0x%02x) stopped", handler->query_bit);
 
-- 
2.55.0

[PATCH 3/4] usb: typec: ucsi: acpi: Evaluate _DSM exclusively

[Ali Ahmet Memiş]

On some Lenovo IdeaPad and Yoga models, the UCSI _DSM accesses the EC 25
times through helpers that take a shared AML mutex with a 1 s timeout
and return 0 when it times out, so UCSI commands fail under contention
with the battery and ideapad methods.

Evaluate the _DSM with acpi_evaluate_dsm_exclusive().

Link: https://bugzilla.kernel.org/show_bug.cgi?id=221065
Signed-off-by: Ali Ahmet Memiş <aliamemis@disroot.org>
---
 drivers/usb/typec/ucsi/ucsi_acpi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index 5697ccc24ec3..0b9aaf317d2b 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -30,8 +30,12 @@ static int ucsi_acpi_dsm(struct ucsi_acpi *ua, int func)
 {
 	union acpi_object *obj;
 
-	obj = acpi_evaluate_dsm(ACPI_HANDLE(ua->dev), &ua->guid, 1, func,
-				NULL);
+	/*
+	 * On some Lenovo laptops the _DSM times out on the AML mutex it
+	 * shares with the battery and platform methods.
+	 */
+	obj = acpi_evaluate_dsm_exclusive(ACPI_HANDLE(ua->dev), &ua->guid, 1,
+					  func, NULL);
 	if (!obj) {
 		dev_err(ua->dev, "%s: failed to evaluate _DSM %d\n",
 			__func__, func);
-- 
2.55.0

[PATCH 4/4] platform/x86: ideapad-laptop: Evaluate ACPI methods exclusively

[Ali Ahmet Memiş]

On some Lenovo IdeaPad and Yoga models, GBMD, SBMC, SALS and DYTC hold a
shared AML mutex while waiting for the EC to finish a command, long
enough for the UCSI and _Qxx methods waiting on it to time out.

Evaluate all methods with the exclusive helpers.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=221065
Signed-off-by: Ali Ahmet Memiş <aliamemis@disroot.org>
---
 drivers/platform/x86/lenovo/ideapad-laptop.c | 21 ++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
index 8213524504ee..9fb842e6383d 100644
--- a/drivers/platform/x86/lenovo/ideapad-laptop.c
+++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
@@ -297,12 +297,18 @@ static void ideapad_shared_exit(struct ideapad_private *priv)
 #define IDEAPAD_EC_POLL_MIN_US 150
 #define IDEAPAD_EC_POLL_MAX_US 300
 
+/*
+ * Some of these methods hold an AML mutex shared with the UCSI and EC
+ * query methods for long enough for those to time out, so evaluate all
+ * of them exclusively.
+ */
 static int eval_int(acpi_handle handle, const char *name, unsigned long *res)
 {
 	unsigned long long result;
 	acpi_status status;
 
-	status = acpi_evaluate_integer(handle, (char *)name, NULL, &result);
+	status = acpi_evaluate_integer_exclusive(handle, (char *)name, NULL,
+						 &result);
 	if (ACPI_FAILURE(status))
 		return -EIO;
 
@@ -324,7 +330,8 @@ static int eval_int_with_arg(acpi_handle handle, const char *name, unsigned long
 	in_obj.type = ACPI_TYPE_INTEGER;
 	in_obj.integer.value = arg;
 
-	status = acpi_evaluate_integer(handle, (char *)name, &params, &result);
+	status = acpi_evaluate_integer_exclusive(handle, (char *)name, &params,
+						 &result);
 	if (ACPI_FAILURE(status))
 		return -EIO;
 
@@ -336,7 +343,13 @@ static int eval_int_with_arg(acpi_handle handle, const char *name, unsigned long
 
 static int exec_simple_method(acpi_handle handle, const char *name, unsigned long arg)
 {
-	acpi_status status = acpi_execute_simple_method(handle, (char *)name, arg);
+	union acpi_object in_obj = { .type = ACPI_TYPE_INTEGER };
+	struct acpi_object_list params = { .count = 1, .pointer = &in_obj };
+	acpi_status status;
+
+	in_obj.integer.value = arg;
+	status = acpi_evaluate_object_exclusive(handle, (char *)name, &params,
+						NULL);
 
 	return ACPI_FAILURE(status) ? -EIO : 0;
 }
@@ -394,7 +407,7 @@ static int eval_vpcw(acpi_handle handle, unsigned long cmd, unsigned long data)
 	in_obj[1].type = ACPI_TYPE_INTEGER;
 	in_obj[1].integer.value = data;
 
-	status = acpi_evaluate_object(handle, "VPCW", &params, NULL);
+	status = acpi_evaluate_object_exclusive(handle, "VPCW", &params, NULL);
 	if (ACPI_FAILURE(status))
 		return -EIO;
 
-- 
2.55.0