On 2022-03-09 22:42:16 Wed, Mahesh Salgaonkar wrote:
> When certain PHB HW failure causes phyp to recover PHB, it marks the PE
> state as temporarily unavailable until recovery is complete. This also
> triggers an EEH handler in Linux which needs to notify drivers, and perform
> recovery. But before notifying the driver about the pci error it uses
> get_adapter_state()->get-sesnor-state() operation of the hotplug_slot to
> determine if the slot contains a device or not. if the slot is empty, the
> recovery is skipped entirely.
>
> However on certain PHB failures, the rtas call get-sesnor-state() returns
> extended busy error (9902) until PHB is recovered by phyp. Once PHB is
> recovered, the get-sensor-state() returns success with correct presence
> status. The rtas call interface rtas_get_sensor() loops over the rtas call
> on extended delay return code (9902) until the return value is either
> success (0) or error (-1). This causes the EEH handler to get stuck for ~6
> seconds before it could notify that the pci error has been detected and
> stop any active operations. Hence with running I/O traffic, during this 6
> seconds, the network driver continues its operation and hits a timeout
> (netdev watchdog). On timeouts, network driver go into ffdc capture mode
> and reset path assuming the PCI device is in fatal condition. This
> sometimes causes EEH recovery to fail. This impacts the ssh connection and
> leads to the system being inaccessible.
>
> ------------
> [52732.244731] DEBUG: ibm_read_slot_reset_state2()
> [52732.244762] DEBUG: ret = 0, rets[0]=5, rets[1]=1, rets[2]=4000, rets[3]=>
> [52732.244798] DEBUG: in eeh_slot_presence_check
> [52732.244804] DEBUG: error state check
> [52732.244807] DEBUG: Is slot hotpluggable
> [52732.244810] DEBUG: hotpluggable ops ?
> [52732.244953] DEBUG: Calling ops->get_adapter_status
> [52732.244958] DEBUG: calling rpaphp_get_sensor_state
> [52736.564262] ------------[ cut here ]------------
> [52736.564299] NETDEV WATCHDOG: enP64p1s0f3 (tg3): transmit queue 0 timed o>
> [52736.564324] WARNING: CPU: 1442 PID: 0 at net/sched/sch_generic.c:478 dev>
> [...]
> [52736.564505] NIP [c000000000c32368] dev_watchdog+0x438/0x440
> [52736.564513] LR [c000000000c32364] dev_watchdog+0x434/0x440
> ------------
>
> To avoid this issue, fix the pci hotplug driver (rpaphp) to return an error
> if the slot presence state can not be detected immediately while pe is in
> EEH recovery state. Current implementation uses rtas_get_sensor() API which
> blocks the slot check state until rtas call returns success. Change
> rpaphp_get_sensor_state() to invoke rtas_call(get-sensor-state) directly
> only if the respective pe is in EEH recovery state, and take actions based
> on rtas return status.
>
> In normal cases (non-EEH case) rpaphp_get_sensor_state() will continue to
> invoke rtas_get_sensor() as it was earlier with no change in existing
> behavior.
>
> Signed-off-by: Mahesh Salgaonkar <mah...@linux.ibm.com>
> ---
> Change in v5:
> - Fixup #define macros with parentheses around the values.
>
> Change in V4:
> - Error out on sensor busy only if pe is going through EEH recovery instead
> of always error out.
>
> Change in V3:
> - Invoke rtas_call(get-sensor-state) directly from
> rpaphp_get_sensor_state() directly and do special handling.
> - See v2 at
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-November/237336.html
>
> Change in V2:
> - Alternate approach to fix the EEH issue instead of delaying slot presence
> check proposed at
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-November/236956.html
>
> Also refer:
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-November/237027.html
> ---
> drivers/pci/hotplug/rpaphp_pci.c | 100
> +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 97 insertions(+), 3 deletions(-)
Any comments on this patch?
Thanks,
-Mahesh.
>
> diff --git a/drivers/pci/hotplug/rpaphp_pci.c
> b/drivers/pci/hotplug/rpaphp_pci.c
> index c380bdacd1466..e463e915a052a 100644
> --- a/drivers/pci/hotplug/rpaphp_pci.c
> +++ b/drivers/pci/hotplug/rpaphp_pci.c
> @@ -18,12 +18,107 @@
> #include "../pci.h" /* for pci_add_new_bus */
> #include "rpaphp.h"
>
> +/*
> + * RTAS call get-sensor-state(DR_ENTITY_SENSE) return values as per PAPR:
> + * -1: Hardware Error
> + * -2: RTAS_BUSY
> + * -3: Invalid sensor. RTAS Parameter Error.
> + * -9000: Need DR entity to be powered up and unisolated before RTAS call
> + * -9001: Need DR entity to be powered up, but not unisolated, before RTAS
> call
> + * -9002: DR entity unusable
> + * 990x: Extended delay - where x is a number in the range of 0-5
> + */
> +#define RTAS_HARDWARE_ERROR (-1)
> +#define RTAS_INVALID_SENSOR (-3)
> +#define SLOT_UNISOLATED (-9000)
> +#define SLOT_NOT_UNISOLATED (-9001)
> +#define SLOT_NOT_USABLE (-9002)
> +
> +static int rtas_to_errno(int rtas_rc)
> +{
> + int rc;
> +
> + switch (rtas_rc) {
> + case RTAS_HARDWARE_ERROR:
> + rc = -EIO;
> + break;
> + case RTAS_INVALID_SENSOR:
> + rc = -EINVAL;
> + break;
> + case SLOT_UNISOLATED:
> + case SLOT_NOT_UNISOLATED:
> + rc = -EFAULT;
> + break;
> + case SLOT_NOT_USABLE:
> + rc = -ENODEV;
> + break;
> + case RTAS_BUSY:
> + case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
> + rc = -EBUSY;
> + break;
> + default:
> + err("%s: unexpected RTAS error %d\n", __func__, rtas_rc);
> + rc = -ERANGE;
> + break;
> + }
> + return rc;
> +}
> +
> +/*
> + * get_adapter_status() can be called by the EEH handler during EEH recovery.
> + * On certain PHB failures, the rtas call get-sensor-state() returns extended
> + * busy error (9902) until PHB is recovered by phyp. The rtas call interface
> + * rtas_get_sensor() loops over the rtas call on extended delay return code
> + * (9902) until the return value is either success (0) or error (-1). This
> + * causes the EEH handler to get stuck for ~6 seconds before it could notify
> + * that the pci error has been detected and stop any active operations. This
> + * sometimes causes EEH recovery to fail. To avoid this issue, invoke
> + * rtas_call(get-sensor-state) directly if the respective pe is in EEH
> recovery
> + * state and return -EBUSY error based on rtas return status. This will help
> + * the EEH handler to notify the driver about the pci error immediately and
> + * successfully proceed with EEH recovery steps.
> + */
> +static int __rpaphp_get_sensor_state(struct slot *slot, int *state)
> +{
> + int rc;
> +#ifdef CONFIG_EEH
> + int token = rtas_token("get-sensor-state");
> + struct pci_dn *pdn;
> + struct eeh_pe *pe;
> + struct pci_controller *phb = PCI_DN(slot->dn)->phb;
> +
> + if (token == RTAS_UNKNOWN_SERVICE)
> + return -ENOENT;
> +
> + /*
> + * Fallback to existing method for empty slot or pe isn't in EEH
> + * recovery.
> + */
> + if (list_empty(&PCI_DN(phb->dn)->child_list))
> + goto fallback;
> +
> + pdn = list_first_entry(&PCI_DN(phb->dn)->child_list,
> + struct pci_dn, list);
> + pe = eeh_dev_to_pe(pdn->edev);
> + if (pe && (pe->state & EEH_PE_RECOVERING)) {
> + rc = rtas_call(token, 2, 2, state, DR_ENTITY_SENSE,
> + slot->index);
> + if (rc)
> + rc = rtas_to_errno(rc);
> + return rc;
> + }
> +fallback:
> +#endif
> + rc = rtas_get_sensor(DR_ENTITY_SENSE, slot->index, state);
> + return rc;
> +}
> +
> int rpaphp_get_sensor_state(struct slot *slot, int *state)
> {
> int rc;
> int setlevel;
>
> - rc = rtas_get_sensor(DR_ENTITY_SENSE, slot->index, state);
> + rc = __rpaphp_get_sensor_state(slot, state);
>
> if (rc < 0) {
> if (rc == -EFAULT || rc == -EEXIST) {
> @@ -39,8 +134,7 @@ int rpaphp_get_sensor_state(struct slot *slot, int *state)
> dbg("%s: power on slot[%s] failed rc=%d.\n",
> __func__, slot->name, rc);
> } else {
> - rc = rtas_get_sensor(DR_ENTITY_SENSE,
> - slot->index, state);
> + rc = __rpaphp_get_sensor_state(slot, state);
> }
> } else if (rc == -ENODEV)
> info("%s: slot is unusable\n", __func__);
>
>
--
Mahesh J Salgaonkar
