lkml.org 
[lkml]   [2018]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] PCI/AER: Enable SERR# forwarding in non ACPI flow
    On Thu, Aug 09, 2018 at 08:27:37PM +0530, Bharat Kumar Gogada wrote:
    > As per Figure 6-3 in PCIe r4.0, sec 6.2.6, ERR_ messages
    > will be forwarded from the secondary interface to the primary interface,
    > if the SERR# Enable bit in the Bridge Control register is set.
    > Currently PCI_BRIDGE_CTL_SERR is being enabled only in
    > ACPI flow.
    > This patch enables PCI_BRIDGE_CTL_SERR for Type-1 PCI device.

    I agree this is a problem. We should not depend on how firmware set
    PCI_BRIDGE_CTL_SERR.

    I would prefer to set it in the pci_configure_device() path so we
    always do it the same way for every device, independent of whether AER
    is enabled.

    Some platform firmware must already enable PCI_BRIDGE_CTL_SERR, even
    though it can't know whether the OS will support AER, so it should be
    safe for the OS to enable it unconditionally.

    The code in program_hpp_type0() that sets PCI_BRIDGE_CTL_SERR is based
    on 40abb96c51bb ("[PATCH] pciehp: Fix programming hotplug
    parameters") [1], but I think it is incorrect.

    ACPI v6.0, sec 6.2.9.1, (_HPX PCI Setting Record) says:

    If the hot plug device includes bridge(s) in the hierarchy, the
    above settings apply to the primary side (command register) of the
    hot plugged bridge(s). The settings for the secondary side of the
    bridge(s) (Bridge Control Register) are assumed to be provided by
    the bridge driver.

    Sec 6.2.8 (for the old _HPP method) doesn't contain that text, but it
    has the same description of the contents:

    Enable SERR When set to 1, indicates that action must be
    performed to enable SERR in the command register.

    Both sections talk about enabling SERR in the *command* register (not
    the bridge control register) and the _HPX section is explicit about
    the fact that "Enable SERR" does not apply to the bridge control
    register.

    So I think pci_configure_device() should unconditionally set
    PCI_BRIDGE_CTL_SERR for all bridge devices. And we should remove the
    corresponding code from program_hpp_type0().

    [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=40abb96c51bb

    > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
    > ---
    > drivers/pci/pcie/aer.c | 23 +++++++++++++++++++++++
    > 1 files changed, 23 insertions(+), 0 deletions(-)
    >
    > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
    > index a2e8838..4fb0d24 100644
    > --- a/drivers/pci/pcie/aer.c
    > +++ b/drivers/pci/pcie/aer.c
    > @@ -343,6 +343,20 @@ int pci_enable_pcie_error_reporting(struct pci_dev *dev)
    > if (!dev->aer_cap)
    > return -EIO;
    >
    > + if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
    > + u16 control;
    > +
    > + /*
    > + * A Type-1 PCI bridge will not forward ERR_ messages coming
    > + * from an endpoint if SERR# forwarding is not enabled.
    > + */
    > + pci_read_config_word(dev, PCI_BRIDGE_CONTROL, &control);
    > + if (!(control & PCI_BRIDGE_CTL_SERR)) {
    > + control |= PCI_BRIDGE_CTL_SERR;
    > + pci_write_config_word(dev, PCI_BRIDGE_CONTROL, control);
    > + }
    > + }
    > +
    > return pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS);
    > }
    > EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting);
    > @@ -352,6 +366,15 @@ int pci_disable_pcie_error_reporting(struct pci_dev *dev)
    > if (pcie_aer_get_firmware_first(dev))
    > return -EIO;
    >
    > + if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
    > + u16 control;
    > +
    > + /* Clear SERR Forwarding */
    > + pci_read_config_word(dev, PCI_BRIDGE_CONTROL, &control);
    > + control &= ~PCI_BRIDGE_CTL_SERR;
    > + pci_write_config_word(dev, PCI_BRIDGE_CONTROL, control);
    > + }
    > +
    > return pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
    > PCI_EXP_AER_FLAGS);
    > }
    > --
    > 1.7.1
    >

    \
     
     \ /
      Last update: 2018-09-05 22:05    [W:4.573 / U:0.412 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site