<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<small>Hi Alistair,</small><br>
<br>
<div class="moz-cite-prefix">On 07/31/2015 10:56 AM, Alistair Popple
wrote:<br>
</div>
<blockquote cite="mid:9164950.gzUp6injqn@mexican" type="cite">
<pre wrap="">Hi Neelesh,
Comments below.
On Thu, 16 Jul 2015 16:41:35 Neelesh Gupta wrote:
</pre>
<blockquote type="cite">
<pre wrap="">In the error path of the OPAL function to receive the ipmi message,
the function returns the error code without deleting the message
containing response. Though the kernel doesn't claim this message
later and continue with the subsequent ipmi commands. This leads to
a scenario when there is a mismatch between the ipmi command and its
response for all the subsequent ipmi commands.
Signed-off-by: Neelesh Gupta <a class="moz-txt-link-rfc2396E" href="mailto:neelegup@linux.vnet.ibm.com"><neelegup@linux.vnet.ibm.com></a>
Cc: Alistair Popple <a class="moz-txt-link-rfc2396E" href="mailto:alistair@popple.id.au"><alistair@popple.id.au></a>
---
hw/ipmi/ipmi-opal.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/hw/ipmi/ipmi-opal.c b/hw/ipmi/ipmi-opal.c
index 237f8c0..d37f151 100644
--- a/hw/ipmi/ipmi-opal.c
+++ b/hw/ipmi/ipmi-opal.c
@@ -75,25 +75,25 @@ static int64_t opal_ipmi_recv(uint64_t interface,
msg = list_top(&msgq, struct ipmi_msg, link);
if (!msg) {
- rc = OPAL_EMPTY;
- goto out_unlock;
</pre>
</blockquote>
<pre wrap="">
My preference would be to keep out_unlock and add out_del_msg. Eg:
out_del_msg:
list_del(&msg->link);
if (list_empty(&msgq))
opal_update_pending_evt(ipmi_backend->opal_event_ipmi_recv,
0);
ipmi_free_msg(msg);
out_unlock:
unlock(&msgq_lock);
return rc;</pre>
</blockquote>
<br>
<small>I earlier thought of not calling an interface (ipmi_free_msg)
while taking the lock,<br>
but not a concern in this case, so will change as you suggested.<br>
<br>
Thanks,<br>
Neelesh.<br>
</small><br>
<blockquote cite="mid:9164950.gzUp6injqn@mexican" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ unlock(&msgq_lock);
+ return OPAL_EMPTY;
}
if (opal_ipmi_msg->version != OPAL_IPMI_MSG_FORMAT_VERSION_1) {
prerror("OPAL IPMI: Incorrect version\n");
rc = OPAL_UNSUPPORTED;
- goto out_unlock;
+ goto out_del_msg;
}
if (interface != IPMI_DEFAULT_INTERFACE) {
prerror("IPMI: Invalid interface 0x%llx in opal_ipmi_recv\n",
</pre>
</blockquote>
<pre wrap="">interface);
</pre>
<blockquote type="cite">
<pre wrap="">- rc = OPAL_EMPTY;
- goto out_unlock;
+ rc = OPAL_PARAMETER;
+ goto out_del_msg;
}
if (*msg_len - sizeof(struct opal_ipmi_msg) < msg->resp_size + 1) {
rc = OPAL_RESOURCE;
- goto out_unlock;
+ goto out_del_msg;
}
list_del(&msg->link);
@@ -115,8 +115,13 @@ static int64_t opal_ipmi_recv(uint64_t interface,
return OPAL_SUCCESS;
-out_unlock:
+out_del_msg:
+ list_del(&msg->link);
+ if (list_empty(&msgq))
+ opal_update_pending_evt(ipmi_backend->opal_event_ipmi_recv,
</pre>
</blockquote>
<pre wrap="">0);
</pre>
<blockquote type="cite">
<pre wrap=""> unlock(&msgq_lock);
+
+ ipmi_free_msg(msg);
return rc;
}
_______________________________________________
Skiboot mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Skiboot@lists.ozlabs.org">Skiboot@lists.ozlabs.org</a>
<a class="moz-txt-link-freetext" href="https://lists.ozlabs.org/listinfo/skiboot">https://lists.ozlabs.org/listinfo/skiboot</a>
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<br>
</body>
</html>