lkml.org 
[lkml]   [2019]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 05/15] habanalabs: add command buffer module
    On Wed, Jan 23, 2019 at 02:00:47AM +0200, Oded Gabbay wrote:
    > This patch adds the CB module, which allows the user to create and
    > destroy CBs and to map them to the user's process address-space.

    Can you please spell "command buffer" at least first time it's mentioned?

    > A command buffer is a memory blocks that reside in DMA-able address-space
    > and is physically contiguous so it can be accessed by the device without
    > MMU translation. The command buffer memory is allocated using the
    > coherent DMA API.
    >
    > When creating a new CB, the IOCTL returns a handle of it, and the
    > user-space process needs to use that handle to mmap the buffer to get a VA
    > in the user's address-space.
    >
    > Before destroying (freeing) a CB, the user must unmap the CB's VA using the
    > CB handle.
    >
    > Each CB has a reference counter, which tracks its usage in command
    > submissions and also its mmaps (only a single mmap is allowed).
    >
    > The driver maintains a pool of pre-allocated CBs in order to reduce
    > latency during command submissions. In case the pool is empty, the driver
    > will go to the slow-path of allocating a new CB, i.e. calling
    > dma_alloc_coherent.
    >
    > Signed-off-by: Oded Gabbay <oded.gabbay@gmail.com>
    > ---
    > drivers/misc/habanalabs/Makefile | 3 +-
    > drivers/misc/habanalabs/command_buffer.c | 414 +++++++++++++++++++++
    > drivers/misc/habanalabs/device.c | 43 ++-
    > drivers/misc/habanalabs/goya/goya.c | 28 ++
    > drivers/misc/habanalabs/habanalabs.h | 95 ++++-
    > drivers/misc/habanalabs/habanalabs_drv.c | 2 +
    > drivers/misc/habanalabs/habanalabs_ioctl.c | 102 +++++
    > include/uapi/misc/habanalabs.h | 62 +++
    > 8 files changed, 746 insertions(+), 3 deletions(-)
    > create mode 100644 drivers/misc/habanalabs/command_buffer.c
    > create mode 100644 drivers/misc/habanalabs/habanalabs_ioctl.c
    > create mode 100644 include/uapi/misc/habanalabs.h
    >
    > diff --git a/drivers/misc/habanalabs/Makefile b/drivers/misc/habanalabs/Makefile
    > index 3ffbadc2ca01..2530c9b78ca4 100644
    > --- a/drivers/misc/habanalabs/Makefile
    > +++ b/drivers/misc/habanalabs/Makefile
    > @@ -4,7 +4,8 @@
    >
    > obj-m := habanalabs.o
    >
    > -habanalabs-y := habanalabs_drv.o device.o context.o asid.o
    > +habanalabs-y := habanalabs_drv.o device.o context.o asid.o habanalabs_ioctl.o \
    > + command_buffer.o
    >
    > include $(src)/goya/Makefile
    > habanalabs-y += $(HL_GOYA_FILES)
    > diff --git a/drivers/misc/habanalabs/command_buffer.c b/drivers/misc/habanalabs/command_buffer.c
    > new file mode 100644
    > index 000000000000..535ed6cc5bda
    > --- /dev/null
    > +++ b/drivers/misc/habanalabs/command_buffer.c
    > @@ -0,0 +1,414 @@
    > +// SPDX-License-Identifier: GPL-2.0
    > +
    > +/*
    > + * Copyright 2016-2018 HabanaLabs, Ltd.
    > + * All Rights Reserved.
    > + */
    > +
    > +#include <uapi/misc/habanalabs.h>
    > +#include "habanalabs.h"
    > +
    > +#include <linux/dma-mapping.h>
    > +
    > +static void cb_fini(struct hl_device *hdev, struct hl_cb *cb)
    > +{
    > + hdev->asic_funcs->dma_free_coherent(hdev, cb->size,
    > + (void *) cb->kernel_address, cb->bus_address);

    As it seems, ASIC specific dma_free_coherent is a shortcut for a generic
    dma_free_coherent. Why not use it directly?

    > + kfree(cb);
    > +}
    > +
    > +static void cb_do_release(struct hl_device *hdev, struct hl_cb *cb)
    > +{
    > + if (cb->is_pool) {
    > + spin_lock(&hdev->cb_pool_lock);
    > + list_add(&cb->pool_list, &hdev->cb_pool);
    > + spin_unlock(&hdev->cb_pool_lock);
    > + } else {
    > + cb_fini(hdev, cb);
    > + }
    > +}
    > +
    > +static void cb_release(struct kref *ref)
    > +{
    > + struct hl_device *hdev;
    > + struct hl_cb *cb;
    > +
    > + cb = container_of(ref, struct hl_cb, refcount);
    > + hdev = cb->hdev;
    > +
    > + cb_do_release(hdev, cb);
    > +}
    > +
    > +static struct hl_cb *hl_cb_alloc(struct hl_device *hdev, u32 cb_size,
    > + int ctx_id)
    > +{
    > + struct hl_cb *cb;
    > + void *p;
    > +
    > + if (ctx_id == HL_KERNEL_ASID_ID)
    > + cb = kzalloc(sizeof(*cb), GFP_ATOMIC);

    The GFP_ATOMIC should be used when the caller cannot tolerate reclaim or
    sleep and it does not seem to be the case here.

    > + else
    > + cb = kzalloc(sizeof(*cb), GFP_KERNEL);
    > +
    > + if (!cb)
    > + return NULL;
    > +
    > + if (ctx_id == HL_KERNEL_ASID_ID)
    > + p = hdev->asic_funcs->dma_alloc_coherent(hdev, cb_size,
    > + &cb->bus_address, GFP_ATOMIC);

    GFP_KERNEL?

    > + else
    > + p = hdev->asic_funcs->dma_alloc_coherent(hdev, cb_size,
    > + &cb->bus_address,
    > + GFP_USER | __GFP_ZERO);
    > + if (!p) {
    > + dev_err(hdev->dev,
    > + "failed to allocate %d of dma memory for CB\n",
    > + cb_size);
    > + kfree(cb);
    > + return NULL;
    > + }
    > +
    > + cb->kernel_address = (u64) p;
    > + cb->size = cb_size;
    > +
    > + return cb;
    > +}
    > +
    > +int hl_cb_create(struct hl_device *hdev, struct hl_cb_mgr *mgr,
    > + u32 cb_size, u64 *handle, int ctx_id)
    > +{
    > + struct hl_cb *cb;
    > + bool alloc_new_cb = true;
    > + int rc;
    > +
    > + if (hdev->disabled) {
    > + dev_warn_ratelimited(hdev->dev,
    > + "Device is disabled !!! Can't create new CBs\n");
    > + rc = -EBUSY;
    > + goto out_err;
    > + }
    > +
    > + /* Minimum allocation must be PAGE SIZE */
    > + if (cb_size < PAGE_SIZE)
    > + cb_size = PAGE_SIZE;
    > +
    > + if (ctx_id == HL_KERNEL_ASID_ID &&
    > + cb_size <= hdev->asic_prop.cb_pool_cb_size) {
    > +
    > + spin_lock(&hdev->cb_pool_lock);
    > + if (!list_empty(&hdev->cb_pool)) {
    > + cb = list_first_entry(&hdev->cb_pool, typeof(*cb),
    > + pool_list);
    > + list_del(&cb->pool_list);
    > + spin_unlock(&hdev->cb_pool_lock);
    > + alloc_new_cb = false;
    > + } else {
    > + spin_unlock(&hdev->cb_pool_lock);
    > + dev_warn_once(hdev->dev, "CB pool is empty\n");

    Isn't it going to be a false alarm when you allocate the cb for the first
    time?

    > + }
    > + }
    > +
    > + if (alloc_new_cb) {
    > + cb = hl_cb_alloc(hdev, cb_size, ctx_id);
    > + if (!cb) {
    > + rc = -ENOMEM;
    > + goto out_err;
    > + }
    > + }
    > +
    > + cb->hdev = hdev;
    > + cb->ctx_id = ctx_id;
    > +
    > + spin_lock(&mgr->cb_lock);
    > + rc = idr_alloc(&mgr->cb_handles, cb, 1, 0, GFP_ATOMIC);

    It seems the ID will remain dangling if the cb is reused.

    > + spin_unlock(&mgr->cb_lock);
    > +
    > + if (rc < 0) {
    > + dev_err(hdev->dev, "Failed to allocate IDR for a new CB\n");
    > + goto release_cb;
    > + }
    > +
    > + cb->id = rc;
    > +
    > + kref_init(&cb->refcount);
    > + spin_lock_init(&cb->lock);
    > +
    > + /*
    > + * idr is 32-bit so we can safely OR it with a mask that is above
    > + * 32 bit
    > + */
    > + *handle = cb->id | HL_MMAP_CB_MASK;
    > + *handle <<= PAGE_SHIFT;
    > +
    > + return 0;
    > +
    > +release_cb:
    > + cb_do_release(hdev, cb);
    > +out_err:
    > + *handle = 0;
    > +
    > + return rc;
    > +}
    > +
    > +int hl_cb_destroy(struct hl_device *hdev, struct hl_cb_mgr *mgr, u64 cb_handle)
    > +{
    > + struct hl_cb *cb;
    > + u32 handle;
    > + int rc = 0;
    > +
    > + /*
    > + * handle was given to user to do mmap, I need to shift it back to
    > + * how the idr module gave it to me
    > + */
    > + cb_handle >>= PAGE_SHIFT;
    > + handle = (u32) cb_handle;
    > +
    > + spin_lock(&mgr->cb_lock);
    > +
    > + cb = idr_find(&mgr->cb_handles, handle);
    > + if (cb) {
    > + idr_remove(&mgr->cb_handles, handle);
    > + spin_unlock(&mgr->cb_lock);
    > + kref_put(&cb->refcount, cb_release);
    > + } else {
    > + spin_unlock(&mgr->cb_lock);
    > + dev_err(hdev->dev,
    > + "CB destroy failed, no match to handle 0x%x\n", handle);
    > + rc = -EINVAL;
    > + }
    > +
    > + return rc;
    > +}
    > +
    > +int hl_cb_ioctl(struct hl_fpriv *hpriv, void *data)
    > +{
    > + union hl_cb_args *args = data;
    > + struct hl_device *hdev = hpriv->hdev;
    > + u64 handle;
    > + int rc;
    > +
    > + switch (args->in.op) {
    > + case HL_CB_OP_CREATE:
    > + rc = hl_cb_create(hdev, &hpriv->cb_mgr, args->in.cb_size,
    > + &handle, hpriv->ctx->asid);
    > + memset(args, 0, sizeof(*args));
    > + args->out.cb_handle = handle;
    > + break;
    > + case HL_CB_OP_DESTROY:
    > + rc = hl_cb_destroy(hdev, &hpriv->cb_mgr,
    > + args->in.cb_handle);
    > + memset(args, 0, sizeof(*args));
    > + break;
    > + default:
    > + rc = -EINVAL;
    > + break;
    > + }
    > +
    > + return rc;
    > +}
    > +
    > +static void cb_vm_close(struct vm_area_struct *vma)
    > +{
    > + struct hl_cb *cb = (struct hl_cb *) vma->vm_private_data;
    > +
    > + hl_cb_put(cb);
    > +
    > + spin_lock(&cb->lock);
    > + cb->mmap = false;
    > + cb->vm_start = 0;
    > + cb->vm_end = 0;
    > + spin_unlock(&cb->lock);
    > +
    > + vma->vm_private_data = NULL;
    > +}
    > +
    > +static const struct vm_operations_struct cb_vm_ops = {
    > + .close = cb_vm_close
    > +};
    > +
    > +int hl_cb_mmap(struct hl_fpriv *hpriv, struct vm_area_struct *vma)
    > +{
    > + struct hl_device *hdev = hpriv->hdev;
    > + struct hl_cb *cb;
    > + phys_addr_t address;
    > + u32 handle;
    > + int rc;
    > +
    > + handle = vma->vm_pgoff;
    > +
    > + /* reference was taken here */
    > + cb = hl_cb_get(hdev, &hpriv->cb_mgr, handle);
    > + if (!cb) {
    > + dev_err(hdev->dev,
    > + "CB mmap failed, no match to handle %d\n", handle);
    > + goto err_out;

    why no simply return -EINVAL?

    > + }
    > +
    > + /* Validation check */
    > + if (vma->vm_end - vma->vm_start != cb->size) {
    > + dev_err(hdev->dev,
    > + "CB mmap failed, mmap size 0x%lx != 0x%x cb size\n",
    > + vma->vm_end - vma->vm_start, cb->size);
    > + goto put_cb;
    > + }
    > +
    > + spin_lock(&cb->lock);
    > +
    > + if (cb->mmap) {
    > + dev_err(hdev->dev,
    > + "CB mmap failed, CB already mmaped to user\n");
    > + goto release_lock;
    > + }
    > +
    > + cb->mmap = true;
    > +
    > + spin_unlock(&cb->lock);
    > +
    > + vma->vm_ops = &cb_vm_ops;
    > +
    > + /*
    > + * Note: We're transferring the cb reference to
    > + * vma->vm_private_data here.
    > + */
    > +
    > + vma->vm_private_data = cb;
    > +
    > + /* Calculate address for CB */
    > + address = virt_to_phys((void *) cb->kernel_address);
    > +
    > + rc = hdev->asic_funcs->cb_mmap(hdev, vma, cb->kernel_address,
    > + address, cb->size);
    > +
    > + if (rc) {
    > + spin_lock(&cb->lock);
    > + cb->mmap = false;
    > + goto release_lock;
    > + }
    > +
    > + cb->vm_start = vma->vm_start;
    > + cb->vm_end = vma->vm_end;
    > +
    > + return 0;
    > +
    > +release_lock:
    > + spin_unlock(&cb->lock);
    > +put_cb:
    > + hl_cb_put(cb);
    > +err_out:
    > + return -EINVAL;
    > +}
    > +
    > +struct hl_cb *hl_cb_get(struct hl_device *hdev, struct hl_cb_mgr *mgr,
    > + u32 handle)
    > +{
    > + struct hl_cb *cb;
    > +
    > + spin_lock(&mgr->cb_lock);
    > + cb = idr_find(&mgr->cb_handles, handle);
    > +
    > + if (!cb) {
    > + spin_unlock(&mgr->cb_lock);
    > + dev_warn(hdev->dev,
    > + "CB get failed, no match to handle %d\n", handle);
    > + return NULL;
    > + }
    > +
    > + kref_get(&cb->refcount);
    > +
    > + spin_unlock(&mgr->cb_lock);
    > +
    > + return cb;
    > +
    > +}
    > +
    > +void hl_cb_put(struct hl_cb *cb)
    > +{
    > + kref_put(&cb->refcount, cb_release);
    > +}
    > +
    > +void hl_cb_mgr_init(struct hl_cb_mgr *mgr)
    > +{
    > + spin_lock_init(&mgr->cb_lock);
    > + idr_init(&mgr->cb_handles);
    > +}
    > +
    > +void hl_cb_mgr_fini(struct hl_device *hdev, struct hl_cb_mgr *mgr)
    > +{
    > + struct hl_cb *cb;
    > + struct idr *idp;
    > + u32 id;
    > +
    > + idp = &mgr->cb_handles;
    > +
    > + idr_for_each_entry(idp, cb, id) {
    > + if (kref_put(&cb->refcount, cb_release) != 1)
    > + dev_err(hdev->dev,
    > + "CB %d for CTX ID %d is still alive\n",
    > + id, cb->ctx_id);
    > + }
    > +
    > + idr_destroy(&mgr->cb_handles);
    > +}
    > +
    > +struct hl_cb *hl_cb_kernel_create(struct hl_device *hdev, u32 cb_size)
    > +{
    > + u64 cb_handle;
    > + struct hl_cb *cb;
    > + int rc;
    > +
    > + rc = hl_cb_create(hdev, &hdev->kernel_cb_mgr, cb_size, &cb_handle,
    > + HL_KERNEL_ASID_ID);
    > + if (rc) {
    > + dev_err(hdev->dev, "Failed to allocate CB for KMD %d\n", rc);
    > + return NULL;
    > + }
    > +
    > + cb_handle >>= PAGE_SHIFT;
    > + cb = hl_cb_get(hdev, &hdev->kernel_cb_mgr, (u32) cb_handle);
    > + /* hl_cb_get should never fail here so use kernel WARN */
    > + WARN(!cb, "Kernel CB handle invalid 0x%x\n", (u32) cb_handle);
    > + if (!cb)
    > + goto destroy_cb;
    > +
    > + return cb;
    > +
    > +destroy_cb:
    > + hl_cb_destroy(hdev, &hdev->kernel_cb_mgr, cb_handle << PAGE_SHIFT);
    > +
    > + return NULL;
    > +}
    > +
    > +int hl_cb_pool_init(struct hl_device *hdev)
    > +{
    > + struct hl_cb *cb;
    > + int i;
    > +
    > + INIT_LIST_HEAD(&hdev->cb_pool);
    > + spin_lock_init(&hdev->cb_pool_lock);
    > +
    > + for (i = 0 ; i < hdev->asic_prop.cb_pool_cb_cnt ; i++) {
    > + cb = hl_cb_alloc(hdev, hdev->asic_prop.cb_pool_cb_size,
    > + HL_KERNEL_ASID_ID);
    > + if (cb) {
    > + cb->is_pool = true;
    > + list_add(&cb->pool_list, &hdev->cb_pool);
    > + } else {
    > + hl_cb_pool_fini(hdev);
    > + return -ENOMEM;
    > + }
    > + }
    > +
    > + return 0;
    > +}
    > +
    > +int hl_cb_pool_fini(struct hl_device *hdev)
    > +{
    > + struct hl_cb *cb, *tmp;
    > +
    > + list_for_each_entry_safe(cb, tmp, &hdev->cb_pool, pool_list) {
    > + list_del(&cb->pool_list);
    > + cb_fini(hdev, cb);
    > + }
    > +
    > + return 0;
    > +}
    > diff --git a/drivers/misc/habanalabs/device.c b/drivers/misc/habanalabs/device.c
    > index 84ce9fcb52da..0bd86a7d34db 100644
    > --- a/drivers/misc/habanalabs/device.c
    > +++ b/drivers/misc/habanalabs/device.c
    > @@ -53,6 +53,7 @@ static int hl_device_release(struct inode *inode, struct file *filp)
    > {
    > struct hl_fpriv *hpriv = filp->private_data;
    >
    > + hl_cb_mgr_fini(hpriv->hdev, &hpriv->cb_mgr);
    > hl_ctx_mgr_fini(hpriv->hdev, &hpriv->ctx_mgr);
    >
    > filp->private_data = NULL;
    > @@ -62,10 +63,34 @@ static int hl_device_release(struct inode *inode, struct file *filp)
    > return 0;
    > }
    >
    > +/**
    > + * hl_mmap - mmap function for habanalabs device
    > + *
    > + * @*filp: pointer to file structure
    > + * @*vma: pointer to vm_area_struct of the process
    > + *
    > + * Called when process does an mmap on habanalabs device. Call the device's mmap
    > + * function at the end of the common code.
    > + */
    > +static int hl_mmap(struct file *filp, struct vm_area_struct *vma)
    > +{
    > + struct hl_fpriv *hpriv = filp->private_data;
    > +
    > + if ((vma->vm_pgoff & HL_MMAP_CB_MASK) == HL_MMAP_CB_MASK) {
    > + vma->vm_pgoff ^= HL_MMAP_CB_MASK;
    > + return hl_cb_mmap(hpriv, vma);
    > + }
    > +
    > + return hpriv->hdev->asic_funcs->mmap(hpriv, vma);
    > +}
    > +
    > static const struct file_operations hl_ops = {
    > .owner = THIS_MODULE,
    > .open = hl_device_open,
    > - .release = hl_device_release
    > + .release = hl_device_release,
    > + .mmap = hl_mmap,
    > + .unlocked_ioctl = hl_ioctl,
    > + .compat_ioctl = hl_ioctl
    > };
    >
    > /**
    > @@ -145,6 +170,8 @@ static int device_early_init(struct hl_device *hdev)
    > if (rc)
    > goto early_fini;
    >
    > + hl_cb_mgr_init(&hdev->kernel_cb_mgr);
    > +
    > mutex_init(&hdev->device_open);
    > atomic_set(&hdev->fd_open_cnt, 0);
    >
    > @@ -166,6 +193,8 @@ static int device_early_init(struct hl_device *hdev)
    > static void device_early_fini(struct hl_device *hdev)
    > {
    >
    > + hl_cb_mgr_fini(hdev, &hdev->kernel_cb_mgr);
    > +
    > hl_asid_fini(hdev);
    >
    > if (hdev->asic_funcs->early_fini)
    > @@ -280,11 +309,21 @@ int hl_device_init(struct hl_device *hdev, struct class *hclass)
    > goto free_ctx;
    > }
    >
    > + rc = hl_cb_pool_init(hdev);
    > + if (rc) {
    > + dev_err(hdev->dev, "failed to initialize CB pool\n");
    > + goto release_ctx;
    > + }
    > +
    > dev_notice(hdev->dev,
    > "Successfully added device to habanalabs driver\n");
    >
    > return 0;
    >
    > +release_ctx:
    > + if (hl_ctx_put(hdev->kernel_ctx) != 1)
    > + dev_err(hdev->dev,
    > + "kernel ctx is still alive on initialization failure\n");
    > free_ctx:
    > kfree(hdev->kernel_ctx);
    > sw_fini:
    > @@ -321,6 +360,8 @@ void hl_device_fini(struct hl_device *hdev)
    > /* Mark device as disabled */
    > hdev->disabled = true;
    >
    > + hl_cb_pool_fini(hdev);
    > +
    > /* Release kernel context */
    > if ((hdev->kernel_ctx) && (hl_ctx_put(hdev->kernel_ctx) != 1))
    > dev_err(hdev->dev, "kernel ctx is still alive\n");
    > diff --git a/drivers/misc/habanalabs/goya/goya.c b/drivers/misc/habanalabs/goya/goya.c
    > index b2952296b890..341ac085af82 100644
    > --- a/drivers/misc/habanalabs/goya/goya.c
    > +++ b/drivers/misc/habanalabs/goya/goya.c
    > @@ -92,6 +92,9 @@
    >
    > #define GOYA_MAX_INITIATORS 20
    >
    > +#define GOYA_CB_POOL_CB_CNT 512
    > +#define GOYA_CB_POOL_CB_SIZE 0x20000 /* 128KB */
    > +
    > static void goya_get_fixed_properties(struct hl_device *hdev)
    > {
    > struct asic_fixed_properties *prop = &hdev->asic_prop;
    > @@ -119,6 +122,8 @@ static void goya_get_fixed_properties(struct hl_device *hdev)
    > prop->tpc_enabled_mask = TPC_ENABLED_MASK;
    >
    > prop->high_pll = PLL_HIGH_DEFAULT;
    > + prop->cb_pool_cb_cnt = GOYA_CB_POOL_CB_CNT;
    > + prop->cb_pool_cb_size = GOYA_CB_POOL_CB_SIZE;
    > }
    >
    > /**
    > @@ -598,6 +603,27 @@ int goya_resume(struct hl_device *hdev)
    > return 0;
    > }
    >
    > +int goya_mmap(struct hl_fpriv *hpriv, struct vm_area_struct *vma)
    > +{
    > + return -EINVAL;
    > +}
    > +
    > +int goya_cb_mmap(struct hl_device *hdev, struct vm_area_struct *vma,
    > + u64 kaddress, phys_addr_t paddress, u32 size)
    > +{
    > + int rc;
    > +
    > + vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP |
    > + VM_DONTCOPY | VM_NORESERVE;
    > +
    > + rc = remap_pfn_range(vma, vma->vm_start, paddress >> PAGE_SHIFT,
    > + size, vma->vm_page_prot);
    > + if (rc)
    > + dev_err(hdev->dev, "remap_pfn_range error %d", rc);
    > +
    > + return rc;
    > +}
    > +
    > void *goya_dma_alloc_coherent(struct hl_device *hdev, size_t size,
    > dma_addr_t *dma_handle, gfp_t flags)
    > {
    > @@ -617,6 +643,8 @@ static const struct hl_asic_funcs goya_funcs = {
    > .sw_fini = goya_sw_fini,
    > .suspend = goya_suspend,
    > .resume = goya_resume,
    > + .mmap = goya_mmap,
    > + .cb_mmap = goya_cb_mmap,
    > .dma_alloc_coherent = goya_dma_alloc_coherent,
    > .dma_free_coherent = goya_dma_free_coherent,
    > };
    > diff --git a/drivers/misc/habanalabs/habanalabs.h b/drivers/misc/habanalabs/habanalabs.h
    > index d003a6af2131..6ad476df65b0 100644
    > --- a/drivers/misc/habanalabs/habanalabs.h
    > +++ b/drivers/misc/habanalabs/habanalabs.h
    > @@ -21,10 +21,12 @@
    >
    > #define HL_NAME "habanalabs"
    >
    > +#define HL_MMAP_CB_MASK (0x8000000000000000ull >> PAGE_SHIFT)
    > +
    > #define HL_MAX_QUEUES 128
    >
    > struct hl_device;
    > -
    > +struct hl_fpriv;
    >
    >
    >
    > @@ -53,6 +55,8 @@ struct hl_device;
    > * @max_asid: maximum number of open contexts (ASIDs).
    > * @completion_queues_count: number of completion queues.
    > * @high_pll: high PLL frequency used by the device.
    > + * @cb_pool_cb_cnt: number of CBs in the CB pool.
    > + * @cb_pool_cb_size: size of each CB in the CB pool.
    > * @tpc_enabled_mask: which TPCs are enabled.
    > */
    > struct asic_fixed_properties {
    > @@ -73,11 +77,68 @@ struct asic_fixed_properties {
    > u32 sram_size;
    > u32 max_asid;
    > u32 high_pll;
    > + u32 cb_pool_cb_cnt;
    > + u32 cb_pool_cb_size;
    > u8 completion_queues_count;
    > u8 tpc_enabled_mask;
    > };
    >
    >
    > +
    > +
    > +
    > +
    > +/*
    > + * Command Buffers
    > + */
    > +
    > +/**
    > + * struct hl_cb_mgr - describes a Command Buffer Manager.
    > + * @cb_lock: protects cb_handles.
    > + * @cb_handles: an idr to hold all command buffer handles.
    > + */
    > +struct hl_cb_mgr {
    > + spinlock_t cb_lock;
    > + struct idr cb_handles; /* protected by cb_lock */
    > +};
    > +
    > +/**
    > + * struct hl_cb - describes a Command Buffer.
    > + * @refcount: reference counter for usage of the CB.
    > + * @hdev: pointer to device this CB belongs to.
    > + * @lock: spinlock to protect mmap/cs flows.
    > + * @pool_list: node in pool list of command buffers.
    > + * @kernel_address: Holds the CB's kernel virtual address.
    > + * @bus_address: Holds the CB's DMA address.
    > + * @vm_start: Holds the CB's user start virtual address (when mmaped).
    > + * @vm_end: Holds the CB's user end virtual address (when mmaped).
    > + * @size: holds the CB's size.
    > + * @id: the CB's ID.
    > + * @ctx_id: holds the ID of the owner's context.
    > + * @mmap: true if the CB is currently mmaped to user.
    > + * @is_pool: true if CB was acquired from the pool, false otherwise.
    > + */
    > +struct hl_cb {
    > + struct kref refcount;
    > + struct hl_device *hdev;
    > + spinlock_t lock;
    > + struct list_head pool_list;
    > + u64 kernel_address;
    > + dma_addr_t bus_address;
    > + u64 vm_start;
    > + u64 vm_end;
    > + u32 size;
    > + u32 id;
    > + u32 ctx_id;
    > + u8 mmap;
    > + u8 is_pool;
    > +};
    > +
    > +
    > +
    > +
    > +
    > +
    > #define HL_QUEUE_LENGTH 256
    >
    >
    > @@ -109,6 +170,8 @@ enum hl_asic_type {
    > * @sw_fini: tears down driver state, does not configure H/W.
    > * @suspend: handles IP specific H/W or SW changes for suspend.
    > * @resume: handles IP specific H/W or SW changes for resume.
    > + * @mmap: mmap function, does nothing.
    > + * @cb_mmap: maps a CB.
    > * @dma_alloc_coherent: DMA allocate coherent memory.
    > * @dma_free_coherent: free DMA allocation.
    > */
    > @@ -119,6 +182,9 @@ struct hl_asic_funcs {
    > int (*sw_fini)(struct hl_device *hdev);
    > int (*suspend)(struct hl_device *hdev);
    > int (*resume)(struct hl_device *hdev);
    > + int (*mmap)(struct hl_fpriv *hpriv, struct vm_area_struct *vma);
    > + int (*cb_mmap)(struct hl_device *hdev, struct vm_area_struct *vma,
    > + u64 kaddress, phys_addr_t paddress, u32 size);
    > void* (*dma_alloc_coherent)(struct hl_device *hdev, size_t size,
    > dma_addr_t *dma_handle, gfp_t flag);
    > void (*dma_free_coherent)(struct hl_device *hdev, size_t size,
    > @@ -175,6 +241,7 @@ struct hl_ctx_mgr {
    > * @taskpid: current process ID.
    > * @ctx: current executing context.
    > * @ctx_mgr: context manager to handle multiple context for this FD.
    > + * @cb_mgr: command buffer manager to handle multiple buffers for this FD.
    > * @refcount: number of related contexts.
    > */
    > struct hl_fpriv {
    > @@ -183,6 +250,7 @@ struct hl_fpriv {
    > struct pid *taskpid;
    > struct hl_ctx *ctx; /* TODO: remove for multiple ctx */
    > struct hl_ctx_mgr ctx_mgr;
    > + struct hl_cb_mgr cb_mgr;
    > struct kref refcount;
    > };
    >
    > @@ -239,6 +307,7 @@ void hl_wreg(struct hl_device *hdev, u32 reg, u32 val);
    > * @asic_name: ASIC specific nmae.
    > * @asic_type: ASIC specific type.
    > * @kernel_ctx: KMD context structure.
    > + * @kernel_cb_mgr: command buffer manager for creating/destroying/handling CGs.
    > * @dma_pool: DMA pool for small allocations.
    > * @cpu_accessible_dma_mem: KMD <-> ArmCP shared memory CPU address.
    > * @cpu_accessible_dma_address: KMD <-> ArmCP shared memory DMA address.
    > @@ -249,6 +318,8 @@ void hl_wreg(struct hl_device *hdev, u32 reg, u32 val);
    > * @asic_prop: ASIC specific immutable properties.
    > * @asic_funcs: ASIC specific functions.
    > * @asic_specific: ASIC specific information to use only from ASIC files.
    > + * @cb_pool: list of preallocated CBs.
    > + * @cb_pool_lock: protects the CB pool.
    > * @user_ctx: current user context executing.
    > * @fd_open_cnt: number of open context executing.
    > * @major: habanalabs KMD major.
    > @@ -264,6 +335,7 @@ struct hl_device {
    > char asic_name[16];
    > enum hl_asic_type asic_type;
    > struct hl_ctx *kernel_ctx;
    > + struct hl_cb_mgr kernel_cb_mgr;
    > struct dma_pool *dma_pool;
    > void *cpu_accessible_dma_mem;
    > dma_addr_t cpu_accessible_dma_address;
    > @@ -275,6 +347,10 @@ struct hl_device {
    > struct asic_fixed_properties asic_prop;
    > const struct hl_asic_funcs *asic_funcs;
    > void *asic_specific;
    > +
    > + struct list_head cb_pool;
    > + spinlock_t cb_pool_lock;
    > +
    > /* TODO: The following fields should be moved for multi-context */
    > struct hl_ctx *user_ctx;
    > atomic_t fd_open_cnt;
    > @@ -345,6 +421,23 @@ int hl_device_resume(struct hl_device *hdev);
    > void hl_hpriv_get(struct hl_fpriv *hpriv);
    > void hl_hpriv_put(struct hl_fpriv *hpriv);
    >
    > +int hl_cb_create(struct hl_device *hdev, struct hl_cb_mgr *mgr, u32 cb_size,
    > + u64 *handle, int ctx_id);
    > +int hl_cb_destroy(struct hl_device *hdev, struct hl_cb_mgr *mgr, u64 cb_handle);
    > +int hl_cb_mmap(struct hl_fpriv *hpriv, struct vm_area_struct *vma);
    > +struct hl_cb *hl_cb_get(struct hl_device *hdev, struct hl_cb_mgr *mgr,
    > + u32 handle);
    > +void hl_cb_put(struct hl_cb *cb);
    > +void hl_cb_mgr_init(struct hl_cb_mgr *mgr);
    > +void hl_cb_mgr_fini(struct hl_device *hdev, struct hl_cb_mgr *mgr);
    > +struct hl_cb *hl_cb_kernel_create(struct hl_device *hdev, u32 cb_size);
    > +int hl_cb_pool_init(struct hl_device *hdev);
    > +int hl_cb_pool_fini(struct hl_device *hdev);
    > +
    > void goya_set_asic_funcs(struct hl_device *hdev);
    >
    > +/* IOCTLs */
    > +long hl_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
    > +int hl_cb_ioctl(struct hl_fpriv *hpriv, void *data);
    > +
    > #endif /* HABANALABSP_H_ */
    > diff --git a/drivers/misc/habanalabs/habanalabs_drv.c b/drivers/misc/habanalabs/habanalabs_drv.c
    > index 0646da83eb53..5c312dd3aa50 100644
    > --- a/drivers/misc/habanalabs/habanalabs_drv.c
    > +++ b/drivers/misc/habanalabs/habanalabs_drv.c
    > @@ -123,6 +123,7 @@ int hl_device_open(struct inode *inode, struct file *filp)
    > kref_init(&hpriv->refcount);
    > nonseekable_open(inode, filp);
    >
    > + hl_cb_mgr_init(&hpriv->cb_mgr);
    > hl_ctx_mgr_init(&hpriv->ctx_mgr);
    >
    > rc = hl_ctx_create(hdev, hpriv);
    > @@ -138,6 +139,7 @@ int hl_device_open(struct inode *inode, struct file *filp)
    > out_err:
    > filp->private_data = NULL;
    > hl_ctx_mgr_fini(hpriv->hdev, &hpriv->ctx_mgr);
    > + hl_cb_mgr_fini(hpriv->hdev, &hpriv->cb_mgr);
    > kfree(hpriv);
    >
    > close_device:
    > diff --git a/drivers/misc/habanalabs/habanalabs_ioctl.c b/drivers/misc/habanalabs/habanalabs_ioctl.c
    > new file mode 100644
    > index 000000000000..fa2287569e0e
    > --- /dev/null
    > +++ b/drivers/misc/habanalabs/habanalabs_ioctl.c
    > @@ -0,0 +1,102 @@
    > +// SPDX-License-Identifier: GPL-2.0
    > +
    > +/*
    > + * Copyright 2016-2018 HabanaLabs, Ltd.
    > + * All Rights Reserved.
    > + */
    > +
    > +#include <uapi/misc/habanalabs.h>
    > +#include "habanalabs.h"
    > +
    > +#include <linux/fs.h>
    > +#include <linux/uaccess.h>
    > +#include <linux/cred.h>
    > +
    > +#define HL_IOCTL_DEF(ioctl, _func) \
    > + [_IOC_NR(ioctl)] = {.cmd = ioctl, .func = _func}
    > +
    > +static const struct hl_ioctl_desc hl_ioctls[] = {
    > + HL_IOCTL_DEF(HL_IOCTL_CB, hl_cb_ioctl)
    > +};
    > +
    > +#define HL_CORE_IOCTL_COUNT ARRAY_SIZE(hl_ioctls)
    > +
    > +long hl_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
    > +{
    > + struct hl_fpriv *hpriv = filep->private_data;
    > + struct hl_device *hdev = hpriv->hdev;
    > + hl_ioctl_t *func;
    > + const struct hl_ioctl_desc *ioctl = NULL;
    > + unsigned int nr = _IOC_NR(cmd);
    > + char stack_kdata[128];
    > + char *kdata = NULL;
    > + unsigned int usize, asize;
    > + int retcode = -EINVAL;
    > +
    > + if (nr >= HL_CORE_IOCTL_COUNT)

    nr > HL_CORE_IOCTL_COUNT, isn't it?

    > + goto err_i1;

    err_i1 is not very meaningfull. Maybe invalid_ioctl?

    > +
    > + if ((nr >= HL_COMMAND_START) && (nr < HL_COMMAND_END)) {

    The HL_COMMAND_{START,END} do not seem to be defined.
    Besides, this check seem to be overlapped with

    if (nr > HL_CORE_IOCTL_COUNT)

    > + u32 hl_size;
    > +
    > + ioctl = &hl_ioctls[nr];
    > +
    > + hl_size = _IOC_SIZE(ioctl->cmd);
    > + usize = asize = _IOC_SIZE(cmd);
    > + if (hl_size > asize)
    > + asize = hl_size;
    > +
    > + cmd = ioctl->cmd;
    > + } else {
    > + goto err_i1;
    > + }
    > +
    > + /* Do not trust userspace, use our own definition */
    > + func = ioctl->func;
    > +
    > + if (unlikely(!func)) {
    > + dev_dbg(hdev->dev, "no function\n");
    > + retcode = -EINVAL;
    > + goto err_i1;
    > + }
    > +
    > + if (cmd & (IOC_IN | IOC_OUT)) {
    > + if (asize <= sizeof(stack_kdata)) {
    > + kdata = stack_kdata;
    > + } else {
    > + kdata = kmalloc(asize, GFP_KERNEL);
    > + if (!kdata) {
    > + retcode = -ENOMEM;
    > + goto err_i1;
    > + }
    > + }
    > + if (asize > usize)
    > + memset(kdata + usize, 0, asize - usize);

    Just init stack_kdata to 0 and use kzalloc instead of malloc.

    > + }
    > +
    > + if (cmd & IOC_IN) {
    > + if (copy_from_user(kdata, (void __user *)arg, usize)) {
    > + retcode = -EFAULT;
    > + goto err_i1;
    > + }
    > + } else if (cmd & IOC_OUT) {
    > + memset(kdata, 0, usize);
    > + }
    > +
    > + retcode = func(hpriv, kdata);
    > +
    > + if (cmd & IOC_OUT)
    > + if (copy_to_user((void __user *)arg, kdata, usize))
    > + retcode = -EFAULT;
    > +
    > +err_i1:
    > + if (!ioctl)
    > + dev_dbg(hdev->dev,
    > + "invalid ioctl: pid=%d, cmd=0x%02x, nr=0x%02x\n",
    > + task_pid_nr(current), cmd, nr);

    I think this can move right after the 'nr' sanity check and there you can
    simple return -EINVAL after dev_dbg().

    > +
    > + if (kdata != stack_kdata)
    > + kfree(kdata);
    > +
    > + return retcode;
    > +}
    > diff --git a/include/uapi/misc/habanalabs.h b/include/uapi/misc/habanalabs.h
    > new file mode 100644
    > index 000000000000..b3f9213d4709
    > --- /dev/null
    > +++ b/include/uapi/misc/habanalabs.h
    > @@ -0,0 +1,62 @@
    > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
    > + *
    > + * Copyright 2016-2018 HabanaLabs, Ltd.
    > + * All Rights Reserved.
    > + *
    > + * Author: Oded Gabbay <oded.gabbay@gmail.com>
    > + *
    > + */
    > +
    > +#ifndef HABANALABS_H_
    > +#define HABANALABS_H_
    > +
    > +#include <linux/types.h>
    > +#include <linux/ioctl.h>
    > +
    > +/* Opcode to create a new command buffer */
    > +#define HL_CB_OP_CREATE 0
    > +/* Opcode to destroy previously created command buffer */
    > +#define HL_CB_OP_DESTROY 1
    > +
    > +struct hl_cb_in {
    > + /* Handle of CB or 0 if we want to create one */
    > + __u64 cb_handle;
    > + /* HL_CB_OP_* */
    > + __u32 op;
    > + /* Size of CB. Minimum requested size must be PAGE_SIZE */
    > + __u32 cb_size;
    > + /* Context ID - Currently not in use */
    > + __u32 ctx_id;
    > + __u32 pad;
    > +};
    > +
    > +struct hl_cb_out {
    > + /* Handle of CB */
    > + __u64 cb_handle;
    > +};
    > +
    > +union hl_cb_args {
    > + struct hl_cb_in in;
    > + struct hl_cb_out out;
    > +};
    > +
    > +/*
    > + * Command Buffer
    > + * - Request a Command Buffer
    > + * - Destroy a Command Buffer
    > + *
    > + * The command buffers are memory blocks that reside in DMA-able address
    > + * space and are physically contiguous so they can be accessed by the device
    > + * directly. They are allocated using the coherent DMA API.
    > + *
    > + * When creating a new CB, the IOCTL returns a handle of it, and the user-space
    > + * process needs to use that handle to mmap the buffer so it can access them.
    > + *
    > + */
    > +#define HL_IOCTL_CB \
    > + _IOWR('H', 0x02, union hl_cb_args)
    > +
    > +#define HL_COMMAND_START 0x02
    > +#define HL_COMMAND_END 0x03
    > +
    > +#endif /* HABANALABS_H_ */
    > --
    > 2.17.1

    --
    Sincerely yours,
    Mike.

    \
     
     \ /
      Last update: 2019-01-23 13:29    [W:3.610 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site