On Mon, Oct 26, 2026 at 12:46:31PM +0530, Subrahmanya Lingappa wrote:
> Charles,
>
> On Wed, Oct 14, 2026 at 7:47 PM Charles Perry
> <[email protected]> wrote:
> >
> > Add support for transport of RPMI messages over shared memory as
> > described in the RPMI spec [1]. This uses two or four ring buffers
> > composed of a fixed number of "slots" to pass messages between the AP
> > and the PuC. U-Boot is the Application processor (AP) and sends messages
> > through the a2p-req queue and receives response though the p2a-ack
> > queue. There is no support for platform microcontroller (PuC) initiated
> > requests nor notifications.
> >
> > This transport can be used in M or S mode U-Boot, but in the S-mode
> > case, one must make sure that the supervisor software isn't already
> > using the AP side of the shared memory.
> >
> > [1]: https://github.com/riscv-non-isa/riscv-rpmi (chapter 2)
> >
> > Signed-off-by: Charles Perry <[email protected]>
> > ---
> > Changes in v4:
> > - Fix the default value of the doorbell from 0 to 1
> >
> > Changes in v3: None
> >
> > Changes in v2: (from Yao Zi review)
> > - Reorder headers
> > - Don't null check pointers before free()
> > ---
> > drivers/firmware/rpmi/Kconfig | 8 +
> > drivers/firmware/rpmi/Makefile | 1 +
> > drivers/firmware/rpmi/rpmi-shmem.c | 473
> > +++++++++++++++++++++++++++++++++++++
> > include/rpmi_proto.h | 23 ++
> > 4 files changed, 505 insertions(+)
> >
> > diff --git a/drivers/firmware/rpmi/Kconfig b/drivers/firmware/rpmi/Kconfig
> > index 708d931263ff..421a1fa9deba 100644
> > --- a/drivers/firmware/rpmi/Kconfig
> > +++ b/drivers/firmware/rpmi/Kconfig
> > @@ -16,3 +16,11 @@ config RPMI_SBI_MPXY
> > (MPXY) extension of the supervisor binary interface (SBI). The
> > specification for the MPXY extension is available at:
> > https://github.com/riscv-non-isa/riscv-sbi-doc (chapter 20)
> > +
> > +config RPMI_SHMEM
> > + bool "RPMI over shared memory transport"
> > + depends on RPMI_FIRMWARE
> > + help
> > + Enable support for transport of RPMI messages over shared memory
> > as
> > + described in chapter 2 of the RPMI specification, available at:
> > + https://github.com/riscv-non-isa/riscv-rpmi
> > diff --git a/drivers/firmware/rpmi/Makefile b/drivers/firmware/rpmi/Makefile
> > index 71c4aeaed910..4f4bcb929db4 100644
> > --- a/drivers/firmware/rpmi/Makefile
> > +++ b/drivers/firmware/rpmi/Makefile
> > @@ -1,2 +1,3 @@
> > obj-y += rpmi-uclass.o
> > obj-$(CONFIG_RPMI_SBI_MPXY) += rpmi-sbi-mpxy.o
> > +obj-$(CONFIG_RPMI_SHMEM) += rpmi-shmem.o
> > diff --git a/drivers/firmware/rpmi/rpmi-shmem.c
> > b/drivers/firmware/rpmi/rpmi-shmem.c
> > new file mode 100644
> > index 000000000000..db9949e39c7e
> > --- /dev/null
> > +++ b/drivers/firmware/rpmi/rpmi-shmem.c
> > @@ -0,0 +1,473 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2026 Microchip Technology Inc. All rights reserved.
> > + */
> > +
> > +#define LOG_CATEGORY UCLASS_RPMI
> > +
> > +#include <asm/barrier.h>
> > +#include <asm/io.h>
> > +#include <asm/types.h>
> > +#include <dm.h>
> > +#include <dm/device_compat.h>
> > +#include <linux/delay.h>
> > +#include <log.h>
> > +#include <malloc.h>
> > +#include <rpmi-uclass.h>
> > +
> > +#define RPMI_SLOT_SIZE_MIN 64
> > +
> > +enum rpmi_queue_idx {
> > + RPMI_QUEUE_IDX_A2P_REQ = 0,
> > + RPMI_QUEUE_IDX_P2A_ACK = 1,
> > + RPMI_QUEUE_IDX_P2A_REQ = 2,
> > + RPMI_QUEUE_IDX_A2P_ACK = 3,
> > + RPMI_QUEUE_IDX_MAX_COUNT = 4,
> > + RPMI_A2P_DOORBELL = 5,
> > +};
> > +
> > +struct shmem_queue {
> > + u32 num_slot;
> > + __le32 *headptr;
> > + __le32 *tailptr;
> > + void *buffer;
> > +};
> > +
> > +struct rpmi_shmem {
> > + struct udevice *dev;
> > + struct shmem_queue a2p_req;
> > + struct shmem_queue p2a_ack;
> > + int slot_size;
> > + int rx_timeout_ms;
> > + u32 token;
> > + void *doorbell;
> > + u32 doorbell_value;
> > + u32 spec_version;
> > + u32 impl_version;
> > + u32 impl_id;
> > +};
> > +
> > +struct rpmi_shmem_chan_priv {
> > + struct rpmi_shmem *priv;
> > + u32 servicegroup_id;
> > + u32 servicegroup_version;
> > +};
> > +
> > +static int rpmi_shmem_queue_idx(const char *name)
> > +{
> > + static const struct {
> > + int id;
> > + const char *name;
> > + } id_names[] = {
> > + { RPMI_QUEUE_IDX_A2P_REQ, "a2p-req" },
> > + { RPMI_QUEUE_IDX_P2A_ACK, "p2a-ack" },
> > + { RPMI_QUEUE_IDX_P2A_REQ, "p2a-req" },
> > + { RPMI_QUEUE_IDX_A2P_ACK, "a2p-ack" },
> > + { RPMI_A2P_DOORBELL, "a2p-doorbell" },
> > + };
> > +
> > + for (int i = 0; i < ARRAY_SIZE(id_names); i++) {
> > + if (strcmp(name, id_names[i].name) == 0)
> > + return id_names[i].id;
> > + }
> > +
> > + return -ENODEV;
> > +}
> > +
> > +static bool rpmi_smq_queue_full(struct shmem_queue *qctx)
> > +{
> > + return ((le32_to_cpu(*qctx->tailptr) + 1) % qctx->num_slot ==
> > + le32_to_cpu(*qctx->headptr));
> > +}
> > +
> > +/* 31 0
> > + * +---------------------+-----------------------+
> > + * | FLAGS | SERVICE_ID | SERVICEGROUP_ID |
> > + * +---------------------+-----------------------+
> > + * | TOKEN | DATA LENGTH |
> > + * +---------------------+-----------------------+
> > + * | DATA/PAYLOAD |
> > + * +---------------------------------------------+
> > + */
> > +struct rpmi_message_header {
> > + __le16 servicegroup_id;
> > + u8 service_id;
> > + u8 flags;
> > + __le16 datalen;
> > + __le16 token;
> > +} __packed;
> > +
> > +static int rpmi_smq_rx(struct rpmi_shmem *priv, struct shmem_queue *qctx,
> > + u32 service_group_id, u32 service_id, u16 token,
> > + void *response, unsigned long max_response_len,
> > + unsigned long *out_response_len)
> > +{
> > + struct rpmi_message_header *msg;
> > + u32 dlen, headidx, tailidx;
> > + int rxretry = 0;
> > + void *src;
> > +
> > + headidx = le32_to_cpu(*qctx->headptr);
> > +
> > + /* Wait for the Rx message with matching token, discard any other
> > + * messages received in the mean time
> > + */
> > + while (true) {
> > + tailidx = le32_to_cpu(*qctx->tailptr);
> > +
> > + if (headidx != tailidx) {
> > + msg = (void *)qctx->buffer +
> > + (headidx * priv->slot_size);
>
> The transmit path uses `wmb()` before publishing the tail, but this receive
> path consumes the slot immediately after observing the tail. Please add the
> platform-appropriate acquire/read barrier before reading `msg` and document
> the ordering requirement for this shared-memory mapping.
I'll remove the wmb().
There's no point in adding memory barriers since U-Boot is always running
on a single processor. volatile* should be enough to ensure that
reads/writes happen in order. However, I am missing volatile* from headptr
and tailptr since I'm accessing those directly without going through
readl/writel. I'll fix this one by adding the volatile*.
>
> > + if (le16_to_cpu(msg->token) == token)
> > + break;
>
> The response is accepted using only the token. This function already receives
> `service_group_id` and `service_id`; please compare those fields with the
> response header as well. The token counter starts again for a new driver
> instance, while old replies can remain in the shared queue across a firmware
> transition. An old reply for a different service carrying the same token
> could
> otherwise satisfy this request.
Ok, good point.
>
> > +
> > + headidx = (headidx + 1) % qctx->num_slot;
> > + }
> > +
> > + mdelay(1);
> > + rxretry += 1;
> > + if (rxretry > priv->rx_timeout_ms)
> > + return -ETIMEDOUT;
> > + }
> > +
> > + /* Extract data from the message */
> > + if (response) {
> > + dlen = le16_to_cpu(msg->datalen);
> > + if (dlen > max_response_len)
> > + dlen = max_response_len;
> > + if (out_response_len)
> > + *out_response_len = dlen;
> > + src = (void *)msg + sizeof(struct rpmi_message_header);
> > + memcpy(response, src, dlen);
> `dlen` is bounded only by the caller's response buffer, not by the current
> shared-memory slot. The current clock callers use response buffers smaller
> than the slot payload, but this generic transport accepts arbitrary response
> sizes. Please reject a `dlen` greater than
> `slot_size - sizeof(struct rpmi_message_header)` before copying, so a
> malformed
> response or future caller cannot make the read cross the slot boundary.
Ok
>
> > + }
> > +
> > + /* Update the head index */
> > + headidx = (headidx + 1) % qctx->num_slot;
> > + *qctx->headptr = cpu_to_le32(headidx);
> > +
> > + return 0;
> > +}
> > +
> > +static int rpmi_smq_tx(struct rpmi_shmem *priv, struct shmem_queue *qctx,
> > + u32 service_group_id, u32 service_id,
> > + enum rpmi_message_type type, u16 token,
> > + const void *request, unsigned long request_len)
> > +{
> > + struct rpmi_message_header header = { 0 };
> > + u32 tailidx;
> > + void *dst;
> > +
> > + /* Tx sanity checks */
> > + if (request_len >
> > + (priv->slot_size - sizeof(struct rpmi_message_header)))
> > + return -EINVAL;
> > +
> > + if (rpmi_smq_queue_full(qctx))
> > + return -ENOMEM;
> > +
> > + tailidx = le32_to_cpu(*qctx->tailptr);
> > +
> > + /* Prepare the header to be written into the slot */
> > + header.servicegroup_id = cpu_to_le16(service_group_id);
> > + header.service_id = service_id;
> > + header.flags = type;
> > + header.datalen = cpu_to_le16(request_len);
> > + header.token = cpu_to_le16(token);
> > +
> > + /* Write header into the slot */
> > + dst = (char *)qctx->buffer + (tailidx * priv->slot_size);
> > + memcpy(dst, &header, sizeof(header));
> > + dst += sizeof(header);
> > +
> > + /* Write data into the slot */
> > + if (request)
> > + memcpy(dst, request, request_len);
> > +
> > + /* Make sure queue chanages are visible to PuC before updating tail
> > */
> > + wmb();
> > +
> > + /* Update the tail/write index */
> > + tailidx = (tailidx + 1) % qctx->num_slot;
> > + *qctx->tailptr = cpu_to_le32(tailidx);
> > +
> > + /* Ring the RPMI doorbell if present */
> > + if (priv->doorbell)
> > + writel(priv->doorbell_value, priv->doorbell);
> > +
> > + return 0;
> > +}
> > +
> > +static int rpmi_shmem_request_priv(struct rpmi_shmem *priv, u32
> > servicegroup_id,
> > + u32 service_id, const void *request,
> > + unsigned long request_len, void
> > *response,
> > + unsigned long max_response_len,
> > + unsigned long *out_response_len)
> > +{
> > + enum rpmi_message_type type = response ? RPMI_MSG_NORMAL_REQUEST :
> > + RPMI_MSG_POSTED_REQUEST;
> > + int err;
> > +
> > + if (!request && request_len)
> > + return -EINVAL;
> > +
> > + err = rpmi_smq_tx(priv, &priv->a2p_req, servicegroup_id, service_id,
> > + type, priv->token, request, request_len);
> > + if (err) {
> > + dev_err(priv->dev, "tx failed %i\n", err);
> > + goto out;
> > + }
> > +
> > + if (response) {
> > + err = rpmi_smq_rx(priv, &priv->p2a_ack, servicegroup_id,
> > + service_id, priv->token, response,
> > + max_response_len, out_response_len);
> > + if (err) {
> > + dev_err(priv->dev, "rx failed %i\n", err);
> > + goto out;
> > + }
> > + }
> > +
> > +out:
> > + priv->token++;
> > + return err;
> > +}
> > +
> > +static int rpmi_shmem_request(struct rpmi_chan *chan, u32 service_id,
> > + const void *request, unsigned long
> > request_len,
> > + void *response, unsigned long
> > max_response_len,
> > + unsigned long *out_response_len)
> > +{
> > + struct rpmi_shmem *priv = dev_get_priv(chan->dev);
> > + struct rpmi_shmem_chan_priv *chan_priv = chan->con_priv;
> > +
> > + return rpmi_shmem_request_priv(priv, chan_priv->servicegroup_id,
> > + service_id, request, request_len,
> > + response, max_response_len,
> > + out_response_len);
> > +}
> > +
> > +static int rpmi_shmem_get_attr(struct rpmi_chan *chan,
> > + enum rpmi_attribute_id id, u32 *data)
> > +{
> > + struct rpmi_shmem *priv = dev_get_priv(chan->dev);
> > + struct rpmi_shmem_chan_priv *chan_priv = chan->con_priv;
> > +
> > + switch (id) {
> > + case RPMI_ATTR_SPEC_VERSION:
> > + *data = priv->spec_version;
> > + break;
> > + case RPMI_ATTR_MAX_MSG_DATA_SIZE:
> > + *data = priv->slot_size - sizeof(struct
> > rpmi_message_header);
> > + break;
> > + case RPMI_ATTR_SERVICEGROUP_ID:
> > + *data = chan_priv->servicegroup_id;
> > + break;
> > + case RPMI_ATTR_SERVICEGROUP_VERSION:
> > + *data = chan_priv->servicegroup_version;
> > + break;
> > + case RPMI_ATTR_IMPL_ID:
> > + *data = priv->impl_id;
> > + break;
> > + case RPMI_ATTR_IMPL_VERSION:
> > + *data = priv->impl_version;
> > + break;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int rpmi_shmem_base_service_req(struct rpmi_shmem *priv, u32
> > service_id,
> > + const u32 *arg_in, u32 *arg_out)
> > +{
> > + int len = 0;
> > + u32 rx[2];
> > + u32 tx[1];
> > + int ret;
> > +
> > + if (arg_in) {
> > + tx[0] = cpu_to_le32(*arg_in);
> > + len = sizeof(*arg_in);
> > + }
> > +
> > + ret = rpmi_shmem_request_priv(priv, RPMI_SRVGRP_BASE, service_id,
> > tx,
> > + len, rx, sizeof(rx), NULL);
> > + if (ret) {
> > + dev_err(priv->dev, "BASE service %i failed (%i)\n",
> > service_id,
> > + ret);
> > + return ret;
> > + }
> > +
> > + if (rx[0]) {
> > + ret = rpmi_to_linux_error(le32_to_cpu(rx[0]));
> > + dev_err(priv->dev, "BASE service %i returned code %i\n",
> > + service_id, ret);
> > + return ret;
> > + }
> > +
> > + if (arg_out)
> > + *arg_out = le32_to_cpu(rx[1]);
> > +
> > + return 0;
> > +}
> > +
> > +static int rpmi_shmem_of_xlate(struct rpmi_chan *chan,
> > + struct ofnode_phandle_args *args)
> > +{
> > + if (args->args_count != 1)
> > + return -EINVAL;
> > +
> > + // args[0] is the servicegroup id.
> > + chan->id = args->args[0];
> > +
> > + return 0;
> > +}
> > +
> > +static int rpmi_shmem_open(struct rpmi_chan *chan)
> > +{
> > + struct rpmi_shmem *priv = dev_get_priv(chan->dev);
> > + u32 servicegroup_id, servicegroup_version;
> > + struct rpmi_shmem_chan_priv *chan_priv;
> > + int ret;
> > +
> > + servicegroup_id = chan->id;
> > +
> > + ret = rpmi_shmem_base_service_req(priv,
> > + RPMI_BASE_SRV_PROBE_SERVICE_GROUP,
> > + &servicegroup_id,
> > + &servicegroup_version);
> > + if (ret)
> > + return ret;
> > +
> > + chan_priv = malloc(sizeof(*chan_priv));
> > + if (!chan_priv)
> > + return -ENOMEM;
> > +
> > + chan->con_priv = chan_priv;
> > +
> > + chan_priv->priv = priv;
> > + chan_priv->servicegroup_id = servicegroup_id;
> > + chan_priv->servicegroup_version = servicegroup_version;
> > +
> > + return 0;
> > +}
> > +
> > +static void rpmi_shmem_close(struct rpmi_chan *chan)
> > +{
> > + free(chan->con_priv);
> > +}
> > +
> > +static int rpmi_shmem_probe(struct udevice *dev)
> > +{
> > + struct rpmi_shmem *priv = dev_get_priv(dev);
> > + int i, ret, qidx, qcount, qslotcnt;
> > + struct shmem_queue *queue;
> > + u32 arg_in, arg_out;
> > + const char *qname;
> > + fdt_size_t qsize;
> > + void *start;
> > +
> > + priv->dev = dev;
> > + priv->rx_timeout_ms = 20;
> > +
> > + priv->slot_size = dev_read_u32_default(dev, "riscv,slot-size", 0);
> > + if (priv->slot_size < RPMI_SLOT_SIZE_MIN)
> > + return -EINVAL;
> The probe enforces only the minimum slot size. RPMI also requires a power-of-
> two slot size; validate that property before deriving queue pointers and slot
> counts.
Ok
Thanks,
Charles
>
> regards,
> Subrahmanya
