[PATCH 0/2] bnx2: Hard reset bnx2 chip at probe stage

[PATCH 0/2] bnx2: Hard reset bnx2 chip at probe stage

[Baoquan He]

Hi bnx2 experts,

In commit 3e1be7a ("bnx2: Reset device during driver initialization"),
firmware requesting code was moved from open stage to probe stage.
The reason is in kdump kernel hardware iommu need device be reset in
driver probe stage, otherwise those in-flight DMA from 1st kernel
will continue going and look up into the newly created io-page tables.
So we need reset device to stop in-flight DMA as early as possibe.

But with commit 3e1be7a merged, people reported their bnx2 driver init
failed because of failed firmware loading. After discussion, it's found
that they built bnx2 driver into kernel, and that makes probe function
bnx2_init_one be called in do_initcalls(). But at this time the initramfs
has not been uncompressed yet and mounted, kernel can't detect firmware.

So there's only one way to cover both. Try to hard reset the bnx2 device
at probe stage, without involving firmware issues. I tried to add function
bnx2_hard_reset_chip() to do this and it's only called in kdump kernel.
The thing is I am not quite familiar with bnx2 chip spec, just abstract
code from bnx2_reset_chip, the testing result is good.

Any suggestions are welcomed and much appreciated!

Baoquan He (2):
  Revert "bnx2: Reset device during driver initialization"
  bnx2: Hard reset bnx2 chip at probe stage

 drivers/net/ethernet/broadcom/bnx2.c | 70 +++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 5 deletions(-)

-- 
2.5.5

[PATCH 1/2] Revert "bnx2: Reset device during driver initialization"

[Baoquan He]

This reverts commit 3e1be7ad2d38c6bd6aeef96df9bd0a7822f4e51c.

When people build bnx2 driver into kernel, it will fail to detect
and load firmware because firmware is contained in initramfs and
initramfs has not been uncompressed yet during do_initcalls. So
revert commit 3e1be7a and work out a new way in the later patch.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 drivers/net/ethernet/broadcom/bnx2.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
index b3791b3..c557972 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -6361,6 +6361,10 @@ bnx2_open(struct net_device *dev)
 	struct bnx2 *bp = netdev_priv(dev);
 	int rc;
 
+	rc = bnx2_request_firmware(bp);
+	if (rc < 0)
+		goto out;
+
 	netif_carrier_off(dev);
 
 	bnx2_disable_int(bp);
@@ -6429,6 +6433,7 @@ bnx2_open(struct net_device *dev)
 	bnx2_free_irq(bp);
 	bnx2_free_mem(bp);
 	bnx2_del_napi(bp);
+	bnx2_release_firmware(bp);
 	goto out;
 }
 
@@ -8575,12 +8580,6 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pdev, dev);
 
-	rc = bnx2_request_firmware(bp);
-	if (rc < 0)
-		goto error;
-
-
-	bnx2_reset_chip(bp, BNX2_DRV_MSG_CODE_RESET);
 	memcpy(dev->dev_addr, bp->mac_addr, ETH_ALEN);
 
 	dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
@@ -8613,7 +8612,6 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return 0;
 
 error:
-	bnx2_release_firmware(bp);
 	pci_iounmap(pdev, bp->regview);
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
-- 
2.5.5

[PATCH 2/2] bnx2: Hard reset bnx2 chip at probe stage

[Baoquan He]

In commit 3e1be7a ("bnx2: Reset device during driver initialization"),
firmware requesting code was moved to driver probe stage, into function
bnx2_init_one. But if bnx2 driver is build into kernel, it will fail
to request firmware because firmware is contained in initramfs, but
initramfs has not been uncommpressed and mounted yet when do_initcalls
called.

So in order to reset the device at probe stage, have to hard reset bnx2
chip wihtout involving firmware handling. So in this patch add function
bnx2_hard_reset_chip which only trys to hard reset bnx2 chip and only
will be called in kdump kernel.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 drivers/net/ethernet/broadcom/bnx2.c | 62 ++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
index c557972..84e3f12 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -49,6 +49,7 @@
 #include <linux/firmware.h>
 #include <linux/log2.h>
 #include <linux/aer.h>
+#include <linux/crash_dump.h>
 
 #if IS_ENABLED(CONFIG_CNIC)
 #define BCM_CNIC 1
@@ -4765,6 +4766,58 @@ bnx2_setup_msix_tbl(struct bnx2 *bp)
 }
 
 static int
+bnx2_hard_reset_chip(struct bnx2 *bp)
+{
+	u32 val;
+	int i, rc = 0;
+
+	if (BNX2_CHIP(bp) == BNX2_CHIP_5709) {
+		BNX2_WR(bp, BNX2_MISC_COMMAND, BNX2_MISC_COMMAND_HD_RESET);
+		BNX2_RD(bp, BNX2_MISC_COMMAND);
+		udelay(5);
+
+		val = BNX2_PCICFG_MISC_CONFIG_REG_WINDOW_ENA |
+		      BNX2_PCICFG_MISC_CONFIG_TARGET_MB_WORD_SWAP;
+
+		BNX2_WR(bp, BNX2_PCICFG_MISC_CONFIG, val);
+
+	} else {
+		val = BNX2_PCICFG_MISC_CONFIG_CORE_RST_REQ |
+		      BNX2_PCICFG_MISC_CONFIG_REG_WINDOW_ENA |
+		      BNX2_PCICFG_MISC_CONFIG_TARGET_MB_WORD_SWAP;
+
+		/* Chip reset. */
+		BNX2_WR(bp, BNX2_PCICFG_MISC_CONFIG, val);
+
+		/* Reading back any register after chip reset will hang the
+		 * bus on 5706 A0 and A1.  The msleep below provides plenty
+		 * of margin for write posting.
+		 */
+		if ((BNX2_CHIP_ID(bp) == BNX2_CHIP_ID_5706_A0) ||
+		    (BNX2_CHIP_ID(bp) == BNX2_CHIP_ID_5706_A1))
+			msleep(20);
+
+		/* Reset takes approximate 30 usec */
+		for (i = 0; i < 10; i++) {
+			val = BNX2_RD(bp, BNX2_PCICFG_MISC_CONFIG);
+			if ((val & (BNX2_PCICFG_MISC_CONFIG_CORE_RST_REQ |
+				    BNX2_PCICFG_MISC_CONFIG_CORE_RST_BSY)) == 0)
+				break;
+			udelay(10);
+		}
+
+		if (val & (BNX2_PCICFG_MISC_CONFIG_CORE_RST_REQ |
+			   BNX2_PCICFG_MISC_CONFIG_CORE_RST_BSY)) {
+			pr_err("Chip reset did not complete\n");
+			return -EBUSY;
+		}
+	}
+
+	return rc;
+}
+
+
+static int
 bnx2_reset_chip(struct bnx2 *bp, u32 reset_code)
 {
 	u32 val;
@@ -8580,6 +8633,15 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pdev, dev);
 
+
+	/*
+	 * Kdump kernel need reset device at probe stage if hardware iommu
+	 * is deployed. Otherwise in-flight DMA will continue going until
+	 * reset is done in open stage.
+	 */
+	if (is_kdump_kernel())
+		bnx2_hard_reset_chip(bp);
+
 	memcpy(dev->dev_addr, bp->mac_addr, ETH_ALEN);
 
 	dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
-- 
2.5.5

Re: [PATCH 1/2] Revert "bnx2: Reset device during driver initialization"

[Paul Menzel]

Dear Baoquan,


On 11/11/16 14:46, Baoquan He wrote:
> This reverts commit 3e1be7ad2d38c6bd6aeef96df9bd0a7822f4e51c.

Thanks a lot.

> When people build bnx2 driver into kernel, it will fail to detect
> and load firmware because firmware is contained in initramfs and
> initramfs has not been uncompressed yet during do_initcalls. So
> revert commit 3e1be7a and work out a new way in the later patch.

Just to note, that the other reason is, that in some installations 
people don’t have the firmware in initramfs at all or don’t use an 
initramfs.

> Signed-off-by: Baoquan He <bhe@redhat.com>

Please mark this for inclusion into the stable Linux kernel.

Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>


Thanks,

Paul

Re: [PATCH 0/2] bnx2: Hard reset bnx2 chip at probe stage

[Baoquan He]

On 11/11/16 at 09:46pm, Baoquan He wrote:
> Hi bnx2 experts,
> 
> In commit 3e1be7a ("bnx2: Reset device during driver initialization"),
> firmware requesting code was moved from open stage to probe stage.
> The reason is in kdump kernel hardware iommu need device be reset in
> driver probe stage, otherwise those in-flight DMA from 1st kernel
> will continue going and look up into the newly created io-page tables.
> So we need reset device to stop in-flight DMA as early as possibe.
> 
> But with commit 3e1be7a merged, people reported their bnx2 driver init
> failed because of failed firmware loading. After discussion, it's found
> that they built bnx2 driver into kernel, and that makes probe function
> bnx2_init_one be called in do_initcalls(). But at this time the initramfs
> has not been uncompressed yet and mounted, kernel can't detect firmware.
> 
> So there's only one way to cover both. Try to hard reset the bnx2 device
> at probe stage, without involving firmware issues. I tried to add function
> bnx2_hard_reset_chip() to do this and it's only called in kdump kernel.
> The thing is I am not quite familiar with bnx2 chip spec, just abstract
> code from bnx2_reset_chip, the testing result is good.

Here I changed to send BNX2_MISC_COMMAND_HD_RESET in BNX2_CHIP_5709
case.

> 
> Any suggestions are welcomed and much appreciated!
> 
> Baoquan He (2):
>   Revert "bnx2: Reset device during driver initialization"
>   bnx2: Hard reset bnx2 chip at probe stage
> 
>  drivers/net/ethernet/broadcom/bnx2.c | 70 +++++++++++++++++++++++++++++++++---
>  1 file changed, 65 insertions(+), 5 deletions(-)
> 
> -- 
> 2.5.5
> 

Re: [PATCH 0/2] bnx2: Hard reset bnx2 chip at probe stage

[Michael Chan]

On Fri, Nov 11, 2016 at 6:02 AM, Baoquan He <bhe@redhat.com> wrote:
> On 11/11/16 at 09:46pm, Baoquan He wrote:
>> Hi bnx2 experts,
>>
>> In commit 3e1be7a ("bnx2: Reset device during driver initialization"),
>> firmware requesting code was moved from open stage to probe stage.
>> The reason is in kdump kernel hardware iommu need device be reset in
>> driver probe stage, otherwise those in-flight DMA from 1st kernel
>> will continue going and look up into the newly created io-page tables.
>> So we need reset device to stop in-flight DMA as early as possibe.
>>
>> But with commit 3e1be7a merged, people reported their bnx2 driver init
>> failed because of failed firmware loading. After discussion, it's found
>> that they built bnx2 driver into kernel, and that makes probe function
>> bnx2_init_one be called in do_initcalls(). But at this time the initramfs
>> has not been uncompressed yet and mounted, kernel can't detect firmware.
>>
>> So there's only one way to cover both. Try to hard reset the bnx2 device
>> at probe stage, without involving firmware issues. I tried to add function
>> bnx2_hard_reset_chip() to do this and it's only called in kdump kernel.
>> The thing is I am not quite familiar with bnx2 chip spec, just abstract
>> code from bnx2_reset_chip, the testing result is good.
>
> Here I changed to send BNX2_MISC_COMMAND_HD_RESET in BNX2_CHIP_5709
> case.
>

>From my old 5709 Documentation:

Bit 6 HD_RESET:  Writing this bit as 1 will cause the chip to do a
hard reset like bit 5 except the sticky bits in the PCI function are
not reset.

Bit 5 POR_RESET: Writing this bit as 1 will cause the chip to do an
internal reset exactly like a power-up reset.  There is no protection
for this request and it may cause any current PCI cycle to lock up.
This reset is intended for use under manufacturing conditions only.

So it sounds like doing HD_RESET can potentially cause a PCI bus lock up.

Why not just disable DMA gracefully as done at the beginning of
bnx2_reset_chip()?

Re: [PATCH 0/2] bnx2: Hard reset bnx2 chip at probe stage

[Baoquan He]

Hi Michael,


On 11/11/16 at 09:37am, Michael Chan wrote:
> On Fri, Nov 11, 2016 at 6:02 AM, Baoquan He <bhe@redhat.com> wrote:
> > On 11/11/16 at 09:46pm, Baoquan He wrote:
> >> Hi bnx2 experts,
> >>
> >> In commit 3e1be7a ("bnx2: Reset device during driver initialization"),
> >> firmware requesting code was moved from open stage to probe stage.
> >> The reason is in kdump kernel hardware iommu need device be reset in
> >> driver probe stage, otherwise those in-flight DMA from 1st kernel
> >> will continue going and look up into the newly created io-page tables.
> >> So we need reset device to stop in-flight DMA as early as possibe.
> >>
> >> But with commit 3e1be7a merged, people reported their bnx2 driver init
> >> failed because of failed firmware loading. After discussion, it's found
> >> that they built bnx2 driver into kernel, and that makes probe function
> >> bnx2_init_one be called in do_initcalls(). But at this time the initramfs
> >> has not been uncompressed yet and mounted, kernel can't detect firmware.
> >>
> >> So there's only one way to cover both. Try to hard reset the bnx2 device
> >> at probe stage, without involving firmware issues. I tried to add function
> >> bnx2_hard_reset_chip() to do this and it's only called in kdump kernel.
> >> The thing is I am not quite familiar with bnx2 chip spec, just abstract
> >> code from bnx2_reset_chip, the testing result is good.
> >
> > Here I changed to send BNX2_MISC_COMMAND_HD_RESET in BNX2_CHIP_5709
> > case.
> >
> 
> From my old 5709 Documentation:
> 
> Bit 6 HD_RESET:  Writing this bit as 1 will cause the chip to do a
> hard reset like bit 5 except the sticky bits in the PCI function are
> not reset.
> 
> Bit 5 POR_RESET: Writing this bit as 1 will cause the chip to do an
> internal reset exactly like a power-up reset.  There is no protection
> for this request and it may cause any current PCI cycle to lock up.
> This reset is intended for use under manufacturing conditions only.
> 
> So it sounds like doing HD_RESET can potentially cause a PCI bus lock up.
> 
> Why not just disable DMA gracefully as done at the beginning of
> bnx2_reset_chip()?

Thanks for your suggestion.

If what I undertand is correct, you meant waiting for the current PCI
transaction to complete, right? I tried and it also works. I like this
idea, will repost v2. Please help check if it meets your thoughts.

Thanks
Baoquan

[PATCH v2 2/2] bnx2: Wait for in-flight DMA to complete at probe stage

[Baoquan He]

In-flight DMA from 1st kernel could continue going in kdump kernel.
New io-page table has been created before bnx2 does reset at open stage.
We have to wait for the in-flight DMA to complete to avoid it look up
into the newly created io-page table at probe stage.

Suggested-by: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Baoquan He <bhe@redhat.com>
---
v1->v2:
    Michael suggested to wait for the in-flight DMA to complete at probe
    stage. So give up the old method of trying to reset chip at probe
    stage, take the new way accordingly.

 drivers/net/ethernet/broadcom/bnx2.c | 38 ++++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
index c557972..1f7034d 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -49,6 +49,7 @@
 #include <linux/firmware.h>
 #include <linux/log2.h>
 #include <linux/aer.h>
+#include <linux/crash_dump.h>
 
 #if IS_ENABLED(CONFIG_CNIC)
 #define BCM_CNIC 1
@@ -4764,15 +4765,16 @@ bnx2_setup_msix_tbl(struct bnx2 *bp)
 	BNX2_WR(bp, BNX2_PCI_GRC_WINDOW3_ADDR, BNX2_MSIX_PBA_ADDR);
 }
 
-static int
-bnx2_reset_chip(struct bnx2 *bp, u32 reset_code)
+static void
+bnx2_wait_dma_complete(struct bnx2 *bp)
 {
 	u32 val;
-	int i, rc = 0;
-	u8 old_port;
+	int i;
 
-	/* Wait for the current PCI transaction to complete before
-	 * issuing a reset. */
+	/*
+	 * Wait for the current PCI transaction to complete before
+	 * issuing a reset.
+	 */
 	if ((BNX2_CHIP(bp) == BNX2_CHIP_5706) ||
 	    (BNX2_CHIP(bp) == BNX2_CHIP_5708)) {
 		BNX2_WR(bp, BNX2_MISC_ENABLE_CLR_BITS,
@@ -4796,6 +4798,21 @@ bnx2_reset_chip(struct bnx2 *bp, u32 reset_code)
 		}
 	}
 
+	return;
+}
+
+
+static int
+bnx2_reset_chip(struct bnx2 *bp, u32 reset_code)
+{
+	u32 val;
+	int i, rc = 0;
+	u8 old_port;
+
+	/* Wait for the current PCI transaction to complete before
+	 * issuing a reset. */
+	bnx2_wait_dma_complete(bp);
+
 	/* Wait for the firmware to tell us it is ok to issue a reset. */
 	bnx2_fw_sync(bp, BNX2_DRV_MSG_DATA_WAIT0 | reset_code, 1, 1);
 
@@ -8580,6 +8597,15 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pdev, dev);
 
+	/*
+	 * In-flight DMA from 1st kernel could continue going in kdump kernel.
+	 * New io-page table has been created before bnx2 does reset at open stage.
+	 * We have to wait for the in-flight DMA to complete to avoid it look up
+	 * into the newly created io-page table.
+	 */
+	if (is_kdump_kernel())
+		bnx2_wait_dma_complete(bp);
+
 	memcpy(dev->dev_addr, bp->mac_addr, ETH_ALEN);
 
 	dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
-- 
2.5.5

Re: [PATCH v2 2/2] bnx2: Wait for in-flight DMA to complete at probe stage

[David Miller]

From: Baoquan He <bhe@redhat.com>
Date: Sun, 13 Nov 2016 12:15:24 +0800

> In-flight DMA from 1st kernel could continue going in kdump kernel.
> New io-page table has been created before bnx2 does reset at open stage.
> We have to wait for the in-flight DMA to complete to avoid it look up
> into the newly created io-page table at probe stage.
> 
> Suggested-by: Michael Chan <michael.chan@broadcom.com>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
> v1->v2:
>     Michael suggested to wait for the in-flight DMA to complete at probe
>     stage. So give up the old method of trying to reset chip at probe
>     stage, take the new way accordingly.

Patch updates don't work this way.

When you update a patch that is part of a patch series, you must
resubmit the entire series anew.

Thank you.

Re: [PATCH v2 2/2] bnx2: Wait for in-flight DMA to complete at probe stage

[Baoquan He]

On 11/12/16 at 11:40pm, David Miller wrote:
> From: Baoquan He <bhe@redhat.com>
> Date: Sun, 13 Nov 2016 12:15:24 +0800
> 
> > In-flight DMA from 1st kernel could continue going in kdump kernel.
> > New io-page table has been created before bnx2 does reset at open stage.
> > We have to wait for the in-flight DMA to complete to avoid it look up
> > into the newly created io-page table at probe stage.
> > 
> > Suggested-by: Michael Chan <michael.chan@broadcom.com>
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > ---
> > v1->v2:
> >     Michael suggested to wait for the in-flight DMA to complete at probe
> >     stage. So give up the old method of trying to reset chip at probe
> >     stage, take the new way accordingly.
> 
> Patch updates don't work this way.
> 
> When you update a patch that is part of a patch series, you must
> resubmit the entire series anew.

Thanks for telling, David!

Learned it. I am not very sure if this is what Michael is suggesting.
Will resubmit the entire patch series.

Thanks
Baoquan