Re: [PATCH] powerpc/xmon: check before calling xive functions

[Benjamin Herrenschmidt]

On Thu, 2017-10-19 at 15:09 +0200, Cédric Le Goater wrote:
> > No that's wrong. xive_enabled() is only set if Linux is using native
> > xive mode but some of those xmon functions dump the emulated state.
> > 
> > We should fix the actual cause of the crash.
> 
> which should be in the OPAL XIVE dump routines then ? 

Yup, probably needs a NULL check or something...

Cheers,
Ben.

Re: [PATCH] powerpc/xmon: check before calling xive functions

[Cédric Le Goater]

On 10/19/2017 09:10 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2017-10-19 at 00:02 +1100, Michael Ellerman wrote:
>> Breno Leitao  writes:
>>
>>> Currently xmon could call XIVE functions from OPAL even if the XIVE is
>>> disabled or does not exist in the system, as in POWER8 machines.  This
>>> causes the following exception:
>>>
>>>  1:mon> dx
>>>  cpu 0x1: Vector: 700 (Program Check) at [c00423c93450]
>>>  pc: c009cfa4: opal_xive_dump+0x50/0x68
>>>  lr: c00997b8: opal_return+0x0/0x50
>>>
>>> This patch simply checks if XIVE is enabled before calling XIVE
>>> functions.
>>
>> Thanks. I'll merge this.
>>
>> But we should also fix it in skiboot.
> 
> No that's wrong. xive_enabled() is only set if Linux is using native
> xive mode but some of those xmon functions dump the emulated state.
> 
> We should fix the actual cause of the crash.

which should be in the OPAL XIVE dump routines then ? 

Cheers 
C. 

> 
> Cheers,
> Ben.
> 
>> cheers
>>
>>> Suggested-by: Guilherme G. Piccoli 
>>> Signed-off-by: Breno Leitao 
>>> ---
>>>  arch/powerpc/xmon/xmon.c | 6 ++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>> index 4679aeb84767..b34976c4a6ba 100644
>>> --- a/arch/powerpc/xmon/xmon.c
>>> +++ b/arch/powerpc/xmon/xmon.c
>>> @@ -2508,6 +2508,12 @@ static void dump_xives(void)
>>> unsigned long num;
>>> int c;
>>>  
>>> +   if (!xive_enabled()) {
>>> +   printf("Xive disabled on this system\n");
>>> +
>>> +   return;
>>> +   }
>>> +
>>> c = inchar();
>>> if (c == 'a') {
>>> dump_all_xives();
>>> -- 
>>> 2.14.2
> 

Re: [PATCH] powerpc/xmon: check before calling xive functions

[Benjamin Herrenschmidt]

On Thu, 2017-10-19 at 00:02 +1100, Michael Ellerman wrote:
> Breno Leitao  writes:
> 
> > Currently xmon could call XIVE functions from OPAL even if the XIVE is
> > disabled or does not exist in the system, as in POWER8 machines.  This
> > causes the following exception:
> > 
> >  1:mon> dx
> >  cpu 0x1: Vector: 700 (Program Check) at [c00423c93450]
> >  pc: c009cfa4: opal_xive_dump+0x50/0x68
> >  lr: c00997b8: opal_return+0x0/0x50
> > 
> > This patch simply checks if XIVE is enabled before calling XIVE
> > functions.
> 
> Thanks. I'll merge this.
> 
> But we should also fix it in skiboot.

No that's wrong. xive_enabled() is only set if Linux is using native
xive mode but some of those xmon functions dump the emulated state.

We should fix the actual cause of the crash.

Cheers,
Ben.

> cheers
> 
> > Suggested-by: Guilherme G. Piccoli 
> > Signed-off-by: Breno Leitao 
> > ---
> >  arch/powerpc/xmon/xmon.c | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index 4679aeb84767..b34976c4a6ba 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -2508,6 +2508,12 @@ static void dump_xives(void)
> > unsigned long num;
> > int c;
> >  
> > +   if (!xive_enabled()) {
> > +   printf("Xive disabled on this system\n");
> > +
> > +   return;
> > +   }
> > +
> > c = inchar();
> > if (c == 'a') {
> > dump_all_xives();
> > -- 
> > 2.14.2

Re: [PATCH] powerpc/xmon: check before calling xive functions

[Michael Ellerman]

Breno Leitao  writes:

> Currently xmon could call XIVE functions from OPAL even if the XIVE is
> disabled or does not exist in the system, as in POWER8 machines.  This
> causes the following exception:
>
>  1:mon> dx
>  cpu 0x1: Vector: 700 (Program Check) at [c00423c93450]
>  pc: c009cfa4: opal_xive_dump+0x50/0x68
>  lr: c00997b8: opal_return+0x0/0x50
>
> This patch simply checks if XIVE is enabled before calling XIVE
> functions.

Thanks. I'll merge this.

But we should also fix it in skiboot.

cheers

> Suggested-by: Guilherme G. Piccoli 
> Signed-off-by: Breno Leitao 
> ---
>  arch/powerpc/xmon/xmon.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 4679aeb84767..b34976c4a6ba 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -2508,6 +2508,12 @@ static void dump_xives(void)
>   unsigned long num;
>   int c;
>  
> + if (!xive_enabled()) {
> + printf("Xive disabled on this system\n");
> +
> + return;
> + }
> +
>   c = inchar();
>   if (c == 'a') {
>   dump_all_xives();
> -- 
> 2.14.2

[PATCH] powerpc/xmon: check before calling xive functions

[Breno Leitao]

Currently xmon could call XIVE functions from OPAL even if the XIVE is
disabled or does not exist in the system, as in POWER8 machines.  This
causes the following exception:

 1:mon> dx
 cpu 0x1: Vector: 700 (Program Check) at [c00423c93450]
 pc: c009cfa4: opal_xive_dump+0x50/0x68
 lr: c00997b8: opal_return+0x0/0x50

This patch simply checks if XIVE is enabled before calling XIVE
functions.

Suggested-by: Guilherme G. Piccoli 
Signed-off-by: Breno Leitao 
---
 arch/powerpc/xmon/xmon.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 4679aeb84767..b34976c4a6ba 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2508,6 +2508,12 @@ static void dump_xives(void)
unsigned long num;
int c;
 
+   if (!xive_enabled()) {
+   printf("Xive disabled on this system\n");
+
+   return;
+   }
+
c = inchar();
if (c == 'a') {
dump_all_xives();
-- 
2.14.2