| Message ID | 1459353236-153290-1-git-send-email-agraf@suse.de |
|---|---|
| State | Accepted |
| Commit | 3ffe39ed2b66af71c7271d0cef2a248b5bf7dfdb |
| Delegated to: | Hans de Goede |
| Headers | show |
On Wed, 2016-03-30 at 17:53 +0200, Alexander Graf wrote: > On the A64 we usually boot with ATF running in EL3. ATF as it is available > today resides in the first 16MB of RAM. So we should make sure we reserve > that space in our memory maps. Would using fdt_add_mem_rsv() be better than fiddling with the DRAM banks? Ian.
On 01.04.16 13:06, Ian Campbell wrote: > On Wed, 2016-03-30 at 17:53 +0200, Alexander Graf wrote: >> On the A64 we usually boot with ATF running in EL3. ATF as it is available >> today resides in the first 16MB of RAM. So we should make sure we reserve >> that space in our memory maps. > > Would using fdt_add_mem_rsv() be better than fiddling with the DRAM > banks? Nope, because that wouldn't populate into the EFI memory descriptors ;). Alex
On Fri, 2016-04-01 at 13:08 +0200, Alexander Graf wrote: > > On 01.04.16 13:06, Ian Campbell wrote: > > > > On Wed, 2016-03-30 at 17:53 +0200, Alexander Graf wrote: > > > > > > On the A64 we usually boot with ATF running in EL3. ATF as it is available > > > today resides in the first 16MB of RAM. So we should make sure we reserve > > > that space in our memory maps. > > Would using fdt_add_mem_rsv() be better than fiddling with the DRAM > > banks? > Nope, because that wouldn't populate into the EFI memory descriptors ;). Isn't that a bug? ;-) Ian.
On 01.04.16 13:12, Ian Campbell wrote: > On Fri, 2016-04-01 at 13:08 +0200, Alexander Graf wrote: >> >> On 01.04.16 13:06, Ian Campbell wrote: >>> >>> On Wed, 2016-03-30 at 17:53 +0200, Alexander Graf wrote: >>>> >>>> On the A64 we usually boot with ATF running in EL3. ATF as it is available >>>> today resides in the first 16MB of RAM. So we should make sure we reserve >>>> that space in our memory maps. >>> Would using fdt_add_mem_rsv() be better than fiddling with the DRAM >>> banks? >> Nope, because that wouldn't populate into the EFI memory descriptors ;). > > Isn't that a bug? ;-) I guess to make this clean we'd need a new API that both calls fdt_add_mem_rsv() and an efi allocate. And yes, it's a bug :). Alex
Hi, sorry for the late reply, just found your series here. On 30/03/16 16:53, Alexander Graf wrote: > On the A64 we usually boot with ATF running in EL3. ATF as it is available > today resides in the first 16MB of RAM. So this is actually a mistake Allwinner made and which we haven't fixed yet. ATF (at least BL3-1, which is the runtime service part we use for the A64) should at least run in secure memory, actually in secure SRAM. Having it in DRAM is a kludge, unnecessary (it's small enough to reside in some SRAM), a waste of memory (it should get along with something like 32KB) and also insecure, as long as we don't use the TrustZone controller to mark this part of DRAM as secure. > So we should make sure we reserve > that space in our memory maps. I will try to load ATF into one of the SRAM regions the A64 has, and tag that as secure. U-Boot shouldn't care about ATF then, we don't need to reserve any memory for it - after all those SRAM regions look like some kind of MMIO device which we wouldn't touch anyway. Cheers, Andre. > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > board/sunxi/board.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > index 74510c5..331cb0a 100644 > --- a/board/sunxi/board.c > +++ b/board/sunxi/board.c > @@ -136,6 +136,15 @@ int dram_init(void) > return 0; > } > > +#ifdef CONFIG_MACH_SUN50I > +void dram_init_banksize(void) > +{ > + /* We need to reserve the first 16MB of RAM for ATF */ > + gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE + (16 * 1024 * 1024); > + gd->bd->bi_dram[0].size = get_effective_memsize() - (16 * 1024 * 1024); > +} > +#endif > + > #if defined(CONFIG_NAND_SUNXI) && defined(CONFIG_SPL_BUILD) > static void nand_pinmux_setup(void) > { > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On 13.04.16 21:46, Andre Przywara wrote: > Hi, > > sorry for the late reply, just found your series here. > > On 30/03/16 16:53, Alexander Graf wrote: >> On the A64 we usually boot with ATF running in EL3. ATF as it is available >> today resides in the first 16MB of RAM. > > So this is actually a mistake Allwinner made and which we haven't fixed yet. > ATF (at least BL3-1, which is the runtime service part we use for the > A64) should at least run in secure memory, actually in secure SRAM. > Having it in DRAM is a kludge, unnecessary (it's small enough to reside > in some SRAM), a waste of memory (it should get along with something > like 32KB) and also insecure, as long as we don't use the TrustZone > controller to mark this part of DRAM as secure. > >> So we should make sure we reserve >> that space in our memory maps. > > I will try to load ATF into one of the SRAM regions the A64 has, and tag > that as secure. U-Boot shouldn't care about ATF then, we don't need to > reserve any memory for it - after all those SRAM regions look like some > kind of MMIO device which we wouldn't touch anyway. I think that's a great plan moving forward. Is there any way we can runtime detect this in U-Boot to run with both old and new ATF versions or should we just break backwards compatibility? Alex
On 13/04/16 20:48, Alexander Graf wrote: > > > On 13.04.16 21:46, Andre Przywara wrote: >> Hi, >> >> sorry for the late reply, just found your series here. >> >> On 30/03/16 16:53, Alexander Graf wrote: >>> On the A64 we usually boot with ATF running in EL3. ATF as it is available >>> today resides in the first 16MB of RAM. >> >> So this is actually a mistake Allwinner made and which we haven't fixed yet. >> ATF (at least BL3-1, which is the runtime service part we use for the >> A64) should at least run in secure memory, actually in secure SRAM. >> Having it in DRAM is a kludge, unnecessary (it's small enough to reside >> in some SRAM), a waste of memory (it should get along with something >> like 32KB) and also insecure, as long as we don't use the TrustZone >> controller to mark this part of DRAM as secure. >> >>> So we should make sure we reserve >>> that space in our memory maps. >> >> I will try to load ATF into one of the SRAM regions the A64 has, and tag >> that as secure. U-Boot shouldn't care about ATF then, we don't need to >> reserve any memory for it - after all those SRAM regions look like some >> kind of MMIO device which we wouldn't touch anyway. > > I think that's a great plan moving forward. Is there any way we can > runtime detect this in U-Boot to run with both old and new ATF versions > or should we just break backwards compatibility? I wouldn't give anything on compatibility to those code versions. I expect both U-Boot, SPL/boot0 and ATF bundled together in some kind of firmware build or image. Also in the moment this is all in early development stage - I have removed more cruft from the ATF source and am tempted to switch to a proper upstream port soon. Also I am about to remove the SCP completely. So I'd like to see us doing proper upstream ports of all components, without caring about outdated and abandoned code bases. If people care about a certain feature or capability of some legacy firmware version, they are welcome to either port this properly or use that old build. That being said, let me fix the ATF to live in SRAM and publish that patch, then I will send a patch to U-Boot to revert this patch here. Cheers, Andre.
On 13.04.16 22:10, André Przywara wrote: > On 13/04/16 20:48, Alexander Graf wrote: >> >> >> On 13.04.16 21:46, Andre Przywara wrote: >>> Hi, >>> >>> sorry for the late reply, just found your series here. >>> >>> On 30/03/16 16:53, Alexander Graf wrote: >>>> On the A64 we usually boot with ATF running in EL3. ATF as it is available >>>> today resides in the first 16MB of RAM. >>> >>> So this is actually a mistake Allwinner made and which we haven't fixed yet. >>> ATF (at least BL3-1, which is the runtime service part we use for the >>> A64) should at least run in secure memory, actually in secure SRAM. >>> Having it in DRAM is a kludge, unnecessary (it's small enough to reside >>> in some SRAM), a waste of memory (it should get along with something >>> like 32KB) and also insecure, as long as we don't use the TrustZone >>> controller to mark this part of DRAM as secure. >>> >>>> So we should make sure we reserve >>>> that space in our memory maps. >>> >>> I will try to load ATF into one of the SRAM regions the A64 has, and tag >>> that as secure. U-Boot shouldn't care about ATF then, we don't need to >>> reserve any memory for it - after all those SRAM regions look like some >>> kind of MMIO device which we wouldn't touch anyway. >> >> I think that's a great plan moving forward. Is there any way we can >> runtime detect this in U-Boot to run with both old and new ATF versions >> or should we just break backwards compatibility? > > I wouldn't give anything on compatibility to those code versions. I > expect both U-Boot, SPL/boot0 and ATF bundled together in some kind of > firmware build or image. > Also in the moment this is all in early development stage - I have > removed more cruft from the ATF source and am tempted to switch to a > proper upstream port soon. Also I am about to remove the SCP completely. > > So I'd like to see us doing proper upstream ports of all components, > without caring about outdated and abandoned code bases. > If people care about a certain feature or capability of some legacy > firmware version, they are welcome to either port this properly or use > that old build. I agree, but let's add some way to ask ATF for its current version, so that U-Boot can at least panic if it finds the wrong one. Alex
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 74510c5..331cb0a 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -136,6 +136,15 @@ int dram_init(void) return 0; } +#ifdef CONFIG_MACH_SUN50I +void dram_init_banksize(void) +{ + /* We need to reserve the first 16MB of RAM for ATF */ + gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE + (16 * 1024 * 1024); + gd->bd->bi_dram[0].size = get_effective_memsize() - (16 * 1024 * 1024); +} +#endif + #if defined(CONFIG_NAND_SUNXI) && defined(CONFIG_SPL_BUILD) static void nand_pinmux_setup(void) {
On the A64 we usually boot with ATF running in EL3. ATF as it is available today resides in the first 16MB of RAM. So we should make sure we reserve that space in our memory maps. Signed-off-by: Alexander Graf <agraf@suse.de> --- board/sunxi/board.c | 9 +++++++++ 1 file changed, 9 insertions(+)