| Message ID | 20241228202212.89069-1-lleyton@fyralabs.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86/efistub: Add options for forcing Apple set_os protocol | expand |
On Sun, 29 Dec 2024 at 11:00, Lukas Wunner <lukas@wunner.de> wrote: > > On Sat, Dec 28, 2024 at 12:21:58PM -0800, Lleyton Gray wrote: > > commit 71e49eccdca6 ("x86/efistub: Call Apple set_os protocol on dual GPU > > Intel Macs") calls the Apple set_os protocol, but only on T2 Macbook Pro > > models. This causes issues on other T2 models when an egpu is used. > > How so? The commit specifically constrains the quirk only to > affected dual GPU MacBook Pros. So I don't quite see how this > would affect other T2 models? > > And what kind of issues are you seeing? Could you provide > specifics please? > > > Add two options which allow force enabling or disabling usage of the > > protocol to fix. > > We generally try to avoid command line options. The kernel > should automatically determine whether to apply the quirk > or not. So I think the proper solution is to amend the > quirk such that it's applied to additional models or not > applied under certain circumstances. > Indeed. But first I'd like to understand whether this is really the same issue, or another issue that requires the quirk. IIRC, the original issue is about not being able to use the discrete GPU with the built-in panel, and the issue about an external GPU did come up in the discussion - however, this does not affect the built-in panel at all.
On Sun, 29 Dec 2024 at 11:38, Lukas Wunner <lukas@wunner.de> wrote: > > On Sun, Dec 29, 2024 at 11:08:55AM +0100, Ard Biesheuvel wrote: > > IIRC, the original issue is about not being able to use the discrete > > GPU with the built-in panel, and the issue about an external GPU did > > come up in the discussion - however, this does not affect the built-in > > panel at all. > > The original issue was that the integrated GPU is hidden (powered off) > unless the set_os protocol is used. So only the discrete GPU is > available, which results in terrible battery life. Using set_os > keeps the iGPU exposed so the OS can switch to it and power off > the dGPU. > OK, thanks for clearing that up. But this is distinctly different from the eGPU case we are dealing with here. > We could have solved this by checking whether there are two PCI devices > with VGA class in the system. But that would have triggered in the > iGPU + eGPU case. We wanted to avoid that and thus quirked for the > DMI product names instead. > Yeah, so it would be good to know what issue are trying to fix with this patch.
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 1518343bbe22..1d1b88c57c44 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1447,7 +1447,8 @@ efi= [EFI,EARLY] Format: { "debug", "disable_early_pci_dma", "nochunk", "noruntime", "nosoftreserve", - "novamap", "no_disable_early_pci_dma" } + "novamap", "no_disable_early_pci_dma", + "enable_apple_set_os", "disable_apple_set_os" } debug: enable misc debug output. disable_early_pci_dma: disable the busmaster bit on all PCI bridges while in the EFI boot stub. @@ -1464,6 +1465,10 @@ novamap: do not call SetVirtualAddressMap(). no_disable_early_pci_dma: Leave the busmaster bit set on all PCI bridges while in the EFI boot stub + enable_apple_set_os: Report that macOS is being booted + to the firmware, even if the device is not a dual GPU Mac. + disable_apple_set_os: Disable reporting that macOS is being booted + to the firmware, even if the device is a dual GPU Mac. efi_no_storage_paranoia [EFI,X86,EARLY] Using this parameter you can use more than 50% of diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c index de659f6a815f..c33bb98bf79d 100644 --- a/drivers/firmware/efi/libstub/efi-stub-helper.c +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c @@ -20,6 +20,8 @@ bool efi_nochunk; bool efi_nokaslr = !IS_ENABLED(CONFIG_RANDOMIZE_BASE); bool efi_novamap; +bool efi_enable_apple_set_os; +bool efi_disable_apple_set_os; static bool efi_noinitrd; static bool efi_nosoftreserve; @@ -95,6 +97,10 @@ efi_status_t efi_parse_options(char const *cmdline) efi_disable_pci_dma = true; if (parse_option_str(val, "no_disable_early_pci_dma")) efi_disable_pci_dma = false; + if (parse_option_str(val, "enable_apple_set_os")) + efi_enable_apple_set_os = true; + if (parse_option_str(val, "disable_apple_set_os")) + efi_disable_apple_set_os = true; if (parse_option_str(val, "debug")) efi_loglevel = CONSOLE_LOGLEVEL_DEBUG; } else if (!strcmp(param, "video") && diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h index 685098f9626f..3be4b5393812 100644 --- a/drivers/firmware/efi/libstub/efistub.h +++ b/drivers/firmware/efi/libstub/efistub.h @@ -39,6 +39,8 @@ extern bool efi_nokaslr; extern int efi_loglevel; extern int efi_mem_encrypt; extern bool efi_novamap; +extern bool efi_enable_apple_set_os; +extern bool efi_disable_apple_set_os; extern const efi_system_table_t *efi_system_table; typedef union efi_dxe_services_table efi_dxe_services_table_t; diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c index f8e465da344d..566118195f92 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -265,7 +265,8 @@ static void apple_set_os(void) } *set_os; efi_status_t status; - if (!efi_is_64bit() || !apple_match_product_name()) + if (efi_disable_apple_set_os || !efi_is_64bit() || + !efi_enable_apple_set_os && !apple_match_product_name()) return; status = efi_bs_call(locate_protocol, &APPLE_SET_OS_PROTOCOL_GUID, NULL,
commit 71e49eccdca6 ("x86/efistub: Call Apple set_os protocol on dual GPU Intel Macs") calls the Apple set_os protocol, but only on T2 Macbook Pro models. This causes issues on other T2 models when an egpu is used. Add two options which allow force enabling or disabling usage of the protocol to fix. Signed-off-by: Lleyton Gray <lleyton@fyralabs.com> --- Documentation/admin-guide/kernel-parameters.txt | 7 ++++++- drivers/firmware/efi/libstub/efi-stub-helper.c | 6 ++++++ drivers/firmware/efi/libstub/efistub.h | 2 ++ drivers/firmware/efi/libstub/x86-stub.c | 3 ++- 4 files changed, 16 insertions(+), 2 deletions(-)