| Message ID | 20151030223120.GA22550@ram.oc3035372033.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
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
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
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
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); } _______________________________________________
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(-) Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev