[PATCH 4/9] net/gve: separate device resource alloc and config

[Jasper Tran O'Leary]

Separate gve_setup_device_resources into two distinct functions:
- gve_alloc_device_resources: allocates host DMA memory structures
  (counter array, IRQ doorbells, ptype LUT).
- gve_configure_device_resources: registers and configures those
  allocated resources with the device hardware.

Symmetrically, partition gve_teardown_device_resources into a function
for freeing memory and one for deconfiguring resources.

This clean separation allows device resources to be re-configured
with the hardware during device reset recovery without re-allocating
host memory.

Signed-off-by: Jasper Tran O'Leary <jtranoleary at google.com>
Reviewed-by: Joshua Washington <joshwash at google.com>
---
 drivers/net/gve/gve_ethdev.c | 172 +++++++++++++++++-------------
 1 file changed, 97 insertions(+), 75 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 680fc9947..44810b864 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -742,15 +742,20 @@ gve_free_queues(struct rte_eth_dev *dev)
 static void
 gve_free_counter_array(struct gve_priv *priv)
 {
-	rte_memzone_free(priv->cnt_array_mz);
-	priv->cnt_array = NULL;
+	if (priv->cnt_array_mz) {
+		rte_memzone_free(priv->cnt_array_mz);
+		priv->cnt_array_mz = NULL;
+		priv->cnt_array = NULL;
+	}
 }
-
 static void
 gve_free_irq_db(struct gve_priv *priv)
 {
-	rte_memzone_free(priv->irq_dbs_mz);
-	priv->irq_dbs = NULL;
+	if (priv->irq_dbs_mz) {
+		rte_memzone_free(priv->irq_dbs_mz);
+		priv->irq_dbs_mz = NULL;
+		priv->irq_dbs = NULL;
+	}
 }
 
 static void
@@ -805,33 +810,38 @@ gve_teardown_flow_subsystem(struct gve_priv *priv)
 }
 
 static void
-gve_teardown_device_resources(struct gve_priv *priv)
+gve_free_device_resources(struct gve_priv *priv)
+{
+	gve_free_nic_ts_report(priv);
+	gve_free_ptype_lut_dqo(priv);
+	gve_free_counter_array(priv);
+	gve_free_irq_db(priv);
+	gve_free_rss_config(&priv->rss_config);
+}
+
+static void
+gve_deconfigure_device_resources(struct gve_priv *priv)
 {
 	struct gve_process_private *process_priv;
 	int err;
 
-	process_priv = rte_eth_devices[priv->port_id].process_private;
-
-	/* Tell device its resources are being freed */
-	if (gve_get_device_resources_ok(priv)) {
-		err = process_priv->ctrl_ops->free_db_resources(priv);
-		if (err)
-			PMD_DRV_LOG(ERR,
-				"Could not deconfigure device resources: err=%d",
-				err);
-	}
+	if (!gve_get_device_resources_ok(priv))
+		return;
 
 	if (priv->nic_ts_report_mz) {
 		rte_atomic_store_explicit(&priv->nic_ts_thread_should_stop, 1,
 					  rte_memory_order_relaxed);
 		rte_thread_join(priv->nic_ts_thread_id, NULL);
-		gve_free_nic_ts_report(priv);
 	}
 
-	gve_free_ptype_lut_dqo(priv);
-	gve_free_counter_array(priv);
-	gve_free_irq_db(priv);
-	gve_free_rss_config(&priv->rss_config);
+	process_priv = rte_eth_devices[priv->port_id].process_private;
+	if (process_priv->ctrl_ops->free_db_resources) {
+		err = process_priv->ctrl_ops->free_db_resources(priv);
+		if (err)
+			PMD_DRV_LOG(ERR,
+				    "Could not deconfigure device resources: err=%d",
+				    err);
+	}
 	gve_clear_device_resources_ok(priv);
 }
 
@@ -864,8 +874,9 @@ gve_dev_close(struct rte_eth_dev *dev)
 	priv->zombie_rx_queues = NULL;
 	priv->zombie_tx_queues = NULL;
 
-	gve_teardown_device_resources(priv);
+	gve_deconfigure_device_resources(priv);
 	process_priv->ctrl_ops->free_ctrl_plane(priv);
+	gve_free_device_resources(priv);
 
 	pthread_mutex_destroy(&priv->flow_rule_lock);
 	pthread_mutex_destroy(&priv->nic_ts_lock);
@@ -907,8 +918,9 @@ gve_dev_reset(struct rte_eth_dev *dev)
 	 * destroyed on dev_close.
 	 */
 	gve_free_queues(dev);
-	gve_teardown_device_resources(priv);
+	gve_deconfigure_device_resources(priv);
 	process_priv->ctrl_ops->free_ctrl_plane(priv);
+	gve_free_device_resources(priv);
 
 	err = gve_init_priv(priv, true);
 	if (err != 0) {
@@ -1574,36 +1586,27 @@ gve_setup_nic_timestamp(struct gve_priv *priv)
 {
 	int err;
 
-	if (priv->clk_read_type == GVE_DEV_CLK_UNSUPPORTED)
+	if (!priv->nic_ts_report_mz)
 		return;
 
 	rte_atomic_store_explicit(&priv->nic_ts_read_fails, 0, rte_memory_order_relaxed);
 	rte_atomic_store_explicit(&priv->nic_ts_stale, 1, rte_memory_order_relaxed);
-	err = gve_alloc_nic_ts_report(priv);
-	if (err == 0) {
-		gve_read_nic_clock(priv);
-		rte_atomic_store_explicit(&priv->nic_ts_thread_should_stop, 0,
-					  rte_memory_order_relaxed);
-		err = rte_thread_create_internal_control(&priv->nic_ts_thread_id, "gve-ts",
-							 gve_nic_ts_thread, priv);
-		if (err != 0) {
-			PMD_DRV_LOG(ERR, "Failed to create NIC clock sync thread, err=%d", err);
-			gve_free_nic_ts_report(priv);
-		}
-	} else {
-		PMD_DRV_LOG(ERR, "Failed to allocate memory for NIC timestamping subsystem. Please reset device to retry.");
+	gve_read_nic_clock(priv);
+	rte_atomic_store_explicit(&priv->nic_ts_thread_should_stop, 0,
+				  rte_memory_order_relaxed);
+	err = rte_thread_create_internal_control(&priv->nic_ts_thread_id, "gve-ts",
+						 gve_nic_ts_thread, priv);
+	if (err != 0) {
+		PMD_DRV_LOG(ERR, "Failed to create NIC clock sync thread, err=%d", err);
+		gve_free_nic_ts_report(priv);
 	}
 }
 
 static int
-gve_setup_device_resources(struct gve_priv *priv)
+gve_alloc_device_resources(struct gve_priv *priv)
 {
-	struct gve_process_private *process_priv;
 	char z_name[RTE_MEMZONE_NAMESIZE];
 	const struct rte_memzone *mz;
-	int err = 0;
-
-	process_priv = rte_eth_devices[priv->port_id].process_private;
 
 	snprintf(z_name, sizeof(z_name), "gve_%s_cnt_arr", priv->pci_dev->device.name);
 	mz = rte_memzone_reserve_aligned(z_name,
@@ -1619,34 +1622,56 @@ gve_setup_device_resources(struct gve_priv *priv)
 
 	snprintf(z_name, sizeof(z_name), "gve_%s_irqmz", priv->pci_dev->device.name);
 	mz = rte_memzone_reserve_aligned(z_name,
-					 sizeof(*priv->irq_dbs) * (priv->num_ntfy_blks),
+					 sizeof(*priv->irq_dbs) * priv->num_ntfy_blks,
 					 rte_socket_id(), RTE_MEMZONE_IOVA_CONTIG,
 					 PAGE_SIZE);
 	if (mz == NULL) {
 		PMD_DRV_LOG(ERR, "Could not alloc memzone for irq_dbs");
-		err = -ENOMEM;
-		goto free_cnt_array;
+		goto err_free;
 	}
 	priv->irq_dbs = (struct gve_irq_db *)mz->addr;
 	priv->irq_dbs_mz = mz;
 
-	err = process_priv->ctrl_ops->get_interrupt_dbs(priv);
-	if (unlikely(err)) {
-		PMD_DRV_LOG(ERR, "Could not config device resources: err=%d", err);
-		goto free_irq_dbs;
-	}
 	if (!gve_is_gqi(priv)) {
 		priv->ptype_lut_dqo = rte_zmalloc("gve_ptype_lut_dqo",
 			sizeof(struct gve_ptype_lut), 0);
 		if (priv->ptype_lut_dqo == NULL) {
 			PMD_DRV_LOG(ERR, "Failed to alloc ptype lut.");
-			err = -ENOMEM;
-			goto free_irq_dbs;
+			goto err_free;
 		}
+	}
+
+	if (priv->clk_read_type != GVE_DEV_CLK_UNSUPPORTED) {
+		if (gve_alloc_nic_ts_report(priv) != 0)
+			PMD_DRV_LOG(ERR, "Failed to allocate memory for NIC timestamping subsystem.");
+	}
+
+	return 0;
+
+err_free:
+	gve_free_device_resources(priv);
+	return -ENOMEM;
+}
+
+static int
+gve_configure_device_resources(struct gve_priv *priv)
+{
+	struct gve_process_private *process_priv;
+	int err;
+
+	process_priv = rte_eth_devices[priv->port_id].process_private;
+
+	err = process_priv->ctrl_ops->get_interrupt_dbs(priv);
+	if (unlikely(err)) {
+		PMD_DRV_LOG(ERR, "Could not config device resources: err=%d", err);
+		return err;
+	}
+
+	if (!gve_is_gqi(priv)) {
 		err = process_priv->ctrl_ops->get_ptype_map(priv);
 		if (unlikely(err)) {
 			PMD_DRV_LOG(ERR, "Failed to get ptype map: err=%d", err);
-			goto free_ptype_lut;
+			return err;
 		}
 	}
 	gve_setup_nic_timestamp(priv);
@@ -1654,15 +1679,6 @@ gve_setup_device_resources(struct gve_priv *priv)
 	gve_set_device_resources_ok(priv);
 
 	return 0;
-free_ptype_lut:
-	rte_free(priv->ptype_lut_dqo);
-	priv->ptype_lut_dqo = NULL;
-free_irq_dbs:
-	gve_free_irq_db(priv);
-free_cnt_array:
-	gve_free_counter_array(priv);
-
-	return err;
 }
 
 static void
@@ -1797,19 +1813,19 @@ gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
 	err = process_priv->ctrl_ops->get_device_properties(priv);
 	if (err) {
 		PMD_DRV_LOG(ERR, "Could not get device information: err=%d", err);
-		goto free_adminq;
+		goto free_ctrl_plane;
 	}
 
 	num_ntfy = pci_dev_msix_vec_count(priv->pci_dev);
 	if (num_ntfy <= 0) {
 		PMD_DRV_LOG(ERR, "Could not count MSI-x vectors");
 		err = -EIO;
-		goto free_adminq;
+		goto free_ctrl_plane;
 	} else if (num_ntfy < GVE_MIN_MSIX) {
 		PMD_DRV_LOG(ERR, "GVE needs at least %d MSI-x vectors, but only has %d",
 			    GVE_MIN_MSIX, num_ntfy);
 		err = -EINVAL;
-		goto free_adminq;
+		goto free_ctrl_plane;
 	}
 
 	priv->num_registered_pages = 0;
@@ -1867,8 +1883,11 @@ setup_device:
 				    "Failed to set up flow subsystem: err=%d, flow steering will be disabled.",
 				    err);
 	}
+	err = gve_alloc_device_resources(priv);
+	if (err)
+		goto free_flow_subsystem;
 
-	err = gve_setup_device_resources(priv);
+	err = gve_configure_device_resources(priv);
 	if (err)
 		goto free_flow_subsystem;
 
@@ -1877,16 +1896,19 @@ free_flow_subsystem:
 	if (gve_get_flow_subsystem_ok(priv))
 		gve_teardown_flow_subsystem(priv);
 free_queue_configs:
-	rte_free(priv->zombie_tx_queues);
-	priv->zombie_tx_queues = NULL;
-	rte_free(priv->zombie_rx_queues);
-	priv->zombie_rx_queues = NULL;
-	rte_free(priv->txq_configs);
-	priv->txq_configs = NULL;
-	rte_free(priv->rxq_configs);
-	priv->rxq_configs = NULL;
-free_adminq:
+	if (!skip_describe_device) {
+		rte_free(priv->zombie_tx_queues);
+		priv->zombie_tx_queues = NULL;
+		rte_free(priv->zombie_rx_queues);
+		priv->zombie_rx_queues = NULL;
+		rte_free(priv->txq_configs);
+		priv->txq_configs = NULL;
+		rte_free(priv->rxq_configs);
+		priv->rxq_configs = NULL;
+	}
+free_ctrl_plane:
 	process_priv->ctrl_ops->free_ctrl_plane(priv);
+	gve_free_device_resources(priv);
 	return err;
 }
 
-- 
2.56.0.307.g5fe7bd680c-goog