* [PATCH 2/6] habanalabs: fix debugfs device memory MMU VA translation
2021-10-04 10:08 [PATCH 1/6] habanalabs: add support for a long interrupt target value Oded Gabbay
@ 2021-10-04 10:08 ` Oded Gabbay
2021-10-04 10:08 ` [PATCH 3/6] habanalabs: define soft-reset as inference op Oded Gabbay
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Oded Gabbay @ 2021-10-04 10:08 UTC (permalink / raw)
To: linux-kernel; +Cc: Yuri Nudelman
From: Yuri Nudelman <ynudelman@habana.ai>
The translation in debugfs of device memory MMU virtual addresses was
wrong as it did not take into consideration the fact that the page
sizes there can be not power of 2.
Signed-off-by: Yuri Nudelman <ynudelman@habana.ai>
Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
---
drivers/misc/habanalabs/common/mmu/mmu.c | 30 +++++++++++++-----------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/habanalabs/common/mmu/mmu.c b/drivers/misc/habanalabs/common/mmu/mmu.c
index 792d25b79ea6..aa96917f62e5 100644
--- a/drivers/misc/habanalabs/common/mmu/mmu.c
+++ b/drivers/misc/habanalabs/common/mmu/mmu.c
@@ -501,23 +501,25 @@ static void hl_mmu_pa_page_with_offset(struct hl_ctx *ctx, u64 virt_addr,
if ((hops->range_type == HL_VA_RANGE_TYPE_DRAM) &&
!is_power_of_2(prop->dram_page_size)) {
- unsigned long dram_page_size = prop->dram_page_size;
- u64 page_offset_mask;
- u64 phys_addr_mask;
- u32 bit;
+ u64 dram_page_size, dram_base, abs_phys_addr, abs_virt_addr,
+ page_id, page_start;
+ u32 page_off;
/*
- * find last set bit in page_size to cover all bits of page
- * offset. note that 1 has to be added to bit index.
- * note that the internal ulong variable is used to avoid
- * alignment issue.
+ * Bit arithmetics cannot be used for non power of two page
+ * sizes. In addition, since bit arithmetics is not used,
+ * we cannot ignore dram base. All that shall be considerd.
*/
- bit = find_last_bit(&dram_page_size,
- sizeof(dram_page_size) * BITS_PER_BYTE) + 1;
- page_offset_mask = (BIT_ULL(bit) - 1);
- phys_addr_mask = ~page_offset_mask;
- *phys_addr = (tmp_phys_addr & phys_addr_mask) |
- (virt_addr & page_offset_mask);
+
+ dram_page_size = prop->dram_page_size;
+ dram_base = prop->dram_base_address;
+ abs_phys_addr = tmp_phys_addr - dram_base;
+ abs_virt_addr = virt_addr - dram_base;
+ page_id = DIV_ROUND_DOWN_ULL(abs_phys_addr, dram_page_size);
+ page_start = page_id * dram_page_size;
+ div_u64_rem(abs_virt_addr, dram_page_size, &page_off);
+
+ *phys_addr = page_start + page_off + dram_base;
} else {
/*
* find the correct hop shift field in hl_mmu_properties
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/6] habanalabs: define soft-reset as inference op
2021-10-04 10:08 [PATCH 1/6] habanalabs: add support for a long interrupt target value Oded Gabbay
2021-10-04 10:08 ` [PATCH 2/6] habanalabs: fix debugfs device memory MMU VA translation Oded Gabbay
@ 2021-10-04 10:08 ` Oded Gabbay
2021-10-04 10:08 ` [PATCH 4/6] habanalabs: refactor reset log message Oded Gabbay
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Oded Gabbay @ 2021-10-04 10:08 UTC (permalink / raw)
To: linux-kernel
Soft-reset is the procedure where we reset only the compute/DMA engines
of the device, without requiring the current user-space process to
release the device.
This type of reset can happen if TDR event occurred (a workload got
stuck) or by a root request through sysfs.
This is only relevant for inference ASICs, as there is no real-world
use-case to do that in training, because training runs on multiple
devices.
In addition, we also do (in certain ASICs) a reset upon device release.
That reset uses the same code as the soft-reset.
Therefore, to better differentiate between the two resets, it is better
to rename the soft-reset support as "inference soft-reset", to make
the code more self-explanatory.
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
---
drivers/misc/habanalabs/common/device.c | 2 +-
drivers/misc/habanalabs/common/habanalabs.h | 10 +++++++---
drivers/misc/habanalabs/common/sysfs.c | 6 +++---
drivers/misc/habanalabs/goya/goya.c | 2 +-
4 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/misc/habanalabs/common/device.c b/drivers/misc/habanalabs/common/device.c
index 97c7c86580e6..8f96f2bb14ec 100644
--- a/drivers/misc/habanalabs/common/device.c
+++ b/drivers/misc/habanalabs/common/device.c
@@ -954,7 +954,7 @@ int hl_device_reset(struct hl_device *hdev, u32 flags)
goto do_reset;
}
- if (!hard_reset && !hdev->allow_external_soft_reset) {
+ if (!hard_reset && !hdev->allow_inference_soft_reset) {
hard_instead_soft = true;
hard_reset = true;
}
diff --git a/drivers/misc/habanalabs/common/habanalabs.h b/drivers/misc/habanalabs/common/habanalabs.h
index f8e23ca18a57..99de80915eed 100644
--- a/drivers/misc/habanalabs/common/habanalabs.h
+++ b/drivers/misc/habanalabs/common/habanalabs.h
@@ -2440,8 +2440,12 @@ struct multi_cs_data {
* @collective_mon_idx: helper index for collective initialization
* @supports_coresight: is CoreSight supported.
* @supports_soft_reset: is soft reset supported.
- * @allow_external_soft_reset: true if soft reset initiated by user or TDR is
- * allowed.
+ * @allow_inference_soft_reset: true if the ASIC supports soft reset that is
+ * initiated by user or TDR. This is only true
+ * in inference ASICs, as there is no real-world
+ * use-case of doing soft-reset in training (due
+ * to the fact that training runs on multiple
+ * devices)
* @supports_cb_mapping: is mapping a CB to the device's MMU supported.
* @needs_reset: true if reset_on_lockup is false and device should be reset
* due to lockup.
@@ -2572,7 +2576,7 @@ struct hl_device {
u8 collective_mon_idx;
u8 supports_coresight;
u8 supports_soft_reset;
- u8 allow_external_soft_reset;
+ u8 allow_inference_soft_reset;
u8 supports_cb_mapping;
u8 needs_reset;
u8 process_kill_trial_cnt;
diff --git a/drivers/misc/habanalabs/common/sysfs.c b/drivers/misc/habanalabs/common/sysfs.c
index 34f9f2779962..42c1769ad25d 100644
--- a/drivers/misc/habanalabs/common/sysfs.c
+++ b/drivers/misc/habanalabs/common/sysfs.c
@@ -206,12 +206,12 @@ static ssize_t soft_reset_store(struct device *dev,
goto out;
}
- if (!hdev->allow_external_soft_reset) {
- dev_err(hdev->dev, "Device does not support soft-reset\n");
+ if (!hdev->allow_inference_soft_reset) {
+ dev_err(hdev->dev, "Device does not support inference soft-reset\n");
goto out;
}
- dev_warn(hdev->dev, "Soft-Reset requested through sysfs\n");
+ dev_warn(hdev->dev, "Inference Soft-Reset requested through sysfs\n");
hl_device_reset(hdev, 0);
diff --git a/drivers/misc/habanalabs/goya/goya.c b/drivers/misc/habanalabs/goya/goya.c
index 031c1849da14..ef67e2586ede 100644
--- a/drivers/misc/habanalabs/goya/goya.c
+++ b/drivers/misc/habanalabs/goya/goya.c
@@ -959,7 +959,7 @@ static int goya_sw_init(struct hl_device *hdev)
spin_lock_init(&goya->hw_queues_lock);
hdev->supports_coresight = true;
hdev->supports_soft_reset = true;
- hdev->allow_external_soft_reset = true;
+ hdev->allow_inference_soft_reset = true;
hdev->supports_wait_for_multi_cs = false;
hdev->asic_funcs->set_pci_memory_regions(hdev);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/6] habanalabs: refactor reset log message
2021-10-04 10:08 [PATCH 1/6] habanalabs: add support for a long interrupt target value Oded Gabbay
2021-10-04 10:08 ` [PATCH 2/6] habanalabs: fix debugfs device memory MMU VA translation Oded Gabbay
2021-10-04 10:08 ` [PATCH 3/6] habanalabs: define soft-reset as inference op Oded Gabbay
@ 2021-10-04 10:08 ` Oded Gabbay
2021-10-04 10:08 ` [PATCH 5/6] habanalabs: prevent race between fd close/open Oded Gabbay
2021-10-04 10:08 ` [PATCH 6/6] habanalabs: take timestamp on wait for interrupt Oded Gabbay
4 siblings, 0 replies; 6+ messages in thread
From: Oded Gabbay @ 2021-10-04 10:08 UTC (permalink / raw)
To: linux-kernel
Reset to the device is not necessarily due to an error, so print it
as info instead of error.
In addition, print the type of reset we are doing:
- reset of the entire device (aka hard reset)
- reset of the device after user have released it (less than hard reset)
- lighter reset of an inference device (aka soft reset)
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
---
drivers/misc/habanalabs/common/device.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/habanalabs/common/device.c b/drivers/misc/habanalabs/common/device.c
index 8f96f2bb14ec..be18ad0c1bfc 100644
--- a/drivers/misc/habanalabs/common/device.c
+++ b/drivers/misc/habanalabs/common/device.c
@@ -1018,7 +1018,14 @@ int hl_device_reset(struct hl_device *hdev, u32 flags)
take_release_locks(hdev);
- dev_err(hdev->dev, "Going to RESET device!\n");
+ if (hard_reset)
+ dev_info(hdev->dev, "Going to reset device\n");
+ else if (flags & HL_RESET_DEVICE_RELEASE)
+ dev_info(hdev->dev,
+ "Going to reset device after it was released by user\n");
+ else
+ dev_info(hdev->dev,
+ "Going to reset compute engines of inference device\n");
}
again:
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 5/6] habanalabs: prevent race between fd close/open
2021-10-04 10:08 [PATCH 1/6] habanalabs: add support for a long interrupt target value Oded Gabbay
` (2 preceding siblings ...)
2021-10-04 10:08 ` [PATCH 4/6] habanalabs: refactor reset log message Oded Gabbay
@ 2021-10-04 10:08 ` Oded Gabbay
2021-10-04 10:08 ` [PATCH 6/6] habanalabs: take timestamp on wait for interrupt Oded Gabbay
4 siblings, 0 replies; 6+ messages in thread
From: Oded Gabbay @ 2021-10-04 10:08 UTC (permalink / raw)
To: linux-kernel
The driver allows only a single process to open a device's FD at any
single time. This is done by checking "hdev->compute_ctx" under mutex.
Therefore, to prevent a race between the moment a user closes it's FD
and when another user tries to open the device, we need to make sure
that clearing this variable is the very last thing that is done in the
code of the FD's release.
I'm moving the idle check before clearing this variable and the
"reset on device release". btw, if the reset happens it will prevent
any other user from opening the device until the reset is finished.
An important thing to note is that we need to remove the user process
that is closing the device from the process list BEFORE calling the
reset function. That is to prevent a case where the reset code will
try to kill that user process and it is unnecessary as the process
doesn't hold any device/driver resources anymore.
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
---
drivers/misc/habanalabs/common/device.c | 30 +++++++++++++++++++------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/drivers/misc/habanalabs/common/device.c b/drivers/misc/habanalabs/common/device.c
index be18ad0c1bfc..e1949b087ae3 100644
--- a/drivers/misc/habanalabs/common/device.c
+++ b/drivers/misc/habanalabs/common/device.c
@@ -69,13 +69,6 @@ static void hpriv_release(struct kref *ref)
mutex_destroy(&hpriv->restore_phase_mutex);
- mutex_lock(&hdev->fpriv_list_lock);
- list_del(&hpriv->dev_node);
- hdev->compute_ctx = NULL;
- mutex_unlock(&hdev->fpriv_list_lock);
-
- kfree(hpriv);
-
if ((!hdev->pldm) && (hdev->pdev) &&
(!hdev->asic_funcs->is_device_idle(hdev,
idle_mask,
@@ -87,9 +80,32 @@ static void hpriv_release(struct kref *ref)
device_is_idle = false;
}
+ /* We need to remove the user from the list to make sure the reset process won't
+ * try to kill the user process. Because, if we got here, it means there are no
+ * more driver/device resources that the user process is occupying so there is
+ * no need to kill it
+ *
+ * However, we can't set the compute_ctx to NULL at this stage. This is to prevent
+ * a race between the release and opening the device again. We don't want to let
+ * a user open the device while there a reset is about to happen.
+ */
+ mutex_lock(&hdev->fpriv_list_lock);
+ list_del(&hpriv->dev_node);
+ mutex_unlock(&hdev->fpriv_list_lock);
+
if ((hdev->reset_if_device_not_idle && !device_is_idle)
|| hdev->reset_upon_device_release)
hl_device_reset(hdev, HL_RESET_DEVICE_RELEASE);
+
+ /* Now we can mark the compute_ctx as empty. Even if a reset is running in a different
+ * thread, we don't care because the in_reset is marked so if a user will try to open
+ * the device it will fail on that, even if compute_ctx is NULL.
+ */
+ mutex_lock(&hdev->fpriv_list_lock);
+ hdev->compute_ctx = NULL;
+ mutex_unlock(&hdev->fpriv_list_lock);
+
+ kfree(hpriv);
}
void hl_hpriv_get(struct hl_fpriv *hpriv)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 6/6] habanalabs: take timestamp on wait for interrupt
2021-10-04 10:08 [PATCH 1/6] habanalabs: add support for a long interrupt target value Oded Gabbay
` (3 preceding siblings ...)
2021-10-04 10:08 ` [PATCH 5/6] habanalabs: prevent race between fd close/open Oded Gabbay
@ 2021-10-04 10:08 ` Oded Gabbay
4 siblings, 0 replies; 6+ messages in thread
From: Oded Gabbay @ 2021-10-04 10:08 UTC (permalink / raw)
To: linux-kernel; +Cc: Yuri Nudelman
From: Yuri Nudelman <ynudelman@habana.ai>
Taking an accurate timestamp in a close proximity of the interrupt is
required for user side statistics management.
Signed-off-by: Yuri Nudelman <ynudelman@habana.ai>
Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
---
.../misc/habanalabs/common/command_submission.c | 17 +++++++++++++++--
drivers/misc/habanalabs/common/habanalabs.h | 2 ++
drivers/misc/habanalabs/common/irq.c | 5 ++++-
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/habanalabs/common/command_submission.c b/drivers/misc/habanalabs/common/command_submission.c
index 44bab01cd033..4bc24852a283 100644
--- a/drivers/misc/habanalabs/common/command_submission.c
+++ b/drivers/misc/habanalabs/common/command_submission.c
@@ -2740,7 +2740,8 @@ static int hl_cs_wait_ioctl(struct hl_fpriv *hpriv, void *data)
static int _hl_interrupt_wait_ioctl(struct hl_device *hdev, struct hl_ctx *ctx,
u32 timeout_us, u64 user_address,
u64 target_value, u16 interrupt_offset,
- enum hl_cs_wait_status *status)
+ enum hl_cs_wait_status *status,
+ bool take_timestamp, u64 *timestamp)
{
struct hl_user_pending_interrupt *pend;
struct hl_user_interrupt *interrupt;
@@ -2764,6 +2765,8 @@ static int _hl_interrupt_wait_ioctl(struct hl_device *hdev, struct hl_ctx *ctx,
hl_fence_init(&pend->fence, ULONG_MAX);
+ pend->fence.take_timestamp = take_timestamp;
+
if (interrupt_offset == HL_COMMON_USER_INTERRUPT_ID)
interrupt = &hdev->common_user_interrupt;
else
@@ -2838,6 +2841,8 @@ static int _hl_interrupt_wait_ioctl(struct hl_device *hdev, struct hl_ctx *ctx,
list_del(&pend->wait_list_node);
spin_unlock_irqrestore(&interrupt->wait_list_lock, flags);
+ *timestamp = ktime_to_ns(pend->fence.timestamp);
+
kfree(pend);
hl_ctx_put(ctx);
@@ -2851,6 +2856,7 @@ static int hl_interrupt_wait_ioctl(struct hl_fpriv *hpriv, void *data)
struct asic_fixed_properties *prop;
union hl_wait_cs_args *args = data;
enum hl_cs_wait_status status;
+ u64 timestamp;
int rc;
prop = &hdev->asic_prop;
@@ -2880,7 +2886,9 @@ static int hl_interrupt_wait_ioctl(struct hl_fpriv *hpriv, void *data)
rc = _hl_interrupt_wait_ioctl(hdev, hpriv->ctx,
args->in.interrupt_timeout_us, args->in.addr,
- args->in.target, interrupt_offset, &status);
+ args->in.target, interrupt_offset, &status,
+ args->in.flags & HL_CS_FLAGS_TIMESTAMP,
+ ×tamp);
if (rc) {
if (rc != -EINTR)
@@ -2892,6 +2900,11 @@ static int hl_interrupt_wait_ioctl(struct hl_fpriv *hpriv, void *data)
memset(args, 0, sizeof(*args));
+ if (timestamp) {
+ args->out.timestamp_nsec = timestamp;
+ args->out.flags |= HL_WAIT_CS_STATUS_FLAG_TIMESTAMP_VLD;
+ }
+
switch (status) {
case CS_WAIT_STATUS_COMPLETED:
args->out.status = HL_WAIT_CS_STATUS_COMPLETED;
diff --git a/drivers/misc/habanalabs/common/habanalabs.h b/drivers/misc/habanalabs/common/habanalabs.h
index 99de80915eed..2d9edd734d1c 100644
--- a/drivers/misc/habanalabs/common/habanalabs.h
+++ b/drivers/misc/habanalabs/common/habanalabs.h
@@ -601,6 +601,7 @@ struct asic_fixed_properties {
* masters QIDs that multi cs is waiting on
* @error: mark this fence with error
* @timestamp: timestamp upon completion
+ * @take_timestamp: timestamp shall be taken upon completion
*/
struct hl_fence {
struct completion completion;
@@ -609,6 +610,7 @@ struct hl_fence {
u32 stream_master_qid_map;
int error;
ktime_t timestamp;
+ u8 take_timestamp;
};
/**
diff --git a/drivers/misc/habanalabs/common/irq.c b/drivers/misc/habanalabs/common/irq.c
index 39b14a933393..0dd00ffd1809 100644
--- a/drivers/misc/habanalabs/common/irq.c
+++ b/drivers/misc/habanalabs/common/irq.c
@@ -143,8 +143,11 @@ static void handle_user_cq(struct hl_device *hdev,
struct hl_user_pending_interrupt *pend;
spin_lock(&user_cq->wait_list_lock);
- list_for_each_entry(pend, &user_cq->wait_list_head, wait_list_node)
+ list_for_each_entry(pend, &user_cq->wait_list_head, wait_list_node) {
+ if (pend->fence.take_timestamp)
+ pend->fence.timestamp = ktime_get();
complete_all(&pend->fence.completion);
+ }
spin_unlock(&user_cq->wait_list_lock);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread