* [U-Boot] [PATCH] cmd: booti: fix the image runtime location @ 2016-09-12 9:07 Peng Fan 2016-09-12 9:18 ` Peng Fan 0 siblings, 1 reply; 5+ messages in thread From: Peng Fan @ 2016-09-12 9:07 UTC (permalink / raw) To: u-boot 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 <peng.fan@nxp.com> Cc: Tom Rini <trini@konsulko.com> --- cmd/booti.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/booti.c b/cmd/booti.c index 6c1c998..afc87e3 100644 --- a/cmd/booti.c +++ b/cmd/booti.c @@ -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); -- 2.6.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] cmd: booti: fix the image runtime location 2016-09-12 9:07 [U-Boot] [PATCH] cmd: booti: fix the image runtime location Peng Fan @ 2016-09-12 9:18 ` Peng Fan 2016-09-12 12:04 ` Tom Rini 0 siblings, 1 reply; 5+ messages in thread From: Peng Fan @ 2016-09-12 9:18 UTC (permalink / raw) To: u-boot 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? Thanks, Peng. > >Signed-off-by: Peng Fan <peng.fan@nxp.com> >Cc: Tom Rini <trini@konsulko.com> >--- > cmd/booti.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff --git a/cmd/booti.c b/cmd/booti.c >index 6c1c998..afc87e3 100644 >--- a/cmd/booti.c >+++ b/cmd/booti.c >@@ -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); > >-- >2.6.2 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] cmd: booti: fix the image runtime location 2016-09-12 9:18 ` Peng Fan @ 2016-09-12 12:04 ` Tom Rini 2016-09-13 6:02 ` Peng Fan 0 siblings, 1 reply; 5+ messages in thread From: Tom Rini @ 2016-09-12 12:04 UTC (permalink / raw) To: u-boot 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. -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160912/1a5ff6e4/attachment.sig> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] cmd: booti: fix the image runtime location 2016-09-12 12:04 ` Tom Rini @ 2016-09-13 6:02 ` Peng Fan 2016-09-13 11:19 ` Tom Rini 0 siblings, 1 reply; 5+ messages in thread From: Peng Fan @ 2016-09-13 6:02 UTC (permalink / raw) To: u-boot 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@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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] cmd: booti: fix the image runtime location 2016-09-13 6:02 ` Peng Fan @ 2016-09-13 11:19 ` Tom Rini 0 siblings, 0 replies; 5+ messages in thread From: Tom Rini @ 2016-09-13 11:19 UTC (permalink / raw) To: u-boot On Tue, Sep 13, 2016 at 02:02:53PM +0800, Peng Fan wrote: > 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. Pretty close: > > 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) Just '1' not '0x1' > > 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); Can you do this as if/else and expand the comment so that it explains that earlier it was recommended to be as close to the start of memory as possible but later a flag was introduced to allow a more flexible placement? Thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160913/d95dab95/attachment.sig> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-13 11:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-09-12 9:07 [U-Boot] [PATCH] cmd: booti: fix the image runtime location Peng Fan 2016-09-12 9:18 ` Peng Fan 2016-09-12 12:04 ` Tom Rini 2016-09-13 6:02 ` Peng Fan 2016-09-13 11:19 ` Tom Rini
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.