Dragos Tatulea
2023-Aug-31 15:50 UTC
[PATCH v2] vdpa/mlx5: Fix firmware error on creation of 1k VQs
A firmware error is triggered when configuring a 9k MTU on the PF after
switching to switchdev mode and then using a vdpa device with larger
(1k) rings:
mlx5_cmd_out_err: CREATE_GENERAL_OBJECT(0xa00) op_mod(0xd) failed, status bad
resource(0x5), syndrome (0xf6db90), err(-22)
This is due to the fact that the hw VQ size parameters are computed
based on the umem_1/2/3_buffer_param_a/b capabilities and all
device capabilities are read only when the driver is moved to switchdev mode.
The problematic configuration flow looks like this:
1) Create VF
2) Unbind VF
3) Switch PF to switchdev mode.
4) Bind VF
5) Set PF MTU to 9k
6) create vDPA device
7) Start VM with vDPA device and 1K queue size
Note that setting the MTU before step 3) doesn't trigger this issue.
This patch reads the forementioned umem parameters at the latest point
possible before the VQs of the device are created.
v2:
- Allocate output with kmalloc to reduce stack frame size.
- Removed stable from cc.
Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5
devices")
Signed-off-by: Dragos Tatulea <dtatulea at nvidia.com>
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 63 ++++++++++++++++++++++++++-----
drivers/vdpa/mlx5/net/mlx5_vnet.h | 9 +++++
2 files changed, 63 insertions(+), 9 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 40a03b08d7cf..ef5907b1d513 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -625,30 +625,70 @@ static void cq_destroy(struct mlx5_vdpa_net *ndev, u16
idx)
mlx5_db_free(ndev->mvdev.mdev, &vcq->db);
}
+static int read_umem_params(struct mlx5_vdpa_net *ndev)
+{
+ u32 in[MLX5_ST_SZ_DW(query_hca_cap_in)] = {};
+ u16 opmod = (MLX5_CAP_VDPA_EMULATION << 1) | (HCA_CAP_OPMOD_GET_CUR
& 0x01);
+ struct mlx5_core_dev *mdev = ndev->mvdev.mdev;
+ int out_size;
+ void *caps;
+ void *out;
+ int err;
+
+ out_size = MLX5_ST_SZ_BYTES(query_hca_cap_out);
+ out = kzalloc(out_size, GFP_KERNEL);
+ if (!out)
+ return -ENOMEM;
+
+ MLX5_SET(query_hca_cap_in, in, opcode, MLX5_CMD_OP_QUERY_HCA_CAP);
+ MLX5_SET(query_hca_cap_in, in, op_mod, opmod);
+ err = mlx5_cmd_exec_inout(mdev, query_hca_cap, in, out);
+ if (err) {
+ mlx5_vdpa_warn(&ndev->mvdev,
+ "Failed reading vdpa umem capabilities with err %d\n", err);
+ goto out;
+ }
+
+ caps = MLX5_ADDR_OF(query_hca_cap_out, out, capability);
+
+ ndev->umem_1_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps,
umem_1_buffer_param_a);
+ ndev->umem_1_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps,
umem_1_buffer_param_b);
+
+ ndev->umem_2_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps,
umem_2_buffer_param_a);
+ ndev->umem_2_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps,
umem_2_buffer_param_b);
+
+ ndev->umem_3_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps,
umem_3_buffer_param_a);
+ ndev->umem_3_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps,
umem_3_buffer_param_b);
+
+out:
+ kfree(out);
+ return 0;
+}
+
static void set_umem_size(struct mlx5_vdpa_net *ndev, struct
mlx5_vdpa_virtqueue *mvq, int num,
struct mlx5_vdpa_umem **umemp)
{
- struct mlx5_core_dev *mdev = ndev->mvdev.mdev;
- int p_a;
- int p_b;
+ u32 p_a;
+ u32 p_b;
switch (num) {
case 1:
- p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_1_buffer_param_a);
- p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_1_buffer_param_b);
+ p_a = ndev->umem_1_buffer_param_a;
+ p_b = ndev->umem_1_buffer_param_b;
*umemp = &mvq->umem1;
break;
case 2:
- p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_2_buffer_param_a);
- p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_2_buffer_param_b);
+ p_a = ndev->umem_2_buffer_param_a;
+ p_b = ndev->umem_2_buffer_param_b;
*umemp = &mvq->umem2;
break;
case 3:
- p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_3_buffer_param_a);
- p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_3_buffer_param_b);
+ p_a = ndev->umem_3_buffer_param_a;
+ p_b = ndev->umem_3_buffer_param_b;
*umemp = &mvq->umem3;
break;
}
+
(*umemp)->size = p_a * mvq->num_ent + p_b;
}
@@ -2679,6 +2719,11 @@ static int setup_driver(struct mlx5_vdpa_dev *mvdev)
goto out;
}
mlx5_vdpa_add_debugfs(ndev);
+
+ err = read_umem_params(ndev);
+ if (err)
+ goto err_setup;
+
err = setup_virtqueues(mvdev);
if (err) {
mlx5_vdpa_warn(mvdev, "setup_virtqueues\n");
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.h
b/drivers/vdpa/mlx5/net/mlx5_vnet.h
index 36c44d9fdd16..65ebbba20662 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.h
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.h
@@ -65,6 +65,15 @@ struct mlx5_vdpa_net {
struct hlist_head macvlan_hash[MLX5V_MACVLAN_SIZE];
struct mlx5_vdpa_irq_pool irqp;
struct dentry *debugfs;
+
+ u32 umem_1_buffer_param_a;
+ u32 umem_1_buffer_param_b;
+
+ u32 umem_2_buffer_param_a;
+ u32 umem_2_buffer_param_b;
+
+ u32 umem_3_buffer_param_a;
+ u32 umem_3_buffer_param_b;
};
struct mlx5_vdpa_counter {
--
2.41.0
Dragos Tatulea
2023-Sep-14 09:40 UTC
[PATCH v2] vdpa/mlx5: Fix firmware error on creation of 1k VQs
On Thu, 2023-08-31 at 18:50 +0300, Dragos Tatulea wrote:> A firmware error is triggered when configuring a 9k MTU on the PF after > switching to switchdev mode and then using a vdpa device with larger > (1k) rings: > mlx5_cmd_out_err: CREATE_GENERAL_OBJECT(0xa00) op_mod(0xd) failed, status bad > resource(0x5), syndrome (0xf6db90), err(-22) > > This is due to the fact that the hw VQ size parameters are computed > based on the umem_1/2/3_buffer_param_a/b capabilities and all > device capabilities are read only when the driver is moved to switchdev mode. > > The problematic configuration flow looks like this: > 1) Create VF > 2) Unbind VF > 3) Switch PF to switchdev mode. > 4) Bind VF > 5) Set PF MTU to 9k > 6) create vDPA device > 7) Start VM with vDPA device and 1K queue size > > Note that setting the MTU before step 3) doesn't trigger this issue. > > This patch reads the forementioned umem parameters at the latest point > possible before the VQs of the device are created. > > v2: > - Allocate output with kmalloc to reduce stack frame size. > - Removed stable from cc. >Gentle ping. Any thoughts on this fix? Thanks, Dragos> Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices") > Signed-off-by: Dragos Tatulea <dtatulea at nvidia.com> > > --- > ?drivers/vdpa/mlx5/net/mlx5_vnet.c | 63 ++++++++++++++++++++++++++----- > ?drivers/vdpa/mlx5/net/mlx5_vnet.h |? 9 +++++ > ?2 files changed, 63 insertions(+), 9 deletions(-) > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c > b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index 40a03b08d7cf..ef5907b1d513 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -625,30 +625,70 @@ static void cq_destroy(struct mlx5_vdpa_net *ndev, u16 > idx) > ????????mlx5_db_free(ndev->mvdev.mdev, &vcq->db); > ?} > ? > +static int read_umem_params(struct mlx5_vdpa_net *ndev) > +{ > +???????u32 in[MLX5_ST_SZ_DW(query_hca_cap_in)] = {}; > +???????u16 opmod = (MLX5_CAP_VDPA_EMULATION << 1) | (HCA_CAP_OPMOD_GET_CUR & > 0x01); > +???????struct mlx5_core_dev *mdev = ndev->mvdev.mdev; > +???????int out_size; > +???????void *caps; > +???????void *out; > +???????int err; > + > +???????out_size = MLX5_ST_SZ_BYTES(query_hca_cap_out); > +???????out = kzalloc(out_size, GFP_KERNEL); > +???????if (!out) > +???????????????return -ENOMEM; > + > +???????MLX5_SET(query_hca_cap_in, in, opcode, MLX5_CMD_OP_QUERY_HCA_CAP); > +???????MLX5_SET(query_hca_cap_in, in, op_mod, opmod); > +???????err = mlx5_cmd_exec_inout(mdev, query_hca_cap, in, out); > +???????if (err) { > +???????????????mlx5_vdpa_warn(&ndev->mvdev, > +???????????????????????"Failed reading vdpa umem capabilities with err %d\n", > err); > +???????????????goto out; > +???????} > + > +???????caps =? MLX5_ADDR_OF(query_hca_cap_out, out, capability); > + > +???????ndev->umem_1_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps, > umem_1_buffer_param_a); > +???????ndev->umem_1_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps, > umem_1_buffer_param_b); > + > +???????ndev->umem_2_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps, > umem_2_buffer_param_a); > +???????ndev->umem_2_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps, > umem_2_buffer_param_b); > + > +???????ndev->umem_3_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps, > umem_3_buffer_param_a); > +???????ndev->umem_3_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps, > umem_3_buffer_param_b); > + > +out: > +???????kfree(out); > +???????return 0; > +} > + > ?static void set_umem_size(struct mlx5_vdpa_net *ndev, struct > mlx5_vdpa_virtqueue *mvq, int num, > ????????????????????????? struct mlx5_vdpa_umem **umemp) > ?{ > -???????struct mlx5_core_dev *mdev = ndev->mvdev.mdev; > -???????int p_a; > -???????int p_b; > +???????u32 p_a; > +???????u32 p_b; > ? > ????????switch (num) { > ????????case 1: > -???????????????p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev, > umem_1_buffer_param_a); > -???????????????p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev, > umem_1_buffer_param_b); > +???????????????p_a = ndev->umem_1_buffer_param_a; > +???????????????p_b = ndev->umem_1_buffer_param_b; > ????????????????*umemp = &mvq->umem1; > ????????????????break; > ????????case 2: > -???????????????p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev, > umem_2_buffer_param_a); > -???????????????p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev, > umem_2_buffer_param_b); > +???????????????p_a = ndev->umem_2_buffer_param_a; > +???????????????p_b = ndev->umem_2_buffer_param_b; > ????????????????*umemp = &mvq->umem2; > ????????????????break; > ????????case 3: > -???????????????p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev, > umem_3_buffer_param_a); > -???????????????p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev, > umem_3_buffer_param_b); > +???????????????p_a = ndev->umem_3_buffer_param_a; > +???????????????p_b = ndev->umem_3_buffer_param_b; > ????????????????*umemp = &mvq->umem3; > ????????????????break; > ????????} > + > ????????(*umemp)->size = p_a * mvq->num_ent + p_b; > ?} > ? > @@ -2679,6 +2719,11 @@ static int setup_driver(struct mlx5_vdpa_dev *mvdev) > ????????????????goto out; > ????????} > ????????mlx5_vdpa_add_debugfs(ndev); > + > +???????err = read_umem_params(ndev); > +???????if (err) > +???????????????goto err_setup; > + > ????????err = setup_virtqueues(mvdev); > ????????if (err) { > ????????????????mlx5_vdpa_warn(mvdev, "setup_virtqueues\n"); > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.h > b/drivers/vdpa/mlx5/net/mlx5_vnet.h > index 36c44d9fdd16..65ebbba20662 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.h > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.h > @@ -65,6 +65,15 @@ struct mlx5_vdpa_net { > ????????struct hlist_head macvlan_hash[MLX5V_MACVLAN_SIZE]; > ????????struct mlx5_vdpa_irq_pool irqp; > ????????struct dentry *debugfs; > + > +???????u32 umem_1_buffer_param_a; > +???????u32 umem_1_buffer_param_b; > + > +???????u32 umem_2_buffer_param_a; > +???????u32 umem_2_buffer_param_b; > + > +???????u32 umem_3_buffer_param_a; > +???????u32 umem_3_buffer_param_b; > ?}; > ? > ?struct mlx5_vdpa_counter {
Jason Wang
2023-Sep-15 06:54 UTC
[PATCH v2] vdpa/mlx5: Fix firmware error on creation of 1k VQs
On Thu, Aug 31, 2023 at 11:58?PM Dragos Tatulea <dtatulea at nvidia.com> wrote:> > A firmware error is triggered when configuring a 9k MTU on the PF after > switching to switchdev mode and then using a vdpa device with larger > (1k) rings: > mlx5_cmd_out_err: CREATE_GENERAL_OBJECT(0xa00) op_mod(0xd) failed, status bad resource(0x5), syndrome (0xf6db90), err(-22) > > This is due to the fact that the hw VQ size parameters are computed > based on the umem_1/2/3_buffer_param_a/b capabilities and all > device capabilities are read only when the driver is moved to switchdev mode. > > The problematic configuration flow looks like this: > 1) Create VF > 2) Unbind VF > 3) Switch PF to switchdev mode. > 4) Bind VF > 5) Set PF MTU to 9k > 6) create vDPA device > 7) Start VM with vDPA device and 1K queue size > > Note that setting the MTU before step 3) doesn't trigger this issue. > > This patch reads the forementioned umem parameters at the latest point > possible before the VQs of the device are created. > > v2: > - Allocate output with kmalloc to reduce stack frame size. > - Removed stable from cc. > > Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices") > Signed-off-by: Dragos Tatulea <dtatulea at nvidia.com>Acked-by: Jason Wang <jasowang at redhat.com> Thanks> --- > drivers/vdpa/mlx5/net/mlx5_vnet.c | 63 ++++++++++++++++++++++++++----- > drivers/vdpa/mlx5/net/mlx5_vnet.h | 9 +++++ > 2 files changed, 63 insertions(+), 9 deletions(-) > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index 40a03b08d7cf..ef5907b1d513 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -625,30 +625,70 @@ static void cq_destroy(struct mlx5_vdpa_net *ndev, u16 idx) > mlx5_db_free(ndev->mvdev.mdev, &vcq->db); > } > > +static int read_umem_params(struct mlx5_vdpa_net *ndev) > +{ > + u32 in[MLX5_ST_SZ_DW(query_hca_cap_in)] = {}; > + u16 opmod = (MLX5_CAP_VDPA_EMULATION << 1) | (HCA_CAP_OPMOD_GET_CUR & 0x01); > + struct mlx5_core_dev *mdev = ndev->mvdev.mdev; > + int out_size; > + void *caps; > + void *out; > + int err; > + > + out_size = MLX5_ST_SZ_BYTES(query_hca_cap_out); > + out = kzalloc(out_size, GFP_KERNEL); > + if (!out) > + return -ENOMEM; > + > + MLX5_SET(query_hca_cap_in, in, opcode, MLX5_CMD_OP_QUERY_HCA_CAP); > + MLX5_SET(query_hca_cap_in, in, op_mod, opmod); > + err = mlx5_cmd_exec_inout(mdev, query_hca_cap, in, out); > + if (err) { > + mlx5_vdpa_warn(&ndev->mvdev, > + "Failed reading vdpa umem capabilities with err %d\n", err); > + goto out; > + } > + > + caps = MLX5_ADDR_OF(query_hca_cap_out, out, capability); > + > + ndev->umem_1_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps, umem_1_buffer_param_a); > + ndev->umem_1_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps, umem_1_buffer_param_b); > + > + ndev->umem_2_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps, umem_2_buffer_param_a); > + ndev->umem_2_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps, umem_2_buffer_param_b); > + > + ndev->umem_3_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps, umem_3_buffer_param_a); > + ndev->umem_3_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps, umem_3_buffer_param_b); > + > +out: > + kfree(out); > + return 0; > +} > + > static void set_umem_size(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *mvq, int num, > struct mlx5_vdpa_umem **umemp) > { > - struct mlx5_core_dev *mdev = ndev->mvdev.mdev; > - int p_a; > - int p_b; > + u32 p_a; > + u32 p_b; > > switch (num) { > case 1: > - p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_1_buffer_param_a); > - p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_1_buffer_param_b); > + p_a = ndev->umem_1_buffer_param_a; > + p_b = ndev->umem_1_buffer_param_b; > *umemp = &mvq->umem1; > break; > case 2: > - p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_2_buffer_param_a); > - p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_2_buffer_param_b); > + p_a = ndev->umem_2_buffer_param_a; > + p_b = ndev->umem_2_buffer_param_b; > *umemp = &mvq->umem2; > break; > case 3: > - p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_3_buffer_param_a); > - p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev, umem_3_buffer_param_b); > + p_a = ndev->umem_3_buffer_param_a; > + p_b = ndev->umem_3_buffer_param_b; > *umemp = &mvq->umem3; > break; > } > + > (*umemp)->size = p_a * mvq->num_ent + p_b; > } > > @@ -2679,6 +2719,11 @@ static int setup_driver(struct mlx5_vdpa_dev *mvdev) > goto out; > } > mlx5_vdpa_add_debugfs(ndev); > + > + err = read_umem_params(ndev); > + if (err) > + goto err_setup; > + > err = setup_virtqueues(mvdev); > if (err) { > mlx5_vdpa_warn(mvdev, "setup_virtqueues\n"); > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.h b/drivers/vdpa/mlx5/net/mlx5_vnet.h > index 36c44d9fdd16..65ebbba20662 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.h > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.h > @@ -65,6 +65,15 @@ struct mlx5_vdpa_net { > struct hlist_head macvlan_hash[MLX5V_MACVLAN_SIZE]; > struct mlx5_vdpa_irq_pool irqp; > struct dentry *debugfs; > + > + u32 umem_1_buffer_param_a; > + u32 umem_1_buffer_param_b; > + > + u32 umem_2_buffer_param_a; > + u32 umem_2_buffer_param_b; > + > + u32 umem_3_buffer_param_a; > + u32 umem_3_buffer_param_b; > }; > > struct mlx5_vdpa_counter { > -- > 2.41.0 >