Skip to content

Commit

Permalink
habanalabs: fix resetting args in wait for CS IOCTL
Browse files Browse the repository at this point in the history
In wait for CS IOCTL code, the driver resets the incoming args structure
before returning to the user, regardless of the return value of the
IOCTL.

In case the IOCTL returns EINTR, resetting the args will result in error
in case the userspace will repeat the ioctl call immediately (which is
the behavior in the hl-thunk userspace library).

The solution is to reset the args only if the driver returns success (0)
as a return value for the IOCTL.

Signed-off-by: Rajaravi Krishna Katta <rkatta@habana.ai>
Reviewed-by: Oded Gabbay <ogabbay@kernel.org>
Signed-off-by: Oded Gabbay <ogabbay@kernel.org>
  • Loading branch information
Rajaravi Krishna Katta authored and Oded Gabbay committed Sep 29, 2021
1 parent bb8a4fc commit 4ca57d5
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 18 deletions.
33 changes: 19 additions & 14 deletions drivers/misc/habanalabs/common/command_submission.c
Original file line number Diff line number Diff line change
Expand Up @@ -2649,11 +2649,18 @@ static int hl_multi_cs_wait_ioctl(struct hl_fpriv *hpriv, void *data)
free_seq_arr:
kfree(cs_seq_arr);

/* update output args */
memset(args, 0, sizeof(*args));
if (rc)
return rc;

if (mcs_data.wait_status == -ERESTARTSYS) {
dev_err_ratelimited(hdev->dev,
"user process got signal while waiting for Multi-CS\n");
return -EINTR;
}

/* update output args */
memset(args, 0, sizeof(*args));

if (mcs_data.completion_bitmap) {
args->out.status = HL_WAIT_CS_STATUS_COMPLETED;
args->out.cs_completion_map = mcs_data.completion_bitmap;
Expand All @@ -2667,8 +2674,6 @@ static int hl_multi_cs_wait_ioctl(struct hl_fpriv *hpriv, void *data)
/* update if some CS was gone */
if (mcs_data.timestamp)
args->out.flags |= HL_WAIT_CS_STATUS_FLAG_GONE;
} else if (mcs_data.wait_status == -ERESTARTSYS) {
args->out.status = HL_WAIT_CS_STATUS_INTERRUPTED;
} else {
args->out.status = HL_WAIT_CS_STATUS_BUSY;
}
Expand All @@ -2688,16 +2693,17 @@ static int hl_cs_wait_ioctl(struct hl_fpriv *hpriv, void *data)
rc = _hl_cs_wait_ioctl(hdev, hpriv->ctx, args->in.timeout_us, seq,
&status, &timestamp);

if (rc == -ERESTARTSYS) {
dev_err_ratelimited(hdev->dev,
"user process got signal while waiting for CS handle %llu\n",
seq);
return -EINTR;
}

memset(args, 0, sizeof(*args));

if (rc) {
if (rc == -ERESTARTSYS) {
dev_err_ratelimited(hdev->dev,
"user process got signal while waiting for CS handle %llu\n",
seq);
args->out.status = HL_WAIT_CS_STATUS_INTERRUPTED;
rc = -EINTR;
} else if (rc == -ETIMEDOUT) {
if (rc == -ETIMEDOUT) {
dev_err_ratelimited(hdev->dev,
"CS %llu has timed-out while user process is waiting for it\n",
seq);
Expand Down Expand Up @@ -2823,7 +2829,6 @@ static int _hl_interrupt_wait_ioctl(struct hl_device *hdev, struct hl_ctx *ctx,
dev_err_ratelimited(hdev->dev,
"user process got signal while waiting for interrupt ID %d\n",
interrupt->interrupt_id);
*status = HL_WAIT_CS_STATUS_INTERRUPTED;
rc = -EINTR;
} else {
*status = CS_WAIT_STATUS_BUSY;
Expand Down Expand Up @@ -2878,8 +2883,6 @@ static int hl_interrupt_wait_ioctl(struct hl_fpriv *hpriv, void *data)
args->in.interrupt_timeout_us, args->in.addr,
args->in.target, interrupt_offset, &status);

memset(args, 0, sizeof(*args));

if (rc) {
if (rc != -EINTR)
dev_err_ratelimited(hdev->dev,
Expand All @@ -2888,6 +2891,8 @@ static int hl_interrupt_wait_ioctl(struct hl_fpriv *hpriv, void *data)
return rc;
}

memset(args, 0, sizeof(*args));

switch (status) {
case CS_WAIT_STATUS_COMPLETED:
args->out.status = HL_WAIT_CS_STATUS_COMPLETED;
Expand Down
6 changes: 2 additions & 4 deletions include/uapi/misc/habanalabs.h
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,6 @@ struct hl_wait_cs_in {
#define HL_WAIT_CS_STATUS_BUSY 1
#define HL_WAIT_CS_STATUS_TIMEDOUT 2
#define HL_WAIT_CS_STATUS_ABORTED 3
#define HL_WAIT_CS_STATUS_INTERRUPTED 4

#define HL_WAIT_CS_STATUS_FLAG_GONE 0x1
#define HL_WAIT_CS_STATUS_FLAG_TIMESTAMP_VLD 0x2
Expand Down Expand Up @@ -1286,16 +1285,15 @@ struct hl_debug_args {
* EIO - The CS was aborted (usually because the device was reset)
* ENODEV - The device wants to do hard-reset (so user need to close FD)
*
* The driver also returns a custom define inside the IOCTL which can be:
* The driver also returns a custom define in case the IOCTL call returned 0.
* The define can be one of the following:
*
* HL_WAIT_CS_STATUS_COMPLETED - The CS has been completed successfully (0)
* HL_WAIT_CS_STATUS_BUSY - The CS is still executing (0)
* HL_WAIT_CS_STATUS_TIMEDOUT - The CS has caused a timeout on the device
* (ETIMEDOUT)
* HL_WAIT_CS_STATUS_ABORTED - The CS was aborted, usually because the
* device was reset (EIO)
* HL_WAIT_CS_STATUS_INTERRUPTED - Waiting for the CS was interrupted (EINTR)
*
*/

#define HL_IOCTL_WAIT_CS \
Expand Down

0 comments on commit 4ca57d5

Please sign in to comment.