diff mbox

powerpc/powernv: Fix the hmi event version check.

Message ID 20141120041345.3339.6873.stgit@mars.in.ibm.com (mailing list archive)
State Accepted
Commit 6acbc5a1dab30aa8f7be7bf3852f343f89147ac5
Delegated to: Michael Ellerman
Headers show

Commit Message

Mahesh J Salgaonkar Nov. 20, 2014, 4:14 a.m. UTC
From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

The current HMI event structure is an ABI and carries a version field to
accommodate future changes without affecting/rearranging current structure
members that are valid for previous versions. The current version check
"if (hmi_evt->version != OpalHMIEvt_V1)" seems to consider that version
will always be V1 which may not be true in future. If we start supporting
HMI event > V1, this check would fail without printing anything on older
kernels. This patch fixes this issue.

Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/opal-hmi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Ellerman Nov. 26, 2014, 3:44 a.m. UTC | #1
On Thu, 2014-20-11 at 04:14:36 UTC, Mahesh Salgaonkar wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> 
> The current HMI event structure is an ABI and carries a version field to
> accommodate future changes without affecting/rearranging current structure
> members that are valid for previous versions. The current version check
> "if (hmi_evt->version != OpalHMIEvt_V1)" seems to consider that version
> will always be V1 which may not be true in future. If we start supporting
> HMI event > V1, this check would fail without printing anything on older
> kernels. This patch fixes this issue.

It's not clear what you mean when you say "this check would fail without
printing anything". The check will fail, and it will print something, ie. the
error message.

What you mean is "the check will fail, and the HMI info will not be printed".

I'll CC this to stable unless you disagree.

cheers
Mahesh J Salgaonkar Nov. 26, 2014, 10:26 a.m. UTC | #2
On 11/26/2014 09:14 AM, Michael Ellerman wrote:
> On Thu, 2014-20-11 at 04:14:36 UTC, Mahesh Salgaonkar wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> The current HMI event structure is an ABI and carries a version field to
>> accommodate future changes without affecting/rearranging current structure
>> members that are valid for previous versions. The current version check
>> "if (hmi_evt->version != OpalHMIEvt_V1)" seems to consider that version
>> will always be V1 which may not be true in future. If we start supporting
>> HMI event > V1, this check would fail without printing anything on older
>> kernels. This patch fixes this issue.
> 
> It's not clear what you mean when you say "this check would fail without
> printing anything". The check will fail, and it will print something, ie. the
> error message.
> 
> What you mean is "the check will fail, and the HMI info will not be printed".

My Bad, Yes. I meant 'HMI info will not be printed'. Do you want me to
re spin the patch with correction.

> 
> I'll CC this to stable unless you disagree.

Yes. This patch needs to go to stable.

Thanks,
-Mahesh.
Michael Ellerman Nov. 26, 2014, 10:33 p.m. UTC | #3
On Wed, 2014-11-26 at 15:56 +0530, Mahesh Jagannath Salgaonkar wrote:
> On 11/26/2014 09:14 AM, Michael Ellerman wrote:
> > On Thu, 2014-20-11 at 04:14:36 UTC, Mahesh Salgaonkar wrote:
> >> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> >>
> >> The current HMI event structure is an ABI and carries a version field to
> >> accommodate future changes without affecting/rearranging current structure
> >> members that are valid for previous versions. The current version check
> >> "if (hmi_evt->version != OpalHMIEvt_V1)" seems to consider that version
> >> will always be V1 which may not be true in future. If we start supporting
> >> HMI event > V1, this check would fail without printing anything on older
> >> kernels. This patch fixes this issue.
> > 
> > It's not clear what you mean when you say "this check would fail without
> > printing anything". The check will fail, and it will print something, ie. the
> > error message.
> > 
> > What you mean is "the check will fail, and the HMI info will not be printed".
> 
> My Bad, Yes. I meant 'HMI info will not be printed'. Do you want me to
> re spin the patch with correction.

No that's fine, I've fixed it up to be:

    The current HMI event structure is an ABI and carries a version field to
    accommodate future changes without affecting/rearranging current structure
    members that are valid for previous versions.
    
    The current version check "if (hmi_evt->version != OpalHMIEvt_V1)"
    doesn't accomodate the fact that the version number may change in
    future.
    
    If firmware starts returning an HMI event with version > 1, this check
    will fail and no HMI information will be printed on older kernels.
    
    This patch fixes this issue.


> > I'll CC this to stable unless you disagree.
> 
> Yes. This patch needs to go to stable.

Yep I've added Cc stable to the patch.

cheers
diff mbox

Patch

diff --git a/arch/powerpc/platforms/powernv/opal-hmi.c b/arch/powerpc/platforms/powernv/opal-hmi.c
index 5e1ed15..b322bfb 100644
--- a/arch/powerpc/platforms/powernv/opal-hmi.c
+++ b/arch/powerpc/platforms/powernv/opal-hmi.c
@@ -57,7 +57,7 @@  static void print_hmi_event_info(struct OpalHMIEvent *hmi_evt)
 	};
 
 	/* Print things out */
-	if (hmi_evt->version != OpalHMIEvt_V1) {
+	if (hmi_evt->version < OpalHMIEvt_V1) {
 		pr_err("HMI Interrupt, Unknown event version %d !\n",
 			hmi_evt->version);
 		return;