Re: [PATCH v3] PCI hotplug: rpaphp: Error out on busy status from get-sensor-state
flat view
From: Nathan Lynch <hidden>
Date: 2021-12-09 15:03:45
Mahesh Salgaonkar [off-list ref] writes:
To avoid this issue, fix the pci hotplug driver (rpaphp) to return an error if the slot presence state can not be detected immediately. 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 and take actions based on rtas return status. This patch now errors out immediately on busy return status from rtas_call. Please note that, only on certain PHB failures, the slot presence check returns BUSY condition. In normal cases it returns immediately with a correct presence state value. Hence this change has no impact on normal pci dlpar operations.
I was wondering about this. This seems to be saying -2/990x cannot happen in other cases. I couldn't find this specified in the architecture. It seems a bit risky to me to *always* error out on -2/990x - won't we have intermittent slot enable failures?
quoted hunk ↗ jump to hunk
+/* + * 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; +}
These conversions look OK to me.