linux-kernel - Re: [PATCH v2] apple-gmux: preserve brightness using EFI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aYNjob2b9MTusPvO@wunner.de>
Date: Wed, 4 Feb 2026 16:20:01 +0100
From: Lukas Wunner <lukas@...ner.de>
To: Atharva Tiwari <atharvatiwarilinuxdev@...il.com>
Cc: Hans de Goede <hansg@...nel.org>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] apple-gmux: preserve brightness using EFI

On Tue, Feb 03, 2026 at 10:06:58AM +0000, Atharva Tiwari wrote:
> Make apple-gmux save the current backlight brightness to EFI on shutdown,
> so the brightness level is preserved across reboots.
[...]
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -1012,6 +1016,27 @@ static void gmux_remove(struct pnp_dev *pnp)
>  	kfree(gmux_data);
>  }
>  
> +static void gmux_shutdown(struct pnp_dev *pnp)
> +{
> +	struct apple_gmux_data *gmux_data = pnp_get_drvdata(pnp);
> +	efi_status_t status = EFI_UNSUPPORTED;
> +	u32 efi_attr;
> +	u16 efi_data;
> +
> +	gmux_remove(pnp);
> +
> +	efi_data = (u16)gmux_get_brightness(gmux_data->bdev);
> +	efi_attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +		EFI_VARIABLE_RUNTIME_ACCESS;
> +
> +	if (efi_rt_services_supported(EFI_RT_SUPPORTED_SET_VARIABLE))
> +		status = efi.set_variable(EFI_BRIGHTNESS_NAME, &EFI_BRIGHTNESS_GUID,
> +				efi_attr, sizeof(efi_data), &efi_data);
> +
> +	if (status != EFI_SUCCESS)
> +		pr_warn("Unable to save brightness to EFI: 0x%lx\n", status);
> +
> +}

Hm, isn't efivar_set_variable() the proper API to use?
Please cc linux-efi@...r.kernel.org on future submissions so that the
EFI maintainer gets a chance to look over your patch and confirm it's
using the right API.

Older Intel Macs with a gmux can be booted in CSM mode (bypassing EFI).
I'm wondering what happens on those with the above change?
Do they only get the warning or do they crash?
The warning is probably uncalled for on CSM-booted Macs.

CONFIG_EFI may not even be enabled, I suspect that apple-gmux.c does not
compile in that case with the above change.

On my MacBookPro9,1 (which does have a gmux), the backlight-level variable
has 6 bytes which currently contain (hexdump): 07 00 00 80 35 00
I guess this is actually a struct of a 32-bit and a 16-bit value
or alternatively three 16-bit values.  Yet you're only writing a
single 16-bit value to the variable.  How many bytes does the variable
have on your iMac and what are their contents?  I'm worried that
your change may not be compatible with all Macs which have this variable.

Instead of setting this variable only on shutdown, it may be better to
set it whenever brightness is changed.  That way, if the system isn't
properly shut down (runs out of battery or crashes), brightness is still
saved.  I'd introduce a work_struct which gets submitted with
queue_delayed_work() after, say, 300 msec whenever brightness is changed.
That way, multiple brightness changes are coalesced into a single
variable write.

Finally, there are Macs which don't have a gmux but which do have a
backlight.  They usually control brightness through i915 I think.
It would be nice to save brightness to the EFI variable on those as well.
Could probably be achieved by amending the backlight subsystem to
trigger the variable write if x86_apple_machine is true.
The variable write would then live in the backlight subsystem as an
x86-specific quirk, instead of in the gmux driver.  Perhaps the backlight
subsystem could be amended with a blocking notifier chain which is invoked
on brightness changes and the arch/x86 subsystem could then install a
notifier_block to trigger the variable write on x86_apple_machine systems
if EFI is enabled and used as boot mode.

Thanks,

Lukas