[PATCH V2 0/4] Enable bus master till the FLR completes for Chelsio T4

[PATCH V2 0/4] Enable bus master till the FLR completes for Chelsio T4

[Vipul Pandya]

This patch series adds device specific reset method for Chelsio T4 adapters.

The series also adds new routine, pci_wait_for_pending_transaction, to wait for
pending pci transaction. There are few places where waiting for pending pci
transaction code was duplicated. This patch series also replaces occurances of
duplicate code.

This patch series is built against Bjorn Helgaas pci tree next branch. We would
like to merge this series via the same.

I have Cced respective maintainers of the modified drivers. Kindly review and
let us know in case of any review comments.

Thanks,
Vipul Pandya

V2: Added pci_wait_for_pending_transaction routine and replaced it at respective
places. Made the patch series for the same.
V2: Used dev->msix_cap directly for capability offset
V2: Removed use of 'rc' variable.

Casey Leedom (4):
  pci: Add pci_wait_for_pending_transaction to wait for pending pci
    transaction
  pci: Enable bus master till the FLR completes
  bnx2x: Use pci_wait_for_pending_transaction instead of for loop
  pci: Use pci_wait_for_pending_transaction instead of for loop

 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 18 +----
 drivers/pci/pci.c                                | 38 ++++++----
 drivers/pci/quirks.c                             | 97 ++++++++++++++++++++----
 include/linux/pci.h                              |  1 +
 4 files changed, 109 insertions(+), 45 deletions(-)

-- 
1.8.0

[PATCH V2 1/4] pci: Add pci_wait_for_pending_transaction to wait for pending pci transaction

[Vipul Pandya]

From: Casey Leedom <leedom@chelsio.com>

New routine to avoid duplication of waiting for pending pci transaction code.

Signed-off-by: Casey Leedom <leedom@chelsio.com>
Signed-off-by: Vipul Pandya <vipul@chelsio.com>
---
V2: Added pci_wait_for_pending_transaction routine and replaced it at respective
places. Made the patch series for the same.
V2: Used dev->msix_cap directly for capability offset
V2: Removed use of 'rc' variable.

 drivers/pci/pci.c   | 38 +++++++++++++++++++++++++-------------
 include/linux/pci.h |  1 +
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a8d5fd0..7d6ce2e 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3159,19 +3159,17 @@ int pci_set_dma_seg_boundary(struct pci_dev *dev, unsigned long mask)
 }
 EXPORT_SYMBOL(pci_set_dma_seg_boundary);
 
-static int pcie_flr(struct pci_dev *dev, int probe)
+/**
+ * pci_wait_for_pending_transaction - waits for pending transaction
+ * @dev: the PCI device to operate on
+ *
+ * Return 0 if transaction is pending 1 otherwise.
+ */
+int pci_wait_for_pending_transaction(struct pci_dev *dev)
 {
 	int i;
-	u32 cap;
 	u16 status;
 
-	pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
-	if (!(cap & PCI_EXP_DEVCAP_FLR))
-		return -ENOTTY;
-
-	if (probe)
-		return 0;
-
 	/* Wait for Transaction Pending bit clean */
 	for (i = 0; i < 4; i++) {
 		if (i)
@@ -3179,13 +3177,27 @@ static int pcie_flr(struct pci_dev *dev, int probe)
 
 		pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &status);
 		if (!(status & PCI_EXP_DEVSTA_TRPND))
-			goto clear;
+			return 1;
 	}
 
-	dev_err(&dev->dev, "transaction is not cleared; "
-			"proceeding with reset anyway\n");
+	return 0;
+}
+EXPORT_SYMBOL(pci_wait_for_pending_transaction);
+
+static int pcie_flr(struct pci_dev *dev, int probe)
+{
+	u32 cap;
+
+	pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
+	if (!(cap & PCI_EXP_DEVCAP_FLR))
+		return -ENOTTY;
+
+	if (probe)
+		return 0;
+
+	if (!pci_wait_for_pending_transaction(dev))
+		dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
 
-clear:
 	pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
 
 	msleep(100);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 0fd1f15..e647001 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -914,6 +914,7 @@ bool pci_check_and_unmask_intx(struct pci_dev *dev);
 void pci_msi_off(struct pci_dev *dev);
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size);
 int pci_set_dma_seg_boundary(struct pci_dev *dev, unsigned long mask);
+int pci_wait_for_pending_transaction(struct pci_dev *dev);
 int pcix_get_max_mmrbc(struct pci_dev *dev);
 int pcix_get_mmrbc(struct pci_dev *dev);
 int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc);
-- 
1.8.0

[PATCH V2 2/4] pci: Enable bus master till the FLR completes

[Vipul Pandya]

From: Casey Leedom <leedom@chelsio.com>

T4 can wedge if there are DMAs in flight within the chip and Bus master has
been disabled.  We need to have it on till the Function Level Reset completes.
T4 can also suffer a Head Of Line blocking problem if MSI-X interrupts are
disabled before the FLR has completed.

Signed-off-by: Casey Leedom <leedom@chelsio.com>
Signed-off-by: Vipul Pandya <vipul@chelsio.com>
---
V2: Added pci_wait_for_pending_transaction routine and replaced it at respective
places. Made the patch series for the same.
V2: Used dev->msix_cap directly for capability offset
V2: Removed use of 'rc' variable.

 drivers/pci/quirks.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 2e2ea22..4a64fc4 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3208,6 +3208,83 @@ reset_complete:
 	return 0;
 }
 
+/*
+ * Device-specific reset method for Chelsio T4-based adapters.
+ */
+static int reset_chelsio_generic_dev(struct pci_dev *dev, int probe)
+{
+	u16 old_command;
+	u16 msix_flags;
+
+	/*
+	 * If this isn't a Chelsio T4-based device, return -ENOTTY indicating
+	 * that we have no device-specific reset method.
+	 */
+	if ((dev->device & 0xf000) != 0x4000)
+		return -ENOTTY;
+
+	/*
+	 * If this is the "probe" phase, return 0 indicating that we can
+	 * reset this device.
+	 */
+	if (probe)
+		return 0;
+
+	/*
+	 * T4 can wedge if their are DMAs in flight within the chip and Bus
+	 * master has been disabled.  We need to have it on till the Function
+	 * Level Reset completes.  (BUS_MASTER is disabled in
+	 * pci_reset_function()).
+	 */
+	pci_read_config_word(dev, PCI_COMMAND, &old_command);
+	pci_write_config_word(dev, PCI_COMMAND,
+			      old_command | PCI_COMMAND_MASTER);
+
+	/*
+	 * Perform the actual device function reset, saving and restoring
+	 * configuration information around the reset.
+	 */
+	pci_save_state(dev);
+
+	/*
+	 * T4 also suffers a Head-Of-Line blocking problem if MSI-X interrupts
+	 * are disabled when an MSI-X interrupt message needs to be delivered.
+	 * So we briefly re-enable MSI-X interrupts for the duration of the
+	 * FLR.  The pci_restore_state() below will restore the original
+	 * MSI-X state.
+	 */
+	pci_read_config_word(dev, dev->msix_cap+PCI_MSIX_FLAGS, &msix_flags);
+	if ((msix_flags & PCI_MSIX_FLAGS_ENABLE) == 0)
+		pci_write_config_word(dev, dev->msix_cap+PCI_MSIX_FLAGS,
+				      msix_flags |
+				      PCI_MSIX_FLAGS_ENABLE |
+				      PCI_MSIX_FLAGS_MASKALL);
+
+	/*
+	 * Start of pcie_flr() code sequence.  This reset code is a copy of
+	 * the guts of pcie_flr() because that's not an exported function.
+	 */
+
+	if (!pci_wait_for_pending_transaction(dev))
+		dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
+
+	pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
+	msleep(100);
+
+	/*
+	 * End of pcie_flr() code sequence.
+	 */
+
+	/*
+	 * Restore the configuration information (BAR values, etc.) including
+	 * the original PCI Configuration Space Command word, and return
+	 * success.
+	 */
+	pci_restore_state(dev);
+	pci_write_config_word(dev, PCI_COMMAND, old_command);
+	return 0;
+}
+
 #define PCI_DEVICE_ID_INTEL_82599_SFP_VF   0x10ed
 #define PCI_DEVICE_ID_INTEL_IVB_M_VGA      0x0156
 #define PCI_DEVICE_ID_INTEL_IVB_M2_VGA     0x0166
@@ -3221,6 +3298,8 @@ static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
 		reset_ivb_igd },
 	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
 		reset_intel_generic_dev },
+	{ PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID,
+		reset_chelsio_generic_dev },
 	{ 0 }
 };
 
-- 
1.8.0

[PATCH V2 3/4] bnx2x: Use pci_wait_for_pending_transaction instead of for loop

[Vipul Pandya]

From: Casey Leedom <leedom@chelsio.com>

New routine has been added to avoid duplication of waiting for pending pci
transaction code. This patch makes use of that routine.

Signed-off-by: Casey Leedom <leedom@chelsio.com>
Signed-off-by: Vipul Pandya <vipul@chelsio.com>
---
V2: Added pci_wait_for_pending_transaction routine and replaced it at respective
places. Made the patch series for the same.
V2: Used dev->msix_cap directly for capability offset
V2: Removed use of 'rc' variable.

 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index e5da078..c51d2f8 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -9935,8 +9935,6 @@ static int bnx2x_prev_mark_path(struct bnx2x *bp, bool after_undi)
 
 static int bnx2x_do_flr(struct bnx2x *bp)
 {
-	int i;
-	u16 status;
 	struct pci_dev *dev = bp->pdev;
 
 	if (CHIP_IS_E1x(bp)) {
@@ -9951,20 +9949,8 @@ static int bnx2x_do_flr(struct bnx2x *bp)
 		return -EINVAL;
 	}
 
-	/* Wait for Transaction Pending bit clean */
-	for (i = 0; i < 4; i++) {
-		if (i)
-			msleep((1 << (i - 1)) * 100);
-
-		pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &status);
-		if (!(status & PCI_EXP_DEVSTA_TRPND))
-			goto clear;
-	}
-
-	dev_err(&dev->dev,
-		"transaction is not cleared; proceeding with reset anyway\n");
-
-clear:
+	if (!pci_wait_for_pending_transaction(dev))
+		dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
 
 	BNX2X_DEV_INFO("Initiating FLR\n");
 	bnx2x_fw_command(bp, DRV_MSG_CODE_INITIATE_FLR, 0);
-- 
1.8.0

Re: [PATCH V2 3/4] bnx2x: Use pci_wait_for_pending_transaction instead of for loop

[Eilon Greenstein]

On Tue, 2013-08-06 at 15:48 +0530, Vipul Pandya wrote:
> From: Casey Leedom <leedom@chelsio.com>
> 
> New routine has been added to avoid duplication of waiting for pending pci
> transaction code. This patch makes use of that routine.
> 
> Signed-off-by: Casey Leedom <leedom@chelsio.com>
> Signed-off-by: Vipul Pandya <vipul@chelsio.com>

Thanks Casey!

Acked-by: Eilon Greenstein <eilong@broadcom.com>

[PATCH V2 4/4] pci: Use pci_wait_for_pending_transaction instead of for loop

[Vipul Pandya]

From: Casey Leedom <leedom@chelsio.com>

New routine has been added to avoid duplication of waiting for pending pci
transaction code. This patch makes use of that function.

Signed-off-by: Casey Leedom <leedom@chelsio.com>
Signed-off-by: Vipul Pandya <vipul@chelsio.com>
---
V2: Added pci_wait_for_pending_transaction routine and replaced it at respective
places. Made the patch series for the same.
V2: Used dev->msix_cap directly for capability offset
V2: Removed use of 'rc' variable.

 drivers/pci/quirks.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 4a64fc4..ea9108d 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3126,9 +3126,6 @@ static int reset_intel_generic_dev(struct pci_dev *dev, int probe)
 
 static int reset_intel_82599_sfp_virtfn(struct pci_dev *dev, int probe)
 {
-	int i;
-	u16 status;
-
 	/*
 	 * http://www.intel.com/content/dam/doc/datasheet/82599-10-gbe-controller-datasheet.pdf
 	 *
@@ -3140,20 +3137,9 @@ static int reset_intel_82599_sfp_virtfn(struct pci_dev *dev, int probe)
 	if (probe)
 		return 0;
 
-	/* Wait for Transaction Pending bit clean */
-	for (i = 0; i < 4; i++) {
-		if (i)
-			msleep((1 << (i - 1)) * 100);
-
-		pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &status);
-		if (!(status & PCI_EXP_DEVSTA_TRPND))
-			goto clear;
-	}
-
-	dev_err(&dev->dev, "transaction is not cleared; "
-			"proceeding with reset anyway\n");
+	if (!pci_wait_for_pending_transaction(dev))
+		dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
 
-clear:
 	pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
 
 	msleep(100);
-- 
1.8.0

Re: [PATCH V2 0/4] Enable bus master till the FLR completes for Chelsio T4

[David Miller]

From: Vipul Pandya <vipul@chelsio.com>
Date: Tue,  6 Aug 2013 15:48:35 +0530

> This patch series adds device specific reset method for Chelsio T4 adapters.
> 
> The series also adds new routine, pci_wait_for_pending_transaction, to wait for
> pending pci transaction. There are few places where waiting for pending pci
> transaction code was duplicated. This patch series also replaces occurances of
> duplicate code.
> 
> This patch series is built against Bjorn Helgaas pci tree next branch. We would
> like to merge this series via the same.
> 
> I have Cced respective maintainers of the modified drivers. Kindly review and
> let us know in case of any review comments.

Please push this via the PCI tree, and for the networking bits you can add:

Acked-by: David S. Miller <davem@davemloft.net>

Thanks.

Re: [PATCH V2 0/4] Enable bus master till the FLR completes for Chelsio T4

[Casey Leedom]

On 08/07/13 17:08, David Miller wrote:
> From: Vipul Pandya <vipul@chelsio.com>
> Date: Tue,  6 Aug 2013 15:48:35 +0530
>
>> This patch series adds device specific reset method for Chelsio T4 adapters.
>>
>> The series also adds new routine, pci_wait_for_pending_transaction, to wait for
>> pending pci transaction. There are few places where waiting for pending pci
>> transaction code was duplicated. This patch series also replaces occurances of
>> duplicate code.
>>
>> This patch series is built against Bjorn Helgaas pci tree next branch. We would
>> like to merge this series via the same.
>>
>> I have Cced respective maintainers of the modified drivers. Kindly review and
>> let us know in case of any review comments.
> Please push this via the PCI tree, and for the networking bits you can add:
>
> Acked-by: David S. Miller <davem@davemloft.net>
>
> Thanks.

Thanks David.  I ~think~ that's covered and we've already gone through 
one round of reviews that Bjorn Helgaas graciously provided.

Note that this patch should also obviate the need for the patch which we 
attempted to get into the Chelsio T4 network driver cxgb4 several months 
ago which monitored the chip for a hang caused by KVM detaching a T4 
Virtual Function from a Virtual Machine.  You were unhappy with that 
patch and requested that we approach the problem differently.  Thanks 
for your push.

Casey

Re: [PATCH V2 0/4] Enable bus master till the FLR completes for Chelsio T4

[Bjorn Helgaas]

On Tue, Aug 6, 2013 at 4:18 AM, Vipul Pandya <vipul@chelsio.com> wrote:
> This patch series adds device specific reset method for Chelsio T4 adapters.
>
> The series also adds new routine, pci_wait_for_pending_transaction, to wait for
> pending pci transaction. There are few places where waiting for pending pci
> transaction code was duplicated. This patch series also replaces occurances of
> duplicate code.
>
> This patch series is built against Bjorn Helgaas pci tree next branch. We would
> like to merge this series via the same.
>
> I have Cced respective maintainers of the modified drivers. Kindly review and
> let us know in case of any review comments.
>
> Thanks,
> Vipul Pandya
>
> V2: Added pci_wait_for_pending_transaction routine and replaced it at respective
> places. Made the patch series for the same.
> V2: Used dev->msix_cap directly for capability offset
> V2: Removed use of 'rc' variable.
>
> Casey Leedom (4):
>   pci: Add pci_wait_for_pending_transaction to wait for pending pci
>     transaction
>   pci: Enable bus master till the FLR completes
>   bnx2x: Use pci_wait_for_pending_transaction instead of for loop
>   pci: Use pci_wait_for_pending_transaction instead of for loop
>
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 18 +----
>  drivers/pci/pci.c                                | 38 ++++++----
>  drivers/pci/quirks.c                             | 97 ++++++++++++++++++++----
>  include/linux/pci.h                              |  1 +
>  4 files changed, 109 insertions(+), 45 deletions(-)

Applied to my "next" branch for v3.12, thanks!

Bjorn

Re: [PATCH V2 0/4] Enable bus master till the FLR completes for Chelsio T4

[Casey Leedom]

On 08/12/13 14:17, Bjorn Helgaas wrote:
> On Tue, Aug 6, 2013 at 4:18 AM, Vipul Pandya <vipul@chelsio.com> wrote:
>> This patch series adds device specific reset method for Chelsio T4 adapters.
>>
>> ...
> Applied to my "next" branch for v3.12, thanks!
>

   Thanks Bjorn!  Do you have an idea when these commits will hit Linus' 
repository and what their git commit IDs will be?  We have a customer 
using SLES11sp2 who needs this and once the commits hit Linus' 
repository we'll need to send a request to Novell to pull the change 
sets into SLES11sp2.  Thanks!

Casey

Re: [PATCH V2 0/4] Enable bus master till the FLR completes for Chelsio T4

[Bjorn Helgaas]

On Tue, Aug 13, 2013 at 3:53 PM, Casey Leedom <leedom@chelsio.com> wrote:
>
> On 08/12/13 14:17, Bjorn Helgaas wrote:
>>
>> On Tue, Aug 6, 2013 at 4:18 AM, Vipul Pandya <vipul@chelsio.com> wrote:
>>>
>>> This patch series adds device specific reset method for Chelsio T4
>>> adapters.
>>>
>>> ...
>>
>> Applied to my "next" branch for v3.12, thanks!
>>
>
>   Thanks Bjorn!  Do you have an idea when these commits will hit Linus'
> repository and what their git commit IDs will be?  We have a customer using
> SLES11sp2 who needs this and once the commits hit Linus' repository we'll
> need to send a request to Novell to pull the change sets into SLES11sp2.

They should appear in Linus' repository a few days after the v3.11
release, which will likely be in the first half of September.

The commit IDs should be the same as those in my repo, which you can
see here: http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/vipul-chelsio-reset-v2

Bjorn