Re: [PATCH RFC 5/5] dt-bindings: tee: add RISC-V RPMI TEE transport

[Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>]

Hi Anup,

On 9/17/2026 6:38 PM, Anup Patel wrote:
> On Thu, Sep 17, 2026 at 2:46 AM Amirreza Zarrabi
> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>>
>> Hi Anup,
>>
>> On 9/16/2026 8:12 PM, Anup Patel wrote:
>>> On Sat, Sep 12, 2026 at 3:45 PM Amirreza Zarrabi
>>> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>>>>
>>>> Add a device-tree binding for the OP-TEE RISC-V transport using the RPMI
>>>> TEE service group over SBI MPXY.
>>>>
>>>> Describe one mailbox channel per hart and an optional interrupt used as
>>>> the availability doorbell for asynchronous notifications.
>>>>
>>>> Add the binding to the existing OP-TEE MAINTAINERS entry.
>>>>
>>>> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>>>> ---
>>>>  .../bindings/tee/riscv,rpmi-mpxy-tee.yaml          | 65 ++++++++++++++++++++++
>>>>  MAINTAINERS                                        |  1 +
>>>>  2 files changed, 66 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/tee/riscv,rpmi-mpxy-tee.yaml b/Documentation/devicetree/bindings/tee/riscv,rpmi-mpxy-tee.yaml
>>>> new file mode 100644
>>>> index 000000000000..8f6ff313fd42
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/tee/riscv,rpmi-mpxy-tee.yaml
>>>> @@ -0,0 +1,65 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +# Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/tee/riscv,rpmi-mpxy-tee.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: RISC-V RPMI TEE service group based message proxy
>>>> +
>>>> +maintainers:
>>>> +  - Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>>>> +
>>>> +description: |
>>>> +  The RISC-V Platform Management Interface (RPMI) [1] defines a messaging
>>>> +  protocol which is modular and extensible. The supervisor software can
>>>> +  send/receive RPMI messages via the SBI MPXY extension [2] or some dedicated
>>>> +  supervisor-mode RPMI transport.
>>>> +
>>>> +  The RPMI specification [1] defines a TEE service group which is the RISC-V
>>>> +  analog of Arm FF-A: OP-TEE and the rich execution environment (REE, i.e.
>>>> +  Linux) are peer endpoints and the RPMI framework (machine mode firmware)
>>>> +  mediates every message. Entering OP-TEE on a hart runs it on that hart until
>>>> +  it responds, so the SBI implementation provides one SBI MPXY channel per
>>>> +  hart; all of them are listed, in hart order, on a single node.
>>>> +
>>>> +  ===========================================
>>>> +  References
>>>> +  ===========================================
>>>> +
>>>> +  [1] RISC-V Platform Management Interface (RPMI) v1.0 (or higher)
>>>> +      https://github.com/riscv-non-isa/riscv-rpmi/releases
>>>> +
>>>> +  [2] RISC-V Supervisor Binary Interface (SBI) v3.0 (or higher)
>>>> +      https://github.com/riscv-non-isa/riscv-sbi-doc/releases
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: riscv,rpmi-mpxy-tee
>>>> +
>>>> +  mboxes:
>>>> +    minItems: 1
>>>> +    description:
>>>> +      One SBI MPXY channel implementing the RPMI TEE service group per hart,
>>>> +      listed in the same order as the CPU nodes.
>>>
>>> I am not sure why you need separate MPXY channel per hart. The MPXY shared
>>> memory is already per-hart whereas the MPXY channel will be doman specific
>>> for TEE.
>>
>> True. My reasoning was that TEE_CALL, as I understand it, is expected to execute on
>> the same hart that issued it. Given that, the mailbox core holds the per-channel spinlock
>> across the call to `send_data()`, while the underlying `sbi_ecall()` is synchronous
>> and does not return until the TEE call completes.
>>
>> So, if two harts shared the same channel, a hart executing a long-running TEE operation
>> would hold that channel's lock for the duration of the call, and another hart
>> trying to enqueue on the same channel would spin waiting for it.
>>
>> Using per-hart channels avoids that cross-hart contention. I agree that this may
>> not be the right layer in which to address the issue. I did not pull it to the discussion in to this RFC.
>> I'm happy to use single channel for now and follow up on it separately if useful.
> 
> The per-channel spinlock serialization is not required for SBI MPXY based
> mailbox channels because the underlying SBI implementation will take care
> of the serialization where required.
> 
> In other words, we need to improve the Linux mailbox framework to allow
> SBI MPXY mailbox controller tell Linux mailbox framework to not use
> per-channel spinlock based serialization.
> 
> One possible approach to enhance Linux mailbox framework is show
> below. May be include this (or some other approach) as a separate
> patch in your series ?
> 
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index efacd24a085d..437e7e2e6ad5 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
> @@ -48,7 +48,7 @@ static int add_to_rbuf(struct mbox_chan *chan, void *mssg)
>  static void msg_submit(struct mbox_chan *chan)
>  {
>   unsigned count, idx;
> - void *data;
> + void *data = NULL;
>   int err = -EBUSY;
> 
>   scoped_guard(spinlock_irqsave, &chan->lock) {
> @@ -66,14 +66,22 @@ static void msg_submit(struct mbox_chan *chan)
> 
>   if (chan->cl->tx_prepare)
>   chan->cl->tx_prepare(chan->cl, data);
> - /* Try to submit a message to the MBOX controller */
> - err = chan->mbox->ops->send_data(chan, data);
> +
> + /* Try to submit a message to the MBOX controller in atomic context */
> + if (chan->mbox->ops->send_data)
> + err = chan->mbox->ops->send_data(chan, data);
> + else if (chan->mbox->ops->send_data_nonatomic)
> + err = 0;
>   if (!err) {
>   chan->active_req = data;
>   chan->msg_count--;
>   }
>   }
> 
> + /* Try to submit a message to the MBOX controller in non-atomic context */
> + if (!err && chan->mbox->ops->send_data_nonatomic)
> + err = chan->mbox->ops->send_data_nonatomic(chan, data);
> +
>   if (!err && (chan->txdone_method & MBOX_TXDONE_BY_POLL)) {
>   /* kick start the timer immediately to avoid delays */
>   scoped_guard(spinlock_irqsave, &chan->mbox->poll_hrt_lock)
> diff --git a/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> index b02c17c2c64e..9afa10c72bd7 100644
> --- a/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> +++ b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> @@ -420,7 +420,7 @@ static void mpxy_mbox_shutdown(struct mbox_chan *chan)
>  }
> 
>  static const struct mbox_chan_ops mpxy_mbox_ops = {
> - .send_data = mpxy_mbox_send_data,
> + .send_data_nonatomic = mpxy_mbox_send_data,
>   .peek_data = mpxy_mbox_peek_data,
>   .startup = mpxy_mbox_startup,
>   .shutdown = mpxy_mbox_shutdown,
> diff --git a/include/linux/mailbox_controller.h
> b/include/linux/mailbox_controller.h
> index 26a238a6f941..4bc4798c4b1f 100644
> --- a/include/linux/mailbox_controller.h
> +++ b/include/linux/mailbox_controller.h
> @@ -28,6 +28,10 @@ struct mbox_chan;
>   * transmission of data is reported by the controller via
>   * mbox_chan_txdone (if it has some TX ACK irq). It must not
>   * sleep.
> + * @send_data_nonatomic: The API asks the MBOX controller driver, in non-atomic
> + * context try to transmit a message on the bus. Returns 0 if
> + * data is accepted for transmission, negative error while rejecting
> + * if the remote not acccepted.
>   * @flush: Called when a client requests transmissions to be blocking but
>   * the context doesn't allow sleeping. Typically the controller
>   * will implement a busy loop waiting for the data to flush out.
> @@ -53,6 +57,7 @@ struct mbox_chan;
>   */
>  struct mbox_chan_ops {
>   int (*send_data)(struct mbox_chan *chan, void *data);
> + int (*send_data_nonatomic)(struct mbox_chan *chan, void *data);
>   int (*flush)(struct mbox_chan *chan, unsigned long timeout);
>   int (*startup)(struct mbox_chan *chan);
>   void (*shutdown)(struct mbox_chan *chan);
> 
> Regards,
> Anup

You are right. I'll include it in a separate patch in the next series.

Thanks.
Amir