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

[Leon Romanovsky]

On Tue, May 19, 2026 at 12:32:37PM -0300, Jason Gunthorpe wrote:
> 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()

<...>

> 
> 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

Sure, let's change it v1.

Thanks

> 
> Jason