<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<small>Hi Alistair,<br>
<br>
Thanks for the review.<br>
</small><br>
<div class="moz-cite-prefix">On 07/28/2015 11:21 PM, Alistair Popple
wrote:<br>
</div>
<blockquote cite="mid:2100038.nvIKqFquOH@mexican" type="cite">
<pre wrap="">Hi Neelesh,
This fix looks reasonable to me, although Jeremy would be the best person to
comment if he has time. I wonder why we bother polling at all given that our
event interface should call opal_ipmi_recv() whenever a message is ready?</pre>
</blockquote>
<br>
<small>Agree. I thought about it and didn't find any reason to have
it as we have event<br>
mechanism.. but didn't think of changing as it is not causing any
issue..</small><br>
<br>
<blockquote cite="mid:2100038.nvIKqFquOH@mexican" type="cite">
<pre wrap="">
Also the firmware fix you refer to and this fix are independent of each other
so there's no ordering issues there.</pre>
</blockquote>
<br>
<small>Correct. Though, there is no relation, but I figured out the
skiboot issue after<br>
this change.. yes, they are independent. Please find time to
review the<br>
skiboot patch.</small><br>
<br>
<small>Corey,<br>
<br>
Please queue this patch for upstream if you Ok with it.<br>
<br>
Thanks,<br>
Neelesh.<br>
</small><br>
<blockquote cite="mid:2100038.nvIKqFquOH@mexican" type="cite">
<pre wrap="">
Reviewed-By: Alistair Popple <a class="moz-txt-link-rfc2396E" href="mailto:alistair@popple.id.au"><alistair@popple.id.au></a>
On Tue, 28 Jul 2015 13:20:07 Neelesh Gupta wrote:
</pre>
<blockquote type="cite">
<pre wrap="">
On 07/17/2015 02:12 PM, Neelesh Gupta wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi Corey,
On 07/16/2015 08:31 PM, Corey Minyard wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Ok, this looks fine. A couple of question...
Do I need to send this upstream right now? How well has this been
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">tested?
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">
I would want either Jeremy or Alistair to review this patch before you
send this
upstream. There is also firmware piece
<a class="moz-txt-link-freetext" href="http://patchwork.ozlabs.org/patch/496645/">http://patchwork.ozlabs.org/patch/496645/</a>
awaiting review.
In the testing front, I manually made the opal_ipmi_recv() function to
fail for testing
the error path and see if the driver recovers from it and subsequent
ipmi commands
work all good.
</pre>
</blockquote>
<pre wrap="">
Hi Jeremy/Alistair,
Could you please review it and the corresponding skiboot patch...
Thanks,
Neelesh.
</pre>
<blockquote type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">Do you want this backported to 4.0 stable?
</pre>
</blockquote>
<pre wrap="">
Yes, I want this to be be backported to 4.0 stable.
Thanks,
Neelesh.
</pre>
<blockquote type="cite">
<pre wrap="">-corey
On 07/16/2015 06:16 AM, Neelesh Gupta wrote:
</pre>
<blockquote type="cite">
<pre wrap="">If the OPAL call to receive the ipmi message fails, then we free up the
smi message and return. But, the driver still holds the reference to
old smi message in the 'cur_msg' which can potentially be accessed later
and freed again leading to kernel oops. To fix it up,
The kernel driver should reset the 'cur_msg' and send reply to the user
in addition to freeing the message.
Signed-off-by: Neelesh Gupta<a class="moz-txt-link-rfc2396E" href="mailto:neelegup@linux.vnet.ibm.com"><neelegup@linux.vnet.ibm.com></a>
---
drivers/char/ipmi/ipmi_powernv.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_powernv.c
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">b/drivers/char/ipmi/ipmi_powernv.c
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">index 9b409c0..637486d 100644
--- a/drivers/char/ipmi/ipmi_powernv.c
+++ b/drivers/char/ipmi/ipmi_powernv.c
@@ -143,9 +143,16 @@ static int ipmi_powernv_recv(struct
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">ipmi_smi_powernv *smi)
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap=""> pr_devel("%s: -> %d (size %lld)\n", __func__,
rc, rc == 0 ? size : 0);
if (rc) {
- spin_unlock_irqrestore(&smi->msg_lock, flags);
- ipmi_free_smi_msg(msg);
- return 0;
+ /* If came via the poll, and response was not yet ready */
+ if (rc == OPAL_EMPTY) {
+ spin_unlock_irqrestore(&smi->msg_lock, flags);
+ return 0;
+ } else {
+ smi->cur_msg = NULL;
+ spin_unlock_irqrestore(&smi->msg_lock, flags);
+ send_error_reply(smi, msg, IPMI_ERR_UNSPECIFIED);
+ return 0;
+ }
}
if (size < sizeof(*opal_msg)) {
</pre>
</blockquote>
</blockquote>
<pre wrap="">
_______________________________________________
Linuxppc-dev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Linuxppc-dev@lists.ozlabs.org">Linuxppc-dev@lists.ozlabs.org</a>
<a class="moz-txt-link-freetext" href="https://lists.ozlabs.org/listinfo/linuxppc-dev">https://lists.ozlabs.org/listinfo/linuxppc-dev</a>
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<br>
</body>
</html>