diff mbox

[net-next] liquidio: fix rare pci_driver.probe failure of VF driver

Message ID 20170525174214.GA1444@felix-thinkpad.cavium.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Manlunas, Felix May 25, 2017, 5:42 p.m. UTC 
From: Prasad Kanneganti <prasad.kanneganti@cavium.com>

There's a rare pci_driver.probe failure of the VF driver that's caused by
PF/VF handshake going out of sync.  The culprit is octeon_mbox_write() who
ignores an ack timeout condition; it just keeps unconditionally writing all
elements of mbox_cmd->data[] even when the other side is not ready for
them.  Fix it by making each write of mbox_cmd->data[i] conditional to
having previously received an ack.

Also fix the octeon_mbox_state enum such that each state gets a unique
value.  Also add ULL suffix to numeric literals in macro definitions.

Signed-off-by: Prasad Kanneganti <prasad.kanneganti@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
 drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c |  5 ++++-
 drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h | 12 ++++++------
 2 files changed, 10 insertions(+), 7 deletions(-)

Comments

Chickles, Derek May 25, 2017, 5:52 p.m. UTC | #1 
Looks good to me.

Derek

> -----Original Message-----
> From: Manlunas, Felix
> Sent: Thursday, May 25, 2017 10:42 AM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; Vatsavayi, Raghu <Raghu.Vatsavayi@cavium.com>;
> Chickles, Derek <Derek.Chickles@cavium.com>; Burla, Satananda
> <Satananda.Burla@cavium.com>; Kanneganti, Prasad
> <Prasad.Kanneganti@cavium.com>
> Subject: [PATCH net-next] liquidio: fix rare pci_driver.probe failure of
> VF driver
> 
> From: Prasad Kanneganti <prasad.kanneganti@cavium.com>
> 
> There's a rare pci_driver.probe failure of the VF driver that's caused by
> PF/VF handshake going out of sync.  The culprit is octeon_mbox_write() who
> ignores an ack timeout condition; it just keeps unconditionally writing
> all elements of mbox_cmd->data[] even when the other side is not ready for
> them.  Fix it by making each write of mbox_cmd->data[i] conditional to
> having previously received an ack.
> 
> Also fix the octeon_mbox_state enum such that each state gets a unique
> value.  Also add ULL suffix to numeric literals in macro definitions.
> 
> Signed-off-by: Prasad Kanneganti <prasad.kanneganti@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
> ---
>  drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c |  5 ++++-
> drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h | 12 ++++++------
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
> b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
> index 5cca73b..57af7df 100644
> --- a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
> +++ b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
> @@ -178,7 +178,10 @@ int octeon_mbox_write(struct octeon_device *oct,
>  					break;
>  				}
>  			}
> -			writeq(mbox_cmd->data[i], mbox->mbox_write_reg);
> +			if (ret == OCTEON_MBOX_STATUS_SUCCESS)
> +				writeq(mbox_cmd->data[i], mbox->mbox_write_reg);
> +			else
> +				break;
>  		}
>  	}
> 
> diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h
> b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h
> index c9376fe..1def22a 100644
> --- a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h
> +++ b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h
> @@ -20,16 +20,16 @@
> 
>  /* Macros for Mail Box Communication */
> 
> -#define OCTEON_MBOX_DATA_MAX	32
> +#define OCTEON_MBOX_DATA_MAX		32
> 
>  #define OCTEON_VF_ACTIVE		0x1
>  #define OCTEON_VF_FLR_REQUEST		0x2
>  #define OCTEON_PF_CHANGED_VF_MACADDR	0x4
> 
>  /*Macro for Read acknowldgement*/
> -#define OCTEON_PFVFACK			0xffffffffffffffff
> -#define OCTEON_PFVFSIG			0x1122334455667788
> -#define OCTEON_PFVFERR			0xDEADDEADDEADDEAD
> +#define OCTEON_PFVFACK			0xffffffffffffffffULL
> +#define OCTEON_PFVFSIG			0x1122334455667788ULL
> +#define OCTEON_PFVFERR			0xDEADDEADDEADDEADULL
> 
>  #define LIO_MBOX_WRITE_WAIT_CNT         1000
>  #define LIO_MBOX_WRITE_WAIT_TIME        msecs_to_jiffies(1)
> @@ -74,8 +74,8 @@ enum octeon_mbox_state {
>  	OCTEON_MBOX_STATE_REQUEST_RECEIVED = 4,
>  	OCTEON_MBOX_STATE_RESPONSE_PENDING = 8,
>  	OCTEON_MBOX_STATE_RESPONSE_RECEIVING = 16,
> -	OCTEON_MBOX_STATE_RESPONSE_RECEIVED = 16,
> -	OCTEON_MBOX_STATE_ERROR = 32
> +	OCTEON_MBOX_STATE_RESPONSE_RECEIVED = 32,
> +	OCTEON_MBOX_STATE_ERROR = 64
>  };
> 
>  struct octeon_mbox {
David Miller May 26, 2017, 6:42 p.m. UTC | #2 
From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Thu, 25 May 2017 10:42:14 -0700

> From: Prasad Kanneganti <prasad.kanneganti@cavium.com>
> 
> There's a rare pci_driver.probe failure of the VF driver that's caused by
> PF/VF handshake going out of sync.  The culprit is octeon_mbox_write() who
> ignores an ack timeout condition; it just keeps unconditionally writing all
> elements of mbox_cmd->data[] even when the other side is not ready for
> them.  Fix it by making each write of mbox_cmd->data[i] conditional to
> having previously received an ack.
> 
> Also fix the octeon_mbox_state enum such that each state gets a unique
> value.  Also add ULL suffix to numeric literals in macro definitions.
> 
> Signed-off-by: Prasad Kanneganti <prasad.kanneganti@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>

Applied.
diff mbox

Patch

diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
index 5cca73b..57af7df 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
@@ -178,7 +178,10 @@  int octeon_mbox_write(struct octeon_device *oct,
 					break;
 				}
 			}
-			writeq(mbox_cmd->data[i], mbox->mbox_write_reg);
+			if (ret == OCTEON_MBOX_STATUS_SUCCESS)
+				writeq(mbox_cmd->data[i], mbox->mbox_write_reg);
+			else
+				break;
 		}
 	}
 
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h
index c9376fe..1def22a 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.h
@@ -20,16 +20,16 @@ 
 
 /* Macros for Mail Box Communication */
 
-#define OCTEON_MBOX_DATA_MAX	32
+#define OCTEON_MBOX_DATA_MAX		32
 
 #define OCTEON_VF_ACTIVE		0x1
 #define OCTEON_VF_FLR_REQUEST		0x2
 #define OCTEON_PF_CHANGED_VF_MACADDR	0x4
 
 /*Macro for Read acknowldgement*/
-#define OCTEON_PFVFACK			0xffffffffffffffff
-#define OCTEON_PFVFSIG			0x1122334455667788
-#define OCTEON_PFVFERR			0xDEADDEADDEADDEAD
+#define OCTEON_PFVFACK			0xffffffffffffffffULL
+#define OCTEON_PFVFSIG			0x1122334455667788ULL
+#define OCTEON_PFVFERR			0xDEADDEADDEADDEADULL
 
 #define LIO_MBOX_WRITE_WAIT_CNT         1000
 #define LIO_MBOX_WRITE_WAIT_TIME        msecs_to_jiffies(1)
@@ -74,8 +74,8 @@  enum octeon_mbox_state {
 	OCTEON_MBOX_STATE_REQUEST_RECEIVED = 4,
 	OCTEON_MBOX_STATE_RESPONSE_PENDING = 8,
 	OCTEON_MBOX_STATE_RESPONSE_RECEIVING = 16,
-	OCTEON_MBOX_STATE_RESPONSE_RECEIVED = 16,
-	OCTEON_MBOX_STATE_ERROR = 32
+	OCTEON_MBOX_STATE_RESPONSE_RECEIVED = 32,
+	OCTEON_MBOX_STATE_ERROR = 64
 };
 
 struct octeon_mbox {