Message-ID: <3933560.kQq0lBPeGt@rafael.j.wysocki>
Date: Thu, 18 Dec 2025 21:39:43 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Linux ACPI <linux-acpi@...r.kernel.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
 Linux PCI <linux-pci@...r.kernel.org>, Bjorn Helgaas <helgaas@...nel.org>,
 Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
 Hans de Goede <hansg@...nel.org>,
 Mario Limonciello <mario.limonciello@....com>
Subject:
 [PATCH v1 6/8] ACPI: bus: Rework the handling of \_SB._OSC platform features

From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>

The current handling of \_SB._OSC is susceptible to problems with
setting error bits in the output buffer by mistake if the platform
firmware is supplied with a feature mask previously acknowledged
by the analogous _OSC call with OSC_QUERY_ENABLE set.  If that
happens, acpi_run_osc() will return an error and the kernel will
assume that it cannot control any of the features it has asked
for.  If an error bit has been set by mistake, however, the platform
firmware may expect the kernel to actually take over the control of
those features and nobody will take care of them going forward.

If the given feature mask has been already acknowledged once though,
the kernel may reasonably expect the _OSC evaluation to succeed and
acknowledge all of the features in the current mask again, but that
is not generally guaranteed to happen, so it is actually good to
verify the return buffer.  Still, it is sufficient to check the
feature bits in the return buffer for this purpose.

Namely, the OSC_INVALID_UUID_ERROR and OSC_INVALID_REVISION_ERROR bits
should not be set then because they were not set during the previous
_OSC evaluation that has acknowledged the feature mask.  Moreover,
if all of the feature bits that are set in the capabilities buffer
are also set in the return buffer, the OSC_CAPABILITIES_MASK_ERROR
should not be set either and the OSC_REQUEST_ERROR bit doesn't matter
even if set.  Thus if that is the case, the kernel may regard the
entire feature mask as acknowledged and take over the control of the
given features as per Section 6.2.12 of ACPI 6.6 [1].

If the feature masks in the capabilities buffer and in the return
buffer are different, the bits that are set in both masks may still
be regarded as acknowledged and the corresponding features may be
controlled by the kernel.

Introduce a new function carrying out an _OSC handshake along the
lines of the above description and make the \_SB._OSC handling code
use it to avoid failing in some cases in which it may succeed
regardless of platform firmware deficiencies.

Link: https://uefi.org/specs/ACPI/6.6/06_Device_Configuration.html#osc-operating-system-capabilities
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
---
 drivers/acpi/bus.c |  128 ++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 88 insertions(+), 40 deletions(-)

--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -311,6 +311,79 @@ out:
 }
 EXPORT_SYMBOL(acpi_run_osc);
 
+static int acpi_osc_handshake(acpi_handle handle, const char *uuid_str,
+			      int rev, struct acpi_buffer *cap)
+{
+	union acpi_object in_params[4], *out_obj;
+	size_t bufsize = cap->length / sizeof(u32);
+	struct acpi_object_list input;
+	struct acpi_buffer output;
+	u32 *capbuf, *retbuf, test;
+	guid_t guid;
+	int ret, i;
+
+	if (!cap || cap->length < 2 * sizeof(32) || guid_parse(uuid_str, &guid))
+		return -EINVAL;
+
+	/* First evaluate _OSC with OSC_QUERY_ENABLE set. */
+	capbuf = cap->pointer;
+	capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
+
+	ret = acpi_eval_osc(handle, &guid, rev, cap, in_params, &output);
+	if (ret)
+		return ret;
+
+	out_obj = output.pointer;
+	retbuf = (u32 *)out_obj->buffer.pointer;
+
+	if (acpi_osc_error_check(handle, &guid, rev, cap, retbuf)) {
+		ret = -ENODATA;
+		goto out;
+	}
+
+	/*
+	 * Clear the feature bits in the capabilities buffer that have not been
+	 * acknowledged and clear the return buffer.
+	 */
+	for (i = OSC_QUERY_DWORD + 1, test = 0; i < bufsize; i++) {
+		capbuf[i] &= retbuf[i];
+		test |= capbuf[i];
+		retbuf[i] = 0;
+	}
+	/*
+	 * If none of the feature bits have been acknowledged, there's nothing
+	 * more to do.
+	 */
+	if (!test)
+		goto out;
+
+	retbuf[OSC_QUERY_DWORD] = 0;
+	/*
+	 * Now evaluate _OSC again (directly) with OSC_QUERY_ENABLE clear and
+	 * the updated input and output buffers used before.
+	 */
+	capbuf[OSC_QUERY_DWORD] = 0;
+	/* Reuse in_params[] populated by acpi_eval_osc(). */
+	input.pointer = in_params;
+	input.count = 4;
+
+	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_OSC", &input, &output))) {
+		ret = -ENODATA;
+		goto out;
+	}
+
+	/* Clear the feature bits that have not been acknowledged in capbuf[]. */
+	for (i = OSC_QUERY_DWORD + 1; i < bufsize; i++)
+		capbuf[i] &= retbuf[i];
+
+	/* Check _OSC errors to print debug messages if any. */
+	acpi_osc_error_check(handle, &guid, rev, cap, retbuf);
+
+out:
+	ACPI_FREE(out_obj);
+	return ret;
+}
+
 bool osc_sb_apei_support_acked;
 
 /*
@@ -342,19 +415,16 @@ EXPORT_SYMBOL_GPL(osc_sb_native_usb4_sup
 
 bool osc_sb_cppc2_support_acked;
 
-static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
 static void acpi_bus_osc_negotiate_platform_control(void)
 {
-	u32 capbuf[2], *capbuf_ret;
-	struct acpi_osc_context context = {
-		.uuid_str = sb_uuid_str,
-		.rev = 1,
-		.cap.length = 8,
-		.cap.pointer = capbuf,
+	static const u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
+	u32 capbuf[2];
+	struct acpi_buffer cap = {
+		.pointer = capbuf,
+		.length = sizeof(capbuf),
 	};
 	acpi_handle handle;
 
-	capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
 	capbuf[OSC_SUPPORT_DWORD] = OSC_SB_PR3_SUPPORT; /* _PR3 is in use */
 	if (IS_ENABLED(CONFIG_ACPI_PROCESSOR_AGGREGATOR))
 		capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PAD_SUPPORT;
@@ -400,43 +470,21 @@ static void acpi_bus_osc_negotiate_platf
 	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
 		return;
 
-	if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
-		return;
-
-	capbuf_ret = context.ret.pointer;
-	if (context.ret.length <= OSC_SUPPORT_DWORD) {
-		kfree(context.ret.pointer);
-		return;
-	}
-
-	/*
-	 * Now run _OSC again with query flag clear and with the caps
-	 * supported by both the OS and the platform.
-	 */
-	capbuf[OSC_QUERY_DWORD] = 0;
-	capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD];
-	kfree(context.ret.pointer);
-
-	if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
+	if (acpi_osc_handshake(handle, sb_uuid_str, 1, &cap))
 		return;
 
-	capbuf_ret = context.ret.pointer;
-	if (context.ret.length > OSC_SUPPORT_DWORD) {
 #ifdef CONFIG_ACPI_CPPC_LIB
-		osc_sb_cppc2_support_acked = capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_CPCV2_SUPPORT;
+	osc_sb_cppc2_support_acked = capbuf[OSC_SUPPORT_DWORD] & OSC_SB_CPCV2_SUPPORT;
 #endif
 
-		osc_sb_apei_support_acked =
-			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
-		osc_pc_lpi_support_confirmed =
-			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
-		osc_sb_native_usb4_support_confirmed =
-			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT;
-		osc_cpc_flexible_adr_space_confirmed =
-			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_CPC_FLEXIBLE_ADR_SPACE;
-	}
-
-	kfree(context.ret.pointer);
+	osc_sb_apei_support_acked =
+			capbuf[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
+	osc_pc_lpi_support_confirmed =
+			capbuf[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
+	osc_sb_native_usb4_support_confirmed =
+			capbuf[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT;
+	osc_cpc_flexible_adr_space_confirmed =
+			capbuf[OSC_SUPPORT_DWORD] & OSC_SB_CPC_FLEXIBLE_ADR_SPACE;
 }
 
 /*

Powered by blists - more mailing lists