Re: [PATCH 12/15] habanalabs: add virtual memory and MMU modules
From: Mike Rapoport
Date: Sun Jan 27 2019 - 11:17:27 EST
On Wed, Jan 23, 2019 at 02:00:54AM +0200, Oded Gabbay wrote:
> From: Omer Shpigelman <oshpigelman@xxxxxxxxx>
>
> This patch adds the Virtual Memory and MMU modules.
>
> Goya has an internal MMU which provides process isolation on the internal
> DDR. The internal MMU also performs translations for transactions that go
> from Goya to the Host.
>
> The driver is responsible for allocating and freeing memory on the DDR
> upon user request. It also provides an interface to map and unmap DDR and
> Host memory to the device address space.
>
> Signed-off-by: Omer Shpigelman <oshpigelman@xxxxxxxxx>
> Signed-off-by: Oded Gabbay <oded.gabbay@xxxxxxxxx>
> ---
> drivers/misc/habanalabs/Makefile | 2 +-
> drivers/misc/habanalabs/context.c | 19 +-
> drivers/misc/habanalabs/device.c | 20 +-
> drivers/misc/habanalabs/goya/goya.c | 391 +++++
> drivers/misc/habanalabs/habanalabs.h | 195 +++
> drivers/misc/habanalabs/habanalabs_drv.c | 2 +-
> drivers/misc/habanalabs/habanalabs_ioctl.c | 3 +-
> drivers/misc/habanalabs/include/goya/goya.h | 6 +-
> .../include/hw_ip/mmu/mmu_general.h | 45 +
> .../habanalabs/include/hw_ip/mmu/mmu_v1_0.h | 15 +
> drivers/misc/habanalabs/memory.c | 1506 +++++++++++++++++
> drivers/misc/habanalabs/mmu.c | 604 +++++++
> include/uapi/misc/habanalabs.h | 122 +-
> 13 files changed, 2922 insertions(+), 8 deletions(-)
> create mode 100644 drivers/misc/habanalabs/include/hw_ip/mmu/mmu_general.h
> create mode 100644 drivers/misc/habanalabs/include/hw_ip/mmu/mmu_v1_0.h
> create mode 100644 drivers/misc/habanalabs/mmu.c
[ ... ]
> diff --git a/drivers/misc/habanalabs/goya/goya.c b/drivers/misc/habanalabs/goya/goya.c
> index e3867615b974..94ee4cb00a49 100644
> --- a/drivers/misc/habanalabs/goya/goya.c
> +++ b/drivers/misc/habanalabs/goya/goya.c
[ ... ]
> @@ -265,6 +332,10 @@ static u32 goya_non_fatal_events[GOYA_ASYC_EVENT_GROUP_NON_FATAL_SIZE] = {
> };
>
> static int goya_armcp_info_get(struct hl_device *hdev);
> +static void goya_mmu_prepare(struct hl_device *hdev, u32 asid);
> +static int goya_mmu_clear_pgt_range(struct hl_device *hdev);
> +static int goya_mmu_update_asid_hop0_addr(struct hl_device *hdev, u32 asid,
> + u64 phys_addr);
Nit: are the static declarations are necessary? Or it's a matter of moving
code around?
>
> static void goya_get_fixed_properties(struct hl_device *hdev)
> {
> @@ -303,6 +374,16 @@ static void goya_get_fixed_properties(struct hl_device *hdev)
> prop->sram_user_base_address = prop->sram_base_address +
> SRAM_USER_BASE_OFFSET;
>
> + prop->mmu_pgt_addr = MMU_PAGE_TABLES_ADDR;
> + if (hdev->pldm)
> + prop->mmu_pgt_size = 0x800000; /* 8MB */
> + else
> + prop->mmu_pgt_size = MMU_PAGE_TABLES_SIZE;
> + prop->mmu_pte_size = PTE_SIZE;
> + prop->mmu_hop_table_size = HOP_TABLE_SIZE;
> + prop->mmu_hop0_tables_total_size = HOP0_TABLES_TOTAL_SIZE;
> + prop->dram_page_size = PAGE_SIZE_2MB;
> +
> prop->host_phys_base_address = HOST_PHYS_BASE;
> prop->va_space_host_start_address = VA_HOST_SPACE_START;
> prop->va_space_host_end_address = VA_HOST_SPACE_END;
[ ... ]
> diff --git a/drivers/misc/habanalabs/include/hw_ip/mmu/mmu_general.h b/drivers/misc/habanalabs/include/hw_ip/mmu/mmu_general.h
> new file mode 100644
> index 000000000000..8d61ee4f2d17
> --- /dev/null
> +++ b/drivers/misc/habanalabs/include/hw_ip/mmu/mmu_general.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright 2016-2018 HabanaLabs, Ltd.
> + * All Rights Reserved.
> + *
> + */
> +
> +#ifndef INCLUDE_MMU_GENERAL_H_
> +#define INCLUDE_MMU_GENERAL_H_
> +
> +#define PAGE_SHIFT_4KB 12
> +#define PAGE_SHIFT_2MB 21
> +#define PAGE_SIZE_2MB (_AC(1, UL) << PAGE_SHIFT_2MB)
> +#define PAGE_SIZE_4KB (_AC(1, UL) << PAGE_SHIFT_4KB)
> +
> +#define PAGE_PRESENT_MASK 0x0000000000001
> +#define SWAP_OUT_MASK 0x0000000000004
> +#define LAST_MASK 0x0000000000800
> +#define PHYS_ADDR_MASK 0x3FFFFFFFFF000ull
> +#define HOP0_MASK 0x3000000000000ull
> +#define HOP1_MASK 0x0FF8000000000ull
> +#define HOP2_MASK 0x0007FC0000000ull
> +#define HOP3_MASK 0x000003FE00000
> +#define HOP4_MASK 0x00000001FF000
> +#define OFFSET_MASK 0x0000000000FFF
> +
> +#define HOP0_SHIFT 48
> +#define HOP1_SHIFT 39
> +#define HOP2_SHIFT 30
> +#define HOP3_SHIFT 21
> +#define HOP4_SHIFT 12
> +
> +#define PTE_PHYS_ADDR_SHIFT 12
> +#define PTE_PHYS_ADDR_MASK ~0xFFF
> +
> +#define PTE_SIZE sizeof(u64)
I suspect some architectures define PTE_SIZE in arch/*/include/asm
Probably you'd want to namespace this.
> +#define HOP_TABLE_SIZE PAGE_SIZE_4KB
> +#define HOP0_TABLES_TOTAL_SIZE (HOP_TABLE_SIZE * MAX_ASID)
> +
> +#define MMU_HOP0_PA43_12_SHIFT 12
> +#define MMU_HOP0_PA49_44_SHIFT (12 + 32)
> +
> +#define MMU_CONFIG_TIMEOUT_USEC 2000 /* 2 ms */
> +
> +#endif /* INCLUDE_MMU_GENERAL_H_ */
> diff --git a/drivers/misc/habanalabs/memory.c b/drivers/misc/habanalabs/memory.c
> index 94cbb252656d..c41ea19502e5 100644
> --- a/drivers/misc/habanalabs/memory.c
> +++ b/drivers/misc/habanalabs/memory.c
> @@ -5,12 +5,1193 @@
> * All Rights Reserved.
> */
>
> +#include <uapi/misc/habanalabs.h>
> #include "habanalabs.h"
> +#include "include/hw_ip/mmu/mmu_general.h"
>
> #include <linux/sched.h>
> #include <linux/uaccess.h>
> #include <linux/genalloc.h>
>
> +#define HL_MMU_DEBUG 0
> +
> +/*
> + * The va ranges in context object contain a list with the available chunks of
> + * device virtual memory.
> + * There is one range for host allocations and one for DRAM allocations.
> + *
> + * On initialization each range contains one chunk of all of its available
> + * virtual range which is a half of the total device virtual range.
> + *
> + * On each mapping of physical pages, a suitable virtual range chunk (with a
> + * minimum size) is selected from the list. If the chunk size equals the
> + * requested size, the chunk is returned. Otherwise, the chunk is split into
> + * two chunks - one to return as result and a remainder to stay in the list.
> + *
> + * On each Unmapping of a virtual address, the relevant virtual chunk is
> + * returned to the list. The chunk is added to the list and if its edges match
> + * the edges of the adjacent chunks (means a contiguous chunk can be created),
> + * the chunks are merged.
> + *
> + * On finish, the list is checked to have only one chunk of all the relevant
> + * virtual range (which is a half of the device total virtual range).
> + * If not (means not all mappings were unmapped), a warning is printed.
> + */
> +
> +/**
> + * alloc_device_memory - allocate device memory
> + *
> + * @ctx : current context
> + * @args : host parameters containing the requested size
> + * @ret_handle : result handle
> + *
> + * This function does the following:
> + * - Allocate the requested size rounded up to 2MB pages
> + * - Return unique handle
> + */
> +static int alloc_device_memory(struct hl_ctx *ctx, struct hl_mem_in *args,
> + u32 *ret_handle)
> +{
> + struct hl_device *hdev = ctx->hdev;
> + struct hl_vm *vm = &hdev->vm;
> + struct hl_vm_phys_pg_list *phys_pg_list;
> + struct hl_vm_phys_pg *phys_pg, *tmp;
> + u64 paddr = 0;
> + u32 total_size, num_pgs, page_size, page_shift;
> + int handle, rc, i;
> + bool contiguous;
> +
> + page_size = hdev->asic_prop.dram_page_size;
> + page_shift = __ffs(page_size);
Maybe it's worth storing page_shift in the asi_prop and calculating
page_size.
> + num_pgs = (args->alloc.mem_size + (page_size - 1)) >> page_shift;
> + total_size = num_pgs << page_shift;
> +
> + contiguous = args->flags & HL_MEM_CONTIGUOUS;
> +
> + if (contiguous) {
> + paddr = (u64) gen_pool_alloc(vm->dram_pg_pool, total_size);
> + if (!paddr) {
> + dev_err(hdev->dev,
> + "failed to allocate %u huge contiguous pages\n",
> + num_pgs);
> + return -ENOMEM;
> + }
> + }
> +
> + phys_pg_list = kzalloc(sizeof(*phys_pg_list), GFP_KERNEL);
> + if (!phys_pg_list) {
> + rc = -ENOMEM;
> + goto page_list_err;
> + }
> +
> + phys_pg_list->vm_type = VM_TYPE_PHYS_LIST;
> + phys_pg_list->asid = ctx->asid;
> + phys_pg_list->total_size = total_size;
> + phys_pg_list->flags = args->flags;
> + phys_pg_list->contiguous = contiguous;
> + INIT_LIST_HEAD(&phys_pg_list->list);
> +
> + for (i = 0 ; i < num_pgs ; i++) {
> + phys_pg = kzalloc(sizeof(*phys_pg), GFP_KERNEL);
Consider adding *phys_pgs to phys_pg_list using kcalloc() before the loop.
> + if (!phys_pg) {
> + rc = -ENOMEM;
> + goto pb_err;
> + }
> +
> + phys_pg->page_size = page_size;
> +
> + if (phys_pg_list->contiguous) {
> + phys_pg->paddr = paddr + i * phys_pg->page_size;
> + } else {
> + phys_pg->paddr =
> + (u64) gen_pool_alloc(vm->dram_pg_pool,
> + phys_pg->page_size);
> + if (!phys_pg->paddr) {
> + dev_err(hdev->dev, "ioctl failed to allocate page\n");
> + kfree(phys_pg);
> + rc = -ENOMEM;
> + goto pb_err;
> + }
> + }
> +
> + list_add_tail(&phys_pg->node, &phys_pg_list->list);
> + }
> +
> + spin_lock(&vm->idr_lock);
> + handle = idr_alloc(&vm->phys_pg_list_handles, phys_pg_list, 1, 0,
> + GFP_ATOMIC);
> + spin_unlock(&vm->idr_lock);
> +
> + if (handle < 0) {
> + dev_err(hdev->dev, "Failed to get handle for page\n");
> + rc = -EFAULT;
> + goto idr_err;
> + }
> +
> + for (i = 0; i < num_pgs ; i++)
> + kref_get(&vm->dram_pg_pool_refcount);
> +
> + phys_pg_list->handle = handle;
> +
> + atomic64_add(phys_pg_list->total_size, &ctx->dram_phys_mem);
> + atomic64_add(phys_pg_list->total_size, &hdev->dram_used_mem);
> +
> + *ret_handle = handle;
> +
> + return 0;
> +
> +idr_err:
> +pb_err:
> + list_for_each_entry_safe(phys_pg, tmp, &phys_pg_list->list, node) {
> + if (!phys_pg_list->contiguous)
> + gen_pool_free(vm->dram_pg_pool, phys_pg->paddr,
> + phys_pg->page_size);
> +
> + list_del(&phys_pg->node);
> + kfree(phys_pg);
> + }
> +
> + kfree(phys_pg_list);
> +page_list_err:
> + if (contiguous)
> + gen_pool_free(vm->dram_pg_pool, paddr, total_size);
> +
> + return rc;
> +}
[ ... ]
> +/**
> + * free_phys_pg_list - free physical page list
> + *
> + * @hdev : habanalabs device structure
> + * @phys_pg_list : physical page list to free
> + *
> + * This function does the following:
> + * - Iterate over the list and free each physical block structure
> + * - In case of allocated memory, return the physical memory to the general pool
> + * - Free the hl_vm_phys_pg_list structure
> + */
> +static void free_phys_pg_list(struct hl_device *hdev,
> + struct hl_vm_phys_pg_list *phys_pg_list)
> +{
> + struct hl_vm *vm = &hdev->vm;
> + struct hl_vm_phys_pg *phys_pg, *tmp;
> + u32 num_pgs;
> + bool first = true;
> + int i;
> +
> + list_for_each_entry_safe(phys_pg, tmp, &phys_pg_list->list, node) {
> + /*
> + * this if statement is relevant only when called from
> + * hl_vm_ctx_fini() and free_device_memory()
> + */
> + if (!phys_pg_list->created_from_userptr) {
> + if ((phys_pg_list->contiguous) && (first)) {
> + first = false;
> + gen_pool_free(vm->dram_pg_pool,
> + phys_pg->paddr,
> + phys_pg_list->total_size);
> +
> + num_pgs = phys_pg_list->total_size >>
> + __ffs(hdev->asic_prop.dram_page_size);
> +
> + for (i = 0; i < num_pgs ; i++)
> + kref_put(&vm->dram_pg_pool_refcount,
> + dram_pg_pool_do_release);
> +
> + } else if (!phys_pg_list->contiguous) {
> + gen_pool_free(vm->dram_pg_pool, phys_pg->paddr,
> + phys_pg->page_size);
> + kref_put(&vm->dram_pg_pool_refcount,
> + dram_pg_pool_do_release);
> + }
> + }
> +
> + list_del(&phys_pg->node);
> + kfree(phys_pg);
> + }
Unless I'm missing something this can be simplified a bit:
if (!phys_pg_list->created_from_userptr) {
for (i = 0; i < num_pgs ; i++)
kref_put(&vm->dram_pg_pool_refcount,
dram_pg_pool_do_release);
if (phys_pg_list->contiguous)
gen_pool_free(vm->dram_pg_pool, phys_pg->paddr,
phys_pg_list->total_size);
}
list_for_each_entry_safe(phys_pg, tmp, &phys_pg_list->list, node) {
if (!phys_pg_list->created_from_userptr &&
!phys_pg_list->contiguous)
gen_pool_free(vm->dram_pg_pool, phys_pg->paddr,
phys_pg->page_size);
list_del(&phys_pg->node);
kfree(phys_pg);
}
nd with phys_pg's array hanging from phys_pg_list it would be even simpler
;-)
> +
> + kfree(phys_pg_list);
> +}
> +
--
Sincerely yours,
Mike.