On Thu, Oct 11, 2018 at 07:58:47PM +0800, Dongdong Liu wrote: > Hi Bjorn > > > commit 15a6711649915ca3e9d1086dc88ff4b616b99aac > > Author: Bjorn Helgaas <bhelg...@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.derr...@intel.com > > Based-on-patch-by: Jon Derrick <jonathan.derr...@intel.com> > > Signed-off-by: Bjorn Helgaas <bhelg...@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, ®32); > > 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.derr...@intel.com v2 - My rework, https://lore.kernel.org/linux-pci/20181009231915.gc5...@bhelgaas-glaptop.roam.corp.google.com v3 - My rework + enable error reporting for Root Ports Bjorn
