On Thu, 2018-03-22 at 07:20 +0200, Leon Romanovsky wrote:
> On Wed, Mar 21, 2018 at 02:18:22PM -0700, Saeed Mahameed wrote:
> > On Wed, Mar 21, 2018 at 7:39 AM, Leon Romanovsky <leon@xxxxxxxxxx>
> > wrote:
> > > From: Ariel Levkovich <lariel@xxxxxxxxxxxx>
> > >
> > > Adding MEMIC (device memory) allocation/deallocation for
> > > supporting
> > > Mellanox devices.
> > >
> > > The device capability to allocate device memory is reported by
> > > the
> > > device firmware in the general capabilities structure and the
> > > specific device memory capabilities are queried via a new
> > > capabilities
> > > type named device memory capabilities.
> > >
> > > The allocation/deallocation is performed by 2 new firmware
> > > commands
> > > which are posted to the device's firwmare.
> > >
> > > mlx5_core exports the allocation/deallocation functions for use
> > > by
> > > its clients.
> > >
> > > Signed-off-by: Ariel Levkovich <lariel@xxxxxxxxxxxx>
> > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > ---
> > > drivers/net/ethernet/mellanox/mlx5/core/alloc.c | 110
> > > ++++++++++++++++++++++++
> > > drivers/net/ethernet/mellanox/mlx5/core/fw.c | 6 ++
> > > drivers/net/ethernet/mellanox/mlx5/core/main.c | 1 +
> > > include/linux/mlx5/device.h | 17 ++++
> > > include/linux/mlx5/driver.h | 8 ++
> > > include/linux/mlx5/mlx5_ifc.h | 75
> > > +++++++++++++++-
> > > 6 files changed, 216 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/alloc.c
> > > b/drivers/net/ethernet/mellanox/mlx5/core/alloc.c
> > > index 323ffe8bf7e4..ab1eac6b6ae3 100644
> > > --- a/drivers/net/ethernet/mellanox/mlx5/core/alloc.c
> > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/alloc.c
> > > @@ -309,3 +309,113 @@ void mlx5_fill_page_frag_array(struct
> > > mlx5_frag_buf *buf, __be64 *pas)
> > > pas[i] = cpu_to_be64(buf->frags[i].map);
> > > }
> > > EXPORT_SYMBOL_GPL(mlx5_fill_page_frag_array);
> > > +
> > > +int mlx5_core_alloc_memic(struct mlx5_core_dev *dev, phys_addr_t
> > > *addr,
> > > + u64 length, u32 alignment)
> > > +{
> > > + u64 num_memic_hw_pages = MLX5_CAP_DEV_MEM(dev,
> > > memic_bar_size)
> > > + >> PAGE_SHIFT;
> > > + u64 hw_start_addr = MLX5_CAP64_DEV_MEM(dev,
> > > memic_bar_start_addr);
> > > + u32 max_alignment = MLX5_CAP_DEV_MEM(dev,
> > > log_max_memic_addr_alignment);
> > > + u32 num_pages = DIV_ROUND_UP(length, PAGE_SIZE);
> > > + u32 out[MLX5_ST_SZ_DW(alloc_memic_out)] = {};
> > > + u32 in[MLX5_ST_SZ_DW(alloc_memic_in)] = {};
> > > + struct mlx5_priv *priv = &dev->priv;
> > > + u32 mlx5_alignment;
> > > + u64 page_idx = 0;
> > > + int ret = 0;
> > > +
> > > + mlx5_core_dbg(dev, "alloc_memic req: length=0x%llx
> > > log_alignment=%d\n",
> > > + length, alignment);
> > > +
> > > + if (!length || (length & MLX5_MEMIC_ALLOC_SIZE_MASK))
> > > + return -EINVAL;
> > > +
> > > + /* mlx5 device sets alignment as 64*2^driver_value
> > > + * so normalizing is needed.
> > > + */
> > > + mlx5_alignment = (alignment < MLX5_MEMIC_BASE_ALIGN) ? 0
> > > :
> > > + alignment - MLX5_MEMIC_BASE_ALIGN;
> > > + if (mlx5_alignment > max_alignment)
> > > + return -EINVAL;
> > > +
> > > + MLX5_SET(alloc_memic_in, in, opcode,
> > > MLX5_CMD_OP_ALLOC_MEMIC);
> > > + MLX5_SET(alloc_memic_in, in, range_size, num_pages *
> > > PAGE_SIZE);
> > > + MLX5_SET(alloc_memic_in, in, memic_size, length);
> > > + MLX5_SET(alloc_memic_in, in, log_memic_addr_alignment,
> > > + mlx5_alignment);
> > > +
> > > + do {
> > > + spin_lock(&dev->priv.memic_lock);
> > > + page_idx = bitmap_find_next_zero_area(priv-
> > > >memic_alloc_pages,
> > > + num_memic_h
> > > w_pages,
> > > + page_idx,
> > > + num_pages,
> > > 0);
> > > +
> > > + if (page_idx + num_pages <= num_memic_hw_pages)
> > > + bitmap_set(dev->priv.memic_alloc_pages,
> > > + page_idx, num_pages);
> > > + else
> > > + ret = -ENOMEM;
> > > +
> > > + spin_unlock(&dev->priv.memic_lock);
> > > +
> > > + if (ret)
> > > + return ret;
> > > +
> > > + MLX5_SET64(alloc_memic_in, in, range_start_addr,
> > > + hw_start_addr + (page_idx *
> > > PAGE_SIZE));
> > > +
> > > + ret = mlx5_cmd_exec(dev, in, sizeof(in), out,
> > > sizeof(out));
> > > + if (ret) {
> > > + spin_lock(&dev->priv.memic_lock);
> > > + bitmap_clear(dev->priv.memic_alloc_pages,
> > > + page_idx, num_pages);
> > > + spin_unlock(&dev->priv.memic_lock);
> > > +
> > > + if (ret == -EAGAIN) {
> > > + page_idx++;
> > > + continue;
> > > + }
> > > +
> > > + return ret;
> > > + }
> > > +
> > > + *addr = pci_resource_start(dev->pdev, 0) +
> > > + MLX5_GET64(alloc_memic_out, out,
> > > memic_start_addr);
> > > +
> > > + return ret;
> > > + } while (page_idx < num_memic_hw_pages);
> > > +
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL(mlx5_core_alloc_memic);
> > > +
> > > +int mlx5_core_dealloc_memic(struct mlx5_core_dev *dev, u64 addr,
> > > u64 length)
> > > +{
> > > + u64 hw_start_addr = MLX5_CAP64_DEV_MEM(dev,
> > > memic_bar_start_addr);
> > > + u32 num_pages = DIV_ROUND_UP(length, PAGE_SIZE);
> > > + u32 out[MLX5_ST_SZ_DW(dealloc_memic_out)] = {0};
> > > + u32 in[MLX5_ST_SZ_DW(dealloc_memic_in)] = {0};
> > > + u64 start_page_idx;
> > > + int err;
> > > +
> > > + addr -= pci_resource_start(dev->pdev, 0);
> > > + start_page_idx = (addr - hw_start_addr) >> PAGE_SHIFT;
> > > +
> > > + MLX5_SET(dealloc_memic_in, in, opcode,
> > > MLX5_CMD_OP_DEALLOC_MEMIC);
> > > + MLX5_SET64(dealloc_memic_in, in, memic_start_addr, addr);
> > > + MLX5_SET(dealloc_memic_in, in, memic_size, length);
> > > +
> > > + err = mlx5_cmd_exec(dev, in, sizeof(in), out,
> > > sizeof(out));
> > > +
> > > + if (!err) {
> > > + spin_lock(&dev->priv.memic_lock);
> > > + bitmap_clear(dev->priv.memic_alloc_pages,
> > > + start_page_idx, num_pages);
> > > + spin_unlock(&dev->priv.memic_lock);
> > > + }
> > > +
> > > + return err;
> > > +}
> > > +EXPORT_SYMBOL(mlx5_core_dealloc_memic);
> >
> > No! this doesn't belong to
> > drivers/net/ethernet/mellanox/mlx5/core/alloc.c
> > if you look carefully at the file you woun't find any FW command in
> > here and this is done intentionally
> > this file is only for io/dma buffer allocations/management, no more
> > ..
> >
> > Please re-spin and let me review any change
> > drivers/net/ethernet/mellanox/mlx5/core/*
> > before upstreaming.
> >
>
> Please calm down, no one is going to take this series before IPsec
> and it
> is posted here to gather feedback.
>
let's consider moving the whole memic HW interface to mlx5_ib,
including the memic page bitmap and spinlock, no need for mlx5_core to
do baby sitting for rdma only interfaces, we talked about this in the
past, I think this is a good opportunity to do this in this feature.
mlx5_core should include only really shared HW
Interfaces/APIs/resources and hw boot flows. this is the best practice
we need to follow to really reduce conflicts and shared code impacts.
> Thanks��.n��������+%������w��{.n�����{���fk��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
