Re: [PATCH mlx5-next] net/mlx5: Consolidate UAR index to PFN helpers

[Jason Gunthorpe]

On Tue, May 19, 2026 at 06:08:18PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@xxxxxxxxxx>
> 
> mlx5_core's uar2pfn() and mlx5_ib's uar_index2pfn() compute the same
> value via slightly different idioms.  Given:
> 
>   MLX5_ADAPTER_PAGE_SHIFT = 12
>   MLX5_UARS_IN_PAGE       = PAGE_SIZE / MLX5_ADAPTER_PAGE_SIZE
>                           = 1 << (PAGE_SHIFT - 12)
> 
> when uar_4k is set, uar2pfn()'s "index >> (PAGE_SHIFT - 12)" reduces to
> "index / MLX5_UARS_IN_PAGE", which is exactly what uar_index2pfn() does.
> When uar_4k is clear, both fall through to the identity case.  The same
> arithmetic is also open-coded a third time in uar_index2paddress(), which
> just multiplies the result by PAGE_SIZE.
> 
> The duplication is historical: uar_index2pfn() landed with the original
> mlx5_ib driver in 2013 (e126ba97dba9), uar2pfn() was added in 2017
> (a6d51b68611e) when the bfreg allocator moved into mlx5_core, and no
> shared header ever exposed the helper.  The two were last touched in
> parallel by aa8106f137b9 ("net/mlx5: Add explicit bar address field"),
> confirming they are meant to behave identically.
> 
> Replace all three local copies with two static inlines in
> include/linux/mlx5/driver.h returning phys_addr_t, which is the
> appropriate type for a value that subsequently feeds ioremap*() and
> rdma_user_mmap_io().  No functional change.
> 
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
> ---
> mlx5_core's uar2pfn() and mlx5_ib's uar_index2pfn() compute the same
> value via slightly different idioms. Let's consolidate them.
> ---
>  drivers/infiniband/hw/mlx5/main.c             | 25 ++-----------------------
>  drivers/net/ethernet/mellanox/mlx5/core/uar.c | 14 +-------------
>  include/linux/mlx5/driver.h                   | 16 ++++++++++++++++
>  3 files changed, 19 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> index 428811fa805b..e61db29bc166 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -2373,27 +2373,6 @@ static void mlx5_ib_dealloc_ucontext(struct ib_ucontext *ibcontext)
>  	}
>  }
>  
> -static phys_addr_t uar_index2pfn(struct mlx5_ib_dev *dev,
> -				 int uar_idx)
> -{
> -	int fw_uars_per_page;
> -
> -	fw_uars_per_page = MLX5_CAP_GEN(dev->mdev, uar_4k) ? MLX5_UARS_IN_PAGE : 1;
> -
> -	return (dev->mdev->bar_addr >> PAGE_SHIFT) + uar_idx / fw_uars_per_page;
> -}
> -
> -static u64 uar_index2paddress(struct mlx5_ib_dev *dev,
> -				 int uar_idx)
> -{
> -	unsigned int fw_uars_per_page;
> -
> -	fw_uars_per_page = MLX5_CAP_GEN(dev->mdev, uar_4k) ?
> -				MLX5_UARS_IN_PAGE : 1;
> -
> -	return (dev->mdev->bar_addr + (uar_idx / fw_uars_per_page) * PAGE_SIZE);
> -}
> -
>  static int get_command(unsigned long offset)
>  {
>  	return (offset >> MLX5_IB_MMAP_CMD_SHIFT) & MLX5_IB_MMAP_CMD_MASK;
> @@ -2643,7 +2622,7 @@ static int uar_mmap(struct mlx5_ib_dev *dev, enum mlx5_ib_mmap_cmd cmd,
>  		uar_index = bfregi->sys_pages[idx];
>  	}
>  
> -	pfn = uar_index2pfn(dev, uar_index);
> +	pfn = mlx5_uar_index_to_pfn(dev->mdev, uar_index);
>  	mlx5_ib_dbg(dev, "uar idx 0x%lx, pfn %pa\n", idx, &pfn);
>
>  	err = rdma_user_mmap_io(&context->ibucontext, vma, pfn, PAGE_SIZE,
> @@ -4327,7 +4306,7 @@ alloc_uar_entry(struct mlx5_ib_ucontext *c,
>  		goto end;
>  
>  	entry->page_idx = uar_index;
> -	entry->address = uar_index2paddress(dev, uar_index);
> +	entry->address = mlx5_uar_index_to_paddr(dev->mdev, uar_index);
>  	if (alloc_type == MLX5_IB_UAPI_UAR_ALLOC_TYPE_BF)
>  		entry->mmap_flag = MLX5_IB_MMAP_TYPE_UAR_WC;
>  	else
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/uar.c b/drivers/net/ethernet/mellanox/mlx5/core/uar.c
> index 1513112ecec8..a85d8fed1546 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/uar.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/uar.c
> @@ -66,18 +66,6 @@ static int uars_per_sys_page(struct mlx5_core_dev *mdev)
>  	return 1;
>  }
>  
> -static u64 uar2pfn(struct mlx5_core_dev *mdev, u32 index)
> -{
> -	u32 system_page_index;
> -
> -	if (MLX5_CAP_GEN(mdev, uar_4k))
> -		system_page_index = index >> (PAGE_SHIFT - MLX5_ADAPTER_PAGE_SHIFT);
> -	else
> -		system_page_index = index;
> -
> -	return (mdev->bar_addr >> PAGE_SHIFT) + system_page_index;
> -}
> -
>  static void up_rel_func(struct kref *kref)
>  {
>  	struct mlx5_uars_page *up = container_of(kref, struct mlx5_uars_page, ref_count);
> @@ -132,7 +120,7 @@ static struct mlx5_uars_page *alloc_uars_page(struct mlx5_core_dev *mdev,
>  		goto error1;
>  	}
>  
> -	pfn = uar2pfn(mdev, up->index);
> +	pfn = mlx5_uar_index_to_pfn(mdev, up->index);
>  	if (map_wc) {
>  		up->map = ioremap_wc(pfn << PAGE_SHIFT, PAGE_SIZE);

The only places using PFN here immediately shift it, this should be
using mlx5_uar_index_to_paddr()


> +static inline phys_addr_t mlx5_uar_index_to_pfn(struct mlx5_core_dev *mdev,
> +						u32 uar_idx)
> +{
> +	u32 fw_uars_per_page = MLX5_CAP_GEN(mdev, uar_4k) ? MLX5_UARS_IN_PAGE : 1;
> +
> +	return (mdev->bar_addr >> PAGE_SHIFT) + uar_idx / fw_uars_per_page;
> +}
> +
> +static inline phys_addr_t mlx5_uar_index_to_paddr(struct mlx5_core_dev *mdev,
> +						  u32 uar_idx)
> +{
> +	u32 fw_uars_per_page = MLX5_CAP_GEN(mdev, uar_4k) ? MLX5_UARS_IN_PAGE : 1;
> +
> +	return mdev->bar_addr + (uar_idx / fw_uars_per_page) * PAGE_SIZE;
> +}

Then there is only one caller of the pfn version in uar_mmap which can
just be written using phys and open code the >> PAGE_SHIFT

No reason to duplicate this

Jason