[PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

v3:
- add patch to make the SMBIOS protocol glue code compatible with mixed
  mode on x86
- update Aditya's patch to limit the effect to dual GPU Macs that are
  known to require the set_os() calls in order for both GPUs to remain
  active after boot
- drop mixed mode handling of set_os() protocol, and dereference the
  struct members directly

Cc: Aditya Garg <gargaditya08@live.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Kerem Karabay <kekrby@gmail.com>
Cc: Orlando Chamberlain <orlandoch.dev@gmail.com>

Aditya Garg (1):
  x86/efistub: Call Apple set_os protocol on dual GPU Intel Macs

Ard Biesheuvel (1):
  efistub/x86: Enable SMBIOS protocol handling for x86

 drivers/firmware/efi/libstub/Makefile   |  2 +-
 drivers/firmware/efi/libstub/smbios.c   | 42 ++++++++----
 drivers/firmware/efi/libstub/x86-stub.c | 72 +++++++++++++++++++-
 include/linux/efi.h                     |  1 +
 4 files changed, 98 insertions(+), 19 deletions(-)

-- 
2.45.2.803.g4e1b14247a-goog

[PATCH v3 1/2] efistub/x86: Enable SMBIOS protocol handling for x86

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

The smbios.c source file is not currently included in the x86 build, and
before we can do so, it needs some tweaks to build correctly in
combination with the EFI mixed mode support.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile |  2 +-
 drivers/firmware/efi/libstub/smbios.c | 42 +++++++++++++-------
 2 files changed, 28 insertions(+), 16 deletions(-)

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
@@ -76,7 +76,7 @@ lib-$(CONFIG_EFI_GENERIC_STUB)	+= efi-stub.o string.o intrinsics.o systable.o \
 
 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
 lib-$(CONFIG_LOONGARCH)		+= loongarch.o loongarch-stub.o
diff --git a/drivers/firmware/efi/libstub/smbios.c b/drivers/firmware/efi/libstub/smbios.c
index c217de2cc8d5..2b54b9c289be 100644
--- a/drivers/firmware/efi/libstub/smbios.c
+++ b/drivers/firmware/efi/libstub/smbios.c
@@ -6,30 +6,42 @@
 
 #include "efistub.h"
 
-typedef struct efi_smbios_protocol efi_smbios_protocol_t;
-
-struct efi_smbios_protocol {
-	efi_status_t (__efiapi *add)(efi_smbios_protocol_t *, efi_handle_t,
-				     u16 *, struct efi_smbios_record *);
-	efi_status_t (__efiapi *update_string)(efi_smbios_protocol_t *, u16 *,
-					       unsigned long *, u8 *);
-	efi_status_t (__efiapi *remove)(efi_smbios_protocol_t *, u16);
-	efi_status_t (__efiapi *get_next)(efi_smbios_protocol_t *, u16 *, u8 *,
-					  struct efi_smbios_record **,
-					  efi_handle_t *);
-
-	u8 major_version;
-	u8 minor_version;
+typedef union efi_smbios_protocol efi_smbios_protocol_t;
+
+union efi_smbios_protocol {
+	struct {
+		efi_status_t (__efiapi *add)(efi_smbios_protocol_t *, efi_handle_t,
+					     u16 *, struct efi_smbios_record *);
+		efi_status_t (__efiapi *update_string)(efi_smbios_protocol_t *, u16 *,
+						       unsigned long *, u8 *);
+		efi_status_t (__efiapi *remove)(efi_smbios_protocol_t *, u16);
+		efi_status_t (__efiapi *get_next)(efi_smbios_protocol_t *, u16 *, u8 *,
+						  struct efi_smbios_record **,
+						  efi_handle_t *);
+
+		u8 major_version;
+		u8 minor_version;
+	};
+	struct {
+		u32 add;
+		u32 update_string;
+		u32 remove;
+		u32 get_next;
+
+		u8 major_version;
+		u8 minor_version;
+	} mixed_mode;
 };
 
 const struct efi_smbios_record *efi_get_smbios_record(u8 type)
 {
+	static const efi_guid_t guid = EFI_SMBIOS_PROTOCOL_GUID;
 	struct efi_smbios_record *record;
 	efi_smbios_protocol_t *smbios;
 	efi_status_t status;
 	u16 handle = 0xfffe;
 
-	status = efi_bs_call(locate_protocol, &EFI_SMBIOS_PROTOCOL_GUID, NULL,
+	status = efi_bs_call(locate_protocol, (efi_guid_t *)&guid, NULL,
 			     (void **)&smbios) ?:
 		 efi_call_proto(smbios, get_next, &handle, &type, &record, NULL);
 	if (status != EFI_SUCCESS)
-- 
2.45.2.803.g4e1b14247a-goog

[PATCH v3 2/2] x86/efistub: Call Apple set_os protocol on dual GPU Intel Macs

[Ard Biesheuvel]

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.

Cc: Lukas Wunner <lukas@wunner.de>
Cc: Orlando Chamberlain <orlandoch.dev@gmail.com>
Signed-off-by: Aditya Garg <gargaditya08@live.com>
[ardb: limit using list of DMI matches kindly provided by Lukas]
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/x86-stub.c | 72 +++++++++++++++++++-
 include/linux/efi.h                     |  1 +
 2 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 68df27bd71c9..4870950c96d5 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -225,6 +225,69 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params)
 	}
 }
 
+static bool apple_match_product_name(void)
+{
+	static const char type1_product_matches[][15] = {
+		"MacBookPro11,3",
+		"MacBookPro11,5",
+		"MacBookPro13,3",
+		"MacBookPro14,3",
+		"MacBookPro15,1",
+		"MacBookPro15,3",
+		"MacBookPro16,1",
+		"MacBookPro16,4",
+	};
+	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_product_matches); i++) {
+		if (!strcmp(product, type1_product_matches[i]))
+			return true;
+	}
+
+	return false;
+}
+
+static void apple_set_os(void)
+{
+	struct {
+		unsigned long version;
+		efi_status_t (__efiapi *set_os_version)(const char *);
+		efi_status_t (__efiapi *set_os_vendor)(const char *);
+	} *set_os;
+	static const efi_guid_t guid = APPLE_SET_OS_PROTOCOL_GUID;
+	efi_status_t status;
+
+	if (!efi_is_64bit() || !apple_match_product_name())
+		return;
+
+	status = efi_bs_call(locate_protocol, (efi_guid_t *)&guid, NULL,
+			     (void **)&set_os);
+	if (status != EFI_SUCCESS)
+		return;
+
+	if (set_os->version >= 2) {
+		status = set_os->set_os_vendor("Apple Inc.");
+		if (status != EFI_SUCCESS)
+			efi_err("Failed to set OS vendor via apple_set_os\n");
+	}
+
+	if (set_os->version > 0) {
+		/* The version being set doesn't seem to matter */
+		status = 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 +398,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 418e555459da..e28873eb19ed 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.45.2.803.g4e1b14247a-goog

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Lukas Wunner]

On Mon, Jul 01, 2024 at 04:09:41PM +0200, Ard Biesheuvel wrote:
> Aditya Garg (1):
>   x86/efistub: Call Apple set_os protocol on dual GPU Intel Macs
> 
> Ard Biesheuvel (1):
>   efistub/x86: Enable SMBIOS protocol handling for x86

For the series,

Reviewed-by: Lukas Wunner <lukas@wunner.de>

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

Tested-by: Aditya Garg <gargaditya08@live.com>

Thanks for the support Ard :)

> On 1 Jul 2024, at 7:40 PM, Ard Biesheuvel <ardb+git@google.com> wrote:
> 
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> v3:
> - add patch to make the SMBIOS protocol glue code compatible with mixed
>  mode on x86
> - update Aditya's patch to limit the effect to dual GPU Macs that are
>  known to require the set_os() calls in order for both GPUs to remain
>  active after boot
> - drop mixed mode handling of set_os() protocol, and dereference the
>  struct members directly
> 
> Cc: Aditya Garg <gargaditya08@live.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Kerem Karabay <kekrby@gmail.com>
> Cc: Orlando Chamberlain <orlandoch.dev@gmail.com>
> 
> Aditya Garg (1):
>  x86/efistub: Call Apple set_os protocol on dual GPU Intel Macs
> 
> Ard Biesheuvel (1):
>  efistub/x86: Enable SMBIOS protocol handling for x86
> 
> drivers/firmware/efi/libstub/Makefile   |  2 +-
> drivers/firmware/efi/libstub/smbios.c   | 42 ++++++++----
> drivers/firmware/efi/libstub/x86-stub.c | 72 +++++++++++++++++++-
> include/linux/efi.h                     |  1 +
> 4 files changed, 98 insertions(+), 19 deletions(-)
> 
> --
> 2.45.2.803.g4e1b14247a-goog
> 

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

Hi Ard, Lukas

Although the patch has been upstreamed, and works well for the Macs included,
we have noticed another issue. For the Macs having a single GPU, in case a person
uses an eGPU, they still need this apple-set-os quirk for hybrid graphics. This was
not reported that time by anyone since people who use an eGPU are quite rare.

I'm not sure how to handle this. Had a few ideas like:

1. Enable this for all T2 Macs.
2. Enable this for all Macs.
3. Add a kernel parameter.

Would like your ideas and a possible fix for the same.

Thanks
Aditya

> On 2 Jul 2024, at 1:12 AM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> Tested-by: Aditya Garg <gargaditya08@live.com>
> 
> Thanks for the support Ard :)
> 
>> On 1 Jul 2024, at 7:40 PM, Ard Biesheuvel <ardb+git@google.com> wrote:
>> 
>> From: Ard Biesheuvel <ardb@kernel.org>
>> 
>> v3:
>> - add patch to make the SMBIOS protocol glue code compatible with mixed
>> mode on x86
>> - update Aditya's patch to limit the effect to dual GPU Macs that are
>> known to require the set_os() calls in order for both GPUs to remain
>> active after boot
>> - drop mixed mode handling of set_os() protocol, and dereference the
>> struct members directly
>> 
>> Cc: Aditya Garg <gargaditya08@live.com>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Cc: Lukas Wunner <lukas@wunner.de>
>> Cc: Kerem Karabay <kekrby@gmail.com>
>> Cc: Orlando Chamberlain <orlandoch.dev@gmail.com>
>> 
>> Aditya Garg (1):
>> x86/efistub: Call Apple set_os protocol on dual GPU Intel Macs
>> 
>> Ard Biesheuvel (1):
>> efistub/x86: Enable SMBIOS protocol handling for x86
>> 
>> drivers/firmware/efi/libstub/Makefile   |  2 +-
>> drivers/firmware/efi/libstub/smbios.c   | 42 ++++++++----
>> drivers/firmware/efi/libstub/x86-stub.c | 72 +++++++++++++++++++-
>> include/linux/efi.h                     |  1 +
>> 4 files changed, 98 insertions(+), 19 deletions(-)
>> 
>> --
>> 2.45.2.803.g4e1b14247a-goog
>> 

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Ard Biesheuvel]

On Wed, 17 Jul 2024 at 09:35, Aditya Garg <gargaditya08@live.com> wrote:
>
> Hi Ard, Lukas
>
> Although the patch has been upstreamed, and works well for the Macs included,
> we have noticed another issue. For the Macs having a single GPU, in case a person
> uses an eGPU, they still need this apple-set-os quirk for hybrid graphics. This was
> not reported that time by anyone since people who use an eGPU are quite rare.
>
> I'm not sure how to handle this. Had a few ideas like:
>
> 1. Enable this for all T2 Macs.
> 2. Enable this for all Macs.
> 3. Add a kernel parameter.
>
> Would like your ideas and a possible fix for the same.
>

Hi,

Is this a theoretical concern? Or are you aware of any user that is
actually affected by this?

In any case, given the niche nature, enabling this more widely by
default does not seem like a great approach, as the risk for
regressions outweighs the benefit.

I could imagine the use of a EFI variable to opt into this, would that
work? It would have to be set only once from user space (using
efivarfs)

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

Hi Ard

> Hi,
> 
> Is this a theoretical concern? Or are you aware of any user that is
> actually affected by this?

We had a Mac Mini user who wasn't able to use hybrid graphics
on his Mac after using the eGPU. So no, it's not a theoretical concern
> In any case, given the niche nature, enabling this more widely by
> default does not seem like a great approach, as the risk for
> regressions outweighs the benefit.
> 
> I could imagine the use of a EFI variable to opt into this, would that
> work? It would have to be set only once from user space (using
> efivarfs)

I'm not really sure about efivars here, but am ready to test. As long as
it doesn't break booting of macOS or Windows, I don't find it an issue.
macOS may also reset the variable, again have to verify by testing the same.
I guess it will also get reset if a users resets their NVRAM.

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Sharpened Blade]

> Hi Ard
> 
> > Hi,
> > 
> > Is this a theoretical concern? Or are you aware of any user that is
> > actually affected by this?
> 
> 
> We had a Mac Mini user who wasn't able to use hybrid graphics
> on his Mac after using the eGPU. So no, it's not a theoretical concern
> 
> > In any case, given the niche nature, enabling this more widely by
> > default does not seem like a great approach, as the risk for
> > regressions outweighs the benefit.
> > 
> > I could imagine the use of a EFI variable to opt into this, would that
> > work? It would have to be set only once from user space (using
> > efivarfs)
> 
> 
> I'm not really sure about efivars here, but am ready to test. As long as
> it doesn't break booting of macOS or Windows, I don't find it an issue.
> macOS may also reset the variable, again have to verify by testing the same.

On my mac, macOS does not reset EFI variables that it does not use. It should be the same for other models and firmware versions.

> I guess it will also get reset if a users resets their NVRAM.

Users have to manually reset their nvram so this will not happen during normal usage. This is actually a good thing since users can easily disable the eGPU if it is misbehaving and they dont have graphical output.

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Lukas Wunner]

On Wed, Jul 17, 2024 at 04:35:15PM +0000, Aditya Garg wrote:
> For the Macs having a single GPU, in case a person uses an eGPU,
> they still need this apple-set-os quirk for hybrid graphics.

I don't quite follow.  You mean the integrated graphics are
disabled by EFI firmware if an eGPU is attached?

This sounds like a bug on Apple's part:  The panel can be switched
between integrated graphics and discrete graphics, but an external
display can't be switched between eGPU and iGPU.  Is the person
affected using the latest EFI firmware update from Apple?

We need more information before we can devise a way to solve the
issue:

- Exact model name
  (I'd be surprised if this affected pre-Haswell models)
- Full dmesg output with command line option "dump_apple_properties"
  (EFI drivers provide various properties for Thunderbolt-attached
  devices and graphics cards, those could be queried to recognize
  the situation causing the issue)
- Full lspci output with and without eGPU attached

Perhaps you can open a bug at bugzilla.kernel.org and attach the
files there.

Thanks,

Lukas

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

Sending this message again as for some reason it got sent only to Lukas:

Full model name: Mac mini (2018) (Macmini8,1)

The drive link below has the logs:

https://drive.google.com/file/d/1P3-GlksU6WppvzvWC0A-nAoTZh7oPPxk/view?usp=drive_link

________________________________________
From: Lukas Wunner <lukas@wunner.de>
Sent: 18 July 2024 00:28
To: Aditya Garg
Cc: Ard Biesheuvel; linux-efi@vger.kernel.org; linux-kernel@vger.kernel.org; Ard Biesheuvel; Hans de Goede; Kerem Karabay; Orlando Chamberlain; sharpenedblade@proton.me
Subject: Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

On Wed, Jul 17, 2024 at 04:35:15PM +0000, Aditya Garg wrote:
> For the Macs having a single GPU, in case a person uses an eGPU,
> they still need this apple-set-os quirk for hybrid graphics.

I don't quite follow.  You mean the integrated graphics are
disabled by EFI firmware if an eGPU is attached?

This sounds like a bug on Apple's part:  The panel can be switched
between integrated graphics and discrete graphics, but an external
display can't be switched between eGPU and iGPU.  Is the person
affected using the latest EFI firmware update from Apple?

We need more information before we can devise a way to solve the
issue:

- Exact model name
  (I'd be surprised if this affected pre-Haswell models)
- Full dmesg output with command line option "dump_apple_properties"
  (EFI drivers provide various properties for Thunderbolt-attached
  devices and graphics cards, those could be queried to recognize
  the situation causing the issue)
- Full lspci output with and without eGPU attached

Perhaps you can open a bug at bugzilla.kernel.org and attach the
files there.

Thanks,

Lukas

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

Sending this message again because it previously got sent only to Lukas by mistake.

Hi,

Not sure whether it's relevant, but this user here also claims that without setting the os to macOS, the external GPUs had issues.

https://www.phoronix.com/forums/forum/software/general-linux-open-source/1479158-linux-6-11-efi-will-fake-that-it-s-booting-apple-macos-to-fix-some-dual-gpu-macs?p=1479205#post1479205

________________________________________
From: Aditya Garg <gargaditya08@live.com>
Sent: 23 July 2024 21:55
To: Lukas Wunner
Cc: Ard Biesheuvel; linux-efi@vger.kernel.org; linux-kernel@vger.kernel.org; Hans de Goede; Kerem Karabay; Orlando Chamberlain; sharpenedblade@proton.me
Subject: Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

Sending this message again as for some reason it got sent only to Lukas:

Full model name: Mac mini (2018) (Macmini8,1)

The drive link below has the logs:

https://drive.google.com/file/d/1P3-GlksU6WppvzvWC0A-nAoTZh7oPPxk/view?usp=drive_link

________________________________________
From: Lukas Wunner <lukas@wunner.de>
Sent: 18 July 2024 00:28
To: Aditya Garg
Cc: Ard Biesheuvel; linux-efi@vger.kernel.org; linux-kernel@vger.kernel.org; Ard Biesheuvel; Hans de Goede; Kerem Karabay; Orlando Chamberlain; sharpenedblade@proton.me
Subject: Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

On Wed, Jul 17, 2024 at 04:35:15PM +0000, Aditya Garg wrote:
> For the Macs having a single GPU, in case a person uses an eGPU,
> they still need this apple-set-os quirk for hybrid graphics.

I don't quite follow.  You mean the integrated graphics are
disabled by EFI firmware if an eGPU is attached?

This sounds like a bug on Apple's part:  The panel can be switched
between integrated graphics and discrete graphics, but an external
display can't be switched between eGPU and iGPU.  Is the person
affected using the latest EFI firmware update from Apple?

We need more information before we can devise a way to solve the
issue:

- Exact model name
  (I'd be surprised if this affected pre-Haswell models)
- Full dmesg output with command line option "dump_apple_properties"
  (EFI drivers provide various properties for Thunderbolt-attached
  devices and graphics cards, those could be queried to recognize
  the situation causing the issue)
- Full lspci output with and without eGPU attached

Perhaps you can open a bug at bugzilla.kernel.org and attach the
files there.

Thanks,

Lukas

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Lukas Wunner]

On Tue, Jul 23, 2024 at 04:25:19PM +0000, Aditya Garg wrote:
> On Wed, Jul 17, 2024 at 04:35:15PM +0000, Aditya Garg wrote:
> > For the Macs having a single GPU, in case a person uses an eGPU,
> > they still need this apple-set-os quirk for hybrid graphics.
> 
> Sending this message again as for some reason it got sent only to Lukas:
> 
> Full model name: Mac mini (2018) (Macmini8,1)
> 
> The drive link below has the logs:
> 
> https://drive.google.com/file/d/1P3-GlksU6WppvzvWC0A-nAoTZh7oPPxk/view?usp=drive_link

Some observations:

* dmesg-with-egpu.txt:  It seems the system was actually booted *without*
  an eGPU, so the filename appears to be a misnomer.

* The two files in the with_apple_set_os_efi directory only contain
  incomplete dmesg output.  Boot with log_buf_len=16M to solve this.
  Fortunately the truncated log is sufficient to see what's going on.

* If the apple_set_os protocol is not used, the attached eGPU is not
  enumerated by the kernel on boot and a rescan is required.
  So neither the iGPU nor the eGPU are working.  The reason is BIOS
  sets up incorrect bridge windows for the Thunderbolt host controller:
  Its two downstream ports' 64-bit windows overlap.  The 32-bit windows
  do not overlap.  If apple_set_os is used, the eGPU is using the
  (non-overlapping) 32-bit window.  If apple_set_os is not used,
  the attached eGPU is using the (overlapping, hence broken) 64-bit window.  

  So not only is apple_set_os needed to keep the iGPU enabled,
  but also to ensure BIOS sets up bridge windows in a manner that is
  only halfway broken and not totally broken.

  Below, 0000:06:01.0 and 0000:06:04.0 are the downstream ports on the
  Thunderbolt host controller and 0000:09:00.0 is the upstream port of
  the attached eGPU.

  iGPU enabled, no eGPU attached (dmesg.txt):
  pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
  pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xb83fffff 64bit pref]
  pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
  pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]

  iGPU disabled, eGPU attached, apple_set_os not used (journalctl.txt):
  pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
  pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xc6ffffff 64bit pref]
  pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
  pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]
  pci 0000:06:04.0: bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]: can't claim; address conflict with PCI Bus 0000:09 [mem 0xb1400000-0xbf3fffff 64bit pref]

  iGPU enabled, eGPU attached, apple_set_os used (working-journalctl.txt):
  pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
  pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xc6ffffff 64bit pref]
  pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
  pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]
  pci 0000:09:00.0:   bridge window [mem 0x81900000-0x81cfffff]

* As to how we can solve this and keep using apple_set_os only when
  necessary:

  I note that on x86, the efistub walks over all PCI devices in the system
  (see setup_efi_pci() in drivers/firmware/efi/libstub/x86-stub.c) and
  retrieves the Device ID and Vendor ID.  We could additionally retrieve
  the Class Code and count the number of GPUs in the system by checking
  whether the Class Code matches PCI_BASE_CLASS_DISPLAY.  If there's
  at least 2 GPUs in the system, invoke apple_set_os.

  The question is whether this is needed on *all* Apple products or only
  on newer ones.  I suspect that the eGPU issue may be specific to
  recent products.  Ideally we'd find someone with a Haswell or Ivy Bridge
  era Mac Mini and an eGPU who could verify whether apple_set_os is needed
  on older models as well.

  We could constrain apple_set_os to newer models by checking for
  presence of the T2 PCI device [106b:1802].  Alternatively, we could
  use the BIOS date (DMI_BIOS_DATE in SMBIOS data) to enforce a
  cut-off such that only machines with a recent BIOS use apple_set_os.

Thanks,

Lukas

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

https://www.phoronix.com/forums/forum/software/general-linux-open-source/1479158-linux-6-11-efi-will-fake-that-it-s-booting-apple-macos-to-fix-some-dual-gpu-macs?p=1479205#post1479205

By looking at this post, it seems to affect non T2 Macs as well.

Also, T2 Macs report that EFI v2 is being used, although by a kernel patch, we force them to use v1.  Have we can use this as a quirk for enabling apple-set-os?

https://lore.kernel.org/linux-efi/20220112101413.188234-1-ardb@kernel.org/

Although, the NVRAM variable idea by Ard looks better to me.

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Lukas Wunner]

On Wed, Jul 24, 2024 at 04:21:03PM +0000, Aditya Garg wrote:
> https://www.phoronix.com/forums/forum/software/general-linux-open-source/1479158-linux-6-11-efi-will-fake-that-it-s-booting-apple-macos-to-fix-some-dual-gpu-macs?p=1479205#post1479205
> 
> By looking at this post, it seems to affect non T2 Macs as well.

Disabling iGPU if dGPU is present was a behavior that existed well
before T2 Macs, yes.  That affected MacBook Pros from Haswell onward.

However the question is since when iGPU is disabled if an eGPU is present.
I had never heard of this before.  It would be good to have more datapoints
to come up with a quirk that's as narrowly applied as possible to avoid
regressions.

Thanks,

Lukas

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

> On 24 Jul 2024, at 9:58 PM, Lukas Wunner <lukas@wunner.de> wrote:
> 
> On Wed, Jul 24, 2024 at 04:21:03PM +0000, Aditya Garg wrote:
>> https://www.phoronix.com/forums/forum/software/general-linux-open-source/1479158-linux-6-11-efi-will-fake-that-it-s-booting-apple-macos-to-fix-some-dual-gpu-macs?p=1479205#post1479205
>> 
>> By looking at this post, it seems to affect non T2 Macs as well.
> 
> Disabling iGPU if dGPU is present was a behavior that existed well
> before T2 Macs, yes.  That affected MacBook Pros from Haswell onward.

If you read the person's comment carefully, he has said it affected the thunderbolt eGPUs as well. Fat chance he is using a T2 Mac, because I have been a part of the t2linux community for a long time and we received such case recently.

> 
> However the question is since when iGPU is disabled if an eGPU is present.
> I had never heard of this before.  It would be good to have more datapoints
> to come up with a quirk that's as narrowly applied as possible to avoid
> regressions.
> 
> Thanks,
> 
> Lukas

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

> On 24 Jul 2024, at 9:31 PM, Lukas Wunner <lukas@wunner.de> wrote:
> 
> On Tue, Jul 23, 2024 at 04:25:19PM +0000, Aditya Garg wrote:
>>> On Wed, Jul 17, 2024 at 04:35:15PM +0000, Aditya Garg wrote:
>>> For the Macs having a single GPU, in case a person uses an eGPU,
>>> they still need this apple-set-os quirk for hybrid graphics.
>> 
>> Sending this message again as for some reason it got sent only to Lukas:
>> 
>> Full model name: Mac mini (2018) (Macmini8,1)
>> 
>> The drive link below has the logs:
>> 
>> https://drive.google.com/file/d/1P3-GlksU6WppvzvWC0A-nAoTZh7oPPxk/view?usp=drive_link
> 
> Some observations:
> 
> * dmesg-with-egpu.txt:  It seems the system was actually booted *without*
>  an eGPU, so the filename appears to be a misnomer.
> 
> * The two files in the with_apple_set_os_efi directory only contain
>  incomplete dmesg output.  Boot with log_buf_len=16M to solve this.
>  Fortunately the truncated log is sufficient to see what's going on.
> 
> * If the apple_set_os protocol is not used, the attached eGPU is not
>  enumerated by the kernel on boot and a rescan is required.
>  So neither the iGPU nor the eGPU are working.  The reason is BIOS
>  sets up incorrect bridge windows for the Thunderbolt host controller:
>  Its two downstream ports' 64-bit windows overlap.  The 32-bit windows
>  do not overlap.  If apple_set_os is used, the eGPU is using the
>  (non-overlapping) 32-bit window.  If apple_set_os is not used,
>  the attached eGPU is using the (overlapping, hence broken) 64-bit window.  
> 
>  So not only is apple_set_os needed to keep the iGPU enabled,
>  but also to ensure BIOS sets up bridge windows in a manner that is
>  only halfway broken and not totally broken.
> 
>  Below, 0000:06:01.0 and 0000:06:04.0 are the downstream ports on the
>  Thunderbolt host controller and 0000:09:00.0 is the upstream port of
>  the attached eGPU.
> 
>  iGPU enabled, no eGPU attached (dmesg.txt):
>  pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
>  pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xb83fffff 64bit pref]
>  pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
>  pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]
> 
>  iGPU disabled, eGPU attached, apple_set_os not used (journalctl.txt):
>  pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
>  pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xc6ffffff 64bit pref]
>  pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
>  pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]
>  pci 0000:06:04.0: bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]: can't claim; address conflict with PCI Bus 0000:09 [mem 0xb1400000-0xbf3fffff 64bit pref]
> 
>  iGPU enabled, eGPU attached, apple_set_os used (working-journalctl.txt):
>  pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
>  pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xc6ffffff 64bit pref]
>  pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
>  pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]
>  pci 0000:09:00.0:   bridge window [mem 0x81900000-0x81cfffff]
> 
> * As to how we can solve this and keep using apple_set_os only when
>  necessary:
> 
>  I note that on x86, the efistub walks over all PCI devices in the system
>  (see setup_efi_pci() in drivers/firmware/efi/libstub/x86-stub.c) and
>  retrieves the Device ID and Vendor ID.  We could additionally retrieve
>  the Class Code and count the number of GPUs in the system by checking
>  whether the Class Code matches PCI_BASE_CLASS_DISPLAY.  If there's
>  at least 2 GPUs in the system, invoke apple_set_os.

This also looks like a good idea, but I'm not well aware of the pci quirks in the Linux kernel. So, would consider it a bug report for the maintainers to fix.
> 
>  The question is whether this is needed on *all* Apple products or only
>  on newer ones.  I suspect that the eGPU issue may be specific to
>  recent products.  Ideally we'd find someone with a Haswell or Ivy Bridge
>  era Mac Mini and an eGPU who could verify whether apple_set_os is needed
>  on older models as well.
> 
>  We could constrain apple_set_os to newer models by checking for
>  presence of the T2 PCI device [106b:1802].  Alternatively, we could
>  use the BIOS date (DMI_BIOS_DATE in SMBIOS data) to enforce a
>  cut-off such that only machines with a recent BIOS use apple_set_os.
> 
> Thanks,
> 
> Lukas

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Lukas Wunner]

On Wed, Jul 24, 2024 at 04:26:58PM +0000, Aditya Garg wrote:
> > On 24 Jul 2024, at 9:31 PM, Lukas Wunner <lukas@wunner.de> wrote:
> > I note that on x86, the efistub walks over all PCI devices in the system
> > (see setup_efi_pci() in drivers/firmware/efi/libstub/x86-stub.c) and
> > retrieves the Device ID and Vendor ID.  We could additionally retrieve
> > the Class Code and count the number of GPUs in the system by checking
> > whether the Class Code matches PCI_BASE_CLASS_DISPLAY.  If there's
> > at least 2 GPUs in the system, invoke apple_set_os.
> 
> This also looks like a good idea, but I'm not well aware of the pci
> quirks in the Linux kernel. So, would consider it a bug report for
> the maintainers to fix.

This is not a PCI quirk in the kernel.  The efistub is a separate
program.  I'm saying that the efistub already walks over all PCI devices,
it would be trivial to hook into this to count GPUs, recognize the T2
device or do something else entirely.

Thanks,

Lukas

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

> On 24 Jul 2024, at 10:01 PM, Lukas Wunner <lukas@wunner.de> wrote:
> 
> On Wed, Jul 24, 2024 at 04:26:58PM +0000, Aditya Garg wrote:
>>>> On 24 Jul 2024, at 9:31 PM, Lukas Wunner <lukas@wunner.de> wrote:
>>> I note that on x86, the efistub walks over all PCI devices in the system
>>> (see setup_efi_pci() in drivers/firmware/efi/libstub/x86-stub.c) and
>>> retrieves the Device ID and Vendor ID.  We could additionally retrieve
>>> the Class Code and count the number of GPUs in the system by checking
>>> whether the Class Code matches PCI_BASE_CLASS_DISPLAY.  If there's
>>> at least 2 GPUs in the system, invoke apple_set_os.
>> 
>> This also looks like a good idea, but I'm not well aware of the pci
>> quirks in the Linux kernel. So, would consider it a bug report for
>> the maintainers to fix.
> 
> This is not a PCI quirk in the kernel.  The efistub is a separate
> program.  I'm saying that the efistub already walks over all PCI devices,
> it would be trivial to hook into this to count GPUs, recognize the T2
> device or do something else entirely.
> 

I'll leave this to Ard. I'm really confused in choosing the best possible option here.

> Thanks,
> 
> Lukas

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Ard Biesheuvel]

On Wed, 24 Jul 2024 at 18:31, Lukas Wunner <lukas@wunner.de> wrote:
>
> On Wed, Jul 24, 2024 at 04:26:58PM +0000, Aditya Garg wrote:
> > > On 24 Jul 2024, at 9:31 PM, Lukas Wunner <lukas@wunner.de> wrote:
> > > I note that on x86, the efistub walks over all PCI devices in the system
> > > (see setup_efi_pci() in drivers/firmware/efi/libstub/x86-stub.c) and
> > > retrieves the Device ID and Vendor ID.  We could additionally retrieve
> > > the Class Code and count the number of GPUs in the system by checking
> > > whether the Class Code matches PCI_BASE_CLASS_DISPLAY.  If there's
> > > at least 2 GPUs in the system, invoke apple_set_os.
> >
> > This also looks like a good idea, but I'm not well aware of the pci
> > quirks in the Linux kernel. So, would consider it a bug report for
> > the maintainers to fix.
>
> This is not a PCI quirk in the kernel.  The efistub is a separate
> program.  I'm saying that the efistub already walks over all PCI devices,
> it would be trivial to hook into this to count GPUs, recognize the T2
> device or do something else entirely.
>

Thanks for the analysis, and for the suggestions.

I wouldn't object to changes to the EFI stub that implement something
along these lines, although I'd like to understand a bit better what
the actual issue is.

If PCI resource allocation is the culprit here, wouldn't it be better
to force Linux to reallocate those from scratch? IIRC there is already
a command line option for this.

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

> On 25 Jul 2024, at 6:12 PM, Ard Biesheuvel <ardb@kernel.org> wrote:
> 
> On Wed, 24 Jul 2024 at 18:31, Lukas Wunner <lukas@wunner.de> wrote:
>> 
>> On Wed, Jul 24, 2024 at 04:26:58PM +0000, Aditya Garg wrote:
>>>>> On 24 Jul 2024, at 9:31 PM, Lukas Wunner <lukas@wunner.de> wrote:
>>>>> I note that on x86, the efistub walks over all PCI devices in the system
>>>>> (see setup_efi_pci() in drivers/firmware/efi/libstub/x86-stub.c) and
>>>>> retrieves the Device ID and Vendor ID.  We could additionally retrieve
>>>>> the Class Code and count the number of GPUs in the system by checking
>>>>> whether the Class Code matches PCI_BASE_CLASS_DISPLAY.  If there's
>>>>> at least 2 GPUs in the system, invoke apple_set_os.
>>> 
>>> This also looks like a good idea, but I'm not well aware of the pci
>>> quirks in the Linux kernel. So, would consider it a bug report for
>>> the maintainers to fix.
>> 
>> This is not a PCI quirk in the kernel.  The efistub is a separate
>> program.  I'm saying that the efistub already walks over all PCI devices,
>> it would be trivial to hook into this to count GPUs, recognize the T2
>> device or do something else entirely.
>> 
> 
> Thanks for the analysis, and for the suggestions.
> 
> I wouldn't object to changes to the EFI stub that implement something
> along these lines, although I'd like to understand a bit better what
> the actual issue is.
> 
> If PCI resource allocation is the culprit here, wouldn't it be better
> to force Linux to reallocate those from scratch? IIRC there is already
> a command line option for this.

I guess you are talking about 'pci=realloc'. Adding this doesn't fix the issue.

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Ard Biesheuvel]

On Wed, 24 Jul 2024 at 18:27, Aditya Garg <gargaditya08@live.com> wrote:
>
>
>
> > On 24 Jul 2024, at 9:31 PM, Lukas Wunner <lukas@wunner.de> wrote:
> >
> > On Tue, Jul 23, 2024 at 04:25:19PM +0000, Aditya Garg wrote:
> >>> On Wed, Jul 17, 2024 at 04:35:15PM +0000, Aditya Garg wrote:
> >>> For the Macs having a single GPU, in case a person uses an eGPU,
> >>> they still need this apple-set-os quirk for hybrid graphics.
> >>
> >> Sending this message again as for some reason it got sent only to Lukas:
> >>
> >> Full model name: Mac mini (2018) (Macmini8,1)
> >>
> >> The drive link below has the logs:
> >>
> >> https://drive.google.com/file/d/1P3-GlksU6WppvzvWC0A-nAoTZh7oPPxk/view?usp=drive_link
> >
> > Some observations:
> >
> > * dmesg-with-egpu.txt:  It seems the system was actually booted *without*
> >  an eGPU, so the filename appears to be a misnomer.
> >
> > * The two files in the with_apple_set_os_efi directory only contain
> >  incomplete dmesg output.  Boot with log_buf_len=16M to solve this.
> >  Fortunately the truncated log is sufficient to see what's going on.
> >
> > * If the apple_set_os protocol is not used, the attached eGPU is not
> >  enumerated by the kernel on boot and a rescan is required.
> >  So neither the iGPU nor the eGPU are working.  The reason is BIOS
> >  sets up incorrect bridge windows for the Thunderbolt host controller:
> >  Its two downstream ports' 64-bit windows overlap.  The 32-bit windows
> >  do not overlap.  If apple_set_os is used, the eGPU is using the
> >  (non-overlapping) 32-bit window.  If apple_set_os is not used,
> >  the attached eGPU is using the (overlapping, hence broken) 64-bit window.
> >
> >  So not only is apple_set_os needed to keep the iGPU enabled,
> >  but also to ensure BIOS sets up bridge windows in a manner that is
> >  only halfway broken and not totally broken.
> >
> >  Below, 0000:06:01.0 and 0000:06:04.0 are the downstream ports on the
> >  Thunderbolt host controller and 0000:09:00.0 is the upstream port of
> >  the attached eGPU.
> >
> >  iGPU enabled, no eGPU attached (dmesg.txt):
> >  pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
> >  pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xb83fffff 64bit pref]
> >  pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
> >  pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]
> >
> >  iGPU disabled, eGPU attached, apple_set_os not used (journalctl.txt):
> >  pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
> >  pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xc6ffffff 64bit pref]
> >  pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
> >  pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]
> >  pci 0000:06:04.0: bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]: can't claim; address conflict with PCI Bus 0000:09 [mem 0xb1400000-0xbf3fffff 64bit pref]
> >
> >  iGPU enabled, eGPU attached, apple_set_os used (working-journalctl.txt):
> >  pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
> >  pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xc6ffffff 64bit pref]
> >  pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
> >  pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]
> >  pci 0000:09:00.0:   bridge window [mem 0x81900000-0x81cfffff]
> >
> > * As to how we can solve this and keep using apple_set_os only when
> >  necessary:
> >
> >  I note that on x86, the efistub walks over all PCI devices in the system
> >  (see setup_efi_pci() in drivers/firmware/efi/libstub/x86-stub.c) and
> >  retrieves the Device ID and Vendor ID.  We could additionally retrieve
> >  the Class Code and count the number of GPUs in the system by checking
> >  whether the Class Code matches PCI_BASE_CLASS_DISPLAY.  If there's
> >  at least 2 GPUs in the system, invoke apple_set_os.
>
> This also looks like a good idea, but I'm not well aware of the pci quirks in the Linux kernel. So, would consider it a bug report for the maintainers to fix.

That is not how it works.

This is not a regression in Linux, and even if it was, it is not the
maintainers' job to fix bugs.

If Linux is lacking functionality that you find important, please
propose a patch the implements it, and argue why it should be merged.

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

> On 25 Jul 2024, at 6:09 PM, Ard Biesheuvel <ardb@kernel.org> wrote:
> 
> On Wed, 24 Jul 2024 at 18:27, Aditya Garg <gargaditya08@live.com> wrote:
>> 
>> 
>> 
>>>> On 24 Jul 2024, at 9:31 PM, Lukas Wunner <lukas@wunner.de> wrote:
>>> 
>>> On Tue, Jul 23, 2024 at 04:25:19PM +0000, Aditya Garg wrote:
>>>>> On Wed, Jul 17, 2024 at 04:35:15PM +0000, Aditya Garg wrote:
>>>>> For the Macs having a single GPU, in case a person uses an eGPU,
>>>>> they still need this apple-set-os quirk for hybrid graphics.
>>>> 
>>>> Sending this message again as for some reason it got sent only to Lukas:
>>>> 
>>>> Full model name: Mac mini (2018) (Macmini8,1)
>>>> 
>>>> The drive link below has the logs:
>>>> 
>>>> https://drive.google.com/file/d/1P3-GlksU6WppvzvWC0A-nAoTZh7oPPxk/view?usp=drive_link
>>> 
>>> Some observations:
>>> 
>>> * dmesg-with-egpu.txt:  It seems the system was actually booted *without*
>>> an eGPU, so the filename appears to be a misnomer.
>>> 
>>> * The two files in the with_apple_set_os_efi directory only contain
>>> incomplete dmesg output.  Boot with log_buf_len=16M to solve this.
>>> Fortunately the truncated log is sufficient to see what's going on.
>>> 
>>> * If the apple_set_os protocol is not used, the attached eGPU is not
>>> enumerated by the kernel on boot and a rescan is required.
>>> So neither the iGPU nor the eGPU are working.  The reason is BIOS
>>> sets up incorrect bridge windows for the Thunderbolt host controller:
>>> Its two downstream ports' 64-bit windows overlap.  The 32-bit windows
>>> do not overlap.  If apple_set_os is used, the eGPU is using the
>>> (non-overlapping) 32-bit window.  If apple_set_os is not used,
>>> the attached eGPU is using the (overlapping, hence broken) 64-bit window.
>>> 
>>> So not only is apple_set_os needed to keep the iGPU enabled,
>>> but also to ensure BIOS sets up bridge windows in a manner that is
>>> only halfway broken and not totally broken.
>>> 
>>> Below, 0000:06:01.0 and 0000:06:04.0 are the downstream ports on the
>>> Thunderbolt host controller and 0000:09:00.0 is the upstream port of
>>> the attached eGPU.
>>> 
>>> iGPU enabled, no eGPU attached (dmesg.txt):
>>> pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
>>> pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xb83fffff 64bit pref]
>>> pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
>>> pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]
>>> 
>>> iGPU disabled, eGPU attached, apple_set_os not used (journalctl.txt):
>>> pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
>>> pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xc6ffffff 64bit pref]
>>> pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
>>> pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]
>>> pci 0000:06:04.0: bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]: can't claim; address conflict with PCI Bus 0000:09 [mem 0xb1400000-0xbf3fffff 64bit pref]
>>> 
>>> iGPU enabled, eGPU attached, apple_set_os used (working-journalctl.txt):
>>> pci 0000:06:01.0:   bridge window [mem 0x81900000-0x888fffff]
>>> pci 0000:06:01.0:   bridge window [mem 0xb1400000-0xc6ffffff 64bit pref]
>>> pci 0000:06:04.0:   bridge window [mem 0x88900000-0x8f8fffff]
>>> pci 0000:06:04.0:   bridge window [mem 0xb8400000-0xbf3fffff 64bit pref]
>>> pci 0000:09:00.0:   bridge window [mem 0x81900000-0x81cfffff]
>>> 
>>> * As to how we can solve this and keep using apple_set_os only when
>>> necessary:
>>> 
>>> I note that on x86, the efistub walks over all PCI devices in the system
>>> (see setup_efi_pci() in drivers/firmware/efi/libstub/x86-stub.c) and
>>> retrieves the Device ID and Vendor ID.  We could additionally retrieve
>>> the Class Code and count the number of GPUs in the system by checking
>>> whether the Class Code matches PCI_BASE_CLASS_DISPLAY.  If there's
>>> at least 2 GPUs in the system, invoke apple_set_os.
>> 
>> This also looks like a good idea, but I'm not well aware of the pci quirks in the Linux kernel. So, would consider it a bug report for the maintainers to fix.
> 
> That is not how it works.
> 
> This is not a regression in Linux, and even if it was, it is not the
> maintainers' job to fix bugs.
> 
> If Linux is lacking functionality that you find important, please
> propose a patch the implements it, and argue why it should be merged.

Hi Ard

I believe Linux needs the functionality to be able to properly use eGPUs on Macs. Since the data availability on these Macs is too low and not clear, I believe we should be atleast be able to enable this from userspace as well. Maybe we can do something like:

1. Add 'efi=enable_apple_set_os' and 'efi=disable_apple_set_os' to efi-stub-helper.c (I can make a patch for the same).
2. Use an NVRAM variable as suggested by you (Would need help for this).

If you find any of the option out of these acceptable, I'll start working on them.

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Orlando Chamberlain]

On Wed, 24 Jul 2024 18:01:08 +0200, Lukas Wunner Wrote:
> On Tue, Jul 23, 2024 at 04:25:19PM +0000, Aditya Garg wrote:
> > On Wed, Jul 17, 2024 at 04:35:15PM +0000, Aditya Garg wrote:
> > > For the Macs having a single GPU, in case a person uses an eGPU,
> > > they still need this apple-set-os quirk for hybrid graphics.
> > 
> > Sending this message again as for some reason it got sent only to
> > Lukas:
> > 
> > Full model name: Mac mini (2018) (Macmini8,1)
> > 
> > The drive link below has the logs:
> > 
> > https://drive.google.com/file/d/1P3-GlksU6WppvzvWC0A-nAoTZh7oPPxk/view?usp=drive_link
>
> Some observations:
>
> * dmesg-with-egpu.txt:  It seems the system was actually booted
>   *without* an eGPU, so the filename appears to be a misnomer.
> 
> * The two files in the with_apple_set_os_efi directory only contain
>   incomplete dmesg output.  Boot with log_buf_len=16M to solve this.
>   Fortunately the truncated log is sufficient to see what's going on.

Hi Lukas, in case it helps, I got the user with the macmini and egpu to
get logs that don't have the start cut off [0].

>   We could constrain apple_set_os to newer models by checking for
>   presence of the T2 PCI device [106b:1802].  Alternatively, we could
>   use the BIOS date (DMI_BIOS_DATE in SMBIOS data) to enforce a
>   cut-off such that only machines with a recent BIOS use apple_set_os.

It might be simpler to match "iBridge" in the DMI bios_version as this
indicates that a computer has the T2 (aka iBridge which runs bridgeOS).
I don't think there have been any issues from when the downstream T2
kernels had apple-set-os unconditionally so it should be fine to enable
for all T2 macs at least. This would exclude the T1 Macs with possibly
missing trackpad dimensions in applespi that you mentioned [1].

On my MacBookPro16,1:

$ cat /sys/class/dmi/id/bios_version
1715.60.5.0.0 (iBridge: 19.16.10647.0.0,0)

[0]: https://gist.github.com/Redecorating/8111324065016363223b5ce719e48676/
[1]: https://lore.kernel.org/all/ZoJPgSlZJ3ZlU2zL@wunner.de/

--

Regards,

Orlando

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Ard Biesheuvel]

On Sun, 28 Jul 2024 at 14:03, Orlando Chamberlain
<orlandoch.dev@gmail.com> wrote:
>
> On Wed, 24 Jul 2024 18:01:08 +0200, Lukas Wunner Wrote:
> > On Tue, Jul 23, 2024 at 04:25:19PM +0000, Aditya Garg wrote:
> > > On Wed, Jul 17, 2024 at 04:35:15PM +0000, Aditya Garg wrote:
> > > > For the Macs having a single GPU, in case a person uses an eGPU,
> > > > they still need this apple-set-os quirk for hybrid graphics.
> > >
> > > Sending this message again as for some reason it got sent only to
> > > Lukas:
> > >
> > > Full model name: Mac mini (2018) (Macmini8,1)
> > >
> > > The drive link below has the logs:
> > >
> > > https://drive.google.com/file/d/1P3-GlksU6WppvzvWC0A-nAoTZh7oPPxk/view?usp=drive_link
> >
> > Some observations:
> >
> > * dmesg-with-egpu.txt:  It seems the system was actually booted
> >   *without* an eGPU, so the filename appears to be a misnomer.
> >
> > * The two files in the with_apple_set_os_efi directory only contain
> >   incomplete dmesg output.  Boot with log_buf_len=16M to solve this.
> >   Fortunately the truncated log is sufficient to see what's going on.
>
> Hi Lukas, in case it helps, I got the user with the macmini and egpu to
> get logs that don't have the start cut off [0].
>
> >   We could constrain apple_set_os to newer models by checking for
> >   presence of the T2 PCI device [106b:1802].  Alternatively, we could
> >   use the BIOS date (DMI_BIOS_DATE in SMBIOS data) to enforce a
> >   cut-off such that only machines with a recent BIOS use apple_set_os.
>
> It might be simpler to match "iBridge" in the DMI bios_version as this
> indicates that a computer has the T2 (aka iBridge which runs bridgeOS).
> I don't think there have been any issues from when the downstream T2
> kernels had apple-set-os unconditionally so it should be fine to enable
> for all T2 macs at least. This would exclude the T1 Macs with possibly
> missing trackpad dimensions in applespi that you mentioned [1].
>
> On my MacBookPro16,1:
>
> $ cat /sys/class/dmi/id/bios_version
> 1715.60.5.0.0 (iBridge: 19.16.10647.0.0,0)
>
> [0]: https://gist.github.com/Redecorating/8111324065016363223b5ce719e48676/
> [1]: https://lore.kernel.org/all/ZoJPgSlZJ3ZlU2zL@wunner.de/
>

Thanks, this seems useful.

Does this mean we can drop the type1 product name list, and just look
for 'iBridge' in the type0 version string? Or does that list contain
non-T2 hardware as well?

Re: [PATCH v3 0/2] efi/x86: Call set_os() protocol on dual GPU Macs

[Aditya Garg]

> Thanks, this seems useful.
> 
> Does this mean we can drop the type1 product name list, and just look
> for 'iBridge' in the type0 version string? Or does that list contain
> non-T2 hardware as well?

MacBookPro11,3
MacBookPro11,5
MacBookPro13,3
MacBookPro14,3

These are non T2 Macs that were in the previous list