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@xxxxxxxxxxxxxxx> # v3.15+
>Signed-off-by: Gavin Shan <gwshan@xxxxxxxxxxxxxxxxxx>
>Tested-by: Wen Xiong <wenxiong@xxxxxxxxxxxxxxxxxx>
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
>
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html