On Thu, Mar 24, 2022 at 6:15 PM Limonciello, Mario <Mario.Limonciello@xxxxxxx> wrote: > > [Public] > > > -----Original Message----- > > From: Bjorn Helgaas <helgaas@xxxxxxxxxx> > > Sent: Thursday, March 24, 2022 11:35 > > To: Limonciello, Mario <Mario.Limonciello@xxxxxxx> > > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; open list:PCI SUBSYSTEM <linux- > > pci@xxxxxxxxxxxxxxx>; Linux PM <linux-pm@xxxxxxxxxxxxxxx>; Mehta, Sanju > > <Sanju.Mehta@xxxxxxx>; Rafael J. Wysocki <rafael@xxxxxxxxxx>; Mika > > Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > > Subject: Re: [PATCH v4] PCI / ACPI: Assume `HotPlugSupportInD3` only if > > device can wake from D3 > > > > [+cc Mika, "HotPlugSupportInD3" scope question below] > > FYI - Mika had approved some earlier versions of this, so I expect conceptual > Alignment at least with the latest one. > > <snip> > > > > > Can we at least list some common scenarios? E.g., it affects > > > > kernels after commit X, or it affects machines with CPUs newer > > > > than Y, or it affects a certain kind of tunneling, etc? Distros > > > > need this information so they can figure whether and how far to > > > > backport things like this. > > > > > > It's going to affect any retail machine with the SOC we refer to in > > > the kernel as "Yellow Carp". This is one of the first non-Intel > > > USB4 hosts and will be using the USB4 SW CM in the kernel. > > > > > > Without this change, effectively PCIe tunneling will not work when > > > any downstream PCIe device is hotplugged. In the right > > > circumstances it might work if it's coldbooted (if the paths > > > selected by the pre-boot firmware connection manager are identical > > > to that selected by SW CM). > > > > Thanks a lot for this context! As far as I can tell from grubbing > > through the git history, there are no PCI, USB4, or Thunderbolt > > changes related to Yellow Carp, so I assume this has to do with Yellow > > Carp firmware doing things differently than previous platforms. > > There have been a variety of Thunderbolt/USB4 changes as a result of > Yellow Carp development and findings, but they have not been quirks; > they have been done as generic changes that still make sense for all > USB4 devices. > > Sanju (on CC) has submitted a majority of these, so if you want to see > a sense of what these are you can look for his commits in drivers/thunderbolt. > > > > > Previously, if a Root Port implemented the HotPlugSupportInD3 > > property, we assumed that the Root Port and any downstream bridges > > could handle hot-plug events while in D3hot. > > > > I guess the difference here is that on Yellow Carp firmware, even if > > there is a HotPlugSupportInD3 property on the Root Port, the Root Port > > cannot handle hot-plug events in D3hot UNLESS there is also an _S0W > > method AND that _S0W says the Root Port can wakeup from D3hot or > > D3cold, right? > > Yes, correct! > > > > > I have some heartburn about this that's only partly related to this > > patch. The Microsoft doc clearly says "HotPlugSupportInD3" must be > > implemented on Root Ports and its presence tells us that the *Root > > Port* can handle hot-plug events while in D3. > > > > But acpi_pci_bridge_d3() looks up the Root Port at the top of the > > hierarchy and assume that its "HotPlugSupportInD3" applies to any > > switch ports that may be below that Root Port (added by 26ad34d510a8 > > ("PCI / ACPI: Whitelist D3 for more PCIe hotplug ports") [1]). Not really. "HotPlugSupportInD3" applies to the root port only and the platform firmware may not know about any ports below it. However, the presence of "HotPlugSupportInD3" is used as an indicator that the entire hierarchy is "D3cold-aware", so to speak. Essentially, this boils down to the "Is the hardware modern enough?" consideration and the answer is assumed to be "yes" if the property in question is present for the root port. But if "HotPlugSupportInD3" is not consistent with the other pieces of information regarding the root port available from the firmware (_PRW and _S0W in this particular case), the presence of it is questionable in the first place, so IMO the approach here makes sense.