Linux USB
 help / color / mirror / Atom feed
* Re: [PATCH] ACPI: PCI: take native PME control on Apple machines
       [not found] <20260924132859.24751-1-fbeltranmillalen@gmail.com>
@ 2026-10-08 23:29 ` Bjorn Helgaas
  2026-10-09  8:23   ` Matthew Garrett
  0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Helgaas @ 2026-10-08 23:29 UTC (permalink / raw)
  To: Francisco Beltrán Millalén
  Cc: rafael, bhelgaas, linux-acpi, linux-pci, lenb, linux-kernel,
	Darrell Gum, Matthew Garrett, Andreas Noever, Mika Westerberg,
	Yehezkel Bernat, Lukas Wunner, linux-usb

[+cc Matthew (author of 7bc5a2bad0b8), Darrell, Andreas, Mika,
Yehezkel, Lukas, linux-usb]

On Thu, Sep 24, 2026 at 10:28:59AM -0300, Francisco Beltrán Millalén wrote:
> Since commit 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly")
> negotiate_os_control() skips the _OSC handshake on Apple machines and
> hardcodes the set of features the OS takes over, because those firmwares
> return failure once _OSI("Darwin") has been acknowledged.  The hardcoded
> set is ~OSC_PCI_EXPRESS_PME_CONTROL, i.e. everything except PME.  No
> reason was given for leaving PME out.

It's clear there *was* a reason, but I don't know either.  Maybe
Matthew will remember?

> The result is that host_bridge->native_pme stays 0, so portdrv never
> registers the PME service for any root port.  A PCIe device that
> runtime suspends still arms PME# (the PCI core enables it because
> pci_dev_run_wake() only looks at capability), asserts it when it needs
> attention, and is never resumed: no service handles the root port
> interrupt, and the polling fallback in pci_pme_list_scan() skips the
> device whenever the bridge above it is runtime suspended, which is
> exactly when the device itself is suspended.
> 
> On a MacBookPro14,3 this silently breaks USB-C hotplug.  Both xHCI
> controllers sit inside the Thunderbolt (Alpine Ridge) switches and
> runtime suspend to D3hot when idle, together with their parent
> bridges.  Plugging a SuperSpeed device in is then never noticed:
> 
>   0000:7d:00.0 suspended, PMCSR 0x8103 (D3hot, PME_En=1, PME_Status=1)
>   ... no kernel message at all, for minutes ...
> 
> The device only appears once something else resumes the controller,
> which makes it look intermittent.  The firmware on this machine does not
> handle PME on behalf of the OS either: XHC2/XHC3 declare _PRW returning
> GPE 0x6D, that GPE is enabled and has never fired, and its _L6D handler
> notifies PWRB, XHC1 and HDEF only - never XHC2 or XHC3.
> 
> With PME control taken, the root port PME interrupt is serviced and the
> controller is resumed as soon as something is plugged in.
> 
> Use calculate_control() for the assumed set rather than open coding it:
> that is by definition the set that would have been requested, it drops a
> magic constant, and on a kernel built without some of those features it
> no longer claims control of services the OS cannot provide.
> 
> Note this also makes pme_is_native() true for pciehp (drivers/pci/
> hotplug/pciehp_core.c), so hotplug ports now disable their interrupt
> across suspend and clear spurious events on resume, as they do on every
> non-Apple machine.  x86_apple_machine covers Macs back to 2006, so this
> reaches models that were never tested with native PME.
> 
> Tested on a MacBookPro14,3, 6.18.49, with a USB 3.1 SATA enclosure:
> 
>   - before: plugged in at 09:20 with 0000:7d:00.0 runtime suspended, still
>     invisible at 09:21 with PME_Status latched at 1; only appeared when
>     0000:7d:00.0 was forced to power/control=on.
>   - with pcie_ports=native (which reaches the same code path in
>     get_port_device_capability()): pcie_pme bound to 5 root ports, and the
>     same enclosure enumerated in the same second it was plugged in, 66 s
>     after the controller had suspended, with nothing forced.
>   - with this patch, nothing forced: pcie_pme bound to 5 root ports and the
>     _OSC line now reads "OS assumes control of [PCIeHotplug SHPCHotplug PME
>     AER PCIeCapability LTR DPC]".  The enclosure was unplugged at 20:42:13,
>     0000:7d:00.0 runtime suspended to D3hot, and on plugging it back in at
>     20:43:06 (53 s later) the root port took a PME interrupt and the device
>     enumerated in the same second.
>   - wifi, internal keyboard, touchbar and audio all came up; 66 warnings
>     and 45 errors during boot, against 65/43 on the distribution kernel.
> 
> One rough edge, reproducible on both boots: the port driver logs
> "PME: Spurious native interrupt!" right before the device is resumed.
> pcie_pme_handle_request() falls back to pcie_pme_walk_bus() when the root
> port reports its own requester ID without PCI_PM_CTRL_PME_STATUS set, and
> that walk skips PCIe devices, so the xHCI is never attributed.  The device
> is still resumed and the wakeup works, but attributing PMEs from PCIe
> devices behind a switch looks like a separate shortcoming; it is not
> addressed here.
> 
> Boot with pcie_ports=native also showed no new AER/DPC activity and
> slightly fewer warnings than a boot without it, so taking the remaining
> control bit does not appear to disturb anything else on this machine.
> 
> Fixes: 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly")
> Signed-off-by: Francisco Beltrán Millalén <fbeltranmillalen@gmail.com>

I don't object from a PCI standpoint, but Rafael generally takes care
of acpi/pci_root.c.  I would again suggest putting some of the commit
log detail below "---" and relying on the https://patch.msgid.link/
tag Rafael adds.

> ---
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -568,9 +568,14 @@
>  	 * Apple always return failure on _OSC calls when _OSI("Darwin") has
>  	 * been called successfully. We know the feature set supported by the
>  	 * platform, so avoid calling _OSC at all
> +	 *
> +	 * Assume control of the same features that would have been requested,
> +	 * PCIe PME included.  Leaving PME out disables the PME service for
> +	 * every root port, and nothing takes over: a device that runtime
> +	 * suspends asserts PME# and is never resumed.
>  	 */
>  	if (x86_apple_machine) {
> -		root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
> +		root->osc_control_set = calculate_control();
>  		decode_osc_control(root, "OS assumes control of",
>  				   root->osc_control_set);
>  		return;

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ACPI: PCI: take native PME control on Apple machines
  2026-10-08 23:29 ` [PATCH] ACPI: PCI: take native PME control on Apple machines Bjorn Helgaas
@ 2026-10-09  8:23   ` Matthew Garrett
  2026-10-09 12:14     ` Francisco Beltrán Millalén
  0 siblings, 1 reply; 3+ messages in thread
From: Matthew Garrett @ 2026-10-09  8:23 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Francisco Beltrán Millalén, rafael, bhelgaas,
	linux-acpi, linux-pci, lenb, linux-kernel, Darrell Gum,
	Andreas Noever, Mika Westerberg, Yehezkel Bernat, Lukas Wunner,
	linux-usb

On Thu, Oct 08, 2026 at 06:29:12PM -0500, Bjorn Helgaas wrote:
> [+cc Matthew (author of 7bc5a2bad0b8), Darrell, Andreas, Mika,
> Yehezkel, Lukas, linux-usb]
> 
> On Thu, Sep 24, 2026 at 10:28:59AM -0300, Francisco Beltrán Millalén wrote:
> > Since commit 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly")
> > negotiate_os_control() skips the _OSC handshake on Apple machines and
> > hardcodes the set of features the OS takes over, because those firmwares
> > return failure once _OSI("Darwin") has been acknowledged.  The hardcoded
> > set is ~OSC_PCI_EXPRESS_PME_CONTROL, i.e. everything except PME.  No
> > reason was given for leaving PME out.
> 
> It's clear there *was* a reason, but I don't know either.  Maybe
> Matthew will remember?

I /think/ this was the set of parameters that my machine would hand over 
via _OSC if we hadn't called _OSI("Darwin") - but it was over a decade 
ago so I can't promise that's the case. I think this probably needs to 
be tested on some older machines as well just to ensure there's no weird 
breakage as a result, but if PME is actually hooked up and there's no 
SMM managing it instead I think that's fine.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ACPI: PCI: take native PME control on Apple machines
  2026-10-09  8:23   ` Matthew Garrett
@ 2026-10-09 12:14     ` Francisco Beltrán Millalén
  0 siblings, 0 replies; 3+ messages in thread
From: Francisco Beltrán Millalén @ 2026-10-09 12:14 UTC (permalink / raw)
  To: mjg59
  Cc: helgaas, rafael, bhelgaas, linux-acpi, linux-pci, lenb,
	linux-kernel, d, andreas.noever, westeri, YehezkelShB, lukas,
	linux-usb

Hi Matthew,

Thanks for going back that far, and for the explanation.

On Fri, Oct 09, 2026 at 01:23:08AM -0700, Matthew Garrett wrote:
> I /think/ this was the set of parameters that my machine would hand over
> via _OSC if we hadn't called _OSI("Darwin") - but it was over a decade
> ago so I can't promise that's the case. I think this probably needs to
> be tested on some older machines as well just to ensure there's no weird
> breakage as a result, but if PME is actually hooked up and there's no
> SMM managing it instead I think that's fine.

On the MacBookPro14,3 both conditions seem to hold.  With the patch, the
root port takes the PME interrupt as soon as a device is plugged in, and
the controller is resumed in the same second.  Without it, nothing picks
the event up: the controller stays suspended for over a minute with
PME_Status set, the GPE the firmware declares for those two xHCI
controllers never fires, and its handler doesn't notify them anyway.  I
can't look at SMM directly, but if the firmware were handling the event
itself, the controller would have been woken without the patch, and it
never was.  Darrell saw the same fix on his MacBookPro14,3.

You're right that older Macs should be tested, and I don't have any: the
patch has only run on my machine and Darrell's, both MacBookPro14,3.
Two things make me think the risk is small.  Handling PME natively
doesn't turn off the firmware's own wake path, so if an older Mac does
use a GPE for this, that keeps working.  The only side effect I would
expect is the PME driver logging "Spurious native interrupt".  And on
these machines the kernel already takes over hotplug and AER without
asking the firmware, so PME would be treated the same way as the rest.
If anyone on Cc has an older Mac and can try it, I'd be glad to hear how
it goes.

Thanks again,
Francisco

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-09 12:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260924132859.24751-1-fbeltranmillalen@gmail.com>
2026-10-08 23:29 ` [PATCH] ACPI: PCI: take native PME control on Apple machines Bjorn Helgaas
2026-10-09  8:23   ` Matthew Garrett
2026-10-09 12:14     ` Francisco Beltrán Millalén

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox