From: Yishai Hadas <yishaih@nvidia.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: <kvm@vger.kernel.org>
Subject: Re: [bug report] vfio/mlx5: Let firmware knows upon leaving PRE_COPY back to RUNNING
Date: Thu, 9 May 2024 17:04:43 +0300 [thread overview]
Message-ID: <cff7ba09-2437-45f5-93ee-e5d941e550f7@nvidia.com> (raw)
In-Reply-To: <3412835f-4927-4c9a-830d-4029fa0dc7e0@moroto.mountain>
On 09/05/2024 16:36, Dan Carpenter wrote:
> Hello Yishai Hadas,
>
> Commit 6de042240b0f ("vfio/mlx5: Let firmware knows upon leaving
> PRE_COPY back to RUNNING") from Feb 5, 2024 (linux-next), leads to
> the following Smatch static checker warning:
>
> drivers/vfio/pci/mlx5/main.c:1164 mlx5vf_pci_step_device_state_locked()
> error: uninitialized symbol 'state'.
>
> drivers/vfio/pci/mlx5/main.c
> 1142 if ((cur == VFIO_DEVICE_STATE_PRE_COPY && new == VFIO_DEVICE_STATE_RUNNING) ||
> 1143 (cur == VFIO_DEVICE_STATE_PRE_COPY_P2P &&
> 1144 new == VFIO_DEVICE_STATE_RUNNING_P2P)) {
> 1145 struct mlx5_vf_migration_file *migf = mvdev->saving_migf;
> 1146 struct mlx5_vhca_data_buffer *buf;
> 1147 enum mlx5_vf_migf_state state;
> ^^^^^
> 1148 size_t size;
> 1149
> 1150 ret = mlx5vf_cmd_query_vhca_migration_state(mvdev, &size, NULL,
> 1151 MLX5VF_QUERY_INC | MLX5VF_QUERY_CLEANUP);
> 1152 if (ret)
> 1153 return ERR_PTR(ret);
> 1154 buf = mlx5vf_get_data_buffer(migf, size, DMA_FROM_DEVICE);
> 1155 if (IS_ERR(buf))
> 1156 return ERR_CAST(buf);
> 1157 /* pre_copy cleanup */
> 1158 ret = mlx5vf_cmd_save_vhca_state(mvdev, migf, buf, false, false);
> 1159 if (ret) {
> 1160 mlx5vf_put_data_buffer(buf);
> 1161 return ERR_PTR(ret);
> 1162 }
> 1163 mlx5vf_disable_fds(mvdev, &state);
> ^^^^^^
> state is only set some of the time.
The 'state' will *always* be set in the above flow.
As we are in the source side of the migration we have a valid
saving_migf (see line 1145 above), as we pass in a non NULL pointer for
the state, it will be always filled inside.
We not just make mlx5vf_disable_fds()
> return an error code?
mlx5vf_disable_fd() is a cleanup function that can't fail.
It just holds/sets the state of the migf following the completion of the
asynchronous SAVE command that was issued in line 1158.
So, it's a false alarm.
Thanks,
Yishai
>
> --> 1164 return (state != MLX5_MIGF_STATE_ERROR) ? NULL : ERR_PTR(-EIO);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Uninitialized.
>
> 1165 }
>
> regards,
> dan carpenter
next prev parent reply other threads:[~2024-05-09 14:05 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-09 13:36 [bug report] vfio/mlx5: Let firmware knows upon leaving PRE_COPY back to RUNNING Dan Carpenter
2024-05-09 14:04 ` Yishai Hadas [this message]
2024-05-09 14:43 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cff7ba09-2437-45f5-93ee-e5d941e550f7@nvidia.com \
--to=yishaih@nvidia.com \
--cc=dan.carpenter@linaro.org \
--cc=kvm@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.