Hi,
Thanks for your patch and sorry about the late reply.
On 16.10.26 05:58, Yuho Choi wrote:
> qp_destroy() only warns when DESTROY_QP fails and then frees the RQ
> buffer and doorbell record of the driver-owned QP anyway. The QP still
> exists in firmware, so the device can keep reading WQEs and the doorbell
> record from memory that has been returned to the allocator. This is
> reachable from teardown_vq(), which destroys the QPs even when
> destroy_virtqueue() fails and the virtqueue still uses them.
>
> Return after the warning, as cq_destroy() does, and leak the buffers
> instead. A device in internal error is not affected, since the mlx5 core
> reports success for DESTROY_QP in that state.
>
This commit message is long and hard to read. Could you make it
shorter and more readable please?
> Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
> Cc: [email protected]
Nothing against the change. But it does not deserve a fixes tag.
> Signed-off-by: Yuho Choi <[email protected]>
> ---
> No code changes, only added Cc: stable. Resending as the original posting
> got no comments:
> https://lore.kernel.org/all/[email protected]/
>
> This was found with a static analysis tool from ongoing, not yet
> published research, and confirmed by code inspection. It has not been
> reproduced on hardware.
>
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 8563fec2855d..a01cd890bb58 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -483,8 +483,11 @@ static void qp_destroy(struct mlx5_vdpa_net *ndev,
> struct mlx5_vdpa_qp *vqp)
> MLX5_SET(destroy_qp_in, in, opcode, MLX5_CMD_OP_DESTROY_QP);
> MLX5_SET(destroy_qp_in, in, qpn, vqp->mqp.qpn);
> MLX5_SET(destroy_qp_in, in, uid, ndev->mvdev.res.uid);
> - if (mlx5_cmd_exec_in(ndev->mvdev.mdev, destroy_qp, in))
> + if (mlx5_cmd_exec_in(ndev->mvdev.mdev, destroy_qp, in)) {
> + /* The QP still owns its buffers */
> mlx5_vdpa_warn(&ndev->mvdev, "destroy qp 0x%x\n", vqp->mqp.qpn);
While here, could you improve the error message please?
mlx5_vdpa_warn(..., "destroy %s qp 0x%x failed %s\n", vqp->fw ? "fw" : "sw");
Thanks,
Dragos
