2020-05-24 20:13:16

by Omer Shpigelman

[permalink] [raw]
Subject: [PATCH] habanalabs: handle MMU cache invalidation timeout

MMU cache invalidation timeout indicates that the device is unstable and
therefore unusable.
Hence in such case do hard reset and return an error to the user if was
called from ioctl.
In addition, change the print to error level and rephrase its text.

Signed-off-by: Omer Shpigelman <[email protected]>
---
drivers/misc/habanalabs/gaudi/gaudi.c | 36 ++++++++++++++++-----------
drivers/misc/habanalabs/goya/goya.c | 34 +++++++++++++++----------
drivers/misc/habanalabs/habanalabs.h | 8 +++---
drivers/misc/habanalabs/memory.c | 35 ++++++++++++++++++++------
4 files changed, 75 insertions(+), 38 deletions(-)

diff --git a/drivers/misc/habanalabs/gaudi/gaudi.c b/drivers/misc/habanalabs/gaudi/gaudi.c
index 92a5130f06fb..61f88e9884ce 100644
--- a/drivers/misc/habanalabs/gaudi/gaudi.c
+++ b/drivers/misc/habanalabs/gaudi/gaudi.c
@@ -5975,7 +5975,7 @@ static void *gaudi_get_events_stat(struct hl_device *hdev, bool aggregate,
return gaudi->events_stat;
}

-static void gaudi_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
+static int gaudi_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
u32 flags)
{
struct gaudi_device *gaudi = hdev->asic_specific;
@@ -5984,15 +5984,15 @@ static void gaudi_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,

if (!(gaudi->hw_cap_initialized & HW_CAP_MMU) ||
hdev->hard_reset_pending)
- return;
-
- mutex_lock(&hdev->mmu_cache_lock);
+ return 0;

if (hdev->pldm)
timeout_usec = GAUDI_PLDM_MMU_TIMEOUT_USEC;
else
timeout_usec = MMU_CONFIG_TIMEOUT_USEC;

+ mutex_lock(&hdev->mmu_cache_lock);
+
/* L0 & L1 invalidation */
WREG32(mmSTLB_INV_PS, 2);

@@ -6006,14 +6006,18 @@ static void gaudi_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,

WREG32(mmSTLB_INV_SET, 0);

- if (rc)
- dev_notice_ratelimited(hdev->dev,
- "Timeout when waiting for MMU cache invalidation\n");
-
mutex_unlock(&hdev->mmu_cache_lock);
+
+ if (rc) {
+ dev_err_ratelimited(hdev->dev,
+ "MMU cache invalidation timeout\n");
+ hl_device_reset(hdev, true, false);
+ }
+
+ return rc;
}

-static void gaudi_mmu_invalidate_cache_range(struct hl_device *hdev,
+static int gaudi_mmu_invalidate_cache_range(struct hl_device *hdev,
bool is_hard, u32 asid, u64 va, u64 size)
{
struct gaudi_device *gaudi = hdev->asic_specific;
@@ -6024,7 +6028,7 @@ static void gaudi_mmu_invalidate_cache_range(struct hl_device *hdev,

if (!(gaudi->hw_cap_initialized & HW_CAP_MMU) ||
hdev->hard_reset_pending)
- return;
+ return 0;

mutex_lock(&hdev->mmu_cache_lock);

@@ -6055,11 +6059,15 @@ static void gaudi_mmu_invalidate_cache_range(struct hl_device *hdev,
1000,
timeout_usec);

- if (rc)
- dev_notice_ratelimited(hdev->dev,
- "Timeout when waiting for MMU cache invalidation\n");
-
mutex_unlock(&hdev->mmu_cache_lock);
+
+ if (rc) {
+ dev_err_ratelimited(hdev->dev,
+ "MMU cache invalidation timeout\n");
+ hl_device_reset(hdev, true, false);
+ }
+
+ return rc;
}

static int gaudi_mmu_update_asid_hop0_addr(struct hl_device *hdev,
diff --git a/drivers/misc/habanalabs/goya/goya.c b/drivers/misc/habanalabs/goya/goya.c
index 152418dfe20c..0d2952bb58df 100644
--- a/drivers/misc/habanalabs/goya/goya.c
+++ b/drivers/misc/habanalabs/goya/goya.c
@@ -4884,7 +4884,7 @@ static void goya_mmu_prepare(struct hl_device *hdev, u32 asid)
goya_mmu_prepare_reg(hdev, goya_mmu_regs[i], asid);
}

-static void goya_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
+static int goya_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
u32 flags)
{
struct goya_device *goya = hdev->asic_specific;
@@ -4893,11 +4893,11 @@ static void goya_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,

if (!(goya->hw_cap_initialized & HW_CAP_MMU) ||
hdev->hard_reset_pending)
- return;
+ return 0;

/* no need in L1 only invalidation in Goya */
if (!is_hard)
- return;
+ return 0;

if (hdev->pldm)
timeout_usec = GOYA_PLDM_MMU_TIMEOUT_USEC;
@@ -4919,13 +4919,17 @@ static void goya_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,

mutex_unlock(&hdev->mmu_cache_lock);

- if (rc)
- dev_notice_ratelimited(hdev->dev,
- "Timeout when waiting for MMU cache invalidation\n");
+ if (rc) {
+ dev_err_ratelimited(hdev->dev,
+ "MMU cache invalidation timeout\n");
+ hl_device_reset(hdev, true, false);
+ }
+
+ return rc;
}

-static void goya_mmu_invalidate_cache_range(struct hl_device *hdev,
- bool is_hard, u32 asid, u64 va, u64 size)
+static int goya_mmu_invalidate_cache_range(struct hl_device *hdev,
+ bool is_hard, u32 asid, u64 va, u64 size)
{
struct goya_device *goya = hdev->asic_specific;
u32 status, timeout_usec, inv_data, pi;
@@ -4933,11 +4937,11 @@ static void goya_mmu_invalidate_cache_range(struct hl_device *hdev,

if (!(goya->hw_cap_initialized & HW_CAP_MMU) ||
hdev->hard_reset_pending)
- return;
+ return 0;

/* no need in L1 only invalidation in Goya */
if (!is_hard)
- return;
+ return 0;

if (hdev->pldm)
timeout_usec = GOYA_PLDM_MMU_TIMEOUT_USEC;
@@ -4970,9 +4974,13 @@ static void goya_mmu_invalidate_cache_range(struct hl_device *hdev,

mutex_unlock(&hdev->mmu_cache_lock);

- if (rc)
- dev_notice_ratelimited(hdev->dev,
- "Timeout when waiting for MMU cache invalidation\n");
+ if (rc) {
+ dev_err_ratelimited(hdev->dev,
+ "MMU cache invalidation timeout\n");
+ hl_device_reset(hdev, true, false);
+ }
+
+ return rc;
}

int goya_send_heartbeat(struct hl_device *hdev)
diff --git a/drivers/misc/habanalabs/habanalabs.h b/drivers/misc/habanalabs/habanalabs.h
index 0f0691875298..1ecdcf8b763a 100644
--- a/drivers/misc/habanalabs/habanalabs.h
+++ b/drivers/misc/habanalabs/habanalabs.h
@@ -675,9 +675,9 @@ struct hl_asic_funcs {
u32 *size);
u64 (*read_pte)(struct hl_device *hdev, u64 addr);
void (*write_pte)(struct hl_device *hdev, u64 addr, u64 val);
- void (*mmu_invalidate_cache)(struct hl_device *hdev, bool is_hard,
+ int (*mmu_invalidate_cache)(struct hl_device *hdev, bool is_hard,
u32 flags);
- void (*mmu_invalidate_cache_range)(struct hl_device *hdev, bool is_hard,
+ int (*mmu_invalidate_cache_range)(struct hl_device *hdev, bool is_hard,
u32 asid, u64 va, u64 size);
int (*send_heartbeat)(struct hl_device *hdev);
void (*enable_clock_gating)(struct hl_device *hdev);
@@ -755,8 +755,8 @@ struct hl_va_range {
* with huge pages.
* @dram_va_range: holds available virtual addresses for DRAM mappings.
* @mem_hash_lock: protects the mem_hash.
- * @mmu_lock: protects the MMU page tables. Any change to the PGT, modifing the
- * MMU hash or walking the PGT requires talking this lock
+ * @mmu_lock: protects the MMU page tables. Any change to the PGT, modifying the
+ * MMU hash or walking the PGT requires talking this lock.
* @debugfs_list: node in debugfs list of contexts.
* @cs_sequence: sequence number for CS. Value is assigned to a CS and passed
* to user so user could inquire about CS. It is used as
diff --git a/drivers/misc/habanalabs/memory.c b/drivers/misc/habanalabs/memory.c
index a72f766ca470..4b8eed1ca513 100644
--- a/drivers/misc/habanalabs/memory.c
+++ b/drivers/misc/habanalabs/memory.c
@@ -886,6 +886,7 @@ static int map_device_va(struct hl_ctx *ctx, struct hl_mem_in *args,

vm_type = (enum vm_type_t *) userptr;
hint_addr = args->map_host.hint_addr;
+ handle = phys_pg_pack->handle;
} else {
handle = lower_32_bits(args->map_device.handle);

@@ -954,10 +955,17 @@ static int map_device_va(struct hl_ctx *ctx, struct hl_mem_in *args,
goto map_err;
}

- hdev->asic_funcs->mmu_invalidate_cache(hdev, false, *vm_type);
+ rc = hdev->asic_funcs->mmu_invalidate_cache(hdev, false, *vm_type);

mutex_unlock(&ctx->mmu_lock);

+ if (rc) {
+ dev_err(hdev->dev,
+ "mapping handle %u failed due to MMU cache invalidation\n",
+ handle);
+ goto map_err;
+ }
+
ret_vaddr += phys_pg_pack->offset;

hnode->ptr = vm_type;
@@ -1083,21 +1091,34 @@ static int unmap_device_va(struct hl_ctx *ctx, u64 vaddr, bool ctx_free)
* at the loop end rather than for each iteration
*/
if (!ctx_free)
- hdev->asic_funcs->mmu_invalidate_cache(hdev, true, *vm_type);
+ rc = hdev->asic_funcs->mmu_invalidate_cache(hdev, true,
+ *vm_type);

mutex_unlock(&ctx->mmu_lock);

/*
- * No point in maintaining the free VA block list if the context is
- * closing as the list will be freed anyway
+ * If the context is closing we don't need to check for the MMU cache
+ * invalidation return code and update the VA free list as in this flow
+ * we invalidate the MMU cache outside of this unmap function and the VA
+ * free list will be freed anyway.
*/
if (!ctx_free) {
- rc = add_va_block(hdev, va_range, vaddr,
- vaddr + phys_pg_pack->total_size - 1);
+ int tmp_rc;
+
if (rc)
+ dev_err(hdev->dev,
+ "unmapping vaddr 0x%llx failed due to MMU cache invalidation\n",
+ vaddr);
+
+ tmp_rc = add_va_block(hdev, va_range, vaddr,
+ vaddr + phys_pg_pack->total_size - 1);
+ if (tmp_rc) {
dev_warn(hdev->dev,
"add va block failed for vaddr: 0x%llx\n",
vaddr);
+ if (!rc)
+ rc = tmp_rc;
+ }
}

atomic_dec(&phys_pg_pack->mapping_cnt);
@@ -1108,7 +1129,7 @@ static int unmap_device_va(struct hl_ctx *ctx, u64 vaddr, bool ctx_free)
dma_unmap_host_va(hdev, userptr);
}

- return 0;
+ return rc;

mapping_cnt_err:
if (is_userptr)
--
2.17.1


2020-05-25 05:29:56

by Oded Gabbay

[permalink] [raw]
Subject: Re: [PATCH] habanalabs: handle MMU cache invalidation timeout

On Sun, May 24, 2020 at 11:07 PM Omer Shpigelman <[email protected]> wrote:
>
> MMU cache invalidation timeout indicates that the device is unstable and
> therefore unusable.
> Hence in such case do hard reset and return an error to the user if was
> called from ioctl.
> In addition, change the print to error level and rephrase its text.
>
> Signed-off-by: Omer Shpigelman <[email protected]>
> ---
> drivers/misc/habanalabs/gaudi/gaudi.c | 36 ++++++++++++++++-----------
> drivers/misc/habanalabs/goya/goya.c | 34 +++++++++++++++----------
> drivers/misc/habanalabs/habanalabs.h | 8 +++---
> drivers/misc/habanalabs/memory.c | 35 ++++++++++++++++++++------
> 4 files changed, 75 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/misc/habanalabs/gaudi/gaudi.c b/drivers/misc/habanalabs/gaudi/gaudi.c
> index 92a5130f06fb..61f88e9884ce 100644
> --- a/drivers/misc/habanalabs/gaudi/gaudi.c
> +++ b/drivers/misc/habanalabs/gaudi/gaudi.c
> @@ -5975,7 +5975,7 @@ static void *gaudi_get_events_stat(struct hl_device *hdev, bool aggregate,
> return gaudi->events_stat;
> }
>
> -static void gaudi_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
> +static int gaudi_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
> u32 flags)
> {
> struct gaudi_device *gaudi = hdev->asic_specific;
> @@ -5984,15 +5984,15 @@ static void gaudi_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
>
> if (!(gaudi->hw_cap_initialized & HW_CAP_MMU) ||
> hdev->hard_reset_pending)
> - return;
> -
> - mutex_lock(&hdev->mmu_cache_lock);
> + return 0;
>
> if (hdev->pldm)
> timeout_usec = GAUDI_PLDM_MMU_TIMEOUT_USEC;
> else
> timeout_usec = MMU_CONFIG_TIMEOUT_USEC;
>
> + mutex_lock(&hdev->mmu_cache_lock);
> +
> /* L0 & L1 invalidation */
> WREG32(mmSTLB_INV_PS, 2);
>
> @@ -6006,14 +6006,18 @@ static void gaudi_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
>
> WREG32(mmSTLB_INV_SET, 0);
>
> - if (rc)
> - dev_notice_ratelimited(hdev->dev,
> - "Timeout when waiting for MMU cache invalidation\n");
> -
> mutex_unlock(&hdev->mmu_cache_lock);
> +
> + if (rc) {
> + dev_err_ratelimited(hdev->dev,
> + "MMU cache invalidation timeout\n");
> + hl_device_reset(hdev, true, false);
> + }
> +
> + return rc;
> }
>
> -static void gaudi_mmu_invalidate_cache_range(struct hl_device *hdev,
> +static int gaudi_mmu_invalidate_cache_range(struct hl_device *hdev,
> bool is_hard, u32 asid, u64 va, u64 size)
> {
> struct gaudi_device *gaudi = hdev->asic_specific;
> @@ -6024,7 +6028,7 @@ static void gaudi_mmu_invalidate_cache_range(struct hl_device *hdev,
>
> if (!(gaudi->hw_cap_initialized & HW_CAP_MMU) ||
> hdev->hard_reset_pending)
> - return;
> + return 0;
>
> mutex_lock(&hdev->mmu_cache_lock);
>
> @@ -6055,11 +6059,15 @@ static void gaudi_mmu_invalidate_cache_range(struct hl_device *hdev,
> 1000,
> timeout_usec);
>
> - if (rc)
> - dev_notice_ratelimited(hdev->dev,
> - "Timeout when waiting for MMU cache invalidation\n");
> -
> mutex_unlock(&hdev->mmu_cache_lock);
> +
> + if (rc) {
> + dev_err_ratelimited(hdev->dev,
> + "MMU cache invalidation timeout\n");
> + hl_device_reset(hdev, true, false);
> + }
> +
> + return rc;
> }
>
> static int gaudi_mmu_update_asid_hop0_addr(struct hl_device *hdev,
> diff --git a/drivers/misc/habanalabs/goya/goya.c b/drivers/misc/habanalabs/goya/goya.c
> index 152418dfe20c..0d2952bb58df 100644
> --- a/drivers/misc/habanalabs/goya/goya.c
> +++ b/drivers/misc/habanalabs/goya/goya.c
> @@ -4884,7 +4884,7 @@ static void goya_mmu_prepare(struct hl_device *hdev, u32 asid)
> goya_mmu_prepare_reg(hdev, goya_mmu_regs[i], asid);
> }
>
> -static void goya_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
> +static int goya_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
> u32 flags)
> {
> struct goya_device *goya = hdev->asic_specific;
> @@ -4893,11 +4893,11 @@ static void goya_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
>
> if (!(goya->hw_cap_initialized & HW_CAP_MMU) ||
> hdev->hard_reset_pending)
> - return;
> + return 0;
>
> /* no need in L1 only invalidation in Goya */
> if (!is_hard)
> - return;
> + return 0;
>
> if (hdev->pldm)
> timeout_usec = GOYA_PLDM_MMU_TIMEOUT_USEC;
> @@ -4919,13 +4919,17 @@ static void goya_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard,
>
> mutex_unlock(&hdev->mmu_cache_lock);
>
> - if (rc)
> - dev_notice_ratelimited(hdev->dev,
> - "Timeout when waiting for MMU cache invalidation\n");
> + if (rc) {
> + dev_err_ratelimited(hdev->dev,
> + "MMU cache invalidation timeout\n");
> + hl_device_reset(hdev, true, false);
> + }
> +
> + return rc;
> }
>
> -static void goya_mmu_invalidate_cache_range(struct hl_device *hdev,
> - bool is_hard, u32 asid, u64 va, u64 size)
> +static int goya_mmu_invalidate_cache_range(struct hl_device *hdev,
> + bool is_hard, u32 asid, u64 va, u64 size)
> {
> struct goya_device *goya = hdev->asic_specific;
> u32 status, timeout_usec, inv_data, pi;
> @@ -4933,11 +4937,11 @@ static void goya_mmu_invalidate_cache_range(struct hl_device *hdev,
>
> if (!(goya->hw_cap_initialized & HW_CAP_MMU) ||
> hdev->hard_reset_pending)
> - return;
> + return 0;
>
> /* no need in L1 only invalidation in Goya */
> if (!is_hard)
> - return;
> + return 0;
>
> if (hdev->pldm)
> timeout_usec = GOYA_PLDM_MMU_TIMEOUT_USEC;
> @@ -4970,9 +4974,13 @@ static void goya_mmu_invalidate_cache_range(struct hl_device *hdev,
>
> mutex_unlock(&hdev->mmu_cache_lock);
>
> - if (rc)
> - dev_notice_ratelimited(hdev->dev,
> - "Timeout when waiting for MMU cache invalidation\n");
> + if (rc) {
> + dev_err_ratelimited(hdev->dev,
> + "MMU cache invalidation timeout\n");
> + hl_device_reset(hdev, true, false);
> + }
> +
> + return rc;
> }
>
> int goya_send_heartbeat(struct hl_device *hdev)
> diff --git a/drivers/misc/habanalabs/habanalabs.h b/drivers/misc/habanalabs/habanalabs.h
> index 0f0691875298..1ecdcf8b763a 100644
> --- a/drivers/misc/habanalabs/habanalabs.h
> +++ b/drivers/misc/habanalabs/habanalabs.h
> @@ -675,9 +675,9 @@ struct hl_asic_funcs {
> u32 *size);
> u64 (*read_pte)(struct hl_device *hdev, u64 addr);
> void (*write_pte)(struct hl_device *hdev, u64 addr, u64 val);
> - void (*mmu_invalidate_cache)(struct hl_device *hdev, bool is_hard,
> + int (*mmu_invalidate_cache)(struct hl_device *hdev, bool is_hard,
> u32 flags);
> - void (*mmu_invalidate_cache_range)(struct hl_device *hdev, bool is_hard,
> + int (*mmu_invalidate_cache_range)(struct hl_device *hdev, bool is_hard,
> u32 asid, u64 va, u64 size);
> int (*send_heartbeat)(struct hl_device *hdev);
> void (*enable_clock_gating)(struct hl_device *hdev);
> @@ -755,8 +755,8 @@ struct hl_va_range {
> * with huge pages.
> * @dram_va_range: holds available virtual addresses for DRAM mappings.
> * @mem_hash_lock: protects the mem_hash.
> - * @mmu_lock: protects the MMU page tables. Any change to the PGT, modifing the
> - * MMU hash or walking the PGT requires talking this lock
> + * @mmu_lock: protects the MMU page tables. Any change to the PGT, modifying the
> + * MMU hash or walking the PGT requires talking this lock.
> * @debugfs_list: node in debugfs list of contexts.
> * @cs_sequence: sequence number for CS. Value is assigned to a CS and passed
> * to user so user could inquire about CS. It is used as
> diff --git a/drivers/misc/habanalabs/memory.c b/drivers/misc/habanalabs/memory.c
> index a72f766ca470..4b8eed1ca513 100644
> --- a/drivers/misc/habanalabs/memory.c
> +++ b/drivers/misc/habanalabs/memory.c
> @@ -886,6 +886,7 @@ static int map_device_va(struct hl_ctx *ctx, struct hl_mem_in *args,
>
> vm_type = (enum vm_type_t *) userptr;
> hint_addr = args->map_host.hint_addr;
> + handle = phys_pg_pack->handle;
> } else {
> handle = lower_32_bits(args->map_device.handle);
>
> @@ -954,10 +955,17 @@ static int map_device_va(struct hl_ctx *ctx, struct hl_mem_in *args,
> goto map_err;
> }
>
> - hdev->asic_funcs->mmu_invalidate_cache(hdev, false, *vm_type);
> + rc = hdev->asic_funcs->mmu_invalidate_cache(hdev, false, *vm_type);
>
> mutex_unlock(&ctx->mmu_lock);
>
> + if (rc) {
> + dev_err(hdev->dev,
> + "mapping handle %u failed due to MMU cache invalidation\n",
> + handle);
> + goto map_err;
> + }
> +
> ret_vaddr += phys_pg_pack->offset;
>
> hnode->ptr = vm_type;
> @@ -1083,21 +1091,34 @@ static int unmap_device_va(struct hl_ctx *ctx, u64 vaddr, bool ctx_free)
> * at the loop end rather than for each iteration
> */
> if (!ctx_free)
> - hdev->asic_funcs->mmu_invalidate_cache(hdev, true, *vm_type);
> + rc = hdev->asic_funcs->mmu_invalidate_cache(hdev, true,
> + *vm_type);
>
> mutex_unlock(&ctx->mmu_lock);
>
> /*
> - * No point in maintaining the free VA block list if the context is
> - * closing as the list will be freed anyway
> + * If the context is closing we don't need to check for the MMU cache
> + * invalidation return code and update the VA free list as in this flow
> + * we invalidate the MMU cache outside of this unmap function and the VA
> + * free list will be freed anyway.
> */
> if (!ctx_free) {
> - rc = add_va_block(hdev, va_range, vaddr,
> - vaddr + phys_pg_pack->total_size - 1);
> + int tmp_rc;
> +
> if (rc)
> + dev_err(hdev->dev,
> + "unmapping vaddr 0x%llx failed due to MMU cache invalidation\n",
> + vaddr);
> +
> + tmp_rc = add_va_block(hdev, va_range, vaddr,
> + vaddr + phys_pg_pack->total_size - 1);
> + if (tmp_rc) {
> dev_warn(hdev->dev,
> "add va block failed for vaddr: 0x%llx\n",
> vaddr);
> + if (!rc)
> + rc = tmp_rc;
> + }
> }
>
> atomic_dec(&phys_pg_pack->mapping_cnt);
> @@ -1108,7 +1129,7 @@ static int unmap_device_va(struct hl_ctx *ctx, u64 vaddr, bool ctx_free)
> dma_unmap_host_va(hdev, userptr);
> }
>
> - return 0;
> + return rc;
>
> mapping_cnt_err:
> if (is_userptr)
> --
> 2.17.1
>

This patch is:
Reviewed-by: Oded Gabbay <[email protected]>