Skip to content

Commit c05dfce

Browse files
quic-bjorandeandersson
authored andcommitted
rpmsg: glink: Wait for intent, not just request ack
In some implementations of the remote firmware, an intent request acknowledgment is sent when it's determined if the intent allocation will be fulfilled, but then the intent is queued after the acknowledgment. The result is that upon receiving a granted allocation request, the search for the newly allocated intent will fail and an additional request will be made. This will at best waste memory, but if the second request is rejected the transaction will be incorrectly rejected. Take the incoming intent into account in the wait to mitigate this problem. The above scenario can still happen, in the case that, on that same channel, an unrelated intent is delivered prior to the request acknowledgment and a separate process enters the send path and picks up the intent. Given that there's no relationship between the acknowledgment and the delivered (or to be delivered intent), there doesn't seem to be a solution to this problem. Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> Reviewed-by: Chris Lew <quic_clew@quicinc.com> [bjorn: Fixed spelling issues pointed out by checkpatch in commit message] Signed-off-by: Bjorn Andersson <andersson@kernel.org> Link: https://lore.kernel.org/r/20230327144617.3134175-3-quic_bjorande@quicinc.com
1 parent 0a7eee8 commit c05dfce

1 file changed

Lines changed: 15 additions & 3 deletions

File tree

drivers/rpmsg/qcom_glink_native.c

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ enum {
146146
* @open_req: completed once open-request has been received
147147
* @intent_req_lock: Synchronises multiple intent requests
148148
* @intent_req_result: Result of intent request
149+
* @intent_received: flag indicating that an intent has been received
149150
* @intent_req_wq: wait queue for intent_req signalling
150151
*/
151152
struct glink_channel {
@@ -177,6 +178,7 @@ struct glink_channel {
177178

178179
struct mutex intent_req_lock;
179180
int intent_req_result;
181+
bool intent_received;
180182
wait_queue_head_t intent_req_wq;
181183
};
182184

@@ -420,13 +422,13 @@ static void qcom_glink_handle_intent_req_ack(struct qcom_glink *glink,
420422
return;
421423
}
422424

423-
channel->intent_req_result = granted;
425+
WRITE_ONCE(channel->intent_req_result, granted);
424426
wake_up_all(&channel->intent_req_wq);
425427
}
426428

427429
static void qcom_glink_intent_req_abort(struct glink_channel *channel)
428430
{
429-
channel->intent_req_result = 0;
431+
WRITE_ONCE(channel->intent_req_result, 0);
430432
wake_up_all(&channel->intent_req_wq);
431433
}
432434

@@ -757,6 +759,11 @@ static void qcom_glink_handle_rx_done(struct qcom_glink *glink,
757759
kfree(intent);
758760
}
759761
spin_unlock_irqrestore(&channel->intent_lock, flags);
762+
763+
if (reuse) {
764+
WRITE_ONCE(channel->intent_received, true);
765+
wake_up_all(&channel->intent_req_wq);
766+
}
760767
}
761768

762769
/**
@@ -994,6 +1001,9 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink,
9941001
dev_err(glink->dev, "failed to store remote intent\n");
9951002
}
9961003

1004+
WRITE_ONCE(channel->intent_received, true);
1005+
wake_up_all(&channel->intent_req_wq);
1006+
9971007
kfree(msg);
9981008
qcom_glink_rx_advance(glink, ALIGN(msglen, 8));
9991009
}
@@ -1273,6 +1283,7 @@ static int qcom_glink_request_intent(struct qcom_glink *glink,
12731283
mutex_lock(&channel->intent_req_lock);
12741284

12751285
WRITE_ONCE(channel->intent_req_result, -1);
1286+
WRITE_ONCE(channel->intent_received, false);
12761287

12771288
cmd.id = GLINK_CMD_RX_INTENT_REQ;
12781289
cmd.cid = channel->lcid;
@@ -1283,7 +1294,8 @@ static int qcom_glink_request_intent(struct qcom_glink *glink,
12831294
goto unlock;
12841295

12851296
ret = wait_event_timeout(channel->intent_req_wq,
1286-
READ_ONCE(channel->intent_req_result) >= 0,
1297+
READ_ONCE(channel->intent_req_result) >= 0 &&
1298+
READ_ONCE(channel->intent_received),
12871299
10 * HZ);
12881300
if (!ret) {
12891301
dev_err(glink->dev, "intent request timed out\n");

0 commit comments

Comments
 (0)