Re: [PATCH] doc: sandbox: Add additional valgrind documentation
On 5/27/22 05:36, Sean Anderson wrote: This document some additional options which can be used with valgrind, as Thanks for enhancing this document nits %s/document/documents/ well as directions for future work. It also fixes up inline literals to actually be inline literals (and not italics). The content of this documentation is primarily adapted from [1]. [1] https://lore.kernel.org/u-boot/57cb4b49-fa30-1194-9ac3-faa53e803...@gmail.com/ Signed-off-by: Sean Anderson --- doc/arch/sandbox.rst | 65 +--- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/doc/arch/sandbox.rst b/doc/arch/sandbox.rst index e1119492b4..cd5090be71 100644 --- a/doc/arch/sandbox.rst +++ b/doc/arch/sandbox.rst @@ -479,19 +479,76 @@ It is possible to run U-Boot under valgrind to check memory allocations:: valgrind ./u-boot -For more detailed results, enable `CONFIG_VALGRIND`. There are many false -positives due to `malloc` itself. Suppress these with:: +For more detailed results, enable ``CONFIG_VALGRIND``. There are many false +positives due to ``malloc`` itself. Suppress these with:: What do you mean by 'malloc itself'? Is it the internal implementation of malloc? Or is it the act of calling malloc()? This paragraph should explain what CONFIG_VALGRIND does: The sandbox allocates a memory pool via mmap(). U-Boot's internal malloc() and free() work on this memory pool. Custom allocators and deallocators are by default invisible to valgrind. It is CONFIG_VARLGRIND=y that exposes U-Boot's malloc() and free() to valgrind. If I understand include/valgrind/valgrind.h correctly, it injects placeholder assembler code that valgrind can replace by library calls into valgrind itself when loading the U-Boot binary. I miss a statement indicating that the sandbox on RISC-V has no valgrind support, yet. The U-Boot code does not use VALGRIND_MEMPOOL* macros to indicate which memory pools it manages. Is this the reason for the problems we are facing? valgrind --suppressions=scripts/u-boot.supp ./u-boot If you are running sandbox SPL or TPL, then valgrind will not by default notice when U-Boot jumps from TPL to SPL, or from SPL to U-Boot proper. To -fix this, use `--trace-children=yes`. To show who alloc'd some troublesome -memory, use `--track-origins=yes`. To uncover possible errors, try running all +fix this, use ``--trace-children=yes``. To show who alloc'd some troublesome +memory, use ``--track-origins=yes``. To uncover possible errors, try running all unit tests with:: valgrind --track-origins=yes --suppressions=scripts/u-boot.supp ./u-boot -Tc 'ut all' I would prefer a list like: --suppressions=scripts/u-boot.supp Suppress false positives due to the internal implementation of malloc --trace-children=yes Let valgrind consider the progression from TPL to SPL to main U-Boot +Additional options +^^ + +The following options are useful in addition to the above examples: + +* ``--error-limit=no`` will enable printing more than 1000 errors in a + single session. +* ``--vgdb=yes --vgdb-error=0`` will let you use gdb to attach like:: + +gdb -ex "target remote | vgdb" u-boot + + This is very helpful for inspecting the program state when there is + an error. +* Passing ``-t cooked`` to U-Boot will keep the console in a sane state if you + terminate it early (instead of having to run tset). + +Future work +^^^ + +The biggest limitation to the current approach is that +supressions don't "un-taint" uninitialized memory accesses. Currently, I +have dlmalloc's reads its bookkeeping information marked as a "red This documentation does not have a single named author. The pronoun "I" has no reference point in this context. "its" does not refer to anything. +zone." This means that all reads to it are marked as illegal by "it" has no clear reference point. +valgrind. This is fine for regular code, but dlmalloc really does need +to access this area, so we suppress its violations. However, if dlmalloc +then passes a result calculated from a "tainted" access, that result is +still tainted. So the first accessor will raise a warning. This means +that every construct like + +.. code-block:: + +foo = malloc(sizeof(*foo)); +if (!foo) +return -ENOMEM; + +will raise a warning when we check the result of malloc. Whoops. + +There are three ways (as I see it) to address this: %s/(as I see it)// Best regards Heinrich + +* Don't mark dlmalloc bookkeeping information as a red zone. This is the + simplest solution, but reduces the power of valgrind immensely, since + we can no longer determine that (e.g.) access past the end of an array + is undefined. +* Implement red zones properly. This would involve growing every + allocation by a fixed amount (16 bytes or so) and then using that + extra space for a real red zone that neither regular code nor dlmalloc + needs to access. Unfort
Re: [PATCH 2/2] tpm: add support for TPMv2.x I2C chips
On Mon, 23 May 2022 at 13:25, Ilias Apalodimas wrote: > > On Mon, May 23, 2022 at 08:19:51AM -0500, Eddie James wrote: > > Yes, I think so. I tested with an older u-boot (openbmc uses a modified > > v2019.04) with all the TPM core patches, and I think I missed this bit in > > rebasing. > > No worries, I can fix this while merging, there's no need for a v2. I > don't see anything obviously wrong with the patchset, unfortunately I > don't have an i2c tpm to test. Anyway > > Reviewed-by: Ilias Apalodimas > > P.S: Was the new TIS API useful? I'll reply on behalf of Eddie; he said it made it much easier to implement! Thanks! We've backported the patches to our v2019.04 branch and are using them there. Cheers, Joel
Re: [PATCH 2/2] tpm: add support for TPMv2.x I2C chips
On Fri, 27 May 2022 at 10:26, Joel Stanley wrote: > > On Mon, 23 May 2022 at 13:25, Ilias Apalodimas > wrote: > > > > On Mon, May 23, 2022 at 08:19:51AM -0500, Eddie James wrote: > > > Yes, I think so. I tested with an older u-boot (openbmc uses a modified > > > v2019.04) with all the TPM core patches, and I think I missed this bit in > > > rebasing. > > > > No worries, I can fix this while merging, there's no need for a v2. I > > don't see anything obviously wrong with the patchset, unfortunately I > > don't have an i2c tpm to test. Anyway > > > > Reviewed-by: Ilias Apalodimas > > > > P.S: Was the new TIS API useful? > > I'll reply on behalf of Eddie; he said it made it much easier to > implement! Thanks! > > We've backported the patches to our v2019.04 branch and are using them there. > > Cheers, Great thanks! Patches are in -master now Cheers /Ilias > > Joel
Re: Fwd: dlmalloc hanging
Am 27. Mai 2022 09:52:04 MESZ schrieb Oleksii Kurochko : >Hello Heinrich, > >get_maintainer.pl script told me that you are a maintainer of dlmalloc.c Hello Oleksii, I am not the maintainer of dlmalloc.c but only a contributor. >so if you don't mind I would like to ask you for advice about >how could debugging of dlmalloc allocation be done? > >I faced the issue that hanging happens during the freeing of >resources [ >https://elixir.bootlin.com/u-boot/latest/source/arch/arm/lib/bootm.c#L77] >before U-boot finally will transfer control to Linux Kernel. >I did a little investigation and found out that hanging >happens here [ >https://elixir.bootlin.com/u-boot/latest/source/common/dlmalloc.c#L940] >and it happens because FD->fd points to itself so a cycle occurs. Does the problem exist in mainline U-Boot? How can it be reproduced in mainline U-Boot? If it is a Bootlin specific problem, please, consider contacting that company. Possible reasons for running into problems in dlmalloc.c are double frees and out of bound writing to memory. Best regards Heinrich > >Should be a check added here which will check that FD->fd does not point to >itself or >does a guarantee exist that the cycle can't be at all? > >Also, I tried to do "#define DEBUG" at the top of dlmalloc.c >and I started to be asserted here: >https://elixir.bootlin.com/u-boot/latest/source/common/dlmalloc.c#L842 >during the mentioned transfer from U-boot to kernel. > >I also tried to understand why it is in the used state and I couldn't find >where it is actually switched to the unused state. At least clear_inuse() >macros aren't used elsewhere in file dlmalloc.c. > >Interesting behavior has been obtained. It looks like that for >the number of lines in CONFIG_EXTRA_ENV_SETTINGS there is no >hanging at all during mentioned above transfer. So I can assume that >it might be allocated a buffer or pointer to its buffer is overwritten >because of the memory alignment of an array that uses >CONFIG_EXTRA_ENV_SETTINGS. > >Could you please give me some advice on how such behavior >could be debugged? > >Thanks in advance. > >Best regards, > > Oleksii
Re: [PATCH v2 2/2] bootmenu: U-Boot console is enabled as default
On Thu, 26 May 2022 at 13:08, Masahisa Kojima
wrote:
>
> The commit 2158b0da220c ("bootmenu: add Kconfig option
> not to enter U-Boot console") disables to enter U-Boot
> console from bootmenu as default, this change affects the
> existing bootmenu users.
>
> This commit reverts the default behavior, the bootmenu can
> enter U-Boot console same as before.
> CMD_BOOTMENU_ENTER_UBOOT_CONSOLE is renamed
> BOOTMENU_DISABLE_UBOOT_CONSOLE and depends on
> AUTOBOOT_MENU_SHOW.
>
> Fixes: 2158b0da220c ("bootmenu: add Kconfig option not to enter U-Boot
> console")
> Signed-off-by: Masahisa Kojima
> Tested-by: Pali Rohar
> ---
> Changes in v2:
> - remove "default n" since it is default option
> - use 80 characters in one line
>
> boot/Kconfig | 7 +++
> cmd/Kconfig| 10 --
> cmd/bootmenu.c | 4 ++--
> 3 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/boot/Kconfig b/boot/Kconfig
> index dff4d23b88..08451c65a5 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -1143,6 +1143,13 @@ config AUTOBOOT_MENU_SHOW
> environmnent variable (if enabled) and before handling the boot
> delay.
> See README.bootmenu for more details.
>
> +config BOOTMENU_DISABLE_UBOOT_CONSOLE
> + bool "Disallow bootmenu to enter the U-Boot console"
> + depends on AUTOBOOT_MENU_SHOW
> + help
> + If this option is enabled, user can not enter the U-Boot console
> from
> + bootmenu. It increases the system security.
> +
> config BOOT_RETRY
> bool "Boot retry feature"
> help
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 69c1814d24..09193b61b9 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -357,16 +357,6 @@ config CMD_BOOTMENU
> help
> Add an ANSI terminal boot menu command.
>
> -config CMD_BOOTMENU_ENTER_UBOOT_CONSOLE
> - bool "Allow Bootmenu to enter the U-Boot console"
> - depends on CMD_BOOTMENU
> - default n
> - help
> - Add an entry to enter U-Boot console in bootmenu.
> - If this option is disabled, user can not enter
> - the U-Boot console from bootmenu. It increases
> - the system security.
> -
> config CMD_ADTIMG
> bool "adtimg"
> help
> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> index bf88c2127b..1002c6b20a 100644
> --- a/cmd/bootmenu.c
> +++ b/cmd/bootmenu.c
> @@ -362,7 +362,7 @@ static struct bootmenu_data *bootmenu_create(int delay)
> goto cleanup;
>
> /* Add Quit entry if entering U-Boot console is disabled */
> - if (IS_ENABLED(CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE))
> + if (!IS_ENABLED(CONFIG_BOOTMENU_DISABLE_UBOOT_CONSOLE))
> entry->title = strdup("U-Boot console");
> else
> entry->title = strdup("Quit");
> @@ -595,7 +595,7 @@ int menu_show(int bootdelay)
> if (ret == BOOTMENU_RET_UPDATED)
> continue;
>
> - if (!IS_ENABLED(CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE)) {
> + if (IS_ENABLED(CONFIG_BOOTMENU_DISABLE_UBOOT_CONSOLE)) {
> if (ret == BOOTMENU_RET_QUIT) {
> /* default boot process */
> if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR))
> --
> 2.17.1
>
Reviewed-by: Ilias Apalodimas
Re: Fwd: dlmalloc hanging
Heinrich, > Does the problem exist in mainline U-Boot? I am using U-boot the following U-boot [ git:// source.denx.de/u-boot/u-boot.git ] with xenguest_arm64_defconfig. > How can it be reproduced in mainline U-Boot? I would try to use QEMU to reproduce the problem. If I do it successfully I'll post instruction on how to reproduce it. Best regards, Oleksii пт, 27 мая 2022 г. в 11:29, Heinrich Schuchardt : > > > Am 27. Mai 2022 09:52:04 MESZ schrieb Oleksii Kurochko < > oleksii.kuroc...@gmail.com>: > >Hello Heinrich, > > > >get_maintainer.pl script told me that you are a maintainer of dlmalloc.c > > Hello Oleksii, > > I am not the maintainer of dlmalloc.c but only a contributor. > > >so if you don't mind I would like to ask you for advice about > >how could debugging of dlmalloc allocation be done? > > > >I faced the issue that hanging happens during the freeing of > >resources [ > >https://elixir.bootlin.com/u-boot/latest/source/arch/arm/lib/bootm.c#L77] > >before U-boot finally will transfer control to Linux Kernel. > >I did a little investigation and found out that hanging > >happens here [ > >https://elixir.bootlin.com/u-boot/latest/source/common/dlmalloc.c#L940] > >and it happens because FD->fd points to itself so a cycle occurs. > > Does the problem exist in mainline U-Boot? > How can it be reproduced in mainline U-Boot? > > If it is a Bootlin specific problem, please, consider contacting that > company. > > Possible reasons for running into problems in dlmalloc.c are double frees > and out of bound writing to memory. > > Best regards > > Heinrich > > > > >Should be a check added here which will check that FD->fd does not point > to > >itself or > >does a guarantee exist that the cycle can't be at all? > > > >Also, I tried to do "#define DEBUG" at the top of dlmalloc.c > >and I started to be asserted here: > >https://elixir.bootlin.com/u-boot/latest/source/common/dlmalloc.c#L842 > >during the mentioned transfer from U-boot to kernel. > > > >I also tried to understand why it is in the used state and I couldn't find > >where it is actually switched to the unused state. At least clear_inuse() > >macros aren't used elsewhere in file dlmalloc.c. > > > >Interesting behavior has been obtained. It looks like that for > >the number of lines in CONFIG_EXTRA_ENV_SETTINGS there is no > >hanging at all during mentioned above transfer. So I can assume that > >it might be allocated a buffer or pointer to its buffer is overwritten > >because of the memory alignment of an array that uses > >CONFIG_EXTRA_ENV_SETTINGS. > > > >Could you please give me some advice on how such behavior > >could be debugged? > > > >Thanks in advance. > > > >Best regards, > > > > Oleksii >
Re: [PATCH] doc: sandbox: Add additional valgrind documentation
On 5/27/22 3:14 AM, Heinrich Schuchardt wrote: On 5/27/22 05:36, Sean Anderson wrote: This document some additional options which can be used with valgrind, as Thanks for enhancing this document nits %s/document/documents/ well as directions for future work. It also fixes up inline literals to actually be inline literals (and not italics). The content of this documentation is primarily adapted from [1]. [1] https://lore.kernel.org/u-boot/57cb4b49-fa30-1194-9ac3-faa53e803...@gmail.com/ Signed-off-by: Sean Anderson --- doc/arch/sandbox.rst | 65 +--- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/doc/arch/sandbox.rst b/doc/arch/sandbox.rst index e1119492b4..cd5090be71 100644 --- a/doc/arch/sandbox.rst +++ b/doc/arch/sandbox.rst @@ -479,19 +479,76 @@ It is possible to run U-Boot under valgrind to check memory allocations:: valgrind ./u-boot -For more detailed results, enable `CONFIG_VALGRIND`. There are many false -positives due to `malloc` itself. Suppress these with:: +For more detailed results, enable ``CONFIG_VALGRIND``. There are many false +positives due to ``malloc`` itself. Suppress these with:: What do you mean by 'malloc itself'? Is it the internal implementation of malloc? Or is it the act of calling malloc()? This paragraph should explain what CONFIG_VALGRIND does: The sandbox allocates a memory pool via mmap(). U-Boot's internal malloc() and free() work on this memory pool. Custom allocators and deallocators are by default invisible to valgrind. It is CONFIG_VARLGRIND=y that exposes U-Boot's malloc() and free() to valgrind. If I understand include/valgrind/valgrind.h correctly, it injects placeholder assembler code that valgrind can replace by library calls into valgrind itself when loading the U-Boot binary. I believe this is correct. I will adapt your above description for v2. I miss a statement indicating that the sandbox on RISC-V has no valgrind support, yet. I was not aware of this. The Kconfig should probably be updated. The U-Boot code does not use VALGRIND_MEMPOOL* macros to indicate which memory pools it manages. Is this the reason for the problems we are facing? valgrind --suppressions=scripts/u-boot.supp ./u-boot If you are running sandbox SPL or TPL, then valgrind will not by default notice when U-Boot jumps from TPL to SPL, or from SPL to U-Boot proper. To -fix this, use `--trace-children=yes`. To show who alloc'd some troublesome -memory, use `--track-origins=yes`. To uncover possible errors, try running all +fix this, use ``--trace-children=yes``. To show who alloc'd some troublesome +memory, use ``--track-origins=yes``. To uncover possible errors, try running all unit tests with:: valgrind --track-origins=yes --suppressions=scripts/u-boot.supp ./u-boot -Tc 'ut all' I would prefer a list like: --suppressions=scripts/u-boot.supp Suppress false positives due to the internal implementation of malloc --trace-children=yes Let valgrind consider the progression from TPL to SPL to main U-Boot I will merge this with the below options. +Additional options +^^ + +The following options are useful in addition to the above examples: + +* ``--error-limit=no`` will enable printing more than 1000 errors in a + single session. +* ``--vgdb=yes --vgdb-error=0`` will let you use gdb to attach like:: + + gdb -ex "target remote | vgdb" u-boot + + This is very helpful for inspecting the program state when there is + an error. +* Passing ``-t cooked`` to U-Boot will keep the console in a sane state if you + terminate it early (instead of having to run tset). + +Future work +^^^ + +The biggest limitation to the current approach is that +supressions don't "un-taint" uninitialized memory accesses. Currently, I +have dlmalloc's reads its bookkeeping information marked as a "red This documentation does not have a single named author. The pronoun "I" has no reference point in this context. "its" does not refer to anything. Sorry, this is a bit unclear, the wording should be something like Currently, dlmalloc's bookkeeping information is marked as a "red zone" +zone." This means that all reads to it are marked as illegal by "it" has no clear reference point. s/it/that area/ +valgrind. This is fine for regular code, but dlmalloc really does need +to access this area, so we suppress its violations. However, if dlmalloc +then passes a result calculated from a "tainted" access, that result is +still tainted. So the first accessor will raise a warning. This means +that every construct like + +.. code-block:: + + foo = malloc(sizeof(*foo)); + if (!foo) + return -ENOMEM; + +will raise a warning when we check the result of malloc. Whoops. + +There are three ways (as I see it) to address this: %s/(as I see it)// Will update. Thanks for the feedback. --Sean + +* Don't mark dlmalloc bookkeeping info
Re: [PATCH] net: e1000: Depend on CONFIG_PCI
On Tue, Apr 26, 2022 at 02:35:33PM -0400, Sean Anderson wrote: > This driver depends on PCI. Update the Kconfig accordingly. > > Signed-off-by: Sean Anderson > Reviewed-by: Tim Harvey > Reviewed-by: Stefan Roese > Reviewed-by: Ramon Fried Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 2/3] test/py: test_fs: Correct the test case name
On Tue, May 17, 2022 at 11:24:44PM +0800, Bin Meng wrote: > Use test_fstypes as the name instead of test_dm_compact. > > Signed-off-by: Bin Meng Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 3/3] test/py: test_part: Correct the test case name
On Tue, May 17, 2022 at 11:24:45PM +0800, Bin Meng wrote: > Use test_part_types as the name instead of dm_compact. > > Signed-off-by: Bin Meng Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/3] test/py: Reset the console timeout value
On Tue, May 17, 2022 at 11:24:43PM +0800, Bin Meng wrote: > Reset the console timeout value as some tests may change its default > value during the execution. > > This fixes the random case timeout issue seen in the U-Boot CI. > > Signed-off-by: Bin Meng Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2] Fix CVE-2022-30767 (old CVE-2019-14196)
On Wed, May 18, 2022 at 04:30:08PM +, Andrea zi0Black Cappa wrote: > This patch mitigates the vulnerability identified via CVE-2019-14196. > The previous patch was bypassed/ineffective, and now the vulnerability is > identified via CVE-2022-30767. The patch removes the sanity check introduced > to mitigate CVE-2019-14196 since it's ineffective. > filefh3_length is changed to unsigned type integer, preventing negative > numbers from being used during comparison with positive values during size > sanity checks. > > Signed-off-by: Andrea zi0Black Cappa Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] MAINTAINERS: aspeed: Add more files and myself as a reviewer
On Thu, May 19, 2022 at 10:06:33AM +0930, Joel Stanley wrote: > Add the rest of the ASPEED drivers that are in tree. Most are obvious, > except for ftgmac100 which matches the register layout used in the > ASPEED SoC. > > I am the Linux maintainer for the ASPEED kernel port, and help maintain > the fork of u-boot used for OpenBMC, so add myself as a reviewer so I > can stay informed about u-boot changes. > > Signed-off-by: Joel Stanley > Reviewed-by: Chia-Wei Wang Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] arm: dts: k3-am642-*: Mark the memory node with u-boot, dm-spl
On Fri, May 20, 2022 at 03:30:26PM +0300, Georgi Vlaev wrote:
> Since commit dffdb1f8eb ("board: ti: am64x: Use fdt functions
> for ram and bank init") ddr_init() and dram_bank_init() have
> switched to fdtdec for getting the memory configuration from
> the am64xx dts files instead of using hardcoded values. This
> requires an accessible memory node in SPL as we already have
> in k3-am642-r5-evm.dts.
>
> Make the memory node accessible in A53 SPL for both am642-sk
> and am642-evm and in am642-sk R5 SPL.
>
> Signed-off-by: Georgi Vlaev
> Reviewed-by: Tom Rini
Applied to u-boot/master, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] board: freescale: Update MAINTAINERS List
On Sun, May 22, 2022 at 07:23:58AM +0200, Wasim Khan wrote: > From: Wasim Khan > > Update MAINTAINERS List for LS2088ARDB and LS2088AQDS > platforms > > Signed-off-by: Wasim Khan Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] test: fix parsing the mksquashfs version number
On Tue, May 24, 2022 at 01:36:21PM +0200, Heinrich Schuchardt wrote:
> Testing with mksquasshfs 4.5.1 results in an error
>
> ValueError: could not convert string to float: '4.5.1'
>
> Version 4.10 would be considered to be lower than 4.4.
>
> Fixes: 04c9813e951f ("test/py: rewrite common tools for SquashFS tests")
> Signed-off-by: Heinrich Schuchardt
Applied to u-boot/master, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PULL] u-boot-riscv/master
On Fri, May 27, 2022 at 02:36:29AM +, Leo Liang wrote: > Hi Tom, > > The following changes since commit 7e0edcadb09d55d5319fdc862041fd1b874476f5: > > Merge branch 'master' of > https://source.denx.de/u-boot/custodians/u-boot-sunxi (2022-05-24 23:29:00 > -0400) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-riscv.git > > for you to fetch changes up to c544b281cd3e549a4fcbf4ba9a05a5d72c9557dd: > > riscv: qemu: Set kernel_comp_addr_r for compressed kernel (2022-05-26 > 18:42:34 +0800) > > CI result shows no issue: > https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/12131 First, I've applied this to u-boot/master now. Second, will https://patchwork.ozlabs.org/project/uboot/patch/ph7pr14mb5594fd11d1be74284f554bebce...@ph7pr14mb5594.namprd14.prod.outlook.com/ be coming soon? Thanks! -- Tom signature.asc Description: PGP signature
[PATCH v2 2/2] valgrind: Disable on Risc-V
There are no defined instruction sequences in include/valgrind.h for Risc-V, so CONFIG_VALGRIND will do nothing on this arch (and possibly won't compile?). Update Kconfig accordingly. Signed-off-by: Sean Anderson --- Changes in v2: - New Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/Kconfig b/Kconfig index bdae59e06f..a83b534b34 100644 --- a/Kconfig +++ b/Kconfig @@ -307,6 +307,7 @@ config TPL_SYS_MALLOC_F_LEN config VALGRIND bool "Inform valgrind about memory allocations" + depends on !RISCV help Valgrind is an instrumentation framework for building dynamic analysis tools. In particular, it may be used to detect memory management bugs -- 2.35.1
[PATCH v2 1/2] doc: sandbox: Add additional valgrind documentation
This documents some additional options which can be used with valgrind, as well as directions for future work. It also fixes up inline literals to actually be inline literals (and not italics). The content of this documentation is primarily adapted from [1]. [1] https://lore.kernel.org/u-boot/57cb4b49-fa30-1194-9ac3-faa53e803...@gmail.com/ Signed-off-by: Sean Anderson --- Changes in v2: - Reworked most documentation to address Heinrich's feedback doc/arch/sandbox.rst | 91 1 file changed, 83 insertions(+), 8 deletions(-) diff --git a/doc/arch/sandbox.rst b/doc/arch/sandbox.rst index e1119492b4..ede760a965 100644 --- a/doc/arch/sandbox.rst +++ b/doc/arch/sandbox.rst @@ -479,18 +479,93 @@ It is possible to run U-Boot under valgrind to check memory allocations:: valgrind ./u-boot -For more detailed results, enable `CONFIG_VALGRIND`. There are many false -positives due to `malloc` itself. Suppress these with:: +However, this does not give very useful results. The sandbox allocates a memory +pool via mmap(). U-Boot's internal malloc() and free() work on this memory pool. +Custom allocators and deallocators are invisible to valgrind by default. To +expose U-Boot's malloc() and free() to valgrind, enable ``CONFIG_VALGRIND``. +Enabling this option will inject placeholder assembler code which valgrind +interprets. This is used to annotate sections of memory as safe or unsafe, and +to inform valgrind about malloc()s and free()s. There are currently no standard +placeholder addembly sequences for RISC-V, so this option cannot be enabled on +that architecture. + +Malloc's bookeeping information is marked as unsafe by default. However, this +will generate many false positives when malloc itself accesses this information. +These warnings can be suppressed with:: valgrind --suppressions=scripts/u-boot.supp ./u-boot -If you are running sandbox SPL or TPL, then valgrind will not by default -notice when U-Boot jumps from TPL to SPL, or from SPL to U-Boot proper. To -fix this, use `--trace-children=yes`. To show who alloc'd some troublesome -memory, use `--track-origins=yes`. To uncover possible errors, try running all -unit tests with:: +Additionally, you may experience false positives if U-Boot is using a smaller +pointer size than your host architecture. This is because the pointers used by +U-Boot will only contain 32 bits of addressing information. When interpreted as +64-bit pointers, valgrind will think that they are not initialized properly. To +fix this, enable ``CONFIG_SANDBOX64`` (such as via ``sandbox64_defconfig``) +when running on a 64-bit host. -valgrind --track-origins=yes --suppressions=scripts/u-boot.supp ./u-boot -Tc 'ut all' +Additional options +^^ + +The following options are useful in addition to the above examples: + +* ``--trace-childen=yes`` will tell valgrind to keep tracking subprocesses, such + as when U-Boot jumps from TPL to SPL, or from SPL to U-Boot proper. +* ``--track-origins=yes`` will (for a small overhead) tell valgrind to keep + track of who allocated some troublesome memory. +* ``--error-limit=no`` will enable printing more than 1000 errors in a + single session. +* ``--vgdb=yes --vgdb-error=0`` will let you use gdb to attach like:: + +gdb -ex "target remote | vgdb" u-boot + + This is very helpful for inspecting the program state when there is + an error. +* Passing ``-Tc 'ut all'`` will cause U-Boot to run all unit tests + automatically. Note that not all unit tests will succeed in the default + configuration. +* Passing ``-t cooked`` to U-Boot will keep the console in a sane state if you + terminate it early (instead of having to run tset). + +Future work +^^^ + +The biggest limitation to the current approach is that supressions don't +"un-taint" uninitialized memory accesses. Currently, dlmalloc's bookkeeping +information is marked as a "red zone." This means that all reads to that zone +are marked as illegal by valgrind. This is fine for regular code, but dlmalloc +really does need to access this area, so we suppress its violations. However, if +dlmalloc then passes a result calculated from a "tainted" access, that result is +still tainted. So the first accessor will raise a warning. This means that every +construct like + +.. code-block:: + +foo = malloc(sizeof(*foo)); +if (!foo) +return -ENOMEM; + +will raise a warning when we check the result of malloc. Whoops. + +There are at least four possible ways to address this: + +* Don't mark dlmalloc bookkeeping information as a red zone. This is the + simplest solution, but reduces the power of valgrind immensely, since we can + no longer determine that (e.g.) access past the end of an array is undefined. +* Implement red zones properly. This would involve growing every allocation by a + fixed amount (16 bytes or so) and then using that extra space for a real red + zone that neither regular code n
[PATCH v3 2/2] valgrind: Disable on Risc-V
There are no defined instruction sequences in include/valgrind.h for Risc-V, so CONFIG_VALGRIND will do nothing on this arch (and possibly won't compile?). Update Kconfig accordingly. Signed-off-by: Sean Anderson --- (no changes since v2) Changes in v2: - New Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/Kconfig b/Kconfig index bdae59e06f..a83b534b34 100644 --- a/Kconfig +++ b/Kconfig @@ -307,6 +307,7 @@ config TPL_SYS_MALLOC_F_LEN config VALGRIND bool "Inform valgrind about memory allocations" + depends on !RISCV help Valgrind is an instrumentation framework for building dynamic analysis tools. In particular, it may be used to detect memory management bugs -- 2.35.1
[PATCH v3 1/2] doc: sandbox: Add additional valgrind documentation
This documents some additional options which can be used with valgrind, as well as directions for future work. It also fixes up inline literals to actually be inline literals (and not italics). The content of this documentation is primarily adapted from [1]. [1] https://lore.kernel.org/u-boot/57cb4b49-fa30-1194-9ac3-faa53e803...@gmail.com/ Signed-off-by: Sean Anderson --- Changes in v3: - Fix some spelling errors - Reword ut all docs for clarity Changes in v2: - Reworked most documentation to address Heinrich's feedback doc/arch/sandbox.rst | 90 1 file changed, 82 insertions(+), 8 deletions(-) diff --git a/doc/arch/sandbox.rst b/doc/arch/sandbox.rst index e1119492b4..79bcef4f5e 100644 --- a/doc/arch/sandbox.rst +++ b/doc/arch/sandbox.rst @@ -479,18 +479,92 @@ It is possible to run U-Boot under valgrind to check memory allocations:: valgrind ./u-boot -For more detailed results, enable `CONFIG_VALGRIND`. There are many false -positives due to `malloc` itself. Suppress these with:: +However, this does not give very useful results. The sandbox allocates a memory +pool via mmap(). U-Boot's internal malloc() and free() work on this memory pool. +Custom allocators and deallocators are invisible to valgrind by default. To +expose U-Boot's malloc() and free() to valgrind, enable ``CONFIG_VALGRIND``. +Enabling this option will inject placeholder assembler code which valgrind +interprets. This is used to annotate sections of memory as safe or unsafe, and +to inform valgrind about malloc()s and free()s. There are currently no standard +placeholder assembly sequences for RISC-V, so this option cannot be enabled on +that architecture. + +Malloc's bookkeeping information is marked as unsafe by default. However, this +will generate many false positives when malloc itself accesses this information. +These warnings can be suppressed with:: valgrind --suppressions=scripts/u-boot.supp ./u-boot -If you are running sandbox SPL or TPL, then valgrind will not by default -notice when U-Boot jumps from TPL to SPL, or from SPL to U-Boot proper. To -fix this, use `--trace-children=yes`. To show who alloc'd some troublesome -memory, use `--track-origins=yes`. To uncover possible errors, try running all -unit tests with:: +Additionally, you may experience false positives if U-Boot is using a smaller +pointer size than your host architecture. This is because the pointers used by +U-Boot will only contain 32 bits of addressing information. When interpreted as +64-bit pointers, valgrind will think that they are not initialized properly. To +fix this, enable ``CONFIG_SANDBOX64`` (such as via ``sandbox64_defconfig``) +when running on a 64-bit host. -valgrind --track-origins=yes --suppressions=scripts/u-boot.supp ./u-boot -Tc 'ut all' +Additional options +^^ + +The following options are useful in addition to the above examples: + +* ``--trace-childen=yes`` will tell valgrind to keep tracking subprocesses, such + as when U-Boot jumps from TPL to SPL, or from SPL to U-Boot proper. +* ``--track-origins=yes`` will (for a small overhead) tell valgrind to keep + track of who allocated some troublesome memory. +* ``--error-limit=no`` will enable printing more than 1000 errors in a + single session. +* ``--vgdb=yes --vgdb-error=0`` will let you use gdb to attach like:: + +gdb -ex "target remote | vgdb" u-boot + + This is very helpful for inspecting the program state when there is + an error. +* Passing ``-Tc 'ut all'`` to U-Boot will run unit tests automatically. Note + that not all unit tests will succeed in the default configuration. +* Passing ``-t cooked`` to U-Boot will keep the console in a sane state if you + terminate it early (instead of having to run tset). + +Future work +^^^ + +The biggest limitation to the current approach is that supressions don't +"un-taint" uninitialized memory accesses. Currently, dlmalloc's bookkeeping +information is marked as a "red zone." This means that all reads to that zone +are marked as illegal by valgrind. This is fine for regular code, but dlmalloc +really does need to access this area, so we suppress its violations. However, if +dlmalloc then passes a result calculated from a "tainted" access, that result is +still tainted. So the first accessor will raise a warning. This means that every +construct like + +.. code-block:: + +foo = malloc(sizeof(*foo)); +if (!foo) +return -ENOMEM; + +will raise a warning when we check the result of malloc. Whoops. + +There are at least four possible ways to address this: + +* Don't mark dlmalloc bookkeeping information as a red zone. This is the + simplest solution, but reduces the power of valgrind immensely, since we can + no longer determine that (e.g.) access past the end of an array is undefined. +* Implement red zones properly. This would involve growing every allocation by a + fixed amount (16 bytes or so) and then using tha
Re: [PATCH v6] pinctrl: nuvoton: Add NPCM8xx pinctrl driver
Hi Sean, Thanks for the review, it will be revised in the next version. -- Stanley On Fri, May 27, 2022 at 12:06 PM Sean Anderson wrote: > > On 5/3/22 1:33 AM, Stanley Chu wrote: > > Add Nuvoton BMC NPCM845 Pinmux and Pinconf support. > > > > Signed-off-by: Stanley Chu > > --- > > v6: > > - sync pin name with Linux driver > > - add support for gpi35/gpi36/gpio183~189 > > v5: > > - lower-case hex consistently > > - use uint type for pin list in the group_config struct > > v4: > > - correct the pin flags, add slew rate control suuport for rgmii pins > > v3: > > - separate group names and function names in different tables > > to allow for adding additional functions > > v2: > > - drop the WDnRCRB/CORSTCB register access, it is not for > > GPIO modules reset control > > --- > > drivers/pinctrl/Kconfig |1 + > > drivers/pinctrl/Makefile |1 + > > drivers/pinctrl/nuvoton/Kconfig | 12 + > > drivers/pinctrl/nuvoton/Makefile |1 + > > drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c | 1225 + > > 5 files changed, 1240 insertions(+) > > create mode 100644 drivers/pinctrl/nuvoton/Kconfig > > create mode 100644 drivers/pinctrl/nuvoton/Makefile > > create mode 100644 drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c > > > > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig > > index 13033198f9..e14e885c3e 100644 > > --- a/drivers/pinctrl/Kconfig > > +++ b/drivers/pinctrl/Kconfig > > @@ -339,6 +339,7 @@ source "drivers/pinctrl/mscc/Kconfig" > > source "drivers/pinctrl/mtmips/Kconfig" > > source "drivers/pinctrl/mvebu/Kconfig" > > source "drivers/pinctrl/nexell/Kconfig" > > +source "drivers/pinctrl/nuvoton/Kconfig" > > source "drivers/pinctrl/nxp/Kconfig" > > source "drivers/pinctrl/renesas/Kconfig" > > source "drivers/pinctrl/rockchip/Kconfig" > > diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile > > index 9b4978253b..aa31f31c16 100644 > > --- a/drivers/pinctrl/Makefile > > +++ b/drivers/pinctrl/Makefile > > @@ -12,6 +12,7 @@ obj-$(CONFIG_ARCH_ASPEED) += aspeed/ > > obj-$(CONFIG_ARCH_ATH79) += ath79/ > > obj-$(CONFIG_PINCTRL_INTEL) += intel/ > > obj-$(CONFIG_ARCH_MTMIPS) += mtmips/ > > +obj-$(CONFIG_ARCH_NPCM) += nuvoton/ > > obj-$(CONFIG_ARCH_RMOBILE) += renesas/ > > obj-$(CONFIG_PINCTRL_SANDBOX) += pinctrl-sandbox.o > > obj-$(CONFIG_PINCTRL_SUNXI) += sunxi/ > > diff --git a/drivers/pinctrl/nuvoton/Kconfig > > b/drivers/pinctrl/nuvoton/Kconfig > > new file mode 100644 > > index 00..519539d6ae > > --- /dev/null > > +++ b/drivers/pinctrl/nuvoton/Kconfig > > @@ -0,0 +1,12 @@ > > +config PINCTRL_NPCM8XX > > + bool "Pinctrl driver for Nuvoton NPCM8XX" > > + depends on DM && PINCTRL_GENERIC && ARCH_NPCM8XX > > + help > > + Support pin muxing and pin configuration on > > + Nuvoton NPCM8XX SoC. > > + > > + The NPCM8XX contains 256 GPIO pins. Most of them are > > + multiplexed with other system functions. These pins can > > + be configured as either GPIO pin or alternate function. > > + It also supports basic configurations such as pull up/down, > > + drive-strength, and slew rate control for some of the pins. > > diff --git a/drivers/pinctrl/nuvoton/Makefile > > b/drivers/pinctrl/nuvoton/Makefile > > new file mode 100644 > > index 00..a6dfdf3672 > > --- /dev/null > > +++ b/drivers/pinctrl/nuvoton/Makefile > > @@ -0,0 +1 @@ > > +obj-$(CONFIG_PINCTRL_NPCM8XX)+= pinctrl-npcm8xx.o > > diff --git a/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c > > b/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c > > new file mode 100644 > > index 00..cc49310506 > > --- /dev/null > > +++ b/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c > > @@ -0,0 +1,1225 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (c) 2022 Nuvoton Technology Corp. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +/* GCR register offsets */ > > +#define WD0RCR 0x38 > > +#define WD1RCR 0x3c > > +#define WD2RCR 0x40 > > +#define SWRSTC1 0x44 > > +#define SWRSTC2 0x48 > > +#define SWRSTC3 0x4c > > +#define SWRSTC4 0x50 > > +#define CORSTC 0x5c > > +#define FLOCKR1 0x74 > > +#define INTCR4 0xc0 > > +#define I2CSEGSEL0xe0 > > +#define MFSEL1 0x260 > > +#define MFSEL2 0x264 > > +#define MFSEL3 0x268 > > +#define MFSEL4 0x26c > > +#define MFSEL5 0x270 > > +#define MFSEL6 0x274 > > +#define MFSEL7 0x278 > > + > > +/* GPIO register offsets */ > > +#define GPIO_POL 0x08 /* Polarity */ > > +#define GPIO_DOUT0x0c /* Data OUT */ > > +#define GPIO_OTYP0x14 /* Output Type */
[PATCH] boot: pxe_utils: Do not use fdtcontroladdr for FIT Image format
When patch [1] introduced "fdtcontroladdr" fallback in order to use
built-in DT when no DT in provided in PXE config, it broke FIT image
boot where the DT from the FIT configuration was used.
This disables the "fdtcontroladdr" fallback when the provided
image is from the FIT image type.
[1] d5ba6188dfbf ("cmd: pxe_utils: Check fdtcontroladdr in label_boot")
Fixes: d5ba6188dfbf ("cmd: pxe_utils: Check fdtcontroladdr in label_boot")
Cc: Tom Rini
Cc: Peter Hoyes
Signed-off-by: Neil Armstrong
---
boot/pxe_utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index b08aee9896..5eb328c86b 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -725,7 +725,7 @@ static int label_boot(struct pxe_context *ctx, struct
pxe_label *label)
if (!bootm_argv[3])
bootm_argv[3] = env_get("fdt_addr");
- if (!bootm_argv[3])
+ if (genimg_get_format(buf) != IMAGE_FORMAT_FIT && !bootm_argv[3])
bootm_argv[3] = env_get("fdtcontroladdr");
if (bootm_argv[3]) {
--
2.25.1
Re: [PATCH v2 01/11] arm: dts: Add Mercury+ AA1 devicetrees
On Thu, 26 May 2022 at 07:37, Paweł Anikiel wrote: > > Devicetree headers for Mercury+ AA1 module > > Signed-off-by: Paweł Anikiel > --- > .../socfpga_arria10_mercury_aa1-u-boot.dtsi | 54 ++ > arch/arm/dts/socfpga_arria10_mercury_aa1.dtsi | 72 +++ > 2 files changed, 126 insertions(+) > create mode 100644 arch/arm/dts/socfpga_arria10_mercury_aa1-u-boot.dtsi > create mode 100644 arch/arm/dts/socfpga_arria10_mercury_aa1.dtsi Reviewed-by: Simon Glass
Re: [PATCH v2 02/11] arm: dts: Add Chameleonv3 handoff headers
On Thu, 26 May 2022 at 07:37, Paweł Anikiel wrote: > > Add handoff headers for the Google Chameleonv3 variants: 480-2 and > 270-3. Both files were generated using qts-filter-a10.sh. > > Signed-off-by: Paweł Anikiel > --- > ...ocfpga_arria10_chameleonv3_270_3_handoff.h | 305 ++ > ...ocfpga_arria10_chameleonv3_480_2_handoff.h | 305 ++ > 2 files changed, 610 insertions(+) > create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_3_handoff.h > create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_480_2_handoff.h > Reviewed-by: Simon Glass
Re: [PATCH v2 04/11] board: Add Chameleonv3 board dir
On Thu, 26 May 2022 at 07:37, Paweł Anikiel wrote: > > Add board directory for Google Chameleon V3 board > > Signed-off-by: Paweł Anikiel > --- > board/google/chameleonv3/Makefile | 5 +++ > board/google/chameleonv3/board.c | 27 ++ > board/google/chameleonv3/fpga.its | 28 ++ > board/google/chameleonv3/fpga_early_io.its | 35 ++ > board/google/chameleonv3/mercury_aa1.c | 43 ++ > board/google/chameleonv3/mercury_aa1.h | 12 ++ > 6 files changed, 150 insertions(+) > create mode 100644 board/google/chameleonv3/Makefile > create mode 100644 board/google/chameleonv3/board.c > create mode 100644 board/google/chameleonv3/fpga.its > create mode 100644 board/google/chameleonv3/fpga_early_io.its > create mode 100644 board/google/chameleonv3/mercury_aa1.c > create mode 100644 board/google/chameleonv3/mercury_aa1.h Reviewed-by: Simon Glass
Re: [PATCH v2 03/11] arm: dts: Add Chameleonv3 devicetrees
On Thu, 26 May 2022 at 07:37, Paweł Anikiel wrote: > > Add devicetrees for Google Chameleon V3 board > > Signed-off-by: Paweł Anikiel > Signed-off-by: Alexandru M Stan > --- > arch/arm/dts/Makefile | 2 + > arch/arm/dts/socfpga_arria10_chameleonv3.dts | 90 +++ > ...fpga_arria10_chameleonv3_270_3-u-boot.dtsi | 8 ++ > .../dts/socfpga_arria10_chameleonv3_270_3.dts | 5 ++ > ...fpga_arria10_chameleonv3_480_2-u-boot.dtsi | 8 ++ > .../dts/socfpga_arria10_chameleonv3_480_2.dts | 5 ++ > 6 files changed, 118 insertions(+) > create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3.dts > create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi > create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_3.dts > create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi > create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_480_2.dts > Reviewed-by: Simon Glass
Re: [PATCH v2 06/11] misc: atsha204a: Increase wake delay by tWHI
On Thu, 26 May 2022 at 07:38, Paweł Anikiel wrote: > > From the ATSHA204A datasheet (document DS40002025A): > > Wake: If SDA is held low for a period greater than tWLO, the device > exits low-power mode and, after a delay of tWHI, is ready to receive > I2C commands. > > tWHI value can be found in table 7-2. > > Signed-off-by: Paweł Anikiel > --- > drivers/misc/atsha204a-i2c.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 05/11] config: Add Chameleonv3 config
On Thu, 26 May 2022 at 07:38, Paweł Anikiel wrote: > > Add defconfig and Kconfig files for Google Chameleon V3 board > > Signed-off-by: Paweł Anikiel > --- > arch/arm/mach-socfpga/Kconfig | 7 + > configs/socfpga_chameleonv3_defconfig | 29 ++ > include/configs/socfpga_chameleonv3.h | 44 +++ > 3 files changed, 80 insertions(+) > create mode 100644 configs/socfpga_chameleonv3_defconfig > create mode 100644 include/configs/socfpga_chameleonv3.h > Reviewed-by: Simon Glass
Re: [PATCH v2 07/11] sysreset: socfpga: Use parent device for reading base address
On Thu, 26 May 2022 at 07:38, Paweł Anikiel wrote: > > This driver is a child of the rstmgr driver, both of which share the > same devicetree node. As a result, passing the child's udevice pointer > to dev_read_addr_ptr results in a failure of reading the #address-cells > property. Use the parent udevice pointer instead. > > Signed-off-by: Paweł Anikiel > --- > drivers/sysreset/sysreset_socfpga.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 09/11] socfpga: arria10: Improve bitstream loading speed
On Thu, 26 May 2022 at 07:38, Paweł Anikiel wrote: > > Apply some optimizations to speed up bitstream loading > (both for full and split periph/core bitstreams): > > * Change the size of the first fs read, so that all the subsequent >reads are aligned to a specific value (called MAX_FIRST_LOAD_SIZE). >This value was chosen so that in subsequent reads the fat fs driver >doesn't have to allocate a temporary buffer in get_contents >(assuming 8KiB clusters). > > * Change the buffer size to a larger value when reading to ddr >(but not too large, because large transfers cause a stack overflow >in the dwmmc driver). When the size is too large, where exactly does that stack overflow happen? > > Signed-off-by: Paweł Anikiel > --- > drivers/fpga/socfpga_arria10.c | 20 ++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 10/11] socfpga: arria10: Wait for fifo empty after writing bitstream
On Thu, 26 May 2022 at 07:38, Paweł Anikiel wrote: > > For some reason, on the Mercury+ AA1 module, calling > fpgamgr_wait_early_user_mode immediately after writing the peripheral > bitstream leaves the fpga in a broken state (ddr calibration hangs). > Adding a delay before the first sync word is written seems to fix this. > Inspecting the fpgamgr registers before and after the delay, > imgcfg_FifoEmpty is the only bit that changes. Waiting for this bit > (instead of a hardcoded delay) also fixes the issue. > > Signed-off-by: Paweł Anikiel > --- > drivers/fpga/socfpga_arria10.c | 8 > 1 file changed, 8 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH v2 08/11] socfpga: arria10: Replace delays with busy waiting in cm_full_cfg
On Thu, 26 May 2022 at 07:38, Paweł Anikiel wrote: > > Using udelay while the clocks aren't fully configured causes the timer > system to save the wrong clock rate. Use sdelay and wait_on_value > instead (the values used in these functions were found experimentally). > > Signed-off-by: Paweł Anikiel > --- > arch/arm/mach-socfpga/clock_manager.c | 7 --- > arch/arm/mach-socfpga/clock_manager_arria10.c | 12 ++-- > arch/arm/mach-socfpga/include/mach/clock_manager.h | 4 > 3 files changed, 14 insertions(+), 9 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 11/11] socfpga: arria10: Allow dcache_enable before relocation
On Thu, 26 May 2022 at 07:38, Paweł Anikiel wrote:
>
> Before relocating to SDRAM, the ECC is initialized by clearing the
> whole SDRAM. In order to speed this up, dcache_enable is used (see
> sdram_init_ecc_bits).
>
> Since commit 503eea451903 ("arm: cp15: update DACR value to activate
> access control"), this no longer works, because running code in OCRAM
> with the XN bit set causes a page fault. Override dram_bank_mmu_setup
> to disable XN in the OCRAM and setup DRAM dcache before relocation.
>
> Signed-off-by: Paweł Anikiel
> ---
> arch/arm/mach-socfpga/misc_arria10.c | 26 ++
> 1 file changed, 26 insertions(+)
>
Reviewed-by: Simon Glass
Re: [PATCH V5 1/9] arm: dts: imx8m: update binman ddr firmware node name
On 24/05/2022 11:00, Peng Fan (OSS) wrote:
> From: Peng Fan
>
> We are migrating to use BINMAN SYMBOLS, the current name is not
> a valid binman type, so update to unify them.
'not a valid binman type, so' here is misleading. Names do not need to
be binman types if you have the 'type' property. 'the current names are
inconsistent across different boards, so' would be better.
> Also add `type = "blob-ext";` for generating a valid binman symbol
The 'type' doesn't matter for symbol generation. '... since the new
names are not valid binman types' is the real reason.
> Tested-by: Tim Harvey #imx8m[m,n,p]-venice
> Signed-off-by: Peng Fan
> ---
> arch/arm/dts/imx8mm-u-boot.dtsi | 8
> arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi| 12
> arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi | 4 ++--
> arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi | 12
> arch/arm/dts/imx8mn-evk-u-boot.dtsi | 12
> arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi | 8
> arch/arm/dts/imx8mn-venice-u-boot.dtsi| 8
> arch/arm/dts/imx8mp-u-boot.dtsi | 12
> arch/arm/dts/imx8mq-cm-u-boot.dtsi| 12
> arch/arm/dts/imx8mq-u-boot.dtsi | 8
> 10 files changed, 58 insertions(+), 38 deletions(-)
>
> diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi b/arch/arm/dts/imx8mm-u-boot.dtsi
> index 9f66cdb65a9..dc036894d82 100644
> --- a/arch/arm/dts/imx8mm-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mm-u-boot.dtsi
> @@ -39,25 +39,25 @@
> filename = "u-boot-spl.bin";
> };
>
> - 1d-imem {
> + imem-1d {
I suggested 'ddr-1d-imem-fw' in the diff I sent. Saying again in case
you didn't notice. But these names are fine too.
> filename = "lpddr4_pmu_train_1d_imem.bin";
> size = <0x8000>;
> type = "blob-ext";
> };
>
> [...]
Re: [PATCH V5 3/9] tools: binman: section: replace @ with -
On 24/05/2022 11:00, Peng Fan (OSS) wrote:
> From: Peng Fan
>
> In arch/arm/dts/imx8mp-u-boot.dtsi, there are blob-ext@1, blob-ext@2 and
> etc which is for packing ddr phy firmware. However we could not declare
> symbol name such as 'binman_sym_declare(ulong, blob_ext@1, image_pos)',
> because '@' is not allowed, so we choose to declare the symbol
> 'binman_sym_declare(ulong, blob_ext_1, image_pos);' with '@' replaced with
> '_'. It does not impact if there is no '@' in section name.
You don't need this patch after the entry name changes.
Either drop this patch, or handle this in the LookupSymbol() function.
> Tested-by: Tim Harvey #imx8m[m,n,p]-venice
> Reviewed-by: Tom Rini
> Signed-off-by: Peng Fan
> ---
> tools/binman/etype/section.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
> index bd67238b919..e3f362b442b 100644
> --- a/tools/binman/etype/section.py
> +++ b/tools/binman/etype/section.py
> @@ -875,7 +875,7 @@ class Entry_section(Entry):
> entries[entry.GetPath()] = entry
> for entry in to_add.values():
> self._CollectEntries(entries, entries_by_name, entry)
> -entries_by_name[add_entry.name] = add_entry
> +entries_by_name[add_entry.name.replace('@', '-')] = add_entry
>
> def MissingArgs(self, entry, missing):
> """Report a missing argument, if enabled
Re: [PATCH V5 4/9] binman: introduce no-u-boot-any property
On 24/05/2022 11:00, Peng Fan (OSS) wrote: > By default when BINMAN_SYMBOLS is enabled, common/spl/spl.c has a code > piece `binman_sym_declare(ulong, u_boot_any, image_pos);` which requires > u-boot* node in device tree binman node section. But some > platforms(i.MX8M) not need it. To avoid build break with BINMAN_SYMBOLS, > introduce a no-u-boot-any property. This is just a hacky workaround, I don't think it's OK. Anyway, turns out you don't need this patch (and 5/9) either. I tried a few things and found CONFIG_SPL_RAW_IMAGE_SUPPORT (enabled on most imx8m) and CONFIG_SPL_RAM_DEVICE (enabled on imx8mm_data_modul_edm_sbc) are using the 'u_boot_any' symbols. Disable them and the symbols get optimized away and the error disappears. I also found some relevant discussions from a while back, see [1], [2]. [1] using binman fails boot https://lore.kernel.org/u-boot/CAJ+vNU0BZDr2q0ZPQkoQBP1eBhbYmQfJMYraSgOvWXwZ=yf...@mail.gmail.com/T/#u [2] imx8: ls1028a: Drop raw image support https://lore.kernel.org/u-boot/20210801205951.2202789-1-...@chromium.org/T/#u
Re: [PATCH V5 8/9] binman_sym: guard with CONFIG_SPL_BINMAN_SYMBOLS
On 24/05/2022 11:00, Peng Fan (OSS) wrote:
> From: Peng Fan
>
> There is case that CONFIG_BINMAN is defined, but
> CONFIG_SPL_BINMAN_SYMBOLS is not defined. In that case, there will be
> build failure. So use CONFIG_SPL_BINMAN_SYMBOLS to guard the macros, and
> define CONFIG_SPL_BINMAN_SYMBOLS in binman syms test.
>
> Tested-by: Tim Harvey #imx8m[m,n,p]-venice
> Signed-off-by: Peng Fan
> ---
> include/binman_sym.h| 2 +-
> tools/binman/test/u_boot_binman_syms.c | 1 +
> tools/binman/test/u_boot_binman_syms_size.c | 1 +
> 3 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/binman_sym.h b/include/binman_sym.h
> index 72e6765fe52..548d8f5654c 100644
> --- a/include/binman_sym.h
> +++ b/include/binman_sym.h
> @@ -13,7 +13,7 @@
>
> #define BINMAN_SYM_MISSING (-1UL)
>
> -#ifdef CONFIG_BINMAN
> +#ifdef CONFIG_SPL_BINMAN_SYMBOLS
This must handle all of CONFIG_{TPL,VPL,SPL}_BINMAN_SYMBOLS.
You can use CONFIG_IS_ENABLED(BINMAN_SYMBOLS) if you:
- Create a binman/test/generated/autoconf.h with binman CONFIGs
- Add `-I $(SRC)` to CFLAGS in binman/test/Makefile
- Add `#include ` to test .c files
- Maybe add `#define CONFIG_SPL_BUILD` to test .c files
>
> /**
> * binman_symname() - Internal function to get a binman symbol name
> diff --git a/tools/binman/test/u_boot_binman_syms.c
> b/tools/binman/test/u_boot_binman_syms.c
> index 37fc339ce84..f4a4d1f6846 100644
> --- a/tools/binman/test/u_boot_binman_syms.c
> +++ b/tools/binman/test/u_boot_binman_syms.c
> @@ -6,6 +6,7 @@
> */
>
> #define CONFIG_BINMAN
> +#define CONFIG_SPL_BINMAN_SYMBOLS
> #include
>
> binman_sym_declare(unsigned long, u_boot_spl_any, offset);
> diff --git a/tools/binman/test/u_boot_binman_syms_size.c
> b/tools/binman/test/u_boot_binman_syms_size.c
> index 7224bc1863c..3a01d8ca4be 100644
> --- a/tools/binman/test/u_boot_binman_syms_size.c
> +++ b/tools/binman/test/u_boot_binman_syms_size.c
> @@ -6,6 +6,7 @@
> */
>
> #define CONFIG_BINMAN
> +#define CONFIG_SPL_BINMAN_SYMBOLS
> #include
>
> binman_sym_declare(char, u_boot_spl, pos);
[PATCH v2] board: gateworks: venice: add GW7903 PMIC
The GW7903 has a BD71847 PMIC on I2C1. Adjust the model compare strings
to add it.
Signed-off-by: Tim Harvey
Reviewed-by: Fabio Estevam
---
v2: fixed typo in commit log and added Fabio's rb tag
---
board/gateworks/venice/spl.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/board/gateworks/venice/spl.c b/board/gateworks/venice/spl.c
index 7e3e91e1d9cc..ae460296cb99 100644
--- a/board/gateworks/venice/spl.c
+++ b/board/gateworks/venice/spl.c
@@ -175,11 +175,12 @@ static int power_init_board(void)
}
else if ((!strncmp(model, "GW7901", 6)) ||
-(!strncmp(model, "GW7902", 6))) {
- if (!strncmp(model, "GW7901", 6))
- ret = uclass_get_device_by_seq(UCLASS_I2C, 1, &bus);
- else
+(!strncmp(model, "GW7902", 6)) ||
+(!strncmp(model, "GW7903", 6))) {
+ if (!strncmp(model, "GW7902", 6))
ret = uclass_get_device_by_seq(UCLASS_I2C, 0, &bus);
+ else
+ ret = uclass_get_device_by_seq(UCLASS_I2C, 1, &bus);
if (ret) {
printf("PMIC: failed I2C2 probe: %d\n", ret);
return ret;
--
2.17.1
Re: [u-boot PATCH 3/3] k3-am642-evm-u-boot: Use binman to generate u-boot.img and tispl.bin
On 26/05/2022 17:15, Tom Rini wrote:
> On Thu, May 26, 2022 at 10:28:45AM +0300, Roger Quadros wrote:
>> Any thoughts on how to get the new ti-secure etype work with atf-bl31 and
>> tee-os etypes so that it can take the data output of those entries and create
>> a signed binary with filenames from those entries or atf-bl31-path and
>> tee-os-path?
>>
>> Can something like this work?
>>
>> ti-secure {
>> atf-bl31 {
>> filename = "bl31.bin";
>> };
>> }
>>
>> We could probably get rid of filename property from ti-secure etype and use
>> blob for regular files.
>>
>> ti-secure {
>> blob {
>> filename = "somefile.ext";
>> }
>> }
This would definitely work, see etype/mkimage.py for example. I'd prefer
to know the file-format details (and maybe replicate them in binman) if
you could afford to publish them, though...
Sorry I couldn't look at either series yet, but I see mentions of
k3_fit_atf.sh, so let me point out another series [1][2] that might also
interest you:
[1] [RESEND, RFC 0/8] Integration of sysfw and tispl with U-Boot
https://lore.kernel.org/u-boot/20220406122919.6104-1-n-fran...@ti.com/
[2] [PATCH RFC v2 00/11] Integration of sysfw, tispl and tiboot3
https://lore.kernel.org/u-boot/20220506043759.8193-1-n-fran...@ti.com/
>
> Adding in Alper as well..
>
[PATCH v2 0/2] Rockchip: Add option to prevent boot on plug-in
From: Chris Morgan Sometimes it is desirable to prevent a board from automatically booting as soon as the power cable is plugged in. For boards with an rk8xx PMIC, (excluding the rk808) we can actually query the power up source. Changes from V1: - Moved a comment as requested to be above a function. - Removed unneeded variable of "plug_in". Chris Morgan (2): power: pmic: rk8xx: Support sysreset shutdown method rockchip: Add option to prevent booting on power plug-in arch/arm/mach-rockchip/Kconfig | 10 drivers/power/pmic/rk8xx.c | 84 +- include/power/rk8xx_pmic.h | 3 ++ 3 files changed, 96 insertions(+), 1 deletion(-) -- 2.25.1
[PATCH v2 1/2] power: pmic: rk8xx: Support sysreset shutdown method
From: Chris Morgan
Add support for sysreset shutdown for this PMIC. The values were pulled
from the various datasheets, but for now it has only been tested on
the rk817 (for an Odroid Go Advance).
Signed-off-by: Chris Morgan
Reviewed-by: Jaehoon Chung
Reviewed-by: Kever Yang
---
drivers/power/pmic/rk8xx.c | 50 +-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
index 5f442fea68..1ffbecc02a 100644
--- a/drivers/power/pmic/rk8xx.c
+++ b/drivers/power/pmic/rk8xx.c
@@ -6,10 +6,50 @@
#include
#include
+#include
#include
#include
#include
#include
+#include
+
+static int rk8xx_sysreset_request(struct udevice *dev, enum sysreset_t type)
+{
+ struct rk8xx_priv *priv = dev_get_priv(dev->parent);
+
+ if (type != SYSRESET_POWER_OFF)
+ return -EPROTONOSUPPORT;
+
+ switch (priv->variant) {
+ case RK805_ID:
+ case RK808_ID:
+ case RK816_ID:
+ case RK818_ID:
+ pmic_clrsetbits(dev->parent, REG_DEVCTRL, 0, BIT(0));
+ break;
+ case RK809_ID:
+ case RK817_ID:
+ pmic_clrsetbits(dev->parent, RK817_REG_SYS_CFG3, 0,
+ BIT(0));
+ break;
+ default:
+ printf("Unknown PMIC RK%x: Cannot shutdown\n",
+ priv->variant);
+ return -EPROTONOSUPPORT;
+ };
+
+ return -EINPROGRESS;
+}
+
+static struct sysreset_ops rk8xx_sysreset_ops = {
+ .request= rk8xx_sysreset_request,
+};
+
+U_BOOT_DRIVER(rk8xx_sysreset) = {
+ .name = "rk8xx_sysreset",
+ .id = UCLASS_SYSRESET,
+ .ops= &rk8xx_sysreset_ops,
+};
static struct reg_data rk817_init_reg[] = {
/* enable the under-voltage protection,
@@ -61,7 +101,7 @@ static int rk8xx_read(struct udevice *dev, uint reg, uint8_t
*buff, int len)
static int rk8xx_bind(struct udevice *dev)
{
ofnode regulators_node;
- int children;
+ int children, ret;
regulators_node = dev_read_subnode(dev, "regulators");
if (!ofnode_valid(regulators_node)) {
@@ -72,6 +112,14 @@ static int rk8xx_bind(struct udevice *dev)
debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
+ if (CONFIG_IS_ENABLED(SYSRESET)) {
+ ret = device_bind_driver_to_node(dev, "rk8xx_sysreset",
+"rk8xx_sysreset",
+dev_ofnode(dev), NULL);
+ if (ret)
+ return ret;
+ }
+
children = pmic_bind_children(dev, regulators_node, pmic_children_info);
if (!children)
debug("%s: %s - no child found\n", __func__, dev->name);
--
2.25.1
[PATCH v2 2/2] rockchip: Add option to prevent booting on power plug-in
From: Chris Morgan
For Rockchip boards with the all rk8xx series PMICs (excluding the
rk808), it is sometimes desirable to not boot whenever the device is
plugged in. An example would be for the Odroid Go Advance.
This provides a configurable option to check the PMIC says it was
powered because of a plug-in event. If the value is 1 and this option
is selected, the device shuts down shortly after printing a message
to console stating the reason why it's shutting down. Powering up the
board with the power button is not affected.
This patch parallels the work done in the following patch series:
https://lore.kernel.org/u-boot/20220121133732.2397273-1-andre.przyw...@arm.com/
Signed-off-by: Chris Morgan
---
arch/arm/mach-rockchip/Kconfig | 10 ++
drivers/power/pmic/rk8xx.c | 34 ++
include/power/rk8xx_pmic.h | 3 +++
3 files changed, 47 insertions(+)
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index 18aff5480b..c561a77e6a 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -361,6 +361,16 @@ config ROCKCHIP_BOOT_MODE_REG
The Soc will enter to different boot mode(defined in
asm/arch-rockchip/boot_mode.h)
according to the value from this register.
+config ROCKCHIP_RK8XX_DISABLE_BOOT_ON_POWERON
+ bool "Disable device boot on power plug-in"
+ depends on PMIC_RK8XX
+ default n
+ ---help---
+ Say Y here to prevent the device from booting up because of a plug-in
+ event. When set, the device will boot briefly to determine why it was
+ powered on, and if it was determined because of a plug-in event
+ instead of a button press event it will shut back off.
+
config ROCKCHIP_STIMER
bool "Rockchip STIMER support"
default y
diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
index 1ffbecc02a..25ef621f8d 100644
--- a/drivers/power/pmic/rk8xx.c
+++ b/drivers/power/pmic/rk8xx.c
@@ -51,6 +51,38 @@ U_BOOT_DRIVER(rk8xx_sysreset) = {
.ops= &rk8xx_sysreset_ops,
};
+/* In the event of a plug-in and the appropriate option has been
+ * selected, we simply shutdown instead of continue the normal boot
+ * process. Please note the rk808 is not supported as it doesn't
+ * have the appropriate register.
+ */
+void rk8xx_off_for_plugin(struct udevice *dev)
+{
+ struct rk8xx_priv *priv = dev_get_priv(dev);
+
+ switch (priv->variant) {
+ case RK805_ID:
+ case RK816_ID:
+ case RK818_ID:
+ if (pmic_reg_read(dev, RK8XX_ON_SOURCE) & RK8XX_ON_PLUG_IN) {
+ printf("Power Off due to plug-in event\n");
+ pmic_clrsetbits(dev, REG_DEVCTRL, 0, BIT(0));
+ }
+ break;
+ case RK809_ID:
+ case RK817_ID:
+ if (pmic_reg_read(dev, RK817_ON_SOURCE) & RK8XX_ON_PLUG_IN) {
+ printf("Power Off due to plug-in event\n");
+ pmic_clrsetbits(dev, RK817_REG_SYS_CFG3, 0,
+ BIT(0));
+ }
+ break;
+ default:
+ printf("PMIC RK%x: Cannot read boot reason.\n",
+ priv->variant);
+ }
+}
+
static struct reg_data rk817_init_reg[] = {
/* enable the under-voltage protection,
* the under-voltage protection will shutdown the LDO3 and reset the PMIC
@@ -211,6 +243,8 @@ static int rk8xx_probe(struct udevice *dev)
pmic_reg_read(dev, on_source),
pmic_reg_read(dev, off_source));
printf("\n");
+ if (CONFIG_IS_ENABLED(ROCKCHIP_RK8XX_DISABLE_BOOT_ON_POWERON))
+ rk8xx_off_for_plugin(dev);
return 0;
}
diff --git a/include/power/rk8xx_pmic.h b/include/power/rk8xx_pmic.h
index 8ff0af35c5..3cbfc02195 100644
--- a/include/power/rk8xx_pmic.h
+++ b/include/power/rk8xx_pmic.h
@@ -214,6 +214,9 @@ enum {
#define RK817_ON_SOURCE0xf5
#define RK817_OFF_SOURCE 0xf6
+#define RK8XX_ON_PWRON BIT(7)
+#define RK8XX_ON_PLUG_IN BIT(6)
+
struct reg_data {
u8 reg;
u8 val;
--
2.25.1
Re: [PATCH] imx8mn_evk: Add the missing spl.bin entry
On 03/05/2022 22:03, Fabio Estevam wrote:
> From: Fabio Estevam
>
> The generated flash.bin does not boot the imx8mn evk LPDDR4 variant
> as it misses the spl.bin description in binman.
The dtsi file #includes "imx8mn-ddr4-evk-u-boot.dtsi", which provides an
'spl' description. It also provides a 'imx-boot' description for a
working 'flash.bin' file. The issue here is the 'flash' description here
is overwriting the same file with its own broken definition. You're
removing it, so things work fine.
> Add its entry to fix the boot on the imx8mn evk LPDDR4 variant.
>
> Signed-off-by: Fabio Estevam
> ---
> arch/arm/dts/imx8mn-evk-u-boot.dtsi | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Alper Nebi Yasak
> diff --git a/arch/arm/dts/imx8mn-evk-u-boot.dtsi
> b/arch/arm/dts/imx8mn-evk-u-boot.dtsi
> index 3db46d4cbc..593cf06eb9 100644
> --- a/arch/arm/dts/imx8mn-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mn-evk-u-boot.dtsi
> @@ -58,7 +58,9 @@
> };
>
>
> - flash {
> + spl {
> + filename = "spl.bin";
> +
> mkimage {
> args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e
> 0x912000";
>
You might want to copy 'imx-boot' from "imx8mn-ddr4-evk-u-boot.dtsi" for
completeness. But it doesn't really matter.
Re: [PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format
On 11/05/2022 18:35, Jerome Forissier wrote:
> This commit adds support for the OP-TEE 'tee.bin' v1 format for Rockchip
> platforms.
>
> Since OP-TEE 3.8.0, tee.bin contains meta-data in a proprietary format
> in addition to the ELF data. They are essential information for proper
> initialization of the TEE core, such as the size of the memory region
> covered by the TEE or a compact representation of runtime relocation
> data when ASLR is enabled.
>
> With OP-TEE 3.8.0 onwards, 'tee.elf' MUST NOT be used and 'tee.bin'
> MUST be used instead. Ignoring this recommendation can lead to crashes
> as described in [3].
>
> Link: [1]
> https://github.com/OP-TEE/optee_os/commit/5dd1570ac5b0f6563b1a9c074533a19107b8222d
> Link: [2]
> https://github.com/OP-TEE/optee_os/blob/3.17.0/scripts/gen_tee_bin.py#L275-L302
> Link: [3] https://github.com/OP-TEE/optee_os/issues/4542
> Signed-off-by: Jerome Forissier
> ---
> arch/arm/mach-rockchip/make_fit_atf.py | 43 +-
> 1 file changed, 35 insertions(+), 8 deletions(-)
This might make it slightly harder to convert this file to binman, since
it's not as good at deconstructing things. I can't say I understand
OP-TEE OS or its files/formats, so:
Acked-by: Alper Nebi Yasak
> diff --git a/arch/arm/mach-rockchip/make_fit_atf.py
> b/arch/arm/mach-rockchip/make_fit_atf.py
> index f3224d2555..fcea652388 100755
> --- a/arch/arm/mach-rockchip/make_fit_atf.py
> +++ b/arch/arm/mach-rockchip/make_fit_atf.py
> @@ -137,7 +137,7 @@ def generate_atf_fit_dts_bl31(fit_file, bl31_file_name,
> tee_file_name, dtbs_file
> num_segments = len(segments)
>
> if tee_file_name:
> -tee_segments = unpack_elf(tee_file_name)
> +tee_segments = unpack_tee_file(tee_file_name)
> for index, entry, paddr, data in tee_segments:
> append_tee_node(fit_file, num_segments + index + 1, paddr, entry)
> num_segments = num_segments + len(tee_segments)
> @@ -169,7 +169,7 @@ def generate_atf_binary(bl31_file_name):
>
> def generate_tee_binary(tee_file_name):
> if tee_file_name:
> -for index, entry, paddr, data in unpack_elf(tee_file_name):
> +for index, entry, paddr, data in unpack_tee_file(tee_file_name):
> file_name = 'tee_0x%08x.bin' % paddr
> with open(file_name, "wb") as atf:
> atf.write(data)
> @@ -194,6 +194,31 @@ def unpack_elf(filename):
> segments.append((index, e_entry, p_paddr, p_data))
> return segments
>
> +def unpack_tee_file(filename):
> +if filename.endswith('.elf'):
> +return unpack_elf(filename)
> +with open(filename, 'rb') as file:
> +bin = file.read()
> +segments = []
> +if bin[0:5] == b'OPTE\x01':
> +# OP-TEE v1 format (tee.bin)
> +init_sz, start_hi, start_lo, _, paged_sz = struct.unpack_from('<5I',
> + bin,
> + 0x8)
> +if paged_sz != 0:
> +raise ValueError("OP-TEE paged mode not supported")
Is this not feasible/necessary to do, or just not implemented yet?
> +e_entry = (start_hi << 32) + start_lo
> +p_addr = e_entry
> +p_data = bin[0x1c:]
> +if len(p_data) != init_sz:
> +raise ValueError("Invalid file '%s': size mismatch "
> + "(expected %d, have %d)" % (filename, init_sz,
> + len(p_data)))
> +segments.append((0, e_entry, p_addr, p_data))
> +else:
> +raise ValueError("Unknown format for TEE file '%s'" % filename)
I see an 'output_header_v2' in your link [2], what about that?
> +return segments
> +
> def main():
> uboot_elf = "./u-boot"
> fit_its = sys.stdout
> @@ -210,11 +235,13 @@ def main():
> logging.warning(' Please read Building section in
> doc/README.rockchip')
>
> if "TEE" in os.environ:
> -tee_elf = os.getenv("TEE")
> +tee_file = os.getenv("TEE")
> +elif os.path.isfile("./tee.bin"):
> +tee_file = "./tee.bin"
> elif os.path.isfile("./tee.elf"):
> -tee_elf = "./tee.elf"
> +tee_file = "./tee.elf"
> else:
> -tee_elf = ""
> +tee_file = ""
>
> opts, args = getopt.getopt(sys.argv[1:], "o:u:b:t:h")
> for opt, val in opts:
> @@ -225,16 +252,16 @@ def main():
> elif opt == "-b":
> bl31_elf = val
> elif opt == "-t":
> -tee_elf = val
> +tee_file = val
> elif opt == "-h":
> print(__doc__)
> sys.exit(2)
>
> dtbs = args
>
> -generate_atf_fit_dts(fit_its, bl31_elf, tee_elf, uboot_elf, dtbs)
> +generate_atf_fit_dts(fit_its, bl31_elf, tee_file, uboot_elf, dtbs)
> generate_atf_binary(bl31_elf)
> -generate_tee_binary(tee_elf)
> +generate_tee_binary(te
[PATCH] serial: Replace CONFIG_DEBUG_UART_BASE by CONFIG_VAL(DEBUG_UART_BASE)
CONFIG_VAL(DEBUG_UART_BASE) expands to CONFIG_DEBUG_UART_BASE or
CONFIG_SPL_DEBUG_UART_BASE or CONFIG_TPL_DEBUG_UART_BASE and allows boards
to set different values for SPL, TPL and U-Boot Proper.
For ns16550 driver this support is there since commit d293759d55cc
("serial: ns16550: Add support for SPL_DEBUG_UART_BASE").
Signed-off-by: Pali Rohár
---
CI test passed on https://github.com/u-boot/u-boot/pull/177
---
arch/arm/mach-uniphier/debug-uart/debug-uart.c | 4 ++--
arch/x86/cpu/apollolake/cpu_common.c | 2 +-
board/eets/pdu001/board.c | 2 +-
drivers/serial/altera_jtag_uart.c | 2 +-
drivers/serial/altera_uart.c | 4 ++--
drivers/serial/atmel_usart.c | 4 ++--
drivers/serial/serial_ar933x.c | 4 ++--
drivers/serial/serial_arc.c| 4 ++--
drivers/serial/serial_bcm6345.c| 4 ++--
drivers/serial/serial_linflexuart.c| 4 ++--
drivers/serial/serial_meson.c | 2 +-
drivers/serial/serial_msm_geni.c | 6 +++---
drivers/serial/serial_mt7620.c | 4 ++--
drivers/serial/serial_mtk.c| 4 ++--
drivers/serial/serial_mvebu_a3700.c| 4 ++--
drivers/serial/serial_mxc.c| 4 ++--
drivers/serial/serial_omap.c | 4 ++--
drivers/serial/serial_pic32.c | 4 ++--
drivers/serial/serial_pl01x.c | 4 ++--
drivers/serial/serial_s5p.c| 4 ++--
drivers/serial/serial_sifive.c | 4 ++--
drivers/serial/serial_stm32.c | 4 ++--
drivers/serial/serial_xuartlite.c | 4 ++--
drivers/serial/serial_zynq.c | 4 ++--
24 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/arch/arm/mach-uniphier/debug-uart/debug-uart.c
b/arch/arm/mach-uniphier/debug-uart/debug-uart.c
index d116d46812d5..1ba012ca45d6 100644
--- a/arch/arm/mach-uniphier/debug-uart/debug-uart.c
+++ b/arch/arm/mach-uniphier/debug-uart/debug-uart.c
@@ -18,7 +18,7 @@
static void _debug_uart_putc(int c)
{
- void __iomem *base = (void __iomem *)CONFIG_DEBUG_UART_BASE;
+ void __iomem *base = (void __iomem *)CONFIG_VAL(DEBUG_UART_BASE);
while (!(readl(base + UNIPHIER_UART_LSR) & UART_LSR_THRE))
;
@@ -57,7 +57,7 @@ void sg_set_iectrl(unsigned int pin)
void _debug_uart_init(void)
{
#ifdef CONFIG_SPL_BUILD
- void __iomem *base = (void __iomem *)CONFIG_DEBUG_UART_BASE;
+ void __iomem *base = (void __iomem *)CONFIG_VAL(DEBUG_UART_BASE);
unsigned int divisor;
switch (uniphier_get_soc_id()) {
diff --git a/arch/x86/cpu/apollolake/cpu_common.c
b/arch/x86/cpu/apollolake/cpu_common.c
index 5d7d26b140fc..9a5502617bf5 100644
--- a/arch/x86/cpu/apollolake/cpu_common.c
+++ b/arch/x86/cpu/apollolake/cpu_common.c
@@ -72,7 +72,7 @@ static void pch_uart_init(void)
}
#ifdef CONFIG_DEBUG_UART
- apl_uart_init(PCH_DEV_UART, CONFIG_DEBUG_UART_BASE);
+ apl_uart_init(PCH_DEV_UART, CONFIG_VAL(DEBUG_UART_BASE));
#endif
}
diff --git a/board/eets/pdu001/board.c b/board/eets/pdu001/board.c
index 2b483dab8e12..1054837d434d 100644
--- a/board/eets/pdu001/board.c
+++ b/board/eets/pdu001/board.c
@@ -273,7 +273,7 @@ void board_debug_uart_init(void)
setup_early_clocks();
/* done by pin controller driver if not debugging */
- enable_uart_pin_mux(CONFIG_DEBUG_UART_BASE);
+ enable_uart_pin_mux(CONFIG_VAL(DEBUG_UART_BASE));
}
#endif
diff --git a/drivers/serial/altera_jtag_uart.c
b/drivers/serial/altera_jtag_uart.c
index 4435fcf56b9a..9e39da7dd246 100644
--- a/drivers/serial/altera_jtag_uart.c
+++ b/drivers/serial/altera_jtag_uart.c
@@ -134,7 +134,7 @@ static inline void _debug_uart_init(void)
static inline void _debug_uart_putc(int ch)
{
- struct altera_jtaguart_regs *regs = (void *)CONFIG_DEBUG_UART_BASE;
+ struct altera_jtaguart_regs *regs = (void *)CONFIG_VAL(DEBUG_UART_BASE);
while (1) {
u32 st = readl(®s->control);
diff --git a/drivers/serial/altera_uart.c b/drivers/serial/altera_uart.c
index b18be6e24549..35920480841a 100644
--- a/drivers/serial/altera_uart.c
+++ b/drivers/serial/altera_uart.c
@@ -123,7 +123,7 @@ U_BOOT_DRIVER(altera_uart) = {
static inline void _debug_uart_init(void)
{
- struct altera_uart_regs *regs = (void *)CONFIG_DEBUG_UART_BASE;
+ struct altera_uart_regs *regs = (void *)CONFIG_VAL(DEBUG_UART_BASE);
u32 div;
div = (CONFIG_DEBUG_UART_CLOCK / CONFIG_BAUDRATE) - 1;
@@ -132,7 +132,7 @@ static inline void _debug_uart_init(void)
static inline void _debug_uart_putc(int ch)
{
- struct altera_uart_regs *regs = (void *)CONFIG_DEBUG_UART_BASE;
+ struct altera_uart_regs *regs = (void *)CONFIG_VAL(DEBUG_UART_BASE);
while (1) {
u32 st = readl(®s->status);
diff --git a/drivers/seria
Re: [PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format
On 5/27/22 21:24, Alper Nebi Yasak wrote:
> On 11/05/2022 18:35, Jerome Forissier wrote:
>> This commit adds support for the OP-TEE 'tee.bin' v1 format for Rockchip
>> platforms.
>>
>> Since OP-TEE 3.8.0, tee.bin contains meta-data in a proprietary format
>> in addition to the ELF data. They are essential information for proper
>> initialization of the TEE core, such as the size of the memory region
>> covered by the TEE or a compact representation of runtime relocation
>> data when ASLR is enabled.
>>
>> With OP-TEE 3.8.0 onwards, 'tee.elf' MUST NOT be used and 'tee.bin'
>> MUST be used instead. Ignoring this recommendation can lead to crashes
>> as described in [3].
>>
>> Link: [1]
>> https://github.com/OP-TEE/optee_os/commit/5dd1570ac5b0f6563b1a9c074533a19107b8222d
>> Link: [2]
>> https://github.com/OP-TEE/optee_os/blob/3.17.0/scripts/gen_tee_bin.py#L275-L302
>> Link: [3] https://github.com/OP-TEE/optee_os/issues/4542
>> Signed-off-by: Jerome Forissier
>> ---
>> arch/arm/mach-rockchip/make_fit_atf.py | 43 +-
>> 1 file changed, 35 insertions(+), 8 deletions(-)
>
> This might make it slightly harder to convert this file to binman, since
> it's not as good at deconstructing things.
Yes it is a bit unfortunate, but nothing we can't fix I'm sure. Well, I
say this knowing absolutely nothing about binman... ;-)
> I can't say I understand
> OP-TEE OS or its files/formats, so:
>
> Acked-by: Alper Nebi Yasak
Thanks, answers to your questions below.
>> diff --git a/arch/arm/mach-rockchip/make_fit_atf.py
>> b/arch/arm/mach-rockchip/make_fit_atf.py
>> index f3224d2555..fcea652388 100755
>> --- a/arch/arm/mach-rockchip/make_fit_atf.py
>> +++ b/arch/arm/mach-rockchip/make_fit_atf.py
>> @@ -137,7 +137,7 @@ def generate_atf_fit_dts_bl31(fit_file, bl31_file_name,
>> tee_file_name, dtbs_file
>> num_segments = len(segments)
>>
>> if tee_file_name:
>> -tee_segments = unpack_elf(tee_file_name)
>> +tee_segments = unpack_tee_file(tee_file_name)
>> for index, entry, paddr, data in tee_segments:
>> append_tee_node(fit_file, num_segments + index + 1, paddr,
>> entry)
>> num_segments = num_segments + len(tee_segments)
>> @@ -169,7 +169,7 @@ def generate_atf_binary(bl31_file_name):
>>
>> def generate_tee_binary(tee_file_name):
>> if tee_file_name:
>> -for index, entry, paddr, data in unpack_elf(tee_file_name):
>> +for index, entry, paddr, data in unpack_tee_file(tee_file_name):
>> file_name = 'tee_0x%08x.bin' % paddr
>> with open(file_name, "wb") as atf:
>> atf.write(data)
>> @@ -194,6 +194,31 @@ def unpack_elf(filename):
>> segments.append((index, e_entry, p_paddr, p_data))
>> return segments
>>
>> +def unpack_tee_file(filename):
>> +if filename.endswith('.elf'):
>> +return unpack_elf(filename)
>> +with open(filename, 'rb') as file:
>> +bin = file.read()
>> +segments = []
>> +if bin[0:5] == b'OPTE\x01':
>> +# OP-TEE v1 format (tee.bin)
>> +init_sz, start_hi, start_lo, _, paged_sz = struct.unpack_from('<5I',
>> + bin,
>> + 0x8)
>> +if paged_sz != 0:
>> +raise ValueError("OP-TEE paged mode not supported")
>
> Is this not feasible/necessary to do, or just not implemented yet?
Just not implemented yet. Pager support is not strictly needed, its use or not
depends on the platform and on the threat model. In other words, whether or not
it is OK to have the TEE and TAs run in DRAM, usually isolated only from Normal
World software by TrustZone or some kind of memory firewall. Pager allows to
protect from physical access to the DRAM too. It provides authentication and/or
encryption to anything stored outside the internal SRAM of the SoC.
Testing this mode on RockPi4 would require some non trivial work. Here I simply
focused on implementing the current use case properly.
>> +e_entry = (start_hi << 32) + start_lo
>> +p_addr = e_entry
>> +p_data = bin[0x1c:]
>> +if len(p_data) != init_sz:
>> +raise ValueError("Invalid file '%s': size mismatch "
>> + "(expected %d, have %d)" % (filename, init_sz,
>> + len(p_data)))
>> +segments.append((0, e_entry, p_addr, p_data))
>> +else:
>> +raise ValueError("Unknown format for TEE file '%s'" % filename)
>
> I see an 'output_header_v2' in your link [2], what about that?
v2 is useful only when the OP-TEE pager is used, in which case it is a matter
of preference whether to use a single binary and have the loader split it as
expected (tee.bin) or use separate binaries instead.
Historically (Jens please correct me if I'm wrong!), there was a single raw
binary for OP-T
BUG in u-boot for ODROID-XU3/XU4/HC1/HC2
hi, i've recently tried u-boot 2022.04 (u-boot-v2022.04.tar.bz2) and my odroid-hc2 didn't boot with uefi (default since Fedora 35) but worked when i used exlinux.conf trying to track down the problem, i've found that in the default u-boot environment the "board" and "boardname" variables were "smdk5420" and not "odroid" (like before) ... and thus the "fdtfile" variable was "exynos5422-smdk5420hc1.dtb" and not "exynos5422-odroidhc1.dtb". continuing i've made a diff between the u-boot-v2021.10.tar.bz2 and u-boot-v2022.04.tar.bz2 ... and long story short, i've found the culprit in commit f76750d1 "Convert CONFIG_CONS_INDEX et al to Kconfig" : - --- u-boot-v2021.10/include/configs/odroid_xu3.h 2021-10-04 18:09:26.0 +0300 +++ u-boot-v2022.04/include/configs/odroid_xu3.h 2022-04-04 17:31:32.0 +0300 @@ -75,14 +75,11 @@ /* Set soc_rev, soc_id, board_rev, board_name, fdtfile */ #define CONFIG_ODROID_REV_AIN 9 -#define CONFIG_REVISION_TAG /* * Need to override existing one (smdk5420) with odroid so set_board_info will * use proper prefix when creating full board_name (SYS_BOARD + type) */ -#undef CONFIG_SYS_BOARD -#define CONFIG_SYS_BOARD "odroid" /* Define new extra env settings, including DFU settings */ #undef CONFIG_EXTRA_ENV_SETTINGS - and following part needs a revert - -#undef CONFIG_SYS_BOARD -#define CONFIG_SYS_BOARD "odroid" - or, as an (untested) alternative, define CONFIG_SYS_BOARD correctly in board/samsung/smdk5420/Kconfig as follows: - --- Kconfig 2022-04-04 17:31:32.0 +0300 +++ Kconfig.new 2022-05-28 02:57:19.867518057 +0300 @@ -1,7 +1,7 @@ if TARGET_ODROID_XU3 config SYS_BOARD - default "smdk5420" + default "odroid" config SYS_VENDOR default "samsung" - Gabriel
Re: [PATCH] armv8: Fix TCR 64-bit writes
On Tue, May 10, 2022 at 12:56 AM Peng Fan wrote:
>
> > Subject: [PATCH] armv8: Fix TCR 64-bit writes
> >
> > The AArch64 TCR_ELx register is a 64-bit register, and many newer
> > architecture
> > features use bits in the upper half. So far U-Boot was igorant of those
> > bits,
> > trying to leave them alone.
> > However, in an effort to set bit 31 to 1, it failed doing so, because the
> > compiler
> > sign-extended "1 << 31", so that all bits[63:31] got set.
> >
> > Older ARMv8.0 cores don't define anything dangerous up there, but newer
> > architecture revisions do, and setting all those bits will end badly:
> > =
> > $ qemu-system-aarch64 -cpu max
> > U-Boot 2022.07-rc1 (May 09 2022 - 15:21:00 +0100)
> >
> > DRAM: 1.5 GiB
> > = (hangs here)
> >
> > Defining TCR_ELx_RSVD to "1U << 31" avoids the sign-extension, so all upper
> > bits stay at a safe 0 value. This means no more surprises when U-Boot runs
> > on a
> > more capable CPU core.
> >
> > Reported-by: Balaji Anandapadmanaban
> > Signed-off-by: Andre Przywara
>
> Reviewed-by: Peng Fan
I ran into the same problem as you and developed the same fix. I was
about to send it out before I found your patch on the list.
Tested-by: Peter Collingbourne
Reviewed-by: Peter Collingbourne
You may also want to add:
Fixes: ad3d6e88a1a4 ("armv8/mmu: Set bits marked RES1 in TCR")
Peter
