[PATCH v1 0/2] ACPI: bus: Adjustments related to \_SB._OSC

[PATCH v1 0/2] ACPI: bus: Adjustments related to \_SB._OSC

[Rafael J. Wysocki]

Hi All,

The first patch works around a platform firmware issue causing
acpi_run_osc() to return an error when run with "query enable" bit
clear after negotiating the supported feature mask.

The second one is a cleanup on top of the first one.

Thanks!

[PATCH v1 1/2] ACPI: bus: Adjust error handling in acpi_run_osc()

[Rafael J. Wysocki]

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

Some platform firmware incorrectly sets the OSC_CAPABILITIES_MASK_ERROR
bit in its _OSC return buffer even if no support bits have been actually
masked, which causes acpi_run_osc() to return an error when executed
with OSC_QUERY_ENABLE clear in the OC capabilities buffer.  As a result,
the OS assumes that the _OSC evaluation has failed and the platform has
not acknowledged any capabilities, while the platform assumes that it
actually has acknowledged some of them.  This confusion may lead to
missing functionality (and possibly other issues) down the road.

To address this problem, adjust acpi_run_osc() to avoid returning an
error when OSC_CAPABILITIES_MASK_ERROR is set in the return buffer
and OSC_QUERY_ENABLE is clear in the OC capabilities, but all of the
bits in the support mask supplied by the OS are also set in the bit
mask returned by the platform firmware.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/bus.c |   23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -240,9 +240,13 @@ acpi_status acpi_run_osc(acpi_handle han
 		status = AE_TYPE;
 		goto out_kfree;
 	}
+	if (out_obj->buffer.length <= OSC_SUPPORT_DWORD) {
+		status = AE_BAD_DATA;
+		goto out_kfree;
+	}
 	/* Need to ignore the bit0 in result code */
 	errors = *((u32 *)out_obj->buffer.pointer) & ~(1 << 0);
-	if (errors) {
+	if (errors & ~OSC_CAPABILITIES_MASK_ERROR) {
 		if (errors & OSC_REQUEST_ERROR)
 			acpi_print_osc_error(handle, context,
 				"_OSC request failed");
@@ -252,17 +256,20 @@ acpi_status acpi_run_osc(acpi_handle han
 		if (errors & OSC_INVALID_REVISION_ERROR)
 			acpi_print_osc_error(handle, context,
 				"_OSC invalid revision");
-		if (errors & OSC_CAPABILITIES_MASK_ERROR) {
-			if (((u32 *)context->cap.pointer)[OSC_QUERY_DWORD]
-			    & OSC_QUERY_ENABLE)
-				goto out_success;
+		status = AE_ERROR;
+		goto out_kfree;
+	}
+	if (errors) {
+		u32 retbuf = ((u32 *)out_obj->buffer.pointer)[OSC_SUPPORT_DWORD];
+		u32 capbuf = ((u32 *)context->cap.pointer)[OSC_SUPPORT_DWORD];
+		u32 querybuf = ((u32 *)context->cap.pointer)[OSC_QUERY_DWORD];
+
+		/* OSC_CAPABILITIES_MASK_ERROR is set in errors. */
+		if (!(querybuf & OSC_QUERY_ENABLE) && (capbuf & retbuf) != capbuf) {
 			status = AE_SUPPORT;
 			goto out_kfree;
 		}
-		status = AE_ERROR;
-		goto out_kfree;
 	}
-out_success:
 	context->ret.length = out_obj->buffer.length;
 	context->ret.pointer = kmemdup(out_obj->buffer.pointer,
 				       context->ret.length, GFP_KERNEL);

[PATCH v1 2/2] ACPI: bus: Adjust acpi_bus_osc_negotiate_platform_control()

[Rafael J. Wysocki]

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

Remove checks that have become redundant after a previous change from
acpi_bus_osc_negotiate_platform_control(), add a debug statement
regarding the negotiated support bit mask to it and add an empty
code line to it for more clarity.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/bus.c |   28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -319,7 +319,7 @@ 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;
+	u32 capbuf[2], *capbuf_ret, ret_support;
 	struct acpi_osc_context context = {
 		.uuid_str = sb_uuid_str,
 		.rev = 1,
@@ -371,6 +371,7 @@ static void acpi_bus_osc_negotiate_platf
 
 	if (!ghes_disable)
 		capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
+
 	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
 		return;
 
@@ -378,10 +379,9 @@ static void acpi_bus_osc_negotiate_platf
 		return;
 
 	capbuf_ret = context.ret.pointer;
-	if (context.ret.length <= OSC_SUPPORT_DWORD) {
-		kfree(context.ret.pointer);
-		return;
-	}
+
+	acpi_handle_debug(handle, "Negotiated _OSC mask [%04x]\n",
+			  capbuf_ret[OSC_SUPPORT_DWORD]);
 
 	/*
 	 * Now run _OSC again with query flag clear and with the caps
@@ -395,20 +395,14 @@ static void acpi_bus_osc_negotiate_platf
 		return;
 
 	capbuf_ret = context.ret.pointer;
-	if (context.ret.length > OSC_SUPPORT_DWORD) {
+	ret_support = capbuf_ret[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 = ret_support & 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;
-	}
+	osc_sb_apei_support_acked = ret_support & OSC_SB_APEI_SUPPORT;
+	osc_pc_lpi_support_confirmed = ret_support & OSC_SB_PCLPI_SUPPORT;
+	osc_sb_native_usb4_support_confirmed = ret_support & OSC_SB_NATIVE_USB4_SUPPORT;
+	osc_cpc_flexible_adr_space_confirmed = ret_support & OSC_SB_CPC_FLEXIBLE_ADR_SPACE;
 
 	kfree(context.ret.pointer);
 }

Re: [PATCH v1 1/2] ACPI: bus: Adjust error handling in acpi_run_osc()

[Mario Limonciello]

On 12/16/2025 2:17 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Some platform firmware incorrectly sets the OSC_CAPABILITIES_MASK_ERROR
> bit in its _OSC return buffer even if no support bits have been actually
> masked, which causes acpi_run_osc() to return an error when executed
> with OSC_QUERY_ENABLE clear in the OC capabilities buffer.  As a result,
> the OS assumes that the _OSC evaluation has failed and the platform has
> not acknowledged any capabilities, while the platform assumes that it
> actually has acknowledged some of them.  This confusion may lead to
> missing functionality (and possibly other issues) down the road.
> 
> To address this problem, adjust acpi_run_osc() to avoid returning an
> error when OSC_CAPABILITIES_MASK_ERROR is set in the return buffer
> and OSC_QUERY_ENABLE is clear in the OC capabilities, but all of the
> bits in the support mask supplied by the OS are also set in the bit
> mask returned by the platform firmware.

If possible can you add some more context to explain which _OSC was 
behaving this way?  And if it's production hardware what hardware was 
affected?

It sounds like a viable workaround, I just want to make sure that if we 
have to change this again later we have more information to look back upon.

> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/bus.c |   23 +++++++++++++++--------
>   1 file changed, 15 insertions(+), 8 deletions(-)
> 
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -240,9 +240,13 @@ acpi_status acpi_run_osc(acpi_handle han
>   		status = AE_TYPE;
>   		goto out_kfree;
>   	}
> +	if (out_obj->buffer.length <= OSC_SUPPORT_DWORD) {
> +		status = AE_BAD_DATA;
> +		goto out_kfree;
> +	}
>   	/* Need to ignore the bit0 in result code */
>   	errors = *((u32 *)out_obj->buffer.pointer) & ~(1 << 0);
> -	if (errors) {
> +	if (errors & ~OSC_CAPABILITIES_MASK_ERROR) {
>   		if (errors & OSC_REQUEST_ERROR)
>   			acpi_print_osc_error(handle, context,
>   				"_OSC request failed");
> @@ -252,17 +256,20 @@ acpi_status acpi_run_osc(acpi_handle han
>   		if (errors & OSC_INVALID_REVISION_ERROR)
>   			acpi_print_osc_error(handle, context,
>   				"_OSC invalid revision");
> -		if (errors & OSC_CAPABILITIES_MASK_ERROR) {
> -			if (((u32 *)context->cap.pointer)[OSC_QUERY_DWORD]
> -			    & OSC_QUERY_ENABLE)
> -				goto out_success;
> +		status = AE_ERROR;
> +		goto out_kfree;
> +	}
> +	if (errors) {
> +		u32 retbuf = ((u32 *)out_obj->buffer.pointer)[OSC_SUPPORT_DWORD];
> +		u32 capbuf = ((u32 *)context->cap.pointer)[OSC_SUPPORT_DWORD];
> +		u32 querybuf = ((u32 *)context->cap.pointer)[OSC_QUERY_DWORD];
> +
> +		/* OSC_CAPABILITIES_MASK_ERROR is set in errors. */
> +		if (!(querybuf & OSC_QUERY_ENABLE) && (capbuf & retbuf) != capbuf) {
>   			status = AE_SUPPORT;
>   			goto out_kfree;
>   		}
> -		status = AE_ERROR;
> -		goto out_kfree;
>   	}
> -out_success:
>   	context->ret.length = out_obj->buffer.length;
>   	context->ret.pointer = kmemdup(out_obj->buffer.pointer,
>   				       context->ret.length, GFP_KERNEL);
> 
> 
> 

Re: [PATCH v1 1/2] ACPI: bus: Adjust error handling in acpi_run_osc()

[Rafael J. Wysocki]

On Tue, Dec 16, 2025 at 11:04 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
>
>
> On 12/16/2025 2:17 PM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Some platform firmware incorrectly sets the OSC_CAPABILITIES_MASK_ERROR
> > bit in its _OSC return buffer even if no support bits have been actually
> > masked, which causes acpi_run_osc() to return an error when executed
> > with OSC_QUERY_ENABLE clear in the OC capabilities buffer.  As a result,
> > the OS assumes that the _OSC evaluation has failed and the platform has
> > not acknowledged any capabilities, while the platform assumes that it
> > actually has acknowledged some of them.  This confusion may lead to
> > missing functionality (and possibly other issues) down the road.
> >
> > To address this problem, adjust acpi_run_osc() to avoid returning an
> > error when OSC_CAPABILITIES_MASK_ERROR is set in the return buffer
> > and OSC_QUERY_ENABLE is clear in the OC capabilities, but all of the
> > bits in the support mask supplied by the OS are also set in the bit
> > mask returned by the platform firmware.
>
> If possible can you add some more context to explain which _OSC was
> behaving this way?

\_SB._OSC

>  And if it's production hardware what hardware was affected?

No, it isn't, but this was not caught by Windows validation, so it is
likely that Linux will be exposed to this again.

> It sounds like a viable workaround, I just want to make sure that if we
> have to change this again later we have more information to look back upon.
>
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >   drivers/acpi/bus.c |   23 +++++++++++++++--------
> >   1 file changed, 15 insertions(+), 8 deletions(-)
> >
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -240,9 +240,13 @@ acpi_status acpi_run_osc(acpi_handle han
> >               status = AE_TYPE;
> >               goto out_kfree;
> >       }
> > +     if (out_obj->buffer.length <= OSC_SUPPORT_DWORD) {
> > +             status = AE_BAD_DATA;
> > +             goto out_kfree;
> > +     }
> >       /* Need to ignore the bit0 in result code */
> >       errors = *((u32 *)out_obj->buffer.pointer) & ~(1 << 0);
> > -     if (errors) {
> > +     if (errors & ~OSC_CAPABILITIES_MASK_ERROR) {
> >               if (errors & OSC_REQUEST_ERROR)
> >                       acpi_print_osc_error(handle, context,
> >                               "_OSC request failed");
> > @@ -252,17 +256,20 @@ acpi_status acpi_run_osc(acpi_handle han
> >               if (errors & OSC_INVALID_REVISION_ERROR)
> >                       acpi_print_osc_error(handle, context,
> >                               "_OSC invalid revision");
> > -             if (errors & OSC_CAPABILITIES_MASK_ERROR) {
> > -                     if (((u32 *)context->cap.pointer)[OSC_QUERY_DWORD]
> > -                         & OSC_QUERY_ENABLE)
> > -                             goto out_success;
> > +             status = AE_ERROR;
> > +             goto out_kfree;
> > +     }
> > +     if (errors) {
> > +             u32 retbuf = ((u32 *)out_obj->buffer.pointer)[OSC_SUPPORT_DWORD];
> > +             u32 capbuf = ((u32 *)context->cap.pointer)[OSC_SUPPORT_DWORD];
> > +             u32 querybuf = ((u32 *)context->cap.pointer)[OSC_QUERY_DWORD];
> > +
> > +             /* OSC_CAPABILITIES_MASK_ERROR is set in errors. */
> > +             if (!(querybuf & OSC_QUERY_ENABLE) && (capbuf & retbuf) != capbuf) {
> >                       status = AE_SUPPORT;
> >                       goto out_kfree;
> >               }
> > -             status = AE_ERROR;
> > -             goto out_kfree;
> >       }
> > -out_success:
> >       context->ret.length = out_obj->buffer.length;
> >       context->ret.pointer = kmemdup(out_obj->buffer.pointer,
> >                                      context->ret.length, GFP_KERNEL);
> >
> >
> >
>