Hi Tom,
On Mon, Sep 12, 2016 at 08:04:01AM -0400, Tom Rini wrote:
On Mon, Sep 12, 2016 at 05:18:53PM +0800, Peng Fan wrote:
On Mon, Sep 12, 2016 at 05:07:58PM +0800, Peng Fan wrote:
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?
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.
How about the following patch? If it is ok, I'll send out a formal one.
diff --git a/cmd/booti.c b/cmd/booti.c index 6c1c998..efa8bc8 100644 --- a/cmd/booti.c +++ b/cmd/booti.c @@ -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 */ @@ -29,6 +29,7 @@ struct Image_header { };
#define LINUX_ARM64_IMAGE_MAGIC 0x644d5241 +#define LINUX_PHYSICAL_PLACEMENT (0x1 << 3)
static int booti_setup(bootm_headers_t *images) { @@ -54,7 +55,11 @@ 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 = gd->bd->bi_dram[0].start; + if (le64_to_cpu(ih->flags) & LINUX_PHYSICAL_PLACEMENT) + dst = round_up(images->ep, SZ_2M); + + dst += le64_to_cpu(ih->text_offset);
unmap_sysmem(ih);
Thanks, Peng.
-- Tom