diff mbox

[RFC,v2,10/12] opal-prd: Add firmware_request & firmware_notify implementations

Message ID 1495767271-8216-11-git-send-email-jk@ozlabs.org
State Accepted
Headers show

Commit Message

Jeremy Kerr May 26, 2017, 2:54 a.m. UTC
This change adds the implementation of firmware_request() and
firmware_notify(). To do this, we need to add a message queue, so that
we can properly handle out-of-order messages coming from firmware.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 external/opal-prd/opal-prd.c | 157 ++++++++++++++++++++++++++++++++++++++++++-
 external/opal-prd/thunk.S    |   2 +-
 include/opal-api.h           |  16 +++++
 3 files changed, 173 insertions(+), 2 deletions(-)

Comments

Vasant Hegde May 26, 2017, 11:21 a.m. UTC | #1
On 05/26/2017 08:24 AM, Jeremy Kerr wrote:
> This change adds the implementation of firmware_request() and
> firmware_notify(). To do this, we need to add a message queue, so that
> we can properly handle out-of-order messages coming from firmware.
>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> ---

.../...

>
> +uint64_t hservice_firmware_request(uint64_t req_len, void *req,
> +		uint64_t *resp_lenp, void *resp)
> +{
> +	struct opal_prd_msg *msg = ctx->msg;
> +	uint64_t resp_len;
> +	size_t size;
> +	int rc, n;
> +
> +	resp_len = be64_to_cpu(*resp_lenp);
> +
> +	pr_log(LOG_DEBUG,
> +			"HBRT: firmware request: %lu bytes req, %lu bytes resp",
> +			req_len, resp_len);
> +
> +	/* sanity check for potential overflows */
> +	if (req_len > 0xffff || resp_len > 0xffff)

Sorry. I missed to comment on this one earlier.
I think we should log error message here so that it becomes easy to debug.

> +		return -1;
> +
> +	size = sizeof(msg->hdr) + sizeof(msg->token) +
> +		sizeof(msg->fw_req) + req_len;
> +
> +	/* we need the entire message to fit within the 2-byte size field */
> +	if (size > 0xffff)

ditto.

> +		return -1;
> +


.../...

> +
> +	/* We have an "inner" poll loop here, as we want to ensure that the
> +	 * next entry into HBRT is the return from this function. So, only
> +	 * read from the prd fd, and queue anything that isn't a response
> +	 * to this request
> +	 */

This logic works fine for most cases. In worst case we may endup waiting for 
max_msgq_len messages before returning error. How about having timer here ? Say:
   run timer for 1 minute, if we don't get response then error out?


-Vasant

> +	n = 0;
> +	for (;;) {
> +		struct prd_msgq_item *item;
> +
> +		rc = read_prd_msg(ctx);
> +		if (rc)
> +			return -1;
> +
> +		msg = ctx->msg;
> +		if (msg->hdr.type == OPAL_PRD_MSG_TYPE_FIRMWARE_RESPONSE) {
> +			size = be64toh(msg->fw_resp.len);
> +			if (size > resp_len)
> +				return -1;
> +
> +			/* success! a valid response that fits into HBRT's
> +			 * resp buffer */
> +			memcpy(resp, msg->fw_resp.data, size);
> +			*resp_lenp = htobe64(size);
> +			return 0;
> +		}
> +
> +		/* not a response? queue up for later consumption */
> +		if (++n > max_msgq_len) {
> +			pr_log(LOG_ERR,
> +				"FW: too many messages queued (%d) while "
> +				"waiting for FIRMWARE_RESPONSE", n);
> +			return -1;
> +		}
> +		size = be16toh(msg->hdr.size);
> +		item = malloc(sizeof(*item) + size);
> +		memcpy(&item->msg, msg, size);
> +		list_add_tail(&ctx->msgq, &item->list);
> +	}
> +}
> +
diff mbox

Patch

diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c
index e6cd353..a266441 100644
--- a/external/opal-prd/opal-prd.c
+++ b/external/opal-prd/opal-prd.c
@@ -50,6 +50,8 @@ 
 #include <opal-api.h>
 #include <types.h>
 
+#include <ccan/list/list.h>
+
 #include "opal-prd.h"
 #include "hostboot-interface.h"
 #include "module.h"
@@ -65,6 +67,11 @@  struct prd_range {
 	uint32_t		instance;
 };
 
+struct prd_msgq_item {
+	struct list_node	list;
+	struct opal_prd_msg	msg;
+};
+
 struct opal_prd_ctx {
 	int			fd;
 	int			socket;
@@ -80,6 +87,7 @@  struct opal_prd_ctx {
 	char			*hbrt_file_name;
 	bool			use_syslog;
 	bool			expert_mode;
+	struct list_head	msgq;
 	struct opal_prd_msg	*msg;
 	size_t			msg_alloc_len;
 	void			(*vlog)(int, const char *, va_list);
@@ -124,6 +132,8 @@  static const char *hbrt_code_region_name = "ibm,hbrt-code-image";
 static const int opal_prd_version = 1;
 static uint64_t opal_prd_ipoll = 0xf000000000000000;
 
+static const int max_msgq_len = 16;
+
 static const char *ipmi_devnode = "/dev/ipmi0";
 static const int ipmi_timeout_ms = 5000;
 
@@ -153,6 +163,8 @@  struct func_desc {
 	void *toc;
 } hbrt_entry;
 
+static int read_prd_msg(struct opal_prd_ctx *ctx);
+
 static struct prd_range *find_range(const char *name, uint32_t instance)
 {
 	struct prd_range *range;
@@ -270,6 +282,7 @@  extern int call_mfg_htmgt_pass_thru(uint16_t i_cmdLength, uint8_t *i_cmdData,
 extern int call_apply_attr_override(uint8_t *i_data, size_t size);
 extern int call_run_command(int argc, const char **argv, char **o_outString);
 extern uint64_t call_get_ipoll_events(void);
+extern int call_firmware_notify(uint64_t len, void *data);
 
 void hservice_puts(const char *str)
 {
@@ -680,6 +693,102 @@  uint64_t hservice_get_interface_capabilities(uint64_t set)
 	return 0;
 }
 
+uint64_t hservice_firmware_request(uint64_t req_len, void *req,
+		uint64_t *resp_lenp, void *resp)
+{
+	struct opal_prd_msg *msg = ctx->msg;
+	uint64_t resp_len;
+	size_t size;
+	int rc, n;
+
+	resp_len = be64_to_cpu(*resp_lenp);
+
+	pr_log(LOG_DEBUG,
+			"HBRT: firmware request: %lu bytes req, %lu bytes resp",
+			req_len, resp_len);
+
+	/* sanity check for potential overflows */
+	if (req_len > 0xffff || resp_len > 0xffff)
+		return -1;
+
+	size = sizeof(msg->hdr) + sizeof(msg->token) +
+		sizeof(msg->fw_req) + req_len;
+
+	/* we need the entire message to fit within the 2-byte size field */
+	if (size > 0xffff)
+		return -1;
+
+	/* variable sized message, so we may need to expand our buffer */
+	if (size > ctx->msg_alloc_len) {
+		msg = realloc(ctx->msg, size);
+		if (!msg) {
+			pr_log(LOG_ERR,
+				"FW: failed to expand message buffer: %m");
+			return -1;
+		}
+		ctx->msg = msg;
+		ctx->msg_alloc_len = size;
+	}
+
+	memset(msg, 0, size);
+
+	/* construct request message... */
+	msg->hdr.type = OPAL_PRD_MSG_TYPE_FIRMWARE_REQUEST;
+	msg->hdr.size = htobe16(size);
+	msg->fw_req.req_len = htobe64(req_len);
+	msg->fw_req.resp_len = htobe64(resp_len);
+	memcpy(msg->fw_req.data, req, req_len);
+
+	hexdump((void *)msg, size);
+
+	/* ... and send to firmware */
+	rc = write(ctx->fd, msg, size);
+	if (rc != size) {
+		pr_log(LOG_WARNING,
+			"FW: Failed to send FIRMWARE_REQUEST message: %m");
+		return -1;
+	}
+
+	/* We have an "inner" poll loop here, as we want to ensure that the
+	 * next entry into HBRT is the return from this function. So, only
+	 * read from the prd fd, and queue anything that isn't a response
+	 * to this request
+	 */
+	n = 0;
+	for (;;) {
+		struct prd_msgq_item *item;
+
+		rc = read_prd_msg(ctx);
+		if (rc)
+			return -1;
+
+		msg = ctx->msg;
+		if (msg->hdr.type == OPAL_PRD_MSG_TYPE_FIRMWARE_RESPONSE) {
+			size = be64toh(msg->fw_resp.len);
+			if (size > resp_len)
+				return -1;
+
+			/* success! a valid response that fits into HBRT's
+			 * resp buffer */
+			memcpy(resp, msg->fw_resp.data, size);
+			*resp_lenp = htobe64(size);
+			return 0;
+		}
+
+		/* not a response? queue up for later consumption */
+		if (++n > max_msgq_len) {
+			pr_log(LOG_ERR,
+				"FW: too many messages queued (%d) while "
+				"waiting for FIRMWARE_RESPONSE", n);
+			return -1;
+		}
+		size = be16toh(msg->hdr.size);
+		item = malloc(sizeof(*item) + size);
+		memcpy(&item->msg, msg, size);
+		list_add_tail(&ctx->msgq, &item->list);
+	}
+}
+
 int hservices_init(struct opal_prd_ctx *ctx, void *code)
 {
 	uint64_t *s, *d;
@@ -1228,6 +1337,27 @@  static int handle_msg_occ_reset(struct opal_prd_ctx *ctx,
 	return 0;
 }
 
+static int handle_msg_firmware_notify(struct opal_prd_ctx *ctx,
+		struct opal_prd_msg *msg)
+{
+	uint64_t len;
+	void *buf;
+
+	len = be64toh(msg->fw_notify.len);
+	buf = msg->fw_notify.data;
+
+	pr_debug("FW: firmware notification, %ld bytes", len);
+
+	if (!hservice_runtime->firmware_notify) {
+		pr_log_nocall("firmware_notify");
+		return -1;
+	}
+
+	call_firmware_notify(len, buf);
+
+	return 0;
+}
+
 static int handle_prd_msg(struct opal_prd_ctx *ctx, struct opal_prd_msg *msg)
 {
 	int rc = -1;
@@ -1242,6 +1372,9 @@  static int handle_prd_msg(struct opal_prd_ctx *ctx, struct opal_prd_msg *msg)
 	case OPAL_PRD_MSG_TYPE_OCC_ERROR:
 		rc = handle_msg_occ_error(ctx, msg);
 		break;
+	case OPAL_PRD_MSG_TYPE_FIRMWARE_NOTIFY:
+		rc = handle_msg_firmware_notify(ctx, msg);
+		break;
 	default:
 		pr_log(LOG_WARNING, "Invalid incoming message type 0x%x",
 				msg->hdr.type);
@@ -1250,6 +1383,24 @@  static int handle_prd_msg(struct opal_prd_ctx *ctx, struct opal_prd_msg *msg)
 	return rc;
 }
 
+#define list_for_each_pop(h, i, type, member) \
+	for (i = list_pop((h), type, member); \
+		i; \
+		i = list_pop((h), type, member))
+
+
+static int process_msgq(struct opal_prd_ctx *ctx)
+{
+	struct prd_msgq_item *item;
+
+	list_for_each_pop(&ctx->msgq, item, struct prd_msgq_item, list) {
+		handle_prd_msg(ctx, &item->msg);
+		free(item);
+	}
+
+	return 0;
+}
+
 static int read_prd_msg(struct opal_prd_ctx *ctx)
 {
 	struct opal_prd_msg *msg;
@@ -1600,6 +1751,9 @@  static int run_attn_loop(struct opal_prd_ctx *ctx)
 	pollfds[1].events = POLLIN | POLLERR;
 
 	for (;;) {
+		/* run through any pending messages */
+		process_msgq(ctx);
+
 		rc = poll(pollfds, 2, -1);
 		if (rc < 0) {
 			pr_log(LOG_ERR, "FW: event poll failed: %m");
@@ -1691,6 +1845,8 @@  static int run_prd_daemon(struct opal_prd_ctx *ctx)
 	}
 
 
+	list_head_init(&ctx->msgq);
+
 	i2c_init();
 
 #ifdef DEBUG_I2C
@@ -1720,7 +1876,6 @@  static int run_prd_daemon(struct opal_prd_ctx *ctx)
 		goto out_close;
 	}
 
-
 	if (ctx->hbrt_file_name) {
 		rc = map_hbrt_file(ctx, ctx->hbrt_file_name);
 		if (rc) {
diff --git a/external/opal-prd/thunk.S b/external/opal-prd/thunk.S
index b18e3cb..cca5890 100644
--- a/external/opal-prd/thunk.S
+++ b/external/opal-prd/thunk.S
@@ -197,7 +197,7 @@  hinterface:
 	DISABLED_THUNK(hservice_map_phys_mem)
 	DISABLED_THUNK(hservice_unmap_phys_mem)
 	DISABLED_THUNK(hservice_hcode_scom_update)
-	DISABLED_THUNK(hservice_firmware_request)
+	CALLBACK_THUNK(hservice_firmware_request)
 .globl __hinterface_pad
 __hinterface_pad:
 	/* Reserved space for future growth */
diff --git a/include/opal-api.h b/include/opal-api.h
index 80033c6..4d5b1eb 100644
--- a/include/opal-api.h
+++ b/include/opal-api.h
@@ -1029,6 +1029,9 @@  enum opal_prd_msg_type {
 	OPAL_PRD_MSG_TYPE_OCC_ERROR,	/* HBRT <-- OPAL */
 	OPAL_PRD_MSG_TYPE_OCC_RESET,	/* HBRT <-- OPAL */
 	OPAL_PRD_MSG_TYPE_OCC_RESET_NOTIFY, /* HBRT --> OPAL */
+	OPAL_PRD_MSG_TYPE_FIRMWARE_REQUEST, /* HBRT --> OPAL */
+	OPAL_PRD_MSG_TYPE_FIRMWARE_RESPONSE, /* HBRT <-- OPAL */
+	OPAL_PRD_MSG_TYPE_FIRMWARE_NOTIFY, /* HBRT <-- OPAL */
 };
 
 struct opal_prd_msg_header {
@@ -1060,6 +1063,19 @@  struct opal_prd_msg {
 		struct {
 			__be64	chip;
 		} occ_reset;
+		struct {
+			__be64	req_len;
+			__be64	resp_len;
+			char	data[];
+		} fw_req;
+		struct {
+			__be64	len;
+			char	data[];
+		} fw_resp;
+		struct {
+			__be64	len;
+			char	data[];
+		} fw_notify;
 	};
 };