lkml.org 
[lkml]   [2020]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v12 07/15] PCI/ERR: Use "bridge" for clarity in pcie_do_recovery()
On Fri, Nov 20, 2020 at 04:10:28PM -0800, Sean V Kelley wrote:
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index 7a5af873d8bc..46a5b84f8842 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -151,24 +151,27 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
> pci_ers_result_t (*reset_subordinates)(struct pci_dev *pdev))
> {
> int type = pci_pcie_type(dev);
> - pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER;
> + struct pci_dev *bridge;
> struct pci_bus *bus;
> + pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER;
>
> /*
> - * Error recovery runs on all subordinates of the first downstream port.
> - * If the downstream port detected the error, it is cleared at the end.
> + * Error recovery runs on all subordinates of the bridge. If the
> + * bridge detected the error, it is cleared at the end.
> */
> if (!(type == PCI_EXP_TYPE_ROOT_PORT ||
> type == PCI_EXP_TYPE_DOWNSTREAM))
> - dev = pci_upstream_bridge(dev);
> - bus = dev->subordinate;
> + bridge = pci_upstream_bridge(dev);
> + else
> + bridge = dev;

I think there's a bug here even before your series. We started with:

pcie_do_recovery(struct pci_dev *dev, ..., pci_ers_result_t (*reset_link)())
{
if (!(pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM))
dev = dev->bus->self;
...
reset_link(dev);

so if we called pcie_do_recovery() with an Endpoint, we set "dev" to
the upstream bridge, either a Root Port or a Switch Downstream Port,
which we then pass on to reset_link(). For native AER and APEI,
that's aer_root_reset(), which assumes it gets a Root Port.

If we pass a Switch Downstream Port, aer_root_reset() writes to the
*switch port's* PCI_ERR_ROOT_COMMAND and PCI_ERR_ROOT_STATUS, which
are reserved since it's not a Root Port or an RCEC.

The writes probably don't *break* anything since those registers are
reserved, but they also don't disable the interrupt or clear the Root
Error Status.

Bjorn

\
 
 \ /
  Last update: 2020-12-03 00:20    [W:0.163 / U:0.360 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site