Re: [PATCH 04/15] habanalabs: add context and ASID modules

From: Oded Gabbay
Date: Fri Jan 25 2019 - 16:08:25 EST


On Wed, Jan 23, 2019 at 2:28 PM Mike Rapoport <rppt@xxxxxxxxxxxxx> wrote:
>
> On Wed, Jan 23, 2019 at 02:00:46AM +0200, Oded Gabbay wrote:
> > This patch adds two modules - ASID and context.
> >
> > Each user process the opens a device's file must have at least one context
>
> ^that
>
> > before it is able to "work" with the device. Each context has its own
> > device address-space and contains information about its runtime state (its
> > active command submissions).
> >
> > To have address-space separation between contexts, each context is assigned
> > a unique ASID, which stands for "address-space id". Goya supports up to
> > 1024 ASIDs.
> >
> > Currently, the driver doesn't support multiple contexts. Therefore, the
> > user doesn't need to actively create a context. A "primary context" is
> > created automatically when the user opens the device's file.
> >
> > Signed-off-by: Oded Gabbay <oded.gabbay@xxxxxxxxx>
> > ---
> > drivers/misc/habanalabs/Makefile | 2 +-
> > drivers/misc/habanalabs/asid.c | 58 +++++++++
> > drivers/misc/habanalabs/context.c | 155 +++++++++++++++++++++++
> > drivers/misc/habanalabs/device.c | 47 +++++++
> > drivers/misc/habanalabs/habanalabs.h | 70 ++++++++++
> > drivers/misc/habanalabs/habanalabs_drv.c | 46 ++++++-
> > 6 files changed, 375 insertions(+), 3 deletions(-)
> > create mode 100644 drivers/misc/habanalabs/asid.c
> > create mode 100644 drivers/misc/habanalabs/context.c
> >
> > diff --git a/drivers/misc/habanalabs/Makefile b/drivers/misc/habanalabs/Makefile
> > index 6f1ead69bd77..3ffbadc2ca01 100644
> > --- a/drivers/misc/habanalabs/Makefile
> > +++ b/drivers/misc/habanalabs/Makefile
> > @@ -4,7 +4,7 @@
> >
> > obj-m := habanalabs.o
> >
> > -habanalabs-y := habanalabs_drv.o device.o
> > +habanalabs-y := habanalabs_drv.o device.o context.o asid.o
> >
> > include $(src)/goya/Makefile
> > habanalabs-y += $(HL_GOYA_FILES)
> > diff --git a/drivers/misc/habanalabs/asid.c b/drivers/misc/habanalabs/asid.c
> > new file mode 100644
> > index 000000000000..0ce84c8f5a47
> > --- /dev/null
> > +++ b/drivers/misc/habanalabs/asid.c
> > @@ -0,0 +1,58 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * Copyright 2016-2018 HabanaLabs, Ltd.
> > + * All Rights Reserved.
> > + */
> > +
> > +#include "habanalabs.h"
> > +
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +
> > +int hl_asid_init(struct hl_device *hdev)
> > +{
> > + hdev->asid_bitmap = kcalloc(BITS_TO_LONGS(hdev->asic_prop.max_asid),
> > + sizeof(*hdev->asid_bitmap), GFP_KERNEL);
> > + if (!hdev->asid_bitmap)
> > + return -ENOMEM;
> > +
> > + mutex_init(&hdev->asid_mutex);
> > +
> > + /* ASID 0 is reserved for KMD */
> > + set_bit(0, hdev->asid_bitmap);
> > +
> > + return 0;
> > +}
> > +
> > +void hl_asid_fini(struct hl_device *hdev)
> > +{
> > + mutex_destroy(&hdev->asid_mutex);
> > + kfree(hdev->asid_bitmap);
> > +}
> > +
> > +unsigned long hl_asid_alloc(struct hl_device *hdev)
> > +{
> > + unsigned long found;
> > +
> > + mutex_lock(&hdev->asid_mutex);
> > +
> > + found = find_first_zero_bit(hdev->asid_bitmap,
> > + hdev->asic_prop.max_asid);
> > + if (found == hdev->asic_prop.max_asid)
> > + found = 0;
> > + else
> > + set_bit(found, hdev->asid_bitmap);
> > +
> > + mutex_unlock(&hdev->asid_mutex);
> > +
> > + return found;
> > +}
> > +
> > +void hl_asid_free(struct hl_device *hdev, unsigned long asid)
> > +{
> > + if (WARN((asid == 0 || asid >= hdev->asic_prop.max_asid),
> > + "Invalid ASID %lu", asid))
> > + return;
> > + clear_bit(asid, hdev->asid_bitmap);
> > +}
> > diff --git a/drivers/misc/habanalabs/context.c b/drivers/misc/habanalabs/context.c
> > new file mode 100644
> > index 000000000000..cdcad077e5cf
> > --- /dev/null
> > +++ b/drivers/misc/habanalabs/context.c
> > @@ -0,0 +1,155 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * Copyright 2016-2018 HabanaLabs, Ltd.
> > + * All Rights Reserved.
> > + */
> > +
> > +#include "habanalabs.h"
> > +
> > +#include <linux/sched.h>
> > +#include <linux/delay.h>
> > +
> > +static void hl_ctx_fini(struct hl_ctx *ctx)
> > +{
> > + struct hl_device *hdev = ctx->hdev;
> > +
> > + if (ctx->asid != HL_KERNEL_ASID_ID)
> > + hl_asid_free(hdev, ctx->asid);
> > +}
> > +
> > +void hl_ctx_do_release(struct kref *ref)
> > +{
> > + struct hl_ctx *ctx;
> > +
> > + ctx = container_of(ref, struct hl_ctx, refcount);
> > +
> > + dev_dbg(ctx->hdev->dev, "Now really releasing context %d\n", ctx->asid);
> > +
> > + hl_ctx_fini(ctx);
> > +
> > + if (ctx->hpriv)
> > + hl_hpriv_put(ctx->hpriv);
> > +
> > + kfree(ctx);
> > +}
> > +
> > +int hl_ctx_create(struct hl_device *hdev, struct hl_fpriv *hpriv)
> > +{
> > + struct hl_ctx_mgr *mgr = &hpriv->ctx_mgr;
> > + struct hl_ctx *ctx;
> > + int rc;
> > +
> > + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> > + if (!ctx) {
> > + rc = -ENOMEM;
> > + goto out_err;
> > + }
> > +
> > + rc = hl_ctx_init(hdev, ctx, false);
> > + if (rc)
> > + goto free_ctx;
> > +
> > + hl_hpriv_get(hpriv);
> > + ctx->hpriv = hpriv;
> > +
> > + /* TODO: remove for multiple contexts */
> > + hpriv->ctx = ctx;
> > + hdev->user_ctx = ctx;
> > +
> > + mutex_lock(&mgr->ctx_lock);
> > + rc = idr_alloc(&mgr->ctx_handles, ctx, 1, 0, GFP_KERNEL);
> > + mutex_unlock(&mgr->ctx_lock);
> > +
> > + if (rc < 0) {
> > + dev_err(hdev->dev, "Failed to allocate IDR for a new CTX\n");
> > + hl_ctx_free(hdev, ctx);
> > + goto out_err;
> > + }
> > +
> > + return 0;
> > +
> > +free_ctx:
> > + kfree(ctx);
> > +out_err:
> > + return rc;
> > +}
> > +
> > +void hl_ctx_free(struct hl_device *hdev, struct hl_ctx *ctx)
> > +{
> > + if (kref_put(&ctx->refcount, hl_ctx_do_release) == 1)
> > + return;
> > +
> > + dev_warn(hdev->dev,
> > + "Context %d closed or terminated but its CS are executing\n",
> > + ctx->asid);
> > +}
> > +
> > +int hl_ctx_init(struct hl_device *hdev, struct hl_ctx *ctx, bool is_kernel_ctx)
> > +{
> > + ctx->hdev = hdev;
> > +
> > + kref_init(&ctx->refcount);
> > +
> > + if (is_kernel_ctx) {
> > + ctx->asid = HL_KERNEL_ASID_ID; /* KMD gets ASID 0 */
> > + } else {
> > + ctx->asid = hl_asid_alloc(hdev);
> > + if (!ctx->asid) {
> > + dev_err(hdev->dev, "No free ASID, failed to create context\n");
> > + return -ENOMEM;
> > + }
> > + }
> > +
> > + dev_dbg(hdev->dev, "Created context with ASID %u\n", ctx->asid);
> > +
> > + return 0;
> > +}
> > +
> > +void hl_ctx_get(struct hl_device *hdev, struct hl_ctx *ctx)
> > +{
> > + kref_get(&ctx->refcount);
> > +}
> > +
> > +int hl_ctx_put(struct hl_ctx *ctx)
> > +{
> > + return kref_put(&ctx->refcount, hl_ctx_do_release);
> > +}
> > +
> > +/**
> > + * hl_ctx_mgr_init - initialize the context manager
> > + *
> > + * @mgr: pointer to context manager structure
> > + *
> > + * This manager is an object inside the hpriv object of the user process.
> > + * The function is called when a user process opens the FD.
> > + */
> > +void hl_ctx_mgr_init(struct hl_ctx_mgr *mgr)
> > +{
> > + mutex_init(&mgr->ctx_lock);
> > + idr_init(&mgr->ctx_handles);
> > +}
> > +
> > +/**
> > + * hl_ctx_mgr_fini - finalize the context manager
> > + *
> > + * @hdev: pointer to device structure
> > + * @mgr: pointer to context manager structure
> > + *
> > + * This function goes over all the contexts in the manager and frees them.
> > + * It is called when a process closes the FD.
> > + */
> > +void hl_ctx_mgr_fini(struct hl_device *hdev, struct hl_ctx_mgr *mgr)
> > +{
> > + struct hl_ctx *ctx;
> > + struct idr *idp;
> > + u32 id;
> > +
> > + idp = &mgr->ctx_handles;
> > +
> > + idr_for_each_entry(idp, ctx, id)
> > + hl_ctx_free(hdev, ctx);
> > +
> > + idr_destroy(&mgr->ctx_handles);
> > + mutex_destroy(&mgr->ctx_lock);
> > +}
> > diff --git a/drivers/misc/habanalabs/device.c b/drivers/misc/habanalabs/device.c
> > index a4276ef559b3..84ce9fcb52da 100644
> > --- a/drivers/misc/habanalabs/device.c
> > +++ b/drivers/misc/habanalabs/device.c
> > @@ -23,6 +23,12 @@ static void hpriv_release(struct kref *ref)
> > put_pid(hpriv->taskpid);
> >
> > kfree(hpriv);
> > +
> > + /* Now the FD is really closed */
> > + atomic_dec(&hdev->fd_open_cnt);
> > +
> > + /* This allows a new user context to open the device */
> > + hdev->user_ctx = NULL;
> > }
> >
> > void hl_hpriv_get(struct hl_fpriv *hpriv)
> > @@ -47,6 +53,8 @@ static int hl_device_release(struct inode *inode, struct file *filp)
> > {
> > struct hl_fpriv *hpriv = filp->private_data;
> >
> > + hl_ctx_mgr_fini(hpriv->hdev, &hpriv->ctx_mgr);
> > +
> > filp->private_data = NULL;
> >
> > hl_hpriv_put(hpriv);
> > @@ -133,7 +141,20 @@ static int device_early_init(struct hl_device *hdev)
> > if (rc)
> > return rc;
> >
> > + rc = hl_asid_init(hdev);
> > + if (rc)
> > + goto early_fini;
> > +
> > + mutex_init(&hdev->device_open);
> > + atomic_set(&hdev->fd_open_cnt, 0);
> > +
> > return 0;
> > +
> > +early_fini:
> > + if (hdev->asic_funcs->early_fini)
> > + hdev->asic_funcs->early_fini(hdev);
> > +
> > + return rc;
> > }
> >
> > /**
> > @@ -145,9 +166,12 @@ static int device_early_init(struct hl_device *hdev)
> > static void device_early_fini(struct hl_device *hdev)
> > {
> >
> > + hl_asid_fini(hdev);
> > +
> > if (hdev->asic_funcs->early_fini)
> > hdev->asic_funcs->early_fini(hdev);
> >
> > + mutex_destroy(&hdev->device_open);
> > }
> >
> > /**
> > @@ -241,11 +265,30 @@ int hl_device_init(struct hl_device *hdev, struct class *hclass)
> > if (rc)
> > goto early_fini;
> >
> > + /* Allocate the kernel context */
> > + hdev->kernel_ctx = kzalloc(sizeof(*hdev->kernel_ctx), GFP_KERNEL);
> > + if (!hdev->kernel_ctx) {
> > + rc = -ENOMEM;
> > + goto sw_fini;
> > + }
> > +
> > + hdev->user_ctx = NULL;
> > +
> > + rc = hl_ctx_init(hdev, hdev->kernel_ctx, true);
> > + if (rc) {
> > + dev_err(hdev->dev, "failed to initialize kernel context\n");
> > + goto free_ctx;
> > + }
> > +
> > dev_notice(hdev->dev,
> > "Successfully added device to habanalabs driver\n");
> >
> > return 0;
> >
> > +free_ctx:
> > + kfree(hdev->kernel_ctx);
> > +sw_fini:
> > + hdev->asic_funcs->sw_fini(hdev);
> > early_fini:
> > device_early_fini(hdev);
> > release_device:
> > @@ -278,6 +321,10 @@ void hl_device_fini(struct hl_device *hdev)
> > /* Mark device as disabled */
> > hdev->disabled = true;
> >
> > + /* Release kernel context */
> > + if ((hdev->kernel_ctx) && (hl_ctx_put(hdev->kernel_ctx) != 1))
> > + dev_err(hdev->dev, "kernel ctx is still alive\n");
> > +
> > /* Call ASIC S/W finalize function */
> > hdev->asic_funcs->sw_fini(hdev);
> >
> > diff --git a/drivers/misc/habanalabs/habanalabs.h b/drivers/misc/habanalabs/habanalabs.h
> > index 97844825f7a8..d003a6af2131 100644
> > --- a/drivers/misc/habanalabs/habanalabs.h
> > +++ b/drivers/misc/habanalabs/habanalabs.h
> > @@ -125,6 +125,45 @@ struct hl_asic_funcs {
> > void *cpu_addr, dma_addr_t dma_handle);
> > };
> >
> > +
> > +
> > +
> > +
> > +/*
> > + * CONTEXTS
> > + */
> > +
> > +#define HL_KERNEL_ASID_ID 0
> > +
> > +/**
> > + * struct hl_ctx - user/kernel context.
> > + * @hpriv: pointer to the private (KMD) data of the process (fd).
> > + * @hdev: pointer to the device structure.
> > + * @refcount: reference counter for the context. Context is released only when
> > + * this hits 0l. It is incremented on CS and CS_WAIT.
> > + * @asid: context's unique address space ID in the device's MMU.
> > + */
> > +struct hl_ctx {
> > + struct hl_fpriv *hpriv;
> > + struct hl_device *hdev;
> > + struct kref refcount;
> > + u32 asid;
> > +};
> > +
> > +/**
> > + * struct hl_ctx_mgr - for handling multiple contexts.
> > + * @ctx_lock: protects ctx_handles.
> > + * @ctx_handles: idr to hold all ctx handles.
> > + */
> > +struct hl_ctx_mgr {
> > + struct mutex ctx_lock;
> > + struct idr ctx_handles;
> > +};
> > +
> > +
> > +
> > +
> > +
> > /*
> > * FILE PRIVATE STRUCTURE
> > */
> > @@ -134,12 +173,16 @@ struct hl_asic_funcs {
> > * @hdev: habanalabs device structure.
> > * @filp: pointer to the given file structure.
> > * @taskpid: current process ID.
> > + * @ctx: current executing context.
> > + * @ctx_mgr: context manager to handle multiple context for this FD.
> > * @refcount: number of related contexts.
> > */
> > struct hl_fpriv {
> > struct hl_device *hdev;
> > struct file *filp;
> > struct pid *taskpid;
> > + struct hl_ctx *ctx; /* TODO: remove for multiple ctx */
> > + struct hl_ctx_mgr ctx_mgr;
> > struct kref refcount;
> > };
> >
> > @@ -195,13 +238,19 @@ void hl_wreg(struct hl_device *hdev, u32 reg, u32 val);
> > * @dev: realted kernel basic device structure.
> > * @asic_name: ASIC specific nmae.
> > * @asic_type: ASIC specific type.
> > + * @kernel_ctx: KMD context structure.
> > * @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.
> > * @cpu_accessible_dma_pool: KMD <-> ArmCP shared memory pool.
> > + * @asid_bitmap: holds used/available ASIDs.
> > + * @asid_mutex: protects asid_bitmap.
> > + * @device_open: lock for sanity checks upon FD open.
>
> device_open is an ambiguous name for a lock
>
> > * @asic_prop: ASIC specific immutable properties.
> > * @asic_funcs: ASIC specific functions.
> > * @asic_specific: ASIC specific information to use only from ASIC files.
> > + * @user_ctx: current user context executing.
> > + * @fd_open_cnt: number of open context executing.
> > * @major: habanalabs KMD major.
> > * @id: device minor.
> > * @disabled: is device disabled.
> > @@ -214,13 +263,21 @@ struct hl_device {
> > struct device *dev;
> > char asic_name[16];
> > enum hl_asic_type asic_type;
> > + struct hl_ctx *kernel_ctx;
> > struct dma_pool *dma_pool;
> > void *cpu_accessible_dma_mem;
> > dma_addr_t cpu_accessible_dma_address;
> > struct gen_pool *cpu_accessible_dma_pool;
> > + unsigned long *asid_bitmap;
> > + struct mutex asid_mutex;
> > + /* TODO: change to rw_sem for multiple contexts (same as other IOCTL) */
> > + struct mutex device_open;
> > struct asic_fixed_properties asic_prop;
> > const struct hl_asic_funcs *asic_funcs;
> > void *asic_specific;
> > + /* TODO: The following fields should be moved for multi-context */
> > + struct hl_ctx *user_ctx;
> > + atomic_t fd_open_cnt;
> > u32 major;
> > u16 id;
> > u8 disabled;
> > @@ -270,10 +327,23 @@ int hl_poll_timeout_memory(struct hl_device *hdev, u64 addr, u32 timeout_us,
> > int hl_poll_timeout_device_memory(struct hl_device *hdev, void __iomem *addr,
> > u32 timeout_us, u32 *val);
> >
> > +int hl_asid_init(struct hl_device *hdev);
> > +void hl_asid_fini(struct hl_device *hdev);
> > +unsigned long hl_asid_alloc(struct hl_device *hdev);
> > +void hl_asid_free(struct hl_device *hdev, unsigned long asid);
> > +
> > +int hl_ctx_create(struct hl_device *hdev, struct hl_fpriv *hpriv);
> > +void hl_ctx_free(struct hl_device *hdev, struct hl_ctx *ctx);
> > +int hl_ctx_init(struct hl_device *hdev, struct hl_ctx *ctx, bool is_kernel_ctx);
> > +int hl_ctx_put(struct hl_ctx *ctx);
> > +void hl_ctx_mgr_init(struct hl_ctx_mgr *mgr);
> > +void hl_ctx_mgr_fini(struct hl_device *hdev, struct hl_ctx_mgr *mgr);
> > int hl_device_init(struct hl_device *hdev, struct class *hclass);
> > void hl_device_fini(struct hl_device *hdev);
> > int hl_device_suspend(struct hl_device *hdev);
> > int hl_device_resume(struct hl_device *hdev);
> > +void hl_hpriv_get(struct hl_fpriv *hpriv);
> > +void hl_hpriv_put(struct hl_fpriv *hpriv);
> >
> > void goya_set_asic_funcs(struct hl_device *hdev);
> >
> > diff --git a/drivers/misc/habanalabs/habanalabs_drv.c b/drivers/misc/habanalabs/habanalabs_drv.c
> > index 79545003b7c2..0646da83eb53 100644
> > --- a/drivers/misc/habanalabs/habanalabs_drv.c
> > +++ b/drivers/misc/habanalabs/habanalabs_drv.c
> > @@ -77,6 +77,7 @@ int hl_device_open(struct inode *inode, struct file *filp)
> > {
> > struct hl_device *hdev;
> > struct hl_fpriv *hpriv;
> > + int rc;
> >
> > mutex_lock(&hl_devs_idr_lock);
> > hdev = idr_find(&hl_devs_idr, iminor(inode));
> > @@ -88,9 +89,33 @@ int hl_device_open(struct inode *inode, struct file *filp)
> > return -ENXIO;
> > }
> >
> > + mutex_lock(&hdev->device_open);
> > +
> > + if (hdev->disabled) {
> > + dev_err_ratelimited(hdev->dev,
> > + "Can't open %s because it is disabled\n",
> > + dev_name(hdev->dev));
> > + mutex_unlock(&hdev->device_open);
> > + return -EPERM;
> > + }
> > +
> > + if (hdev->user_ctx) {
> > + dev_info_ratelimited(hdev->dev,
> > + "Device %s is already attached to application\n",
> > + dev_name(hdev->dev));
> > + mutex_unlock(&hdev->device_open);
> > + return -EBUSY;
> > + }
> > +
> > + atomic_inc(&hdev->fd_open_cnt);
> > +
> > + mutex_unlock(&hdev->device_open);
> > +
> > hpriv = kzalloc(sizeof(*hpriv), GFP_KERNEL);
> > - if (!hpriv)
> > - return -ENOMEM;
> > + if (!hpriv) {
> > + rc = -ENOMEM;
> > + goto close_device;
> > + }
> >
> > hpriv->hdev = hdev;
> > filp->private_data = hpriv;
> > @@ -98,9 +123,26 @@ int hl_device_open(struct inode *inode, struct file *filp)
> > kref_init(&hpriv->refcount);
> > nonseekable_open(inode, filp);
> >
> > + hl_ctx_mgr_init(&hpriv->ctx_mgr);
> > +
> > + rc = hl_ctx_create(hdev, hpriv);
> > + if (rc) {
> > + dev_err(hdev->dev, "Failed to open FD (CTX fail)\n");
> > + goto out_err;
> > + }
> > +
> > hpriv->taskpid = find_get_pid(current->pid);
> >
> > return 0;
> > +
> > +out_err:
> > + filp->private_data = NULL;
> > + hl_ctx_mgr_fini(hpriv->hdev, &hpriv->ctx_mgr);
> > + kfree(hpriv);
> > +
> > +close_device:
> > + atomic_dec(&hdev->fd_open_cnt);
> > + return rc;
> > }
> >
> > /**
> > --
> > 2.17.1
> >
>
> --
> Sincerely yours,
> Mike.
>
Fixed, thanks.
Oded