Re: [PATCH] firmware: arm_scmi: Add lock for mailbox do_xfer

[Cristian Marussi <cristian.marussi@arm.com>]

On Sun, Oct 22, 2023 at 08:15:17PM +0800, sean yang wrote:
> 
>    The function do_xfer may cause multithreading send message at almost
>    the same time.
> 
>    send_message will write the message in the shared memory, This causes
>    the second write
> 
>    to overwrite the first write, resulting in the loss of the first write.
> 
>    Add lock to avoid this situation. Release lock when the message has
>    responded.
> 
> 

Hi Xinglong ,

In the current SCMI stack, channels are protected from issues of
concurrency at the transport layer, not here in the core, because each
transport has different requirements in these regards.

As an example, VirtIO SCMI transport can indeed support multiple
outgoing messages in-flight if needed, so placing a global mutex here
around the send_message will kill this possibility. (and in this case
will serialize all because the muetx is not even per-channel but global)

Each SCMI transport has its own locking placed where it is deemed
needed, just look at smc or optee in send_message()/ mark_txdone().

Regarding mailboxes, which is probably the case you are referring to,
the shmem_tx_prepare accessors contain some spin_until_cond() to protect
the shared mem, but the core of the mutual exclusion around the channel
is done really by the mailbox framework, since we register the scmi mbox
client with 'knows_txdone = true' and then we use mbox_send_message() and
mbox_client_txdone() to enqueue messages to the mailbox framework: in a
nutshell, no message transmission will be attempted if the previous one
has not completed by calling mbox_client_txdone().

I've never seen any anomalies on this. Did you notice any issue with
mailboxes ? What is your setup ?

Thanks,
Cristian


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel