From: Bjorn Helgaas <helgaas@kernel.org>
To: Austin.Bolen@dell.com
Cc: sathyanarayanan.kuppuswamy@linux.intel.com, bhelgaas@google.com,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
ashok.raj@intel.com, Mario.Limonciello@dell.com,
jonathan.derrick@intel.com, mr.nuke.me@gmail.com,
rjw@rjwysocki.net, kbusch@kernel.org, okaya@kernel.org,
tbaicar@codeaurora.org
Subject: Re: [PATCH v1 1/1] PCI/AER: Use _OSC negotiation to determine AER ownership
Date: Fri, 1 May 2020 10:02:04 -0500 [thread overview]
Message-ID: <20200501150204.GA109407@bjorn-Precision-5520> (raw)
In-Reply-To: <42c53d43bc094889b33c1ac8c2b99d33@AUSX13MPC107.AMER.DELL.COM>
On Fri, May 01, 2020 at 02:40:23PM +0000, Austin.Bolen@dell.com wrote:
> On 4/30/2020 6:02 PM, Bjorn Helgaas wrote:
> > [Austin, help us understand the FIRMWARE_FIRST bit! :)]
> >
> > On Thu, Apr 30, 2020 at 05:40:22PM -0500, Bjorn Helgaas wrote:
> >> On Sun, Apr 26, 2020 at 11:30:06AM -0700, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
> >>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> >>>
> >>> Currently PCIe AER driver uses HEST FIRMWARE_FIRST bit to
> >>> determine the PCIe AER Capability ownership between OS and
> >>> firmware. This support is added based on following spec
> >>> reference.
> >>>
> >>> Per ACPI spec r6.3, table 18-387, 18-388, 18-389, HEST table
> >>> flags field BIT-0 and BIT-1 can be used to expose the
> >>> ownership of error source between firmware and OSPM.
> >>>
> >>> Bit [0] - FIRMWARE_FIRST: If set, indicates that system
> >>> firmware will handle errors from this source
> >>> first.
> >>> Bit [1] – GLOBAL: If set, indicates that the settings
> >>> contained in this structure apply globally to all
> >>> PCI Express Bridges.
> >>>
> >>> Although above spec reference states that setting
> >>> FIRMWARE_FIRST bit means firmware will handle the error source
> >>> first, it does not explicitly state anything about AER
> >>> ownership. So using HEST to determine AER ownership is
> >>> incorrect.
> >>>
> >>> Also, as per following specification references, _OSC can be
> >>> used to negotiate the AER ownership between firmware and OS.
> >>> Details are,
> >>>
> >>> Per ACPI spec r6.3, sec 6.2.11.3, table titled “Interpretation
> >>> of _OSC Control Field” and as per PCI firmware specification r3.2,
> >>> sec 4.5.1, table 4-5, OS can set bit 3 of _OSC control field
> >>> to request control over PCI Express AER. If the OS successfully
> >>> receives control of this feature, it must handle error reporting
> >>> through the AER Capability as described in the PCI Express Base
> >>> Specification.
> >>>
> >>> Since above spec references clearly states _OSC can be used to
> >>> determine AER ownership, don't depend on HEST FIRMWARE_FIRST bit.
> >> I pulled out the _OSC part of this to a separate patch. What's left
> >> is below, and is essentially equivalent to Alex's patch:
> >>
> >> https://lore.kernel.org/r/20190326172343.28946-3-mr.nuke.me@gmail.com/
> >>
> >> I like what this does, but what I don't like is the fact that we now
> >> have this thing called pcie_aer_get_firmware_first() that is not
> >> connected with the ACPI FIRMWARE_FIRST bit at all.
> >
> > Austin, if we remove this, we'll have no PCIe-related code that looks
> > at the HEST FIRMWARE_FIRST bit at all. Presumably it's there for some
> > reason, but I'm not sure what the reason is.
> >
> > Alex's mail [1] has a nice table of _OSC AER/HEST FFS bits that looks
> > useful, but the only actionable thing I can see is that in the (1,1)
> > case, OSPM should do some initialization with masks/enables.
> >
> > But I have no clue what that means or how to connect that with the
> > spec. What are the masks/enables? Is that something connected with
> > ERST?
> >
> > [1] https://lore.kernel.org/r/20190326172343.28946-1-mr.nuke.me@gmail.com/
>
> The only values that make sense to me are (1, 0) for full native OS
> init/handling of AER and (0, 1) for the firmware first model where
> firmware does the init and handles errors first then passes control to
> the OS. For these cases the FIRMWARE_FIRST flag in HEST is redundant and
> not needed.
>
> We did discuss the (1, 1) case in the ACPI working group and there were
> a potential use case (which Alex documented in the link you provided)
> but there is nothing specified in the standard about how that model
> would actually work AFAICT. And no x86 system has the hardware support
> needed for what was proposed that I'm aware of (not sure about other
> architectures).
>
> So unless and until someone documents how the firmware and OS are
> supposed to behave in the (1, 1) or (0, 0) scenario and expresses a need
> for those models, I would not bother adding support for them. Just my 2
> cents.
Perfect, I think we should ignore the FIRMWARE_FIRST bit in the HEST
PCIe entries for now. Thanks a lot for your help with this!
Bjorn
next prev parent reply other threads:[~2020-05-01 15:02 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-26 18:30 [PATCH v1 1/1] PCI/AER: Use _OSC negotiation to determine AER ownership sathyanarayanan.kuppuswamy
2020-04-28 0:02 ` Bjorn Helgaas
2020-04-28 3:20 ` Kuppuswamy, Sathyanarayanan
2020-04-28 11:45 ` Bjorn Helgaas
2020-04-28 14:49 ` Derrick, Jonathan
2020-04-28 20:36 ` Bjorn Helgaas
2020-04-29 15:24 ` Austin.Bolen
2020-04-29 17:10 ` Bjorn Helgaas
2020-04-30 21:04 ` Bjorn Helgaas
2020-04-30 22:40 ` Bjorn Helgaas
2020-04-30 23:02 ` Bjorn Helgaas
2020-05-01 14:40 ` Austin.Bolen
2020-05-01 15:02 ` Bjorn Helgaas [this message]
2020-05-01 0:35 ` Kuppuswamy, Sathyanarayanan
2020-05-01 15:29 ` Bjorn Helgaas
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=20200501150204.GA109407@bjorn-Precision-5520 \
--to=helgaas@kernel.org \
--cc=Austin.Bolen@dell.com \
--cc=Mario.Limonciello@dell.com \
--cc=ashok.raj@intel.com \
--cc=bhelgaas@google.com \
--cc=jonathan.derrick@intel.com \
--cc=kbusch@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mr.nuke.me@gmail.com \
--cc=okaya@kernel.org \
--cc=rjw@rjwysocki.net \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=tbaicar@codeaurora.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).