linux-kernel - Re: [PATCH] vdpa/mlx5: Fix MQ to support non power of two num queues

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <27fec483-e860-aea4-f2d6-3082d702f561@oracle.com>
Date:   Wed, 14 Sep 2022 12:14:48 +0100
From:   Si-Wei Liu <si-wei.liu@...cle.com>
To:     Eli Cohen <elic@...dia.com>, mst@...hat.com, jasowang@...hat.com,
        eperezma@...hat.com, virtualization@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vdpa/mlx5: Fix MQ to support non power of two num queues



On 9/12/2022 1:50 PM, Eli Cohen wrote:
> RQT objects require that a power of two value be configured for both
> rqt_max_size and rqt_actual size.
>
> For create_rqt, make sure to round up to the power of two the value of
> given by the user who created the vdpa device and given by
> ndev->rqt_size. The actual size is also rounded up to the power of two
> using the current number of VQs given by ndev->cur_num_vqs.
>
> Same goes with modify_rqt where we need to make sure act size is power
> of two based on the new number of QPs.
>
> Without this patch, attempt to create a device with non power of two QPs
> would result in error from firmware.

What kind of error would it end up with, is there explicit warning in 
dmesg or it's just implicit? And is there performance impact? It'd be 
nice to add such description that we can easily match the symptom just 
in case.

>
> Fixes: 52893733f2c5 ("vdpa/mlx5: Add multiqueue support")
> Signed-off-by: Eli Cohen <elic@...dia.com>
Acked-by: Si-Wei Liu <si-wei.liu@...cle.com>

Thanks,
-Siwei
> ---
>   drivers/vdpa/mlx5/net/mlx5_vnet.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index ed100a35e596..90913365def4 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1320,6 +1320,8 @@ static void teardown_vq(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *
>   
>   static int create_rqt(struct mlx5_vdpa_net *ndev)
>   {
> +	int rqt_table_size = roundup_pow_of_two(ndev->rqt_size);
> +	int act_sz = roundup_pow_of_two(ndev->cur_num_vqs / 2);
>   	__be32 *list;
>   	void *rqtc;
>   	int inlen;
> @@ -1327,7 +1329,7 @@ static int create_rqt(struct mlx5_vdpa_net *ndev)
>   	int i, j;
>   	int err;
>   
> -	inlen = MLX5_ST_SZ_BYTES(create_rqt_in) + ndev->rqt_size * MLX5_ST_SZ_BYTES(rq_num);
> +	inlen = MLX5_ST_SZ_BYTES(create_rqt_in) + rqt_table_size * MLX5_ST_SZ_BYTES(rq_num);
>   	in = kzalloc(inlen, GFP_KERNEL);
>   	if (!in)
>   		return -ENOMEM;
> @@ -1336,12 +1338,12 @@ static int create_rqt(struct mlx5_vdpa_net *ndev)
>   	rqtc = MLX5_ADDR_OF(create_rqt_in, in, rqt_context);
>   
>   	MLX5_SET(rqtc, rqtc, list_q_type, MLX5_RQTC_LIST_Q_TYPE_VIRTIO_NET_Q);
> -	MLX5_SET(rqtc, rqtc, rqt_max_size, ndev->rqt_size);
> +	MLX5_SET(rqtc, rqtc, rqt_max_size, rqt_table_size);
>   	list = MLX5_ADDR_OF(rqtc, rqtc, rq_num[0]);
> -	for (i = 0, j = 0; i < ndev->rqt_size; i++, j += 2)
> +	for (i = 0, j = 0; i < act_sz; i++, j += 2)
>   		list[i] = cpu_to_be32(ndev->vqs[j % ndev->cur_num_vqs].virtq_id);
>   
> -	MLX5_SET(rqtc, rqtc, rqt_actual_size, ndev->rqt_size);
> +	MLX5_SET(rqtc, rqtc, rqt_actual_size, act_sz);
>   	err = mlx5_vdpa_create_rqt(&ndev->mvdev, in, inlen, &ndev->res.rqtn);
>   	kfree(in);
>   	if (err)
> @@ -1354,6 +1356,7 @@ static int create_rqt(struct mlx5_vdpa_net *ndev)
>   
>   static int modify_rqt(struct mlx5_vdpa_net *ndev, int num)
>   {
> +	int act_sz = roundup_pow_of_two(num / 2);
>   	__be32 *list;
>   	void *rqtc;
>   	int inlen;
> @@ -1361,7 +1364,7 @@ static int modify_rqt(struct mlx5_vdpa_net *ndev, int num)
>   	int i, j;
>   	int err;
>   
> -	inlen = MLX5_ST_SZ_BYTES(modify_rqt_in) + ndev->rqt_size * MLX5_ST_SZ_BYTES(rq_num);
> +	inlen = MLX5_ST_SZ_BYTES(modify_rqt_in) + act_sz * MLX5_ST_SZ_BYTES(rq_num);
>   	in = kzalloc(inlen, GFP_KERNEL);
>   	if (!in)
>   		return -ENOMEM;
> @@ -1372,10 +1375,10 @@ static int modify_rqt(struct mlx5_vdpa_net *ndev, int num)
>   	MLX5_SET(rqtc, rqtc, list_q_type, MLX5_RQTC_LIST_Q_TYPE_VIRTIO_NET_Q);
>   
>   	list = MLX5_ADDR_OF(rqtc, rqtc, rq_num[0]);
> -	for (i = 0, j = 0; i < ndev->rqt_size; i++, j += 2)
> +	for (i = 0, j = 0; i < act_sz; i++, j = j + 2)
>   		list[i] = cpu_to_be32(ndev->vqs[j % num].virtq_id);
>   
> -	MLX5_SET(rqtc, rqtc, rqt_actual_size, ndev->rqt_size);
> +	MLX5_SET(rqtc, rqtc, rqt_actual_size, act_sz);
>   	err = mlx5_vdpa_modify_rqt(&ndev->mvdev, in, inlen, ndev->res.rqtn);
>   	kfree(in);
>   	if (err)