[PATCH 2/7] firmware: rpmi: add support for the SBI MPXY transport

[Charles Perry]

On Sat, Jun 27, 2026 at 01:30:24AM +0000, Yao Zi wrote:
> On Fri, Jun 26, 2026 at 01:15:43PM -0700, Charles Perry wrote:
> > This adds an RPMI transport driver for the Supervisor Binary Interface
> > (SBI) message proxy (MPXY) extension [1].
> > 
> > This is for S-Mode U-Boot where some supervisor software (such as
> > OpenSBI) runs in M-Mode and handles the low level communication with the
> > PuC.
> > 
> > [1]: https://github.com/riscv-non-isa/riscv-sbi-doc (chapter 20)
> > 
> > Signed-off-by: Charles Perry <charles.perry at microchip.com>
> > ---
> >  arch/riscv/include/asm/sbi.h          |  57 ++++
> >  drivers/firmware/rpmi/Kconfig         |   9 +
> >  drivers/firmware/rpmi/Makefile        |   1 +
> >  drivers/firmware/rpmi/rpmi-sbi-mpxy.c | 449 ++++++++++++++++++++++++++
> >  4 files changed, 516 insertions(+)
> >  create mode 100644 drivers/firmware/rpmi/rpmi-sbi-mpxy.c
> 
> ...
> 
> > diff --git a/drivers/firmware/rpmi/rpmi-sbi-mpxy.c b/drivers/firmware/rpmi/rpmi-sbi-mpxy.c
> > new file mode 100644
> > index 000000000000..84af4787187a
> > --- /dev/null
> > +++ b/drivers/firmware/rpmi/rpmi-sbi-mpxy.c
> 
> ...
> 
> > +/* RPMI message protocol specific MPXY attributes */
> > +enum sbi_mpxy_rpmi_attribute_id {
> > +	SBI_MPXY_RPMI_ATTR_SERVICEGROUP_ID = SBI_MPXY_ATTR_MSGPROTO_ATTR_START,
> > +	SBI_MPXY_RPMI_ATTR_SERVICEGROUP_VERSION,
> > +	SBI_MPXY_RPMI_ATTR_IMPL_ID,
> > +	SBI_MPXY_RPMI_ATTR_IMPL_VERSION,
> > +	SBI_MPXY_RPMI_ATTR_MAX_ID
> > +};
> > +
> > +/*
> > + * RPMI specific SBI MPXY channel attributes.
> > + */
> > +struct sbi_mpxy_rpmi_channel_attrs {
> 
> Would it make sense to organize sbi_mpxy_rpmi_channel_attrs as an
> array indexed by sbi_mpxy_rpmi_attributed_id subtracted by
> SBI_MPXY_RPMI_ATTR_MSGPROTO_ATTR_START? With which we could simplify
> rpmi_sbi_mpxy_get_attr().

Yes I'll give this a try. I also find this a bit repetitive that the
attributes have an id and a struct where each members are sorted by
attribute id. Thanks!

> 
> > +	/* RPMI service group ID */
> > +	u32 servicegroup_id;
> > +	/* RPMI service group version */
> > +	u32 servicegroup_version;
> > +	/* RPMI implementation ID */
> > +	u32 impl_id;
> > +	/* RPMI implementation version */
> > +	u32 impl_version;
> > +};
> 
> ...
> 
> > +static int rpmi_sbi_mpxy_get_attr(struct rpmi_chan *chan,
> > +				  enum rpmi_attribute_id id, u32 *data)
> > +{
> > +	struct rpmi_sbi_mpxy *mpxy = dev_get_priv(chan->dev);
> > +	struct sbi_mpxy_chan_desc *desc = &mpxy->channel_descs[chan->id];
> > +
> > +	switch (id) {
> > +	case RPMI_ATTR_SPEC_VERSION:
> > +		*data = desc->attrs.msg_proto_version;
> > +		break;
> > +	case RPMI_ATTR_MAX_MSG_DATA_SIZE:
> > +		*data = desc->max_xfer_len;
> > +		break;
> > +	case RPMI_ATTR_SERVICEGROUP_ID:
> > +		*data = desc->rpmi_attrs.servicegroup_id;
> > +		break;
> > +	case RPMI_ATTR_SERVICEGROUP_VERSION:
> > +		*data = desc->rpmi_attrs.servicegroup_version;
> > +		break;
> > +	case RPMI_ATTR_IMPL_ID:
> > +		*data = desc->rpmi_attrs.impl_id;
> > +		break;
> > +	case RPMI_ATTR_IMPL_VERSION:
> > +		*data = desc->rpmi_attrs.impl_version;
> > +		break;
> > +	default:
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	return 0;
> > +}
> 
> ...
> 
> > +static int rpmi_sbi_mpxy_probe(struct udevice *dev)
> > +{
> > +	struct rpmi_sbi_mpxy *mpxy = dev_get_priv(dev);
> > +	struct sbi_mpxy_chan_desc *desc;
> > +	unsigned long flags;
> > +	struct sbiret sret;
> > +	int i, ret;
> > +
> > +	mpxy->dev = dev;
> > +
> > +	/* Probe for SBI MPXY extension */
> > +	if (sbi_get_spec_version() < sbi_mk_version(1, 0) ||
> > +	    sbi_probe_extension(SBI_EXT_MPXY) <= 0) {
> > +		dev_info(dev, "SBI MPXY extension not available\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	/* Find-out shared memory size */
> > +	ret = rpmi_sbi_mpxy_get_shmem_size(&mpxy->shmem_size);
> > +	if (ret) {
> > +		dev_err(dev, "failed to get MPXY shared memory size\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	mpxy->shmem = memalign(PAGE_SIZE, mpxy->shmem_size);
> > +	if (!mpxy->shmem)
> > +		return -ENOMEM;
> > +
> > +	/* Setup shmem in OVERWRITE mode. (flags[1:0] = 00b) */
> > +	flags = 0;
> > +	sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_SET_SHMEM,
> > +			 virt_to_phys(mpxy->shmem), 0, flags, 0, 0, 0);
> > +	if (sret.error) {
> > +		ret = sbi_to_linux_error(sret.error);
> > +		goto err;
> > +	}
> > +
> > +	ret = rpmi_sbi_mpxy_get_channel_ids(mpxy);
> > +	if (ret)
> > +		goto err;
> > +
> > +	for (i = 0; i < mpxy->channel_count; i++) {
> > +		desc = &mpxy->channel_descs[i];
> > +		desc->max_xfer_len =
> > +			min((u32)mpxy->shmem_size, desc->attrs.msg_max_len);
> > +	}
> > +
> > +	return 0;
> > +
> > +err:
> > +	if (mpxy->channel_descs)
> > +		free(mpxy->channel_descs);
> > +
> > +	if (mpxy->shmem)
> > +		free(mpxy->shmem);
> 
> free() is no-op when the argument is NULL, so the check is redundant.
> This applies to rpmi_sbi_mpxy_remove() and other similar cases in the
> series, too.

Ack.

Thanks,
Charles