All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] p2371-2180: Build position independent binary
@ 2019-03-08 20:10 Thierry Reding
  2019-03-18 18:31 ` Stephen Warren
  0 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2019-03-08 20:10 UTC (permalink / raw)
  To: u-boot

From: Thierry Reding <treding@nvidia.com>

In order to support chainloading of U-Boot by an earlier bootloader,
make sure the binary is position independent, so that the earlier boot-
loader can relocate it if necessary.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 configs/p2371-2180_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/p2371-2180_defconfig b/configs/p2371-2180_defconfig
index b66459e379ac..8d7cf3fb5346 100644
--- a/configs/p2371-2180_defconfig
+++ b/configs/p2371-2180_defconfig
@@ -1,6 +1,7 @@
 CONFIG_ARM=y
 CONFIG_TEGRA=y
 CONFIG_SYS_TEXT_BASE=0x80110000
+CONFIG_POSITION_INDEPENDENT=y
 CONFIG_TEGRA210=y
 CONFIG_TARGET_P2371_2180=y
 CONFIG_NR_DRAM_BANKS=2
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] p2371-2180: Build position independent binary
  2019-03-08 20:10 [U-Boot] [PATCH] p2371-2180: Build position independent binary Thierry Reding
@ 2019-03-18 18:31 ` Stephen Warren
  2019-03-19 12:12   ` Thierry Reding
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2019-03-18 18:31 UTC (permalink / raw)
  To: u-boot

On 3/8/19 1:10 PM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> In order to support chainloading of U-Boot by an earlier bootloader,
> make sure the binary is position independent, so that the earlier boot-
> loader can relocate it if necessary.

Why not enable this for all 64-bit Tegra? They're all booted the exact 
same way at least with recent L4T builds.

Also, U-Boot is typically linked to the address that cboot loads it to, 
and cboot typically always loads to precisely that address. I'm not sure 
why this patch is required.

That said, it don't think it harms anything, so I'm fine with it being 
applied.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] p2371-2180: Build position independent binary
  2019-03-18 18:31 ` Stephen Warren
@ 2019-03-19 12:12   ` Thierry Reding
  2019-03-19 17:05     ` Stephen Warren
  0 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2019-03-19 12:12 UTC (permalink / raw)
  To: u-boot

On Mon, Mar 18, 2019 at 12:31:32PM -0600, Stephen Warren wrote:
> On 3/8/19 1:10 PM, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > In order to support chainloading of U-Boot by an earlier bootloader,
> > make sure the binary is position independent, so that the earlier boot-
> > loader can relocate it if necessary.
> 
> Why not enable this for all 64-bit Tegra? They're all booted the exact same
> way at least with recent L4T builds.

Yeah, I think that would make sense.

> Also, U-Boot is typically linked to the address that cboot loads it to, and
> cboot typically always loads to precisely that address. I'm not sure why
> this patch is required.

I encountered this issue when I was trying to chainload U-Boot from
cboot on Jetson TX1. It seems like your above comment is no longer true,
though I suppose that could just mean that the link address for U-Boot
has become stale?

> That said, it don't think it harms anything, so I'm fine with it being
> applied.

I suppose there's a bit of extra code to do the indirect jumps, but
overall U-Boot seems to work well and not noticeably slower if this is
enabled. Might be nice for extra flexibility and to avoid any surprises
if we ever end up loading U-Boot to a location other than where it was
liked to.

My understanding is that this could happen on Tegra186 if cboot detects
bad memory blocks in the area where U-Boot was meant to be loaded to. I
guess this doesn't apply to earlier chips, but perhaps it's good to have
it there for consistency anyway.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190319/384b1e35/attachment.sig>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] p2371-2180: Build position independent binary
  2019-03-19 12:12   ` Thierry Reding
@ 2019-03-19 17:05     ` Stephen Warren
  2019-03-20 10:27       ` Thierry Reding
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2019-03-19 17:05 UTC (permalink / raw)
  To: u-boot

On 3/19/19 6:12 AM, Thierry Reding wrote:
> On Mon, Mar 18, 2019 at 12:31:32PM -0600, Stephen Warren wrote:
>> On 3/8/19 1:10 PM, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> In order to support chainloading of U-Boot by an earlier bootloader,
>>> make sure the binary is position independent, so that the earlier boot-
>>> loader can relocate it if necessary.
>>
>> Why not enable this for all 64-bit Tegra? They're all booted the exact same
>> way at least with recent L4T builds.
> 
> Yeah, I think that would make sense.
> 
>> Also, U-Boot is typically linked to the address that cboot loads it to, and
>> cboot typically always loads to precisely that address. I'm not sure why
>> this patch is required.
> 
> I encountered this issue when I was trying to chainload U-Boot from
> cboot on Jetson TX1. It seems like your above comment is no longer true,
> though I suppose that could just mean that the link address for U-Boot
> has become stale?

Looks like the upstream CONFIG_SYS_TEXT_BASE is indeed stale relative to 
the latest L4T builds:

Upstream:
Jetson TX1: CONFIG_SYS_TEXT_BASE=0x80110000
Jetson TX2: CONFIG_SYS_TEXT_BASE=0x80080000

L4T r32.1:
Jetson TX1: #define CONFIG_SYS_TEXT_BASE    0x80080000
Jetson TX2: #define CONFIG_SYS_TEXT_BASE    0x80080000

>> That said, it don't think it harms anything, so I'm fine with it being
>> applied.
> 
> I suppose there's a bit of extra code to do the indirect jumps, but
> overall U-Boot seems to work well and not noticeably slower if this is
> enabled. Might be nice for extra flexibility and to avoid any surprises
> if we ever end up loading U-Boot to a location other than where it was
> liked to.
> 
> My understanding is that this could happen on Tegra186 if cboot detects
> bad memory blocks in the area where U-Boot was meant to be loaded to. I
> guess this doesn't apply to earlier chips, but perhaps it's good to have
> it there for consistency anyway.

Yes, this could happen on Jetson TX2i at least.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] p2371-2180: Build position independent binary
  2019-03-19 17:05     ` Stephen Warren
@ 2019-03-20 10:27       ` Thierry Reding
  2019-03-20 16:53         ` Stephen Warren
  0 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2019-03-20 10:27 UTC (permalink / raw)
  To: u-boot

On Tue, Mar 19, 2019 at 11:05:43AM -0600, Stephen Warren wrote:
> On 3/19/19 6:12 AM, Thierry Reding wrote:
> > On Mon, Mar 18, 2019 at 12:31:32PM -0600, Stephen Warren wrote:
> > > On 3/8/19 1:10 PM, Thierry Reding wrote:
> > > > From: Thierry Reding <treding@nvidia.com>
> > > > 
> > > > In order to support chainloading of U-Boot by an earlier bootloader,
> > > > make sure the binary is position independent, so that the earlier boot-
> > > > loader can relocate it if necessary.
> > > 
> > > Why not enable this for all 64-bit Tegra? They're all booted the exact same
> > > way at least with recent L4T builds.
> > 
> > Yeah, I think that would make sense.
> > 
> > > Also, U-Boot is typically linked to the address that cboot loads it to, and
> > > cboot typically always loads to precisely that address. I'm not sure why
> > > this patch is required.
> > 
> > I encountered this issue when I was trying to chainload U-Boot from
> > cboot on Jetson TX1. It seems like your above comment is no longer true,
> > though I suppose that could just mean that the link address for U-Boot
> > has become stale?
> 
> Looks like the upstream CONFIG_SYS_TEXT_BASE is indeed stale relative to the
> latest L4T builds:
> 
> Upstream:
> Jetson TX1: CONFIG_SYS_TEXT_BASE=0x80110000
> Jetson TX2: CONFIG_SYS_TEXT_BASE=0x80080000
> 
> L4T r32.1:
> Jetson TX1: #define CONFIG_SYS_TEXT_BASE    0x80080000
> Jetson TX2: #define CONFIG_SYS_TEXT_BASE    0x80080000

Okay, let me fix that up while at it. I think the 0x80110000 text base
was something that we cargo-culted from 32-bit ARM boards. I vaguely
recall that this might have been related to the SPL split in some way,
but I can't find anything to corroborate that.

> > > That said, it don't think it harms anything, so I'm fine with it being
> > > applied.
> > 
> > I suppose there's a bit of extra code to do the indirect jumps, but
> > overall U-Boot seems to work well and not noticeably slower if this is
> > enabled. Might be nice for extra flexibility and to avoid any surprises
> > if we ever end up loading U-Boot to a location other than where it was
> > liked to.
> > 
> > My understanding is that this could happen on Tegra186 if cboot detects
> > bad memory blocks in the area where U-Boot was meant to be loaded to. I
> > guess this doesn't apply to earlier chips, but perhaps it's good to have
> > it there for consistency anyway.
> 
> Yes, this could happen on Jetson TX2i at least.

Let me send out a proposal than makes all 64-bit Tegra builds position
independent and throws in the TEXT_BASE changes discussed above.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190320/0436c534/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] p2371-2180: Build position independent binary
  2019-03-20 10:27       ` Thierry Reding
@ 2019-03-20 16:53         ` Stephen Warren
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Warren @ 2019-03-20 16:53 UTC (permalink / raw)
  To: u-boot

On 3/20/19 4:27 AM, Thierry Reding wrote:
> On Tue, Mar 19, 2019 at 11:05:43AM -0600, Stephen Warren wrote:
>> On 3/19/19 6:12 AM, Thierry Reding wrote:
>>> On Mon, Mar 18, 2019 at 12:31:32PM -0600, Stephen Warren wrote:
>>>> On 3/8/19 1:10 PM, Thierry Reding wrote:
>>>>> From: Thierry Reding <treding@nvidia.com>
>>>>>
>>>>> In order to support chainloading of U-Boot by an earlier bootloader,
>>>>> make sure the binary is position independent, so that the earlier boot-
>>>>> loader can relocate it if necessary.
>>>>
>>>> Why not enable this for all 64-bit Tegra? They're all booted the exact same
>>>> way at least with recent L4T builds.
>>>
>>> Yeah, I think that would make sense.
>>>
>>>> Also, U-Boot is typically linked to the address that cboot loads it to, and
>>>> cboot typically always loads to precisely that address. I'm not sure why
>>>> this patch is required.
>>>
>>> I encountered this issue when I was trying to chainload U-Boot from
>>> cboot on Jetson TX1. It seems like your above comment is no longer true,
>>> though I suppose that could just mean that the link address for U-Boot
>>> has become stale?
>>
>> Looks like the upstream CONFIG_SYS_TEXT_BASE is indeed stale relative to the
>> latest L4T builds:
>>
>> Upstream:
>> Jetson TX1: CONFIG_SYS_TEXT_BASE=0x80110000
>> Jetson TX2: CONFIG_SYS_TEXT_BASE=0x80080000
>>
>> L4T r32.1:
>> Jetson TX1: #define CONFIG_SYS_TEXT_BASE    0x80080000
>> Jetson TX2: #define CONFIG_SYS_TEXT_BASE    0x80080000
> 
> Okay, let me fix that up while at it. I think the 0x80110000 text base
> was something that we cargo-culted from 32-bit ARM boards. I vaguely
> recall that this might have been related to the SPL split in some way,
> but I can't find anything to corroborate that.

I believe it was accurate for the first L4T release to support Tegra210, 
which chain-loaded U-Boot from nvtboot-cpu rather than chain-loading it 
from cboot. But, I could be wrong.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-03-20 16:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-08 20:10 [U-Boot] [PATCH] p2371-2180: Build position independent binary Thierry Reding
2019-03-18 18:31 ` Stephen Warren
2019-03-19 12:12   ` Thierry Reding
2019-03-19 17:05     ` Stephen Warren
2019-03-20 10:27       ` Thierry Reding
2019-03-20 16:53         ` Stephen Warren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.