* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).