On Sat, Jan 25, 2020 at 03:25:00PM -0800, Kuppuswamy Sathyanarayanan wrote: > On Fri, Jan 24, 2020 at 09:04:50AM -0600, Bjorn Helgaas wrote: > > On Sat, Jan 18, 2020 at 08:00:33PM -0800, sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx wrote: > > > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> > > > > > > As per ACPI specification r6.3, sec 5.6.6, when firmware owns Downstream > > > Port Containment (DPC), its expected to use the "Error Disconnect > > > Recover" (EDR) notification to alert OSPM of a DPC event and if OS > > > supports EDR, its expected to handle the software state invalidation and > > > port recovery in OS, and also let firmware know the recovery status via > > > _OST ACPI call. Related _OST status codes can be found in ACPI > > > specification r6.3, sec 6.3.5.2. > > > > > > Also, as per PCI firmware specification r3.2 Downstream Port Containment > > > Related Enhancements ECN, sec 4.5.1, table 4-6, If DPC is controlled by > > > firmware (firmware first mode), firmware is responsible for > > > configuring the DPC and OS is responsible for error recovery. Also, OS > > > is allowed to modify DPC registers only during the EDR notification > > > window. So with EDR support, OS should provide DPC port services even in > > > firmware first mode. > ... > > > + acpi_status astatus; > > > + > > > + dpc->adev = adev; > > > + > > > + astatus = acpi_install_notify_handler(adev->handle, > > > + ACPI_SYSTEM_NOTIFY, > > > + edr_handle_event, > > > + dpc); > > > > I think there are a couple issues with the ECN here: > > > > - The ECN allows EDR notifications on host bridges (sec 4.5.1, table > > 4-4), but it does not allow the "Locate" _DSM under host bridges > > (sec 4.6.13). > > > > - The ECN allows EDR notifications on root ports or switch ports > > that do not support DPC (sec 4.5.1), but it does not allow the > > "Locate" _DSM on those ports (sec 4.6.13). > > Can you please give more details on where its mentioned? Following is > the copy of "Locate" _DSM location related specification. All it says is, > this object can be placed under any object representing root port or > switch port. It does not seem to add any restrictions. Please let me know > your comments. > > Location: > This object can be placed under any object representing a DPC capable > PCI Express Root Port or Switch Downstream Port. If a port implements > this DSM, its child devices cannot instantiate this DSM function You quoted it: "This object [the 'Locate' _DSM] can be placed under any object representing a *DPC capable* PCI Express Root Port or Switch Downstream Port." If the intention was to allow the Locate _DSM for *any* root ports or switch downstream ports, the spec should not include the "DPC capable" restriction. Bjorn