Message-ID: <ZoJPgSlZJ3ZlU2zL@wunner.de>
Date: Mon, 1 Jul 2024 08:41:05 +0200
From: Lukas Wunner <lukas@...ner.de>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: Aditya Garg <gargaditya08@...e.com>,
	Hans de Goede <hdegoede@...hat.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
	Kerem Karabay <kekrby@...il.com>,
	Orlando Chamberlain <orlandoch.dev@...il.com>
Subject: Re: [PATCH v2] efi: libstub: add support for the apple_set_os
 protocol

On Mon, Jul 01, 2024 at 07:56:04AM +0200, Ard Biesheuvel wrote:
> On Mon, 1 Jul 2024 at 07:50, Lukas Wunner <lukas@...ner.de> wrote:
> > On Mon, Jul 01, 2024 at 07:38:38AM +0200, Ard Biesheuvel wrote:
> > > Any thoughts on whether this should depend on CONFIG_APPLE_GMUX or not?
> >
> > I tend towards *not* making it depend on CONFIG_APPLE_GMUX:
> >
> > * The gpu-switch utility Orlando linked to doesn't use the apple-gmux
> >   driver.  (It changes EFI variables that influence to which GPU the
> >   EFI driver will switch on next reboot.)
> >
> > * apple_set_os() has side effects beyond exposing the iGPU (such as
> >   switching the keyboard+trackpad access method to SPI instead of USB).
> >   If there are issues, they will be harder to debug if their occurrence
> >   depends on a Kconfig option.
> 
> Understood. I agree that having fewer possible combinations is
> strongly preferred.
> 
> However, this change affects all Intel Macs. Is the latter side effect
> likely to cause any regressions on Intel Mac users that don't have two
> GPUs to begin with?

MacBook Air models introduced 2013/2014 will use SPI instead of USB
to access the keyboard and trackpad.  And from a quick look, the
applespi_tp_models[] array in drivers/input/keyboard/applespi.c
seems to be missing the trackpad dimensions of those models.
The driver may also need a quirk to work around missing properties
in the DSDT on those models.  Back in 2018 someone tested apple_set_os()
on such a machine and reported these issues, but the GitHub discussion
to narrow them down and fix them fizzled out:

https://github.com/cb22/macbook12-spi-driver/issues/65

The problem is that users of such models will generally run a
distribution kernel which has CONFIG_APPLE_GMUX enabled,
so constraining apple_set_os() to CONFIG_APPLE_GMUX won't help them.
Also, there is no incentive to amend the applespi.c driver for
affected machines if apple_set_os() is never called on them,
which is regrettable.

Another potential regression is that exposing the iGPU may consume
more power.  Or maybe the i915 driver will autosuspend if the panel
is not connected, I don't know.  Likewise there is no incentive to
fix the issue if apple_set_os() is never run.

I've also heard rumors that the EFI firmware configures different
CPU frequency scaling parameters if apple_set_os() is called,
but maybe Linux overwrites them anyway.  Apple never cared for Linux,
so apple_set_os() basically just differentiates between macOS and
Windows (via Boot Camp).  We generally choose to masquerade as macOS
to get access to the full set of hardware features, not the crippled
setup exposed to Windows.

If you think that we absolutely need to avoid these potential regressions,
a better approach than constraining to CONFIG_APPLE_GMUX would be to
match DMI data for dual-GPU MacBook Pros.  I notice that the efistub
has been amended with SMBIOS support through efi_get_smbios_record() +
efi_get_smbios_string().  Would that get us to the laptop model name?
If so that would seem to be a viable way to avoid or at least minimize
regressions.

Thanks,

Lukas

Powered by blists - more mailing lists