[PATCH v3 net-next 0/4] ionic: add FLR support

[PATCH v3 net-next 0/4] ionic: add FLR support

[Shannon Nelson]

Add support for handing and recovering from a PCI FLR event.
This patchset first moves some code around to make it usable
from multiple paths, then adds the PCI error handler callbacks
for reset_prepare and reset_done.

Example test:
    echo 1 > /sys/bus/pci/devices/0000:2a:00.0/reset

v3:
 - removed first patch, it is already merged into net

v2:
Link: https://lore.kernel.org/netdev/20230713192936.45152-1-shannon.nelson@amd.com/
 - removed redundant pci_save/restore_state() calls

Shannon Nelson (4):
  ionic: extract common bits from ionic_remove
  ionic: extract common bits from ionic_probe
  ionic: pull out common bits from fw_up
  ionic: add FLR recovery support

 .../ethernet/pensando/ionic/ionic_bus_pci.c   | 161 +++++++++++++-----
 .../net/ethernet/pensando/ionic/ionic_lif.c   |  72 +++++---
 .../net/ethernet/pensando/ionic/ionic_lif.h   |   5 +
 3 files changed, 165 insertions(+), 73 deletions(-)

-- 
2.17.1

[PATCH v3 net-next 1/4] ionic: extract common bits from ionic_remove

[Shannon Nelson]

Pull out a chunk of code from ionic_remove() that will
be common in teardown paths.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
 .../ethernet/pensando/ionic/ionic_bus_pci.c   | 25 ++++++++++---------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index ab7d217b98b3..2bc3cab3967d 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -213,6 +213,13 @@ static int ionic_sriov_configure(struct pci_dev *pdev, int num_vfs)
 	return ret;
 }
 
+static void ionic_clear_pci(struct ionic *ionic)
+{
+	ionic_unmap_bars(ionic);
+	pci_release_regions(ionic->pdev);
+	pci_disable_device(ionic->pdev);
+}
+
 static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct device *dev = &pdev->dev;
@@ -249,20 +256,20 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	err = pci_request_regions(pdev, IONIC_DRV_NAME);
 	if (err) {
 		dev_err(dev, "Cannot request PCI regions: %d, aborting\n", err);
-		goto err_out_pci_disable_device;
+		goto err_out_clear_pci;
 	}
 
 	pcie_print_link_status(pdev);
 
 	err = ionic_map_bars(ionic);
 	if (err)
-		goto err_out_pci_release_regions;
+		goto err_out_clear_pci;
 
 	/* Configure the device */
 	err = ionic_setup(ionic);
 	if (err) {
 		dev_err(dev, "Cannot setup device: %d, aborting\n", err);
-		goto err_out_unmap_bars;
+		goto err_out_clear_pci;
 	}
 	pci_set_master(pdev);
 
@@ -353,12 +360,8 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	ionic_reset(ionic);
 err_out_teardown:
 	ionic_dev_teardown(ionic);
-err_out_unmap_bars:
-	ionic_unmap_bars(ionic);
-err_out_pci_release_regions:
-	pci_release_regions(pdev);
-err_out_pci_disable_device:
-	pci_disable_device(pdev);
+err_out_clear_pci:
+	ionic_clear_pci(ionic);
 err_out_debugfs_del_dev:
 	ionic_debugfs_del_dev(ionic);
 err_out_clear_drvdata:
@@ -386,9 +389,7 @@ static void ionic_remove(struct pci_dev *pdev)
 	ionic_port_reset(ionic);
 	ionic_reset(ionic);
 	ionic_dev_teardown(ionic);
-	ionic_unmap_bars(ionic);
-	pci_release_regions(pdev);
-	pci_disable_device(pdev);
+	ionic_clear_pci(ionic);
 	ionic_debugfs_del_dev(ionic);
 	mutex_destroy(&ionic->dev_cmd_lock);
 	ionic_devlink_free(ionic);
-- 
2.17.1

Re: [PATCH v3 net-next 1/4] ionic: extract common bits from ionic_remove

[Simon Horman]

On Mon, Jul 17, 2023 at 09:59:58AM -0700, Shannon Nelson wrote:
> Pull out a chunk of code from ionic_remove() that will
> be common in teardown paths.
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> ---
>  .../ethernet/pensando/ionic/ionic_bus_pci.c   | 25 ++++++++++---------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> index ab7d217b98b3..2bc3cab3967d 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> @@ -213,6 +213,13 @@ static int ionic_sriov_configure(struct pci_dev *pdev, int num_vfs)
>  	return ret;
>  }
>  
> +static void ionic_clear_pci(struct ionic *ionic)
> +{
> +	ionic_unmap_bars(ionic);
> +	pci_release_regions(ionic->pdev);
> +	pci_disable_device(ionic->pdev);

Hi Shannon,

is it safe to call pci_release_regions() even if a successful call to
pci_request_regions() has not been made?

> +}
> +
>  static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -249,20 +256,20 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	err = pci_request_regions(pdev, IONIC_DRV_NAME);
>  	if (err) {
>  		dev_err(dev, "Cannot request PCI regions: %d, aborting\n", err);
> -		goto err_out_pci_disable_device;
> +		goto err_out_clear_pci;
>  	}
>  
>  	pcie_print_link_status(pdev);
>  
>  	err = ionic_map_bars(ionic);
>  	if (err)
> -		goto err_out_pci_release_regions;
> +		goto err_out_clear_pci;
>  
>  	/* Configure the device */
>  	err = ionic_setup(ionic);
>  	if (err) {
>  		dev_err(dev, "Cannot setup device: %d, aborting\n", err);
> -		goto err_out_unmap_bars;
> +		goto err_out_clear_pci;
>  	}
>  	pci_set_master(pdev);
>  
> @@ -353,12 +360,8 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	ionic_reset(ionic);
>  err_out_teardown:
>  	ionic_dev_teardown(ionic);
> -err_out_unmap_bars:
> -	ionic_unmap_bars(ionic);
> -err_out_pci_release_regions:
> -	pci_release_regions(pdev);
> -err_out_pci_disable_device:
> -	pci_disable_device(pdev);
> +err_out_clear_pci:
> +	ionic_clear_pci(ionic);
>  err_out_debugfs_del_dev:
>  	ionic_debugfs_del_dev(ionic);
>  err_out_clear_drvdata:

...

Re: [PATCH v3 net-next 1/4] ionic: extract common bits from ionic_remove

[Shannon Nelson]

On 7/19/23 10:34 AM, Simon Horman wrote:
> 
> On Mon, Jul 17, 2023 at 09:59:58AM -0700, Shannon Nelson wrote:
>> Pull out a chunk of code from ionic_remove() that will
>> be common in teardown paths.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>> ---
>>   .../ethernet/pensando/ionic/ionic_bus_pci.c   | 25 ++++++++++---------
>>   1 file changed, 13 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> index ab7d217b98b3..2bc3cab3967d 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> @@ -213,6 +213,13 @@ static int ionic_sriov_configure(struct pci_dev *pdev, int num_vfs)
>>        return ret;
>>   }
>>
>> +static void ionic_clear_pci(struct ionic *ionic)
>> +{
>> +     ionic_unmap_bars(ionic);
>> +     pci_release_regions(ionic->pdev);
>> +     pci_disable_device(ionic->pdev);
> 
> Hi Shannon,
> 
> is it safe to call pci_release_regions() even if a successful call to
> pci_request_regions() has not been made?

It complains a bit about freeing non-existent regions, but otherwise is 
safe.  Of course if we're on that path there are other more seriously 
broken things for this device.  I figured the cleaner code is worth an 
extra log complaint in a probably never seen path.

sln

> 
>> +}
>> +
>>   static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>   {
>>        struct device *dev = &pdev->dev;
>> @@ -249,20 +256,20 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>        err = pci_request_regions(pdev, IONIC_DRV_NAME);
>>        if (err) {
>>                dev_err(dev, "Cannot request PCI regions: %d, aborting\n", err);
>> -             goto err_out_pci_disable_device;
>> +             goto err_out_clear_pci;
>>        }
>>
>>        pcie_print_link_status(pdev);
>>
>>        err = ionic_map_bars(ionic);
>>        if (err)
>> -             goto err_out_pci_release_regions;
>> +             goto err_out_clear_pci;
>>
>>        /* Configure the device */
>>        err = ionic_setup(ionic);
>>        if (err) {
>>                dev_err(dev, "Cannot setup device: %d, aborting\n", err);
>> -             goto err_out_unmap_bars;
>> +             goto err_out_clear_pci;
>>        }
>>        pci_set_master(pdev);
>>
>> @@ -353,12 +360,8 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>        ionic_reset(ionic);
>>   err_out_teardown:
>>        ionic_dev_teardown(ionic);
>> -err_out_unmap_bars:
>> -     ionic_unmap_bars(ionic);
>> -err_out_pci_release_regions:
>> -     pci_release_regions(pdev);
>> -err_out_pci_disable_device:
>> -     pci_disable_device(pdev);
>> +err_out_clear_pci:
>> +     ionic_clear_pci(ionic);
>>   err_out_debugfs_del_dev:
>>        ionic_debugfs_del_dev(ionic);
>>   err_out_clear_drvdata:
> 
> ...

[PATCH v3 net-next 2/4] ionic: extract common bits from ionic_probe

[Shannon Nelson]

Pull out some chunks of code from ionic_probe() that will
be common in rebuild paths.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
 .../ethernet/pensando/ionic/ionic_bus_pci.c   | 85 +++++++++++--------
 1 file changed, 49 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 2bc3cab3967d..b141a29177df 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -220,30 +220,12 @@ static void ionic_clear_pci(struct ionic *ionic)
 	pci_disable_device(ionic->pdev);
 }
 
-static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+static int ionic_setup_one(struct ionic *ionic)
 {
-	struct device *dev = &pdev->dev;
-	struct ionic *ionic;
-	int num_vfs;
+	struct pci_dev *pdev = ionic->pdev;
+	struct device *dev = ionic->dev;
 	int err;
 
-	ionic = ionic_devlink_alloc(dev);
-	if (!ionic)
-		return -ENOMEM;
-
-	ionic->pdev = pdev;
-	ionic->dev = dev;
-	pci_set_drvdata(pdev, ionic);
-	mutex_init(&ionic->dev_cmd_lock);
-
-	/* Query system for DMA addressing limitation for the device. */
-	err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(IONIC_ADDR_LEN));
-	if (err) {
-		dev_err(dev, "Unable to obtain 64-bit DMA for consistent allocations, aborting.  err=%d\n",
-			err);
-		goto err_out_clear_drvdata;
-	}
-
 	ionic_debugfs_add_dev(ionic);
 
 	/* Setup PCI device */
@@ -258,7 +240,6 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		dev_err(dev, "Cannot request PCI regions: %d, aborting\n", err);
 		goto err_out_clear_pci;
 	}
-
 	pcie_print_link_status(pdev);
 
 	err = ionic_map_bars(ionic);
@@ -286,24 +267,64 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_out_teardown;
 	}
 
-	/* Configure the ports */
+	/* Configure the port */
 	err = ionic_port_identify(ionic);
 	if (err) {
 		dev_err(dev, "Cannot identify port: %d, aborting\n", err);
-		goto err_out_reset;
+		goto err_out_teardown;
 	}
 
 	err = ionic_port_init(ionic);
 	if (err) {
 		dev_err(dev, "Cannot init port: %d, aborting\n", err);
-		goto err_out_reset;
+		goto err_out_teardown;
 	}
 
+	return 0;
+
+err_out_teardown:
+	ionic_dev_teardown(ionic);
+err_out_clear_pci:
+	ionic_clear_pci(ionic);
+err_out_debugfs_del_dev:
+	ionic_debugfs_del_dev(ionic);
+
+	return err;
+}
+
+static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+	struct device *dev = &pdev->dev;
+	struct ionic *ionic;
+	int num_vfs;
+	int err;
+
+	ionic = ionic_devlink_alloc(dev);
+	if (!ionic)
+		return -ENOMEM;
+
+	ionic->pdev = pdev;
+	ionic->dev = dev;
+	pci_set_drvdata(pdev, ionic);
+	mutex_init(&ionic->dev_cmd_lock);
+
+	/* Query system for DMA addressing limitation for the device. */
+	err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(IONIC_ADDR_LEN));
+	if (err) {
+		dev_err(dev, "Unable to obtain 64-bit DMA for consistent allocations, aborting.  err=%d\n",
+			err);
+		goto err_out;
+	}
+
+	err = ionic_setup_one(ionic);
+	if (err)
+		goto err_out;
+
 	/* Allocate and init the LIF */
 	err = ionic_lif_size(ionic);
 	if (err) {
 		dev_err(dev, "Cannot size LIF: %d, aborting\n", err);
-		goto err_out_port_reset;
+		goto err_out_pci;
 	}
 
 	err = ionic_lif_alloc(ionic);
@@ -354,17 +375,9 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	ionic->lif = NULL;
 err_out_free_irqs:
 	ionic_bus_free_irq_vectors(ionic);
-err_out_port_reset:
-	ionic_port_reset(ionic);
-err_out_reset:
-	ionic_reset(ionic);
-err_out_teardown:
-	ionic_dev_teardown(ionic);
-err_out_clear_pci:
+err_out_pci:
 	ionic_clear_pci(ionic);
-err_out_debugfs_del_dev:
-	ionic_debugfs_del_dev(ionic);
-err_out_clear_drvdata:
+err_out:
 	mutex_destroy(&ionic->dev_cmd_lock);
 	ionic_devlink_free(ionic);
 
-- 
2.17.1

Re: [PATCH v3 net-next 2/4] ionic: extract common bits from ionic_probe

[Simon Horman]

On Mon, Jul 17, 2023 at 09:59:59AM -0700, Shannon Nelson wrote:
> Pull out some chunks of code from ionic_probe() that will
> be common in rebuild paths.
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>

...

> +static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ionic *ionic;
> +	int num_vfs;
> +	int err;
> +
> +	ionic = ionic_devlink_alloc(dev);
> +	if (!ionic)
> +		return -ENOMEM;
> +
> +	ionic->pdev = pdev;
> +	ionic->dev = dev;
> +	pci_set_drvdata(pdev, ionic);
> +	mutex_init(&ionic->dev_cmd_lock);
> +
> +	/* Query system for DMA addressing limitation for the device. */
> +	err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(IONIC_ADDR_LEN));
> +	if (err) {
> +		dev_err(dev, "Unable to obtain 64-bit DMA for consistent allocations, aborting.  err=%d\n",
> +			err);
> +		goto err_out;
> +	}
> +
> +	err = ionic_setup_one(ionic);
> +	if (err)
> +		goto err_out;
> +
>  	/* Allocate and init the LIF */
>  	err = ionic_lif_size(ionic);
>  	if (err) {
>  		dev_err(dev, "Cannot size LIF: %d, aborting\n", err);
> -		goto err_out_port_reset;
> +		goto err_out_pci;

Hi Shannon,

Prior to this patch, if this error occurred then the following cleanup
would occur.

	ionic_port_reset(ionic);
	ionic_reset(ionic);
	ionic_dev_teardown(ionic);
	ionic_clear_pci(ionic);
	ionic_debugfs_del_dev(ionic);
	mutex_destroy(&ionic->dev_cmd_lock);
	ionic_devlink_free(ionic);

With this patch I am assuming that the same setup has occurred at
this point (maybe I am mistaken). But with the following cleanup on error.

	ionic_clear_pci(ionic);
	mutex_destroy(&ionic->dev_cmd_lock);
	ionic_devlink_free(ionic);

I feel that I'm reading this wrong.

>  	}
>  
>  	err = ionic_lif_alloc(ionic);
> @@ -354,17 +375,9 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	ionic->lif = NULL;
>  err_out_free_irqs:
>  	ionic_bus_free_irq_vectors(ionic);
> -err_out_port_reset:
> -	ionic_port_reset(ionic);
> -err_out_reset:
> -	ionic_reset(ionic);
> -err_out_teardown:
> -	ionic_dev_teardown(ionic);
> -err_out_clear_pci:
> +err_out_pci:
>  	ionic_clear_pci(ionic);
> -err_out_debugfs_del_dev:
> -	ionic_debugfs_del_dev(ionic);
> -err_out_clear_drvdata:
> +err_out:
>  	mutex_destroy(&ionic->dev_cmd_lock);
>  	ionic_devlink_free(ionic);
>  
> -- 
> 2.17.1
> 
> 

Re: [PATCH v3 net-next 2/4] ionic: extract common bits from ionic_probe

[Shannon Nelson]

On 7/19/23 10:48 AM, Simon Horman wrote:
> 
> On Mon, Jul 17, 2023 at 09:59:59AM -0700, Shannon Nelson wrote:
>> Pull out some chunks of code from ionic_probe() that will
>> be common in rebuild paths.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> 
> ...
> 
>> +static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>> +{
>> +     struct device *dev = &pdev->dev;
>> +     struct ionic *ionic;
>> +     int num_vfs;
>> +     int err;
>> +
>> +     ionic = ionic_devlink_alloc(dev);
>> +     if (!ionic)
>> +             return -ENOMEM;
>> +
>> +     ionic->pdev = pdev;
>> +     ionic->dev = dev;
>> +     pci_set_drvdata(pdev, ionic);
>> +     mutex_init(&ionic->dev_cmd_lock);
>> +
>> +     /* Query system for DMA addressing limitation for the device. */
>> +     err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(IONIC_ADDR_LEN));
>> +     if (err) {
>> +             dev_err(dev, "Unable to obtain 64-bit DMA for consistent allocations, aborting.  err=%d\n",
>> +                     err);
>> +             goto err_out;
>> +     }
>> +
>> +     err = ionic_setup_one(ionic);
>> +     if (err)
>> +             goto err_out;
>> +
>>        /* Allocate and init the LIF */
>>        err = ionic_lif_size(ionic);
>>        if (err) {
>>                dev_err(dev, "Cannot size LIF: %d, aborting\n", err);
>> -             goto err_out_port_reset;
>> +             goto err_out_pci;
> 
> Hi Shannon,
> 
> Prior to this patch, if this error occurred then the following cleanup
> would occur.
> 
>          ionic_port_reset(ionic);
>          ionic_reset(ionic);
>          ionic_dev_teardown(ionic);
>          ionic_clear_pci(ionic);
>          ionic_debugfs_del_dev(ionic);
>          mutex_destroy(&ionic->dev_cmd_lock);
>          ionic_devlink_free(ionic);
> 
> With this patch I am assuming that the same setup has occurred at
> this point (maybe I am mistaken). But with the following cleanup on error.
> 
>          ionic_clear_pci(ionic);
>          mutex_destroy(&ionic->dev_cmd_lock);
>          ionic_devlink_free(ionic);
> 
> I feel that I'm reading this wrong.

You read that right.  The port_reset and reset are superflous so are 
dropped here.  However, that dev_teardown() does need to happen to be 
sure we don't leak a couple CMB related things.  I'll add that.

Thanks,
sln

> 
>>        }
>>
>>        err = ionic_lif_alloc(ionic);
>> @@ -354,17 +375,9 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>        ionic->lif = NULL;
>>   err_out_free_irqs:
>>        ionic_bus_free_irq_vectors(ionic);
>> -err_out_port_reset:
>> -     ionic_port_reset(ionic);
>> -err_out_reset:
>> -     ionic_reset(ionic);
>> -err_out_teardown:
>> -     ionic_dev_teardown(ionic);
>> -err_out_clear_pci:
>> +err_out_pci:
>>        ionic_clear_pci(ionic);
>> -err_out_debugfs_del_dev:
>> -     ionic_debugfs_del_dev(ionic);
>> -err_out_clear_drvdata:
>> +err_out:
>>        mutex_destroy(&ionic->dev_cmd_lock);
>>        ionic_devlink_free(ionic);
>>
>> --
>> 2.17.1
>>
>>

[PATCH v3 net-next 3/4] ionic: pull out common bits from fw_up

[Shannon Nelson]

Pull out some code from ionic_lif_handle_fw_up() that can be
used in the coming FLR recovery patch.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 66 ++++++++++++-------
 1 file changed, 43 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 612b0015dc43..94b14ea6ffee 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -3266,27 +3266,11 @@ static void ionic_lif_handle_fw_down(struct ionic_lif *lif)
 	dev_info(ionic->dev, "FW Down: LIFs stopped\n");
 }
 
-static void ionic_lif_handle_fw_up(struct ionic_lif *lif)
+static int ionic_restart_lif(struct ionic_lif *lif)
 {
 	struct ionic *ionic = lif->ionic;
 	int err;
 
-	if (!test_bit(IONIC_LIF_F_FW_RESET, lif->state))
-		return;
-
-	dev_info(ionic->dev, "FW Up: restarting LIFs\n");
-
-	ionic_init_devinfo(ionic);
-	err = ionic_identify(ionic);
-	if (err)
-		goto err_out;
-	err = ionic_port_identify(ionic);
-	if (err)
-		goto err_out;
-	err = ionic_port_init(ionic);
-	if (err)
-		goto err_out;
-
 	mutex_lock(&lif->queue_lock);
 
 	if (test_and_clear_bit(IONIC_LIF_F_BROKEN, lif->state))
@@ -3317,17 +3301,13 @@ static void ionic_lif_handle_fw_up(struct ionic_lif *lif)
 			goto err_txrx_free;
 	}
 
+	clear_bit(IONIC_LIF_F_FW_RESET, lif->state);
 	mutex_unlock(&lif->queue_lock);
 
-	clear_bit(IONIC_LIF_F_FW_RESET, lif->state);
 	ionic_link_status_check_request(lif, CAN_SLEEP);
 	netif_device_attach(lif->netdev);
-	dev_info(ionic->dev, "FW Up: LIFs restarted\n");
 
-	/* restore the hardware timestamping queues */
-	ionic_lif_hwstamp_replay(lif);
-
-	return;
+	return 0;
 
 err_txrx_free:
 	ionic_txrx_free(lif);
@@ -3337,6 +3317,46 @@ static void ionic_lif_handle_fw_up(struct ionic_lif *lif)
 	ionic_qcqs_free(lif);
 err_unlock:
 	mutex_unlock(&lif->queue_lock);
+
+	return err;
+}
+
+static void ionic_lif_handle_fw_up(struct ionic_lif *lif)
+{
+	struct ionic *ionic = lif->ionic;
+	int err;
+
+	if (!test_bit(IONIC_LIF_F_FW_RESET, lif->state))
+		return;
+
+	dev_info(ionic->dev, "FW Up: restarting LIFs\n");
+
+	/* This is a little different from what happens at
+	 * probe time because the LIF already exists so we
+	 * just need to reanimate it.
+	 */
+	ionic_init_devinfo(ionic);
+	err = ionic_identify(ionic);
+	if (err)
+		goto err_out;
+	err = ionic_port_identify(ionic);
+	if (err)
+		goto err_out;
+	err = ionic_port_init(ionic);
+	if (err)
+		goto err_out;
+
+	err = ionic_restart_lif(lif);
+	if (err)
+		goto err_out;
+
+	dev_info(ionic->dev, "FW Up: LIFs restarted\n");
+
+	/* restore the hardware timestamping queues */
+	ionic_lif_hwstamp_replay(lif);
+
+	return;
+
 err_out:
 	dev_err(ionic->dev, "FW Up: LIFs restart failed - err %d\n", err);
 }
-- 
2.17.1

Re: [PATCH v3 net-next 3/4] ionic: pull out common bits from fw_up

[Simon Horman]

On Mon, Jul 17, 2023 at 10:00:00AM -0700, Shannon Nelson wrote:
> Pull out some code from ionic_lif_handle_fw_up() that can be
> used in the coming FLR recovery patch.
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>

...

> @@ -3317,17 +3301,13 @@ static void ionic_lif_handle_fw_up(struct ionic_lif *lif)
>  			goto err_txrx_free;
>  	}
>  
> +	clear_bit(IONIC_LIF_F_FW_RESET, lif->state);
>  	mutex_unlock(&lif->queue_lock);
>  
> -	clear_bit(IONIC_LIF_F_FW_RESET, lif->state);

Hi Shannon,

Moving clear_bit() inside the critical section seems
unrelated to the patch description.

>  	ionic_link_status_check_request(lif, CAN_SLEEP);
>  	netif_device_attach(lif->netdev);
> -	dev_info(ionic->dev, "FW Up: LIFs restarted\n");
>  
> -	/* restore the hardware timestamping queues */
> -	ionic_lif_hwstamp_replay(lif);
> -
> -	return;
> +	return 0;
>  
>  err_txrx_free:
>  	ionic_txrx_free(lif);

...

Re: [PATCH v3 net-next 3/4] ionic: pull out common bits from fw_up

[Shannon Nelson]

On 7/19/23 10:56 AM, Simon Horman wrote:
> 
> On Mon, Jul 17, 2023 at 10:00:00AM -0700, Shannon Nelson wrote:
>> Pull out some code from ionic_lif_handle_fw_up() that can be
>> used in the coming FLR recovery patch.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> 
> ...
> 
>> @@ -3317,17 +3301,13 @@ static void ionic_lif_handle_fw_up(struct ionic_lif *lif)
>>                        goto err_txrx_free;
>>        }
>>
>> +     clear_bit(IONIC_LIF_F_FW_RESET, lif->state);
>>        mutex_unlock(&lif->queue_lock);
>>
>> -     clear_bit(IONIC_LIF_F_FW_RESET, lif->state);
> 
> Hi Shannon,
> 
> Moving clear_bit() inside the critical section seems
> unrelated to the patch description.

I can make that a separate patch in the future, I'll pull it out for a 
next rev.

Thanks,
sln


> 
>>        ionic_link_status_check_request(lif, CAN_SLEEP);
>>        netif_device_attach(lif->netdev);
>> -     dev_info(ionic->dev, "FW Up: LIFs restarted\n");
>>
>> -     /* restore the hardware timestamping queues */
>> -     ionic_lif_hwstamp_replay(lif);
>> -
>> -     return;
>> +     return 0;
>>
>>   err_txrx_free:
>>        ionic_txrx_free(lif);
> 
> ...

[PATCH v3 net-next 4/4] ionic: add FLR recovery support

[Shannon Nelson]

Add support for the PCI reset handlers in order to manage an FLR event.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
 .../ethernet/pensando/ionic/ionic_bus_pci.c   | 53 +++++++++++++++++++
 .../net/ethernet/pensando/ionic/ionic_lif.c   |  8 +--
 .../net/ethernet/pensando/ionic/ionic_lif.h   |  5 ++
 3 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index b141a29177df..3fb7925c2d47 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -408,12 +408,65 @@ static void ionic_remove(struct pci_dev *pdev)
 	ionic_devlink_free(ionic);
 }
 
+static void ionic_reset_prepare(struct pci_dev *pdev)
+{
+	struct ionic *ionic = pci_get_drvdata(pdev);
+	struct ionic_lif *lif = ionic->lif;
+
+	dev_dbg(ionic->dev, "%s: device stopping\n", __func__);
+
+	del_timer_sync(&ionic->watchdog_timer);
+	cancel_work_sync(&lif->deferred.work);
+
+	mutex_lock(&lif->queue_lock);
+	ionic_stop_queues_reconfig(lif);
+	ionic_txrx_free(lif);
+	ionic_lif_deinit(lif);
+	ionic_qcqs_free(lif);
+	mutex_unlock(&lif->queue_lock);
+
+	ionic_dev_teardown(ionic);
+	ionic_clear_pci(ionic);
+	ionic_debugfs_del_dev(ionic);
+}
+
+static void ionic_reset_done(struct pci_dev *pdev)
+{
+	struct ionic *ionic = pci_get_drvdata(pdev);
+	struct ionic_lif *lif = ionic->lif;
+	int err;
+
+	err = ionic_setup_one(ionic);
+	if (err)
+		goto err_out;
+
+	ionic_debugfs_add_sizes(ionic);
+	ionic_debugfs_add_lif(ionic->lif);
+
+	err = ionic_restart_lif(lif);
+	if (err)
+		goto err_out;
+
+	mod_timer(&ionic->watchdog_timer, jiffies + 1);
+
+err_out:
+	dev_dbg(ionic->dev, "%s: device recovery %s\n",
+		__func__, err ? "failed" : "done");
+}
+
+static const struct pci_error_handlers ionic_err_handler = {
+	/* FLR handling */
+	.reset_prepare      = ionic_reset_prepare,
+	.reset_done         = ionic_reset_done,
+};
+
 static struct pci_driver ionic_driver = {
 	.name = IONIC_DRV_NAME,
 	.id_table = ionic_id_table,
 	.probe = ionic_probe,
 	.remove = ionic_remove,
 	.sriov_configure = ionic_sriov_configure,
+	.err_handler = &ionic_err_handler
 };
 
 int ionic_bus_register_driver(void)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 94b14ea6ffee..9a10458f4a31 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -434,7 +434,7 @@ static void ionic_qcq_free(struct ionic_lif *lif, struct ionic_qcq *qcq)
 	}
 }
 
-static void ionic_qcqs_free(struct ionic_lif *lif)
+void ionic_qcqs_free(struct ionic_lif *lif)
 {
 	struct device *dev = lif->ionic->dev;
 	struct ionic_qcq *adminqcq;
@@ -1754,7 +1754,7 @@ static int ionic_set_mac_address(struct net_device *netdev, void *sa)
 	return ionic_lif_addr_add(netdev_priv(netdev), mac);
 }
 
-static void ionic_stop_queues_reconfig(struct ionic_lif *lif)
+void ionic_stop_queues_reconfig(struct ionic_lif *lif)
 {
 	/* Stop and clean the queues before reconfiguration */
 	netif_device_detach(lif->netdev);
@@ -2009,7 +2009,7 @@ static void ionic_txrx_deinit(struct ionic_lif *lif)
 	}
 }
 
-static void ionic_txrx_free(struct ionic_lif *lif)
+void ionic_txrx_free(struct ionic_lif *lif)
 {
 	unsigned int i;
 
@@ -3266,7 +3266,7 @@ static void ionic_lif_handle_fw_down(struct ionic_lif *lif)
 	dev_info(ionic->dev, "FW Down: LIFs stopped\n");
 }
 
-static int ionic_restart_lif(struct ionic_lif *lif)
+int ionic_restart_lif(struct ionic_lif *lif)
 {
 	struct ionic *ionic = lif->ionic;
 	int err;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index fd2ea670e7d8..457c24195ca6 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -325,6 +325,11 @@ void ionic_lif_deinit(struct ionic_lif *lif);
 int ionic_lif_addr_add(struct ionic_lif *lif, const u8 *addr);
 int ionic_lif_addr_del(struct ionic_lif *lif, const u8 *addr);
 
+void ionic_stop_queues_reconfig(struct ionic_lif *lif);
+void ionic_txrx_free(struct ionic_lif *lif);
+void ionic_qcqs_free(struct ionic_lif *lif);
+int ionic_restart_lif(struct ionic_lif *lif);
+
 int ionic_lif_register(struct ionic_lif *lif);
 void ionic_lif_unregister(struct ionic_lif *lif);
 int ionic_lif_identify(struct ionic *ionic, u8 lif_type,
-- 
2.17.1