Hey Rajendra, Sorry for the delay in response. Needed to clarify with internal team members on these questions before responding. On Aug 02 2022 17:07, Rajendra Nayak wrote: > > On 7/23/2022 4:07 AM, Guru Das Srinagesh wrote: > >When the firmware (FW) supports multiple requests per VM, and the VM > >also supports it via the `allow-multi-call` device tree flag, the > >floodgates are thrown open for them to all reach the firmware at the > >same time. [...] > > 2) SCM_WAITQ_WAKE: > > > > When an SCM call receives this return value instead of success > > or error, FW wishes to signal HLOS to wake up a (different) > > previously sleeping call. > > > > FW tells HLOS which call to wake up via the additional return > > values `wq_ctx`, `smc_call_ctx` and `flags`. The first two have > > already been explained above. > > > > `flags` can be either WAKE_ONE or WAKE_ALL. Meaning, wake either > > one, or all, of the SCM calls that HLOS is associating with the > > given `wq_ctx`. > > > >A sleeping SCM call can be woken up by either an interrupt that FW > >raises, or via a SCM_WAITQ_WAKE return value for a new SCM call. > > Do you know why the FW was not designed to always use an interrupt? > That would have made the handling of this in kernel a lot less complicated. Because: 1. Our firmware in TrustZone cannot raise interrupts on its own - it needs the hypervisor to do that. 2. Thus, in platforms where there is no hypervisor, there is no interrupt possible - only SMC_WAITQ_WAKE. Therefore, relying only on an interrupt would render the driver unable to support platforms without a hypervisor, which we didn't want to do. > >The handshake mechanism that HLOS uses to talk to FW about wait-queue > >operations involves three new SMC calls. These are: > > [...] > >+static void scm_irq_work(struct work_struct *work) > >+{ > >+ int ret; > >+ u32 wq_ctx, flags, more_pending = 0; > >+ struct completion *wq_to_wake; > >+ struct qcom_scm_waitq *w = container_of(work, struct qcom_scm_waitq, scm_irq_work); > >+ struct qcom_scm *scm = container_of(w, struct qcom_scm, waitq); > >+ > >+ do { > >+ ret = scm_get_wq_ctx(&wq_ctx, &flags, &more_pending); > >+ if (ret) { > >+ pr_err("GET_WQ_CTX SMC call failed: %d\n", ret); > >+ return; > >+ } > >+ > >+ wq_to_wake = qcom_scm_lookup_wq(scm, wq_ctx); > >+ if (IS_ERR_OR_NULL(wq_to_wake)) { > >+ pr_err("No waitqueue found for wq_ctx %d: %ld\n", > >+ wq_ctx, PTR_ERR(wq_to_wake)); > >+ return; > > What happens if at this point 'more_pending' was true? will the FW raise > another interrupt? Hmm. At this point, the interrupt handler is early-exiting without waking up a sleeping call via the flag_handler() because firmware has goofed up and given it an invalid wq_ctx. We have bigger problems than `more_pending` being true. > > >+ } > >+ > >+ scm_waitq_flag_handler(wq_to_wake, flags); > >+ } while (more_pending); > >+} Thank you. Guru Das.