On Thu, Apr 24, 2025 at 01:32:31PM +0200, Larysa Zaremba wrote:
> From: Pavan Kumar Linga <[email protected]>
> 
> Support to initialize and configure controlq, Xn manager,
> MMIO and reset APIs was introduced in libie. As part of it,
> most of the existing controlq structures are renamed and
> modified. Use those APIs in idpf and make all the necessary changes.
> 
> Previously for the send and receive virtchnl messages, there
> used to be a memcpy involved in controlq code to copy the buffer
> info passed by the send function into the controlq specific
> buffers. There was no restriction to use automatic memory
> in that case. The new implementation in libie removed copying
> of the send buffer info and introduced DMA mapping of the
> send buffer itself. To accommodate it, use dynamic memory for
> the send buffers. In case of receive, idpf receives a page pool
> buffer allocated by the libie and care should be taken to
> release it after use in the idpf.
> 
> The changes are fairly trivial and localized, with a notable exception
> being the consolidation of idpf_vc_xn_shutdown and idpf_deinit_dflt_mbx
> under the latter name. This has some additional consequences that are
> addressed in the following patches.
> 
> Reviewed-by: Michal Kubiak <[email protected]>
> Signed-off-by: Pavan Kumar Linga <[email protected]>
> Co-developed-by: Larysa Zaremba <[email protected]>
> Signed-off-by: Larysa Zaremba <[email protected]>
> ---
>  drivers/net/ethernet/intel/idpf/Kconfig       |    1 +
>  drivers/net/ethernet/intel/idpf/Makefile      |    2 -
>  drivers/net/ethernet/intel/idpf/idpf.h        |   42 +-
>  .../net/ethernet/intel/idpf/idpf_controlq.c   |  624 -------
>  .../net/ethernet/intel/idpf/idpf_controlq.h   |  130 --
>  .../ethernet/intel/idpf/idpf_controlq_api.h   |  177 --
>  .../ethernet/intel/idpf/idpf_controlq_setup.c |  171 --
>  drivers/net/ethernet/intel/idpf/idpf_dev.c    |   91 +-
>  drivers/net/ethernet/intel/idpf/idpf_lib.c    |   49 +-
>  drivers/net/ethernet/intel/idpf/idpf_main.c   |   87 +-
>  drivers/net/ethernet/intel/idpf/idpf_mem.h    |   20 -
>  drivers/net/ethernet/intel/idpf/idpf_txrx.h   |    2 +-
>  drivers/net/ethernet/intel/idpf/idpf_vf_dev.c |   89 +-
>  .../net/ethernet/intel/idpf/idpf_virtchnl.c   | 1622 ++++++-----------
>  .../net/ethernet/intel/idpf/idpf_virtchnl.h   |   89 +-
>  .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   |  303 ++-
>  16 files changed, 886 insertions(+), 2613 deletions(-)

This patch is rather large.
Is there a way it could be split up into more easily reviewable chunks?

>  delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.c
>  delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.h
>  delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq_api.h
>  delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq_setup.c
>  delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_mem.h

...

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c 
> b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c

...

> @@ -2520,15 +2045,18 @@ static void idpf_finalize_ptype_lookup(struct 
> libeth_rx_pt *ptype)
>   */
>  int idpf_send_get_rx_ptype_msg(struct idpf_vport *vport)
>  {
> -     struct virtchnl2_get_ptype_info *get_ptype_info __free(kfree) = NULL;
> -     struct virtchnl2_get_ptype_info *ptype_info __free(kfree) = NULL;
> +     struct libie_ctlq_xn_send_params xn_params = {
> +             .timeout_ms     = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
> +             .chnl_opcode    = VIRTCHNL2_OP_GET_PTYPE_INFO,
> +     };
>       struct libeth_rx_pt *ptype_lkup __free(kfree) = NULL;
> +     struct virtchnl2_get_ptype_info *get_ptype_info;
>       int max_ptype, ptypes_recvd = 0, ptype_offset;
>       struct idpf_adapter *adapter = vport->adapter;
> -     struct idpf_vc_xn_params xn_params = {};
> +     struct virtchnl2_get_ptype_info *ptype_info;
> +     int buf_size = sizeof(*get_ptype_info);
>       u16 next_ptype_id = 0;
> -     ssize_t reply_sz;
> -     int i, j, k;
> +     int i, j, k, err = 0;
>  
>       if (vport->rx_ptype_lkup)
>               return 0;
> @@ -2542,22 +2070,11 @@ int idpf_send_get_rx_ptype_msg(struct idpf_vport 
> *vport)
>       if (!ptype_lkup)
>               return -ENOMEM;
>  
> -     get_ptype_info = kzalloc(sizeof(*get_ptype_info), GFP_KERNEL);
> -     if (!get_ptype_info)
> -             return -ENOMEM;
> -
> -     ptype_info = kzalloc(IDPF_CTLQ_MAX_BUF_LEN, GFP_KERNEL);
> -     if (!ptype_info)
> -             return -ENOMEM;
> -
> -     xn_params.vc_op = VIRTCHNL2_OP_GET_PTYPE_INFO;
> -     xn_params.send_buf.iov_base = get_ptype_info;
> -     xn_params.send_buf.iov_len = sizeof(*get_ptype_info);
> -     xn_params.recv_buf.iov_base = ptype_info;
> -     xn_params.recv_buf.iov_len = IDPF_CTLQ_MAX_BUF_LEN;
> -     xn_params.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC;
> -
>       while (next_ptype_id < max_ptype) {
> +             get_ptype_info = kzalloc(buf_size, GFP_KERNEL);
> +             if (!get_ptype_info)
> +                     return -ENOMEM;
> +
>               get_ptype_info->start_ptype_id = cpu_to_le16(next_ptype_id);
>  
>               if ((next_ptype_id + IDPF_RX_MAX_PTYPES_PER_BUF) > max_ptype)
> @@ -2567,13 +2084,15 @@ int idpf_send_get_rx_ptype_msg(struct idpf_vport 
> *vport)
>                       get_ptype_info->num_ptypes =
>                               cpu_to_le16(IDPF_RX_MAX_PTYPES_PER_BUF);
>  
> -             reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
> -             if (reply_sz < 0)
> -                     return reply_sz;
> +             err = idpf_send_mb_msg(adapter, &xn_params, get_ptype_info,
> +                                    buf_size);
> +             if (err)
> +                     goto free_tx_buf;
>  
> +             ptype_info = xn_params.recv_mem.iov_base;
>               ptypes_recvd += le16_to_cpu(ptype_info->num_ptypes);
>               if (ptypes_recvd > max_ptype)
> -                     return -EINVAL;

Should err be set to -EINVAL here?

Flagged by Smatch.

> +                     goto free_rx_buf;
>  
>               next_ptype_id = le16_to_cpu(get_ptype_info->start_ptype_id) +
>                               le16_to_cpu(get_ptype_info->num_ptypes);
> @@ -2589,8 +2108,8 @@ int idpf_send_get_rx_ptype_msg(struct idpf_vport *vport)
>                                       ((u8 *)ptype_info + ptype_offset);
>  
>                       ptype_offset += IDPF_GET_PTYPE_SIZE(ptype);
> -                     if (ptype_offset > IDPF_CTLQ_MAX_BUF_LEN)
> -                             return -EINVAL;
> +                     if (ptype_offset > LIBIE_CTLQ_MAX_BUF_LEN)
> +                             goto free_rx_buf;
>  
>                       /* 0xFFFF indicates end of ptypes */
>                       if (le16_to_cpu(ptype->ptype_id_10) ==
> @@ -2720,12 +2239,24 @@ int idpf_send_get_rx_ptype_msg(struct idpf_vport 
> *vport)
>  
>                       idpf_finalize_ptype_lookup(&ptype_lkup[k]);
>               }
> +             libie_ctlq_release_rx_buf(adapter->arq,
> +                                       &xn_params.recv_mem);
> +             if (libie_cp_can_send_onstack(buf_size))
> +                     kfree(get_ptype_info);
>       }
>  
>  out:
>       vport->rx_ptype_lkup = no_free_ptr(ptype_lkup);
>  
>       return 0;
> +
> +free_rx_buf:
> +     libie_ctlq_release_rx_buf(adapter->arq, &xn_params.recv_mem);
> +free_tx_buf:
> +     if (libie_cp_can_send_onstack(buf_size))
> +             kfree(get_ptype_info);
> +
> +     return err;
>  }
>  
>  /**

...

Reply via email to