Re: [PATCH] powerpc/opal: add warning for unimplemented OPAL message type

Re: [PATCH] powerpc/opal: add warning for unimplemented OPAL message type

[蒲文]

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

Hi Michael,

On Wednesday, December 09, 2015 11:17:12 AM Michael Ellerman wrote:
> Hi Wen,
> 
> Thanks for the patch.
> 
> On Tue, 2015-12-08 at 15:30 +0800, wen.pu@powercore.com.cn wrote:
> > I found that there are unimplemented OPAL message types "OPAL_MSG_EPOW"
> > and
> > "OPAL_MSG_DPO" in PowerNV Platform. If skiboot send such messages to
> > kernel, nothing will happen(notifier_call_chain() will silently drop it).
> > So I add a warning to inform what has happened, is it necessary?
> 
> What kernel version are you using?
The kernel version is v4.2.1-openpower2, from the newest version at 
https://github.com/open-power/linux.

> 
> Looking at mainline (4.4-rc4), I do see handlers registered for EPOW and
> DPO:
> 
>  
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/p
> owerpc/platforms/powernv/opal-power.c?#n82
> 
> That code was merged in 4.3.
I do find that EPOW and DPO are registered in kernel 4.3. So maybe in the near 
future it will be merged into open-power/linux.

> 
> 
> In general I don't think we want to warn for unhandled message types,
> because we support old kernels running on new skiboot. In that case if we
> warned on every unhandled message we'd potentially spam the console with
> warnings.
> 
> But perhaps we can come up with something smarter, that only warns once for
> each unhandled message type, or something like that? Did you have a bug that
> would have been easier to debug with a warning here?
Yes, I think it is a good idea that only warns once, or warns every a few 
seconds. 
I found that EPOW and DPO only used in IBM FSP platform, not in Habanero 
platform that I had. So I modified the skiboot code and sent a EPOW/DPO message 
to kernel, and found that kernel warn nothing. I have no bug with a warning 
here, and just get a log message via dmesg.

> 
> cheers

Thanks!
Wen Pu

[-- Attachment #2: Type: text/html, Size: 2151 bytes --]

Re: [PATCH] powerpc/opal: add warning for unimplemented OPAL message type

[Stewart Smith]

=E8=92=B2=E6=96=87 <wen.pu@powercore.com.cn> writes:
> On Wednesday, December 09, 2015 11:17:12 AM Michael Ellerman wrote:
>> On Tue, 2015-12-08 at 15:30 +0800, wen.pu@powercore.com.cn wrote:
>> > I found that there are unimplemented OPAL message types "OPAL_MSG_EPOW"
>> > and
>> > "OPAL_MSG_DPO" in PowerNV Platform. If skiboot send such messages to
>> > kernel, nothing will happen(notifier_call_chain() will silently drop i=
t).
>> > So I add a warning to inform what has happened, is it necessary?
>>=20
>> What kernel version are you using?
> The kernel version is v4.2.1-openpower2, from the newest version at=20
> https://github.com/open-power/linux.
>
>>=20
>> Looking at mainline (4.4-rc4), I do see handlers registered for EPOW and
>> DPO:
>>=20
>>=20=20
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arc=
h/p
>> owerpc/platforms/powernv/opal-power.c?#n82
>>=20
>> That code was merged in 4.3.
> I do find that EPOW and DPO are registered in kernel 4.3. So maybe in the=
 near=20
> future it will be merged into open-power/linux.

Way back in 24366360035a9e0a9870ed7208aa2ba1948f844d where the opal_msg
code was introduced there was a warning added for opal messages that
were unknown... arguably this is actually a bad thing to have these
days... I'll submit a patch to print it *once*.

>> In general I don't think we want to warn for unhandled message types,
>> because we support old kernels running on new skiboot. In that case if we
>> warned on every unhandled message we'd potentially spam the console with
>> warnings.
>>=20
>> But perhaps we can come up with something smarter, that only warns once =
for
>> each unhandled message type, or something like that? Did you have a bug =
that
>> would have been easier to debug with a warning here?
> Yes, I think it is a good idea that only warns once, or warns every a few=
=20
> seconds.=20
> I found that EPOW and DPO only used in IBM FSP platform, not in Habanero=
=20
> platform that I had. So I modified the skiboot code and sent a EPOW/DPO m=
essage=20
> to kernel, and found that kernel warn nothing. I have no bug with a warni=
ng=20
> here, and just get a log message via dmesg.

Currently only FSP platform, yes. No real reason why a BMC based
platform couldn't emit those though.

Re: [PATCH] powerpc/opal: add warning for unimplemented OPAL message type

[Wen Pu]

Hi Michael,

On Wednesday, December 09, 2015 11:17:12 AM Michael Ellerman wrote:
> Hi Wen,
> 
> Thanks for the patch.
> 
> On Tue, 2015-12-08 at 15:30 +0800, wen.pu@powercore.com.cn wrote:
> > I found that there are unimplemented OPAL message types "OPAL_MSG_EPOW"
> > and
> > "OPAL_MSG_DPO" in PowerNV Platform. If skiboot send such messages to
> > kernel, nothing will happen(notifier_call_chain() will silently drop it).
> > So I add a warning to inform what has happened, is it necessary?
> 
> What kernel version are you using?
The kernel version is v4.2.1-openpower2, from the newest version at 
https://github.com/open-power/linux.

> 
> Looking at mainline (4.4-rc4), I do see handlers registered for EPOW and
> DPO:
> 
>  
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/p
> owerpc/platforms/powernv/opal-power.c?#n82
> 
> That code was merged in 4.3.
I do find that EPOW and DPO are registered in kernel 4.3. So maybe in the near 
future it will be merged into open-power/linux.

> 
> 
> In general I don't think we want to warn for unhandled message types,
> because we support old kernels running on new skiboot. In that case if we
> warned on every unhandled message we'd potentially spam the console with
> warnings.
> 
> But perhaps we can come up with something smarter, that only warns once for
> each unhandled message type, or something like that? Did you have a bug that
> would have been easier to debug with a warning here?
Yes, I think it is a good idea that only warns once, or warns every a few 
seconds. 
I found that EPOW and DPO only used in IBM FSP platform, not in Habanero 
platform that I had. So I modified the skiboot code and sent a EPOW/DPO message 
to kernel, and found that kernel warn nothing. I have no bug with a warning 
here, and just get a log message via dmesg.

> 
> cheers

Thanks!
Wen Pu

Re: [PATCH] powerpc/opal: add warning for unimplemented OPAL message type

[Michael Ellerman]

Hi Wen,

Thanks for the patch.

On Tue, 2015-12-08 at 15:30 +0800, wen.pu@powercore.com.cn wrote:
> I found that there are unimplemented OPAL message types "OPAL_MSG_EPOW" and 
> "OPAL_MSG_DPO" in PowerNV Platform. If skiboot send such messages to kernel, 
> nothing will happen(notifier_call_chain() will silently drop it). So I add a warning 
> to inform what has happened, is it necessary? 

What kernel version are you using?

Looking at mainline (4.4-rc4), I do see handlers registered for EPOW and DPO:

  https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/platforms/powernv/opal-power.c?#n82

That code was merged in 4.3.


In general I don't think we want to warn for unhandled message types, because
we support old kernels running on new skiboot. In that case if we warned on
every unhandled message we'd potentially spam the console with warnings.

But perhaps we can come up with something smarter, that only warns once for
each unhandled message type, or something like that? Did you have a bug that
would have been easier to debug with a warning here?

cheers

[PATCH] powerpc/opal: add warning for unimplemented OPAL message type

[wen.pu]

[-- Attachment #1: Type: text/plain, Size: 919 bytes --]

I found that there are unimplemented OPAL message types "OPAL_MSG_EPOW" and 
"OPAL_MSG_DPO" in PowerNV Platform. If skiboot send such messages to kernel, 
nothing will happen(notifier_call_chain() will silently drop it). So I add a warning 
to inform what has happened, is it necessary? 

Signed-off-by: Wen Pu <wen.pu@powercore.com.cn>
---
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index f084afa..ef498ab 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -281,6 +281,10 @@ static void opal_handle_message(void)
  pr_warning("%s: Unknown message type: %u\n", __func__, type);
  return;
  }
+ if(type == OPAL_MSG_EPOW || type == OPAL_MSG_DPO) {
+ pr_warning("%s: Unimplemented message type: %u\n", __func__, type);
+ return;
+ }
  opal_message_do_notify(type, (void *)&msg);
 }
---

Thanks!
Wen Pu

[-- Attachment #2: Type: text/html, Size: 3212 bytes --]