Re: [U-Boot] [PULL] [Update] Please pull u-boot-imx

2018-03-09 Thread Stefano Babic
Hi Tom,

On 09/03/2018 21:42, Tom Rini wrote:
> On Fri, Mar 09, 2018 at 01:10:53PM +0100, Stefano Babic wrote:
> 
>> Hi Tom,
>>
>> as discussed: this contains two fixes that should go into release - thanks !
>>
>>
>> The following changes since commit 5e62f828256d66e2b28def4f9ef20a2a05c2d04f:
>>
>>   Prepare v2018.03-rc4 (2018-03-05 20:27:08 -0500)
>>
>> are available in the git repository at:
>>
>>   git://www.denx.de/git/u-boot-imx.git master
>>
>> for you to fetch changes up to 314d9f7e3eff41873011d9a0687f469680a00074:
>>
>>   imx: syscounter: make sure asm is volatile (2018-03-09 13:06:14 +0100)
>>
> 
> Note that I expect another PR with other fixes that just got posted
> today, but, applied to u-boot/master, thanks!


Yes, I saw Brian's post for HAB, and there are a couple from Jagan that
I'll pick up. I will send a new PR, then.

Stefano

-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v5 02/15] dma: add channels support

2018-03-09 Thread Álvaro Fernández Rojas

Hi Grygorii,

El 09/03/2018 a las 0:07, Grygorii Strashko escribió:

Hi Álvaro,

On 03/05/2018 02:05 PM, Álvaro Fernández Rojas wrote:

This adds channels support for dma controllers that have multiple channels
which can transfer data to/from different devices (enet, usb...).

Signed-off-by: Álvaro Fernández Rojas 
Reviewed-by: Simon Glass 
---
   v5: remove unneeded dma.h include
   v4: no changes
   v3: Introduce changes reported by Simon Glass:
- Improve dma-uclass.h documentation.
- Switch to live tree API.

   drivers/dma/Kconfig  |   7 ++
   drivers/dma/dma-uclass.c | 188 
+--
   include/dma-uclass.h |  78 
   include/dma.h| 174 ++-
   4 files changed, 439 insertions(+), 8 deletions(-)

Small note first. I don't know if this is common practice for u-boot -
but Isn't it preferable to send new version of the series *not as reply* to the 
old one?
I was sending it as a reply to the first version, not the last one, but 
I will do that from now on.


I've tried this and, in general, it works for me (unfortunately I can't post 
code yet).
Thanks a lot for you work.

Thanks to you for testing :).


But... (it's always but ;)

¯\_(ツ)_/¯

[...]

+   /**
+* receive() - Receive a DMA transfer.
+*
+* @dma: The DMA Channel to manipulate.
+* @dst: The destination pointer.
+* @return zero on success, or -ve error code.
+*/
+   int (*receive)(struct dma *dma, void **dst);
+   /**
+* send() - Send a DMA transfer.
+*
+* @dma: The DMA Channel to manipulate.
+* @src: The source pointer.
+* @len: Length of the data to be sent (number of bytes).
+* @return zero on success, or -ve error code.
+*/
+   int (*send)(struct dma *dma, void *src, size_t len);

Can we have additional *optional* parameter in above two callbacks and in 
dma_receive/dma_send() API?
Like:
  int (*send)(struct dma *dma, void *src, size_t len, void *metadata)
  int (*receive)(struct dma *dma, void **dst, void **metadata);

Reason:
It's not a common practice to implement Networking DMA using generic DMA 
frameworks, simply
because Networking DMA HW is terribly different between different SoCs :(, so 
it's mostly impossible
to fit all of them in any generic DMA framework (at least this has never ever 
worked for Linux kernel):
- totally different HW rings/queues impl, multi-queues & multi DMA channels
- special requirements for IRQ handling
- necessity to pass additional information within each Net DMA descriptor
- availability and support of different Net traffic HW acceleration features

But in case of u-boot, it, theoretically, might work because most of Net
drivers have much more simplified implementation comparing to Linux kernel
- UP, polling, one RX/TX channel and disabled Net traffic HW acceleration 
features.

As result, Proposed here interface can be used with much more different HW and
drivers (and especially Net drivers), but only if it will be possible to pass
additional DMA driver's specific information from DMA user to DMA driver
per each send/rsv request.
For example, two TI driver cpsw.c and keystone_net.c required to pass
minimum one additional parameter with each sending packet - Destination Port 
number,
which must be passed to DMA in DMA descriptor. And Source Port number
has to be passed back with each received packet.

! I'm not insisting here ! and be happy to hear third opinion.

(if proposition will not be accepted - .. :( I'll just need to do the same 
later,
  but there might be hight number of active users of this interface in u-boot 
already)
If everyone else agrees I can add those two parameters in the next 
version even though my implementation doesn't need them.



+#endif /* CONFIG_DMA_CHANNELS */
/**
 * transfer() - Issue a DMA transfer. The implementation must
 *   wait until the transfer is done.
diff --git a/include/dma.h b/include/dma.h
index 89320f10d9..bf8123fa9e 100644
--- a/include/dma.h
+++ b/include/dma.h
@@ -1,6 +1,7 @@
   /*
- * (C) Copyright 2015
- * Texas Instruments Incorporated, 
+ * Copyright (C) 2018 Álvaro Fernández Rojas 
+ * Copyright (C) 2015 Texas Instruments Incorporated 
+ * Written by Mugunthan V N 
*
* SPDX-License-Identifier: GPL-2.0+
*/
@@ -8,6 +9,9 @@
   #ifndef _DMA_H_
   #define _DMA_H_
   
+#include 

+#include 
+
   /*
* enum dma_direction - dma transfer direction indicator
* @DMA_MEM_TO_MEM: Memcpy mode
@@ -37,6 +41,172 @@ struct dma_dev_priv {
u32 supported;
   };
   
+#ifdef CONFIG_DMA_CHANNELS

+/**
+ * A DMA is a feature of computer systems that allows certain hardware
+ * subsystems to access main system memory, independent of the CPU.
+ * DMA channels are typically generated externally to the HW 

Re: [U-Boot] [PATCH] dm: mmc: socfpga: call dwmci_probe()

2018-03-09 Thread Marek Vasut

On 03/09/2018 07:51 AM, Patrick Brünn wrote:

From: Patrick Brünn
Sent: Donnerstag, 8. März 2018 06:39

From: Jaehoon Chung [mailto:jh80.ch...@samsung.com]
Sent: Donnerstag, 8. März 2018 04:57
On 03/08/2018 12:12 PM, Marek Vasut wrote:

On 03/08/2018 03:17 AM, Jaehoon Chung wrote:

On 03/06/2018 05:07 PM, linux-kernel-...@beckhoff.com wrote:

From: Patrick Bruenn 

On a socfpga_cyclone5 based board the SD card, was never powered

up.

For

other dw_mmc based SoCs dwmci_probe() is called in the platform

specific

probe(). It seems this call is missing for socfpga_dw_mmc.

With this change DWMCI_PWREN is set by dmwci_init().

Signed-off-by: Patrick Bruenn 


Reviewed-by: Jaehoon Chung 

Will apply this patch before releasing v2018.03.
(I have a problem about accessing git.denx.de. After fixing my problem,

will resend email about applying.)


DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand

what this patch is trying to fix. I'd prefer if you did not hastily apply this.

It's my misunderstanding. When i checked more. I think that Marek is right.
Thanks Marek for pointing out.


Okay, but do you have any hint what I am doing wrong? My board (cx8100 not
mainline, yet) is based on socfpga_cyclone5. And on my board "
dwmci_writel(host, DWMCI_PWREN, 1);" is never called, because
dwmci_init() is never called.
As far as I can see with CONFIG_DM_MMC enabled dwmci_init() should be
called by dwmci_probe().

exynos  and rockchip do call dwmci_probe() within
exynos/rockchip_dwmmc_probe().
but socfpga_dwmmc_probe() is missing this call. So I looked deeper but found
no place for socfpga platform to call dwmci_probe() or dwmci_init().
What am I missing?


I got an idea, what might be the difference between my board and your boards.
I suspect you use U-BOOT SPL without CONFIG_DM_MMC set, so
dwmci_init() is called indirectly by mmc_start_init().
Now, when your main U-Boot (with CONFIG_DM_MMC) is launched,
everything is already configured and it isn't necessary to call dwmci_init() 
again.


Yes, that's the only supported configuration -- U-Boot SPL and U-Boot. 
Any such mix-and-match setups are NOT supported.



On my board the Altera MPL is used (and I can't replace it).  Which seems to
disable DWMCI_PWREN before launching U-Boot.
If my assumption is correct, I still think it is a U-Boot bug to assume code 
like in
dwmci_init() was already run before U-Boot gets in control.
Besides exynos/rockchip_dw_mmc don't have this precondition requirement.


We're too close to the release and it's not broken in mainline, it's 
only broken if you use this horrible shitty MPL configuration, which is 
not supported.


So we should revisit this patch after release, after it can be tested 
properly and checked if it doesn't break standard setups.



Please take your time to look deeper into this issue, before deciding anything.
I don't think we need to rush this into the next release, as normal mainline
boards are "accidently" not affected.


Sounds good
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PULL] [Update] Please pull u-boot-imx

2018-03-09 Thread Tom Rini
On Fri, Mar 09, 2018 at 01:10:53PM +0100, Stefano Babic wrote:

> Hi Tom,
> 
> as discussed: this contains two fixes that should go into release - thanks !
> 
> 
> The following changes since commit 5e62f828256d66e2b28def4f9ef20a2a05c2d04f:
> 
>   Prepare v2018.03-rc4 (2018-03-05 20:27:08 -0500)
> 
> are available in the git repository at:
> 
>   git://www.denx.de/git/u-boot-imx.git master
> 
> for you to fetch changes up to 314d9f7e3eff41873011d9a0687f469680a00074:
> 
>   imx: syscounter: make sure asm is volatile (2018-03-09 13:06:14 +0100)
> 

Note that I expect another PR with other fixes that just got posted
today, but, applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] ARM: qemu-arm: Increase CONFIG_SYS_CBSIZE

2018-03-09 Thread Tom Rini
On Mon, Mar 05, 2018 at 11:20:41PM +0200, Tuomas Tynkkynen wrote:

> CONFIG_SYS_CBSIZE determines the maximum length of the kernel command
> line, and the default value of 256 is too small for booting some Linux
> images in the wild.
> 
> Signed-off-by: Tuomas Tynkkynen 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot,1/3] MIPS: Drop unreferenced CONFIG_* defines

2018-03-09 Thread Tom Rini
On Tue, Mar 06, 2018 at 01:46:37AM +0200, Tuomas Tynkkynen wrote:

> The following config symbols are only defined once and never referenced
> anywhere else:
> 
> CONFIG_DBAU1X00
> CONFIG_PB1X00
> 
> Most of them are config symbols named after the respective boards which
> seems to have been a standard practice at some point.
> 
> Signed-off-by: Tuomas Tynkkynen 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot, 2/3] ARM: Drop unreferenced CONFIG_* defines named after SoCs

2018-03-09 Thread Tom Rini
On Tue, Mar 06, 2018 at 01:46:38AM +0200, Tuomas Tynkkynen wrote:

> The following config symbols are only defined once and never referenced
> anywhere else:
> 
> CONFIG_ARM926EJS
> CONFIG_CPUAT91
> CONFIG_EXYNOS5800
> CONFIG_SYS_CORTEX_R4
> 
> Most of them are config symbols named after the respective SoCs which
> seems to have been a standard practice at some point.
> 
> Signed-off-by: Tuomas Tynkkynen 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot, 3/3] ARM: Drop unreferenced CONFIG_* defines named after boards

2018-03-09 Thread Tom Rini
On Tue, Mar 06, 2018 at 01:46:39AM +0200, Tuomas Tynkkynen wrote:

> The following config symbols are only defined once and never referenced
> anywhere else:
> 
> CONFIG_AT91SAM9263EK
> CONFIG_AT91SAM9RLEK
> CONFIG_BARIX_IPAM390
> CONFIG_BOARD_H2200
> CONFIG_EP9301
> CONFIG_KZM_A9_GT
> CONFIG_PICOSAM
> CONFIG_PLATINUM_PICON
> CONFIG_PLATINUM_TITANIUM
> CONFIG_PM9261
> CONFIG_PM9263
> CONFIG_PM9G45
> CONFIG_SIEMENS_DRACO
> CONFIG_SIEMENS_PXM2
> CONFIG_SIEMENS_RUT
> CONFIG_SMDKC100
> CONFIG_SMDKV310
> CONFIG_STM32F4DISCOVERY
> 
> Most of them are config symbols named after the respective boards which
> seems to have been a standard practice at some point.
> 
> Signed-off-by: Tuomas Tynkkynen 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot, 1/1] MAINTAINERS: bring sections into alphabetic order

2018-03-09 Thread Tom Rini
On Wed, Mar 07, 2018 at 04:04:29AM +0100, Heinrich Schuchardt wrote:

> NETWORK should be after NAND_FLASH.
> 
> Signed-off-by: Heinrich Schuchardt 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] tools/mkimage: Use proper output parameter in dtc-system call

2018-03-09 Thread Tom Rini
On Thu, Mar 08, 2018 at 09:00:13AM +0100, Stefan Theil wrote:

> The system call used by mkimage to run dtc redirects stdout to a
> temporary file. This can cause problems on Windows (with a MinGW
> cross-compiled version). Using the "-o" dtc parameter avoids
> this problem.
> 
> Signed-off-by: Stefan Theil 
> Reviewed-by: Tom Rini 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot, v2, 1/1] scripts/coccinelle: add some more coccinelle tests

2018-03-09 Thread Tom Rini
On Thu, Mar 08, 2018 at 08:56:17PM +0100, Heinrich Schuchardt wrote:

> kmerr: verify that malloc and calloc are followed by a check to verify
> that we are not out of memory.
> 
> badzero: Compare pointer-typed values to NULL rather than 0
> 
> Both checks are copied from the Linux kernel archive.
> 
> Signed-off-by: Heinrich Schuchardt 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot, v2, 2/2] bcm283x_pl011: Flush RX queue after setting baud rate

2018-03-09 Thread Tom Rini
On Wed, Mar 07, 2018 at 10:08:25PM +0100, Alexander Graf wrote:

> After the UART was initialized, we may still have bogus data in the
> RX queue if it was enabled with incorrect pin muxing before.
> 
> So let's flush the RX queue whenever we initialize baud rates.
> 
> This fixes a regression with the dynamic pinmuxing code when enable_uart=1
> is not set in config.txt on Raspberry Pis that use pl011 for serial.
> 
> Fixes: caf2233b28 ("bcm283x: Add pinctrl driver")
> Reported-by: Göran Lundberg 
> Reported-by: Peter Robinson 
> Signed-off-by: Alexander Graf 
> Tested-by: Peter Robinson 
> Tested-by: Tuomas Tynkkynen 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot, v2, 1/2] serial_bcm283x_mu: Flush RX queue after setting baud rate

2018-03-09 Thread Tom Rini
On Wed, Mar 07, 2018 at 10:08:24PM +0100, Alexander Graf wrote:

> After the UART was initialized, we may still have bogus data in the
> RX queue if it was enabled with incorrect pin muxing before.
> 
> So let's flush the RX queue whenever we initialize baud rates.
> 
> This fixes a regression with the dynamic pinmuxing code when enable_uart=1
> is not set in config.txt.
> 
> Fixes: caf2233b28 ("bcm283x: Add pinctrl driver")
> Reported-by: Göran Lundberg 
> Reported-by: Peter Robinson 
> Signed-off-by: Alexander Graf 
> Tested-by: Peter Robinson 
> Tested-by: Tuomas Tynkkynen 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] treewide: Fix gdsys mail addresses

2018-03-09 Thread Tom Rini
On Tue, Mar 06, 2018 at 08:04:58AM +0100, Mario Six wrote:

> From: Mario Six 
> 
> The @gdsys.cc addresses are supposed to be used for mailing lists.
> Switch all occurrences of @gdsys.de mail addresses to their @gdsys.cc
> equivalent.
> 
> Also, Dirk's address was wrong in one place; fix that as well.
> 
> Signed-off-by: Mario Six 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] dm: mmc: socfpga: call dwmci_probe()

2018-03-09 Thread Marek Vasut

On 03/09/2018 09:24 AM, Jaehoon Chung wrote:

Dear Patrick,

On 03/09/2018 03:51 PM, Patrick Brünn wrote:

From: Patrick Brünn
Sent: Donnerstag, 8. März 2018 06:39

From: Jaehoon Chung [mailto:jh80.ch...@samsung.com]
Sent: Donnerstag, 8. März 2018 04:57
On 03/08/2018 12:12 PM, Marek Vasut wrote:

On 03/08/2018 03:17 AM, Jaehoon Chung wrote:

On 03/06/2018 05:07 PM, linux-kernel-...@beckhoff.com wrote:

From: Patrick Bruenn 

On a socfpga_cyclone5 based board the SD card, was never powered

up.

For

other dw_mmc based SoCs dwmci_probe() is called in the platform

specific

probe(). It seems this call is missing for socfpga_dw_mmc.

With this change DWMCI_PWREN is set by dmwci_init().

Signed-off-by: Patrick Bruenn 


Reviewed-by: Jaehoon Chung 

Will apply this patch before releasing v2018.03.
(I have a problem about accessing git.denx.de. After fixing my problem,

will resend email about applying.)


DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand

what this patch is trying to fix. I'd prefer if you did not hastily apply this.

It's my misunderstanding. When i checked more. I think that Marek is right.
Thanks Marek for pointing out.


Okay, but do you have any hint what I am doing wrong? My board (cx8100 not
mainline, yet) is based on socfpga_cyclone5. And on my board "
dwmci_writel(host, DWMCI_PWREN, 1);" is never called, because
dwmci_init() is never called.
As far as I can see with CONFIG_DM_MMC enabled dwmci_init() should be
called by dwmci_probe().

exynos  and rockchip do call dwmci_probe() within
exynos/rockchip_dwmmc_probe().
but socfpga_dwmmc_probe() is missing this call. So I looked deeper but found
no place for socfpga platform to call dwmci_probe() or dwmci_init().
What am I missing?


I got an idea, what might be the difference between my board and your boards.
I suspect you use U-BOOT SPL without CONFIG_DM_MMC set, so
dwmci_init() is called indirectly by mmc_start_init().


Right, it's difference with CONFIG_DM_MMC. I had checked this.


Now, when your main U-Boot (with CONFIG_DM_MMC) is launched,
everything is already configured and it isn't necessary to call dwmci_init() 
again.
On my board the Altera MPL is used (and I can't replace it).  Which seems to
disable DWMCI_PWREN before launching U-Boot.
If my assumption is correct, I still think it is a U-Boot bug to assume code 
like in
dwmci_init() was already run before U-Boot gets in control.
Besides exynos/rockchip_dw_mmc don't have this precondition requirement.

Please take your time to look deeper into this issue, before deciding anything.
I don't think we need to rush this into the next release, as normal mainline
boards are "accidently" not affected.


Sure. I will check this patch. Before applying this, will check more carefully.
I agreed about Marek's opinion "did not hastily apply this."
And i think that your approach also is right.


Please queue for -next , NOT this release.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot,1/1] yaffs2: iterator variable cannot be NULL

2018-03-09 Thread Tom Rini
On Thu, Mar 08, 2018 at 10:52:29PM +0100, Heinrich Schuchardt wrote:

> The iterator of list_for_each() is never NULL.
> 
> Identified with coccinelle.
> 
> Signed-off-by: Heinrich Schuchardt 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] fs: ext4: Do not print mount fail message when not ext4 filesystem

2018-03-09 Thread Tom Rini
On Thu, Mar 08, 2018 at 12:26:08AM +0100, Marek Behún wrote:

> Other filesystem drivers don't do this.
> 
> Signed-off-by: Marek Behun 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] imx: hab: Convert DCD non-NULL error to warning

2018-03-09 Thread Fabio Estevam
Hi Bryan,

On Fri, Mar 9, 2018 at 10:07 AM, Bryan O'Donoghue
 wrote:
> commit 8c4037a09a5c ("imx: hab: Ensure the IVT DCD pointer is Null prior
> to calling HAB authenticate function.") makes the DCD field being NULL a
> dependency.
>
> This change though will break loading and executing of existing pre-signed
> binaries on a u-boot update i.e. if this change is deployed on a board you
> will be forced to redo all images on that board to NULL out the DCD.
>
> There is no prior guidance from NXP that the DCD must be NULL similarly
> public guidance on usage of the HAB doesn't call out this NULL dependency
> (see boundary devices link).
>
> Since later SoCs will reject a non-NULL DCD there's no reason to make a
> NULL DCD a requirement, however if there is an actual dependency for later
> SoCs the appropriate fix would be to do SoC version checking.
>
> Earlier SoCs are capable (and happy) to authenticate images with non-NULL
> DCDs, we should not be forcing this change on downstream users -
> particularly if it means those users now must rewrite their build systems
> and/or redeploy signed images in the field.
>
> Fixes: 8c4037a09a5c ("imx: hab: Ensure the IVT DCD pointer is Null prior
> to calling HAB authenticate function.")
>
> Signed-off-by: Bryan O'Donoghue 
> Cc: Utkarsh Gupta 
> Cc: Breno Lima 
> Cc: Fabio Estevam 
> Link: https://boundarydevices.com/high-assurance-boot-hab-dummies

Utkarsh is currently out of the office, but Breno knows the details
about 8c4037a09a5c ("imx: hab: Ensure the IVT DCD pointer is Null
prior
to calling HAB authenticate function.") and will comment.

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


[U-Boot] [PATCH 1/5] net: fec_mxc: Fix DM driver issue in recv

2018-03-09 Thread Peng Fan
From: Ye Li 

When using ethernet DM driver, the recv interface has a
change with non-DM interface, that driver needs to set
the packet pointer and provide it to upper layer to process.

In fec driver, the fecmxc_recv functions does not handle the
packet pointer parameter. This may cause crash in upper layer
processing because the packet pointer is not set.

This patch allocates a buffer for the packet pointer and free it
through free_pkt interface.

Signed-off-by: Ye Li 
Reviewed-by: Peng Fan 
---
 drivers/net/fec_mxc.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index ff7ad91116..7c396d8d95 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -806,7 +806,16 @@ static int fec_recv(struct eth_device *dev)
uint16_t bd_status;
ulong addr, size, end;
int i;
+
+#ifdef CONFIG_DM_ETH
+   *packetp = memalign(ARCH_DMA_MINALIGN, FEC_MAX_PKT_SIZE);
+   if (*packetp == 0) {
+   printf("%s: error allocating packetp\n", __func__);
+   return -ENOMEM;
+   }
+#else
ALLOC_CACHE_ALIGN_BUFFER(uchar, buff, FEC_MAX_PKT_SIZE);
+#endif
 
/* Check if any critical events have happened */
ievent = readl(>eth->ievent);
@@ -882,8 +891,13 @@ static int fec_recv(struct eth_device *dev)
 #ifdef CONFIG_FEC_MXC_SWAP_PACKET
swap_packet((uint32_t *)addr, frame_length);
 #endif
+
+#ifdef CONFIG_DM_ETH
+   memcpy(*packetp, (char *)addr, frame_length);
+#else
memcpy(buff, (char *)addr, frame_length);
net_process_received_packet(buff, frame_length);
+#endif
len = frame_length;
} else {
if (bd_status & FEC_RBD_ERR)
@@ -917,6 +931,16 @@ static int fec_recv(struct eth_device *dev)
return len;
 }
 
+static int fecmxc_free_pkt(struct udevice *dev, uchar *packet, int length)
+{
+#ifdef CONFIG_DM_ETH
+   if (packet)
+   free(packet);
+#endif
+
+   return 0;
+}
+
 static void fec_set_dev_name(char *dest, int dev_id)
 {
sprintf(dest, (dev_id == -1) ? "FEC" : "FEC%i", dev_id);
@@ -1205,6 +1229,7 @@ static const struct eth_ops fecmxc_ops = {
.start  = fecmxc_init,
.send   = fecmxc_send,
.recv   = fecmxc_recv,
+   .free_pkt   = fecmxc_free_pkt,
.stop   = fecmxc_halt,
.write_hwaddr   = fecmxc_set_hwaddr,
.read_rom_hwaddr= fecmxc_read_rom_hwaddr,
-- 
2.14.1

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


Re: [U-Boot] [PATCH] mx6sabresd: Select the CONFIG_EFI_PARTITION option

2018-03-09 Thread Jagan Teki
On Sat, Mar 10, 2018 at 7:16 AM, Fabio Estevam  wrote:
> Hi Jagan,
>
> On Fri, Mar 9, 2018 at 9:09 AM, Jagan Teki  wrote:
>
>> Do you have a partition table details, I'm trying GPT as below
>
> Here is the partition scheme that Shawn used in his test:
> https://patchwork.ozlabs.org/patch/867705/

I've seen this before, I'm trying to understand here - how can we
write SPL, u-boot? which are started from sector 2 and 8a

I've observed GPT can hold partition tables on these areas.

Disk /dev/sda: 7389184 sectors, 3.5 GiB
Model: UMS disk 0
Sector size (logical/physical): 512/512 bytes
Disk identifier (GUID): E6BC20C3-1526-44AF-BF61-D494E13B957D
Partition table holds up to 128 entries
Main partition table begins at sector 2 and ends at sector 33
First usable sector is 2048, last usable sector is 7389150
Partitions will be aligned on 2048-sector boundaries
Total free space is 7387103 sectors (3.5 GiB)

When I try to write raw through dd, partition table is broken.

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


Re: [U-Boot] [PATCH 2/3] imx: hab: Make usage of packed attribute consistent

2018-03-09 Thread Fabio Estevam
On Fri, Mar 9, 2018 at 10:07 AM, Bryan O'Donoghue
 wrote:
> commit cd2d46003ce1 ("arm: imx: hab: Add IVT header definitions") declares
> struct ivt_header as "__attribute__((packed))".
>
> commit ed286bc80e9d ("imx: hab: Check if CSF is valid before
> authenticating image") declares struct hab_hdr with __packed.
>
> This patch makes the __packed convention consistent.
>
> Signed-off-by: Bryan O'Donoghue 
> Cc: Utkarsh Gupta 
> Cc: Breno Lima 
> Cc: Fabio Estevam 

Reviewed-by: Fabio Estevam 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/3] imx: hab: Fix usage of packed attribute

2018-03-09 Thread Fabio Estevam
On Fri, Mar 9, 2018 at 10:07 AM, Bryan O'Donoghue
 wrote:
> commit ed286bc80e9d ("imx: hab: Check if CSF is valid before authenticating
> image") makes use of "__packed" as a prefix to the "struct hab_hdr"
> declaration.
>
> With my compiler "gcc version 7.2.1 20171011 (Linaro GCC 7.2-2017.11)" we
> get:
>
> ./arch/arm/include/asm/mach-imx/hab.h:42:25: error: expected ‘=’, ‘,’, ‘;’,
> ‘asm’ or ‘__attribute__’ before ‘{’ token
>  struct __packed hab_hdr {
>
> Fix this problem by including 
>
> Signed-off-by: Bryan O'Donoghue 
> Cc: Utkarsh Gupta 
> Cc: Breno Lima 
> Cc: Fabio Estevam 

Reviewed-by: Fabio Estevam 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] imx: hab: Convert DCD non-NULL error to warning

2018-03-09 Thread Breno Matheus Lima
Hi Bryan,

2018-03-09 10:07 GMT-03:00 Bryan O'Donoghue :
> commit 8c4037a09a5c ("imx: hab: Ensure the IVT DCD pointer is Null prior
> to calling HAB authenticate function.") makes the DCD field being NULL a
> dependency.
>
> This change though will break loading and executing of existing pre-signed
> binaries on a u-boot update i.e. if this change is deployed on a board you
> will be forced to redo all images on that board to NULL out the DCD.
>
> There is no prior guidance from NXP that the DCD must be NULL similarly
> public guidance on usage of the HAB doesn't call out this NULL dependency
> (see boundary devices link).
>
> Since later SoCs will reject a non-NULL DCD there's no reason to make a
> NULL DCD a requirement, however if there is an actual dependency for later
> SoCs the appropriate fix would be to do SoC version checking.
>
> Earlier SoCs are capable (and happy) to authenticate images with non-NULL
> DCDs, we should not be forcing this change on downstream users -
> particularly if it means those users now must rewrite their build systems
> and/or redeploy signed images in the field.
>
> Fixes: 8c4037a09a5c ("imx: hab: Ensure the IVT DCD pointer is Null prior
> to calling HAB authenticate function.")

We understand the concern being raised however would prefer to leave
this as an error, and selected users relying on images with DCD
pointers can modify the code accordingly. This does not just apply to
new SoC’s but also applies to existing SoC’s. Users performing such an
update should definitely test the image prior to making an OTA
available. It has never been intended for DCD to be used in any post
boot image and we realize the lack of documentation is a hindsight by
us, and we are currently addressing that with updated guidance.

Best regards,
Breno Lima
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] mx6sabresd: Select the CONFIG_EFI_PARTITION option

2018-03-09 Thread Fabio Estevam
Hi Jagan,

On Fri, Mar 9, 2018 at 9:09 AM, Jagan Teki  wrote:

> Do you have a partition table details, I'm trying GPT as below

Here is the partition scheme that Shawn used in his test:
https://patchwork.ozlabs.org/patch/867705/
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH 5/5] net: fex_mxc: add i.MX6UL/SX/SL compatible

2018-03-09 Thread Peng Fan
Add i.MX6UL/SX/SL compatible.

Signed-off-by: Peng Fan 
---
 drivers/net/fec_mxc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index e8f8fef66a..ffe3bae59f 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -1349,6 +1349,9 @@ static int fecmxc_ofdata_to_platdata(struct udevice *dev)
 
 static const struct udevice_id fecmxc_ids[] = {
{ .compatible = "fsl,imx6q-fec" },
+   { .compatible = "fsl,imx6sl-fec" },
+   { .compatible = "fsl,imx6sx-fec" },
+   { .compatible = "fsl,imx6ul-fec" },
{ }
 };
 
-- 
2.14.1

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


[U-Boot] [PATCH 4/5] net: fec: sharing MDIO for two enet controllers

2018-03-09 Thread Peng Fan
On i.MX6SX, 6UL and 7D, there are two enet controllers each has a
MDIO port. But Some boards share one MDIO port for the two enets. So
introduce a configuration CONFIG_FEC_MXC_MDIO_BASE to indicate
the MDIO port for sharing.

Signed-off-by: Peng Fan 
---
 drivers/net/Kconfig   | 7 +++
 drivers/net/fec_mxc.c | 9 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index de1947ccc1..3a468a7c59 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -147,6 +147,13 @@ config ETHOC
help
  This MAC is present in OpenRISC and Xtensa XTFPGA boards.
 
+config FEC_MXC_MDIO_BASE
+   hex "MDIO base address for the FEC controller"
+   depends on FEC_MXC
+   help
+ This specifies the MDIO registers base address. It is used when
+ two FEC controllers share MDIO bus.
+
 config FEC_MXC
bool "FEC Ethernet controller"
depends on MX5 || MX6
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 2c3171ecc9..e8f8fef66a 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -1161,8 +1161,12 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int 
phy_id, uint32_t addr)
 * Only the first one can access the MDIO bus.
 */
base_mii = MXS_ENET0_BASE;
+#else
+#ifdef CONFIG_FEC_MXC_MDIO_BASE
+   base_mii = CONFIG_FEC_MXC_MDIO_BASE;
 #else
base_mii = addr;
+#endif
 #endif
debug("eth_init: fec_probe(bd, %i, %i) @ %08x\n", dev_id, phy_id, addr);
bus = fec_get_miibus(base_mii, dev_id);
@@ -1274,7 +1278,12 @@ static int fecmxc_probe(struct udevice *dev)
fec_reg_setup(priv);
 
priv->dev_id = dev->seq;
+
+#ifdef CONFIG_FEC_MXC_MDIO_BASE
+   bus = fec_get_miibus((ulong)CONFIG_FEC_MXC_MDIO_BASE, dev->seq);
+#else
bus = fec_get_miibus((ulong)priv->eth, dev->seq);
+#endif
if (!bus) {
ret = -ENOMEM;
goto err_mii;
-- 
2.14.1

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


[U-Boot] [PATCH 2/5] net: fec_mxc: simplify fec_get_miibus

2018-03-09 Thread Peng Fan
No need to provide two prototype for this function.
Use ulong for the first parameter, then this function
could be shared for DM/non DM case.

Signed-off-by: Peng Fan 
---
 drivers/net/fec_mxc.c | 13 ++---
 include/netdev.h  |  6 +-
 2 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 7c396d8d95..2bd4ba4ef1 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -1021,18 +1021,9 @@ static void fec_free_descs(struct fec_priv *fec)
free(fec->tbd_base);
 }
 
-#ifdef CONFIG_DM_ETH
-struct mii_dev *fec_get_miibus(struct udevice *dev, int dev_id)
-#else
-struct mii_dev *fec_get_miibus(uint32_t base_addr, int dev_id)
-#endif
+struct mii_dev *fec_get_miibus(ulong base_addr, int dev_id)
 {
-#ifdef CONFIG_DM_ETH
-   struct fec_priv *priv = dev_get_priv(dev);
-   struct ethernet_regs *eth = priv->eth;
-#else
struct ethernet_regs *eth = (struct ethernet_regs *)(ulong)base_addr;
-#endif
struct mii_dev *bus;
int ret;
 
@@ -1284,7 +1275,7 @@ static int fecmxc_probe(struct udevice *dev)
fec_reg_setup(priv);
priv->dev_id = (dev_id == -1) ? 0 : dev_id;
 
-   bus = fec_get_miibus(dev, dev_id);
+   bus = fec_get_miibus((ulong)priv->eth, dev_id);
if (!bus) {
ret = -ENOMEM;
goto err_mii;
diff --git a/include/netdev.h b/include/netdev.h
index 3958a4cd32..c96f851be0 100644
--- a/include/netdev.h
+++ b/include/netdev.h
@@ -119,11 +119,7 @@ static inline int pci_eth_init(bd_t *bis)
return num;
 }
 
-#ifdef CONFIG_DM_ETH
-struct mii_dev *fec_get_miibus(struct udevice *dev, int dev_id);
-#else
-struct mii_dev *fec_get_miibus(uint32_t base_addr, int dev_id);
-#endif
+struct mii_dev *fec_get_miibus(ulong base_addr, int dev_id);
 
 #ifdef CONFIG_PHYLIB
 struct phy_device;
-- 
2.14.1

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


[U-Boot] [PATCH 3/5] net: fec: set dev->seq to priv->dev_id

2018-03-09 Thread Peng Fan
To platforms has two enet interface, using dev->seq could
avoid conflict.

i.MX6UL/ULL evk board net get the wrong MAC address from fuse,
eth1 get MAC0 address, eth0 get MAC1 address from fuse. Set the
priv->dev_id to device->seq as the real net interface alias id then
.fec_get_hwaddr() read the related MAC address from fuse.

Signed-off-by: Peng Fan 
---
 drivers/net/fec_mxc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 2bd4ba4ef1..2c3171ecc9 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -1252,7 +1252,6 @@ static int fecmxc_probe(struct udevice *dev)
struct eth_pdata *pdata = dev_get_platdata(dev);
struct fec_priv *priv = dev_get_priv(dev);
struct mii_dev *bus = NULL;
-   int dev_id = -1;
uint32_t start;
int ret;
 
@@ -1273,9 +1272,9 @@ static int fecmxc_probe(struct udevice *dev)
}
 
fec_reg_setup(priv);
-   priv->dev_id = (dev_id == -1) ? 0 : dev_id;
 
-   bus = fec_get_miibus((ulong)priv->eth, dev_id);
+   priv->dev_id = dev->seq;
+   bus = fec_get_miibus((ulong)priv->eth, dev->seq);
if (!bus) {
ret = -ENOMEM;
goto err_mii;
-- 
2.14.1

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


Re: [U-Boot] [PULL] [Update] Please pull u-boot-imx

2018-03-09 Thread Fabio Estevam
Hi Stefano,

On Fri, Mar 9, 2018 at 6:20 PM, Stefano Babic  wrote:

> Yes, I saw Brian's post for HAB, and there are a couple from Jagan that
> I'll pick up. I will send a new PR, then.

Patches 1/3 and 2/3 from Bryan look good.

We have some concerns about patch 3/3 and Breno will reply to this patch soon.

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


[U-Boot] [PATCH v2] imximage: Remove failure when no IVT offset is found

2018-03-09 Thread Fabio Estevam
Sometimes imximage throws the following error:

  CFGSboard/freescale/vf610twr/imximage.cfg.cfgtmp
  CFGSboard/freescale/vf610twr/imximage.cfg.cfgtmp
  MKIMAGE u-boot-dtb.imx
Error: No BOOT_FROM tag in board/freescale/vf610twr/imximage.cfg.cfgtmp
arch/arm/mach-imx/Makefile:100: recipe for target 'u-boot-dtb.imx' failed

Later on, when running mkimage for the u-boot.imx it will succeed in
finding the IVT offset.

Looks like some race condition happening during parallel build when
processing mkimage for u-boot-dtb.imx and u-boot.imx.

A proper fix still needs to be implemented, but as a workaround let's
remove the error when the IVT offset is not found.

It is useful to have such message, especially during bring-up phase,
but the build error that it causes is severe, so better avoid the
build error for now.

The error checking can be re-implemented later when we have a proper
fix.

Reported-by: Breno Lima 
Reported-by: Thomas Petazzoni 
Signed-off-by: Fabio Estevam 

Signed-off-by: Fabio Estevam 
---
 tools/imximage.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/tools/imximage.c b/tools/imximage.c
index eb7e682..ed9d935 100644
--- a/tools/imximage.c
+++ b/tools/imximage.c
@@ -777,11 +777,6 @@ static uint32_t parse_cfg_file(struct imx_header *imxhdr, 
char *name)
(*set_dcd_rst)(imxhdr, dcd_len, name, lineno);
fclose(fd);
 
-   /* Exit if there is no BOOT_FROM field specifying the flash_offset */
-   if (imximage_ivt_offset == FLASH_OFFSET_UNDEFINED) {
-   fprintf(stderr, "Error: No BOOT_FROM tag in %s\n", name);
-   exit(EXIT_FAILURE);
-   }
return dcd_len;
 }
 
-- 
2.7.4

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


[U-Boot] [PULL] [Update] Please pull u-boot-imx

2018-03-09 Thread Stefano Babic
Hi Tom,

as discussed: this contains two fixes that should go into release - thanks !


The following changes since commit 5e62f828256d66e2b28def4f9ef20a2a05c2d04f:

  Prepare v2018.03-rc4 (2018-03-05 20:27:08 -0500)

are available in the git repository at:

  git://www.denx.de/git/u-boot-imx.git master

for you to fetch changes up to 314d9f7e3eff41873011d9a0687f469680a00074:

  imx: syscounter: make sure asm is volatile (2018-03-09 13:06:14 +0100)


Fabio Estevam (1):
  imximage: Remove failure when no IVT offset is found

Yasushi SHOJI (1):
  imx: syscounter: make sure asm is volatile

 arch/arm/mach-imx/syscounter.c | 4 ++--
 tools/imximage.c   | 5 -
 2 files changed, 2 insertions(+), 7 deletions(-)

Stefano

-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v2 1/4] tools: env: Pass through indent

2018-03-09 Thread Alex Kiernan
Pass tools/env/fw_env.c through indent to correct style violations. This
commit consists of only one non-whitespace change:

  tools/env/fw_env.c:549: error: do not use assignment in if condition

Signed-off-by: Alex Kiernan 
---

Changes in v2:
- Clean style violations in existing code

 tools/env/fw_env.c | 346 ++---
 1 file changed, 170 insertions(+), 176 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 0e3e343..9d5ccfc 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -64,14 +64,14 @@ struct envdev_s {
int is_ubi; /* set if we use UBI volume */
 };
 
-static struct envdev_s envdevices[2] =
-{
+static struct envdev_s envdevices[2] = {
{
.mtd_type = MTD_ABSENT,
}, {
.mtd_type = MTD_ABSENT,
},
 };
+
 static int dev_current;
 
 #define DEVNAME(i)envdevices[(i)].devname
@@ -88,14 +88,14 @@ static unsigned long usable_envsize;
 #define ENV_SIZE  usable_envsize
 
 struct env_image_single {
-   uint32_tcrc;/* CRC32 over data bytes*/
-   chardata[];
+   uint32_t crc;   /* CRC32 over data bytes*/
+   char data[];
 };
 
 struct env_image_redundant {
-   uint32_tcrc;/* CRC32 over data bytes*/
-   unsigned char   flags;  /* active or obsolete */
-   chardata[];
+   uint32_t crc;   /* CRC32 over data bytes*/
+   unsigned char flags;/* active or obsolete */
+   char data[];
 };
 
 enum flag_scheme {
@@ -105,11 +105,11 @@ enum flag_scheme {
 };
 
 struct environment {
-   void*image;
-   uint32_t*crc;
-   unsigned char   *flags;
-   char*data;
-   enum flag_schemeflag_scheme;
+   void *image;
+   uint32_t *crc;
+   unsigned char *flags;
+   char *data;
+   enum flag_scheme flag_scheme;
 };
 
 static struct environment environment = {
@@ -347,11 +347,11 @@ static int ubi_write(int fd, const void *buf, size_t 
count)
return 0;
 }
 
-static int flash_io (int mode);
+static int flash_io(int mode);
 static int parse_config(struct env_opts *opts);
 
 #if defined(CONFIG_FILE)
-static int get_config (char *);
+static int get_config(char *);
 #endif
 
 static char *skip_chars(char *s)
@@ -394,7 +394,7 @@ static char *envmatch(char *s1, char *s2)
  * Search the environment for a variable.
  * Return the value, if found, or NULL, if not found.
  */
-char *fw_getenv (char *name)
+char *fw_getenv(char *name)
 {
char *env, *nxt;
 
@@ -403,12 +403,12 @@ char *fw_getenv (char *name)
 
for (nxt = env; *nxt; ++nxt) {
if (nxt >= [ENV_SIZE]) {
-   fprintf (stderr, "## Error: "
+   fprintf(stderr, "## Error: "
"environment not terminated\n");
return NULL;
}
}
-   val = envmatch (name, env);
+   val = envmatch(name, env);
if (!val)
continue;
return val;
@@ -462,18 +462,18 @@ int fw_printenv(int argc, char *argv[], int value_only, 
struct env_opts *opts)
if (fw_env_open(opts))
return -1;
 
-   if (argc == 0) {/* Print all env variables  */
+   if (argc == 0) {/* Print all env variables  */
char *env, *nxt;
for (env = environment.data; *env; env = nxt + 1) {
for (nxt = env; *nxt; ++nxt) {
if (nxt >= [ENV_SIZE]) {
-   fprintf (stderr, "## Error: "
+   fprintf(stderr, "## Error: "
"environment not terminated\n");
return -1;
}
}
 
-   printf ("%s\n", env);
+   printf("%s\n", env);
}
fw_env_close(opts);
return 0;
@@ -485,7 +485,7 @@ int fw_printenv(int argc, char *argv[], int value_only, 
struct env_opts *opts)
 
val = fw_getenv(name);
if (!val) {
-   fprintf (stderr, "## Error: \"%s\" not defined\n", 
name);
+   fprintf(stderr, "## Error: \"%s\" not defined\n", name);
rc = -1;
continue;
}
@@ -515,15 +515,13 @@ int fw_env_flush(struct env_opts *opts)
 
/* write environment back to flash */
if (flash_io(O_RDWR)) {
-   fprintf(stderr,
-   "Error: can't write fw_env to flash\n");
-   

[U-Boot] [PATCH v2 2/4] tools: env: Fix CamelCasing style violation

2018-03-09 Thread Alex Kiernan
Replace HaveRedundEnv with have_redund_env to fix style violation.

Signed-off-by: Alex Kiernan 
---

Changes in v2:
- new

 tools/env/fw_env.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 9d5ccfc..f3bfee4 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -116,7 +116,7 @@ static struct environment environment = {
.flag_scheme = FLAG_NONE,
 };
 
-static int HaveRedundEnv = 0;
+static int have_redund_env;
 
 static unsigned char active_flag = 1;
 /* obsolete_flag must be 0 to efficiently set it on NOR flash without erasing 
*/
@@ -1239,7 +1239,7 @@ static int flash_io(int mode)
}
 
if (mode == O_RDWR) {
-   if (HaveRedundEnv) {
+   if (have_redund_env) {
/* switch to next partition for writing */
dev_target = !dev_current;
/* dev_target: fd_target, erase_target */
@@ -1264,7 +1264,7 @@ static int flash_io(int mode)
DEVNAME(dev_current), strerror(errno));
}
 
-   if (HaveRedundEnv) {
+   if (have_redund_env) {
if (fsync(fd_target) &&
!(errno == EINVAL || errno == EROFS)) {
fprintf(stderr,
@@ -1330,7 +1330,7 @@ int fw_env_open(struct env_opts *opts)
/* read environment from FLASH to local buffer */
environment.image = addr0;
 
-   if (HaveRedundEnv) {
+   if (have_redund_env) {
redundant = addr0;
environment.crc = >crc;
environment.flags = >flags;
@@ -1351,7 +1351,7 @@ int fw_env_open(struct env_opts *opts)
crc0 = crc32(0, (uint8_t *)environment.data, ENV_SIZE);
 
crc0_ok = (crc0 == *environment.crc);
-   if (!HaveRedundEnv) {
+   if (!have_redund_env) {
if (!crc0_ok) {
fprintf(stderr,
"Warning: Bad CRC, using default 
environment\n");
@@ -1644,14 +1644,14 @@ static int parse_config(struct env_opts *opts)
 #ifdef DEVICE2_ENVSECTORS
ENVSECTORS(1) = DEVICE2_ENVSECTORS;
 #endif
-   HaveRedundEnv = 1;
+   have_redund_env = 1;
 #endif
 #endif
rc = check_device_config(0);
if (rc < 0)
return rc;
 
-   if (HaveRedundEnv) {
+   if (have_redund_env) {
rc = check_device_config(1);
if (rc < 0)
return rc;
@@ -1664,7 +1664,7 @@ static int parse_config(struct env_opts *opts)
}
 
usable_envsize = CUR_ENVSIZE - sizeof(uint32_t);
-   if (HaveRedundEnv)
+   if (have_redund_env)
usable_envsize -= sizeof(char);
 
return 0;
@@ -1706,7 +1706,7 @@ static int get_config(char *fname)
}
fclose(fp);
 
-   HaveRedundEnv = i - 1;
+   have_redund_env = i - 1;
if (!i) {   /* No valid entries found */
errno = EINVAL;
return -1;
-- 
2.7.4

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


[U-Boot] [PATCH v2 3/4] tools: env: Refactor write path of flash_io()

2018-03-09 Thread Alex Kiernan
Extract write path of flash_io() into a separate function. This patch
should be a functional no-op.

Signed-off-by: Alex Kiernan 
Reviewed-by: Stefano Babic 
---

Changes in v2:
- Acommodate white space changes from predecessor commit

 tools/env/fw_env.c | 92 +-
 1 file changed, 50 insertions(+), 42 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index f3bfee4..600fe5d 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1225,9 +1225,57 @@ static int flash_read(int fd)
return 0;
 }
 
+static int flash_io_write(int fd_current)
+{
+   int fd_target, rc, dev_target;
+
+   if (have_redund_env) {
+   /* switch to next partition for writing */
+   dev_target = !dev_current;
+   /* dev_target: fd_target, erase_target */
+   fd_target = open(DEVNAME(dev_target), O_RDWR);
+   if (fd_target < 0) {
+   fprintf(stderr,
+   "Can't open %s: %s\n",
+   DEVNAME(dev_target), strerror(errno));
+   rc = -1;
+   goto exit;
+   }
+   } else {
+   dev_target = dev_current;
+   fd_target = fd_current;
+   }
+
+   rc = flash_write(fd_current, fd_target, dev_target);
+
+   if (fsync(fd_current) && !(errno == EINVAL || errno == EROFS)) {
+   fprintf(stderr,
+   "fsync failed on %s: %s\n",
+   DEVNAME(dev_current), strerror(errno));
+   }
+
+   if (have_redund_env) {
+   if (fsync(fd_target) &&
+   !(errno == EINVAL || errno == EROFS)) {
+   fprintf(stderr,
+   "fsync failed on %s: %s\n",
+   DEVNAME(dev_current), strerror(errno));
+   }
+
+   if (close(fd_target)) {
+   fprintf(stderr,
+   "I/O error on %s: %s\n",
+   DEVNAME(dev_target), strerror(errno));
+   rc = -1;
+   }
+   }
+ exit:
+   return rc;
+}
+
 static int flash_io(int mode)
 {
-   int fd_current, fd_target, rc, dev_target;
+   int fd_current, rc;
 
/* dev_current: fd_current, erase_current */
fd_current = open(DEVNAME(dev_current), mode);
@@ -1239,51 +1287,11 @@ static int flash_io(int mode)
}
 
if (mode == O_RDWR) {
-   if (have_redund_env) {
-   /* switch to next partition for writing */
-   dev_target = !dev_current;
-   /* dev_target: fd_target, erase_target */
-   fd_target = open(DEVNAME(dev_target), mode);
-   if (fd_target < 0) {
-   fprintf(stderr,
-   "Can't open %s: %s\n",
-   DEVNAME(dev_target), strerror(errno));
-   rc = -1;
-   goto exit;
-   }
-   } else {
-   dev_target = dev_current;
-   fd_target = fd_current;
-   }
-
-   rc = flash_write(fd_current, fd_target, dev_target);
-
-   if (fsync(fd_current) && !(errno == EINVAL || errno == EROFS)) {
-   fprintf(stderr,
-   "fsync failed on %s: %s\n",
-   DEVNAME(dev_current), strerror(errno));
-   }
-
-   if (have_redund_env) {
-   if (fsync(fd_target) &&
-   !(errno == EINVAL || errno == EROFS)) {
-   fprintf(stderr,
-   "fsync failed on %s: %s\n",
-   DEVNAME(dev_current), strerror(errno));
-   }
-
-   if (close(fd_target)) {
-   fprintf(stderr,
-   "I/O error on %s: %s\n",
-   DEVNAME(dev_target), strerror(errno));
-   rc = -1;
-   }
-   }
+   rc = flash_io_write(fd_current);
} else {
rc = flash_read(fd_current);
}
 
- exit:
if (close(fd_current)) {
fprintf(stderr,
"I/O error on %s: %s\n",
-- 
2.7.4

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


Re: [U-Boot] [PATCH 00/18] Introduce SPI TPM v2.0 support

2018-03-09 Thread Tom Rini
On Fri, Mar 09, 2018 at 08:53:40AM +0100, Miquel Raynal wrote:
> Hi Tom,
> 
> On Thu, 8 Mar 2018 12:20:30 -0500, Tom Rini  wrote:
> 
> > On Thu, Mar 08, 2018 at 04:40:03PM +0100, Miquel Raynal wrote:
> > 
> > > Current U-Boot supports TPM v1.2 specification. The new specification
> > > (v2.0) is not backward compatible and renames/introduces several
> > > functions.
> > > 
> > > This series introduces a new SPI driver following the TPM v2.0
> > > specification. It has been tested on a ST TPM but should be usable with
> > > others v2.0 compliant chips.
> > > 
> > > Then, basic functionalities are introduced one by one for the v2.0
> > > specification. The INIT command now can receive a parameter to
> > > distinguish further TPMv1/TPMv2 commands. After that, the library itself
> > > will know which one is pertinent and will return a special error if the
> > > desired command is not supported for the selected specification.  
> > 
> > Thanks for doing all of this.  Can you please enable this feature on
> > sandbox and/or an x86 QEMU variant where I assume we could also then
> > setup automated testing?
> > 
> 
> Not sure I understand your request correctly: the TPM commands are
> already available in the sandbox (I don't see what I could add), I just
> extended the current set of commands.
> 
> However, even with these commands, we won't be able to test them in a
> sandbox unless with an actual device.
> 
> I probably miss something, can you explain a bit more what you would
> like?

Can we add a valid TPM via QEMU and then test it that way?  If so, we
should enable the TPM code on qemu-x86_64 (and, well, if we can pass it
on other arches, other QEMU targets) and write some test/py/tests/ code
that exercises the TPM commands.  Does that make sense?

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm: Disable the strict alignment of data on armv7

2018-03-09 Thread Michal Simek
Dear Wolfgang,

On 9.3.2018 13:02, Wolfgang Denk wrote:
> Dear Michal,
> 
> In message  you wrote:
>>
>> We are not seeing any issue from u-boot code itself and I can believe
>> that structures and accesses are aligned properly.
>>
>> The initial reason was that u-boot allows you to run for example command
>> md 1 and it ends up in panic. Which is something what should be possible
>> to run or code should check that architecture has alignment restriction.
> 
> I'm not sure this needs any "fixing".  Actually I use this on a
> regular base as a simple means to find out if an architecutre allos
> unaligned accesses...  U-Boot is just a boot loader.  It has no
> "normal users" - users are usually developers who know what they are
> doing.  I think this is an area where this old quote applies nicely:
> 
> "UNIX was not designed to stop you from doing stupid things,  because
> that would also stop you from doing clever things."   - Doug Gwyn

I understand your opinion and no problem with it.


>> Definitely panic is not proper reaction on command like this.
> 
> Why not?  Yes, we can blow up the code to handle this in a better
> way, and add some kB of stricgs for nice and user-friendy warnings
> not to do this or that.  But is it really worth the effort?
> I consider this a pretty useful feature and would be sad if it went
> missing. If someone runs into it by accident, he learns a lesson -
> but htere is no real damage done.  Or am I missing anything?

It is not that the patch is removing or changing current behavior.
Default option is to use strict alignment and have an option to change
this behavior.


>> It means we have two options:
>> 1. enable non aligned accesses
>> 2. fix code/commands which enables to pass these non aligned addresses
>> which ends up in panic
> 
> My vorte is for option 3:
> 
> 3. Leave as is.  It's a boot loader, and like other sharp tools it
> can be dangerous if improperly used.

Sure that's also an option but it seems to me weird that by default
armv7 has this requirement and a53 not.


>>> But I guess they still come under a (severe?) performance penalty?
>>
>> It is really a question where that penalty is coming from and when it
>> can happen.
> 
> It comes from the memory and cache architecture.  Just assume an
> unaligned read of 2 bytes that happen to cross a cacheline boundary.
> This access causes a double cache miss; in the worst case this
> causes two cache lines to be flushed to memory and then two
> cachelines to be written from memory.  Instead of just reading two
> bytes, we write 128 bytes and read 128 bytes.  Yes, this is a
> pathological case, but you get the idea.

If this is WB case that write shouldn't be there but you are right that
this could happen.

>> But again the code is align and there is not an issue in code that's why
>> this penalty is not happening. And non align accesses are performed when
>> user asks for this.
> 
> Why not tell the user: don't perform unaligend accesses on this
> platform then?  I can't see the situation where he really _needs_ to
> do something which is not natively supported by his hardware?

It is supported by hardware. U-Boot/SW just doing this configuration not
to allow it.

>>> Also, I wonder if ARM still maintains atomicity for unaligned reads/
>>> writes ? [IIRC, x86 doesn't, while it's guaranteed for aligned
>>> accesses.]
>>
>> Don't know.
> 
> Well, I'm willing to bet a few beer this doesn't work at least in
> pathological cases like example of crossing cache lines above.

:-)

Thanks,
Michal

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


Re: [U-Boot] [PATCH] arm: Disable the strict alignment of data on armv7

2018-03-09 Thread Michal Simek
On 9.3.2018 13:23, Mark Kettenis wrote:
>> From: Wolfgang Denk 
>> Date: Fri, 09 Mar 2018 12:26:01 +0100
>>
>> Dear Michal,
>>
>> In message  you wrote:
>>>
 Can you please add some comments what the consequences of this
 change are?  I guess there are advantages, but I also guess these
 come at a price?
>>>
>>> That's something what I am expecting from this discussion if there are
>>> any corner cases which we are not aware of.
>>>
>>> We found this setting based on randomized testing where simply non
>>> aligned access for memory read was causing exception.
>>
>> Hm... One possible position one can take is that unaligned accesses
>> are likely an indication for incorrect or sloppy programming.
>> usually we design data structures, buffers etc. such that no
>> unaligned accesses take place.  Most code is explicitly written in
>> such a way that it also runs on architectures where unaligned access
>> simply don't work.
>>
>> I feel that providing such an option just opens the door for lousy
>> programmers who don't think throroughly about the code they write.
>>
>> Yes, in some cases this may be conveniendt.  But there are also
>> cases where the problem is just papered over, and hits all the
>> harder later like when you also enable caching.
> 
> That is pretty much why we run OpenBSD in strict alignment mode on as
> much architectures as possible.  More strict-alignment architectures
> means that bugs are found quicker.  So as long as U-Boot supports
> hardware that cares about alignment (e.g. older ARM, MIPS, SuperH) I'd
> say there is a clear benefit of running armv7 in strict alignment mode.
> 
> Also note that some armv7 instructions still require properly aligned
> data.  And that really old ARM hardware silently fetches the wrong
> data for a misaligned load instruction.
> 

No problem with this at all. I wanted to check that this is intended setup.

Thanks,
Michal
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v3] imximage: Remove failure when no IVT offset is found

2018-03-09 Thread Fabio Estevam
From: Fabio Estevam 

Sometimes imximage throws the following error:

  CFGSboard/freescale/vf610twr/imximage.cfg.cfgtmp
  CFGSboard/freescale/vf610twr/imximage.cfg.cfgtmp
  MKIMAGE u-boot-dtb.imx
Error: No BOOT_FROM tag in board/freescale/vf610twr/imximage.cfg.cfgtmp
arch/arm/mach-imx/Makefile:100: recipe for target 'u-boot-dtb.imx' failed

Later on, when running mkimage for the u-boot.imx it will succeed in
finding the IVT offset.

Looks like some race condition happening during parallel build when
processing mkimage for u-boot-dtb.imx and u-boot.imx.

A proper fix still needs to be implemented, but as a workaround let's
remove the error when the IVT offset is not found.

It is useful to have such message, especially during bring-up phase,
but the build error that it causes is severe, so better avoid the
build error for now.

The error checking can be re-implemented later when we have a proper
fix.

Reported-by: Breno Lima 
Reported-by: Thomas Petazzoni 
Signed-off-by: Fabio Estevam 
---
Changes since v2:
- Use my NXP address in the From field.

 tools/imximage.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/tools/imximage.c b/tools/imximage.c
index eb7e682..ed9d935 100644
--- a/tools/imximage.c
+++ b/tools/imximage.c
@@ -777,11 +777,6 @@ static uint32_t parse_cfg_file(struct imx_header *imxhdr, 
char *name)
(*set_dcd_rst)(imxhdr, dcd_len, name, lineno);
fclose(fd);
 
-   /* Exit if there is no BOOT_FROM field specifying the flash_offset */
-   if (imximage_ivt_offset == FLASH_OFFSET_UNDEFINED) {
-   fprintf(stderr, "Error: No BOOT_FROM tag in %s\n", name);
-   exit(EXIT_FAILURE);
-   }
return dcd_len;
 }
 
-- 
2.7.4

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


Re: [U-Boot] [PATCH] arm: Disable the strict alignment of data on armv7

2018-03-09 Thread Wolfgang Denk
Dear Michal,

In message  you wrote:
> 
> > Can you please add some comments what the consequences of this
> > change are?  I guess there are advantages, but I also guess these
> > come at a price?
> 
> That's something what I am expecting from this discussion if there are
> any corner cases which we are not aware of.
> 
> We found this setting based on randomized testing where simply non
> aligned access for memory read was causing exception.

Hm... One possible position one can take is that unaligned accesses
are likely an indication for incorrect or sloppy programming.
usually we design data structures, buffers etc. such that no
unaligned accesses take place.  Most code is explicitly written in
such a way that it also runs on architectures where unaligned access
simply don't work.

I feel that providing such an option just opens the door for lousy
programmers who don't think throroughly about the code they write.

Yes, in some cases this may be conveniendt.  But there are also
cases where the problem is just papered over, and hits all the
harder later like when you also enable caching.

Thi is why I'm asking for the advantages.  If it's just is that
broken code starts working, then I'd rather see a clear error
message that forces the code to be fixed.

> The same tests were running fine on arm64 where non aligned accesses are
> permitted.

But I guess they still come under a (severe?) performance penalty?

> It is hard to compare performance impact on u-boot but from
> functionality point of view we are not able to see difference.
> If there is any performance impact that it is quite low.

Hm I have to admit that I don't know ARM/ARM64 that well, but I
am pretty sure that even on ARM an aligned 32 bit access on a 32 bit
bus will result in a single read cycle on that bus - while I would
expect that an unaligned 32 bit access might get broken down into 4
byte accesses?

Also, I wonder if ARM still maintains atomicity for unaligned reads/
writes ? [IIRC, x86 doesn't, while it's guaranteed for aligned
accesses.]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
What is mind?  No matter.  What is matter?  Never mind.
  -- Thomas Hewitt Key, 1799-1875
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm: Disable the strict alignment of data on armv7

2018-03-09 Thread Wolfgang Denk
Dear Michal,

In message  you wrote:
> 
> We are not seeing any issue from u-boot code itself and I can believe
> that structures and accesses are aligned properly.
> 
> The initial reason was that u-boot allows you to run for example command
> md 1 and it ends up in panic. Which is something what should be possible
> to run or code should check that architecture has alignment restriction.

I'm not sure this needs any "fixing".  Actually I use this on a
regular base as a simple means to find out if an architecutre allos
unaligned accesses...  U-Boot is just a boot loader.  It has no
"normal users" - users are usually developers who know what they are
doing.  I think this is an area where this old quote applies nicely:

"UNIX was not designed to stop you from doing stupid things,  because
that would also stop you from doing clever things."   - Doug Gwyn

> Definitely panic is not proper reaction on command like this.

Why not?  Yes, we can blow up the code to handle this in a better
way, and add some kB of stricgs for nice and user-friendy warnings
not to do this or that.  But is it really worth the effort?
I consider this a pretty useful feature and would be sad if it went
missing. If someone runs into it by accident, he learns a lesson -
but htere is no real damage done.  Or am I missing anything?

> It means we have two options:
> 1. enable non aligned accesses
> 2. fix code/commands which enables to pass these non aligned addresses
> which ends up in panic

My vorte is for option 3:

3. Leave as is.  It's a boot loader, and like other sharp tools it
can be dangerous if improperly used.

> > But I guess they still come under a (severe?) performance penalty?
> 
> It is really a question where that penalty is coming from and when it
> can happen.

It comes from the memory and cache architecture.  Just assume an
unaligned read of 2 bytes that happen to cross a cacheline boundary.
This access causes a double cache miss; in the worst case this
causes two cache lines to be flushed to memory and then two
cachelines to be written from memory.  Instead of just reading two
bytes, we write 128 bytes and read 128 bytes.  Yes, this is a
pathological case, but you get the idea.

> But again the code is align and there is not an issue in code that's why
> this penalty is not happening. And non align accesses are performed when
> user asks for this.

Why not tell the user: don't perform unaligend accesses on this
platform then?  I can't see the situation where he really _needs_ to
do something which is not natively supported by his hardware?

> > Also, I wonder if ARM still maintains atomicity for unaligned reads/
> > writes ? [IIRC, x86 doesn't, while it's guaranteed for aligned
> > accesses.]
> 
> Don't know.

Well, I'm willing to bet a few beer this doesn't work at least in
pathological cases like example of crossing cache lines above.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
He'd heard her use that sweet, innocent  tone  of  voice  before.  It
meant that, pretty soon, there was going to be trouble.
- Terry Pratchett, _Truckers_
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 2/2] [for v2018.03] ARM: dts: imx6dl-icore-rqs: Fix to include correct dtsi

2018-03-09 Thread Jagan Teki
Hi Stefano,

On Tue, Mar 6, 2018 at 6:45 PM, Fabio Estevam  wrote:
> On Tue, Mar 6, 2018 at 8:45 AM, Jagan Teki  wrote:
>> This patch fixes the wrongly included dtsi file which was
>> breaking mainline support for Engicam i.CoreM6 DualLite/Solo RQS.
>>
>> Linux commit details for the same change as
>> "ARM: dts: imx6dl: Include correct dtsi file for Engicam i.CoreM6
>> DualLite/Solo RQS"
>> (sha1: c0c6bb2322964bd264b4ddedaa5776f40c709f0c)
>>
>> Signed-off-by: Jagan Teki 
>
> Reviewed-by: Fabio Estevam 

Can you pick these two for the release?
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v2 4/4] tools: env: Implement atomic replace for filesystem

2018-03-09 Thread Alex Kiernan
If the U-Boot environment is stored in a regular file and redundant
operation isn't set, then write to a temporary file and perform an
atomic rename.

Signed-off-by: Alex Kiernan 
---

Changes in v2:
- Use mkstemp() to create temporary filename, not a fixed one

 tools/env/fw_env.c | 83 --
 1 file changed, 80 insertions(+), 3 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 600fe5d..77eac3d 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1225,9 +1226,48 @@ static int flash_read(int fd)
return 0;
 }
 
+static int flash_open_tempfile(const char **dname, const char **target_temp)
+{
+   char *dup_name = strdup(DEVNAME(dev_current));
+   char *temp_name = NULL;
+   int rc = -1;
+
+   if (!dup_name)
+   return -1;
+
+   *dname = dirname(dup_name);
+   if (!*dname)
+   goto err;
+
+   rc = asprintf(_name, "%s/XX", *dname);
+   if (rc == -1)
+   goto err;
+
+   rc = mkstemp(temp_name);
+   if (rc == -1) {
+   /* fall back to in place write */
+   fprintf(stderr,
+   "Can't create %s: %s\n", temp_name, strerror(errno));
+   free(temp_name);
+   } else {
+   *target_temp = temp_name;
+   /* deliberately leak dup_name as dname /might/ point into
+* it and we need it for our caller
+*/
+   dup_name = NULL;
+   }
+
+err:
+   if (dup_name)
+   free(dup_name);
+
+   return rc;
+}
+
 static int flash_io_write(int fd_current)
 {
-   int fd_target, rc, dev_target;
+   int fd_target = -1, rc, dev_target;
+   const char *dname, *target_temp = NULL;
 
if (have_redund_env) {
/* switch to next partition for writing */
@@ -1242,8 +1282,17 @@ static int flash_io_write(int fd_current)
goto exit;
}
} else {
+   struct stat sb;
+
+   if (fstat(fd_current, ) == 0 && S_ISREG(sb.st_mode)) {
+   /* if any part of flash_open_tempfile() fails we fall
+* back to in-place writes
+*/
+   fd_target = flash_open_tempfile(, _temp);
+   }
dev_target = dev_current;
-   fd_target = fd_current;
+   if (fd_target == -1)
+   fd_target = fd_current;
}
 
rc = flash_write(fd_current, fd_target, dev_target);
@@ -1254,7 +1303,7 @@ static int flash_io_write(int fd_current)
DEVNAME(dev_current), strerror(errno));
}
 
-   if (have_redund_env) {
+   if (fd_current != fd_target) {
if (fsync(fd_target) &&
!(errno == EINVAL || errno == EROFS)) {
fprintf(stderr,
@@ -1268,6 +1317,34 @@ static int flash_io_write(int fd_current)
DEVNAME(dev_target), strerror(errno));
rc = -1;
}
+
+   if (target_temp) {
+   int dir_fd;
+
+   dir_fd = open(dname, O_DIRECTORY | O_RDONLY);
+   if (dir_fd == -1)
+   fprintf(stderr,
+   "Can't open %s: %s\n",
+   dname, strerror(errno));
+
+   if (rename(target_temp, DEVNAME(dev_target))) {
+   fprintf(stderr,
+   "rename failed %s => %s: %s\n",
+   target_temp, DEVNAME(dev_target),
+   strerror(errno));
+   rc = -1;
+   }
+
+   if (dir_fd != -1 && fsync(dir_fd))
+   fprintf(stderr,
+   "fsync failed on %s: %s\n",
+   dname, strerror(errno));
+
+   if (dir_fd != -1 && close(dir_fd))
+   fprintf(stderr,
+   "I/O error on %s: %s\n",
+   dname, strerror(errno));
+   }
}
  exit:
return rc;
-- 
2.7.4

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


Re: [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems

2018-03-09 Thread Wolfgang Denk
Dear Alex,

In message <1520597582-12979-1-git-send-email-alex.kier...@gmail.com> you wrote:
> 
> For environments stored on filesystems where you can't have a redundant
> configuration, rather than just over-writing the existing environment
> in fw_setenv, do the tradtional create temporary file, rename, sync,
> sync directory dance to achieve ACID semantics when writing through
> fw_setenv.

I isagree with the statement that we _can't have_ redundant
environments in a file system.  It may not be supported by the
current implementation, but there is actually nothing that would
prevent us to come up with more powerful code that supports exactly
such a feature.

[This does not mean that I disagree with your patch - on contary, I
think it's good.]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
A person with one watch knows what time it  is;  a  person  with  two
watches is never sure.   Proverb
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PULL] Please pull u-boot-imx

2018-03-09 Thread Stefano Babic
Hi Tom,

Fabio fixed a build issue yesterday. It is just a single patch, but it
should be flow into the release. You can pick it udirectly p, but I have
also applied to u-boot-imx to make things easier.


 The following changes since commit
5e62f828256d66e2b28def4f9ef20a2a05c2d04f:

  Prepare v2018.03-rc4 (2018-03-05 20:27:08 -0500)

are available in the git repository at:

  git://www.denx.de/git/u-boot-imx.git master

for you to fetch changes up to eca6f053c89bca8ac702e8cf74d187cce2197acb:

  imximage: Check the IVT offset in the correct location (2018-03-08
22:36:01 +0100)


Fabio Estevam (1):
  imximage: Check the IVT offset in the correct location

 tools/imximage.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

Stefano

-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 2/2] tools: env: Implement atomic replace for filesystem

2018-03-09 Thread Alex Kiernan
On Fri, Mar 9, 2018 at 10:03 AM, Stefano Babic  wrote:
> On 09/03/2018 10:54, Alex Kiernan wrote:
>> On Thu, Mar 8, 2018 at 5:04 PM, Stefano Babic  wrote:
>>> Hi alex,
>>>
>>> On 08/03/2018 12:52, Alex Kiernan wrote:
 If the U-Boot environment is stored in a regular file and redundant
 operation isn't set, then write to a temporary file and perform an
 atomic rename.

>>>
>>> Even if it is not explicitely set (IMHO it should), this code can
>>> be used as library and linked to own application. That means that
>>> concurrency can happens. I mean:
>>>
>>
>> At this point you're writing the new environment, so we should hold a
>> lock to avoid concurrent writes.
>
> This is already done, even if not in the way I like.
> tools/env/fw_env_main.c calls flock() to acquire the resource. This
> works using fw_printenv / fw_setenv, but not as library. Library's users
> as me have to provide themselves a lock before calling the fw_* functions.
>

True... that particular implementation also causes me a different
problem in that it fails on a read-only fs and I have a need for a
call to fw_printenv before the filesystem has gone read-write.

-- 
Alex Kiernan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] imx: syscounter: make sure asm is volatile

2018-03-09 Thread Fabio Estevam
[Adding Stefano]

On Thu, Mar 8, 2018 at 1:21 AM, Yasushi SHOJI  wrote:
> Without the volatile attribute, compilers are entitled to optimize out
> the same asm().  In the case of __udelay() in syscounter.c, it calls
> `get_ticks()` twice, one for the starting time and the second in the
> loop to check the current time.  When compilers inline `get_ticks()`
> they see the same `mrrc` instructions and optimize out the second one.
> This leads to infinite loop since we don't get updated value from the
> system counter.
>
> Here is a portion of the disassembly of __udelay:
>
>   88:   428bcmp r3, r1
>   8a:   f8ce 20a4   str.w   r2, [lr, #164]  ; 0xa4
>   8e:   bf08it  eq
>   90:   4282cmpeq   r2, r0
>   92:   f8ce 30a0   str.w   r3, [lr, #160]  ; 0xa0
>   96:   d3f7bcc.n   88 <__udelay+0x88>
>   98:   e8bd 8cf0   ldmia.w sp!, {r4, r5, r6, r7, sl, fp, pc}
>
> Note that final jump / loop at 96 to 88, we don't have any `mrrc`.
>
> With a volatile attribute, the above changes to this:
>
>   8a:   ec53 2f0e   mrrc15, 0, r2, r3, cr14
>   8e:   42abcmp r3, r5
>   90:   f8c1 20a4   str.w   r2, [r1, #164]  ; 0xa4
>   94:   bf08it  eq
>   96:   42a2cmpeq   r2, r4
>   98:   f8c1 30a0   str.w   r3, [r1, #160]  ; 0xa0
>   9c:   d3f5bcc.n   8a <__udelay+0x8a>
>   9e:   e8bd 8cf0   ldmia.w sp!, {r4, r5, r6, r7, sl, fp, pc}
>   a2:   bf00nop
>
> I'm advised[1] to put volatile on all asm(), so this commit also adds it
> to the asm() in timer_init().
>
> [1]: https://lists.denx.de/pipermail/u-boot/2018-March/322062.html
>
> Signed-off-by: Yasushi SHOJI 
> Reviewed-by: Fabio Estevam 

Hi Stefano,

Do you think this one could be applied for the upcoming release?

Thanks

> ---
>  arch/arm/mach-imx/syscounter.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-imx/syscounter.c b/arch/arm/mach-imx/syscounter.c
> index 9290918dca..1d4ebfe343 100644
> --- a/arch/arm/mach-imx/syscounter.c
> +++ b/arch/arm/mach-imx/syscounter.c
> @@ -62,7 +62,7 @@ int timer_init(void)
> unsigned long val, freq;
>
> freq = CONFIG_SC_TIMER_CLK;
> -   asm("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq));
> +   asm volatile("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq));
>
> writel(freq, >cntfid0);
>
> @@ -82,7 +82,7 @@ unsigned long long get_ticks(void)
>  {
> unsigned long long now;
>
> -   asm("mrrc p15, 0, %Q0, %R0, c14" : "=r" (now));
> +   asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (now));
>
> gd->arch.tbl = (unsigned long)(now & 0x);
> gd->arch.tbu = (unsigned long)(now >> 32);
> --
> 2.16.2
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PULL] Please pull u-boot-imx

2018-03-09 Thread Fabio Estevam
Hi Stefano,

On Fri, Mar 9, 2018 at 8:11 AM, Stefano Babic  wrote:
If you like I can resent a v2 of option 1 where the error message is removed.
>
> It is a hack, but we can do just for the release. I have already applied
> V1, I will drop it from u-boot-imx. I will also apply "imx: syscounter:
> make sure asm is volatile" as you told me today, and then I send a PR
> with these two patches to Tom.

Sounds good! I have sent a v3 patch.

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


Re: [U-Boot] [PATCH] dm: mmc: socfpga: call dwmci_probe()

2018-03-09 Thread Patrick Brünn
>From: Patrick Brünn
>Sent: Donnerstag, 8. März 2018 06:39
>>From: Jaehoon Chung [mailto:jh80.ch...@samsung.com]
>>Sent: Donnerstag, 8. März 2018 04:57
>>On 03/08/2018 12:12 PM, Marek Vasut wrote:
>>> On 03/08/2018 03:17 AM, Jaehoon Chung wrote:
 On 03/06/2018 05:07 PM, linux-kernel-...@beckhoff.com wrote:
> From: Patrick Bruenn 
>
> On a socfpga_cyclone5 based board the SD card, was never powered
>up.
>>For
> other dw_mmc based SoCs dwmci_probe() is called in the platform
>>specific
> probe(). It seems this call is missing for socfpga_dw_mmc.
>
> With this change DWMCI_PWREN is set by dmwci_init().
>
> Signed-off-by: Patrick Bruenn 

 Reviewed-by: Jaehoon Chung 

 Will apply this patch before releasing v2018.03.
 (I have a problem about accessing git.denx.de. After fixing my problem,
>>will resend email about applying.)
>>>
>>> DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand
>>what this patch is trying to fix. I'd prefer if you did not hastily apply 
>>this.
>>
>>It's my misunderstanding. When i checked more. I think that Marek is right.
>>Thanks Marek for pointing out.
>>
>Okay, but do you have any hint what I am doing wrong? My board (cx8100 not
>mainline, yet) is based on socfpga_cyclone5. And on my board "
>dwmci_writel(host, DWMCI_PWREN, 1);" is never called, because
>dwmci_init() is never called.
>As far as I can see with CONFIG_DM_MMC enabled dwmci_init() should be
>called by dwmci_probe().
>
>exynos  and rockchip do call dwmci_probe() within
>exynos/rockchip_dwmmc_probe().
>but socfpga_dwmmc_probe() is missing this call. So I looked deeper but found
>no place for socfpga platform to call dwmci_probe() or dwmci_init().
>What am I missing?
>
I got an idea, what might be the difference between my board and your boards.
I suspect you use U-BOOT SPL without CONFIG_DM_MMC set, so
dwmci_init() is called indirectly by mmc_start_init().
Now, when your main U-Boot (with CONFIG_DM_MMC) is launched,
everything is already configured and it isn't necessary to call dwmci_init() 
again.
On my board the Altera MPL is used (and I can't replace it).  Which seems to
disable DWMCI_PWREN before launching U-Boot.
If my assumption is correct, I still think it is a U-Boot bug to assume code 
like in
dwmci_init() was already run before U-Boot gets in control.
Besides exynos/rockchip_dw_mmc don't have this precondition requirement.

Please take your time to look deeper into this issue, before deciding anything.
I don't think we need to rush this into the next release, as normal mainline
boards are "accidently" not affected.

Thanks and regards,
Patrick

Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075


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


Re: [U-Boot] [PATCH] mx6sabresd: Select the CONFIG_EFI_PARTITION option

2018-03-09 Thread Jagan Teki
Hi Fabio,

On Sun, Feb 18, 2018 at 3:14 AM, Fabio Estevam  wrote:
> From: Fabio Estevam 
>
> With fastboot support enabled, it is useful to be able to list
> the eMMC EFI partitions, so select the CONFIG_EFI_PARTITION option.
>
> Signed-off-by: Fabio Estevam 
> ---
> Hi Stefano,
>
> It seems you picked the v1 of my fastboot patch instead of v2.
>
> Hence I am sending a new patch that only selects CONFIG_EFI_PARTITION.
>
>  configs/mx6sabresd_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configs/mx6sabresd_defconfig b/configs/mx6sabresd_defconfig
> index feb385d..55709e8 100644
> --- a/configs/mx6sabresd_defconfig
> +++ b/configs/mx6sabresd_defconfig
> @@ -48,6 +48,7 @@ CONFIG_CMD_EXT4=y
>  CONFIG_CMD_EXT4_WRITE=y
>  CONFIG_CMD_FAT=y
>  CONFIG_CMD_FS_GENERIC=y
> +CONFIG_EFI_PARTITION=y

Do you have a partition table details, I'm trying GPT as below

+#define ROOT_UUID   "69dad710-2ce4-4e3c-b16c-21a1d49abed3"
+#define PARTS_DEFAULT \
+"name=loader1,start=1k,size=64K,uuid=${uuid_gpt_loader1};" \
+"name=loader2,start=69K,size=827K,uuid=${uuid_gpt_loader2};" \
+"name=uEnv,start=896K,size=128K,uuid=${uuid_gpt_uenv};" \
+"name=rootfs,size=-,uuid="ROOT_UUID

=> gpt write mmc 0 $partitions
Writing GPT: Partition overlap
error!

Look like loader1 start=1k wont working, it 's always starting
0x0022 offset if we unspecify start but SPL wont start there
right?

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


[U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems

2018-03-09 Thread Alex Kiernan

For environments stored on filesystems where you can't have a redundant
configuration, rather than just over-writing the existing environment
in fw_setenv, do the tradtional create temporary file, rename, sync,
sync directory dance to achieve ACID semantics when writing through
fw_setenv.

fw_env.c has had whitespace modified and HaveRedundEnv renamed in order
to avoid triggering large numbers of checkpatch warnings because of the
existing code style:

  warning: space prohibited between function name and open parenthesis '('
  check: Avoid CamelCase: 

Changes in v2:
- Clean style violations in existing code
- Acommodate white space changes from predecessor commit
- Use mkstemp() to create temporary filename, not a fixed one

Alex Kiernan (4):
  tools: env: Pass through indent
  tools: env: Fix CamelCasing style violation
  tools: env: Refactor write path of flash_io()
  tools: env: Implement atomic replace for filesystem

 tools/env/fw_env.c | 483 +++--
 1 file changed, 281 insertions(+), 202 deletions(-)

-- 
2.7.4

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


Re: [U-Boot] [PATCH] arm: Disable the strict alignment of data on armv7

2018-03-09 Thread Mark Kettenis
> From: Wolfgang Denk 
> Date: Fri, 09 Mar 2018 12:26:01 +0100
> 
> Dear Michal,
> 
> In message  you wrote:
> > 
> > > Can you please add some comments what the consequences of this
> > > change are?  I guess there are advantages, but I also guess these
> > > come at a price?
> > 
> > That's something what I am expecting from this discussion if there are
> > any corner cases which we are not aware of.
> > 
> > We found this setting based on randomized testing where simply non
> > aligned access for memory read was causing exception.
> 
> Hm... One possible position one can take is that unaligned accesses
> are likely an indication for incorrect or sloppy programming.
> usually we design data structures, buffers etc. such that no
> unaligned accesses take place.  Most code is explicitly written in
> such a way that it also runs on architectures where unaligned access
> simply don't work.
> 
> I feel that providing such an option just opens the door for lousy
> programmers who don't think throroughly about the code they write.
> 
> Yes, in some cases this may be conveniendt.  But there are also
> cases where the problem is just papered over, and hits all the
> harder later like when you also enable caching.

That is pretty much why we run OpenBSD in strict alignment mode on as
much architectures as possible.  More strict-alignment architectures
means that bugs are found quicker.  So as long as U-Boot supports
hardware that cares about alignment (e.g. older ARM, MIPS, SuperH) I'd
say there is a clear benefit of running armv7 in strict alignment mode.

Also note that some armv7 instructions still require properly aligned
data.  And that really old ARM hardware silently fetches the wrong
data for a misaligned load instruction.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PULL] Please pull u-boot-imx

2018-03-09 Thread Fabio Estevam
Hi Stefano,

On Fri, Mar 9, 2018 at 7:13 AM, Stefano Babic  wrote:
> Hi Tom,
>
> Fabio fixed a build issue yesterday. It is just a single patch, but it
> should be flow into the release. You can pick it udirectly p, but I have
> also applied to u-boot-imx to make things easier.

Troy made a good observation yesterday: he asked me if after applying
this patch the error is still shown when BOOT_FROM is not passed in
the cfg file.

I tested it and it does not.

So we have some options here that I would like to discuss:

1. Drop the error message completely. This message is only helpful for
developers during bring up phase

2. Understand better the problem and fix the parallel build issue

I think 1 can be done for the upcoming release as the negative effect
of breaking builds is worse than showing the lack of BOOT_FROM tag.

Option number 2 would require more time to implement.

If you like I can resent a v2 of option 1 where the error message is removed.

Please let me know your thoughts.

Thanks,

Fabio Estevam
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PULL] Please pull u-boot-imx

2018-03-09 Thread Stefano Babic
Hi Fabio,

On 09/03/2018 11:42, Fabio Estevam wrote:
> Hi Stefano,
> 
> On Fri, Mar 9, 2018 at 7:13 AM, Stefano Babic  wrote:
>> Hi Tom,
>>
>> Fabio fixed a build issue yesterday. It is just a single patch, but it
>> should be flow into the release. You can pick it udirectly p, but I have
>> also applied to u-boot-imx to make things easier.
> 
> Troy made a good observation yesterday: he asked me if after applying
> this patch the error is still shown when BOOT_FROM is not passed in
> the cfg file.
> 
> I tested it and it does not.

Something strange happens...

> 
> So we have some options here that I would like to discuss:
> 
> 1. Drop the error message completely. This message is only helpful for
> developers during bring up phase
> 
> 2. Understand better the problem and fix the parallel build issue
> 
> I think 1 can be done for the upcoming release as the negative effect
> of breaking builds is worse than showing the lack of BOOT_FROM tag.

My concern is more that build is broken, yes. Release is now upcoming,
we can do just a few changes.

> 
> Option number 2 would require more time to implement.
> 
> If you like I can resent a v2 of option 1 where the error message is removed.

It is a hack, but we can do just for the release. I have already applied
V1, I will drop it from u-boot-imx. I will also apply "imx: syscounter:
make sure asm is volatile" as you told me today, and then I send a PR
with these two patches to Tom.

Best regards,
Stefano

-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm: Disable the strict alignment of data on armv7

2018-03-09 Thread Michal Simek
Dear Wolfgang,

On 9.3.2018 12:26, Wolfgang Denk wrote:
> Dear Michal,
> 
> In message  you wrote:
>>
>>> Can you please add some comments what the consequences of this
>>> change are?  I guess there are advantages, but I also guess these
>>> come at a price?
>>
>> That's something what I am expecting from this discussion if there are
>> any corner cases which we are not aware of.
>>
>> We found this setting based on randomized testing where simply non
>> aligned access for memory read was causing exception.
> 
> Hm... One possible position one can take is that unaligned accesses
> are likely an indication for incorrect or sloppy programming.
> usually we design data structures, buffers etc. such that no
> unaligned accesses take place.  Most code is explicitly written in
> such a way that it also runs on architectures where unaligned access
> simply don't work.
> 
> I feel that providing such an option just opens the door for lousy
> programmers who don't think throroughly about the code they write.
> 
> Yes, in some cases this may be conveniendt.  But there are also
> cases where the problem is just papered over, and hits all the
> harder later like when you also enable caching.
> 
> Thi is why I'm asking for the advantages.  If it's just is that
> broken code starts working, then I'd rather see a clear error
> message that forces the code to be fixed.

We are not seeing any issue from u-boot code itself and I can believe
that structures and accesses are aligned properly.

The initial reason was that u-boot allows you to run for example command
md 1 and it ends up in panic. Which is something what should be possible
to run or code should check that architecture has alignment restriction.

Definitely panic is not proper reaction on command like this.
It means we have two options:
1. enable non aligned accesses
2. fix code/commands which enables to pass these non aligned addresses
which ends up in panic

The patch is targeting option 1 because on arm64 it is permitted.

>> The same tests were running fine on arm64 where non aligned accesses are
>> permitted.
> 
> But I guess they still come under a (severe?) performance penalty?

It is really a question where that penalty is coming from and when it
can happen.

>> It is hard to compare performance impact on u-boot but from
>> functionality point of view we are not able to see difference.
>> If there is any performance impact that it is quite low.
> 
> Hm I have to admit that I don't know ARM/ARM64 that well, but I
> am pretty sure that even on ARM an aligned 32 bit access on a 32 bit
> bus will result in a single read cycle on that bus - while I would
> expect that an unaligned 32 bit access might get broken down into 4
> byte accesses?

I would expect that there will be more cycles for read as you mentioned.

But again the code is align and there is not an issue in code that's why
this penalty is not happening. And non align accesses are performed when
user asks for this.

> 
> Also, I wonder if ARM still maintains atomicity for unaligned reads/
> writes ? [IIRC, x86 doesn't, while it's guaranteed for aligned
> accesses.]

Don't know.

Thanks,
Michal
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] imx: syscounter: make sure asm is volatile

2018-03-09 Thread Stefano Babic
On 09/03/2018 11:46, Fabio Estevam wrote:
> [Adding Stefano]
> 
> On Thu, Mar 8, 2018 at 1:21 AM, Yasushi SHOJI  wrote:
>> Without the volatile attribute, compilers are entitled to optimize out
>> the same asm().  In the case of __udelay() in syscounter.c, it calls
>> `get_ticks()` twice, one for the starting time and the second in the
>> loop to check the current time.  When compilers inline `get_ticks()`
>> they see the same `mrrc` instructions and optimize out the second one.
>> This leads to infinite loop since we don't get updated value from the
>> system counter.
>>
>> Here is a portion of the disassembly of __udelay:
>>
>>   88:   428bcmp r3, r1
>>   8a:   f8ce 20a4   str.w   r2, [lr, #164]  ; 0xa4
>>   8e:   bf08it  eq
>>   90:   4282cmpeq   r2, r0
>>   92:   f8ce 30a0   str.w   r3, [lr, #160]  ; 0xa0
>>   96:   d3f7bcc.n   88 <__udelay+0x88>
>>   98:   e8bd 8cf0   ldmia.w sp!, {r4, r5, r6, r7, sl, fp, pc}
>>
>> Note that final jump / loop at 96 to 88, we don't have any `mrrc`.
>>
>> With a volatile attribute, the above changes to this:
>>
>>   8a:   ec53 2f0e   mrrc15, 0, r2, r3, cr14
>>   8e:   42abcmp r3, r5
>>   90:   f8c1 20a4   str.w   r2, [r1, #164]  ; 0xa4
>>   94:   bf08it  eq
>>   96:   42a2cmpeq   r2, r4
>>   98:   f8c1 30a0   str.w   r3, [r1, #160]  ; 0xa0
>>   9c:   d3f5bcc.n   8a <__udelay+0x8a>
>>   9e:   e8bd 8cf0   ldmia.w sp!, {r4, r5, r6, r7, sl, fp, pc}
>>   a2:   bf00nop
>>
>> I'm advised[1] to put volatile on all asm(), so this commit also adds it
>> to the asm() in timer_init().
>>
>> [1]: https://lists.denx.de/pipermail/u-boot/2018-March/322062.html
>>
>> Signed-off-by: Yasushi SHOJI 
>> Reviewed-by: Fabio Estevam 
> 
> Hi Stefano,
> 
> Do you think this one could be applied for the upcoming release?
> 


Applied as fix as already discussed in other thread, thanks !

Best regards,
Stefano

-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v3] imximage: Remove failure when no IVT offset is found

2018-03-09 Thread Stefano Babic
On 09/03/2018 12:25, Fabio Estevam wrote:
> From: Fabio Estevam 
> 
> Sometimes imximage throws the following error:
> 
>   CFGSboard/freescale/vf610twr/imximage.cfg.cfgtmp
>   CFGSboard/freescale/vf610twr/imximage.cfg.cfgtmp
>   MKIMAGE u-boot-dtb.imx
> Error: No BOOT_FROM tag in board/freescale/vf610twr/imximage.cfg.cfgtmp
> arch/arm/mach-imx/Makefile:100: recipe for target 'u-boot-dtb.imx' failed
> 
> Later on, when running mkimage for the u-boot.imx it will succeed in
> finding the IVT offset.
> 
> Looks like some race condition happening during parallel build when
> processing mkimage for u-boot-dtb.imx and u-boot.imx.
> 
> A proper fix still needs to be implemented, but as a workaround let's
> remove the error when the IVT offset is not found.
> 
> It is useful to have such message, especially during bring-up phase,
> but the build error that it causes is severe, so better avoid the
> build error for now.
> 
> The error checking can be re-implemented later when we have a proper
> fix.
> 
> Reported-by: Breno Lima 
> Reported-by: Thomas Petazzoni 
> Signed-off-by: Fabio Estevam 
> ---
> Changes since v2:
> - Use my NXP address in the From field.
> 
>  tools/imximage.c | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/tools/imximage.c b/tools/imximage.c
> index eb7e682..ed9d935 100644
> --- a/tools/imximage.c
> +++ b/tools/imximage.c
> @@ -777,11 +777,6 @@ static uint32_t parse_cfg_file(struct imx_header 
> *imxhdr, char *name)
>   (*set_dcd_rst)(imxhdr, dcd_len, name, lineno);
>   fclose(fd);
>  
> - /* Exit if there is no BOOT_FROM field specifying the flash_offset */
> - if (imximage_ivt_offset == FLASH_OFFSET_UNDEFINED) {
> - fprintf(stderr, "Error: No BOOT_FROM tag in %s\n", name);
> - exit(EXIT_FAILURE);
> - }
>   return dcd_len;
>  }

As discussed, applied as fix for the release - thanks !

Best regards,
Stefano Babic

-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v3 13/20] Revert: arm64: allwinner: a64: pine64: Use dcdc1 regulator for mmc0

2018-03-09 Thread Maxime Ripard
Hi,

On Fri, Mar 02, 2018 at 04:24:22PM +, Andre Przywara wrote:
> On 02/03/18 15:58, Maxime Ripard wrote:
> > On Fri, Mar 02, 2018 at 12:56:52AM +, Andre Przywara wrote:
> >> Linux kernels before 4.12-rc1 don't know about the AXP803 PMIC, so can't
> >> enable the MMC driver with the current DT anymore, because that now
> >> depends on this regulator.
> > 
> > Given that only I2C, USB and MMC were supported at the time, is it
> > really worth it? There's a lot of incentive to move to a newer kernel
> > given the minimal support it had at the time.
> 
> Yeah, this is somewhat true, although serial, USB and MMC are somewhat
> enough if one uses a USB Ethernet or WiFi adapter. And the kernel choice
> might not always a decision of the user (thinking of distributions here).
> But I was actually unsure about this as well, and wanted to hear some
> opinions.
> 
> One thing that comes to mind is other OSes. Does anyone know how if for
> instance FreeBSD supports the AXP and its Linux bindings? The whole goal
> of this series is to allow booting OSes with U-Boot's DT copy
> ($fdtcontroladdr), so if this patch would make their life easier, it
> might be worth it.

I thought the DT was only about the hardware (and the firmware to some
extent) and not about a particular implementation?

> Regarding the whole forward/backward compatibility:
> I clearly see the difficulty in coming up with a "perfect" DT from day
> one, especially for badly documented SoCs, where mainlining is driven by
> hobbyists. So I was wondering if we introduce a grace period, where we
> declare the DT "unstable" or "subject to incompatible changes" for some
> releases (not too many). In hindsight we might declare 4.12 the stable
> DT base for the A64, for instance.
> This would allow us to start upstreaming early, with a small feature set
> only (just serial + clocks + pinctrl, as for the H6). Additional
> features (PMIC) might then add small incompatibilities (like this one
> here), until we are reasonably confident about the DT.
> Does that sound useful?

Well, yeah. It's one of reasons I listed when the DT stability was
discussed. I'm glad we reached an agreement on this :)

On a more serious basis, while that would sound reasonable, there's
also the issue that you're basically never done, so that period might
extend indefinitely.

Even on the older SoCs that we first introduce 5 years ago we have to
deal with remodelling the bindings, for mostly three reasons:
  - We have some bindings that we were aware were suboptimal, and the
stability lockdown took us by surprise, and we got stuck with
bindings no one really liked.

  - We have been ignoring some issues for quite some time that will
need to get fixed eventually. For example, the power domains in
the A31 are in this situation: since we didn't have any device
that would allow us to test it, and since we couldn't use the
genpd code for the CPUs in Linux at the time, we don't have any
power domains described. However, there will probably be a time
where we will actually need it for example to deal with the GPU.

  - We are still discovering some quirks that need to remodel the
DT. The DMA bus offset is a good example of that.

For the first one, I guess you could say that it's a bullet we have to
bite. No binding will ever be perfect, and we'll always have to
remodel things as we discover new usecases.

However, the two others are really the one that prevent you from
saying "that's it, we're done" at a particular moment in time.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] dm: mmc: socfpga: call dwmci_probe()

2018-03-09 Thread Jaehoon Chung
Dear Patrick,

On 03/09/2018 03:51 PM, Patrick Brünn wrote:
>> From: Patrick Brünn
>> Sent: Donnerstag, 8. März 2018 06:39
>>> From: Jaehoon Chung [mailto:jh80.ch...@samsung.com]
>>> Sent: Donnerstag, 8. März 2018 04:57
>>> On 03/08/2018 12:12 PM, Marek Vasut wrote:
 On 03/08/2018 03:17 AM, Jaehoon Chung wrote:
> On 03/06/2018 05:07 PM, linux-kernel-...@beckhoff.com wrote:
>> From: Patrick Bruenn 
>>
>> On a socfpga_cyclone5 based board the SD card, was never powered
>> up.
>>> For
>> other dw_mmc based SoCs dwmci_probe() is called in the platform
>>> specific
>> probe(). It seems this call is missing for socfpga_dw_mmc.
>>
>> With this change DWMCI_PWREN is set by dmwci_init().
>>
>> Signed-off-by: Patrick Bruenn 
>
> Reviewed-by: Jaehoon Chung 
>
> Will apply this patch before releasing v2018.03.
> (I have a problem about accessing git.denx.de. After fixing my problem,
>>> will resend email about applying.)

 DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand
>>> what this patch is trying to fix. I'd prefer if you did not hastily apply 
>>> this.
>>>
>>> It's my misunderstanding. When i checked more. I think that Marek is right.
>>> Thanks Marek for pointing out.
>>>
>> Okay, but do you have any hint what I am doing wrong? My board (cx8100 not
>> mainline, yet) is based on socfpga_cyclone5. And on my board "
>> dwmci_writel(host, DWMCI_PWREN, 1);" is never called, because
>> dwmci_init() is never called.
>> As far as I can see with CONFIG_DM_MMC enabled dwmci_init() should be
>> called by dwmci_probe().
>>
>> exynos  and rockchip do call dwmci_probe() within
>> exynos/rockchip_dwmmc_probe().
>> but socfpga_dwmmc_probe() is missing this call. So I looked deeper but found
>> no place for socfpga platform to call dwmci_probe() or dwmci_init().
>> What am I missing?
>>
> I got an idea, what might be the difference between my board and your boards.
> I suspect you use U-BOOT SPL without CONFIG_DM_MMC set, so
> dwmci_init() is called indirectly by mmc_start_init().

Right, it's difference with CONFIG_DM_MMC. I had checked this.

> Now, when your main U-Boot (with CONFIG_DM_MMC) is launched,
> everything is already configured and it isn't necessary to call dwmci_init() 
> again.
> On my board the Altera MPL is used (and I can't replace it).  Which seems to
> disable DWMCI_PWREN before launching U-Boot.
> If my assumption is correct, I still think it is a U-Boot bug to assume code 
> like in
> dwmci_init() was already run before U-Boot gets in control.
> Besides exynos/rockchip_dw_mmc don't have this precondition requirement.
> 
> Please take your time to look deeper into this issue, before deciding 
> anything.
> I don't think we need to rush this into the next release, as normal mainline
> boards are "accidently" not affected.

Sure. I will check this patch. Before applying this, will check more carefully. 
I agreed about Marek's opinion "did not hastily apply this."
And i think that your approach also is right.

Best Regards,
Jaehoon Chung

> 
> Thanks and regards,
> Patrick
> 
> Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans 
> Beckhoff
> Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
> 
> 

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


Re: [U-Boot] imx: get_ticks in syscounter.c get miscompiled by GCC 6

2018-03-09 Thread Yasushi SHOJI
Hi,

On Wed, Mar 7, 2018 at 11:27 PM, Tom Rini  wrote:
> On Wed, Mar 07, 2018 at 10:42:44AM -0300, Fabio Estevam wrote:
>> Patch looks good. Make sure to add your Signed-off-by line, then you
>> can send it via git send-email.
>
> Yes please, thanks!

I've sent it to you with my signed-of-by.

thanks,
-- 
  yashi
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 2/2] tools: env: Implement atomic replace for filesystem

2018-03-09 Thread Stefano Babic
On 09/03/2018 10:54, Alex Kiernan wrote:
> On Thu, Mar 8, 2018 at 5:04 PM, Stefano Babic  wrote:
>> Hi alex,
>>
>> On 08/03/2018 12:52, Alex Kiernan wrote:
>>> If the U-Boot environment is stored in a regular file and redundant
>>> operation isn't set, then write to a temporary file and perform an
>>> atomic rename.
>>>
>>
>> Even if it is not explicitely set (IMHO it should), this code can
>> be used as library and linked to own application. That means that
>> concurrency can happens. I mean:
>>
> 
> At this point you're writing the new environment, so we should hold a
> lock to avoid concurrent writes.

This is already done, even if not in the way I like.
tools/env/fw_env_main.c calls flock() to acquire the resource. This
works using fw_printenv / fw_setenv, but not as library. Library's users
as me have to provide themselves a lock before calling the fw_* functions.

> 
>>> Signed-off-by: Alex Kiernan 
>>> ---
>>>
>>>  tools/env/fw_env.c | 81 
>>> --
>>>  1 file changed, 78 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
>>> index 2df3504..b814c4e 100644
>>> --- a/tools/env/fw_env.c
>>> +++ b/tools/env/fw_env.c
>>> @@ -14,6 +14,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -1229,9 +1230,46 @@ static int flash_read (int fd)
>>>   return 0;
>>>  }
>>>
>>> +static int flash_open_tempfile(const char **dname, const char 
>>> **target_temp)
>>> +{
>>> + char *dup_name = strdup(DEVNAME (dev_current));
>>> + char *temp_name = NULL;
>>> + int rc = -1;
>>> +
>>> + if (!dup_name)
>>> + return -1;
>>> +
>>> + *dname = dirname(dup_name);
>>> + if (!*dname)
>>> + goto err;
>>> +
>>> + rc = asprintf(_name, "%s/uboot.tmp", *dname);
>>
>> Filename is fixed - should we not use mkstemp ?
>>
> 
> I went round all the temp functions before in the end deciding to fix
> it. However it looks like I totally misread the contract that mkstemp
> delivers - I'd thought it didn't pass you the name back, but it
> clearly does; I'll go update it.
> 


Best regards,
Stefano Babic

-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 2/2] tools: env: Implement atomic replace for filesystem

2018-03-09 Thread Alex Kiernan
On Thu, Mar 8, 2018 at 5:04 PM, Stefano Babic  wrote:
> Hi alex,
>
> On 08/03/2018 12:52, Alex Kiernan wrote:
>> If the U-Boot environment is stored in a regular file and redundant
>> operation isn't set, then write to a temporary file and perform an
>> atomic rename.
>>
>
> Even if it is not explicitely set (IMHO it should), this code can
> be used as library and linked to own application. That means that
> concurrency can happens. I mean:
>

At this point you're writing the new environment, so we should hold a
lock to avoid concurrent writes.

>> Signed-off-by: Alex Kiernan 
>> ---
>>
>>  tools/env/fw_env.c | 81 
>> --
>>  1 file changed, 78 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
>> index 2df3504..b814c4e 100644
>> --- a/tools/env/fw_env.c
>> +++ b/tools/env/fw_env.c
>> @@ -14,6 +14,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -1229,9 +1230,46 @@ static int flash_read (int fd)
>>   return 0;
>>  }
>>
>> +static int flash_open_tempfile(const char **dname, const char **target_temp)
>> +{
>> + char *dup_name = strdup(DEVNAME (dev_current));
>> + char *temp_name = NULL;
>> + int rc = -1;
>> +
>> + if (!dup_name)
>> + return -1;
>> +
>> + *dname = dirname(dup_name);
>> + if (!*dname)
>> + goto err;
>> +
>> + rc = asprintf(_name, "%s/uboot.tmp", *dname);
>
> Filename is fixed - should we not use mkstemp ?
>

I went round all the temp functions before in the end deciding to fix
it. However it looks like I totally misread the contract that mkstemp
delivers - I'd thought it didn't pass you the name back, but it
clearly does; I'll go update it.

-- 
Alex Kiernan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v5 01/10] optee: Add lib entries for sharing OPTEE code across ports

2018-03-09 Thread Bryan O'Donoghue
This patch adds code to lib to enable sharing of useful OPTEE code between
board-ports and architectures. The code on lib/optee/optee.c comes from the
TI omap2 port. Eventually the OMAP2 code will be patched to include the
shared code. The intention here is to add more useful OPTEE specific code
as more functionality gets added.

Signed-off-by: Bryan O'Donoghue 
Cc: Harinarayan Bhatta 
Cc: Andrew F. Davis 
Cc: Tom Rini 
Cc: Kever Yang 
Cc: Philipp Tomsich 
Cc: Peng Fan 
Tested-by: Peng Fan 
---
 include/tee/optee.h | 16 
 lib/Kconfig |  1 +
 lib/Makefile|  1 +
 lib/optee/Kconfig   |  8 
 lib/optee/Makefile  |  7 +++
 lib/optee/optee.c   | 31 +++
 6 files changed, 64 insertions(+)
 create mode 100644 lib/optee/Kconfig
 create mode 100644 lib/optee/Makefile
 create mode 100644 lib/optee/optee.c

diff --git a/include/tee/optee.h b/include/tee/optee.h
index 9ab0d08..8943afb 100644
--- a/include/tee/optee.h
+++ b/include/tee/optee.h
@@ -10,6 +10,8 @@
 #ifndef_OPTEE_H
 #define _OPTEE_H
 
+#include 
+
 #define OPTEE_MAGIC 0x4554504f
 #define OPTEE_VERSION   1
 #define OPTEE_ARCH_ARM320
@@ -27,4 +29,18 @@ struct optee_header {
uint32_t paged_size;
 };
 
+#if defined(CONFIG_OPTEE)
+int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
+  unsigned long tzdram_len, unsigned long image_len);
+#else
+static inline int optee_verify_image(struct optee_header *hdr,
+unsigned long tzdram_start,
+unsigned long tzdram_len,
+unsigned long image_len)
+{
+   return -EPERM;
+}
+
+#endif
+
 #endif /* _OPTEE_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index 4fd41c4..a4029a6 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -310,5 +310,6 @@ endmenu
 
 source lib/efi/Kconfig
 source lib/efi_loader/Kconfig
+source lib/optee/Kconfig
 
 endmenu
diff --git a/lib/Makefile b/lib/Makefile
index 0db41c1..35da570 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_FIT) += libfdt/
 obj-$(CONFIG_OF_LIVE) += of_live.o
 obj-$(CONFIG_CMD_DHRYSTONE) += dhry/
 obj-$(CONFIG_ARCH_AT91) += at91/
+obj-$(CONFIG_OPTEE) += optee/
 
 obj-$(CONFIG_AES) += aes.o
 obj-y += charset.o
diff --git a/lib/optee/Kconfig b/lib/optee/Kconfig
new file mode 100644
index 000..2e406fe
--- /dev/null
+++ b/lib/optee/Kconfig
@@ -0,0 +1,8 @@
+config OPTEE
+   bool "Support OPTEE images"
+   help
+ U-Boot can be configured to boot OPTEE images.
+ Selecting this option will enable shared OPTEE library code and
+  enable an OPTEE specific bootm command that will perform additional
+  OPTEE specific checks before booting an OPTEE image created with
+  mkimage.
diff --git a/lib/optee/Makefile b/lib/optee/Makefile
new file mode 100644
index 000..03e832f
--- /dev/null
+++ b/lib/optee/Makefile
@@ -0,0 +1,7 @@
+#
+# (C) Copyright 2017 Linaro
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+obj-$(CONFIG_OPTEE) += optee.o
diff --git a/lib/optee/optee.c b/lib/optee/optee.c
new file mode 100644
index 000..2cc16d7
--- /dev/null
+++ b/lib/optee/optee.c
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2017 Linaro
+ * Bryan O'Donoghue 
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include 
+#include 
+
+int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
+  unsigned long tzdram_len, unsigned long image_len)
+{
+   unsigned long tzdram_end = tzdram_start + tzdram_len;
+   uint32_t tee_file_size;
+
+   tee_file_size = hdr->init_size + hdr->paged_size +
+   sizeof(struct optee_header);
+
+   if (hdr->magic != OPTEE_MAGIC ||
+   hdr->version != OPTEE_VERSION ||
+   hdr->init_load_addr_hi > tzdram_end ||
+   hdr->init_load_addr_lo < tzdram_start ||
+   tee_file_size > tzdram_len ||
+   tee_file_size != image_len ||
+   (hdr->init_load_addr_lo + tee_file_size) > tzdram_end) {
+   return -EINVAL;
+   }
+
+   return 0;
+}
-- 
2.7.4

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


[U-Boot] [PATCH v5 00/10] Add new OPTEE bootm support to u-boot

2018-03-09 Thread Bryan O'Donoghue
v5:

This patchset now works by making a bootable OPTEE image

mkimage -A arm -T kernel -O tee -C none -d tee.bin uTee.optee

The concept is the same as the earlier version of this patchset except
instead of "mkimage -T tee" we do "mkimage -T kernel -O tee". Andrew
suggested this and it is technically feasible.

So here is the revised patchset.

- Converted IH_TYPE_OPTEE to IH_OS_TEE - Andrew

- Removed Tested-by: for Peng Fan on patches with churn as a result

- Added patch for CONFIG_OPTEE_ADDR
  This CONFIG entry will be used in an upcoming set of patch for the
  board I'm working with.

v4:
- New type "optee" renamed to "tee-bootable". We discussed making the
  namespace here more logical and obvious in another thread.

  Kever may or may not end up adding "tee-combo".

  This patchset will result in
  "tee" and "tee-bootable" being valid names. Since "tee" is an existing
  image type the name will be maintained. - Tom

- Added doc/README.trusted-execution-environment
  This gives a brief introduction on TEE plus some links to the spec and
  the op-tee website.

  In then lays out the difference between these two types
  "tee" (tee-standalone)
  "tee-bootable"

  - Bryan, Philipp

- Small change made to comment on existing TEE - Bryan

- Reworded the Kconfig option "OPTEE"
  Makes a little bit more sense to me re-reading now - Bryan

- Add patch to define CONFIG_OPTEE_LOAD_ADDR
  An upcoming set of patches for a board will make use of this define in an
  OPTEE context.

v3:

- Rework printout to be added at the end as opposed to churn over three
  separate patches - Andrew

- Reword patch 006 to better explain the thinking behind new image type
  - Andrew

v2:
- Added CONFIG_OPTEE_TZDRAM_BASE instead of #ifndef OPTEE_TZDRAM_BASE
  as an error. - Tom Rini

- Added Tested-by: Peng Fan  - as indicated

- Added better explanation text to patch 6/9
  "tools: mkimage: add optee image type"

- Fixed some checkpatch warnings in optee.c

v1:
This series adds a new OPTEE bootable image type to u-boot, which is
directly bootable with the bootm command.

There is already a TEE image type but, in this case the TEE firmware is
loaded into RAM, jumped into and then back out of. This image type is a
directly bootable image as described here :
http://mrvan.github.io/optee-imx6ul

Instead of reusing the Linux bootable image type instead a new image type
is defined, which allows us to perform additional image verification, prior
to handing off control via bootm.

OPTEE images get linked to a specific address at compile time and must be
loaded to this address too. This series extends out mkimage with a new
image type that allows the OPTEE binary link location to be validated
against CONFIG_OPTEE_TZDRAM_BASE and CONFIG_OPTEE_TZDRAM_SIZE respectively
prior to proceeding through the bootm phase.

Once applied you can generate a bootable OPTEE image like this

mkimage -A arm -T optee -C none -d ./out/arm-plat-imx/core/tee.bin uTee.optee

That image can then be booted directly by bootm. bootm will verify the
header contents of the OPTEE binary against the DRAM area carved out in
u-boot. If the defined DRAM area does not match the link address specified
we refuse to boot.

Kever - I'd like to suggest that your OPTEE SPL image takes a different
image type IH_TYPE_OPTEE_SPL ? to indicate the different behavior your
image type has versus a directly bootable bootm image.

Bryan O'Donoghue (10):
  optee: Add lib entries for sharing OPTEE code across ports
  optee: Add CONFIG_OPTEE_TZDRAM_SIZE
  optee: Add CONFIG_OPTEE_TZDRAM_BASE
  optee: Add CONFIG_OPTEE_LOAD_ADDR
  optee: Add optee_image_get_entry_point()
  optee: Add optee_image_get_load_addr()
  optee: Add optee_verify_bootm_image()
  optee: Add error printout
  image: Add IH_OS_TEE for TEE chain-load boot
  bootm: optee: Add a bootm command for type IH_OS_TEE

 common/bootm_os.c | 32 +
 common/image.c|  1 +
 include/image.h   |  1 +
 include/tee/optee.h   | 41 
 lib/Kconfig   |  1 +
 lib/Makefile  |  1 +
 lib/optee/Kconfig | 39 ++
 lib/optee/Makefile|  7 ++
 lib/optee/optee.c | 66 +++
 tools/default_image.c | 15 ++--
 10 files changed, 202 insertions(+), 2 deletions(-)
 create mode 100644 lib/optee/Kconfig
 create mode 100644 lib/optee/Makefile
 create mode 100644 lib/optee/optee.c

-- 
2.7.4

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


[U-Boot] [PATCH v5 05/10] optee: Add optee_image_get_entry_point()

2018-03-09 Thread Bryan O'Donoghue
Add a helper function for extracting the least significant 32 bits from the
OPTEE entry point address, which will be good enough to load OPTEE binaries
up to (2^32)-1 bytes.

We may need to extend this out later on but for now (2^32)-1 should be
fine.

Signed-off-by: Bryan O'Donoghue 
Cc: Harinarayan Bhatta 
Cc: Andrew F. Davis 
Cc: Tom Rini 
Cc: Kever Yang 
Cc: Philipp Tomsich 
Cc: Peng Fan 
Tested-by: Peng Fan 
---
 include/tee/optee.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/include/tee/optee.h b/include/tee/optee.h
index 8943afb..eb328d3 100644
--- a/include/tee/optee.h
+++ b/include/tee/optee.h
@@ -29,6 +29,13 @@ struct optee_header {
uint32_t paged_size;
 };
 
+static inline uint32_t optee_image_get_entry_point(const image_header_t *hdr)
+{
+   struct optee_header *optee_hdr = (struct optee_header *)(hdr + 1);
+
+   return optee_hdr->init_load_addr_lo;
+}
+
 #if defined(CONFIG_OPTEE)
 int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
   unsigned long tzdram_len, unsigned long image_len);
-- 
2.7.4

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


[U-Boot] [PATCH v5 02/10] optee: Add CONFIG_OPTEE_TZDRAM_SIZE

2018-03-09 Thread Bryan O'Donoghue
OPTEE is currently linked to a specific area of memory called the TrustZone
DRAM. This patch adds a CONFIG entry for the default size of TrustZone DRAM
that a board-port can over-ride. The region that U-Boot sets aside for the
OPTEE run-time should be verified before attempting to hand off to the
OPTEE run-time. Each board-port should carefully ensure that the TZDRAM
size specified in the OPTEE build and the TZDRAM size specified in U-Boot
match-up.

Further patches will use TZDRAM size with other defines and variables to
carry out a degree of automated verification in U-Boot prior to trying to
boot an OPTEE image.

Signed-off-by: Bryan O'Donoghue 
Cc: Harinarayan Bhatta 
Cc: Andrew F. Davis 
Cc: Tom Rini 
Cc: Kever Yang 
Cc: Philipp Tomsich 
Cc: Peng Fan 
Tested-by: Peng Fan 
---
 lib/optee/Kconfig | 8 
 1 file changed, 8 insertions(+)

diff --git a/lib/optee/Kconfig b/lib/optee/Kconfig
index 2e406fe..41c0ab7 100644
--- a/lib/optee/Kconfig
+++ b/lib/optee/Kconfig
@@ -6,3 +6,11 @@ config OPTEE
   enable an OPTEE specific bootm command that will perform additional
   OPTEE specific checks before booting an OPTEE image created with
   mkimage.
+
+config OPTEE_TZDRAM_SIZE
+   hex "Amount of Trust-Zone RAM for the OPTEE image"
+   depends on OPTEE
+   default 0x300
+   help
+ The size of pre-allocated Trust Zone DRAM to allocate for the OPTEE
+ runtime.
-- 
2.7.4

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


[U-Boot] [PATCH v5 09/10] image: Add IH_OS_TEE for TEE chain-load boot

2018-03-09 Thread Bryan O'Donoghue
This patch adds a new type IH_OS_TEE. This new OS type will be used for
chain-loading to Linux via a TEE.

With this patch in-place you can generate a bootable OPTEE image like this:

mkimage -A arm -T kernel -O tee -C none -d tee.bin uTee.optee

where "tee.bin" is the input binary prefixed with an OPTEE header and
uTee.optee is the output prefixed with a u-boot wrapper header.

This image type "-T kernel -O tee" is differentiated from the existing
IH_TYPE_TEE "-T tee" in that the IH_TYPE is installed by u-boot (flow
control returns to u-boot) whereas for the new IH_OS_TEE control passes to
the OPTEE firmware and the firmware chainloads onto Linux.

Andrew Davis gave the following ASCII diagram:

IH_OS_TEE: (mkimage -T kernel -O tee)
Non-Secure   Secure

 BootROM
   |
  -
 |
 v
SPL
 |
 v
   U-Boot -->
  <-  OP-TEE
  |
  V
Linux

IH_TYPE_TEE: (mkimage -T tee)
Non-Secure   Secure

 BootROM
   |
  -
 |
 v
SPL --->
 <-  OP-TEE
 |
 v
   U-Boot
  |
  V
Linux

Signed-off-by: Bryan O'Donoghue 
Suggested-by: Andrew F. Davis 
Cc: Harinarayan Bhatta 
Cc: Andrew F. Davis 
Cc: Tom Rini 
Cc: Kever Yang 
Cc: Philipp Tomsich 
Cc: Peng Fan 
Link: http://mrvan.github.io/optee-imx6ul
---
 common/image.c|  1 +
 include/image.h   |  1 +
 tools/default_image.c | 15 +--
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/common/image.c b/common/image.c
index 14be3ca..61e3d25 100644
--- a/common/image.c
+++ b/common/image.c
@@ -100,6 +100,7 @@ static const table_entry_t uimage_os[] = {
{   IH_OS_OSE,  "ose",  "Enea OSE", },
{   IH_OS_PLAN9,"plan9","Plan 9",   },
{   IH_OS_RTEMS,"rtems","RTEMS",},
+   {   IH_OS_TEE,  "tee",  "Trusted Execution Environment" 
},
{   IH_OS_U_BOOT,   "u-boot",   "U-Boot",   },
{   IH_OS_VXWORKS,  "vxworks",  "VxWorks",  },
 #if defined(CONFIG_CMD_ELF) || defined(USE_HOSTCC)
diff --git a/include/image.h b/include/image.h
index dbdaecb..a0a530d 100644
--- a/include/image.h
+++ b/include/image.h
@@ -153,6 +153,7 @@ enum {
IH_OS_PLAN9,/* Plan 9   */
IH_OS_OPENRTOS, /* OpenRTOS */
IH_OS_ARM_TRUSTED_FIRMWARE, /* ARM Trusted Firmware */
+   IH_OS_TEE,  /* Trusted Execution Environment */
 
IH_OS_COUNT,
 };
diff --git a/tools/default_image.c b/tools/default_image.c
index 4e5568e..c67f66b 100644
--- a/tools/default_image.c
+++ b/tools/default_image.c
@@ -18,6 +18,7 @@
 #include "mkimage.h"
 
 #include 
+#include 
 #include 
 
 static image_header_t header;
@@ -90,6 +91,8 @@ static void image_set_header(void *ptr, struct stat *sbuf, 
int ifd,
uint32_t checksum;
time_t time;
uint32_t imagesize;
+   uint32_t ep;
+   uint32_t addr;
 
image_header_t * hdr = (image_header_t *)ptr;
 
@@ -99,18 +102,26 @@ static void image_set_header(void *ptr, struct stat *sbuf, 
int ifd,
sbuf->st_size - sizeof(image_header_t));
 
time = imagetool_get_source_date(params, sbuf->st_mtime);
+   ep = params->ep;
+   addr = params->addr;
+
if (params->type == IH_TYPE_FIRMWARE_IVT)
/* Add size of CSF minus IVT */
imagesize = sbuf->st_size - sizeof(image_header_t) + 0x1FE0;
else
imagesize = sbuf->st_size - sizeof(image_header_t);
 
+   if (params->os == IH_OS_TEE) {
+   addr = optee_image_get_load_addr(hdr);
+   ep = optee_image_get_entry_point(hdr);
+   }
+
/* Build new header */
image_set_magic(hdr, IH_MAGIC);
image_set_time(hdr, time);
image_set_size(hdr, imagesize);
-   image_set_load(hdr, params->addr);
-   image_set_ep(hdr, params->ep);
+   image_set_load(hdr, addr);
+   image_set_ep(hdr, ep);
image_set_dcrc(hdr, checksum);
image_set_os(hdr, params->os);
image_set_arch(hdr, params->arch);
-- 
2.7.4

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


[U-Boot] [PATCH v5 03/10] optee: Add CONFIG_OPTEE_TZDRAM_BASE

2018-03-09 Thread Bryan O'Donoghue
OPTEE is currently linked to a specific area of memory called the TrustZone
DRAM. This patch adds a CONFIG entry for the default address of TrustZone
DRAM that a board-port can over-ride. The region that U-Boot sets aside for
the OPTEE run-time should be verified before attempting to hand off to the
OPTEE run-time. Each board-port should carefully ensure that the TZDRAM
address specified in the OPTEE build and the TZDRAM address specified in
U-Boot match-up.

Further patches will use TZDRAM address with other defines and variables to
carry out a degree of automated verification in U-Boot prior to trying to
boot an OPTEE image.

Signed-off-by: Bryan O'Donoghue 
Cc: Harinarayan Bhatta 
Cc: Andrew F. Davis 
Cc: Tom Rini 
Cc: Kever Yang 
Cc: Philipp Tomsich 
---
 lib/optee/Kconfig | 8 
 1 file changed, 8 insertions(+)

diff --git a/lib/optee/Kconfig b/lib/optee/Kconfig
index 41c0ab7..a3b7332 100644
--- a/lib/optee/Kconfig
+++ b/lib/optee/Kconfig
@@ -14,3 +14,11 @@ config OPTEE_TZDRAM_SIZE
help
  The size of pre-allocated Trust Zone DRAM to allocate for the OPTEE
  runtime.
+
+config OPTEE_TZDRAM_BASE
+   hex "Base address of Trust-Zone RAM for the OPTEE image"
+   depends on OPTEE
+   default 0x9d00
+   help
+ The base address of pre-allocated Trust Zone DRAM for
+ the OPTEE runtime.
-- 
2.7.4

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


[U-Boot] [PATCH v5 08/10] optee: Add error printout

2018-03-09 Thread Bryan O'Donoghue
When encountering an error in OPTEE verification print out various details
of the OPTEE header to aid in further debugging of encountered errors.

Signed-off-by: Bryan O'Donoghue 
Cc: Harinarayan Bhatta 
Cc: Andrew F. Davis 
Cc: Tom Rini 
Cc: Kever Yang 
Cc: Philipp Tomsich 
Cc: Peng Fan 
Tested-by: Peng Fan 
---
 lib/optee/optee.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/optee/optee.c b/lib/optee/optee.c
index 365c078..78a15e8 100644
--- a/lib/optee/optee.c
+++ b/lib/optee/optee.c
@@ -8,6 +8,12 @@
 #include 
 #include 
 
+#define optee_hdr_err_msg \
+   "OPTEE verification error:" \
+   "\n\thdr=%p image=0x%08lx magic=0x%08x tzdram 0x%08lx-0x%08lx " \
+   "\n\theader lo=0x%08x hi=0x%08x size=0x%08lx arch=0x%08x" \
+   "\n\tuimage params 0x%08lx-0x%08lx\n"
+
 int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
   unsigned long tzdram_len, unsigned long image_len)
 {
@@ -42,10 +48,19 @@ int optee_verify_bootm_image(unsigned long image_addr,
 
ret = optee_verify_image(hdr, tzdram_start, tzdram_len, image_len);
if (ret)
-   return ret;
+   goto error;
 
-   if (image_load_addr + sizeof(*hdr) != hdr->init_load_addr_lo)
+   if (image_load_addr + sizeof(*hdr) != hdr->init_load_addr_lo) {
ret = -EINVAL;
+   goto error;
+   }
+
+   return ret;
+error:
+   printf(optee_hdr_err_msg, hdr, image_addr, hdr->magic, tzdram_start,
+  tzdram_start + tzdram_len, hdr->init_load_addr_lo,
+  hdr->init_load_addr_hi, image_len, hdr->arch, image_load_addr,
+  image_load_addr + image_len);
 
return ret;
 }
-- 
2.7.4

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


[U-Boot] [PATCH v5 07/10] optee: Add optee_verify_bootm_image()

2018-03-09 Thread Bryan O'Donoghue
This patch adds optee_verify_bootm_image() which will be subsequently used
to verify the parameters encoded in the OPTEE header match the memory
allocated to the OPTEE region, OPTEE header magic and version prior to
handing off control to the OPTEE image.

Signed-off-by: Bryan O'Donoghue 
Cc: Harinarayan Bhatta 
Cc: Andrew F. Davis 
Cc: Tom Rini 
Cc: Kever Yang 
Cc: Philipp Tomsich 
Cc: Peng Fan 
---
 include/tee/optee.h | 13 +
 lib/optee/optee.c   | 20 
 2 files changed, 33 insertions(+)

diff --git a/include/tee/optee.h b/include/tee/optee.h
index e782cb0..4b9e94c 100644
--- a/include/tee/optee.h
+++ b/include/tee/optee.h
@@ -55,4 +55,17 @@ static inline int optee_verify_image(struct optee_header 
*hdr,
 
 #endif
 
+#if defined(CONFIG_OPTEE)
+int optee_verify_bootm_image(unsigned long image_addr,
+unsigned long image_load_addr,
+unsigned long image_len);
+#else
+static inline int optee_verify_bootm_image(unsigned long image_addr,
+  unsigned long image_load_addr,
+  unsigned long image_len)
+{
+   return -EPERM;
+}
+#endif
+
 #endif /* _OPTEE_H */
diff --git a/lib/optee/optee.c b/lib/optee/optee.c
index 2cc16d7..365c078 100644
--- a/lib/optee/optee.c
+++ b/lib/optee/optee.c
@@ -29,3 +29,23 @@ int optee_verify_image(struct optee_header *hdr, unsigned 
long tzdram_start,
 
return 0;
 }
+
+int optee_verify_bootm_image(unsigned long image_addr,
+unsigned long image_load_addr,
+unsigned long image_len)
+{
+   struct optee_header *hdr = (struct optee_header *)image_addr;
+   unsigned long tzdram_start = CONFIG_OPTEE_TZDRAM_BASE;
+   unsigned long tzdram_len = CONFIG_OPTEE_TZDRAM_SIZE;
+
+   int ret;
+
+   ret = optee_verify_image(hdr, tzdram_start, tzdram_len, image_len);
+   if (ret)
+   return ret;
+
+   if (image_load_addr + sizeof(*hdr) != hdr->init_load_addr_lo)
+   ret = -EINVAL;
+
+   return ret;
+}
-- 
2.7.4

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


[U-Boot] [PATCH v5 06/10] optee: Add optee_image_get_load_addr()

2018-03-09 Thread Bryan O'Donoghue
This patch adds optee_image_get_load_addr() a helper function used to
calculate the load-address of an OPTEE image based on the lower
entry-point address given in the OPTEE header.

Signed-off-by: Bryan O'Donoghue 
Cc: Harinarayan Bhatta 
Cc: Andrew F. Davis 
Cc: Tom Rini 
Cc: Kever Yang 
Cc: Philipp Tomsich 
Cc: Peng Fan 
Tested-by: Peng Fan 
---
 include/tee/optee.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/tee/optee.h b/include/tee/optee.h
index eb328d3..e782cb0 100644
--- a/include/tee/optee.h
+++ b/include/tee/optee.h
@@ -36,6 +36,11 @@ static inline uint32_t optee_image_get_entry_point(const 
image_header_t *hdr)
return optee_hdr->init_load_addr_lo;
 }
 
+static inline uint32_t optee_image_get_load_addr(const image_header_t *hdr)
+{
+   return optee_image_get_entry_point(hdr) - sizeof(struct optee_header);
+}
+
 #if defined(CONFIG_OPTEE)
 int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
   unsigned long tzdram_len, unsigned long image_len);
-- 
2.7.4

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


[U-Boot] [PATCH v5 04/10] optee: Add CONFIG_OPTEE_LOAD_ADDR

2018-03-09 Thread Bryan O'Donoghue
CONFIG_OPTEE_LOAD_ADDR is used to tell u-boot where to load the OPTEE
binary into memory prior to handing off control to OPTEE.

We need to pull this value out of u-boot in order to produce an IMX IVT/CSF
signed pair for the purposes of secure boot. The best way to do that is to
have CONFIG_OPTEE_LOAD_ADDR appear in u-boot.cfg.

Adding new CONFIG entires to u-boot should be kconfig driven so this patch
does just that.

Signed-off-by: Bryan O'Donoghue 
Reviewed-by: Ryan Harkin 
---
 lib/optee/Kconfig | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/lib/optee/Kconfig b/lib/optee/Kconfig
index a3b7332..cc73ec3 100644
--- a/lib/optee/Kconfig
+++ b/lib/optee/Kconfig
@@ -7,6 +7,12 @@ config OPTEE
   OPTEE specific checks before booting an OPTEE image created with
   mkimage.
 
+config OPTEE_LOAD_ADDR
+   hex "OPTEE load address"
+   default 0x
+   help
+ The load address of the bootable OPTEE binary.
+
 config OPTEE_TZDRAM_SIZE
hex "Amount of Trust-Zone RAM for the OPTEE image"
depends on OPTEE
-- 
2.7.4

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


[U-Boot] [PATCH v5 10/10] bootm: optee: Add a bootm command for type IH_OS_TEE

2018-03-09 Thread Bryan O'Donoghue
This patch makes it possible to verify the contents and location of an
OPTEE image in DRAM prior to handing off control to that image. If image
verification fails we won't try to boot any further.

Signed-off-by: Bryan O'Donoghue 
Suggested-by: Andrew F. Davis 
Cc: Harinarayan Bhatta 
Cc: Andrew F. Davis 
Cc: Tom Rini 
Cc: Kever Yang 
Cc: Philipp Tomsich 
Cc: Peng Fan 
---
 common/bootm_os.c | 32 
 lib/optee/Kconfig |  9 +
 2 files changed, 41 insertions(+)

diff --git a/common/bootm_os.c b/common/bootm_os.c
index 5e6b177..cddf98e 100644
--- a/common/bootm_os.c
+++ b/common/bootm_os.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -433,6 +434,34 @@ static int do_bootm_openrtos(int flag, int argc, char * 
const argv[],
 }
 #endif
 
+#ifdef CONFIG_BOOTM_TEE
+static int do_bootm_tee(int flag, int argc, char * const argv[],
+   bootm_headers_t *images)
+{
+   int ret;
+
+   /* Verify OS type */
+   if (images->os.os != IH_OS_TEE) {
+   return 1;
+   };
+
+   /* Validate TEE header */
+   ret = optee_verify_bootm_image(images->os.image_start,
+  images->os.load,
+  images->os.image_len);
+   if (ret)
+   return ret;
+
+   /* Locate FDT etc */
+   ret = bootm_find_images(flag, argc, argv);
+   if (ret)
+   return ret;
+
+   /* From here we can run the regular linux boot path */
+   return do_bootm_linux(flag, argc, argv, images);
+}
+#endif
+
 static boot_os_fn *boot_os[] = {
[IH_OS_U_BOOT] = do_bootm_standalone,
 #ifdef CONFIG_BOOTM_LINUX
@@ -466,6 +495,9 @@ static boot_os_fn *boot_os[] = {
 #ifdef CONFIG_BOOTM_OPENRTOS
[IH_OS_OPENRTOS] = do_bootm_openrtos,
 #endif
+#ifdef CONFIG_BOOTM_TEE
+   [IH_OS_TEE] = do_bootm_tee,
+#endif
 };
 
 /* Allow for arch specific config before we boot */
diff --git a/lib/optee/Kconfig b/lib/optee/Kconfig
index cc73ec3..1e5ab45 100644
--- a/lib/optee/Kconfig
+++ b/lib/optee/Kconfig
@@ -28,3 +28,12 @@ config OPTEE_TZDRAM_BASE
help
  The base address of pre-allocated Trust Zone DRAM for
  the OPTEE runtime.
+
+config BOOTM_OPTEE
+   bool "Support OPTEE bootm command"
+   select BOOTM_LINUX
+   default n
+   help
+ Select this command to enable chain-loading of a Linux kernel
+ via an OPTEE firmware.
+ The bootflow is BootROM -> u-boot -> OPTEE -> Linux in this case.
-- 
2.7.4

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


Re: [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems

2018-03-09 Thread Alex Kiernan
On Fri, Mar 9, 2018 at 12:30 PM, Wolfgang Denk  wrote:
> Dear Alex,
>
> In message <1520597582-12979-1-git-send-email-alex.kier...@gmail.com> you 
> wrote:
>>
>> For environments stored on filesystems where you can't have a redundant
>> configuration, rather than just over-writing the existing environment
>> in fw_setenv, do the tradtional create temporary file, rename, sync,
>> sync directory dance to achieve ACID semantics when writing through
>> fw_setenv.
>
> I isagree with the statement that we _can't have_ redundant

Yeah... sorry, sloppy wording on my part.

> environments in a file system.  It may not be supported by the
> current implementation, but there is actually nothing that would
> prevent us to come up with more powerful code that supports exactly
> such a feature.
>
> [This does not mean that I disagree with your patch - on contary, I
> think it's good.]
>

Thanks!

-- 
Alex Kiernan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH 3/3] imx: hab: Convert DCD non-NULL error to warning

2018-03-09 Thread Bryan O'Donoghue
commit 8c4037a09a5c ("imx: hab: Ensure the IVT DCD pointer is Null prior
to calling HAB authenticate function.") makes the DCD field being NULL a
dependency.

This change though will break loading and executing of existing pre-signed
binaries on a u-boot update i.e. if this change is deployed on a board you
will be forced to redo all images on that board to NULL out the DCD.

There is no prior guidance from NXP that the DCD must be NULL similarly
public guidance on usage of the HAB doesn't call out this NULL dependency
(see boundary devices link).

Since later SoCs will reject a non-NULL DCD there's no reason to make a
NULL DCD a requirement, however if there is an actual dependency for later
SoCs the appropriate fix would be to do SoC version checking.

Earlier SoCs are capable (and happy) to authenticate images with non-NULL
DCDs, we should not be forcing this change on downstream users -
particularly if it means those users now must rewrite their build systems
and/or redeploy signed images in the field.

Fixes: 8c4037a09a5c ("imx: hab: Ensure the IVT DCD pointer is Null prior
to calling HAB authenticate function.")

Signed-off-by: Bryan O'Donoghue 
Cc: Utkarsh Gupta 
Cc: Breno Lima 
Cc: Fabio Estevam 
Link: https://boundarydevices.com/high-assurance-boot-hab-dummies
---
 arch/arm/mach-imx/hab.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index c3fc699..c730c8f 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -526,10 +526,8 @@ int imx_hab_authenticate_image(uint32_t ddr_start, 
uint32_t image_size,
}
 
/* Verify if IVT DCD pointer is NULL */
-   if (ivt->dcd) {
-   puts("Error: DCD pointer must be NULL\n");
-   goto hab_authentication_exit;
-   }
+   if (ivt->dcd)
+   puts("Warning: DCD pointer should be NULL\n");
 
start = ddr_start;
bytes = image_size;
-- 
2.7.4

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


[U-Boot] [PATCH 0/3] HAB Fixes for 2018.03-rc4

2018-03-09 Thread Bryan O'Donoghue
This set fixes a few errors and gotchas I've found when rebasing my project
on top of 2018-rc4.

First is a breakage with the Linaro compiler -> gcc-linaro-7.2.1 which is a
simple one.

Second is consistency of the __packed attribute - more of a housekeeping
activity.

Last patch fixes a behaviour that has been introduced without I believe
fully thinking through the impact on downstream users.

If we force the DCD to be NULL for olders SoCs - that means people have to
go out and re-sign all of their binaries if they are updating u-boot.

It should be possible to update u-boot in isolation without forcing update
of signed binaries u-boot authenticates via HAB.

If newer NXP SoCs rely on the DCD being NULL then either

1. Just let the authenticate_image() call fail
   It can return an error - this should "just work" if the SoC will
   reject an API callback with the DCD non-NULL then - let it.

2. If the SoC actually has a bug that _requires_ the DCD to be NULL.
   Then add a check into u-boot to check SoC versions.

In either case older SoCs should "just work" and it should be possible to
update u-boot without having to update all associated signed binaries.

Please apply before next official u-boot release.

Bryan O'Donoghue (3):
  imx: hab: Fix usage of packed attribute
  imx: hab: Make usage of packed attribute consistent
  imx: hab: Convert DCD non-NULL error to warning

 arch/arm/include/asm/mach-imx/hab.h | 5 +++--
 arch/arm/mach-imx/hab.c | 6 ++
 2 files changed, 5 insertions(+), 6 deletions(-)

-- 
2.7.4

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


Re: [U-Boot] [PATCH 1/1] efi_loader: Initial EFI_DEVICE_PATH_UTILITIES_PROTOCOL

2018-03-09 Thread Alexander Graf

On 03/03/2018 03:52 PM, Heinrich Schuchardt wrote:

From: Leif Lindholm 

Not complete, but enough for Shell.efi and SCT.efi.  We'll implement the
rest as needed or once we have SCT running properly so there is a way to
validate the interface against the conformance test suite.

Initial skeleton written by Leif, and then implementation by Rob.

Rebased on v2018.03-rc1.

Suggested-by: Leif Lindholm 
Suggested-by: Rob Clark 
Signed-off-by: Heinrich Schuchardt 


The Signed-off-by chain seems incorrect. I guess you based your patch on 
top of Rob's? In that case, you need to preserve Rob's Signed-off-by, 
then add a note what you changed and do your SoB line below. The same 
probably goes for Leif's SoB line.



Alex

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


Re: [U-Boot] [PATCH 1/1] efi_loader: efi_allocate_pages is too restrictive

2018-03-09 Thread Alexander Graf

On 03/03/2018 03:48 PM, Heinrich Schuchardt wrote:

When running on the sandbox the stack is not necessarily at a higher memory
address than the highest free memory.

There is no reason why the checking of the highest memory address should be
more restrictive for EFI_ALLOCATE_ANY_PAGES than for
EFI_ALLOCATE_MAX_ADDRESS.

Signed-off-by: Heinrich Schuchardt 
---
  lib/efi_loader/efi_memory.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index cc271e0709d..0efbb973231 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -294,7 +294,7 @@ efi_status_t efi_allocate_pages(int type, int memory_type,
switch (type) {
case EFI_ALLOCATE_ANY_PAGES:
/* Any page */
-   addr = efi_find_free_memory(len, gd->start_addr_sp);
+   addr = efi_find_free_memory(len, (uint64_t)-1);


This will break on systems that do not map high address space into the 
linear map (IIRC nvidia systems had that issue).



Alex

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


Re: [U-Boot] [PATCH v2] common: add a prototype for mach_cpu_init()

2018-03-09 Thread Tom Rini
On Thu, Mar 08, 2018 at 09:57:58AM +0900, Masahiro Yamada wrote:
> 2018-03-07 23:45 GMT+09:00 Tom Rini :
> > On Wed, Mar 07, 2018 at 03:28:20PM +0100, Patrick Delaunay wrote:
> >
> >> avoid warning: no previous prototype for ‘mach_cpu_init’
> >>
> >> Signed-off-by: Patrick Delaunay 
> >
> > Reviewed-by: Tom Rini 
> >
> > --
> 
> 
> People tend to put all sort of misc thingy into common.h
> but this is one of the ugliest parts in U-Boot.
> 
> Most of files parse  that contains
> unrelated / unnecessary defines.
> Please realize this madness.
> 
> I have tried to slim it down several times,
> but people have added more and more.  So this task never ends.
> 
> How about splitting out platform init hooks?
> ( or anything suitable is OK.)

OK, sure, lets move these and relevant bits over to init.h, or
init_helpers.h which already exists and is in a few of the appropriate
locations already.

> Also, I'd like to ban new additions to 
> like we do for config_whitelist.txt
> (for example, record the number of lines of 
> then build fails if somebody increases it.)

Sure, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/1] efi_loader: remove deprecated ConsoleControlProtocol

2018-03-09 Thread Alexander Graf

On 03/03/2018 03:41 PM, Heinrich Schuchardt wrote:

The console control protocol is not defined in the UEFI standard.

It exists in EDK2's EdkCompatiblityPkg package. But this package
is deprecated according to
https://github.com/tianocore/tianocore.github.io/wiki/Differences-between-EDK-and-EDK-II

Signed-off-by: Heinrich Schuchardt 


Grub uses it, but I can confirm that the code path it's using it in is 
perfectly happy with it not being available. So I think it's safe to remove.



Alex

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


[U-Boot] [PATCHv1] board: ge: bx50v3: enable backlight on demand

2018-03-09 Thread Sebastian Reichel
From: Ian Ray 

Enable display backlight only if a message needs to be displayed.
The kernel re-initializes the backlight, which results in some
unwanted artifacts.

Signed-off-by: Ian Ray 
Signed-off-by: Sebastian Reichel 
---
 board/ge/bx50v3/bx50v3.c| 43 ++-
 include/configs/ge_bx50v3.h |  1 +
 2 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/board/ge/bx50v3/bx50v3.c b/board/ge/bx50v3/bx50v3.c
index c7a29185bf49..35efe159d598 100644
--- a/board/ge/bx50v3/bx50v3.c
+++ b/board/ge/bx50v3/bx50v3.c
@@ -786,23 +786,6 @@ int board_late_init(void)
add_board_boot_modes(board_boot_modes);
 #endif
 
-#ifdef CONFIG_VIDEO_IPUV3
-   /* We need at least 200ms between power on and backlight on
-* as per specifications from CHI MEI */
-   mdelay(250);
-
-   /* enable backlight PWM 1 */
-   pwm_init(0, 0, 0);
-
-   /* duty cycle 500ns, period: 500ns */
-   pwm_config(0, 500, 500);
-
-   /* Backlight Power */
-   gpio_direction_output(LVDS_BACKLIGHT_GP, 1);
-
-   pwm_enable(0);
-#endif
-
/* board specific pmic init */
pmic_init();
 
@@ -843,3 +826,29 @@ int checkboard(void)
printf("BOARD: %s\n", CONFIG_BOARD_NAME);
return 0;
 }
+
+static int do_backlight_enable(void)
+{
+#ifdef CONFIG_VIDEO_IPUV3
+   /* We need at least 200ms between power on and backlight on
+* as per specifications from CHI MEI */
+   mdelay(250);
+
+   /* enable backlight PWM 1 */
+   pwm_init(0, 0, 0);
+
+   /* duty cycle 500ns, period: 500ns */
+   pwm_config(0, 500, 500);
+
+   /* Backlight Power */
+   gpio_direction_output(LVDS_BACKLIGHT_GP, 1);
+
+   pwm_enable(0);
+#endif
+}
+
+U_BOOT_CMD(
+   bx50_backlight_enable, 1,  1,  do_backlight_enable,
+   "enable Bx50 backlight",
+   "no parameters"
+);
diff --git a/include/configs/ge_bx50v3.h b/include/configs/ge_bx50v3.h
index cbfe30d536d0..925507fe81f1 100644
--- a/include/configs/ge_bx50v3.h
+++ b/include/configs/ge_bx50v3.h
@@ -128,6 +128,7 @@
"swappartitions=" \
"setexpr partnum 3 - ${partnum}\0" \
"failbootcmd=" \
+   "bx50_backlight_enable; " \
"msg=\"Monitor failed to start.  Try again, or contact GE 
Service for support.\"; " \
"echo $msg; " \
"setenv stdout vga; " \
-- 
2.16.1

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


[U-Boot] [PATCH] net: Fix netretry condition

2018-03-09 Thread Leonid Iziumtsev
The "net_try_count" counter starts from "1".
And the "retrycnt" contains requested amount of retries.

With current logic, that means that the actual retry amount
will be one time less then what we set in "netretry" env.
For example setting "netretry" to "once" will make "retrycnt"
equal "1", so no retries will be triggered at all.

Fix the logic by changing the statement of "if" condition.

Signed-off-by: Leonid Iziumtsev 
---
 net/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/net.c b/net/net.c
index 4259c9e..8a9b69c 100644
--- a/net/net.c
+++ b/net/net.c
@@ -683,7 +683,7 @@ int net_start_again(void)
retry_forever = 0;
}

-   if ((!retry_forever) && (net_try_count >= retrycnt)) {
+   if ((!retry_forever) && (net_try_count > retrycnt)) {
eth_halt();
net_set_state(NETLOOP_FAIL);
/*
-- 
2.7.4
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [ANN] U-Boot v2018.03-rc4 released

2018-03-09 Thread Bryan O'Donoghue



On 06/03/18 01:28, Tom Rini wrote:

Hey all,

It's release day and I've released v2018.03-rc4.  I've included a few
different regression fixes I've seen along with some fixes and cleanups.
I would like to do the release on March 12th, and likely will so long as
no big regressions show up.  I am worried about the RPi3 issue I've seen
reported.  Are there any other issues people know of currently?

And to repeat myself again, boards and SoC families, etc, that have been
orphaned for more than a year should probably get dropped.  I've
contacted a few people off-list (which is why we've had a few pickups of
late) for things, but if you know someone that would be interested in
something, please speak up.

Thanks all!


Build error for me.

gcc -v

gcc version 7.2.1 20171011 (Linaro GCC 7.2-2017.11)

./arch/arm/include/asm/mach-imx/hab.h:42:25: error: expected ‘=’, ‘,’, ‘;’,
‘asm’ or ‘__attribute__’ before ‘{’ token
 struct __packed hab_hdr {

Fixed with

[PATCH 1/3] imx: hab: Fix usage of packed attribute

from my series just posted.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] Cleanup Whitelist entries

2018-03-09 Thread Tom Rini
On Mon, Mar 05, 2018 at 02:57:26PM -0600, Adam Ford wrote:

> There are several items in the whitelist which don't appear to be
> used anywhere outside of the whitelist itself.  These include:
> 
> CONFIG_IMX_NAND
> CONFIG_MXS_AUART
> CONFIG_MXS_AUART_BASE
> CONFIG_NORFLASH_PS32BIT
> CONFIG_QSPI
> CONFIG_REV3
> CONFIG_SIMU
> CONFIG_STV0991
> CONFIG_SYS_MEMORY_TOP
> CONFIG_SYS_SERIAL_BOOT
> CONFIG_SYS_TEXT_ADDR
> CONFIG_SYS_USBCTRL
> CONFIG_SYS_VIDEO
> 
> Signed-off-by: Adam Ford 

This misses things that are set in CONFIG_SYS_EXTRA_OPTIONS and need to
be converted first.  Thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/1] efi_loader: remove deprecated ConsoleControlProtocol

2018-03-09 Thread Heinrich Schuchardt
On 03/09/2018 02:23 PM, Alexander Graf wrote:
> On 03/03/2018 03:41 PM, Heinrich Schuchardt wrote:
>> The console control protocol is not defined in the UEFI standard.
>>
>> It exists in EDK2's EdkCompatiblityPkg package. But this package
>> is deprecated according to
>> https://github.com/tianocore/tianocore.github.io/wiki/Differences-between-EDK-and-EDK-II
>>
>>
>> Signed-off-by: Heinrich Schuchardt 
> 
> Grub uses it, but I can confirm that the code path it's using it in is
> perfectly happy with it not being available. So I think it's safe to
> remove.
> 
> 
> Alex
> 
> 

Even the very first UEFI spec (2.0, 2006) did not contain the
ConsoleControlProtocol.

Grub is trying to switch the console to text mode (see function
grub_efi_set_text_mode()).  With UEFI text output is always available.
This is why the protocol was obsoleted by UEFI. The current grub coding
that allows to live without the console control protocol was introduced
in 2006 in response to the first UEFI spec.

Regards

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


[U-Boot] [PATCH v3] common: add a prototype for mach_cpu_init()

2018-03-09 Thread Patrick Delaunay
Add a new file init.h with the prototype for arch_cpu_init
Add a prototype for mach_cpu_init() to avoid a warning:
no previous prototype for ‘mach_cpu_init’

It is a first step to move all the functions prototype
used during U-Boot initialization (board_f.c / board_r.c)
from common.h to init.h

Signed-off-by: Patrick Delaunay 
---

Changes in v3:
- create file init.h after Yamada Masahiro review

Changes in v2:
- add return info for functions mach_cpu_init() and arch_cpu_init()

 include/common.h | 13 +++--
 include/init.h   | 51 +++
 2 files changed, 54 insertions(+), 10 deletions(-)
 create mode 100644 include/init.h

diff --git a/include/common.h b/include/common.h
index 0fe9439..11e5459 100644
--- a/include/common.h
+++ b/include/common.h
@@ -62,6 +62,9 @@ typedef void (interrupt_handler_t)(void *);
 #defineTOTAL_MALLOC_LENCONFIG_SYS_MALLOC_LEN
 #endif
 
+/* startup functions */
+#include 
+
 /*
  * Function Prototypes
  */
@@ -464,16 +467,6 @@ u32cpu_mask  (void);
 u32cpu_dsp_mask(void);
 intis_core_valid (unsigned int);
 
-/**
- * arch_cpu_init() - basic cpu-dependent setup for an architecture
- *
- * This is called after early malloc is available. It should handle any
- * CPU- or SoC- specific init needed to continue the init sequence. See
- * board_f.c for where it is called. If this is not provided, a default
- * version (which does nothing) will be used.
- */
-int arch_cpu_init(void);
-
 void s_init(void);
 
 intcheckcpu  (void);
diff --git a/include/init.h b/include/init.h
new file mode 100644
index 000..324fbe0
--- /dev/null
+++ b/include/init.h
@@ -0,0 +1,51 @@
+/*
+ * (C) Copyright 2000-2009
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * Copy the startup prototype, previously defined in common.h
+ * Copyright (C) 2018, STMicroelectronics - All Rights Reserved
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#ifndef __INIT_H_
+#define __INIT_H_  1
+
+#ifndef __ASSEMBLY__   /* put C only stuff in this section */
+
+/*
+ * Function Prototypes
+ */
+
+/* common/board_f.c */
+
+/**
+ * arch_cpu_init() - basic cpu-dependent setup for an architecture
+ *
+ * This is called after early malloc is available. It should handle any
+ * CPU- or SoC- specific init needed to continue the init sequence. See
+ * board_f.c for where it is called. If this is not provided, a default
+ * version (which does nothing) will be used.
+ *
+ * @return: 0 on success, otherwise error
+ */
+int arch_cpu_init(void);
+
+/**
+ * mach_cpu_init() - SoC/machine dependent CPU setup
+ *
+ * This is called after arch_cpu_init(). It should handle any
+ * SoC or machine specific init needed to continue the init sequence. See
+ * board_f.c for where it is called. If this is not provided, a default
+ * version (which does nothing) will be used.
+ *
+ * @return: 0 on success, otherwise error
+ */
+int mach_cpu_init(void);
+
+/* common/board_r.c */
+
+#endif /* __ASSEMBLY__ */
+/* Put only stuff here that the assembler can digest */
+
+#endif /* __INIT_H_ */
-- 
2.7.4

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


[U-Boot] [PATCH 2/4] imx: hab: Encase majority of header in __ASSEMBLY__ declaration

2018-03-09 Thread Bryan O'Donoghue
Subsequent patches will want to include hab.h but in doing so include it on
an assembly compile path causing a range of compile errors. Fix the errors
pre-emptively by encasing the majority of the declarations in hab.h inside
an ifdef __ASSEMBLY__ block.

Signed-off-by: Bryan O'Donoghue 
Cc: Utkarsh Gupta 
Cc: Breno Lima 
Cc: Fabio Estevam 
---
 arch/arm/include/asm/mach-imx/hab.h | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h 
b/arch/arm/include/asm/mach-imx/hab.h
index ce9a44d..1bebdbe 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -8,6 +8,7 @@
 #ifndef __SECURE_MX6Q_H__
 #define __SECURE_MX6Q_H__
 
+#ifndef __ASSEMBLY__
 #include 
 #include 
 
@@ -196,13 +197,14 @@ typedef void hapi_clock_init_t(void);
 #define HAB_CMD_SET  0xB1  /* Set command tag */
 #define HAB_PAR_MID  0x01  /* MID parameter value */
 
-#define IVT_SIZE   0x20
-#define CSF_PAD_SIZE   0x2000
-
 /* --- end of HAB API updates */
 
 int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size,
   uint32_t ivt_offset);
 bool imx_hab_is_enabled(void);
+#endif /* __ASSEMBLY__ */
+
+#define IVT_SIZE   0x20
+#define CSF_PAD_SIZE   0x2000
 
 #endif
-- 
2.7.4

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


[U-Boot] [PATCH 3/4] imx: hab: Specify IVT padding size

2018-03-09 Thread Bryan O'Donoghue
This patch adds IVT_PAD_SIZE at 0xC00. The IVT header is padded to this
size. Defining the size explicitly makes it possible to use the define to
locate the start/end of an IVT header without using magic numbers in code.

Signed-off-by: Bryan O'Donoghue 
Cc: Utkarsh Gupta 
Cc: Breno Lima 
Cc: Fabio Estevam 
---
 arch/arm/include/asm/mach-imx/hab.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/include/asm/mach-imx/hab.h 
b/arch/arm/include/asm/mach-imx/hab.h
index 1bebdbe..f903e3e 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -205,6 +205,7 @@ bool imx_hab_is_enabled(void);
 #endif /* __ASSEMBLY__ */
 
 #define IVT_SIZE   0x20
+#define IVT_PAD_SIZE   0xC00
 #define CSF_PAD_SIZE   0x2000
 
 #endif
-- 
2.7.4

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


Re: [U-Boot] [PATCH 1/1] sunxi: video: mark framebuffer as EFI reserved memory

2018-03-09 Thread Anatolij Gustschin
On Sat,  3 Mar 2018 10:30:17 +0100
Heinrich Schuchardt xypron.g...@gmx.de wrote:

> Inform the EFI subsystem that the framebuffer memory is reserved.
> 
> Without the patch the AllocatePool boot service allocates memory from the
> framebuffer which will will be overwritten by screen output.
> 
> Signed-off-by: Heinrich Schuchardt 
> ---
>  drivers/video/sunxi/sunxi_display.c | 8 
>  1 file changed, 8 insertions(+)

Applied to u-boot-video/next, thanks!

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


Re: [U-Boot] [PATCH 1/1] sunxi: video: mark framebuffer as EFI reserved memory

2018-03-09 Thread Anatolij Gustschin
On Fri, 9 Mar 2018 16:49:49 +0100
Heinrich Schuchardt xypron.g...@gmx.de wrote:
...
> If efi_allocate_memory is called with type = EFI_ALLOCATE_MAX_ADDRESS we
> can already have an error. So, please, put the patch into your v2018.05
> queue. There is no need to wait for Alex.

okay, done.

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


[U-Boot] [PATCH 0/4] imx: hab: Add helper functions for scripted HAB auth

2018-03-09 Thread Bryan O'Donoghue
Greetings.

This set adds some helper functions as a pre-cursor to an upcoming set of
changes to a BSP adding scripted HAB authentication.

Calculating a HAB IVT address based on a base address and a +/- offset is a
trivial but, useful function for HAB. It means you can have a load address
for a HAB image inside of your environment and specify the IVT offset
relative to that address. All you need to do then is to call the function
to obtain the correct IVT address to pass into hab_auth_img.

Two relatively minor changes then - one encasing the hab.h in ifndef
__ASSEMBLY__ which is required if you want to include hab.h in a board.h.

Specifying the IVT padding size is again properly done as a define as
opposed to a magic number in code.

The final patch then is wrappering up two common use-cases in the upcoming
BSP
- hab_auth_image ? continue-to-boot : drop-to-bootrom USB mode.

In other words if you fail to authenticate an image on the secure-boot path
the appropriate next step is typically to drop into USB recovery mode.

In USB recovery mode you need to provide a signed image on a secure-boot
(closed in the parlance) board. So hab_auth_img_or_fail() encapsulates that
behaviour in one place - again allowing for scripting to reuse instead of
replicate functionality over and over again.

These helper functions could all be buried in the board-port but, they are
made available here in the hopes they will be of use to others.

Bryan O'Donoghue (4):
  imx: hab: Add routine to set HAB IVT address
  imx: hab: Encase majority of header in __ASSEMBLY__ declaration
  imx: hab: Specify IVT padding size
  imx: hab: Provide hab_auth_img_or_fail command

 arch/arm/include/asm/mach-imx/hab.h |  9 --
 arch/arm/mach-imx/hab.c | 59 +
 2 files changed, 65 insertions(+), 3 deletions(-)

-- 
2.7.4

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


[U-Boot] [PATCH 4/4] imx: hab: Provide hab_auth_img_or_fail command

2018-03-09 Thread Bryan O'Donoghue
This patch adds hab_auth_img_or_fail() a command line function that
encapsulates a common usage of authenticate and failover, namely if
authenticate image fails, then drop to BootROM USB recovery mode.

For secure-boot systems, this type of locked down behavior is important to
ensure no unsigned images can be run.

It's possible to script this logic but, when done over and over again the
environment starts get very complex and repetitive, reducing that script
repetition down to a command line function makes sense.

Signed-off-by: Bryan O'Donoghue 
Cc: Utkarsh Gupta 
Cc: Breno Lima 
Cc: Fabio Estevam 
---
 arch/arm/mach-imx/hab.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 0c18b2e..61ccdeb 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -366,6 +366,22 @@ static int do_hab_get_ivt_addr(cmd_tbl_t *cmdtp, int flag, 
int argc,
return CMD_RET_SUCCESS;
 }
 
+static int do_authenticate_image_or_failover(cmd_tbl_t *cmdtp, int flag,
+int argc, char * const argv[])
+{
+   if (!imx_hab_is_enabled())
+   goto done;
+
+   if (do_authenticate_image(NULL, flag, argc, argv) != CMD_RET_SUCCESS) {
+   fprintf(stderr, "authentication fail -> %s %s %s %s\n",
+   argv[0], argv[1], argv[2], argv[3]);
+   do_hab_failsafe(0, 0, 1, NULL);
+   };
+
+done:
+   return CMD_RET_SUCCESS;
+}
+
 U_BOOT_CMD(
hab_status, CONFIG_SYS_MAXARGS, 1, do_hab_status,
"display HAB status",
@@ -395,6 +411,16 @@ U_BOOT_CMD(
"ivt_offset - hex offset of IVT in the image"
  );
 
+U_BOOT_CMD(
+   hab_auth_img_or_fail, 4, 0,
+   do_authenticate_image_or_failover,
+   "authenticate image via HAB on failure drop to USB BootROM 
mode",
+   "addr length ivt_offset\n"
+   "addr - image hex address\n"
+   "length - image hex length\n"
+   "ivt_offset - hex offset of IVT in the image"
+ );
+
 #endif /* !defined(CONFIG_SPL_BUILD) */
 
 /* Get CSF Header length */
-- 
2.7.4

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


[U-Boot] hardware versions to Kernel

2018-03-09 Thread Andy Rea
I have an ARM CPU on a board that has 3 optional sub boards plugged into it,
all of which have 3 hardware rev bits connected to the CPU

 

Rather than getting the CPU to do mmap type stuff or sys/class/gpio stuff to
read those versions in the kernel, is there a way for u-boot to push that
information ( even it's a U32 or a string ) into the kernel somewhere that
can be parsed?  I tried some code in the board startup to create env vars to
append into the bootargs but they aren't evaluated each boot properly (
boards are optional and revs can change )  so cat /proc/cmdlne stayed the
same value all the time

 

Is there a better way to do this?

 

thanks

 

Andy

 



smime.p7s
Description: S/MIME cryptographic signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/1] efi_loader: efi_allocate_pages is too restrictive

2018-03-09 Thread Heinrich Schuchardt
On 03/09/2018 01:48 PM, Alexander Graf wrote:
> On 03/03/2018 03:48 PM, Heinrich Schuchardt wrote:
>> When running on the sandbox the stack is not necessarily at a higher
>> memory
>> address than the highest free memory.
>>
>> There is no reason why the checking of the highest memory address
>> should be
>> more restrictive for EFI_ALLOCATE_ANY_PAGES than for
>> EFI_ALLOCATE_MAX_ADDRESS.
>>
>> Signed-off-by: Heinrich Schuchardt 
>> ---
>>   lib/efi_loader/efi_memory.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index cc271e0709d..0efbb973231 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -294,7 +294,7 @@ efi_status_t efi_allocate_pages(int type, int
>> memory_type,
>>   switch (type) {
>>   case EFI_ALLOCATE_ANY_PAGES:
>>   /* Any page */
>> -    addr = efi_find_free_memory(len, gd->start_addr_sp);
>> +    addr = efi_find_free_memory(len, (uint64_t)-1);
> 
> This will break on systems that do not map high address space into the
> linear map (IIRC nvidia systems had that issue).
> 

The memory map is also passed on to the operating system when booting.
If a memory reservation is missing for any board it has to be fixed in
the board or driver files, cf.

sunxi: video: mark framebuffer as EFI reserved memory
https://lists.denx.de/pipermail/u-boot/2018-March/321820.htm

For type = EFI_ALLOCATE_MAX_ADDRESS we don't care about
gd->start_addr_sp. So if the memory map is incomplete the current code
may fail. Keeping things as they are is not a viable option.

Could you, please, identify for which Nvidia system a problem was
reported? Then we can add a call to efi_add_memory_map() for the board.

Best regards

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


Re: [U-Boot] [PATCH v2 2/2] bcm283x_pl011: Flush RX queue after setting baud rate

2018-03-09 Thread Tuomas Tynkkynen
On Wed, 7 Mar 2018 22:08:25 +0100
Alexander Graf  wrote:

> After the UART was initialized, we may still have bogus data in the
> RX queue if it was enabled with incorrect pin muxing before.
> 
> So let's flush the RX queue whenever we initialize baud rates.
> 
> This fixes a regression with the dynamic pinmuxing code when enable_uart=1
> is not set in config.txt on Raspberry Pis that use pl011 for serial.
> 
> Fixes: caf2233b28 ("bcm283x: Add pinctrl driver")
> Reported-by: Göran Lundberg 
> Reported-by: Peter Robinson 
> Signed-off-by: Alexander Graf 
> 
> ---
> 
> v1 -> v2:
> 
>   - correctly drain the queue
> ---
>  drivers/serial/serial_bcm283x_pl011.c  | 25 -
>  drivers/serial/serial_pl01x.c  | 10 +-
>  drivers/serial/serial_pl01x_internal.h |  7 ++-
>  3 files changed, 35 insertions(+), 7 deletions(-)
> 

On a RPi 1, with no enable_uart in config.txt:

Tested-by: Tuomas Tynkkynen 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/2] serial_bcm283x_mu: Flush RX queue after setting baud rate

2018-03-09 Thread Tuomas Tynkkynen
On Wed, 7 Mar 2018 22:08:24 +0100
Alexander Graf  wrote:

> After the UART was initialized, we may still have bogus data in the
> RX queue if it was enabled with incorrect pin muxing before.
> 
> So let's flush the RX queue whenever we initialize baud rates.
> 
> This fixes a regression with the dynamic pinmuxing code when enable_uart=1
> is not set in config.txt.
> 
> Fixes: caf2233b28 ("bcm283x: Add pinctrl driver")
> Reported-by: Göran Lundberg 
> Reported-by: Peter Robinson 
> Signed-off-by: Alexander Graf 
> ---
>  drivers/serial/serial_bcm283x_mu.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 

On a RPi 1, with no enable_uart in config.txt:

Tested-by: Tuomas Tynkkynen 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v3 1/1] efi_loader: Initial EFI_DEVICE_PATH_UTILITIES_PROTOCOL

2018-03-09 Thread Heinrich Schuchardt
From: Leif Lindholm 

Not complete, but enough for Shell.efi and SCT.efi.  We'll implement the
rest as needed or once we have SCT running properly so there is a way to
validate the interface against the conformance test suite.

Initial skeleton written by Leif, and then implementation by Rob.

Signed-off-by: Leif Lindholm 
[Fill initial skeleton]
Signed-off-by: Rob Clark 
[Rebase on v2018.03-rc1]
Signed-off-by: Heinrich Schuchardt 
---
v3
Rebase on v2018.03-rc1
v2
Fill initial skeleton
https://lists.denx.de/pipermail/u-boot/2017-October/308997.html
v1 
Initial patch by Leif
https://lists.denx.de/pipermail/u-boot/2017-September/305230.html
---
 include/efi_api.h  | 29 ++
 include/efi_loader.h   |  4 ++
 lib/efi_loader/Makefile|  3 +-
 lib/efi_loader/efi_boottime.c  |  6 ++
 lib/efi_loader/efi_device_path_utilities.c | 89 ++
 5 files changed, 130 insertions(+), 1 deletion(-)
 create mode 100644 lib/efi_loader/efi_device_path_utilities.c

diff --git a/include/efi_api.h b/include/efi_api.h
index 559e58e5501..993e8231fca 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -588,6 +588,35 @@ struct efi_device_path_to_text_protocol
bool allow_shortcuts);
 };
 
+#define EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID \
+   EFI_GUID(0x0379be4e, 0xd706, 0x437d, \
+0xb0, 0x37, 0xed, 0xb8, 0x2f, 0xb7, 0x72, 0xa4)
+
+struct efi_device_path_utilities_protocol {
+   efi_uintn_t (EFIAPI *get_device_path_size)(
+   const struct efi_device_path *device_path);
+   struct efi_device_path *(EFIAPI *duplicate_device_path)(
+   const struct efi_device_path *device_path);
+   struct efi_device_path *(EFIAPI *append_device_path)(
+   const struct efi_device_path *src1,
+   const struct efi_device_path *src2);
+   struct efi_device_path *(EFIAPI *append_device_node)(
+   const struct efi_device_path *device_path,
+   const struct efi_device_path *device_node);
+   struct efi_device_path *(EFIAPI *append_device_path_instance)(
+   const struct efi_device_path *device_path,
+   const struct efi_device_path *device_path_instance);
+   struct efi_device_path *(EFIAPI *get_next_device_path_instance)(
+   struct efi_device_path **device_path_instance,
+   efi_uintn_t *device_path_instance_size);
+   bool (EFIAPI *is_device_path_multi_instance)(
+   const struct efi_device_path *device_path);
+   struct efi_device_path *(EFIAPI *create_device_node)(
+   uint8_t node_type,
+   uint8_t node_sub_type,
+   uint16_t node_length);
+};
+
 #define EFI_GOP_GUID \
EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \
 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 7caae97ea70..0937b47e4bf 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -83,6 +83,9 @@ extern struct efi_simple_text_output_protocol efi_con_out;
 extern struct efi_simple_input_interface efi_con_in;
 extern struct efi_console_control_protocol efi_console_control;
 extern const struct efi_device_path_to_text_protocol efi_device_path_to_text;
+/* implementation of the EFI_DEVICE_PATH_UTILITIES_PROTOCOL */
+extern const struct efi_device_path_utilities_protocol
+   efi_device_path_utilities;
 
 uint16_t *efi_dp_str(struct efi_device_path *dp);
 
@@ -97,6 +100,7 @@ extern const efi_guid_t efi_guid_loaded_image;
 extern const efi_guid_t efi_guid_device_path_to_text_protocol;
 extern const efi_guid_t efi_simple_file_system_protocol_guid;
 extern const efi_guid_t efi_file_info_guid;
+extern const efi_guid_t efi_guid_device_path_utilities_protocol;
 
 extern unsigned int __efi_runtime_start, __efi_runtime_stop;
 extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 2722265ee3d..55c97c04766 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -17,7 +17,8 @@ endif
 obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
 obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
 obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
-obj-y += efi_file.o efi_variable.o efi_bootmgr.o efi_watchdog.o
+obj-y += efi_device_path_utilities.o efi_file.o efi_variable.o efi_bootmgr.o
+obj-y += efi_watchdog.o
 obj-$(CONFIG_LCD) += efi_gop.o
 obj-$(CONFIG_DM_VIDEO) += efi_gop.o
 obj-$(CONFIG_PARTITIONS) += efi_disk.o
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 013e0353c3a..0fd272253d2 100644
--- a/lib/efi_loader/efi_boottime.c
+++ 

Re: [U-Boot] [PATCH 1/1] sunxi: video: mark framebuffer as EFI reserved memory

2018-03-09 Thread Heinrich Schuchardt
On 03/06/2018 11:21 AM, Anatolij Gustschin wrote:
> Hello Heinrich,
> 
> On Mon, 5 Mar 2018 17:55:52 +0100
> Heinrich Schuchardt xypron.g...@gmx.de wrote:
> ...
>> v2018.05 is fine. The problem will become visible in more cases with
>>
>> [PATCH 1/1] efi_loader: efi_allocate_pages is too restrictive
>> https://patchwork.ozlabs.org/patch/881055/
>> https://lists.denx.de/pipermail/u-boot/2018-March/321840.html
>>
>> which is also pending.
> 
> Okay, I'll apply your patch when efi_loader patch is merged.

If efi_allocate_memory is called with type = EFI_ALLOCATE_MAX_ADDRESS we
can already have an error. So, please, put the patch into your v2018.05
queue. There is no need to wait for Alex.

Regards

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


Re: [U-Boot] [PATCH 1/1] efi_loader: efi_allocate_pages is too restrictive

2018-03-09 Thread Alexander Graf

On 03/09/2018 04:58 PM, Heinrich Schuchardt wrote:

On 03/09/2018 01:48 PM, Alexander Graf wrote:

On 03/03/2018 03:48 PM, Heinrich Schuchardt wrote:

When running on the sandbox the stack is not necessarily at a higher
memory
address than the highest free memory.

There is no reason why the checking of the highest memory address
should be
more restrictive for EFI_ALLOCATE_ANY_PAGES than for
EFI_ALLOCATE_MAX_ADDRESS.

Signed-off-by: Heinrich Schuchardt 
---
   lib/efi_loader/efi_memory.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index cc271e0709d..0efbb973231 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -294,7 +294,7 @@ efi_status_t efi_allocate_pages(int type, int
memory_type,
   switch (type) {
   case EFI_ALLOCATE_ANY_PAGES:
   /* Any page */
-addr = efi_find_free_memory(len, gd->start_addr_sp);
+addr = efi_find_free_memory(len, (uint64_t)-1);

This will break on systems that do not map high address space into the
linear map (IIRC nvidia systems had that issue).


The memory map is also passed on to the operating system when booting.
If a memory reservation is missing for any board it has to be fixed in
the board or driver files, cf.

sunxi: video: mark framebuffer as EFI reserved memory
https://lists.denx.de/pipermail/u-boot/2018-March/321820.htm

For type = EFI_ALLOCATE_MAX_ADDRESS we don't care about
gd->start_addr_sp. So if the memory map is incomplete the current code
may fail. Keeping things as they are is not a viable option.

Could you, please, identify for which Nvidia system a problem was
reported? Then we can add a call to efi_add_memory_map() for the board.


Git blame points to this commit. I guess -1 should do the same thing 
then, true.


Andreas, would you see any reason -1 will not work?


Alex

commit dede284d1ce9f9d9e79a5114fe7eb814fec07679
Author: Andreas Färber 
Date:   Wed Apr 13 14:04:38 2016 +0200

efi_loader: Handle memory overflows

jetson-tk1 has 2 GB of RAM at 0x8000, causing gd->ram_top to be 
zero.

Handle this by either avoiding ram_top or by using the same type as
ram_top to reverse the overflow effect.

Cc: Alexander Graf 
Signed-off-by: Andreas Färber 
Reviewed-by: Alexander Graf 

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index df995858ed..71a3d19269 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -220,7 +220,7 @@ efi_status_t efi_allocate_pages(int type, int 
memory_type,

switch (type) {
case 0:
/* Any page */
-   addr = efi_find_free_memory(len, gd->ram_top);
+   addr = efi_find_free_memory(len, gd->start_addr_sp);
if (!addr) {
r = EFI_NOT_FOUND;
break;
@@ -320,9 +320,9 @@ efi_status_t efi_get_memory_map(unsigned long 
*memory_map_size,


 int efi_memory_init(void)
 {
-   uint64_t runtime_start, runtime_end, runtime_pages;
-   uint64_t uboot_start, uboot_pages;
-   uint64_t uboot_stack_size = 16 * 1024 * 1024;
+   unsigned long runtime_start, runtime_end, runtime_pages;
+   unsigned long uboot_start, uboot_pages;
+   unsigned long uboot_stack_size = 16 * 1024 * 1024;
int i;

/* Add RAM */



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


Re: [U-Boot] [PATCH 1/1] efi_loader: efi_allocate_pages is too restrictive

2018-03-09 Thread Heinrich Schuchardt
On 03/09/2018 05:19 PM, Alexander Graf wrote:
> On 03/09/2018 04:58 PM, Heinrich Schuchardt wrote:
>> On 03/09/2018 01:48 PM, Alexander Graf wrote:
>>> On 03/03/2018 03:48 PM, Heinrich Schuchardt wrote:
 When running on the sandbox the stack is not necessarily at a higher
 memory
 address than the highest free memory.

 There is no reason why the checking of the highest memory address
 should be
 more restrictive for EFI_ALLOCATE_ANY_PAGES than for
 EFI_ALLOCATE_MAX_ADDRESS.

 Signed-off-by: Heinrich Schuchardt 
 ---
    lib/efi_loader/efi_memory.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
 index cc271e0709d..0efbb973231 100644
 --- a/lib/efi_loader/efi_memory.c
 +++ b/lib/efi_loader/efi_memory.c
 @@ -294,7 +294,7 @@ efi_status_t efi_allocate_pages(int type, int
 memory_type,
    switch (type) {
    case EFI_ALLOCATE_ANY_PAGES:
    /* Any page */
 -    addr = efi_find_free_memory(len, gd->start_addr_sp);
 +    addr = efi_find_free_memory(len, (uint64_t)-1);
>>> This will break on systems that do not map high address space into the
>>> linear map (IIRC nvidia systems had that issue).
>>>
>> The memory map is also passed on to the operating system when booting.
>> If a memory reservation is missing for any board it has to be fixed in
>> the board or driver files, cf.
>>
>> sunxi: video: mark framebuffer as EFI reserved memory
>> https://lists.denx.de/pipermail/u-boot/2018-March/321820.htm
>>
>> For type = EFI_ALLOCATE_MAX_ADDRESS we don't care about
>> gd->start_addr_sp. So if the memory map is incomplete the current code
>> may fail. Keeping things as they are is not a viable option.
>>
>> Could you, please, identify for which Nvidia system a problem was
>> reported? Then we can add a call to efi_add_memory_map() for the board.
> 
> Git blame points to this commit. I guess -1 should do the same thing
> then, true.
> 
> Andreas, would you see any reason -1 will not work?

Are we talking about this line:

arch/arm/mach-tegra/board2.c:317:
gd->pci_ram_top = gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size;

Regards

Heinrich
> 
> 
> Alex
> 
> commit dede284d1ce9f9d9e79a5114fe7eb814fec07679
> Author: Andreas Färber 
> Date:   Wed Apr 13 14:04:38 2016 +0200
> 
>     efi_loader: Handle memory overflows
> 
>     jetson-tk1 has 2 GB of RAM at 0x8000, causing gd->ram_top to be
> zero.
>     Handle this by either avoiding ram_top or by using the same type as
>     ram_top to reverse the overflow effect.
> 
>     Cc: Alexander Graf 
>     Signed-off-by: Andreas Färber 
>     Reviewed-by: Alexander Graf 
> 
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index df995858ed..71a3d19269 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -220,7 +220,7 @@ efi_status_t efi_allocate_pages(int type, int
> memory_type,
>     switch (type) {
>     case 0:
>     /* Any page */
> -   addr = efi_find_free_memory(len, gd->ram_top);
> +   addr = efi_find_free_memory(len, gd->start_addr_sp);
>     if (!addr) {
>     r = EFI_NOT_FOUND;
>     break;
> @@ -320,9 +320,9 @@ efi_status_t efi_get_memory_map(unsigned long
> *memory_map_size,
> 
>  int efi_memory_init(void)
>  {
> -   uint64_t runtime_start, runtime_end, runtime_pages;
> -   uint64_t uboot_start, uboot_pages;
> -   uint64_t uboot_stack_size = 16 * 1024 * 1024;
> +   unsigned long runtime_start, runtime_end, runtime_pages;
> +   unsigned long uboot_start, uboot_pages;
> +   unsigned long uboot_stack_size = 16 * 1024 * 1024;
>     int i;
> 
>     /* Add RAM */
> 
> 
> 
> 

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