* [PATCH RFC v2 01/10] mailbox: add direct synchronous send support
2026-10-08 3:27 [PATCH RFC v2 00/10] firmware: riscv: add RPMI TEE service group transport Amirreza Zarrabi
@ 2026-10-08 3:27 ` Amirreza Zarrabi
2026-10-08 3:39 ` sashiko-bot
2026-10-08 3:27 ` [PATCH RFC v2 02/10] mailbox: mpxy: add direct synchronous send Amirreza Zarrabi
` (8 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Amirreza Zarrabi @ 2026-10-08 3:27 UTC (permalink / raw)
To: Jassi Brar, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rahul Pathak, Anup Patel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amirreza Zarrabi,
Jens Wiklander, Sumit Garg, Marouene Boubakri
Cc: linux-arm-msm, linux-kernel, linux-riscv, op-tee, devicetree,
Amirreza Zarrabi
Some mailbox controllers can complete a transaction entirely within
the calling context, without going through the queued TX state
machine or a TX-done interrupt. Add a synchronous send path so their
clients can request one and wait for the result directly.
Controllers advertise support via the new send_data_sync() op and
clients opt in by setting tx_sync when requesting the channel.
Channels bound this way must not call mbox_chan_txdone() or
mbox_client_txdone(), and mbox_send_message() and mbox_flush() now
reject them, since only mbox_send_message_sync() is a valid
transmit path. The client remains responsible for serializing calls
to mbox_send_message_sync() against mbox_free_channel().
Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
drivers/mailbox/mailbox.c | 72 ++++++++++++++++++++++++++++++++++++--
include/linux/mailbox_client.h | 3 ++
include/linux/mailbox_controller.h | 10 ++++++
3 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index efacd24a085d..c640b19de608 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -166,6 +166,12 @@ EXPORT_SYMBOL_GPL(mbox_chan_received_data);
*/
void mbox_chan_txdone(struct mbox_chan *chan, int r)
{
+ if (unlikely(chan->txdone_method & MBOX_TXDONE_BY_RETURN)) {
+ dev_err(chan->mbox->dev,
+ "TX-done notification on direct synchronous channel\n");
+ return;
+ }
+
if (unlikely(!(chan->txdone_method & MBOX_TXDONE_BY_IRQ))) {
dev_err(chan->mbox->dev,
"Controller can't run the TX ticker\n");
@@ -187,6 +193,12 @@ EXPORT_SYMBOL_GPL(mbox_chan_txdone);
*/
void mbox_client_txdone(struct mbox_chan *chan, int r)
{
+ if (unlikely(chan->txdone_method & MBOX_TXDONE_BY_RETURN)) {
+ dev_err(chan->mbox->dev,
+ "TX-done notification on direct synchronous channel\n");
+ return;
+ }
+
if (unlikely(!(chan->txdone_method & MBOX_TXDONE_BY_ACK))) {
dev_err(chan->mbox->dev, "Client can't run the TX ticker\n");
return;
@@ -278,6 +290,9 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
if (!chan || !chan->cl || mssg == MBOX_NO_MSG)
return -EINVAL;
+ if (chan->txdone_method & MBOX_TXDONE_BY_RETURN)
+ return -EOPNOTSUPP;
+
t = add_to_rbuf(chan, mssg);
if (t < 0) {
dev_err(chan->mbox->dev, "Try increasing MBOX_TX_QUEUE_LEN\n");
@@ -308,6 +323,43 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
}
EXPORT_SYMBOL_GPL(mbox_send_message);
+/**
+ * mbox_send_message_sync - Send data and wait for transaction completion
+ * @chan: Mailbox channel assigned to this client
+ * @mssg: Client specific message typecasted
+ *
+ * For a channel bound with tx_sync, ask the controller to transmit @mssg and
+ * only return on completion. This function may sleep and must not be called
+ * from atomic context. @mssg must remain valid until this function returns.
+ *
+ * The direct synchronous path does not queue @mssg, does not use active_req,
+ * and does not use a TX-done notification. The client must serialize this
+ * function against mbox_free_channel().
+ *
+ * Return: 0 on success or a negative error code.
+ */
+int mbox_send_message_sync(struct mbox_chan *chan, void *mssg)
+{
+ int ret;
+
+ if (!chan || !chan->cl || mssg == MBOX_NO_MSG)
+ return -EINVAL;
+
+ if (!(chan->txdone_method & MBOX_TXDONE_BY_RETURN))
+ return -EOPNOTSUPP;
+
+ if (chan->cl->tx_prepare)
+ chan->cl->tx_prepare(chan->cl, mssg);
+ /* Try to submit a message to the MBOX controller synchonously */
+ ret = chan->mbox->ops->send_data_sync(chan, mssg);
+
+ if (chan->cl->tx_done)
+ chan->cl->tx_done(chan->cl, mssg, ret);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(mbox_send_message_sync);
+
/**
* mbox_flush - flush a mailbox channel
* @chan: mailbox channel to flush
@@ -326,8 +378,11 @@ int mbox_flush(struct mbox_chan *chan, unsigned long timeout)
{
int ret;
+ if (chan->txdone_method & MBOX_TXDONE_BY_RETURN)
+ return -EOPNOTSUPP;
+
if (!chan->mbox->ops->flush)
- return -ENOTSUPP;
+ return -EOPNOTSUPP;
ret = chan->mbox->ops->flush(chan, timeout);
if (ret < 0)
@@ -343,7 +398,9 @@ static void mbox_clean_and_put_channel(struct mbox_chan *chan)
scoped_guard(spinlock_irqsave, &chan->lock) {
chan->cl = NULL;
chan->active_req = MBOX_NO_MSG;
- if (chan->txdone_method == MBOX_TXDONE_BY_ACK)
+ if (chan->txdone_method & MBOX_TXDONE_BY_RETURN)
+ chan->txdone_method &= ~MBOX_TXDONE_BY_RETURN;
+ else if (chan->txdone_method == MBOX_TXDONE_BY_ACK)
chan->txdone_method = MBOX_TXDONE_BY_POLL;
}
@@ -355,6 +412,14 @@ static int __mbox_bind_client(struct mbox_chan *chan, struct mbox_client *cl)
struct device *dev = cl->dev;
int ret;
+ if (cl->tx_sync) {
+ if (!chan->mbox->ops->send_data_sync)
+ return -EOPNOTSUPP;
+
+ if (cl->tx_block || cl->tx_tout || cl->knows_txdone)
+ return -EINVAL;
+ }
+
if (chan->cl || !try_module_get(chan->mbox->dev->driver->owner)) {
dev_err(dev, "%s: mailbox not free\n", __func__);
return -EBUSY;
@@ -380,6 +445,9 @@ static int __mbox_bind_client(struct mbox_chan *chan, struct mbox_client *cl)
}
}
+ if (cl->tx_sync)
+ chan->txdone_method |= MBOX_TXDONE_BY_RETURN;
+
return 0;
}
diff --git a/include/linux/mailbox_client.h b/include/linux/mailbox_client.h
index e5997120f45c..32b1d5ad3bfa 100644
--- a/include/linux/mailbox_client.h
+++ b/include/linux/mailbox_client.h
@@ -21,6 +21,7 @@ struct mbox_chan;
* @knows_txdone: If the client could run the TX state machine. Usually
* if the client receives some ACK packet for transmission.
* Unused if the controller already has TX_Done/RTR IRQ.
+ * @tx_sync: Bind the channel for mbox_send_message_sync().
* @rx_callback: Atomic callback to provide client the data received
* @tx_prepare: Atomic callback to ask client to prepare the payload
* before initiating the transmission if required.
@@ -31,6 +32,7 @@ struct mbox_client {
bool tx_block;
unsigned long tx_tout;
bool knows_txdone;
+ bool tx_sync;
void (*rx_callback)(struct mbox_client *cl, void *mssg);
void (*tx_prepare)(struct mbox_client *cl, void *mssg);
@@ -42,6 +44,7 @@ struct mbox_chan *mbox_request_channel_byname(struct mbox_client *cl,
const char *name);
struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index);
int mbox_send_message(struct mbox_chan *chan, void *mssg);
+int mbox_send_message_sync(struct mbox_chan *chan, void *mssg);
int mbox_flush(struct mbox_chan *chan, unsigned long timeout);
void mbox_client_txdone(struct mbox_chan *chan, int r); /* atomic */
bool mbox_client_peek_data(struct mbox_chan *chan); /* atomic */
diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h
index 26a238a6f941..c7dc098324ef 100644
--- a/include/linux/mailbox_controller.h
+++ b/include/linux/mailbox_controller.h
@@ -18,6 +18,7 @@ struct mbox_chan;
#define MBOX_TXDONE_BY_IRQ BIT(0) /* controller has remote RTR irq */
#define MBOX_TXDONE_BY_POLL BIT(1) /* controller can read status of last TX */
#define MBOX_TXDONE_BY_ACK BIT(2) /* S/W ACK received by Client ticks the TX */
+#define MBOX_TXDONE_BY_RETURN BIT(3) /* TX completes by function return */
/**
* struct mbox_chan_ops - methods to control mailbox channels
@@ -28,6 +29,14 @@ 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_sync: The API asks the MBOX controller driver, in non-atomic
+ * context, to transmit a message on the bus and wait for the
+ * transaction to complete. It returns 0 if the transaction
+ * completed successfully or a negative error code otherwise.
+ * The controller must not call mbox_chan_txdone() or
+ * mbox_client_txdone() for this operation. Concurrent calls for one
+ * controller must support them, serialize them internally, or
+ * return -EBUSY for a conflicting transaction.
* @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 +62,7 @@ struct mbox_chan;
*/
struct mbox_chan_ops {
int (*send_data)(struct mbox_chan *chan, void *data);
+ int (*send_data_sync)(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);
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH RFC v2 01/10] mailbox: add direct synchronous send support
2026-10-08 3:27 ` [PATCH RFC v2 01/10] mailbox: add direct synchronous send support Amirreza Zarrabi
@ 2026-10-08 3:39 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-10-08 3:39 UTC (permalink / raw)
To: Amirreza Zarrabi; +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mbox: missing check for send_data ops when binding async clients causes NULL dereference
--
commit 32ed128a065b48b8882da987a95570f9f1da93f8
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
mailbox: add direct synchronous send support
This commit introduces a direct synchronous send path, allowing controllers
to complete transactions within the calling context without the TX state
machine. It adds the send_data_sync operation and validates that clients
requesting synchronous transmission are bound to compatible controllers.
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index efacd24a085d1..c640b19de6080 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
[ ... ]
> @@ -355,6 +412,14 @@ static int __mbox_bind_client(struct mbox_chan *chan, struct mbox_client *cl)
> struct device *dev = cl->dev;
> int ret;
>
> + if (cl->tx_sync) {
> + if (!chan->mbox->ops->send_data_sync)
> + return -EOPNOTSUPP;
> +
> + if (cl->tx_block || cl->tx_tout || cl->knows_txdone)
> + return -EINVAL;
> + }
> +
[Severity: High]
If a controller implements only the new send_data_sync path, could a legacy
asynchronous client (where cl->tx_sync is false) successfully bind to it?
If this happens, wouldn't mbox_send_message() unconditionally execute
chan->mbox->ops->send_data() inside msg_submit():
drivers/mailbox/mailbox.c:msg_submit() {
...
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);
if (!err) {
...
}
Will this cause a NULL pointer dereference? Does __mbox_bind_client() need a
reciprocal check to ensure chan->mbox->ops->send_data exists when
cl->tx_sync is false?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-riscv-rpmi-tee-abi-v2-0-45939a13d223@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH RFC v2 02/10] mailbox: mpxy: add direct synchronous send
2026-10-08 3:27 [PATCH RFC v2 00/10] firmware: riscv: add RPMI TEE service group transport Amirreza Zarrabi
2026-10-08 3:27 ` [PATCH RFC v2 01/10] mailbox: add direct synchronous send support Amirreza Zarrabi
@ 2026-10-08 3:27 ` Amirreza Zarrabi
2026-10-08 3:38 ` sashiko-bot
2026-10-08 3:27 ` [PATCH RFC v2 03/10] firmware: add RPMI TEE bus support Amirreza Zarrabi
` (7 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Amirreza Zarrabi @ 2026-10-08 3:27 UTC (permalink / raw)
To: Jassi Brar, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rahul Pathak, Anup Patel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amirreza Zarrabi,
Jens Wiklander, Sumit Garg, Marouene Boubakri
Cc: linux-arm-msm, linux-kernel, linux-riscv, op-tee, devicetree,
Amirreza Zarrabi
Implement send_data_sync() for RPMI MPXY channels, reusing the
existing RPMI message dispatch and returning its status directly
instead of going through mbox_chan_txdone().
Factor per-hart shared-memory acquisition into mpxy_shmem_get()/
mpxy_shmem_put() so both the queued and the new synchronous path pin
the CPU around the same per-hart buffer, keeping a request and its
response on one hart. Calls on separate harts remain independent,
subject to firmware support.
Add rpmi_mbox_send_message_sync() as the RPMI counterpart to
rpmi_mbox_send_message(), for use by the upcoming TEE transport.
Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
drivers/mailbox/riscv-sbi-mpxy-mbox.c | 196 ++++++++++++++++++-----------
include/linux/mailbox/riscv-rpmi-message.h | 13 ++
2 files changed, 139 insertions(+), 70 deletions(-)
diff --git a/drivers/mailbox/riscv-sbi-mpxy-mbox.c b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
index ea69c6b6b4f9..5ca1b6d87f5c 100644
--- a/drivers/mailbox/riscv-sbi-mpxy-mbox.c
+++ b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
@@ -125,54 +125,83 @@ static DEFINE_PER_CPU(struct mpxy_local, mpxy_local);
static unsigned long mpxy_shmem_size;
static bool mpxy_shmem_init_done;
+static int mpxy_shmem_get(struct mpxy_local **out)
+{
+ struct mpxy_local *mpxy;
+
+ get_cpu();
+ mpxy = this_cpu_ptr(&mpxy_local);
+ if (!mpxy->shmem_active) {
+ put_cpu();
+ return -ENODEV;
+ }
+
+ *out = mpxy;
+ return 0;
+}
+
+static void mpxy_shmem_put(void)
+{
+ put_cpu();
+}
+
static int mpxy_get_channel_count(u32 *channel_count)
{
- struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
- struct sbi_mpxy_channel_ids_data *sdata = mpxy->shmem;
+ struct mpxy_local *mpxy;
+ struct sbi_mpxy_channel_ids_data *sdata;
u32 remaining, returned;
struct sbiret sret;
+ int rc;
- if (!mpxy->shmem_active)
- return -ENODEV;
if (!channel_count)
return -EINVAL;
- get_cpu();
+ rc = mpxy_shmem_get(&mpxy);
+ if (rc)
+ return rc;
+ sdata = mpxy->shmem;
/* Get the remaining and returned fields to calculate total */
sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_GET_CHANNEL_IDS,
0, 0, 0, 0, 0, 0);
- if (sret.error)
- goto err_put_cpu;
+ if (sret.error) {
+ rc = sbi_err_map_linux_errno(sret.error);
+ goto out;
+ }
remaining = le32_to_cpu(sdata->remaining);
returned = le32_to_cpu(sdata->returned);
*channel_count = remaining + returned;
+ rc = 0;
-err_put_cpu:
- put_cpu();
- return sbi_err_map_linux_errno(sret.error);
+out:
+ mpxy_shmem_put();
+ return rc;
}
static int mpxy_get_channel_ids(u32 channel_count, u32 *channel_ids)
{
- struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
- struct sbi_mpxy_channel_ids_data *sdata = mpxy->shmem;
+ struct mpxy_local *mpxy;
+ struct sbi_mpxy_channel_ids_data *sdata;
u32 remaining, returned, count, start_index = 0;
struct sbiret sret;
+ int rc;
- if (!mpxy->shmem_active)
- return -ENODEV;
if (!channel_count || !channel_ids)
return -EINVAL;
- get_cpu();
+ rc = mpxy_shmem_get(&mpxy);
+ if (rc)
+ return rc;
+ sdata = mpxy->shmem;
do {
sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_GET_CHANNEL_IDS,
start_index, 0, 0, 0, 0, 0);
- if (sret.error)
- goto err_put_cpu;
+ if (sret.error) {
+ rc = sbi_err_map_linux_errno(sret.error);
+ goto out;
+ }
remaining = le32_to_cpu(sdata->remaining);
returned = le32_to_cpu(sdata->returned);
@@ -182,55 +211,61 @@ static int mpxy_get_channel_ids(u32 channel_count, u32 *channel_ids)
memcpy_from_le32(&channel_ids[start_index], sdata->channel_array, count);
start_index += count;
} while (remaining && start_index < channel_count);
+ rc = 0;
-err_put_cpu:
- put_cpu();
- return sbi_err_map_linux_errno(sret.error);
+out:
+ mpxy_shmem_put();
+ return rc;
}
static int mpxy_read_attrs(u32 channel_id, u32 base_attrid, u32 attr_count,
u32 *attrs_buf)
{
- struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+ struct mpxy_local *mpxy;
struct sbiret sret;
+ int rc;
- if (!mpxy->shmem_active)
- return -ENODEV;
if (!attr_count || !attrs_buf)
return -EINVAL;
- get_cpu();
+ rc = mpxy_shmem_get(&mpxy);
+ if (rc)
+ return rc;
sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_READ_ATTRS,
channel_id, base_attrid, attr_count, 0, 0, 0);
- if (sret.error)
- goto err_put_cpu;
+ if (sret.error) {
+ rc = sbi_err_map_linux_errno(sret.error);
+ goto out;
+ }
memcpy_from_le32(attrs_buf, (__le32 *)mpxy->shmem, attr_count);
+ rc = 0;
-err_put_cpu:
- put_cpu();
- return sbi_err_map_linux_errno(sret.error);
+out:
+ mpxy_shmem_put();
+ return rc;
}
static int mpxy_write_attrs(u32 channel_id, u32 base_attrid, u32 attr_count,
u32 *attrs_buf)
{
- struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+ struct mpxy_local *mpxy;
struct sbiret sret;
+ int rc;
- if (!mpxy->shmem_active)
- return -ENODEV;
if (!attr_count || !attrs_buf)
return -EINVAL;
- get_cpu();
+ rc = mpxy_shmem_get(&mpxy);
+ if (rc)
+ return rc;
memcpy_to_le32((__le32 *)mpxy->shmem, attrs_buf, attr_count);
sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_WRITE_ATTRS,
channel_id, base_attrid, attr_count, 0, 0, 0);
- put_cpu();
+ mpxy_shmem_put();
return sbi_err_map_linux_errno(sret.error);
}
@@ -239,16 +274,17 @@ static int mpxy_send_message_with_resp(u32 channel_id, u32 msg_id,
void *rx, unsigned long max_rx_len,
unsigned long *rx_len)
{
- struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+ struct mpxy_local *mpxy;
unsigned long rx_bytes;
struct sbiret sret;
+ int rc;
- if (!mpxy->shmem_active)
- return -ENODEV;
if (!tx && tx_len)
return -EINVAL;
- get_cpu();
+ rc = mpxy_shmem_get(&mpxy);
+ if (rc)
+ return rc;
/* Message protocols allowed to have no data in messages */
if (tx_len)
@@ -259,8 +295,8 @@ static int mpxy_send_message_with_resp(u32 channel_id, u32 msg_id,
if (rx && !sret.error) {
rx_bytes = sret.value;
if (rx_bytes > max_rx_len) {
- put_cpu();
- return -ENOSPC;
+ rc = -ENOSPC;
+ goto out;
}
memcpy(rx, mpxy->shmem, rx_bytes);
@@ -268,22 +304,25 @@ static int mpxy_send_message_with_resp(u32 channel_id, u32 msg_id,
*rx_len = rx_bytes;
}
- put_cpu();
- return sbi_err_map_linux_errno(sret.error);
+ rc = sbi_err_map_linux_errno(sret.error);
+out:
+ mpxy_shmem_put();
+ return rc;
}
static int mpxy_send_message_without_resp(u32 channel_id, u32 msg_id,
void *tx, unsigned long tx_len)
{
- struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+ struct mpxy_local *mpxy;
struct sbiret sret;
+ int rc;
- if (!mpxy->shmem_active)
- return -ENODEV;
if (!tx && tx_len)
return -EINVAL;
- get_cpu();
+ rc = mpxy_shmem_get(&mpxy);
+ if (rc)
+ return rc;
/* Message protocols allowed to have no data in messages */
if (tx_len)
@@ -292,40 +331,45 @@ static int mpxy_send_message_without_resp(u32 channel_id, u32 msg_id,
sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_SEND_MSG_WITHOUT_RESP,
channel_id, msg_id, tx_len, 0, 0, 0);
- put_cpu();
- return sbi_err_map_linux_errno(sret.error);
+ rc = sbi_err_map_linux_errno(sret.error);
+ mpxy_shmem_put();
+ return rc;
}
static int mpxy_get_notifications(u32 channel_id,
struct sbi_mpxy_notification_data *notif_data,
unsigned long *events_data_len)
{
- struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+ struct mpxy_local *mpxy;
struct sbiret sret;
+ int rc;
- if (!mpxy->shmem_active)
- return -ENODEV;
if (!notif_data || !events_data_len)
return -EINVAL;
- get_cpu();
+ rc = mpxy_shmem_get(&mpxy);
+ if (rc)
+ return rc;
sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_GET_NOTIFICATION_EVENTS,
channel_id, 0, 0, 0, 0, 0);
- if (sret.error)
- goto err_put_cpu;
+ if (sret.error) {
+ rc = sbi_err_map_linux_errno(sret.error);
+ goto out;
+ }
if (sret.value < 0 || mpxy_shmem_size < sizeof(*notif_data) ||
sret.value > mpxy_shmem_size - sizeof(*notif_data)) {
- put_cpu();
- return -EOVERFLOW;
+ rc = -EOVERFLOW;
+ goto out;
}
memcpy(notif_data, mpxy->shmem, sret.value + sizeof(*notif_data));
*events_data_len = sret.value;
-err_put_cpu:
- put_cpu();
- return sbi_err_map_linux_errno(sret.error);
+ rc = sbi_err_map_linux_errno(sret.error);
+out:
+ mpxy_shmem_put();
+ return rc;
}
static int mpxy_get_shmem_size(unsigned long *shmem_size)
@@ -402,8 +446,8 @@ struct mpxy_mbox {
/* ====== MPXY RPMI processing ====== */
-static void mpxy_mbox_send_rpmi_data(struct mpxy_mbox_channel *mchan,
- struct rpmi_mbox_message *msg)
+static int mpxy_mbox_send_rpmi_data(struct mpxy_mbox_channel *mchan,
+ struct rpmi_mbox_message *msg)
{
msg->error = 0;
switch (msg->type) {
@@ -474,6 +518,8 @@ static void mpxy_mbox_send_rpmi_data(struct mpxy_mbox_channel *mchan,
msg->error = -EOPNOTSUPP;
break;
}
+
+ return msg->error;
}
static void mpxy_mbox_peek_rpmi_data(struct mbox_chan *chan,
@@ -516,12 +562,21 @@ static int mpxy_mbox_send_data(struct mbox_chan *chan, void *data)
{
struct mpxy_mbox_channel *mchan = chan->con_priv;
- if (mchan->attrs.msg_proto_id == SBI_MPXY_MSGPROTO_RPMI_ID) {
- mpxy_mbox_send_rpmi_data(mchan, data);
- return 0;
- }
+ if (mchan->attrs.msg_proto_id != SBI_MPXY_MSGPROTO_RPMI_ID)
+ return -EOPNOTSUPP;
+
+ mpxy_mbox_send_rpmi_data(mchan, data);
+ return 0;
+}
+
+static int mpxy_mbox_send_data_sync(struct mbox_chan *chan, void *data)
+{
+ struct mpxy_mbox_channel *mchan = chan->con_priv;
+
+ if (mchan->attrs.msg_proto_id != SBI_MPXY_MSGPROTO_RPMI_ID)
+ return -EOPNOTSUPP;
- return -EOPNOTSUPP;
+ return mpxy_mbox_send_rpmi_data(mchan, data);
}
static bool mpxy_mbox_peek_data(struct mbox_chan *chan)
@@ -713,10 +768,11 @@ static void mpxy_mbox_shutdown(struct mbox_chan *chan)
}
static const struct mbox_chan_ops mpxy_mbox_ops = {
- .send_data = mpxy_mbox_send_data,
- .peek_data = mpxy_mbox_peek_data,
- .startup = mpxy_mbox_startup,
- .shutdown = mpxy_mbox_shutdown,
+ .send_data = mpxy_mbox_send_data,
+ .send_data_sync = mpxy_mbox_send_data_sync,
+ .peek_data = mpxy_mbox_peek_data,
+ .startup = mpxy_mbox_startup,
+ .shutdown = mpxy_mbox_shutdown,
};
/* ====== MPXY platform driver ===== */
diff --git a/include/linux/mailbox/riscv-rpmi-message.h b/include/linux/mailbox/riscv-rpmi-message.h
index d5362b5821f9..16fa85f65d80 100644
--- a/include/linux/mailbox/riscv-rpmi-message.h
+++ b/include/linux/mailbox/riscv-rpmi-message.h
@@ -251,4 +251,17 @@ static inline int rpmi_mbox_send_message(struct mbox_chan *chan,
return ret;
}
+static inline int rpmi_mbox_send_message_sync(struct mbox_chan *chan,
+ struct rpmi_mbox_message *msg)
+{
+ int ret;
+
+ /* Send message for the underlying mailbox channel synchronously */
+ ret = mbox_send_message_sync(chan, msg);
+ if (ret)
+ return ret;
+
+ return msg->error;
+}
+
#endif /* _LINUX_RISCV_RPMI_MESSAGE_H_ */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH RFC v2 02/10] mailbox: mpxy: add direct synchronous send
2026-10-08 3:27 ` [PATCH RFC v2 02/10] mailbox: mpxy: add direct synchronous send Amirreza Zarrabi
@ 2026-10-08 3:38 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-10-08 3:38 UTC (permalink / raw)
To: Amirreza Zarrabi; +Cc: robh, devicetree, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mailbox: mpxy: interrupt re-entrancy corrupts per-CPU shared memory
- [High] mailbox: mpxy: out-of-bounds reads from shared memory due to unvalidated firmware lengths
--
commit f39e41c9bf232ff624a7e653d9065bd9586d45d9
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
mailbox: mpxy: add direct synchronous send
This patch implements send_data_sync() for RPMI MPXY channels, allowing
RPMI messages to be dispatched synchronously. It factors per-CPU shared
memory acquisition into mpxy_shmem_get() and mpxy_shmem_put(), keeping
requests and their responses on the same CPU. It also adds a counterpart
helper rpmi_mbox_send_message_sync() for the TEE transport.
> diff --git a/drivers/mailbox/riscv-sbi-mpxy-mbox.c b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> index ea69c6b6b4f99..5ca1b6d87f5c6 100644
> --- a/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> +++ b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> @@ -125,54 +125,83 @@ static DEFINE_PER_CPU(struct mpxy_local, mpxy_local);
> static unsigned long mpxy_shmem_size;
> static bool mpxy_shmem_init_done;
>
> +static int mpxy_shmem_get(struct mpxy_local **out)
> +{
> + struct mpxy_local *mpxy;
> +
> + get_cpu();
> + mpxy = this_cpu_ptr(&mpxy_local);
[Severity: High]
Does this leave per-CPU shared memory vulnerable to interrupt re-entrancy?
Since get_cpu() disables preemption but leaves hardware interrupts enabled,
what happens if an interrupt fires on this CPU while a request holds the
mpxy->shmem buffer?
Because the new mpxy_mbox_send_data_sync() runs with interrupts enabled, and
the async send path (e.g. from clock or power management interrupt handlers)
also drops the channel lock before calling send_data, an async message could
interrupt the synchronous send.
The async path would then re-enter mpxy_shmem_get() on the same CPU, receive
the exact same mpxy_local buffer, and silently overwrite the interrupted
task's request payload or response buffer.
Should mpxy_shmem_get() use local_irq_save() (or local_lock_irqsave())
to disable local interrupts and prevent this same-CPU re-entrancy?
[ ... ]
> @@ -182,55 +211,61 @@ static int mpxy_get_channel_ids(u32 channel_count, u32 *channel_ids)
> memcpy_from_le32(&channel_ids[start_index], sdata->channel_array, count);
> start_index += count;
> } while (remaining && start_index < channel_count);
> + rc = 0;
[Severity: High]
Can firmware trigger an out-of-bounds read here?
The number of returned channels is derived from the firmware's
sdata->returned. While count is bounded by the total remaining channels
expected, it doesn't appear that count * sizeof(u32) is checked against
mpxy_shmem_size.
If a buggy or compromised firmware provides an unusually large returned value,
will memcpy_from_le32() read past the end of the mpxy->shmem page into
adjacent kernel memory?
[ ... ]
> @@ -259,8 +295,8 @@ static int mpxy_send_message_with_resp(u32 channel_id, u32 msg_id,
> if (rx && !sret.error) {
> rx_bytes = sret.value;
> if (rx_bytes > max_rx_len) {
> - put_cpu();
> - return -ENOSPC;
> + rc = -ENOSPC;
> + goto out;
> }
>
> memcpy(rx, mpxy->shmem, rx_bytes);
[Severity: High]
Does this memcpy() lack a bounds check against the shared memory size?
The response size sret.value is assigned to rx_bytes, and while it is
checked to ensure it fits within the client-provided max_rx_len, it does
not appear to be checked against mpxy_shmem_size.
If a client requests a large response and firmware returns a length that
exceeds the shared memory allocation, will this read past the end of the
mpxy->shmem buffer and leak kernel memory?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-riscv-rpmi-tee-abi-v2-0-45939a13d223@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH RFC v2 03/10] firmware: add RPMI TEE bus support
2026-10-08 3:27 [PATCH RFC v2 00/10] firmware: riscv: add RPMI TEE service group transport Amirreza Zarrabi
2026-10-08 3:27 ` [PATCH RFC v2 01/10] mailbox: add direct synchronous send support Amirreza Zarrabi
2026-10-08 3:27 ` [PATCH RFC v2 02/10] mailbox: mpxy: add direct synchronous send Amirreza Zarrabi
@ 2026-10-08 3:27 ` Amirreza Zarrabi
2026-10-08 3:38 ` sashiko-bot
2026-10-08 3:27 ` [PATCH RFC v2 04/10] dt-bindings: firmware: add RISC-V RPMI TEE transport Amirreza Zarrabi
` (6 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Amirreza Zarrabi @ 2026-10-08 3:27 UTC (permalink / raw)
To: Jassi Brar, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rahul Pathak, Anup Patel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amirreza Zarrabi,
Jens Wiklander, Sumit Garg, Marouene Boubakri
Cc: linux-arm-msm, linux-kernel, linux-riscv, op-tee, devicetree,
Amirreza Zarrabi
RPMI TEE endpoints can provide multiple services identified by UUID.
Add a service bus so consumer drivers can bind to these services
without depending on the underlying mailbox transport.
Introduce device and driver registration interfaces with UUID-based
matching. Represent each endpoint and service UUID pair as a device
with a unique name.
Expose the endpoint ID, service UUID and modalias through sysfs, and
provide modalias uevents for consumer modules with matching aliases.
Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
drivers/firmware/Kconfig | 2 +
drivers/firmware/Makefile | 1 +
drivers/firmware/riscv_rpmi_tee/Kconfig | 8 ++
drivers/firmware/riscv_rpmi_tee/Makefile | 5 +
drivers/firmware/riscv_rpmi_tee/bus.c | 202 +++++++++++++++++++++++++++++++
include/linux/rpmi_tee.h | 83 +++++++++++++
6 files changed, 301 insertions(+)
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index c183d98c1e8f..46297332288f 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -6,6 +6,8 @@
menu "Firmware Drivers"
+source "drivers/firmware/riscv_rpmi_tee/Kconfig"
+
source "drivers/firmware/arm_scmi/Kconfig"
config ARM_SCPI_PROTOCOL
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index a855d3696173..772fe024baab 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_TURRIS_MOX_RWTM) += turris-mox-rwtm.o
obj-y += arm_ffa/
obj-y += arm_scmi/
+obj-y += riscv_rpmi_tee/
obj-y += broadcom/
obj-y += cirrus/
obj-y += meson/
diff --git a/drivers/firmware/riscv_rpmi_tee/Kconfig b/drivers/firmware/riscv_rpmi_tee/Kconfig
new file mode 100644
index 000000000000..b0396183b87d
--- /dev/null
+++ b/drivers/firmware/riscv_rpmi_tee/Kconfig
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config RISCV_RPMI_TEE_TRANSPORT
+ tristate "RISC-V RPMI TEE service group transport"
+ depends on RISCV && OF && RISCV_SBI_MPXY_MBOX
+ help
+ Say Y or M here to enable the transport for services provided by the
+ RISC-V RPMI TEE service group.
diff --git a/drivers/firmware/riscv_rpmi_tee/Makefile b/drivers/firmware/riscv_rpmi_tee/Makefile
new file mode 100644
index 000000000000..d80ab5bc9dc4
--- /dev/null
+++ b/drivers/firmware/riscv_rpmi_tee/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+
+rpmi-tee-bus-y = bus.o
+rpmi-tee-core-objs := $(rpmi-tee-bus-y)
+obj-$(CONFIG_RISCV_RPMI_TEE_TRANSPORT) = rpmi-tee-core.o
diff --git a/drivers/firmware/riscv_rpmi_tee/bus.c b/drivers/firmware/riscv_rpmi_tee/bus.c
new file mode 100644
index 000000000000..1393adda0d3a
--- /dev/null
+++ b/drivers/firmware/riscv_rpmi_tee/bus.c
@@ -0,0 +1,202 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * RISC-V RPMI TEE bus
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/device.h>
+#include <linux/idr.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/rpmi_tee.h>
+#include <linux/slab.h>
+
+#define RPMI_TEE_UEVENT_MODALIAS_FMT "rpmi_tee:%pUb"
+
+static DEFINE_IDA(rpmi_tee_bus_id);
+
+static int rpmi_tee_device_match(struct device *dev,
+ const struct device_driver *drv)
+{
+ const struct rpmi_tee_device_id *id_table;
+ struct rpmi_tee_device *rdev = to_rpmi_tee_dev(dev);
+
+ id_table = to_rpmi_tee_drv(drv)->id_table;
+ if (!id_table)
+ return 0;
+
+ while (!uuid_is_null(&id_table->uuid)) {
+ if (uuid_equal(&rdev->uuid, &id_table->uuid))
+ return 1;
+ id_table++;
+ }
+
+ return 0;
+}
+
+static int rpmi_tee_device_probe(struct device *dev)
+{
+ struct rpmi_tee_driver *rdrv = to_rpmi_tee_drv(dev->driver);
+
+ return rdrv->probe(to_rpmi_tee_dev(dev));
+}
+
+static void rpmi_tee_device_remove(struct device *dev)
+{
+ struct rpmi_tee_driver *rdrv = to_rpmi_tee_drv(dev->driver);
+
+ if (rdrv->remove)
+ rdrv->remove(to_rpmi_tee_dev(dev));
+}
+
+static int rpmi_tee_device_uevent(const struct device *dev,
+ struct kobj_uevent_env *env)
+{
+ const struct rpmi_tee_device *rdev = to_rpmi_tee_dev(dev);
+
+ return add_uevent_var(env, "MODALIAS=" RPMI_TEE_UEVENT_MODALIAS_FMT,
+ &rdev->uuid);
+}
+
+static ssize_t endpoint_id_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct rpmi_tee_device *rdev = to_rpmi_tee_dev(dev);
+
+ return sysfs_emit(buf, "0x%x\n", rdev->endpoint_id);
+}
+static DEVICE_ATTR_RO(endpoint_id);
+
+static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct rpmi_tee_device *rdev = to_rpmi_tee_dev(dev);
+
+ return sysfs_emit(buf, "%pUb\n", &rdev->uuid);
+}
+static DEVICE_ATTR_RO(uuid);
+
+static ssize_t modalias_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct rpmi_tee_device *rdev = to_rpmi_tee_dev(dev);
+
+ return sysfs_emit(buf, RPMI_TEE_UEVENT_MODALIAS_FMT, &rdev->uuid);
+}
+static DEVICE_ATTR_RO(modalias);
+
+static struct attribute *rpmi_tee_device_attrs[] = {
+ &dev_attr_endpoint_id.attr,
+ &dev_attr_uuid.attr,
+ &dev_attr_modalias.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(rpmi_tee_device);
+
+const struct bus_type rpmi_tee_bus_type = {
+ .name = "rpmi_tee",
+ .match = rpmi_tee_device_match,
+ .probe = rpmi_tee_device_probe,
+ .remove = rpmi_tee_device_remove,
+ .uevent = rpmi_tee_device_uevent,
+ .dev_groups = rpmi_tee_device_groups,
+};
+EXPORT_SYMBOL_GPL(rpmi_tee_bus_type);
+
+int rpmi_tee_driver_register(struct rpmi_tee_driver *driver,
+ struct module *owner, const char *mod_name)
+{
+ if (!driver->probe || !driver->id_table)
+ return -EINVAL;
+
+ driver->driver.bus = &rpmi_tee_bus_type;
+ driver->driver.name = driver->name;
+ driver->driver.owner = owner;
+ driver->driver.mod_name = mod_name;
+
+ return driver_register(&driver->driver);
+}
+EXPORT_SYMBOL_GPL(rpmi_tee_driver_register);
+
+void rpmi_tee_driver_unregister(struct rpmi_tee_driver *driver)
+{
+ driver_unregister(&driver->driver);
+}
+EXPORT_SYMBOL_GPL(rpmi_tee_driver_unregister);
+
+static void rpmi_tee_device_release(struct device *dev)
+{
+ struct rpmi_tee_device *rdev = to_rpmi_tee_dev(dev);
+
+ ida_free(&rpmi_tee_bus_id, rdev->id);
+ kfree(rdev);
+}
+
+struct rpmi_tee_device *
+rpmi_tee_device_register(const uuid_t *uuid, u32 endpoint_id,
+ const struct rpmi_tee_ops *ops, struct device *parent)
+{
+ struct rpmi_tee_device *rdev;
+ int id;
+ int ret;
+
+ if (!uuid || !ops)
+ return ERR_PTR(-EINVAL);
+
+ id = ida_alloc_min(&rpmi_tee_bus_id, 1, GFP_KERNEL);
+ if (id < 0)
+ return ERR_PTR(id);
+
+ rdev = kzalloc_obj(*rdev, GFP_KERNEL);
+ if (!rdev) {
+ ida_free(&rpmi_tee_bus_id, id);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ rdev->dev.parent = parent;
+ rdev->dev.bus = &rpmi_tee_bus_type;
+ rdev->dev.release = rpmi_tee_device_release;
+ dev_set_name(&rdev->dev, "rpmi-tee-%d", id);
+
+ rdev->id = id;
+ rdev->endpoint_id = endpoint_id;
+ rdev->ops = ops;
+ uuid_copy(&rdev->uuid, uuid);
+
+ ret = device_register(&rdev->dev);
+ if (ret) {
+ put_device(&rdev->dev);
+ return ERR_PTR(ret);
+ }
+
+ return rdev;
+}
+EXPORT_SYMBOL_GPL(rpmi_tee_device_register);
+
+void rpmi_tee_device_unregister(struct rpmi_tee_device *rdev)
+{
+ if (rdev)
+ device_unregister(&rdev->dev);
+}
+EXPORT_SYMBOL_GPL(rpmi_tee_device_unregister);
+
+static int __init rpmi_tee_bus_init(void)
+{
+ return bus_register(&rpmi_tee_bus_type);
+}
+
+subsys_initcall(rpmi_tee_bus_init);
+
+static void __exit rpmi_tee_bus_exit(void)
+{
+ bus_unregister(&rpmi_tee_bus_type);
+ ida_destroy(&rpmi_tee_bus_id);
+}
+
+module_exit(rpmi_tee_bus_exit);
+
+MODULE_DESCRIPTION("RISC-V RPMI TEE bus");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/rpmi_tee.h b/include/linux/rpmi_tee.h
new file mode 100644
index 000000000000..c499f0427833
--- /dev/null
+++ b/include/linux/rpmi_tee.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ *
+ * RISC-V RPMI TEE transport interface
+ */
+
+#ifndef _LINUX_RPMI_TEE_H
+#define _LINUX_RPMI_TEE_H
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/uuid.h>
+
+struct rpmi_tee_ops;
+
+struct rpmi_tee_device {
+ struct device dev;
+ u32 id;
+ u32 endpoint_id; /* RPMI endpoint identifier of the TEE. */
+ uuid_t uuid; /* UUID identifying the TEE service. */
+ const struct rpmi_tee_ops *ops;
+};
+
+#define to_rpmi_tee_dev(d) container_of(d, struct rpmi_tee_device, dev)
+
+struct rpmi_tee_device_id {
+ uuid_t uuid;
+};
+
+struct rpmi_tee_driver {
+ const char *name;
+ int (*probe)(struct rpmi_tee_device *rdev);
+ void (*remove)(struct rpmi_tee_device *rdev);
+ /* NULL-UUID-terminated list of supported service UUIDs. */
+ const struct rpmi_tee_device_id *id_table;
+ struct device_driver driver;
+};
+
+#define to_rpmi_tee_drv(d) \
+ container_of_const(d, struct rpmi_tee_driver, driver)
+
+extern const struct bus_type rpmi_tee_bus_type;
+
+#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
+struct rpmi_tee_device *
+rpmi_tee_device_register(const uuid_t *uuid, u32 endpoint_id,
+ const struct rpmi_tee_ops *ops, struct device *parent);
+void rpmi_tee_device_unregister(struct rpmi_tee_device *rdev);
+int rpmi_tee_driver_register(struct rpmi_tee_driver *driver,
+ struct module *owner, const char *mod_name);
+void rpmi_tee_driver_unregister(struct rpmi_tee_driver *driver);
+#else
+static inline struct rpmi_tee_device *
+rpmi_tee_device_register(const uuid_t *uuid, u32 endpoint_id,
+ const struct rpmi_tee_ops *ops, struct device *parent)
+{
+ return NULL;
+}
+
+static inline void rpmi_tee_device_unregister(struct rpmi_tee_device *rdev)
+{
+}
+
+static inline int rpmi_tee_driver_register(struct rpmi_tee_driver *driver,
+ struct module *owner,
+ const char *mod_name)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline void rpmi_tee_driver_unregister(struct rpmi_tee_driver *driver)
+{
+}
+#endif
+
+#define rpmi_tee_register(driver) \
+ rpmi_tee_driver_register(driver, THIS_MODULE, KBUILD_MODNAME)
+#define rpmi_tee_unregister(driver) \
+ rpmi_tee_driver_unregister(driver)
+
+#endif /* _LINUX_RPMI_TEE_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH RFC v2 03/10] firmware: add RPMI TEE bus support
2026-10-08 3:27 ` [PATCH RFC v2 03/10] firmware: add RPMI TEE bus support Amirreza Zarrabi
@ 2026-10-08 3:38 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-10-08 3:38 UTC (permalink / raw)
To: Amirreza Zarrabi; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] firmware: rpmi_tee_device_register() stub returns NULL breaking IS_ERR() expectations
- [Medium] firmware: rpmi_tee: missing modpost support breaks module auto-loading
- [Medium] firmware: rpmi_tee: UAF on module unload via device release callback
- [Low] sysfs: modalias_show() output lacks a trailing newline
--
commit 8388a45d46f5e78edd5f56a8926f96afd8a74a91
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
firmware: add RPMI TEE bus support
RPMI TEE endpoints can provide multiple services identified by UUID.
This patch adds a service bus to allow consumer drivers to bind to these
services, exposing endpoint IDs, UUIDs, and modaliases via sysfs.
> diff --git a/drivers/firmware/riscv_rpmi_tee/bus.c b/drivers/firmware/riscv_rpmi_tee/bus.c
> new file mode 100644
> index 0000000000000..1393adda0d3a7
> --- /dev/null
> +++ b/drivers/firmware/riscv_rpmi_tee/bus.c
[ ... ]
> +#define RPMI_TEE_UEVENT_MODALIAS_FMT "rpmi_tee:%pUb"
[ ... ]
> +static ssize_t modalias_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct rpmi_tee_device *rdev = to_rpmi_tee_dev(dev);
> +
> + return sysfs_emit(buf, RPMI_TEE_UEVENT_MODALIAS_FMT, &rdev->uuid);
> +}
[Severity: Low]
Does this sysfs output need a trailing newline? RPMI_TEE_UEVENT_MODALIAS_FMT
is defined without a newline (which is correct for add_uevent_var()), but
reusing it here with sysfs_emit() will result in an output string that doesn't
have one. Should a newline be appended for the sysfs interface?
[ ... ]
> +static void rpmi_tee_device_release(struct device *dev)
> +{
> + struct rpmi_tee_device *rdev = to_rpmi_tee_dev(dev);
> +
> + ida_free(&rpmi_tee_bus_id, rdev->id);
> + kfree(rdev);
> +}
[ ... ]
> +struct rpmi_tee_device *
> +rpmi_tee_device_register(const uuid_t *uuid, u32 endpoint_id,
> + const struct rpmi_tee_ops *ops, struct device *parent)
> +{
[ ... ]
> + rdev->dev.parent = parent;
> + rdev->dev.bus = &rpmi_tee_bus_type;
> + rdev->dev.release = rpmi_tee_device_release;
[Severity: Medium]
Could this lead to a use-after-free on module unload? If userspace holds a
sysfs attribute open for an rpmi_tee device, the device kobject's refcount
remains non-zero.
If the module is unloaded during this time, the release function and the
global rpmi_tee_bus_id IDA are unmapped.
When the sysfs file is eventually closed and the final kobject refcount is
dropped, could the kernel jump to the unmapped rpmi_tee_device_release()
and access freed IDA memory?
> diff --git a/include/linux/rpmi_tee.h b/include/linux/rpmi_tee.h
> new file mode 100644
> index 0000000000000..c499f0427833e
> --- /dev/null
> +++ b/include/linux/rpmi_tee.h
[ ... ]
> +struct rpmi_tee_device_id {
> + uuid_t uuid;
> +};
[Severity: Medium]
Is this missing from include/linux/mod_devicetable.h? Without adding
rpmi_tee_device_id to mod_devicetable.h and scripts/mod/file2alias.c, modpost
will silently ignore MODULE_DEVICE_TABLE(rpmi_tee, ...) in consumer drivers.
Will this break udev-based automatic module loading for TEE service drivers?
[ ... ]
> +#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
> +struct rpmi_tee_device *
> +rpmi_tee_device_register(const uuid_t *uuid, u32 endpoint_id,
> + const struct rpmi_tee_ops *ops, struct device *parent);
[ ... ]
> +#else
> +static inline struct rpmi_tee_device *
> +rpmi_tee_device_register(const uuid_t *uuid, u32 endpoint_id,
> + const struct rpmi_tee_ops *ops, struct device *parent)
> +{
> + return NULL;
> +}
[Severity: High]
Does this fallback stub break IS_ERR() expectations? The actual implementation
of rpmi_tee_device_register() returns an ERR_PTR() on failure, and callers
typically check the return value using IS_ERR().
Since IS_ERR(NULL) evaluates to false, will callers assume device registration
succeeded and attempt to dereference the NULL pointer? Should this stub return
an error pointer such as ERR_PTR(-ENODEV) instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-riscv-rpmi-tee-abi-v2-0-45939a13d223@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH RFC v2 04/10] dt-bindings: firmware: add RISC-V RPMI TEE transport
2026-10-08 3:27 [PATCH RFC v2 00/10] firmware: riscv: add RPMI TEE service group transport Amirreza Zarrabi
` (2 preceding siblings ...)
2026-10-08 3:27 ` [PATCH RFC v2 03/10] firmware: add RPMI TEE bus support Amirreza Zarrabi
@ 2026-10-08 3:27 ` Amirreza Zarrabi
2026-10-08 3:27 ` [PATCH RFC v2 05/10] firmware: add RPMI TEE transport core Amirreza Zarrabi
` (5 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Amirreza Zarrabi @ 2026-10-08 3:27 UTC (permalink / raw)
To: Jassi Brar, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rahul Pathak, Anup Patel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amirreza Zarrabi,
Jens Wiklander, Sumit Garg, Marouene Boubakri
Cc: linux-arm-msm, linux-kernel, linux-riscv, op-tee, devicetree,
Amirreza Zarrabi
Add a binding for the RPMI TEE service group carried over an SBI MPXY
mailbox channel. The transport uses this channel to discover TEE
endpoints and their services.
Require a single mailbox reference. Notification interrupt information
is obtained from firmware through TEE_PROBE_FEATURES rather than an
interrupt property in the TEE transport node.
Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
.../bindings/firmware/riscv,rpmi-tee.yaml | 35 ++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/Documentation/devicetree/bindings/firmware/riscv,rpmi-tee.yaml b/Documentation/devicetree/bindings/firmware/riscv,rpmi-tee.yaml
new file mode 100644
index 000000000000..b7bbe8893e56
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/riscv,rpmi-tee.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/riscv,rpmi-tee.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V RPMI TEE service group transport
+
+maintainers:
+ - Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
+
+description: |
+ The RISC-V Platform Management Interface TEE service group is carried over
+ an SBI MPXY RPMI mailbox channel. The transport discovers TEE services and
+ creates child devices for their endpoint and UUID pairs.
+
+properties:
+ compatible:
+ const: riscv,rpmi-tee
+
+ mboxes:
+ maxItems: 1
+
+required:
+ - compatible
+ - mboxes
+
+additionalProperties: false
+
+examples:
+ - |
+ rpmi-tee {
+ compatible = "riscv,rpmi-tee";
+ mboxes = <&mpxy_mbox 0x3000 0x0>;
+ };
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH RFC v2 05/10] firmware: add RPMI TEE transport core
2026-10-08 3:27 [PATCH RFC v2 00/10] firmware: riscv: add RPMI TEE service group transport Amirreza Zarrabi
` (3 preceding siblings ...)
2026-10-08 3:27 ` [PATCH RFC v2 04/10] dt-bindings: firmware: add RISC-V RPMI TEE transport Amirreza Zarrabi
@ 2026-10-08 3:27 ` Amirreza Zarrabi
2026-10-08 3:27 ` [PATCH RFC v2 06/10] firmware: riscv: rpmi-tee: discover TEE endpoints Amirreza Zarrabi
` (4 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Amirreza Zarrabi @ 2026-10-08 3:27 UTC (permalink / raw)
To: Jassi Brar, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rahul Pathak, Anup Patel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amirreza Zarrabi,
Jens Wiklander, Sumit Garg, Marouene Boubakri
Cc: linux-arm-msm, linux-kernel, linux-riscv, op-tee, devicetree,
Amirreza Zarrabi
Add a platform driver for the RPMI TEE service group using an SBI MPXY
mailbox channel. Select direct synchronous mailbox transfers so RPMI
transactions complete in the calling context.
Query mailbox attributes to check the RPMI specification version, TEE
service-group identifier and service-group version. Cache the maximum
message data size for subsequent service requests.
This establishes the transport state used by later discovery, memory,
and notification support.
Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
drivers/firmware/riscv_rpmi_tee/Makefile | 3 +
drivers/firmware/riscv_rpmi_tee/driver.c | 133 +++++++++++++++++++++
drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h | 26 ++++
3 files changed, 162 insertions(+)
diff --git a/drivers/firmware/riscv_rpmi_tee/Makefile b/drivers/firmware/riscv_rpmi_tee/Makefile
index d80ab5bc9dc4..8984127535ed 100644
--- a/drivers/firmware/riscv_rpmi_tee/Makefile
+++ b/drivers/firmware/riscv_rpmi_tee/Makefile
@@ -3,3 +3,6 @@
rpmi-tee-bus-y = bus.o
rpmi-tee-core-objs := $(rpmi-tee-bus-y)
obj-$(CONFIG_RISCV_RPMI_TEE_TRANSPORT) = rpmi-tee-core.o
+rpmi-tee-driver-y = driver.o
+rpmi-tee-module-objs := $(rpmi-tee-driver-y)
+obj-$(CONFIG_RISCV_RPMI_TEE_TRANSPORT) += rpmi-tee-module.o
diff --git a/drivers/firmware/riscv_rpmi_tee/driver.c b/drivers/firmware/riscv_rpmi_tee/driver.c
new file mode 100644
index 000000000000..d9ae161d241b
--- /dev/null
+++ b/drivers/firmware/riscv_rpmi_tee/driver.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * RISC-V RPMI TEE transport
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/mailbox_client.h>
+#include <linux/mailbox/riscv-rpmi-message.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/rpmi_tee.h>
+
+#include "rpmi_tee_private.h"
+
+/**
+ * rpmi_tee_get_attr() - Get an RPMI mailbox attribute
+ * @priv: RPMI TEE transport
+ * @id: Attribute identifier
+ * @value: Returned attribute value
+ *
+ * Return: 0 on success, or a negative error code on failure.
+ */
+static int rpmi_tee_get_attr(struct rpmi_tee_transport *priv,
+ enum rpmi_mbox_attribute_id id, u32 *value)
+{
+ struct rpmi_mbox_message msg;
+ int ret;
+
+ rpmi_mbox_init_get_attribute(&msg, id);
+ ret = rpmi_mbox_send_message_sync(priv->mbox.chan, &msg);
+ if (ret)
+ return ret;
+
+ *value = msg.attr.value;
+
+ return 0;
+}
+
+/* Validate the RPMI mailbox transport and cache its message size. */
+static int rpmi_tee_check_transport(struct rpmi_tee_transport *priv)
+{
+ u32 value;
+ int ret;
+
+ ret = rpmi_tee_get_attr(priv, RPMI_MBOX_ATTR_SPEC_VERSION, &value);
+ if (ret)
+ return ret;
+ if (value < RPMI_MKVER(1, 0))
+ return -EPROTONOSUPPORT;
+
+ ret = rpmi_tee_get_attr(priv, RPMI_MBOX_ATTR_SERVICEGROUP_ID, &value);
+ if (ret)
+ return ret;
+ if (value != RPMI_SRVGRP_TEE)
+ return -ENODEV;
+
+ ret = rpmi_tee_get_attr(priv, RPMI_MBOX_ATTR_SERVICEGROUP_VERSION,
+ &value);
+ if (ret)
+ return ret;
+ if (value < RPMI_MKVER(1, 0))
+ return -EPROTONOSUPPORT;
+
+ ret = rpmi_tee_get_attr(priv, RPMI_MBOX_ATTR_MAX_MSG_DATA_SIZE, &value);
+ if (ret)
+ return ret;
+
+ priv->mbox.max_msg_data_size = value;
+
+ return 0;
+}
+
+static int rpmi_tee_transport_probe(struct platform_device *pdev)
+{
+ struct rpmi_tee_transport *priv;
+ int ret;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev = &pdev->dev;
+ platform_set_drvdata(pdev, priv);
+ priv->mbox.client.dev = &pdev->dev;
+ priv->mbox.client.tx_sync = true;
+ priv->mbox.chan = mbox_request_channel(&priv->mbox.client, 0);
+ if (IS_ERR(priv->mbox.chan))
+ return dev_err_probe(&pdev->dev, PTR_ERR(priv->mbox.chan),
+ "failed to request mailbox channel\n");
+
+ ret = rpmi_tee_check_transport(priv);
+ if (ret) {
+ dev_err_probe(&pdev->dev, ret,
+ "invalid RPMI TEE mailbox channel\n");
+ goto out_failed;
+ }
+
+ return 0;
+
+out_failed:
+ mbox_free_channel(priv->mbox.chan);
+
+ return ret;
+}
+
+static void rpmi_tee_transport_remove(struct platform_device *pdev)
+{
+ struct rpmi_tee_transport *priv = platform_get_drvdata(pdev);
+
+ mbox_free_channel(priv->mbox.chan);
+}
+
+static const struct of_device_id rpmi_tee_transport_match[] = {
+ { .compatible = "riscv,rpmi-tee" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, rpmi_tee_transport_match);
+
+static struct platform_driver rpmi_tee_transport_driver = {
+ .probe = rpmi_tee_transport_probe,
+ .remove = rpmi_tee_transport_remove,
+ .driver = {
+ .name = "riscv-rpmi-tee",
+ .of_match_table = rpmi_tee_transport_match,
+ },
+};
+
+module_platform_driver(rpmi_tee_transport_driver);
+
+MODULE_DESCRIPTION("RISC-V RPMI TEE service group transport");
+MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
new file mode 100644
index 000000000000..b25192c5df00
--- /dev/null
+++ b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#ifndef _RPMI_TEE_PRIVATE_H
+#define _RPMI_TEE_PRIVATE_H
+
+#include <linux/mailbox_client.h>
+#include <linux/types.h>
+
+/* TEE service group and the services used by this module. */
+#define RPMI_SRVGRP_TEE 0x10
+
+struct rpmi_tee_mbox {
+ struct mbox_client client;
+ struct mbox_chan *chan;
+ u32 max_msg_data_size;
+};
+
+struct rpmi_tee_transport {
+ struct device *dev;
+ struct rpmi_tee_mbox mbox;
+};
+
+#endif /* _RPMI_TEE_PRIVATE_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH RFC v2 06/10] firmware: riscv: rpmi-tee: discover TEE endpoints
2026-10-08 3:27 [PATCH RFC v2 00/10] firmware: riscv: add RPMI TEE service group transport Amirreza Zarrabi
` (4 preceding siblings ...)
2026-10-08 3:27 ` [PATCH RFC v2 05/10] firmware: add RPMI TEE transport core Amirreza Zarrabi
@ 2026-10-08 3:27 ` Amirreza Zarrabi
2026-10-08 3:27 ` [PATCH RFC v2 07/10] firmware: riscv: rpmi-tee: register TEE services and support calls Amirreza Zarrabi
` (3 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Amirreza Zarrabi @ 2026-10-08 3:27 UTC (permalink / raw)
To: Jassi Brar, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rahul Pathak, Anup Patel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amirreza Zarrabi,
Jens Wiklander, Sumit Garg, Marouene Boubakri
Cc: linux-arm-msm, linux-kernel, linux-riscv, op-tee, devicetree,
Amirreza Zarrabi
Add discovery through the PROBE_SYSTEM, PROBE_DOMAIN and PROBE_ENDPOINT
services. Obtain the caller's endpoint ID and enumerate reachable
physical TEE endpoints and their service UUIDs.
Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
drivers/firmware/riscv_rpmi_tee/Makefile | 2 +-
drivers/firmware/riscv_rpmi_tee/discovery.c | 383 +++++++++++++++++++++
drivers/firmware/riscv_rpmi_tee/driver.c | 29 ++
drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h | 42 +++
4 files changed, 455 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/riscv_rpmi_tee/Makefile b/drivers/firmware/riscv_rpmi_tee/Makefile
index 8984127535ed..fc6edfb21dbc 100644
--- a/drivers/firmware/riscv_rpmi_tee/Makefile
+++ b/drivers/firmware/riscv_rpmi_tee/Makefile
@@ -3,6 +3,6 @@
rpmi-tee-bus-y = bus.o
rpmi-tee-core-objs := $(rpmi-tee-bus-y)
obj-$(CONFIG_RISCV_RPMI_TEE_TRANSPORT) = rpmi-tee-core.o
-rpmi-tee-driver-y = driver.o
+rpmi-tee-driver-y = driver.o discovery.o
rpmi-tee-module-objs := $(rpmi-tee-driver-y)
obj-$(CONFIG_RISCV_RPMI_TEE_TRANSPORT) += rpmi-tee-module.o
diff --git a/drivers/firmware/riscv_rpmi_tee/discovery.c b/drivers/firmware/riscv_rpmi_tee/discovery.c
new file mode 100644
index 000000000000..fedd68bba639
--- /dev/null
+++ b/drivers/firmware/riscv_rpmi_tee/discovery.c
@@ -0,0 +1,383 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/bits.h>
+#include <linux/errno.h>
+#include <linux/cleanup.h>
+#include <linux/mailbox/riscv-rpmi-message.h>
+#include <linux/slab.h>
+#include <linux/unaligned.h>
+
+#include "rpmi_tee_private.h"
+
+/* Role flags in PROBE_DOMAIN and PROBE_ENDPOINT responses. */
+#define RPMI_TEE_DOMAIN_TEE BIT(31)
+#define RPMI_TEE_ENDPOINT_TEE BIT(31)
+#define RPMI_TEE_ENDPOINT_PHYSICAL BIT(30)
+
+/**
+ * struct rpmi_tee_probe_system_resp - PROBE_SYSTEM response prefix
+ * @status: RPMI completion status.
+ * @caller_domain: Calling endpoint's domain ID.
+ * @caller_endpoint: Calling physical endpoint ID.
+ * @domain_count: Number of entries in @domains.
+ * @domains: Reachable domain identifiers.
+ */
+struct rpmi_tee_probe_system_resp {
+ __le32 status;
+ __le32 caller_domain;
+ __le32 caller_endpoint;
+ __le32 domain_count;
+ __le32 domains[];
+} __packed;
+
+/**
+ * struct rpmi_tee_probe_domain_req - PROBE_DOMAIN request
+ * @domain_id: Domain identifier from PROBE_SYSTEM.
+ */
+struct rpmi_tee_probe_domain_req {
+ __le32 domain_id;
+} __packed;
+
+/**
+ * struct rpmi_tee_probe_domain_resp - PROBE_DOMAIN response prefix
+ * @status: RPMI completion status.
+ * @flags: Domain role flags.
+ * @endpoint_count: Number of entries in @endpoints.
+ * @endpoints: Physical and proxied endpoint IDs.
+ */
+struct rpmi_tee_probe_domain_resp {
+ __le32 status;
+ __le32 flags;
+ __le32 endpoint_count;
+ __le32 endpoints[];
+} __packed;
+
+/**
+ * struct rpmi_tee_probe_endpoint_req - PROBE_ENDPOINT request
+ * @endpoint_id: Endpoint identifier from PROBE_DOMAIN.
+ */
+struct rpmi_tee_probe_endpoint_req {
+ __le32 endpoint_id;
+} __packed;
+
+/**
+ * struct rpmi_tee_probe_endpoint_resp - PROBE_ENDPOINT response prefix
+ * @status: RPMI completion status.
+ * @flags: Endpoint role and lifetime flags.
+ * @name: Debug name, not a matching identity.
+ * @proxied_count: Number of proxied endpoint IDs in @data.
+ * @parcel_count: Number of parcel IDs in @data.
+ * @service_count: Number of 16-byte service UUIDs in @data.
+ * @metadata_len: Metadata length in bytes.
+ * @data: Proxied endpoint IDs, parcel IDs, service UUIDs, then metadata.
+ */
+struct rpmi_tee_probe_endpoint_resp {
+ __le32 status;
+ __le32 flags;
+ u8 name[32];
+ __le32 proxied_count;
+ __le32 parcel_count;
+ __le32 service_count;
+ __le32 metadata_len;
+ u8 data[];
+} __packed;
+
+struct rpmi_tee_system_info {
+ u32 caller_domain;
+ u32 caller_endpoint;
+ u32 domain_count;
+ const u8 *domains;
+};
+
+/* Return the domain ID at a validated index in the system response. */
+static inline u32
+rpmi_tee_domain_id_at(const struct rpmi_tee_system_info *info, u32 index)
+{
+ return get_unaligned_le32(info->domains +
+ (size_t)index * sizeof(__le32));
+}
+
+struct rpmi_tee_domain_info {
+ u32 flags;
+ u32 endpoint_count;
+ const u8 *endpoints;
+};
+
+/* Return the endpoint ID at a validated index in the domain response. */
+static inline u32
+rpmi_tee_endpoint_id_at(const struct rpmi_tee_domain_info *info, u32 index)
+{
+ return get_unaligned_le32(info->endpoints +
+ (size_t)index * sizeof(__le32));
+}
+
+struct rpmi_tee_endpoint_info {
+ u32 flags;
+ u32 service_count;
+ const u8 *services;
+};
+
+/* Copy the service UUID at a validated index in the endpoint response. */
+static inline void
+rpmi_tee_service_uuid_at(const struct rpmi_tee_endpoint_info *info,
+ u32 index, uuid_t *uuid)
+{
+ import_uuid(uuid, info->services + (size_t)index * UUID_SIZE);
+}
+
+/* rpmi_tee_parse_*_response parse and validate the response. */
+
+static int rpmi_tee_parse_system_response(const void *data, size_t len,
+ struct rpmi_tee_system_info *info)
+{
+ const struct rpmi_tee_probe_system_resp *resp = data;
+ u32 count;
+
+ if (len < sizeof(*resp))
+ return -EPROTO;
+
+ count = get_unaligned_le32(&resp->domain_count);
+ if (len != sizeof(*resp) + (u64)count * sizeof(__le32))
+ return -EPROTO;
+
+ info->caller_domain = get_unaligned_le32(&resp->caller_domain);
+ info->caller_endpoint = get_unaligned_le32(&resp->caller_endpoint);
+ info->domain_count = count;
+ /* Borrowed pointer, use rpmi_tee_domain_id_at() to access. */
+ info->domains = (const u8 *)data + sizeof(*resp);
+
+ return 0;
+}
+
+static int rpmi_tee_parse_domain_response(const void *data, size_t len,
+ struct rpmi_tee_domain_info *info)
+{
+ const struct rpmi_tee_probe_domain_resp *resp = data;
+ u32 count;
+
+ if (len < sizeof(*resp))
+ return -EPROTO;
+
+ count = get_unaligned_le32(&resp->endpoint_count);
+ if (len != sizeof(*resp) + (u64)count * sizeof(__le32))
+ return -EPROTO;
+
+ info->flags = get_unaligned_le32(&resp->flags);
+ info->endpoint_count = count;
+ /* Borrowed pointer, use rpmi_tee_endpoint_id_at() to access. */
+ info->endpoints = (const u8 *)data + sizeof(*resp);
+
+ return 0;
+}
+
+static int rpmi_tee_parse_endpoint_response(const void *data, size_t len,
+ struct rpmi_tee_endpoint_info *info)
+{
+ const struct rpmi_tee_probe_endpoint_resp *resp = data;
+ size_t remaining, service_offset;
+ u32 proxied, parcels, services;
+
+ if (len < sizeof(*resp))
+ return -EPROTO;
+
+ remaining = len - sizeof(*resp);
+ proxied = get_unaligned_le32(&resp->proxied_count);
+ parcels = get_unaligned_le32(&resp->parcel_count);
+ services = get_unaligned_le32(&resp->service_count);
+
+ /* Check both ID arrays before adding their counts. */
+ if (proxied > remaining / sizeof(__le32) ||
+ parcels > remaining / sizeof(__le32) - proxied)
+ return -EPROTO;
+
+ service_offset = ((size_t)proxied + parcels) * sizeof(__le32);
+ remaining -= service_offset;
+ if (services > remaining / UUID_SIZE)
+ return -EPROTO;
+
+ /* Ignore metadata following the service UUID array. */
+ info->flags = get_unaligned_le32(&resp->flags);
+ info->service_count = services;
+ /* Borrowed pointer, use rpmi_tee_service_uuid_at() to get UUID. */
+ info->services = resp->data + service_offset;
+
+ return 0;
+}
+
+/* Return an owned probe response; vanished domain/endpoint IDs yield -ENOENT. */
+static int rpmi_tee_probe_info(struct rpmi_tee_transport *priv, u32 service,
+ const void *req, size_t req_len, void **data,
+ size_t *data_len)
+{
+ size_t resp_len = priv->mbox.max_msg_data_size;
+ s32 status;
+ int ret;
+
+ void *resp __free(kfree) = kzalloc(resp_len, GFP_KERNEL);
+ if (!resp)
+ return -ENOMEM;
+
+ ret = rpmi_tee_send_with_status(priv, service, req, req_len, resp,
+ &resp_len, &status);
+ if (ret)
+ return ret;
+
+ if (status == RPMI_ERR_INVALID_PARAM &&
+ (service == RPMI_TEE_SRV_PROBE_DOMAIN ||
+ service == RPMI_TEE_SRV_PROBE_ENDPOINT))
+ return -ENOENT;
+
+ ret = rpmi_to_linux_error(status);
+ if (ret)
+ return ret;
+
+ *data_len = resp_len;
+ *data = no_free_ptr(resp);
+
+ return 0;
+}
+
+static int rpmi_tee_add_endpoint(struct rpmi_tee_discovery *system, u32 ep_id,
+ const struct rpmi_tee_endpoint_info *info)
+{
+ struct rpmi_tee_discovered_endpoint *ep;
+ u32 i;
+
+ /* Skip endpoints already discovered. */
+ list_for_each_entry(ep, &system->eps, node)
+ if (ep->ep_id == ep_id)
+ return 0;
+
+ ep = kzalloc(struct_size(ep, services, info->service_count), GFP_KERNEL);
+ if (!ep)
+ return -ENOMEM;
+
+ ep->ep_id = ep_id;
+ ep->service_count = info->service_count;
+ for (i = 0; i < ep->service_count; i++)
+ rpmi_tee_service_uuid_at(info, i, &ep->services[i]);
+
+ list_add_tail(&ep->node, &system->eps);
+
+ return 0;
+}
+
+/* Discover physical TEE endpoints in one domain, skipping vanished IDs. */
+static int rpmi_tee_discover_domain(struct rpmi_tee_transport *priv, u32 id,
+ struct rpmi_tee_discovery *system)
+{
+ struct rpmi_tee_domain_info domain;
+ struct rpmi_tee_probe_domain_req req = {
+ .domain_id = cpu_to_le32(id),
+ };
+ size_t len;
+ int ret;
+ u32 i;
+
+ void *data __free(kfree) = NULL;
+
+ ret = rpmi_tee_probe_info(priv, RPMI_TEE_SRV_PROBE_DOMAIN, &req,
+ sizeof(req), &data, &len);
+ if (ret)
+ return ret == -ENOENT ? 0 : ret;
+
+ ret = rpmi_tee_parse_domain_response(data, len, &domain);
+ /* Only TEE domains are processed. */
+ if (ret || !(domain.flags & RPMI_TEE_DOMAIN_TEE))
+ return ret;
+
+ for (i = 0; i < domain.endpoint_count; i++) {
+ u32 ep_id = rpmi_tee_endpoint_id_at(&domain, i);
+ struct rpmi_tee_endpoint_info endpoint;
+ struct rpmi_tee_probe_endpoint_req ep_req = {
+ .endpoint_id = cpu_to_le32(ep_id),
+ };
+
+ void *ep_data __free(kfree) = NULL;
+
+ ret = rpmi_tee_probe_info(priv, RPMI_TEE_SRV_PROBE_ENDPOINT,
+ &ep_req, sizeof(ep_req), &ep_data,
+ &len);
+ if (ret == -ENOENT)
+ continue;
+
+ if (ret)
+ return ret;
+
+ ret = rpmi_tee_parse_endpoint_response(ep_data, len, &endpoint);
+ if (ret)
+ return ret;
+
+ /* Only physical TEE endpoints are processed. */
+ if (ep_id == priv->self_id ||
+ !(endpoint.flags & RPMI_TEE_ENDPOINT_TEE) ||
+ !(endpoint.flags & RPMI_TEE_ENDPOINT_PHYSICAL))
+ continue;
+
+ ret = rpmi_tee_add_endpoint(system, ep_id, &endpoint);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+/* Free all discovered endpoints and their copied service UUIDs. */
+void rpmi_tee_free_discovery(struct rpmi_tee_discovery *system)
+{
+ struct rpmi_tee_discovered_endpoint *ep, *next;
+
+ list_for_each_entry_safe(ep, next, &system->eps, node) {
+ list_del(&ep->node);
+ kfree(ep);
+ }
+}
+
+/**
+ * rpmi_tee_discover_endpoints() - Discover physical TEE endpoints and services
+ * @priv: RPMI TEE transport
+ * @system: Returned list of endpoints and their service UUIDs
+ *
+ * Obtain caller identity and walk SYSTEM, DOMAIN and ENDPOINT probes. The
+ * caller releases the entries with rpmi_tee_free_discovery() on success.
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+int rpmi_tee_discover_endpoints(struct rpmi_tee_transport *priv,
+ struct rpmi_tee_discovery *system)
+{
+ struct rpmi_tee_system_info info;
+ size_t len;
+ int ret;
+ u32 i;
+
+ void *data __free(kfree) = NULL;
+
+ INIT_LIST_HEAD(&system->eps);
+ /* PROBE_SYSTEM has no request data and returns the caller's identity. */
+ ret = rpmi_tee_probe_info(priv, RPMI_TEE_SRV_PROBE_SYSTEM, NULL, 0,
+ &data, &len);
+ if (ret)
+ return ret;
+
+ ret = rpmi_tee_parse_system_response(data, len, &info);
+ if (ret)
+ return ret;
+
+ priv->self_id = info.caller_endpoint;
+ for (i = 0; i < info.domain_count; i++) {
+ u32 domain_id = rpmi_tee_domain_id_at(&info, i);
+
+ ret = rpmi_tee_discover_domain(priv, domain_id, system);
+ if (ret) {
+ rpmi_tee_free_discovery(system);
+
+ return ret;
+ }
+ }
+
+ return 0;
+}
diff --git a/drivers/firmware/riscv_rpmi_tee/driver.c b/drivers/firmware/riscv_rpmi_tee/driver.c
index d9ae161d241b..a70b8be41bed 100644
--- a/drivers/firmware/riscv_rpmi_tee/driver.c
+++ b/drivers/firmware/riscv_rpmi_tee/driver.c
@@ -5,6 +5,7 @@
* Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
*/
+#include <linux/unaligned.h>
#include <linux/mailbox_client.h>
#include <linux/mailbox/riscv-rpmi-message.h>
#include <linux/module.h>
@@ -14,6 +15,34 @@
#include "rpmi_tee_private.h"
+/* rpmi_tee_send_with_status() - Send an RPMI TEE service request. */
+int rpmi_tee_send_with_status(struct rpmi_tee_transport *priv, u32 service_id,
+ const void *req, size_t req_len, void *resp,
+ size_t *resp_len, s32 *status)
+{
+ size_t max_resp_len = *resp_len;
+ struct rpmi_mbox_message msg;
+ int ret;
+
+ if (req_len > priv->mbox.max_msg_data_size ||
+ max_resp_len > priv->mbox.max_msg_data_size)
+ return -EMSGSIZE;
+
+ rpmi_mbox_init_send_with_response(&msg, service_id, (void *)req,
+ req_len, resp, max_resp_len);
+ ret = rpmi_mbox_send_message_sync(priv->mbox.chan, &msg);
+ if (ret)
+ return ret;
+ /* At least STATUS word should be present. */
+ if (msg.data.out_response_len < sizeof(__le32))
+ return -EPROTO;
+
+ *resp_len = msg.data.out_response_len;
+ *status = (s32)get_unaligned_le32(resp);
+
+ return 0;
+}
+
/**
* rpmi_tee_get_attr() - Get an RPMI mailbox attribute
* @priv: RPMI TEE transport
diff --git a/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
index b25192c5df00..ba0e7cb59b6d 100644
--- a/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
+++ b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
@@ -6,12 +6,18 @@
#ifndef _RPMI_TEE_PRIVATE_H
#define _RPMI_TEE_PRIVATE_H
+#include <linux/list.h>
#include <linux/mailbox_client.h>
#include <linux/types.h>
+#include <linux/uuid.h>
/* TEE service group and the services used by this module. */
#define RPMI_SRVGRP_TEE 0x10
+#define RPMI_TEE_SRV_PROBE_SYSTEM 0x03
+#define RPMI_TEE_SRV_PROBE_DOMAIN 0x04
+#define RPMI_TEE_SRV_PROBE_ENDPOINT 0x05
+
struct rpmi_tee_mbox {
struct mbox_client client;
struct mbox_chan *chan;
@@ -21,6 +27,42 @@ struct rpmi_tee_mbox {
struct rpmi_tee_transport {
struct device *dev;
struct rpmi_tee_mbox mbox;
+ u32 self_id;
+};
+
+/* Report local transport errors separately from the returned RPMI status. */
+int rpmi_tee_send_with_status(struct rpmi_tee_transport *priv, u32 service_id,
+ const void *req, size_t req_len, void *resp,
+ size_t *resp_len, s32 *status);
+
+/**
+ * struct rpmi_tee_discovered_endpoint - Physical TEE endpoint and its services
+ * @node: Entry in &struct rpmi_tee_discovery.eps.
+ * @ep_id: Physical TEE endpoint ID.
+ * @service_count: Number of UUIDs in @services.
+ * @services: Service UUIDs for the endpoint.
+ */
+struct rpmi_tee_discovered_endpoint {
+ struct list_head node;
+ u32 ep_id;
+ u32 service_count;
+ uuid_t services[] __counted_by(service_count);
};
+/**
+ * struct rpmi_tee_discovery - Discovered physical TEE endpoints and services
+ * @eps: List of &struct rpmi_tee_discovered_endpoint entries, excluding the
+ * caller endpoint.
+ *
+ * On successful discovery, the caller owns the entries and releases them with
+ * rpmi_tee_free_discovery(). Endpoints without services are also included.
+ */
+struct rpmi_tee_discovery {
+ struct list_head eps;
+};
+
+void rpmi_tee_free_discovery(struct rpmi_tee_discovery *system);
+int rpmi_tee_discover_endpoints(struct rpmi_tee_transport *priv,
+ struct rpmi_tee_discovery *system);
+
#endif /* _RPMI_TEE_PRIVATE_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH RFC v2 07/10] firmware: riscv: rpmi-tee: register TEE services and support calls
2026-10-08 3:27 [PATCH RFC v2 00/10] firmware: riscv: add RPMI TEE service group transport Amirreza Zarrabi
` (5 preceding siblings ...)
2026-10-08 3:27 ` [PATCH RFC v2 06/10] firmware: riscv: rpmi-tee: discover TEE endpoints Amirreza Zarrabi
@ 2026-10-08 3:27 ` Amirreza Zarrabi
2026-10-08 3:27 ` [PATCH RFC v2 08/10] firmware: riscv: rpmi-tee: cache TEE capabilities Amirreza Zarrabi
` (2 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Amirreza Zarrabi @ 2026-10-08 3:27 UTC (permalink / raw)
To: Jassi Brar, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rahul Pathak, Anup Patel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amirreza Zarrabi,
Jens Wiklander, Sumit Garg, Marouene Boubakri
Cc: linux-arm-msm, linux-kernel, linux-riscv, op-tee, devicetree,
Amirreza Zarrabi
Register discovered TEE services as RPMI TEE bus devices so service
drivers can bind to them.
Provide TEE_CALL operations for exchanging service-defined requests
and responses, and expose the transport's payload limits to clients.
Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
drivers/firmware/riscv_rpmi_tee/driver.c | 278 ++++++++++++++++++++-
drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h | 13 +
include/linux/rpmi_tee.h | 35 +++
3 files changed, 325 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/riscv_rpmi_tee/driver.c b/drivers/firmware/riscv_rpmi_tee/driver.c
index a70b8be41bed..a683f7d28c25 100644
--- a/drivers/firmware/riscv_rpmi_tee/driver.c
+++ b/drivers/firmware/riscv_rpmi_tee/driver.c
@@ -5,16 +5,52 @@
* Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
*/
-#include <linux/unaligned.h>
#include <linux/mailbox_client.h>
#include <linux/mailbox/riscv-rpmi-message.h>
+#include <linux/cleanup.h>
+#include <linux/list.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/rpmi_tee.h>
+#include <linux/slab.h>
+#include <linux/unaligned.h>
#include "rpmi_tee_private.h"
+/**
+ * struct rpmi_tee_call_req - TEE_CALL request prefix
+ * @sender_id: Calling REE endpoint identifier.
+ * @target_id: Destination TEE endpoint identifier.
+ * @service: UUID of the target service.
+ * @service_data_len: Length of @service_data in bytes.
+ * @service_data: Service-defined request data.
+ */
+struct rpmi_tee_call_req {
+ __le32 sender_id;
+ __le32 target_id;
+ u8 service[UUID_SIZE];
+ __le32 service_data_len;
+ u8 service_data[];
+} __packed;
+
+/**
+ * struct rpmi_tee_call_resp - TEE_CALL response prefix
+ * @status: RPMI completion status.
+ * @service_data_len: Length of @service_data in bytes.
+ * @service_data: Service-defined response data.
+ */
+struct rpmi_tee_call_resp {
+ __le32 status;
+ __le32 service_data_len;
+ u8 service_data[];
+} __packed;
+
+struct rpmi_tee_child {
+ struct list_head node;
+ struct rpmi_tee_device *rdev;
+};
+
/* rpmi_tee_send_with_status() - Send an RPMI TEE service request. */
int rpmi_tee_send_with_status(struct rpmi_tee_transport *priv, u32 service_id,
const void *req, size_t req_len, void *resp,
@@ -43,6 +79,47 @@ int rpmi_tee_send_with_status(struct rpmi_tee_transport *priv, u32 service_id,
return 0;
}
+/**
+ * rpmi_tee_send() - Send an RPMI TEE service request
+ * @priv: RPMI TEE transport
+ * @service_id: RPMI TEE service identifier
+ * @req: Request data
+ * @req_len: Request data length
+ * @resp: Response data buffer, or %NULL for a status-only response
+ * @resp_len: On entry, response buffer capacity; on success, response length
+ *
+ * Pass both @resp and @resp_len as %NULL when the service has no response
+ * payload beyond the mandatory RPMI status word.
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+static int rpmi_tee_send(struct rpmi_tee_transport *priv, u32 service_id,
+ const void *req, size_t req_len, void *resp,
+ size_t *resp_len)
+{
+ __le32 status_resp;
+ size_t status_resp_len = sizeof(status_resp);
+ s32 status;
+ int ret;
+
+ if (!resp && !resp_len) {
+ resp = &status_resp;
+ resp_len = &status_resp_len;
+ } else if (!resp || !resp_len) {
+ return -EINVAL;
+ }
+
+ ret = rpmi_tee_send_with_status(priv, service_id, req, req_len, resp,
+ resp_len, &status);
+ if (ret)
+ return ret;
+
+ if (status == RPMI_ERR_NO_DATA)
+ return -ENODATA;
+
+ return rpmi_to_linux_error(status);
+}
+
/**
* rpmi_tee_get_attr() - Get an RPMI mailbox attribute
* @priv: RPMI TEE transport
@@ -95,12 +172,202 @@ static int rpmi_tee_check_transport(struct rpmi_tee_transport *priv)
ret = rpmi_tee_get_attr(priv, RPMI_MBOX_ATTR_MAX_MSG_DATA_SIZE, &value);
if (ret)
return ret;
+ /* The mandatory TEE_CALL request and response must fit the mailbox. */
+ if (value < sizeof(struct rpmi_tee_call_req) ||
+ value < sizeof(struct rpmi_tee_call_resp))
+ return -EMSGSIZE;
priv->mbox.max_msg_data_size = value;
+ priv->max_call_req_size = value - sizeof(struct rpmi_tee_call_req);
+ priv->max_call_resp_size = value - sizeof(struct rpmi_tee_call_resp);
+
+ return 0;
+}
+
+/* RPMI TEE SERVICE GRP API. */
+
+/* Return the transport that owns @rdev. */
+static struct rpmi_tee_transport *
+rpmi_tee_device_to_transport(struct rpmi_tee_device *rdev)
+{
+ return dev_get_drvdata(rdev->dev.parent);
+}
+
+static int rpmi_tee_op_msg_limits_get(struct rpmi_tee_device *rdev,
+ struct rpmi_tee_msg_limits *limits)
+{
+ struct rpmi_tee_transport *priv = rpmi_tee_device_to_transport(rdev);
+
+ if (!limits)
+ return -EINVAL;
+
+ limits->max_req_size = priv->max_call_req_size;
+ limits->max_resp_size = priv->max_call_resp_size;
+
+ return 0;
+}
+
+/**
+ * rpmi_tee_op_call - Invoke a service offered by a TEE endpoint
+ * @rdev: TEE service device.
+ * @req: Service-defined request data.
+ * @req_len: Length of @req in bytes.
+ * @resp: Buffer for service-defined response data.
+ * @resp_len: On entry, capacity of @resp; on success, response length.
+ *
+ * MPXY can return -ENOSPC after the TEE has processed the request when the
+ * response exceeds the supplied buffer. Callers must not blindly retry a
+ * non-idempotent request in that case.
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+static int rpmi_tee_op_call(struct rpmi_tee_device *rdev, const void *req,
+ size_t req_len, void *resp, size_t *resp_len)
+{
+ struct rpmi_tee_transport *priv = rpmi_tee_device_to_transport(rdev);
+ size_t call_req_len, call_resp_len;
+ u32 service_data_len;
+ int ret;
+
+ if (!resp_len || (!req && req_len) || (!resp && *resp_len))
+ return -EINVAL;
+
+ /* TEE_CALL payload must fit the mailbox. */
+ if (req_len > priv->max_call_req_size ||
+ *resp_len > priv->max_call_resp_size)
+ return -EMSGSIZE;
+
+ call_req_len = sizeof(struct rpmi_tee_call_req) + req_len;
+ call_resp_len = sizeof(struct rpmi_tee_call_resp) + *resp_len;
+
+ struct rpmi_tee_call_req *call_req __free(kfree) =
+ kzalloc(call_req_len, GFP_KERNEL);
+ if (!call_req)
+ return -ENOMEM;
+
+ struct rpmi_tee_call_resp *call_resp __free(kfree) =
+ kzalloc(call_resp_len, GFP_KERNEL);
+ if (!call_resp)
+ return -ENOMEM;
+
+ call_req->sender_id = cpu_to_le32(priv->self_id);
+ call_req->target_id = cpu_to_le32(rdev->endpoint_id);
+ export_uuid(call_req->service, &rdev->uuid);
+ call_req->service_data_len = cpu_to_le32(req_len);
+ if (req_len)
+ memcpy(call_req->service_data, req, req_len);
+ /* Make TEE CALL. */
+ ret = rpmi_tee_send(priv, RPMI_TEE_SRV_CALL, call_req, call_req_len,
+ call_resp, &call_resp_len);
+ if (ret)
+ return ret;
+
+ if (call_resp_len < sizeof(*call_resp))
+ return -EPROTO;
+ service_data_len = get_unaligned_le32(&call_resp->service_data_len);
+ /* Verify the firmware-reported length fits the actual response. */
+ if (service_data_len != call_resp_len - sizeof(*call_resp))
+ return -EPROTO;
+
+ if (service_data_len)
+ memcpy(resp, call_resp->service_data, service_data_len);
+ *resp_len = service_data_len;
+
+ return 0;
+}
+
+static const struct rpmi_tee_info_ops rpmi_tee_info_ops = {
+ .msg_limits_get = rpmi_tee_op_msg_limits_get,
+};
+
+static const struct rpmi_tee_msg_ops rpmi_tee_msg_ops = {
+ .call = rpmi_tee_op_call,
+};
+
+static const struct rpmi_tee_ops rpmi_tee_ops = {
+ .info_ops = &rpmi_tee_info_ops,
+ .msg_ops = &rpmi_tee_msg_ops,
+};
+
+static void rpmi_tee_unregister_devices(struct rpmi_tee_transport *priv)
+{
+ struct rpmi_tee_child *child, *tmp;
+
+ list_for_each_entry_safe(child, tmp, &priv->devices, node) {
+ list_del(&child->node);
+ rpmi_tee_device_unregister(child->rdev);
+ kfree(child);
+ }
+}
+
+static struct rpmi_tee_device *
+rpmi_tee_find_device(struct rpmi_tee_transport *priv, const uuid_t *uuid,
+ u32 endpoint_id)
+{
+ struct rpmi_tee_child *child;
+
+ list_for_each_entry(child, &priv->devices, node) {
+ if (child->rdev->endpoint_id == endpoint_id &&
+ uuid_equal(&child->rdev->uuid, uuid))
+ return child->rdev;
+ }
+
+ return NULL;
+}
+
+static int
+rpmi_tee_register_devices(struct rpmi_tee_transport *priv,
+ const struct rpmi_tee_discovered_endpoint *ep)
+{
+ u32 i;
+
+ for (i = 0; i < ep->service_count; i++) {
+ const uuid_t *uuid = &ep->services[i];
+
+ /* Discard duplicate devices in the same endpoint. */
+ if (rpmi_tee_find_device(priv, uuid, ep->ep_id))
+ continue;
+
+ struct rpmi_tee_child *child __free(kfree) =
+ kzalloc_obj(*child, GFP_KERNEL);
+ if (!child)
+ return -ENOMEM;
+
+ child->rdev =
+ rpmi_tee_device_register(uuid, ep->ep_id,
+ &rpmi_tee_ops, priv->dev);
+ if (IS_ERR(child->rdev))
+ return PTR_ERR(child->rdev);
+
+ list_add_tail(&no_free_ptr(child)->node, &priv->devices);
+ }
return 0;
}
+static int rpmi_tee_setup_endpoints(struct rpmi_tee_transport *priv)
+{
+ struct rpmi_tee_discovered_endpoint *ep;
+ struct rpmi_tee_discovery system;
+ int ret;
+
+ ret = rpmi_tee_discover_endpoints(priv, &system);
+ if (ret)
+ return ret;
+
+ list_for_each_entry(ep, &system.eps, node) {
+ ret = rpmi_tee_register_devices(priv, ep);
+ if (ret) {
+ rpmi_tee_unregister_devices(priv);
+ break;
+ }
+ }
+
+ rpmi_tee_free_discovery(&system);
+
+ return ret;
+}
+
static int rpmi_tee_transport_probe(struct platform_device *pdev)
{
struct rpmi_tee_transport *priv;
@@ -112,6 +379,7 @@ static int rpmi_tee_transport_probe(struct platform_device *pdev)
priv->dev = &pdev->dev;
platform_set_drvdata(pdev, priv);
+ INIT_LIST_HEAD(&priv->devices);
priv->mbox.client.dev = &pdev->dev;
priv->mbox.client.tx_sync = true;
priv->mbox.chan = mbox_request_channel(&priv->mbox.client, 0);
@@ -126,6 +394,13 @@ static int rpmi_tee_transport_probe(struct platform_device *pdev)
goto out_failed;
}
+ ret = rpmi_tee_setup_endpoints(priv);
+ if (ret) {
+ dev_err_probe(&pdev->dev, ret,
+ "failed to discover RPMI TEE services\n");
+ goto out_failed;
+ }
+
return 0;
out_failed:
@@ -138,6 +413,7 @@ static void rpmi_tee_transport_remove(struct platform_device *pdev)
{
struct rpmi_tee_transport *priv = platform_get_drvdata(pdev);
+ rpmi_tee_unregister_devices(priv);
mbox_free_channel(priv->mbox.chan);
}
diff --git a/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
index ba0e7cb59b6d..07e6b9913cb7 100644
--- a/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
+++ b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
@@ -17,6 +17,7 @@
#define RPMI_TEE_SRV_PROBE_SYSTEM 0x03
#define RPMI_TEE_SRV_PROBE_DOMAIN 0x04
#define RPMI_TEE_SRV_PROBE_ENDPOINT 0x05
+#define RPMI_TEE_SRV_CALL 0x18
struct rpmi_tee_mbox {
struct mbox_client client;
@@ -24,10 +25,22 @@ struct rpmi_tee_mbox {
u32 max_msg_data_size;
};
+/**
+ * struct rpmi_tee_transport - State for one RPMI TEE transport instance
+ * @dev: Parent platform device.
+ * @mbox: RPMI mailbox transport state.
+ * @max_call_req_size: Maximum TEE_CALL request payload size in bytes.
+ * @max_call_resp_size: Maximum TEE_CALL response payload size in bytes.
+ * @self_id: Local REE physical endpoint identifier.
+ * @devices: List of registered TEE service devices.
+ */
struct rpmi_tee_transport {
struct device *dev;
struct rpmi_tee_mbox mbox;
+ size_t max_call_req_size;
+ size_t max_call_resp_size;
u32 self_id;
+ struct list_head devices;
};
/* Report local transport errors separately from the returned RPMI status. */
diff --git a/include/linux/rpmi_tee.h b/include/linux/rpmi_tee.h
index c499f0427833..a76055149996 100644
--- a/include/linux/rpmi_tee.h
+++ b/include/linux/rpmi_tee.h
@@ -41,6 +41,41 @@ struct rpmi_tee_driver {
#define to_rpmi_tee_drv(d) \
container_of_const(d, struct rpmi_tee_driver, driver)
+/**
+ * struct rpmi_tee_msg_limits - TEE_CALL service payload limits
+ * @max_req_size: Maximum request payload size in bytes.
+ * @max_resp_size: Maximum response payload size in bytes.
+ *
+ * Limits exclude the RPMI TEE_CALL request and response prefixes, but include
+ * any service-specific headers supplied by the caller.
+ */
+struct rpmi_tee_msg_limits {
+ size_t max_req_size;
+ size_t max_resp_size;
+};
+
+/**
+ * struct rpmi_tee_info_ops - RPMI TEE transport information operations
+ * @msg_limits_get: Return cached TEE_CALL payload limits for the transport
+ * serving @rdev in @limits. Limits remain fixed for the transport lifetime.
+ * Return 0 on success, or a negative error code on failure.
+ */
+struct rpmi_tee_info_ops {
+ int (*msg_limits_get)(struct rpmi_tee_device *rdev,
+ struct rpmi_tee_msg_limits *limits);
+};
+
+struct rpmi_tee_msg_ops {
+ int (*call)(struct rpmi_tee_device *rdev, const void *req,
+ size_t req_len, void *resp, size_t *resp_len);
+};
+
+/* RPMI TEE transport operation groups. */
+struct rpmi_tee_ops {
+ const struct rpmi_tee_info_ops *info_ops;
+ const struct rpmi_tee_msg_ops *msg_ops;
+};
+
extern const struct bus_type rpmi_tee_bus_type;
#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH RFC v2 08/10] firmware: riscv: rpmi-tee: cache TEE capabilities
2026-10-08 3:27 [PATCH RFC v2 00/10] firmware: riscv: add RPMI TEE service group transport Amirreza Zarrabi
` (6 preceding siblings ...)
2026-10-08 3:27 ` [PATCH RFC v2 07/10] firmware: riscv: rpmi-tee: register TEE services and support calls Amirreza Zarrabi
@ 2026-10-08 3:27 ` Amirreza Zarrabi
2026-10-08 3:27 ` [PATCH RFC v2 09/10] firmware: riscv: rpmi-tee: add memory parcel operations Amirreza Zarrabi
2026-10-08 3:27 ` [PATCH RFC v2 10/10] firmware: riscv: rpmi-tee: add signal bus support Amirreza Zarrabi
9 siblings, 0 replies; 16+ messages in thread
From: Amirreza Zarrabi @ 2026-10-08 3:27 UTC (permalink / raw)
To: Jassi Brar, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rahul Pathak, Anup Patel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amirreza Zarrabi,
Jens Wiklander, Sumit Garg, Marouene Boubakri
Cc: linux-arm-msm, linux-kernel, linux-riscv, op-tee, devicetree,
Amirreza Zarrabi
Query firmware capabilities during transport initialization and cache
support for memory lending, sharing, segmented parcels and signal buses.
Distinguish REE-accessible memory operations from those restricted to
TEE-to-TEE use so Linux only enables operations available to it.
Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
drivers/firmware/riscv_rpmi_tee/driver.c | 73 ++++++++++++++++++++++
drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h | 15 +++++
2 files changed, 88 insertions(+)
diff --git a/drivers/firmware/riscv_rpmi_tee/driver.c b/drivers/firmware/riscv_rpmi_tee/driver.c
index a683f7d28c25..b43a09f5e853 100644
--- a/drivers/firmware/riscv_rpmi_tee/driver.c
+++ b/drivers/firmware/riscv_rpmi_tee/driver.c
@@ -18,6 +18,33 @@
#include "rpmi_tee_private.h"
+#define RPMI_TEE_FEATURE_MEMORY_LEND 1
+#define RPMI_TEE_FEATURE_MEMORY_SHARE 2
+#define RPMI_TEE_FEATURE_SIGNAL_BUS 3
+#define RPMI_TEE_FEATURE_MULTISEGMENT_OPS 4
+
+#define RPMI_TEE_MEMORY_FEATURE_UNSUPPORTED 0
+#define RPMI_TEE_MEMORY_FEATURE_TEE_ONLY 1
+#define RPMI_TEE_MEMORY_FEATURE_FULLY_SUPPORTED 2
+
+/**
+ * struct rpmi_tee_probe_features_req - TEE_PROBE_FEATURES request
+ * @feature_id: TEE feature identifier to query.
+ */
+struct rpmi_tee_probe_features_req {
+ __le32 feature_id;
+} __packed;
+
+/**
+ * struct rpmi_tee_probe_features_resp - TEE_PROBE_FEATURES response
+ * @status: RPMI completion status.
+ * @value: Feature-specific value.
+ */
+struct rpmi_tee_probe_features_resp {
+ __le32 status;
+ __le32 value;
+} __packed;
+
/**
* struct rpmi_tee_call_req - TEE_CALL request prefix
* @sender_id: Calling REE endpoint identifier.
@@ -186,6 +213,29 @@ static int rpmi_tee_check_transport(struct rpmi_tee_transport *priv)
/* RPMI TEE SERVICE GRP API. */
+/* TEE_PROBE_FEATURES. */
+static int rpmi_tee_probe_features(struct rpmi_tee_transport *priv,
+ u32 feature_id, u32 *value)
+{
+ struct rpmi_tee_probe_features_req req = {
+ .feature_id = cpu_to_le32(feature_id),
+ };
+ struct rpmi_tee_probe_features_resp resp;
+ size_t resp_len = sizeof(resp);
+ int ret;
+
+ ret = rpmi_tee_send(priv, RPMI_TEE_SRV_PROBE_FEATURES, &req,
+ sizeof(req), &resp, &resp_len);
+ if (ret)
+ return ret;
+ if (resp_len != sizeof(resp))
+ return -EPROTO;
+
+ *value = get_unaligned_le32(&resp.value);
+
+ return 0;
+}
+
/* Return the transport that owns @rdev. */
static struct rpmi_tee_transport *
rpmi_tee_device_to_transport(struct rpmi_tee_device *rdev)
@@ -371,6 +421,7 @@ static int rpmi_tee_setup_endpoints(struct rpmi_tee_transport *priv)
static int rpmi_tee_transport_probe(struct platform_device *pdev)
{
struct rpmi_tee_transport *priv;
+ u32 value;
int ret;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
@@ -394,6 +445,28 @@ static int rpmi_tee_transport_probe(struct platform_device *pdev)
goto out_failed;
}
+ ret = rpmi_tee_probe_features(priv, RPMI_TEE_FEATURE_MEMORY_LEND,
+ &value);
+ if (ret)
+ goto out_failed;
+ priv->mem.lend_ok = value == RPMI_TEE_MEMORY_FEATURE_FULLY_SUPPORTED;
+
+ ret = rpmi_tee_probe_features(priv, RPMI_TEE_FEATURE_MEMORY_SHARE,
+ &value);
+ if (ret)
+ goto out_failed;
+ priv->mem.share_ok = value == RPMI_TEE_MEMORY_FEATURE_FULLY_SUPPORTED;
+
+ ret = rpmi_tee_probe_features(priv, RPMI_TEE_FEATURE_MULTISEGMENT_OPS,
+ &priv->mem.multisegment_max);
+ if (ret)
+ goto out_failed;
+
+ ret = rpmi_tee_probe_features(priv, RPMI_TEE_FEATURE_SIGNAL_BUS,
+ &priv->notif.feature);
+ if (ret)
+ goto out_failed;
+
ret = rpmi_tee_setup_endpoints(priv);
if (ret) {
dev_err_probe(&pdev->dev, ret,
diff --git a/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
index 07e6b9913cb7..0c3cfa0f14ec 100644
--- a/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
+++ b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
@@ -14,17 +14,28 @@
/* TEE service group and the services used by this module. */
#define RPMI_SRVGRP_TEE 0x10
+#define RPMI_TEE_SRV_PROBE_FEATURES 0x02
#define RPMI_TEE_SRV_PROBE_SYSTEM 0x03
#define RPMI_TEE_SRV_PROBE_DOMAIN 0x04
#define RPMI_TEE_SRV_PROBE_ENDPOINT 0x05
#define RPMI_TEE_SRV_CALL 0x18
+struct rpmi_tee_notif_state {
+ u32 feature;
+};
+
struct rpmi_tee_mbox {
struct mbox_client client;
struct mbox_chan *chan;
u32 max_msg_data_size;
};
+struct rpmi_tee_mem_state {
+ u32 multisegment_max;
+ bool lend_ok;
+ bool share_ok;
+};
+
/**
* struct rpmi_tee_transport - State for one RPMI TEE transport instance
* @dev: Parent platform device.
@@ -33,6 +44,8 @@ struct rpmi_tee_mbox {
* @max_call_resp_size: Maximum TEE_CALL response payload size in bytes.
* @self_id: Local REE physical endpoint identifier.
* @devices: List of registered TEE service devices.
+ * @mem: Memory parcel operation state.
+ * @notif: Signal notification state.
*/
struct rpmi_tee_transport {
struct device *dev;
@@ -41,6 +54,8 @@ struct rpmi_tee_transport {
size_t max_call_resp_size;
u32 self_id;
struct list_head devices;
+ struct rpmi_tee_mem_state mem;
+ struct rpmi_tee_notif_state notif;
};
/* Report local transport errors separately from the returned RPMI status. */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH RFC v2 09/10] firmware: riscv: rpmi-tee: add memory parcel operations
2026-10-08 3:27 [PATCH RFC v2 00/10] firmware: riscv: add RPMI TEE service group transport Amirreza Zarrabi
` (7 preceding siblings ...)
2026-10-08 3:27 ` [PATCH RFC v2 08/10] firmware: riscv: rpmi-tee: cache TEE capabilities Amirreza Zarrabi
@ 2026-10-08 3:27 ` Amirreza Zarrabi
2026-10-08 3:54 ` sashiko-bot
2026-10-08 3:27 ` [PATCH RFC v2 10/10] firmware: riscv: rpmi-tee: add signal bus support Amirreza Zarrabi
9 siblings, 1 reply; 16+ messages in thread
From: Amirreza Zarrabi @ 2026-10-08 3:27 UTC (permalink / raw)
To: Jassi Brar, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rahul Pathak, Anup Patel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amirreza Zarrabi,
Jens Wiklander, Sumit Garg, Marouene Boubakri
Cc: linux-arm-msm, linux-kernel, linux-riscv, op-tee, devicetree,
Amirreza Zarrabi
Add memory lending, sharing and reclamation through RPMI memory parcels
so service drivers can exchange memory with TEE endpoints.
Support segmented parcels when memory descriptions exceed the mailbox
payload limit, respecting the firmware's limit on concurrent segmented
operations.
Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
drivers/firmware/riscv_rpmi_tee/driver.c | 477 +++++++++++++++++++++
drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h | 6 +
include/linux/rpmi_tee.h | 58 +++
3 files changed, 541 insertions(+)
diff --git a/drivers/firmware/riscv_rpmi_tee/driver.c b/drivers/firmware/riscv_rpmi_tee/driver.c
index b43a09f5e853..690561ed949d 100644
--- a/drivers/firmware/riscv_rpmi_tee/driver.c
+++ b/drivers/firmware/riscv_rpmi_tee/driver.c
@@ -13,6 +13,7 @@
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/rpmi_tee.h>
+#include <linux/scatterlist.h>
#include <linux/slab.h>
#include <linux/unaligned.h>
@@ -27,6 +28,20 @@
#define RPMI_TEE_MEMORY_FEATURE_TEE_ONLY 1
#define RPMI_TEE_MEMORY_FEATURE_FULLY_SUPPORTED 2
+/* TEE_CALL memory-access flags and block format. */
+#define RPMI_TEE_ACCESS_READ BIT(29)
+#define RPMI_TEE_ACCESS_WRITE BIT(30)
+#define RPMI_TEE_ACCESS_EXEC BIT(31)
+
+#define RPMI_TEE_MEM_PAGE_SHIFT 12
+#define RPMI_TEE_MEM_PAGE_SIZE BIT(RPMI_TEE_MEM_PAGE_SHIFT)
+#define RPMI_TEE_BLOCK_MAX_PAGES 4096
+
+/* TEE_MEMORY_PARCEL_CREATE flags. */
+#define RPMI_TEE_PARCEL_MULTI_SEGMENT BIT(31)
+
+/* TEE_MEMORY_SEGMENT_SEND flags. */
+#define RPMI_TEE_SEGMENT_LAST BIT(31)
/**
* struct rpmi_tee_probe_features_req - TEE_PROBE_FEATURES request
* @feature_id: TEE feature identifier to query.
@@ -73,11 +88,116 @@ struct rpmi_tee_call_resp {
u8 service_data[];
} __packed;
+/**
+ * struct rpmi_tee_parcel_create_req - MEMORY_PARCEL_CREATE request prefix
+ * @creator_id: Endpoint identifier creating the parcel.
+ * @creator_access: Creator's residual access permissions.
+ * @receiver_count: Number of receiver endpoint and access pairs in @data.
+ * @flags: Parcel creation flags.
+ * @nonce: Caller-provided parcel nonce.
+ * @block_count: Number of memory blocks included in this request.
+ * @label: Caller-provided parcel label.
+ * @data: Receiver endpoint IDs, receiver access values, then memory blocks.
+ */
+struct rpmi_tee_parcel_create_req {
+ __le32 creator_id;
+ __le32 creator_access;
+ __le32 receiver_count;
+ __le32 flags;
+ __le32 nonce;
+ __le32 block_count;
+ u8 label[16];
+ u8 data[];
+} __packed;
+
+#define RPMI_TEE_PARCEL_CREATE_SIZE \
+ (sizeof(struct rpmi_tee_parcel_create_req))
+/* Size of one RECEIVER_ID[] and ACCESS[] entry pair. */
+#define RPMI_TEE_PARCEL_CREATE_RECEIVER_INFO_SIZE (2 * sizeof(__le32))
+/* Size of one BLOCK_HIGH[] and BLOCK_LOW[] entry pair. */
+#define RPMI_TEE_PARCEL_CREATE_BLOCK_SIZE (2 * sizeof(__le32))
+
+/* Store one receiver in adjacent RECEIVER_ID[] and ACCESS[] arrays at @data. */
+static inline void rpmi_tee_put_receiver(u8 *data, u32 count, u32 index,
+ u32 id, u32 access)
+{
+ put_unaligned_le32(id, data + index * sizeof(__le32));
+ put_unaligned_le32(access, data + (count + index) * sizeof(__le32));
+}
+
+/* Store one block in adjacent BLOCK_HIGH[] and BLOCK_LOW[] arrays at @data. */
+static inline void rpmi_tee_put_block(u8 *data, u32 count, u32 index,
+ u32 high, u32 low)
+{
+ put_unaligned_le32(high, data + index * sizeof(__le32));
+ put_unaligned_le32(low, data + (count + index) * sizeof(__le32));
+}
+
+/**
+ * struct rpmi_tee_parcel_create_resp - MEMORY_PARCEL_CREATE response
+ * @status: RPMI completion status.
+ * @parcel_id: Identifier assigned to the new parcel.
+ */
+struct rpmi_tee_parcel_create_resp {
+ __le32 status;
+ __le32 parcel_id;
+} __packed;
+
+/**
+ * struct rpmi_tee_segment_send_req - MEMORY_SEGMENT_SEND request prefix
+ * @parcel_id: Identifier of the partially created parcel.
+ * @flags: Segment flags.
+ * @block_count: Number of memory blocks in @data.
+ * @data: Memory block address and size pairs.
+ */
+struct rpmi_tee_segment_send_req {
+ __le32 parcel_id;
+ __le32 flags;
+ __le32 block_count;
+ u8 data[];
+} __packed;
+
+#define RPMI_TEE_SEGMENT_SEND_SIZE \
+ (sizeof(struct rpmi_tee_segment_send_req))
+/* Size of one BLOCK_HIGH[] and BLOCK_LOW[] entry pair. */
+#define RPMI_TEE_SEGMENT_SEND_BLOCK_SIZE (2 * sizeof(__le32))
+
+/**
+ * struct rpmi_tee_parcel_reclaim_req - MEMORY_PARCEL_RECLAIM request
+ * @parcel_id: Identifier of the parcel to reclaim.
+ */
+struct rpmi_tee_parcel_reclaim_req {
+ __le32 parcel_id;
+} __packed;
+
+/**
+ * struct rpmi_tee_parcel_reclaim_resp - MEMORY_PARCEL_RECLAIM response
+ * @status: RPMI completion status.
+ * @flags: Reclaim result flags.
+ */
+struct rpmi_tee_parcel_reclaim_resp {
+ __le32 status;
+ __le32 flags;
+} __packed;
+
struct rpmi_tee_child {
struct list_head node;
struct rpmi_tee_device *rdev;
};
+struct rpmi_tee_block_iter {
+ struct scatterlist *sg;
+ phys_addr_t address;
+ size_t length;
+};
+
+struct rpmi_tee_parcel_xfer {
+ struct rpmi_tee_block_iter iter;
+ u32 parcel_id;
+ u32 block_count;
+ u32 next_block;
+};
+
/* rpmi_tee_send_with_status() - Send an RPMI TEE service request. */
int rpmi_tee_send_with_status(struct rpmi_tee_transport *priv, u32 service_id,
const void *req, size_t req_len, void *resp,
@@ -326,6 +446,355 @@ static int rpmi_tee_op_call(struct rpmi_tee_device *rdev, const void *req,
return 0;
}
+/* MEMORY_PARCEL_RECLAIM. */
+static int rpmi_tee_memory_reclaim(struct rpmi_tee_transport *priv,
+ u32 parcel_id)
+{
+ struct rpmi_tee_parcel_reclaim_req req = {
+ .parcel_id = cpu_to_le32(parcel_id),
+ };
+ struct rpmi_tee_parcel_reclaim_resp resp;
+ size_t resp_len = sizeof(resp);
+ int ret;
+
+ ret = rpmi_tee_send(priv, RPMI_TEE_SRV_MEMORY_PARCEL_RECLAIM, &req,
+ sizeof(req), &resp, &resp_len);
+ if (ret)
+ return ret;
+
+ if (resp_len != sizeof(resp))
+ return -EPROTO;
+
+ return 0;
+}
+
+static int rpmi_tee_op_memory_reclaim(struct rpmi_tee_device *rdev,
+ u32 parcel_id)
+{
+ return rpmi_tee_memory_reclaim(rpmi_tee_device_to_transport(rdev),
+ parcel_id);
+}
+
+/**
+ * rpmi_tee_count_blocks_sg - Count RPMI memory blocks in an SG list
+ * @sg: First SG entry describing the memory to share or lend.
+ * @count_out: Returns the number of RPMI memory blocks.
+ *
+ * Validates that every entry represents one or more whole 4 KiB pages. An
+ * RPMI memory block represents at most @RPMI_TEE_BLOCK_MAX_PAGES pages.
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+static int rpmi_tee_count_blocks_sg(struct scatterlist *sg, u32 *count_out)
+{
+ struct scatterlist *entry;
+ u32 count = 0;
+
+ if (!sg)
+ return -EINVAL;
+
+ for (entry = sg; entry; entry = sg_next(entry)) {
+ phys_addr_t address = sg_phys(entry);
+ size_t blocks;
+
+ if (!entry->length ||
+ !IS_ALIGNED(address, RPMI_TEE_MEM_PAGE_SIZE) ||
+ !IS_ALIGNED(entry->length, RPMI_TEE_MEM_PAGE_SIZE))
+ return -EINVAL;
+
+ /* One RPMI block describes at most 4096 pages. */
+ blocks = DIV_ROUND_UP(entry->length >> RPMI_TEE_MEM_PAGE_SHIFT,
+ RPMI_TEE_BLOCK_MAX_PAGES);
+ if (blocks > U32_MAX - count)
+ return -EOVERFLOW;
+
+ count += blocks;
+ }
+
+ *count_out = count;
+
+ return 0;
+}
+
+static void rpmi_tee_block_iter_init(struct rpmi_tee_block_iter *iter,
+ struct scatterlist *sg)
+{
+ iter->sg = sg;
+ iter->address = 0;
+ iter->length = 0;
+}
+
+/* Encode the next RPMI memory block from an SG iterator. */
+static bool rpmi_tee_block_iter_next(struct rpmi_tee_block_iter *iter,
+ u32 *high, u32 *low)
+{
+ u32 pages;
+
+ if (!iter->length) {
+ if (!iter->sg)
+ return false;
+ /* Next SG. */
+ iter->address = sg_phys(iter->sg);
+ iter->length = iter->sg->length;
+ iter->sg = sg_next(iter->sg);
+ }
+
+ /* RPMI memory block represents at most @RPMI_TEE_BLOCK_MAX_PAGES pages. */
+ pages = min_t(size_t, iter->length >> RPMI_TEE_MEM_PAGE_SHIFT,
+ RPMI_TEE_BLOCK_MAX_PAGES);
+
+ *high = upper_32_bits(iter->address);
+ *low = lower_32_bits(iter->address) | (pages - 1);
+
+ iter->address += (phys_addr_t)pages << RPMI_TEE_MEM_PAGE_SHIFT;
+ iter->length -= (size_t)pages << RPMI_TEE_MEM_PAGE_SHIFT;
+
+ return true;
+}
+
+static int rpmi_tee_fill_blocks(struct rpmi_tee_block_iter *iter, u8 *data,
+ u32 count)
+{
+ u32 high, low, i;
+
+ for (i = 0; i < count; i++) {
+ if (!rpmi_tee_block_iter_next(iter, &high, &low))
+ return -EINVAL;
+
+ rpmi_tee_put_block(data, count, i, high, low);
+ }
+
+ return 0;
+}
+
+/* Convert memory access flags RPMI_TEE_MEM_ACCESS_* to RPMI_TEE_ACCESS_*. */
+static u32 rpmi_tee_access(u32 mem_access)
+{
+ u32 tee_access = 0;
+
+ if (mem_access & RPMI_TEE_MEM_ACCESS_READ)
+ tee_access |= RPMI_TEE_ACCESS_READ;
+ if (mem_access & RPMI_TEE_MEM_ACCESS_WRITE)
+ tee_access |= RPMI_TEE_ACCESS_WRITE;
+ if (mem_access & RPMI_TEE_MEM_ACCESS_EXEC)
+ tee_access |= RPMI_TEE_ACCESS_EXEC;
+
+ return tee_access;
+}
+
+static int rpmi_tee_reserve_segment_slot(struct rpmi_tee_transport *priv)
+{
+ int ret = 0;
+
+ guard(mutex)(&priv->mem.lock);
+ if (priv->mem.multisegment_active == priv->mem.multisegment_max)
+ ret = -EBUSY;
+ else
+ priv->mem.multisegment_active++;
+
+ return ret;
+}
+
+static void rpmi_tee_release_segment_slot(struct rpmi_tee_transport *priv)
+{
+ guard(mutex)(&priv->mem.lock);
+ priv->mem.multisegment_active--;
+}
+
+/* Send the remaining blocks of a segmented memory parcel. */
+static int rpmi_tee_parcel_send_segments(struct rpmi_tee_transport *priv,
+ struct rpmi_tee_parcel_xfer *xfer)
+{
+ while (xfer->next_block < xfer->block_count) {
+ size_t req_len;
+ u32 count;
+ int ret;
+
+ count = min_t(u32, xfer->block_count - xfer->next_block,
+ (priv->mbox.max_msg_data_size -
+ RPMI_TEE_SEGMENT_SEND_SIZE) /
+ RPMI_TEE_SEGMENT_SEND_BLOCK_SIZE);
+ if (!count)
+ return -EMSGSIZE;
+
+ req_len = RPMI_TEE_SEGMENT_SEND_SIZE +
+ RPMI_TEE_SEGMENT_SEND_BLOCK_SIZE * count;
+
+ struct rpmi_tee_segment_send_req *req __free(kfree) =
+ kzalloc(req_len, GFP_KERNEL);
+ if (!req)
+ return -ENOMEM;
+
+ /* INIT request. */
+ req->parcel_id = cpu_to_le32(xfer->parcel_id);
+ req->flags = cpu_to_le32(xfer->next_block + count ==
+ xfer->block_count ?
+ RPMI_TEE_SEGMENT_LAST : 0);
+ req->block_count = cpu_to_le32(count);
+ ret = rpmi_tee_fill_blocks(&xfer->iter, req->data, count);
+ if (ret)
+ return ret;
+
+ ret = rpmi_tee_send(priv, RPMI_TEE_SRV_MEMORY_SEGMENT_SEND,
+ req, req_len, NULL, NULL);
+ if (ret)
+ return ret;
+
+ xfer->next_block += count;
+ }
+
+ return 0;
+}
+
+/* Create a memory parcel after the caller has validated its operation. */
+static int rpmi_tee_parcel_create(struct rpmi_tee_transport *priv,
+ struct rpmi_tee_mem_args *args)
+{
+ struct rpmi_tee_parcel_create_resp resp;
+ struct rpmi_tee_parcel_xfer xfer;
+ size_t blk_off, req_len, resp_len;
+ bool segmented;
+ int ret;
+ u32 i;
+
+ ret = rpmi_tee_count_blocks_sg(args->sg, &xfer.block_count);
+ if (ret)
+ return ret;
+
+ /* BLOCK_HIGH[] follows the request header and receiver arrays. */
+ blk_off = RPMI_TEE_PARCEL_CREATE_SIZE + args->receiver_count *
+ RPMI_TEE_PARCEL_CREATE_RECEIVER_INFO_SIZE;
+
+ /* Limit the initial request to the parcel's actual block count. */
+ xfer.next_block = min((priv->mbox.max_msg_data_size - blk_off) /
+ RPMI_TEE_PARCEL_CREATE_BLOCK_SIZE,
+ xfer.block_count);
+
+ segmented = xfer.next_block < xfer.block_count;
+ if (segmented) {
+ if (!priv->mem.multisegment_max)
+ return -EOPNOTSUPP;
+ /* Reserve a slot against the firmware's advertised limit. */
+ ret = rpmi_tee_reserve_segment_slot(priv);
+ if (ret)
+ return ret;
+ }
+
+ req_len = blk_off + RPMI_TEE_PARCEL_CREATE_BLOCK_SIZE * xfer.next_block;
+
+ struct rpmi_tee_parcel_create_req *req __free(kfree) =
+ kzalloc(req_len, GFP_KERNEL);
+ if (!req) {
+ ret = -ENOMEM;
+ goto out_release_slot;
+ }
+
+ rpmi_tee_block_iter_init(&xfer.iter, args->sg);
+
+ /* INIT request. */
+ req->creator_id = cpu_to_le32(priv->self_id);
+ req->creator_access = cpu_to_le32(rpmi_tee_access(args->creator_access));
+ req->receiver_count = cpu_to_le32(args->receiver_count);
+ req->flags = cpu_to_le32(segmented ? RPMI_TEE_PARCEL_MULTI_SEGMENT : 0);
+ req->nonce = cpu_to_le32(args->nonce);
+ req->block_count = cpu_to_le32(xfer.next_block);
+ memcpy(req->label, args->label, sizeof(req->label));
+
+ for (i = 0; i < args->receiver_count; i++) {
+ u32 tee_access = rpmi_tee_access(args->receivers[i].access);
+ /* Store RECEIVER_ID[i] and ACCESS[i]. */
+ rpmi_tee_put_receiver(req->data, args->receiver_count, i,
+ args->receivers[i].endpoint_id,
+ tee_access);
+ }
+
+ ret = rpmi_tee_fill_blocks(&xfer.iter,
+ req->data + 8 * args->receiver_count,
+ xfer.next_block);
+ if (ret)
+ goto out_release_slot;
+
+ resp_len = sizeof(resp);
+ ret = rpmi_tee_send(priv, RPMI_TEE_SRV_MEMORY_PARCEL_CREATE,
+ req, req_len, &resp, &resp_len);
+ if (ret)
+ goto out_release_slot;
+ if (resp_len != sizeof(resp))
+ return -EPROTO;
+
+ xfer.parcel_id = get_unaligned_le32(&resp.parcel_id);
+ /* Send remaining blocks as segments. */
+ ret = rpmi_tee_parcel_send_segments(priv, &xfer);
+ if (ret) {
+ /* On error, retain the slot as firmware may still hold it. */
+ if (rpmi_tee_memory_reclaim(priv, xfer.parcel_id)) {
+ dev_warn(priv->dev, "failed to abort parcel %#x\n",
+ xfer.parcel_id);
+
+ return ret;
+ }
+ } else {
+ args->parcel_id = xfer.parcel_id;
+ }
+
+out_release_slot:
+ if (segmented)
+ rpmi_tee_release_segment_slot(priv);
+
+ return ret;
+}
+
+/* MEMORY_PARCEL_CREATE. */
+static int rpmi_tee_op_parcel_create(struct rpmi_tee_device *rdev,
+ struct rpmi_tee_mem_args *args, bool lend)
+{
+ struct rpmi_tee_transport *priv = rpmi_tee_device_to_transport(rdev);
+ u32 i;
+
+ if (!args || !args->receivers || !args->receiver_count)
+ return -EINVAL;
+
+ /* LEND relinquishes creator access, whereas SHARE retains it. */
+ if (lend ? args->creator_access : !args->creator_access)
+ return -EINVAL;
+
+ if (args->creator_access & ~RPMI_TEE_MEM_ACCESS_MASK)
+ return -EINVAL;
+ for (i = 0; i < args->receiver_count; i++) {
+ if (args->receivers[i].access & ~RPMI_TEE_MEM_ACCESS_MASK)
+ return -EINVAL;
+ }
+
+ if (lend ? !priv->mem.lend_ok : !priv->mem.share_ok)
+ return -EOPNOTSUPP;
+
+ /* A parcel must contain at least one BLOCK_HIGH/BLOCK_LOW pair. */
+ if (priv->mbox.max_msg_data_size < RPMI_TEE_PARCEL_CREATE_SIZE +
+ RPMI_TEE_PARCEL_CREATE_BLOCK_SIZE)
+ return -EMSGSIZE;
+
+ /* Check if receiver's info fit after reserving room for one block. */
+ if (args->receiver_count >
+ (priv->mbox.max_msg_data_size - RPMI_TEE_PARCEL_CREATE_SIZE -
+ RPMI_TEE_PARCEL_CREATE_BLOCK_SIZE) /
+ RPMI_TEE_PARCEL_CREATE_RECEIVER_INFO_SIZE)
+ return -EMSGSIZE;
+
+ return rpmi_tee_parcel_create(priv, args);
+}
+
+static int rpmi_tee_op_memory_lend(struct rpmi_tee_device *rdev,
+ struct rpmi_tee_mem_args *args)
+{
+ return rpmi_tee_op_parcel_create(rdev, args, true);
+}
+
+static int rpmi_tee_op_memory_share(struct rpmi_tee_device *rdev,
+ struct rpmi_tee_mem_args *args)
+{
+ return rpmi_tee_op_parcel_create(rdev, args, false);
+}
+
static const struct rpmi_tee_info_ops rpmi_tee_info_ops = {
.msg_limits_get = rpmi_tee_op_msg_limits_get,
};
@@ -334,9 +803,16 @@ static const struct rpmi_tee_msg_ops rpmi_tee_msg_ops = {
.call = rpmi_tee_op_call,
};
+static const struct rpmi_tee_mem_ops rpmi_tee_mem_ops = {
+ .memory_lend = rpmi_tee_op_memory_lend,
+ .memory_share = rpmi_tee_op_memory_share,
+ .memory_reclaim = rpmi_tee_op_memory_reclaim,
+};
+
static const struct rpmi_tee_ops rpmi_tee_ops = {
.info_ops = &rpmi_tee_info_ops,
.msg_ops = &rpmi_tee_msg_ops,
+ .mem_ops = &rpmi_tee_mem_ops,
};
static void rpmi_tee_unregister_devices(struct rpmi_tee_transport *priv)
@@ -431,6 +907,7 @@ static int rpmi_tee_transport_probe(struct platform_device *pdev)
priv->dev = &pdev->dev;
platform_set_drvdata(pdev, priv);
INIT_LIST_HEAD(&priv->devices);
+ mutex_init(&priv->mem.lock);
priv->mbox.client.dev = &pdev->dev;
priv->mbox.client.tx_sync = true;
priv->mbox.chan = mbox_request_channel(&priv->mbox.client, 0);
diff --git a/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
index 0c3cfa0f14ec..4df47d52724a 100644
--- a/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
+++ b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
@@ -8,6 +8,7 @@
#include <linux/list.h>
#include <linux/mailbox_client.h>
+#include <linux/mutex.h>
#include <linux/types.h>
#include <linux/uuid.h>
@@ -19,6 +20,9 @@
#define RPMI_TEE_SRV_PROBE_DOMAIN 0x04
#define RPMI_TEE_SRV_PROBE_ENDPOINT 0x05
#define RPMI_TEE_SRV_CALL 0x18
+#define RPMI_TEE_SRV_MEMORY_PARCEL_CREATE 0x0e
+#define RPMI_TEE_SRV_MEMORY_PARCEL_RECLAIM 0x11
+#define RPMI_TEE_SRV_MEMORY_SEGMENT_SEND 0x12
struct rpmi_tee_notif_state {
u32 feature;
@@ -31,7 +35,9 @@ struct rpmi_tee_mbox {
};
struct rpmi_tee_mem_state {
+ struct mutex lock;
u32 multisegment_max;
+ u32 multisegment_active;
bool lend_ok;
bool share_ok;
};
diff --git a/include/linux/rpmi_tee.h b/include/linux/rpmi_tee.h
index a76055149996..10f6cae4a3a7 100644
--- a/include/linux/rpmi_tee.h
+++ b/include/linux/rpmi_tee.h
@@ -10,6 +10,7 @@
#include <linux/device.h>
#include <linux/module.h>
+#include <linux/scatterlist.h>
#include <linux/types.h>
#include <linux/uuid.h>
@@ -70,10 +71,67 @@ struct rpmi_tee_msg_ops {
size_t req_len, void *resp, size_t *resp_len);
};
+/* Access permissions used by memory parcel operations. */
+#define RPMI_TEE_MEM_ACCESS_READ BIT(0)
+#define RPMI_TEE_MEM_ACCESS_WRITE BIT(1)
+#define RPMI_TEE_MEM_ACCESS_EXEC BIT(2)
+#define RPMI_TEE_MEM_ACCESS_MASK (RPMI_TEE_MEM_ACCESS_READ | \
+ RPMI_TEE_MEM_ACCESS_WRITE | \
+ RPMI_TEE_MEM_ACCESS_EXEC)
+
+/* One receiver's access rights for a memory parcel. */
+struct rpmi_tee_mem_receiver {
+ /* RPMI endpoint identifier of the receiver. */
+ u32 endpoint_id;
+ /* Bitwise OR of RPMI_TEE_MEM_ACCESS_* permissions. */
+ u32 access;
+};
+
+/* Arguments used to create a memory parcel. */
+struct rpmi_tee_mem_args {
+ /* Scatterlist describing whole, 4 KiB-aligned memory pages. */
+ struct scatterlist *sg;
+ /* Array of @receiver_count parcel receivers. */
+ const struct rpmi_tee_mem_receiver *receivers;
+ /* Number of entries in @receivers. */
+ u32 receiver_count;
+ /* Creator permissions retained by a SHARE operation. */
+ u32 creator_access;
+ /* Implementation-defined value carried in the parcel descriptor. */
+ u32 nonce;
+ /* Implementation-defined 16-byte parcel label. */
+ u8 label[16];
+ /* Returned firmware-assigned parcel identifier on success. */
+ u32 parcel_id;
+};
+
+/* RPMI TEE memory-parcel operations. */
+struct rpmi_tee_mem_ops {
+ /**
+ * @memory_lend: Lend the pages described by @args to its receivers. The
+ * creator must not retain access, so @args->creator_access must be zero.
+ */
+ int (*memory_lend)(struct rpmi_tee_device *rdev,
+ struct rpmi_tee_mem_args *args);
+ /**
+ * @memory_share: Share the pages described by @args with its receivers.
+ * The creator retains the nonzero permissions in
+ * @args->creator_access.
+ */
+ int (*memory_share)(struct rpmi_tee_device *rdev,
+ struct rpmi_tee_mem_args *args);
+ /**
+ * @memory_reclaim: Reclaim the parcel identified by @parcel_id after all
+ * receivers have relinquished it.
+ */
+ int (*memory_reclaim)(struct rpmi_tee_device *rdev, u32 parcel_id);
+};
+
/* RPMI TEE transport operation groups. */
struct rpmi_tee_ops {
const struct rpmi_tee_info_ops *info_ops;
const struct rpmi_tee_msg_ops *msg_ops;
+ const struct rpmi_tee_mem_ops *mem_ops;
};
extern const struct bus_type rpmi_tee_bus_type;
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH RFC v2 09/10] firmware: riscv: rpmi-tee: add memory parcel operations
2026-10-08 3:27 ` [PATCH RFC v2 09/10] firmware: riscv: rpmi-tee: add memory parcel operations Amirreza Zarrabi
@ 2026-10-08 3:54 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-10-08 3:54 UTC (permalink / raw)
To: Amirreza Zarrabi; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] rpmi-tee: segment slot leak and goto mixing in rpmi_tee_parcel_create()
- [Low] cleanup: mixing goto and __free() in rpmi_tee_parcel_create()
--
commit cb2515b520037bb2b95420754a397f9b4e4d3e5d
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
firmware: riscv: rpmi-tee: add memory parcel operations
Add memory lending, sharing and reclamation through RPMI memory parcels
so service drivers can exchange memory with TEE endpoints. This patch
introduces two issues in rpmi_tee_parcel_create(): a mixture of __free()
and goto-based cleanup, and a resource leak caused by bypassing the cleanup
label.
> diff --git a/drivers/firmware/riscv_rpmi_tee/driver.c b/drivers/firmware/riscv_rpmi_tee/driver.c
> index b43a09f5e8536..690561ed949dc 100644
> --- a/drivers/firmware/riscv_rpmi_tee/driver.c
> +++ b/drivers/firmware/riscv_rpmi_tee/driver.c
[ ... ]
> @@ -326,6 +446,355 @@ static int rpmi_tee_op_call(struct rpmi_tee_device *rdev, const void *req,
[ ... ]
> +/* Create a memory parcel after the caller has validated its operation. */
> +static int rpmi_tee_parcel_create(struct rpmi_tee_transport *priv,
> + struct rpmi_tee_mem_args *args)
> +{
> + struct rpmi_tee_parcel_create_resp resp;
> + struct rpmi_tee_parcel_xfer xfer;
> + size_t blk_off, req_len, resp_len;
> + bool segmented;
> + int ret;
> + u32 i;
> +
> + ret = rpmi_tee_count_blocks_sg(args->sg, &xfer.block_count);
> + if (ret)
> + return ret;
> +
> + /* BLOCK_HIGH[] follows the request header and receiver arrays. */
> + blk_off = RPMI_TEE_PARCEL_CREATE_SIZE + args->receiver_count *
> + RPMI_TEE_PARCEL_CREATE_RECEIVER_INFO_SIZE;
> +
> + /* Limit the initial request to the parcel's actual block count. */
> + xfer.next_block = min((priv->mbox.max_msg_data_size - blk_off) /
> + RPMI_TEE_PARCEL_CREATE_BLOCK_SIZE,
> + xfer.block_count);
> +
> + segmented = xfer.next_block < xfer.block_count;
> + if (segmented) {
> + if (!priv->mem.multisegment_max)
> + return -EOPNOTSUPP;
> + /* Reserve a slot against the firmware's advertised limit. */
> + ret = rpmi_tee_reserve_segment_slot(priv);
> + if (ret)
> + return ret;
> + }
> +
> + req_len = blk_off + RPMI_TEE_PARCEL_CREATE_BLOCK_SIZE * xfer.next_block;
> +
> + struct rpmi_tee_parcel_create_req *req __free(kfree) =
> + kzalloc(req_len, GFP_KERNEL);
> + if (!req) {
> + ret = -ENOMEM;
> + goto out_release_slot;
> + }
[Severity: Low]
Does this mix goto-based error handling with scope-based cleanup?
The cleanup.md subsystem guide explicitly forbids mixing goto-based cleanup
labels and __free() declarations in the same function because it creates
confusing ownership semantics and leak hazards.
Could this be refactored to either use scope-based cleanup for all resources or
stick entirely to goto-based cleanup?
> +
> + rpmi_tee_block_iter_init(&xfer.iter, args->sg);
> +
> + /* INIT request. */
> + req->creator_id = cpu_to_le32(priv->self_id);
> + req->creator_access = cpu_to_le32(rpmi_tee_access(args->creator_access));
> + req->receiver_count = cpu_to_le32(args->receiver_count);
> + req->flags = cpu_to_le32(segmented ? RPMI_TEE_PARCEL_MULTI_SEGMENT : 0);
> + req->nonce = cpu_to_le32(args->nonce);
> + req->block_count = cpu_to_le32(xfer.next_block);
> + memcpy(req->label, args->label, sizeof(req->label));
> +
> + for (i = 0; i < args->receiver_count; i++) {
> + u32 tee_access = rpmi_tee_access(args->receivers[i].access);
> + /* Store RECEIVER_ID[i] and ACCESS[i]. */
> + rpmi_tee_put_receiver(req->data, args->receiver_count, i,
> + args->receivers[i].endpoint_id,
> + tee_access);
> + }
> +
> + ret = rpmi_tee_fill_blocks(&xfer.iter,
> + req->data + 8 * args->receiver_count,
> + xfer.next_block);
> + if (ret)
> + goto out_release_slot;
> +
> + resp_len = sizeof(resp);
> + ret = rpmi_tee_send(priv, RPMI_TEE_SRV_MEMORY_PARCEL_CREATE,
> + req, req_len, &resp, &resp_len);
> + if (ret)
> + goto out_release_slot;
> + if (resp_len != sizeof(resp))
> + return -EPROTO;
[Severity: Medium]
Does this direct return leak the reserved segment slot?
If the firmware returns a malformed response with a valid length but not exactly
equal to sizeof(resp), the direct return of -EPROTO bypasses the
goto out_release_slot; cleanup path.
This causes a permanent leak of the multisegment_active accounting slot, which
can permanently block future multisegment memory operations with -EBUSY once
the limit is reached.
This seems to be a direct consequence of mixing __free() and goto in the same
function. Should this also use goto out_release_slot; to ensure the slot
is properly released?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-riscv-rpmi-tee-abi-v2-0-45939a13d223@oss.qualcomm.com?part=9
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH RFC v2 10/10] firmware: riscv: rpmi-tee: add signal bus support
2026-10-08 3:27 [PATCH RFC v2 00/10] firmware: riscv: add RPMI TEE service group transport Amirreza Zarrabi
` (8 preceding siblings ...)
2026-10-08 3:27 ` [PATCH RFC v2 09/10] firmware: riscv: rpmi-tee: add memory parcel operations Amirreza Zarrabi
@ 2026-10-08 3:27 ` Amirreza Zarrabi
2026-10-08 3:56 ` sashiko-bot
9 siblings, 1 reply; 16+ messages in thread
From: Amirreza Zarrabi @ 2026-10-08 3:27 UTC (permalink / raw)
To: Jassi Brar, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Rahul Pathak, Anup Patel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amirreza Zarrabi,
Jens Wiklander, Sumit Garg, Marouene Boubakri
Cc: linux-arm-msm, linux-kernel, linux-riscv, op-tee, devicetree,
Amirreza Zarrabi
Add signal buses for asynchronous notifications between Linux and TEE
endpoints, using the firmware-reported System MSI for interrupt delivery.
Allow service drivers to reserve and relinquish incoming signals and
raise outgoing signals. Dispatch incoming notifications through client
callbacks.
Drain notification callbacks before removing service devices, while
keeping the mailbox and signal buses available until client removal
completes.
Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
drivers/firmware/riscv_rpmi_tee/driver.c | 599 ++++++++++++++++++++-
drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h | 26 +
include/linux/rpmi_tee.h | 24 +
3 files changed, 647 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/riscv_rpmi_tee/driver.c b/drivers/firmware/riscv_rpmi_tee/driver.c
index 690561ed949d..36a5ad651351 100644
--- a/drivers/firmware/riscv_rpmi_tee/driver.c
+++ b/drivers/firmware/riscv_rpmi_tee/driver.c
@@ -8,14 +8,20 @@
#include <linux/mailbox_client.h>
#include <linux/mailbox/riscv-rpmi-message.h>
#include <linux/cleanup.h>
+#include <linux/bitfield.h>
+#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
#include <linux/list.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_irq.h>
#include <linux/platform_device.h>
#include <linux/rpmi_tee.h>
#include <linux/scatterlist.h>
#include <linux/slab.h>
#include <linux/unaligned.h>
+#include <linux/workqueue.h>
+#include <linux/xarray.h>
#include "rpmi_tee_private.h"
@@ -42,6 +48,15 @@
/* TEE_MEMORY_SEGMENT_SEND flags. */
#define RPMI_TEE_SEGMENT_LAST BIT(31)
+
+/* TEE_SIGNAL_BUS_SETUP feature value and TEE_SIGNAL_RETRIEVE flags. */
+#define RPMI_TEE_SIGNAL_MODE_MASK GENMASK(1, 0)
+#define RPMI_TEE_SIGNAL_WIDTH_MASK GENMASK(11, 2)
+#define RPMI_TEE_SIGNAL_INDEX_MASK GENMASK(31, 12)
+
+#define RPMI_TEE_SIGNAL_MODE_SYSTEM_MSI 1
+#define RPMI_TEE_SIGNAL_MORE_AVAILABLE BIT(31)
+
/**
* struct rpmi_tee_probe_features_req - TEE_PROBE_FEATURES request
* @feature_id: TEE feature identifier to query.
@@ -180,6 +195,87 @@ struct rpmi_tee_parcel_reclaim_resp {
__le32 flags;
} __packed;
+/**
+ * struct rpmi_tee_signal_bus_setup_req - SIGNAL_BUS_SETUP request
+ * @target_id: TEE endpoint identifier that owns the signal bus.
+ * @bus_width: Total number of signal IDs in the bus.
+ * @sender_signals: Number of signal IDs reserved for the endpoint sender.
+ */
+struct rpmi_tee_signal_bus_setup_req {
+ __le32 target_id;
+ __le32 bus_width;
+ __le32 sender_signals;
+} __packed;
+
+/**
+ * struct rpmi_tee_signal_bus_teardown_req - SIGNAL_BUS_TEARDOWN request
+ * @target_id: TEE endpoint identifier that owns the signal bus.
+ */
+struct rpmi_tee_signal_bus_teardown_req {
+ __le32 target_id;
+} __packed;
+
+/**
+ * struct rpmi_tee_signal_raise_req - SIGNAL_RAISE request prefix
+ * @target_id: TEE endpoint identifier that owns the signal bus.
+ * @signal_count: Number of signal IDs in @signals.
+ * @signals: Signal IDs to raise.
+ */
+struct rpmi_tee_signal_raise_req {
+ __le32 target_id;
+ __le32 signal_count;
+ __le32 signals[];
+} __packed;
+
+/**
+ * struct rpmi_tee_signal_raise_one_req - Single-signal SIGNAL_RAISE request
+ * @target_id: TEE endpoint identifier that owns the signal bus.
+ * @signal_count: Must be one.
+ * @signal: Signal ID to raise.
+ */
+struct rpmi_tee_signal_raise_one_req {
+ __le32 target_id;
+ __le32 signal_count;
+ __le32 signal;
+} __packed;
+
+/**
+ * struct rpmi_tee_signal_retrieve_resp - SIGNAL_RETRIEVE response prefix
+ * @status: RPMI completion status.
+ * @flags: Response flags.
+ * @target_id: TEE endpoint identifier that owns the signal bus.
+ * @signal_count: Number of signal IDs in @signals.
+ * @signals: Retrieved signal IDs.
+ */
+struct rpmi_tee_signal_retrieve_resp {
+ __le32 status;
+ __le32 flags;
+ __le32 target_id;
+ __le32 signal_count;
+ __le32 signals[];
+} __packed;
+
+enum rpmi_tee_signal_state {
+ RPMI_TEE_SIGNAL_ACTIVE,
+ RPMI_TEE_SIGNAL_RELEASING,
+};
+
+struct rpmi_tee_signal_reservation {
+ struct rpmi_tee_device *rdev;
+ rpmi_tee_notifier_cb cb;
+ void *cb_data;
+ enum rpmi_tee_signal_state state;
+};
+
+struct rpmi_tee_signal_bus {
+ struct list_head node; /* Link in the notification signal-bus list. */
+ struct mutex lock; /* Serializes reservation state and lifetime. */
+ struct xarray reservations;
+ u32 endpoint_id;
+ u32 width;
+ u32 tee_to_ree_count;
+};
+
struct rpmi_tee_child {
struct list_head node;
struct rpmi_tee_device *rdev;
@@ -795,6 +891,280 @@ static int rpmi_tee_op_memory_share(struct rpmi_tee_device *rdev,
return rpmi_tee_op_parcel_create(rdev, args, false);
}
+static struct rpmi_tee_signal_bus *
+__rpmi_tee_find_signal_bus(struct rpmi_tee_transport *priv, u32 endpoint_id)
+{
+ struct rpmi_tee_signal_bus *bus;
+
+ list_for_each_entry(bus, &priv->notif.buses, node) {
+ if (bus->endpoint_id == endpoint_id)
+ return bus;
+ }
+
+ return NULL;
+}
+
+static struct rpmi_tee_signal_bus *
+rpmi_tee_find_signal_bus(struct rpmi_tee_transport *priv, u32 endpoint_id)
+{
+ lockdep_assert_held(&priv->notif.ops_lock);
+ /* Do not access signal buses after notification shutdown starts. */
+ if (priv->notif.shutting_down)
+ return NULL;
+
+ return __rpmi_tee_find_signal_bus(priv, endpoint_id);
+}
+
+/* Invoke an active signal callback without holding the bus lock. */
+static int rpmi_tee_dispatch_signal(struct rpmi_tee_signal_bus *bus,
+ u32 signal)
+{
+ struct rpmi_tee_signal_reservation *resv;
+ rpmi_tee_notifier_cb cb = NULL;
+ struct rpmi_tee_device *rdev = NULL;
+ void *cb_data = NULL;
+
+ if (signal >= bus->tee_to_ree_count)
+ return -EPROTO;
+
+ scoped_guard(mutex, &bus->lock) {
+ resv = xa_load(&bus->reservations, signal);
+ if (resv && resv->state == RPMI_TEE_SIGNAL_ACTIVE) {
+ cb = resv->cb;
+ cb_data = resv->cb_data;
+ rdev = resv->rdev;
+ }
+ }
+
+ if (cb)
+ cb(rdev, signal, cb_data);
+
+ return 0;
+}
+
+/* Release signal IDs that have passed the empty-retrieval barrier. */
+static void rpmi_tee_signal_bus_drop_releasing(struct rpmi_tee_signal_bus *bus)
+{
+ struct rpmi_tee_signal_reservation *resv;
+ unsigned long index;
+
+ guard(mutex)(&bus->lock);
+ xa_for_each(&bus->reservations, index, resv) {
+ if (resv->state != RPMI_TEE_SIGNAL_RELEASING)
+ continue;
+
+ xa_erase(&bus->reservations, index);
+ kfree(resv);
+ }
+}
+
+/**
+ * rpmi_tee_retrieve_signals() - Drain pending TEE-to-REE signals
+ * @priv: RPMI TEE transport
+ *
+ * Retrieve and dispatch signals until the firmware reports no pending data.
+ * Return relinquished signal IDs to their buses only after that empty
+ * retrieval.
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+static int rpmi_tee_retrieve_signals(struct rpmi_tee_transport *priv)
+{
+ size_t resp_len = priv->mbox.max_msg_data_size;
+ u32 flags, endpoint_id, signal_count, i;
+ struct rpmi_tee_signal_bus *bus;
+ s32 status;
+ int ret;
+
+ struct rpmi_tee_signal_retrieve_resp *resp __free(kfree) =
+ kzalloc(resp_len, GFP_KERNEL);
+ if (!resp)
+ return -ENOMEM;
+
+ for (;;) {
+ scoped_guard(mutex, &priv->notif.ops_lock) {
+ /* Stop retrieving so shutdown can drain the worker. */
+ if (priv->notif.shutting_down)
+ return 0;
+
+ resp_len = priv->mbox.max_msg_data_size;
+ ret = rpmi_tee_send_with_status(priv,
+ RPMI_TEE_SRV_SIGNAL_RETRIEVE,
+ NULL, 0, resp, &resp_len, &status);
+ /*
+ * A signal may be raised after a clear MORE_AVAILABLE
+ * response and before relinquish. Reusing the ID could
+ * deliver it to the wrong client.
+ * Reuse relinquished IDs only after an empty retrieve.
+ */
+ if (!ret && status == RPMI_ERR_NO_DATA) {
+ struct rpmi_tee_signal_bus *bus;
+
+ list_for_each_entry(bus, &priv->notif.buses, node)
+ rpmi_tee_signal_bus_drop_releasing(bus);
+ }
+ }
+
+ if (ret)
+ return ret;
+ /* The firmware has no more pending signals. */
+ if (status == RPMI_ERR_NO_DATA)
+ return 0;
+ if (status)
+ return rpmi_to_linux_error(status);
+ if (resp_len < sizeof(*resp))
+ return -EPROTO;
+
+ flags = get_unaligned_le32(&resp->flags);
+ endpoint_id = get_unaligned_le32(&resp->target_id);
+ signal_count = get_unaligned_le32(&resp->signal_count);
+
+ /* Validate the response flags and its variable-length signal array. */
+ if ((flags & ~RPMI_TEE_SIGNAL_MORE_AVAILABLE) || !signal_count ||
+ signal_count != (resp_len - sizeof(*resp)) / sizeof(__le32))
+ return -EPROTO;
+
+ bus = __rpmi_tee_find_signal_bus(priv, endpoint_id);
+ if (!bus || signal_count > bus->tee_to_ree_count)
+ return -EPROTO;
+
+ for (i = 0; i < signal_count; i++) {
+ u32 signal = get_unaligned_le32(&resp->signals[i]);
+
+ ret = rpmi_tee_dispatch_signal(bus, signal);
+ if (ret)
+ return ret;
+ }
+ }
+}
+
+static void rpmi_tee_notif_work(struct work_struct *work)
+{
+ struct rpmi_tee_notif_state *notif =
+ container_of(work, struct rpmi_tee_notif_state, work);
+ struct rpmi_tee_transport *priv =
+ container_of(notif, struct rpmi_tee_transport, notif);
+ int ret;
+
+ ret = rpmi_tee_retrieve_signals(priv);
+ if (ret)
+ dev_warn(priv->dev, "failed to retrieve signals: %d\n", ret);
+}
+
+static irqreturn_t rpmi_tee_notif_irq_handler(int irq, void *data)
+{
+ struct rpmi_tee_transport *priv = data;
+
+ queue_work(priv->notif.wq, &priv->notif.work);
+ return IRQ_HANDLED;
+}
+
+/* Reserve a TEE-to-REE signal for a notification consumer. */
+static int rpmi_tee_op_notify_request(struct rpmi_tee_device *rdev,
+ rpmi_tee_notifier_cb cb, void *cb_data,
+ u32 *signal)
+{
+ struct rpmi_tee_transport *priv = rpmi_tee_device_to_transport(rdev);
+ struct rpmi_tee_signal_bus *bus;
+ u32 id;
+
+ if (!cb || !signal)
+ return -EINVAL;
+
+ guard(mutex)(&priv->notif.ops_lock);
+ bus = rpmi_tee_find_signal_bus(priv, rdev->endpoint_id);
+ if (!bus)
+ return -EOPNOTSUPP;
+
+ struct rpmi_tee_signal_reservation *resv __free(kfree) =
+ kzalloc_obj(*resv, GFP_KERNEL);
+ if (!resv)
+ return -ENOMEM;
+
+ resv->rdev = rdev;
+ resv->cb = cb;
+ resv->cb_data = cb_data;
+ scoped_guard(mutex, &bus->lock) {
+ int ret;
+
+ ret = xa_alloc(&bus->reservations, &id, resv,
+ XA_LIMIT(0, bus->tee_to_ree_count - 1),
+ GFP_KERNEL);
+ if (ret)
+ return ret == -EBUSY ? -ENOSPC : ret;
+ }
+
+ *signal = id;
+ /* xa_alloc owns resv. */
+ retain_and_null_ptr(resv);
+
+ return 0;
+}
+
+/* Relinquish a previously reserved TEE-to-REE signal. */
+static int rpmi_tee_op_notify_relinquish(struct rpmi_tee_device *rdev,
+ u32 signal)
+{
+ struct rpmi_tee_transport *priv = rpmi_tee_device_to_transport(rdev);
+ struct rpmi_tee_signal_bus *bus;
+
+ guard(mutex)(&priv->notif.ops_lock);
+ bus = rpmi_tee_find_signal_bus(priv, rdev->endpoint_id);
+ if (!bus)
+ return -EOPNOTSUPP;
+
+ if (signal >= bus->tee_to_ree_count)
+ return -EINVAL;
+
+ scoped_guard(mutex, &bus->lock) {
+ struct rpmi_tee_signal_reservation *resv;
+
+ resv = xa_load(&bus->reservations, signal);
+ if (!resv)
+ return -ENOENT;
+ /* Release only if @signal belongs to @rdev. */
+ if (resv->rdev != rdev)
+ return -EPERM;
+ if (resv->state == RPMI_TEE_SIGNAL_RELEASING)
+ return -EALREADY;
+
+ resv->state = RPMI_TEE_SIGNAL_RELEASING;
+ }
+
+ queue_work(priv->notif.wq, &priv->notif.work);
+
+ return 0;
+}
+
+/* Raise an REE-to-TEE signal. */
+static int rpmi_tee_op_signal_raise(struct rpmi_tee_device *rdev, u32 signal)
+{
+ struct rpmi_tee_transport *priv = rpmi_tee_device_to_transport(rdev);
+ struct rpmi_tee_signal_raise_one_req req = {
+ .target_id = cpu_to_le32(rdev->endpoint_id),
+ .signal_count = cpu_to_le32(1),
+ .signal = cpu_to_le32(signal),
+ };
+ struct rpmi_tee_signal_bus *bus;
+
+ guard(mutex)(&priv->notif.ops_lock);
+ bus = rpmi_tee_find_signal_bus(priv, rdev->endpoint_id);
+ if (!bus)
+ return -EOPNOTSUPP;
+
+ if (signal < bus->tee_to_ree_count || signal >= bus->width)
+ return -EINVAL;
+
+ return rpmi_tee_send(priv, RPMI_TEE_SRV_SIGNAL_RAISE, &req,
+ sizeof(req), NULL, NULL);
+}
+
+static const struct rpmi_tee_notifier_ops rpmi_tee_notifier_ops = {
+ .notify_request = rpmi_tee_op_notify_request,
+ .notify_relinquish = rpmi_tee_op_notify_relinquish,
+ .signal_raise = rpmi_tee_op_signal_raise,
+};
+
static const struct rpmi_tee_info_ops rpmi_tee_info_ops = {
.msg_limits_get = rpmi_tee_op_msg_limits_get,
};
@@ -813,6 +1183,7 @@ static const struct rpmi_tee_ops rpmi_tee_ops = {
.info_ops = &rpmi_tee_info_ops,
.msg_ops = &rpmi_tee_msg_ops,
.mem_ops = &rpmi_tee_mem_ops,
+ .notifier_ops = &rpmi_tee_notifier_ops,
};
static void rpmi_tee_unregister_devices(struct rpmi_tee_transport *priv)
@@ -826,6 +1197,212 @@ static void rpmi_tee_unregister_devices(struct rpmi_tee_transport *priv)
}
}
+static void rpmi_tee_signal_bus_destroy_resvs(struct rpmi_tee_signal_bus *bus)
+{
+ struct rpmi_tee_signal_reservation *resv;
+ unsigned long index;
+
+ xa_for_each(&bus->reservations, index, resv) {
+ xa_erase(&bus->reservations, index);
+ kfree(resv);
+ }
+
+ xa_destroy(&bus->reservations);
+}
+
+static void rpmi_tee_teardown_signal_buses(struct rpmi_tee_transport *priv)
+{
+ struct rpmi_tee_signal_bus *bus, *tmp;
+
+ list_for_each_entry_safe(bus, tmp, &priv->notif.buses, node) {
+ struct rpmi_tee_signal_bus_teardown_req req = {
+ .target_id = cpu_to_le32(bus->endpoint_id),
+ };
+ int ret;
+
+ /* Completion includes the target TEE's teardown acknowledgment. */
+ ret = rpmi_tee_send(priv, RPMI_TEE_SRV_SIGNAL_BUS_TEARDOWN, &req,
+ sizeof(req), NULL, NULL);
+ if (ret)
+ dev_warn(priv->dev, "failed to tear down signal bus for %#x: %d\n",
+ bus->endpoint_id, ret);
+
+ rpmi_tee_signal_bus_destroy_resvs(bus);
+ list_del(&bus->node);
+ kfree(bus);
+ }
+}
+
+/* Drain callbacks, remove clients, then tear down notification resources. */
+static void rpmi_tee_teardown_endpoints(struct rpmi_tee_transport *priv)
+{
+ /* Initiate a notification shutdown. */
+ scoped_guard(mutex, &priv->notif.ops_lock)
+ priv->notif.shutting_down = true;
+
+ if (priv->notif.irq_requested) {
+ free_irq(priv->notif.irq, priv);
+ priv->notif.irq_requested = false;
+ }
+
+ if (priv->notif.wq) {
+ destroy_workqueue(priv->notif.wq);
+ priv->notif.wq = NULL;
+ }
+
+ /* Clients can still send final TEE_CALLs while signal buses exist. */
+ rpmi_tee_unregister_devices(priv);
+
+ /* Public notification operations and callbacks have stopped. */
+ rpmi_tee_teardown_signal_buses(priv);
+
+ if (priv->notif.irq) {
+ irq_dispose_mapping(priv->notif.irq);
+ priv->notif.irq = 0;
+ }
+}
+
+/* Set up an RPMI signal bus for one @endpoint_id TEE endpoint. */
+static int rpmi_tee_setup_ep_signal_bus(struct rpmi_tee_transport *priv,
+ u32 endpoint_id)
+{
+ struct rpmi_tee_signal_bus_setup_req req;
+ u32 max_width, width, tee_to_ree_count;
+
+ /* Avoid duplicate signal bus for endpoint. */
+ if (__rpmi_tee_find_signal_bus(priv, endpoint_id))
+ return 0;
+
+ if (priv->mbox.max_msg_data_size <
+ sizeof(struct rpmi_tee_signal_retrieve_resp))
+ return -EMSGSIZE;
+
+ max_width = FIELD_GET(RPMI_TEE_SIGNAL_WIDTH_MASK,
+ priv->notif.feature);
+ width = min(max_width,
+ 2 * ((priv->mbox.max_msg_data_size -
+ sizeof(struct rpmi_tee_signal_retrieve_resp)) /
+ sizeof(__le32)) + 1);
+ if (width < 2)
+ return -EMSGSIZE;
+
+ /* Use half available signals for TEE-to-REE range. */
+ tee_to_ree_count = width / 2;
+
+ struct rpmi_tee_signal_bus *bus __free(kfree) =
+ kzalloc_obj(*bus, GFP_KERNEL);
+ if (!bus)
+ return -ENOMEM;
+
+ mutex_init(&bus->lock);
+ xa_init_flags(&bus->reservations, XA_FLAGS_ALLOC);
+ bus->endpoint_id = endpoint_id;
+ bus->width = width;
+ bus->tee_to_ree_count = tee_to_ree_count;
+
+ req.target_id = cpu_to_le32(endpoint_id);
+ req.bus_width = cpu_to_le32(width);
+ req.sender_signals = cpu_to_le32(tee_to_ree_count);
+ /* Publish the local bus only after the target TEE accepts setup. */
+ if (rpmi_tee_send(priv, RPMI_TEE_SRV_SIGNAL_BUS_SETUP, &req,
+ sizeof(req), NULL, NULL))
+ return -EOPNOTSUPP;
+
+ list_add_tail(&no_free_ptr(bus)->node, &priv->notif.buses);
+
+ return 0;
+}
+
+static int rpmi_tee_map_signal_irq(struct rpmi_tee_transport *priv)
+{
+ struct device_node *np;
+ struct of_phandle_args oirq = {};
+ u32 mode, index;
+ int irq;
+
+ mode = FIELD_GET(RPMI_TEE_SIGNAL_MODE_MASK, priv->notif.feature);
+ if (mode != RPMI_TEE_SIGNAL_MODE_SYSTEM_MSI)
+ return 0;
+
+ for_each_compatible_node(np, NULL, "riscv,rpmi-system-msi") {
+ if (of_device_is_available(np))
+ break;
+ }
+
+ if (!np)
+ return 0;
+ if (!irq_find_host(np)) {
+ of_node_put(np);
+ return -EPROBE_DEFER;
+ }
+
+ index = FIELD_GET(RPMI_TEE_SIGNAL_INDEX_MASK, priv->notif.feature);
+ oirq.np = np;
+ oirq.args_count = 1;
+ oirq.args[0] = index;
+ irq = irq_create_of_mapping(&oirq);
+
+ of_node_put(np);
+
+ return irq;
+}
+
+/**
+ * rpmi_tee_setup_signal_bus() - Set up signal notification delivery
+ * @priv: RPMI TEE transport
+ * @system: Discovered TEE endpoints that may own signal buses
+ *
+ * Map the signal interrupt, then set up a bus for each endpoint. Endpoint
+ * setup failures are nonfatal, allowing notifications for the remaining
+ * endpoints. Register the interrupt handler only when at least one bus is
+ * available.
+ *
+ * Return: 0 on completion, or -EPROBE_DEFER when the System MSI IRQ domain
+ * is not ready.
+ */
+static int
+rpmi_tee_setup_signal_bus(struct rpmi_tee_transport *priv,
+ const struct rpmi_tee_discovery *system)
+{
+ struct rpmi_tee_discovered_endpoint *ep;
+ int ret;
+
+ priv->notif.irq = rpmi_tee_map_signal_irq(priv);
+ if (priv->notif.irq < 0)
+ return priv->notif.irq;
+ if (!priv->notif.irq)
+ return 0;
+
+ list_for_each_entry(ep, &system->eps, node) {
+ ret = rpmi_tee_setup_ep_signal_bus(priv, ep->ep_id);
+ if (ret)
+ dev_warn(priv->dev, "failed to set up signal bus for %#x: %d\n",
+ ep->ep_id, ret);
+ }
+
+ if (list_empty(&priv->notif.buses))
+ goto out_failed;
+
+ ret = request_irq(priv->notif.irq, rpmi_tee_notif_irq_handler, 0,
+ dev_name(priv->dev), priv);
+ if (ret) {
+ dev_warn(priv->dev, "failed to request signal IRQ: %d\n", ret);
+ rpmi_tee_teardown_signal_buses(priv);
+ goto out_failed;
+ }
+
+ priv->notif.irq_requested = true;
+
+ return 0;
+
+out_failed:
+ /* Failures are nonfatal; only disable notification. */
+ irq_dispose_mapping(priv->notif.irq);
+ priv->notif.irq = 0;
+
+ return 0;
+}
+
static struct rpmi_tee_device *
rpmi_tee_find_device(struct rpmi_tee_transport *priv, const uuid_t *uuid,
u32 endpoint_id)
@@ -881,14 +1458,19 @@ static int rpmi_tee_setup_endpoints(struct rpmi_tee_transport *priv)
if (ret)
return ret;
+ ret = rpmi_tee_setup_signal_bus(priv, &system);
+ if (ret)
+ goto out_failed;
+
list_for_each_entry(ep, &system.eps, node) {
ret = rpmi_tee_register_devices(priv, ep);
if (ret) {
- rpmi_tee_unregister_devices(priv);
+ rpmi_tee_teardown_endpoints(priv);
break;
}
}
+out_failed:
rpmi_tee_free_discovery(&system);
return ret;
@@ -907,7 +1489,11 @@ static int rpmi_tee_transport_probe(struct platform_device *pdev)
priv->dev = &pdev->dev;
platform_set_drvdata(pdev, priv);
INIT_LIST_HEAD(&priv->devices);
+ INIT_LIST_HEAD(&priv->notif.buses);
mutex_init(&priv->mem.lock);
+ mutex_init(&priv->notif.ops_lock);
+ INIT_WORK(&priv->notif.work, rpmi_tee_notif_work);
+
priv->mbox.client.dev = &pdev->dev;
priv->mbox.client.tx_sync = true;
priv->mbox.chan = mbox_request_channel(&priv->mbox.client, 0);
@@ -915,6 +1501,7 @@ static int rpmi_tee_transport_probe(struct platform_device *pdev)
return dev_err_probe(&pdev->dev, PTR_ERR(priv->mbox.chan),
"failed to request mailbox channel\n");
+ /* Validate the RPMI mailbox transport. */
ret = rpmi_tee_check_transport(priv);
if (ret) {
dev_err_probe(&pdev->dev, ret,
@@ -944,6 +1531,12 @@ static int rpmi_tee_transport_probe(struct platform_device *pdev)
if (ret)
goto out_failed;
+ priv->notif.wq = alloc_workqueue("rpmi_tee_notif", WQ_UNBOUND, 0);
+ if (!priv->notif.wq) {
+ ret = -ENOMEM;
+ goto out_failed;
+ }
+
ret = rpmi_tee_setup_endpoints(priv);
if (ret) {
dev_err_probe(&pdev->dev, ret,
@@ -954,6 +1547,8 @@ static int rpmi_tee_transport_probe(struct platform_device *pdev)
return 0;
out_failed:
+ if (priv->notif.wq)
+ destroy_workqueue(priv->notif.wq);
mbox_free_channel(priv->mbox.chan);
return ret;
@@ -963,7 +1558,7 @@ static void rpmi_tee_transport_remove(struct platform_device *pdev)
{
struct rpmi_tee_transport *priv = platform_get_drvdata(pdev);
- rpmi_tee_unregister_devices(priv);
+ rpmi_tee_teardown_endpoints(priv);
mbox_free_channel(priv->mbox.chan);
}
diff --git a/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
index 4df47d52724a..4d302f8ca16a 100644
--- a/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
+++ b/drivers/firmware/riscv_rpmi_tee/rpmi_tee_private.h
@@ -11,6 +11,7 @@
#include <linux/mutex.h>
#include <linux/types.h>
#include <linux/uuid.h>
+#include <linux/workqueue.h>
/* TEE service group and the services used by this module. */
#define RPMI_SRVGRP_TEE 0x10
@@ -20,12 +21,37 @@
#define RPMI_TEE_SRV_PROBE_DOMAIN 0x04
#define RPMI_TEE_SRV_PROBE_ENDPOINT 0x05
#define RPMI_TEE_SRV_CALL 0x18
+#define RPMI_TEE_SRV_SIGNAL_BUS_SETUP 0x0a
+#define RPMI_TEE_SRV_SIGNAL_BUS_TEARDOWN 0x0b
+#define RPMI_TEE_SRV_SIGNAL_RAISE 0x0c
+#define RPMI_TEE_SRV_SIGNAL_RETRIEVE 0x0d
#define RPMI_TEE_SRV_MEMORY_PARCEL_CREATE 0x0e
#define RPMI_TEE_SRV_MEMORY_PARCEL_RECLAIM 0x11
#define RPMI_TEE_SRV_MEMORY_SEGMENT_SEND 0x12
+/**
+ * struct rpmi_tee_notif_state - Signal notification state
+ * @buses: List of signal buses established for TEE endpoints.
+ * @work: Retrieves and dispatches pending TEE-to-REE signals.
+ * @wq: Workqueue used for @work.
+ * @ops_lock: Serializes public notification operations with signal-bus
+ * teardown. It prevents new operations after @shutting_down is set and
+ * serializes rpmi_tee_op_notify_relinquish() with the empty
+ * SIGNAL_RETRIEVE release barrier.
+ * @feature: SIGNAL feature value reported by the transport.
+ * @irq: Linux IRQ assigned to the signal notification interrupt.
+ * @irq_requested: Whether @irq has been requested.
+ * @shutting_down: Prevents public notification operations during teardown.
+ */
struct rpmi_tee_notif_state {
+ struct list_head buses;
+ struct work_struct work;
+ struct workqueue_struct *wq;
+ struct mutex ops_lock;
u32 feature;
+ int irq;
+ bool irq_requested;
+ bool shutting_down;
};
struct rpmi_tee_mbox {
diff --git a/include/linux/rpmi_tee.h b/include/linux/rpmi_tee.h
index 10f6cae4a3a7..f392b75ecaef 100644
--- a/include/linux/rpmi_tee.h
+++ b/include/linux/rpmi_tee.h
@@ -127,11 +127,35 @@ struct rpmi_tee_mem_ops {
int (*memory_reclaim)(struct rpmi_tee_device *rdev, u32 parcel_id);
};
+typedef void (*rpmi_tee_notifier_cb)(struct rpmi_tee_device *rdev,
+ u32 signal, void *cb_data);
+
+/* RPMI TEE signal notification operations. */
+struct rpmi_tee_notifier_ops {
+ /**
+ * @notify_request: Allocate a TEE-to-REE signal and associate it with
+ * @cb and @cb_data. The allocated signal is returned through @signal.
+ */
+ int (*notify_request)(struct rpmi_tee_device *rdev,
+ rpmi_tee_notifier_cb cb, void *cb_data, u32 *signal);
+ /**
+ * @notify_relinquish: Stop dispatching a TEE-to-REE @signal previously
+ * allocated for @rdev. The release is asynchronous and a callback may
+ * relinquish its own signal.
+ */
+ int (*notify_relinquish)(struct rpmi_tee_device *rdev, u32 signal);
+ /**
+ * @signal_raise: Raise an REE-to-TEE @signal selected by the TEE service.
+ */
+ int (*signal_raise)(struct rpmi_tee_device *rdev, u32 signal);
+};
+
/* RPMI TEE transport operation groups. */
struct rpmi_tee_ops {
const struct rpmi_tee_info_ops *info_ops;
const struct rpmi_tee_msg_ops *msg_ops;
const struct rpmi_tee_mem_ops *mem_ops;
+ const struct rpmi_tee_notifier_ops *notifier_ops;
};
extern const struct bus_type rpmi_tee_bus_type;
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH RFC v2 10/10] firmware: riscv: rpmi-tee: add signal bus support
2026-10-08 3:27 ` [PATCH RFC v2 10/10] firmware: riscv: rpmi-tee: add signal bus support Amirreza Zarrabi
@ 2026-10-08 3:56 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-10-08 3:56 UTC (permalink / raw)
To: Amirreza Zarrabi; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rpmi_tee: use-after-free of client callback and data due to asynchronous notify_relinquish
--
commit dfaa2e6e7e28252af1a3fba98317a99f0ae912d0
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
firmware: riscv: rpmi-tee: add signal bus support
Add signal buses for asynchronous notifications between Linux and TEE
endpoints. Allow service drivers to reserve, relinquish, and raise signals,
dispatching incoming notifications via client callbacks.
diff --git a/drivers/firmware/riscv_rpmi_tee/driver.c b/drivers/firmware/riscv_rpmi_tee/driver.c
--- a/drivers/firmware/riscv_rpmi_tee/driver.c
+++ b/drivers/firmware/riscv_rpmi_tee/driver.c
[ ... ]
> +/* Invoke an active signal callback without holding the bus lock. */
> +static int rpmi_tee_dispatch_signal(struct rpmi_tee_signal_bus *bus,
> + u32 signal)
> +{
> + struct rpmi_tee_signal_reservation *resv;
> + rpmi_tee_notifier_cb cb = NULL;
> + struct rpmi_tee_device *rdev = NULL;
> + void *cb_data = NULL;
> +
> + if (signal >= bus->tee_to_ree_count)
> + return -EPROTO;
> +
> + scoped_guard(mutex, &bus->lock) {
> + resv = xa_load(&bus->reservations, signal);
> + if (resv && resv->state == RPMI_TEE_SIGNAL_ACTIVE) {
> + cb = resv->cb;
> + cb_data = resv->cb_data;
> + rdev = resv->rdev;
> + }
> + }
> +
> + if (cb)
> + cb(rdev, signal, cb_data);
[Severity: High]
Is there a risk of a use-after-free or an execution of unmapped module memory
here if a client module unbinds concurrently?
> +
> + return 0;
> +}
[ ... ]
> +/* Relinquish a previously reserved TEE-to-REE signal. */
> +static int rpmi_tee_op_notify_relinquish(struct rpmi_tee_device *rdev,
> + u32 signal)
> +{
> + struct rpmi_tee_transport *priv = rpmi_tee_device_to_transport(rdev);
> + struct rpmi_tee_signal_bus *bus;
> +
> + guard(mutex)(&priv->notif.ops_lock);
> + bus = rpmi_tee_find_signal_bus(priv, rdev->endpoint_id);
> + if (!bus)
> + return -EOPNOTSUPP;
> +
> + if (signal >= bus->tee_to_ree_count)
> + return -EINVAL;
> +
> + scoped_guard(mutex, &bus->lock) {
> + struct rpmi_tee_signal_reservation *resv;
> +
> + resv = xa_load(&bus->reservations, signal);
> + if (!resv)
> + return -ENOENT;
> + /* Release only if @signal belongs to @rdev. */
> + if (resv->rdev != rdev)
> + return -EPERM;
> + if (resv->state == RPMI_TEE_SIGNAL_RELEASING)
> + return -EALREADY;
> +
> + resv->state = RPMI_TEE_SIGNAL_RELEASING;
> + }
> +
> + queue_work(priv->notif.wq, &priv->notif.work);
> +
> + return 0;
> +}
[Severity: High]
Because this returns immediately after queuing the release work, does this
leave clients without a way to wait for in-flight callbacks?
If a client driver calls rdev->ops->notifier_ops->notify_relinquish() during
its remove routine, frees its context, and finishes unloading, the worker
thread running rpmi_tee_dispatch_signal() may have already read the cb and
cb_data pointers.
Since there is no synchronous release API (such as one using flush_work()
or a wait mechanism) for clients to guarantee that no callbacks are executing
before module teardown, could this lead to executing a callback after the
client module is unloaded?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-riscv-rpmi-tee-abi-v2-0-45939a13d223@oss.qualcomm.com?part=10
^ permalink raw reply [flat|nested] 16+ messages in thread