bug in arch/ppc/boot/common/util.S: cmplwi cr0,r3,r4 ?

bug in arch/ppc/boot/common/util.S: cmplwi cr0,r3,r4 ?

[Frank van Maarseveen]

I think "cmplwi" expects an immediate vale as last operand
around line 255 of arch/ppc/boot/common/util.S:

	addi	r4,r4,_etext@l		# r8 = &_etext
1:	dcbf	r0,r3			# Flush the data cache
	icbi	r0,r3			# Invalidate the instruction cache
	addi	r3,r3,0x10		# Increment by one cache line
	cmplwi	cr0,r3,r4		# Are we at the end yet?
	               ^^
	blt	1b			# No, keep flushing and invalidating

I guess it should have been:

	cmplw	cr0,r3,r4		# Are we at the end yet?

-- 
Frank

Re: bug in arch/ppc/boot/common/util.S: cmplwi cr0,r3,r4 ?

[Benjamin Herrenschmidt]

On Wed, 2005-08-03 at 15:18 +0200, Frank van Maarseveen wrote:
> I think "cmplwi" expects an immediate vale as last operand
> around line 255 of arch/ppc/boot/common/util.S:
> 
> 	addi	r4,r4,_etext@l		# r8 = &_etext
> 1:	dcbf	r0,r3			# Flush the data cache
> 	icbi	r0,r3			# Invalidate the instruction cache
> 	addi	r3,r3,0x10		# Increment by one cache line
> 	cmplwi	cr0,r3,r4		# Are we at the end yet?
> 	               ^^
> 	blt	1b			# No, keep flushing and invalidating
> 
> I guess it should have been:
> 
> 	cmplw	cr0,r3,r4		# Are we at the end yet?

Yup, looks like a real bug to me, Tom ?

Ben.

Re: bug in arch/ppc/boot/common/util.S: cmplwi cr0,r3,r4 ?

[Tom Rini]

On Thu, Aug 04, 2005 at 11:44:08AM +0200, Benjamin Herrenschmidt wrote:
> On Wed, 2005-08-03 at 15:18 +0200, Frank van Maarseveen wrote:
> > I think "cmplwi" expects an immediate vale as last operand
> > around line 255 of arch/ppc/boot/common/util.S:
> > 
> > 	addi	r4,r4,_etext@l		# r8 = &_etext
> > 1:	dcbf	r0,r3			# Flush the data cache
> > 	icbi	r0,r3			# Invalidate the instruction cache
> > 	addi	r3,r3,0x10		# Increment by one cache line
> > 	cmplwi	cr0,r3,r4		# Are we at the end yet?
> > 	               ^^
> > 	blt	1b			# No, keep flushing and invalidating
> > 
> > I guess it should have been:
> > 
> > 	cmplw	cr0,r3,r4		# Are we at the end yet?
> 
> Yup, looks like a real bug to me, Tom ?

Sounds correct to me.  I wonder why the assembler hasn't barfed, or is
is translating the ascii values of r4.  I'll pass this along once 2.6.14
opens just because the code has been that way for so many years, I don't
think it's a critical bug.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: bug in arch/ppc/boot/common/util.S: cmplwi cr0,r3,r4 ?

[Hollis Blanchard]

On Aug 8, 2005, at 7:50 PM, Tom Rini wrote:

> On Thu, Aug 04, 2005 at 11:44:08AM +0200, Benjamin Herrenschmidt wrote:
>> On Wed, 2005-08-03 at 15:18 +0200, Frank van Maarseveen wrote:
>>> I think "cmplwi" expects an immediate vale as last operand
>>> around line 255 of arch/ppc/boot/common/util.S:
>>>
>>> 	addi	r4,r4,_etext@l		# r8 = &_etext
>>> 1:	dcbf	r0,r3			# Flush the data cache
>>> 	icbi	r0,r3			# Invalidate the instruction cache
>>> 	addi	r3,r3,0x10		# Increment by one cache line
>>> 	cmplwi	cr0,r3,r4		# Are we at the end yet?
>>> 	               ^^
>>> 	blt	1b			# No, keep flushing and invalidating
>>>
>>> I guess it should have been:
>>>
>>> 	cmplw	cr0,r3,r4		# Are we at the end yet?
>>
>> Yup, looks like a real bug to me, Tom ?
>
> Sounds correct to me.  I wonder why the assembler hasn't barfed, or is
> is translating the ascii values of r4.

include/asm-ppc/ppc_asm.h:
#define r4      4

Unfortunately -mregnames is quite stupid, so that wouldn't have helped 
us here either.

-Hollis

Re: bug in arch/ppc/boot/common/util.S: cmplwi cr0,r3,r4 ?

[Benjamin Herrenschmidt]

On Mon, 2005-08-08 at 17:50 -0700, Tom Rini wrote:
> On Thu, Aug 04, 2005 at 11:44:08AM +0200, Benjamin Herrenschmidt wrote:
> > On Wed, 2005-08-03 at 15:18 +0200, Frank van Maarseveen wrote:
> > > I think "cmplwi" expects an immediate vale as last operand
> > > around line 255 of arch/ppc/boot/common/util.S:
> > > 
> > > 	addi	r4,r4,_etext@l		# r8 = &_etext
> > > 1:	dcbf	r0,r3			# Flush the data cache
> > > 	icbi	r0,r3			# Invalidate the instruction cache
> > > 	addi	r3,r3,0x10		# Increment by one cache line
> > > 	cmplwi	cr0,r3,r4		# Are we at the end yet?
> > > 	               ^^
> > > 	blt	1b			# No, keep flushing and invalidating
> > > 
> > > I guess it should have been:
> > > 
> > > 	cmplw	cr0,r3,r4		# Are we at the end yet?
> > 
> > Yup, looks like a real bug to me, Tom ?
> 
> Sounds correct to me.  I wonder why the assembler hasn't barfed, or is
> is translating the ascii values of r4.  I'll pass this along once 2.6.14
> opens just because the code has been that way for so many years, I don't
> think it's a critical bug.
> 

Because our assembler has no clue what r4 is :) Look at our ppc_asm.h
and you'll understand :)

Ben.