[PATCH] MIPS: replace add and sub instructions in relocate_kernel.S with addiu

[PATCH] MIPS: replace add and sub instructions in relocate_kernel.S with addiu

[James Cowgill]

Fixes the assembler errors generated when compiling a MIPS R6 kernel with
CONFIG_KEXEC on, by replacing the offending add and sub instructions with
addiu instructions.

Build errors:
arch/mips/kernel/relocate_kernel.S: Assembler messages:
arch/mips/kernel/relocate_kernel.S:27: Error: invalid operands `dadd $16,$16,8'
arch/mips/kernel/relocate_kernel.S:64: Error: invalid operands `dadd $20,$20,8'
arch/mips/kernel/relocate_kernel.S:65: Error: invalid operands `dadd $18,$18,8'
arch/mips/kernel/relocate_kernel.S:66: Error: invalid operands `dsub $22,$22,1'
scripts/Makefile.build:294: recipe for target 'arch/mips/kernel/relocate_kernel.o' failed

Signed-off-by: James Cowgill <James.Cowgill@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: <stable@vger.kernel.org> # 4.0+
---
 arch/mips/kernel/relocate_kernel.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/kernel/relocate_kernel.S b/arch/mips/kernel/relocate_kernel.S
index 74bab9d..c6bbf21 100644
--- a/arch/mips/kernel/relocate_kernel.S
+++ b/arch/mips/kernel/relocate_kernel.S
@@ -24,7 +24,7 @@ LEAF(relocate_new_kernel)
 
 process_entry:
 	PTR_L		s2, (s0)
-	PTR_ADD		s0, s0, SZREG
+	PTR_ADDIU	s0, s0, SZREG
 
 	/*
 	 * In case of a kdump/crash kernel, the indirection page is not
@@ -61,9 +61,9 @@ copy_word:
 	/* copy page word by word */
 	REG_L		s5, (s2)
 	REG_S		s5, (s4)
-	PTR_ADD		s4, s4, SZREG
-	PTR_ADD		s2, s2, SZREG
-	LONG_SUB	s6, s6, 1
+	PTR_ADDIU	s4, s4, SZREG
+	PTR_ADDIU	s2, s2, SZREG
+	LONG_ADDIU	s6, s6, -1
 	beq		s6, zero, process_entry
 	b		copy_word
 	b		process_entry
-- 
2.1.4

[PATCH] MIPS: replace add and sub instructions in relocate_kernel.S with addiu

[James Cowgill]

Fixes the assembler errors generated when compiling a MIPS R6 kernel with
CONFIG_KEXEC on, by replacing the offending add and sub instructions with
addiu instructions.

Build errors:
arch/mips/kernel/relocate_kernel.S: Assembler messages:
arch/mips/kernel/relocate_kernel.S:27: Error: invalid operands `dadd $16,$16,8'
arch/mips/kernel/relocate_kernel.S:64: Error: invalid operands `dadd $20,$20,8'
arch/mips/kernel/relocate_kernel.S:65: Error: invalid operands `dadd $18,$18,8'
arch/mips/kernel/relocate_kernel.S:66: Error: invalid operands `dsub $22,$22,1'
scripts/Makefile.build:294: recipe for target 'arch/mips/kernel/relocate_kernel.o' failed

Signed-off-by: James Cowgill <James.Cowgill@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: <stable@vger.kernel.org> # 4.0+
---
 arch/mips/kernel/relocate_kernel.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/kernel/relocate_kernel.S b/arch/mips/kernel/relocate_kernel.S
index 74bab9d..c6bbf21 100644
--- a/arch/mips/kernel/relocate_kernel.S
+++ b/arch/mips/kernel/relocate_kernel.S
@@ -24,7 +24,7 @@ LEAF(relocate_new_kernel)
 
 process_entry:
 	PTR_L		s2, (s0)
-	PTR_ADD		s0, s0, SZREG
+	PTR_ADDIU	s0, s0, SZREG
 
 	/*
 	 * In case of a kdump/crash kernel, the indirection page is not
@@ -61,9 +61,9 @@ copy_word:
 	/* copy page word by word */
 	REG_L		s5, (s2)
 	REG_S		s5, (s4)
-	PTR_ADD		s4, s4, SZREG
-	PTR_ADD		s2, s2, SZREG
-	LONG_SUB	s6, s6, 1
+	PTR_ADDIU	s4, s4, SZREG
+	PTR_ADDIU	s2, s2, SZREG
+	LONG_ADDIU	s6, s6, -1
 	beq		s6, zero, process_entry
 	b		copy_word
 	b		process_entry
-- 
2.1.4

Re: [PATCH] MIPS: replace add and sub instructions in relocate_kernel.S with addiu

[Ralf Baechle]

On Wed, Jun 17, 2015 at 05:12:50PM +0100, James Cowgill wrote:

> Fixes the assembler errors generated when compiling a MIPS R6 kernel with
> CONFIG_KEXEC on, by replacing the offending add and sub instructions with
> addiu instructions.
> 
> Build errors:
> arch/mips/kernel/relocate_kernel.S: Assembler messages:
> arch/mips/kernel/relocate_kernel.S:27: Error: invalid operands `dadd $16,$16,8'
> arch/mips/kernel/relocate_kernel.S:64: Error: invalid operands `dadd $20,$20,8'
> arch/mips/kernel/relocate_kernel.S:65: Error: invalid operands `dadd $18,$18,8'
> arch/mips/kernel/relocate_kernel.S:66: Error: invalid operands `dsub $22,$22,1'
> scripts/Makefile.build:294: recipe for target 'arch/mips/kernel/relocate_kernel.o' failed
> 
> Signed-off-by: James Cowgill <James.Cowgill@imgtec.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: <stable@vger.kernel.org> # 4.0+
> ---
>  arch/mips/kernel/relocate_kernel.S | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/mips/kernel/relocate_kernel.S b/arch/mips/kernel/relocate_kernel.S
> index 74bab9d..c6bbf21 100644
> --- a/arch/mips/kernel/relocate_kernel.S
> +++ b/arch/mips/kernel/relocate_kernel.S
> @@ -24,7 +24,7 @@ LEAF(relocate_new_kernel)
>  
>  process_entry:
>  	PTR_L		s2, (s0)
> -	PTR_ADD		s0, s0, SZREG
> +	PTR_ADDIU	s0, s0, SZREG
>  
>  	/*
>  	 * In case of a kdump/crash kernel, the indirection page is not
> @@ -61,9 +61,9 @@ copy_word:
>  	/* copy page word by word */
>  	REG_L		s5, (s2)
>  	REG_S		s5, (s4)
> -	PTR_ADD		s4, s4, SZREG
> -	PTR_ADD		s2, s2, SZREG
> -	LONG_SUB	s6, s6, 1
> +	PTR_ADDIU	s4, s4, SZREG
> +	PTR_ADDIU	s2, s2, SZREG
> +	LONG_ADDIU	s6, s6, -1

Thanks, applied.

But I was wondering if maybe we should redefine the PTR_ADD, LONG_SUB etc
macros to expand into a signed operation for R6.  While I can't convince
myself it's the right and conceptually clean thing to do, I don't think
it'd be clearly wrong and it might help preventing numersous bugs by
applications that use <asm/asm.h>.  Opinions?

  Ralf

Re: [PATCH] MIPS: replace add and sub instructions in relocate_kernel.S with addiu

[Maciej W. Rozycki]

On Mon, 3 Aug 2015, Ralf Baechle wrote:

> > Fixes the assembler errors generated when compiling a MIPS R6 kernel with
> > CONFIG_KEXEC on, by replacing the offending add and sub instructions with
> > addiu instructions.
> > 
> > Build errors:
> > arch/mips/kernel/relocate_kernel.S: Assembler messages:
> > arch/mips/kernel/relocate_kernel.S:27: Error: invalid operands `dadd $16,$16,8'
> > arch/mips/kernel/relocate_kernel.S:64: Error: invalid operands `dadd $20,$20,8'
> > arch/mips/kernel/relocate_kernel.S:65: Error: invalid operands `dadd $18,$18,8'
> > arch/mips/kernel/relocate_kernel.S:66: Error: invalid operands `dsub $22,$22,1'
> > scripts/Makefile.build:294: recipe for target 'arch/mips/kernel/relocate_kernel.o' failed
> > 
> > Signed-off-by: James Cowgill <James.Cowgill@imgtec.com>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: <stable@vger.kernel.org> # 4.0+
> > ---
> >  arch/mips/kernel/relocate_kernel.S | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/mips/kernel/relocate_kernel.S b/arch/mips/kernel/relocate_kernel.S
> > index 74bab9d..c6bbf21 100644
> > --- a/arch/mips/kernel/relocate_kernel.S
> > +++ b/arch/mips/kernel/relocate_kernel.S
> > @@ -24,7 +24,7 @@ LEAF(relocate_new_kernel)
> >  
> >  process_entry:
> >  	PTR_L		s2, (s0)
> > -	PTR_ADD		s0, s0, SZREG
> > +	PTR_ADDIU	s0, s0, SZREG
> >  
> >  	/*
> >  	 * In case of a kdump/crash kernel, the indirection page is not
> > @@ -61,9 +61,9 @@ copy_word:
> >  	/* copy page word by word */
> >  	REG_L		s5, (s2)
> >  	REG_S		s5, (s4)
> > -	PTR_ADD		s4, s4, SZREG
> > -	PTR_ADD		s2, s2, SZREG
> > -	LONG_SUB	s6, s6, 1
> > +	PTR_ADDIU	s4, s4, SZREG
> > +	PTR_ADDIU	s2, s2, SZREG
> > +	LONG_ADDIU	s6, s6, -1
> 
> Thanks, applied.
> 
> But I was wondering if maybe we should redefine the PTR_ADD, LONG_SUB etc
> macros to expand into a signed operation for R6.  While I can't convince
> myself it's the right and conceptually clean thing to do, I don't think
> it'd be clearly wrong and it might help preventing numersous bugs by
> applications that use <asm/asm.h>.  Opinions?

 It looks to me like missing assembly language macro implementations.  For 
R6:

	dadd	$16, $16, 8

should merely expand to a sequence of machine instructions like this:

	addiu	$1, $0, 8
	dadd	$16, $16, $1

which for older ISA revisions would happen anyway if the third operand was 
immediate and fell outside the signed 16-bit range.  Here the immediate 
range simply got shrunk to 0 bits with the transition to R6.

 I know some people disagree, but I maintain my point of view, which is 
consistent with how the MIPS assembly language has always been defined. 
That helps with assembly language's backward compatibility at the source 
level too, just as previously observed with the transition to the 
microMIPS instruction set, where some immediate ranges were shrunk to 12 
or 10 bits.

 These macros have also been a part of the user API (defined in 
<sys/asm.h>), which is another argument in favour to fixing the assembler.

  Maciej