[PATCH v2 06/15] vdpa/mlx5: pre-create virtq in the prob

[Maxime Coquelin]

I would rename the title to something like:

"vdpa/mlx5: pre-create virtq at probe time"

On 6/16/22 04:30, Li Zhang wrote:
> dev_config operation is called in LM progress.
> LM time is very critical because all
> the VM packets are dropped directly at that time.
> 
> Move the virtq creation to probe time and
> only modify the configuration later in
> the dev_config stage using the new ability
> to modify virtq.
> 
> This optimization accelerates the LM process and
> reduces its time by 70%.

Nice.

> Signed-off-by: Li Zhang <lizh at nvidia.com>
> Acked-by: Matan Azrad <matan at nvidia.com>
> ---
>   doc/guides/rel_notes/release_22_07.rst |   4 +
>   drivers/vdpa/mlx5/mlx5_vdpa.h          |   4 +
>   drivers/vdpa/mlx5/mlx5_vdpa_lm.c       |  13 +-
>   drivers/vdpa/mlx5/mlx5_vdpa_virtq.c    | 257 +++++++++++++++----------
>   4 files changed, 174 insertions(+), 104 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> index f2cf41def9..2056cd9ee7 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -175,6 +175,10 @@ New Features
>     This is a fall-back implementation for platforms that
>     don't support vector operations.
>   
> +* **Updated Nvidia mlx5 vDPA driver.**
> +
> +  * Added new devargs ``queue_size`` and ``queues`` to allow prior creation of virtq resources.
> +
>   
>   Removed Items
>   -------------
> diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.h b/drivers/vdpa/mlx5/mlx5_vdpa.h
> index bf82026e37..e5553079fe 100644
> --- a/drivers/vdpa/mlx5/mlx5_vdpa.h
> +++ b/drivers/vdpa/mlx5/mlx5_vdpa.h
> @@ -80,6 +80,7 @@ struct mlx5_vdpa_virtq {
>   	uint16_t vq_size;
>   	uint8_t notifier_state;
>   	bool stopped;
> +	uint32_t configured:1;
>   	uint32_t version;
>   	struct mlx5_vdpa_priv *priv;
>   	struct mlx5_devx_obj *virtq;
> @@ -489,4 +490,7 @@ mlx5_vdpa_virtq_stats_reset(struct mlx5_vdpa_priv *priv, int qid);
>    */
>   void
>   mlx5_vdpa_drain_cq(struct mlx5_vdpa_priv *priv);
> +
> +bool
> +mlx5_vdpa_is_modify_virtq_supported(struct mlx5_vdpa_priv *priv);
>   #endif /* RTE_PMD_MLX5_VDPA_H_ */
> diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_lm.c b/drivers/vdpa/mlx5/mlx5_vdpa_lm.c
> index 43a2b98255..a8faf0c116 100644
> --- a/drivers/vdpa/mlx5/mlx5_vdpa_lm.c
> +++ b/drivers/vdpa/mlx5/mlx5_vdpa_lm.c
> @@ -12,14 +12,17 @@ int
>   mlx5_vdpa_logging_enable(struct mlx5_vdpa_priv *priv, int enable)
>   {
>   	struct mlx5_devx_virtq_attr attr = {
> -		.type = MLX5_VIRTQ_MODIFY_TYPE_DIRTY_BITMAP_DUMP_ENABLE,
> +		.mod_fields_bitmap =
> +			MLX5_VIRTQ_MODIFY_TYPE_DIRTY_BITMAP_DUMP_ENABLE,
>   		.dirty_bitmap_dump_enable = enable,
>   	};
> +	struct mlx5_vdpa_virtq *virtq;
>   	int i;
>   
>   	for (i = 0; i < priv->nr_virtqs; ++i) {
>   		attr.queue_index = i;
> -		if (!priv->virtqs[i].virtq) {
> +		virtq = &priv->virtqs[i];
> +		if (!virtq->configured) {
>   			DRV_LOG(DEBUG, "virtq %d is invalid for dirty bitmap "
>   				"enabling.", i);

Please avoid cutting logs, it makes it harder to grep in the code.
Also, now we can have up to 100 chars, so maybe it would fit anyway.


Other than that:

Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>

Thanks,
Maxime