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

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

[Baoquan He]

This is v2 post.

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.
However bnx2 chip resetting involves firmware requesting issue, that
need be done in open stage. 

Michale Chan suggested we can just wait for the old in-flight DMA to
complete at probe stage, then though without device resetting, we
don't need to worry the old in-flight DMA could continue looking up 
the newly created io-page tables.

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.


Baoquan He (2):
  Revert "bnx2: Reset device during driver initialization"
  bnx2: Wait for in-flight DMA to complete at probe stage

 drivers/net/ethernet/broadcom/bnx2.c | 48 +++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 12 deletions(-)

-- 
2.5.5

[PATCH v2 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>
Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 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 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>
---
 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

[Michael Chan]

On Sat, Nov 12, 2016 at 9:01 PM, Baoquan He <bhe@redhat.com> wrote:
> 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>

Acked-by: Michael Chan <michael.chan@broadcom.com>

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

[Paul Menzel]

Dear Baoquan,

On 11/13/16 06:01, Baoquan He wrote:
> This is v2 post.
>
> 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.
> However bnx2 chip resetting involves firmware requesting issue, that
> need be done in open stage.
>
> Michale Chan suggested we can just wait for the old in-flight DMA to
> complete at probe stage, then though without device resetting, we
> don't need to worry the old in-flight DMA could continue looking up
> the newly created io-page tables.
>
> 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.

thank you for posting the updated series. Could you please resend a v3 
with stable@vger.kernel.org added [1]?


Kind regards,

Paul


[1] https://www.kernel.org/doc/Documentation/stable_kernel_rules.txt

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

[Baoquan He]

On 11/14/16 at 09:25am, Paul Menzel wrote:
> Dear Baoquan,
> 
> On 11/13/16 06:01, Baoquan He wrote:
> > This is v2 post.
> > 
> > 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.
> > However bnx2 chip resetting involves firmware requesting issue, that
> > need be done in open stage.
> > 
> > Michale Chan suggested we can just wait for the old in-flight DMA to
> > complete at probe stage, then though without device resetting, we
> > don't need to worry the old in-flight DMA could continue looking up
> > the newly created io-page tables.
> > 
> > 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.
> 
> thank you for posting the updated series. Could you please resend a v3 with
> stable@vger.kernel.org added [1]?

I can add it like:
Cc: <stable@vger.kernel.org> # 4.8.7

Only v4.8.7, right?

Thanks
Baoquan

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

[David Miller]

From: Paul Menzel <pmenzel@molgen.mpg.de>
Date: Mon, 14 Nov 2016 09:25:47 +0100

> Dear Baoquan,
> 
> On 11/13/16 06:01, Baoquan He wrote:
>> This is v2 post.
>>
>> 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.
>> However bnx2 chip resetting involves firmware requesting issue, that
>> need be done in open stage.
>>
>> Michale Chan suggested we can just wait for the old in-flight DMA to
>> complete at probe stage, then though without device resetting, we
>> don't need to worry the old in-flight DMA could continue looking up
>> the newly created io-page tables.
>>
>> 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.
> 
> thank you for posting the updated series. Could you please resend a v3
> with stable@vger.kernel.org added [1]?

This is not the proper procedure for networking patches.

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

[Paul Menzel]

On 11/14/16 18:28, David Miller wrote:
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Date: Mon, 14 Nov 2016 09:25:47 +0100

>> On 11/13/16 06:01, Baoquan He wrote:
>>> This is v2 post.
>>>
>>> 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.
>>> However bnx2 chip resetting involves firmware requesting issue, that
>>> need be done in open stage.
>>>
>>> Michale Chan suggested we can just wait for the old in-flight DMA to
>>> complete at probe stage, then though without device resetting, we
>>> don't need to worry the old in-flight DMA could continue looking up
>>> the newly created io-page tables.
>>>
>>> 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.
>>
>> thank you for posting the updated series. Could you please resend a v3
>> with stable@vger.kernel.org added [1]?
>
> This is not the proper procedure for networking patches.

Oh, I didn’t know that. Sorry for spreading false information.


Kind regards,

Paul

Re: [PATCH v2 0/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 13:01:31 +0800

> This is v2 post.
> 
> 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.
> However bnx2 chip resetting involves firmware requesting issue, that
> need be done in open stage. 
> 
> Michale Chan suggested we can just wait for the old in-flight DMA to
> complete at probe stage, then though without device resetting, we
> don't need to worry the old in-flight DMA could continue looking up 
> the newly created io-page tables.
> 
> 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.

Series applied and queued up for -stable, thanks.