From: Gavin Shan <shangw@linux•vnet.ibm.com>
To: Benjamin Herrenschmidt <benh@kernel•crashing.org>
Cc: linuxppc-dev@lists•ozlabs.org, Gavin Shan <shangw@linux•vnet.ibm.com>
Subject: Re: [PATCH 16/23] powerpc/eeh: I/O chip PE reset
Date: Mon, 3 Jun 2013 09:09:28 +0800 [thread overview]
Message-ID: <20130603010928.GB7958@shangw.(null)> (raw)
In-Reply-To: <1370060670.3766.8.camel@pasglop>
On Sat, Jun 01, 2013 at 02:24:30PM +1000, Benjamin Herrenschmidt wrote:
>On Thu, 2013-05-30 at 16:23 +0800, Gavin Shan wrote:
>> The patch adds the I/O chip backend to do PE reset. For now, we
>> focus on PCI bus dependent PE. If PHB PE has been put into error
>> state, the PHB will take complete reset. Besides, the root bridge
>> will take fundamental or hot reset accordingly if the indicated
>> PE locates at the toppest of PCI hierarchy tree. Otherwise, the
>> upstream p2p bridge will take hot reset.
>>
>> Signed-off-by: Gavin Shan <shangw@linux•vnet.ibm.com>
>
> .../...
>
>> +static s64 ioda_eeh_phb_poll(struct pnv_phb *phb)
>> +{
>> + s64 rc = OPAL_HARDWARE;
>> +
>> + while (1) {
>> + rc = opal_pci_poll(phb->opal_id);
>> + if (rc <= 0)
>> + break;
>> + }
>> +
>> + return rc;
>> +}
>
>This is rude. It should at the very least cond_resched, but better,
>isn't the firmware supposed to return a positive value to indicate
>how long to wait for before calling again ? In that case it should
>msleep() ...
>
Yeah. Thanks for pointing it out. I will use msleep() if possible.
>> +static int ioda_eeh_phb_reset(struct pci_controller *hose, int option)
>> +{
>> + struct pnv_phb *phb = hose->private_data;
>> + s64 rc = OPAL_HARDWARE;
>> +
>> + pr_debug("%s: Reset PHB#%x, option=%d\n",
>> + __func__, hose->global_number, option);
>> +
>> + /* Issue PHB complete reset request */
>> + if (option == EEH_RESET_FUNDAMENTAL ||
>> + option == EEH_RESET_HOT)
>> + rc = opal_pci_reset(phb->opal_id,
>> + OPAL_PHB_COMPLETE,
>> + OPAL_ASSERT_RESET);
>> + else if (option == EEH_RESET_DEACTIVATE)
>> + rc = opal_pci_reset(phb->opal_id,
>> + OPAL_PHB_COMPLETE,
>> + OPAL_DEASSERT_RESET);
>> + if (rc < 0)
>> + goto out;
>> +
>> + /*
>> + * Poll state of the PHB until the request is done
>> + * successfully.
>> + */
>> + rc = ioda_eeh_phb_poll(phb);
>> +out:
>> + if (rc != OPAL_SUCCESS)
>> + return -EIO;
>> +
>> + return 0;
>> +}
>> +
>> +static int ioda_eeh_root_reset(struct pci_controller *hose, int option)
>> +{
>> + struct pnv_phb *phb = hose->private_data;
>> + s64 rc = OPAL_SUCCESS;
>> +
>> + pr_debug("%s: Reset PHB#%x, option=%d\n",
>> + __func__, hose->global_number, option);
>> +
>> + /*
>> + * During the reset deassert time, we needn't care
>> + * the reset scope because the firmware does nothing
>> + * for fundamental or hot reset during deassert phase.
>> + */
>> + if (option == EEH_RESET_FUNDAMENTAL)
>> + rc = opal_pci_reset(phb->opal_id,
>> + OPAL_PCI_FUNDAMENTAL_RESET,
>> + OPAL_ASSERT_RESET);
>> + else if (option == EEH_RESET_HOT)
>> + rc = opal_pci_reset(phb->opal_id,
>> + OPAL_PCI_HOT_RESET,
>> + OPAL_ASSERT_RESET);
>> + else if (option == EEH_RESET_DEACTIVATE)
>> + rc = opal_pci_reset(phb->opal_id,
>> + OPAL_PCI_HOT_RESET,
>> + OPAL_DEASSERT_RESET);
>> + if (rc < 0)
>> + goto out;
>> +
>> + /* Poll state of the PHB until the request is done */
>> + rc = ioda_eeh_phb_poll(phb);
>> +out:
>> + if (rc != OPAL_SUCCESS)
>> + return -EIO;
>> +
>> + return 0;
>> +}
>> +
>> +static int ioda_eeh_bridge_reset(struct pci_controller *hose,
>> + struct pci_dev *dev, int option)
>> +{
>> + u16 ctrl;
>> +
>> + pr_debug("%s: Reset device %04x:%02x:%02x.%01x with option %d\n",
>> + __func__, hose->global_number, dev->bus->number,
>> + PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn), option);
>> +
>> + switch (option) {
>> + case EEH_RESET_FUNDAMENTAL:
>> + case EEH_RESET_HOT:
>> + pci_read_config_word(dev, PCI_BRIDGE_CONTROL, &ctrl);
>> + ctrl |= PCI_BRIDGE_CTL_BUS_RESET;
>> + pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
>> + break;
>> + case EEH_RESET_DEACTIVATE:
>> + pci_read_config_word(dev, PCI_BRIDGE_CONTROL, &ctrl);
>> + ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
>> + pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>
>Is there any minimum delay by spec here ? And is there a delay to
>respect after the rest or is that handled at the upper level ?
>
Yes, the spec requires 100ms between assert/deassert point and 1.5s
for the link to be stable after reset. The upper layer has 250ms and
1.8s for them separately.
>Also, it looks like nothing here checks if the link is coming back
>up below the bridge, at what speed etc... this might be something
>to add in the future.
>
Ok. I've put it into TODO list.
>> +/**
>> + * ioda_eeh_reset - Reset the indicated PE
>> + * @pe: EEH PE
>> + * @option: reset option
>> + *
>> + * Do reset on the indicated PE. For PCI bus sensitive PE,
>> + * we need to reset the parent p2p bridge. The PHB has to
>> + * be reinitialized if the p2p bridge is root bridge. For
>> + * PCI device sensitive PE, we will try to reset the device
>> + * 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)
>> +{
>> + struct pci_controller *hose = pe->phb;
>> + struct eeh_dev *edev;
>> + struct pci_dev *dev;
>> + int ret;
>> +
>> + /*
>> + * Anyway, we have to clear the problematic state for the
>> + * corresponding PE. However, we needn't do it if the PE
>> + * is PHB associated. That means the PHB is having fatal
>> + * errors and it needs reset. Further more, the AIB interface
>> + * isn't reliable any more.
>> + */
>> + if (!(pe->type & EEH_PE_PHB) &&
>> + (option == EEH_RESET_HOT ||
>> + option == EEH_RESET_FUNDAMENTAL)) {
>> + ret = ioda_eeh_pe_clear(pe);
>> + if (ret)
>> + return -EIO;
>> + }
>> +
>> + /*
>> + * The rules applied to reset, either fundamental or hot reset:
>> + *
>> + * We always reset the direct upstream bridge of the PE. If the
>> + * direct upstream bridge isn't root bridge, we always take hot
>> + * reset no matter what option (fundamental or hot) is. Otherwise,
>> + * we should do the reset according to the required option.
>> + */
>> + if (pe->type & EEH_PE_PHB) {
>> + ret = ioda_eeh_phb_reset(hose, option);
>> + } else {
>> + if (pe->type & EEH_PE_DEVICE) {
>> + /*
>> + * If it's device PE, we didn't refer to the parent
>> + * PCI bus yet. So we have to figure it out indirectly.
>> + */
>> + edev = list_first_entry(&pe->edevs,
>> + struct eeh_dev, list);
>> + dev = eeh_dev_to_pci_dev(edev);
>> + dev = dev->bus->self;
>> + } else {
>> + /*
>> + * If it's bus PE, the parent PCI bus is already there
>> + * and just pick it up.
>> + */
>> + dev = pe->bus->self;
>> + }
>> +
>> + /*
>> + * Do reset based on the fact that the direct upstream bridge
>> + * is root bridge (port) or not.
>> + */
>> + if (dev->bus->number == 0)
>> + ret = ioda_eeh_root_reset(hose, option);
>> + else
>> + ret = ioda_eeh_bridge_reset(hose, dev, option);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> struct pnv_eeh_ops ioda_eeh_ops = {
>> .post_init = ioda_eeh_post_init,
>> .set_option = ioda_eeh_set_option,
>> .get_state = ioda_eeh_get_state,
>> - .reset = NULL,
>> + .reset = ioda_eeh_reset,
>> .get_log = NULL,
>> .configure_bridge = NULL
>> };
>
Thanks,
Gavin
next prev parent reply other threads:[~2013-06-03 1:09 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-30 8:23 [PATCH v2 00/23] powerpc/eeh: Enhance converting EEH dev Gavin Shan
2013-05-30 8:23 ` [PATCH 01/23] " Gavin Shan
2013-05-30 8:23 ` [PATCH 02/23] powerpc/eeh: Function to tranverse PCI devices Gavin Shan
2013-06-01 4:13 ` Benjamin Herrenschmidt
2013-06-03 1:00 ` Gavin Shan
2013-05-30 8:23 ` [PATCH 03/23] powerpc/eeh: Make eeh_phb_pe_get() public Gavin Shan
2013-06-01 4:14 ` Benjamin Herrenschmidt
2013-06-03 1:02 ` Gavin Shan
2013-05-30 8:23 ` [PATCH 04/23] powerpc/eeh: Make eeh_pe_get() public Gavin Shan
2013-05-30 8:23 ` [PATCH 05/23] powerpc/eeh: Trace PCI bus from PE Gavin Shan
2013-05-30 8:23 ` [PATCH 06/23] powerpc/eeh: Make eeh_init() public Gavin Shan
2013-05-30 8:23 ` [PATCH 07/23] powerpc/eeh: EEH post initialization operation Gavin Shan
2013-05-30 8:23 ` [PATCH 08/23] powerpc/eeh: Refactor eeh_reset_pe_once() Gavin Shan
2013-06-01 4:18 ` Benjamin Herrenschmidt
2013-06-03 1:03 ` Gavin Shan
2013-05-30 8:23 ` [PATCH 09/23] powerpc/eeh: Delay EEH probe during hotplug Gavin Shan
2013-05-30 8:23 ` [PATCH 10/23] powerpc/eeh: Differentiate EEH events Gavin Shan
2013-05-30 8:23 ` [PATCH 11/23] powerpc/eeh: Sync OPAL API with firmware Gavin Shan
2013-05-30 8:23 ` [PATCH 12/23] powerpc/eeh: EEH backend for P7IOC Gavin Shan
2013-05-30 8:23 ` [PATCH 13/23] powerpc/eeh: I/O chip post initialization Gavin Shan
2013-05-30 8:23 ` [PATCH 14/23] powerpc/eeh: I/O chip EEH enable option Gavin Shan
2013-05-30 8:23 ` [PATCH 15/23] powerpc/eeh: I/O chip EEH state retrieval Gavin Shan
2013-05-30 8:23 ` [PATCH 16/23] powerpc/eeh: I/O chip PE reset Gavin Shan
2013-06-01 4:24 ` Benjamin Herrenschmidt
2013-06-03 1:09 ` Gavin Shan [this message]
2013-05-30 8:23 ` [PATCH 17/23] powerpc/eeh: I/O chip PE log and bridge setup Gavin Shan
2013-05-30 8:24 ` [PATCH 18/23] powerpc/eeh: PowerNV EEH backends Gavin Shan
2013-06-01 4:29 ` Benjamin Herrenschmidt
2013-06-03 1:10 ` Gavin Shan
2013-05-30 8:24 ` [PATCH 19/23] powerpc/eeh: Initialization for PowerNV Gavin Shan
2013-05-30 8:24 ` [PATCH 20/23] powerpc/eeh: Enable EEH check for config access Gavin Shan
2013-05-30 8:24 ` [PATCH 21/23] powerpc/eeh: Process interrupts caused by EEH Gavin Shan
2013-05-30 8:24 ` [PATCH 22/23] powerpc/eeh: Connect EEH error interrupt handle Gavin Shan
2013-06-01 4:32 ` Benjamin Herrenschmidt
2013-05-30 8:24 ` [PATCH 23/23] powerpc/eeh: Add debugfs entry to inject errors Gavin Shan
2013-06-01 4:34 ` Benjamin Herrenschmidt
2013-06-03 1:23 ` Gavin Shan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='20130603010928.GB7958@shangw.(null)' \
--to=shangw@linux$(echo .)vnet.ibm.com \
--cc=benh@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox