On 14-06-10 02:20 PM, Albert ARIBAUD wrote:
Hi Steve,

On Tue, 10 Jun 2014 12:38:42 -0700, Steve Rae <s...@broadcom.com> wrote:



On 14-06-10 11:13 AM, Wolfgang Denk wrote:
Dear Steve,

In message <539746c4.9040...@broadcom.com> you wrote:

There could be "many" reasons why the CONFIG_SYS_TEXT_BASE is not
aligned on a 0x1000 byte boundary (Darwin has attempted to document his
particular use-case...)

We should be more precise here and ask for any _good_ reasons.

I can think of many good reasons to keep the text base aligned.  As
for the reasons to use an unaligned address that were brought up here,
I still think that it would have been better to use an aligned taxe
base and do the rest with a customized linker script.

But we think that the solution to support this is relatively
straightforward:
(1) after determining the "relocation address" (which will be on a
0x1000 byte boundary),
(2) add "CONFIG_SYS_TEXT_BASE % 4096" to that "relocation address"
(which effectively makes the "relocation offset" a multiple of 0x1000
too...)
So, in the scenario #1, (CONFIG_SYS_TEXT_BASE % 4096) = 0, so this
algorithm changes nothing.
And in scenario #2, (CONFIG_SYS_TEXT_BASE % 4096) = 0x20, therefore, we
would now get the following:
the relocation offset is:       0x77f9b000
therefore, after relocation:
_start  symbol is at            0xfff9b020 (0x88000020+0x77f9b000)
vectors symbol is at            0xfff9b800 (0x88000800+0x77f9b000)

I still cannot understand why _start and CONFIG_SYS_TEXT_BASE would
have to be the same.  There is no such requirement.  What exactly
prevents you from assigning _start a location at offset 0x20 to the
start of the text segment, i. e. CONFIG_SYS_TEXT_BASE ?

Wolfgang et al.

I agree that they do not need to be the same...
So our issue is that basically "for every ARMv7 board in the company",
we are currently maintaining our own modified/customized version of
"arch/arm/cpu/armv7/start.S" in order to add the appropriate 32 byte
header...

That is not a good approach -- and I should know, as there are boards
out there which already insist on having a header (mind you, a simple
4-byte constant) in start.S, and I am already trying to tackle this
out of start.S.

Your 32-byte header should not be in start.S, it should be linked in
before start.o in the linker script. Then you would only have one
start.S file and as many headers as you require.

And we could choose to do it using other methods, but they all require
us to maintain a customized version of linker scripts, or some other
code, or ....

Not necessarily. You'll need to configure your target, that's sure,
because I guess different targets will have different (values in
the bytes of their) headers, but you don't need to have different
start.S files.

The request here is that with the addition of some relatively
straightforward (upstreamed) code, then this can be handled
automatically by a post-processing step and we would be able to use a
pristine version of the upstreamed code...
Our desire is to get this into the beginnings of the "ARMv8" boards, so
that we can avoid the maintenance issues we have with the current ARMv7
boards.

We appreciate your consideration of this request.
Thanks, Steve

I (believe I) understand the problem, and I am discussing here because I
too want it solved. On the one hand I do agree with Wolfgang here:

No, I do not see a good reason to add such code.

... because I don't want to allow a feature that is designed to counter
a problem; but on the other hand I want the problem fixed. So Darwin,
in order to find a satisfactory solution, let me try to recap the
situation and then ask a few questions.

Recap:

1. A typical ARM binary image is linked to a base address, defined by
    preprocessor macro (aka U-Boot config option) CONFIG_SYS_TEXT_BASE.

OK


2. A typical ARM target defines CONFIG_SYS_TEXT_BASE so that the image,
    and especially its exception vectors table, is properly aligned upon
    loading the image in RAM.

Nope, the vectors respect the .align directive independent of the CONFIG_SYS_TEXT_BASE setting.


3. When the ARM image relocates itself, it does so to a destination
    address which is computed so that the image and its vectors table
    are still properly aligned after relocation.

Almost: the computed destination address is aligned on a 0x1000 byte boundary, so if the CONFIG_SYS_TEXT_BASE is also a multiple of 0x1000 bytes, then this statement is true.


4. Some targets require that the image stored in MMC be prepended with
    a 32-byte header (and aligned to a sector-related boundary).

OK


5. For some targets (if not all) the header is not separate from the
    image, i.e., it will also be copied from MMC to RAM. Such headers
    must be prepended at the link stage or earlier, so that the
    link-time and run-time values of all image symbols are consistent.

Yes - copied from MMC to RAM, but not really necessary to do this at "link stage or earlier" (we do it with a post-processing tool to prepend a header onto 'u-boot.bin')


6. However, because the header is put at the start of the image before
    the vectors table, the image is still properly aligned but th
    vectors tabled is not any more, both when the image is loaded from
    MMC to RAM, and when it is relocated.

Because we set the CONFIG_SYS_TEXT_BASE to 0xXXXX0020, the image is properly aligned, if CONFIG_SYS_TEXT_BASE is 0xXXXX0000, then the "before" image would not be properly aligned....


7. This mis-alignment issue does not only affect the vectors table; it
    also affects any location addressed using adrp instructions (if I
    did correctly understand Jeroen, Cc:).

Agree - it affects all of the "align" directives, in the code and the linker script file, and all instructions which assume alignment....

This is my understanding of the issue. If it is correct, then I
believe the following solves the issue without changing anything to
the current code:

Instead of prepending a 32-byte header to the image, prepend the header
then a block of 2048-32 bytes. This way, the MMC image base address
(the header address) is aligned, and the alignment of any address in
the image itself is preserved.

Yes - if the header is an exact multiple of 0x1000 (4096), it will work correctly without any code modifications... (not certain about 2048...)


If for some reason it is impossible to pad the header (such as, the
header is actually prepended by a closed-source tool which takes the
'raw' image in and spits the 'MMC image' out), then the raw image
should be prepended with 2048-32 bytes beforehand, so that the 32
header bytes make up a whole 2048-bytes block, and the image remains
aligned.

However, I guess that you, Darwin and Steve, probably have thought of
this already and dismissed it for a reason. If so, please explain
what this reason is.

We wanted to implement a forward looking solution that worked for any size of prepended header, and which removed the constraint for the CONFIG_SYS_TEXT_BASE to be an exact multiple of 0x1000 (4096).

However, since this solution does not look like it will succeed, in a parallel thread, we are pursuing an alternate proposal to conditionally reserve space for the header in u-boot.bin (which is "link stage or earlier"!!!), which would then be modified by a post-processing tool.
So please review that proposal.
Thanks, Steve


Amicalement,

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to