[PATCH 1/6] accel/habanalabs: unmap mapped memory when TLB inv fails

[PATCH 1/6] accel/habanalabs: unmap mapped memory when TLB inv fails

[Oded Gabbay]

From: Koby Elbaz <kelbaz@habana.ai>

Once a memory mapping is added to the page tables, it's followed by
a TLB invalidation request which could potentially fail (HW failure).
Removing the mapping is simply a part of this failure handling routine.
TLB invalidation failure prints were updated to be more accurate.

Signed-off-by: Koby Elbaz <kelbaz@habana.ai>
Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
---
 drivers/accel/habanalabs/common/command_buffer.c | 15 ++++++++++++---
 drivers/accel/habanalabs/common/mmu/mmu.c        |  8 ++++++--
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/accel/habanalabs/common/command_buffer.c b/drivers/accel/habanalabs/common/command_buffer.c
index 3a0535ac28b1..6e09f48750a0 100644
--- a/drivers/accel/habanalabs/common/command_buffer.c
+++ b/drivers/accel/habanalabs/common/command_buffer.c
@@ -45,20 +45,29 @@ static int cb_map_mem(struct hl_ctx *ctx, struct hl_cb *cb)
 	}
 
 	mutex_lock(&hdev->mmu_lock);
+
 	rc = hl_mmu_map_contiguous(ctx, cb->virtual_addr, cb->bus_address, cb->roundup_size);
 	if (rc) {
 		dev_err(hdev->dev, "Failed to map VA %#llx to CB\n", cb->virtual_addr);
-		goto err_va_umap;
+		goto err_va_pool_free;
 	}
+
 	rc = hl_mmu_invalidate_cache(hdev, false, MMU_OP_USERPTR | MMU_OP_SKIP_LOW_CACHE_INV);
+	if (rc)
+		goto err_mmu_unmap;
+
 	mutex_unlock(&hdev->mmu_lock);
 
 	cb->is_mmu_mapped = true;
-	return rc;
 
-err_va_umap:
+	return 0;
+
+err_mmu_unmap:
+	hl_mmu_unmap_contiguous(ctx, cb->virtual_addr, cb->roundup_size);
+err_va_pool_free:
 	mutex_unlock(&hdev->mmu_lock);
 	gen_pool_free(ctx->cb_va_pool, cb->virtual_addr, cb->roundup_size);
+
 	return rc;
 }
 
diff --git a/drivers/accel/habanalabs/common/mmu/mmu.c b/drivers/accel/habanalabs/common/mmu/mmu.c
index 17581b1bcc77..f379e5b461a6 100644
--- a/drivers/accel/habanalabs/common/mmu/mmu.c
+++ b/drivers/accel/habanalabs/common/mmu/mmu.c
@@ -679,7 +679,9 @@ int hl_mmu_invalidate_cache(struct hl_device *hdev, bool is_hard, u32 flags)
 
 	rc = hdev->asic_funcs->mmu_invalidate_cache(hdev, is_hard, flags);
 	if (rc)
-		dev_err_ratelimited(hdev->dev, "MMU cache invalidation failed\n");
+		dev_err_ratelimited(hdev->dev,
+				"%s cache invalidation failed, rc=%d\n",
+				flags == VM_TYPE_USERPTR ? "PMMU" : "HMMU", rc);
 
 	return rc;
 }
@@ -692,7 +694,9 @@ int hl_mmu_invalidate_cache_range(struct hl_device *hdev, bool is_hard,
 	rc = hdev->asic_funcs->mmu_invalidate_cache_range(hdev, is_hard, flags,
 								asid, va, size);
 	if (rc)
-		dev_err_ratelimited(hdev->dev, "MMU cache range invalidation failed\n");
+		dev_err_ratelimited(hdev->dev,
+				"%s cache range invalidation failed: va=%#llx, size=%llu, rc=%d",
+				flags == VM_TYPE_USERPTR ? "PMMU" : "HMMU", va, size, rc);
 
 	return rc;
 }
-- 
2.40.0

[PATCH 2/6] accel/habanalabs: print event type when device is disabled

[Oded Gabbay]

From: Tal Cohen <talcohen@habana.ai>

When the device is in disabled state, the driver isn't suppose to
receive any events from FW. Printing the event type, as part of the
message that was already printed, shall help to get more info if this
unexpected message is received.

Signed-off-by: Tal Cohen <talcohen@habana.ai>
Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
---
 drivers/accel/habanalabs/common/irq.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/accel/habanalabs/common/irq.c b/drivers/accel/habanalabs/common/irq.c
index fab1abc5c910..0d59bb7c9063 100644
--- a/drivers/accel/habanalabs/common/irq.c
+++ b/drivers/accel/habanalabs/common/irq.c
@@ -415,8 +415,8 @@ irqreturn_t hl_irq_handler_eq(int irq, void *arg)
 	struct hl_eq_entry *eq_base;
 	struct hl_eqe_work *handle_eqe_work;
 	bool entry_ready;
-	u32 cur_eqe;
-	u16 cur_eqe_index;
+	u32 cur_eqe, ctl;
+	u16 cur_eqe_index, event_type;
 
 	eq_base = eq->kernel_address;
 
@@ -449,7 +449,10 @@ irqreturn_t hl_irq_handler_eq(int irq, void *arg)
 		dma_rmb();
 
 		if (hdev->disabled && !hdev->reset_info.in_compute_reset) {
-			dev_warn(hdev->dev, "Device disabled but received an EQ event\n");
+			ctl = le32_to_cpu(eq_entry->hdr.ctl);
+			event_type = ((ctl & EQ_CTL_EVENT_TYPE_MASK) >> EQ_CTL_EVENT_TYPE_SHIFT);
+			dev_warn(hdev->dev,
+				"Device disabled but received an EQ event (%u)\n", event_type);
 			goto skip_irq;
 		}
 
-- 
2.40.0

[PATCH 3/6] accel/habanalabs: check return value of add_va_block_locked

[Oded Gabbay]

From: Dafna Hirschfeld <dhirschfeld@habana.ai>

since the function might fail and we should propagate the failure.

Signed-off-by: Dafna Hirschfeld <dhirschfeld@habana.ai>
Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
---
 drivers/accel/habanalabs/common/memory.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/accel/habanalabs/common/memory.c b/drivers/accel/habanalabs/common/memory.c
index 17b79d717896..a7b6a273ce21 100644
--- a/drivers/accel/habanalabs/common/memory.c
+++ b/drivers/accel/habanalabs/common/memory.c
@@ -605,6 +605,7 @@ static u64 get_va_block(struct hl_device *hdev,
 	bool is_align_pow_2  = is_power_of_2(va_range->page_size);
 	bool is_hint_dram_addr = hl_is_dram_va(hdev, hint_addr);
 	bool force_hint = flags & HL_MEM_FORCE_HINT;
+	int rc;
 
 	if (is_align_pow_2)
 		align_mask = ~((u64)va_block_align - 1);
@@ -722,9 +723,13 @@ static u64 get_va_block(struct hl_device *hdev,
 		kfree(new_va_block);
 	}
 
-	if (add_prev)
-		add_va_block_locked(hdev, &va_range->list, prev_start,
-				prev_end);
+	if (add_prev) {
+		rc = add_va_block_locked(hdev, &va_range->list, prev_start, prev_end);
+		if (rc) {
+			reserved_valid_start = 0;
+			goto out;
+		}
+	}
 
 	print_va_list_locked(hdev, &va_range->list);
 out:
-- 
2.40.0

[PATCH 4/6] accel/habanalabs: change COMMS warning messages to error level

[Oded Gabbay]

From: Koby Elbaz <kelbaz@habana.ai>

COMMS protocol is used for LKD <--> FW communication, and any
communication failure between the two might turn out to be
destructive, hence, it should be well emphasized.

Signed-off-by: Koby Elbaz <kelbaz@habana.ai>
Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
---
 drivers/accel/habanalabs/common/firmware_if.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/habanalabs/common/firmware_if.c b/drivers/accel/habanalabs/common/firmware_if.c
index 7ea611392f8c..96027a1c124d 100644
--- a/drivers/accel/habanalabs/common/firmware_if.c
+++ b/drivers/accel/habanalabs/common/firmware_if.c
@@ -1263,7 +1263,7 @@ void hl_fw_ask_hard_reset_without_linux(struct hl_device *hdev)
 				COMMS_RST_DEV, 0, false,
 				hdev->fw_loader.cpu_timeout);
 		if (rc)
-			dev_warn(hdev->dev, "Failed sending COMMS_RST_DEV\n");
+			dev_err(hdev->dev, "Failed sending COMMS_RST_DEV\n");
 	} else {
 		WREG32(static_loader->kmd_msg_to_cpu_reg, KMD_MSG_RST_DEV);
 	}
@@ -1284,7 +1284,7 @@ void hl_fw_ask_halt_machine_without_linux(struct hl_device *hdev)
 				COMMS_GOTO_WFE, 0, true,
 				hdev->fw_loader.cpu_timeout);
 		if (rc)
-			dev_warn(hdev->dev, "Failed sending COMMS_GOTO_WFE\n");
+			dev_err(hdev->dev, "Failed sending COMMS_GOTO_WFE\n");
 	} else {
 		WREG32(static_loader->kmd_msg_to_cpu_reg, KMD_MSG_GOTO_WFE);
 		msleep(static_loader->cpu_reset_wait_msec);
-- 
2.40.0

[PATCH 5/6] accel/habanalabs: remove duplicated disable pci msg

[Oded Gabbay]

From: Tal Cohen <talcohen@habana.ai>

The disable pci message is sent in reset device. It informs the FW not
to raise more EQs. The Driver may ignore received EQs, when the device
is in disabled mode.
The duplication happens when hard reset is scheduled during compute
reset and also performs 'escalate_reset_flow'.

Signed-off-by: Tal Cohen <talcohen@habana.ai>
Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
---
 drivers/accel/habanalabs/common/device.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/accel/habanalabs/common/device.c b/drivers/accel/habanalabs/common/device.c
index 2fb1e2ec3a83..c36de13d6729 100644
--- a/drivers/accel/habanalabs/common/device.c
+++ b/drivers/accel/habanalabs/common/device.c
@@ -1822,9 +1822,7 @@ int hl_device_reset(struct hl_device *hdev, u32 flags)
 			dev_info(hdev->dev, "Performing hard reset scheduled during compute reset\n");
 			flags = hdev->reset_info.hard_reset_schedule_flags;
 			hdev->reset_info.hard_reset_schedule_flags = 0;
-			hdev->disabled = true;
 			hard_reset = true;
-			handle_reset_trigger(hdev, flags);
 			goto escalate_reset_flow;
 		}
 	}
-- 
2.40.0

[PATCH 6/6] accel/habanalabs: send disable pci when compute ctx is active

[Oded Gabbay]

From: Tal Cohen <talcohen@habana.ai>

Fix an issue in hard reset flow in which the driver didn't send a
disable pci message if there was an active compute context.
In hard reset, disable pci message should be sent no matter if
a compute context exists or not.

Signed-off-by: Tal Cohen <talcohen@habana.ai>
Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
---
 drivers/accel/habanalabs/common/device.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/habanalabs/common/device.c b/drivers/accel/habanalabs/common/device.c
index c36de13d6729..3c1af9d43b65 100644
--- a/drivers/accel/habanalabs/common/device.c
+++ b/drivers/accel/habanalabs/common/device.c
@@ -1386,7 +1386,7 @@ static void handle_reset_trigger(struct hl_device *hdev, u32 flags)
 
 	/* No consecutive mechanism when user context exists */
 	if (hdev->is_compute_ctx_active)
-		return;
+		goto disable_pci;
 
 	/*
 	 * 'reset cause' is being updated here, because getting here
@@ -1425,6 +1425,8 @@ static void handle_reset_trigger(struct hl_device *hdev, u32 flags)
 	 * If F/W is performing the reset, no need to send it a message to disable
 	 * PCI access
 	 */
+
+disable_pci:
 	if ((flags & HL_DRV_RESET_HARD) &&
 			!(flags & (HL_DRV_RESET_HEARTBEAT | HL_DRV_RESET_BYPASS_REQ_TO_FW))) {
 		/* Disable PCI access from device F/W so he won't send
-- 
2.40.0

Re: [PATCH 1/6] accel/habanalabs: unmap mapped memory when TLB inv fails

[Stanislaw Gruszka]

On Thu, Mar 23, 2023 at 01:35:20PM +0200, Oded Gabbay wrote:
> From: Koby Elbaz <kelbaz@habana.ai>
> 
> Once a memory mapping is added to the page tables, it's followed by
> a TLB invalidation request which could potentially fail (HW failure).
> Removing the mapping is simply a part of this failure handling routine.
> TLB invalidation failure prints were updated to be more accurate.
> 
> Signed-off-by: Koby Elbaz <kelbaz@habana.ai>
> Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
> Signed-off-by: Oded Gabbay <ogabbay@kernel.org>

Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

>  	rc = hl_mmu_invalidate_cache(hdev, false, MMU_OP_USERPTR | MMU_OP_SKIP_LOW_CACHE_INV);
> +	if (rc)
> +		goto err_mmu_unmap;

BTW: all other calls of that function ignore the error. Assume that is fine.

Regards
Stanislaw

Re: [PATCH 2/6] accel/habanalabs: print event type when device is disabled

[Stanislaw Gruszka]

On Thu, Mar 23, 2023 at 01:35:21PM +0200, Oded Gabbay wrote:
> From: Tal Cohen <talcohen@habana.ai>
> 
> When the device is in disabled state, the driver isn't suppose to
> receive any events from FW. Printing the event type, as part of the
> message that was already printed, shall help to get more info if this
> unexpected message is received.
> 
> Signed-off-by: Tal Cohen <talcohen@habana.ai>
> Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
> Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

Re: [PATCH 3/6] accel/habanalabs: check return value of add_va_block_locked

[Stanislaw Gruszka]

On Thu, Mar 23, 2023 at 01:35:22PM +0200, Oded Gabbay wrote:
> From: Dafna Hirschfeld <dhirschfeld@habana.ai>
> 
> since the function might fail and we should propagate the failure.
> 
> Signed-off-by: Dafna Hirschfeld <dhirschfeld@habana.ai>
> Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
> Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

Re: [PATCH 4/6] accel/habanalabs: change COMMS warning messages to error level

[Stanislaw Gruszka]

On Thu, Mar 23, 2023 at 01:35:23PM +0200, Oded Gabbay wrote:
> From: Koby Elbaz <kelbaz@habana.ai>
> 
> COMMS protocol is used for LKD <--> FW communication, and any
> communication failure between the two might turn out to be
> destructive, hence, it should be well emphasized.
> 
> Signed-off-by: Koby Elbaz <kelbaz@habana.ai>
> Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
> Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

Re: [PATCH 5/6] accel/habanalabs: remove duplicated disable pci msg

[Stanislaw Gruszka]

On Thu, Mar 23, 2023 at 01:35:24PM +0200, Oded Gabbay wrote:
> From: Tal Cohen <talcohen@habana.ai>
> 
> The disable pci message is sent in reset device. It informs the FW not
> to raise more EQs. The Driver may ignore received EQs, when the device
> is in disabled mode.
> The duplication happens when hard reset is scheduled during compute
> reset and also performs 'escalate_reset_flow'.
> 
> Signed-off-by: Tal Cohen <talcohen@habana.ai>
> Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
> Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

Re: [PATCH 6/6] accel/habanalabs: send disable pci when compute ctx is active

[Stanislaw Gruszka]

On Thu, Mar 23, 2023 at 01:35:25PM +0200, Oded Gabbay wrote:
> From: Tal Cohen <talcohen@habana.ai>
> 
> Fix an issue in hard reset flow in which the driver didn't send a
> disable pci message if there was an active compute context.
> In hard reset, disable pci message should be sent no matter if
> a compute context exists or not.
> 
> Signed-off-by: Tal Cohen <talcohen@habana.ai>
> Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
> Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>