More RPMI and MPXY updates

Instead of having one common FDT MPXY RPMI mailbox client drivers for various RPMI service groups, split this driver into two parts:
1) Common MPXY RPMI mailbox client library
2) MPXY driver for RPMI clock service group

The above split enables having a separate MPXY driver for each RPMI clock service group and #1 (above) will allow code sharing between various MPXY RPMI drivers.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h |  70 ++++++++
 lib/utils/mpxy/Kconfig                      |  12 +-
 lib/utils/mpxy/fdt_mpxy_rpmi_clock.c        |  88 ++++++++++
 lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c         | 169 +++-----------------
 lib/utils/mpxy/objects.mk                   |   4 +-
 platform/generic/configs/defconfig          |   1 +
 6 files changed, 190 insertions(+), 154 deletions(-)
 create mode 100644 include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h
 create mode 100644 lib/utils/mpxy/fdt_mpxy_rpmi_clock.c

include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h new file +38 -0

new file mode 100644
index 00000000..e4ca0151
@@ -0,0 +1,70 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2024 Ventana Micro Systems Inc.
+ *
+ * Authors:
+ *   Anup Patel <[email protected]>
+ */
+
+#ifndef __FDT_MPXY_RPMI_MBOX_H__
+#define __FDT_MPXY_RPMI_MBOX_H__
+
+#include <sbi/sbi_types.h>
+#include <sbi/sbi_mpxy.h>
+#include <sbi_utils/mailbox/rpmi_msgprot.h>
+#include <sbi_utils/mpxy/fdt_mpxy.h>
+
+#define MPXY_RPMI_MAJOR_VER		(1)
+#define MPXY_RPMI_MINOR_VER		(0)
+
+/** Convert the mpxy attribute ID to attribute array index */
+#define attr_id2index(attr_id)	(attr_id - SBI_MPXY_ATTR_MSGPROTO_ATTR_START)
+
+enum mpxy_msgprot_rpmi_attr_id {
+	MPXY_MSGPROT_RPMI_ATTR_SERVICEGROUP_ID = SBI_MPXY_ATTR_MSGPROTO_ATTR_START,
+	MPXY_MSGPROT_RPMI_ATTR_SERVICEGROUP_VERSION,
+	MPXY_MSGPROT_RPMI_ATTR_MAX_ID,
+};
+
+/**
+ * MPXY message protocol attributes for RPMI
+ * Order of attribute fields must follow the
+ * attribute IDs in `enum mpxy_msgprot_rpmi_attr_id`
+ */
+struct mpxy_rpmi_channel_attrs {
+	u32 servicegrp_id;
+	u32 servicegrp_ver;
+};

include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h new file +22 -0

+
+/** MPXY RPMI service data for each service group */
+struct mpxy_rpmi_service_data {
+	u8 id;
+	u32 min_tx_len;
+	u32 max_tx_len;
+	u32 min_rx_len;
+	u32 max_rx_len;
+};
+
+/** MPXY RPMI mbox data for each service group */
+struct mpxy_rpmi_mbox_data {
+	u32 servicegrp_id;
+	u32 num_services;
+	u32 notifications_support;
+	struct mpxy_rpmi_service_data *service_data;
+};
+
+/** Common probe function for MPXY RPMI drivers */
+int mpxy_rpmi_mbox_init(const void *fdt, int nodeoff, const struct fdt_match *match);
+
+#endif

lib/utils/mpxy/Kconfig +8 -4

index 131fb91a..ff88f24f 100644
@@ -7,11 +7,15 @@ config FDT_MPXY
 	depends on FDT
 	default n
 
-if FDT_MPXY
-
 config FDT_MPXY_RPMI_MBOX
-	bool "FDT MPXY mailbox client driver"
-	depends on FDT_MAILBOX
+	bool "MPXY drivers as RPMI mailbox client"
+	depends on FDT_MAILBOX && FDT_MPXY
+	default n
+
+if FDT_MPXY_RPMI_MBOX
+
+config FDT_MPXY_RPMI_CLOCK
+	bool "MPXY driver for RPMI clock service group"
 	default n
 
 endif

lib/utils/mpxy/fdt_mpxy_rpmi_clock.c new file +74 -0

new file mode 100644
index 00000000..1f5c6e75
@@ -0,0 +1,88 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2024 Ventana Micro Systems Inc.
+ *
+ * Authors:
+ *   Rahul Pathak <[email protected]>
+ *   Anup Patel <[email protected]>
+ */
+
+#include <sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h>
+
+static struct mpxy_rpmi_service_data clock_services[] = {
+{
+	.id = RPMI_CLOCK_SRV_ENABLE_NOTIFICATION,
+	.min_tx_len = sizeof(struct rpmi_enable_notification_req),
+	.max_tx_len = sizeof(struct rpmi_enable_notification_req),
+	.min_rx_len = sizeof(struct rpmi_enable_notification_resp),
+	.max_rx_len = sizeof(struct rpmi_enable_notification_resp),
+},
+{
+	.id = RPMI_CLOCK_SRV_GET_NUM_CLOCKS,
+	.min_tx_len = 0,
+	.max_tx_len = 0,
+	.min_rx_len = sizeof(struct rpmi_clock_get_num_clocks_resp),
+	.max_rx_len = sizeof(struct rpmi_clock_get_num_clocks_resp),
+},
+{
+	.id = RPMI_CLOCK_SRV_GET_ATTRIBUTES,
+	.min_tx_len = sizeof(struct rpmi_clock_get_attributes_req),
+	.max_tx_len = sizeof(struct rpmi_clock_get_attributes_req),
+	.min_rx_len = sizeof(struct rpmi_clock_get_attributes_resp),
+	.max_rx_len = sizeof(struct rpmi_clock_get_attributes_resp),
+},
+{
+	.id = RPMI_CLOCK_SRV_GET_SUPPORTED_RATES,
+	.min_tx_len = sizeof(struct rpmi_clock_get_supported_rates_req),
+	.max_tx_len = sizeof(struct rpmi_clock_get_supported_rates_req),
+	.min_rx_len = sizeof(struct rpmi_clock_get_supported_rates_resp),
+	.max_rx_len = -1U,
+},
+{
+	.id = RPMI_CLOCK_SRV_SET_CONFIG,
+	.min_tx_len = sizeof(struct rpmi_clock_set_config_req),
+	.max_tx_len = sizeof(struct rpmi_clock_set_config_req),
+	.min_rx_len = sizeof(struct rpmi_clock_set_config_resp),
+	.max_rx_len = sizeof(struct rpmi_clock_set_config_resp),
+},
+{
+	.id = RPMI_CLOCK_SRV_GET_CONFIG,
+	.min_tx_len = sizeof(struct rpmi_clock_get_config_req),
+	.max_tx_len = sizeof(struct rpmi_clock_get_config_req),
+	.min_rx_len = sizeof(struct rpmi_clock_get_config_resp),
+	.max_rx_len = sizeof(struct rpmi_clock_get_config_resp),
+},
+{
+	.id = RPMI_CLOCK_SRV_SET_RATE,
+	.min_tx_len = sizeof(struct rpmi_clock_set_rate_req),
+	.max_tx_len = sizeof(struct rpmi_clock_set_rate_req),
+	.min_rx_len = sizeof(struct rpmi_clock_set_rate_resp),
+	.max_rx_len = sizeof(struct rpmi_clock_set_rate_resp),
+},
+{
+	.id = RPMI_CLOCK_SRV_GET_RATE,
+	.min_tx_len = sizeof(struct rpmi_clock_get_rate_req),
+	.max_tx_len = sizeof(struct rpmi_clock_get_rate_req),
+	.min_rx_len = sizeof(struct rpmi_clock_get_rate_resp),
+	.max_rx_len = sizeof(struct rpmi_clock_get_rate_resp),
+},
+};
+
+static struct mpxy_rpmi_mbox_data clock_data = {
+	.servicegrp_id = RPMI_SRVGRP_CLOCK,
+	.num_services = RPMI_CLOCK_SRV_MAX_COUNT,

lib/utils/mpxy/fdt_mpxy_rpmi_clock.c new file +4 -0

+	.match_table = clock_match,
+	.init = mpxy_rpmi_mbox_init,
+	.experimental = true,
+};

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c +20 -149

index 78020eae..0d4fd1fe 100644
@@ -11,82 +11,32 @@
 #include <libfdt.h>
 #include <sbi/sbi_error.h>
 #include <sbi/sbi_heap.h>
-#include <sbi/sbi_mpxy.h>
 #include <sbi_utils/fdt/fdt_helper.h>
-#include <sbi_utils/mpxy/fdt_mpxy.h>
+#include <sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h>
 #include <sbi_utils/mailbox/fdt_mailbox.h>
-#include <sbi_utils/mailbox/rpmi_msgprot.h>
 #include <sbi/sbi_console.h>
 
-#define RPMI_MAJOR_VER		(1)
-#define RPMI_MINOR_VER		(0)
-
-/** Convert the mpxy attribute ID to attribute array index */
-#define attr_id2index(attr_id)	(attr_id - SBI_MPXY_ATTR_MSGPROTO_ATTR_START)
-
-enum mpxy_msgprot_rpmi_attr_id {
-	MPXY_MSGPROT_RPMI_ATTR_SERVICEGROUP_ID = SBI_MPXY_ATTR_MSGPROTO_ATTR_START,
-	MPXY_MSGPROT_RPMI_ATTR_SERVICEGROUP_VERSION,
-	MPXY_MSGPROT_RPMI_ATTR_MAX_ID,
-};
-
-/**
- * MPXY message protocol attributes for RPMI
- * Order of attribute fields must follow the
- * attribute IDs in `enum mpxy_msgprot_rpmi_attr_id`
- */
-struct mpxy_rpmi_channel_attrs {
-	u32 servicegrp_id;
-	u32 servicegrp_ver;
-};
-
-/* RPMI mbox data per service group */
-struct mpxy_mbox_data {
-	u32 servicegrp_id;
-	u32 num_services;
-	u32 notifications_support;
-	void *priv_data;
-};
-
-/* RPMI service data per service group */
-struct rpmi_service_data {
-	u8 id;
-	u32 min_tx_len;
-	u32 max_tx_len;
-	u32 min_rx_len;
-	u32 max_rx_len;
-};
-
 /**
  * MPXY mbox instance per MPXY channel. This ties
  * an MPXY channel with an RPMI Service group.
  */
-struct mpxy_mbox {
+struct mpxy_rpmi_mbox {
 	struct mbox_chan *chan;
-	struct mpxy_mbox_data *mbox_data;
+	struct mpxy_rpmi_mbox_data *mbox_data;
 	struct mpxy_rpmi_channel_attrs msgprot_attrs;
 	struct sbi_mpxy_channel channel;
 };
 
-/** Make sure all attributes are packed for direct memcpy */
-#define assert_field_offset(field, attr_offset)				\
-	_Static_assert(							\
-		((offsetof(struct mpxy_rpmi_channel_attrs, field)) /	\
-		 sizeof(u32)) == (attr_offset - SBI_MPXY_ATTR_MSGPROTO_ATTR_START),\
-		"field " #field						\
-		" from struct mpxy_rpmi_channel_attrs invalid offset, expected " #attr_offset)
-
-assert_field_offset(servicegrp_id, MPXY_MSGPROT_RPMI_ATTR_SERVICEGROUP_ID);
-
 /**
  * Discover the RPMI service data using message_id
  * MPXY message_id == RPMI service_id
  */
-static struct rpmi_service_data *mpxy_find_rpmi_srvid(u32 message_id,
-					struct mpxy_mbox_data *mbox_data)
+static struct mpxy_rpmi_service_data *mpxy_find_rpmi_srvid(u32 message_id,
+					struct mpxy_rpmi_mbox_data *mbox_data)
 {
 	int mid = 0;
-	struct rpmi_service_data *srv = mbox_data->priv_data;
+	struct mpxy_rpmi_service_data *srv = mbox_data->service_data;
+
 	for (mid = 0; srv[mid].id < mbox_data->num_services; mid++) {
 		if (srv[mid].id == (u8)message_id)
 			return &srv[mid];

@@ -108,9 +58,8 @@ static int mpxy_mbox_read_attributes(struct sbi_mpxy_channel *channel,
 				     u32 attr_count)
 {
 	u32 end_id;
-	struct mpxy_mbox *rmb =
-		container_of(channel, struct mpxy_mbox, channel);
-
+	struct mpxy_rpmi_mbox *rmb =
+		container_of(channel, struct mpxy_rpmi_mbox, channel);
 	u32 *attr_array = (u32 *)&rmb->msgprot_attrs;
 
 	end_id = base_attr_id + attr_count - 1;

@@ -161,8 +110,8 @@ static int mpxy_mbox_write_attributes(struct sbi_mpxy_channel *channel,
 {
 	int ret, mem_idx;
 	u32 end_id, attr_val, idx;
-	struct mpxy_mbox *rmb =
-		container_of(channel, struct mpxy_mbox, channel);
+	struct mpxy_rpmi_mbox *rmb =
+		container_of(channel, struct mpxy_rpmi_mbox, channel);
 
 	end_id = base_attr_id + attr_count - 1;
 

@@ -195,9 +144,9 @@ static int __mpxy_mbox_send_message(struct sbi_mpxy_channel *channel,
 	u32 rx_len = 0;
 	struct mbox_xfer xfer;
 	struct rpmi_message_args args = {0};
-	struct mpxy_mbox *rmb =
-		container_of(channel, struct mpxy_mbox, channel);
-	struct rpmi_service_data *srv =
+	struct mpxy_rpmi_mbox *rmb =
+		container_of(channel, struct mpxy_rpmi_mbox, channel);
+	struct mpxy_rpmi_service_data *srv =
 			mpxy_find_rpmi_srvid(message_id, rmb->mbox_data);
 	if (!srv)
 		return SBI_ENOTSUPP;

@@ -266,15 +215,14 @@ static int mpxy_mbox_get_notifications(struct sbi_mpxy_channel *channel,
 	return SBI_ENOTSUPP;
 }
 
-static int mpxy_mbox_init(const void *fdt, int nodeoff,
-			  const struct fdt_match *match)
+int mpxy_rpmi_mbox_init(const void *fdt, int nodeoff, const struct fdt_match *match)
 {
 	int rc, len;
 	const fdt32_t *val;
 	u32 channel_id;
-	struct mpxy_mbox *rmb;
 	struct mbox_chan *chan;
-	const struct mpxy_mbox_data *data = match->data;
+	struct mpxy_rpmi_mbox *rmb;
+	const struct mpxy_rpmi_mbox_data *data = match->data;
 
 	/* Allocate context for RPXY mbox client */
 	rmb = sbi_zalloc(sizeof(*rmb));

@@ -334,7 +282,7 @@ static int mpxy_mbox_init(const void *fdt, int nodeoff,
 	rmb->channel.attrs.msg_proto_id = SBI_MPXY_MSGPROTO_RPMI_ID;
 	/* RPMI Message Protocol Version */
 	rmb->channel.attrs.msg_proto_version =
-		SBI_MPXY_MSGPROTO_VERSION(RPMI_MAJOR_VER, RPMI_MINOR_VER);
+		SBI_MPXY_MSGPROTO_VERSION(MPXY_RPMI_MAJOR_VER, MPXY_RPMI_MINOR_VER);
 
 	/* RPMI supported max message data length(bytes), same for
 	 * all service groups */

@@ -349,9 +297,9 @@ static int mpxy_mbox_init(const void *fdt, int nodeoff,
 	/* RPMI message protocol attributes */
 	rmb->msgprot_attrs.servicegrp_id = data->servicegrp_id;
 	rmb->msgprot_attrs.servicegrp_ver =
-			SBI_MPXY_MSGPROTO_VERSION(RPMI_MAJOR_VER, RPMI_MINOR_VER);
+			SBI_MPXY_MSGPROTO_VERSION(MPXY_RPMI_MAJOR_VER, MPXY_RPMI_MINOR_VER);
 
-	rmb->mbox_data = (struct mpxy_mbox_data *)data;
+	rmb->mbox_data = (struct mpxy_rpmi_mbox_data *)data;
 	rmb->chan = chan;
 
 	/* Register RPXY service group */

@@ -364,80 +312,3 @@ static int mpxy_mbox_init(const void *fdt, int nodeoff,
 
 	return SBI_OK;
 }
-
-static struct rpmi_service_data clock_services[] = {
-{
-	.id = RPMI_CLOCK_SRV_ENABLE_NOTIFICATION,
-	.min_tx_len = sizeof(struct rpmi_enable_notification_req),
-	.max_tx_len = sizeof(struct rpmi_enable_notification_req),
-	.min_rx_len = sizeof(struct rpmi_enable_notification_resp),
-	.max_rx_len = sizeof(struct rpmi_enable_notification_resp),
-},
-{
-	.id = RPMI_CLOCK_SRV_GET_NUM_CLOCKS,
-	.min_tx_len = 0,
-	.max_tx_len = 0,
-	.min_rx_len = sizeof(struct rpmi_clock_get_num_clocks_resp),
-	.max_rx_len = sizeof(struct rpmi_clock_get_num_clocks_resp),
-},
-{
-	.id = RPMI_CLOCK_SRV_GET_ATTRIBUTES,
-	.min_tx_len = sizeof(struct rpmi_clock_get_attributes_req),
-	.max_tx_len = sizeof(struct rpmi_clock_get_attributes_req),
-	.min_rx_len = sizeof(struct rpmi_clock_get_attributes_resp),
-	.max_rx_len = sizeof(struct rpmi_clock_get_attributes_resp),
-},
-{
-	.id = RPMI_CLOCK_SRV_GET_SUPPORTED_RATES,
-	.min_tx_len = sizeof(struct rpmi_clock_get_supported_rates_req),
-	.max_tx_len = sizeof(struct rpmi_clock_get_supported_rates_req),
-	.min_rx_len = sizeof(struct rpmi_clock_get_supported_rates_resp),
-	.max_rx_len = -1U,
-},
-{
-	.id = RPMI_CLOCK_SRV_SET_CONFIG,
-	.min_tx_len = sizeof(struct rpmi_clock_set_config_req),
-	.max_tx_len = sizeof(struct rpmi_clock_set_config_req),
-	.min_rx_len = sizeof(struct rpmi_clock_set_config_resp),
-	.max_rx_len = sizeof(struct rpmi_clock_set_config_resp),
-},
-{
-	.id = RPMI_CLOCK_SRV_GET_CONFIG,
-	.min_tx_len = sizeof(struct rpmi_clock_get_config_req),
-	.max_tx_len = sizeof(struct rpmi_clock_get_config_req),
-	.min_rx_len = sizeof(struct rpmi_clock_get_config_resp),
-	.max_rx_len = sizeof(struct rpmi_clock_get_config_resp),
-},
-{
-	.id = RPMI_CLOCK_SRV_SET_RATE,
-	.min_tx_len = sizeof(struct rpmi_clock_set_rate_req),
-	.max_tx_len = sizeof(struct rpmi_clock_set_rate_req),
-	.min_rx_len = sizeof(struct rpmi_clock_set_rate_resp),
-	.max_rx_len = sizeof(struct rpmi_clock_set_rate_resp),
-},
-{
-	.id = RPMI_CLOCK_SRV_GET_RATE,
-	.min_tx_len = sizeof(struct rpmi_clock_get_rate_req),
-	.max_tx_len = sizeof(struct rpmi_clock_get_rate_req),
-	.min_rx_len = sizeof(struct rpmi_clock_get_rate_resp),
-	.max_rx_len = sizeof(struct rpmi_clock_get_rate_resp),
-},
-};
-
-static struct mpxy_mbox_data clock_data = {
-	.servicegrp_id = RPMI_SRVGRP_CLOCK,
-	.num_services = RPMI_CLOCK_SRV_MAX_COUNT,
-	.notifications_support = 1,
-	.priv_data = clock_services,
-};
-
-static const struct fdt_match mpxy_mbox_match[] = {
-	{ .compatible = "riscv,rpmi-mpxy-clock", .data = &clock_data },
-	{ },
-};
-
-struct fdt_driver fdt_mpxy_rpmi_mbox = {
-	.match_table = mpxy_mbox_match,
-	.init = mpxy_mbox_init,
-	.experimental = true,
-};

lib/utils/mpxy/objects.mk +3 -1

index ccb28b55..1c9afed9 100644
@@ -10,5 +10,7 @@
 libsbiutils-objs-$(CONFIG_FDT_MPXY) += mpxy/fdt_mpxy.o
 libsbiutils-objs-$(CONFIG_FDT_MPXY) += mpxy/fdt_mpxy_drivers.carray.o
 
-carray-fdt_mpxy_drivers-$(CONFIG_FDT_MPXY_RPMI_MBOX) += fdt_mpxy_rpmi_mbox
 libsbiutils-objs-$(CONFIG_FDT_MPXY_RPMI_MBOX) += mpxy/fdt_mpxy_rpmi_mbox.o
+
+carray-fdt_mpxy_drivers-$(CONFIG_FDT_MPXY_RPMI_CLOCK) += fdt_mpxy_rpmi_clock
+libsbiutils-objs-$(CONFIG_FDT_MPXY_RPMI_CLOCK) += mpxy/fdt_mpxy_rpmi_clock.o

platform/generic/configs/defconfig +1 -0

index e23b38b2..55607a28 100644
@@ -55,3 +55,4 @@ CONFIG_FDT_TIMER_MTIMER=y
 CONFIG_FDT_TIMER_PLMT=y
 CONFIG_FDT_MPXY=y
 CONFIG_FDT_MPXY_RPMI_MBOX=y
+CONFIG_FDT_MPXY_RPMI_CLOCK=y

Hi Anup,

A couple of comments on existing issues in the code. Maybe it makes sense to fix them in this series to avoid conflicts?

Quoted 69 lines · Anup Patel

Instead of having one common FDT MPXY RPMI mailbox client drivers for various RPMI service groups, split this driver into two parts:
1) Common MPXY RPMI mailbox client library
2) MPXY driver for RPMI clock service group

The above split enables having a separate MPXY driver for each RPMI clock service group and #1 (above) will allow code sharing between various MPXY RPMI drivers.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h |  70 ++++++++
 lib/utils/mpxy/Kconfig                      |  12 +-
 lib/utils/mpxy/fdt_mpxy_rpmi_clock.c        |  88 ++++++++++
 lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c         | 169 +++-----------------
 lib/utils/mpxy/objects.mk                   |   4 +-
 platform/generic/configs/defconfig          |   1 +
 6 files changed, 190 insertions(+), 154 deletions(-)
 create mode 100644 include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h
 create mode 100644 lib/utils/mpxy/fdt_mpxy_rpmi_clock.c

include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h new file +42 -0

new file mode 100644
index 00000000..e4ca0151
@@ -0,0 +1,70 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2024 Ventana Micro Systems Inc.
+ *
+ * Authors:
+ *   Anup Patel <[email protected]>
+ */
+
+#ifndef __FDT_MPXY_RPMI_MBOX_H__
+#define __FDT_MPXY_RPMI_MBOX_H__
+
+#include <sbi/sbi_types.h>
+#include <sbi/sbi_mpxy.h>
+#include <sbi_utils/mailbox/rpmi_msgprot.h>
+#include <sbi_utils/mpxy/fdt_mpxy.h>
+
+#define MPXY_RPMI_MAJOR_VER          (1)
+#define MPXY_RPMI_MINOR_VER          (0)
+
+/** Convert the mpxy attribute ID to attribute array index */
+#define attr_id2index(attr_id)       (attr_id - SBI_MPXY_ATTR_MSGPROTO_ATTR_START)
+
+enum mpxy_msgprot_rpmi_attr_id {
+     MPXY_MSGPROT_RPMI_ATTR_SERVICEGROUP_ID = SBI_MPXY_ATTR_MSGPROTO_ATTR_START,
+     MPXY_MSGPROT_RPMI_ATTR_SERVICEGROUP_VERSION,
+     MPXY_MSGPROT_RPMI_ATTR_MAX_ID,
+};
+
+/**
+ * MPXY message protocol attributes for RPMI
+ * Order of attribute fields must follow the
+ * attribute IDs in `enum mpxy_msgprot_rpmi_attr_id`
+ */
+struct mpxy_rpmi_channel_attrs {
+     u32 servicegrp_id;
+     u32 servicegrp_ver;
+};
+
+/** Make sure all attributes are packed for direct memcpy */
+#define assert_field_offset(field, attr_offset)                              \
+     _Static_assert(                                                 \

include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h line 48

Quoted 6 lines

include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h new file +6 -0

+             ((offsetof(struct mpxy_rpmi_channel_attrs, field)) /    \
+              sizeof(u32)) == (attr_offset - SBI_MPXY_ATTR_MSGPROTO_ATTR_START),\
+             "field " #field                                         \
+             " from struct mpxy_rpmi_channel_attrs invalid offset, expected " #attr_offset)
+
+assert_field_offset(servicegrp_id, MPXY_MSGPROT_RPMI_ATTR_SERVICEGROUP_ID);

Missing assertion for servicegrp_ver?

Quoted 132 lines

include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h new file +22 -0

+
+/** MPXY RPMI service data for each service group */
+struct mpxy_rpmi_service_data {
+     u8 id;
+     u32 min_tx_len;
+     u32 max_tx_len;
+     u32 min_rx_len;
+     u32 max_rx_len;
+};
+
+/** MPXY RPMI mbox data for each service group */
+struct mpxy_rpmi_mbox_data {
+     u32 servicegrp_id;
+     u32 num_services;
+     u32 notifications_support;
+     struct mpxy_rpmi_service_data *service_data;
+};
+
+/** Common probe function for MPXY RPMI drivers */
+int mpxy_rpmi_mbox_init(const void *fdt, int nodeoff, const struct fdt_match *match);
+
+#endif

lib/utils/mpxy/Kconfig +8 -4

index 131fb91a..ff88f24f 100644
@@ -7,11 +7,15 @@ config FDT_MPXY
      depends on FDT
      default n
 
-if FDT_MPXY
-
 config FDT_MPXY_RPMI_MBOX
-     bool "FDT MPXY mailbox client driver"
-     depends on FDT_MAILBOX
+     bool "MPXY drivers as RPMI mailbox client"
+     depends on FDT_MAILBOX && FDT_MPXY
+     default n
+
+if FDT_MPXY_RPMI_MBOX
+
+config FDT_MPXY_RPMI_CLOCK
+     bool "MPXY driver for RPMI clock service group"
      default n
 
 endif

lib/utils/mpxy/fdt_mpxy_rpmi_clock.c new file +79 -0

new file mode 100644
index 00000000..1f5c6e75
@@ -0,0 +1,88 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2024 Ventana Micro Systems Inc.
+ *
+ * Authors:
+ *   Rahul Pathak <[email protected]>
+ *   Anup Patel <[email protected]>
+ */
+
+#include <sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h>
+
+static struct mpxy_rpmi_service_data clock_services[] = {
+{
+     .id = RPMI_CLOCK_SRV_ENABLE_NOTIFICATION,
+     .min_tx_len = sizeof(struct rpmi_enable_notification_req),
+     .max_tx_len = sizeof(struct rpmi_enable_notification_req),
+     .min_rx_len = sizeof(struct rpmi_enable_notification_resp),
+     .max_rx_len = sizeof(struct rpmi_enable_notification_resp),
+},
+{
+     .id = RPMI_CLOCK_SRV_GET_NUM_CLOCKS,
+     .min_tx_len = 0,
+     .max_tx_len = 0,
+     .min_rx_len = sizeof(struct rpmi_clock_get_num_clocks_resp),
+     .max_rx_len = sizeof(struct rpmi_clock_get_num_clocks_resp),
+},
+{
+     .id = RPMI_CLOCK_SRV_GET_ATTRIBUTES,
+     .min_tx_len = sizeof(struct rpmi_clock_get_attributes_req),
+     .max_tx_len = sizeof(struct rpmi_clock_get_attributes_req),
+     .min_rx_len = sizeof(struct rpmi_clock_get_attributes_resp),
+     .max_rx_len = sizeof(struct rpmi_clock_get_attributes_resp),
+},
+{
+     .id = RPMI_CLOCK_SRV_GET_SUPPORTED_RATES,
+     .min_tx_len = sizeof(struct rpmi_clock_get_supported_rates_req),
+     .max_tx_len = sizeof(struct rpmi_clock_get_supported_rates_req),
+     .min_rx_len = sizeof(struct rpmi_clock_get_supported_rates_resp),
+     .max_rx_len = -1U,
+},
+{
+     .id = RPMI_CLOCK_SRV_SET_CONFIG,
+     .min_tx_len = sizeof(struct rpmi_clock_set_config_req),
+     .max_tx_len = sizeof(struct rpmi_clock_set_config_req),
+     .min_rx_len = sizeof(struct rpmi_clock_set_config_resp),
+     .max_rx_len = sizeof(struct rpmi_clock_set_config_resp),
+},
+{
+     .id = RPMI_CLOCK_SRV_GET_CONFIG,
+     .min_tx_len = sizeof(struct rpmi_clock_get_config_req),
+     .max_tx_len = sizeof(struct rpmi_clock_get_config_req),
+     .min_rx_len = sizeof(struct rpmi_clock_get_config_resp),
+     .max_rx_len = sizeof(struct rpmi_clock_get_config_resp),
+},
+{
+     .id = RPMI_CLOCK_SRV_SET_RATE,
+     .min_tx_len = sizeof(struct rpmi_clock_set_rate_req),
+     .max_tx_len = sizeof(struct rpmi_clock_set_rate_req),
+     .min_rx_len = sizeof(struct rpmi_clock_set_rate_resp),
+     .max_rx_len = sizeof(struct rpmi_clock_set_rate_resp),
+},
+{
+     .id = RPMI_CLOCK_SRV_GET_RATE,
+     .min_tx_len = sizeof(struct rpmi_clock_get_rate_req),
+     .max_tx_len = sizeof(struct rpmi_clock_get_rate_req),
+     .min_rx_len = sizeof(struct rpmi_clock_get_rate_resp),
+     .max_rx_len = sizeof(struct rpmi_clock_get_rate_resp),
+},
+};
+
+static struct mpxy_rpmi_mbox_data clock_data = {
+     .servicegrp_id = RPMI_SRVGRP_CLOCK,
+     .num_services = RPMI_CLOCK_SRV_MAX_COUNT,
+     .notifications_support = 1,
+     .service_data = clock_services,
+};
+
+static const struct fdt_match clock_match[] = {

lib/utils/mpxy/fdt_mpxy_rpmi_clock.c line 84

Quoted 5 lines

lib/utils/mpxy/fdt_mpxy_rpmi_clock.c new file +5 -0

+     { .compatible = "riscv,rpmi-mpxy-clock", .data = &clock_data },
+     { },
+};
+
+struct fdt_driver fdt_mpxy_rpmi_clock = {

This structure is required to be const both by the carray and the declaration of fdt_mpxy_drivers.

The mpxy_rpmi_mbox_data is provided by RPMI service group specific MPXY driver to the common MPXY RPMI mailbox client implementation so let's constantify mpxy_rpmi_mbox_data in mpxy_rpmi_mbox so that it is not accidently modified.

Signed-off-by Anup Patel <[email protected]>

---
 lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c +7 -6

index 0d4fd1fe..7034fb9e 100644
@@ -22,7 +22,7 @@
  */
 struct mpxy_rpmi_mbox {
 	struct mbox_chan *chan;
-	struct mpxy_rpmi_mbox_data *mbox_data;
+	const struct mpxy_rpmi_mbox_data *mbox_data;
 	struct mpxy_rpmi_channel_attrs msgprot_attrs;
 	struct sbi_mpxy_channel channel;
 };

@@ -31,11 +31,11 @@ struct mpxy_rpmi_mbox {
  * Discover the RPMI service data using message_id
  * MPXY message_id == RPMI service_id
  */
-static struct mpxy_rpmi_service_data *mpxy_find_rpmi_srvid(u32 message_id,
-					struct mpxy_rpmi_mbox_data *mbox_data)
+static const struct mpxy_rpmi_service_data *mpxy_find_rpmi_srvid(u32 message_id,
+					const struct mpxy_rpmi_mbox_data *mbox_data)
 {
 	int mid = 0;
-	struct mpxy_rpmi_service_data *srv = mbox_data->service_data;
+	const struct mpxy_rpmi_service_data *srv = mbox_data->service_data;
 
 	for (mid = 0; srv[mid].id < mbox_data->num_services; mid++) {
 		if (srv[mid].id == (u8)message_id)

@@ -146,8 +146,9 @@ static int __mpxy_mbox_send_message(struct sbi_mpxy_channel *channel,
 	struct rpmi_message_args args = {0};
 	struct mpxy_rpmi_mbox *rmb =
 		container_of(channel, struct mpxy_rpmi_mbox, channel);
-	struct mpxy_rpmi_service_data *srv =
-			mpxy_find_rpmi_srvid(message_id, rmb->mbox_data);
+	const struct mpxy_rpmi_service_data *srv =
+		mpxy_find_rpmi_srvid(message_id, rmb->mbox_data);
+
 	if (!srv)
 		return SBI_ENOTSUPP;
 

Quoted 54 lines · Anup Patel

The mpxy_rpmi_mbox_data is provided by RPMI service group specific MPXY driver to the common MPXY RPMI mailbox client implementation so let's constantify mpxy_rpmi_mbox_data in mpxy_rpmi_mbox so that it is not accidently modified.

Signed-off-by Anup Patel <[email protected]>

---
 lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c +7 -6

index 0d4fd1fe..7034fb9e 100644
@@ -22,7 +22,7 @@
  */
 struct mpxy_rpmi_mbox {
      struct mbox_chan *chan;
-     struct mpxy_rpmi_mbox_data *mbox_data;
+     const struct mpxy_rpmi_mbox_data *mbox_data;
      struct mpxy_rpmi_channel_attrs msgprot_attrs;
      struct sbi_mpxy_channel channel;
 };

@@ -31,11 +31,11 @@ struct mpxy_rpmi_mbox {
  * Discover the RPMI service data using message_id
  * MPXY message_id == RPMI service_id
  */
-static struct mpxy_rpmi_service_data *mpxy_find_rpmi_srvid(u32 message_id,
-                                     struct mpxy_rpmi_mbox_data *mbox_data)
+static const struct mpxy_rpmi_service_data *mpxy_find_rpmi_srvid(u32 message_id,
+                                     const struct mpxy_rpmi_mbox_data *mbox_data)
 {
      int mid = 0;
-     struct mpxy_rpmi_service_data *srv = mbox_data->service_data;
+     const struct mpxy_rpmi_service_data *srv = mbox_data->service_data;
 
      for (mid = 0; srv[mid].id < mbox_data->num_services; mid++) {
              if (srv[mid].id == (u8)message_id)

@@ -146,8 +146,9 @@ static int __mpxy_mbox_send_message(struct sbi_mpxy_channel *channel,
      struct rpmi_message_args args = {0};
      struct mpxy_rpmi_mbox *rmb =
              container_of(channel, struct mpxy_rpmi_mbox, channel);
-     struct mpxy_rpmi_service_data *srv =
-                     mpxy_find_rpmi_srvid(message_id, rmb->mbox_data);
+     const struct mpxy_rpmi_service_data *srv =
+             mpxy_find_rpmi_srvid(message_id, rmb->mbox_data);
+
      if (!srv)
              return SBI_ENOTSUPP;
 

@@ -299,7 +300,7 @@ int mpxy_rpmi_mbox_init(const void *fdt, int nodeoff, const struct fdt_match *ma
      rmb->msgprot_attrs.servicegrp_ver =
                      SBI_MPXY_MSGPROTO_VERSION(MPXY_RPMI_MAJOR_VER, MPXY_RPMI_MINOR_VER);
 

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c int mpxy_rpmi_mbox_init(const void *fdt, int nodeoff, const struct fdt_match *ma · line 306

Quoted 5 lines

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c +1 -1

-     rmb->mbox_data = (struct mpxy_rpmi_mbox_data *)data;
+     rmb->mbox_data = data;
      rmb->chan = chan;
 
      /* Register RPXY service group */

The underlying object (e.g. clock_data) can also be const now.

Reviewed-by Samuel Holland <[email protected]>

Some of the RPMI service groups may additional contex and special handling when transfering message to underlying mailbox channel

so introduce optional MPXY RPMI service group operations for this purpose.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h | 18 ++++++++++++++++
 lib/utils/mpxy/fdt_mpxy_rpmi_clock.c        |  4 ++++
 lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c         | 24 +++++++++++++++++----
 3 files changed, 42 insertions(+), 4 deletions(-)

include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h +14 -0

index e4ca0151..08b32a0a 100644
@@ -12,6 +12,7 @@
 
 #include <sbi/sbi_types.h>
 #include <sbi/sbi_mpxy.h>
+#include <sbi_utils/mailbox/fdt_mailbox.h>
 #include <sbi_utils/mailbox/rpmi_msgprot.h>
 #include <sbi_utils/mpxy/fdt_mpxy.h>
 

@@ -56,12 +57,29 @@ struct mpxy_rpmi_service_data {
 	u32 max_rx_len;
 };
 
+struct mpxy_rpmi_mbox_data;
+
+/** MPXY RPMI service group operations */
+struct mpxy_rpmi_service_group_ops {
+	/** Transfer RPMI service group message */
+	int (*xfer_group)(void *context, struct mbox_chan *chan,
+			  struct mbox_xfer *xfer);
+
+	/** Setup RPMI service group context for MPXY */
+	int (*setup_group)(void **context, struct mbox_chan *chan,
+			   const struct mpxy_rpmi_mbox_data *data);
+
+	/** Cleanup RPMI service group context for MPXY */

include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h +0 -0

 };
 
 /** Common probe function for MPXY RPMI drivers */

lib/utils/mpxy/fdt_mpxy_rpmi_clock.c +4 -0

index 1f5c6e75..3d275cd8 100644
@@ -10,6 +10,9 @@
 
 #include <sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h>
 
+static struct mpxy_rpmi_service_group_ops clock_ops = {
+};
+
 static struct mpxy_rpmi_service_data clock_services[] = {
 {
 	.id = RPMI_CLOCK_SRV_ENABLE_NOTIFICATION,

@@ -74,6 +77,7 @@ static struct mpxy_rpmi_mbox_data clock_data = {
 	.num_services = RPMI_CLOCK_SRV_MAX_COUNT,
 	.notifications_support = 1,
 	.service_data = clock_services,
+	.group_ops = &clock_ops,
 };
 
 static const struct fdt_match clock_match[] = {

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c +20 -4

index 7034fb9e..8d892cef 100644
@@ -13,7 +13,6 @@
 #include <sbi/sbi_heap.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h>
-#include <sbi_utils/mailbox/fdt_mailbox.h>
 #include <sbi/sbi_console.h>
 
 /**

@@ -25,6 +24,7 @@ struct mpxy_rpmi_mbox {
 	const struct mpxy_rpmi_mbox_data *mbox_data;
 	struct mpxy_rpmi_channel_attrs msgprot_attrs;
 	struct sbi_mpxy_channel channel;
+	void *group_context;
 };
 
 /**

@@ -146,8 +146,9 @@ static int __mpxy_mbox_send_message(struct sbi_mpxy_channel *channel,
 	struct rpmi_message_args args = {0};
 	struct mpxy_rpmi_mbox *rmb =
 		container_of(channel, struct mpxy_rpmi_mbox, channel);
+	const struct mpxy_rpmi_mbox_data *data = rmb->mbox_data;
 	const struct mpxy_rpmi_service_data *srv =
-		mpxy_find_rpmi_srvid(message_id, rmb->mbox_data);
+		mpxy_find_rpmi_srvid(message_id, data);
 
 	if (!srv)
 		return SBI_ENOTSUPP;

@@ -183,7 +184,10 @@ static int __mpxy_mbox_send_message(struct sbi_mpxy_channel *channel,
 				  tx, tx_len, RPMI_DEF_TX_TIMEOUT);
 	}
 
-	ret = mbox_chan_xfer(rmb->chan, &xfer);
+	if (data->group_ops->xfer_group)
+		ret = data->group_ops->xfer_group(rmb->group_context, rmb->chan, &xfer);
+	else
+		ret = mbox_chan_xfer(rmb->chan, &xfer);
 	if (ret)
 		return ret;
 

@@ -303,9 +307,21 @@ int mpxy_rpmi_mbox_init(const void *fdt, int nodeoff, const struct fdt_match *ma
 	rmb->mbox_data = data;
 	rmb->chan = chan;
 
-	/* Register RPXY service group */
+	/* Setup RPMI service group context */
+	if (data->group_ops->setup_group) {
+		rc = data->group_ops->setup_group(&rmb->group_context, chan, data);
+		if (rc) {
+			mbox_controller_free_chan(chan);
+			sbi_free(rmb);
+			return rc;
+		}
+	}
+
+	/* Register RPMI service group */
 	rc = sbi_mpxy_register_channel(&rmb->channel);
 	if (rc) {
+		if (data->group_ops->cleanup_group)
+			data->group_ops->cleanup_group(rmb->group_context);
 		mbox_controller_free_chan(chan);
 		sbi_free(rmb);
 		return rc;

Hi Anup,

Quoted 2 lines · Anup Patel

Some of the RPMI service groups may additional contex and special handling when transfering message to underlying mailbox channel

typos: context, transferring

Quoted 40 lines

so introduce optional MPXY RPMI service group operations for this purpose.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h | 18 ++++++++++++++++
 lib/utils/mpxy/fdt_mpxy_rpmi_clock.c        |  4 ++++
 lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c         | 24 +++++++++++++++++----
 3 files changed, 42 insertions(+), 4 deletions(-)

include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h +14 -0

index e4ca0151..08b32a0a 100644
@@ -12,6 +12,7 @@
 
 #include <sbi/sbi_types.h>
 #include <sbi/sbi_mpxy.h>
+#include <sbi_utils/mailbox/fdt_mailbox.h>
 #include <sbi_utils/mailbox/rpmi_msgprot.h>
 #include <sbi_utils/mpxy/fdt_mpxy.h>
 

@@ -56,12 +57,29 @@ struct mpxy_rpmi_service_data {
      u32 max_rx_len;
 };
 
+struct mpxy_rpmi_mbox_data;
+
+/** MPXY RPMI service group operations */
+struct mpxy_rpmi_service_group_ops {
+     /** Transfer RPMI service group message */
+     int (*xfer_group)(void *context, struct mbox_chan *chan,
+                       struct mbox_xfer *xfer);
+
+     /** Setup RPMI service group context for MPXY */
+     int (*setup_group)(void **context, struct mbox_chan *chan,
+                        const struct mpxy_rpmi_mbox_data *data);
+
+     /** Cleanup RPMI service group context for MPXY */

include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h struct mpxy_rpmi_service_data { · line 82

Quoted 10 lines

include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h +4 -0

+     void (*cleanup_group)(void *context);
+};
+
 /** MPXY RPMI mbox data for each service group */
 struct mpxy_rpmi_mbox_data {
      u32 servicegrp_id;
      u32 num_services;
      u32 notifications_support;
      struct mpxy_rpmi_service_data *service_data;
+     struct mpxy_rpmi_service_group_ops *group_ops;

Do you expect several service groups to share a mpxy_rpmi_service_group_ops? If not, I would suggest to reduce pointer chasing by putting the function pointers directly in mpxy_rpmi_mbox_data.

If you do keep it as a pointer, it should be a pointer to const.

The capability bit assignment in MPXY framework does not match the SBI MPXY extension in latest SBI specification so update it.

Fixes 7939bf1329eb ("lib: sbi: Add SBI Message Proxy (MPXY) framework")

Signed-off-by Anup patel <[email protected]>

---
 lib/sbi/sbi_mpxy.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

The local variable declarations should be at the start of function and preferrably organized like a inverted pyramid.

Signed-off-by Anup patel <[email protected]>

---
 lib/sbi/sbi_mpxy.c | 65 ++++++++++++++++++----------------------------
 1 file changed, 25 insertions(+), 40 deletions(-)

Currently, the common MPXY RPMI mailbox client does not support notifications so no need for dummy notifications support.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h |  1 -
 lib/utils/mpxy/fdt_mpxy_rpmi_clock.c        |  1 -
 lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c         | 10 ----------
 3 files changed, 12 deletions(-)

The local variable declarations should be at the start of function and preferrably organized like a inverted pyramid.

Signed-off-by Anup Patel <[email protected]>

---
 lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c | 31 ++++++++++++-----------------
 1 file changed, 13 insertions(+), 18 deletions(-)

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c +13 -16

index 26b9e6a6..27f4c571 100644
@@ -34,8 +34,8 @@ struct mpxy_rpmi_mbox {
 static const struct mpxy_rpmi_service_data *mpxy_find_rpmi_srvid(u32 message_id,
 					const struct mpxy_rpmi_mbox_data *mbox_data)
 {
-	int mid = 0;
 	const struct mpxy_rpmi_service_data *srv = mbox_data->service_data;
+	int mid = 0;
 
 	for (mid = 0; srv[mid].id < mbox_data->num_services; mid++) {
 		if (srv[mid].id == (u8)message_id)

@@ -57,12 +57,10 @@ static int mpxy_mbox_read_attributes(struct sbi_mpxy_channel *channel,
 				     u32 *outmem, u32 base_attr_id,
 				     u32 attr_count)
 {
-	u32 end_id;
 	struct mpxy_rpmi_mbox *rmb =
 		container_of(channel, struct mpxy_rpmi_mbox, channel);
 	u32 *attr_array = (u32 *)&rmb->msgprot_attrs;
-
-	end_id = base_attr_id + attr_count - 1;
+	u32 end_id = base_attr_id + attr_count - 1;
 
 	if (end_id >= MPXY_MSGPROT_RPMI_ATTR_MAX_ID)
 		return SBI_EBAD_RANGE;

@@ -108,12 +106,11 @@ static int mpxy_mbox_write_attributes(struct sbi_mpxy_channel *channel,
 				     u32 *outmem, u32 base_attr_id,
 				     u32 attr_count)
 {
-	int ret, mem_idx;
-	u32 end_id, attr_val, idx;
 	struct mpxy_rpmi_mbox *rmb =
 		container_of(channel, struct mpxy_rpmi_mbox, channel);
-
-	end_id = base_attr_id + attr_count - 1;
+	u32 end_id = base_attr_id + attr_count - 1;
+	u32 attr_val, idx;
+	int ret, mem_idx;
 
 	if (end_id >= MPXY_MSGPROT_RPMI_ATTR_MAX_ID)
 		return SBI_EBAD_RANGE;

@@ -140,15 +137,15 @@ static int __mpxy_mbox_send_message(struct sbi_mpxy_channel *channel,
 				  void *rx, u32 rx_max_len,
 				  unsigned long *ack_len)
 {
-	int ret;
-	u32 rx_len = 0;
-	struct mbox_xfer xfer;
-	struct rpmi_message_args args = {0};
 	struct mpxy_rpmi_mbox *rmb =
 		container_of(channel, struct mpxy_rpmi_mbox, channel);
 	const struct mpxy_rpmi_mbox_data *data = rmb->mbox_data;
 	const struct mpxy_rpmi_service_data *srv =
 		mpxy_find_rpmi_srvid(message_id, data);
+	struct rpmi_message_args args = {0};
+	struct mbox_xfer xfer;
+	u32 rx_len = 0;
+	int ret;
 
 	if (!srv)
 		return SBI_ENOTSUPP;

@@ -215,12 +212,12 @@ static int mpxy_mbox_send_message_withoutresp(struct sbi_mpxy_channel *channel,
 
 int mpxy_rpmi_mbox_init(const void *fdt, int nodeoff, const struct fdt_match *match)
 {
-	int rc, len;
+	const struct mpxy_rpmi_mbox_data *data = match->data;
+	struct mpxy_rpmi_mbox *rmb;
+	struct mbox_chan *chan;
 	const fdt32_t *val;
 	u32 channel_id;
-	struct mbox_chan *chan;
-	struct mpxy_rpmi_mbox *rmb;
-	const struct mpxy_rpmi_mbox_data *data = match->data;
+	int rc, len;
 
 	/* Allocate context for RPXY mbox client */
 	rmb = sbi_zalloc(sizeof(*rmb));

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c +0 -0

 	/* RPMI Message Protocol ID */
 	rmb->channel.attrs.msg_proto_id = SBI_MPXY_MSGPROTO_RPMI_ID;

Quoted 87 lines · Anup Patel

The local variable declarations should be at the start of function and preferrably organized like a inverted pyramid.

Signed-off-by Anup Patel <[email protected]>

---
 lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c | 31 ++++++++++++-----------------
 1 file changed, 13 insertions(+), 18 deletions(-)

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c +13 -16

index 26b9e6a6..27f4c571 100644
@@ -34,8 +34,8 @@ struct mpxy_rpmi_mbox {
 static const struct mpxy_rpmi_service_data *mpxy_find_rpmi_srvid(u32 message_id,
                                      const struct mpxy_rpmi_mbox_data *mbox_data)
 {
-     int mid = 0;
      const struct mpxy_rpmi_service_data *srv = mbox_data->service_data;
+     int mid = 0;
 
      for (mid = 0; srv[mid].id < mbox_data->num_services; mid++) {
              if (srv[mid].id == (u8)message_id)

@@ -57,12 +57,10 @@ static int mpxy_mbox_read_attributes(struct sbi_mpxy_channel *channel,
                                   u32 *outmem, u32 base_attr_id,
                                   u32 attr_count)
 {
-     u32 end_id;
      struct mpxy_rpmi_mbox *rmb =
              container_of(channel, struct mpxy_rpmi_mbox, channel);
      u32 *attr_array = (u32 *)&rmb->msgprot_attrs;
-
-     end_id = base_attr_id + attr_count - 1;
+     u32 end_id = base_attr_id + attr_count - 1;
 
      if (end_id >= MPXY_MSGPROT_RPMI_ATTR_MAX_ID)
              return SBI_EBAD_RANGE;

@@ -108,12 +106,11 @@ static int mpxy_mbox_write_attributes(struct sbi_mpxy_channel *channel,
                                   u32 *outmem, u32 base_attr_id,
                                   u32 attr_count)
 {
-     int ret, mem_idx;
-     u32 end_id, attr_val, idx;
      struct mpxy_rpmi_mbox *rmb =
              container_of(channel, struct mpxy_rpmi_mbox, channel);
-
-     end_id = base_attr_id + attr_count - 1;
+     u32 end_id = base_attr_id + attr_count - 1;
+     u32 attr_val, idx;
+     int ret, mem_idx;
 
      if (end_id >= MPXY_MSGPROT_RPMI_ATTR_MAX_ID)
              return SBI_EBAD_RANGE;

@@ -140,15 +137,15 @@ static int __mpxy_mbox_send_message(struct sbi_mpxy_channel *channel,
                                void *rx, u32 rx_max_len,
                                unsigned long *ack_len)
 {
-     int ret;
-     u32 rx_len = 0;
-     struct mbox_xfer xfer;
-     struct rpmi_message_args args = {0};
      struct mpxy_rpmi_mbox *rmb =
              container_of(channel, struct mpxy_rpmi_mbox, channel);
      const struct mpxy_rpmi_mbox_data *data = rmb->mbox_data;
      const struct mpxy_rpmi_service_data *srv =
              mpxy_find_rpmi_srvid(message_id, data);
+     struct rpmi_message_args args = {0};
+     struct mbox_xfer xfer;
+     u32 rx_len = 0;
+     int ret;
 
      if (!srv)
              return SBI_ENOTSUPP;

@@ -215,12 +212,12 @@ static int mpxy_mbox_send_message_withoutresp(struct sbi_mpxy_channel *channel,
 
 int mpxy_rpmi_mbox_init(const void *fdt, int nodeoff, const struct fdt_match *match)
 {
-     int rc, len;
+     const struct mpxy_rpmi_mbox_data *data = match->data;
+     struct mpxy_rpmi_mbox *rmb;
+     struct mbox_chan *chan;
      const fdt32_t *val;
      u32 channel_id;
-     struct mbox_chan *chan;
-     struct mpxy_rpmi_mbox *rmb;
-     const struct mpxy_rpmi_mbox_data *data = match->data;
+     int rc, len;
 

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c int mpxy_rpmi_mbox_init(const void *fdt, int nodeoff, const struct fdt_match *ma · line 274

Quoted 8 lines

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c +0 -2

      /* Allocate context for RPXY mbox client */
      rmb = sbi_zalloc(sizeof(*rmb));

@@ -270,8 +267,6 @@ int mpxy_rpmi_mbox_init(const void *fdt, int nodeoff, const struct fdt_match *ma
                                      mpxy_mbox_send_message_withresp;
      rmb->channel.send_message_without_response =
                                      mpxy_mbox_send_message_withoutresp;
-     /* No callback to switch events state data */
-     rmb->channel.switch_eventsstate = NULL;

Does this belong as part of the previous patch? Either way:

Reviewed-by Samuel Holland <[email protected]>

Quoted 3 lines

lib/utils/mpxy/fdt_mpxy_rpmi_mbox.c +0 -0

      /* RPMI Message Protocol ID */
      rmb->channel.attrs.msg_proto_id = SBI_MPXY_MSGPROTO_RPMI_ID;

The rpmi_mailbox.h uses structures defined in mailbox.h so let's include mailbox.h in rpmi_mailbox.h header.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi_utils/mailbox/rpmi_mailbox.h | 1 +
 1 file changed, 1 insertion(+)

The service group ID assignment and some of the BASE services have changes in the latest RPMI specification so let's update the RPMI implementation accordingly.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi_utils/mailbox/rpmi_msgprot.h   | 16 +++++++---------
 lib/utils/mailbox/fdt_mailbox_rpmi_shmem.c |  4 ----
 2 files changed, 7 insertions(+), 13 deletions(-)

The supervisor software can directly receive most of the system MSIs except P2A doorbell and MSIs preferred to be handled in M-mode.

Add MPXY RPMI mailbox client driver for the System MSI service group.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi_utils/mailbox/rpmi_msgprot.h |  88 ++++++++++
 lib/utils/mpxy/Kconfig                   |   4 +
 lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c    | 208 +++++++++++++++++++++++
 lib/utils/mpxy/objects.mk                |   3 +
 platform/generic/configs/defconfig       |   1 +
 5 files changed, 304 insertions(+)
 create mode 100644 lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c

include/sbi_utils/mailbox/rpmi_msgprot.h +88 -0

index 363cc653..8d8fa5e5 100644
@@ -208,6 +208,7 @@ enum rpmi_channel_attribute_id {
 enum rpmi_servicegroup_id {
 	RPMI_SRVGRP_ID_MIN = 0,
 	RPMI_SRVGRP_BASE = 0x0001,
+	RPMI_SRVGRP_SYSTEM_MSI = 0x0002,
 	RPMI_SRVGRP_SYSTEM_RESET = 0x0003,
 	RPMI_SRVGRP_SYSTEM_SUSPEND = 0x0004,
 	RPMI_SRVGRP_HSM = 0x0005,

@@ -267,6 +268,93 @@ struct rpmi_base_get_platform_info_resp {
 	char plat_info[];
 };
 
+/** RPMI System MSI ServiceGroup Service IDs */
+enum rpmi_sysmsi_service_id {
+	RPMI_SYSMSI_SRV_ENABLE_NOTIFICATION = 0x01,
+	RPMI_SYSMSI_SRV_GET_ATTRIBUTES = 0x2,
+	RPMI_SYSMSI_SRV_GET_MSI_ATTRIBUTES = 0x3,
+	RPMI_SYSMSI_SRV_SET_MSI_STATE = 0x4,
+	RPMI_SYSMSI_SRV_GET_MSI_STATE = 0x5,
+	RPMI_SYSMSI_SRV_SET_MSI_TARGET = 0x6,
+	RPMI_SYSMSI_SRV_GET_MSI_TARGET = 0x7,
+	RPMI_SYSMSI_SRV_ID_MAX_COUNT,
+};
+
+/** Response for system MSI service group attributes */
+struct rpmi_sysmsi_get_attributes_resp {
+	s32 status;
+	u32 sys_num_msi;
+	u32 p2a_db_index;
+	u32 flag0;
+	u32 flag1;
+};
+
+/** Request for system MSI attributes */
+struct rpmi_sysmsi_get_msi_attributes_req {
+	u32 sys_msi_index;
+};
+
+/** Response for system MSI attributes */
+struct rpmi_sysmsi_get_msi_attributes_resp {
+	s32 status;
+	u32 flag0;
+	u32 flag1;
+	u8 name[16];
+};
+
+#define RPMI_SYSMSI_MSI_ATTRIBUTES_FLAG0_PREF_PRIV	(1U << 0)
+
+/** Request for system MSI set state */
+struct rpmi_sysmsi_set_msi_state_req {
+	u32 sys_msi_index;
+	u32 sys_msi_state;
+};
+
+#define RPMI_SYSMSI_MSI_STATE_ENABLE			(1U << 0)
+#define RPMI_SYSMSI_MSI_STATE_PENDING			(1U << 1)
+
+/** Response for system MSI set state */
+struct rpmi_sysmsi_set_msi_state_resp {
+	s32 status;
+};
+
+/** Request for system MSI get state */
+struct rpmi_sysmsi_get_msi_state_req {
+	u32 sys_msi_index;
+};
+
+/** Response for system MSI get state */
+struct rpmi_sysmsi_get_msi_state_resp {
+	s32 status;
+	u32 sys_msi_state;
+};
+
+/** Request for system MSI set target */
+struct rpmi_sysmsi_set_msi_target_req {
+	u32 sys_msi_index;
+	u32 sys_msi_address_low;
+	u32 sys_msi_address_high;
+	u32 sys_msi_data;
+};
+
+/** Response for system MSI set target */
+struct rpmi_sysmsi_set_msi_target_resp {
+	s32 status;
+};
+
+/** Request for system MSI get target */
+struct rpmi_sysmsi_get_msi_target_req {
+	u32 sys_msi_index;
+};
+
+/** Response for system MSI get target */
+struct rpmi_sysmsi_get_msi_target_resp {
+	s32 status;
+	u32 sys_msi_address_low;
+	u32 sys_msi_address_high;
+	u32 sys_msi_data;
+};
+
 /** RPMI System Reset ServiceGroup Service IDs */
 enum rpmi_system_reset_service_id {
 	RPMI_SYSRST_SRV_ENABLE_NOTIFICATION = 0x01,

lib/utils/mpxy/Kconfig +4 -0

index ff88f24f..8d1f5183 100644
@@ -18,6 +18,10 @@ config FDT_MPXY_RPMI_CLOCK
 	bool "MPXY driver for RPMI clock service group"
 	default n
 
+config FDT_MPXY_RPMI_SYSMSI
+	bool "MPXY driver for RPMI system MSI service group"
+	default n
+
 endif
 
 endmenu

lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c new file +119 -0

new file mode 100644
index 00000000..3dacf28f
@@ -0,0 +1,208 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2025 Ventana Micro Systems Inc.
+ */
+
+#include <sbi/sbi_bitmap.h>
+#include <sbi/sbi_domain.h>
+#include <sbi/sbi_byteorder.h>
+#include <sbi/sbi_heap.h>
+#include <sbi_utils/mailbox/rpmi_mailbox.h>
+#include <sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h>
+
+struct mpxy_rpmi_sysmsi {
+	u32 sys_num_msi;
+	unsigned long *sys_msi_denied_bmap;
+};
+
+static int mpxy_rpmi_sysmis_xfer(void *context, struct mbox_chan *chan,
+				 struct mbox_xfer *xfer)
+{
+	struct rpmi_message_args *args = xfer->args;
+	struct mpxy_rpmi_sysmsi *smg = context;
+	u64 sys_msi_address;
+	u32 sys_msi_index;
+	int rc = 0;
+
+	if (!xfer->rx || args->type != RPMI_MSG_NORMAL_REQUEST)
+		return 0;
+
+	switch (args->service_id) {
+	case RPMI_SYSMSI_SRV_GET_ATTRIBUTES:
+		((u32 *)xfer->rx)[0] = cpu_to_le32(RPMI_SUCCESS);
+		((u32 *)xfer->rx)[1] = cpu_to_le32(smg->sys_num_msi);
+		((u32 *)xfer->rx)[2] = -1U;
+		((u32 *)xfer->rx)[3] = 0;
+		((u32 *)xfer->rx)[4] = 0;
+		args->rx_data_len = 5 * sizeof(u32);
+		break;
+	case RPMI_SYSMSI_SRV_GET_MSI_ATTRIBUTES:
+	case RPMI_SYSMSI_SRV_SET_MSI_STATE:
+	case RPMI_SYSMSI_SRV_GET_MSI_STATE:
+	case RPMI_SYSMSI_SRV_SET_MSI_TARGET:
+	case RPMI_SYSMSI_SRV_GET_MSI_TARGET:
+		sys_msi_index = le32_to_cpu(((u32 *)xfer->tx)[0]);
+		if (smg->sys_num_msi <= sys_msi_index) {
+			((u32 *)xfer->rx)[0] = cpu_to_le32(RPMI_ERR_INVALID_PARAM);
+			args->rx_data_len = sizeof(u32);
+			break;
+		}
+		if (bitmap_test(smg->sys_msi_denied_bmap, sys_msi_index)) {
+			((u32 *)xfer->rx)[0] = cpu_to_le32(RPMI_ERR_DENIED);
+			args->rx_data_len = sizeof(u32);
+			break;
+		}
+		if (args->service_id == RPMI_SYSMSI_SRV_SET_MSI_TARGET) {
+			sys_msi_address = le32_to_cpu(((u32 *)xfer->tx)[1]);
+			sys_msi_address |= ((u64)le32_to_cpu(((u32 *)xfer->tx)[2])) << 32;
+			if (!sbi_domain_check_addr_range(sbi_domain_thishart_ptr(),
+							 sys_msi_address, 0x4, PRV_S,
+							 SBI_DOMAIN_READ | SBI_DOMAIN_WRITE)) {
+				((u32 *)xfer->rx)[0] = cpu_to_le32(RPMI_ERR_INVALID_ADDR);
+				args->rx_data_len = sizeof(u32);
+				break;
+			}
+		}
+		rc = mbox_chan_xfer(chan, xfer);
+		break;
+	default:
+		((u32 *)xfer->rx)[0] = cpu_to_le32(RPMI_ERR_NOTSUPP);
+		args->rx_data_len = sizeof(u32);
+		break;
+	};
+
+	return rc;
+}
+
+static void mpxy_rpmi_sysmsi_cleanup(void *context)
+{
+	struct mpxy_rpmi_sysmsi *smg = context;
+
+	sbi_free(smg->sys_msi_denied_bmap);
+	sbi_free(smg);
+}
+
+static int mpxy_rpmi_sysmsi_setup(void **context, struct mbox_chan *chan,
+				  const struct mpxy_rpmi_mbox_data *data)
+{
+	struct rpmi_sysmsi_get_msi_attributes_resp gmaresp;
+	struct rpmi_sysmsi_get_msi_attributes_req gmareq;
+	struct rpmi_sysmsi_get_attributes_resp garesp;
+	struct mpxy_rpmi_sysmsi *smg;
+	int rc, i;
+
+	rc = rpmi_normal_request_with_status(chan, RPMI_SYSMSI_SRV_GET_ATTRIBUTES,
+					     NULL, 0, 0, &garesp, rpmi_u32_count(garesp),
+					     rpmi_u32_count(garesp));
+	if (rc)
+		return rc;
+
+	smg = sbi_zalloc(sizeof(*smg));
+	if (!smg)
+		return SBI_ENOMEM;
+
+	smg->sys_num_msi = garesp.sys_num_msi;
+	smg->sys_msi_denied_bmap = sbi_zalloc(bitmap_estimate_size(smg->sys_num_msi));
+	if (!smg->sys_msi_denied_bmap) {
+		sbi_free(smg);
+		return SBI_ENOMEM;
+	}
+
+	for (i = 0; i < smg->sys_num_msi; i++) {
+		gmareq.sys_msi_index = i;
+		rc = rpmi_normal_request_with_status(chan,
+						     RPMI_SYSMSI_SRV_GET_MSI_ATTRIBUTES,
+						     &gmareq, rpmi_u32_count(gmareq),
+						     rpmi_u32_count(gmareq),
+						     &gmaresp, rpmi_u32_count(gmaresp),
+						     rpmi_u32_count(gmaresp));

lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c new file +69 -0

+	*context = smg;
+	return 0;
+}
+
+static struct mpxy_rpmi_service_group_ops sysmsi_ops = {
+	.xfer_group = mpxy_rpmi_sysmis_xfer,
+	.setup_group = mpxy_rpmi_sysmsi_setup,
+	.cleanup_group = mpxy_rpmi_sysmsi_cleanup,
+};
+
+static struct mpxy_rpmi_service_data sysmsi_services[] = {
+{
+	.id = RPMI_SYSMSI_SRV_ENABLE_NOTIFICATION,
+	.min_tx_len = sizeof(struct rpmi_enable_notification_req),
+	.max_tx_len = sizeof(struct rpmi_enable_notification_req),
+	.min_rx_len = sizeof(struct rpmi_enable_notification_resp),
+	.max_rx_len = sizeof(struct rpmi_enable_notification_resp),
+},
+{
+	.id = RPMI_SYSMSI_SRV_GET_ATTRIBUTES,
+	.min_tx_len = 0,
+	.max_tx_len = 0,
+	.min_rx_len = sizeof(struct rpmi_sysmsi_get_attributes_resp),
+	.max_rx_len = sizeof(struct rpmi_sysmsi_get_attributes_resp),
+},
+{
+	.id = RPMI_SYSMSI_SRV_GET_MSI_ATTRIBUTES,
+	.min_tx_len = sizeof(struct rpmi_sysmsi_get_msi_attributes_req),
+	.max_tx_len = sizeof(struct rpmi_sysmsi_get_msi_attributes_req),
+	.min_rx_len = sizeof(struct rpmi_sysmsi_get_msi_attributes_resp),
+	.max_rx_len = sizeof(struct rpmi_sysmsi_get_msi_attributes_resp),
+},
+{
+	.id = RPMI_SYSMSI_SRV_SET_MSI_STATE,
+	.min_tx_len = sizeof(struct rpmi_sysmsi_set_msi_state_req),
+	.max_tx_len = sizeof(struct rpmi_sysmsi_set_msi_state_req),
+	.min_rx_len = sizeof(struct rpmi_sysmsi_set_msi_state_resp),
+	.max_rx_len = sizeof(struct rpmi_sysmsi_set_msi_state_resp),
+},
+{
+	.id = RPMI_SYSMSI_SRV_GET_MSI_STATE,
+	.min_tx_len = sizeof(struct rpmi_sysmsi_get_msi_state_req),
+	.max_tx_len = sizeof(struct rpmi_sysmsi_get_msi_state_req),
+	.min_rx_len = sizeof(struct rpmi_sysmsi_get_msi_state_resp),
+	.max_rx_len = sizeof(struct rpmi_sysmsi_get_msi_state_resp),
+},
+{
+	.id = RPMI_SYSMSI_SRV_SET_MSI_TARGET,
+	.min_tx_len = sizeof(struct rpmi_sysmsi_set_msi_target_req),
+	.max_tx_len = sizeof(struct rpmi_sysmsi_set_msi_target_req),
+	.min_rx_len = sizeof(struct rpmi_sysmsi_set_msi_target_resp),
+	.max_rx_len = sizeof(struct rpmi_sysmsi_set_msi_target_resp),
+},
+{
+	.id = RPMI_SYSMSI_SRV_GET_MSI_TARGET,
+	.min_tx_len = sizeof(struct rpmi_sysmsi_get_msi_target_req),
+	.max_tx_len = sizeof(struct rpmi_sysmsi_get_msi_target_req),
+	.min_rx_len = sizeof(struct rpmi_sysmsi_get_msi_target_resp),
+	.max_rx_len = sizeof(struct rpmi_sysmsi_get_msi_target_resp),
+},
+};
+
+static struct mpxy_rpmi_mbox_data sysmsi_data = {
+	.servicegrp_id = RPMI_SRVGRP_SYSTEM_MSI,
+	.num_services = RPMI_SYSMSI_SRV_ID_MAX_COUNT,
+	.service_data = sysmsi_services,
+	.group_ops = &sysmsi_ops,
+};
+

lib/utils/mpxy/objects.mk +3 -0

index 1c9afed9..5b6e9bdf 100644
@@ -14,3 +14,6 @@ libsbiutils-objs-$(CONFIG_FDT_MPXY_RPMI_MBOX) += mpxy/fdt_mpxy_rpmi_mbox.o
 
 carray-fdt_mpxy_drivers-$(CONFIG_FDT_MPXY_RPMI_CLOCK) += fdt_mpxy_rpmi_clock
 libsbiutils-objs-$(CONFIG_FDT_MPXY_RPMI_CLOCK) += mpxy/fdt_mpxy_rpmi_clock.o
+
+carray-fdt_mpxy_drivers-$(CONFIG_FDT_MPXY_RPMI_SYSMSI) += fdt_mpxy_rpmi_sysmsi
+libsbiutils-objs-$(CONFIG_FDT_MPXY_RPMI_SYSMSI) += mpxy/fdt_mpxy_rpmi_sysmsi.o

platform/generic/configs/defconfig +1 -0

index 55607a28..bb260626 100644
@@ -56,3 +56,4 @@ CONFIG_FDT_TIMER_PLMT=y
 CONFIG_FDT_MPXY=y
 CONFIG_FDT_MPXY_RPMI_MBOX=y
 CONFIG_FDT_MPXY_RPMI_CLOCK=y
+CONFIG_FDT_MPXY_RPMI_SYSMSI=y

Hi Anup,

Quoted 266 lines · Anup Patel

The supervisor software can directly receive most of the system MSIs except P2A doorbell and MSIs preferred to be handled in M-mode.

Add MPXY RPMI mailbox client driver for the System MSI service group.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi_utils/mailbox/rpmi_msgprot.h |  88 ++++++++++
 lib/utils/mpxy/Kconfig                   |   4 +
 lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c    | 208 +++++++++++++++++++++++
 lib/utils/mpxy/objects.mk                |   3 +
 platform/generic/configs/defconfig       |   1 +
 5 files changed, 304 insertions(+)
 create mode 100644 lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c

include/sbi_utils/mailbox/rpmi_msgprot.h +88 -0

index 363cc653..8d8fa5e5 100644
@@ -208,6 +208,7 @@ enum rpmi_channel_attribute_id {
 enum rpmi_servicegroup_id {
      RPMI_SRVGRP_ID_MIN = 0,
      RPMI_SRVGRP_BASE = 0x0001,
+     RPMI_SRVGRP_SYSTEM_MSI = 0x0002,
      RPMI_SRVGRP_SYSTEM_RESET = 0x0003,
      RPMI_SRVGRP_SYSTEM_SUSPEND = 0x0004,
      RPMI_SRVGRP_HSM = 0x0005,

@@ -267,6 +268,93 @@ struct rpmi_base_get_platform_info_resp {
      char plat_info[];
 };
 
+/** RPMI System MSI ServiceGroup Service IDs */
+enum rpmi_sysmsi_service_id {
+     RPMI_SYSMSI_SRV_ENABLE_NOTIFICATION = 0x01,
+     RPMI_SYSMSI_SRV_GET_ATTRIBUTES = 0x2,
+     RPMI_SYSMSI_SRV_GET_MSI_ATTRIBUTES = 0x3,
+     RPMI_SYSMSI_SRV_SET_MSI_STATE = 0x4,
+     RPMI_SYSMSI_SRV_GET_MSI_STATE = 0x5,
+     RPMI_SYSMSI_SRV_SET_MSI_TARGET = 0x6,
+     RPMI_SYSMSI_SRV_GET_MSI_TARGET = 0x7,
+     RPMI_SYSMSI_SRV_ID_MAX_COUNT,
+};
+
+/** Response for system MSI service group attributes */
+struct rpmi_sysmsi_get_attributes_resp {
+     s32 status;
+     u32 sys_num_msi;
+     u32 p2a_db_index;
+     u32 flag0;
+     u32 flag1;
+};
+
+/** Request for system MSI attributes */
+struct rpmi_sysmsi_get_msi_attributes_req {
+     u32 sys_msi_index;
+};
+
+/** Response for system MSI attributes */
+struct rpmi_sysmsi_get_msi_attributes_resp {
+     s32 status;
+     u32 flag0;
+     u32 flag1;
+     u8 name[16];
+};
+
+#define RPMI_SYSMSI_MSI_ATTRIBUTES_FLAG0_PREF_PRIV   (1U << 0)
+
+/** Request for system MSI set state */
+struct rpmi_sysmsi_set_msi_state_req {
+     u32 sys_msi_index;
+     u32 sys_msi_state;
+};
+
+#define RPMI_SYSMSI_MSI_STATE_ENABLE                 (1U << 0)
+#define RPMI_SYSMSI_MSI_STATE_PENDING                        (1U << 1)
+
+/** Response for system MSI set state */
+struct rpmi_sysmsi_set_msi_state_resp {
+     s32 status;
+};
+
+/** Request for system MSI get state */
+struct rpmi_sysmsi_get_msi_state_req {
+     u32 sys_msi_index;
+};
+
+/** Response for system MSI get state */
+struct rpmi_sysmsi_get_msi_state_resp {
+     s32 status;
+     u32 sys_msi_state;
+};
+
+/** Request for system MSI set target */
+struct rpmi_sysmsi_set_msi_target_req {
+     u32 sys_msi_index;
+     u32 sys_msi_address_low;
+     u32 sys_msi_address_high;
+     u32 sys_msi_data;
+};
+
+/** Response for system MSI set target */
+struct rpmi_sysmsi_set_msi_target_resp {
+     s32 status;
+};
+
+/** Request for system MSI get target */
+struct rpmi_sysmsi_get_msi_target_req {
+     u32 sys_msi_index;
+};
+
+/** Response for system MSI get target */
+struct rpmi_sysmsi_get_msi_target_resp {
+     s32 status;
+     u32 sys_msi_address_low;
+     u32 sys_msi_address_high;
+     u32 sys_msi_data;
+};
+
 /** RPMI System Reset ServiceGroup Service IDs */
 enum rpmi_system_reset_service_id {
      RPMI_SYSRST_SRV_ENABLE_NOTIFICATION = 0x01,

lib/utils/mpxy/Kconfig +4 -0

index ff88f24f..8d1f5183 100644
@@ -18,6 +18,10 @@ config FDT_MPXY_RPMI_CLOCK
      bool "MPXY driver for RPMI clock service group"
      default n
 
+config FDT_MPXY_RPMI_SYSMSI
+     bool "MPXY driver for RPMI system MSI service group"
+     default n
+
 endif
 
 endmenu

lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c new file +124 -0

new file mode 100644
index 00000000..3dacf28f
@@ -0,0 +1,208 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2025 Ventana Micro Systems Inc.
+ */
+
+#include <sbi/sbi_bitmap.h>
+#include <sbi/sbi_domain.h>
+#include <sbi/sbi_byteorder.h>
+#include <sbi/sbi_heap.h>
+#include <sbi_utils/mailbox/rpmi_mailbox.h>
+#include <sbi_utils/mpxy/fdt_mpxy_rpmi_mbox.h>
+
+struct mpxy_rpmi_sysmsi {
+     u32 sys_num_msi;
+     unsigned long *sys_msi_denied_bmap;
+};
+
+static int mpxy_rpmi_sysmis_xfer(void *context, struct mbox_chan *chan,
+                              struct mbox_xfer *xfer)
+{
+     struct rpmi_message_args *args = xfer->args;
+     struct mpxy_rpmi_sysmsi *smg = context;
+     u64 sys_msi_address;
+     u32 sys_msi_index;
+     int rc = 0;
+
+     if (!xfer->rx || args->type != RPMI_MSG_NORMAL_REQUEST)
+             return 0;
+
+     switch (args->service_id) {
+     case RPMI_SYSMSI_SRV_GET_ATTRIBUTES:
+             ((u32 *)xfer->rx)[0] = cpu_to_le32(RPMI_SUCCESS);
+             ((u32 *)xfer->rx)[1] = cpu_to_le32(smg->sys_num_msi);
+             ((u32 *)xfer->rx)[2] = -1U;
+             ((u32 *)xfer->rx)[3] = 0;
+             ((u32 *)xfer->rx)[4] = 0;
+             args->rx_data_len = 5 * sizeof(u32);
+             break;
+     case RPMI_SYSMSI_SRV_GET_MSI_ATTRIBUTES:
+     case RPMI_SYSMSI_SRV_SET_MSI_STATE:
+     case RPMI_SYSMSI_SRV_GET_MSI_STATE:
+     case RPMI_SYSMSI_SRV_SET_MSI_TARGET:
+     case RPMI_SYSMSI_SRV_GET_MSI_TARGET:
+             sys_msi_index = le32_to_cpu(((u32 *)xfer->tx)[0]);
+             if (smg->sys_num_msi <= sys_msi_index) {
+                     ((u32 *)xfer->rx)[0] = cpu_to_le32(RPMI_ERR_INVALID_PARAM);
+                     args->rx_data_len = sizeof(u32);
+                     break;
+             }
+             if (bitmap_test(smg->sys_msi_denied_bmap, sys_msi_index)) {
+                     ((u32 *)xfer->rx)[0] = cpu_to_le32(RPMI_ERR_DENIED);
+                     args->rx_data_len = sizeof(u32);
+                     break;
+             }
+             if (args->service_id == RPMI_SYSMSI_SRV_SET_MSI_TARGET) {
+                     sys_msi_address = le32_to_cpu(((u32 *)xfer->tx)[1]);
+                     sys_msi_address |= ((u64)le32_to_cpu(((u32 *)xfer->tx)[2])) << 32;
+                     if (!sbi_domain_check_addr_range(sbi_domain_thishart_ptr(),
+                                                      sys_msi_address, 0x4, PRV_S,
+                                                      SBI_DOMAIN_READ | SBI_DOMAIN_WRITE)) {
+                             ((u32 *)xfer->rx)[0] = cpu_to_le32(RPMI_ERR_INVALID_ADDR);
+                             args->rx_data_len = sizeof(u32);
+                             break;
+                     }
+             }
+             rc = mbox_chan_xfer(chan, xfer);
+             break;
+     default:
+             ((u32 *)xfer->rx)[0] = cpu_to_le32(RPMI_ERR_NOTSUPP);
+             args->rx_data_len = sizeof(u32);
+             break;
+     };
+
+     return rc;
+}
+
+static void mpxy_rpmi_sysmsi_cleanup(void *context)
+{
+     struct mpxy_rpmi_sysmsi *smg = context;
+
+     sbi_free(smg->sys_msi_denied_bmap);
+     sbi_free(smg);
+}
+
+static int mpxy_rpmi_sysmsi_setup(void **context, struct mbox_chan *chan,
+                               const struct mpxy_rpmi_mbox_data *data)
+{
+     struct rpmi_sysmsi_get_msi_attributes_resp gmaresp;
+     struct rpmi_sysmsi_get_msi_attributes_req gmareq;
+     struct rpmi_sysmsi_get_attributes_resp garesp;
+     struct mpxy_rpmi_sysmsi *smg;
+     int rc, i;
+
+     rc = rpmi_normal_request_with_status(chan, RPMI_SYSMSI_SRV_GET_ATTRIBUTES,
+                                          NULL, 0, 0, &garesp, rpmi_u32_count(garesp),
+                                          rpmi_u32_count(garesp));
+     if (rc)
+             return rc;
+
+     smg = sbi_zalloc(sizeof(*smg));
+     if (!smg)
+             return SBI_ENOMEM;
+
+     smg->sys_num_msi = garesp.sys_num_msi;
+     smg->sys_msi_denied_bmap = sbi_zalloc(bitmap_estimate_size(smg->sys_num_msi));
+     if (!smg->sys_msi_denied_bmap) {
+             sbi_free(smg);
+             return SBI_ENOMEM;
+     }
+
+     for (i = 0; i < smg->sys_num_msi; i++) {
+             gmareq.sys_msi_index = i;
+             rc = rpmi_normal_request_with_status(chan,
+                                                  RPMI_SYSMSI_SRV_GET_MSI_ATTRIBUTES,
+                                                  &gmareq, rpmi_u32_count(gmareq),
+                                                  rpmi_u32_count(gmareq),
+                                                  &gmaresp, rpmi_u32_count(gmaresp),
+                                                  rpmi_u32_count(gmaresp));
+             if (rc) {
+                     mpxy_rpmi_sysmsi_cleanup(smg);
+                     return rc;
+             }
+

lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c line 129

Quoted 5 lines

lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c new file +5 -0

+             if (garesp.p2a_db_index == i ||
+                 (gmaresp.flag0 & RPMI_SYSMSI_MSI_ATTRIBUTES_FLAG0_PREF_PRIV))
+                     bitmap_set(smg->sys_msi_denied_bmap, i, 1);
+     }
+

Is my understanding correct that this function will eventually contain code to wire up the P2A doorbell to M-mode for notification support?

Quoted 74 lines

lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c new file +73 -0

+     *context = smg;
+     return 0;
+}
+
+static struct mpxy_rpmi_service_group_ops sysmsi_ops = {
+     .xfer_group = mpxy_rpmi_sysmis_xfer,
+     .setup_group = mpxy_rpmi_sysmsi_setup,
+     .cleanup_group = mpxy_rpmi_sysmsi_cleanup,
+};
+
+static struct mpxy_rpmi_service_data sysmsi_services[] = {
+{
+     .id = RPMI_SYSMSI_SRV_ENABLE_NOTIFICATION,
+     .min_tx_len = sizeof(struct rpmi_enable_notification_req),
+     .max_tx_len = sizeof(struct rpmi_enable_notification_req),
+     .min_rx_len = sizeof(struct rpmi_enable_notification_resp),
+     .max_rx_len = sizeof(struct rpmi_enable_notification_resp),
+},
+{
+     .id = RPMI_SYSMSI_SRV_GET_ATTRIBUTES,
+     .min_tx_len = 0,
+     .max_tx_len = 0,
+     .min_rx_len = sizeof(struct rpmi_sysmsi_get_attributes_resp),
+     .max_rx_len = sizeof(struct rpmi_sysmsi_get_attributes_resp),
+},
+{
+     .id = RPMI_SYSMSI_SRV_GET_MSI_ATTRIBUTES,
+     .min_tx_len = sizeof(struct rpmi_sysmsi_get_msi_attributes_req),
+     .max_tx_len = sizeof(struct rpmi_sysmsi_get_msi_attributes_req),
+     .min_rx_len = sizeof(struct rpmi_sysmsi_get_msi_attributes_resp),
+     .max_rx_len = sizeof(struct rpmi_sysmsi_get_msi_attributes_resp),
+},
+{
+     .id = RPMI_SYSMSI_SRV_SET_MSI_STATE,
+     .min_tx_len = sizeof(struct rpmi_sysmsi_set_msi_state_req),
+     .max_tx_len = sizeof(struct rpmi_sysmsi_set_msi_state_req),
+     .min_rx_len = sizeof(struct rpmi_sysmsi_set_msi_state_resp),
+     .max_rx_len = sizeof(struct rpmi_sysmsi_set_msi_state_resp),
+},
+{
+     .id = RPMI_SYSMSI_SRV_GET_MSI_STATE,
+     .min_tx_len = sizeof(struct rpmi_sysmsi_get_msi_state_req),
+     .max_tx_len = sizeof(struct rpmi_sysmsi_get_msi_state_req),
+     .min_rx_len = sizeof(struct rpmi_sysmsi_get_msi_state_resp),
+     .max_rx_len = sizeof(struct rpmi_sysmsi_get_msi_state_resp),
+},
+{
+     .id = RPMI_SYSMSI_SRV_SET_MSI_TARGET,
+     .min_tx_len = sizeof(struct rpmi_sysmsi_set_msi_target_req),
+     .max_tx_len = sizeof(struct rpmi_sysmsi_set_msi_target_req),
+     .min_rx_len = sizeof(struct rpmi_sysmsi_set_msi_target_resp),
+     .max_rx_len = sizeof(struct rpmi_sysmsi_set_msi_target_resp),
+},
+{
+     .id = RPMI_SYSMSI_SRV_GET_MSI_TARGET,
+     .min_tx_len = sizeof(struct rpmi_sysmsi_get_msi_target_req),
+     .max_tx_len = sizeof(struct rpmi_sysmsi_get_msi_target_req),
+     .min_rx_len = sizeof(struct rpmi_sysmsi_get_msi_target_resp),
+     .max_rx_len = sizeof(struct rpmi_sysmsi_get_msi_target_resp),
+},
+};
+
+static struct mpxy_rpmi_mbox_data sysmsi_data = {
+     .servicegrp_id = RPMI_SRVGRP_SYSTEM_MSI,
+     .num_services = RPMI_SYSMSI_SRV_ID_MAX_COUNT,
+     .service_data = sysmsi_services,
+     .group_ops = &sysmsi_ops,
+};
+
+static const struct fdt_match sysmsi_match[] = {
+     { .compatible = "riscv,rpmi-mpxy-system-msi", .data = &sysmsi_data },
+     { },
+};

lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c line 208

Quoted 6 lines

lib/utils/mpxy/fdt_mpxy_rpmi_sysmsi.c new file +6 -0

+
+struct fdt_driver fdt_mpxy_rpmi_sysmsi = {
+     .match_table = sysmsi_match,
+     .init = mpxy_rpmi_mbox_init,
+     .experimental = true,
+};

All of these object definitions should be const.