lkml.org 
[lkml]   [2018]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] PCI/AER: Enable reporting for ports enumerated after AER driver registration
On Thu, Oct 11, 2018 at 08:26:18AM -0700, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> 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 (there are no children, 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. Enable AER error reporting for all
> these Ports, but not for any children.
>
> 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>
> ---
> drivers/pci/pcie/aer.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 90b53abf621d..c40c6607849b 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);
> -
> /* Enable Root Port's interrupt in response to error messages */
> pci_read_config_dword(pdev, aer_pos + PCI_ERR_ROOT_COMMAND, &reg32);
> reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> @@ -1378,10 +1372,17 @@ static void aer_remove(struct pcie_device *dev)
> */
> static int aer_probe(struct pcie_device *dev)
> {
> + struct pci_dev *pdev = dev->port;
> + int type = pci_pcie_type(pdev);
> int status;
> struct aer_rpc *rpc;
> struct device *device = &dev->device;
>
> + if (type == PCI_EXP_TYPE_UPSTREAM || type == PCI_EXP_TYPE_DOWNSTREAM) {
> + pci_enable_pcie_error_reporting(pdev);
> + return 0;
> + }

I think we need to either return an error in this case so that the
pcie_device won't be eligable for the .remove() callback, or add a
similiar type check in aer_remove().

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