[PATCH] cxl: Route eeh events to all slices for pci_channel_io_perm_failure state

[PATCH] cxl: Route eeh events to all slices for pci_channel_io_perm_failure state

[Vaibhav Jain]

Fix a boundary condition where in some cases an eeh event with
state == pci_channel_io_perm_failure wont be passed on to a driver
attached to the virtual pci device associated with a slice. This will
happen in case the slice just before (n-1) doesn't have any vph bus
associated with it, that results in an early return from
cxl_pci_error_detected callback.

This also simplifies branch that handles this eeh state by returning
PCI_ERS_RESULT_DISCONNECTED in all cases as the adapter is will be
removed irrespective of the return value of cxl_pci_error_detected
callback.

Fixes: e4f5fc001a6("cxl: Do not create vPHB if there are no AFU configuration records")
Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
---
 drivers/misc/cxl/pci.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 80a87ab..679afc9 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -1780,15 +1780,13 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
 
 	/* If we're permanently dead, give up. */
 	if (state == pci_channel_io_perm_failure) {
-		/* Tell the AFU drivers; but we don't care what they
-		 * say, we're going away.
-		 */
 		for (i = 0; i < adapter->slices; i++) {
 			afu = adapter->afu[i];
-			/* Only participate in EEH if we are on a virtual PHB */
-			if (afu->phb == NULL)
-				return PCI_ERS_RESULT_NONE;
-			cxl_vphb_error_detected(afu, state);
+			/* Tell the AFU drivers; but we don't care what they
+			 * say, we're going away.
+			 */
+			if (afu->phb != NULL)
+				cxl_vphb_error_detected(afu, state);
 		}
 		return PCI_ERS_RESULT_DISCONNECT;
 	}
-- 
2.9.3

Re: [PATCH] cxl: Route eeh events to all slices for pci_channel_io_perm_failure state

[Matthew R. Ochs]

> On Feb 22, 2017, at 4:51 AM, Vaibhav Jain <vaibhav@linux.vnet.ibm.com> =
wrote:
>=20
> Fix a boundary condition where in some cases an eeh event with
> state =3D=3D pci_channel_io_perm_failure wont be passed on to a driver
> attached to the virtual pci device associated with a slice. This will
> happen in case the slice just before (n-1) doesn't have any vph bus
> associated with it, that results in an early return from
> cxl_pci_error_detected callback.
>=20
> This also simplifies branch that handles this eeh state by returning
> PCI_ERS_RESULT_DISCONNECTED in all cases as the adapter is will be
> removed irrespective of the return value of cxl_pci_error_detected
> callback.

This sentence could be cleaned up a bit.

>=20
> Fixes: e4f5fc001a6("cxl: Do not create vPHB if there are no AFU =
configuration records")
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>

Fix looks good!

Reviewed-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

Re: [PATCH] cxl: Route eeh events to all slices for pci_channel_io_perm_failure state

[Andrew Donnellan]

On 22/02/17 21:51, Vaibhav Jain wrote:
> Fix a boundary condition where in some cases an eeh event with
> state == pci_channel_io_perm_failure wont be passed on to a driver
> attached to the virtual pci device associated with a slice. This will
> happen in case the slice just before (n-1) doesn't have any vph bus

s/vph/vPHB/?

> associated with it, that results in an early return from
> cxl_pci_error_detected callback.
>
> This also simplifies branch that handles this eeh state by returning
> PCI_ERS_RESULT_DISCONNECTED in all cases as the adapter is will be
> removed irrespective of the return value of cxl_pci_error_detected
> callback.

Agree with Matthew that this could be simplified a bit. If I'm 
interpreting this correctly, we could reword it something like:

"As the adapter will be removed irrespective of the return value of 
cxl_vphb_error_detected(), return PCI_ERS_RESULT_DISCONNECT in all 
cases, even if the AFU isn't using a vPHB (in which case we currently 
return PCI_ERS_RESULT_NONE)."

>
> Fixes: e4f5fc001a6("cxl: Do not create vPHB if there are no AFU configuration records")
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>

Fix looks reasonable to me!

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

> ---
>  drivers/misc/cxl/pci.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 80a87ab..679afc9 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -1780,15 +1780,13 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
>
>  	/* If we're permanently dead, give up. */
>  	if (state == pci_channel_io_perm_failure) {
> -		/* Tell the AFU drivers; but we don't care what they
> -		 * say, we're going away.
> -		 */
>  		for (i = 0; i < adapter->slices; i++) {
>  			afu = adapter->afu[i];
> -			/* Only participate in EEH if we are on a virtual PHB */
> -			if (afu->phb == NULL)
> -				return PCI_ERS_RESULT_NONE;
> -			cxl_vphb_error_detected(afu, state);
> +			/* Tell the AFU drivers; but we don't care what they
> +			 * say, we're going away.
> +			 */
> +			if (afu->phb != NULL)
> +				cxl_vphb_error_detected(afu, state);
>  		}
>  		return PCI_ERS_RESULT_DISCONNECT;
>  	}
>

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: [PATCH] cxl: Route eeh events to all slices for pci_channel_io_perm_failure state

[Vaibhav Jain]

Thanks for reviewing the patch Matt and Andrew. I have updated the patch
description as you have suggested and sent-out an updated one.

-- 
Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.