On Tue, 2023-12-12 at 20:21 +0100, Eugenio Perez Martin wrote:
> On Tue, Dec 5, 2023 at 11:46 AM Dragos Tatulea <dtatu...@nvidia.com> wrote:
> >
> > Addresses get set by .set_vq_address. hw vq addresses will be updated on
> > next modify_virtqueue.
> >
> > Signed-off-by: Dragos Tatulea <dtatu...@nvidia.com>
> > Reviewed-by: Gal Pressman <g...@nvidia.com>
> > Acked-by: Eugenio Pérez <epere...@redhat.com>
>
> I'm kind of ok with this patch and the next one about state, but I
> didn't ack them in the previous series.
>
Sorry about the Ack misplacement. I got confused.
> My main concern is that it is not valid to change the vq address after
> DRIVER_OK in VirtIO, which vDPA follows. Only memory maps are ok to
> change at this moment. I'm not sure about vq state in vDPA, but vhost
> forbids changing it with an active backend.
>
> Suspend is not defined in VirtIO at this moment though, so maybe it is
> ok to decide that all of these parameters may change during suspend.
> Maybe the best thing is to protect this with a vDPA feature flag.
>
> Jason, what do you think?
>
> Thanks!
>
> > ---
> > drivers/vdpa/mlx5/net/mlx5_vnet.c | 9 +++++++++
> > include/linux/mlx5/mlx5_ifc_vdpa.h | 1 +
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > index f8f088cced50..80e066de0866 100644
> > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > @@ -1209,6 +1209,7 @@ static int modify_virtqueue(struct mlx5_vdpa_net
> > *ndev,
> > bool state_change = false;
> > void *obj_context;
> > void *cmd_hdr;
> > + void *vq_ctx;
> > void *in;
> > int err;
> >
> > @@ -1230,6 +1231,7 @@ static int modify_virtqueue(struct mlx5_vdpa_net
> > *ndev,
> > 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);
> > + 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))) {
> > @@ -1241,6 +1243,12 @@ static int modify_virtqueue(struct mlx5_vdpa_net
> > *ndev,
> > 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);
> > + MLX5_SET64(virtio_q, vq_ctx, used_addr, mvq->device_addr);
> > + MLX5_SET64(virtio_q, vq_ctx, available_addr,
> > mvq->driver_addr);
> > + }
> > +
> > 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)
> > @@ -2202,6 +2210,7 @@ static int mlx5_vdpa_set_vq_address(struct
> > vdpa_device *vdev, u16 idx, u64 desc_
> > mvq->desc_addr = desc_area;
> > mvq->device_addr = device_area;
> > mvq->driver_addr = driver_area;
> > + mvq->modified_fields |= MLX5_VIRTQ_MODIFY_MASK_VIRTIO_Q_ADDRS;
> > return 0;
> > }
> >
> > diff --git a/include/linux/mlx5/mlx5_ifc_vdpa.h
> > b/include/linux/mlx5/mlx5_ifc_vdpa.h
> > index b86d51a855f6..9594ac405740 100644
> > --- a/include/linux/mlx5/mlx5_ifc_vdpa.h
> > +++ b/include/linux/mlx5/mlx5_ifc_vdpa.h
> > @@ -145,6 +145,7 @@ enum {
> > MLX5_VIRTQ_MODIFY_MASK_STATE = (u64)1 << 0,
> > MLX5_VIRTQ_MODIFY_MASK_DIRTY_BITMAP_PARAMS = (u64)1 << 3,
> > MLX5_VIRTQ_MODIFY_MASK_DIRTY_BITMAP_DUMP_ENABLE = (u64)1 << 4,
> > + MLX5_VIRTQ_MODIFY_MASK_VIRTIO_Q_ADDRS = (u64)1 << 6,
> > MLX5_VIRTQ_MODIFY_MASK_DESC_GROUP_MKEY = (u64)1 << 14,
> > };
> >
> > --
> > 2.42.0
> >
>
