lkml.org 
[lkml]   [2018]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] PCI/portdrv: Enable error reporting on managed ports
On Thu, Oct 11, 2018 at 07:58:47PM +0800, Dongdong Liu wrote:
> Hi Bjorn
>
> > commit 15a6711649915ca3e9d1086dc88ff4b616b99aac
> > Author: Bjorn Helgaas <bhelgaas@google.com>
> > Date: Tue Oct 9 17:25:25 2018 -0500
> >
> > PCI/AER: Enable reporting for ports enumerated after AER driver registration
> >
> > Previously we enabled AER error reporting only for Switch Ports that were
> > enumerated prior to registering the AER service driver. Switch Ports
> > enumerated after AER driver registration were left with error reporting
> > disabled.
> >
> > A common order, which works correctly, is that we enumerate devices before
> > registering portdrv and the AER driver:
> >
> > - Enumerate all the devices at boot-time
> >
> > - Register portdrv and bind it to all Root Ports and Switch Ports, which
> > disables error reporting for these Ports
> >
> > - Register AER service driver and bind it to all Root Ports, which
> > enables error reporting for the Root Ports and any Switch Ports below
> > them
> >
> > But if we enumerate devices *after* registering portdrv and the AER driver,
> > e.g., if a host bridge driver is loaded as a module, error reporting is not
> > enabled correctly:
> >
> > - Register portdrv and AER driver (this happens at boot-time)
> >
> > - Enumerate a Root Port
> >
> > - Bind portdrv to Root Port, disabling its error reporting
> >
> > - Bind AER service driver to Root Port, enabling error reporting for it
> > and its children (none, since we haven't enumerated them yet)
> >
> > - Enumerate Switch Port below the Root Port
> >
> > - Bind portdrv to Switch Port, disabling its error reporting
> >
> > - AER service driver doesn't bind to Switch Ports, so error reporting
> > remains disabled
> >
> > Hot-adding a Switch fails similarly: error reporting is enabled correctly
> > for the Root Port, but when the Switch is enumerated, the AER service
> > driver doesn't claim it, so there's nothing to enable error reporting for
> > the Switch Ports.
> >
> > Change the AER service driver so it binds to *all* PCIe ports, including
> > Switch Upstream and Downstream Ports. For Switch Ports, enable AER error
> > reporting.
> >
> > Link: https://lore.kernel.org/linux-pci/1536085989-2956-1-git-send-email-jonathan.derrick@intel.com
> > Based-on-patch-by: Jon Derrick <jonathan.derrick@intel.com>
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> >
> > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> > index 90b53abf621d..fe6c16461367 100644
> > --- a/drivers/pci/pcie/aer.c
> > +++ b/drivers/pci/pcie/aer.c
> > @@ -1316,12 +1316,6 @@ static void aer_enable_rootport(struct aer_rpc *rpc)
> > pci_read_config_dword(pdev, aer_pos + PCI_ERR_UNCOR_STATUS, &reg32);
> > pci_write_config_dword(pdev, aer_pos + PCI_ERR_UNCOR_STATUS, reg32);
> >
> > - /*
> > - * Enable error reporting for the root port device and downstream port
> > - * devices.
> > - */
> > - set_downstream_devices_error_reporting(pdev, true);
> > -
>
> Delete the code will also disable error reporting for the root port as
> the portdrv to Root Port has disabled its error reporting,
> so need to enable enable error reporting for the root port.
> +pci_enable_pcie_error_reporting(pdev);

Oh, you're right, thank you!

I'll post a "v3" to fix this, i.e.,

v1 - Jon's original post, https://lore.kernel.org/linux-pci/1536085989-2956-1-git-send-email-jonathan.derrick@intel.com
v2 - My rework, https://lore.kernel.org/linux-pci/20181009231915.GC5906@bhelgaas-glaptop.roam.corp.google.com
v3 - My rework + enable error reporting for Root Ports

Bjorn

\
 
 \ /
  Last update: 2018-10-11 17:16    [W:0.049 / U:1.736 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site