Dear Darwin,

In message 5374CD55.3010703@broadcom.com you wrote:

...

Do you really want to check a define at runtime? Placement is typically at the end of RAM and allocation goes down, not up as in this patch. Aren't you overlapping memory here?

Yes, I wanted the runtime check since the adjustment to the relocation address is also done at runtime.

This makes no sense to me. CONFIG_SYS_TEXT_BASE is a compile time constant. So the result of all this is always known at compile time, too. I feel you misunderstand that CONFIG_SYS_TEXT_BASE is just the start address of the text segment. If you want to offset this by a specific amount, you can just define this as needed.

There is no overlap here. The reason is that the original masking operation to mask to a 4K boundary removed the small offset and backed up too far. So adding the lost offset is guaranteed to not overlap, and furthermore, correct the relocation offset so that arm64 images can run at smaller alignments than we normally use. This might even be a generic fix but can't be tested easily by me.

Argh... This is black magic depending on specific properties of your process (which you don;t really explain). Sorry, but this is a full NAK for code that is build on sand like this.

Best regards,

Wolfgang Denk

Dear Darwin,

In message 5374E64B.1060104@broadcom.com you wrote:

...

This makes no sense to me. CONFIG_SYS_TEXT_BASE is a compile time constant. So the result of all this is always known at compile time, too. I feel you misunderstand that CONFIG_SYS_TEXT_BASE is just the start address of the text segment. If you want to offset this by a specific amount, you can just define this as needed.

May I respectfully ask that you please bear with me a just a bit longer so I can try to explain things better?

CONFIG_SYS_TEXT_BASE is also used by the relocation calculations at runtime to determine the relocation offset, which is also used for the symbol fixup logic. It is known at compile time, but is used at runtime by the stock u-boot code to determine the relocation offset. I am doing nothing more than existing code in this regard.

Let's have a look at your proposed code:

...

• int offset = CONFIG_SYS_TEXT_BASE % 4096;
  

• if (offset) {
  

• 	addr += offset;
  

• 	debug("Relocation Addr bumped to 0x%08lx\n", addr);
  

• }
  

If I set it to 0x88000020, then the code crashes because the symbol fixup depends on the relocation offset which is now wrong. The reason is that the relocation offset calculated in the code doesn't account for this offset. If we adjust the relocation address, then the relocation offset is now correct and the symbol fixups work perfectly.

First, please keep in mind that you cannot set CONFIG_SYS_TEXT_BASE in arbitrary ways, at least not without adjusting your linker script accordingly; also you may have limitations due to the way how the code gets loaded / booted.

_If_ your code is correct, then the target address computed for the relocation is completely irrelevant.

Here's an example: CONFIG_SYS_TEXT_BASE = 0x88000020 relocated address = 0xfffa8000 (This 4K alignment assumption breaks the relocation) relocation offset = 0x77fa7fe0 (This is now wrong and the relocation breaks)

My patch does the following: CONFIG_SYS_TEXT_BASE = 0x88000020 relocated address = 0xfffa8020 relocation offset = 0x77fa8000 (symbol fixups now work)

You are mixing up copying code to another address range and relocating symbols.

So this patch just gives us a way to run u-boot at text sections with smaller alignments. In the past I never really understood why CONFIG_SYS_TEXT_BASE had to be on specific alignments like XXXXX000 for our architecture and the symbol fixup issue helps to explain this.

In many cases there is no strict reason. When storing the images in NOR flash or other block oriented storage it is usually convenient to align them to sector boundaaries. In systems where you have a MMU on, it may be useful / convenient / necessary to have it aligned to page boundaries. But al this is totally irrelevant here. Consider the address arbitrarily chosen, it does not matter.

Also, moving the relocation address around by arbitrary amounts (that are big enough not to cause alignment issues, say anything that is a multiple of the cache line size) does not change anything.

If there are problems, these are somewhere in your implementation, and not in the generic code.

I know this is not the normal use case but it does fix the crash in the symbol relocation with arm64 and allows u-boot to be more flexible in it's text alignment requirements.

You ask for a "flexibility" that actually is already there; it's just conveniend when debugging etc. to have "nice" addresses. If there is something not working, it's in your code.

Setting CONFIG_SYS_TEXT_BASE to something like 0x88000020 is extremly fishy. If you want to add some header data to your image, you should not shift the text segment, but rather include your header in the start of the text segment. Or keep it completely separate, without messing with CONFIG_SYS_TEXT_BASE.

Best regards,

Wolfgang Denk

Hi Albert,

The previous stage bootloader (which I had no control over) wanted it's header to be aligned to a 512 byte MMC block boundary, presumably since this allowed DMA operations without copy/shifting. At the same time, I didn't want to hack a header into start.S because I didn't want to carry another downstream patch. So I investigated if I could shift u-boot's base address as a feature that would allow an aligned header to be used without the start.S patch.

I know that a custom header patch to start.S would work, and that a header plus padding will also work. But I found out that you can align the base on certain smaller offsets if you keep the relocation offset at nice boundaries like 0x1000 and if the relocation offset is a multiple of the maximum alignment requirements of the image.

The original patch I submitted didn't handle an end condition properly, was ARM64-specific (wasn't tested on other architectures), and because the patch was NAK'd, I didn't bother to submit a v2 patch and consider the idea to be dead. I'm happy to abandon the patch. I hope this helps.

Best regards, Darwin

On 14-05-26 02:50 AM, Albert ARIBAUD wrote:

Hi Wolfgang, Darwin,

On Thu, 15 May 2014 21:19:57 +0200, Wolfgang Denk wd@denx.de wrote:

...

Setting CONFIG_SYS_TEXT_BASE to something like 0x88000020 is extremly fishy. If you want to add some header data to your image, you should not shift the text segment, but rather include your header in the start of the text segment. Or keep it completely separate, without messing with CONFIG_SYS_TEXT_BASE.

Back to the origin of the discussion and patch:

Darwin, can you describe the actual technical difficulty which caused you to you write and submitting this patch?

...

Best regards,

Wolfgang Denk

Amicalement,

On 14-06-02 12:26 AM, Albert ARIBAUD wrote:

Hi Darwin,

On Mon, 26 May 2014 09:11:35 -0700, Darwin Rambo drambo@broadcom.com wrote:

...

Hi Albert,

The previous stage bootloader (which I had no control over) wanted it's header to be aligned to a 512 byte MMC block boundary, presumably since this allowed DMA operations without copy/shifting. At the same time, I didn't want to hack a header into start.S because I didn't want to carry another downstream patch. So I investigated if I could shift u-boot's base address as a feature that would allow an aligned header to be used without the start.S patch.

I know that a custom header patch to start.S would work, and that a header plus padding will also work. But I found out that you can align the base on certain smaller offsets if you keep the relocation offset at nice boundaries like 0x1000 and if the relocation offset is a multiple of the maximum alignment requirements of the image.

The original patch I submitted didn't handle an end condition properly, was ARM64-specific (wasn't tested on other architectures), and because the patch was NAK'd, I didn't bother to submit a v2 patch and consider the idea to be dead. I'm happy to abandon the patch. I hope this helps.

Thanks.

If I understand correctly, your target has a requirement for storing the image on a 512-byte boundary. But how does this affect the loading of the image into RAM, where the requirement is only that the vectors table be 32-bytes aligned? I mean, if you store the image in MMC at offset 0x200 (thus satisfying the 512-byte boundary requirement) and load it to, say, offset 0x10020 in RAM, how is it a problem for your target?

If my example above inadequately represents the issue, then can you please provide a similar but adequate example, a failure case scenario, so that I can hve a correct understanding of the problem?

Hi Albert,

The constraints I have that I can't change, are that - the 32 byte header is postprocessed and prepended to the image after the build is complete - the header is at a 512 byte alignment in MMC - the header and image are copied to SDRAM to an alignment like 0x88000000. Thus the u-boot image is linked at and starts at 0x88000020. - the vectors need to be 0x800 aligned for armv8 (.align 11 directive)

So the failure case is that when the relocation happens, it relocates to a 0x1000 alignment, say something like 0xffffa000. The relocation offset is not a multiple of 0x1000 (0xffffa000 - 0x88000020) and the relocation fails. Adjusting the relocation offset to a multiple of 0x1000 (by making the relocation address end in 0xNNNNN020) fixes the issues and allows u-boot to relocate and run from this address without failing. I hope this helps explain it a bit better.

Best regards, Darwin

...

Best regards, Darwin

Amicalement,

On 14-06-09 03:23 AM, Albert ARIBAUD wrote:

Hi Darwin,

On Mon, 2 Jun 2014 17:37:25 -0700, Darwin Rambo drambo@broadcom.com wrote:

...

On 14-06-02 12:26 AM, Albert ARIBAUD wrote:

...

Hi Darwin,

On Mon, 26 May 2014 09:11:35 -0700, Darwin Rambo drambo@broadcom.com wrote:

...

Hi Albert,

The previous stage bootloader (which I had no control over) wanted it's header to be aligned to a 512 byte MMC block boundary, presumably since this allowed DMA operations without copy/shifting. At the same time, I didn't want to hack a header into start.S because I didn't want to carry another downstream patch. So I investigated if I could shift u-boot's base address as a feature that would allow an aligned header to be used without the start.S patch.

I know that a custom header patch to start.S would work, and that a header plus padding will also work. But I found out that you can align the base on certain smaller offsets if you keep the relocation offset at nice boundaries like 0x1000 and if the relocation offset is a multiple of the maximum alignment requirements of the image.

The original patch I submitted didn't handle an end condition properly, was ARM64-specific (wasn't tested on other architectures), and because the patch was NAK'd, I didn't bother to submit a v2 patch and consider the idea to be dead. I'm happy to abandon the patch. I hope this helps.

Thanks.

If I understand correctly, your target has a requirement for storing the image on a 512-byte boundary. But how does this affect the loading of the image into RAM, where the requirement is only that the vectors table be 32-bytes aligned? I mean, if you store the image in MMC at offset 0x200 (thus satisfying the 512-byte boundary requirement) and load it to, say, offset 0x10020 in RAM, how is it a problem for your target?

If my example above inadequately represents the issue, then can you please provide a similar but adequate example, a failure case scenario, so that I can hve a correct understanding of the problem?

Hi Albert,

The constraints I have that I can't change, are that

• the 32 byte header is postprocessed and prepended to the image after

the build is complete

• the header is at a 512 byte alignment in MMC
• the header and image are copied to SDRAM to an alignment like

0x88000000. Thus the u-boot image is linked at and starts at 0x88000020.

• the vectors need to be 0x800 aligned for armv8 (.align 11 directive)

So far, so good -- I understand that the link-time location of the vectors table is incorrect.

...

So the failure case is that when the relocation happens, it relocates to a 0x1000 alignment, say something like 0xffffa000. The relocation offset is not a multiple of 0x1000 (0xffffa000 - 0x88000020) and the relocation fails.

What does "relocation fails" mean exactly, i.e., where and how exactly does the relocation code behave differently from expected? I'm asking because I don't understand why the relocation offset should be a multiple of 0x1000.

...

Adjusting the relocation offset to a multiple of 0x1000 (by making the relocation address end in 0xNNNNN020) fixes the issues and allows u-boot to relocate and run from this address without failing. I hope this helps explain it a bit better.

I do understand, however, that if the relocation offset must indeed be a multiple of 0x1000, then obviously the vectors table will end up as misaligned as it was before relocation.

Also, personally I would like it if the vectors table was always aligned as it should, and there are at least three other boards which require a prefix/header before their vectors table, as Masahiro (cc:) indicated recently, so that makes the problem a generic one: how to properly integrate a header in-image (as opposed to an out-of-image one, which is just a matter of doing a 'cat', so to speak.

Therefore I'd like a generic solution to this, where the header is prepended *and* aligned properly without breaking the start symbol alignment constraints. This /might/ be possible by cleverly adding a '.header' or '.signature' section to the linker script, possibly doing a two-stage link; but this should not require the source code to contain ad hoc relocation tricks.

Let me tinker with it in the next few days; I'll try and come up with a clean and generic solution to this "in-code header" question.

Thanks again for your explanation!

...

Best regards, Darwin

Amicalement,

Perhaps an oversimplified example of the current code would help to explain this better:

scenario #1: CONFIG_SYS_TEXT_BASE 0x88000000 vectors: .align 11 /* exception vectors need to be on a 0x800 byte boundary */ compile/linker produces (before relocation): _start symbol is at 0x88000000 vectors symbol is at 0x88000800 the relocation offset is: 0x77f9b000 therefore, after relocation: _start symbol is at 0xfff9b000 (0x88000000+0xfff9b000) vectors symbol is at 0xfff9b800 (0x88000800+0x77f9b000)

scenario #2: CONFIG_SYS_TEXT_BASE 0x88000020 vectors: .align 11 /* exception vectors need to be on a 0x800 byte boundary */ compiler/linker produces (before relocation): _start symbol is at 0x88000020 vectors symbol is at 0x88000800 the relocation offset is: 0x77f9afe0 therefore, after relocation: _start symbol is at 0xfff9b000 (0x88000020+0x77f9afe0) vectors symbol is at 0xfff9b7e0 (0x88000800+0x77f9afe0)

Note that in scenario #2, after relocation, the vectors are not on a 0x800 byte boundary anymore.

Thanks, Steve

Hi Steve,

(sorry for the duplicate)

On Mon, 9 Jun 2014 13:45:50 -0700, Steve Rae srae@broadcom.com wrote:

On 14-06-09 03:23 AM, Albert ARIBAUD wrote:

...

Hi Darwin,

On Mon, 2 Jun 2014 17:37:25 -0700, Darwin Rambo drambo@broadcom.com wrote:

...

On 14-06-02 12:26 AM, Albert ARIBAUD wrote:

...

Hi Darwin,

On Mon, 26 May 2014 09:11:35 -0700, Darwin Rambo drambo@broadcom.com wrote:

...

Hi Albert,

The previous stage bootloader (which I had no control over) wanted it's header to be aligned to a 512 byte MMC block boundary, presumably since this allowed DMA operations without copy/shifting. At the same time, I didn't want to hack a header into start.S because I didn't want to carry another downstream patch. So I investigated if I could shift u-boot's base address as a feature that would allow an aligned header to be used without the start.S patch.

I know that a custom header patch to start.S would work, and that a header plus padding will also work. But I found out that you can align the base on certain smaller offsets if you keep the relocation offset at nice boundaries like 0x1000 and if the relocation offset is a multiple of the maximum alignment requirements of the image.

The original patch I submitted didn't handle an end condition properly, was ARM64-specific (wasn't tested on other architectures), and because the patch was NAK'd, I didn't bother to submit a v2 patch and consider the idea to be dead. I'm happy to abandon the patch. I hope this helps.

Thanks.

If I understand correctly, your target has a requirement for storing the image on a 512-byte boundary. But how does this affect the loading of the image into RAM, where the requirement is only that the vectors table be 32-bytes aligned? I mean, if you store the image in MMC at offset 0x200 (thus satisfying the 512-byte boundary requirement) and load it to, say, offset 0x10020 in RAM, how is it a problem for your target?

If my example above inadequately represents the issue, then can you please provide a similar but adequate example, a failure case scenario, so that I can hve a correct understanding of the problem?

Hi Albert,

The constraints I have that I can't change, are that

• the 32 byte header is postprocessed and prepended to the image after

the build is complete

• the header is at a 512 byte alignment in MMC
• the header and image are copied to SDRAM to an alignment like

0x88000000. Thus the u-boot image is linked at and starts at 0x88000020.

• the vectors need to be 0x800 aligned for armv8 (.align 11 directive)

So far, so good -- I understand that the link-time location of the vectors table is incorrect.

...

So the failure case is that when the relocation happens, it relocates to a 0x1000 alignment, say something like 0xffffa000. The relocation offset is not a multiple of 0x1000 (0xffffa000 - 0x88000020) and the relocation fails.

What does "relocation fails" mean exactly, i.e., where and how exactly does the relocation code behave differently from expected? I'm asking because I don't understand why the relocation offset should be a multiple of 0x1000.

...

Adjusting the relocation offset to a multiple of 0x1000 (by making the relocation address end in 0xNNNNN020) fixes the issues and allows u-boot to relocate and run from this address without failing. I hope this helps explain it a bit better.

I do understand, however, that if the relocation offset must indeed be a multiple of 0x1000, then obviously the vectors table will end up as misaligned as it was before relocation.

Also, personally I would like it if the vectors table was always aligned as it should, and there are at least three other boards which require a prefix/header before their vectors table, as Masahiro (cc:) indicated recently, so that makes the problem a generic one: how to properly integrate a header in-image (as opposed to an out-of-image one, which is just a matter of doing a 'cat', so to speak.

Therefore I'd like a generic solution to this, where the header is prepended *and* aligned properly without breaking the start symbol alignment constraints. This /might/ be possible by cleverly adding a '.header' or '.signature' section to the linker script, possibly doing a two-stage link; but this should not require the source code to contain ad hoc relocation tricks.

Let me tinker with it in the next few days; I'll try and come up with a clean and generic solution to this "in-code header" question.

Thanks again for your explanation!

...

Best regards, Darwin

Amicalement,

Perhaps an oversimplified example of the current code would help to explain this better:

scenario #1: CONFIG_SYS_TEXT_BASE 0x88000000 vectors: .align 11 /* exception vectors need to be on a 0x800 byte boundary */ compile/linker produces (before relocation): _start symbol is at 0x88000000 vectors symbol is at 0x88000800 the relocation offset is: 0x77f9b000 therefore, after relocation: _start symbol is at 0xfff9b000 (0x88000000+0xfff9b000) vectors symbol is at 0xfff9b800 (0x88000800+0x77f9b000)

scenario #2: CONFIG_SYS_TEXT_BASE 0x88000020 vectors: .align 11 /* exception vectors need to be on a 0x800 byte boundary */ compiler/linker produces (before relocation): _start symbol is at 0x88000020 vectors symbol is at 0x88000800 the relocation offset is: 0x77f9afe0 therefore, after relocation: _start symbol is at 0xfff9b000 (0x88000020+0x77f9afe0) vectors symbol is at 0xfff9b7e0 (0x88000800+0x77f9afe0)

Note that in scenario #2, after relocation, the vectors are not on a 0x800 byte boundary anymore.

Ok, so technically relocation works as it was told to, just was given "garbage in", with an image with an unaligned base and _start symbol (and an aligned vectors symbol); relocation worked as expected, and aligned base and start, thus not aligning vectors.

Now, on to understanding why the CONFIG_SYS_TEXT_BASE is not aligned.

Can you provide details from the case where the actual issue has arisen, or even better yet, provide a source tree and build command line, that I could reproduce the build here?

Thanks, Steve

Amicalement,

Dear Steve,

In message 539746C4.9040004@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 ?

Then everything should be still the same for you, and no voodoo coding would be needed.

(3) HOWEVER, shifting the address UP may cause the end of the relocated code to run past the end of the available memory... So we could:

This problem is void if you just use a poperly aligned text base in combination with a prope start.S resp. linker script to make sure _start is where you want it.

I trust that everyone will find this explanation acceptable...

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

Best regards,

Wolfgang Denk

Dear Steve,

In message 53975EC2.1080209@broadcom.com you wrote:

...

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...

There should be more clever ways to implement this. If nothing else comes to mind, an #ifdef in "arch/arm/cpu/armv7/start.S" should be sufficient to condistionally insert / adjust any offset that might be needed for a specific board.

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 ....

... or a CONFIG setting in your board config file resp. your board's defconfig.

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...

I'm sorry, but I disagree especially on the "straightforward" part. Also, I see no reason to make the existing code unnecessarily complex, and add more disadvantages (like increased meory footprint etc.) when the same purpose can be implemented without adding any special code at all.

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.

These are two different things: implementing a clean and easy way to support a necessary feature is one thing; to do it in the suggested way by adding code where none is needed is another thing.

I'm all for adding support for any features that are useful, even if only for a single user, as long as they don't hurt other users. All I'm asking is to chose another way to implement this feature. So far, I did not see any arguments that my alternative proposals would cause any problems to you?

Best regards,

Wolfgang Denk

Dear Steve,

In message 53979199.5010100@broadcom.com you wrote:

OK - I think that one of the alternate proposals would be to conditionally reserve a "32 byte block" prior to the _start symbol (in "arch/arm/cpu/armv8/start.S") which would then be filled in by a post-processing step... This could be implemented by:

Yes, that illustrates the idea. However, this implementation suffers from the use of an #ifdef where none is actually needed. Instead, you can create your own source file which defines the header; this could be then even in it's own segment, say:

your_header.c:

struct your_header { u_int32[8]; } your_header __attribute__ ((__section__ (".your_hdr")));

All that is needed then is to make the linker place this segment in front of the text segment.

This avoids an ugly #ifdef, and also modifications in the common code.

Best regards,

Wolfgang Denk

On 14-06-10 11:45 PM, Albert ARIBAUD wrote:

Hi Wolfgang,

On Wed, 11 Jun 2014 06:49:28 +0200, Wolfgang Denk wd@denx.de wrote:

...

Dear Steve,

In message 53979199.5010100@broadcom.com you wrote:

...

OK - I think that one of the alternate proposals would be to conditionally reserve a "32 byte block" prior to the _start symbol (in "arch/arm/cpu/armv8/start.S") which would then be filled in by a post-processing step... This could be implemented by:

Yes, that illustrates the idea. However, this implementation suffers from the use of an #ifdef where none is actually needed. Instead, you can create your own source file which defines the header; this could be then even in it's own segment, say:

your_header.c:

struct your_header { u_int32[8]; } your_header __attribute__ ((__section__ (".your_hdr")));

All that is needed then is to make the linker place this segment in front of the text segment.

This avoids an ugly #ifdef, and also modifications in the common code.

Agreed and seconded.

Plus, using a dedicated 'header' section and a separate C source file for the header makes it automatic that if no input 'header' section were provided then no output 'header' section would be emitted; IOW, we would not need two different linker scripts, a single one would be useable for both 'headerless' and 'headerful' image types.

Also, the alignment constraint should be configurable.

...

Best regards,

Wolfgang Denk

Amicalement,

Albert, Wolfgang, et al.

I didn't know about the automatic handling of "conditional" sections in the linker script file - Thanks!!!

So if I add a "your_header.c" as above, then

(1) I need to modify "arch/arm/cpu/armv8/u-boot.lds": . = 0x00000000;

+ . = ALIGN(8); + .your_hdr : { + KEEP(*(.your_hdr*)); + } + . = ALIGN(8); .text : {

(2) then (I believe) I need to modify the "Makefile" to define the start address of this new section: +LDFLAGS_u-boot += --section-start=".your_hdr"=CONFIG_YOUR_HEADER_ADDR

(3) and (I believe) I need to modify the OBJCOPYFLAGS (somewhere) in order to include this new section in the u-boot.bin: +OBJCOPYFLAGS += -j .your_hdr

(... I don't actually have this working yet; so I suspect more changes are required ...)

And in the end, (I believe) I am just going to have a block (likely 4096 bytes) prepended to the "original" u-boot.bin; which I can do today with no code changes at all....

Remember that original design request was effectively a two line change: + gd->mon_len += CONFIG_SYS_TEXT_BASE % 4096; and + gd->relocaddr += CONFIG_SYS_TEXT_BASE % 4096;

Regrettably, since this is not going to be accepted, I am abandoning this change. Thanks, Steve

Hi Wolfgang,

On Wed, 11 Jun 2014 23:16:51 +0200, Wolfgang Denk wd@denx.de wrote:

Dear Steve,

In message 5398A640.3050105@broadcom.com you wrote:

...

So if I add a "your_header.c" as above, then

(1) I need to modify "arch/arm/cpu/armv8/u-boot.lds": . = 0x00000000;

• . = ALIGN(8);
• .your_hdr : {

• KEEP(*(.your_hdr*));
  

• }
• . = ALIGN(8); .text : {

ALIGN(8) looks pretty much bogus to me?

Quoting the "ld" docs:

'ALIGN(ALIGN)' 'ALIGN(EXP,ALIGN)' Return the location counter ('.') or arbitrary expression aligned to the next ALIGN boundary. The single operand 'ALIGN' doesn't change the value of the location counter--it just does arithmetic on it. The two operand 'ALIGN' allows an arbitrary expression to be aligned upwards ('ALIGN(ALIGN)' is equivalent to 'ALIGN(., ALIGN)').

 Here is an example which aligns the output '.data' section to the
 next '0x2000' byte boundary after the preceding section and sets a
 variable within the section to the next '0x8000' boundary after the
 input sections:
      SECTIONS { ...
        .data ALIGN(0x2000): {
          *(.data)
          variable = ALIGN(0x8000);
        }
      ... }
 The first use of 'ALIGN' in this example specifies the location of
 a section because it is used as the optional ADDRESS attribute of a
 section definition (*note Output Section Address::).  The second
 use of 'ALIGN' is used to defines the value of a symbol.

Are you sure you do not ant to hae an ALIGN(4096) there?

Better yet, align to some CONFIG_SYS_ALIGN_xxx option. Linker scripts are preprocessed, so that should be a breeze.

...

(2) then (I believe) I need to modify the "Makefile" to define the start address of this new section: +LDFLAGS_u-boot += --section-start=".your_hdr"=CONFIG_YOUR_HEADER_ADDR

Why?

I see no reason for this indeed.

...

(3) and (I believe) I need to modify the OBJCOPYFLAGS (somewhere) in order to include this new section in the u-boot.bin: +OBJCOPYFLAGS += -j .your_hdr

Why?

I can see reasons for this one, but it depends on whether the C file for the added section produces the section's content or just reserves space for it.

If the C file just reserves space, we can dispense with copying it from ELF to bin and the -j option above needs not be added (and then, an external utility should prepend the actual header content to the .bin).

If the C file produces the content, then we *must* copy it from ELF to bin and the -j option above (and any utility that wants to set the section's content should overwrite it, not prepend to it).

I am not personally sure which option is best.

TBH, my personal preference is that if a header must be added to the image, it should not affect at all the image build, load and run addresses... For instance, if the image's text section start is 0x00100000 and its entry point is 0x00100100, and we need to add a 4KB header to it, the resulting biary should *still* load so that the original image's text segment starts at 0x00100000; if what is loaded is the pair header+image, then it should load at 0x000FF000 (and the entry point would still be 0x00100100).

But I can understand that some platfoms /will/ load the header along with the image, and that in this case, we need to take the header into account in the build process.

Steve: can the header content be expressed entirely in the source C module? If it can, then there might be no need for an external utility to set it -- but that's up for debate, as it might be useful to be able to modify the header without rebuilding anyway.

...

And in the end, (I believe) I am just going to have a block (likely 4096 bytes) prepended to the "original" u-boot.bin; which I can do today with no code changes at all....

Well, we don't chaneg any code here, right? Just the linker script. It's this, or some other script. But the linker is the standard way to create a linked image with the correct memory map, so this is the right place...

Just nitpicking here, but...

The problem we were presented with initially was as follows:

link some code for a given run address; prepend resulting image with a 4KB header; load prepended header followed by image at original image run address. (iow, load original image 4KB above intended run address)

With the code as it is, we know this fails. Ergo, whatever solution fixes the issue will *necessarily* change the code. The initial proposal was to try and put the change in the relocation process; the final one puts the change the linker script -- which BTW *is* code, at least in my book, since it is input to the build process and controls the content of the generated image.

...

Remember that original design request was effectively a two line change:

Yes, indeed But a pretty much bogus one.

Best regards,

Wolfgang Denk

Amicalement,

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

Hi Steve,

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

...

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

...

Dear Steve,

In message 539746C4.9040004@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,

Dear Albert,

In message E1WuTT8-0007AF-9l@janus you wrote:

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.

We shoul specifically keep in mind here that the "base address" is defined as the start address of the text segment, which may or may not be the same as the entry point address _start. In general, these are not the same.

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.

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.

These items are not ARM specific.

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

Why would such headers have to be part of the text segment?

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.

If the headers are in their own segment, this should be trivial.

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.

This would be a bug, then. If we have an alignment requirement for the text segment, and we load other data (or code) in front of the text segment, then the size of such additional data must be made such that the text segment is still properly aligned. Usually I would expect that the heaer is in a separate segment, and the linker script would take care of the required alignment for the text segment and insert the necessary gap to keep it properly aligned.

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.

Agreed. Or should .text be aligned on a 4 k boundary?

Best regards,

Wolfgang Denk

On 14-05-14 09:26 PM, Wolfgang Denk wrote:

Dear Darwin Rambo,

In message 1400105145-6628-1-git-send-email-drambo@broadcom.com you wrote:

...

If an earlier loader stage requires an image header and a specific offset, then u-boot's base address (CONFIG_SYS_TEXT_BASE) may be advanced beyond an aligned address. In this case the relocation

Sorry, I cannot parse that. CONFIG_SYS_TEXT_BASE is a compile time constant, it cannot be "advanced" by a loader. Do you mean that some loader loads U-Boot to an incorrect address? Well, in this case the loader should be fixed, or?

Thank you for your comments.

I mean that the loader loads u-boot to it's correct address, which is offset by a small amount because of a previous header requiring alignment. Here's an example. u-boot is compiled to run at 0x88000020 because we want to put a small header in front of the image, which starts at 0x88000000 and needs to be aligned for its own reasons. Now for arm64, I believe that u-boot cannot normally be positioned at any alignment less than 0x800 hex. So u-boot would normally run at addresses like 0x88000000, 0x88000800, 0x88001000, etc. And in these cases the relocation works fine. But if we want to position u-boot at a smaller offset than 0x800, the symbol relocation breaks for arm64. It turns out that there is a trivial fix so that u-boot can run at smaller offset addresses, which I have provided here, is tested, and solves our problem nicely, but only for arm64 right now.

...

This change is done under CONFIG_ARM64 conditional compilation because it has only been tested there and may not be appropriate for other architectures.

In any case, any such changes (if there should be agreement that they are actually useful) should be done in an architecture-neutral way. Implementing it for one specific architecture only is wrong.

Yes, I agree, but I am not sure if this is a arm64-only problem or not. Armv7 doesn't show this problem, and I can't test other architectures for their alignment issues. So I thought that I would at least show the fix for arm64 so we can decide if and how to proceed. Any suggestions you can provide on how to proceed would be appreciated. And if the fix is not suitable for upstreaming, then we should know it.

Is there a way to have architecture specific hooks like this called from the generic common/board_f.c? The fix is also in arch/arm/lib/board.c but it sounds like that file might be disappearing.

Also there might be a generic fix possible that works for all architectures (by removing the ifdef CONFIG_ARM64), but I don't have the resources to test them. Maybe it would be best to decide if we want to support this feature or not first. Thanks!

Regards, Darwin

Best regards,

Wolfgang Denk