Re: [PATCH RESEND V2 3/3] vdpa/mlx5: Use consistent RQT size

2022-04-06 Thread Jason Wang


在 2022/4/6 上午10:35, Jason Wang 写道:


在 2022/4/4 下午7:24, Michael S. Tsirkin 写道:

On Mon, Apr 04, 2022 at 11:07:36AM +, Eli Cohen wrote:

From: Michael S. Tsirkin 
Sent: Monday, April 4, 2022 1:35 PM
To: Jason Wang 
Cc: Eli Cohen ; hdan...@sina.com; 
virtualization@lists.linux-foundation.org; 
linux-ker...@vger.kernel.org

Subject: Re: [PATCH RESEND V2 3/3] vdpa/mlx5: Use consistent RQT size

On Tue, Mar 29, 2022 at 12:21:09PM +0800, Jason Wang wrote:

From: Eli Cohen 

The current code evaluates RQT size based on the configured number of
virtqueues. This can raise an issue in the following scenario:

Assume MQ was negotiated.
1. mlx5_vdpa_set_map() gets called.
2. handle_ctrl_mq() is called setting cur_num_vqs to some value, 
lower

    than the configured max VQs.
3. A second set_map gets called, but now a smaller number of VQs 
is used

    to evaluate the size of the RQT.
4. handle_ctrl_mq() is called with a value larger than what the 
RQT can

    hold. This will emit errors and the driver state is compromised.

To fix this, we use a new field in struct mlx5_vdpa_net to hold the
required number of entries in the RQT. This value is evaluated in
mlx5_vdpa_set_driver_features() where we have the negotiated features
all set up.

In addtion

addition?

Do you need me to send another version?

It's a bit easier that way but I can handle it manually too.



Let me send a new version with this fixed.



Ok, it looks like if I use git-send-email when From:tag is not me. The 
patch will be sent as a attachment as spotted by Maxime.


Eli, would you please send a v3 with my acked-by? (Since I don't want to 
change the author)


Thanks








If so, let's wait for Jason's reply.

Right.


to that, we take into consideration the max capability of RQT
entries early when the device is added so we don't need to take 
consider

it when creating the RQT.

Last, we remove the use of mlx5_vdpa_max_qps() which just returns the
max_vas / 2 and make the code clearer.

Fixes: 52893733f2c5 ("vdpa/mlx5: Add multiqueue support")
Signed-off-by: Eli Cohen 

Jason I don't have your ack or S.O.B on this one.



My bad, for some reason, I miss that.

Will fix.

Thanks






---
  drivers/vdpa/mlx5/net/mlx5_vnet.c | 61 
+++

  1 file changed, 21 insertions(+), 40 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c

index 53b8c1a68f90..61bec1ed0bc9 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -161,6 +161,7 @@ struct mlx5_vdpa_net {
  struct mlx5_flow_handle *rx_rule_mcast;
  bool setup;
  u32 cur_num_vqs;
+    u32 rqt_size;
  struct notifier_block nb;
  struct vdpa_callback config_cb;
  struct mlx5_vdpa_wq_ent cvq_ent;
@@ -204,17 +205,12 @@ static __virtio16 cpu_to_mlx5vdpa16(struct 
mlx5_vdpa_dev *mvdev, u16 val)
  return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), 
val);

  }

-static inline u32 mlx5_vdpa_max_qps(int max_vqs)
-{
-    return max_vqs / 2;
-}
-
  static u16 ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev)
  {
  if (!(mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
  return 2;

-    return 2 * mlx5_vdpa_max_qps(mvdev->max_vqs);
+    return mvdev->max_vqs;
  }

  static bool is_ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev, u16 idx)
@@ -1236,25 +1232,13 @@ static void teardown_vq(struct 
mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *

  static int create_rqt(struct mlx5_vdpa_net *ndev)
  {
  __be32 *list;
-    int max_rqt;
  void *rqtc;
  int inlen;
  void *in;
  int i, j;
  int err;
-    int num;
-
-    if (!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
-    num = 1;
-    else
-    num = ndev->cur_num_vqs / 2;

-    max_rqt = min_t(int, roundup_pow_of_two(num),
-    1 << MLX5_CAP_GEN(ndev->mvdev.mdev, log_max_rqt_size));
-    if (max_rqt < 1)
-    return -EOPNOTSUPP;
-
-    inlen = MLX5_ST_SZ_BYTES(create_rqt_in) + max_rqt * 
MLX5_ST_SZ_BYTES(rq_num);
+    inlen = MLX5_ST_SZ_BYTES(create_rqt_in) + ndev->rqt_size * 
MLX5_ST_SZ_BYTES(rq_num);

  in = kzalloc(inlen, GFP_KERNEL);
  if (!in)
  return -ENOMEM;
@@ -1263,12 +1247,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, max_rqt);
+    MLX5_SET(rqtc, rqtc, rqt_max_size, ndev->rqt_size);
  list = MLX5_ADDR_OF(rqtc, rqtc, rq_num[0]);
-    for (i = 0, j = 0; i < max_rqt; i++, j += 2)
-    list[i] = cpu_to_be32(ndev->vqs[j % (2 * num)].virtq_id);
+    for (i = 0, j = 0; i < ndev->rqt_size; i++, j += 2)
+    list[i] = cpu_to_be32(ndev->vqs[j % 
ndev->cur_num_vqs].virtq_id);


-    MLX5_SET(rqtc, rqtc, rqt_actual_size, max_rqt);
+    MLX5_SET(rqtc, rqtc, rqt_actual_size

Re: [PATCH RESEND V2 3/3] vdpa/mlx5: Use consistent RQT size

2022-04-05 Thread Jason Wang


在 2022/4/4 下午7:24, Michael S. Tsirkin 写道:

On Mon, Apr 04, 2022 at 11:07:36AM +, Eli Cohen wrote:

From: Michael S. Tsirkin 
Sent: Monday, April 4, 2022 1:35 PM
To: Jason Wang 
Cc: Eli Cohen ; hdan...@sina.com; 
virtualization@lists.linux-foundation.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH RESEND V2 3/3] vdpa/mlx5: Use consistent RQT size

On Tue, Mar 29, 2022 at 12:21:09PM +0800, Jason Wang wrote:

From: Eli Cohen 

The current code evaluates RQT size based on the configured number of
virtqueues. This can raise an issue in the following scenario:

Assume MQ was negotiated.
1. mlx5_vdpa_set_map() gets called.
2. handle_ctrl_mq() is called setting cur_num_vqs to some value, lower
than the configured max VQs.
3. A second set_map gets called, but now a smaller number of VQs is used
to evaluate the size of the RQT.
4. handle_ctrl_mq() is called with a value larger than what the RQT can
hold. This will emit errors and the driver state is compromised.

To fix this, we use a new field in struct mlx5_vdpa_net to hold the
required number of entries in the RQT. This value is evaluated in
mlx5_vdpa_set_driver_features() where we have the negotiated features
all set up.

In addtion

addition?

Do you need me to send another version?

It's a bit easier that way but I can handle it manually too.



Let me send a new version with this fixed.





If so, let's wait for Jason's reply.

Right.


to that, we take into consideration the max capability of RQT
entries early when the device is added so we don't need to take consider
it when creating the RQT.

Last, we remove the use of mlx5_vdpa_max_qps() which just returns the
max_vas / 2 and make the code clearer.

Fixes: 52893733f2c5 ("vdpa/mlx5: Add multiqueue support")
Signed-off-by: Eli Cohen 

Jason I don't have your ack or S.O.B on this one.



My bad, for some reason, I miss that.

Will fix.

Thanks






---
  drivers/vdpa/mlx5/net/mlx5_vnet.c | 61 +++
  1 file changed, 21 insertions(+), 40 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 53b8c1a68f90..61bec1ed0bc9 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -161,6 +161,7 @@ struct mlx5_vdpa_net {
struct mlx5_flow_handle *rx_rule_mcast;
bool setup;
u32 cur_num_vqs;
+   u32 rqt_size;
struct notifier_block nb;
struct vdpa_callback config_cb;
struct mlx5_vdpa_wq_ent cvq_ent;
@@ -204,17 +205,12 @@ static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev 
*mvdev, u16 val)
return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
  }

-static inline u32 mlx5_vdpa_max_qps(int max_vqs)
-{
-   return max_vqs / 2;
-}
-
  static u16 ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev)
  {
if (!(mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
return 2;

-   return 2 * mlx5_vdpa_max_qps(mvdev->max_vqs);
+   return mvdev->max_vqs;
  }

  static bool is_ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev, u16 idx)
@@ -1236,25 +1232,13 @@ static void teardown_vq(struct mlx5_vdpa_net *ndev, 
struct mlx5_vdpa_virtqueue *
  static int create_rqt(struct mlx5_vdpa_net *ndev)
  {
__be32 *list;
-   int max_rqt;
void *rqtc;
int inlen;
void *in;
int i, j;
int err;
-   int num;
-
-   if (!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
-   num = 1;
-   else
-   num = ndev->cur_num_vqs / 2;

-   max_rqt = min_t(int, roundup_pow_of_two(num),
-   1 << MLX5_CAP_GEN(ndev->mvdev.mdev, log_max_rqt_size));
-   if (max_rqt < 1)
-   return -EOPNOTSUPP;
-
-   inlen = MLX5_ST_SZ_BYTES(create_rqt_in) + max_rqt * 
MLX5_ST_SZ_BYTES(rq_num);
+   inlen = MLX5_ST_SZ_BYTES(create_rqt_in) + ndev->rqt_size * 
MLX5_ST_SZ_BYTES(rq_num);
in = kzalloc(inlen, GFP_KERNEL);
if (!in)
return -ENOMEM;
@@ -1263,12 +1247,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, max_rqt);
+   MLX5_SET(rqtc, rqtc, rqt_max_size, ndev->rqt_size);
list = MLX5_ADDR_OF(rqtc, rqtc, rq_num[0]);
-   for (i = 0, j = 0; i < max_rqt; i++, j += 2)
-   list[i] = cpu_to_be32(ndev->vqs[j % (2 * num)].virtq_id);
+   for (i = 0, j = 0; i < ndev->rqt_size; i++, j += 2)
+   list[i] = cpu_to_be32(ndev->vqs[j % 
ndev->cur_num_vqs].virtq_id);

-   MLX5_SET(rqtc, rqtc, rqt_actual_size, max_rqt);
+   MLX5_SET(rqtc, rqtc, rqt_actual_size, ndev->rqt_size);
err = mlx5_vdpa_create_rqt(>mvdev, in, inlen, >res.rqtn);
kfree(in);
if (err)
@@ -1282,19 +1266,13 @@ st

Re: [PATCH RESEND V2 3/3] vdpa/mlx5: Use consistent RQT size

2022-04-04 Thread Michael S. Tsirkin
On Mon, Apr 04, 2022 at 11:07:36AM +, Eli Cohen wrote:
> > From: Michael S. Tsirkin 
> > Sent: Monday, April 4, 2022 1:35 PM
> > To: Jason Wang 
> > Cc: Eli Cohen ; hdan...@sina.com; 
> > virtualization@lists.linux-foundation.org; linux-ker...@vger.kernel.org
> > Subject: Re: [PATCH RESEND V2 3/3] vdpa/mlx5: Use consistent RQT size
> > 
> > On Tue, Mar 29, 2022 at 12:21:09PM +0800, Jason Wang wrote:
> > > From: Eli Cohen 
> > >
> > > The current code evaluates RQT size based on the configured number of
> > > virtqueues. This can raise an issue in the following scenario:
> > >
> > > Assume MQ was negotiated.
> > > 1. mlx5_vdpa_set_map() gets called.
> > > 2. handle_ctrl_mq() is called setting cur_num_vqs to some value, lower
> > >than the configured max VQs.
> > > 3. A second set_map gets called, but now a smaller number of VQs is used
> > >to evaluate the size of the RQT.
> > > 4. handle_ctrl_mq() is called with a value larger than what the RQT can
> > >hold. This will emit errors and the driver state is compromised.
> > >
> > > To fix this, we use a new field in struct mlx5_vdpa_net to hold the
> > > required number of entries in the RQT. This value is evaluated in
> > > mlx5_vdpa_set_driver_features() where we have the negotiated features
> > > all set up.
> > >
> > > In addtion
> > 
> > addition?
> 
> Do you need me to send another version?

It's a bit easier that way but I can handle it manually too.

> If so, let's wait for Jason's reply.

Right.

> > 
> > > to that, we take into consideration the max capability of RQT
> > > entries early when the device is added so we don't need to take consider
> > > it when creating the RQT.
> > >
> > > Last, we remove the use of mlx5_vdpa_max_qps() which just returns the
> > > max_vas / 2 and make the code clearer.
> > >
> > > Fixes: 52893733f2c5 ("vdpa/mlx5: Add multiqueue support")
> > > Signed-off-by: Eli Cohen 
> > 
> > Jason I don't have your ack or S.O.B on this one.
> > 
> > 
> > > ---
> > >  drivers/vdpa/mlx5/net/mlx5_vnet.c | 61 +++
> > >  1 file changed, 21 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
> > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > index 53b8c1a68f90..61bec1ed0bc9 100644
> > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > @@ -161,6 +161,7 @@ struct mlx5_vdpa_net {
> > >   struct mlx5_flow_handle *rx_rule_mcast;
> > >   bool setup;
> > >   u32 cur_num_vqs;
> > > + u32 rqt_size;
> > >   struct notifier_block nb;
> > >   struct vdpa_callback config_cb;
> > >   struct mlx5_vdpa_wq_ent cvq_ent;
> > > @@ -204,17 +205,12 @@ static __virtio16 cpu_to_mlx5vdpa16(struct 
> > > mlx5_vdpa_dev *mvdev, u16 val)
> > >   return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
> > >  }
> > >
> > > -static inline u32 mlx5_vdpa_max_qps(int max_vqs)
> > > -{
> > > - return max_vqs / 2;
> > > -}
> > > -
> > >  static u16 ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev)
> > >  {
> > >   if (!(mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
> > >   return 2;
> > >
> > > - return 2 * mlx5_vdpa_max_qps(mvdev->max_vqs);
> > > + return mvdev->max_vqs;
> > >  }
> > >
> > >  static bool is_ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev, u16 idx)
> > > @@ -1236,25 +1232,13 @@ static void teardown_vq(struct mlx5_vdpa_net 
> > > *ndev, struct mlx5_vdpa_virtqueue *
> > >  static int create_rqt(struct mlx5_vdpa_net *ndev)
> > >  {
> > >   __be32 *list;
> > > - int max_rqt;
> > >   void *rqtc;
> > >   int inlen;
> > >   void *in;
> > >   int i, j;
> > >   int err;
> > > - int num;
> > > -
> > > - if (!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
> > > - num = 1;
> > > - else
> > > - num = ndev->cur_num_vqs / 2;
> > >
> > > - max_rqt = min_t(int, roundup_pow_of_two(num),
> > > - 1 << MLX5_CAP_GEN(ndev->mvdev.mdev, log_max_rqt_size));
> > > - if (max_rqt < 1)
> > > - return -EOPNOTSUPP;
> > > -
> > > - inlen = MLX5_ST_SZ_BYTES(create_rqt_in

Re: [PATCH RESEND V2 3/3] vdpa/mlx5: Use consistent RQT size

2022-04-04 Thread Michael S. Tsirkin
On Tue, Mar 29, 2022 at 12:21:09PM +0800, Jason Wang wrote:
> From: Eli Cohen 
> 
> The current code evaluates RQT size based on the configured number of
> virtqueues. This can raise an issue in the following scenario:
> 
> Assume MQ was negotiated.
> 1. mlx5_vdpa_set_map() gets called.
> 2. handle_ctrl_mq() is called setting cur_num_vqs to some value, lower
>than the configured max VQs.
> 3. A second set_map gets called, but now a smaller number of VQs is used
>to evaluate the size of the RQT.
> 4. handle_ctrl_mq() is called with a value larger than what the RQT can
>hold. This will emit errors and the driver state is compromised.
> 
> To fix this, we use a new field in struct mlx5_vdpa_net to hold the
> required number of entries in the RQT. This value is evaluated in
> mlx5_vdpa_set_driver_features() where we have the negotiated features
> all set up.
> 
> In addtion

addition?

> to that, we take into consideration the max capability of RQT
> entries early when the device is added so we don't need to take consider
> it when creating the RQT.
> 
> Last, we remove the use of mlx5_vdpa_max_qps() which just returns the
> max_vas / 2 and make the code clearer.
> 
> Fixes: 52893733f2c5 ("vdpa/mlx5: Add multiqueue support")
> Signed-off-by: Eli Cohen 

Jason I don't have your ack or S.O.B on this one.


> ---
>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 61 +++
>  1 file changed, 21 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 53b8c1a68f90..61bec1ed0bc9 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -161,6 +161,7 @@ struct mlx5_vdpa_net {
>   struct mlx5_flow_handle *rx_rule_mcast;
>   bool setup;
>   u32 cur_num_vqs;
> + u32 rqt_size;
>   struct notifier_block nb;
>   struct vdpa_callback config_cb;
>   struct mlx5_vdpa_wq_ent cvq_ent;
> @@ -204,17 +205,12 @@ static __virtio16 cpu_to_mlx5vdpa16(struct 
> mlx5_vdpa_dev *mvdev, u16 val)
>   return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
>  }
>  
> -static inline u32 mlx5_vdpa_max_qps(int max_vqs)
> -{
> - return max_vqs / 2;
> -}
> -
>  static u16 ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev)
>  {
>   if (!(mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
>   return 2;
>  
> - return 2 * mlx5_vdpa_max_qps(mvdev->max_vqs);
> + return mvdev->max_vqs;
>  }
>  
>  static bool is_ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev, u16 idx)
> @@ -1236,25 +1232,13 @@ static void teardown_vq(struct mlx5_vdpa_net *ndev, 
> struct mlx5_vdpa_virtqueue *
>  static int create_rqt(struct mlx5_vdpa_net *ndev)
>  {
>   __be32 *list;
> - int max_rqt;
>   void *rqtc;
>   int inlen;
>   void *in;
>   int i, j;
>   int err;
> - int num;
> -
> - if (!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
> - num = 1;
> - else
> - num = ndev->cur_num_vqs / 2;
>  
> - max_rqt = min_t(int, roundup_pow_of_two(num),
> - 1 << MLX5_CAP_GEN(ndev->mvdev.mdev, log_max_rqt_size));
> - if (max_rqt < 1)
> - return -EOPNOTSUPP;
> -
> - inlen = MLX5_ST_SZ_BYTES(create_rqt_in) + max_rqt * 
> MLX5_ST_SZ_BYTES(rq_num);
> + inlen = MLX5_ST_SZ_BYTES(create_rqt_in) + ndev->rqt_size * 
> MLX5_ST_SZ_BYTES(rq_num);
>   in = kzalloc(inlen, GFP_KERNEL);
>   if (!in)
>   return -ENOMEM;
> @@ -1263,12 +1247,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, max_rqt);
> + MLX5_SET(rqtc, rqtc, rqt_max_size, ndev->rqt_size);
>   list = MLX5_ADDR_OF(rqtc, rqtc, rq_num[0]);
> - for (i = 0, j = 0; i < max_rqt; i++, j += 2)
> - list[i] = cpu_to_be32(ndev->vqs[j % (2 * num)].virtq_id);
> + for (i = 0, j = 0; i < ndev->rqt_size; i++, j += 2)
> + list[i] = cpu_to_be32(ndev->vqs[j % 
> ndev->cur_num_vqs].virtq_id);
>  
> - MLX5_SET(rqtc, rqtc, rqt_actual_size, max_rqt);
> + MLX5_SET(rqtc, rqtc, rqt_actual_size, ndev->rqt_size);
>   err = mlx5_vdpa_create_rqt(>mvdev, in, inlen, >res.rqtn);
>   kfree(in);
>   if (err)
> @@ -1282,19 +1266,13 @@ static int create_rqt(struct mlx5_vdpa_net *ndev)
>  static int modify_rqt(struct mlx5_vdpa_net *ndev, int num)
>  {
>   __be32 *list;
> - int max_rqt;
>   void *rqtc;
>   int inlen;
>   void *in;
>   int i, j;
>   int err;
>  
> - max_rqt = min_t(int, roundup_pow_of_two(ndev->cur_num_vqs / 2),
> - 1 << MLX5_CAP_GEN(ndev->mvdev.mdev, log_max_rqt_size));
> - if (max_rqt < 1)
> - return -EOPNOTSUPP;
> -
> - inlen = MLX5_ST_SZ_BYTES(modify_rqt_in) + max_rqt * 
> MLX5_ST_SZ_BYTES(rq_num);
> + inlen = 

[PATCH RESEND V2 3/3] vdpa/mlx5: Use consistent RQT size

2022-03-28 Thread Jason Wang
From: Eli Cohen 

The current code evaluates RQT size based on the configured number of
virtqueues. This can raise an issue in the following scenario:

Assume MQ was negotiated.
1. mlx5_vdpa_set_map() gets called.
2. handle_ctrl_mq() is called setting cur_num_vqs to some value, lower
   than the configured max VQs.
3. A second set_map gets called, but now a smaller number of VQs is used
   to evaluate the size of the RQT.
4. handle_ctrl_mq() is called with a value larger than what the RQT can
   hold. This will emit errors and the driver state is compromised.

To fix this, we use a new field in struct mlx5_vdpa_net to hold the
required number of entries in the RQT. This value is evaluated in
mlx5_vdpa_set_driver_features() where we have the negotiated features
all set up.

In addtion to that, we take into consideration the max capability of RQT
entries early when the device is added so we don't need to take consider
it when creating the RQT.

Last, we remove the use of mlx5_vdpa_max_qps() which just returns the
max_vas / 2 and make the code clearer.

Fixes: 52893733f2c5 ("vdpa/mlx5: Add multiqueue support")
Signed-off-by: Eli Cohen 
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 61 +++
 1 file changed, 21 insertions(+), 40 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 53b8c1a68f90..61bec1ed0bc9 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -161,6 +161,7 @@ struct mlx5_vdpa_net {
struct mlx5_flow_handle *rx_rule_mcast;
bool setup;
u32 cur_num_vqs;
+   u32 rqt_size;
struct notifier_block nb;
struct vdpa_callback config_cb;
struct mlx5_vdpa_wq_ent cvq_ent;
@@ -204,17 +205,12 @@ static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev 
*mvdev, u16 val)
return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
 }
 
-static inline u32 mlx5_vdpa_max_qps(int max_vqs)
-{
-   return max_vqs / 2;
-}
-
 static u16 ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev)
 {
if (!(mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
return 2;
 
-   return 2 * mlx5_vdpa_max_qps(mvdev->max_vqs);
+   return mvdev->max_vqs;
 }
 
 static bool is_ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev, u16 idx)
@@ -1236,25 +1232,13 @@ static void teardown_vq(struct mlx5_vdpa_net *ndev, 
struct mlx5_vdpa_virtqueue *
 static int create_rqt(struct mlx5_vdpa_net *ndev)
 {
__be32 *list;
-   int max_rqt;
void *rqtc;
int inlen;
void *in;
int i, j;
int err;
-   int num;
-
-   if (!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
-   num = 1;
-   else
-   num = ndev->cur_num_vqs / 2;
 
-   max_rqt = min_t(int, roundup_pow_of_two(num),
-   1 << MLX5_CAP_GEN(ndev->mvdev.mdev, log_max_rqt_size));
-   if (max_rqt < 1)
-   return -EOPNOTSUPP;
-
-   inlen = MLX5_ST_SZ_BYTES(create_rqt_in) + max_rqt * 
MLX5_ST_SZ_BYTES(rq_num);
+   inlen = MLX5_ST_SZ_BYTES(create_rqt_in) + ndev->rqt_size * 
MLX5_ST_SZ_BYTES(rq_num);
in = kzalloc(inlen, GFP_KERNEL);
if (!in)
return -ENOMEM;
@@ -1263,12 +1247,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, max_rqt);
+   MLX5_SET(rqtc, rqtc, rqt_max_size, ndev->rqt_size);
list = MLX5_ADDR_OF(rqtc, rqtc, rq_num[0]);
-   for (i = 0, j = 0; i < max_rqt; i++, j += 2)
-   list[i] = cpu_to_be32(ndev->vqs[j % (2 * num)].virtq_id);
+   for (i = 0, j = 0; i < ndev->rqt_size; i++, j += 2)
+   list[i] = cpu_to_be32(ndev->vqs[j % 
ndev->cur_num_vqs].virtq_id);
 
-   MLX5_SET(rqtc, rqtc, rqt_actual_size, max_rqt);
+   MLX5_SET(rqtc, rqtc, rqt_actual_size, ndev->rqt_size);
err = mlx5_vdpa_create_rqt(>mvdev, in, inlen, >res.rqtn);
kfree(in);
if (err)
@@ -1282,19 +1266,13 @@ static int create_rqt(struct mlx5_vdpa_net *ndev)
 static int modify_rqt(struct mlx5_vdpa_net *ndev, int num)
 {
__be32 *list;
-   int max_rqt;
void *rqtc;
int inlen;
void *in;
int i, j;
int err;
 
-   max_rqt = min_t(int, roundup_pow_of_two(ndev->cur_num_vqs / 2),
-   1 << MLX5_CAP_GEN(ndev->mvdev.mdev, log_max_rqt_size));
-   if (max_rqt < 1)
-   return -EOPNOTSUPP;
-
-   inlen = MLX5_ST_SZ_BYTES(modify_rqt_in) + max_rqt * 
MLX5_ST_SZ_BYTES(rq_num);
+   inlen = MLX5_ST_SZ_BYTES(modify_rqt_in) + ndev->rqt_size * 
MLX5_ST_SZ_BYTES(rq_num);
in = kzalloc(inlen, GFP_KERNEL);
if (!in)
return -ENOMEM;
@@ -1305,10 +1283,10 @@ static int modify_rqt(struct mlx5_vdpa_net *ndev, int 
num)