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

[Mario Limonciello <mario.limonciello@amd.com>]

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);
> 
> 
>