diff mbox series

[net-next,2/2] mlx5: fix type mismatch

Message ID 20190909195024.3268499-2-arnd@arndb.de
State New
Headers show
Series [net-next,1/2] mlx5: steering: use correct enum type | expand

Commit Message

Arnd Bergmann Sept. 9, 2019, 7:50 p.m. UTC 
In mlx5, pointers to 'phys_addr_t' and 'u64' are mixed since the addition
of the pool memory allocator, leading to incorrect behavior on 32-bit
architectures and this compiler warning:

drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c:121:8: error: incompatible pointer types passing 'u64 *' (aka 'unsigned long long *') to parameter of type 'phys_addr_t *' (aka 'unsigned int *') [-Werror,-Wincompatible-pointer-types]
                                   &icm_mr->dm.addr, &icm_mr->dm.obj_id);
                                   ^~~~~~~~~~~~~~~~
include/linux/mlx5/driver.h:1092:39: note: passing argument to parameter 'addr' here
                         u64 length, u16 uid, phys_addr_t *addr, u32 *obj_id);

Change the code to use 'u64' consistently in place of 'phys_addr_t' to
fix this. The alternative of using phys_addr_t more would require a larger
rework.

Fixes: 29cf8febd185 ("net/mlx5: DR, ICM pool memory allocator")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/infiniband/hw/mlx5/cmd.c                 | 4 ++--
 drivers/infiniband/hw/mlx5/cmd.h                 | 4 ++--
 drivers/infiniband/hw/mlx5/mlx5_ib.h             | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/lib/dm.c | 4 ++--
 include/linux/mlx5/driver.h                      | 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)

-- 
2.20.0

Comments

Saeed Mahameed Sept. 10, 2019, 5:56 p.m. UTC | #1 
On Mon, 2019-09-09 at 21:50 +0200, Arnd Bergmann wrote:
> In mlx5, pointers to 'phys_addr_t' and 'u64' are mixed since the

> addition

> of the pool memory allocator, leading to incorrect behavior on 32-bit

> architectures and this compiler warning:

> 

> drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c:121:8:

> error: incompatible pointer types passing 'u64 *' (aka 'unsigned long

> long *') to parameter of type 'phys_addr_t *' (aka 'unsigned int *')

> [-Werror,-Wincompatible-pointer-types]

>                                    &icm_mr->dm.addr, &icm_mr-

> >dm.obj_id);

>                                    ^~~~~~~~~~~~~~~~

> include/linux/mlx5/driver.h:1092:39: note: passing argument to

> parameter 'addr' here

>                          u64 length, u16 uid, phys_addr_t *addr, u32

> *obj_id);

> 

> Change the code to use 'u64' consistently in place of 'phys_addr_t'

> to

> fix this. The alternative of using phys_addr_t more would require a

> larger

> rework.

> 

> Fixes: 29cf8febd185 ("net/mlx5: DR, ICM pool memory allocator")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


Hi Arnd,

Nathan Chancellor Already submitted a patch to fix this and it is more
minimal:
https://patchwork.ozlabs.org/patch/1158177/

I would like to use that patch if it is ok with you.. 

> ---

>  drivers/infiniband/hw/mlx5/cmd.c                 | 4 ++--

>  drivers/infiniband/hw/mlx5/cmd.h                 | 4 ++--

>  drivers/infiniband/hw/mlx5/mlx5_ib.h             | 2 +-

>  drivers/net/ethernet/mellanox/mlx5/core/lib/dm.c | 4 ++--

>  include/linux/mlx5/driver.h                      | 4 ++--

>  5 files changed, 9 insertions(+), 9 deletions(-)

> 

> diff --git a/drivers/infiniband/hw/mlx5/cmd.c

> b/drivers/infiniband/hw/mlx5/cmd.c

> index 4937947400cd..fbcad70ef6dc 100644

> --- a/drivers/infiniband/hw/mlx5/cmd.c

> +++ b/drivers/infiniband/hw/mlx5/cmd.c

> @@ -82,7 +82,7 @@ int mlx5_cmd_modify_cong_params(struct

> mlx5_core_dev *dev,

>  	return mlx5_cmd_exec(dev, in, in_size, out, sizeof(out));

>  }

>  

> -int mlx5_cmd_alloc_memic(struct mlx5_dm *dm, phys_addr_t *addr,

> +int mlx5_cmd_alloc_memic(struct mlx5_dm *dm, u64 *addr,

>  			 u64 length, u32 alignment)

>  {

>  	struct mlx5_core_dev *dev = dm->dev;

> @@ -157,7 +157,7 @@ int mlx5_cmd_alloc_memic(struct mlx5_dm *dm,

> phys_addr_t *addr,

>  	return -ENOMEM;

>  }

>  

> -int mlx5_cmd_dealloc_memic(struct mlx5_dm *dm, phys_addr_t addr, u64

> length)

> +int mlx5_cmd_dealloc_memic(struct mlx5_dm *dm, u64 addr, u64 length)

>  {

>  	struct mlx5_core_dev *dev = dm->dev;

>  	u64 hw_start_addr = MLX5_CAP64_DEV_MEM(dev,

> memic_bar_start_addr);

> diff --git a/drivers/infiniband/hw/mlx5/cmd.h

> b/drivers/infiniband/hw/mlx5/cmd.h

> index 169cab4915e3..2ea7a45a6abb 100644

> --- a/drivers/infiniband/hw/mlx5/cmd.h

> +++ b/drivers/infiniband/hw/mlx5/cmd.h

> @@ -44,9 +44,9 @@ int mlx5_cmd_query_cong_params(struct mlx5_core_dev

> *dev, int cong_point,

>  int mlx5_cmd_query_ext_ppcnt_counters(struct mlx5_core_dev *dev,

> void *out);

>  int mlx5_cmd_modify_cong_params(struct mlx5_core_dev *mdev,

>  				void *in, int in_size);

> -int mlx5_cmd_alloc_memic(struct mlx5_dm *dm, phys_addr_t *addr,

> +int mlx5_cmd_alloc_memic(struct mlx5_dm *dm, u64 *addr,

>  			 u64 length, u32 alignment);

> -int mlx5_cmd_dealloc_memic(struct mlx5_dm *dm, phys_addr_t addr, u64

> length);

> +int mlx5_cmd_dealloc_memic(struct mlx5_dm *dm, u64 addr, u64

> length);

>  void mlx5_cmd_dealloc_pd(struct mlx5_core_dev *dev, u32 pdn, u16

> uid);

>  void mlx5_cmd_destroy_tir(struct mlx5_core_dev *dev, u32 tirn, u16

> uid);

>  void mlx5_cmd_destroy_tis(struct mlx5_core_dev *dev, u32 tisn, u16

> uid);

> diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h

> b/drivers/infiniband/hw/mlx5/mlx5_ib.h

> index 2ceaef3ea3fb..476d4447f901 100644

> --- a/drivers/infiniband/hw/mlx5/mlx5_ib.h

> +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h

> @@ -561,7 +561,7 @@ enum mlx5_ib_mtt_access_flags {

>  

>  struct mlx5_ib_dm {

>  	struct ib_dm		ibdm;

> -	phys_addr_t		dev_addr;

> +	u64			dev_addr;

>  	u32			type;

>  	size_t			size;

>  	union {

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/dm.c

> b/drivers/net/ethernet/mellanox/mlx5/core/lib/dm.c

> index e065c2f68f5a..ad4d7484fa63 100644

> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/dm.c

> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/dm.c

> @@ -90,7 +90,7 @@ void mlx5_dm_cleanup(struct mlx5_core_dev *dev)

>  }

>  

>  int mlx5_dm_sw_icm_alloc(struct mlx5_core_dev *dev, enum

> mlx5_sw_icm_type type,

> -			 u64 length, u16 uid, phys_addr_t *addr, u32

> *obj_id)

> +			 u64 length, u16 uid, u64 *addr, u32 *obj_id)

>  {

>  	u32 num_blocks = DIV_ROUND_UP_ULL(length,

> MLX5_SW_ICM_BLOCK_SIZE(dev));

>  	u32 out[MLX5_ST_SZ_DW(general_obj_out_cmd_hdr)] = {};

> @@ -175,7 +175,7 @@ int mlx5_dm_sw_icm_alloc(struct mlx5_core_dev

> *dev, enum mlx5_sw_icm_type type,

>  EXPORT_SYMBOL_GPL(mlx5_dm_sw_icm_alloc);

>  

>  int mlx5_dm_sw_icm_dealloc(struct mlx5_core_dev *dev, enum

> mlx5_sw_icm_type type,

> -			   u64 length, u16 uid, phys_addr_t addr, u32

> obj_id)

> +			   u64 length, u16 uid, u64 addr, u32 obj_id)

>  {

>  	u32 num_blocks = DIV_ROUND_UP_ULL(length,

> MLX5_SW_ICM_BLOCK_SIZE(dev));

>  	u32 out[MLX5_ST_SZ_DW(general_obj_out_cmd_hdr)] = {};

> diff --git a/include/linux/mlx5/driver.h

> b/include/linux/mlx5/driver.h

> index 3e80f03a387f..e07f9daf7d42 100644

> --- a/include/linux/mlx5/driver.h

> +++ b/include/linux/mlx5/driver.h

> @@ -1089,9 +1089,9 @@ int mlx5_lag_query_cong_counters(struct

> mlx5_core_dev *dev,

>  struct mlx5_uars_page *mlx5_get_uars_page(struct mlx5_core_dev

> *mdev);

>  void mlx5_put_uars_page(struct mlx5_core_dev *mdev, struct

> mlx5_uars_page *up);

>  int mlx5_dm_sw_icm_alloc(struct mlx5_core_dev *dev, enum

> mlx5_sw_icm_type type,

> -			 u64 length, u16 uid, phys_addr_t *addr, u32

> *obj_id);

> +			 u64 length, u16 uid, u64 *addr, u32 *obj_id);

>  int mlx5_dm_sw_icm_dealloc(struct mlx5_core_dev *dev, enum

> mlx5_sw_icm_type type,

> -			   u64 length, u16 uid, phys_addr_t addr, u32

> obj_id);

> +			   u64 length, u16 uid, u64 addr, u32 obj_id);

>  

>  #ifdef CONFIG_MLX5_CORE_IPOIB

>  struct net_device *mlx5_rdma_netdev_alloc(struct mlx5_core_dev

> *mdev,
Arnd Bergmann Sept. 10, 2019, 7:51 p.m. UTC | #2 
On Tue, Sep 10, 2019 at 7:56 PM Saeed Mahameed <saeedm@mellanox.com> wrote:
>

> On Mon, 2019-09-09 at 21:50 +0200, Arnd Bergmann wrote:

> > In mlx5, pointers to 'phys_addr_t' and 'u64' are mixed since the

> > addition

> > of the pool memory allocator, leading to incorrect behavior on 32-bit

> > architectures and this compiler warning:

> >

> > drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c:121:8:

> > error: incompatible pointer types passing 'u64 *' (aka 'unsigned long

> > long *') to parameter of type 'phys_addr_t *' (aka 'unsigned int *')

> > [-Werror,-Wincompatible-pointer-types]

> >                                    &icm_mr->dm.addr, &icm_mr-

> > >dm.obj_id);

> >                                    ^~~~~~~~~~~~~~~~

> > include/linux/mlx5/driver.h:1092:39: note: passing argument to

> > parameter 'addr' here

> >                          u64 length, u16 uid, phys_addr_t *addr, u32

> > *obj_id);

> >

> > Change the code to use 'u64' consistently in place of 'phys_addr_t'

> > to

> > fix this. The alternative of using phys_addr_t more would require a

> > larger

> > rework.

> >

> > Fixes: 29cf8febd185 ("net/mlx5: DR, ICM pool memory allocator")

> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

>

> Hi Arnd,

>

> Nathan Chancellor Already submitted a patch to fix this and it is more

> minimal:

> https://patchwork.ozlabs.org/patch/1158177/

>

> I would like to use that patch if it is ok with you..


Yes, please do. I think I had tried something like that
initially and concluded it wasn't quite right before I went
into a different direction with my patch.

Looking at the two versions now, I also prefer Nathan's,
and I just confirmed that it fixes all the randconfig failures
I ran into.

       Arnd
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/mlx5/cmd.c b/drivers/infiniband/hw/mlx5/cmd.c
index 4937947400cd..fbcad70ef6dc 100644
--- a/drivers/infiniband/hw/mlx5/cmd.c
+++ b/drivers/infiniband/hw/mlx5/cmd.c
@@ -82,7 +82,7 @@  int mlx5_cmd_modify_cong_params(struct mlx5_core_dev *dev,
 	return mlx5_cmd_exec(dev, in, in_size, out, sizeof(out));
 }
 
-int mlx5_cmd_alloc_memic(struct mlx5_dm *dm, phys_addr_t *addr,
+int mlx5_cmd_alloc_memic(struct mlx5_dm *dm, u64 *addr,
 			 u64 length, u32 alignment)
 {
 	struct mlx5_core_dev *dev = dm->dev;
@@ -157,7 +157,7 @@  int mlx5_cmd_alloc_memic(struct mlx5_dm *dm, phys_addr_t *addr,
 	return -ENOMEM;
 }
 
-int mlx5_cmd_dealloc_memic(struct mlx5_dm *dm, phys_addr_t addr, u64 length)
+int mlx5_cmd_dealloc_memic(struct mlx5_dm *dm, u64 addr, u64 length)
 {
 	struct mlx5_core_dev *dev = dm->dev;
 	u64 hw_start_addr = MLX5_CAP64_DEV_MEM(dev, memic_bar_start_addr);
diff --git a/drivers/infiniband/hw/mlx5/cmd.h b/drivers/infiniband/hw/mlx5/cmd.h
index 169cab4915e3..2ea7a45a6abb 100644
--- a/drivers/infiniband/hw/mlx5/cmd.h
+++ b/drivers/infiniband/hw/mlx5/cmd.h
@@ -44,9 +44,9 @@  int mlx5_cmd_query_cong_params(struct mlx5_core_dev *dev, int cong_point,
 int mlx5_cmd_query_ext_ppcnt_counters(struct mlx5_core_dev *dev, void *out);
 int mlx5_cmd_modify_cong_params(struct mlx5_core_dev *mdev,
 				void *in, int in_size);
-int mlx5_cmd_alloc_memic(struct mlx5_dm *dm, phys_addr_t *addr,
+int mlx5_cmd_alloc_memic(struct mlx5_dm *dm, u64 *addr,
 			 u64 length, u32 alignment);
-int mlx5_cmd_dealloc_memic(struct mlx5_dm *dm, phys_addr_t addr, u64 length);
+int mlx5_cmd_dealloc_memic(struct mlx5_dm *dm, u64 addr, u64 length);
 void mlx5_cmd_dealloc_pd(struct mlx5_core_dev *dev, u32 pdn, u16 uid);
 void mlx5_cmd_destroy_tir(struct mlx5_core_dev *dev, u32 tirn, u16 uid);
 void mlx5_cmd_destroy_tis(struct mlx5_core_dev *dev, u32 tisn, u16 uid);
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 2ceaef3ea3fb..476d4447f901 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -561,7 +561,7 @@  enum mlx5_ib_mtt_access_flags {
 
 struct mlx5_ib_dm {
 	struct ib_dm		ibdm;
-	phys_addr_t		dev_addr;
+	u64			dev_addr;
 	u32			type;
 	size_t			size;
 	union {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/dm.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/dm.c
index e065c2f68f5a..ad4d7484fa63 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/dm.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/dm.c
@@ -90,7 +90,7 @@  void mlx5_dm_cleanup(struct mlx5_core_dev *dev)
 }
 
 int mlx5_dm_sw_icm_alloc(struct mlx5_core_dev *dev, enum mlx5_sw_icm_type type,
-			 u64 length, u16 uid, phys_addr_t *addr, u32 *obj_id)
+			 u64 length, u16 uid, u64 *addr, u32 *obj_id)
 {
 	u32 num_blocks = DIV_ROUND_UP_ULL(length, MLX5_SW_ICM_BLOCK_SIZE(dev));
 	u32 out[MLX5_ST_SZ_DW(general_obj_out_cmd_hdr)] = {};
@@ -175,7 +175,7 @@  int mlx5_dm_sw_icm_alloc(struct mlx5_core_dev *dev, enum mlx5_sw_icm_type type,
 EXPORT_SYMBOL_GPL(mlx5_dm_sw_icm_alloc);
 
 int mlx5_dm_sw_icm_dealloc(struct mlx5_core_dev *dev, enum mlx5_sw_icm_type type,
-			   u64 length, u16 uid, phys_addr_t addr, u32 obj_id)
+			   u64 length, u16 uid, u64 addr, u32 obj_id)
 {
 	u32 num_blocks = DIV_ROUND_UP_ULL(length, MLX5_SW_ICM_BLOCK_SIZE(dev));
 	u32 out[MLX5_ST_SZ_DW(general_obj_out_cmd_hdr)] = {};
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 3e80f03a387f..e07f9daf7d42 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -1089,9 +1089,9 @@  int mlx5_lag_query_cong_counters(struct mlx5_core_dev *dev,
 struct mlx5_uars_page *mlx5_get_uars_page(struct mlx5_core_dev *mdev);
 void mlx5_put_uars_page(struct mlx5_core_dev *mdev, struct mlx5_uars_page *up);
 int mlx5_dm_sw_icm_alloc(struct mlx5_core_dev *dev, enum mlx5_sw_icm_type type,
-			 u64 length, u16 uid, phys_addr_t *addr, u32 *obj_id);
+			 u64 length, u16 uid, u64 *addr, u32 *obj_id);
 int mlx5_dm_sw_icm_dealloc(struct mlx5_core_dev *dev, enum mlx5_sw_icm_type type,
-			   u64 length, u16 uid, phys_addr_t addr, u32 obj_id);
+			   u64 length, u16 uid, u64 addr, u32 obj_id);
 
 #ifdef CONFIG_MLX5_CORE_IPOIB
 struct net_device *mlx5_rdma_netdev_alloc(struct mlx5_core_dev *mdev,