All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Kuppuswamy Sathyanarayanan  <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	ashok.raj@intel.com, Keith Busch <keith.busch@intel.com>,
	Huong Nguyen <huong.nguyen@dell.com>,
	Austin Bolen <Austin.Bolen@dell.com>
Subject: Re: [PATCH v13 4/8] PCI/DPC: Add Error Disconnect Recover (EDR) support
Date: Mon, 27 Jan 2020 07:50:17 -0600	[thread overview]
Message-ID: <20200127135017.GA260782@google.com> (raw)
In-Reply-To: <20200125232500.GA112031@skuppusw-desk.amr.corp.intel.com>

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

  reply	other threads:[~2020-01-27 13:50 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-19  4:00 [PATCH v13 0/8] Add Error Disconnect Recover (EDR) support sathyanarayanan.kuppuswamy
2020-01-19  4:00 ` [PATCH v13 1/8] PCI/ERR: Update error status after reset_link() sathyanarayanan.kuppuswamy
2020-02-05 18:28   ` Kuppuswamy Sathyanarayanan
2020-02-05 19:03     ` Bjorn Helgaas
2020-01-19  4:00 ` [PATCH v13 2/8] PCI/DPC: Allow dpc_probe() even if firmware first mode is enabled sathyanarayanan.kuppuswamy
2020-01-22 23:17   ` Bjorn Helgaas
2020-01-23  0:42     ` Kuppuswamy Sathyanarayanan
2020-01-23  3:10       ` Bjorn Helgaas
2020-01-23 18:04         ` Kuppuswamy Sathyanarayanan
2020-01-19  4:00 ` [PATCH v13 3/8] PCI/DPC: Add dpc_process_error() wrapper function sathyanarayanan.kuppuswamy
2020-01-19  4:00 ` [PATCH v13 4/8] PCI/DPC: Add Error Disconnect Recover (EDR) support sathyanarayanan.kuppuswamy
2020-01-24 15:04   ` Bjorn Helgaas
2020-01-25 23:25     ` Kuppuswamy Sathyanarayanan
2020-01-27 13:50       ` Bjorn Helgaas [this message]
2020-01-19  4:00 ` [PATCH v13 5/8] PCI/AER: Allow clearing Error Status Register in FF mode sathyanarayanan.kuppuswamy
2020-01-19  4:00 ` [PATCH v13 6/8] PCI/DPC: Update comments related to DPC recovery on NON_FATAL errors sathyanarayanan.kuppuswamy
2020-01-19  4:00 ` [PATCH v13 7/8] PCI/DPC: Clear AER registers in EDR mode sathyanarayanan.kuppuswamy
2020-01-19  4:00 ` [PATCH v13 8/8] PCI/ACPI: Enable EDR support sathyanarayanan.kuppuswamy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200127135017.GA260782@google.com \
    --to=helgaas@kernel.org \
    --cc=Austin.Bolen@dell.com \
    --cc=ashok.raj@intel.com \
    --cc=huong.nguyen@dell.com \
    --cc=keith.busch@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.