lkml.org 
[lkml]   [2022]   [Jul]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 5/5] firmware: qcom: scm: Add wait-queue handling logic
From


On 7/14/2022 6:27 AM, Guru Das Srinagesh wrote:
> On Jul 01 2022 16:51, Rajendra Nayak wrote:
>>
>>
>> On 7/1/2022 4:32 PM, Rajendra Nayak wrote:
>>>
>>>
>>> On 6/28/2022 1:14 AM, Guru Das Srinagesh wrote:
>>>> Add logic to handle QCOM_SCM_WAITQ_SLEEP or QCOM_SCM_WAITQ_WAKE return
>>>> codes.
>>>>
>>>> Scenario 1: Requests made by 2 different VMs:
>>>>
>>>>    VM_1                     VM_2                            Firmware
>>>>      │                        │                                 │
>>>>      │                        │                                 │
>>>>      │                        │                                 │
>>>>      │                        │                                 │
>>>>      │      REQUEST_1         │                                 │
>>>>      ├────────────────────────┼─────────────────────────────────┤
>>>>      │                        │                                 │
>>>>      │                        │                              ┌──┼──┐
>>>>      │                        │                              │  │  │
>>>>      │                        │     REQUEST_2                │  │  │
>>>>      │                        ├──────────────────────────────┼──┤  │
>>>>      │                        │                              │  │  │Resource
>>>>      │                        │                              │  │  │is busy
>>>>      │                        │       {WQ_SLEEP}             │  │  │
>>>>      │                        │◄─────────────────────────────┼──┤  │
>>>>      │                        │  wq_ctx, smc_call_ctx        │  │  │
>>>>      │                        │                              └──┼──┘
>>>>      │   REQUEST_1 COMPLETE   │                                 │
>>>>      │◄───────────────────────┼─────────────────────────────────┤
>>>>      │                        │                                 │
>>>>      │                        │         IRQ                     │
>>>>      │                        │◄─-------------------------------│
>>>>      │                        │                                 │
>>>>      │                        │      get_wq_ctx()               │
>>>>      │                        ├────────────────────────────────►│
>>>>      │                        │                                 │
>>>>      │                        │                                 │
>>>>      │                        │◄────────────────────────────────┤
>>>>      │                        │   wq_ctx, flags, and            │
>>>>      │                        │        more_pending             │
>>>>      │                        │                                 │
>>>>      │                        │                                 │
>>>>      │                        │ wq_resume(smc_call_ctx)         │
>>>>      │                        ├────────────────────────────────►│
>>>>      │                        │                                 │
>>>>      │                        │                                 │
>>>>      │                        │      REQUEST_2 COMPLETE         │
>>>>      │                        │◄────────────────────────────────┤
>>>>      │                        │                                 │
>>>>      │                        │                                 │
>>>>
>>>> Scenario 2: Two Requests coming in from same VM:
>>>>
>>>>    VM_1                                                     Firmware
>>>>      │                                                          │
>>>>      │                                                          │
>>>>      │                                                          │
>>>>      │                                                          │
>>>>      │      REQUEST_1                                           │
>>>>      ├──────────────────────────────────────────────────────────┤
>>>>      │                                                          │
>>>>      │                                                     ┌────┼───┐
>>>>      │                                                     │    │   │
>>>>      │                                                     │    │   │
>>>>      │                                                     │    │   │
>>>>      │      REQUEST_2                                      │    │   │
>>>>      ├─────────────────────────────────────────────────────┼───►│   │
>>>>      │                                                     │    │   │Resource
>>>>      │                                                     │    │   │is busy
>>>>      │      {WQ_SLEEP}                                     │    │   │
>>>>      │◄────────────────────────────────────────────────────┼────┤   │
>>>>      │      wq_ctx, req2_smc_call_ctx                      │    │   │
>>>>      │                                                     │    │   │
>>>>      │                                                     └────┼───┘
>>>>      │                                                          │
>>>>      │      {WQ_WAKE}                                           │
>>>>      │◄─────────────────────────────────────────────────────────┤
>>>>      │      wq_ctx, req1_smc_call_ctx, flags                    │
>>>
>>>
>>> This is perhaps the same thing I asked on the previous patch,
>>> I am guessing {WQ_WAKE} is returned in respone to REQUEST_1?
>>> How do you know in this case if REQUEST_1 was a success or failure?
>>>
>>
>> Ok looking at this some more, I think what we are saying is that the FW returns
>> {WQ_WAKE} to REQUEST_1, we then call wq_wake_ack and the return of
>> *that* will tell if REQUEST_1 was success or failure?
>> Did I get it right?
>
> Yes, that is correct. I should have added an explanatory note in the commit
> message to this effect:
>
>
>      │      {WQ_WAKE} <-- Return value  │
>      │◄─────────────────────────────────────────────────────────┤
>      │      wq_ctx, req1_smc_call_ctx, flags  <-- Its payload   │
>
> What this means is that the WQ_WAKE is sent by the FW to VM1 (direction of
> arrow is from right to left) and that the additional data packed as payload
> indicate that it is meant for REQUEST_1 (`req1_smc_call_ctx`).
>
> Hopefully this will help understand the diagram better.

Ok thanks for the explanation, I actually had a few more comments down in that
patch which you did not answer, can you clarify them too?

>> + } else if ((long)res->a0 < 0) {
>> + /* Error, simply return to caller */
>> + break;

if my understanding above is correct, shouldn't we do a
>> + if (wq)
>> + scm_waitq_flag_handler(wq, flags);
in the error case also?

Also why no just scm_waitq_flag_handler(wq, flags); before fill_wq_wake_ack_args(smc, smc_call_ctx);?

>
> Thank you.
>
> Guru Das.

\
 
 \ /
  Last update: 2022-07-19 12:34    [W:0.089 / U:0.636 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site