| Message ID | 75C90B50-9AB9-4F0A-B2CD-43427354D15C@live.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] efi: libstub: add support for the apple_set_os protocol | expand |
I've checked that this patch works for me when applied to 6.10-rc6. I can also use https://github.com/0xbb/gpu-switch to make the boot gpu the iGPU now. Tested-By: Orlando Chamberlain <orlandoch.dev@gmail.com> (MacBookPro16,1) On Mon, 1 Jul 2024 at 05:24, Aditya Garg <gargaditya08@live.com> wrote: > > From: Aditya Garg <gargaditya08@live.com> > > 0c18184de990 ("platform/x86: apple-gmux: support MMIO gmux on T2 Macs") > brought support for T2 Macs in apple-gmux. But in order to use dual GPU, > the integrated GPU has to be enabled. On such dual GPU EFI Macs, the EFI > stub needs to report that it is booting macOS in order to prevent the > firmware from disabling the iGPU. > > This patch is also applicable for some non T2 Intel Macs. > > Based on this patch for GRUB by Andreas Heider <andreas@heider.io>: > https://lists.gnu.org/archive/html/grub-devel/2013-12/msg00442.html > > Credits also goto Kerem Karabay <kekrby@gmail.com> for helping porting > the patch to the Linux kernel. > > Signed-off-by: Aditya Garg <gargaditya08@live.com> > --- > drivers/firmware/efi/libstub/efistub.h | 15 +++++++++++ > drivers/firmware/efi/libstub/x86-stub.c | 33 ++++++++++++++++++++++--- > include/linux/efi.h | 1 + > 3 files changed, 46 insertions(+), 3 deletions(-) > > diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h > index 27abb4ce0..4257a8b7c 100644 > --- a/drivers/firmware/efi/libstub/efistub.h > +++ b/drivers/firmware/efi/libstub/efistub.h > @@ -825,6 +825,21 @@ union apple_properties_protocol { > } mixed_mode; > }; > > +typedef union apple_set_os_protocol apple_set_os_protocol_t; > + > +union apple_set_os_protocol { > + struct { > + unsigned long version; > + efi_status_t (__efiapi *set_os_version) (const char *); > + efi_status_t (__efiapi *set_os_vendor) (const char *); > + }; > + struct { > + u32 version; > + u32 set_os_version; > + u32 set_os_vendor; > + } mixed_mode; > +}; > + > typedef u32 efi_tcg2_event_log_format; > > #define INITRD_EVENT_TAG_ID 0x8F3B22ECU > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c > index 1983fd3bf..1eea4f7ba 100644 > --- a/drivers/firmware/efi/libstub/x86-stub.c > +++ b/drivers/firmware/efi/libstub/x86-stub.c > @@ -225,6 +225,30 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params) > } > } > > +static void apple_set_os(void) > +{ > + efi_guid_t guid = APPLE_SET_OS_PROTOCOL_GUID; > + apple_set_os_protocol_t *set_os; > + efi_status_t status; > + > + status = efi_bs_call(locate_protocol, &guid, NULL, (void **)&set_os); > + if (status != EFI_SUCCESS) > + return; > + > + if (efi_table_attr(set_os, version) >= 2) { > + status = efi_fn_call(set_os, set_os_vendor, "Apple Inc."); > + if (status != EFI_SUCCESS) > + efi_err("Failed to set OS vendor via apple_set_os\n"); > + } > + > + /* The version being set doesn't seem to matter */ > + if (efi_table_attr(set_os, version) > 0) { > + status = efi_fn_call(set_os, set_os_version, "Mac OS X 10.9"); > + if (status != EFI_SUCCESS) > + efi_err("Failed to set OS version via apple_set_os\n"); > + } > +} > + > efi_status_t efi_adjust_memory_range_protection(unsigned long start, > unsigned long size) > { > @@ -335,9 +359,12 @@ static const efi_char16_t apple[] = L"Apple"; > > static void setup_quirks(struct boot_params *boot_params) > { > - if (IS_ENABLED(CONFIG_APPLE_PROPERTIES) && > - !memcmp(efistub_fw_vendor(), apple, sizeof(apple))) > - retrieve_apple_device_properties(boot_params); > + if (!memcmp(efistub_fw_vendor(), apple, sizeof(apple))) { > + if (IS_ENABLED(CONFIG_APPLE_PROPERTIES)) { > + retrieve_apple_device_properties(boot_params); > + } > + apple_set_os(); > + } > } > > /* > diff --git a/include/linux/efi.h b/include/linux/efi.h > index 418e55545..e28873eb1 100644 > --- a/include/linux/efi.h > +++ b/include/linux/efi.h > @@ -385,6 +385,7 @@ void efi_native_runtime_setup(void); > #define EFI_MEMORY_ATTRIBUTES_TABLE_GUID EFI_GUID(0xdcfa911d, 0x26eb, 0x469f, 0xa2, 0x20, 0x38, 0xb7, 0xdc, 0x46, 0x12, 0x20) > #define EFI_CONSOLE_OUT_DEVICE_GUID EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4, 0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d) > #define APPLE_PROPERTIES_PROTOCOL_GUID EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb, 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0) > +#define APPLE_SET_OS_PROTOCOL_GUID EFI_GUID(0xc5c5da95, 0x7d5c, 0x45e6, 0xb2, 0xf1, 0x3f, 0xd5, 0x2b, 0xb1, 0x00, 0x77) > #define EFI_TCG2_PROTOCOL_GUID EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f) > #define EFI_TCG2_FINAL_EVENTS_TABLE_GUID EFI_GUID(0x1e2ed096, 0x30e2, 0x4254, 0xbd, 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25) > #define EFI_LOAD_FILE_PROTOCOL_GUID EFI_GUID(0x56ec3091, 0x954c, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b) > -- > 2.39.3 (Apple Git-146) >
On Mon, 1 Jul 2024 at 06:46, Orlando Chamberlain <orlandoch.dev@gmail.com> wrote: > > > On Mon, 1 Jul 2024 at 05:24, Aditya Garg <gargaditya08@live.com> wrote: > > > > From: Aditya Garg <gargaditya08@live.com> > > > > 0c18184de990 ("platform/x86: apple-gmux: support MMIO gmux on T2 Macs") > > brought support for T2 Macs in apple-gmux. But in order to use dual GPU, > > the integrated GPU has to be enabled. On such dual GPU EFI Macs, the EFI > > stub needs to report that it is booting macOS in order to prevent the > > firmware from disabling the iGPU. > > > > This patch is also applicable for some non T2 Intel Macs. > > > > Based on this patch for GRUB by Andreas Heider <andreas@heider.io>: > > https://lists.gnu.org/archive/html/grub-devel/2013-12/msg00442.html > > > > Credits also goto Kerem Karabay <kekrby@gmail.com> for helping porting > > the patch to the Linux kernel. > > > > Signed-off-by: Aditya Garg <gargaditya08@live.com> > I've checked that this patch works for me when applied to 6.10-rc6. > I can also use https://github.com/0xbb/gpu-switch to make the boot gpu > the iGPU now. > > Tested-By: Orlando Chamberlain <orlandoch.dev@gmail.com> (MacBookPro16,1) > Thanks. Is there any point to making this set_os() call if CONFIG_APPLE_GMUX is not enabled?
On Sun, Jun 30, 2024 at 07:24:54PM +0000, Aditya Garg wrote: > @@ -335,9 +359,12 @@ static const efi_char16_t apple[] = L"Apple"; > > static void setup_quirks(struct boot_params *boot_params) > { > - if (IS_ENABLED(CONFIG_APPLE_PROPERTIES) && > - !memcmp(efistub_fw_vendor(), apple, sizeof(apple))) > - retrieve_apple_device_properties(boot_params); > + if (!memcmp(efistub_fw_vendor(), apple, sizeof(apple))) { > + if (IS_ENABLED(CONFIG_APPLE_PROPERTIES)) { > + retrieve_apple_device_properties(boot_params); > + } > + apple_set_os(); > + } Nit: Unnecessary curly braces around retrieve_apple_device_properties(). (And I'd appreciate a blank line between it and the apple_set_os() call.
On Mon, 1 Jul 2024 at 07:35, Lukas Wunner <lukas@wunner.de> wrote: > > On Sun, Jun 30, 2024 at 07:24:54PM +0000, Aditya Garg wrote: > > @@ -335,9 +359,12 @@ static const efi_char16_t apple[] = L"Apple"; > > > > static void setup_quirks(struct boot_params *boot_params) > > { > > - if (IS_ENABLED(CONFIG_APPLE_PROPERTIES) && > > - !memcmp(efistub_fw_vendor(), apple, sizeof(apple))) > > - retrieve_apple_device_properties(boot_params); > > + if (!memcmp(efistub_fw_vendor(), apple, sizeof(apple))) { > > + if (IS_ENABLED(CONFIG_APPLE_PROPERTIES)) { > > + retrieve_apple_device_properties(boot_params); > > + } > > + apple_set_os(); > > + } > > Nit: Unnecessary curly braces around retrieve_apple_device_properties(). > > (And I'd appreciate a blank line between it and the apple_set_os() call. Indeed. I can fix that up when applying. Any thoughts on whether this should depend on CONFIG_APPLE_GMUX or not?
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.
On Mon, 1 Jul 2024 at 07:50, Lukas Wunner <lukas@wunner.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?
On Mon, Jul 01, 2024 at 07:56:04AM +0200, Ard Biesheuvel wrote: > On Mon, 1 Jul 2024 at 07:50, Lukas Wunner <lukas@wunner.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
> On 1 Jul 2024, at 11:26 AM, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Mon, 1 Jul 2024 at 07:50, Lukas Wunner <lukas@wunner.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? It doesn't affect T2 Macs without dual GPU, I'm not sure about other Macs, although I doubt it would cause a regression. Although of the reasons of adding a parameter was this concern as well.
On Mon, 1 Jul 2024 at 08:41, Lukas Wunner <lukas@wunner.de> wrote: > > On Mon, Jul 01, 2024 at 07:56:04AM +0200, Ard Biesheuvel wrote: > > On Mon, 1 Jul 2024 at 07:50, Lukas Wunner <lukas@wunner.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: > > > ... > > > * 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. > > ... > > 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. > DMI matching would be a nice way to apply this change only to systems that have a need for it, without manual intervention via a command line switch. Assuming that Intel Macs implement the EFI SMBIOS protocol, reusing the existing pieces should be rather straight-forward. Something like the below should work in that case (whitespace damage courtesy of gmail) Note that the smbios.c libstub source file needs some changes to build correctly for x86 with CONFIG_EFI_MIXED=y, but I can take care of that. diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c index 250c964e93ee..826756c02657 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -225,12 +225,39 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params) } } +static bool apple_match_product_name(void) +{ + static const char type1_matches[][16] = { + "MacBookPro16,1", + }; + const struct efi_smbios_type1_record *record; + const u8 *product; + + record = (struct efi_smbios_type1_record *)efi_get_smbios_record(1); + if (!record) + return false; + + product = efi_get_smbios_string(&record->header, 1, product_name); + if (!product) + return false; + + for (int i = 0; i < ARRAY_SIZE(type1_matches); i++) { + if (!strcmp(product, type1_matches[i])) + return true; + } + + return false; +} + static void apple_set_os(void) { efi_guid_t guid = APPLE_SET_OS_PROTOCOL_GUID; apple_set_os_protocol_t *set_os; efi_status_t status; + if (!apple_match_product_name()) + return; + status = efi_bs_call(locate_protocol, &guid, NULL, (void **)&set_os); if (status != EFI_SUCCESS) return;
On Mon, Jul 01, 2024 at 08:41:05AM +0200, Lukas Wunner wrote: > 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. FWIW, there would be only 6 models to match if this needs to be constrained to ones with dual GPUs: MacBookPro11,3 MacBookPro11,5 MacBookPro13,3 MacBookPro14,3 MacBookPro15,1 MacBookPro16,1 And it seems to me that the product_name in efi_smbios_type1_record would contain that model name. Thanks, Lukas
On Mon, Jul 01, 2024 at 09:30:34AM +0200, Ard Biesheuvel wrote: > Assuming that Intel Macs implement the EFI SMBIOS protocol, reusing > the existing pieces should be rather straight-forward. Something like > the below should work in that case (whitespace damage courtesy of > gmail) > > Note that the smbios.c libstub source file needs some changes to > build correctly for x86 with CONFIG_EFI_MIXED=y, but I can take care > of that. Orlando, Aditya, could you test Ard's patch with CONFIG_EFI_MIXED=n? > +static bool apple_match_product_name(void) > +{ Maybe something like apple_has_dual_gpu() ?
On Mon, Jul 01, 2024 at 09:44:21AM +0200, Lukas Wunner wrote: > > +static bool apple_match_product_name(void) > > Maybe something like apple_has_dual_gpu() ? On second thought, maybe not. A neutral name allows us to add models like the MacBook Airs once we have support for them in the SPI keyboard driver. Thanks, Lukas
On Mon, 1 Jul 2024 at 09:44, Lukas Wunner <lukas@wunner.de> wrote: > > On Mon, Jul 01, 2024 at 09:30:34AM +0200, Ard Biesheuvel wrote: > > Assuming that Intel Macs implement the EFI SMBIOS protocol, reusing > > the existing pieces should be rather straight-forward. Something like > > the below should work in that case (whitespace damage courtesy of > > gmail) > > > > Note that the smbios.c libstub source file needs some changes to > > build correctly for x86 with CONFIG_EFI_MIXED=y, but I can take care > > of that. > > Orlando, Aditya, could you test Ard's patch with CONFIG_EFI_MIXED=n? > Yes, please test so we can check whether Intel Macs expose this protocol in the first place. Note that the following hunk is needed too: diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index 06f0428a723c..1f32d6cf98d6 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -77,5 +77,5 @@ lib-$(CONFIG_ARM) += arm32-stub.o lib-$(CONFIG_ARM64) += kaslr.o arm64.o arm64-stub.o smbios.o -lib-$(CONFIG_X86) += x86-stub.o +lib-$(CONFIG_X86) += x86-stub.o smbios.o lib-$(CONFIG_X86_64) += x86-5lvl.o lib-$(CONFIG_RISCV) += kaslr.o riscv.o riscv-stub.o
> On 1 Jul 2024, at 1:21 PM, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Mon, 1 Jul 2024 at 09:44, Lukas Wunner <lukas@wunner.de> wrote: >> >>> On Mon, Jul 01, 2024 at 09:30:34AM +0200, Ard Biesheuvel wrote: >>> Assuming that Intel Macs implement the EFI SMBIOS protocol, reusing >>> the existing pieces should be rather straight-forward. Something like >>> the below should work in that case (whitespace damage courtesy of >>> gmail) >>> >>> Note that the smbios.c libstub source file needs some changes to >>> build correctly for x86 with CONFIG_EFI_MIXED=y, but I can take care >>> of that. >> >> Orlando, Aditya, could you test Ard's patch with CONFIG_EFI_MIXED=n? >> > > Yes, please test so we can check whether Intel Macs expose this > protocol in the first place. Sure > > Note that the following hunk is needed too: > > diff --git a/drivers/firmware/efi/libstub/Makefile > b/drivers/firmware/efi/libstub/Makefile > index 06f0428a723c..1f32d6cf98d6 100644 > --- a/drivers/firmware/efi/libstub/Makefile > +++ b/drivers/firmware/efi/libstub/Makefile > @@ -77,5 +77,5 @@ > lib-$(CONFIG_ARM) += arm32-stub.o > lib-$(CONFIG_ARM64) += kaslr.o arm64.o arm64-stub.o smbios.o > -lib-$(CONFIG_X86) += x86-stub.o > +lib-$(CONFIG_X86) += x86-stub.o smbios.o > lib-$(CONFIG_X86_64) += x86-5lvl.o > lib-$(CONFIG_RISCV) += kaslr.o riscv.o riscv-stub.o
> On 1 Jul 2024, at 1:23 PM, Aditya Garg <gargaditya08@live.com> wrote: > > > >>> On 1 Jul 2024, at 1:21 PM, Ard Biesheuvel <ardb@kernel.org> wrote: >>> >>> On Mon, 1 Jul 2024 at 09:44, Lukas Wunner <lukas@wunner.de> wrote: >>> >>>> On Mon, Jul 01, 2024 at 09:30:34AM +0200, Ard Biesheuvel wrote: >>>> Assuming that Intel Macs implement the EFI SMBIOS protocol, reusing >>>> the existing pieces should be rather straight-forward. Something like >>>> the below should work in that case (whitespace damage courtesy of >>>> gmail) >>>> >>>> Note that the smbios.c libstub source file needs some changes to >>>> build correctly for x86 with CONFIG_EFI_MIXED=y, but I can take care >>>> of that. >>> >>> Orlando, Aditya, could you test Ard's patch with CONFIG_EFI_MIXED=n? >>> >> >> Yes, please test so we can check whether Intel Macs expose this >> protocol in the first place. > > Sure IIRC, t2 macs do not support SMBIOS, but I'll test this anyways.
> On 1 Jul 2024, at 1:45 PM, Lukas Wunner <lukas@wunner.de> wrote: > > On Mon, Jul 01, 2024 at 08:02:55AM +0000, Aditya Garg wrote: >> IIRC, t2 macs do not support SMBIOS, but I'll test this anyways. > > From MacBookPro16,1 dmesg: > > [ 0.000000] efi: EFI v2.40 by Apple > [ 0.000000] efi: ACPI=0x7affe000 ACPI 2.0=0x7affe014 SMBIOS=0x7aed1000 SMBIOS 3.0=0x7aecf000 > [ 0.000000] secureboot: Secure boot disabled > [ 0.000000] SMBIOS 3.0.0 present. > [ 0.000000] DMI: Apple Inc. MacBookPro16,1/Mac-E1008331FDC96864, BIOS 1037.80.53.0.0 (iBridge: 17.16.13050.0.0,0) 01/17/2020 > > Source: > > https://github.com/Dunedan/mbp-2016-linux/blob/master/MacBookPro16%2C1/dmesg Thanks. I've put the kernel to compile already.
> On 1 Jul 2024, at 1:21 PM, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Mon, 1 Jul 2024 at 09:44, Lukas Wunner <lukas@wunner.de> wrote: >> >> On Mon, Jul 01, 2024 at 09:30:34AM +0200, Ard Biesheuvel wrote: >>> Assuming that Intel Macs implement the EFI SMBIOS protocol, reusing >>> the existing pieces should be rather straight-forward. Something like >>> the below should work in that case (whitespace damage courtesy of >>> gmail) >>> >>> Note that the smbios.c libstub source file needs some changes to >>> build correctly for x86 with CONFIG_EFI_MIXED=y, but I can take care >>> of that. >> >> Orlando, Aditya, could you test Ard's patch with CONFIG_EFI_MIXED=n? >> > > Yes, please test so we can check whether Intel Macs expose this > protocol in the first place. > > Note that the following hunk is needed too: > The patch works Ard Tested-by: Aditya Garg <gargaditya08@live.com> For reference, here is kernel configuration of the kernel used to test this: https://pastebin.com/GPaxRWC6 > diff --git a/drivers/firmware/efi/libstub/Makefile > b/drivers/firmware/efi/libstub/Makefile > index 06f0428a723c..1f32d6cf98d6 100644 > --- a/drivers/firmware/efi/libstub/Makefile > +++ b/drivers/firmware/efi/libstub/Makefile > @@ -77,5 +77,5 @@ > lib-$(CONFIG_ARM) += arm32-stub.o > lib-$(CONFIG_ARM64) += kaslr.o arm64.o arm64-stub.o smbios.o > -lib-$(CONFIG_X86) += x86-stub.o > +lib-$(CONFIG_X86) += x86-stub.o smbios.o > lib-$(CONFIG_X86_64) += x86-5lvl.o > lib-$(CONFIG_RISCV) += kaslr.o riscv.o riscv-stub.o
> FWIW, there would be only 6 models to match if this needs to be > constrained to ones with dual GPUs: > > MacBookPro11,3 > MacBookPro11,5 > MacBookPro13,3 > MacBookPro14,3 > MacBookPro15,1 > MacBookPro16,1 I know that at least these two also need it: MacBookPro16,4 MacBookPro15,3 I think there could be more older ones too. Apple has a list of MacBookPros [0], but I don't know how many of the older models that list 2 gpus on their "Tech Specs" pages need apple-set-os. The original apple-set-os code was posted to the GRUB mailing list in December 2013 [1] so maybe it was in 2013 that new dual GPU Macbooks started needing apple-set-os? [0] https://support.apple.com/en-us/108052 [1] https://lists.gnu.org/archive/html/grub-devel/2013-12/msg00442.html
On Mon, Jul 01, 2024 at 11:14:52PM +1000, Orlando Chamberlain wrote: > > FWIW, there would be only 6 models to match if this needs to be > > constrained to ones with dual GPUs: > > > > MacBookPro11,3 > > MacBookPro11,5 > > MacBookPro13,3 > > MacBookPro14,3 > > MacBookPro15,1 > > MacBookPro16,1 > > I know that at least these two also need it: > MacBookPro16,4 > MacBookPro15,3 Ah okay, thanks. I went by the list in Wikipedia, disappointing that it apparently wasn't updated with those final T2 models: https://en.wikipedia.org/wiki/MacBook_Pro_(Intel-based) > I think there could be more older ones too. Apple has a list of > MacBookPros [0], but I don't know how many of the older models that > list 2 gpus on their "Tech Specs" pages need apple-set-os. No, Bruno Bierbaumer's page lists the MacBookPro11,3 and 11,5 as the first models that need apple_set_os: https://github.com/0xbb/gpu-switch I'm still using a MacBookPro9,1 (last pre-retina) on a daily basis and apple_set_os isn't needed there. Neither on the MacBookPro10,1 (first retina). Both were introduced mid 2012. > The original apple-set-os code was posted to the GRUB mailing list in > December 2013 [1] so maybe it was in 2013 that new dual GPU Macbooks > started needing apple-set-os? Yes, Haswell generation introduced Oct 2013 was the first one that needed it. Thanks, Lukas
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h index 27abb4ce0..4257a8b7c 100644 --- a/drivers/firmware/efi/libstub/efistub.h +++ b/drivers/firmware/efi/libstub/efistub.h @@ -825,6 +825,21 @@ union apple_properties_protocol { } mixed_mode; }; +typedef union apple_set_os_protocol apple_set_os_protocol_t; + +union apple_set_os_protocol { + struct { + unsigned long version; + efi_status_t (__efiapi *set_os_version) (const char *); + efi_status_t (__efiapi *set_os_vendor) (const char *); + }; + struct { + u32 version; + u32 set_os_version; + u32 set_os_vendor; + } mixed_mode; +}; + typedef u32 efi_tcg2_event_log_format; #define INITRD_EVENT_TAG_ID 0x8F3B22ECU diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c index 1983fd3bf..1eea4f7ba 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -225,6 +225,30 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params) } } +static void apple_set_os(void) +{ + efi_guid_t guid = APPLE_SET_OS_PROTOCOL_GUID; + apple_set_os_protocol_t *set_os; + efi_status_t status; + + status = efi_bs_call(locate_protocol, &guid, NULL, (void **)&set_os); + if (status != EFI_SUCCESS) + return; + + if (efi_table_attr(set_os, version) >= 2) { + status = efi_fn_call(set_os, set_os_vendor, "Apple Inc."); + if (status != EFI_SUCCESS) + efi_err("Failed to set OS vendor via apple_set_os\n"); + } + + /* The version being set doesn't seem to matter */ + if (efi_table_attr(set_os, version) > 0) { + status = efi_fn_call(set_os, set_os_version, "Mac OS X 10.9"); + if (status != EFI_SUCCESS) + efi_err("Failed to set OS version via apple_set_os\n"); + } +} + efi_status_t efi_adjust_memory_range_protection(unsigned long start, unsigned long size) { @@ -335,9 +359,12 @@ static const efi_char16_t apple[] = L"Apple"; static void setup_quirks(struct boot_params *boot_params) { - if (IS_ENABLED(CONFIG_APPLE_PROPERTIES) && - !memcmp(efistub_fw_vendor(), apple, sizeof(apple))) - retrieve_apple_device_properties(boot_params); + if (!memcmp(efistub_fw_vendor(), apple, sizeof(apple))) { + if (IS_ENABLED(CONFIG_APPLE_PROPERTIES)) { + retrieve_apple_device_properties(boot_params); + } + apple_set_os(); + } } /* diff --git a/include/linux/efi.h b/include/linux/efi.h index 418e55545..e28873eb1 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -385,6 +385,7 @@ void efi_native_runtime_setup(void); #define EFI_MEMORY_ATTRIBUTES_TABLE_GUID EFI_GUID(0xdcfa911d, 0x26eb, 0x469f, 0xa2, 0x20, 0x38, 0xb7, 0xdc, 0x46, 0x12, 0x20) #define EFI_CONSOLE_OUT_DEVICE_GUID EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4, 0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d) #define APPLE_PROPERTIES_PROTOCOL_GUID EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb, 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0) +#define APPLE_SET_OS_PROTOCOL_GUID EFI_GUID(0xc5c5da95, 0x7d5c, 0x45e6, 0xb2, 0xf1, 0x3f, 0xd5, 0x2b, 0xb1, 0x00, 0x77) #define EFI_TCG2_PROTOCOL_GUID EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f) #define EFI_TCG2_FINAL_EVENTS_TABLE_GUID EFI_GUID(0x1e2ed096, 0x30e2, 0x4254, 0xbd, 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25) #define EFI_LOAD_FILE_PROTOCOL_GUID EFI_GUID(0x56ec3091, 0x954c, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)