Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 31 Jan 2021 20:55:36 +0200
From:   Eli Cohen <elic@nvidia.com>
To:     Jason Wang <jasowang@redhat.com>
CC:     <mst@redhat.com>, <virtualization@lists.linux-foundation.org>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <lulu@redhat.com>
Subject: Re: [PATCH 2/2] vdpa/mlx5: Restore the hardware used index after
 change map
Message-ID: <20210131185536.GA164217@mtl-vdi-166.wap.labs.mlnx>
References: <20210128134130.3051-1-elic@nvidia.com>
 <20210128134130.3051-3-elic@nvidia.com>
 <54239b51-918c-3475-dc88-4da1a4548da8@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <54239b51-918c-3475-dc88-4da1a4548da8@redhat.com>
User-Agent: Mutt/1.9.5 (bf161cf53efb) (2018-04-13)
Precedence: bulk

On Fri, Jan 29, 2021 at 11:49:45AM +0800, Jason Wang wrote:
>=20
> On 2021/1/28 =E4=B8=8B=E5=8D=889:41, Eli Cohen wrote:
> > When a change of memory map occurs, the hardware resources are destroye=
d
> > and then re-created again with the new memory map. In such case, we nee=
d
> > to restore the hardware available and used indices. The driver failed t=
o
> > restore the used index which is added here.
> >=20
> > Fixes 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devi=
ces")
> > Signed-off-by: Eli Cohen <elic@nvidia.com>
>=20
>=20
> A question. Does this mean after a vq is suspended, the hw used index is =
not
> equal to vq used index?

Surely there is just one "Used index" for a VQ. What I was trying to say
is that after the VQ is suspended, I read the used index by querying the
hardware. The read result is the used index that the hardware wrote to
memory. After the I create the new hardware object, I need to tell it
what is the used index (and the available index) as a way to sync it
with the existing VQ.

This sync is especially important when a change of map occurs while the
VQ was already used (hence the indices are likely to be non zero). This
can be triggered by hot adding memory after the VQs have been used.=20

>=20
> Thanks
>=20
>=20
> > ---
> >   drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >=20
> > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/=
mlx5_vnet.c
> > index 549ded074ff3..3fc8588cecae 100644
> > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > @@ -87,6 +87,7 @@ struct mlx5_vq_restore_info {
> >   	u64 device_addr;
> >   	u64 driver_addr;
> >   	u16 avail_index;
> > +	u16 used_index;
> >   	bool ready;
> >   	struct vdpa_callback cb;
> >   	bool restore;
> > @@ -121,6 +122,7 @@ struct mlx5_vdpa_virtqueue {
> >   	u32 virtq_id;
> >   	struct mlx5_vdpa_net *ndev;
> >   	u16 avail_idx;
> > +	u16 used_idx;
> >   	int fw_state;
> >   	/* keep last in the struct */
> > @@ -804,6 +806,7 @@ static int create_virtqueue(struct mlx5_vdpa_net *n=
dev, struct mlx5_vdpa_virtque
> >   	obj_context =3D MLX5_ADDR_OF(create_virtio_net_q_in, in, obj_context=
);
> >   	MLX5_SET(virtio_net_q_object, obj_context, hw_available_index, mvq->=
avail_idx);
> > +	MLX5_SET(virtio_net_q_object, obj_context, hw_used_index, mvq->used_i=
dx);
> >   	MLX5_SET(virtio_net_q_object, obj_context, queue_feature_bit_mask_12=
_3,
> >   		 get_features_12_3(ndev->mvdev.actual_features));
> >   	vq_ctx =3D MLX5_ADDR_OF(virtio_net_q_object, obj_context, virtio_q_c=
ontext);
> > @@ -1022,6 +1025,7 @@ static int connect_qps(struct mlx5_vdpa_net *ndev=
, struct mlx5_vdpa_virtqueue *m
> >   struct mlx5_virtq_attr {
> >   	u8 state;
> >   	u16 available_index;
> > +	u16 used_index;
> >   };
> >   static int query_virtqueue(struct mlx5_vdpa_net *ndev, struct mlx5_vd=
pa_virtqueue *mvq,
> > @@ -1052,6 +1056,7 @@ static int query_virtqueue(struct mlx5_vdpa_net *=
ndev, struct mlx5_vdpa_virtqueu
> >   	memset(attr, 0, sizeof(*attr));
> >   	attr->state =3D MLX5_GET(virtio_net_q_object, obj_context, state);
> >   	attr->available_index =3D MLX5_GET(virtio_net_q_object, obj_context,=
 hw_available_index);
> > +	attr->used_index =3D MLX5_GET(virtio_net_q_object, obj_context, hw_us=
ed_index);
> >   	kfree(out);
> >   	return 0;
> > @@ -1602,6 +1607,7 @@ static int save_channel_info(struct mlx5_vdpa_net=
 *ndev, struct mlx5_vdpa_virtqu
> >   		return err;
> >   	ri->avail_index =3D attr.available_index;
> > +	ri->used_index =3D attr.used_index;
> >   	ri->ready =3D mvq->ready;
> >   	ri->num_ent =3D mvq->num_ent;
> >   	ri->desc_addr =3D mvq->desc_addr;
> > @@ -1646,6 +1652,7 @@ static void restore_channels_info(struct mlx5_vdp=
a_net *ndev)
> >   			continue;
> >   		mvq->avail_idx =3D ri->avail_index;
> > +		mvq->used_idx =3D ri->used_index;
> >   		mvq->ready =3D ri->ready;
> >   		mvq->num_ent =3D ri->num_ent;
> >   		mvq->desc_addr =3D ri->desc_addr;
>=20