On Mon, Oct 26, 2026 at 12:51:37PM +0530, Subrahmanya Lingappa wrote: > Charles, > > On Wed, Oct 14, 2026 at 7:47 PM Charles Perry > <[email protected]> wrote: > > > > From: Rahul Pathak <[email protected]> > > > > Add the mpxy command which allows listing MPXY channels, reading/writing > > arbitrary channel attributes and sending raw messages. > > > > The mpxy command is useful for interacting with custom RPMI service groups > > that have no associated driver. It can also be useful for debugging and > > testing SBI proxy drivers as well as the PuC firmware. > > > > Signed-off-by: Rahul Pathak <[email protected]> > > Signed-off-by: Charles Perry <[email protected]> > > --- > > Changes in v4: > > - Force a device_probe() of all UCLASS_RPMI devices if g_mpxy is NULL > > - Remove the unnecessary edit to "cmd/riscv/Makefile" > > - Fix a printf placeholder for 32 bits %ld -> %zu > > > > Changes in v3: > > - Add this patch > > --- > > drivers/firmware/rpmi/Kconfig | 7 + > > drivers/firmware/rpmi/rpmi-sbi-mpxy.c | 404 > > +++++++++++++++++++++++++++++++++- > > 2 files changed, 407 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/firmware/rpmi/Kconfig b/drivers/firmware/rpmi/Kconfig > > index b462d3693987..c6a6b6fe61de 100644 > > --- a/drivers/firmware/rpmi/Kconfig > > +++ b/drivers/firmware/rpmi/Kconfig > > @@ -18,6 +18,13 @@ config RPMI_SBI_MPXY > > specification for the MPXY extension is available at: > > https://github.com/riscv-non-isa/riscv-sbi-doc (chapter 20) > > > > +config CMD_MPXY > > + bool "mpxy command" > > + depends on RPMI_SBI_MPXY > > + help > > + Enable the 'mpxy' U-Boot command for listing MPXY channels, > > + reading/writing arbitrary channel attributes and sending raw > > messages. > > + > > config RPMI_SHMEM > > bool "RPMI over shared memory transport" > > depends on RPMI_FIRMWARE > > diff --git a/drivers/firmware/rpmi/rpmi-sbi-mpxy.c > > b/drivers/firmware/rpmi/rpmi-sbi-mpxy.c > > index 9ccca0eb59b2..5f01a6303323 100644 > > --- a/drivers/firmware/rpmi/rpmi-sbi-mpxy.c > > +++ b/drivers/firmware/rpmi/rpmi-sbi-mpxy.c > > @@ -1,6 +1,7 @@ > > // SPDX-License-Identifier: GPL-2.0 > > /* > > * Copyright (C) 2026 Microchip Technology Inc. All rights reserved. > > + * Copyright (c) 2026, Rahul Pathak <[email protected]> > > */ > > > > #define LOG_CATEGORY UCLASS_RPMI > > @@ -16,6 +17,7 @@ > > #include <malloc.h> > > #include <rpmi-uclass.h> > > #include <string.h> > > +#include <dm/device-internal.h> > > > > /* SBI MPXY channel IDs data in shared memory */ > > struct sbi_mpxy_channel_ids_data { > > @@ -60,10 +62,10 @@ enum sbi_mpxy_attribute_id { > > > > /* RPMI message protocol specific MPXY attributes */ > > enum sbi_mpxy_rpmi_attribute_id { > > - SBI_MPXY_RPMI_ATTR_SERVICEGROUP_ID, > > - SBI_MPXY_RPMI_ATTR_SERVICEGROUP_VERSION, > > - SBI_MPXY_RPMI_ATTR_IMPL_ID, > > - SBI_MPXY_RPMI_ATTR_IMPL_VERSION, > > + SBI_MPXY_RPMI_ATTR_SERVICEGROUP_ID = 0x00, > > + SBI_MPXY_RPMI_ATTR_SERVICEGROUP_VERSION = 0x01, > > + SBI_MPXY_RPMI_ATTR_IMPL_ID = 0x02, > > + SBI_MPXY_RPMI_ATTR_IMPL_VERSION = 0x03, > > SBI_MPXY_RPMI_ATTR_MAX_ID > > }; > > > > @@ -87,6 +89,24 @@ struct rpmi_sbi_mpxy { > > struct sbi_mpxy_chan_desc *channel_descs; > > }; > > > > +static struct rpmi_sbi_mpxy *g_mpxy; > > + > > +static __maybe_unused struct rpmi_sbi_mpxy *mpxy_get_global(void) > > +{ > > + struct udevice *dev; > > + struct uclass *uc; > > + > > + if (!g_mpxy) { > > + uclass_get(UCLASS_RPMI, &uc); > > + uclass_foreach_dev(dev, uc) > > + { > > + device_probe(dev); > > + } > > + } > > + > > + return g_mpxy; > > +} > > + > > static int rpmi_sbi_mpxy_read_attrs(struct rpmi_sbi_mpxy *mpxy, u32 > > channel_id, > > u32 base_attrid, u32 attr_count, > > u32 *attrs_buf) > > @@ -105,6 +125,24 @@ static int rpmi_sbi_mpxy_read_attrs(struct > > rpmi_sbi_mpxy *mpxy, u32 channel_id, > > return 0; > > } > > > > +static __maybe_unused int > > +rpmi_sbi_mpxy_write_attrs(struct rpmi_sbi_mpxy *mpxy, u32 channel_id, > > + u32 base_attrid, u32 attr_count, u32 *attrs_buf) > > +{ > > + struct sbiret sret; > > + int i; > > + > > + for (i = 0; i < attr_count; i++) > > + ((__le32 *)mpxy->shmem)[i] = cpu_to_le32(attrs_buf[i]); > > + > > + sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_WRITE_ATTRS, channel_id, > > + base_attrid, attr_count, 0, 0, 0); > > + if (sret.error) > > + return sbi_to_linux_error(sret.error); > > + > > + return 0; > > +} > > + > > static int rpmi_sbi_mpxy_send_message(void *shmem, u32 mpxy_fid, u32 > > channel_id, > > u32 msg_id, const void *tx, > > unsigned long tx_len, void *rx, > > @@ -368,6 +406,8 @@ static int rpmi_sbi_mpxy_probe(struct udevice *dev) > > desc->attrs[SBI_MPXY_ATTR_MSG_MAX_LEN]); > > } > > > > + g_mpxy = mpxy; > > + > > return 0; > > > > err: > > @@ -408,3 +448,359 @@ U_BOOT_DRIVER(rpmi_sbi_mpxy) = { > > .ops = &rpmi_sbi_mpxy_ops, > > .flags = DM_FLAG_PRE_RELOC, > > }; > > + > > +#if CONFIG_IS_ENABLED(CMD_MPXY) && !CONFIG_IS_ENABLED(XPL_BUILD) > > + > > +#include <command.h> > > +#include <hexdump.h> > > + > > +static const char *mpxy_proto_to_str(u32 proto_id) > > +{ > > + switch (proto_id) { > > + case SBI_MPXY_MSGPROTO_RPMI_ID: > > + return "RPMI"; > > + default: > > + return "Unknown"; > > + } > > +} > > + > > +/** > > + * mpxy list > > + * > > + * List the mpxy channels > > + */ > > +static int do_mpxy_list(struct cmd_tbl *cmdtp, int flag, int argc, > > + char *const argv[]) > > +{ > > + struct rpmi_sbi_mpxy *mpxy = mpxy_get_global(); > > + struct sbi_mpxy_chan_desc *chan; > > + u32 proto; > > + int i; > > + > > + if (!mpxy) > > + return CMD_RET_FAILURE; > > + > > + for (i = 0; i < mpxy->channel_count; i++) { > > + chan = &mpxy->channel_descs[i]; > > + proto = chan->attrs[SBI_MPXY_ATTR_MSG_PROT_ID]; > > + printf("Channel 0x%x: proto_id=0x%x (%s), ver=0x%x\n", > > chan->id, > > + proto, mpxy_proto_to_str(proto), > > + chan->attrs[SBI_MPXY_ATTR_MSG_PROT_VER]); > > + } > > + > > + printf("sbi_mpxy: found %d channels\n", mpxy->channel_count); > > + return CMD_RET_SUCCESS; > > +} > > + > > +static void mpxy_print_capability(u32 val) > > +{ > > + static const struct { > > + u32 bit; > > + const char *name; > > + } caps[] = { > > + { SBI_MPXY_CHAN_CAP_MSI, "MSI" }, > > + { SBI_MPXY_CHAN_CAP_SSE, "SSE_EVENT" }, > > + { SBI_MPXY_CHAN_CAP_EVENTS_STATE, "EVENTS_STATE" }, > > + { SBI_MPXY_CHAN_CAP_SEND_WITH_RESP, "SEND_WITH_RESP" }, > > + { SBI_MPXY_CHAN_CAP_SEND_WITHOUT_RESP, "SEND_WITHOUT_RESP" > > }, > > + { SBI_MPXY_CHAN_CAP_GET_NOTIFICATIONS, "GET_NOTIFICATIONS" > > }, > > + }; > > + > > + bool any = false; > > + int i; > > + > > + printf(" ("); > > + > > + if (!val) { > > + printf("none)\n"); > > + return; > > + } > > + > > + for (i = 0; i < ARRAY_SIZE(caps); i++) { > > + if (val & caps[i].bit) { > > + if (any) > > + printf("|"); > > + printf("%s", caps[i].name); > > + any = true; > > + } > > + } > > + printf(")\n"); > > +} > > + > > +/* Decode well-known field values for selected standard attributes */ > > +static void mpxy_print_std_attr(u32 attr_id, u32 val) > > +{ > > + /* Standard MPXY attribute names, indexed by attr_id */ > > + static const char *const mpxy_std_attr_names[] = { > > + [SBI_MPXY_ATTR_MSG_PROT_ID] = "MSG_PROT_ID", > > + [SBI_MPXY_ATTR_MSG_PROT_VER] = "MSG_PROT_VER", > > + [SBI_MPXY_ATTR_MSG_MAX_LEN] = "MSG_MAX_LEN", > > + [SBI_MPXY_ATTR_MSG_SEND_TIMEOUT] = "MSG_SEND_TIMEOUT", > > + [SBI_MPXY_ATTR_MSG_COMPLETION_TIMEOUT] = > > + "MSG_COMPLETION_TIMEOUT", > > + [SBI_MPXY_ATTR_CHANNEL_CAPABILITY] = "CHANNEL_CAPABILITY", > > + [SBI_MPXY_ATTR_SSE_EVENT_ID] = "SSE_EVENT_ID", > > + [SBI_MPXY_ATTR_MSI_CONTROL] = "MSI_CONTROL", > > + [SBI_MPXY_ATTR_MSI_ADDR_LO] = "MSI_ADDR_LO", > > + [SBI_MPXY_ATTR_MSI_ADDR_HI] = "MSI_ADDR_HI", > > + [SBI_MPXY_ATTR_MSI_DATA] = "MSI_DATA", > > + [SBI_MPXY_ATTR_EVENTS_STATE_CONTROL] = > > "EVENTS_STATE_CONTROL", > > + }; > > + > > + printf(" [0x%02x] %-28s: 0x%08x", attr_id, > > + mpxy_std_attr_names[attr_id], val); > > + > > + switch (attr_id) { > > + case SBI_MPXY_ATTR_MSG_PROT_ID: > > + printf(" (%s)\n", mpxy_proto_to_str(val)); > > + break; > > + case SBI_MPXY_ATTR_MSG_PROT_VER: > > + printf(" (v%u.%u)\n", (val >> 16) & 0xffff, val & 0xffff); > > + break; > > + case SBI_MPXY_ATTR_MSG_MAX_LEN: > > + printf(" (%u bytes)\n", val); > > + break; > > + case SBI_MPXY_ATTR_MSG_SEND_TIMEOUT: > > + case SBI_MPXY_ATTR_MSG_COMPLETION_TIMEOUT: > > + printf(" (%u ms)\n", val); > > + break; > > + case SBI_MPXY_ATTR_CHANNEL_CAPABILITY: > > + mpxy_print_capability(val); > > + break; > > + default: > > + printf("\n"); > > + break; > > + } > > +} > > + > > +static const char *mpxy_rpmi_proto_attr_name(u32 attr_id) > > +{ > > + switch (attr_id) { > > + case SBI_MPXY_RPMI_ATTR_SERVICEGROUP_ID: > > + return "SERVICEGROUP_ID"; > > + case SBI_MPXY_RPMI_ATTR_SERVICEGROUP_VERSION: > > + return "SERVICEGROUP_VERSION"; > > + case SBI_MPXY_RPMI_ATTR_IMPL_ID: > > + return "IMPLEMENTATION_ID"; > > + case SBI_MPXY_RPMI_ATTR_IMPL_VERSION: > > + return "IMPLEMENTATION_VER"; > > + default: > > + return "UNKNOWN"; > > + }; > > +} > > + > > +static void mpxy_print_proto_attr(u32 attr_id, u32 val, bool is_rpmi) > > +{ > > + u32 rpmi_attr_id = attr_id - SBI_MPXY_ATTR_MSGPROTO_ATTR_START; > > + const char *name = is_rpmi ? > > mpxy_rpmi_proto_attr_name(rpmi_attr_id) : > > + "UNKNOWN"; > > + > > + printf(" [0x%08x] %-24s: 0x%08x", attr_id, name, val); > > + > > + if (is_rpmi && > > + (rpmi_attr_id == SBI_MPXY_RPMI_ATTR_SERVICEGROUP_VERSION || > > + rpmi_attr_id == SBI_MPXY_RPMI_ATTR_IMPL_VERSION)) { > > + printf(" (v%u.%u)\n", (val >> 16) & 0xffff, val & 0xffff); > > + } else { > > + printf("\n"); > > + } > > +} > > + > > +/** > > + * mpxy readattr <channel_id> <attr_id> > > + * > > + * Read the attribute on a channel > > + */ > > +static int do_mpxy_readattr(struct cmd_tbl *cmdtp, int flag, int argc, > > + char *const argv[]) > > +{ > > + struct rpmi_sbi_mpxy *mpxy = mpxy_get_global(); > > + u32 chan_id, attr_id, attr_val; > > + int ret; > > + > > + if (argc < 3) > > + return CMD_RET_USAGE; > > + > > + if (!mpxy) > > + return CMD_RET_FAILURE; > > + > > + chan_id = simple_strtoul(argv[1], NULL, 0); > > + attr_id = simple_strtoul(argv[2], NULL, 0); > > + > > + ret = rpmi_sbi_mpxy_read_attrs(mpxy, chan_id, attr_id, 1, > > &attr_val); > > + if (ret) { > > + printf("Read attr 0x%x on channel 0x%x failed (%d)\n", > > attr_id, > > + chan_id, ret); > > + return CMD_RET_FAILURE; > > + } > > + > > + printf("Channel 0x%x: attr[0x%x] = 0x%x\n", chan_id, attr_id, > > attr_val); > > + return CMD_RET_SUCCESS; > > +} > > + > > +/** > > + * mpxy readattrall <channel_id> > > + */ > > +static int do_mpxy_readattr_all(struct cmd_tbl *cmdtp, int flag, int argc, > > + char *const argv[]) > > +{ > > + struct rpmi_sbi_mpxy *mpxy = mpxy_get_global(); > > + u32 chan_id, attr_val[SBI_MPXY_ATTR_STD_ATTR_MAX_IDX]; > > + bool is_rpmi; > > + int i, ret; > > + > > + if (argc < 2) > > + return CMD_RET_USAGE; > > + > > + if (!mpxy) > > + return CMD_RET_FAILURE; > > + > > + chan_id = simple_strtoul(argv[1], NULL, 0); > > + > > + ret = rpmi_sbi_mpxy_read_attrs(mpxy, chan_id, > > SBI_MPXY_ATTR_MSG_PROT_ID, > > + SBI_MPXY_ATTR_STD_ATTR_MAX_IDX, > > + attr_val); > > + if (ret) { > > + printf("Read attr on channel 0x%x failed (%d)\n", chan_id, > > ret); > > + return CMD_RET_FAILURE; > > + } > > + > > + /* Standard attributes */ > > + printf("Channel 0x%x standard attributes:\n", chan_id); > > + for (i = 0; i < SBI_MPXY_ATTR_STD_ATTR_MAX_IDX; i++) > > + mpxy_print_std_attr(i, attr_val[i]); > > + > > + /* Protocol-specific attributes */ > > + printf("Channel 0x%x protocol attributes:\n", chan_id); > > + > > + for (i = SBI_MPXY_ATTR_MSGPROTO_ATTR_START; > > + i < SBI_MPXY_ATTR_MSGPROTO_ATTR_END; i++) { > > + ret = rpmi_sbi_mpxy_read_attrs(mpxy, chan_id, i, 1, > > + &attr_val[0]); > > + if (ret) > > + break; > > + > > + is_rpmi = mpxy->channel_descs[chan_id] > > + .attrs[SBI_MPXY_ATTR_MSG_PROT_ID] == > > + SBI_MPXY_MSGPROTO_RPMI_ID; > `chan_id` is an external MPXY channel ID, not necessarily the descriptor > array index. The descriptors are stored by discovery position, so a valid > non-contiguous channel ID can cause an out-of-bounds read here. Please > resolve > the channel ID to its descriptor index before accessing `channel_descs`.
Oops! I guess that one worked because MSGPROTO_RPMI_ID is zero and I happened to access zeroed memory here. I'll change this to use "attr_val" that was read in this function instead of using the cached channel attributes. Thanks, Charles > > Regards, > Subrahmanya
