On 4/3/2023 12:20 PM, Eli Cohen wrote:> Add support for generation of interrupts from the device directly to the
> VM to the VCPU thus avoiding the overhead on the host CPU.
>
> When supported, the driver will attempt to allocate vectors for each
> data virtqueue. If a vector for a virtqueue cannot be provided it will
> use the QP mode where notifications go through the driver.
>
> In addition, we add a shutdown callback to make sure allocated
> interrupts are released in case of shutdown to allow clean shutdown.
>
> Signed-off-by: Eli Cohen <elic at nvidia.com>
> Signed-off-by: Saeed Mahameed <saeedm at nvidia.com>
> ---
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 139 ++++++++++++++++++++++++++++--
> drivers/vdpa/mlx5/net/mlx5_vnet.h | 14 +++
> 2 files changed, 144 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 520646ae7fa0..215a46cf8a98 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -83,6 +83,7 @@ struct mlx5_vq_restore_info {
> u64 driver_addr;
> u16 avail_index;
> u16 used_index;
> + struct msi_map map;
> bool ready;
> bool restore;
> };
> @@ -118,6 +119,7 @@ struct mlx5_vdpa_virtqueue {
> u16 avail_idx;
> u16 used_idx;
> int fw_state;
> + struct msi_map map;
>
> /* keep last in the struct */
> struct mlx5_vq_restore_info ri;
> @@ -792,6 +794,13 @@ static bool counters_supported(const struct
mlx5_vdpa_dev *mvdev)
> BIT_ULL(MLX5_OBJ_TYPE_VIRTIO_Q_COUNTERS);
> }
>
> +static bool msix_mode_supported(struct mlx5_vdpa_dev *mvdev)
> +{
Better to have const struct. It makes it clear that it is read only API
and there is no accidental modification of hca cap or other fields.
> + return (MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev, event_mode) &
> + (1 << MLX5_VIRTIO_Q_EVENT_MODE_MSIX_MODE) &&
> + pci_msix_can_alloc_dyn(mvdev->mdev->pdev));
> +}
> +
> static int create_virtqueue(struct mlx5_vdpa_net *ndev, struct
mlx5_vdpa_virtqueue *mvq)
> {
> int inlen = MLX5_ST_SZ_BYTES(create_virtio_net_q_in);
> @@ -829,9 +838,15 @@ static int create_virtqueue(struct mlx5_vdpa_net
*ndev, struct mlx5_vdpa_virtque
> if (vq_is_tx(mvq->index))
> MLX5_SET(virtio_net_q_object, obj_context, tisn_or_qpn,
ndev->res.tisn);
>
> - MLX5_SET(virtio_q, vq_ctx, event_mode, MLX5_VIRTIO_Q_EVENT_MODE_QP_MODE);
> + if (mvq->map.virq) {
> + MLX5_SET(virtio_q, vq_ctx, event_mode,
MLX5_VIRTIO_Q_EVENT_MODE_MSIX_MODE);
> + MLX5_SET(virtio_q, vq_ctx, event_qpn_or_msix, mvq->map.index);
> + } else {
> + MLX5_SET(virtio_q, vq_ctx, event_mode,
MLX5_VIRTIO_Q_EVENT_MODE_QP_MODE);
> + MLX5_SET(virtio_q, vq_ctx, event_qpn_or_msix, mvq->fwqp.mqp.qpn);
> + }
> +
> MLX5_SET(virtio_q, vq_ctx, queue_index, mvq->index);
> - MLX5_SET(virtio_q, vq_ctx, event_qpn_or_msix, mvq->fwqp.mqp.qpn);
> MLX5_SET(virtio_q, vq_ctx, queue_size, mvq->num_ent);
> MLX5_SET(virtio_q, vq_ctx, virtio_version_1_0,
> !!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_F_VERSION_1)));
> @@ -1174,6 +1189,32 @@ static void counter_set_dealloc(struct mlx5_vdpa_net
*ndev, struct mlx5_vdpa_vir
> mlx5_vdpa_warn(&ndev->mvdev, "dealloc counter set
0x%x\n", mvq->counter_set_id);
> }
>
> +static void alloc_vector(struct mlx5_vdpa_net *ndev,
> + struct mlx5_vdpa_virtqueue *mvq)
> +{
> + struct mlx5_vdpa_irq_pool *irqp = &ndev->irqp;
> + int i;
> +
> + for (i = 0; i < irqp->num_ent; i++) {
> + if (!irqp->entries[i].usecount) {
The usage appears to be boolean.
So better to rename it from usecount to -> used.
used = true/false;
In future, if you plan to reuse the same vector, may be at that point
usecount makes more sense.
> + irqp->entries[i].usecount++;
> + mvq->map = irqp->entries[i].map;
> + return;
> + }
> + }
> +}
> +
> +static void dealloc_vector(struct mlx5_vdpa_net *ndev,
> + struct mlx5_vdpa_virtqueue *mvq)
> +{
> + struct mlx5_vdpa_irq_pool *irqp = &ndev->irqp;
> + int i;
> +
> + for (i = 0; i < irqp->num_ent; i++)
> + if (mvq->map.virq == irqp->entries[i].map.virq)
> + irqp->entries[i].usecount--;
You should add return here too like alloc to not go over rest of the
entries once a specific vq is taken care.
> +}
> +
> static int setup_vq(struct mlx5_vdpa_net *ndev, struct
mlx5_vdpa_virtqueue *mvq)
> {
> u16 idx = mvq->index;
> @@ -1203,27 +1244,31 @@ static int setup_vq(struct mlx5_vdpa_net *ndev,
struct mlx5_vdpa_virtqueue *mvq)
>
> err = counter_set_alloc(ndev, mvq);
> if (err)
> - goto err_counter;
> + goto err_connect;
>
> + alloc_vector(ndev, mvq);
> err = create_virtqueue(ndev, mvq);
> if (err)
> - goto err_connect;
> + goto err_vq;
>
> if (mvq->ready) {
> err = modify_virtqueue(ndev, mvq, MLX5_VIRTIO_NET_Q_OBJECT_STATE_RDY);
> if (err) {
> mlx5_vdpa_warn(&ndev->mvdev, "failed to modify to ready vq
idx %d(%d)\n",
> idx, err);
> - goto err_connect;
> + goto err_modify;
> }
> }
>
> mvq->initialized = true;
> return 0;
>
> -err_connect:
> +err_modify:
> + destroy_virtqueue(ndev, mvq);
> +err_vq:
> + dealloc_vector(ndev, mvq);
> counter_set_dealloc(ndev, mvq);
> -err_counter:
> +err_connect:
> qp_destroy(ndev, &mvq->vqqp);
> err_vqqp:
> qp_destroy(ndev, &mvq->fwqp);
> @@ -1267,6 +1312,7 @@ static void teardown_vq(struct mlx5_vdpa_net *ndev,
struct mlx5_vdpa_virtqueue *
> return;
>
> suspend_vq(ndev, mvq);
> + dealloc_vector(ndev, mvq);
Should be destroyed after destroying the vq to keep the cleanup as
mirror of the create flow.
> destroy_virtqueue(ndev, mvq);
> counter_set_dealloc(ndev, mvq);
> qp_destroy(ndev, &mvq->vqqp);
> @@ -2374,6 +2420,7 @@ static int save_channel_info(struct mlx5_vdpa_net
*ndev, struct mlx5_vdpa_virtqu
> ri->desc_addr = mvq->desc_addr;
> ri->device_addr = mvq->device_addr;
> ri->driver_addr = mvq->driver_addr;
> + ri->map = mvq->map;
> ri->restore = true;
> return 0;
> }
> @@ -2418,6 +2465,7 @@ static void restore_channels_info(struct
mlx5_vdpa_net *ndev)
> mvq->desc_addr = ri->desc_addr;
> mvq->device_addr = ri->device_addr;
> mvq->driver_addr = ri->driver_addr;
> + mvq->map = ri->map;
> }
> }
>
> @@ -2693,6 +2741,22 @@ static struct device *mlx5_get_vq_dma_dev(struct
vdpa_device *vdev, u16 idx)
> return mvdev->vdev.dma_dev;
> }
>
> +static void free_irqs(struct mlx5_vdpa_net *ndev)
> +{
> + struct mlx5_vdpa_irq_pool_entry *ent;
> + int i;
> +
> + if (!msix_mode_supported(&ndev->mvdev))
> + return;
> +
> + for (i = ndev->irqp.num_ent - 1; i >= 0; i--) {
> + ent = ndev->irqp.entries + i;
> + mlx5_msix_free(ndev->mvdev.mdev, ent->map);
> + ndev->irqp.num_ent--;
No need to reduce num_ent here.