diff mbox

[1/3] ipmi: Add a function to (re)initialise a message without allocation

Message ID 1421122875-2963-1-git-send-email-alistair@popple.id.au
State Changes Requested
Headers show

Commit Message

Alistair Popple Jan. 13, 2015, 4:21 a.m. UTC 
Currently the only functions we have for initialising ipmi messages
with the correct values also allocate memory for the message. In some
cases we want to reuse previously allocated messages to avoid
continually freeing/allocating memory.

This patch introduces a function which (re)initialises a previously
allocated message and converts existing instances of this behaviour
over to the new function.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
 core/ipmi.c        | 28 ++++++++++++++++++----------
 hw/ipmi/ipmi-fru.c |  7 +++----
 hw/ipmi/ipmi-sel.c | 24 ++++++++++++------------
 include/ipmi.h     |  7 +++++++
 4 files changed, 40 insertions(+), 26 deletions(-)

Comments

Stewart Smith Jan. 28, 2015, 5:40 a.m. UTC | #1 
Alistair Popple <alistair@popple.id.au> writes:
> Currently the only functions we have for initialising ipmi messages
> with the correct values also allocate memory for the message. In some
> cases we want to reuse previously allocated messages to avoid
> continually freeing/allocating memory.
>
> This patch introduces a function which (re)initialises a previously
> allocated message and converts existing instances of this behaviour
> over to the new function.

fails make check:

/tmp/cctmIzV0.o: In function `fru_write_complete':
/home/stewart/skiboot/hw/ipmi/test/../ipmi-fru.c:184: undefined reference to `ipmi_init_msg'
Alistair Popple Jan. 28, 2015, 7:30 a.m. UTC | #2 
Hi,

On Wed, 28 Jan 2015 16:40:05 Stewart Smith wrote:
> Alistair Popple <alistair@popple.id.au> writes:
> > Currently the only functions we have for initialising ipmi messages
> > with the correct values also allocate memory for the message. In some
> > cases we want to reuse previously allocated messages to avoid
> > continually freeing/allocating memory.
> > 
> > This patch introduces a function which (re)initialises a previously
> > allocated message and converts existing instances of this behaviour
> > over to the new function.
> 
> fails make check:
> 
> /tmp/cctmIzV0.o: In function `fru_write_complete':
> /home/stewart/skiboot/hw/ipmi/test/../ipmi-fru.c:184: undefined reference to
> `ipmi_init_msg'

So apparently I just write the tests, not run them :)

Thanks for catching. I will resubmit with fixes for the test.

Regards,

Alistair
diff mbox

Patch

diff --git a/core/ipmi.c b/core/ipmi.c
index 14c35f3..a8d99e1 100644
--- a/core/ipmi.c
+++ b/core/ipmi.c
@@ -27,6 +27,22 @@  void ipmi_free_msg(struct ipmi_msg *msg)
 	msg->backend->free_msg(msg);
 }
 
+void ipmi_init_msg(struct ipmi_msg *msg, int interface,
+		   uint32_t code, void (*complete)(struct ipmi_msg *),
+		   void *user_data, size_t req_size, size_t resp_size)
+{
+	/* We don't actually support multiple interfaces at the moment. */
+	assert(interface == IPMI_DEFAULT_INTERFACE);
+
+	msg->backend = ipmi_backend;
+	msg->cmd = IPMI_CMD(code);
+	msg->netfn = IPMI_NETFN(code) << 2;
+	msg->req_size = req_size;
+	msg->resp_size = resp_size;
+	msg->complete = complete;
+	msg->user_data = user_data;
+}
+
 struct ipmi_msg *ipmi_mkmsg_simple(uint32_t code, void *req_data, size_t req_size)
 {
 	return ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, code, ipmi_free_msg, NULL,
@@ -40,20 +56,12 @@  struct ipmi_msg *ipmi_mkmsg(int interface, uint32_t code,
 {
 	struct ipmi_msg *msg;
 
-	/* We don't actually support multiple interfaces at the moment. */
-	assert(interface == IPMI_DEFAULT_INTERFACE);
-
 	msg = ipmi_backend->alloc_msg(req_size, resp_size);
 	if (!msg)
 		return NULL;
 
-	msg->backend = ipmi_backend;
-	msg->cmd = IPMI_CMD(code);
-	msg->netfn = IPMI_NETFN(code) << 2;
-	msg->req_size = req_size;
-	msg->resp_size = resp_size;
-	msg->complete = complete;
-	msg->user_data = user_data;
+	ipmi_init_msg(msg, interface, code, complete, user_data, req_size,
+		      resp_size);
 
 	/* Commands are free to over ride this if they want to handle errors */
 	msg->error = ipmi_free_msg;
diff --git a/hw/ipmi/ipmi-fru.c b/hw/ipmi/ipmi-fru.c
index 49a7e0f..77b23e6 100644
--- a/hw/ipmi/ipmi-fru.c
+++ b/hw/ipmi/ipmi-fru.c
@@ -181,10 +181,9 @@  static void fru_write_complete(struct ipmi_msg *msg)
 		goto out;
 
 	offset = msg->data[WRITE_INDEX];
-	msg->req_size = MIN(msg->data[REMAINING] + 3, IPMI_MAX_REQ_SIZE);
-	msg->cmd = IPMI_CMD(IPMI_WRITE_FRU);
-	msg->netfn = IPMI_NETFN(IPMI_WRITE_FRU) << 2;
-	msg->resp_size = 2;
+	ipmi_init_msg(msg, IPMI_DEFAULT_INTERFACE, IPMI_WRITE_FRU,
+		      fru_write_complete, NULL,
+		      MIN(msg->data[REMAINING] + 3, IPMI_MAX_REQ_SIZE), 2);
 
 	memmove(&msg->data[3], &msg->data[offset + 3], msg->req_size - 3);
 
diff --git a/hw/ipmi/ipmi-sel.c b/hw/ipmi/ipmi-sel.c
index d6f7f47..22d48fd 100644
--- a/hw/ipmi/ipmi-sel.c
+++ b/hw/ipmi/ipmi-sel.c
@@ -67,6 +67,7 @@  static void ipmi_elog_poll(struct ipmi_msg *msg)
 	static unsigned int reservation_id = 0;
 	static unsigned int record_id = 0;
 	struct errorlog *elog_buf = (struct errorlog *) msg->user_data;
+	size_t req_size;
 
 	if (msg->cmd == IPMI_CMD(IPMI_RESERVE_SEL)) {
 		reservation_id = msg->data[0];
@@ -101,9 +102,17 @@  static void ipmi_elog_poll(struct ipmi_msg *msg)
 		return;
 	}
 
-	msg->cmd = IPMI_CMD(IPMI_PARTIAL_ADD_ESEL);
-	msg->netfn = IPMI_NETFN(IPMI_PARTIAL_ADD_ESEL) << 2;
-	msg->resp_size = 2;
+	if ((pel_size - index) < (IPMI_MAX_REQ_SIZE - ESEL_HDR_SIZE)) {
+		/* Last data to send */
+		msg->data[6] = 1;
+		req_size = pel_size - index + ESEL_HDR_SIZE;
+	} else {
+		msg->data[6] = 0;
+		req_size = IPMI_MAX_REQ_SIZE;
+	}
+
+	ipmi_init_msg(msg, IPMI_DEFAULT_INTERFACE, IPMI_PARTIAL_ADD_ESEL,
+		      ipmi_elog_poll, elog_buf, req_size, 2);
 
 	msg->data[0] = reservation_id & 0xff;
 	msg->data[1] = (reservation_id >> 8) & 0xff;
@@ -112,15 +121,6 @@  static void ipmi_elog_poll(struct ipmi_msg *msg)
 	msg->data[4] = index & 0xff;
 	msg->data[5] = (index >> 8) & 0xff;
 
-	if ((pel_size - index) < (IPMI_MAX_REQ_SIZE - ESEL_HDR_SIZE)) {
-		/* Last data to send */
-		msg->data[6] = 1;
-		msg->req_size = pel_size - index + ESEL_HDR_SIZE;
-	} else {
-		msg->data[6] = 0;
-		msg->req_size = IPMI_MAX_REQ_SIZE;
-	}
-
 	memcpy(&msg->data[ESEL_HDR_SIZE], &pel_buf[index], msg->req_size - ESEL_HDR_SIZE);
 	index += msg->req_size - ESEL_HDR_SIZE;
 
diff --git a/include/ipmi.h b/include/ipmi.h
index 50de293..bbeae5a 100644
--- a/include/ipmi.h
+++ b/include/ipmi.h
@@ -163,6 +163,13 @@  struct ipmi_msg *ipmi_mkmsg(int interface, uint32_t code,
 			    void *user_data, void *req_data, size_t req_size,
 			    size_t resp_size);
 
+/* Initialise a previously allocated message with the required
+fields. The caller must ensure the message is large enough to hold the
+request and response data. */
+void ipmi_init_msg(struct ipmi_msg *msg, int interface,
+		   uint32_t code, void (*complete)(struct ipmi_msg *),
+		   void *user_data, size_t req_size, size_t resp_size);
+
 /* Add an ipmi message to the queue */
 int ipmi_queue_msg(struct ipmi_msg *msg);