Re: [U-Boot] [PATCH 7/6] sunxi: Reserve ATF memory space on A64

[Alexander Graf]

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 7/6] sunxi: Reserve ATF memory space on A64

[André Przywara]

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.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 7/6] sunxi: Reserve ATF memory space on A64

[Andre Przywara]

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

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 7/6] sunxi: Reserve ATF memory space on A64

[Alexander Graf]

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 7/6] sunxi: Reserve ATF memory space on A64

[Alexander Graf]

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 7/6] sunxi: Reserve ATF memory space on A64

[Ian Campbell]

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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 7/6] sunxi: Reserve ATF memory space on A64

[Alexander Graf]

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 7/6] sunxi: Reserve ATF memory space on A64

[Ian Campbell]

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.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 7/6] sunxi: Reserve ATF memory space on A64

[Alexander Graf]

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 
---
 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)
 {
-- 
1.8.5.6

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot