Re: [PATCH] ACPI: bus: Avoid misleading _OSC missing-feature errors

[Jonathan Cameron]

On Tue, 21 Jul 2026 21:29:14 +0200
Saverio Miroddi <saverio.pub2@xxxxxxxxx> wrote:

> The ACPI specification defines OSC_CAPABILITIES_MASK_ERROR to mean that
> firmware cleared capability bits requested by the operating system. Some
> firmware sets this status during the control request while returning every
> requested capability unchanged. On the affected system, the requested and
> returned masks were both 0x006a7eff.

Just to check my understanding.  When a system does this there is no error?
Is it happy with the capabilities requested?  So the return value is simply
a firmware bug? (just one we didn't notice before).

> 
> Commit e5322888e6bf ("ACPI: bus: Rework the handling of \_SB._OSC
> platform features") made all control-response errors produce error-level
> messages. Consequently, the inconsistent response above now claims that
> features may be missing even though none were removed.
> 
> Track whether the response actually clears a requested bit and omit the
> error-level messages only if OSC_CAPABILITIES_MASK_ERROR is the sole
> error and no capability was cleared. Keep the dynamic-debug diagnostic and
> leave the negotiated capability mask and all other error cases unchanged.
> 
> Fixes: e5322888e6bf ("ACPI: bus: Rework the handling of \_SB._OSC platform features")
> Signed-off-by: Saverio Miroddi <saverio.pub2@xxxxxxxxx>
> ---
>  drivers/acpi/bus.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index a30a904f6535..c13128cbc2f6 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -335,7 +335,8 @@ static int acpi_osc_handshake(acpi_handle handle, const char *uuid_str,
>  		.length = bufsize * sizeof(u32),
>  	};
>  	struct acpi_buffer output;
> -	u32 *retbuf, test;
> +	u32 *retbuf, test, errors;
> +	bool capabilities_masked = false;
>  	guid_t guid;
>  	int ret, i;
>  
> @@ -395,16 +396,24 @@ static int acpi_osc_handshake(acpi_handle handle, const char *uuid_str,
>  	 * Clear the feature bits in capbuf[] that have not been acknowledged.
>  	 * After that, capbuf[] contains the resultant feature mask.
>  	 */
> -	for (i = OSC_QUERY_DWORD + 1; i < bufsize; i++)
> +	for (i = OSC_QUERY_DWORD + 1; i < bufsize; i++) {
> +		if (capbuf[i] & ~retbuf[i])
> +			capabilities_masked = true;
> +
>  		capbuf[i] &= retbuf[i];
> +	}
>  
> -	if (retbuf[OSC_QUERY_DWORD] & OSC_ERROR_MASK) {
> +	errors = retbuf[OSC_QUERY_DWORD] & OSC_ERROR_MASK;
> +	if (errors) {
>  		/*
> -		 * Complain about the unexpected errors and print diagnostic
> -		 * information related to them.
> +		 * Some firmware sets OSC_CAPABILITIES_MASK_ERROR without clearing
> +		 * any requested capability.  Only complain if another error is
> +		 * present or a capability was actually masked.

Can you add a clarification here on what we think the firmware is indicating
when it does this.

>  		 */
> -		acpi_handle_err(handle, "_OSC: errors while processing control request\n");
> -		acpi_handle_err(handle, "_OSC: some features may be missing\n");
> +		if (errors != OSC_CAPABILITIES_MASK_ERROR || capabilities_masked) {
> +			acpi_handle_err(handle, "_OSC: errors while processing control request\n");

Related to questions above, but this print is true on any error return even if perhaps
we are sure the next print is not.

> +			acpi_handle_err(handle, "_OSC: some features may be missing\n");
> +		}
>  		acpi_osc_error_check(handle, &guid, rev, &cap, retbuf);
>  	}
>