Message ID | 1421985702-18882-1-git-send-email-gwshan@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Benjamin Herrenschmidt |
Headers | show |
On Fri, Jan 23, 2015 at 03:01:42PM +1100, Gavin Shan wrote: >pcibios_set_pcie_reset_state(), implemented based on platform's EEH reset >backends, helps to reset PCI adapters. IPR driver might call this function >in timer handler and it's hard to make reset assertion and settlement delays >with msleep() in the atomic context. The issue was caused by commit 26833a5 >("powerpc/eeh: Make the delay for PE reset unified"). > >The patch drops the reset assertion and settlement delays in the API and >caller will have to take corresponding delays. With the patch applied, the >reset assertion and settlement delays related to EEH reset become: > > * Reset for EEH recovery: inband > * Reset for Guest PE recovery: inband > * pcibios_set_pcie_reset_state(): outband > >Cc: <stable@vger.kernel.org> # v3.15+ >Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >Tested-by: Wen Xiong <wenxiong@linux.vnet.ibm.com> Ben and Michael, please ignore it: When rethinking the issue, I believe the driver need change to use non-atomic context to do reset. This function is still called by pci_reset_function() or pci_try_reset_function(), on which VFIO PCI driver depends to do reset and we need the delay for the case. Thanks, Gavin >--- > arch/powerpc/include/asm/eeh.h | 2 +- > arch/powerpc/kernel/eeh.c | 16 +++++++-------- > arch/powerpc/platforms/powernv/eeh-ioda.c | 29 ++++++++++++++++------------ > arch/powerpc/platforms/powernv/eeh-powernv.c | 4 ++-- > arch/powerpc/platforms/powernv/pci.h | 2 +- > arch/powerpc/platforms/pseries/eeh_pseries.c | 6 +++++- > 6 files changed, 34 insertions(+), 25 deletions(-) > >diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h >index 55abfd0..0bc7d3f 100644 >--- a/arch/powerpc/include/asm/eeh.h >+++ b/arch/powerpc/include/asm/eeh.h >@@ -205,7 +205,7 @@ struct eeh_ops { > int (*set_option)(struct eeh_pe *pe, int option); > int (*get_pe_addr)(struct eeh_pe *pe); > int (*get_state)(struct eeh_pe *pe, int *state); >- int (*reset)(struct eeh_pe *pe, int option); >+ int (*reset)(struct eeh_pe *pe, int option, bool delay); > int (*wait_state)(struct eeh_pe *pe, int max_wait); > int (*get_log)(struct eeh_pe *pe, int severity, char *drv_log, unsigned long len); > int (*configure_bridge)(struct eeh_pe *pe); >diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c >index 3b2252e..b0352b5c 100644 >--- a/arch/powerpc/kernel/eeh.c >+++ b/arch/powerpc/kernel/eeh.c >@@ -688,16 +688,16 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state stat > > switch (state) { > case pcie_deassert_reset: >- eeh_ops->reset(pe, EEH_RESET_DEACTIVATE); >+ eeh_ops->reset(pe, EEH_RESET_DEACTIVATE, false); > eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED); > break; > case pcie_hot_reset: > eeh_pe_state_mark(pe, EEH_PE_CFG_BLOCKED); >- eeh_ops->reset(pe, EEH_RESET_HOT); >+ eeh_ops->reset(pe, EEH_RESET_HOT, false); > break; > case pcie_warm_reset: > eeh_pe_state_mark(pe, EEH_PE_CFG_BLOCKED); >- eeh_ops->reset(pe, EEH_RESET_FUNDAMENTAL); >+ eeh_ops->reset(pe, EEH_RESET_FUNDAMENTAL, false); > break; > default: > eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED); >@@ -749,11 +749,11 @@ static void eeh_reset_pe_once(struct eeh_pe *pe) > eeh_pe_dev_traverse(pe, eeh_set_dev_freset, &freset); > > if (freset) >- eeh_ops->reset(pe, EEH_RESET_FUNDAMENTAL); >+ eeh_ops->reset(pe, EEH_RESET_FUNDAMENTAL, true); > else >- eeh_ops->reset(pe, EEH_RESET_HOT); >+ eeh_ops->reset(pe, EEH_RESET_HOT, true); > >- eeh_ops->reset(pe, EEH_RESET_DEACTIVATE); >+ eeh_ops->reset(pe, EEH_RESET_DEACTIVATE, true); > } > > /** >@@ -1547,7 +1547,7 @@ int eeh_pe_reset(struct eeh_pe *pe, int option) > > switch (option) { > case EEH_RESET_DEACTIVATE: >- ret = eeh_ops->reset(pe, option); >+ ret = eeh_ops->reset(pe, option, true); > eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED); > if (ret) > break; >@@ -1564,7 +1564,7 @@ int eeh_pe_reset(struct eeh_pe *pe, int option) > eeh_ops->set_option(pe, EEH_OPT_FREEZE_PE); > > eeh_pe_state_mark(pe, EEH_PE_CFG_BLOCKED); >- ret = eeh_ops->reset(pe, option); >+ ret = eeh_ops->reset(pe, option, true); > break; > default: > pr_debug("%s: Unsupported option %d\n", >diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c >index 2809c98..1a53cda 100644 >--- a/arch/powerpc/platforms/powernv/eeh-ioda.c >+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c >@@ -548,7 +548,8 @@ out: > return 0; > } > >-static int ioda_eeh_root_reset(struct pci_controller *hose, int option) >+static int ioda_eeh_root_reset(struct pci_controller *hose, >+ int option, bool delay) > { > struct pnv_phb *phb = hose->private_data; > s64 rc = OPAL_SUCCESS; >@@ -578,7 +579,7 @@ static int ioda_eeh_root_reset(struct pci_controller *hose, int option) > > /* Poll state of the PHB until the request is done */ > rc = ioda_eeh_phb_poll(phb); >- if (option == EEH_RESET_DEACTIVATE) >+ if (delay && option == EEH_RESET_DEACTIVATE) > msleep(EEH_PE_RST_SETTLE_TIME); > out: > if (rc != OPAL_SUCCESS) >@@ -587,7 +588,7 @@ out: > return 0; > } > >-static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option) >+static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option, bool delay) > > { > struct device_node *dn = pci_device_to_OF_node(dev); >@@ -614,14 +615,18 @@ static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option) > eeh_ops->read_config(dn, PCI_BRIDGE_CONTROL, 2, &ctrl); > ctrl |= PCI_BRIDGE_CTL_BUS_RESET; > eeh_ops->write_config(dn, PCI_BRIDGE_CONTROL, 2, ctrl); >- msleep(EEH_PE_RST_HOLD_TIME); >+ >+ if (delay) >+ msleep(EEH_PE_RST_HOLD_TIME); > > break; > case EEH_RESET_DEACTIVATE: > eeh_ops->read_config(dn, PCI_BRIDGE_CONTROL, 2, &ctrl); > ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET; > eeh_ops->write_config(dn, PCI_BRIDGE_CONTROL, 2, ctrl); >- msleep(EEH_PE_RST_SETTLE_TIME); >+ >+ if (delay) >+ msleep(EEH_PE_RST_SETTLE_TIME); > > /* Continue reporting linkDown event */ > if (aer) { >@@ -644,11 +649,11 @@ void pnv_pci_reset_secondary_bus(struct pci_dev *dev) > > if (pci_is_root_bus(dev->bus)) { > hose = pci_bus_to_host(dev->bus); >- ioda_eeh_root_reset(hose, EEH_RESET_HOT); >- ioda_eeh_root_reset(hose, EEH_RESET_DEACTIVATE); >+ ioda_eeh_root_reset(hose, EEH_RESET_HOT, true); >+ ioda_eeh_root_reset(hose, EEH_RESET_DEACTIVATE, true); > } else { >- ioda_eeh_bridge_reset(dev, EEH_RESET_HOT); >- ioda_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); >+ ioda_eeh_bridge_reset(dev, EEH_RESET_HOT, true); >+ ioda_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE, true); > } > } > >@@ -664,7 +669,7 @@ void pnv_pci_reset_secondary_bus(struct pci_dev *dev) > * through FLR. For now, we don't have OPAL APIs to do HARD > * reset yet, so all reset would be SOFT (HOT) reset. > */ >-static int ioda_eeh_reset(struct eeh_pe *pe, int option) >+static int ioda_eeh_reset(struct eeh_pe *pe, int option, bool delay) > { > struct pci_controller *hose = pe->phb; > struct pci_bus *bus; >@@ -715,9 +720,9 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option) > bus = eeh_pe_bus_get(pe); > if (pci_is_root_bus(bus) || > pci_is_root_bus(bus->parent)) >- ret = ioda_eeh_root_reset(hose, option); >+ ret = ioda_eeh_root_reset(hose, option, delay); > else >- ret = ioda_eeh_bridge_reset(bus->self, option); >+ ret = ioda_eeh_bridge_reset(bus->self, option, delay); > } > > return ret; >diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c >index e261869..c89a7c4 100644 >--- a/arch/powerpc/platforms/powernv/eeh-powernv.c >+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c >@@ -298,14 +298,14 @@ static int powernv_eeh_get_state(struct eeh_pe *pe, int *delay) > * > * Reset the specified PE > */ >-static int powernv_eeh_reset(struct eeh_pe *pe, int option) >+static int powernv_eeh_reset(struct eeh_pe *pe, int option, bool delay) > { > struct pci_controller *hose = pe->phb; > struct pnv_phb *phb = hose->private_data; > int ret = -EEXIST; > > if (phb->eeh_ops && phb->eeh_ops->reset) >- ret = phb->eeh_ops->reset(pe, option); >+ ret = phb->eeh_ops->reset(pe, option, delay); > > return ret; > } >diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h >index 6c02ff8..a998f2e 100644 >--- a/arch/powerpc/platforms/powernv/pci.h >+++ b/arch/powerpc/platforms/powernv/pci.h >@@ -81,7 +81,7 @@ struct pnv_eeh_ops { > int (*post_init)(struct pci_controller *hose); > int (*set_option)(struct eeh_pe *pe, int option); > int (*get_state)(struct eeh_pe *pe); >- int (*reset)(struct eeh_pe *pe, int option); >+ int (*reset)(struct eeh_pe *pe, int option, bool delay); > int (*get_log)(struct eeh_pe *pe, int severity, > char *drv_log, unsigned long len); > int (*configure_bridge)(struct eeh_pe *pe); >diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c >index a6c7e19..0c8d550 100644 >--- a/arch/powerpc/platforms/pseries/eeh_pseries.c >+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c >@@ -501,7 +501,7 @@ static int pseries_eeh_get_state(struct eeh_pe *pe, int *state) > * > * Reset the specified PE > */ >-static int pseries_eeh_reset(struct eeh_pe *pe, int option) >+static int pseries_eeh_reset(struct eeh_pe *pe, int option, bool delay) > { > int config_addr; > int ret; >@@ -525,6 +525,9 @@ static int pseries_eeh_reset(struct eeh_pe *pe, int option) > BUID_LO(pe->phb->buid), option); > } > >+ if (!delay) >+ goto out; >+ > /* We need reset hold or settlement delay */ > if (option == EEH_RESET_FUNDAMENTAL || > option == EEH_RESET_HOT) >@@ -532,6 +535,7 @@ static int pseries_eeh_reset(struct eeh_pe *pe, int option) > else > msleep(EEH_PE_RST_SETTLE_TIME); > >+out: > return ret; > } > >-- >1.8.3.2 >
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h index 55abfd0..0bc7d3f 100644 --- a/arch/powerpc/include/asm/eeh.h +++ b/arch/powerpc/include/asm/eeh.h @@ -205,7 +205,7 @@ struct eeh_ops { int (*set_option)(struct eeh_pe *pe, int option); int (*get_pe_addr)(struct eeh_pe *pe); int (*get_state)(struct eeh_pe *pe, int *state); - int (*reset)(struct eeh_pe *pe, int option); + int (*reset)(struct eeh_pe *pe, int option, bool delay); int (*wait_state)(struct eeh_pe *pe, int max_wait); int (*get_log)(struct eeh_pe *pe, int severity, char *drv_log, unsigned long len); int (*configure_bridge)(struct eeh_pe *pe); diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c index 3b2252e..b0352b5c 100644 --- a/arch/powerpc/kernel/eeh.c +++ b/arch/powerpc/kernel/eeh.c @@ -688,16 +688,16 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state stat switch (state) { case pcie_deassert_reset: - eeh_ops->reset(pe, EEH_RESET_DEACTIVATE); + eeh_ops->reset(pe, EEH_RESET_DEACTIVATE, false); eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED); break; case pcie_hot_reset: eeh_pe_state_mark(pe, EEH_PE_CFG_BLOCKED); - eeh_ops->reset(pe, EEH_RESET_HOT); + eeh_ops->reset(pe, EEH_RESET_HOT, false); break; case pcie_warm_reset: eeh_pe_state_mark(pe, EEH_PE_CFG_BLOCKED); - eeh_ops->reset(pe, EEH_RESET_FUNDAMENTAL); + eeh_ops->reset(pe, EEH_RESET_FUNDAMENTAL, false); break; default: eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED); @@ -749,11 +749,11 @@ static void eeh_reset_pe_once(struct eeh_pe *pe) eeh_pe_dev_traverse(pe, eeh_set_dev_freset, &freset); if (freset) - eeh_ops->reset(pe, EEH_RESET_FUNDAMENTAL); + eeh_ops->reset(pe, EEH_RESET_FUNDAMENTAL, true); else - eeh_ops->reset(pe, EEH_RESET_HOT); + eeh_ops->reset(pe, EEH_RESET_HOT, true); - eeh_ops->reset(pe, EEH_RESET_DEACTIVATE); + eeh_ops->reset(pe, EEH_RESET_DEACTIVATE, true); } /** @@ -1547,7 +1547,7 @@ int eeh_pe_reset(struct eeh_pe *pe, int option) switch (option) { case EEH_RESET_DEACTIVATE: - ret = eeh_ops->reset(pe, option); + ret = eeh_ops->reset(pe, option, true); eeh_pe_state_clear(pe, EEH_PE_CFG_BLOCKED); if (ret) break; @@ -1564,7 +1564,7 @@ int eeh_pe_reset(struct eeh_pe *pe, int option) eeh_ops->set_option(pe, EEH_OPT_FREEZE_PE); eeh_pe_state_mark(pe, EEH_PE_CFG_BLOCKED); - ret = eeh_ops->reset(pe, option); + ret = eeh_ops->reset(pe, option, true); break; default: pr_debug("%s: Unsupported option %d\n", diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c index 2809c98..1a53cda 100644 --- a/arch/powerpc/platforms/powernv/eeh-ioda.c +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c @@ -548,7 +548,8 @@ out: return 0; } -static int ioda_eeh_root_reset(struct pci_controller *hose, int option) +static int ioda_eeh_root_reset(struct pci_controller *hose, + int option, bool delay) { struct pnv_phb *phb = hose->private_data; s64 rc = OPAL_SUCCESS; @@ -578,7 +579,7 @@ static int ioda_eeh_root_reset(struct pci_controller *hose, int option) /* Poll state of the PHB until the request is done */ rc = ioda_eeh_phb_poll(phb); - if (option == EEH_RESET_DEACTIVATE) + if (delay && option == EEH_RESET_DEACTIVATE) msleep(EEH_PE_RST_SETTLE_TIME); out: if (rc != OPAL_SUCCESS) @@ -587,7 +588,7 @@ out: return 0; } -static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option) +static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option, bool delay) { struct device_node *dn = pci_device_to_OF_node(dev); @@ -614,14 +615,18 @@ static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option) eeh_ops->read_config(dn, PCI_BRIDGE_CONTROL, 2, &ctrl); ctrl |= PCI_BRIDGE_CTL_BUS_RESET; eeh_ops->write_config(dn, PCI_BRIDGE_CONTROL, 2, ctrl); - msleep(EEH_PE_RST_HOLD_TIME); + + if (delay) + msleep(EEH_PE_RST_HOLD_TIME); break; case EEH_RESET_DEACTIVATE: eeh_ops->read_config(dn, PCI_BRIDGE_CONTROL, 2, &ctrl); ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET; eeh_ops->write_config(dn, PCI_BRIDGE_CONTROL, 2, ctrl); - msleep(EEH_PE_RST_SETTLE_TIME); + + if (delay) + msleep(EEH_PE_RST_SETTLE_TIME); /* Continue reporting linkDown event */ if (aer) { @@ -644,11 +649,11 @@ void pnv_pci_reset_secondary_bus(struct pci_dev *dev) if (pci_is_root_bus(dev->bus)) { hose = pci_bus_to_host(dev->bus); - ioda_eeh_root_reset(hose, EEH_RESET_HOT); - ioda_eeh_root_reset(hose, EEH_RESET_DEACTIVATE); + ioda_eeh_root_reset(hose, EEH_RESET_HOT, true); + ioda_eeh_root_reset(hose, EEH_RESET_DEACTIVATE, true); } else { - ioda_eeh_bridge_reset(dev, EEH_RESET_HOT); - ioda_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); + ioda_eeh_bridge_reset(dev, EEH_RESET_HOT, true); + ioda_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE, true); } } @@ -664,7 +669,7 @@ void pnv_pci_reset_secondary_bus(struct pci_dev *dev) * through FLR. For now, we don't have OPAL APIs to do HARD * reset yet, so all reset would be SOFT (HOT) reset. */ -static int ioda_eeh_reset(struct eeh_pe *pe, int option) +static int ioda_eeh_reset(struct eeh_pe *pe, int option, bool delay) { struct pci_controller *hose = pe->phb; struct pci_bus *bus; @@ -715,9 +720,9 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option) bus = eeh_pe_bus_get(pe); if (pci_is_root_bus(bus) || pci_is_root_bus(bus->parent)) - ret = ioda_eeh_root_reset(hose, option); + ret = ioda_eeh_root_reset(hose, option, delay); else - ret = ioda_eeh_bridge_reset(bus->self, option); + ret = ioda_eeh_bridge_reset(bus->self, option, delay); } return ret; diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c index e261869..c89a7c4 100644 --- a/arch/powerpc/platforms/powernv/eeh-powernv.c +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c @@ -298,14 +298,14 @@ static int powernv_eeh_get_state(struct eeh_pe *pe, int *delay) * * Reset the specified PE */ -static int powernv_eeh_reset(struct eeh_pe *pe, int option) +static int powernv_eeh_reset(struct eeh_pe *pe, int option, bool delay) { struct pci_controller *hose = pe->phb; struct pnv_phb *phb = hose->private_data; int ret = -EEXIST; if (phb->eeh_ops && phb->eeh_ops->reset) - ret = phb->eeh_ops->reset(pe, option); + ret = phb->eeh_ops->reset(pe, option, delay); return ret; } diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h index 6c02ff8..a998f2e 100644 --- a/arch/powerpc/platforms/powernv/pci.h +++ b/arch/powerpc/platforms/powernv/pci.h @@ -81,7 +81,7 @@ struct pnv_eeh_ops { int (*post_init)(struct pci_controller *hose); int (*set_option)(struct eeh_pe *pe, int option); int (*get_state)(struct eeh_pe *pe); - int (*reset)(struct eeh_pe *pe, int option); + int (*reset)(struct eeh_pe *pe, int option, bool delay); int (*get_log)(struct eeh_pe *pe, int severity, char *drv_log, unsigned long len); int (*configure_bridge)(struct eeh_pe *pe); diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c index a6c7e19..0c8d550 100644 --- a/arch/powerpc/platforms/pseries/eeh_pseries.c +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c @@ -501,7 +501,7 @@ static int pseries_eeh_get_state(struct eeh_pe *pe, int *state) * * Reset the specified PE */ -static int pseries_eeh_reset(struct eeh_pe *pe, int option) +static int pseries_eeh_reset(struct eeh_pe *pe, int option, bool delay) { int config_addr; int ret; @@ -525,6 +525,9 @@ static int pseries_eeh_reset(struct eeh_pe *pe, int option) BUID_LO(pe->phb->buid), option); } + if (!delay) + goto out; + /* We need reset hold or settlement delay */ if (option == EEH_RESET_FUNDAMENTAL || option == EEH_RESET_HOT) @@ -532,6 +535,7 @@ static int pseries_eeh_reset(struct eeh_pe *pe, int option) else msleep(EEH_PE_RST_SETTLE_TIME); +out: return ret; }