Re: [PATCH v2 2/2] firmware: arm_scmi: Augment SMC/HVC to allow optional interrupt
On Fri, Nov 20, 2020 at 08:27:38AM -0500, Jim Quinlan wrote:
> On Fri, Nov 20, 2020 at 6:14 AM Sudeep Holla wrote:
> >
> > On Thu, Nov 19, 2020 at 01:34:18PM -0500, Jim Quinlan wrote:
> > > On Fri, Nov 13, 2020 at 10:12 AM Jim Quinlan
> > > wrote:
> > > >
> > > > On Fri, Nov 13, 2020 at 9:36 AM Sudeep Holla
> > > > wrote:
> > > > >
> > > > > On Fri, Nov 13, 2020 at 09:26:43AM -0500, Jim Quinlan wrote:
> > > > > > Hi, these are fast calls. Regards, Jim
> > > > > >
> > > > > >
> > > > > > On Fri, Nov 13, 2020 at 4:47 AM Sudeep Holla
> > > > > > wrote:
> > > > > > >
> > > > > > > On Thu, Nov 12, 2020 at 12:56:27PM -0500, Jim Quinlan wrote:
> > > > > > > > The SMC/HVC SCMI transport is modified to allow the completion
> > > > > > > > of an SCMI
> > > > > > > > message to be indicated by an interrupt rather than the return
> > > > > > > > of the smc
> > > > > > > > call. This accommodates the existing behavior of the BrcmSTB
> > > > > > > > SCMI
> > > > > > > > "platform" whose SW is already out in the field and cannot be
> > > > > > > > changed.
> > > > > > > >
> > > > > > >
> > > > > > > Sorry for missing to check with you earlier. Are these not fast
> > > > > > > smc calls ?
> > > > > > > Can we check the SMC Function IDs for the same and expect IRQ to
> > > > > > > be present
> > > > > > > if they are not fast calls ?
> > > > > > Hi, if I understand you correctly you want to do something like
> > > > > > this:
> > > > > >
> > > > > > if (! ARM_SMCCC_IS_FAST_CALL(func_id)) {
> > > > > > /* look for irq and request it */
> > > > > > }
> > > > > >
> > > > >
> > > > > Yes.
> > > > >
> > > > > > But we do use fast calls.
> > > > >
> > > > > What was the rationale for retaining fast SMC calls but use IRQ for Tx
> > > > > completion ?
> > > > >
> > > > > Is it because you offload it to some other microprocessor and don't
> > > > > continue execution on secure side in whcih case you can afford fast
> > > > > call ?
> > > Hi Sudeep,
> > >
> >
> > Thanks for the details. Unfortunately more questions:
> >
> > > Here is my understanding: Some SMC calls may take a few longer to
> > > complete than others. The longer ones tie up the CPU core that is
> > > handling the SMC call, and so nothing can be scheduled on that
> > > specific core.
> >
> > So far good.
> >
> > > Unfortunately, we have a real-time OS that runs
> > > sporadically on one specific core and if that happens to be the same
> > > core that is handling the SMC, the RTOS will miss its deadline. So we
> > > need to have the SMC return immediately and use an SGI for task
> > > completion.
> > >
> >
> > So it sounds more like it can't be fast call then.
> Hi Sudeep,
>
> To be honest, I'm not sure what the big difference between fast and
> slow SMC calls are other than the latter is "yielding" and
> interruptible. We cannot tolerate them being interruptible.
>
OK
> >
> > Does that me, it will always return early and send SGI when the request
> > is complete ?
> Most calls send the SGI and return immediately. The ones that take
> longer return from the SMC and send the SGI when the operation is
> completed.
That's relief.
> >
> > 1. If yes, what happens if there are multiple requests in parallel and
> >second one completes before the first. Can we handle that with this
> >patch set. Of will the second request fails until the first one is
> >complete ? It extends to number of cpus in the system worst case.
>
> With SCMI we only have one message pending at a time; perhaps I do
> not understand your question. Having the SMC return is mostly a no-op
> as far as the SCMI driver is concerned.
>
Yes we have lock, I forgot. There are requirements to make the smc atomic
by some vendors, was thinking about that and forgot about the lock and
how what I explained can never happen. Thanks for the patience.
If you ping and get Rob's ack on DT, I can take this patch along with
DT bindings for now as is. We can always enhance if required.
> Our SCMI messages cannot fail. When we do have timeouts it indicates
> that something is wrong and needs to be fixed.
>
Good to know.
--
Regards,
Sudeep
Re: [PATCH v2 2/2] firmware: arm_scmi: Augment SMC/HVC to allow optional interrupt
On Fri, Nov 20, 2020 at 6:14 AM Sudeep Holla wrote:
>
> On Thu, Nov 19, 2020 at 01:34:18PM -0500, Jim Quinlan wrote:
> > On Fri, Nov 13, 2020 at 10:12 AM Jim Quinlan
> > wrote:
> > >
> > > On Fri, Nov 13, 2020 at 9:36 AM Sudeep Holla wrote:
> > > >
> > > > On Fri, Nov 13, 2020 at 09:26:43AM -0500, Jim Quinlan wrote:
> > > > > Hi, these are fast calls. Regards, Jim
> > > > >
> > > > >
> > > > > On Fri, Nov 13, 2020 at 4:47 AM Sudeep Holla
> > > > > wrote:
> > > > > >
> > > > > > On Thu, Nov 12, 2020 at 12:56:27PM -0500, Jim Quinlan wrote:
> > > > > > > The SMC/HVC SCMI transport is modified to allow the completion of
> > > > > > > an SCMI
> > > > > > > message to be indicated by an interrupt rather than the return of
> > > > > > > the smc
> > > > > > > call. This accommodates the existing behavior of the BrcmSTB SCMI
> > > > > > > "platform" whose SW is already out in the field and cannot be
> > > > > > > changed.
> > > > > > >
> > > > > >
> > > > > > Sorry for missing to check with you earlier. Are these not fast smc
> > > > > > calls ?
> > > > > > Can we check the SMC Function IDs for the same and expect IRQ to be
> > > > > > present
> > > > > > if they are not fast calls ?
> > > > > Hi, if I understand you correctly you want to do something like this:
> > > > >
> > > > > if (! ARM_SMCCC_IS_FAST_CALL(func_id)) {
> > > > > /* look for irq and request it */
> > > > > }
> > > > >
> > > >
> > > > Yes.
> > > >
> > > > > But we do use fast calls.
> > > >
> > > > What was the rationale for retaining fast SMC calls but use IRQ for Tx
> > > > completion ?
> > > >
> > > > Is it because you offload it to some other microprocessor and don't
> > > > continue execution on secure side in whcih case you can afford fast
> > > > call ?
> > Hi Sudeep,
> >
>
> Thanks for the details. Unfortunately more questions:
>
> > Here is my understanding: Some SMC calls may take a few longer to
> > complete than others. The longer ones tie up the CPU core that is
> > handling the SMC call, and so nothing can be scheduled on that
> > specific core.
>
> So far good.
>
> > Unfortunately, we have a real-time OS that runs
> > sporadically on one specific core and if that happens to be the same
> > core that is handling the SMC, the RTOS will miss its deadline. So we
> > need to have the SMC return immediately and use an SGI for task
> > completion.
> >
>
> So it sounds more like it can't be fast call then.
Hi Sudeep,
To be honest, I'm not sure what the big difference between fast and
slow SMC calls are other than the latter is "yielding" and
interruptible. We cannot tolerate them being interruptible.
>
> Does that me, it will always return early and send SGI when the request
> is complete ?
Most calls send the SGI and return immediately. The ones that take
longer return from the SMC and send the SGI when the operation is
completed.
>
> 1. If yes, what happens if there are multiple requests in parallel and
>second one completes before the first. Can we handle that with this
>patch set. Of will the second request fails until the first one is
>complete ? It extends to number of cpus in the system worst case.
With SCMI we only have one message pending at a time; perhaps I do
not understand your question. Having the SMC return is mostly a no-op
as far as the SCMI driver is concerned.
Our SCMI messages cannot fail. When we do have timeouts it indicates
that something is wrong and needs to be fixed.
Regards,
Jim Quinlan
Broadcom STB
>
> 2. If no, will this not cause issues if we unconditional wait for interrupt
>every single time ?
>
> --
> Regards,
> Sudeep
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH v2 2/2] firmware: arm_scmi: Augment SMC/HVC to allow optional interrupt
On Thu, Nov 19, 2020 at 01:34:18PM -0500, Jim Quinlan wrote:
> On Fri, Nov 13, 2020 at 10:12 AM Jim Quinlan
> wrote:
> >
> > On Fri, Nov 13, 2020 at 9:36 AM Sudeep Holla wrote:
> > >
> > > On Fri, Nov 13, 2020 at 09:26:43AM -0500, Jim Quinlan wrote:
> > > > Hi, these are fast calls. Regards, Jim
> > > >
> > > >
> > > > On Fri, Nov 13, 2020 at 4:47 AM Sudeep Holla
> > > > wrote:
> > > > >
> > > > > On Thu, Nov 12, 2020 at 12:56:27PM -0500, Jim Quinlan wrote:
> > > > > > The SMC/HVC SCMI transport is modified to allow the completion of
> > > > > > an SCMI
> > > > > > message to be indicated by an interrupt rather than the return of
> > > > > > the smc
> > > > > > call. This accommodates the existing behavior of the BrcmSTB SCMI
> > > > > > "platform" whose SW is already out in the field and cannot be
> > > > > > changed.
> > > > > >
> > > > >
> > > > > Sorry for missing to check with you earlier. Are these not fast smc
> > > > > calls ?
> > > > > Can we check the SMC Function IDs for the same and expect IRQ to be
> > > > > present
> > > > > if they are not fast calls ?
> > > > Hi, if I understand you correctly you want to do something like this:
> > > >
> > > > if (! ARM_SMCCC_IS_FAST_CALL(func_id)) {
> > > > /* look for irq and request it */
> > > > }
> > > >
> > >
> > > Yes.
> > >
> > > > But we do use fast calls.
> > >
> > > What was the rationale for retaining fast SMC calls but use IRQ for Tx
> > > completion ?
> > >
> > > Is it because you offload it to some other microprocessor and don't
> > > continue execution on secure side in whcih case you can afford fast call ?
> Hi Sudeep,
>
Thanks for the details. Unfortunately more questions:
> Here is my understanding: Some SMC calls may take a few longer to
> complete than others. The longer ones tie up the CPU core that is
> handling the SMC call, and so nothing can be scheduled on that
> specific core.
So far good.
> Unfortunately, we have a real-time OS that runs
> sporadically on one specific core and if that happens to be the same
> core that is handling the SMC, the RTOS will miss its deadline. So we
> need to have the SMC return immediately and use an SGI for task
> completion.
>
So it sounds more like it can't be fast call then.
Does that me, it will always return early and send SGI when the request
is complete ?
1. If yes, what happens if there are multiple requests in parallel and
second one completes before the first. Can we handle that with this
patch set. Of will the second request fails until the first one is
complete ? It extends to number of cpus in the system worst case.
2. If no, will this not cause issues if we unconditional wait for interrupt
every single time ?
--
Regards,
Sudeep
Re: [PATCH v2 2/2] firmware: arm_scmi: Augment SMC/HVC to allow optional interrupt
On Fri, Nov 13, 2020 at 10:12 AM Jim Quinlan wrote:
>
> On Fri, Nov 13, 2020 at 9:36 AM Sudeep Holla wrote:
> >
> > On Fri, Nov 13, 2020 at 09:26:43AM -0500, Jim Quinlan wrote:
> > > Hi, these are fast calls. Regards, Jim
> > >
> > >
> > > On Fri, Nov 13, 2020 at 4:47 AM Sudeep Holla wrote:
> > > >
> > > > On Thu, Nov 12, 2020 at 12:56:27PM -0500, Jim Quinlan wrote:
> > > > > The SMC/HVC SCMI transport is modified to allow the completion of an
> > > > > SCMI
> > > > > message to be indicated by an interrupt rather than the return of the
> > > > > smc
> > > > > call. This accommodates the existing behavior of the BrcmSTB SCMI
> > > > > "platform" whose SW is already out in the field and cannot be changed.
> > > > >
> > > >
> > > > Sorry for missing to check with you earlier. Are these not fast smc
> > > > calls ?
> > > > Can we check the SMC Function IDs for the same and expect IRQ to be
> > > > present
> > > > if they are not fast calls ?
> > > Hi, if I understand you correctly you want to do something like this:
> > >
> > > if (! ARM_SMCCC_IS_FAST_CALL(func_id)) {
> > > /* look for irq and request it */
> > > }
> > >
> >
> > Yes.
> >
> > > But we do use fast calls.
> >
> > What was the rationale for retaining fast SMC calls but use IRQ for Tx
> > completion ?
> >
> > Is it because you offload it to some other microprocessor and don't
> > continue execution on secure side in whcih case you can afford fast call ?
Hi Sudeep,
Here is my understanding: Some SMC calls may take a few longer to
complete than others. The longer ones tie up the CPU core that is
handling the SMC call, and so nothing can be scheduled on that
specific core. Unfortunately, we have a real-time OS that runs
sporadically on one specific core and if that happens to be the same
core that is handling the SMC, the RTOS will miss its deadline. So we
need to have the SMC return immediately and use an SGI for task
completion.
Regards,
Jim Quinlan
Broadcom STB
>
> Hi Sudeep,
> I have an answer for this but allow me time to contact the platform FW
> engineer to make sure I have the full picture -- this may take a day
> or two. Regardless, our implementation has already "shipped" to
> customers for some time so we may not be able to change it.
> Regards, Jim
> >
> >
> > --
> > Regards,
> > Sudeep
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH v2 2/2] firmware: arm_scmi: Augment SMC/HVC to allow optional interrupt
On Fri, Nov 13, 2020 at 9:36 AM Sudeep Holla wrote:
>
> On Fri, Nov 13, 2020 at 09:26:43AM -0500, Jim Quinlan wrote:
> > Hi, these are fast calls. Regards, Jim
> >
> >
> > On Fri, Nov 13, 2020 at 4:47 AM Sudeep Holla wrote:
> > >
> > > On Thu, Nov 12, 2020 at 12:56:27PM -0500, Jim Quinlan wrote:
> > > > The SMC/HVC SCMI transport is modified to allow the completion of an
> > > > SCMI
> > > > message to be indicated by an interrupt rather than the return of the
> > > > smc
> > > > call. This accommodates the existing behavior of the BrcmSTB SCMI
> > > > "platform" whose SW is already out in the field and cannot be changed.
> > > >
> > >
> > > Sorry for missing to check with you earlier. Are these not fast smc calls
> > > ?
> > > Can we check the SMC Function IDs for the same and expect IRQ to be
> > > present
> > > if they are not fast calls ?
> > Hi, if I understand you correctly you want to do something like this:
> >
> > if (! ARM_SMCCC_IS_FAST_CALL(func_id)) {
> > /* look for irq and request it */
> > }
> >
>
> Yes.
>
> > But we do use fast calls.
>
> What was the rationale for retaining fast SMC calls but use IRQ for Tx
> completion ?
>
> Is it because you offload it to some other microprocessor and don't
> continue execution on secure side in whcih case you can afford fast call ?
Hi Sudeep,
I have an answer for this but allow me time to contact the platform FW
engineer to make sure I have the full picture -- this may take a day
or two. Regardless, our implementation has already "shipped" to
customers for some time so we may not be able to change it.
Regards, Jim
>
>
> --
> Regards,
> Sudeep
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH v2 2/2] firmware: arm_scmi: Augment SMC/HVC to allow optional interrupt
On Fri, Nov 13, 2020 at 09:26:43AM -0500, Jim Quinlan wrote:
> Hi, these are fast calls. Regards, Jim
>
>
> On Fri, Nov 13, 2020 at 4:47 AM Sudeep Holla wrote:
> >
> > On Thu, Nov 12, 2020 at 12:56:27PM -0500, Jim Quinlan wrote:
> > > The SMC/HVC SCMI transport is modified to allow the completion of an SCMI
> > > message to be indicated by an interrupt rather than the return of the smc
> > > call. This accommodates the existing behavior of the BrcmSTB SCMI
> > > "platform" whose SW is already out in the field and cannot be changed.
> > >
> >
> > Sorry for missing to check with you earlier. Are these not fast smc calls ?
> > Can we check the SMC Function IDs for the same and expect IRQ to be present
> > if they are not fast calls ?
> Hi, if I understand you correctly you want to do something like this:
>
> if (! ARM_SMCCC_IS_FAST_CALL(func_id)) {
> /* look for irq and request it */
> }
>
Yes.
> But we do use fast calls.
What was the rationale for retaining fast SMC calls but use IRQ for Tx
completion ?
Is it because you offload it to some other microprocessor and don't
continue execution on secure side in whcih case you can afford fast call ?
--
Regards,
Sudeep
Re: [PATCH v2 2/2] firmware: arm_scmi: Augment SMC/HVC to allow optional interrupt
Hi, these are fast calls. Regards, Jim
On Fri, Nov 13, 2020 at 4:47 AM Sudeep Holla wrote:
>
> On Thu, Nov 12, 2020 at 12:56:27PM -0500, Jim Quinlan wrote:
> > The SMC/HVC SCMI transport is modified to allow the completion of an SCMI
> > message to be indicated by an interrupt rather than the return of the smc
> > call. This accommodates the existing behavior of the BrcmSTB SCMI
> > "platform" whose SW is already out in the field and cannot be changed.
> >
>
> Sorry for missing to check with you earlier. Are these not fast smc calls ?
> Can we check the SMC Function IDs for the same and expect IRQ to be present
> if they are not fast calls ?
Hi, if I understand you correctly you want to do something like this:
if (! ARM_SMCCC_IS_FAST_CALL(func_id)) {
/* look for irq and request it */
}
But we do use fast calls.
Regards,
Jim
>
> --
> Regards,
> Sudeep
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH v2 2/2] firmware: arm_scmi: Augment SMC/HVC to allow optional interrupt
On Thu, Nov 12, 2020 at 12:56:27PM -0500, Jim Quinlan wrote: > The SMC/HVC SCMI transport is modified to allow the completion of an SCMI > message to be indicated by an interrupt rather than the return of the smc > call. This accommodates the existing behavior of the BrcmSTB SCMI > "platform" whose SW is already out in the field and cannot be changed. > Sorry for missing to check with you earlier. Are these not fast smc calls ? Can we check the SMC Function IDs for the same and expect IRQ to be present if they are not fast calls ? -- Regards, Sudeep
[PATCH v2 2/2] firmware: arm_scmi: Augment SMC/HVC to allow optional interrupt
The SMC/HVC SCMI transport is modified to allow the completion of an SCMI
message to be indicated by an interrupt rather than the return of the smc
call. This accommodates the existing behavior of the BrcmSTB SCMI
"platform" whose SW is already out in the field and cannot be changed.
Signed-off-by: Jim Quinlan
---
drivers/firmware/arm_scmi/smc.c | 38 -
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index 82a82a5dc86a..68ee6eb5fc27 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -9,9 +9,11 @@
#include
#include
#include
+#include
#include
#include
#include
+#include
#include
#include "common.h"
@@ -23,6 +25,8 @@
* @shmem: Transmit/Receive shared memory area
* @shmem_lock: Lock to protect access to Tx/Rx shared memory area
* @func_id: smc/hvc call function id
+ * @irq: Optional; employed when platforms indicates msg completion by intr.
+ * @tx_complete: Optional, employed only when irq is valid.
*/
struct scmi_smc {
@@ -30,8 +34,19 @@ struct scmi_smc {
struct scmi_shared_mem __iomem *shmem;
struct mutex shmem_lock;
u32 func_id;
+ int irq;
+ struct completion tx_complete;
};
+static irqreturn_t smc_msg_done_isr(int irq, void *data)
+{
+ struct scmi_smc *scmi_info = data;
+
+ complete(_info->tx_complete);
+
+ return IRQ_HANDLED;
+}
+
static bool smc_chan_available(struct device *dev, int idx)
{
struct device_node *np = of_parse_phandle(dev->of_node, "shmem", 0);
@@ -51,7 +66,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo,
struct device *dev,
struct resource res;
struct device_node *np;
u32 func_id;
- int ret;
+ int ret, irq = 0;
if (!tx)
return -ENODEV;
@@ -79,10 +94,29 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo,
struct device *dev,
if (ret < 0)
return ret;
+ /*
+* If there is an interrupt named "message-serviced", then the
+* service and completion of a message is signaled by an interrupt
+* rather than by the return of the SMC call.
+*/
+ ret = of_irq_get_byname(cdev->of_node, "message-serviced");
+ if (ret > 0) {
+ irq = ret;
+ ret = devm_request_irq(dev, irq, smc_msg_done_isr,
+ IRQF_NO_SUSPEND,
+ dev_name(dev),
+ scmi_info);
+ if (ret) {
+ dev_err(dev, "failed to setup SCMI smc irq\n");
+ return ret;
+ }
+ init_completion(_info->tx_complete);
+ }
scmi_info->func_id = func_id;
scmi_info->cinfo = cinfo;
mutex_init(_info->shmem_lock);
cinfo->transport_info = scmi_info;
+ scmi_info->irq = irq;
return 0;
}
@@ -111,6 +145,8 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
shmem_tx_prepare(scmi_info->shmem, xfer);
arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, );
+ if (scmi_info->irq)
+ wait_for_completion(_info->tx_complete);
scmi_rx_callback(scmi_info->cinfo, shmem_read_header(scmi_info->shmem));
mutex_unlock(_info->shmem_lock);
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
