Re: [PATCH net-next v2 13/15] gve: setup and teardown management interrupts

[Joshua Washington]

On Tue, Jun 2, 2026 at 4:59 PM Harshitha Ramamurthy
<hramamurthy@xxxxxxxxxx> wrote:
>
> From: Joshua Washington <joshwash@xxxxxxxxxx>
>
> Introduce control ops to setup/teardown control plane IRQs. Admin
> queue-specific functionality is moved to admin queue files.
>
> Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx>
> Reviewed-by: Jordan Rhee <jordanrhee@xxxxxxxxxx>
> Signed-off-by: Joshua Washington <joshwash@xxxxxxxxxx>
> Signed-off-by: Harshitha Ramamurthy <hramamurthy@xxxxxxxxxx>
> ---
>  drivers/net/ethernet/google/gve/gve.h        |  4 +++
>  drivers/net/ethernet/google/gve/gve_adminq.c | 32 ++++++++++++++++++++
>  drivers/net/ethernet/google/gve/gve_adminq.h |  2 ++
>  drivers/net/ethernet/google/gve/gve_main.c   | 28 ++++++-----------
>  4 files changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
> index 6db5fbc0b321..b2e578b3d450 100644
> --- a/drivers/net/ethernet/google/gve/gve.h
> +++ b/drivers/net/ethernet/google/gve/gve.h
> @@ -835,6 +835,8 @@ struct gve_device_info {
>   *                     initialization.
>   * @request_db_info: Request and store doorbell information into @priv
>   * @free_db_resources: Free DMA memory holding doorbell info (AdminQ only)
> + * @setup_mgmt_irq: Setup control plane IRQ
> + * @teardown_mgmt_irq: Teardown control plane IRQ
>   * @get_ptype_map: Learn packet type map from device and store it in @priv
>   * @configure_rss: Set up default RSS configuration
>   * @setup_stats_report: Set up DMA region for stats report (AdminQ only)
> @@ -847,6 +849,8 @@ struct gve_ctrl_ops {
>         int (*set_num_ntfy_blks)(struct gve_priv *priv);
>         int (*request_db_info)(struct gve_priv *priv);
>         void (*free_db_resources)(struct gve_priv *priv);
> +       int (*setup_mgmt_irq)(struct gve_priv *priv);
> +       void (*teardown_mgmt_irq)(struct gve_priv *priv);
>         int (*get_ptype_map)(struct gve_priv *priv);
>         int (*configure_rss)(struct gve_priv *priv,
>                              struct ethtool_rxfh_param *param);
> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index 259dcd617216..5dc5c63cee32 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> @@ -1790,3 +1790,35 @@ void gve_adminq_free_db_resources(struct gve_priv *priv)
>         priv->irq_db_indices = NULL;
>         gve_free_counter_array(priv);
>  }
> +
> +static irqreturn_t gve_mgmnt_intr(int irq, void *arg)
> +{
> +       struct gve_priv *priv = arg;
> +
> +       queue_work(priv->gve_wq, &priv->service_task);
> +       return IRQ_HANDLED;
> +}
> +
> +int gve_adminq_setup_mgmt_irq(struct gve_priv *priv)
> +{
> +       int err;
> +
> +       snprintf(priv->mgmt_msix_name, sizeof(priv->mgmt_msix_name),
> +                "gve-mgmnt@pci:%s", pci_name(priv->pdev));
> +       err = request_irq(priv->msix_vectors[priv->mgmt_msix_idx].vector,
> +                         gve_mgmnt_intr, 0, priv->mgmt_msix_name, priv);
> +       if (err)
> +               return err;
> +
> +       priv->mgmt_irq_requested = true;
> +
> +       return 0;
> +}
> +
> +void gve_adminq_teardown_mgmt_irq(struct gve_priv *priv)
> +{
> +       if (priv->mgmt_irq_requested) {
> +               free_irq(priv->msix_vectors[priv->mgmt_msix_idx].vector, priv);
> +               priv->mgmt_irq_requested = false;
> +       }
> +}
> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.h b/drivers/net/ethernet/google/gve/gve_adminq.h
> index 985b1aefea6e..ff656fb4f113 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.h
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.h
> @@ -659,5 +659,7 @@ int gve_adminq_set_num_ntfy_blks(struct gve_priv *priv);
>  void gve_adminq_set_num_queues(struct gve_priv *priv);
>  int gve_adminq_request_db_info(struct gve_priv *priv);
>  void gve_adminq_free_db_resources(struct gve_priv *priv);
> +int gve_adminq_setup_mgmt_irq(struct gve_priv *priv);
> +void gve_adminq_teardown_mgmt_irq(struct gve_priv *priv);
>
>  #endif /* _GVE_ADMINQ_H */
> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index 55f48aee125e..9d00093826e3 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -267,14 +267,6 @@ static void gve_free_stats_report(struct gve_priv *priv)
>         priv->stats_report = NULL;
>  }

Sashiko says:
This is a pre-existing issue, but does this function fail to cancel
the stats_report_task?

This issue predates this patch series; will have to introduce a net
patch to fix.

>
> -static irqreturn_t gve_mgmnt_intr(int irq, void *arg)
> -{
> -       struct gve_priv *priv = arg;
> -
> -       queue_work(priv->gve_wq, &priv->service_task);
> -       return IRQ_HANDLED;
> -}
> -
>  static irqreturn_t gve_intr(int irq, void *arg)
>  {
>         struct gve_notify_block *block = arg;
> @@ -499,6 +491,7 @@ static int gve_alloc_notify_blocks(struct gve_priv *priv)
>
>  static void gve_teardown_notify_blocks(struct gve_priv *priv)
>  {
> +       const struct gve_ctrl_ops *ops = priv->ctrl_ops;
>         int i;
>
>         if (!priv->ntfy_blocks)
> @@ -518,29 +511,24 @@ static void gve_teardown_notify_blocks(struct gve_priv *priv)
>                 block->irq_requested = false;
>         }
>
> -       if (priv->mgmt_irq_requested) {
> -               free_irq(priv->msix_vectors[priv->mgmt_msix_idx].vector, priv);
> -               priv->mgmt_irq_requested = false;
> -       }
> +       ops->teardown_mgmt_irq(priv);
>  }

Sashiko says:
This isn't a bug introduced by this patch, but is it possible for the
management IRQ to fire after the workqueue is destroyed?

Again, this will be fixed with changes in gve: Simplify Reset Logic.

>
>  static int gve_setup_notify_blocks(struct gve_priv *priv)
>  {
> +       const struct gve_ctrl_ops *ops = priv->ctrl_ops;
>         const struct cpumask *node_mask;
>         unsigned int cur_cpu;
>         int i;
>         int err;
>
> -       /* Setup Management Vector  - the last vector */
> -       snprintf(priv->mgmt_msix_name, sizeof(priv->mgmt_msix_name),
> -                "gve-mgmnt@pci:%s", pci_name(priv->pdev));
> -       err = request_irq(priv->msix_vectors[priv->mgmt_msix_idx].vector,
> -                         gve_mgmnt_intr, 0, priv->mgmt_msix_name, priv);
> +       /* Setup Management Vector */
> +       err = ops->setup_mgmt_irq(priv);
>         if (err) {
> -               dev_err(&priv->pdev->dev, "Did not receive management vector.\n");
> +               dev_err(&priv->pdev->dev,
> +                       "Did not receive management vector.\n");
>                 return err;
>         }
> -       priv->mgmt_irq_requested = true;
>
>         node_mask = gve_get_node_mask(priv);
>         cur_cpu = cpumask_first(node_mask);
> @@ -2460,6 +2448,8 @@ static const struct gve_ctrl_ops gve_adminq_ops = {
>         .configure_rss          = gve_adminq_configure_rss,
>         .request_db_info        = gve_adminq_request_db_info,
>         .free_db_resources      = gve_adminq_free_db_resources,
> +       .setup_mgmt_irq         = gve_adminq_setup_mgmt_irq,
> +       .teardown_mgmt_irq      = gve_adminq_teardown_mgmt_irq,
>  };
>
>  static int gve_init_priv(struct gve_priv *priv)
> --
> 2.54.0.1013.g208068f2d8-goog
>


-- 

Joshua Washington | Software Engineer | joshwash@xxxxxxxxxx | (414) 366-4423