linux-kernel - Re: [PATCH v13 4/8] PCI/DPC: Add Error Disconnect Recover (EDR) support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200127135017.GA260782@google.com>
Date:   Mon, 27 Jan 2020 07:50:17 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc:     linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        ashok.raj@...el.com, Keith Busch <keith.busch@...el.com>,
        Huong Nguyen <huong.nguyen@...l.com>,
        Austin Bolen <Austin.Bolen@...l.com>
Subject: Re: [PATCH v13 4/8] PCI/DPC: Add Error Disconnect Recover (EDR)
 support

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@...ux.intel.com wrote:
> > > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
> > > 
> > > 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