| Message ID | 1334719161-3500-1-git-send-email-Chunhe.Lan@freescale.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Andy Fleming |
| Headers | show |
Dear Chunhe Lan, In message <1334719161-3500-1-git-send-email-Chunhe.Lan@freescale.com> you wrote: > The -fPIC/-fpic flag belongs with -mrelocatable. The -fpic flag can > limit the size of the GOT and produce smaller binaries, so it causes > some GOT entries to be lost in the gcc 4.6 version. But -fPIC flag > allows the maximum possible size of the GOT entries. > > However, currently -mrelocatable promotes -fpic flag to -fPIC flag. > > This reverts that portion of the > commit 33ee4c92339ee386662c0ee2d221098c5cc8b07e. As you state yourself, your modification has the negative impact of increasing the image size. What would be the benefits of it? Are you trying to fix any specific problem? Which one? I am not aware of any related isses for any of the mainline PowerPC systems... Best regards, Wolfgang Denk
Dear Chunhe Lan, In message <4F8E5F11.4020904@freescale.com> you wrote: > <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> > <html> > <head> > <meta content=3D"text/html;charset=3DUTF-8" http-equiv=3D"Content-Type"= Message unreadable, ignored. Please post plain text only. Wolfgang Denk
Wolfgang Denk wrote: > Dear Chunhe Lan, > > In message <1334719161-3500-1-git-send-email-Chunhe.Lan@freescale.com> > you wrote: >> The -fPIC/-fpic flag belongs with -mrelocatable. The -fpic flag can >> limit the size of the GOT and produce smaller binaries, so it causes >> some GOT entries to be lost in the gcc 4.6 version. But -fPIC flag >> allows the maximum possible size of the GOT entries. >> >> However, currently -mrelocatable promotes -fpic flag to -fPIC flag. >> >> This reverts that portion of the >> commit 33ee4c92339ee386662c0ee2d221098c5cc8b07e. > > As you state yourself, your modification has the negative impact of > increasing the image size. What would be the benefits of it? > > Are you trying to fix any specific problem? Which one? I am not aware > of any related isses for any of the mainline PowerPC systems... Yes, I have fixed the booting issue of nand u-boot of P1010/P1022/P1023/P2020 with this patch. When use gcc 4.5 version, produce the size of u-boot-nand.bin to be same with -fpic or -fPIC. The u-boot-nand.bin is OK. But when use gcc 4.6 version, produce the size of u-boot-nand.bin to be different with -fpic or -fPIC. The some GOT entries of u-boot-nand.bin are lost with -fpic(because limit the size), so u-boot-nand.bin hangs when booting. But use -fPIC, the u-boot-nand.bin is OK. And this patch reverts that portion of the commit 33ee4c92339ee386662c0ee2d221098c5cc8b07e. Thanks, Chunhe > > Best regards, > > Wolfgang Denk >
Dear Chunhe Lan, In message <4F8E6E06.3060101@freescale.com> you wrote: > > > As you state yourself, your modification has the negative impact of > > increasing the image size. What would be the benefits of it? > > > > Are you trying to fix any specific problem? Which one? I am not aware > > of any related isses for any of the mainline PowerPC systems... > Yes, I have fixed the booting issue of nand u-boot of > P1010/P1022/P1023/P2020 with this patch. What exactly is "the booting issue" you mention here? Are you referring to any messages previously posted here? Which? > But when use gcc 4.6 version, produce the size of u-boot-nand.bin to > be different with -fpic or -fPIC. > The some GOT entries of u-boot-nand.bin are lost with -fpic(because > limit the size), so > u-boot-nand.bin hangs when booting. But use -fPIC, the > u-boot-nand.bin is OK. What do you mean "GOT entries ... are lost"? Are there any error messages? If yes, what exactly are these? If not, then this has to be considered a serious bug in either the compiler and the linker, and it should be fixed there rather than being papered over. Best regards, Wolfgang Denk
> > > > Wolfgang Denk wrote: > > Dear Chunhe Lan, > > > > In message <1334719161-3500-1-git-send-email-Chunhe.Lan@freescale.com> > > you wrote: > >> The -fPIC/-fpic flag belongs with -mrelocatable. The -fpic flag can > >> limit the size of the GOT and produce smaller binaries, so it causes > >> some GOT entries to be lost in the gcc 4.6 version. But -fPIC flag > >> allows the maximum possible size of the GOT entries. > >> > >> However, currently -mrelocatable promotes -fpic flag to -fPIC flag. Not in newer gcc's. I added this to gcc some time ago and I think it got into gcc 4.6. > >> > >> This reverts that portion of the > >> commit 33ee4c92339ee386662c0ee2d221098c5cc8b07e. > > > > As you state yourself, your modification has the negative impact of > > increasing the image size. What would be the benefits of it? > > > > Are you trying to fix any specific problem? Which one? I am not aware > > of any related isses for any of the mainline PowerPC systems... > Yes, I have fixed the booting issue of nand u-boot of > P1010/P1022/P1023/P2020 with this patch. > > When use gcc 4.5 version, produce the size of u-boot-nand.bin to be > same with -fpic or -fPIC. > The u-boot-nand.bin is OK. > > But when use gcc 4.6 version, produce the size of u-boot-nand.bin to > be different with -fpic or -fPIC. > The some GOT entries of u-boot-nand.bin are lost with -fpic(because > limit the size), so > u-boot-nand.bin hangs when booting. But use -fPIC, the > u-boot-nand.bin is OK. Your linker file is buggy I think. I found u-boot-nand_spl.lds, is that the one? Check out that files reloc part: .reloc : { _GOT2_TABLE_ = .; KEEP(*(.got2)) _FIXUP_TABLE_ = .; KEEP(*(.fixup)) } __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; __fixup_entries = (. - _FIXUP_TABLE_) >> 2; Compare that with(from u-boot.lds): .reloc : { _GOT2_TABLE_ = .; KEEP(*(.got2)) KEEP(*(.got)) PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); _FIXUP_TABLE_ = .; KEEP(*(.fixup)) } __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1; __fixup_entries = (. - _FIXUP_TABLE_) >> 2; You may have to look at start.S for nand too
On 04/18/2012 05:07 AM, Joakim Tjernlund wrote: > Your linker file is buggy I think. I found u-boot-nand_spl.lds, is that the one? That's the one for the SPL part. > Check out that files reloc part: > .reloc : { > _GOT2_TABLE_ = .; > KEEP(*(.got2)) > _FIXUP_TABLE_ = .; > KEEP(*(.fixup)) > } > __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; > __fixup_entries = (. - _FIXUP_TABLE_) >> 2; > > Compare that with(from u-boot.lds): > .reloc : > { > _GOT2_TABLE_ = .; > KEEP(*(.got2)) > KEEP(*(.got)) > PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); > _FIXUP_TABLE_ = .; > KEEP(*(.fixup)) > } > __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1; > __fixup_entries = (. - _FIXUP_TABLE_) >> 2; I notice a difference between u-boot.lds and u-boot-nand.lds -- the latter (used for the main part of U-Boot when loaded from SPL) has: > .reloc : > { > KEEP(*(.got)) > _GOT2_TABLE_ = .; > KEEP(*(.got2)) > _FIXUP_TABLE_ = .; > KEEP(*(.fixup)) > } > __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; > __fixup_entries = (. - _FIXUP_TABLE_) >> 2; Is this wrong as well? -Scott
Scott Wood <scottwood@freescale.com> wrote on 2012/04/25 21:01:00: > > On 04/18/2012 05:07 AM, Joakim Tjernlund wrote: > > Your linker file is buggy I think. I found u-boot-nand_spl.lds, is that the one? > > That's the one for the SPL part. OK, good. > > > Check out that files reloc part: > > .reloc : { > > _GOT2_TABLE_ = .; > > KEEP(*(.got2)) > > _FIXUP_TABLE_ = .; > > KEEP(*(.fixup)) > > } > > __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; > > __fixup_entries = (. - _FIXUP_TABLE_) >> 2; > > > > Compare that with(from u-boot.lds): > > .reloc : > > { > > _GOT2_TABLE_ = .; > > KEEP(*(.got2)) > > KEEP(*(.got)) > > PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); > > _FIXUP_TABLE_ = .; > > KEEP(*(.fixup)) > > } > > __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1; > > __fixup_entries = (. - _FIXUP_TABLE_) >> 2; > > I notice a difference between u-boot.lds and u-boot-nand.lds -- the > latter (used for the main part of U-Boot when loaded from SPL) has: > > > .reloc : > > { > > KEEP(*(.got)) > > _GOT2_TABLE_ = .; > > KEEP(*(.got2)) > > _FIXUP_TABLE_ = .; > > KEEP(*(.fixup)) > > } > > __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; > > __fixup_entries = (. - _FIXUP_TABLE_) >> 2; > > Is this wrong as well? Yes, I think so. This one does not care about the .got entries at all so I suspect never gcc's wont work. Jocke
On 04/26/2012 01:53 AM, Joakim Tjernlund wrote: > Scott Wood <scottwood@freescale.com> wrote on 2012/04/25 21:01:00: >> >> On 04/18/2012 05:07 AM, Joakim Tjernlund wrote: >>> Your linker file is buggy I think. I found u-boot-nand_spl.lds, is that the one? >> >> That's the one for the SPL part. > > OK, good. > >> >>> Check out that files reloc part: >>> .reloc : { >>> _GOT2_TABLE_ = .; >>> KEEP(*(.got2)) >>> _FIXUP_TABLE_ = .; >>> KEEP(*(.fixup)) >>> } >>> __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; >>> __fixup_entries = (. - _FIXUP_TABLE_) >> 2; >>> >>> Compare that with(from u-boot.lds): >>> .reloc : >>> { >>> _GOT2_TABLE_ = .; >>> KEEP(*(.got2)) >>> KEEP(*(.got)) >>> PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); >>> _FIXUP_TABLE_ = .; >>> KEEP(*(.fixup)) >>> } >>> __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1; >>> __fixup_entries = (. - _FIXUP_TABLE_) >> 2; >> >> I notice a difference between u-boot.lds and u-boot-nand.lds -- the >> latter (used for the main part of U-Boot when loaded from SPL) has: >> >>> .reloc : >>> { >>> KEEP(*(.got)) >>> _GOT2_TABLE_ = .; >>> KEEP(*(.got2)) >>> _FIXUP_TABLE_ = .; >>> KEEP(*(.fixup)) >>> } >>> __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; >>> __fixup_entries = (. - _FIXUP_TABLE_) >> 2; >> >> Is this wrong as well? > > Yes, I think so. This one does not care about the .got entries at all so I suspect > never gcc's wont work. Thanks. I'm not entirely sure about the _GLOBAL_OFFSET_TABLE_ stuff, though -- what's wrong with the simpler version that uses _FIXUP_TABLE_? -Scott
Scott Wood <scottwood@freescale.com> wrote on 2012/04/26 23:26:52: > > On 04/26/2012 01:53 AM, Joakim Tjernlund wrote: > > Scott Wood <scottwood@freescale.com> wrote on 2012/04/25 21:01:00: > >> > >> On 04/18/2012 05:07 AM, Joakim Tjernlund wrote: > >>> Your linker file is buggy I think. I found u-boot-nand_spl.lds, is that the one? > >> > >> That's the one for the SPL part. > > > > OK, good. > > > >> > >>> Check out that files reloc part: > >>> .reloc : { > >>> _GOT2_TABLE_ = .; > >>> KEEP(*(.got2)) > >>> _FIXUP_TABLE_ = .; > >>> KEEP(*(.fixup)) > >>> } > >>> __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; > >>> __fixup_entries = (. - _FIXUP_TABLE_) >> 2; > >>> > >>> Compare that with(from u-boot.lds): > >>> .reloc : > >>> { > >>> _GOT2_TABLE_ = .; > >>> KEEP(*(.got2)) > >>> KEEP(*(.got)) > >>> PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); > >>> _FIXUP_TABLE_ = .; > >>> KEEP(*(.fixup)) > >>> } > >>> __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1; > >>> __fixup_entries = (. - _FIXUP_TABLE_) >> 2; > >> > >> I notice a difference between u-boot.lds and u-boot-nand.lds -- the > >> latter (used for the main part of U-Boot when loaded from SPL) has: > >> > >>> .reloc : > >>> { > >>> KEEP(*(.got)) > >>> _GOT2_TABLE_ = .; > >>> KEEP(*(.got2)) > >>> _FIXUP_TABLE_ = .; > >>> KEEP(*(.fixup)) > >>> } > >>> __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; > >>> __fixup_entries = (. - _FIXUP_TABLE_) >> 2; > >> > >> Is this wrong as well? > > > > Yes, I think so. This one does not care about the .got entries at all so I suspect > > never gcc's wont work. > > Thanks. > > I'm not entirely sure about the _GLOBAL_OFFSET_TABLE_ stuff, though -- > what's wrong with the simpler version that uses _FIXUP_TABLE_? _GLOBAL_OFFSET_TABLE_ is a -fpic thing, it marks the beginning of reloc entrires and gcc/binutils defines it if there are -fpic relocs present. If -fpic is present, there is are some extra entrires. If no -fpic I need to manually recreate space, otherwise the reloc routine will be off. Jocke
> > Scott Wood <scottwood@freescale.com> wrote on 2012/04/26 23:26:52: > > > > On 04/26/2012 01:53 AM, Joakim Tjernlund wrote: > > > Scott Wood <scottwood@freescale.com> wrote on 2012/04/25 21:01:00: > > >> > > >> On 04/18/2012 05:07 AM, Joakim Tjernlund wrote: > > >>> Your linker file is buggy I think. I found u-boot-nand_spl.lds, is that the one? > > >> > > >> That's the one for the SPL part. > > > > > > OK, good. > > > > > >> > > >>> Check out that files reloc part: > > >>> .reloc : { > > >>> _GOT2_TABLE_ = .; > > >>> KEEP(*(.got2)) > > >>> _FIXUP_TABLE_ = .; > > >>> KEEP(*(.fixup)) > > >>> } > > >>> __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; > > >>> __fixup_entries = (. - _FIXUP_TABLE_) >> 2; > > >>> > > >>> Compare that with(from u-boot.lds): > > >>> .reloc : > > >>> { > > >>> _GOT2_TABLE_ = .; > > >>> KEEP(*(.got2)) > > >>> KEEP(*(.got)) > > >>> PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4); > > >>> _FIXUP_TABLE_ = .; > > >>> KEEP(*(.fixup)) > > >>> } > > >>> __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1; > > >>> __fixup_entries = (. - _FIXUP_TABLE_) >> 2; > > >> > > >> I notice a difference between u-boot.lds and u-boot-nand.lds -- the > > >> latter (used for the main part of U-Boot when loaded from SPL) has: > > >> > > >>> .reloc : > > >>> { > > >>> KEEP(*(.got)) > > >>> _GOT2_TABLE_ = .; > > >>> KEEP(*(.got2)) > > >>> _FIXUP_TABLE_ = .; > > >>> KEEP(*(.fixup)) > > >>> } > > >>> __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; > > >>> __fixup_entries = (. - _FIXUP_TABLE_) >> 2; > > >> > > >> Is this wrong as well? > > > > > > Yes, I think so. This one does not care about the .got entries at all so I suspect > > > never gcc's wont work. > > > > Thanks. > > > > I'm not entirely sure about the _GLOBAL_OFFSET_TABLE_ stuff, though -- > > what's wrong with the simpler version that uses _FIXUP_TABLE_? > > _GLOBAL_OFFSET_TABLE_ is a -fpic thing, it marks the beginning of reloc entrires and gcc/binutils > defines it if there are -fpic relocs present. If -fpic is present, there is are some extra > entrires. If no -fpic I need to manually recreate space, otherwise the reloc routine will be off. PS. note the -1 diff too: __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1; Jocke
diff --git a/arch/powerpc/config.mk b/arch/powerpc/config.mk index a307154..7c14ff6 100644 --- a/arch/powerpc/config.mk +++ b/arch/powerpc/config.mk @@ -25,7 +25,7 @@ CROSS_COMPILE ?= ppc_8xx- CONFIG_STANDALONE_LOAD_ADDR ?= 0x40000 LDFLAGS_FINAL += --gc-sections -PLATFORM_RELFLAGS += -fpic -mrelocatable -ffunction-sections -fdata-sections +PLATFORM_RELFLAGS += -fPIC -mrelocatable -ffunction-sections -fdata-sections PLATFORM_CPPFLAGS += -DCONFIG_PPC -D__powerpc__ PLATFORM_LDFLAGS += -n
The -fPIC/-fpic flag belongs with -mrelocatable. The -fpic flag can limit the size of the GOT and produce smaller binaries, so it causes some GOT entries to be lost in the gcc 4.6 version. But -fPIC flag allows the maximum possible size of the GOT entries. However, currently -mrelocatable promotes -fpic flag to -fPIC flag. This reverts that portion of the commit 33ee4c92339ee386662c0ee2d221098c5cc8b07e. Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com> --- arch/powerpc/config.mk | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)