diff mbox

[U-Boot,RFC,v1] powerpc: Fix off-by-one error in relocation

Message ID 1410222124-1372-1-git-send-email-judge.packham@gmail.com
State Superseded
Headers show

Commit Message

Chris Packham Sept. 9, 2014, 12:22 a.m. UTC 
When the got2 pointers were adjusted after relocation the last entry was
missed. For most builds was something innocuous and there was no
observable problem but at least for the P2041RDB board this ended up
being NetArpWaitTimerStart which caused the ARP packets to timeout
immediately.

On Mon, Sep 8, 2014 at 9:04 PM,
Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
>
> The relevant code that uses
> __got2_entries is in start.S:
>
> in_ram:
>
>         /*
>          * Relocation Function, r12 point to got2+0x8000
>          *
>          * Adjust got2 pointers, no need to check for 0, this code
>          * already puts a few entries in the table.
>          */
>         li      r0,__got2_entries@sectoff@l
>         la      r3,GOT(_GOT2_TABLE_)
>         lwz     r11,GOT(_GOT2_TABLE_)
>         mtctr   r0
>         sub     r11,r3,r11
>         addi    r3,r3,-4
> 1:      lwzu    r0,4(r3)
>         cmpwi   r0,0
>         beq-    2f
>         add     r0,r0,r11
>         stw     r0,0(r3)
> 2:      bdnz    1b
>
> bdnz does decrement then test for zero so __got2_entries should hold the
> number of entries to relocate.

But __got2_entries is 1 less than it should be. Fix this by removing the
-1 from the __got2_entries calculation.

Cc: Joakim Tjernlund <joakim.tjernlund@transmode.se>
Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
So here's a patch that addresses the issue. I've tested it on a P2041RDB
(mpc85xx/QorIq). I am aware that I'm touching a lot of platforms so I
need some help testing.

I've deliberately left off the following board specific files which at a
glance have the same off-by-one error:

  board/LEOX/elpt860/u-boot.lds
  board/RRvision/u-boot.lds
  board/cogent/u-boot.lds
  board/eltec/mhpc/u-boot.lds
  board/emk/top860/u-boot.lds
  board/esteem192e/u-boot.lds
  board/evb64260/u-boot.lds
  board/hermes/u-boot.lds
  board/hymod/u-boot.lds
  board/icu862/u-boot.lds
  board/ip860/u-boot.lds
  board/ivm/u-boot.lds
  board/kup/kup4k/u-boot.lds
  board/kup/kup4x/u-boot.lds
  board/lwmon/u-boot.lds
  board/manroland/uc100/u-boot.lds
  board/matrix_vision/mvsmr/u-boot.lds
  board/mvblue/u-boot.lds
  board/netvia/u-boot.lds
  board/r360mpi/u-boot.lds
  board/sandpoint/u-boot.lds
  board/spd8xx/u-boot.lds
  board/tqc/tqm8xx/u-boot.lds

Thanks,
Chris

 arch/powerpc/cpu/74xx_7xx/u-boot.lds         |    2 +-
 arch/powerpc/cpu/mpc512x/u-boot.lds          |    2 +-
 arch/powerpc/cpu/mpc5xx/u-boot.lds           |    2 +-
 arch/powerpc/cpu/mpc5xxx/u-boot.lds          |    2 +-
 arch/powerpc/cpu/mpc824x/u-boot.lds          |    2 +-
 arch/powerpc/cpu/mpc8260/u-boot.lds          |    2 +-
 arch/powerpc/cpu/mpc83xx/u-boot-spl.lds      |    2 +-
 arch/powerpc/cpu/mpc83xx/u-boot.lds          |    2 +-
 arch/powerpc/cpu/mpc85xx/u-boot-nand.lds     |    2 +-
 arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds |    2 +-
 arch/powerpc/cpu/mpc85xx/u-boot-spl.lds      |    2 +-
 arch/powerpc/cpu/mpc85xx/u-boot.lds          |    2 +-
 arch/powerpc/cpu/mpc86xx/u-boot.lds          |    2 +-
 arch/powerpc/cpu/ppc4xx/u-boot.lds           |    2 +-
 14 files changed, 14 insertions(+), 14 deletions(-)

Comments

Chris Packham Sept. 10, 2014, 3:54 a.m. UTC | #1 
Hi All,

On Tue, Sep 9, 2014 at 12:22 PM, Chris Packham <judge.packham@gmail.com> wrote:
> When the got2 pointers were adjusted after relocation the last entry was
> missed. For most builds was something innocuous and there was no
> observable problem but at least for the P2041RDB board this ended up
> being NetArpWaitTimerStart which caused the ARP packets to timeout
> immediately.
>
> On Mon, Sep 8, 2014 at 9:04 PM,
> Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
>>
>> The relevant code that uses
>> __got2_entries is in start.S:
>>
>> in_ram:
>>
>>         /*
>>          * Relocation Function, r12 point to got2+0x8000
>>          *
>>          * Adjust got2 pointers, no need to check for 0, this code
>>          * already puts a few entries in the table.
>>          */
>>         li      r0,__got2_entries@sectoff@l
>>         la      r3,GOT(_GOT2_TABLE_)
>>         lwz     r11,GOT(_GOT2_TABLE_)
>>         mtctr   r0
>>         sub     r11,r3,r11
>>         addi    r3,r3,-4
>> 1:      lwzu    r0,4(r3)
>>         cmpwi   r0,0
>>         beq-    2f
>>         add     r0,r0,r11
>>         stw     r0,0(r3)
>> 2:      bdnz    1b
>>
>> bdnz does decrement then test for zero so __got2_entries should hold the
>> number of entries to relocate.
>
> But __got2_entries is 1 less than it should be. Fix this by removing the
> -1 from the __got2_entries calculation.
>
> Cc: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
> So here's a patch that addresses the issue. I've tested it on a P2041RDB
> (mpc85xx/QorIq). I am aware that I'm touching a lot of platforms so I
> need some help testing.
>
> I've deliberately left off the following board specific files which at a
> glance have the same off-by-one error:
>

Sorry for the noise but Jocke has pointed me in the direction of a
better fix. I'll submit that as a new patch. This one can be
abandoned.
diff mbox

Patch

diff --git a/arch/powerpc/cpu/74xx_7xx/u-boot.lds b/arch/powerpc/cpu/74xx_7xx/u-boot.lds
index c099849..2fd39c8 100644
--- a/arch/powerpc/cpu/74xx_7xx/u-boot.lds
+++ b/arch/powerpc/cpu/74xx_7xx/u-boot.lds
@@ -34,7 +34,7 @@  SECTIONS
     _FIXUP_TABLE_ = .;
     KEEP(*(.fixup))
   }
-  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
   __fixup_entries = (. - _FIXUP_TABLE_)>>2;
 
   .data    :
diff --git a/arch/powerpc/cpu/mpc512x/u-boot.lds b/arch/powerpc/cpu/mpc512x/u-boot.lds
index 9658b21..2ca492b 100644
--- a/arch/powerpc/cpu/mpc512x/u-boot.lds
+++ b/arch/powerpc/cpu/mpc512x/u-boot.lds
@@ -29,7 +29,7 @@  SECTIONS
     KEEP(*(.fixup))
     *(.fixup)
   }
-  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
   __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
   .data    :
diff --git a/arch/powerpc/cpu/mpc5xx/u-boot.lds b/arch/powerpc/cpu/mpc5xx/u-boot.lds
index 7198465..c8dbff8 100644
--- a/arch/powerpc/cpu/mpc5xx/u-boot.lds
+++ b/arch/powerpc/cpu/mpc5xx/u-boot.lds
@@ -37,7 +37,7 @@  SECTIONS
     _FIXUP_TABLE_ = .;
     KEEP(*(.fixup))
   }
-  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
   __fixup_entries = (. - _FIXUP_TABLE_)>>2;
 
   .data    :
diff --git a/arch/powerpc/cpu/mpc5xxx/u-boot.lds b/arch/powerpc/cpu/mpc5xxx/u-boot.lds
index cd9e23f..9f6288a 100644
--- a/arch/powerpc/cpu/mpc5xxx/u-boot.lds
+++ b/arch/powerpc/cpu/mpc5xxx/u-boot.lds
@@ -32,7 +32,7 @@  SECTIONS
     _FIXUP_TABLE_ = .;
     KEEP(*(.fixup))
   }
-  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
   __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
   .data    :
diff --git a/arch/powerpc/cpu/mpc824x/u-boot.lds b/arch/powerpc/cpu/mpc824x/u-boot.lds
index 04aba84..b3ffbfc 100644
--- a/arch/powerpc/cpu/mpc824x/u-boot.lds
+++ b/arch/powerpc/cpu/mpc824x/u-boot.lds
@@ -32,7 +32,7 @@  SECTIONS
     _FIXUP_TABLE_ = .;
     KEEP(*(.fixup))
   }
-  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
   __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
   .data    :
diff --git a/arch/powerpc/cpu/mpc8260/u-boot.lds b/arch/powerpc/cpu/mpc8260/u-boot.lds
index 50cbf85..423f0ae 100644
--- a/arch/powerpc/cpu/mpc8260/u-boot.lds
+++ b/arch/powerpc/cpu/mpc8260/u-boot.lds
@@ -31,7 +31,7 @@  SECTIONS
     _FIXUP_TABLE_ = .;
     KEEP(*(.fixup))
   }
-  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
   __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
   .data    :
diff --git a/arch/powerpc/cpu/mpc83xx/u-boot-spl.lds b/arch/powerpc/cpu/mpc83xx/u-boot-spl.lds
index 774772b..9b513ed 100644
--- a/arch/powerpc/cpu/mpc83xx/u-boot-spl.lds
+++ b/arch/powerpc/cpu/mpc83xx/u-boot-spl.lds
@@ -26,7 +26,7 @@  SECTIONS
 		KEEP(*(.got))
 		PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4);
 	}
-	__got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+	__got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
 
 	. = ALIGN(8);
 	__bss_start = .;
diff --git a/arch/powerpc/cpu/mpc83xx/u-boot.lds b/arch/powerpc/cpu/mpc83xx/u-boot.lds
index 3c177fa..7370a3d 100644
--- a/arch/powerpc/cpu/mpc83xx/u-boot.lds
+++ b/arch/powerpc/cpu/mpc83xx/u-boot.lds
@@ -30,7 +30,7 @@  SECTIONS
     _FIXUP_TABLE_ = .;
     KEEP(*(.fixup))
   }
-  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
   __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
   .data    :
diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds b/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds
index f933b21..9b69cf3 100644
--- a/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds
+++ b/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds
@@ -48,7 +48,7 @@  SECTIONS
     _FIXUP_TABLE_ = .;
     KEEP(*(.fixup))
   }
-  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
   __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
   .data    :
diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds b/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds
index b83c553..215f254 100644
--- a/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds
+++ b/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds
@@ -26,7 +26,7 @@  SECTIONS
 		_FIXUP_TABLE_ = .;
 		KEEP(*(.fixup))
 	}
-	__got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+	__got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
 	__fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
 	. = ALIGN(8);
diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds
index 5ae7b3e..4b00bec 100644
--- a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds
+++ b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds
@@ -33,7 +33,7 @@  SECTIONS
 		_FIXUP_TABLE_ = .;
 		KEEP(*(.fixup))
 	}
-	__got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+	__got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
 	__fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
 	. = ALIGN(8);
diff --git a/arch/powerpc/cpu/mpc85xx/u-boot.lds b/arch/powerpc/cpu/mpc85xx/u-boot.lds
index 2cf0b25..36711b0 100644
--- a/arch/powerpc/cpu/mpc85xx/u-boot.lds
+++ b/arch/powerpc/cpu/mpc85xx/u-boot.lds
@@ -54,7 +54,7 @@  SECTIONS
     _FIXUP_TABLE_ = .;
     KEEP(*(.fixup))
   }
-  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
   __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
   .data    :
diff --git a/arch/powerpc/cpu/mpc86xx/u-boot.lds b/arch/powerpc/cpu/mpc86xx/u-boot.lds
index 58467c2..f6f52e3 100644
--- a/arch/powerpc/cpu/mpc86xx/u-boot.lds
+++ b/arch/powerpc/cpu/mpc86xx/u-boot.lds
@@ -36,7 +36,7 @@  SECTIONS
     _FIXUP_TABLE_ = .;
     KEEP(*(.fixup))
   }
-  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
   __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
   .data    :
diff --git a/arch/powerpc/cpu/ppc4xx/u-boot.lds b/arch/powerpc/cpu/ppc4xx/u-boot.lds
index 8773178..22a5ec4 100644
--- a/arch/powerpc/cpu/ppc4xx/u-boot.lds
+++ b/arch/powerpc/cpu/ppc4xx/u-boot.lds
@@ -50,7 +50,7 @@  SECTIONS
     _FIXUP_TABLE_ = .;
     KEEP(*(.fixup))
   }
-  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2);
   __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
   .data    :