* [PATCH v3 net-next 0/4] ionic: add FLR support
@ 2023-07-17 16:59 Shannon Nelson
2023-07-17 16:59 ` [PATCH v3 net-next 1/4] ionic: extract common bits from ionic_remove Shannon Nelson
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Shannon Nelson @ 2023-07-17 16:59 UTC (permalink / raw)
To: netdev, davem, kuba, idosch; +Cc: brett.creeley, drivers, 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 net-next 1/4] ionic: extract common bits from ionic_remove
2023-07-17 16:59 [PATCH v3 net-next 0/4] ionic: add FLR support Shannon Nelson
@ 2023-07-17 16:59 ` Shannon Nelson
2023-07-19 17:34 ` Simon Horman
2023-07-17 16:59 ` [PATCH v3 net-next 2/4] ionic: extract common bits from ionic_probe Shannon Nelson
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Shannon Nelson @ 2023-07-17 16:59 UTC (permalink / raw)
To: netdev, davem, kuba, idosch; +Cc: brett.creeley, drivers, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 net-next 2/4] ionic: extract common bits from ionic_probe
2023-07-17 16:59 [PATCH v3 net-next 0/4] ionic: add FLR support Shannon Nelson
2023-07-17 16:59 ` [PATCH v3 net-next 1/4] ionic: extract common bits from ionic_remove Shannon Nelson
@ 2023-07-17 16:59 ` Shannon Nelson
2023-07-19 17:48 ` Simon Horman
2023-07-17 17:00 ` [PATCH v3 net-next 3/4] ionic: pull out common bits from fw_up Shannon Nelson
2023-07-17 17:00 ` [PATCH v3 net-next 4/4] ionic: add FLR recovery support Shannon Nelson
3 siblings, 1 reply; 11+ messages in thread
From: Shannon Nelson @ 2023-07-17 16:59 UTC (permalink / raw)
To: netdev, davem, kuba, idosch; +Cc: brett.creeley, drivers, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 net-next 3/4] ionic: pull out common bits from fw_up
2023-07-17 16:59 [PATCH v3 net-next 0/4] ionic: add FLR support Shannon Nelson
2023-07-17 16:59 ` [PATCH v3 net-next 1/4] ionic: extract common bits from ionic_remove Shannon Nelson
2023-07-17 16:59 ` [PATCH v3 net-next 2/4] ionic: extract common bits from ionic_probe Shannon Nelson
@ 2023-07-17 17:00 ` Shannon Nelson
2023-07-19 17:56 ` Simon Horman
2023-07-17 17:00 ` [PATCH v3 net-next 4/4] ionic: add FLR recovery support Shannon Nelson
3 siblings, 1 reply; 11+ messages in thread
From: Shannon Nelson @ 2023-07-17 17:00 UTC (permalink / raw)
To: netdev, davem, kuba, idosch; +Cc: brett.creeley, drivers, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 net-next 4/4] ionic: add FLR recovery support
2023-07-17 16:59 [PATCH v3 net-next 0/4] ionic: add FLR support Shannon Nelson
` (2 preceding siblings ...)
2023-07-17 17:00 ` [PATCH v3 net-next 3/4] ionic: pull out common bits from fw_up Shannon Nelson
@ 2023-07-17 17:00 ` Shannon Nelson
3 siblings, 0 replies; 11+ messages in thread
From: Shannon Nelson @ 2023-07-17 17:00 UTC (permalink / raw)
To: netdev, davem, kuba, idosch; +Cc: brett.creeley, drivers, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 net-next 1/4] ionic: extract common bits from ionic_remove
2023-07-17 16:59 ` [PATCH v3 net-next 1/4] ionic: extract common bits from ionic_remove Shannon Nelson
@ 2023-07-19 17:34 ` Simon Horman
2023-07-20 0:04 ` Shannon Nelson
0 siblings, 1 reply; 11+ messages in thread
From: Simon Horman @ 2023-07-19 17:34 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem, kuba, idosch, brett.creeley, drivers
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:
...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 net-next 2/4] ionic: extract common bits from ionic_probe
2023-07-17 16:59 ` [PATCH v3 net-next 2/4] ionic: extract common bits from ionic_probe Shannon Nelson
@ 2023-07-19 17:48 ` Simon Horman
2023-07-20 0:04 ` Shannon Nelson
0 siblings, 1 reply; 11+ messages in thread
From: Simon Horman @ 2023-07-19 17:48 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem, kuba, idosch, brett.creeley, drivers
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
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 net-next 3/4] ionic: pull out common bits from fw_up
2023-07-17 17:00 ` [PATCH v3 net-next 3/4] ionic: pull out common bits from fw_up Shannon Nelson
@ 2023-07-19 17:56 ` Simon Horman
2023-07-20 0:04 ` Shannon Nelson
0 siblings, 1 reply; 11+ messages in thread
From: Simon Horman @ 2023-07-19 17:56 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem, kuba, idosch, brett.creeley, drivers
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);
...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 net-next 1/4] ionic: extract common bits from ionic_remove
2023-07-19 17:34 ` Simon Horman
@ 2023-07-20 0:04 ` Shannon Nelson
0 siblings, 0 replies; 11+ messages in thread
From: Shannon Nelson @ 2023-07-20 0:04 UTC (permalink / raw)
To: Simon Horman; +Cc: netdev, davem, kuba, idosch, brett.creeley, drivers
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:
>
> ...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 net-next 2/4] ionic: extract common bits from ionic_probe
2023-07-19 17:48 ` Simon Horman
@ 2023-07-20 0:04 ` Shannon Nelson
0 siblings, 0 replies; 11+ messages in thread
From: Shannon Nelson @ 2023-07-20 0:04 UTC (permalink / raw)
To: Simon Horman; +Cc: netdev, davem, kuba, idosch, brett.creeley, drivers
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
>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 net-next 3/4] ionic: pull out common bits from fw_up
2023-07-19 17:56 ` Simon Horman
@ 2023-07-20 0:04 ` Shannon Nelson
0 siblings, 0 replies; 11+ messages in thread
From: Shannon Nelson @ 2023-07-20 0:04 UTC (permalink / raw)
To: Simon Horman; +Cc: netdev, davem, kuba, idosch, brett.creeley, drivers
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);
>
> ...
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-07-20 0:04 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-17 16:59 [PATCH v3 net-next 0/4] ionic: add FLR support Shannon Nelson
2023-07-17 16:59 ` [PATCH v3 net-next 1/4] ionic: extract common bits from ionic_remove Shannon Nelson
2023-07-19 17:34 ` Simon Horman
2023-07-20 0:04 ` Shannon Nelson
2023-07-17 16:59 ` [PATCH v3 net-next 2/4] ionic: extract common bits from ionic_probe Shannon Nelson
2023-07-19 17:48 ` Simon Horman
2023-07-20 0:04 ` Shannon Nelson
2023-07-17 17:00 ` [PATCH v3 net-next 3/4] ionic: pull out common bits from fw_up Shannon Nelson
2023-07-19 17:56 ` Simon Horman
2023-07-20 0:04 ` Shannon Nelson
2023-07-17 17:00 ` [PATCH v3 net-next 4/4] ionic: add FLR recovery support Shannon Nelson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.