[PATCH] nx-842: Ignore bit 3 of condition register returned by icswx

[PATCH] nx-842: Ignore bit 3 of condition register returned by icswx

[Ram Pai]

icswx occasionally under heavy load sets bit 3 of condition register 0.
It has no software implication.

Currently that bit is interpreted by the driver as a failure, when
it should have calmly ignored it.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/icswx.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/include/asm/icswx.h
index 9f8402b..bce20c7 100644
--- a/arch/powerpc/include/asm/icswx.h
+++ b/arch/powerpc/include/asm/icswx.h
@@ -177,7 +177,7 @@ static inline int icswx(__be32 ccw, struct coprocessor_request_block *crb)
 	: "r" (ccw_reg), "r" (crb)
 	: "cr0", "memory");
 
-	return (int)((cr >> 28) & 0xf);
+	return (int)((cr >> 28) & 0xe);
 }

Re: [PATCH] nx-842: Ignore bit 3 of condition register returned by icswx

[Michael Ellerman]

On Fri, 2015-10-30 at 15:31 -0700, Ram Pai wrote:
> icswx occasionally under heavy load sets bit 3 of condition register 0.

Why?

Also you seem to be using IBM bit numbering, so please be explicit about that,
or use normal bit numbering.

> It has no software implication.

What does it mean? You might be right but you don't give me enough info to
decide.

> Currently that bit is interpreted by the driver as a failure, when
> it should have calmly ignored it.

Should the fix be in icswx or the driver? Please justify your choice.

This sounds like it's fixing a bug so shouldn't the patch go to stable? And if
so which version(s) should it apply to?

cheers

Re: [PATCH] nx-842: Ignore bit 3 of condition register returned by icswx

[Ram Pai]

On Mon, Nov 02, 2015 at 12:23:36PM +1100, Michael Ellerman wrote:
> On Fri, 2015-10-30 at 15:31 -0700, Ram Pai wrote:
> > icswx occasionally under heavy load sets bit 3 of condition register 0.
> 
> Why?

The hardware manual says that bit is undefined, though it is set under some conditions.

> 
> Also you seem to be using IBM bit numbering, so please be explicit about that,
> or use normal bit numbering.

Ok. 


> 
> > It has no software implication.
> 
> What does it mean? You might be right but you don't give me enough info to
> decide.

The software is not supposed to interpret the bit since its meaning is
undefined. 

> 
> > Currently that bit is interpreted by the driver as a failure, when
> > it should have calmly ignored it.
> 
> Should the fix be in icswx or the driver? Please justify your choice.

Yes there are two solutions. One is icswx macro should not expose that
bit to its consumers.  Or the driver/consumers can ignore that bit.  I
think it makes more sense to contain it in one place, which is icswx
instruction. Drivers or whoever calls icswx should not know
more than what they need to know. This patch uses the first approach.

> 
> This sounds like it's fixing a bug so shouldn't the patch go to stable? And if
> so which version(s) should it apply to?

It should go to stable v4.2.  I will tag it to stable, in my next
version.

RP

Re: [PATCH] nx-842: Ignore bit 3 of condition register returned by icswx

[Michael Ellerman]

On Mon, 2015-11-02 at 10:41 -0800, Ram Pai wrote:
> On Mon, Nov 02, 2015 at 12:23:36PM +1100, Michael Ellerman wrote:
> > On Fri, 2015-10-30 at 15:31 -0700, Ram Pai wrote:
> > > icswx occasionally under heavy load sets bit 3 of condition register 0.
> >
> > Why?
>
> The hardware manual says that bit is undefined, though it is set under some
> conditions.

Which hardware manual?

Last I checked none of this was documented anywhere public. But the out of date
RFC I have does define bit zero to mean something.

> > > Currently that bit is interpreted by the driver as a failure, when
> > > it should have calmly ignored it.
> > 
> > Should the fix be in icswx or the driver? Please justify your choice.
> 
> Yes there are two solutions. One is icswx macro should not expose that
> bit to its consumers.  Or the driver/consumers can ignore that bit.  I
> think it makes more sense to contain it in one place, which is icswx
> instruction. Drivers or whoever calls icswx should not know
> more than what they need to know. This patch uses the first approach.

Yep, I'm fine with doing it in icswx if we're 100% sure the bit is always
undefined.

> > This sounds like it's fixing a bug so shouldn't the patch go to stable? And if
> > so which version(s) should it apply to?
> 
> It should go to stable v4.2.  I will tag it to stable, in my next version.

Thanks.

cheers