On Fri, May 22, 2020 at 05:23:31PM +0000, Derrick, Jonathan wrote: > On Fri, 2020-05-01 at 11:35 -0600, Jonathan Derrick wrote: > > On Fri, 2020-05-01 at 12:16 -0500, Bjorn Helgaas wrote: > > > On Thu, Apr 30, 2020 at 12:46:07PM -0600, Jon Derrick wrote: > > > > Hi Bjorn & Kuppuswamy, > > > > > > > > I see a problem in the DPC ECN [1] to _OSC in that it doesn't > > > > give us a way to determine if firmware supports _OSC DPC > > > > negotation, and therefore how to handle DPC. > > > > > > > > Here is the wording of the ECN that implies that Firmware > > > > without _OSC DPC negotiation support should have the OSPM rely > > > > on _OSC AER negotiation when determining DPC control: > > > > > > > > PCIe Base Specification suggests that Downstream Port > > > > Containment may be controlled either by the Firmware or the > > > > Operating System. It also suggests that the Firmware retain > > > > ownership of Downstream Port Containment if it also owns > > > > AER. When the Firmware owns Downstream Port Containment, it > > > > is expected to use the new "Error Disconnect Recover" > > > > notification to alert OSPM of a Downstream Port Containment > > > > event. > > > > > > > > In legacy platforms, as bits in _OSC are reserved prior to > > > > implementation, ACPI Root Bus enumeration will mark these Host > > > > Bridges as without Native DPC support, even though the > > > > specification implies it's expected that AER _OSC negotiation > > > > determines DPC control for these platforms. There seems to be > > > > a need for a way to determine if the DPC control bit in _OSC > > > > is supported and fallback on AER otherwise. > > > > > > > > > > > > Currently portdrv assumes DPC control if the port has Native > > > > AER services: > > > > > > > > static int get_port_device_capability(struct pci_dev *dev) > > > > ... > > > > if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) && > > > > pci_aer_available() && > > > > (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER))) > > > > services |= PCIE_PORT_SERVICE_DPC; > > > > > > > > Newer firmware may not grant OSPM DPC control, if for > > > > instance, it expects to use Error Disconnect Recovery. However > > > > it looks like ACPI will use DPC services via the EDR driver, > > > > without binding the full DPC port service driver. > > > > > > > > > > > > If we change portdrv to probe based on host->native_dpc and > > > > not AER, then we break instances with legacy firmware where > > > > OSPM will clear host->native_dpc solely due to _OSC bits being > > > > reserved: > > > > > > > > struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, > > > > ... > > > > if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL)) > > > > host_bridge->native_dpc = 0; > > > > > > > > > > > > > > > > So my assumption instead is that host->native_dpc can be 0 and > > > > expect Native DPC services if AER is used. In other words, if > > > > and only if DPC probe is invoked from portdrv, then it needs > > > > to rely on the AER dependency. Otherwise it should be assumed > > > > that ACPI set up DPC via EDR. This covers legacy firmware. > > > > > > > > However it seems like that could be trouble with newer > > > > firmware that might give OSPM control of AER but not DPC, and > > > > would result in both Native DPC and EDR being in effect. > > > > > > > > > > > > Anyways here are two patches that give control of AER and DPC > > > > on the results of _OSC. They don't mess with the HEST parser > > > > as I expect those to be removed at some point. I need these > > > > for VMD support which doesn't even rely on _OSC, but I suspect > > > > this won't be the last effort as we detangle Firmware First. > > > > > > > > [1] https://members.pcisig.com/wg/PCI-SIG/document/12888 > > > > > > Hi Jon, I think we need to sort out the _OSC/FIRMWARE_FIRST patches > > > from Alex and Sathy first, then see what needs to be done on top of > > > those, so I'm going to push these off for a few days and they'll > > > probably need a refresh. > > > > > > Bjorn > > > > Agreed, no need to merge now. Just wanted to bring up the DPC > > ambiguity, which I think was first addressed by dpc-native: > > > > commit 35a0b2378c199d4f26e458b2ca38ea56aaf2d9b8 > > Author: Olof Johansson <olof@xxxxxxxxx> > > Date: Wed Oct 23 12:22:05 2019 -0700 > > > > PCI/DPC: Add "pcie_ports=dpc-native" to allow DPC without AER control > > > > Prior to eed85ff4c0da7 ("PCI/DPC: Enable DPC only if AER is available"), > > Linux handled DPC events regardless of whether firmware had granted it > > ownership of AER or DPC, e.g., via _OSC. > > > > PCIe r5.0, sec 6.2.10, recommends that the OS link control of DPC to > > control of AER, so after eed85ff4c0da7, Linux handles DPC events only if it > > has control of AER. > > > > On platforms that do not grant OS control of AER via _OSC, Linux DPC > > handling worked before eed85ff4c0da7 but not after. > > > > To make Linux DPC handling work on those platforms the same way they did > > before, add a "pcie_ports=dpc-native" kernel parameter that makes Linux > > handle DPC events regardless of whether it has control of AER. > > > > [bhelgaas: commit log, move pcie_ports_dpc_native to drivers/pci/] > > Link: https://lore.kernel.org/r/20191023192205.97024-1-olof@xxxxxxxxx > > Signed-off-by: Olof Johansson <olof@xxxxxxxxx> > > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > Are you still thinking about removing the HEST parser? > > For VMD we still need the ability to bind DPC if native_dpc==1. > I think if we can do that, this set should still pretty much still > apply with a modification to patch 2 to allow matching > pcie_ports_dpc_native in dpc_probe. Yes, I think we should remove the HEST firmware-first parsing, because IIRC the spec really doesn't specify any action the OS should take based on it. I was thinking Sathy might update the patch, and it fell off my radar. Bjorn
