| Message ID | 1439815009-28800-1-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Masahiro, On 17 August 2015 at 06:36, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > I found some of my boards would not boot since commit 2afddae07523 > ("Align global_data to a 16-byte boundary"). Probably, many ARM > boards using Generic Board framework are broken, including ARM64 > ones. That commit aligned the global data pointer, i.e. inserted > some spaces between the gd and the bd. > > The assembly code in arch/arm/lib/crt0(_64).S must be adjusted > because the new gd is no longer just below the bd. Otherwise, > r9 (x18) holds a wrong pointer to the global data. > > This commit uses GD_NEW_GD to directly reference the offset to the > new_gd. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> This is very unfortunate, sorry. I suggest a straight revert of that commit for now. My testing shows this will not break x86 at present since the alignment is precautionary. I can pick up Masahiro's fix and post a new series after looking at aarch64. I'll post a revert. Also I'd like to look at moving ARM to use board_init_f_mem() which I think is a better solution long term. > --- > > Tom, > > I guess many ARM boards are broken. > This issue should be fixed asap. > > I confirmed this commit fixes the problem on my boards. > > Since I have no ARM64 board, I hope somebody could test it > for ARM64. > > > arch/arm/lib/crt0.S | 3 +-- > arch/arm/lib/crt0_64.S | 3 +-- > lib/asm-offsets.c | 2 ++ > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S > index afd4f10..c7bdfbf 100644 > --- a/arch/arm/lib/crt0.S > +++ b/arch/arm/lib/crt0.S > @@ -119,8 +119,7 @@ clr_gd: > #else > bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ > #endif > - ldr r9, [r9, #GD_BD] /* r9 = gd->bd */ > - sub r9, r9, #GD_SIZE /* new GD is below bd */ > + ldr r9, [r9, #GD_NEW_GD] /* r9 = gd->new_gd */ > > adr lr, here > ldr r0, [r9, #GD_RELOC_OFF] /* r0 = gd->reloc_off */ > diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S > index 98a906e..13960d0 100644 > --- a/arch/arm/lib/crt0_64.S > +++ b/arch/arm/lib/crt0_64.S > @@ -90,8 +90,7 @@ zero_gd: > */ > ldr x0, [x18, #GD_START_ADDR_SP] /* x0 <- gd->start_addr_sp */ > bic sp, x0, #0xf /* 16-byte alignment for ABI compliance */ > - ldr x18, [x18, #GD_BD] /* x18 <- gd->bd */ > - sub x18, x18, #GD_SIZE /* new GD is below bd */ > + ldr x18, [x18, #GD_NEW_GD] /* x18 <- gd->new_gd */ > > adr lr, relocation_return > ldr x9, [x18, #GD_RELOC_OFF] /* x9 <- gd->reloc_off */ > diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c > index 221ebbf..88afedf 100644 > --- a/lib/asm-offsets.c > +++ b/lib/asm-offsets.c > @@ -36,6 +36,8 @@ int main(void) > > DEFINE(GD_RELOC_OFF, offsetof(struct global_data, reloc_off)); > > + DEFINE(GD_NEW_GD, offsetof(struct global_data, new_gd)); > + > DEFINE(GD_START_ADDR_SP, offsetof(struct global_data, start_addr_sp)); > > return 0; > -- > 1.9.1 > Regards, Simon
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index afd4f10..c7bdfbf 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -119,8 +119,7 @@ clr_gd: #else bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ #endif - ldr r9, [r9, #GD_BD] /* r9 = gd->bd */ - sub r9, r9, #GD_SIZE /* new GD is below bd */ + ldr r9, [r9, #GD_NEW_GD] /* r9 = gd->new_gd */ adr lr, here ldr r0, [r9, #GD_RELOC_OFF] /* r0 = gd->reloc_off */ diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S index 98a906e..13960d0 100644 --- a/arch/arm/lib/crt0_64.S +++ b/arch/arm/lib/crt0_64.S @@ -90,8 +90,7 @@ zero_gd: */ ldr x0, [x18, #GD_START_ADDR_SP] /* x0 <- gd->start_addr_sp */ bic sp, x0, #0xf /* 16-byte alignment for ABI compliance */ - ldr x18, [x18, #GD_BD] /* x18 <- gd->bd */ - sub x18, x18, #GD_SIZE /* new GD is below bd */ + ldr x18, [x18, #GD_NEW_GD] /* x18 <- gd->new_gd */ adr lr, relocation_return ldr x9, [x18, #GD_RELOC_OFF] /* x9 <- gd->reloc_off */ diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c index 221ebbf..88afedf 100644 --- a/lib/asm-offsets.c +++ b/lib/asm-offsets.c @@ -36,6 +36,8 @@ int main(void) DEFINE(GD_RELOC_OFF, offsetof(struct global_data, reloc_off)); + DEFINE(GD_NEW_GD, offsetof(struct global_data, new_gd)); + DEFINE(GD_START_ADDR_SP, offsetof(struct global_data, start_addr_sp)); return 0;
I found some of my boards would not boot since commit 2afddae07523 ("Align global_data to a 16-byte boundary"). Probably, many ARM boards using Generic Board framework are broken, including ARM64 ones. That commit aligned the global data pointer, i.e. inserted some spaces between the gd and the bd. The assembly code in arch/arm/lib/crt0(_64).S must be adjusted because the new gd is no longer just below the bd. Otherwise, r9 (x18) holds a wrong pointer to the global data. This commit uses GD_NEW_GD to directly reference the offset to the new_gd. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- Tom, I guess many ARM boards are broken. This issue should be fixed asap. I confirmed this commit fixes the problem on my boards. Since I have no ARM64 board, I hope somebody could test it for ARM64. arch/arm/lib/crt0.S | 3 +-- arch/arm/lib/crt0_64.S | 3 +-- lib/asm-offsets.c | 2 ++ 3 files changed, 4 insertions(+), 4 deletions(-)