linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] vdpa/mlx5: Fix memory key MTT population
@ 2021-01-07  7:18 Eli Cohen
  2021-01-08  8:38 ` Jason Wang
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Cohen @ 2021-01-07  7:18 UTC (permalink / raw)
  To: mst, virtualization, netdev, linux-kernel; +Cc: lulu, elic

map_direct_mr() assumed that the number of scatter/gather entries
returned by dma_map_sg_attrs() was equal to the number of segments in
the sgl list. This led to wrong population of the mkey object. Fix this
by properly referring to the returned value.

The hardware expects each MTT entry to contain the DMA address of a
contiguous block of memory of size (1 << mr->log_size) bytes.
dma_map_sg_attrs() can coalesce several sg entries into a single
scatter/gather entry of contiguous DMA range so we need to scan the list
and refer to the size of each s/g entry.

In addition, get rid of fill_sg() which effect is overwritten by
populate_mtts().

Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
Signed-off-by: Eli Cohen <elic@nvidia.com>
---
V0->V1:
1. Fix typos
2. Improve changelog 


 drivers/vdpa/mlx5/core/mlx5_vdpa.h |  1 +
 drivers/vdpa/mlx5/core/mr.c        | 28 ++++++++++++----------------
 2 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
index 5c92a576edae..08f742fd2409 100644
--- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
+++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
@@ -15,6 +15,7 @@ struct mlx5_vdpa_direct_mr {
 	struct sg_table sg_head;
 	int log_size;
 	int nsg;
+	int nent;
 	struct list_head list;
 	u64 offset;
 };
diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
index 4b6195666c58..d300f799efcd 100644
--- a/drivers/vdpa/mlx5/core/mr.c
+++ b/drivers/vdpa/mlx5/core/mr.c
@@ -25,17 +25,6 @@ static int get_octo_len(u64 len, int page_shift)
 	return (npages + 1) / 2;
 }
 
-static void fill_sg(struct mlx5_vdpa_direct_mr *mr, void *in)
-{
-	struct scatterlist *sg;
-	__be64 *pas;
-	int i;
-
-	pas = MLX5_ADDR_OF(create_mkey_in, in, klm_pas_mtt);
-	for_each_sg(mr->sg_head.sgl, sg, mr->nsg, i)
-		(*pas) = cpu_to_be64(sg_dma_address(sg));
-}
-
 static void mlx5_set_access_mode(void *mkc, int mode)
 {
 	MLX5_SET(mkc, mkc, access_mode_1_0, mode & 0x3);
@@ -45,10 +34,18 @@ static void mlx5_set_access_mode(void *mkc, int mode)
 static void populate_mtts(struct mlx5_vdpa_direct_mr *mr, __be64 *mtt)
 {
 	struct scatterlist *sg;
+	int nsg = mr->nsg;
+	u64 dma_addr;
+	u64 dma_len;
+	int j = 0;
 	int i;
 
-	for_each_sg(mr->sg_head.sgl, sg, mr->nsg, i)
-		mtt[i] = cpu_to_be64(sg_dma_address(sg));
+	for_each_sg(mr->sg_head.sgl, sg, mr->nent, i) {
+		for (dma_addr = sg_dma_address(sg), dma_len = sg_dma_len(sg);
+		     nsg && dma_len;
+		     nsg--, dma_addr += BIT(mr->log_size), dma_len -= BIT(mr->log_size))
+			mtt[j++] = cpu_to_be64(dma_addr);
+	}
 }
 
 static int create_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr)
@@ -64,7 +61,6 @@ static int create_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct
 		return -ENOMEM;
 
 	MLX5_SET(create_mkey_in, in, uid, mvdev->res.uid);
-	fill_sg(mr, in);
 	mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
 	MLX5_SET(mkc, mkc, lw, !!(mr->perm & VHOST_MAP_WO));
 	MLX5_SET(mkc, mkc, lr, !!(mr->perm & VHOST_MAP_RO));
@@ -276,8 +272,8 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
 done:
 	mr->log_size = log_entity_size;
 	mr->nsg = nsg;
-	err = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);
-	if (!err)
+	mr->nent = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);
+	if (!mr->nent)
 		goto err_map;
 
 	err = create_direct_mr(mvdev, mr);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] vdpa/mlx5: Fix memory key MTT population
  2021-01-07  7:18 [PATCH v1] vdpa/mlx5: Fix memory key MTT population Eli Cohen
@ 2021-01-08  8:38 ` Jason Wang
  2021-01-20  5:36   ` Eli Cohen
  0 siblings, 1 reply; 7+ messages in thread
From: Jason Wang @ 2021-01-08  8:38 UTC (permalink / raw)
  To: Eli Cohen, mst, virtualization, netdev, linux-kernel; +Cc: lulu


On 2021/1/7 下午3:18, Eli Cohen wrote:
> map_direct_mr() assumed that the number of scatter/gather entries
> returned by dma_map_sg_attrs() was equal to the number of segments in
> the sgl list. This led to wrong population of the mkey object. Fix this
> by properly referring to the returned value.
>
> The hardware expects each MTT entry to contain the DMA address of a
> contiguous block of memory of size (1 << mr->log_size) bytes.
> dma_map_sg_attrs() can coalesce several sg entries into a single
> scatter/gather entry of contiguous DMA range so we need to scan the list
> and refer to the size of each s/g entry.
>
> In addition, get rid of fill_sg() which effect is overwritten by
> populate_mtts().
>
> Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
> Signed-off-by: Eli Cohen <elic@nvidia.com>
> ---
> V0->V1:
> 1. Fix typos
> 2. Improve changelog


Acked-by: Jason Wang <jasowang@redhat.com>


>
>   drivers/vdpa/mlx5/core/mlx5_vdpa.h |  1 +
>   drivers/vdpa/mlx5/core/mr.c        | 28 ++++++++++++----------------
>   2 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> index 5c92a576edae..08f742fd2409 100644
> --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> @@ -15,6 +15,7 @@ struct mlx5_vdpa_direct_mr {
>   	struct sg_table sg_head;
>   	int log_size;
>   	int nsg;
> +	int nent;
>   	struct list_head list;
>   	u64 offset;
>   };
> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> index 4b6195666c58..d300f799efcd 100644
> --- a/drivers/vdpa/mlx5/core/mr.c
> +++ b/drivers/vdpa/mlx5/core/mr.c
> @@ -25,17 +25,6 @@ static int get_octo_len(u64 len, int page_shift)
>   	return (npages + 1) / 2;
>   }
>   
> -static void fill_sg(struct mlx5_vdpa_direct_mr *mr, void *in)
> -{
> -	struct scatterlist *sg;
> -	__be64 *pas;
> -	int i;
> -
> -	pas = MLX5_ADDR_OF(create_mkey_in, in, klm_pas_mtt);
> -	for_each_sg(mr->sg_head.sgl, sg, mr->nsg, i)
> -		(*pas) = cpu_to_be64(sg_dma_address(sg));
> -}
> -
>   static void mlx5_set_access_mode(void *mkc, int mode)
>   {
>   	MLX5_SET(mkc, mkc, access_mode_1_0, mode & 0x3);
> @@ -45,10 +34,18 @@ static void mlx5_set_access_mode(void *mkc, int mode)
>   static void populate_mtts(struct mlx5_vdpa_direct_mr *mr, __be64 *mtt)
>   {
>   	struct scatterlist *sg;
> +	int nsg = mr->nsg;
> +	u64 dma_addr;
> +	u64 dma_len;
> +	int j = 0;
>   	int i;
>   
> -	for_each_sg(mr->sg_head.sgl, sg, mr->nsg, i)
> -		mtt[i] = cpu_to_be64(sg_dma_address(sg));
> +	for_each_sg(mr->sg_head.sgl, sg, mr->nent, i) {
> +		for (dma_addr = sg_dma_address(sg), dma_len = sg_dma_len(sg);
> +		     nsg && dma_len;
> +		     nsg--, dma_addr += BIT(mr->log_size), dma_len -= BIT(mr->log_size))
> +			mtt[j++] = cpu_to_be64(dma_addr);
> +	}
>   }
>   
>   static int create_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr)
> @@ -64,7 +61,6 @@ static int create_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct
>   		return -ENOMEM;
>   
>   	MLX5_SET(create_mkey_in, in, uid, mvdev->res.uid);
> -	fill_sg(mr, in);
>   	mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
>   	MLX5_SET(mkc, mkc, lw, !!(mr->perm & VHOST_MAP_WO));
>   	MLX5_SET(mkc, mkc, lr, !!(mr->perm & VHOST_MAP_RO));
> @@ -276,8 +272,8 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
>   done:
>   	mr->log_size = log_entity_size;
>   	mr->nsg = nsg;
> -	err = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);
> -	if (!err)
> +	mr->nent = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);
> +	if (!mr->nent)
>   		goto err_map;
>   
>   	err = create_direct_mr(mvdev, mr);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] vdpa/mlx5: Fix memory key MTT population
  2021-01-08  8:38 ` Jason Wang
@ 2021-01-20  5:36   ` Eli Cohen
  2021-01-20  7:57     ` Michael S. Tsirkin
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Cohen @ 2021-01-20  5:36 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang
  Cc: mst, virtualization, netdev, linux-kernel, lulu

On Fri, Jan 08, 2021 at 04:38:55PM +0800, Jason Wang wrote:

Hi Michael,
this patch is a fix. Are you going to merge it?

> 
> On 2021/1/7 下午3:18, Eli Cohen wrote:
> > map_direct_mr() assumed that the number of scatter/gather entries
> > returned by dma_map_sg_attrs() was equal to the number of segments in
> > the sgl list. This led to wrong population of the mkey object. Fix this
> > by properly referring to the returned value.
> > 
> > The hardware expects each MTT entry to contain the DMA address of a
> > contiguous block of memory of size (1 << mr->log_size) bytes.
> > dma_map_sg_attrs() can coalesce several sg entries into a single
> > scatter/gather entry of contiguous DMA range so we need to scan the list
> > and refer to the size of each s/g entry.
> > 
> > In addition, get rid of fill_sg() which effect is overwritten by
> > populate_mtts().
> > 
> > Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
> > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > ---
> > V0->V1:
> > 1. Fix typos
> > 2. Improve changelog
> 
> 
> Acked-by: Jason Wang <jasowang@redhat.com>
> 
> 
> > 
> >   drivers/vdpa/mlx5/core/mlx5_vdpa.h |  1 +
> >   drivers/vdpa/mlx5/core/mr.c        | 28 ++++++++++++----------------
> >   2 files changed, 13 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > index 5c92a576edae..08f742fd2409 100644
> > --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > @@ -15,6 +15,7 @@ struct mlx5_vdpa_direct_mr {
> >   	struct sg_table sg_head;
> >   	int log_size;
> >   	int nsg;
> > +	int nent;
> >   	struct list_head list;
> >   	u64 offset;
> >   };
> > diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> > index 4b6195666c58..d300f799efcd 100644
> > --- a/drivers/vdpa/mlx5/core/mr.c
> > +++ b/drivers/vdpa/mlx5/core/mr.c
> > @@ -25,17 +25,6 @@ static int get_octo_len(u64 len, int page_shift)
> >   	return (npages + 1) / 2;
> >   }
> > -static void fill_sg(struct mlx5_vdpa_direct_mr *mr, void *in)
> > -{
> > -	struct scatterlist *sg;
> > -	__be64 *pas;
> > -	int i;
> > -
> > -	pas = MLX5_ADDR_OF(create_mkey_in, in, klm_pas_mtt);
> > -	for_each_sg(mr->sg_head.sgl, sg, mr->nsg, i)
> > -		(*pas) = cpu_to_be64(sg_dma_address(sg));
> > -}
> > -
> >   static void mlx5_set_access_mode(void *mkc, int mode)
> >   {
> >   	MLX5_SET(mkc, mkc, access_mode_1_0, mode & 0x3);
> > @@ -45,10 +34,18 @@ static void mlx5_set_access_mode(void *mkc, int mode)
> >   static void populate_mtts(struct mlx5_vdpa_direct_mr *mr, __be64 *mtt)
> >   {
> >   	struct scatterlist *sg;
> > +	int nsg = mr->nsg;
> > +	u64 dma_addr;
> > +	u64 dma_len;
> > +	int j = 0;
> >   	int i;
> > -	for_each_sg(mr->sg_head.sgl, sg, mr->nsg, i)
> > -		mtt[i] = cpu_to_be64(sg_dma_address(sg));
> > +	for_each_sg(mr->sg_head.sgl, sg, mr->nent, i) {
> > +		for (dma_addr = sg_dma_address(sg), dma_len = sg_dma_len(sg);
> > +		     nsg && dma_len;
> > +		     nsg--, dma_addr += BIT(mr->log_size), dma_len -= BIT(mr->log_size))
> > +			mtt[j++] = cpu_to_be64(dma_addr);
> > +	}
> >   }
> >   static int create_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr)
> > @@ -64,7 +61,6 @@ static int create_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct
> >   		return -ENOMEM;
> >   	MLX5_SET(create_mkey_in, in, uid, mvdev->res.uid);
> > -	fill_sg(mr, in);
> >   	mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
> >   	MLX5_SET(mkc, mkc, lw, !!(mr->perm & VHOST_MAP_WO));
> >   	MLX5_SET(mkc, mkc, lr, !!(mr->perm & VHOST_MAP_RO));
> > @@ -276,8 +272,8 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
> >   done:
> >   	mr->log_size = log_entity_size;
> >   	mr->nsg = nsg;
> > -	err = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);
> > -	if (!err)
> > +	mr->nent = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);
> > +	if (!mr->nent)
> >   		goto err_map;
> >   	err = create_direct_mr(mvdev, mr);
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] vdpa/mlx5: Fix memory key MTT population
  2021-01-20  5:36   ` Eli Cohen
@ 2021-01-20  7:57     ` Michael S. Tsirkin
  2021-01-20  8:11       ` Eli Cohen
  0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2021-01-20  7:57 UTC (permalink / raw)
  To: Eli Cohen; +Cc: Jason Wang, virtualization, netdev, linux-kernel, lulu

On Wed, Jan 20, 2021 at 07:36:19AM +0200, Eli Cohen wrote:
> On Fri, Jan 08, 2021 at 04:38:55PM +0800, Jason Wang wrote:
> 
> Hi Michael,
> this patch is a fix. Are you going to merge it?

yes - in the next pull request.

> > 
> > On 2021/1/7 下午3:18, Eli Cohen wrote:
> > > map_direct_mr() assumed that the number of scatter/gather entries
> > > returned by dma_map_sg_attrs() was equal to the number of segments in
> > > the sgl list. This led to wrong population of the mkey object. Fix this
> > > by properly referring to the returned value.
> > > 
> > > The hardware expects each MTT entry to contain the DMA address of a
> > > contiguous block of memory of size (1 << mr->log_size) bytes.
> > > dma_map_sg_attrs() can coalesce several sg entries into a single
> > > scatter/gather entry of contiguous DMA range so we need to scan the list
> > > and refer to the size of each s/g entry.
> > > 
> > > In addition, get rid of fill_sg() which effect is overwritten by
> > > populate_mtts().
> > > 
> > > Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
> > > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > > ---
> > > V0->V1:
> > > 1. Fix typos
> > > 2. Improve changelog
> > 
> > 
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > 
> > 
> > > 
> > >   drivers/vdpa/mlx5/core/mlx5_vdpa.h |  1 +
> > >   drivers/vdpa/mlx5/core/mr.c        | 28 ++++++++++++----------------
> > >   2 files changed, 13 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > > index 5c92a576edae..08f742fd2409 100644
> > > --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > > +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > > @@ -15,6 +15,7 @@ struct mlx5_vdpa_direct_mr {
> > >   	struct sg_table sg_head;
> > >   	int log_size;
> > >   	int nsg;
> > > +	int nent;
> > >   	struct list_head list;
> > >   	u64 offset;
> > >   };
> > > diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> > > index 4b6195666c58..d300f799efcd 100644
> > > --- a/drivers/vdpa/mlx5/core/mr.c
> > > +++ b/drivers/vdpa/mlx5/core/mr.c
> > > @@ -25,17 +25,6 @@ static int get_octo_len(u64 len, int page_shift)
> > >   	return (npages + 1) / 2;
> > >   }
> > > -static void fill_sg(struct mlx5_vdpa_direct_mr *mr, void *in)
> > > -{
> > > -	struct scatterlist *sg;
> > > -	__be64 *pas;
> > > -	int i;
> > > -
> > > -	pas = MLX5_ADDR_OF(create_mkey_in, in, klm_pas_mtt);
> > > -	for_each_sg(mr->sg_head.sgl, sg, mr->nsg, i)
> > > -		(*pas) = cpu_to_be64(sg_dma_address(sg));
> > > -}
> > > -
> > >   static void mlx5_set_access_mode(void *mkc, int mode)
> > >   {
> > >   	MLX5_SET(mkc, mkc, access_mode_1_0, mode & 0x3);
> > > @@ -45,10 +34,18 @@ static void mlx5_set_access_mode(void *mkc, int mode)
> > >   static void populate_mtts(struct mlx5_vdpa_direct_mr *mr, __be64 *mtt)
> > >   {
> > >   	struct scatterlist *sg;
> > > +	int nsg = mr->nsg;
> > > +	u64 dma_addr;
> > > +	u64 dma_len;
> > > +	int j = 0;
> > >   	int i;
> > > -	for_each_sg(mr->sg_head.sgl, sg, mr->nsg, i)
> > > -		mtt[i] = cpu_to_be64(sg_dma_address(sg));
> > > +	for_each_sg(mr->sg_head.sgl, sg, mr->nent, i) {
> > > +		for (dma_addr = sg_dma_address(sg), dma_len = sg_dma_len(sg);
> > > +		     nsg && dma_len;
> > > +		     nsg--, dma_addr += BIT(mr->log_size), dma_len -= BIT(mr->log_size))
> > > +			mtt[j++] = cpu_to_be64(dma_addr);
> > > +	}
> > >   }
> > >   static int create_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr)
> > > @@ -64,7 +61,6 @@ static int create_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct
> > >   		return -ENOMEM;
> > >   	MLX5_SET(create_mkey_in, in, uid, mvdev->res.uid);
> > > -	fill_sg(mr, in);
> > >   	mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
> > >   	MLX5_SET(mkc, mkc, lw, !!(mr->perm & VHOST_MAP_WO));
> > >   	MLX5_SET(mkc, mkc, lr, !!(mr->perm & VHOST_MAP_RO));
> > > @@ -276,8 +272,8 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
> > >   done:
> > >   	mr->log_size = log_entity_size;
> > >   	mr->nsg = nsg;
> > > -	err = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);
> > > -	if (!err)
> > > +	mr->nent = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);
> > > +	if (!mr->nent)
> > >   		goto err_map;
> > >   	err = create_direct_mr(mvdev, mr);
> > 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] vdpa/mlx5: Fix memory key MTT population
  2021-01-20  7:57     ` Michael S. Tsirkin
@ 2021-01-20  8:11       ` Eli Cohen
  2021-01-20  8:52         ` Michael S. Tsirkin
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Cohen @ 2021-01-20  8:11 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Jason Wang, virtualization, netdev, linux-kernel, lulu

On Wed, Jan 20, 2021 at 02:57:05AM -0500, Michael S. Tsirkin wrote:
> On Wed, Jan 20, 2021 at 07:36:19AM +0200, Eli Cohen wrote:
> > On Fri, Jan 08, 2021 at 04:38:55PM +0800, Jason Wang wrote:
> > 
> > Hi Michael,
> > this patch is a fix. Are you going to merge it?
> 
> yes - in the next pull request.
> 

Great thanks.
Can you send the path to your git tree where you keep the patches you
intend to merge?

> > > 
> > > On 2021/1/7 下午3:18, Eli Cohen wrote:
> > > > map_direct_mr() assumed that the number of scatter/gather entries
> > > > returned by dma_map_sg_attrs() was equal to the number of segments in
> > > > the sgl list. This led to wrong population of the mkey object. Fix this
> > > > by properly referring to the returned value.
> > > > 
> > > > The hardware expects each MTT entry to contain the DMA address of a
> > > > contiguous block of memory of size (1 << mr->log_size) bytes.
> > > > dma_map_sg_attrs() can coalesce several sg entries into a single
> > > > scatter/gather entry of contiguous DMA range so we need to scan the list
> > > > and refer to the size of each s/g entry.
> > > > 
> > > > In addition, get rid of fill_sg() which effect is overwritten by
> > > > populate_mtts().
> > > > 
> > > > Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
> > > > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > > > ---
> > > > V0->V1:
> > > > 1. Fix typos
> > > > 2. Improve changelog
> > > 
> > > 
> > > Acked-by: Jason Wang <jasowang@redhat.com>
> > > 
> > > 
> > > > 
> > > >   drivers/vdpa/mlx5/core/mlx5_vdpa.h |  1 +
> > > >   drivers/vdpa/mlx5/core/mr.c        | 28 ++++++++++++----------------
> > > >   2 files changed, 13 insertions(+), 16 deletions(-)
> > > > 
> > > > diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > > > index 5c92a576edae..08f742fd2409 100644
> > > > --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > > > +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > > > @@ -15,6 +15,7 @@ struct mlx5_vdpa_direct_mr {
> > > >   	struct sg_table sg_head;
> > > >   	int log_size;
> > > >   	int nsg;
> > > > +	int nent;
> > > >   	struct list_head list;
> > > >   	u64 offset;
> > > >   };
> > > > diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> > > > index 4b6195666c58..d300f799efcd 100644
> > > > --- a/drivers/vdpa/mlx5/core/mr.c
> > > > +++ b/drivers/vdpa/mlx5/core/mr.c
> > > > @@ -25,17 +25,6 @@ static int get_octo_len(u64 len, int page_shift)
> > > >   	return (npages + 1) / 2;
> > > >   }
> > > > -static void fill_sg(struct mlx5_vdpa_direct_mr *mr, void *in)
> > > > -{
> > > > -	struct scatterlist *sg;
> > > > -	__be64 *pas;
> > > > -	int i;
> > > > -
> > > > -	pas = MLX5_ADDR_OF(create_mkey_in, in, klm_pas_mtt);
> > > > -	for_each_sg(mr->sg_head.sgl, sg, mr->nsg, i)
> > > > -		(*pas) = cpu_to_be64(sg_dma_address(sg));
> > > > -}
> > > > -
> > > >   static void mlx5_set_access_mode(void *mkc, int mode)
> > > >   {
> > > >   	MLX5_SET(mkc, mkc, access_mode_1_0, mode & 0x3);
> > > > @@ -45,10 +34,18 @@ static void mlx5_set_access_mode(void *mkc, int mode)
> > > >   static void populate_mtts(struct mlx5_vdpa_direct_mr *mr, __be64 *mtt)
> > > >   {
> > > >   	struct scatterlist *sg;
> > > > +	int nsg = mr->nsg;
> > > > +	u64 dma_addr;
> > > > +	u64 dma_len;
> > > > +	int j = 0;
> > > >   	int i;
> > > > -	for_each_sg(mr->sg_head.sgl, sg, mr->nsg, i)
> > > > -		mtt[i] = cpu_to_be64(sg_dma_address(sg));
> > > > +	for_each_sg(mr->sg_head.sgl, sg, mr->nent, i) {
> > > > +		for (dma_addr = sg_dma_address(sg), dma_len = sg_dma_len(sg);
> > > > +		     nsg && dma_len;
> > > > +		     nsg--, dma_addr += BIT(mr->log_size), dma_len -= BIT(mr->log_size))
> > > > +			mtt[j++] = cpu_to_be64(dma_addr);
> > > > +	}
> > > >   }
> > > >   static int create_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr)
> > > > @@ -64,7 +61,6 @@ static int create_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct
> > > >   		return -ENOMEM;
> > > >   	MLX5_SET(create_mkey_in, in, uid, mvdev->res.uid);
> > > > -	fill_sg(mr, in);
> > > >   	mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
> > > >   	MLX5_SET(mkc, mkc, lw, !!(mr->perm & VHOST_MAP_WO));
> > > >   	MLX5_SET(mkc, mkc, lr, !!(mr->perm & VHOST_MAP_RO));
> > > > @@ -276,8 +272,8 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
> > > >   done:
> > > >   	mr->log_size = log_entity_size;
> > > >   	mr->nsg = nsg;
> > > > -	err = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);
> > > > -	if (!err)
> > > > +	mr->nent = dma_map_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);
> > > > +	if (!mr->nent)
> > > >   		goto err_map;
> > > >   	err = create_direct_mr(mvdev, mr);
> > > 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] vdpa/mlx5: Fix memory key MTT population
  2021-01-20  8:11       ` Eli Cohen
@ 2021-01-20  8:52         ` Michael S. Tsirkin
  2021-01-20 12:17           ` Eli Cohen
  0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2021-01-20  8:52 UTC (permalink / raw)
  To: Eli Cohen; +Cc: Jason Wang, virtualization, netdev, linux-kernel, lulu

On Wed, Jan 20, 2021 at 10:11:54AM +0200, Eli Cohen wrote:
> On Wed, Jan 20, 2021 at 02:57:05AM -0500, Michael S. Tsirkin wrote:
> > On Wed, Jan 20, 2021 at 07:36:19AM +0200, Eli Cohen wrote:
> > > On Fri, Jan 08, 2021 at 04:38:55PM +0800, Jason Wang wrote:
> > > 
> > > Hi Michael,
> > > this patch is a fix. Are you going to merge it?
> > 
> > yes - in the next pull request.
> > 
> 
> Great thanks.
> Can you send the path to your git tree where you keep the patches you
> intend to merge?

https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next

Note I often rebase it (e.g. just did).

-- 
MST


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] vdpa/mlx5: Fix memory key MTT population
  2021-01-20  8:52         ` Michael S. Tsirkin
@ 2021-01-20 12:17           ` Eli Cohen
  0 siblings, 0 replies; 7+ messages in thread
From: Eli Cohen @ 2021-01-20 12:17 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Jason Wang, virtualization, netdev, linux-kernel, lulu

On Wed, Jan 20, 2021 at 03:52:00AM -0500, Michael S. Tsirkin wrote:
> On Wed, Jan 20, 2021 at 10:11:54AM +0200, Eli Cohen wrote:
> > On Wed, Jan 20, 2021 at 02:57:05AM -0500, Michael S. Tsirkin wrote:
> > > On Wed, Jan 20, 2021 at 07:36:19AM +0200, Eli Cohen wrote:
> > > > On Fri, Jan 08, 2021 at 04:38:55PM +0800, Jason Wang wrote:
> > > > 
> > > > Hi Michael,
> > > > this patch is a fix. Are you going to merge it?
> > > 
> > > yes - in the next pull request.
> > > 
> > 
> > Great thanks.
> > Can you send the path to your git tree where you keep the patches you
> > intend to merge?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
> 
> Note I often rebase it (e.g. just did).
> 

Great, thanks!

> -- 
> MST
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-01-20 13:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-07  7:18 [PATCH v1] vdpa/mlx5: Fix memory key MTT population Eli Cohen
2021-01-08  8:38 ` Jason Wang
2021-01-20  5:36   ` Eli Cohen
2021-01-20  7:57     ` Michael S. Tsirkin
2021-01-20  8:11       ` Eli Cohen
2021-01-20  8:52         ` Michael S. Tsirkin
2021-01-20 12:17           ` Eli Cohen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).