cmd: booti: fix the image runtime location

We should not use "bi_dram[0].start + text_offset" as the image dst. The text_offset maybe 0 for some images, such as XEN. Then the dst is actually bi_dram[0].start, which maybe the location of spin table.

Let's use "images->ep & ~(ih->text_offset)" as the dst address.

Signed-off-by Peng Fan <[email protected]>

Cc Tom Rini <[email protected]>

---
cmd/booti.c | 4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)

cmd/booti.c +3 -1

index 6c1c998..afc87e3 100644
@@ -54,7 +54,9 @@ static int booti_setup(bootm_headers_t *images)
 	 * If we are not at the correct run-time location, set the new
 	 * correct location and then move the image there.
 	 */
-	dst = gd->bd->bi_dram[0].start + le64_to_cpu(ih->text_offset);
+	dst = images->ep & ~(ih->text_offset);
+	if (dst < gd->bd->bi_dram[0].start)
+		dst = gd->bd->bi_dram[0].start + le64_to_cpu(ih->text_offset);
 
 	unmap_sysmem(ih);
 

Signature · 1 line

2.6.2

Quoted 5 lines · Peng Fan

We should not use "bi_dram[0].start + text_offset" as the image dst. The text_offset maybe 0 for some images, such as XEN. Then the dst is actually bi_dram[0].start, which maybe the location of spin table.

Let's use "images->ep & ~(ih->text_offset)" as the dst address.

This patch maybe not that correct according to the doc from Linux kernel. "
The Image must be placed text_offset bytes from a 2MB aligned base address anywhere in usable system RAM and called there. The region between the 2 MB aligned base address and the start of the image has no special significance to the kernel, and may be used for other purposes. "

Now I do not have a good idea that we may have a spin table in the start of DRAM or even a small firmware.

Is it better to change gd->bd->bi_dram[0].start?
For example physical dram starts from 0x80000000, but 4K is reserved at the beginning. So is it ok to change gd->bd->bi_dram[0].start to 0x80001000?

Quoted 15 lines · Peng Fan

Quoted 5 lines · Peng Fan

We should not use "bi_dram[0].start + text_offset" as the image dst. The text_offset maybe 0 for some images, such as XEN. Then the dst is actually bi_dram[0].start, which maybe the location of spin table.

Let's use "images->ep & ~(ih->text_offset)" as the dst address.

This patch maybe not that correct according to the doc from Linux kernel. "
The Image must be placed text_offset bytes from a 2MB aligned base address anywhere in usable system RAM and called there. The region between the 2 MB aligned base address and the start of the image has no special significance to the kernel, and may be used for other purposes. "

Quoted 6 lines

Now I do not have a good idea that we may have a spin table in the start of DRAM or even a small firmware.

Is it better to change gd->bd->bi_dram[0].start?
For example physical dram starts from 0x80000000, but 4K is reserved at the beginning. So is it ok to change gd->bd->bi_dram[0].start to 0x80001000?

So, to be more precise, with v4.5 the memory location restrictions were relaxed. So we first need to update cmd/booti.c to know that res1 is now 'flags' and that we must check bit3 to determine where / how to align the image. In the case of bit3 == 0 we must continue to do what we do today. In the case of bit3 == 1, we have to decide what exactly to do. My first thought is to see if images->ep is 2MB aligned and if so move to images->ep + ih->text_offset. If not 2MB aligned, align that and then add ih->text_offset.

Then any areas that need to be reserved in memory, wherever they are, can still be marked off as reserved in the DT.

Signature · 2 lines

-- 
Tom

Hi Tom,

Quoted 32 lines · Tom Rini

Quoted 15 lines · Peng Fan

Quoted 5 lines · Peng Fan

We should not use "bi_dram[0].start + text_offset" as the image dst. The text_offset maybe 0 for some images, such as XEN. Then the dst is actually bi_dram[0].start, which maybe the location of spin table.

Let's use "images->ep & ~(ih->text_offset)" as the dst address.

This patch maybe not that correct according to the doc from Linux kernel. "
The Image must be placed text_offset bytes from a 2MB aligned base address anywhere in usable system RAM and called there. The region between the 2 MB aligned base address and the start of the image has no special significance to the kernel, and may be used for other purposes. "

Quoted 6 lines

Now I do not have a good idea that we may have a spin table in the start of DRAM or even a small firmware.

Is it better to change gd->bd->bi_dram[0].start?
For example physical dram starts from 0x80000000, but 4K is reserved at the beginning. So is it ok to change gd->bd->bi_dram[0].start to 0x80001000?

So, to be more precise, with v4.5 the memory location restrictions were relaxed. So we first need to update cmd/booti.c to know that res1 is now 'flags' and that we must check bit3 to determine where / how to align the image. In the case of bit3 == 0 we must continue to do what we do today. In the case of bit3 == 1, we have to decide what exactly to do. My first thought is to see if images->ep is 2MB aligned and if so move to images->ep + ih->text_offset. If not 2MB aligned, align that and then add ih->text_offset.

Quoted 2 lines

Then any areas that need to be reserved in memory, wherever they are, can still be marked off as reserved in the DT.

How about the following patch?
If it is ok, I'll send out a formal one.

cmd/booti.c +1 -1

index 6c1c998..efa8bc8 100644
@@ -20,7 +20,7 @@ struct Image_header {
 	uint32_t	code1;		/* Executable code */
 	uint64_t	text_offset;	/* Image load offset, LE */
 	uint64_t	image_size;	/* Effective Image size, LE */
-	uint64_t	res1;		/* reserved */
+	uint64_t	flags;		/* Information flags, LE */
 	uint64_t	res2;		/* reserved */
 	uint64_t	res3;		/* reserved */
 	uint64_t	res4;		/* reserved */