[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

[Dan Streetman]

On Fri, Oct 30, 2015 at 6:43 PM, Ram Pai <linuxram@us.ibm.com> wrote:
> 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>

Acked-by: Dan Streetman <ddstreet@ieee.org>

> ---
>  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

[Herbert Xu]

On Fri, Oct 30, 2015 at 03:43:01PM -0700, Ram Pai wrote:
> 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>

Your patch is corrupted:

$ patch -s -p1 < .git/rebase-apply/patch 
patch: **** malformed patch at line 21: To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
$
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Ram Pai]

On Tue, Nov 17, 2015 at 10:00:20PM +0800, Herbert Xu wrote:
> On Fri, Oct 30, 2015 at 03:43:01PM -0700, Ram Pai wrote:
> > 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>
> 
> Your patch is corrupted:
> 
> $ patch -s -p1 < .git/rebase-apply/patch 
> patch: **** malformed patch at line 21: To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> $

Herbert, There were some comments about the patch on the ppc mailing
list.  

There are times this bit could be set and could be used by its
consumers; depending on how the RFC is implemented by the hardware. The
RFC does allow the hardware to assign meaning to that bit; though no
hardware currently exploits that feature. However from a software point
of view, we probably should not ignore the bit entirely. So please
ignore this patch till we come with a better solution.

RP

> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

-- 
Ram Pai

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

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 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

[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);
 }