| Message ID | EBE51900-DA87-4113-B389-80B9C9160F0F@live.com |
|---|---|
| State | New |
| Headers | show |
| Series | efi: libstub: add support for the apple_set_os protocol | expand |
On Sun, Jun 30, 2024 at 04:42:55AM +0000, Aditya Garg wrote: > Commit 0c18184de990 brought support for T2 Macs in apple-gmux. But in order to Please run patches through scripts/checkpatch.pl before submission. The subject of the commit is missing here and lines should be wrapped at 72 or at least 74 chars. > Based on this patch for GRUB by Andreas Heider <andreas@heider.io>: > https://lists.gnu.org/archive/html/grub-devel/2013-12/msg00442.html Please include his Signed-off-by and cc him. > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -399,6 +399,8 @@ > useful so that a dump capture kernel won't be > shot down by NMI > > + apple_set_os [KNL] Report that macOS is being booted to the firmware > + Why the kernel parameter? Why not do this unconditionally? > +struct apple_set_os_protocol { > + u64 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; > +}; How about declaring this __packed, just to be on the safe side? Why "mixed_mode"? Seems like an odd name given "mixed mode" in EFI context usually means 64-bit OS, but 32-bit EFI. > +static void apple_set_os(void) > +{ > + efi_guid_t guid = APPLE_SET_OS_PROTOCOL_GUID; My recollection is that if you don't declare this static const, gcc generates suboptimal code. (It constructs the GUID on the stack at runtime instead of storing it in .rodata.) Maybe it's become smarter in the meantime, but I doubt it. Thanks, Lukas
On Sun, Jun 30, 2024 at 09:13:33AM +0000, Aditya Garg wrote: > > On 30 Jun 2024, at 1:34 PM, Lukas Wunner <lukas@wunner.de> wrote: > > On Sun, Jun 30, 2024 at 04:42:55AM +0000, Aditya Garg wrote: > > > Based on this patch for GRUB by Andreas Heider <andreas@heider.io>: > > > https://lists.gnu.org/archive/html/grub-devel/2013-12/msg00442.html > > > > Please include his Signed-off-by and cc him. > > Not sure about this since the patch was send to grub and not lkml, > and his work has been used without informing him for this patch solely > on the basis of GPL. > > I've always been confused in signed-off-by in case of authors whose work > has been used without their explicit consent just because the license > permits it. > > Should I still add his signed-off-by? I would. > > > --- a/Documentation/admin-guide/kernel-parameters.txt > > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > > @@ -399,6 +399,8 @@ > > > useful so that a dump capture kernel won't be > > > shot down by NMI > > > > > > + apple_set_os [KNL] Report that macOS is being booted to the firmware > > > + > > > > Why the kernel parameter? Why not do this unconditionally? > > 1. Not all Macs have dual GPU. We don't want to unnecessarily "fool" > the firmware in thinking macOS is being booted. > 2. apple-gmux is a reverse engineered driver, although upstreamed, > not very efficient in switching GPUs. So it's better to make unlocking > the GPU optional. + not everyone wants the intel GPU, people are happy > working with the dedicated AMD GPU (used by default if apple_set_os > isn't enabled). So my opinion is that these aren't good arguments. We should be identifying as Darwin by default in EFI, just as we've been doing in ACPI since 7bc5a2bad0b8. If there are any adverse effects, we should look into them, but users shouldn't be forced to set an obscure kernel parameter only to enable certain features of their hardware. It is for this reason that you'll usually get Greg KH's trademark "this isn't the 90s anymore" comment when adding new kernel parameters. We need to handle the hardware correctly *automatically*. There is one known issue affecting the keyboard on certain MacBook Air models: Apple had been using dual SPI and USB keyboards and trackpads since about 2013 but started to ship SPI-only models in 2015 with the MacBook 12". The models that shipped in the 2013/2014 time frame used USB by default and were automatically switched to SPI if apple_set_os is invoked. Not a problem since drivers/input/keyboard/applespi.c has been in mainline for a while now, but there are some very specific models that lack certain data in the DSDT which the applespi.c driver needs: https://github.com/cb22/macbook12-spi-driver/issues/65 However at this point that shouldn't stop us from making apple_set_os the default. We should rather just try to amend the applespi.c driver with a DMI quirk or something. That requires someone to test patches, so we need to find someone with an affected machine before we can fix it. But potential issues with 10+ years old hardware shouldn't stop us from enabling a feature by default that's useful on more recent hardware. So I'm strongly in favor of getting rid of the kernel parameter and then let's work on addressing side effects of apple_set_os one by one. > > > +struct apple_set_os_protocol { > > > + u64 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; > > > +}; > > > > How about declaring this __packed, just to be on the safe side? > You mean "struct __attribute__((__packed__)) apple_set_os_protocol {" ? Just add __packed at the end of the declaration. See efi_manage_capsule_header in include/linux/efi.h for an example. I think we haven't been very consistent with __packed declarations. The rationale not to use them everywhere is probably that gcc generally doesn't insert padding if only 32-bit and 64-bit types are used, but that's not guaranteed to be true forever. > > > > Why "mixed_mode"? Seems like an odd name given "mixed mode" > > in EFI context usually means 64-bit OS, but 32-bit EFI. > > EFI firmware on T2 Macs doesn't seem to follow the standard EFI specs > (expected from Apple). Earlier it claimed to follow EFI 2.0, but we > had to force linux to use EFI 1.1 for it. So as far as Apple is > concerned, you'll get to see such strange things. > > I guess this strange behaviour is because the T2 security chip handles > the EFI. I don't quite follow. Andreas' grub patch doesn't have this "mixed_mode" struct, so where is it (and the name) coming from? Thanks, Lukas
> On 30 Jun 2024, at 3:32 PM, Lukas Wunner <lukas@wunner.de> wrote: > > On Sun, Jun 30, 2024 at 09:13:33AM +0000, Aditya Garg wrote: >>>> On 30 Jun 2024, at 1:34 PM, Lukas Wunner <lukas@wunner.de> wrote: >>> On Sun, Jun 30, 2024 at 04:42:55AM +0000, Aditya Garg wrote: >>>> Based on this patch for GRUB by Andreas Heider <andreas@heider.io>: >>>> https://lists.gnu.org/archive/html/grub-devel/2013-12/msg00442.html >>> >>> Please include his Signed-off-by and cc him. >> >> Not sure about this since the patch was send to grub and not lkml, >> and his work has been used without informing him for this patch solely >> on the basis of GPL. >> >> I've always been confused in signed-off-by in case of authors whose work >> has been used without their explicit consent just because the license >> permits it. >> >> Should I still add his signed-off-by? > > I would. > > >>>> --- a/Documentation/admin-guide/kernel-parameters.txt >>>> +++ b/Documentation/admin-guide/kernel-parameters.txt >>>> @@ -399,6 +399,8 @@ >>>> useful so that a dump capture kernel won't be >>>> shot down by NMI >>>> >>>> + apple_set_os [KNL] Report that macOS is being booted to the firmware >>>> + >>> >>> Why the kernel parameter? Why not do this unconditionally? >> >> 1. Not all Macs have dual GPU. We don't want to unnecessarily "fool" >> the firmware in thinking macOS is being booted. >> 2. apple-gmux is a reverse engineered driver, although upstreamed, >> not very efficient in switching GPUs. So it's better to make unlocking >> the GPU optional. + not everyone wants the intel GPU, people are happy >> working with the dedicated AMD GPU (used by default if apple_set_os >> isn't enabled). > > So my opinion is that these aren't good arguments. We should be > identifying as Darwin by default in EFI, just as we've been doing > in ACPI since 7bc5a2bad0b8. If there are any adverse effects, > we should look into them, but users shouldn't be forced to set > an obscure kernel parameter only to enable certain features of > their hardware. It is for this reason that you'll usually get > Greg KH's trademark "this isn't the 90s anymore" comment when > adding new kernel parameters. We need to handle the hardware > correctly *automatically*. > I'm not in a favour of "forcing" dual GPU on users, especially because the features are quite unstable. On some distros like Ubuntu, since kernel 6.8, unlocking the GPU results in blank screen instead of igpu due to a regression (note that this patch has nothing to do with this regression, it's something the platform drivers people will look into). On the 2019 MacBook Pros the vgaswitchroo is not working and inputs from AMD are nil. Basically you get stuck to the Intel GPU and if you try to use the AMD GPU, but the GPUs remain on (currently no way has been found to switch off the AMD one) So I guess we have 2 options: 1. Wait until apple-gmux becomes quite stable before merging this (fat chance) 2. Give me some better idea to handle this. > There is one known issue affecting the keyboard on certain > MacBook Air models: Apple had been using dual SPI and USB > keyboards and trackpads since about 2013 but started to ship > SPI-only models in 2015 with the MacBook 12". > > The models that shipped in the 2013/2014 time frame used USB > by default and were automatically switched to SPI if apple_set_os > is invoked. Not a problem since drivers/input/keyboard/applespi.c > has been in mainline for a while now, but there are some very > specific models that lack certain data in the DSDT which the > applespi.c driver needs: > > https://github.com/cb22/macbook12-spi-driver/issues/65 > > However at this point that shouldn't stop us from making > apple_set_os the default. We should rather just try to amend > the applespi.c driver with a DMI quirk or something. > That requires someone to test patches, so we need to find > someone with an affected machine before we can fix it. > > But potential issues with 10+ years old hardware shouldn't > stop us from enabling a feature by default that's useful on > more recent hardware. So I'm strongly in favor of getting rid > of the kernel parameter and then let's work on addressing side > effects of apple_set_os one by one. > > >>>> +struct apple_set_os_protocol { >>>> + u64 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; >>>> +}; >>> >>> How about declaring this __packed, just to be on the safe side? >> You mean "struct __attribute__((__packed__)) apple_set_os_protocol {" ? > > Just add __packed at the end of the declaration. > See efi_manage_capsule_header in include/linux/efi.h for an example. > Alright. I'll also remove mixed_mode if that works. > I think we haven't been very consistent with __packed declarations. > The rationale not to use them everywhere is probably that gcc generally > doesn't insert padding if only 32-bit and 64-bit types are used, > but that's not guaranteed to be true forever. > > >>> >>> Why "mixed_mode"? Seems like an odd name given "mixed mode" >>> in EFI context usually means 64-bit OS, but 32-bit EFI. >> >> EFI firmware on T2 Macs doesn't seem to follow the standard EFI specs >> (expected from Apple). Earlier it claimed to follow EFI 2.0, but we >> had to force linux to use EFI 1.1 for it. So as far as Apple is >> concerned, you'll get to see such strange things. >> >> I guess this strange behaviour is because the T2 security chip handles >> the EFI. > > I don't quite follow. Andreas' grub patch doesn't have this > "mixed_mode" struct, so where is it (and the name) coming from? > > Thanks, > > Lukas
> On 30 Jun 2024, at 4:35 PM, Orlando Chamberlain <orlandoch.dev@gmail.com> wrote: > > On Sun, 30 Jun 2024 at 20:50, Aditya Garg <gargaditya08@live.com> wrote: >> >> >> >>>> On 30 Jun 2024, at 3:32 PM, Lukas Wunner <lukas@wunner.de> wrote: >>> >>> On Sun, Jun 30, 2024 at 09:13:33AM +0000, Aditya Garg wrote: >>>>>> On 30 Jun 2024, at 1:34 PM, Lukas Wunner <lukas@wunner.de> wrote: >>>>> On Sun, Jun 30, 2024 at 04:42:55AM +0000, Aditya Garg wrote: >>>>>> Based on this patch for GRUB by Andreas Heider <andreas@heider.io>: >>>>>> https://lists.gnu.org/archive/html/grub-devel/2013-12/msg00442.html >>>>> >>>>> Please include his Signed-off-by and cc him. >>>> >>>> Not sure about this since the patch was send to grub and not lkml, >>>> and his work has been used without informing him for this patch solely >>>> on the basis of GPL. >>>> >>>> I've always been confused in signed-off-by in case of authors whose work >>>> has been used without their explicit consent just because the license >>>> permits it. >>>> >>>> Should I still add his signed-off-by? >>> >>> I would. >>> >>> >>>>>> --- a/Documentation/admin-guide/kernel-parameters.txt >>>>>> +++ b/Documentation/admin-guide/kernel-parameters.txt >>>>>> @@ -399,6 +399,8 @@ >>>>>> useful so that a dump capture kernel won't be >>>>>> shot down by NMI >>>>>> >>>>>> + apple_set_os [KNL] Report that macOS is being booted to the firmware >>>>>> + >>>>> >>>>> Why the kernel parameter? Why not do this unconditionally? >>>> >>>> 1. Not all Macs have dual GPU. We don't want to unnecessarily "fool" >>>> the firmware in thinking macOS is being booted. >>>> 2. apple-gmux is a reverse engineered driver, although upstreamed, >>>> not very efficient in switching GPUs. So it's better to make unlocking >>>> the GPU optional. + not everyone wants the intel GPU, people are happy >>>> working with the dedicated AMD GPU (used by default if apple_set_os >>>> isn't enabled). >>> >>> So my opinion is that these aren't good arguments. We should be >>> identifying as Darwin by default in EFI, just as we've been doing >>> in ACPI since 7bc5a2bad0b8. If there are any adverse effects, >>> we should look into them, but users shouldn't be forced to set >>> an obscure kernel parameter only to enable certain features of >>> their hardware. It is for this reason that you'll usually get >>> Greg KH's trademark "this isn't the 90s anymore" comment when >>> adding new kernel parameters. We need to handle the hardware >>> correctly *automatically*. >>> >> I'm not in a favour of "forcing" dual GPU on users, especially because the features are quite unstable. On some distros like Ubuntu, since kernel 6.8, unlocking the GPU results in blank screen instead of igpu due to a regression (note that this patch has nothing to do with this regression, it's something the platform drivers people will look into). > > Is this just with apple-set-os or was > https://github.com/t2linux/linux-t2-patches/blob/main/2009-apple-gmux-allow-switching-to-igpu-at-probe.patch > and the kernel parameter it adds being used when that issue happened? Hi Orlando, thanks for replying My bad I should have checked this as well. Indeed removing the kernel parameter fixes the blank screen. > >> >> On the 2019 MacBook Pros the vgaswitchroo is not working and inputs from AMD are nil. Basically you get stuck to the Intel GPU and if you try to use the AMD GPU, but the GPUs remain on (currently no way has been found to switch off the AMD one) > > Do you mean that switching which gpu is connected to the display at > runtime isn't working? I *think* is is related to the not-upstream > patch I linked above? > >> >> So I guess we have 2 options: >> >> 1. Wait until apple-gmux becomes quite stable before merging this (fat chance) >> 2. Give me some better idea to handle this. >>
Hello Aditya, Lukas, On Sun, 30 Jun 2024 at 10:04, Lukas Wunner <lukas@wunner.de> wrote: > > On Sun, Jun 30, 2024 at 04:42:55AM +0000, Aditya Garg wrote: > > Commit 0c18184de990 brought support for T2 Macs in apple-gmux. But in order to > > Please run patches through scripts/checkpatch.pl before submission. > The subject of the commit is missing here and lines should be wrapped > at 72 or at least 74 chars. > > > > Based on this patch for GRUB by Andreas Heider <andreas@heider.io>: > > https://lists.gnu.org/archive/html/grub-devel/2013-12/msg00442.html > > Please include his Signed-off-by and cc him. > > No. You cannot simply add a signed-off-by on someone else's behalf, even if you cc the person. Andreas contributed code to GRUB (which is a GPLv3 project), and had no involvement whatsoever in creating this patch. A signed-off-by is a statement on the part of the contributor (which may or may not be the author) that the contribution in question complies with the requirements imposed by the project in terms of copyright and licensing. Linux is GPLv2 not v3, so this code should at least be dual licensed in order to be reused directly. I did not look at the GRUB patch, and IANAL, but this code invokes an Apple provided protocol (which is proprietary) in a hardcoded way for interoperability purposes, and so there is not much to copyright/license anyway. I would be fine with having just your signoff on this patch, but you could ask Andreas for a GPLv2+3 version of his GRUB patch if you are not sure. > > --- a/Documentation/admin-guide/kernel-parameters.txt > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > @@ -399,6 +399,8 @@ > > useful so that a dump capture kernel won't be > > shot down by NMI > > > > + apple_set_os [KNL] Report that macOS is being booted to the firmware > > + > > Why the kernel parameter? Why not do this unconditionally? > > Agree that this is suboptimal. If we need a command line param for this, please make add it to the efi= list > > +struct apple_set_os_protocol { This should be a union not a struct > > + u64 version; This should be unsigned long > > + efi_status_t (__efiapi *set_os_version) (const char *); > > + efi_status_t (__efiapi *set_os_vendor) (const char *); > > + struct { > > + u32 version; ... to match the mixed_mode overlay which is u32. Alternatively, they could both be u64 but the current arrangement is definitely incorrect. > > + u32 set_os_version; > > + u32 set_os_vendor; > > + } mixed_mode; > > +}; > > How about declaring this __packed, just to be on the safe side? > I don't think that is necessary. If the mixed_mode overlay is never used, it doesn't really matter and you can use unsigned long vs u32, in which case all struct members are native word size so there is no padding issue. > Why "mixed_mode"? Seems like an odd name given "mixed mode" > in EFI context usually means 64-bit OS, but 32-bit EFI. > This is how the x86 plumbing works for mixed mode. Every EFI protocol is a union, with a mixed_mode member describing the 32-bit view. The accessor macros (efi_bs_call, efi_table_attr) automatically do the right thing depending on the bitness of the firmware. This means, though, that even protocols that are known not to exist on 32-bit firmware need to be implemented in the same way, or the code will not build. > > > +static void apple_set_os(void) > > +{ > > + efi_guid_t guid = APPLE_SET_OS_PROTOCOL_GUID; > > My recollection is that if you don't declare this static const, > gcc generates suboptimal code. (It constructs the GUID on the > stack at runtime instead of storing it in .rodata.) > Maybe it's become smarter in the meantime, but I doubt it. > I don't remember the details but it looks like we stopped doing this a while ago. I don't mind keeping it like this.
> No. You cannot simply add a signed-off-by on someone else's behalf, > even if you cc the person. > > Andreas contributed code to GRUB (which is a GPLv3 project), and had > no involvement whatsoever in creating this patch. > > A signed-off-by is a statement on the part of the contributor (which > may or may not be the author) that the contribution in question > complies with the requirements imposed by the project in terms of > copyright and licensing. Linux is GPLv2 not v3, so this code should at > least be dual licensed in order to be reused directly. > > I did not look at the GRUB patch, and IANAL, but this code invokes an > Apple provided protocol (which is proprietary) in a hardcoded way for > interoperability purposes, and so there is not much to > copyright/license anyway. I would be fine with having just your > signoff on this patch, but you could ask Andreas for a GPLv2+3 version > of his GRUB patch if you are not sure. > Code similar to the patch sent by Andreas uses MIT license https://github.com/0xbb/apple_set_os.efi
> On 30 Jun 2024, at 6:29 PM, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Sun, 30 Jun 2024 at 13:56, Aditya Garg <gargaditya08@live.com> wrote: >> >> >> >>>> On 30 Jun 2024, at 4:59 PM, Ard Biesheuvel <ardb@kernel.org> wrote: >>> >>> Hello Aditya, Lukas, >>> >>>> On Sun, 30 Jun 2024 at 10:04, Lukas Wunner <lukas@wunner.de> wrote: >>>> >>>>> On Sun, Jun 30, 2024 at 04:42:55AM +0000, Aditya Garg wrote: >>>>> Commit 0c18184de990 brought support for T2 Macs in apple-gmux. But in order to >>>> >>>> Please run patches through scripts/checkpatch.pl before submission. >>>> The subject of the commit is missing here and lines should be wrapped >>>> at 72 or at least 74 chars. >>>> >>>> >>>>> Based on this patch for GRUB by Andreas Heider <andreas@heider.io>: >>>>> https://lists.gnu.org/archive/html/grub-devel/2013-12/msg00442.html >>>> >>>> Please include his Signed-off-by and cc him. >>>> >>>> >>> >>> No. You cannot simply add a signed-off-by on someone else's behalf, >>> even if you cc the person. >>> >>> Andreas contributed code to GRUB (which is a GPLv3 project), and had >>> no involvement whatsoever in creating this patch. >>> >>> A signed-off-by is a statement on the part of the contributor (which >>> may or may not be the author) that the contribution in question >>> complies with the requirements imposed by the project in terms of >>> copyright and licensing. Linux is GPLv2 not v3, so this code should at >>> least be dual licensed in order to be reused directly. >>> >>> I did not look at the GRUB patch, and IANAL, but this code invokes an >>> Apple provided protocol (which is proprietary) in a hardcoded way for >>> interoperability purposes, and so there is not much to >>> copyright/license anyway. I would be fine with having just your >>> signoff on this patch, but you could ask Andreas for a GPLv2+3 version >>> of his GRUB patch if you are not sure. >>> >> >> I believe this should be GPL2 compatible since it's simple reverse engineered Apple stuff and there are many kernel drivers with apple specific stuff. >> >>>>> --- a/Documentation/admin-guide/kernel-parameters.txt >>>>> +++ b/Documentation/admin-guide/kernel-parameters.txt >>>>> @@ -399,6 +399,8 @@ >>>>> useful so that a dump capture kernel won't be >>>>> shot down by NMI >>>>> >>>>> + apple_set_os [KNL] Report that macOS is being booted to the firmware >>>>> + >>>> >>>> Why the kernel parameter? Why not do this unconditionally? >>>> >>>> >>> >>> Agree that this is suboptimal. If we need a command line param for >>> this, please make add it to the efi= list >> >> I'll leave this to you. If you are fine with a parameter, I'll add it. If you have to be enforced by default, I'm fine with that. >> >> Or, maybe we add add a parameter that disables this so that in case things break, we can atleast get it done. >> >>>>> +struct apple_set_os_protocol { >>> >>> This should be a union not a struct >>> >>>>> + u64 version; >>> >>> This should be unsigned long >>> >>>>> + efi_status_t (__efiapi *set_os_version) (const char *); >>>>> + efi_status_t (__efiapi *set_os_vendor) (const char *); >>>>> + struct { >>>>> + u32 version; >>> >>> ... to match the mixed_mode overlay which is u32. Alternatively, they >>> could both be u64 but the current arrangement is definitely incorrect. >>> >>>>> + u32 set_os_version; >>>>> + u32 set_os_vendor; >>>>> + } mixed_mode; >>>>> +}; >>>> >>>> How about declaring this __packed, just to be on the safe side? >>>> >>> >>> I don't think that is necessary. If the mixed_mode overlay is never >>> used, it doesn't really matter and you can use unsigned long vs u32, >>> in which case all struct members are native word size so there is no >>> padding issue. >>> >>>> Why "mixed_mode"? Seems like an odd name given "mixed mode" >>>> in EFI context usually means 64-bit OS, but 32-bit EFI. >>>> >>> >>> This is how the x86 plumbing works for mixed mode. Every EFI protocol >>> is a union, with a mixed_mode member describing the 32-bit view. The >>> accessor macros (efi_bs_call, efi_table_attr) automatically do the >>> right thing depending on the bitness of the firmware. >>> >>> This means, though, that even protocols that are known not to exist on >>> 32-bit firmware need to be implemented in the same way, or the code >>> will not build. >>> >> So should I keep mixed mode or not? > > Yes. To summarize: > - keep your signoff as-is > - drop the command line param and handling > - make the outer protocol struct a union > - use 'unsigned long' for the version field > - keep the mixed_mode inner struct > - reorganize the conditional in setup_quirks() so that there are two > nested if blocks, where the memcmp() appears in the outer if() and > apple_set_os() is only called on Apple hardware That's for the summary Ard. Sending a v2
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 41644336e..cbd4697a5 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -399,6 +399,8 @@ useful so that a dump capture kernel won't be shot down by NMI + apple_set_os [KNL] Report that macOS is being booted to the firmware + autoconf= [IPV6] See Documentation/networking/ipv6.rst. diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c index bfa30625f..3d99acc1a 100644 --- a/drivers/firmware/efi/libstub/efi-stub-helper.c +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c @@ -19,6 +19,7 @@ bool efi_nochunk; bool efi_nokaslr = !IS_ENABLED(CONFIG_RANDOMIZE_BASE); bool efi_novamap; +bool efi_apple_set_os; static bool efi_noinitrd; static bool efi_nosoftreserve; @@ -73,6 +74,8 @@ efi_status_t efi_parse_options(char const *cmdline) efi_loglevel = CONSOLE_LOGLEVEL_QUIET; } else if (!strcmp(param, "noinitrd")) { efi_noinitrd = true; + } else if (!strcmp(param, "apple_set_os")) { + efi_apple_set_os = true; } else if (IS_ENABLED(CONFIG_X86_64) && !strcmp(param, "no5lvl")) { efi_no5lvl = true; } else if (IS_ENABLED(CONFIG_ARCH_HAS_MEM_ENCRYPT) && diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h index 212687c30..21b414d09 100644 --- a/drivers/firmware/efi/libstub/efistub.h +++ b/drivers/firmware/efi/libstub/efistub.h @@ -38,6 +38,7 @@ extern bool efi_nochunk; extern int efi_loglevel; extern int efi_mem_encrypt; extern bool efi_novamap; +extern bool efi_apple_set_os; extern const efi_system_table_t *efi_system_table; typedef union efi_dxe_services_table efi_dxe_services_table_t; @@ -825,6 +826,19 @@ union apple_properties_protocol { } mixed_mode; }; +typedef struct apple_set_os_protocol apple_set_os_protocol_t; + +struct apple_set_os_protocol { + u64 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 70b325a2f..2131f8543 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -223,6 +223,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) { @@ -321,6 +345,9 @@ 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 (efi_apple_set_os) + apple_set_os(); } /* diff --git a/include/linux/efi.h b/include/linux/efi.h index 80b21d1c6..f1e58e027 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -387,6 +387,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)