Skip to contentSkip to conversation

ACPI: x86: Apple T2 systems need early CPU offlining

PATCH In review Andre Eikmeyer 3 participants 4 messages Aug 12 – Nov 5 ACPI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ACPI: x86: Apple T2 systems need early CPU offlining

Hello everyone,

Linux reports _OSI("Darwin") on x86 Apple systems. On T2 Macs, the selected firmware suspend path makes secondary CPU startup during early resume take several seconds per CPU. The same CPUs can be brought online normally after platform resume.

We therefore move secondary CPU hotplug outside the generic CPU PM notifier window on T2 systems. A prepare notifier runs before the CPU core blocks hotplug, while a post notifier restores only the CPUs it removed after the core enables hotplug again.

This reduces CPU bring-up during resume from several seconds per CPU to a fraction of a second. The change was tested on MacBookPro15,1, MacBookPro16,2, MacBookAir9,1 and a 27-inch T2 iMac.

---
 drivers/acpi/x86/apple.c | 140 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)
drivers/acpi/x86/apple.c +140 -0
index 45d0f16..5595a6a 100644
@@ -6,7 +6,13 @@
 
 #include <linux/acpi.h>
 #include <linux/bitmap.h>
+#include <linux/cpu.h>
+#include <linux/cpuhplock.h>
+#include <linux/init.h>
+#include <linux/notifier.h>
+#include <linux/pci.h>
 #include <linux/platform_data/x86/apple.h>
+#include <linux/suspend.h>
 #include <linux/uuid.h>
 #include "../internal.h"
 
@@ -146,3 +152,137 @@ out_free:
 	ACPI_FREE(props);
 	bitmap_free(valid);
 }
+
+#ifdef CONFIG_PM_SLEEP_SMP
+/*
+ * The ACPI path selected by _OSI("Darwin") leaves Apple T2 systems in a
+ * state where bringing secondary CPUs online during early resume may take
+ * several seconds per CPU. Normal CPU hotplug after platform resume is not
+ * affected, so we move it outside the generic suspend CPU hotplug window.
+ */
+static cpumask_var_t apple_t2_offlined_cpus;
+
+#define PCI_DEVICE_ID_APPLE_T2_BRIDGE 0x1801
+
+static bool __init apple_t2_present(void)
+{
+	struct pci_dev *pdev;
+
+	if (!x86_apple_machine)
+		return false;
+
+	pdev = pci_get_device(PCI_VENDOR_ID_APPLE,
+			      PCI_DEVICE_ID_APPLE_T2_BRIDGE, NULL);
+	if (!pdev)
+		return false;
+
+	pci_dev_put(pdev);
+	return true;
+}
+
+static void apple_t2_restore_cpus(void)
+{
+	unsigned int cpu;
+	int ret;
+
+	for_each_cpu(cpu, apple_t2_offlined_cpus) {
+		ret = add_cpu(cpu);
+		if (ret) {
+			pr_err("ACPI: Apple T2 failed to restore CPU%u: %d\n",
+			       cpu, ret);
+			continue;
+		}
+
+		cpumask_clear_cpu(cpu, apple_t2_offlined_cpus);
+	}
+}
+
+static void apple_t2_offline_cpus(void)
+{
+	unsigned int cpu;
+	int ret;
+
+	if (!cpumask_empty(apple_t2_offlined_cpus)) {
+		pr_err("ACPI: Apple T2 CPUs from the previous suspend remain offline\n");
+		apple_t2_restore_cpus();
+		if (!cpumask_empty(apple_t2_offlined_cpus)) {
+			pr_err("ACPI: Apple T2 early CPU offlining skipped\n");
+			return;
+		}
+	}
+
+	for_each_online_cpu(cpu) {
+		if (cpu == 0)
+			continue;
+
+		ret = remove_cpu(cpu);
+		if (ret) {
+			pr_err("ACPI: Apple T2 failed to offline CPU%u: %d\n",
+			       cpu, ret);
+			continue;
+		}
+
+		cpumask_set_cpu(cpu, apple_t2_offlined_cpus);
+	}
+}
+
+static int apple_t2_cpu_prepare(struct notifier_block *nb,
+				unsigned long action, void *unused)
+{
+	if (action == PM_SUSPEND_PREPARE)
+		apple_t2_offline_cpus();
+
+	return NOTIFY_OK;
+}
+
+static int apple_t2_cpu_restore(struct notifier_block *nb,
+				unsigned long action, void *unused)
+{
+	if (action == PM_POST_SUSPEND)
+		apple_t2_restore_cpus();
+
+	return NOTIFY_OK;
+}
+
+/*
+ * The CPU core PM notifier runs at priority 0. We offline CPUs before
+ * hotplug is blocked, then restore them after it enables hotplug again.
+ */
+static struct notifier_block apple_t2_cpu_prepare_nb = {
+	.notifier_call = apple_t2_cpu_prepare,
+	.priority = 1,
+};
+
+static struct notifier_block apple_t2_cpu_restore_nb = {
+	.notifier_call = apple_t2_cpu_restore,
+	.priority = -1,
+};
+
+static int __init apple_t2_cpu_pm_init(void)
+{
+	int ret;
+
+	if (!apple_t2_present())
+		return 0;
+
+	if (!alloc_cpumask_var(&apple_t2_offlined_cpus, GFP_KERNEL))
+		return -ENOMEM;
+
+	ret = register_pm_notifier(&apple_t2_cpu_prepare_nb);
+	if (ret)
+		goto free_mask;
+
+	ret = register_pm_notifier(&apple_t2_cpu_restore_nb);
+	if (ret)
+		goto unregister_prepare;
+
+	return 0;
+
+unregister_prepare:
+	unregister_pm_notifier(&apple_t2_cpu_prepare_nb);
+free_mask:
+	free_cpumask_var(apple_t2_offlined_cpus);
+	return ret;
+}
+late_initcall(apple_t2_cpu_pm_init);
+#endif
Signature · 1 line
2.55.0
#1 lore raw

[cc += Chen Yu, start of thread is here:
https://lore.kernel.org/all/[email protected]/
]

Quoted 3 lines · Andre Eikmeyer

Linux reports _OSI("Darwin") on x86 Apple systems. On T2 Macs, the selected firmware suspend path makes secondary CPU startup during early resume take several seconds per CPU.

That's not a T2-specific issue. It occurs on older Intel Macs as well.

Back in 2018, Chen Yu root-caused it to an invalid MTRR upon resume from system sleep:

https://lore.kernel.org/all/[email protected]/

I've been using his patch for 8 years to reduce resume time on my MacBookPro9,1. Unfortunately it was never applied upstream for reasons unknown. It probably just slipped through the cracks.

You may want to test if his patch helps on T2 models as well. I think it's a more generic solution to the problem than the one you're proposing.

Thanks,

Lukas

#2 lore raw

Thanks Lukas for bringing this up,

Quoted 16 lines · Lukas Wunner

[cc += Chen Yu, start of thread is here:
https://lore.kernel.org/all/[email protected]/
]

Quoted 3 lines · Andre Eikmeyer

Linux reports _OSI("Darwin") on x86 Apple systems. On T2 Macs, the selected firmware suspend path makes secondary CPU startup during early resume take several seconds per CPU.

That's not a T2-specific issue. It occurs on older Intel Macs as well.

Back in 2018, Chen Yu root-caused it to an invalid MTRR upon resume from system sleep:

https://lore.kernel.org/all/[email protected]/

Quoted 8 lines

I've been using his patch for 8 years to reduce resume time on my MacBookPro9,1. Unfortunately it was never applied upstream for reasons unknown. It probably just slipped through the cracks.

You may want to test if his patch helps on T2 models as well. I think it's a more generic solution to the problem than the one you're proposing.

Glad it helped! At that time, it seemed that the issue might have been related to broken Mac platforms, and we were changing the generic x86 path. Maybe we can wait for Andre's feedback, I can try again / or assist Andre - or just keep it out-of-tree; that's also fine.

thanks,
Chenyu

#3 lore raw
Quoted 5 lines · Yu C

Glad it helped! At that time, it seemed that the issue might have been related to broken Mac platforms, and we were changing the generic x86 path. Maybe we can wait for Andre's feedback, I can try again / or assist Andre - or just keep it out-of-tree; that's also fine.

And you waited for quite a long time! Sorry for that. I had to fix other things on our cursed Macs before being able to investigate this further. Hybryd graphics offload etc... And in the meanwhile, I forgot about.

First of all, I tried Chenyu's patch and I can confirm it works as good as mine and as such we can forget about my bloaty early offlining.

But! The actual cause for slow smpboot is simply thunderbolt pm on suspend. We don't have that issue when using pcie_ports=compat or acpi_osi=!Darwin. It happens when using pcie_ports=native and also without param. The slow EC-unblock is caused by aer. pci=noaer solves that reliably. I could not find aer to be used in macos anywhere, so I think Apple doesn't use it.

Well, however - OSDW acpi tables are significantly larger and more complicated than !OSDW, so here is a bit of work to do for us in thunderbolt.c.
First attempts are looking promising. Resume is even
faster than with pci_ports=compat. A second or so. But we are struggling here and there. Especially with rtd3 and such. I would like to have this working before submitting.

André