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] [day] [month] [year] [list]
Message-ID: <DM8PR12MB54001AF4AF5093504EB7003AAB469@DM8PR12MB5400.namprd12.prod.outlook.com>
Date:   Wed, 14 Sep 2022 12:23:42 +0000
From:   Eli Cohen <elic@...dia.com>
To:     Si-Wei Liu <si-wei.liu@...cle.com>,
        "mst@...hat.com" <mst@...hat.com>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "eperezma@...hat.com" <eperezma@...hat.com>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] vdpa/mlx5: Fix MQ to support non power of two num queues

> From: Si-Wei Liu <si-wei.liu@...cle.com>
> Sent: Wednesday, 14 September 2022 14:15
> 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?
The error would look something like this:
[Sep14 15:20] mlx5_core.sf mlx5_core.sf.6: mlx5_cmd_out_err:778:(pid 3466): CREATE_RQT(0x916) op_mod(0x0) failed, status bad parameter(0x3), syndrome (0x538fab), err(-22)

> And is there performance impact? It'd be
> nice to add such description that we can easily match the symptom just
> in case.
> 
I don't think there would be performance impact but I haven't checked performance with this fix.

> >
> > 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)