| 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
| ||
|
Message-ID: <CAJaqyWeQDgBAp=uAEy0XO1orCb0z+w-CidBnRORG+CjY2fVK0g@mail.gmail.com>
Date: Wed, 7 Aug 2024 15:12:51 +0200
From: Eugenio Perez Martin <eperezma@...hat.com>
To: Dragos Tatulea <dtatulea@...dia.com>
Cc: "Michael S. Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>,
Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Si-Wei Liu <si-wei.liu@...cle.com>,
Tariq Toukan <tariqt@...dia.com>, virtualization@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH vhost 4/7] vdpa/mlx5: Use async API for vq modify commands
On Fri, Aug 2, 2024 at 9:24 AM Dragos Tatulea <dtatulea@...dia.com> wrote:
>
> Switch firmware vq modify command to be issued via the async API to
> allow future parallelization. The new refactored function applies the
> modify on a range of vqs and waits for their execution to complete.
>
> For now the command is still used in a serial fashion. A later patch
> will switch to modifying multiple vqs in parallel.
>
> Signed-off-by: Dragos Tatulea <dtatulea@...dia.com>
> Reviewed-by: Tariq Toukan <tariqt@...dia.com>
> ---
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 150 ++++++++++++++++++++----------
> 1 file changed, 103 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index be8df9d9f4df..e56a0ee1b725 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1189,6 +1189,12 @@ struct mlx5_virtqueue_query_mem {
> u8 out[MLX5_ST_SZ_BYTES(query_virtio_net_q_out)];
> };
>
> +struct mlx5_virtqueue_modify_mem {
> + u8 in[MLX5_ST_SZ_BYTES(modify_virtio_net_q_in)];
> + u8 out[MLX5_ST_SZ_BYTES(modify_virtio_net_q_out)];
> +};
> +
> +
Extra newline here.
> struct mlx5_vdpa_async_virtqueue_cmd {
> int err;
> struct mlx5_async_work cb_work;
> @@ -1202,6 +1208,7 @@ struct mlx5_vdpa_async_virtqueue_cmd {
>
> union {
> struct mlx5_virtqueue_query_mem query;
> + struct mlx5_virtqueue_modify_mem modify;
> };
> };
>
> @@ -1384,51 +1391,35 @@ static bool modifiable_virtqueue_fields(struct mlx5_vdpa_virtqueue *mvq)
> return true;
> }
>
> -static int modify_virtqueue(struct mlx5_vdpa_net *ndev,
> - struct mlx5_vdpa_virtqueue *mvq,
> - int state)
> +static void fill_modify_virtqueue_cmd(struct mlx5_vdpa_net *ndev,
> + struct mlx5_vdpa_virtqueue *mvq,
> + int state,
> + struct mlx5_vdpa_async_virtqueue_cmd *cmd)
> {
> - int inlen = MLX5_ST_SZ_BYTES(modify_virtio_net_q_in);
> - u32 out[MLX5_ST_SZ_DW(modify_virtio_net_q_out)] = {};
> struct mlx5_vdpa_dev *mvdev = &ndev->mvdev;
> struct mlx5_vdpa_mr *desc_mr = NULL;
> struct mlx5_vdpa_mr *vq_mr = NULL;
> - bool state_change = false;
> void *obj_context;
> void *cmd_hdr;
> void *vq_ctx;
> - void *in;
> - int err;
>
> - if (mvq->fw_state == MLX5_VIRTIO_NET_Q_OBJECT_NONE)
> - return 0;
> -
> - if (!modifiable_virtqueue_fields(mvq))
> - return -EINVAL;
> -
> - in = kzalloc(inlen, GFP_KERNEL);
> - if (!in)
> - return -ENOMEM;
> + cmd->in = &cmd->modify.in;
> + cmd->inlen = sizeof(cmd->modify.in);
> + cmd->out = &cmd->modify.out;
> + cmd->outlen = sizeof(cmd->modify.out);
>
> - cmd_hdr = MLX5_ADDR_OF(modify_virtio_net_q_in, in, general_obj_in_cmd_hdr);
> + cmd_hdr = MLX5_ADDR_OF(modify_virtio_net_q_in, cmd->in, general_obj_in_cmd_hdr);
>
> MLX5_SET(general_obj_in_cmd_hdr, cmd_hdr, opcode, MLX5_CMD_OP_MODIFY_GENERAL_OBJECT);
> MLX5_SET(general_obj_in_cmd_hdr, cmd_hdr, obj_type, MLX5_OBJ_TYPE_VIRTIO_NET_Q);
> MLX5_SET(general_obj_in_cmd_hdr, cmd_hdr, obj_id, mvq->virtq_id);
> MLX5_SET(general_obj_in_cmd_hdr, cmd_hdr, uid, ndev->mvdev.res.uid);
>
> - obj_context = MLX5_ADDR_OF(modify_virtio_net_q_in, in, obj_context);
> + obj_context = MLX5_ADDR_OF(modify_virtio_net_q_in, cmd->in, obj_context);
> vq_ctx = MLX5_ADDR_OF(virtio_net_q_object, obj_context, virtio_q_context);
>
> - if (mvq->modified_fields & MLX5_VIRTQ_MODIFY_MASK_STATE) {
> - if (!is_valid_state_change(mvq->fw_state, state, is_resumable(ndev))) {
> - err = -EINVAL;
> - goto done;
> - }
> -
> + if (mvq->modified_fields & MLX5_VIRTQ_MODIFY_MASK_STATE)
> MLX5_SET(virtio_net_q_object, obj_context, state, state);
> - state_change = true;
> - }
>
> if (mvq->modified_fields & MLX5_VIRTQ_MODIFY_MASK_VIRTIO_Q_ADDRS) {
> MLX5_SET64(virtio_q, vq_ctx, desc_addr, mvq->desc_addr);
> @@ -1474,38 +1465,36 @@ static int modify_virtqueue(struct mlx5_vdpa_net *ndev,
> }
>
> MLX5_SET64(virtio_net_q_object, obj_context, modify_field_select, mvq->modified_fields);
> - err = mlx5_cmd_exec(ndev->mvdev.mdev, in, inlen, out, sizeof(out));
> - if (err)
> - goto done;
> +}
>
> - if (state_change)
> - mvq->fw_state = state;
> +static void modify_virtqueue_end(struct mlx5_vdpa_net *ndev,
> + struct mlx5_vdpa_virtqueue *mvq,
> + int state)
> +{
> + struct mlx5_vdpa_dev *mvdev = &ndev->mvdev;
>
> if (mvq->modified_fields & MLX5_VIRTQ_MODIFY_MASK_VIRTIO_Q_MKEY) {
> + unsigned int asid = mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP];
> + struct mlx5_vdpa_mr *vq_mr = mvdev->mr[asid];
> +
> mlx5_vdpa_put_mr(mvdev, mvq->vq_mr);
> mlx5_vdpa_get_mr(mvdev, vq_mr);
> mvq->vq_mr = vq_mr;
> }
>
> if (mvq->modified_fields & MLX5_VIRTQ_MODIFY_MASK_DESC_GROUP_MKEY) {
> + unsigned int asid = mvdev->group2asid[MLX5_VDPA_DATAVQ_DESC_GROUP];
> + struct mlx5_vdpa_mr *desc_mr = mvdev->mr[asid];
> +
> mlx5_vdpa_put_mr(mvdev, mvq->desc_mr);
> mlx5_vdpa_get_mr(mvdev, desc_mr);
> mvq->desc_mr = desc_mr;
> }
>
> - mvq->modified_fields = 0;
> -
> -done:
> - kfree(in);
> - return err;
> -}
> + if (mvq->modified_fields & MLX5_VIRTQ_MODIFY_MASK_STATE)
> + mvq->fw_state = state;
>
> -static int modify_virtqueue_state(struct mlx5_vdpa_net *ndev,
> - struct mlx5_vdpa_virtqueue *mvq,
> - unsigned int state)
> -{
> - mvq->modified_fields |= MLX5_VIRTQ_MODIFY_MASK_STATE;
> - return modify_virtqueue(ndev, mvq, state);
> + mvq->modified_fields = 0;
> }
>
> static int counter_set_alloc(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *mvq)
> @@ -1658,6 +1647,73 @@ static int setup_vq(struct mlx5_vdpa_net *ndev,
> return err;
> }
>
> +static int modify_virtqueues(struct mlx5_vdpa_net *ndev, int start_vq, int num_vqs, int state)
> +{
> + struct mlx5_vdpa_async_virtqueue_cmd *cmds;
> + struct mlx5_vdpa_dev *mvdev = &ndev->mvdev;
> + int err = 0;
> +
> + WARN(start_vq + num_vqs > mvdev->max_vqs, "modify vq range invalid [%d, %d), max_vqs: %u\n",
> + start_vq, start_vq + num_vqs, mvdev->max_vqs);
> +
> + cmds = kvcalloc(num_vqs, sizeof(*cmds), GFP_KERNEL);
> + if (!cmds)
> + return -ENOMEM;
> +
> + for (int i = 0; i < num_vqs; i++) {
> + struct mlx5_vdpa_async_virtqueue_cmd *cmd = &cmds[i];
> + struct mlx5_vdpa_virtqueue *mvq;
> + int vq_idx = start_vq + i;
> +
> + mvq = &ndev->vqs[vq_idx];
> +
> + if (!modifiable_virtqueue_fields(mvq)) {
> + err = -EINVAL;
> + goto done;
> + }
> +
> + if (mvq->fw_state != state) {
> + if (!is_valid_state_change(mvq->fw_state, state, is_resumable(ndev))) {
> + err = -EINVAL;
> + goto done;
> + }
> +
> + mvq->modified_fields |= MLX5_VIRTQ_MODIFY_MASK_STATE;
> + }
> +
> + fill_modify_virtqueue_cmd(ndev, mvq, state, cmd);
> + }
> +
> + err = exec_virtqueue_async_cmds(ndev, cmds, num_vqs);
> + if (err) {
> + mlx5_vdpa_err(mvdev, "error issuing modify cmd for vq range [%d, %d)\n",
> + start_vq, start_vq + num_vqs);
> + goto done;
> + }
> +
> + for (int i = 0; i < num_vqs; i++) {
> + struct mlx5_vdpa_async_virtqueue_cmd *cmd = &cmds[i];
> + struct mlx5_vdpa_virtqueue *mvq;
> + int vq_idx = start_vq + i;
> +
> + mvq = &ndev->vqs[vq_idx];
> +
> + if (cmd->err) {
> + mlx5_vdpa_err(mvdev, "modify vq %d failed, state: %d -> %d, err: %d\n",
> + vq_idx, mvq->fw_state, state, err);
> + if (!err)
> + err = cmd->err;
> + continue;
> + }
> +
> + modify_virtqueue_end(ndev, mvq, state);
> + }
> +
> +done:
> + kfree(cmds);
> + return err;
> +}
> +
> static int suspend_vq(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *mvq)
> {
> struct mlx5_virtq_attr attr;
> @@ -1669,7 +1725,7 @@ static int suspend_vq(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *mv
> if (mvq->fw_state != MLX5_VIRTIO_NET_Q_OBJECT_STATE_RDY)
> return 0;
>
> - err = modify_virtqueue_state(ndev, mvq, MLX5_VIRTIO_NET_Q_OBJECT_STATE_SUSPEND);
> + err = modify_virtqueues(ndev, mvq->index, 1, MLX5_VIRTIO_NET_Q_OBJECT_STATE_SUSPEND);
> if (err) {
> mlx5_vdpa_err(&ndev->mvdev, "modify to suspend failed, err: %d\n", err);
> return err;
> @@ -1716,7 +1772,7 @@ static int resume_vq(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *mvq
> /* Due to a FW quirk we need to modify the VQ fields first then change state.
> * This should be fixed soon. After that, a single command can be used.
> */
> - err = modify_virtqueue(ndev, mvq, 0);
> + err = modify_virtqueues(ndev, mvq->index, 1, mvq->fw_state);
> if (err) {
> mlx5_vdpa_err(&ndev->mvdev,
> "modify vq properties failed for vq %u, err: %d\n",
> @@ -1738,7 +1794,7 @@ static int resume_vq(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *mvq
> return -EINVAL;
> }
>
> - err = modify_virtqueue_state(ndev, mvq, MLX5_VIRTIO_NET_Q_OBJECT_STATE_RDY);
> + err = modify_virtqueues(ndev, mvq->index, 1, MLX5_VIRTIO_NET_Q_OBJECT_STATE_RDY);
> if (err)
> mlx5_vdpa_err(&ndev->mvdev, "modify to resume failed for vq %u, err: %d\n",
> mvq->index, err);
> --
> 2.45.2
>
Powered by blists - more mailing lists