Re: [PATCH v2 05/28] image: remove redundant hash includes
On Tue, May 7, 2024 at 7:54 PM Raymond Mao wrote: > > Remove the redundant includes of u-boot/md5.h, u-boot/sha1.h, > u-boot/sha256.h and u-boot/sha512.h > > Signed-off-by: Raymond Mao > --- > Changes in v2 > - None. > > boot/image-fit.c | 4 > boot/image.c | 2 -- > 2 files changed, 6 deletions(-) > > diff --git a/boot/image-fit.c b/boot/image-fit.c > index 89e377563ce..1efc39f4408 100644 > --- a/boot/image-fit.c > +++ b/boot/image-fit.c > @@ -38,10 +38,6 @@ DECLARE_GLOBAL_DATA_PTR; > #include > #include > #include > -#include > -#include > -#include > -#include > > > /*/ > /* New uImage format routines */ > diff --git a/boot/image.c b/boot/image.c > index 073931cd7a3..e57d6eae52d 100644 > --- a/boot/image.c > +++ b/boot/image.c > @@ -26,8 +26,6 @@ > #endif > > #include > -#include > -#include > #include > #include > > -- > 2.25.1 > Reviewed-by: Igor Opaniuk -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk
Re: Distro Boot instructions with examples
Hello Antoni, On Tue, May 7, 2024 at 4:14 PM Antoni Jankowski wrote: > > Hi, > > I've recently learned about the Distro Boot feature of u-boot. This sounds > interesting, however the official instructions are sparse and examples seem > to be non-existent. > > The platform I'm working with at the moment is a raspberry pi CM4, are > there any examples of how to implement this concept? Distro Boot seems to > be suitable for updating all the rootfs at once, including what usually is > a separate boot partition. > > Best regards, > Antoni There is a nice manual from Toradex about Distro Boot integration [1]. You can also find a bunch of examples of integration of existing boards just by grep-ing: u-boot.git$ grep -ine "config_distro_bootcmd" -r ./include/configs/ ./include/configs/pico-imx6ul.h:86:#include ./include/configs/kp_imx6q_tpc.h:84:#include ./include/configs/imx8mp_venice.h:21:#include ./include/configs/stm32f746-disco.h:30:#include ... Hope this helps. [1] https://developer.toradex.com/linux-bsp/os-development/boot/distro-boot/ -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk
Re: [PATCH 070/149] board: hisilicon: Remove and add needed includes
On Tue, Apr 30, 2024 at 7:44 PM Tom Rini wrote: > > Remove from this board vendor directory and when needed > add missing include files directly. > > Signed-off-by: Tom Rini > --- > Cc: Peter Griffin > Cc: Manivannan Sadhasivam > Cc: Jorge Ramirez-Ortiz > Cc: Shawn Guo > Cc: Igor Opaniuk > --- > board/hisilicon/hikey/hikey.c | 1 - > board/hisilicon/hikey960/hikey960.c | 1 - > board/hisilicon/poplar/poplar.c | 1 - > 3 files changed, 3 deletions(-) > > diff --git a/board/hisilicon/hikey/hikey.c b/board/hisilicon/hikey/hikey.c > index c9a2d60ee56c..95a831efcafd 100644 > --- a/board/hisilicon/hikey/hikey.c > +++ b/board/hisilicon/hikey/hikey.c > @@ -3,7 +3,6 @@ > * (C) Copyright 2015 Linaro > * Peter Griffin > */ > -#include > #include > #include > #include > diff --git a/board/hisilicon/hikey960/hikey960.c > b/board/hisilicon/hikey960/hikey960.c > index f41fabbad099..5029f4edb2af 100644 > --- a/board/hisilicon/hikey960/hikey960.c > +++ b/board/hisilicon/hikey960/hikey960.c > @@ -4,7 +4,6 @@ > * Author: Manivannan Sadhasivam > */ > > -#include > #include > #include > #include > diff --git a/board/hisilicon/poplar/poplar.c b/board/hisilicon/poplar/poplar.c > index b89e7e869766..c3ea080ff75a 100644 > --- a/board/hisilicon/poplar/poplar.c > +++ b/board/hisilicon/poplar/poplar.c > @@ -4,7 +4,6 @@ > * Jorge Ramirez-Ortiz > */ > > -#include > #include > #include > #include > -- > 2.34.1 > Reviewed-by: Igor Opaniuk -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk
[PATCH v1] tee: sandbox: check for buffer size
Add additional check for buffer size when reading out persistent
storage value and provide back actual value size.
Signed-off-by: Igor Opaniuk
---
drivers/tee/sandbox.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c
index 8ad7c09efdd..86b16a3bb8d 100644
--- a/drivers/tee/sandbox.c
+++ b/drivers/tee/sandbox.c
@@ -174,7 +174,7 @@ static u32 ta_avb_invoke_func(struct udevice *dev, u32
func, uint num_params,
uint slot;
u64 val;
char *value;
- u32 value_sz;
+ u32 value_sz, tmp_sz;
switch (func) {
case TA_AVB_CMD_READ_ROLLBACK_INDEX:
@@ -267,8 +267,12 @@ static u32 ta_avb_invoke_func(struct udevice *dev, u32
func, uint num_params,
if (!ep)
return TEE_ERROR_ITEM_NOT_FOUND;
- value_sz = strlen(ep->data) + 1;
- memcpy(value, ep->data, value_sz);
+ tmp_sz = strlen(ep->data) + 1;
+ if (value_sz < tmp_sz)
+ return TEE_ERROR_SHORT_BUFFER;
+
+ memcpy(value, ep->data, tmp_sz);
+ params[1].u.memref.size = tmp_sz;
return TEE_SUCCESS;
case TA_AVB_CMD_WRITE_PERSIST_VALUE:
--
2.34.1
Re: [PATCH v1 05/25] configs: stm32mp1: Enable BUTTON_GPIO flag for stm32mp13_defconfig
On Tue, Apr 9, 2024 at 5:05 PM Patrice Chotard wrote: > > Enable BUTTON_GPIO flag for STM32MP15. > > Signed-off-by: Patrice Chotard > --- > > configs/stm32mp13_defconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/configs/stm32mp13_defconfig b/configs/stm32mp13_defconfig > index db09e63100e..caaabf39ef3 100644 > --- a/configs/stm32mp13_defconfig > +++ b/configs/stm32mp13_defconfig > @@ -52,6 +52,8 @@ CONFIG_SYS_REDUNDAND_ENVIRONMENT=y > CONFIG_SYS_RELOC_GD_ENV_ADDR=y > CONFIG_SYS_MMC_ENV_DEV=-1 > CONFIG_ENV_MMC_USE_DT=y > +CONFIG_BUTTON=y > +CONFIG_BUTTON_GPIO=y > CONFIG_CLK_SCMI=y > CONFIG_SET_DFU_ALT_INFO=y > CONFIG_USB_FUNCTION_FASTBOOT=y > -- > 2.25.1 > Reviewed-by: Igor Opaniuk -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk
Re: [PATCH v1 01/25] configs: stm32mp13: Enable FASTBOOT
On Tue, Apr 9, 2024 at 5:19 PM Patrice Chotard wrote: > > Enable FASTBOOT relative flags for stm32mp13_defconfig. > > Signed-off-by: Patrice Chotard > > --- > > configs/stm32mp13_defconfig | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/configs/stm32mp13_defconfig b/configs/stm32mp13_defconfig > index c893e272db9..db09e63100e 100644 > --- a/configs/stm32mp13_defconfig > +++ b/configs/stm32mp13_defconfig > @@ -15,6 +15,7 @@ CONFIG_CMD_STM32PROG=y > CONFIG_SYS_LOAD_ADDR=0xc200 > CONFIG_SYS_MEMTEST_START=0xc000 > CONFIG_SYS_MEMTEST_END=0xc400 > +# CONFIG_ANDROID_BOOT_IMAGE is not set > CONFIG_FIT=y > CONFIG_SYS_BOOTM_LEN=0x200 > CONFIG_DISTRO_DEFAULTS=y > @@ -53,6 +54,13 @@ CONFIG_SYS_MMC_ENV_DEV=-1 > CONFIG_ENV_MMC_USE_DT=y > CONFIG_CLK_SCMI=y > CONFIG_SET_DFU_ALT_INFO=y > +CONFIG_USB_FUNCTION_FASTBOOT=y > +CONFIG_FASTBOOT_BUF_ADDR=0xc000 > +CONFIG_FASTBOOT_BUF_SIZE=0x0200 > +CONFIG_FASTBOOT_FLASH=y > +CONFIG_FASTBOOT_FLASH_MMC_DEV=0 > +CONFIG_FASTBOOT_CMD_OEM_FORMAT=y > +CONFIG_FASTBOOT_CMD_OEM_PARTCONF=y > CONFIG_GPIO_HOG=y > CONFIG_DM_I2C=y > CONFIG_SYS_I2C_STM32F7=y > @@ -92,7 +100,6 @@ CONFIG_USB_GADGET_MANUFACTURER="STMicroelectronics" > CONFIG_USB_GADGET_VENDOR_NUM=0x0483 > CONFIG_USB_GADGET_PRODUCT_NUM=0x5720 > CONFIG_USB_GADGET_DWC2_OTG=y > -CONFIG_USB_GADGET_DOWNLOAD=y > CONFIG_ERRNO_STR=y > # CONFIG_LMB_USE_MAX_REGIONS is not set > CONFIG_LMB_MEMORY_REGIONS=2 > -- > 2.25.1 > Reviewed-by: Igor Opaniuk -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk
Re: [PATCH 1/1] test: typo curren
On Tue, Apr 9, 2024 at 9:56 AM Heinrich Schuchardt
wrote:
>
> Fix typos in test_eficonfig.py: %s/curren/current/
>
> Signed-off-by: Heinrich Schuchardt
> ---
> test/py/tests/test_eficonfig/test_eficonfig.py | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/test/py/tests/test_eficonfig/test_eficonfig.py
> b/test/py/tests/test_eficonfig/test_eficonfig.py
> index b0a6cc47df2..1d8e033f75d 100644
> --- a/test/py/tests/test_eficonfig/test_eficonfig.py
> +++ b/test/py/tests/test_eficonfig/test_eficonfig.py
> @@ -224,7 +224,7 @@ def test_efi_eficonfig(u_boot_console,
> efi_eficonfig_data):
>
> # Change the Boot Order
> press_up_down_enter_and_wait(0, 2, True, None)
> -# Check the curren BootOrder
> +# Check the current BootOrder
> for i in ('test 2', 'test 1', 'host 0:1', 'Save', 'Quit'):
> u_boot_console.p.expect([i])
> # move 'test 2' to the second entry
> @@ -269,7 +269,7 @@ def test_efi_eficonfig(u_boot_console,
> efi_eficonfig_data):
> u_boot_console.run_command('eficonfig', wait_for_prompt=False)
> # Select 'Edit Boot Option'
> press_up_down_enter_and_wait(0, 1, True, None)
> -# Check the curren BootOrder
> +# Check the current BootOrder
> for i in ('test 1', 'Quit'):
> u_boot_console.p.expect([i])
> press_up_down_enter_and_wait(0, 0, True, None)
> @@ -326,7 +326,7 @@ def test_efi_eficonfig(u_boot_console,
> efi_eficonfig_data):
>
> # Select 'Delete Boot Option'
> press_up_down_enter_and_wait(0, 3, True, None)
> -# Check the curren BootOrder
> +# Check the current BootOrder
> for i in ('test 3', 'Quit'):
> u_boot_console.p.expect([i])
>
> --
> 2.43.0
>
Reviewed-by: Igor Opaniuk
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
https://www.linkedin.com/in/iopaniuk
Re: [PATCH 1/5] zfs: Fix malloc() success check
Hi Phaedrus,
On Sun, Apr 7, 2024 at 4:00 AM wrote:
> This code was hitting the error code path whenever malloc() succeeded
> rather than when it failed, so presumably this part of the code hasn't
> been tested. I had to apply this fix (and others) to get U-Boot to boot
> from ZFS on an Nvidia Jetson TX2 NX SoM (an aarch64 computer).
>
> Signed-off-by: Phaedrus Leeds
> Tested-by: Phaedrus Leeds
>
It's an abuse of the Tested-by tag. If you are the author of the patch,
that obviously implies that you tested it before sending to ML.
Signed-off-by is enough in this case.
---
> fs/zfs/zfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/zfs/zfs.c b/fs/zfs/zfs.c
> index 1fec96cd5c..14779dee32 100644
> --- a/fs/zfs/zfs.c
> +++ b/fs/zfs/zfs.c
> @@ -648,21 +648,21 @@ dmu_read(dnode_end_t *dn, uint64_t blkid, void **buf,
> if (bp_array != dn->dn.dn_blkptr) {
> free(bp_array);
> bp_array = 0;
> }
>
> if (BP_IS_HOLE(bp)) {
> size_t size = zfs_to_cpu16(dn->dn.dn_datablkszsec,
>
> dn->endian)
> << SPA_MINBLOCKSHIFT;
> *buf = malloc(size);
> - if (*buf) {
> + if (!*buf) {
> err = ZFS_ERR_OUT_OF_MEMORY;
> break;
> }
> memset(*buf, 0, size);
> endian = (zfs_to_cpu64(bp->blk_prop, endian) >>
> 63) & 1;
> break;
> }
> if (level == 0) {
> err = zio_read(bp, endian, buf, 0, data);
> endian = (zfs_to_cpu64(bp->blk_prop, endian) >>
> 63) & 1;
> --
> 2.44.0
>
>
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
https://www.linkedin.com/in/iopaniuk <http://ua.linkedin.com/in/iopaniuk>
[PATCH v5 5/5] tee: remove common.h inclusion
The usage of the common.h include file is deprecated [1], and has already been removed from several files. Get rid of all inclusions in the "drivers/tee" directory, and replace it with required include files directly where needed. [1] doc/develop/codingstyle.rst Reviewed-by: Ilias Apalodimas Signed-off-by: Igor Opaniuk --- Changes in v5: - Extended commit message for "cmd: optee_rpmb: close tee session" - Added R-b tag Changes in v4: - Rebased on the latest master and excluded "tee: sandbox: fix spelling errors", as it was merged already by Heinrich Schuchardt Changes in v3: - Added calls for closing tee session after every read/write operation - Added calls for closing tee session after every read/write operation Changes in v2: - Fixed chimp_optee.c:37:9: error: implicit declaration of function 'memset' - Applied R-b and T-b tags - Fixed chimp_optee.c:37:9: error: implicit declaration of function 'memset' drivers/tee/broadcom/chimp_optee.c | 3 ++- drivers/tee/optee/core.c | 1 - drivers/tee/optee/i2c.c| 1 - drivers/tee/optee/rpmb.c | 1 - drivers/tee/optee/supplicant.c | 2 +- drivers/tee/sandbox.c | 2 +- drivers/tee/tee-uclass.c | 1 - 7 files changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/tee/broadcom/chimp_optee.c b/drivers/tee/broadcom/chimp_optee.c index 37f9b094f76..bd146ef2899 100644 --- a/drivers/tee/broadcom/chimp_optee.c +++ b/drivers/tee/broadcom/chimp_optee.c @@ -3,9 +3,10 @@ * Copyright 2020 Broadcom. */ -#include #include #include +#include +#include #ifdef CONFIG_CHIMP_OPTEE diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 47f845cffe3..5fc0505c788 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -3,7 +3,6 @@ * Copyright (c) 2018-2020 Linaro Limited */ -#include #include #include #include diff --git a/drivers/tee/optee/i2c.c b/drivers/tee/optee/i2c.c index ef4e10f9912..e3fb99897c5 100644 --- a/drivers/tee/optee/i2c.c +++ b/drivers/tee/optee/i2c.c @@ -3,7 +3,6 @@ * Copyright (c) 2020 Foundries.io Ltd */ -#include #include #include #include diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c index 5bc13757ea8..bacced6af6c 100644 --- a/drivers/tee/optee/rpmb.c +++ b/drivers/tee/optee/rpmb.c @@ -3,7 +3,6 @@ * Copyright (c) 2018 Linaro Limited */ -#include #include #include #include diff --git a/drivers/tee/optee/supplicant.c b/drivers/tee/optee/supplicant.c index f9dd874b594..8a426f53ba8 100644 --- a/drivers/tee/optee/supplicant.c +++ b/drivers/tee/optee/supplicant.c @@ -3,10 +3,10 @@ * Copyright (c) 2018, Linaro Limited */ -#include #include #include #include +#include #include #include "optee_msg.h" diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c index ec66401878c..8ad7c09efdd 100644 --- a/drivers/tee/sandbox.c +++ b/drivers/tee/sandbox.c @@ -2,7 +2,7 @@ /* * Copyright (C) 2018 Linaro Limited */ -#include + #include #include #include diff --git a/drivers/tee/tee-uclass.c b/drivers/tee/tee-uclass.c index 52412a4098e..0194d732193 100644 --- a/drivers/tee/tee-uclass.c +++ b/drivers/tee/tee-uclass.c @@ -5,7 +5,6 @@ #define LOG_CATEGORY UCLASS_TEE -#include #include #include #include -- 2.34.1
[PATCH v5 4/5] test: py: add optee_rpmb tests
Add read/write tests for optee_rpmb cmd.
Signed-off-by: Igor Opaniuk
---
(no changes since v1)
test/py/tests/test_optee_rpmb.py | 20
1 file changed, 20 insertions(+)
create mode 100644 test/py/tests/test_optee_rpmb.py
diff --git a/test/py/tests/test_optee_rpmb.py b/test/py/tests/test_optee_rpmb.py
new file mode 100644
index 000..8a081b5c494
--- /dev/null
+++ b/test/py/tests/test_optee_rpmb.py
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Tests for OP-TEE RPMB read/write support
+
+"""
+This tests optee_rpmb cmd in U-Boot
+"""
+
+import pytest
+import u_boot_utils as util
+
+@pytest.mark.buildconfigspec('cmd_optee_rpmb')
+def test_optee_rpmb_read_write(u_boot_console):
+"""Test OP-TEE RPMB cmd read/write
+"""
+response = u_boot_console.run_command('optee_rpmb write_pvalue
test_variable test_value')
+assert response == 'Wrote 11 bytes'
+
+response = u_boot_console.run_command('optee_rpmb read_pvalue
test_variable 11')
+assert response == 'Read 11 bytes, value = test_value'
\ No newline at end of file
--
2.34.1
[PATCH v5 3/5] cmd: optee_rpmb: build cmd for sandbox
Support CMD_OPTEE_RPMB for SANDBOX configurations. Test: $ ./u-boot -d arch/sandbox/dts/test.dtb ... => optee_rpmb write_pvalue test_variable test_value Wrote 11 bytes => optee_rpmb read_pvalue test_variable 11 Read 11 bytes, value = test_value Reviewed-by: Mattijs Korpershoek Tested-by: Mattijs Korpershoek Signed-off-by: Igor Opaniuk --- (no changes since v2) Changes in v2: - Applied R-b and T-b tags cmd/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index 61e280fb1a4..227d66a7eea 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1394,7 +1394,9 @@ config CMD_CLONE config CMD_OPTEE_RPMB bool "Enable read/write support on RPMB via OPTEE" - depends on SUPPORT_EMMC_RPMB && OPTEE + depends on (SUPPORT_EMMC_RPMB && OPTEE) || SANDBOX_TEE + default y if SANDBOX_TEE + select OPTEE_TA_AVB if SANDBOX_TEE help Enable the commands for reading, writing persistent named values in the Replay Protection Memory Block partition in eMMC by -- 2.34.1
[PATCH v5 2/5] cmd: optee_rpmb: close tee session
Close tee session after each optee_rpmb invocation, as there is no
reason to keep it open, considering the absence of any available mechanism
to clean up all open sessions automatically before handing over control
to the Linux kernel. Without proper clean-up we might end up with orphaned
sessions registered in OP-TEE OS core (obvious resource leak).
Signed-off-by: Igor Opaniuk
---
Changes in v5:
- Extended commit message, added more details about the reasons why the
change was needed
cmd/optee_rpmb.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c
index e0e44bbed04..b3cafd92410 100644
--- a/cmd/optee_rpmb.c
+++ b/cmd/optee_rpmb.c
@@ -87,8 +87,10 @@ static int read_persistent_value(const char *name,
rc = tee_shm_alloc(tee, name_size,
TEE_SHM_ALLOC, _name);
- if (rc)
- return -ENOMEM;
+ if (rc) {
+ rc = -ENOMEM;
+ goto close_session;
+ }
rc = tee_shm_alloc(tee, buffer_size,
TEE_SHM_ALLOC, _buf);
@@ -125,6 +127,9 @@ out:
tee_shm_free(shm_buf);
free_name:
tee_shm_free(shm_name);
+close_session:
+ tee_close_session(tee, session);
+ tee = NULL;
return rc;
}
@@ -139,17 +144,20 @@ static int write_persistent_value(const char *name,
struct tee_param param[2];
size_t name_size = strlen(name) + 1;
+ if (!value_size)
+ return -EINVAL;
+
if (!tee) {
if (avb_ta_open_session())
return -ENODEV;
}
- if (!value_size)
- return -EINVAL;
rc = tee_shm_alloc(tee, name_size,
TEE_SHM_ALLOC, _name);
- if (rc)
- return -ENOMEM;
+ if (rc) {
+ rc = -ENOMEM;
+ goto close_session;
+ }
rc = tee_shm_alloc(tee, value_size,
TEE_SHM_ALLOC, _buf);
@@ -178,6 +186,9 @@ out:
tee_shm_free(shm_buf);
free_name:
tee_shm_free(shm_name);
+close_session:
+ tee_close_session(tee, session);
+ tee = NULL;
return rc;
}
--
2.34.1
[PATCH v5 1/5] tee: optee: fix description in Kconfig
Fix OPTEE_TA_AVB symbol description in Kconfig: s/"write"rb"/"write_rb"/g Reviewed-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas Signed-off-by: Igor Opaniuk --- (no changes since v2) Changes in v2: - Applied R-b tags drivers/tee/optee/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig index 9dc65b0501e..db0bcfa6f15 100644 --- a/drivers/tee/optee/Kconfig +++ b/drivers/tee/optee/Kconfig @@ -19,7 +19,7 @@ config OPTEE_TA_AVB default y help Enables support for the AVB Trusted Application (TA) in OP-TEE. - The TA can support the "avb" subcommands "read_rb", "write"rb" + The TA can support the "avb" subcommands "read_rb", "write_rb" and "is_unlocked". config OPTEE_TA_RPC_TEST -- 2.34.1
[PATCH v5 0/5] TEE: minor cleanup
- Address some spelling errors and typos - Support CMD_OPTEE_RPMB for SANDBOX configurations and add python tests - Remove common.h inclusion for drivers/tee - Add calls for closing tee session after every read/write operation. CI build: [1] https://dev.azure.com/igoropaniuk/u-boot/_build/results?buildId=33=results Changes in v5: - Extended commit message for "cmd: optee_rpmb: close tee session" - Added R-b tag Changes in v4: - Rebased on the latest master and excluded "tee: sandbox: fix spelling errors", as it was merged already by Heinrich Schuchardt Changes in v3: - Added calls for closing tee session after every read/write operation Changes in v2: - Fixed chimp_optee.c:37:9: error: implicit declaration of function 'memset' - Applied R-b and T-b tags Igor Opaniuk (5): tee: optee: fix description in Kconfig cmd: optee_rpmb: close tee session cmd: optee_rpmb: build cmd for sandbox test: py: add optee_rpmb tests tee: remove common.h inclusion cmd/Kconfig| 4 +++- cmd/optee_rpmb.c | 23 +-- drivers/tee/broadcom/chimp_optee.c | 3 ++- drivers/tee/optee/Kconfig | 2 +- drivers/tee/optee/core.c | 1 - drivers/tee/optee/i2c.c| 1 - drivers/tee/optee/rpmb.c | 1 - drivers/tee/optee/supplicant.c | 2 +- drivers/tee/sandbox.c | 2 +- drivers/tee/tee-uclass.c | 1 - test/py/tests/test_optee_rpmb.py | 20 11 files changed, 45 insertions(+), 15 deletions(-) create mode 100644 test/py/tests/test_optee_rpmb.py -- 2.34.1
Re: [PATCH v4 2/5] cmd: optee_rpmb: close tee session
Ilias,
On Thu, Apr 4, 2024 at 1:00 PM Ilias Apalodimas
wrote:
> Hi Igor,
>
> On Thu, 4 Apr 2024 at 13:18, Igor Opaniuk wrote:
> >
> > Hi Ilias,
> >
> > On Thu, Apr 4, 2024 at 10:54 AM Ilias Apalodimas <
> ilias.apalodi...@linaro.org> wrote:
> >>
> >> Hi Igor,
> >>
> >> On Thu, 4 Apr 2024 at 11:40, Ilias Apalodimas
> >> wrote:
> >> >
> >> > Hi Igor,
> >> >
> >> > I was about to apply the series, but noticed neither me or Jens were
> cc'ed
> >> > on this. Adding Jens to the party
> >> >
> >> > On Thu, Apr 04, 2024 at 10:13:49AM +0200, Igor Opaniuk wrote:
> >> > > Add calls for closing tee session after every read/write operation.
> >> >
> >> > What the patch is pretty obvious, but I am missing an explanation of
> why
> >> > this is needed
> >> >
> >> > >
> >> > > Signed-off-by: Igor Opaniuk
> >> > > ---
> >> > >
> >> > > (no changes since v1)
> >> > >
> >> > > cmd/optee_rpmb.c | 23 +--
> >> > > 1 file changed, 17 insertions(+), 6 deletions(-)
> >> > >
> >> > > diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c
> >> > > index e0e44bbed04..b3cafd92410 100644
> >> > > --- a/cmd/optee_rpmb.c
> >> > > +++ b/cmd/optee_rpmb.c
> >> > > @@ -87,8 +87,10 @@ static int read_persistent_value(const char
> *name,
> >> > >
> >> > > rc = tee_shm_alloc(tee, name_size,
> >> > > TEE_SHM_ALLOC, _name);
> >> > > - if (rc)
> >> > > - return -ENOMEM;
> >> > > + if (rc) {
> >> > > + rc = -ENOMEM;
> >> > > + goto close_session;
> >> > > + }
> >> >
> >> > I don't think you need the session to be opened to allocate memory.
> >> > So instead of of doing this, why don't we just move the alloc call
> before
> >> > opening the session?
> >>
> >> Looking at it again, we do need tee != NULL, but I think it's cleaner
> >> to add something like
> >> if (!dev)
> >> return -ENODEV
> >> to __tee_shm_add() instead.
> >
> > Do you mind if I address that in a separate patch series, as tbh I
> > don't want to add additional patches to the existing series?
>
> We still completely change the functionality. So, the patchset is not
> a 'tiny cleanup', it instead closes the session every time instead of
> keeping it open.
> There are a few things going on, that aren't explained clearly in the
> commit message
> - Why is this needed? Looking at the code it is an actual problem
> since sessions tied to the AVB uuid will remain open
> - Is there any side effect by always closing the session?
> I don't mind merging it as is with a proper commit message, but I
> think the alternative is just easier.
>
I'll provide a bit more context here. The problem initially was in the TEE
sandbox
driver implementation(drivers/tee/sandbox.c) and it's limitations, which
doesn't
permit to have multiple simultaneous sessions with different TAs.
This is what actually happened in this CI run [1], firstly "optee_rpmb"
cmd was executed (and after execution we had one session open), and
then "scp03", which also makes calls to OP-TEE, however it fails
in sandbox_tee_open_session() because of this check:
if (state->ta) {
printf("A session is already open\n");
return -EBUSY;
}
So basically I had two ways in mind to address that:
1. Close a session on each optee_rpmb cmd invocation.
I don't see any reason to keep this session open, as obviously
there is no other mechanism (tbh, I don't know if DM calls ".remove" for
active
devices) to close it automatically before handing over control to
Linux kernel. As a result we might end up with some orphaned sessions
registered in OP-TEE OS core (obvious resource leak).
2. Extend TEE sandbox driver, add support for multiple
simultaneous sessions just to handle the case.
I've chosen the first approach, as IMO it was "kill two birds with one
stone",
I could address resource leak in OP-TEE and bypass limitations of
TEE sandbox driver.
[1] https://source.denx.de/u-boot/custodians/u-boot-tpm/-/jobs/792216
>
> Thanks
> /Ilias
>
> >
> >>
> >>
> >> Cheers
> >> /Ilias
> >> >
> >> > >
> >> > > rc = tee_shm_al
Re: [PATCH v4 2/5] cmd: optee_rpmb: close tee session
HI Ilias
On Thu, Apr 4, 2024 at 12:15 PM Igor Opaniuk wrote:
> Hi Ilias,
>
> On Thu, Apr 4, 2024 at 10:40 AM Ilias Apalodimas <
> ilias.apalodi...@linaro.org> wrote:
>
>> Hi Igor,
>>
>> I was about to apply the series, but noticed neither me or Jens were cc'ed
>> on this. Adding Jens to the party
>>
> I usually rely on automatic CC-list creation by patman. Looks like there
> is no
> entry in MAINTAINERS for this specific file, that's why only Tom was added.
> I'll send a patch for that if you don't mind.
>
I've added all orphaned tee-related files to MAINTAINERS in [1], so I patman
should generate the CC list correctly next time.
[1]
https://lore.kernel.org/u-boot/20240404105248.3241506-1-igor.opan...@gmail.com/T/#u
>
>
>> On Thu, Apr 04, 2024 at 10:13:49AM +0200, Igor Opaniuk wrote:
>> > Add calls for closing tee session after every read/write operation.
>>
>> What the patch is pretty obvious, but I am missing an explanation of why
>> this is needed
>>
>> >
>> > Signed-off-by: Igor Opaniuk
>> > ---
>> >
>> > (no changes since v1)
>> >
>> > cmd/optee_rpmb.c | 23 +--
>> > 1 file changed, 17 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c
>> > index e0e44bbed04..b3cafd92410 100644
>> > --- a/cmd/optee_rpmb.c
>> > +++ b/cmd/optee_rpmb.c
>> > @@ -87,8 +87,10 @@ static int read_persistent_value(const char *name,
>> >
>> > rc = tee_shm_alloc(tee, name_size,
>> > TEE_SHM_ALLOC, _name);
>> > - if (rc)
>> > - return -ENOMEM;
>> > + if (rc) {
>> > + rc = -ENOMEM;
>> > + goto close_session;
>> > + }
>>
>> I don't think you need the session to be opened to allocate memory.
>> So instead of of doing this, why don't we just move the alloc call before
>> opening the session?
>>
> That's correct, but avb_ta_open_session() wrapper calls
> both tee_find_device()/
> tee_open_session(), and tee_shm_alloc() expects valid tee device as param,
> which
> is initialized by tee_find_device(). My intention was to address the only
> one particular issue
> that was catched by CI and explained in [1] instead of doing overhaul of
> the whole optee_rpmb cmd implementation.
>
>
>> >
>> > rc = tee_shm_alloc(tee, buffer_size,
>> > TEE_SHM_ALLOC, _buf);
>> > @@ -125,6 +127,9 @@ out:
>> > tee_shm_free(shm_buf);
>> > free_name:
>> > tee_shm_free(shm_name);
>> > +close_session:
>> > + tee_close_session(tee, session);
>> > + tee = NULL;
>> >
>> > return rc;
>> > }
>> > @@ -139,17 +144,20 @@ static int write_persistent_value(const char
>> *name,
>> > struct tee_param param[2];
>> > size_t name_size = strlen(name) + 1;
>> >
>> > + if (!value_size)
>> > + return -EINVAL;
>> > +
>> > if (!tee) {
>> > if (avb_ta_open_session())
>> > return -ENODEV;
>> > }
>> > - if (!value_size)
>> > - return -EINVAL;
>> >
>> > rc = tee_shm_alloc(tee, name_size,
>> > TEE_SHM_ALLOC, _name);
>> > - if (rc)
>> > - return -ENOMEM;
>> > + if (rc) {
>> > + rc = -ENOMEM;
>> > + goto close_session;
>> > + }
>>
>> ditto
>>
>> >
>> > rc = tee_shm_alloc(tee, value_size,
>> > TEE_SHM_ALLOC, _buf);
>> > @@ -178,6 +186,9 @@ out:
>> > tee_shm_free(shm_buf);
>> > free_name:
>> > tee_shm_free(shm_name);
>> > +close_session:
>> > + tee_close_session(tee, session);
>> > + tee = NULL;
>> >
>> > return rc;
>> > }
>> > --
>> > 2.34.1
>> >
>>
>> Thanks
>> /Ilias
>>
>
> [1]
> https://lore.kernel.org/all/20240302220108.720637-5-igor.opan...@gmail.com/T/
>
> --
> Best regards - Atentamente - Meilleures salutations
>
> Igor Opaniuk
>
> mailto: igor.opan...@gmail.com
> skype: igor.opanyuk
> https://www.linkedin.com/in/iopaniuk <http://ua.linkedin.com/in/iopaniuk>
>
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
https://www.linkedin.com/in/iopaniuk <http://ua.linkedin.com/in/iopaniuk>
[PATCH v1] MAINTAINERS: add entries for tee-related orphaned files
Add orphaned TEE/OP-TEE-related files to TEE subsystem entry in MAINTAINERS. This includes: - optee_rpmb cmd and test for it - Misc. OP-TEE tests - OP-TEE SCMI agent implementation - Documentation, including device tree bindings Signed-off-by: Igor Opaniuk --- MAINTAINERS | 9 + 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 0462ade4ac6..0f7325d6045 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1642,9 +1642,18 @@ M: Jens Wiklander M: Ilias Apalodimas T: git https://source.denx.de/u-boot/custodians/u-boot-tpm.git S: Maintained +F: cmd/optee* +F: doc/README.tee +F: doc/device-tree-bindings/firmware/linaro,optee-tz.txt +F: drivers/firmware/scmi/optee_agent.c F: drivers/tee/ +F: include/sandboxtee.h F: include/tee.h F: include/tee/ +F: include/test/optee.h +F: test/dm/tee.c +F: test/optee/ +F: test/py/tests/test_optee_rpmb.py TEE-lib M: Bryan O'Donoghue -- 2.34.1
Re: [PATCH v4 2/5] cmd: optee_rpmb: close tee session
Hi Ilias,
On Thu, Apr 4, 2024 at 10:54 AM Ilias Apalodimas <
ilias.apalodi...@linaro.org> wrote:
> Hi Igor,
>
> On Thu, 4 Apr 2024 at 11:40, Ilias Apalodimas
> wrote:
> >
> > Hi Igor,
> >
> > I was about to apply the series, but noticed neither me or Jens were
> cc'ed
> > on this. Adding Jens to the party
> >
> > On Thu, Apr 04, 2024 at 10:13:49AM +0200, Igor Opaniuk wrote:
> > > Add calls for closing tee session after every read/write operation.
> >
> > What the patch is pretty obvious, but I am missing an explanation of why
> > this is needed
> >
> > >
> > > Signed-off-by: Igor Opaniuk
> > > ---
> > >
> > > (no changes since v1)
> > >
> > > cmd/optee_rpmb.c | 23 +--
> > > 1 file changed, 17 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c
> > > index e0e44bbed04..b3cafd92410 100644
> > > --- a/cmd/optee_rpmb.c
> > > +++ b/cmd/optee_rpmb.c
> > > @@ -87,8 +87,10 @@ static int read_persistent_value(const char *name,
> > >
> > > rc = tee_shm_alloc(tee, name_size,
> > > TEE_SHM_ALLOC, _name);
> > > - if (rc)
> > > - return -ENOMEM;
> > > + if (rc) {
> > > + rc = -ENOMEM;
> > > + goto close_session;
> > > + }
> >
> > I don't think you need the session to be opened to allocate memory.
> > So instead of of doing this, why don't we just move the alloc call before
> > opening the session?
>
> Looking at it again, we do need tee != NULL, but I think it's cleaner
> to add something like
> if (!dev)
> return -ENODEV
> to __tee_shm_add() instead.
>
Do you mind if I address that in a separate patch series, as tbh I
don't want to add additional patches to the existing series?
>
> Cheers
> /Ilias
> >
> > >
> > > rc = tee_shm_alloc(tee, buffer_size,
> > > TEE_SHM_ALLOC, _buf);
> > > @@ -125,6 +127,9 @@ out:
> > > tee_shm_free(shm_buf);
> > > free_name:
> > > tee_shm_free(shm_name);
> > > +close_session:
> > > + tee_close_session(tee, session);
> > > + tee = NULL;
> > >
> > > return rc;
> > > }
> > > @@ -139,17 +144,20 @@ static int write_persistent_value(const char
> *name,
> > > struct tee_param param[2];
> > > size_t name_size = strlen(name) + 1;
> > >
> > > + if (!value_size)
> > > + return -EINVAL;
> > > +
> > > if (!tee) {
> > > if (avb_ta_open_session())
> > > return -ENODEV;
> > > }
> > > - if (!value_size)
> > > - return -EINVAL;
> > >
> > > rc = tee_shm_alloc(tee, name_size,
> > > TEE_SHM_ALLOC, _name);
> > > - if (rc)
> > > - return -ENOMEM;
> > > + if (rc) {
> > > + rc = -ENOMEM;
> > > + goto close_session;
> > > + }
> >
> > ditto
> >
> > >
> > > rc = tee_shm_alloc(tee, value_size,
> > > TEE_SHM_ALLOC, _buf);
> > > @@ -178,6 +186,9 @@ out:
> > > tee_shm_free(shm_buf);
> > > free_name:
> > > tee_shm_free(shm_name);
> > > +close_session:
> > > + tee_close_session(tee, session);
> > > + tee = NULL;
> > >
> > > return rc;
> > > }
> > > --
> > > 2.34.1
> > >
> >
> > Thanks
> > /Ilias
>
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
https://www.linkedin.com/in/iopaniuk <http://ua.linkedin.com/in/iopaniuk>
Re: [PATCH v4 2/5] cmd: optee_rpmb: close tee session
Hi Ilias,
On Thu, Apr 4, 2024 at 10:40 AM Ilias Apalodimas <
ilias.apalodi...@linaro.org> wrote:
> Hi Igor,
>
> I was about to apply the series, but noticed neither me or Jens were cc'ed
> on this. Adding Jens to the party
>
I usually rely on automatic CC-list creation by patman. Looks like there is
no
entry in MAINTAINERS for this specific file, that's why only Tom was added.
I'll send a patch for that if you don't mind.
> On Thu, Apr 04, 2024 at 10:13:49AM +0200, Igor Opaniuk wrote:
> > Add calls for closing tee session after every read/write operation.
>
> What the patch is pretty obvious, but I am missing an explanation of why
> this is needed
>
> >
> > Signed-off-by: Igor Opaniuk
> > ---
> >
> > (no changes since v1)
> >
> > cmd/optee_rpmb.c | 23 +--
> > 1 file changed, 17 insertions(+), 6 deletions(-)
> >
> > diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c
> > index e0e44bbed04..b3cafd92410 100644
> > --- a/cmd/optee_rpmb.c
> > +++ b/cmd/optee_rpmb.c
> > @@ -87,8 +87,10 @@ static int read_persistent_value(const char *name,
> >
> > rc = tee_shm_alloc(tee, name_size,
> > TEE_SHM_ALLOC, _name);
> > - if (rc)
> > - return -ENOMEM;
> > + if (rc) {
> > + rc = -ENOMEM;
> > + goto close_session;
> > + }
>
> I don't think you need the session to be opened to allocate memory.
> So instead of of doing this, why don't we just move the alloc call before
> opening the session?
>
That's correct, but avb_ta_open_session() wrapper calls
both tee_find_device()/
tee_open_session(), and tee_shm_alloc() expects valid tee device as param,
which
is initialized by tee_find_device(). My intention was to address the only
one particular issue
that was catched by CI and explained in [1] instead of doing overhaul of
the whole optee_rpmb cmd implementation.
> >
> > rc = tee_shm_alloc(tee, buffer_size,
> > TEE_SHM_ALLOC, _buf);
> > @@ -125,6 +127,9 @@ out:
> > tee_shm_free(shm_buf);
> > free_name:
> > tee_shm_free(shm_name);
> > +close_session:
> > + tee_close_session(tee, session);
> > + tee = NULL;
> >
> > return rc;
> > }
> > @@ -139,17 +144,20 @@ static int write_persistent_value(const char *name,
> > struct tee_param param[2];
> > size_t name_size = strlen(name) + 1;
> >
> > + if (!value_size)
> > + return -EINVAL;
> > +
> > if (!tee) {
> > if (avb_ta_open_session())
> > return -ENODEV;
> > }
> > - if (!value_size)
> > - return -EINVAL;
> >
> > rc = tee_shm_alloc(tee, name_size,
> > TEE_SHM_ALLOC, _name);
> > - if (rc)
> > - return -ENOMEM;
> > + if (rc) {
> > + rc = -ENOMEM;
> > + goto close_session;
> > + }
>
> ditto
>
> >
> > rc = tee_shm_alloc(tee, value_size,
> > TEE_SHM_ALLOC, _buf);
> > @@ -178,6 +186,9 @@ out:
> > tee_shm_free(shm_buf);
> > free_name:
> > tee_shm_free(shm_name);
> > +close_session:
> > + tee_close_session(tee, session);
> > + tee = NULL;
> >
> > return rc;
> > }
> > --
> > 2.34.1
> >
>
> Thanks
> /Ilias
>
[1]
https://lore.kernel.org/all/20240302220108.720637-5-igor.opan...@gmail.com/T/
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
https://www.linkedin.com/in/iopaniuk <http://ua.linkedin.com/in/iopaniuk>
[PATCH v4 5/5] tee: remove common.h inclusion
The usage of the common.h include file is deprecated [1], and has already been removed from several files. Get rid of all inclusions in the "drivers/tee" directory, and replace it with required include files directly where needed. [1] doc/develop/codingstyle.rst Signed-off-by: Igor Opaniuk --- (no changes since v3) Changes in v3: - Added calls for closing tee session after every read/write operation - Added calls for closing tee session after every read/write operation Changes in v2: - Fixed chimp_optee.c:37:9: error: implicit declaration of function 'memset' - Applied R-b and T-b tags - Fixed chimp_optee.c:37:9: error: implicit declaration of function 'memset' drivers/tee/broadcom/chimp_optee.c | 3 ++- drivers/tee/optee/core.c | 1 - drivers/tee/optee/i2c.c| 1 - drivers/tee/optee/rpmb.c | 1 - drivers/tee/optee/supplicant.c | 2 +- drivers/tee/sandbox.c | 2 +- drivers/tee/tee-uclass.c | 1 - 7 files changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/tee/broadcom/chimp_optee.c b/drivers/tee/broadcom/chimp_optee.c index 37f9b094f76..bd146ef2899 100644 --- a/drivers/tee/broadcom/chimp_optee.c +++ b/drivers/tee/broadcom/chimp_optee.c @@ -3,9 +3,10 @@ * Copyright 2020 Broadcom. */ -#include #include #include +#include +#include #ifdef CONFIG_CHIMP_OPTEE diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 47f845cffe3..5fc0505c788 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -3,7 +3,6 @@ * Copyright (c) 2018-2020 Linaro Limited */ -#include #include #include #include diff --git a/drivers/tee/optee/i2c.c b/drivers/tee/optee/i2c.c index ef4e10f9912..e3fb99897c5 100644 --- a/drivers/tee/optee/i2c.c +++ b/drivers/tee/optee/i2c.c @@ -3,7 +3,6 @@ * Copyright (c) 2020 Foundries.io Ltd */ -#include #include #include #include diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c index 5bc13757ea8..bacced6af6c 100644 --- a/drivers/tee/optee/rpmb.c +++ b/drivers/tee/optee/rpmb.c @@ -3,7 +3,6 @@ * Copyright (c) 2018 Linaro Limited */ -#include #include #include #include diff --git a/drivers/tee/optee/supplicant.c b/drivers/tee/optee/supplicant.c index f9dd874b594..8a426f53ba8 100644 --- a/drivers/tee/optee/supplicant.c +++ b/drivers/tee/optee/supplicant.c @@ -3,10 +3,10 @@ * Copyright (c) 2018, Linaro Limited */ -#include #include #include #include +#include #include #include "optee_msg.h" diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c index ec66401878c..8ad7c09efdd 100644 --- a/drivers/tee/sandbox.c +++ b/drivers/tee/sandbox.c @@ -2,7 +2,7 @@ /* * Copyright (C) 2018 Linaro Limited */ -#include + #include #include #include diff --git a/drivers/tee/tee-uclass.c b/drivers/tee/tee-uclass.c index 52412a4098e..0194d732193 100644 --- a/drivers/tee/tee-uclass.c +++ b/drivers/tee/tee-uclass.c @@ -5,7 +5,6 @@ #define LOG_CATEGORY UCLASS_TEE -#include #include #include #include -- 2.34.1
[PATCH v4 4/5] test: py: add optee_rpmb tests
Add read/write tests for optee_rpmb cmd.
Signed-off-by: Igor Opaniuk
---
(no changes since v1)
test/py/tests/test_optee_rpmb.py | 20
1 file changed, 20 insertions(+)
create mode 100644 test/py/tests/test_optee_rpmb.py
diff --git a/test/py/tests/test_optee_rpmb.py b/test/py/tests/test_optee_rpmb.py
new file mode 100644
index 000..8a081b5c494
--- /dev/null
+++ b/test/py/tests/test_optee_rpmb.py
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Tests for OP-TEE RPMB read/write support
+
+"""
+This tests optee_rpmb cmd in U-Boot
+"""
+
+import pytest
+import u_boot_utils as util
+
+@pytest.mark.buildconfigspec('cmd_optee_rpmb')
+def test_optee_rpmb_read_write(u_boot_console):
+"""Test OP-TEE RPMB cmd read/write
+"""
+response = u_boot_console.run_command('optee_rpmb write_pvalue
test_variable test_value')
+assert response == 'Wrote 11 bytes'
+
+response = u_boot_console.run_command('optee_rpmb read_pvalue
test_variable 11')
+assert response == 'Read 11 bytes, value = test_value'
\ No newline at end of file
--
2.34.1
[PATCH v4 3/5] cmd: optee_rpmb: build cmd for sandbox
Support CMD_OPTEE_RPMB for SANDBOX configurations. Test: $ ./u-boot -d arch/sandbox/dts/test.dtb ... => optee_rpmb write_pvalue test_variable test_value Wrote 11 bytes => optee_rpmb read_pvalue test_variable 11 Read 11 bytes, value = test_value Reviewed-by: Mattijs Korpershoek Tested-by: Mattijs Korpershoek Signed-off-by: Igor Opaniuk --- (no changes since v2) Changes in v2: - Applied R-b and T-b tags cmd/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index 61e280fb1a4..227d66a7eea 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1394,7 +1394,9 @@ config CMD_CLONE config CMD_OPTEE_RPMB bool "Enable read/write support on RPMB via OPTEE" - depends on SUPPORT_EMMC_RPMB && OPTEE + depends on (SUPPORT_EMMC_RPMB && OPTEE) || SANDBOX_TEE + default y if SANDBOX_TEE + select OPTEE_TA_AVB if SANDBOX_TEE help Enable the commands for reading, writing persistent named values in the Replay Protection Memory Block partition in eMMC by -- 2.34.1
[PATCH v4 2/5] cmd: optee_rpmb: close tee session
Add calls for closing tee session after every read/write operation.
Signed-off-by: Igor Opaniuk
---
(no changes since v1)
cmd/optee_rpmb.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c
index e0e44bbed04..b3cafd92410 100644
--- a/cmd/optee_rpmb.c
+++ b/cmd/optee_rpmb.c
@@ -87,8 +87,10 @@ static int read_persistent_value(const char *name,
rc = tee_shm_alloc(tee, name_size,
TEE_SHM_ALLOC, _name);
- if (rc)
- return -ENOMEM;
+ if (rc) {
+ rc = -ENOMEM;
+ goto close_session;
+ }
rc = tee_shm_alloc(tee, buffer_size,
TEE_SHM_ALLOC, _buf);
@@ -125,6 +127,9 @@ out:
tee_shm_free(shm_buf);
free_name:
tee_shm_free(shm_name);
+close_session:
+ tee_close_session(tee, session);
+ tee = NULL;
return rc;
}
@@ -139,17 +144,20 @@ static int write_persistent_value(const char *name,
struct tee_param param[2];
size_t name_size = strlen(name) + 1;
+ if (!value_size)
+ return -EINVAL;
+
if (!tee) {
if (avb_ta_open_session())
return -ENODEV;
}
- if (!value_size)
- return -EINVAL;
rc = tee_shm_alloc(tee, name_size,
TEE_SHM_ALLOC, _name);
- if (rc)
- return -ENOMEM;
+ if (rc) {
+ rc = -ENOMEM;
+ goto close_session;
+ }
rc = tee_shm_alloc(tee, value_size,
TEE_SHM_ALLOC, _buf);
@@ -178,6 +186,9 @@ out:
tee_shm_free(shm_buf);
free_name:
tee_shm_free(shm_name);
+close_session:
+ tee_close_session(tee, session);
+ tee = NULL;
return rc;
}
--
2.34.1
[PATCH v4 1/5] tee: optee: fix description in Kconfig
Fix OPTEE_TA_AVB symbol description in Kconfig: s/"write"rb"/"write_rb"/g Reviewed-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas Signed-off-by: Igor Opaniuk --- (no changes since v2) Changes in v2: - Applied R-b tags drivers/tee/optee/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig index 9dc65b0501e..db0bcfa6f15 100644 --- a/drivers/tee/optee/Kconfig +++ b/drivers/tee/optee/Kconfig @@ -19,7 +19,7 @@ config OPTEE_TA_AVB default y help Enables support for the AVB Trusted Application (TA) in OP-TEE. - The TA can support the "avb" subcommands "read_rb", "write"rb" + The TA can support the "avb" subcommands "read_rb", "write_rb" and "is_unlocked". config OPTEE_TA_RPC_TEST -- 2.34.1
[PATCH v4 0/5] TEE: minor cleanup
- Address some spelling errors and typos - Support CMD_OPTEE_RPMB for SANDBOX configurations and add python tests - Remove common.h inclusion for drivers/tee - Add calls for closing tee session after every read/write operation. CI build: [1] https://dev.azure.com/igoropaniuk/u-boot/_build/results?buildId=31=results Changes in v4: - Rebased on the latest master and excluded "tee: sandbox: fix spelling errors", as it was merged already by Heinrich Schuchardt Changes in v3: - Added calls for closing tee session after every read/write operation Changes in v2: - Fixed chimp_optee.c:37:9: error: implicit declaration of function 'memset' - Applied R-b and T-b tags Igor Opaniuk (5): tee: optee: fix description in Kconfig cmd: optee_rpmb: close tee session cmd: optee_rpmb: build cmd for sandbox test: py: add optee_rpmb tests tee: remove common.h inclusion cmd/Kconfig| 4 +++- cmd/optee_rpmb.c | 23 +-- drivers/tee/broadcom/chimp_optee.c | 3 ++- drivers/tee/optee/Kconfig | 2 +- drivers/tee/optee/core.c | 1 - drivers/tee/optee/i2c.c| 1 - drivers/tee/optee/rpmb.c | 1 - drivers/tee/optee/supplicant.c | 2 +- drivers/tee/sandbox.c | 2 +- drivers/tee/tee-uclass.c | 1 - test/py/tests/test_optee_rpmb.py | 20 11 files changed, 45 insertions(+), 15 deletions(-) create mode 100644 test/py/tests/test_optee_rpmb.py -- 2.34.1
Re: [PATCH] mvebu: Enable preboot start for pci/usb/scsi/nvme
Hi Dennis, On Tue, Apr 2, 2024 at 3:40 AM Dennis Gilmore wrote: > While preboot was enabled, it did not work as commands are needed to be > run to enable some of the subsystems. This patch starts pci, USB, Sata, > and nvme and makes sure that the system will boot no mater what storage > is in use. > > Applogies for resending, I accidently left the u-boot list off > > Signed-off-by: Dennis Gilmore > --- > configs/mvebu_espressobin-88f3720_defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/configs/mvebu_espressobin-88f3720_defconfig > b/configs/mvebu_espressobin-88f3720_defconfig > index 7ecf5ab0d64..f4f7a809609 100644 > --- a/configs/mvebu_espressobin-88f3720_defconfig > +++ b/configs/mvebu_espressobin-88f3720_defconfig > @@ -21,6 +21,7 @@ CONFIG_AHCI=y > CONFIG_DISTRO_DEFAULTS=y > CONFIG_OF_BOARD_SETUP=y > CONFIG_USE_PREBOOT=y > +CONFIG_PREBOOT="pci enum; usb start; nvme scan; scsi scan;" > CONFIG_SYS_CONSOLE_INFO_QUIET=y > # CONFIG_DISPLAY_CPUINFO is not set > # CONFIG_DISPLAY_BOARDINFO is not set > -- > 2.44.0 > > The issue is in a different place. This board uses distroboot approach (based on what I see in [1]), which is supposed to automatically initialize boot media before trying to boot from it [2]. Initialization of media is forced when the CONFIG_ symbol is enabled, for example CONFIG_NVMEM=y (check BOOTENV_SET_NVME_NEED_INIT define in [2]). In your final board config "# CONFIG_NVMEM is not set" though. IMO, with your proposed fix you'll drastically increase boot time (by probing all buses regardless of what boot media is going to be used next), and hide side effects of misconfiguration. Hope that helps Regards, Igor [1] include/configs/mvebu_armada-37xx.h [2] include/config_distro_bootcmd.h -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk <http://ua.linkedin.com/in/iopaniuk>
Re: [PATCH v3 0/6] TEE: minor cleanup
Ilias, On Mon, Mar 4, 2024 at 6:45 PM Igor Opaniuk wrote: > - Address some spelling errors and typos > - Support CMD_OPTEE_RPMB for SANDBOX configurations and add python tests > - Remove common.h inclusion for drivers/tee > - Add calls for closing tee session after every read/write operation. > > CI build: > [1] > https://dev.azure.com/igoropaniuk/u-boot/_build/results?buildId=28=results > > Changes in v3: > - Added calls for closing tee session after every read/write operation > > Changes in v2: > - Fixed chimp_optee.c:37:9: error: implicit declaration of function > 'memset' > - Applied R-b and T-b tags > > Igor Opaniuk (6): > tee: optee: fix description in Kconfig > tee: sandbox: fix spelling errors > cmd: optee_rpmb: close tee session > cmd: optee_rpmb: build cmd for sandbox > test: py: add optee_rpmb tests > tee: remove common.h inclusion > > cmd/Kconfig| 4 +++- > cmd/optee_rpmb.c | 23 +-- > drivers/tee/broadcom/chimp_optee.c | 3 ++- > drivers/tee/optee/Kconfig | 2 +- > drivers/tee/optee/core.c | 1 - > drivers/tee/optee/i2c.c| 1 - > drivers/tee/optee/rpmb.c | 1 - > drivers/tee/optee/supplicant.c | 2 +- > drivers/tee/sandbox.c | 10 +- > drivers/tee/tee-uclass.c | 1 - > test/py/tests/test_optee_rpmb.py | 20 > 11 files changed, 49 insertions(+), 19 deletions(-) > create mode 100644 test/py/tests/test_optee_rpmb.py > > -- > 2.34.1 > > Are there currently any comments/objections that can prevent these cosmetic changes from being applied? CI is happy now and all tests are passing successfully. If there are any - just let me know, thanks Regards, Igor -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk <http://ua.linkedin.com/in/iopaniuk>
Re: [PATCH v4 2/2] android_ab: Fix ANDROID_AB_BACKUP_OFFSET
log_err("(expected %.8x, found %.8x),",
> > + crc32_le, backup_abc->crc32_le);
> > + } else {
> > + valid_backup = true;
> > + log_info(" copying A/B metadata from
> backup.\n");
> > + memcpy(abc, backup_abc, sizeof(*abc));
> > + }
> > + }
> >
> > + if (!valid_backup) {
> > + log_err(" re-initializing A/B metadata.\n");
> > ret = ab_control_default(abc);
> > if (ret < 0) {
> > -#if ANDROID_AB_BACKUP_OFFSET
> > - free(backup_abc);
> > -#endif
> > + if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
> > + free(backup_abc);
> > free(abc);
> > return -ENODATA;
> > }
> > -#if ANDROID_AB_BACKUP_OFFSET
> > - } else {
> > - /*
> > - * Backup is valid. Copy it to the primary
> > - */
> > - memcpy(abc, backup_abc, sizeof(*abc));
> > }
> > -#endif
> > store_needed = true;
> > }
> >
> > if (abc->magic != BOOT_CTRL_MAGIC) {
> > log_err("ANDROID: Unknown A/B metadata: %.8x\n",
> abc->magic);
> > -#if ANDROID_AB_BACKUP_OFFSET
> > - free(backup_abc);
> > -#endif
> > + if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
> > + free(backup_abc);
> > free(abc);
> > return -ENODATA;
> > }
> > @@ -259,9 +254,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct
> disk_partition *part_info,
> > if (abc->version > BOOT_CTRL_VERSION) {
> > log_err("ANDROID: Unsupported A/B metadata version:
> %.8x\n",
> > abc->version);
> > -#if ANDROID_AB_BACKUP_OFFSET
> > - free(backup_abc);
> > -#endif
> > + if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
> > + free(backup_abc);
> > free(abc);
> > return -ENODATA;
> > }
> > @@ -338,30 +332,29 @@ int ab_select_slot(struct blk_desc *dev_desc,
> struct disk_partition *part_info,
> > abc->crc32_le = ab_control_compute_crc(abc);
> > ret = ab_control_store(dev_desc, part_info, abc, 0);
> > if (ret < 0) {
> > -#if ANDROID_AB_BACKUP_OFFSET
> > - free(backup_abc);
> > -#endif
> > + if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
> > + free(backup_abc);
> > free(abc);
> > return ret;
> > }
> > }
> >
> > -#if ANDROID_AB_BACKUP_OFFSET
> > - /*
> > - * If the backup doesn't match the primary, write the primary
> > - * to the backup offset
> > - */
> > - if (memcmp(backup_abc, abc, sizeof(*abc)) != 0) {
> > - ret = ab_control_store(dev_desc, part_info, abc,
> > -ANDROID_AB_BACKUP_OFFSET);
> > - if (ret < 0) {
> > - free(backup_abc);
> > - free(abc);
> > - return ret;
> > + if (CONFIG_ANDROID_AB_BACKUP_OFFSET) {
> > + /*
> > + * If the backup doesn't match the primary, write the
> primary
> > + * to the backup offset
> > + */
>
> checkpatch.pl seems to complain about this comment block:
>
> WARNING: Block comments should align the * on each line
> #166: FILE: boot/android_ab.c:344:
> + /*
> + * If the backup doesn't match the primary, write the
> primary
>
> To reproduce, run:
>
> $ ./scripts/checkpatch.pl --strict --u-boot --git HEAD^..HEAD
>
> With the above addressed, please send a v5 including:
>
> Reviewed-by: Mattijs Korpershoek
>
> > + if (memcmp(backup_abc, abc, sizeof(*abc)) != 0) {
> > + ret = ab_control_store(dev_desc, part_info, abc,
> > +
> CONFIG_ANDROID_AB_BACKUP_OFFSET);
> > + if (ret < 0) {
> > + free(backup_abc);
> > + free(abc);
> > + return ret;
> > + }
> > }
> > + free(backup_abc);
> > }
> > - free(backup_abc);
> > -#endif
> >
> > free(abc);
> >
> > --
> > 2.34.1
>
With Mattijs comment addressed:
Reviewed-by: Igor Opaniuk
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
https://www.linkedin.com/in/iopaniuk <http://ua.linkedin.com/in/iopaniuk>
Re: [PATCH v4 1/2] android_ab: Add missing semicolon
Hi Colin,
On Tue, Mar 12, 2024 at 1:57 PM Colin McAllister
wrote:
> From: Colin McAllister
>
> Found a missing semicolon in code protected by a #if that will never
> evaluate to true due to a separate issue. Fixing this issue before
> addressing the #if.
>
> Fixes: 3430f24bc6 ("android_ab: Try backup booloader_message")
> Signed-off-by: Colin McAllister
> Cc: Joshua Watt
> Cc: Simon Glass
> Signed-off-by: Colin McAllister
> ---
> v2: No changes
> v3: Added "Fixes:" tag
> v4: No changes
>
> boot/android_ab.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/boot/android_ab.c b/boot/android_ab.c
> index c9df6d2b4b..9a3d15ec60 100644
> --- a/boot/android_ab.c
> +++ b/boot/android_ab.c
> @@ -221,7 +221,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct
> disk_partition *part_info,
> #if ANDROID_AB_BACKUP_OFFSET
> crc32_le = ab_control_compute_crc(backup_abc);
> if (backup_abc->crc32_le != crc32_le) {
> - log_err("ANDROID: Invalid backup CRC-32 ")
> + log_err("ANDROID: Invalid backup CRC-32 ");
> log_err("expected %.8x, found %.8x),",
> crc32_le, backup_abc->crc32_le);
> #endif
> --
> 2.34.1
>
>
Reviewed-by: Igor Opaniuk
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
https://www.linkedin.com/in/iopaniuk <http://ua.linkedin.com/in/iopaniuk>
Re: ECDSA related PRs
Hi Bob, On Thu, Mar 7, 2024 at 12:49 AM Bob Wolff wrote: > Hey all, > I'm not opposed to using the kernel ecdsa.c and have taken a quick look at > `ecdsa_verify()` - but I'd love if someone could point me in the right > direction for how to set up the context and public key. The > akcipher_request structure seems to address both the signature and the > digest, but I am not seeing how to take my public key data and get it > involved. Any examples of usage, possibly? Doing several google searches > did not bear fruit for me. > > Thanks, > bob > I'd start with the image_sign_info/image_region structs definitions in [1] and examples of how they are both filled in RSA signature verification tests in [2], as ecdsa_verify() also uses them. Then I would extend the existing ECDSA dummy test in [3] by adding some test public key data in DER format, data and encrypted hash of this data, like it's done in [2], so you have the way to test your own UCLASS_ECDSA driver implementation. And obviously the existing ST UCLASS_ECDSA driver might be a good reference [4]. Hope this helps! [1] include/image.h [2] test/lib/rsa.c [3] test/dm/ecdsa.c [4] arch/arm/mach-stm32mp/ecdsa_romapi.c > > On Wed, Feb 28, 2024 at 10:57 PM Dan Carpenter > wrote: > > > On Thu, Feb 22, 2024 at 03:07:01PM -0800, Bob Wolff wrote: > > > Peter, > > > Thanks for helping lead me down the right path here. > > > > > > WRT tinycrypt, the license is quite permissive. > > > https://github.com/intel/tinycrypt > > > > > > Also, I'd like your advice - I was thinking for the larger patch that > I'd > > > do it in two commits. The first would be the addition of the tinycrypt > > > files and the second is the actual changes and additions to support > ecdsa > > > verification. I doubt that's controversial. However when I run a trial > > > `patman` against the tinycrypt commit, I geta huge number of issues: > > > *checkpatch.pl <http://checkpatch.pl> found 186 error(s), 380 > > > warning(s), 481 checks(s)* > > > > > > What's your advice on this? I would tend to think we'd want to /not/ > > change > > > the source files directly for such purposes so that updates could be > > > brought in with greater ease. > > > > Yeah. For this kind of thing you wouldn't want to make a bunch of > > checkpatch changes. They sometimes pull crypto and compression > > libraries into the Linux kernel pretty much unmodified as well for the > > same reason. > > > > Igor's proposal about pull this stuff from the Linux kernel instead > > seems like a good idea though. > > > > regards, > > dan carpenter > > > > > -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
Re: [PATCH 2/2] android_ab: Fix ANDROID_AB_BACKUP_OFFSET
ANDROID_AB_BACKUP_OFFSET);
> +
> CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET));
> if (ret < 0) {
> free(backup_abc);
> free(abc);
>
Thanks for the patch. The only suggestion: I see no reason in wrapping it in
preprocessor conditionals (in this particular case), better to use C
conditionals instead (with CONFIG_VAL macro) as explained in [1].
-
> 2.43.2
>
>
>
>
> CONFIDENTIALITY NOTICE: This email and any attachments are for the sole
> use of the intended recipient(s) and contain information that may be Garmin
> confidential and/or Garmin legally privileged. If you have received this
> email in error, please notify the sender by reply email and delete the
> message. Any disclosure, copying, distribution or use of this communication
> (including attachments) by someone other than the intended recipient is
> prohibited. Thank you.
>
[1]
https://www.kernel.org/doc/html/latest/process/coding-style.html#conditional-compilation
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
http://ua.linkedin.com/in/iopaniuk
Re: [PATCH v2 0/5] TEE: minor cleanup
Hi Ilias, On Mon, Mar 4, 2024 at 12:16 PM Ilias Apalodimas < ilias.apalodi...@linaro.org> wrote: > Hi Igor, > > On Sun, 3 Mar 2024 at 00:01, Igor Opaniuk wrote: > > > > > > - Address some spelling errors and typos > > - Support CMD_OPTEE_RPMB for SANDBOX configurations and add python tests > > - Remove common.h inclusion for drivers/tee > > > > Changes in v2: > > - Fixed chimp_optee.c:37:9: error: implicit declaration of function > 'memset' > > - Applied R-b and T-b tags > > https://source.denx.de/u-boot/custodians/u-boot-tpm/-/pipelines/19808 > this seems to have a few failures > > Cheers > /Ilias > > > > Igor Opaniuk (5): > > tee: optee: fix description in Kconfig > > tee: sandbox: fix spelling errors > > cmd: optee_rpmb: build cmd for sandbox > > test: py: add optee_rpmb tests > > tee: remove common.h inclusion > > > > cmd/Kconfig| 4 +++- > > drivers/tee/broadcom/chimp_optee.c | 3 ++- > > drivers/tee/optee/Kconfig | 2 +- > > drivers/tee/optee/core.c | 1 - > > drivers/tee/optee/i2c.c| 1 - > > drivers/tee/optee/rpmb.c | 1 - > > drivers/tee/optee/supplicant.c | 2 +- > > drivers/tee/sandbox.c | 10 +- > > drivers/tee/tee-uclass.c | 1 - > > test/py/tests/test_optee_rpmb.py | 20 > > 10 files changed, 32 insertions(+), 13 deletions(-) > > create mode 100644 test/py/tests/test_optee_rpmb.py > > > > -- > > 2.34.1 > > > I've sent v3. CI build with python tests passing successfully: https://dev.azure.com/igoropaniuk/u-boot/_build/results?buildId=28=results -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
[PATCH v3 6/6] tee: remove common.h inclusion
The usage of the common.h include file is deprecated [1], and has already been removed from several files. Get rid of all inclusions in the "drivers/tee" directory, and replace it with required include files directly where needed. [1] doc/develop/codingstyle.rst Signed-off-by: Igor Opaniuk --- Changes in v2: - Fixed chimp_optee.c:37:9: error: implicit declaration of function 'memset' drivers/tee/broadcom/chimp_optee.c | 3 ++- drivers/tee/optee/core.c | 1 - drivers/tee/optee/i2c.c| 1 - drivers/tee/optee/rpmb.c | 1 - drivers/tee/optee/supplicant.c | 2 +- drivers/tee/sandbox.c | 2 +- drivers/tee/tee-uclass.c | 1 - 7 files changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/tee/broadcom/chimp_optee.c b/drivers/tee/broadcom/chimp_optee.c index 37f9b094f76..bd146ef2899 100644 --- a/drivers/tee/broadcom/chimp_optee.c +++ b/drivers/tee/broadcom/chimp_optee.c @@ -3,9 +3,10 @@ * Copyright 2020 Broadcom. */ -#include #include #include +#include +#include #ifdef CONFIG_CHIMP_OPTEE diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 47f845cffe3..5fc0505c788 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -3,7 +3,6 @@ * Copyright (c) 2018-2020 Linaro Limited */ -#include #include #include #include diff --git a/drivers/tee/optee/i2c.c b/drivers/tee/optee/i2c.c index ef4e10f9912..e3fb99897c5 100644 --- a/drivers/tee/optee/i2c.c +++ b/drivers/tee/optee/i2c.c @@ -3,7 +3,6 @@ * Copyright (c) 2020 Foundries.io Ltd */ -#include #include #include #include diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c index 5bc13757ea8..bacced6af6c 100644 --- a/drivers/tee/optee/rpmb.c +++ b/drivers/tee/optee/rpmb.c @@ -3,7 +3,6 @@ * Copyright (c) 2018 Linaro Limited */ -#include #include #include #include diff --git a/drivers/tee/optee/supplicant.c b/drivers/tee/optee/supplicant.c index f9dd874b594..8a426f53ba8 100644 --- a/drivers/tee/optee/supplicant.c +++ b/drivers/tee/optee/supplicant.c @@ -3,10 +3,10 @@ * Copyright (c) 2018, Linaro Limited */ -#include #include #include #include +#include #include #include "optee_msg.h" diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c index ec66401878c..8ad7c09efdd 100644 --- a/drivers/tee/sandbox.c +++ b/drivers/tee/sandbox.c @@ -2,7 +2,7 @@ /* * Copyright (C) 2018 Linaro Limited */ -#include + #include #include #include diff --git a/drivers/tee/tee-uclass.c b/drivers/tee/tee-uclass.c index 52412a4098e..0194d732193 100644 --- a/drivers/tee/tee-uclass.c +++ b/drivers/tee/tee-uclass.c @@ -5,7 +5,6 @@ #define LOG_CATEGORY UCLASS_TEE -#include #include #include #include -- 2.34.1
[PATCH v3 5/6] test: py: add optee_rpmb tests
Add read/write tests for optee_rpmb cmd.
Signed-off-by: Igor Opaniuk
---
(no changes since v1)
test/py/tests/test_optee_rpmb.py | 20
1 file changed, 20 insertions(+)
create mode 100644 test/py/tests/test_optee_rpmb.py
diff --git a/test/py/tests/test_optee_rpmb.py b/test/py/tests/test_optee_rpmb.py
new file mode 100644
index 000..8a081b5c494
--- /dev/null
+++ b/test/py/tests/test_optee_rpmb.py
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Tests for OP-TEE RPMB read/write support
+
+"""
+This tests optee_rpmb cmd in U-Boot
+"""
+
+import pytest
+import u_boot_utils as util
+
+@pytest.mark.buildconfigspec('cmd_optee_rpmb')
+def test_optee_rpmb_read_write(u_boot_console):
+"""Test OP-TEE RPMB cmd read/write
+"""
+response = u_boot_console.run_command('optee_rpmb write_pvalue
test_variable test_value')
+assert response == 'Wrote 11 bytes'
+
+response = u_boot_console.run_command('optee_rpmb read_pvalue
test_variable 11')
+assert response == 'Read 11 bytes, value = test_value'
\ No newline at end of file
--
2.34.1
[PATCH v3 4/6] cmd: optee_rpmb: build cmd for sandbox
Support CMD_OPTEE_RPMB for SANDBOX configurations. Test: $ ./u-boot -d arch/sandbox/dts/test.dtb ... => optee_rpmb write_pvalue test_variable test_value Wrote 11 bytes => optee_rpmb read_pvalue test_variable 11 Read 11 bytes, value = test_value Reviewed-by: Mattijs Korpershoek Tested-by: Mattijs Korpershoek Signed-off-by: Igor Opaniuk --- (no changes since v2) Changes in v2: - Applied R-b and T-b tags cmd/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index a86b5705174..8ad8c0c542c 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1370,7 +1370,9 @@ config CMD_CLONE config CMD_OPTEE_RPMB bool "Enable read/write support on RPMB via OPTEE" - depends on SUPPORT_EMMC_RPMB && OPTEE + depends on (SUPPORT_EMMC_RPMB && OPTEE) || SANDBOX_TEE + default y if SANDBOX_TEE + select OPTEE_TA_AVB if SANDBOX_TEE help Enable the commands for reading, writing persistent named values in the Replay Protection Memory Block partition in eMMC by -- 2.34.1
[PATCH v3 3/6] cmd: optee_rpmb: close tee session
Add calls for closing tee session after every read/write operation.
Signed-off-by: Igor Opaniuk
---
(no changes since v1)
cmd/optee_rpmb.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c
index e0e44bbed04..b3cafd92410 100644
--- a/cmd/optee_rpmb.c
+++ b/cmd/optee_rpmb.c
@@ -87,8 +87,10 @@ static int read_persistent_value(const char *name,
rc = tee_shm_alloc(tee, name_size,
TEE_SHM_ALLOC, _name);
- if (rc)
- return -ENOMEM;
+ if (rc) {
+ rc = -ENOMEM;
+ goto close_session;
+ }
rc = tee_shm_alloc(tee, buffer_size,
TEE_SHM_ALLOC, _buf);
@@ -125,6 +127,9 @@ out:
tee_shm_free(shm_buf);
free_name:
tee_shm_free(shm_name);
+close_session:
+ tee_close_session(tee, session);
+ tee = NULL;
return rc;
}
@@ -139,17 +144,20 @@ static int write_persistent_value(const char *name,
struct tee_param param[2];
size_t name_size = strlen(name) + 1;
+ if (!value_size)
+ return -EINVAL;
+
if (!tee) {
if (avb_ta_open_session())
return -ENODEV;
}
- if (!value_size)
- return -EINVAL;
rc = tee_shm_alloc(tee, name_size,
TEE_SHM_ALLOC, _name);
- if (rc)
- return -ENOMEM;
+ if (rc) {
+ rc = -ENOMEM;
+ goto close_session;
+ }
rc = tee_shm_alloc(tee, value_size,
TEE_SHM_ALLOC, _buf);
@@ -178,6 +186,9 @@ out:
tee_shm_free(shm_buf);
free_name:
tee_shm_free(shm_name);
+close_session:
+ tee_close_session(tee, session);
+ tee = NULL;
return rc;
}
--
2.34.1
[PATCH v3 2/6] tee: sandbox: fix spelling errors
Fix spelling errors in comments. Reviewed-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas Signed-off-by: Igor Opaniuk --- (no changes since v2) Changes in v2: - Applied R-b tags drivers/tee/sandbox.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c index 86219a9bb1a..ec66401878c 100644 --- a/drivers/tee/sandbox.c +++ b/drivers/tee/sandbox.c @@ -14,7 +14,7 @@ #include "optee/optee_private.h" /* - * The sandbox tee driver tries to emulate a generic Trusted Exectution + * The sandbox tee driver tries to emulate a generic Trusted Execution * Environment (TEE) with the Trusted Applications (TA) OPTEE_TA_AVB and * OPTEE_TA_RPC_TEST available. */ @@ -23,7 +23,7 @@ static const u32 pstorage_max = 16; /** * struct ta_entry - TA entries * @uuid: UUID of an emulated TA - * @open_session Called when a session is openened to the TA + * @open_session Called when a session is opened to the TA * @invoke_funcCalled when a function in the TA is to be invoked * * This struct is used to register TAs in this sandbox emulation of a TEE. @@ -140,8 +140,8 @@ static u32 pta_scp03_invoke_func(struct udevice *dev, u32 func, uint num_params, provisioned = true; /* -* Either way, we asume both operations succeeded and that -* the communication channel has now been stablished +* Either way, we assume both operations succeeded and that +* the communication channel has now been established */ return TEE_SUCCESS; -- 2.34.1
[PATCH v3 1/6] tee: optee: fix description in Kconfig
Fix OPTEE_TA_AVB symbol description in Kconfig: s/"write"rb"/"write_rb"/g Reviewed-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas Signed-off-by: Igor Opaniuk --- (no changes since v2) Changes in v2: - Applied R-b tags drivers/tee/optee/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig index 9dc65b0501e..db0bcfa6f15 100644 --- a/drivers/tee/optee/Kconfig +++ b/drivers/tee/optee/Kconfig @@ -19,7 +19,7 @@ config OPTEE_TA_AVB default y help Enables support for the AVB Trusted Application (TA) in OP-TEE. - The TA can support the "avb" subcommands "read_rb", "write"rb" + The TA can support the "avb" subcommands "read_rb", "write_rb" and "is_unlocked". config OPTEE_TA_RPC_TEST -- 2.34.1
[PATCH v3 0/6] TEE: minor cleanup
- Address some spelling errors and typos - Support CMD_OPTEE_RPMB for SANDBOX configurations and add python tests - Remove common.h inclusion for drivers/tee - Add calls for closing tee session after every read/write operation. CI build: [1] https://dev.azure.com/igoropaniuk/u-boot/_build/results?buildId=28=results Changes in v3: - Added calls for closing tee session after every read/write operation Changes in v2: - Fixed chimp_optee.c:37:9: error: implicit declaration of function 'memset' - Applied R-b and T-b tags Igor Opaniuk (6): tee: optee: fix description in Kconfig tee: sandbox: fix spelling errors cmd: optee_rpmb: close tee session cmd: optee_rpmb: build cmd for sandbox test: py: add optee_rpmb tests tee: remove common.h inclusion cmd/Kconfig| 4 +++- cmd/optee_rpmb.c | 23 +-- drivers/tee/broadcom/chimp_optee.c | 3 ++- drivers/tee/optee/Kconfig | 2 +- drivers/tee/optee/core.c | 1 - drivers/tee/optee/i2c.c| 1 - drivers/tee/optee/rpmb.c | 1 - drivers/tee/optee/supplicant.c | 2 +- drivers/tee/sandbox.c | 10 +- drivers/tee/tee-uclass.c | 1 - test/py/tests/test_optee_rpmb.py | 20 11 files changed, 49 insertions(+), 19 deletions(-) create mode 100644 test/py/tests/test_optee_rpmb.py -- 2.34.1
Re: [PATCH v2 0/5] TEE: minor cleanup
Hi Ilias, On Mon, Mar 4, 2024 at 12:16 PM Ilias Apalodimas < ilias.apalodi...@linaro.org> wrote: > Hi Igor, > > On Sun, 3 Mar 2024 at 00:01, Igor Opaniuk wrote: > > > > > > - Address some spelling errors and typos > > - Support CMD_OPTEE_RPMB for SANDBOX configurations and add python tests > > - Remove common.h inclusion for drivers/tee > > > > Changes in v2: > > - Fixed chimp_optee.c:37:9: error: implicit declaration of function > 'memset' > > - Applied R-b and T-b tags > > https://source.denx.de/u-boot/custodians/u-boot-tpm/-/pipelines/19808 > this seems to have a few failures > > Cheers > /Ilias > > > > Igor Opaniuk (5): > > tee: optee: fix description in Kconfig > > tee: sandbox: fix spelling errors > > cmd: optee_rpmb: build cmd for sandbox > > test: py: add optee_rpmb tests > > tee: remove common.h inclusion > > > > cmd/Kconfig| 4 +++- > > drivers/tee/broadcom/chimp_optee.c | 3 ++- > > drivers/tee/optee/Kconfig | 2 +- > > drivers/tee/optee/core.c | 1 - > > drivers/tee/optee/i2c.c| 1 - > > drivers/tee/optee/rpmb.c | 1 - > > drivers/tee/optee/supplicant.c | 2 +- > > drivers/tee/sandbox.c | 10 +- > > drivers/tee/tee-uclass.c | 1 - > > test/py/tests/test_optee_rpmb.py | 20 > > 10 files changed, 32 insertions(+), 13 deletions(-) > > create mode 100644 test/py/tests/test_optee_rpmb.py > > > > -- > > 2.34.1 > > > It looks like it's a side effect, the test I added revealed a bug in "cmd/optee_rpmb.c" implementation, which I didn't touch (looks like it doesn't close the tee session automatically). I'll address it and add a fix to the patch series. Just a quick question, are Azure pipelines still used (so I can configure my own account and run all these before sending patches, as explained in [1]) [1] https://docs.u-boot.org/en/latest/develop/ci_testing.html -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
Re: [PATCH v1 0/5] TEE: minor cleanup
Hello Ilias, On Fri, Mar 1, 2024 at 10:41 AM Ilias Apalodimas < ilias.apalodi...@linaro.org> wrote: > Hi Igor, > > On Thu, 29 Feb 2024 at 21:11, Igor Opaniuk wrote: > > > > Hi Ilias, > > > > On Wed, Feb 14, 2024 at 7:34 PM Igor Opaniuk > wrote: > >> > >> - Address some spelling errors and typos > >> - Support CMD_OPTEE_RPMB for SANDBOX configurations and add python tests > >> - Remove common.h inclusion for drivers/tee > > CI fails on > https://source.denx.de/u-boot/custodians/u-boot-tpm/-/pipelines/19780 > Can you have a look? > I've fixed and sent v2, thanks! > > Thanks > /Ilias > >> > >> Igor Opaniuk (5): > >> tee: optee: fix description in Kconfig > >> tee: sandbox: fix spelling errors > >> cmd: optee_rpmb: build cmd for sandbox > >> test: py: add optee_rpmb tests > >> tee: remove common.h inclusion > >> > >> cmd/Kconfig| 4 +++- > >> drivers/tee/broadcom/chimp_optee.c | 2 +- > >> drivers/tee/optee/Kconfig | 2 +- > >> drivers/tee/optee/core.c | 1 - > >> drivers/tee/optee/i2c.c| 1 - > >> drivers/tee/optee/rpmb.c | 1 - > >> drivers/tee/optee/supplicant.c | 2 +- > >> drivers/tee/sandbox.c | 10 +- > >> drivers/tee/tee-uclass.c | 1 - > >> test/py/tests/test_optee_rpmb.py | 20 > >> 10 files changed, 31 insertions(+), 13 deletions(-) > >> create mode 100644 test/py/tests/test_optee_rpmb.py > >> > >> -- > >> 2.34.1 > >> > > > > Are there currently any comments/objections that can prevent these > cosmetic > > changes from being applied? If there are - just let me know, thanks > > > > Regards, > > Igor > > > > -- > > Best regards - Atentamente - Meilleures salutations > > > > Igor Opaniuk > > > > mailto: igor.opan...@gmail.com > > skype: igor.opanyuk > > http://ua.linkedin.com/in/iopaniuk > -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
[PATCH v2 5/5] tee: remove common.h inclusion
The usage of the common.h include file is deprecated [1], and has already been removed from several files. Get rid of all inclusions in the "drivers/tee" directory, and replace it with required include files directly where needed. [1] doc/develop/codingstyle.rst Signed-off-by: Igor Opaniuk --- Changes in v2: - Fixed chimp_optee.c:37:9: error: implicit declaration of function 'memset' drivers/tee/broadcom/chimp_optee.c | 3 ++- drivers/tee/optee/core.c | 1 - drivers/tee/optee/i2c.c| 1 - drivers/tee/optee/rpmb.c | 1 - drivers/tee/optee/supplicant.c | 2 +- drivers/tee/sandbox.c | 2 +- drivers/tee/tee-uclass.c | 1 - 7 files changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/tee/broadcom/chimp_optee.c b/drivers/tee/broadcom/chimp_optee.c index 37f9b094f76..bd146ef2899 100644 --- a/drivers/tee/broadcom/chimp_optee.c +++ b/drivers/tee/broadcom/chimp_optee.c @@ -3,9 +3,10 @@ * Copyright 2020 Broadcom. */ -#include #include #include +#include +#include #ifdef CONFIG_CHIMP_OPTEE diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 47f845cffe3..5fc0505c788 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -3,7 +3,6 @@ * Copyright (c) 2018-2020 Linaro Limited */ -#include #include #include #include diff --git a/drivers/tee/optee/i2c.c b/drivers/tee/optee/i2c.c index ef4e10f9912..e3fb99897c5 100644 --- a/drivers/tee/optee/i2c.c +++ b/drivers/tee/optee/i2c.c @@ -3,7 +3,6 @@ * Copyright (c) 2020 Foundries.io Ltd */ -#include #include #include #include diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c index 5bc13757ea8..bacced6af6c 100644 --- a/drivers/tee/optee/rpmb.c +++ b/drivers/tee/optee/rpmb.c @@ -3,7 +3,6 @@ * Copyright (c) 2018 Linaro Limited */ -#include #include #include #include diff --git a/drivers/tee/optee/supplicant.c b/drivers/tee/optee/supplicant.c index f9dd874b594..8a426f53ba8 100644 --- a/drivers/tee/optee/supplicant.c +++ b/drivers/tee/optee/supplicant.c @@ -3,10 +3,10 @@ * Copyright (c) 2018, Linaro Limited */ -#include #include #include #include +#include #include #include "optee_msg.h" diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c index ec66401878c..8ad7c09efdd 100644 --- a/drivers/tee/sandbox.c +++ b/drivers/tee/sandbox.c @@ -2,7 +2,7 @@ /* * Copyright (C) 2018 Linaro Limited */ -#include + #include #include #include diff --git a/drivers/tee/tee-uclass.c b/drivers/tee/tee-uclass.c index 52412a4098e..0194d732193 100644 --- a/drivers/tee/tee-uclass.c +++ b/drivers/tee/tee-uclass.c @@ -5,7 +5,6 @@ #define LOG_CATEGORY UCLASS_TEE -#include #include #include #include -- 2.34.1
[PATCH v2 4/5] test: py: add optee_rpmb tests
Add read/write tests for optee_rpmb cmd.
Signed-off-by: Igor Opaniuk
---
(no changes since v1)
test/py/tests/test_optee_rpmb.py | 20
1 file changed, 20 insertions(+)
create mode 100644 test/py/tests/test_optee_rpmb.py
diff --git a/test/py/tests/test_optee_rpmb.py b/test/py/tests/test_optee_rpmb.py
new file mode 100644
index 000..8a081b5c494
--- /dev/null
+++ b/test/py/tests/test_optee_rpmb.py
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Tests for OP-TEE RPMB read/write support
+
+"""
+This tests optee_rpmb cmd in U-Boot
+"""
+
+import pytest
+import u_boot_utils as util
+
+@pytest.mark.buildconfigspec('cmd_optee_rpmb')
+def test_optee_rpmb_read_write(u_boot_console):
+"""Test OP-TEE RPMB cmd read/write
+"""
+response = u_boot_console.run_command('optee_rpmb write_pvalue
test_variable test_value')
+assert response == 'Wrote 11 bytes'
+
+response = u_boot_console.run_command('optee_rpmb read_pvalue
test_variable 11')
+assert response == 'Read 11 bytes, value = test_value'
\ No newline at end of file
--
2.34.1
[PATCH v2 3/5] cmd: optee_rpmb: build cmd for sandbox
Support CMD_OPTEE_RPMB for SANDBOX configurations. Test: $ ./u-boot -d arch/sandbox/dts/test.dtb ... => optee_rpmb write_pvalue test_variable test_value Wrote 11 bytes => optee_rpmb read_pvalue test_variable 11 Read 11 bytes, value = test_value Reviewed-by: Mattijs Korpershoek Tested-by: Mattijs Korpershoek Signed-off-by: Igor Opaniuk --- Changes in v2: - Applied R-b and T-b tags cmd/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index a86b5705174..8ad8c0c542c 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1370,7 +1370,9 @@ config CMD_CLONE config CMD_OPTEE_RPMB bool "Enable read/write support on RPMB via OPTEE" - depends on SUPPORT_EMMC_RPMB && OPTEE + depends on (SUPPORT_EMMC_RPMB && OPTEE) || SANDBOX_TEE + default y if SANDBOX_TEE + select OPTEE_TA_AVB if SANDBOX_TEE help Enable the commands for reading, writing persistent named values in the Replay Protection Memory Block partition in eMMC by -- 2.34.1
[PATCH v2 2/5] tee: sandbox: fix spelling errors
Fix spelling errors in comments. Reviewed-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas Signed-off-by: Igor Opaniuk --- Changes in v2: - Applied R-b tags drivers/tee/sandbox.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c index 86219a9bb1a..ec66401878c 100644 --- a/drivers/tee/sandbox.c +++ b/drivers/tee/sandbox.c @@ -14,7 +14,7 @@ #include "optee/optee_private.h" /* - * The sandbox tee driver tries to emulate a generic Trusted Exectution + * The sandbox tee driver tries to emulate a generic Trusted Execution * Environment (TEE) with the Trusted Applications (TA) OPTEE_TA_AVB and * OPTEE_TA_RPC_TEST available. */ @@ -23,7 +23,7 @@ static const u32 pstorage_max = 16; /** * struct ta_entry - TA entries * @uuid: UUID of an emulated TA - * @open_session Called when a session is openened to the TA + * @open_session Called when a session is opened to the TA * @invoke_funcCalled when a function in the TA is to be invoked * * This struct is used to register TAs in this sandbox emulation of a TEE. @@ -140,8 +140,8 @@ static u32 pta_scp03_invoke_func(struct udevice *dev, u32 func, uint num_params, provisioned = true; /* -* Either way, we asume both operations succeeded and that -* the communication channel has now been stablished +* Either way, we assume both operations succeeded and that +* the communication channel has now been established */ return TEE_SUCCESS; -- 2.34.1
[PATCH v2 1/5] tee: optee: fix description in Kconfig
Fix OPTEE_TA_AVB symbol description in Kconfig: s/"write"rb"/"write_rb"/g Reviewed-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas Signed-off-by: Igor Opaniuk --- Changes in v2: - Applied R-b tags drivers/tee/optee/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig index 9dc65b0501e..db0bcfa6f15 100644 --- a/drivers/tee/optee/Kconfig +++ b/drivers/tee/optee/Kconfig @@ -19,7 +19,7 @@ config OPTEE_TA_AVB default y help Enables support for the AVB Trusted Application (TA) in OP-TEE. - The TA can support the "avb" subcommands "read_rb", "write"rb" + The TA can support the "avb" subcommands "read_rb", "write_rb" and "is_unlocked". config OPTEE_TA_RPC_TEST -- 2.34.1
[PATCH v2 0/5] TEE: minor cleanup
- Address some spelling errors and typos - Support CMD_OPTEE_RPMB for SANDBOX configurations and add python tests - Remove common.h inclusion for drivers/tee Changes in v2: - Fixed chimp_optee.c:37:9: error: implicit declaration of function 'memset' - Applied R-b and T-b tags Igor Opaniuk (5): tee: optee: fix description in Kconfig tee: sandbox: fix spelling errors cmd: optee_rpmb: build cmd for sandbox test: py: add optee_rpmb tests tee: remove common.h inclusion cmd/Kconfig| 4 +++- drivers/tee/broadcom/chimp_optee.c | 3 ++- drivers/tee/optee/Kconfig | 2 +- drivers/tee/optee/core.c | 1 - drivers/tee/optee/i2c.c| 1 - drivers/tee/optee/rpmb.c | 1 - drivers/tee/optee/supplicant.c | 2 +- drivers/tee/sandbox.c | 10 +- drivers/tee/tee-uclass.c | 1 - test/py/tests/test_optee_rpmb.py | 20 10 files changed, 32 insertions(+), 13 deletions(-) create mode 100644 test/py/tests/test_optee_rpmb.py -- 2.34.1
Re: [PATCH v1] cmd: md5sum: use hash_command
Hi Tom, On Sat, Mar 2, 2024 at 12:31 AM Tom Rini wrote: > On Sun, Feb 11, 2024 at 07:56:16PM +0100, Igor Opaniuk wrote: > > > From: Igor Opaniuk > > > > Drop old implementation and use hash_command() instead, as > > how it's currently done for crc32 and sha1sum cmds. > > > > Test: > > => md5sum 0x6000 0x200 > > md5 for 6000 ... 61ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050 > > > > Signed-off-by: Igor Opaniuk > > --- > > > > cmd/md5sum.c | 149 --- > > 1 file changed, 9 insertions(+), 140 deletions(-) > > This breaks building of imx8mm_phg. > Thanks, fixed and sent v2. > -- > Tom > -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
[PATCH v2] cmd: md5sum: use hash_command
Drop old implementation and use hash_command() instead, as
how it's currently done for crc32 and sha1sum cmds.
Test:
=> md5sum 0x6000 0x200
md5 for 6000 ... 61ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050
Signed-off-by: Igor Opaniuk
---
Changes in v2:
- Addressed build issues on some platforms
cmd/Kconfig | 1 +
cmd/md5sum.c | 150 --
common/hash.c | 8 ++-
3 files changed, 16 insertions(+), 143 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index a86b5705174..622e96ad756 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -825,6 +825,7 @@ config LOOPW
config CMD_MD5SUM
bool "md5sum"
select MD5
+ select HASH
help
Compute MD5 checksum.
diff --git a/cmd/md5sum.c b/cmd/md5sum.c
index 0f0e1d3dd68..ded3f9e1831 100644
--- a/cmd/md5sum.c
+++ b/cmd/md5sum.c
@@ -7,166 +7,36 @@
* Wolfgang Denk, DENX Software Engineering, w...@denx.de.
*/
-#include
#include
#include
#include
+#include
#include
#include
#include
-/*
- * Store the resulting sum to an address or variable
- */
-static void store_result(const u8 *sum, const char *dest)
-{
- unsigned int i;
-
- if (*dest == '*') {
- u8 *ptr;
-
- ptr = (u8 *)hextoul(dest + 1, NULL);
- for (i = 0; i < 16; i++)
- *ptr++ = sum[i];
- } else {
- char str_output[33];
- char *str_ptr = str_output;
-
- for (i = 0; i < 16; i++) {
- sprintf(str_ptr, "%02x", sum[i]);
- str_ptr += 2;
- }
- env_set(dest, str_output);
- }
-}
-
-#ifdef CONFIG_MD5SUM_VERIFY
-static int parse_verify_sum(char *verify_str, u8 *vsum)
-{
- if (*verify_str == '*') {
- u8 *ptr;
-
- ptr = (u8 *)hextoul(verify_str + 1, NULL);
- memcpy(vsum, ptr, 16);
- } else {
- unsigned int i;
- char *vsum_str;
-
- if (strlen(verify_str) == 32)
- vsum_str = verify_str;
- else {
- vsum_str = env_get(verify_str);
- if (vsum_str == NULL || strlen(vsum_str) != 32)
- return 1;
- }
-
- for (i = 0; i < 16; i++) {
- char *nullp = vsum_str + (i + 1) * 2;
- char end = *nullp;
-
- *nullp = '\0';
- *(u8 *)(vsum + i) =
- hextoul(vsum_str + (i * 2), NULL);
- *nullp = end;
- }
- }
- return 0;
-}
-
-int do_md5sum(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+static int do_md5sum(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
{
- ulong addr, len;
- unsigned int i;
- u8 output[16];
- u8 vsum[16];
- int verify = 0;
+ int flags = HASH_FLAG_ENV;
int ac;
- char * const *av;
- void *buf;
+ char *const *av;
if (argc < 3)
return CMD_RET_USAGE;
av = argv + 1;
ac = argc - 1;
- if (strcmp(*av, "-v") == 0) {
- verify = 1;
+ if (IS_ENABLED(CONFIG_MD5SUM_VERIFY) && strcmp(*av, "-v") == 0) {
+ flags |= HASH_FLAG_VERIFY;
av++;
ac--;
- if (ac < 3)
- return CMD_RET_USAGE;
}
- addr = hextoul(*av++, NULL);
- len = hextoul(*av++, NULL);
-
- buf = map_sysmem(addr, len);
- md5_wd(buf, len, output, CHUNKSZ_MD5);
- unmap_sysmem(buf);
-
- if (!verify) {
- printf("md5 for %08lx ... %08lx ==> ", addr, addr + len - 1);
- for (i = 0; i < 16; i++)
- printf("%02x", output[i]);
- printf("\n");
-
- if (ac > 2)
- store_result(output, *av);
- } else {
- char *verify_str = *av++;
-
- if (parse_verify_sum(verify_str, vsum)) {
- printf("ERROR: %s does not contain a valid md5 sum\n",
- verify_str);
- return 1;
- }
- if (memcmp(output, vsum, 16) != 0) {
- printf("md5 for %08lx ... %08lx ==> ", addr,
- addr + len - 1);
- for (i = 0; i < 16; i++)
- printf("%02x", output[i]);
- printf(" != ");
- for (i = 0; i < 16; i++)
- printf("%02x", vsum[i]);
- printf("
Re: [PATCH v1] cmd: md5sum: use hash_command
Hello Tom,
On Sun, Feb 11, 2024 at 7:56 PM Igor Opaniuk
wrote:
> From: Igor Opaniuk
>
> Drop old implementation and use hash_command() instead, as
> how it's currently done for crc32 and sha1sum cmds.
>
> Test:
> => md5sum 0x6000 0x200
> md5 for 6000 ... 61ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050
>
> Signed-off-by: Igor Opaniuk
> ---
>
> cmd/md5sum.c | 149 ---
> 1 file changed, 9 insertions(+), 140 deletions(-)
>
> diff --git a/cmd/md5sum.c b/cmd/md5sum.c
> index 0f0e1d3dd68..618265e8d50 100644
> --- a/cmd/md5sum.c
> +++ b/cmd/md5sum.c
> @@ -7,7 +7,6 @@
> * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
> */
>
> -#include
> #include
> #include
> #include
> @@ -15,158 +14,28 @@
> #include
> #include
>
> -/*
> - * Store the resulting sum to an address or variable
> - */
> -static void store_result(const u8 *sum, const char *dest)
> -{
> - unsigned int i;
> -
> - if (*dest == '*') {
> - u8 *ptr;
> -
> - ptr = (u8 *)hextoul(dest + 1, NULL);
> - for (i = 0; i < 16; i++)
> - *ptr++ = sum[i];
> - } else {
> - char str_output[33];
> - char *str_ptr = str_output;
> -
> - for (i = 0; i < 16; i++) {
> - sprintf(str_ptr, "%02x", sum[i]);
> - str_ptr += 2;
> - }
> - env_set(dest, str_output);
> - }
> -}
> -
> -#ifdef CONFIG_MD5SUM_VERIFY
> -static int parse_verify_sum(char *verify_str, u8 *vsum)
> -{
> - if (*verify_str == '*') {
> - u8 *ptr;
> -
> - ptr = (u8 *)hextoul(verify_str + 1, NULL);
> - memcpy(vsum, ptr, 16);
> - } else {
> - unsigned int i;
> - char *vsum_str;
> -
> - if (strlen(verify_str) == 32)
> - vsum_str = verify_str;
> - else {
> - vsum_str = env_get(verify_str);
> - if (vsum_str == NULL || strlen(vsum_str) != 32)
> - return 1;
> - }
> -
> - for (i = 0; i < 16; i++) {
> - char *nullp = vsum_str + (i + 1) * 2;
> - char end = *nullp;
> -
> - *nullp = '\0';
> - *(u8 *)(vsum + i) =
> - hextoul(vsum_str + (i * 2), NULL);
> - *nullp = end;
> - }
> - }
> - return 0;
> -}
> -
> -int do_md5sum(struct cmd_tbl *cmdtp, int flag, int argc, char *const
> argv[])
> +static int do_md5sum(struct cmd_tbl *cmdtp, int flag, int argc,
> +char *const argv[])
> {
> - ulong addr, len;
> - unsigned int i;
> - u8 output[16];
> - u8 vsum[16];
> - int verify = 0;
> + int flags = HASH_FLAG_ENV;
> int ac;
> - char * const *av;
> - void *buf;
> + char *const *av;
>
> if (argc < 3)
> return CMD_RET_USAGE;
>
> av = argv + 1;
> ac = argc - 1;
> - if (strcmp(*av, "-v") == 0) {
> - verify = 1;
> + if (IS_ENABLED(CONFIG_MD5SUM_VERIFY) && strcmp(*av, "-v") == 0) {
> + flags |= HASH_FLAG_VERIFY;
> av++;
> ac--;
> - if (ac < 3)
> - return CMD_RET_USAGE;
> }
>
> - addr = hextoul(*av++, NULL);
> - len = hextoul(*av++, NULL);
> -
> - buf = map_sysmem(addr, len);
> - md5_wd(buf, len, output, CHUNKSZ_MD5);
> - unmap_sysmem(buf);
> -
> - if (!verify) {
> - printf("md5 for %08lx ... %08lx ==> ", addr, addr + len -
> 1);
> - for (i = 0; i < 16; i++)
> - printf("%02x", output[i]);
> - printf("\n");
> -
> - if (ac > 2)
> - store_result(output, *av);
> - } else {
> - char *verify_str = *av++;
> -
> - if (parse_verify_sum(verify_str, vsum)) {
> - printf("ERROR: %s does not contain a valid md5
> sum\n",
> - verify_str);
> - return 1;
> - }
> - if (memcmp(output, vsum, 16) != 0) {
> - printf("md5 for %08lx ..
Re: [PATCH v1 0/5] TEE: minor cleanup
Hi Ilias, On Wed, Feb 14, 2024 at 7:34 PM Igor Opaniuk wrote: > - Address some spelling errors and typos > - Support CMD_OPTEE_RPMB for SANDBOX configurations and add python tests > - Remove common.h inclusion for drivers/tee > > Igor Opaniuk (5): > tee: optee: fix description in Kconfig > tee: sandbox: fix spelling errors > cmd: optee_rpmb: build cmd for sandbox > test: py: add optee_rpmb tests > tee: remove common.h inclusion > > cmd/Kconfig| 4 +++- > drivers/tee/broadcom/chimp_optee.c | 2 +- > drivers/tee/optee/Kconfig | 2 +- > drivers/tee/optee/core.c | 1 - > drivers/tee/optee/i2c.c| 1 - > drivers/tee/optee/rpmb.c | 1 - > drivers/tee/optee/supplicant.c | 2 +- > drivers/tee/sandbox.c | 10 +- > drivers/tee/tee-uclass.c | 1 - > test/py/tests/test_optee_rpmb.py | 20 > 10 files changed, 31 insertions(+), 13 deletions(-) > create mode 100644 test/py/tests/test_optee_rpmb.py > > -- > 2.34.1 > > Are there currently any comments/objections that can prevent these cosmetic changes from being applied? If there are - just let me know, thanks Regards, Igor -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
Re: [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream
+CC Mattijs
On Mon, Feb 19, 2024 at 11:16 AM Igor Opaniuk
wrote:
> This takes the latest changes from AOSP from [1][2] (as this
> header was split on two) with minimal changes (this could lead
> to warnings reported by checkpatch).
>
> Some local changes have been applied:
> 1. Enable static_assert() for defined structures to be sure
> that all of them have correct sizes.
> 2. Adjuste types in bootloader_control structure with bitfields
> (uint8_t -> uint16_t). It seems that gcc just doesn't like bitfields
> that cross the width of the type. Changing the type doesn't change
> the layout though.
> This addresses this gcc note:
> In file included from boot/android_ab.c:7:
> include/android_bootloader_message.h:230:1: note: offset of packed
> bit-field ‘merge_status’ has changed in GCC 4.4
> 230 | } __attribute__((packed));
>
> [1]
> https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h
> [2]
> https://android.googlesource.com/platform/hardware/interfaces/+/main/boot/1.1/default/boot_control/include/private/boot_control_definition.h
>
> CC: Alex Deymo
> CC: Sam Protsenko
> CC: Eugeniu Rosca
> CC: Simon Glass
> Signed-off-by: Igor Opaniuk
> ---
>
> include/android_bootloader_message.h | 104 +++
> 1 file changed, 92 insertions(+), 12 deletions(-)
>
> diff --git a/include/android_bootloader_message.h
> b/include/android_bootloader_message.h
> index 286d7ab0f31..75198fc9dc2 100644
> --- a/include/android_bootloader_message.h
> +++ b/include/android_bootloader_message.h
> @@ -21,17 +21,22 @@
> * stddef.h
> */
> #include
> +#include
> #endif
>
> // Spaces used by misc partition are as below:
> // 0 - 2K For bootloader_message
> // 2K - 16KUsed by Vendor's bootloader (the 2K - 4K range may be
> optionally used
> // as bootloader_message_ab struct)
> -// 16K - 64KUsed by uncrypt and recovery to store wipe_package for
> A/B devices
> +// 16K - 32KUsed by uncrypt and recovery to store wipe_package for
> A/B devices
> +// 32K - 64KSystem space, used for miscellanious AOSP features. See
> below.
> // Note that these offsets are admitted by bootloader,recovery and
> uncrypt, so they
> // are not configurable without changing all of them.
> static const size_t BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0;
> +static const size_t VENDOR_SPACE_OFFSET_IN_MISC = 2 * 1024;
> static const size_t WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;
> +static const size_t SYSTEM_SPACE_OFFSET_IN_MISC = 32 * 1024;
> +static const size_t SYSTEM_SPACE_SIZE_IN_MISC = 32 * 1024;
>
> /* Bootloader Message (2-KiB)
> *
> @@ -81,24 +86,67 @@ struct bootloader_message {
> char reserved[1184];
> };
>
> +// Holds Virtual A/B merge status information. Current version is 1. New
> fields
> +// must be added to the end.
> +struct misc_virtual_ab_message {
> + uint8_t version;
> + uint32_t magic;
> + uint8_t merge_status; // IBootControl 1.1, MergeStatus enum.
> + uint8_t source_slot; // Slot number when merge_status was written.
> + uint8_t reserved[57];
> +} __attribute__((packed));
> +
> +struct misc_memtag_message {
> + uint8_t version;
> + uint32_t magic; // magic string for treble compat
> + uint32_t memtag_mode;
> + uint8_t reserved[55];
> +} __attribute__((packed));
> +
> +struct misc_kcmdline_message {
> + uint8_t version;
> + uint32_t magic;
> + uint64_t kcmdline_flags;
> + uint8_t reserved[51];
> +} __attribute__((packed));
> +
> +#define MISC_VIRTUAL_AB_MESSAGE_VERSION 2
> +#define MISC_VIRTUAL_AB_MAGIC_HEADER 0x56740AB0
> +
> +#define MISC_MEMTAG_MESSAGE_VERSION 1
> +#define MISC_MEMTAG_MAGIC_HEADER 0x5afefe5a
> +#define MISC_MEMTAG_MODE_MEMTAG 0x1
> +#define MISC_MEMTAG_MODE_MEMTAG_ONCE 0x2
> +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL 0x4
> +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL_ONCE 0x8
> +#define MISC_MEMTAG_MODE_MEMTAG_OFF 0x10
> +// This is set when the state was overridden forcibly. This does not need
> to be
> +// interpreted by the bootloader but is only for bookkeeping purposes so
> +// userspace knows what to do when the override is undone.
> +// See system/extras/mtectrl in AOSP for more information.
> +#define MISC_MEMTAG_MODE_FORCED 0x20
> +
> +#define MISC_KCMDLINE_MESSAGE_VERSION 1
> +#define MISC_KCMDLINE_MAGIC_HEADER 0x6ab5110c
> +#define MISC_KCMDLINE_BINDER_RUST 0x1
> /**
> * We must be cautious when changing the bootloader_message struct size,
> * because A/B-specific fields may end up with different offsets.
> */
> -#ifndef __UBOOT__
> +
> #if (__STDC_VERSION__ >= 201112L)
Re: ECDSA related PRs
Hi Bob, On Wed, Feb 28, 2024 at 7:14 PM Bob Wolff wrote: > > Any thoughts on how to proceed with the issue mentioned about tinycrypt > warnings/checks? > > Also, I'd like your advice - I was thinking for the larger patch that I'd > do it in two commits. The first would be the addition of the tinycrypt > files and the second is the actual changes and additions to support ecdsa > verification. I doubt that's controversial. However when I run a trial > `patman` against the tinycrypt commit, I geta huge number of issues: > *checkpatch.pl <http://checkpatch.pl> found 186 error(s), 380 > warning(s), 481 checks(s)* > > What's your advice on this? I would tend to think we'd want to /not/ change > the source files directly for such purposes so that updates could be > brought in with greater ease. I didn't form any opinion on that, hence asking. Why not to backport existing ECC/ECDSA implementation from Linux kernel (crypto/ecc.c, ./crypto/ecdsa.c), like it was already done for RSA, X509 parser, ASN.1 decoder. Pulling the whole library into the U-Boot source tree only just for ECDSA is a bit overkill IMO. > > > On Thu, Feb 22, 2024 at 3:07 PM Bob Wolff wrote: > > > Peter, > > Thanks for helping lead me down the right path here. > > > > WRT tinycrypt, the license is quite permissive. > > https://github.com/intel/tinycrypt > > > > Also, I'd like your advice - I was thinking for the larger patch that I'd > > do it in two commits. The first would be the addition of the tinycrypt > > files and the second is the actual changes and additions to support ecdsa > > verification. I doubt that's controversial. However when I run a trial > > `patman` against the tinycrypt commit, I geta huge number of issues: > > *checkpatch.pl <http://checkpatch.pl> found 186 error(s), 380 > > warning(s), 481 checks(s)* > > > > What's your advice on this? I would tend to think we'd want to /not/ > > change the source files directly for such purposes so that updates could be > > brought in with greater ease. > > > > Let me know your thoughts. > > > > Thanks, > > Bob Wolff > > > > > > > > On Wed, Feb 21, 2024 at 6:03 AM Peter Robinson > > wrote: > > > >> > >> > >> On Wed, 21 Feb 2024, 11:30 Bob Wolff, wrote: > >> > >>> Hi there, > >>> I have two separate but related pull requests I'd like to contribute. > >>> They > >>> both have to do with ECDSA support. > >>> - The simple one is a lack of null-pointer check that can cause a crash > >>> in > >>> certain situations. Easy peasy. > >>> > >> > >> Just send that one on it's own > >> > >> - The less simple one (and hopefully not too controversial) adds an ecdsa > >>> verify driver (UCLASS_ECDSA) which utilizes tinycrypt to do the crypto > >>> work. > >>> > >> > >> Do we already use tiny crypt in the project, if not things like license > >> need to be taken into account in the context of the GPLv2 > >> > >> Please advise on how best to proceed. Happy to work within the confines of > >>> what works best for the larger group. > >>> > >>> Thanks, > >>> Bob Wolff > >>> > >> -- Best regards - Freundliche Grüsse - Meilleures salutations Igor Opaniuk Senior Software Engineer, Embedded & Security E: igor.opan...@foundries.io W: www.foundries.io
Re: [PATCH v1 5/7] toradex: common: Add sysinfo driver
Toradex config block stored production data on the on-module
> flash device (NAND, NOR or eMMC). The area is normally preserved by
> diff --git a/board/toradex/common/tdx-common.c
> b/board/toradex/common/tdx-common.c
> index 6084436b48b4..1f3253f4222e 100644
> --- a/board/toradex/common/tdx-common.c
> +++ b/board/toradex/common/tdx-common.c
> @@ -3,15 +3,16 @@
> * Copyright (c) 2016 Toradex, Inc.
> */
>
> +#include
> #include
> #include
> #include
> #include
> #include
> +#include
>
> #ifdef CONFIG_VIDEO
> #include
> -#include
> #include
> #include
> #endif
> @@ -103,13 +104,8 @@ __weak int print_bootinfo(void)
>
> int checkboard(void)
> {
> - if (valid_cfgblock) {
> - printf("Model: Toradex %04d %s %s\n",
> - tdx_hw_tag.prodid,
> - toradex_modules[tdx_hw_tag.prodid].name,
> - tdx_board_rev_str);
> + if (valid_cfgblock)
> printf("Serial#: %s\n", tdx_serial_str);
> - }
>
> #ifdef CONFIG_TDX_CFG_BLOCK_EXTRA
> if (tdx_carrier_board_name)
> @@ -188,6 +184,46 @@ static int settings_r(void)
> }
> EVENT_SPY_SIMPLE(EVT_SETTINGS_R, settings_r);
>
> +static int tdx_detect(struct udevice *dev)
> +{
> + return valid_cfgblock ? 0 : -EINVAL;
> +}
> +
> +static int tdx_get_str(struct udevice *dev, int id, size_t size, char *val)
> +{
> + int ret = -ENOTSUPP;
> +
> + switch (id) {
> + case SYSINFO_ID_BOARD_MODEL:
> + snprintf(val, size,
> +"Toradex %04d %s %s",
> +tdx_hw_tag.prodid,
> +toradex_modules[tdx_hw_tag.prodid].name,
> +tdx_board_rev_str);
> +
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +
> +static const struct udevice_id sysinfo_tdx_ids[] = {
> + { .compatible = "toradex,sysinfo" },
> + { /* sentinel */ }
> +};
> +
> +static const struct sysinfo_ops sysinfo_tdx_ops = {
> + .detect = tdx_detect,
> + .get_str= tdx_get_str,
> +};
> +
> +U_BOOT_DRIVER(sysinfo_toradex) = {
> + .name = "sysinfo_toradex",
> + .id = UCLASS_SYSINFO,
> + .of_match = sysinfo_tdx_ids,
> + .ops= _tdx_ops,
> +};
> +
> #ifdef CONFIG_TDX_CFG_BLOCK_USB_GADGET_PID
> int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name)
> {
> --
> 2.39.2
>
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
http://ua.linkedin.com/in/iopaniuk
Re: [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream
Hi Sam,
On Tue, Feb 20, 2024 at 7:29 PM Sam Protsenko
wrote:
>
> On Mon, Feb 19, 2024 at 4:16 AM Igor Opaniuk wrote:
> >
> > This takes the latest changes from AOSP from [1][2] (as this
> > header was split on two) with minimal changes (this could lead
> > to warnings reported by checkpatch).
>
> Do we want to maybe follow that and also carry two different headers
> in U-Boot? Or it doesn't make much sense? I'm thinking in terms of
> future portability mostly: how easy it's to update this header right
> now, and how easy it's going to be further. I didn't form any opinion
> on that, hence asking.
The problem is licensing. android_bootloader_message.h was
re-licensed by Alex Deymo from Google under BSD-3-Clause,
which is GPLv2 compatible. I'm not sure it's legally correct to pull
boot_control_definition.h from AOSP licensed under Apache as a
separate file.
>
> Another thing: are you sure that changing only the header won't break
> anything in U-Boot .c files that use this header?
I've tested both ab_select and avb verify in QEMU. Or do you mean
something else additionally should be tested?
>
> >
> > Some local changes have been applied:
>
> Is it possible to split this work into two patches:
> 1. Bring the original changes only
> 2. Apply all necessary changes for U-Boot
>
> Or does it break the build, etc? Again, thinking in terms of
> portability easiness, and not sure which approach is better -- just
> asking basically.
Yeah, that's the problem, as splitting this on two commits
will lead to the first one reporting warnings/notes.
>
> > 1. Enable static_assert() for defined structures to be sure
> > that all of them have correct sizes.
> > 2. Adjuste types in bootloader_control structure with bitfields
>
> Adjuste -> adjust
Will fix, thanks!
>
> > (uint8_t -> uint16_t). It seems that gcc just doesn't like bitfields
>
> I wonder if all those extra changes can be upstreamed back to AOSP?
> Ideally we'd want to just copy those headers over from AOSP to U-Boot
> with no changes, would make the porting work easier. What are your
> thoughts on that?
Technically we can, I was planning to do that.
>
> > that cross the width of the type. Changing the type doesn't change
> > the layout though.
> > This addresses this gcc note:
> > In file included from boot/android_ab.c:7:
> > include/android_bootloader_message.h:230:1: note: offset of packed
> > bit-field ‘merge_status’ has changed in GCC 4.4
> > 230 | } __attribute__((packed));
> >
> > [1]
> > https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h
> > [2]
> > https://android.googlesource.com/platform/hardware/interfaces/+/main/boot/1.1/default/boot_control/include/private/boot_control_definition.h
> >
> > CC: Alex Deymo
> > CC: Sam Protsenko
> > CC: Eugeniu Rosca
> > CC: Simon Glass
> > Signed-off-by: Igor Opaniuk
> > ---
> >
> > include/android_bootloader_message.h | 104 +++
> > 1 file changed, 92 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/android_bootloader_message.h
> > b/include/android_bootloader_message.h
> > index 286d7ab0f31..75198fc9dc2 100644
> > --- a/include/android_bootloader_message.h
> > +++ b/include/android_bootloader_message.h
> > @@ -21,17 +21,22 @@
> > * stddef.h
> > */
> > #include
> > +#include
> > #endif
> >
> > // Spaces used by misc partition are as below:
> > // 0 - 2K For bootloader_message
> > // 2K - 16KUsed by Vendor's bootloader (the 2K - 4K range may be
> > optionally used
> > // as bootloader_message_ab struct)
> > -// 16K - 64KUsed by uncrypt and recovery to store wipe_package for A/B
> > devices
> > +// 16K - 32KUsed by uncrypt and recovery to store wipe_package for A/B
> > devices
> > +// 32K - 64KSystem space, used for miscellanious AOSP features. See
> > below.
> > // Note that these offsets are admitted by bootloader,recovery and
> > uncrypt, so they
> > // are not configurable without changing all of them.
> > static const size_t BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0;
> > +static const size_t VENDOR_SPACE_OFFSET_IN_MISC = 2 * 1024;
> > static const size_t WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;
> > +static const size_t SYSTEM_SPACE_OFFSET_IN_MISC = 32 * 1024;
> > +static const size_t SYSTEM_SPACE_SIZE_IN_MISC = 32 * 1024;
> >
> > /* Bootloader Message (2-KiB)
> > *
> > @@ -81,24 +86,67 @@ struct bootloader_message {
> &g
Re: LTO build failure with GCC 13.2.1
Hello Sahaj, On Fri, Feb 16, 2024 at 5:50 AM Sahaj Sarup wrote: > > Hi all, > > Since commit f8cebb4f789c9950caf55a0b73e88049e7a1c3a3 enabled LTO by > default for imx8m platforms, > I have been having issues building u-boot. > I am primarily working on imx8mp instead of imx8mm platforms so the > bug resolved by that commit doesn't affect me yet. > But I guess it's a generic issue with u-boot's LTO requirements and GCC 13. > > Build log: > ``` > LTO u-boot > /usr/bin/aarch64-linux-gnu-ld: > /usr/lib/gcc/aarch64-linux-gnu/13/libgcc.a(lse-init.o): in function > `init_have_lse_atomics': > /builddir/build/BUILD/gcc-13.2.1-20230728/aarch64-linux-gnu/aarch64-linux-gnu/libgcc/../../../gcc-13.2.1-20230728/libgcc/config/aarch64/lse-init.c:46: > undefined reference to `__getauxval' > collect2: fatal error: ld terminated with signal 11 [Segmentation > fault], core dumped > compilation terminated. > make: *** [Makefile:1766: u-boot] Error 1 > make: *** Deleting file 'u-boot' > ``` > > GCC Version: > ``` > $ aarch64-linux-gnu-gcc -v > Using built-in specs. > COLLECT_GCC=/usr/bin/aarch64-linux-gnu-gcc > COLLECT_LTO_WRAPPER=/usr/libexec/gcc/aarch64-linux-gnu/13/lto-wrapper > Target: aarch64-linux-gnu > Configured with: ../gcc-13.2.1-20230728/configure --bindir=/usr/bin > --build=x86_64-redhat-linux-gnu --datadir=/usr/share --disable- > decimal-float --disable-dependency-tracking --disable-gold > --disable-libgcj --disable-libgomp --disable-libmpx > --disable-libquadmat > h --disable-libssp --disable-libunwind-exceptions --disable-shared > --disable-silent-rules --disable-sjlj-exceptions --disable-threa > ds --with-ld=/usr/bin/aarch64-linux-gnu-ld --enable-__cxa_atexit > --enable-checking=release --enable-gnu-unique-object --enable-init > fini-array --enable-languages=c,c++ --enable-linker-build-id > --enable-lto --enable-nls --enable-obsolete --enable-plugin --enable-t > argets=all --exec-prefix=/usr --host=x86_64-redhat-linux-gnu > --includedir=/usr/include --infodir=/usr/share/info --libexecdir=/usr/ > libexec --localstatedir=/var --mandir=/usr/share/man --prefix=/usr > --program-prefix=aarch64-linux-gnu- --sbindir=/usr/sbin --shared > statedir=/var/lib --sysconfdir=/etc --target=aarch64-linux-gnu > --with-bugurl=http://bugzilla.redhat.com/bugzilla/ --with-gcc-major- > version-only --with-isl --with-newlib > --with-plugin-ld=/usr/bin/aarch64-linux-gnu-ld > --with-sysroot=/usr/aarch64-linux-gnu/sys-root > --with-system-libunwind --with-system-zlib --without-headers > --enable-gnu-indirect-function --with-linker-hash-style=gnu > Thread model: single > Supported LTO compression algorithms: zlib zstd > gcc version 13.2.1 20230728 (Red Hat Cross 13.2.1-1) (GCC) > ``` > > Thanks, > Sahaj Sarup This might be related [1] (try tocompile with -mno-outline-atomics): "GCC 10.1.0 introduced the -moutline-atomics option which, when enabled, use LSE instructions when the processor provides them. The option is enabled by default and unfortunately causes the following error at compile time: aarch64-linux-gnu-ld: /usr/lib/gcc/aarch64-linux-gnu/10.1.0/libgcc.a(lse-init.o): in function `init_have_lse_atomics': lse-init.c:(.text.startup+0xc): undefined reference to `__getauxval' This is happening because we are linking against our own libcflat which doesn't implement the function __getauxval(). Disable the use of the out-of-line functions by compiling with -mno-outline-atomics." [1] https://patchwork.kernel.org/project/kvm/patch/20200728121751.15083-1-drjo...@redhat.com/ -- Best regards - Freundliche Grüsse - Meilleures salutations Igor Opaniuk Senior Software Engineer, Embedded & Security E: igor.opan...@foundries.io W: www.foundries.io
[PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream
This takes the latest changes from AOSP from [1][2] (as this
header was split on two) with minimal changes (this could lead
to warnings reported by checkpatch).
Some local changes have been applied:
1. Enable static_assert() for defined structures to be sure
that all of them have correct sizes.
2. Adjuste types in bootloader_control structure with bitfields
(uint8_t -> uint16_t). It seems that gcc just doesn't like bitfields
that cross the width of the type. Changing the type doesn't change
the layout though.
This addresses this gcc note:
In file included from boot/android_ab.c:7:
include/android_bootloader_message.h:230:1: note: offset of packed bit-field
‘merge_status’ has changed in GCC 4.4
230 | } __attribute__((packed));
[1]
https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h
[2]
https://android.googlesource.com/platform/hardware/interfaces/+/main/boot/1.1/default/boot_control/include/private/boot_control_definition.h
CC: Alex Deymo
CC: Sam Protsenko
CC: Eugeniu Rosca
CC: Simon Glass
Signed-off-by: Igor Opaniuk
---
include/android_bootloader_message.h | 104 +++
1 file changed, 92 insertions(+), 12 deletions(-)
diff --git a/include/android_bootloader_message.h
b/include/android_bootloader_message.h
index 286d7ab0f31..75198fc9dc2 100644
--- a/include/android_bootloader_message.h
+++ b/include/android_bootloader_message.h
@@ -21,17 +21,22 @@
* stddef.h
*/
#include
+#include
#endif
// Spaces used by misc partition are as below:
// 0 - 2K For bootloader_message
// 2K - 16KUsed by Vendor's bootloader (the 2K - 4K range may be
optionally used
// as bootloader_message_ab struct)
-// 16K - 64KUsed by uncrypt and recovery to store wipe_package for A/B
devices
+// 16K - 32KUsed by uncrypt and recovery to store wipe_package for A/B
devices
+// 32K - 64KSystem space, used for miscellanious AOSP features. See below.
// Note that these offsets are admitted by bootloader,recovery and uncrypt, so
they
// are not configurable without changing all of them.
static const size_t BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0;
+static const size_t VENDOR_SPACE_OFFSET_IN_MISC = 2 * 1024;
static const size_t WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;
+static const size_t SYSTEM_SPACE_OFFSET_IN_MISC = 32 * 1024;
+static const size_t SYSTEM_SPACE_SIZE_IN_MISC = 32 * 1024;
/* Bootloader Message (2-KiB)
*
@@ -81,24 +86,67 @@ struct bootloader_message {
char reserved[1184];
};
+// Holds Virtual A/B merge status information. Current version is 1. New fields
+// must be added to the end.
+struct misc_virtual_ab_message {
+ uint8_t version;
+ uint32_t magic;
+ uint8_t merge_status; // IBootControl 1.1, MergeStatus enum.
+ uint8_t source_slot; // Slot number when merge_status was written.
+ uint8_t reserved[57];
+} __attribute__((packed));
+
+struct misc_memtag_message {
+ uint8_t version;
+ uint32_t magic; // magic string for treble compat
+ uint32_t memtag_mode;
+ uint8_t reserved[55];
+} __attribute__((packed));
+
+struct misc_kcmdline_message {
+ uint8_t version;
+ uint32_t magic;
+ uint64_t kcmdline_flags;
+ uint8_t reserved[51];
+} __attribute__((packed));
+
+#define MISC_VIRTUAL_AB_MESSAGE_VERSION 2
+#define MISC_VIRTUAL_AB_MAGIC_HEADER 0x56740AB0
+
+#define MISC_MEMTAG_MESSAGE_VERSION 1
+#define MISC_MEMTAG_MAGIC_HEADER 0x5afefe5a
+#define MISC_MEMTAG_MODE_MEMTAG 0x1
+#define MISC_MEMTAG_MODE_MEMTAG_ONCE 0x2
+#define MISC_MEMTAG_MODE_MEMTAG_KERNEL 0x4
+#define MISC_MEMTAG_MODE_MEMTAG_KERNEL_ONCE 0x8
+#define MISC_MEMTAG_MODE_MEMTAG_OFF 0x10
+// This is set when the state was overridden forcibly. This does not need to be
+// interpreted by the bootloader but is only for bookkeeping purposes so
+// userspace knows what to do when the override is undone.
+// See system/extras/mtectrl in AOSP for more information.
+#define MISC_MEMTAG_MODE_FORCED 0x20
+
+#define MISC_KCMDLINE_MESSAGE_VERSION 1
+#define MISC_KCMDLINE_MAGIC_HEADER 0x6ab5110c
+#define MISC_KCMDLINE_BINDER_RUST 0x1
/**
* We must be cautious when changing the bootloader_message struct size,
* because A/B-specific fields may end up with different offsets.
*/
-#ifndef __UBOOT__
+
#if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
static_assert(sizeof(struct bootloader_message) == 2048,
"struct bootloader_message size changes, which may break A/B
devices");
#endif
-#endif /* __UBOOT__ */
/**
* The A/B-specific bootloader message structure (4-KiB).
*
* We separate A/B boot control metadata from the regular bootloader
* message struct and keep it here. Everything that's A/B-specific
- * stays after struct bootloader_message, which should be managed by
- * the A/B-bootloader or boot control HAL.
+ * stays after struct bootloader_message, which belongs to the vendor
+ * space of /misc partition. Also, the A/B-specific
[PATCH v1 5/5] tee: remove common.h inclusion
The usage of the common.h include file is deprecated [1], and has already been removed from several files. Get rid of all inclusions in the "drivers/tee" directory, and replace it with required include files directly where needed. [1] doc/develop/codingstyle.rst Signed-off-by: Igor Opaniuk --- drivers/tee/broadcom/chimp_optee.c | 2 +- drivers/tee/optee/core.c | 1 - drivers/tee/optee/i2c.c| 1 - drivers/tee/optee/rpmb.c | 1 - drivers/tee/optee/supplicant.c | 2 +- drivers/tee/sandbox.c | 2 +- drivers/tee/tee-uclass.c | 1 - 7 files changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/tee/broadcom/chimp_optee.c b/drivers/tee/broadcom/chimp_optee.c index 37f9b094f76..c142ebd542f 100644 --- a/drivers/tee/broadcom/chimp_optee.c +++ b/drivers/tee/broadcom/chimp_optee.c @@ -3,9 +3,9 @@ * Copyright 2020 Broadcom. */ -#include #include #include +#include #ifdef CONFIG_CHIMP_OPTEE diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 47f845cffe3..5fc0505c788 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -3,7 +3,6 @@ * Copyright (c) 2018-2020 Linaro Limited */ -#include #include #include #include diff --git a/drivers/tee/optee/i2c.c b/drivers/tee/optee/i2c.c index ef4e10f9912..e3fb99897c5 100644 --- a/drivers/tee/optee/i2c.c +++ b/drivers/tee/optee/i2c.c @@ -3,7 +3,6 @@ * Copyright (c) 2020 Foundries.io Ltd */ -#include #include #include #include diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c index 5bc13757ea8..bacced6af6c 100644 --- a/drivers/tee/optee/rpmb.c +++ b/drivers/tee/optee/rpmb.c @@ -3,7 +3,6 @@ * Copyright (c) 2018 Linaro Limited */ -#include #include #include #include diff --git a/drivers/tee/optee/supplicant.c b/drivers/tee/optee/supplicant.c index f9dd874b594..8a426f53ba8 100644 --- a/drivers/tee/optee/supplicant.c +++ b/drivers/tee/optee/supplicant.c @@ -3,10 +3,10 @@ * Copyright (c) 2018, Linaro Limited */ -#include #include #include #include +#include #include #include "optee_msg.h" diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c index ec66401878c..8ad7c09efdd 100644 --- a/drivers/tee/sandbox.c +++ b/drivers/tee/sandbox.c @@ -2,7 +2,7 @@ /* * Copyright (C) 2018 Linaro Limited */ -#include + #include #include #include diff --git a/drivers/tee/tee-uclass.c b/drivers/tee/tee-uclass.c index 52412a4098e..0194d732193 100644 --- a/drivers/tee/tee-uclass.c +++ b/drivers/tee/tee-uclass.c @@ -5,7 +5,6 @@ #define LOG_CATEGORY UCLASS_TEE -#include #include #include #include -- 2.34.1
[PATCH v1 4/5] test: py: add optee_rpmb tests
Add read/write tests for optee_rpmb cmd.
Signed-off-by: Igor Opaniuk
---
test/py/tests/test_optee_rpmb.py | 20
1 file changed, 20 insertions(+)
create mode 100644 test/py/tests/test_optee_rpmb.py
diff --git a/test/py/tests/test_optee_rpmb.py b/test/py/tests/test_optee_rpmb.py
new file mode 100644
index 000..8a081b5c494
--- /dev/null
+++ b/test/py/tests/test_optee_rpmb.py
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Tests for OP-TEE RPMB read/write support
+
+"""
+This tests optee_rpmb cmd in U-Boot
+"""
+
+import pytest
+import u_boot_utils as util
+
+@pytest.mark.buildconfigspec('cmd_optee_rpmb')
+def test_optee_rpmb_read_write(u_boot_console):
+"""Test OP-TEE RPMB cmd read/write
+"""
+response = u_boot_console.run_command('optee_rpmb write_pvalue
test_variable test_value')
+assert response == 'Wrote 11 bytes'
+
+response = u_boot_console.run_command('optee_rpmb read_pvalue
test_variable 11')
+assert response == 'Read 11 bytes, value = test_value'
\ No newline at end of file
--
2.34.1
[PATCH v1 2/5] tee: sandbox: fix spelling errors
Fix spelling errors in comments. Signed-off-by: Igor Opaniuk --- drivers/tee/sandbox.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c index 86219a9bb1a..ec66401878c 100644 --- a/drivers/tee/sandbox.c +++ b/drivers/tee/sandbox.c @@ -14,7 +14,7 @@ #include "optee/optee_private.h" /* - * The sandbox tee driver tries to emulate a generic Trusted Exectution + * The sandbox tee driver tries to emulate a generic Trusted Execution * Environment (TEE) with the Trusted Applications (TA) OPTEE_TA_AVB and * OPTEE_TA_RPC_TEST available. */ @@ -23,7 +23,7 @@ static const u32 pstorage_max = 16; /** * struct ta_entry - TA entries * @uuid: UUID of an emulated TA - * @open_session Called when a session is openened to the TA + * @open_session Called when a session is opened to the TA * @invoke_funcCalled when a function in the TA is to be invoked * * This struct is used to register TAs in this sandbox emulation of a TEE. @@ -140,8 +140,8 @@ static u32 pta_scp03_invoke_func(struct udevice *dev, u32 func, uint num_params, provisioned = true; /* -* Either way, we asume both operations succeeded and that -* the communication channel has now been stablished +* Either way, we assume both operations succeeded and that +* the communication channel has now been established */ return TEE_SUCCESS; -- 2.34.1
[PATCH v1 1/5] tee: optee: fix description in Kconfig
Fix OPTEE_TA_AVB symbol description in Kconfig: s/"write"rb"/"write_rb"/g Signed-off-by: Igor Opaniuk --- drivers/tee/optee/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig index 9dc65b0501e..db0bcfa6f15 100644 --- a/drivers/tee/optee/Kconfig +++ b/drivers/tee/optee/Kconfig @@ -19,7 +19,7 @@ config OPTEE_TA_AVB default y help Enables support for the AVB Trusted Application (TA) in OP-TEE. - The TA can support the "avb" subcommands "read_rb", "write"rb" + The TA can support the "avb" subcommands "read_rb", "write_rb" and "is_unlocked". config OPTEE_TA_RPC_TEST -- 2.34.1
[PATCH v1 3/5] cmd: optee_rpmb: build cmd for sandbox
Support CMD_OPTEE_RPMB for SANDBOX configurations. Test: $ ./u-boot -d arch/sandbox/dts/test.dtb ... => optee_rpmb write_pvalue test_variable test_value Wrote 11 bytes => optee_rpmb read_pvalue test_variable 11 Read 11 bytes, value = test_value Signed-off-by: Igor Opaniuk --- cmd/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index a86b5705174..8ad8c0c542c 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1370,7 +1370,9 @@ config CMD_CLONE config CMD_OPTEE_RPMB bool "Enable read/write support on RPMB via OPTEE" - depends on SUPPORT_EMMC_RPMB && OPTEE + depends on (SUPPORT_EMMC_RPMB && OPTEE) || SANDBOX_TEE + default y if SANDBOX_TEE + select OPTEE_TA_AVB if SANDBOX_TEE help Enable the commands for reading, writing persistent named values in the Replay Protection Memory Block partition in eMMC by -- 2.34.1
[PATCH v1 0/5] TEE: minor cleanup
- Address some spelling errors and typos - Support CMD_OPTEE_RPMB for SANDBOX configurations and add python tests - Remove common.h inclusion for drivers/tee Igor Opaniuk (5): tee: optee: fix description in Kconfig tee: sandbox: fix spelling errors cmd: optee_rpmb: build cmd for sandbox test: py: add optee_rpmb tests tee: remove common.h inclusion cmd/Kconfig| 4 +++- drivers/tee/broadcom/chimp_optee.c | 2 +- drivers/tee/optee/Kconfig | 2 +- drivers/tee/optee/core.c | 1 - drivers/tee/optee/i2c.c| 1 - drivers/tee/optee/rpmb.c | 1 - drivers/tee/optee/supplicant.c | 2 +- drivers/tee/sandbox.c | 10 +- drivers/tee/tee-uclass.c | 1 - test/py/tests/test_optee_rpmb.py | 20 10 files changed, 31 insertions(+), 13 deletions(-) create mode 100644 test/py/tests/test_optee_rpmb.py -- 2.34.1
Re: [PATCH v2 1/7] common: avb_verify: don't call mmc_switch_part for SD
Hi Mattijs, On Tue, Feb 13, 2024 at 9:13 AM Mattijs Korpershoek wrote: > > Hi Igor, > > On lun., févr. 12, 2024 at 09:05, Igor Opaniuk wrote: > > > Hi Dan, > > > > On Mon, Feb 12, 2024 at 8:05 AM Dan Carpenter > > wrote: > >> > >> On Fri, Feb 09, 2024 at 08:20:39PM +0100, Igor Opaniuk wrote: > >> > From: Igor Opaniuk > >> > > >> > mmc_switch_part() is used for switching between hw partitions > >> > on eMMC (boot0, boot1, user, rpmb). > >> > There is no need to do that for SD card. > >> > > >> > >> Is this a clean up or a bugfix? What are the visible effects for the > >> user? > > avb cmd fails for SD cards, as mmc_switch_part() fails after trying to > > access > > EXT_CSD register, which obviously is not available. > > Does this means that we only need this patch to fix AVB commands when > booting from SD cards? > > If yes, I propose adding the following note to the commit message: > > "This fixes the avb command usage on on SD cards." > > If you agree, I can do this while applying. Yes, could you please add to the commit message so I don't send v3 for that (if there are no any additional objections/comments) Thanks > > If not, we can keep the message as is. > > >> > >> regards, > >> dan carpenter > >> > > > > > > -- > > Best regards - Atentamente - Meilleures salutations > > > > Igor Opaniuk > > > > mailto: igor.opan...@gmail.com > > skype: igor.opanyuk > > http://ua.linkedin.com/in/iopaniuk -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
[RESEND PATCH v1] MAINTAINERS: add custodian tree info for AVB/AB
From: Igor Opaniuk Add information about a custodian tree [1] for AVB/AB, which is maintained by Mattijs Korpershoek. [1] https://source.denx.de/u-boot/custodians/u-boot-dfu Signed-off-by: Igor Opaniuk --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 46ba17647f3..32f3ee30710 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -62,6 +62,7 @@ M:Igor Opaniuk M: Mattijs Korpershoek R: Sam Protsenko S: Maintained +T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git F: boot/android_ab.c F: cmd/ab_select.c F: doc/android/ab.rst @@ -72,6 +73,7 @@ ANDROID AVB M: Igor Opaniuk M: Mattijs Korpershoek S: Maintained +T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git F: cmd/avb.c F: common/avb_verify.c F: doc/android/avb2.rst -- 2.34.1
[PATCH v1] MAINTAINERS: add custodian tree info for AV/AVB
From: Igor Opaniuk Add information about a custodian tree [1] for AVB/AB, which is maintained by Mattijs Korpershoek. [1] https://source.denx.de/u-boot/custodians/u-boot-dfu Signed-off-by: Igor Opaniuk --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 46ba17647f3..32f3ee30710 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -62,6 +62,7 @@ M:Igor Opaniuk M: Mattijs Korpershoek R: Sam Protsenko S: Maintained +T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git F: boot/android_ab.c F: cmd/ab_select.c F: doc/android/ab.rst @@ -72,6 +73,7 @@ ANDROID AVB M: Igor Opaniuk M: Mattijs Korpershoek S: Maintained +T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git F: cmd/avb.c F: common/avb_verify.c F: doc/android/avb2.rst -- 2.34.1
[PATCH v2] armv8: crypto: SHA-512 using ARMv8 Crypto Extensions
From: Igor Opaniuk Add support for the SHA-512 Secure Hash Algorithm which uses ARMv8 Crypto Extensions. The CPU should support ARMv8.2 instruction set and implement SHA512H, SHA512H2, SHA512SU0, and SHA512SU1 instructions. This information can be obtained from ID_AA64ISAR0_EL1 (AArch64 Instruction Set Attribute Register 0), bits [15:12] should be 0b0010 [1], that indicates support for SHA512* instructions in AArch64 state. As not all ARMv8-base SoCs support that, ARMV8_CE_SHA512 is left disabled by default for now. Validated in QEMU for ARMv8 with compiled-in SHA-2 support (because of absence of hw with ARMv8.2-A ready SoC at hand): => hash sha512 0x4020 0x200 sha512 for 4020 ... 421f ==> 1aeae269f4eb7c37... The implementation is based on original implementation from Ard Biesheuvel in Linux kernel [2] [1] https://developer.arm.com/documentation/ddi0601/2023-12/AArch64-Registers/ID-AA64ISAR0-EL1--AArch64-Instruction-Set-Attribute-Register-0 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/crypto/sha2-ce-core.S CC: Ard Biesheuvel > CC: Loic Poulain Signed-off-by: Igor Opaniuk --- Changes in v2: - Dropped "Default n" in Kconfig as Michal Simek suggested - Adjusted commit message about QEMU validation arch/arm/cpu/armv8/Kconfig | 4 + arch/arm/cpu/armv8/Makefile | 1 + arch/arm/cpu/armv8/sha512_ce_core.S | 210 arch/arm/cpu/armv8/sha512_ce_glue.c | 20 +++ lib/sha512.c| 6 +- 5 files changed, 239 insertions(+), 2 deletions(-) create mode 100644 arch/arm/cpu/armv8/sha512_ce_core.S create mode 100644 arch/arm/cpu/armv8/sha512_ce_glue.c diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig index 9f0fb369f7..37b8b60914 100644 --- a/arch/arm/cpu/armv8/Kconfig +++ b/arch/arm/cpu/armv8/Kconfig @@ -204,6 +204,10 @@ config ARMV8_CE_SHA256 bool "SHA-256 digest algorithm (ARMv8 Crypto Extensions)" default y if SHA256 +config ARMV8_CE_SHA512 + bool "SHA-512 digest algorithm (ARMv8 Crypto Extensions)" + depends on SHA512 + endif endif diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile index bba4f570db..3894f2bb2a 100644 --- a/arch/arm/cpu/armv8/Makefile +++ b/arch/arm/cpu/armv8/Makefile @@ -45,3 +45,4 @@ obj-$(CONFIG_TARGET_BCMNS3) += bcmns3/ obj-$(CONFIG_XEN) += xen/ obj-$(CONFIG_ARMV8_CE_SHA1) += sha1_ce_glue.o sha1_ce_core.o obj-$(CONFIG_ARMV8_CE_SHA256) += sha256_ce_glue.o sha256_ce_core.o +obj-$(CONFIG_ARMV8_CE_SHA512) += sha512_ce_glue.o sha512_ce_core.o \ No newline at end of file diff --git a/arch/arm/cpu/armv8/sha512_ce_core.S b/arch/arm/cpu/armv8/sha512_ce_core.S new file mode 100644 index 00..906291f35b --- /dev/null +++ b/arch/arm/cpu/armv8/sha512_ce_core.S @@ -0,0 +1,210 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * sha512-ce-core.S - core SHA-384/SHA-512 transform using v8 Crypto + * Extensions + * + * Copyright (C) 2018 Linaro Ltd + * Copyright (C) 2024 Igor Opaniuk + */ + + #include + #include + #include + #include + + .macro adr_l, dst, sym + adrp\dst, \sym + add \dst, \dst, :lo12:\sym + .endm + + .irpb,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19 + .set.Lq\b, \b + .set.Lv\b\().2d, \b + .endr + + .macro sha512h, rd, rn, rm + .inst 0xce608000 | .L\rd | (.L\rn << 5) | (.L\rm << 16) + .endm + + .macro sha512h2, rd, rn, rm + .inst 0xce608400 | .L\rd | (.L\rn << 5) | (.L\rm << 16) + .endm + + .macro sha512su0, rd, rn + .inst 0xcec08000 | .L\rd | (.L\rn << 5) + .endm + + .macro sha512su1, rd, rn, rm + .inst 0xce608800 | .L\rd | (.L\rn << 5) | (.L\rm << 16) + .endm + + /* +* The SHA-512 round constants +*/ + .section".rodata", "a" + .align 4 +.Lsha512_rcon: + .quad 0x428a2f98d728ae22, 0x7137449123ef65cd + .quad 0xb5c0fbcfec4d3b2f, 0xe9b5dba58189dbbc + .quad 0x3956c25bf348b538, 0x59f111f1b605d019 + .quad 0x923f82a4af194f9b, 0xab1c5ed5da6d8118 + .quad 0xd807aa98a3030242, 0x12835b0145706fbe + .quad 0x243185be4ee4b28c, 0x550c7dc3d5ffb4e2 + .quad 0x72be5d74f27b896f, 0x80deb1fe3b1696b1 + .quad 0x9bdc06a725c71235, 0xc19bf174cf692694 + .quad 0xe49b69c19ef14ad2, 0xefbe4786384f25e3 + .quad 0x0fc19dc68b8cd5b5, 0x240ca1cc77ac9c65 + .quad 0x2de92c6f592b0275, 0x4a7484aa6ea6e483 + .quad 0x5cb0a9dcbd41fbd4, 0x76f988da831153b5 + .quad 0x983e5152ee66dfab, 0xa831c66d2db43210 +
Re: [PATCH] configs: khadas-vim3*_android: fix AVB oom error
On Fri, Feb 9, 2024 at 10:07 AM Mattijs Korpershoek
wrote:
>
> When booting Android with AVB enabled, an OOM is observed:
>
> => avb init ${mmcdev}
> => avb verify _a
> ## Android Verified Boot 2.0 version 1.1.0
> read_is_device_unlocked not supported yet
> read_rollback_index not supported yet
> avb_util.c:182: ERROR: Failed to allocate memory.
> OOM error occurred during verification
>
> A custom malloc length of 128MB is required as documented in
> commit 285a83b12bdf ("configs: meson64_android: increase SYS_MALLOC_LEN to
> 128M for AVB")
>
> However, this 128M custom malloc length was not ported to Kconfig in
> commit 7cfbba36e9f8 ("Convert CONFIG_SYS_MALLOC_LEN to Kconfig")
>
> Add it back to fix AVB verification on VIM3/VIM3L.
>
> Fixes: 7cfbba36e9f8 ("Convert CONFIG_SYS_MALLOC_LEN to Kconfig")
> Co-developed-by: Guillaume La Roque
> Signed-off-by: Guillaume La Roque
> Signed-off-by: Mattijs Korpershoek
> ---
> configs/khadas-vim3_android_ab_defconfig | 1 +
> configs/khadas-vim3_android_defconfig | 1 +
> configs/khadas-vim3l_android_ab_defconfig | 1 +
> configs/khadas-vim3l_android_defconfig| 1 +
> 4 files changed, 4 insertions(+)
>
> diff --git a/configs/khadas-vim3_android_ab_defconfig
> b/configs/khadas-vim3_android_ab_defconfig
> index b41c2660fff0..ee62fe36d414 100644
> --- a/configs/khadas-vim3_android_ab_defconfig
> +++ b/configs/khadas-vim3_android_ab_defconfig
> @@ -3,6 +3,7 @@ CONFIG_SYS_BOARD="vim3"
> CONFIG_SYS_CONFIG_NAME="khadas-vim3_android"
> CONFIG_ARCH_MESON=y
> CONFIG_TEXT_BASE=0x0100
> +CONFIG_SYS_MALLOC_LEN=0x0800
> CONFIG_NR_DRAM_BANKS=1
> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x2000
> diff --git a/configs/khadas-vim3_android_defconfig
> b/configs/khadas-vim3_android_defconfig
> index 88197f5f5e99..cecbe5061793 100644
> --- a/configs/khadas-vim3_android_defconfig
> +++ b/configs/khadas-vim3_android_defconfig
> @@ -3,6 +3,7 @@ CONFIG_SYS_BOARD="vim3"
> CONFIG_SYS_CONFIG_NAME="khadas-vim3_android"
> CONFIG_ARCH_MESON=y
> CONFIG_TEXT_BASE=0x0100
> +CONFIG_SYS_MALLOC_LEN=0x0800
> CONFIG_NR_DRAM_BANKS=1
> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x2000
> diff --git a/configs/khadas-vim3l_android_ab_defconfig
> b/configs/khadas-vim3l_android_ab_defconfig
> index 3381d2e92701..ec4e0dc72e22 100644
> --- a/configs/khadas-vim3l_android_ab_defconfig
> +++ b/configs/khadas-vim3l_android_ab_defconfig
> @@ -3,6 +3,7 @@ CONFIG_SYS_BOARD="vim3"
> CONFIG_SYS_CONFIG_NAME="khadas-vim3l_android"
> CONFIG_ARCH_MESON=y
> CONFIG_TEXT_BASE=0x0100
> +CONFIG_SYS_MALLOC_LEN=0x0800
> CONFIG_NR_DRAM_BANKS=1
> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x2000
> diff --git a/configs/khadas-vim3l_android_defconfig
> b/configs/khadas-vim3l_android_defconfig
> index 3fa587ef1db5..206f8defca6b 100644
> --- a/configs/khadas-vim3l_android_defconfig
> +++ b/configs/khadas-vim3l_android_defconfig
> @@ -3,6 +3,7 @@ CONFIG_SYS_BOARD="vim3"
> CONFIG_SYS_CONFIG_NAME="khadas-vim3l_android"
> CONFIG_ARCH_MESON=y
> CONFIG_TEXT_BASE=0x0100
> +CONFIG_SYS_MALLOC_LEN=0x0800
> CONFIG_NR_DRAM_BANKS=1
> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x2000
>
> ---
> base-commit: 076529725f16f07a5cb2d5feba25d62b5f5a5872
> change-id: 20240209-vim3-avb-malloc-aa3de534d6a0
>
> Best regards,
> --
> Mattijs Korpershoek
>
Reviewed-by: Igor Opaniuk
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
http://ua.linkedin.com/in/iopaniuk
Re: [PATCH v2 1/7] common: avb_verify: don't call mmc_switch_part for SD
Hi Dan, On Mon, Feb 12, 2024 at 8:05 AM Dan Carpenter wrote: > > On Fri, Feb 09, 2024 at 08:20:39PM +0100, Igor Opaniuk wrote: > > From: Igor Opaniuk > > > > mmc_switch_part() is used for switching between hw partitions > > on eMMC (boot0, boot1, user, rpmb). > > There is no need to do that for SD card. > > > > Is this a clean up or a bugfix? What are the visible effects for the > user? avb cmd fails for SD cards, as mmc_switch_part() fails after trying to access EXT_CSD register, which obviously is not available. > > regards, > dan carpenter > -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
[PATCH v1] cmd: md5sum: use hash_command
From: Igor Opaniuk
Drop old implementation and use hash_command() instead, as
how it's currently done for crc32 and sha1sum cmds.
Test:
=> md5sum 0x6000 0x200
md5 for 6000 ... 61ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050
Signed-off-by: Igor Opaniuk
---
cmd/md5sum.c | 149 ---
1 file changed, 9 insertions(+), 140 deletions(-)
diff --git a/cmd/md5sum.c b/cmd/md5sum.c
index 0f0e1d3dd68..618265e8d50 100644
--- a/cmd/md5sum.c
+++ b/cmd/md5sum.c
@@ -7,7 +7,6 @@
* Wolfgang Denk, DENX Software Engineering, w...@denx.de.
*/
-#include
#include
#include
#include
@@ -15,158 +14,28 @@
#include
#include
-/*
- * Store the resulting sum to an address or variable
- */
-static void store_result(const u8 *sum, const char *dest)
-{
- unsigned int i;
-
- if (*dest == '*') {
- u8 *ptr;
-
- ptr = (u8 *)hextoul(dest + 1, NULL);
- for (i = 0; i < 16; i++)
- *ptr++ = sum[i];
- } else {
- char str_output[33];
- char *str_ptr = str_output;
-
- for (i = 0; i < 16; i++) {
- sprintf(str_ptr, "%02x", sum[i]);
- str_ptr += 2;
- }
- env_set(dest, str_output);
- }
-}
-
-#ifdef CONFIG_MD5SUM_VERIFY
-static int parse_verify_sum(char *verify_str, u8 *vsum)
-{
- if (*verify_str == '*') {
- u8 *ptr;
-
- ptr = (u8 *)hextoul(verify_str + 1, NULL);
- memcpy(vsum, ptr, 16);
- } else {
- unsigned int i;
- char *vsum_str;
-
- if (strlen(verify_str) == 32)
- vsum_str = verify_str;
- else {
- vsum_str = env_get(verify_str);
- if (vsum_str == NULL || strlen(vsum_str) != 32)
- return 1;
- }
-
- for (i = 0; i < 16; i++) {
- char *nullp = vsum_str + (i + 1) * 2;
- char end = *nullp;
-
- *nullp = '\0';
- *(u8 *)(vsum + i) =
- hextoul(vsum_str + (i * 2), NULL);
- *nullp = end;
- }
- }
- return 0;
-}
-
-int do_md5sum(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+static int do_md5sum(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
{
- ulong addr, len;
- unsigned int i;
- u8 output[16];
- u8 vsum[16];
- int verify = 0;
+ int flags = HASH_FLAG_ENV;
int ac;
- char * const *av;
- void *buf;
+ char *const *av;
if (argc < 3)
return CMD_RET_USAGE;
av = argv + 1;
ac = argc - 1;
- if (strcmp(*av, "-v") == 0) {
- verify = 1;
+ if (IS_ENABLED(CONFIG_MD5SUM_VERIFY) && strcmp(*av, "-v") == 0) {
+ flags |= HASH_FLAG_VERIFY;
av++;
ac--;
- if (ac < 3)
- return CMD_RET_USAGE;
}
- addr = hextoul(*av++, NULL);
- len = hextoul(*av++, NULL);
-
- buf = map_sysmem(addr, len);
- md5_wd(buf, len, output, CHUNKSZ_MD5);
- unmap_sysmem(buf);
-
- if (!verify) {
- printf("md5 for %08lx ... %08lx ==> ", addr, addr + len - 1);
- for (i = 0; i < 16; i++)
- printf("%02x", output[i]);
- printf("\n");
-
- if (ac > 2)
- store_result(output, *av);
- } else {
- char *verify_str = *av++;
-
- if (parse_verify_sum(verify_str, vsum)) {
- printf("ERROR: %s does not contain a valid md5 sum\n",
- verify_str);
- return 1;
- }
- if (memcmp(output, vsum, 16) != 0) {
- printf("md5 for %08lx ... %08lx ==> ", addr,
- addr + len - 1);
- for (i = 0; i < 16; i++)
- printf("%02x", output[i]);
- printf(" != ");
- for (i = 0; i < 16; i++)
- printf("%02x", vsum[i]);
- printf(" ** ERROR **\n");
- return 1;
- }
- }
-
- return 0;
-}
-#else
-static int do_md5sum(struct cmd_tbl *cmdtp, int flag, int argc,
-char *const argv[])
-{
- unsigned long addr, len;
- unsigned int i;
- u8 output[16];
- void *buf;
-
- if (argc < 3)
- return CMD_RET
Re: [PATCH] doc/develop/codingstyle.rst: Clarify include section
Hi Tom, On Fri, Feb 9, 2024 at 3:52 PM Tom Rini wrote: > > Rework the section about includes slightly. We should not be using > common.h anywhere, so remove that from examples and ask people to send > patches removing it when found. Doing this also means we need to reword > other parts of this section. Be clearer about using alphabetical > ordering. > > Signed-off-by: Tom Rini > --- > doc/develop/codingstyle.rst | 23 +++ > 1 file changed, 11 insertions(+), 12 deletions(-) > > diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst > index b25bfbd271f0..cdf5aedfcbb0 100644 > --- a/doc/develop/codingstyle.rst > +++ b/doc/develop/codingstyle.rst > @@ -108,30 +108,29 @@ expected size, or that particular members appear at the > right offset. > Include files > - > > -You should follow this ordering in U-Boot. The common.h header (which is > going > -away at some point) should always be first, followed by other headers in > order, > -then headers with directories, then local files: > +You should follow this ordering in U-Boot. In all cases, they should be > listed > +in alphabetical order. First comes headers which are located directly in our > +top-level include diretory. This excludes the common.h header file which is > to > +be removed. Second are headers within subdirectories, Finally directory-local > +includes should be listed. See this example: > > .. code-block:: C > > - #include > #include > #include > #include > #include > - #include > + #include > #include > #include > #include "local.h" > > -Within that order, sort your includes. > - > -It is important to include common.h first since it provides basic features > used > -by most files, e.g. CONFIG options. > - > For files that need to be compiled for the host (e.g. tools), you need to use > -``#ifndef USE_HOSTCC`` to avoid including common.h since it includes a lot of > -internal U-Boot things. See common/image.c for an example. > +``#ifndef USE_HOSTCC`` to avoid including U-Boot specific include files. See > +common/image.c for an example. > + > +If you encounter ccode which still uses a patch to remove that and nitpick: s/ccode/code/g > +replace it with any required include files directly is much appreciated. > > If your file uses driver model, include in the C file. Do not include > dm.h in a header file. Try to use forward declarations (e.g. ``struct > -- > 2.34.1 > Reviewed-by: Igor Opaniuk -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
Re: [PATCH v1] armv8: crypto: SHA-512 using ARMv8 Crypto Extensions
Hi Tom, On Sun, Feb 11, 2024 at 1:37 AM Tom Rini wrote: > > On Sat, Feb 10, 2024 at 01:07:09PM +0100, Igor Opaniuk wrote: > > > From: Igor Opaniuk > > > > Add support for the SHA-512 Secure Hash Algorithm which uses ARMv8 Crypto > > Extensions. The CPU should support ARMv8.2 instruction set and implement > > SHA512H, SHA512H2, SHA512SU0, and SHA512SU1 instructions. > > > > This information can be obtained from ID_AA64ISAR0_EL1 (AArch64 Instruction > > Set Attribute Register 0), bits [15:12] should be 0b0010 [1], that > > indicates support for SHA512* instructions in AArch64 state. As not all > > ARMv8-base SoCs support that, ARMV8_CE_SHA512 is left disabled by > > default for now. > > > > Tested in QEMU for ARMv8 with compiled-in SHA-2 support. > > Even on emulated cpu the hashing speed increase was visible: > > > > With CE usage: > > => time hash sha512 0x4020 0x200 > > Calculate hash > > Calculate hash > > sha512 for 4020 ... 421f ==> 1aeae269f4eb7c37... > > > > time: 0.215 seconds > > > > Without CE usage: > > => time hash sha512 0x4020 0x200 > > sha512 for 4020 ... 421f ==> 1aeae269f4eb7c37... > > > > time: 0.356 seconds > > > > Real HW tests should provide much more improvement and objective results > > with 10x speed increase at least. > > > > The implementation is based on original implementation from Ard Biesheuvel > > in > > Linux kernel [2] > > > > [1] > > https://developer.arm.com/documentation/ddi0601/2023-12/AArch64-Registers/ID-AA64ISAR0-EL1--AArch64-Instruction-Set-Attribute-Register-0 > > [2] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/crypto/sha2-ce-core.S > > > > CC: Ard Biesheuvel > > CC: Loic Poulain > > Signed-off-by: Igor Opaniuk > [snip] > > diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig > > index 9f0fb369f7..fd5c26421b 100644 > > --- a/arch/arm/cpu/armv8/Kconfig > > +++ b/arch/arm/cpu/armv8/Kconfig > > @@ -204,6 +204,11 @@ config ARMV8_CE_SHA256 > > bool "SHA-256 digest algorithm (ARMv8 Crypto Extensions)" > > default y if SHA256 > > > > +config ARMV8_CE_SHA512 > > + bool "SHA-512 digest algorithm (ARMv8 Crypto Extensions)" > > + depends on SHA512 > > + default n > > Like the sha256 one, this should be default y I think, the performance > improvement is likely worth the size increase. That was done on purpose, as SHA512* instructions (comparing to SHA256*) were introduced only in ARMv8.2-A, and most of the currently supported ARMv8 SoCs in U-Boot don't support it. We probably would end up with most of them reporting Synchronous Abort crashes. As Marc suggested in the previous email, we should have both versions compiled-in (sw and hw-accelerated) and dynamic selection of proper version in runtime based on CPU capabilities, and I plan to address that in future in a separate patch series. > > -- > Tom Regards, Igor -- Best regards - Freundliche Grüsse - Meilleures salutations Igor Opaniuk Senior Software Engineer, Embedded & Security E: igor.opan...@foundries.io W: www.foundries.io
Re: [PATCH v1] armv8: crypto: SHA-512 using ARMv8 Crypto Extensions
Hello Marc, On Sun, Feb 11, 2024 at 12:12 AM Marc Zyngier wrote: > > [Fixing Ard's email address for something more current.] > > On Sat, 10 Feb 2024 12:07:09 +0000, > Igor Opaniuk wrote: > > > > From: Igor Opaniuk > > > > Add support for the SHA-512 Secure Hash Algorithm which uses ARMv8 Crypto > > Extensions. The CPU should support ARMv8.2 instruction set and implement > > SHA512H, SHA512H2, SHA512SU0, and SHA512SU1 instructions. > > > > This information can be obtained from ID_AA64ISAR0_EL1 (AArch64 Instruction > > Set Attribute Register 0), bits [15:12] should be 0b0010 [1], that > > indicates support for SHA512* instructions in AArch64 state. As not all > > ARMv8-base SoCs support that, ARMV8_CE_SHA512 is left disabled by > > default for now. > > But since you can actually probe it at runtime, what's the problem? That actually was my initial plan, I just decided to move one step one step at a time and address that in the next patch series. > > > Tested in QEMU for ARMv8 with compiled-in SHA-2 support. > > Even on emulated cpu the hashing speed increase was visible: > > Unfortunately, QEMU is not a good oracle for optimisations, and is > more akin to rolling a dice. In your case, you *should* see an > improvement, but this should be evaluated on bare metal. I fully agree with you here, but unfortunately it turned out that the only board with ARMv8.2-ready SoC (Cortex A55). I have now at hand doesn't support SHA512* instructions, but after all decided so send the patch to get rid of the feeling that it was all in vain :) Maybe I added a bit of confusion to the commit message, as the initial idea was about functional validation (that it works in QEMU at least). I didn't want to make any comparison in a virtualized environment as it obviously didn't make any sense. > > > > > With CE usage: > > => time hash sha512 0x4020 0x200 > > Calculate hash > > Calculate hash > > sha512 for 4020 ... 421f ==> 1aeae269f4eb7c37... > > > > time: 0.215 seconds > > > > Without CE usage: > > => time hash sha512 0x4020 0x200 > > sha512 for 4020 ... 421f ==> 1aeae269f4eb7c37... > > > > time: 0.356 seconds > > > > Real HW tests should provide much more improvement and objective results > > with 10x speed increase at least. > > > > The implementation is based on original implementation from Ard Biesheuvel > > in > > Linux kernel [2] > > > > [1] > > https://developer.arm.com/documentation/ddi0601/2023-12/AArch64-Registers/ID-AA64ISAR0-EL1--AArch64-Instruction-Set-Attribute-Register-0 > > [2] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/crypto/sha2-ce-core.S > > > > CC: Ard Biesheuvel > > CC: Loic Poulain > > Signed-off-by: Igor Opaniuk > > --- > > > > arch/arm/cpu/armv8/Kconfig | 5 + > > arch/arm/cpu/armv8/Makefile | 1 + > > arch/arm/cpu/armv8/sha512_ce_core.S | 210 > > arch/arm/cpu/armv8/sha512_ce_glue.c | 20 +++ > > lib/sha512.c| 6 +- > > 5 files changed, 240 insertions(+), 2 deletions(-) > > create mode 100644 arch/arm/cpu/armv8/sha512_ce_core.S > > create mode 100644 arch/arm/cpu/armv8/sha512_ce_glue.c > > > > diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig > > index 9f0fb369f7..fd5c26421b 100644 > > --- a/arch/arm/cpu/armv8/Kconfig > > +++ b/arch/arm/cpu/armv8/Kconfig > > @@ -204,6 +204,11 @@ config ARMV8_CE_SHA256 > > bool "SHA-256 digest algorithm (ARMv8 Crypto Extensions)" > > default y if SHA256 > > > > +config ARMV8_CE_SHA512 > > + bool "SHA-512 digest algorithm (ARMv8 Crypto Extensions)" > > + depends on SHA512 > > + default n > > + > > endif > > > > endif > > diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile > > index bba4f570db..3894f2bb2a 100644 > > --- a/arch/arm/cpu/armv8/Makefile > > +++ b/arch/arm/cpu/armv8/Makefile > > @@ -45,3 +45,4 @@ obj-$(CONFIG_TARGET_BCMNS3) += bcmns3/ > > obj-$(CONFIG_XEN) += xen/ > > obj-$(CONFIG_ARMV8_CE_SHA1) += sha1_ce_glue.o sha1_ce_core.o > > obj-$(CONFIG_ARMV8_CE_SHA256) += sha256_ce_glue.o sha256_ce_core.o > > +obj-$(CONFIG_ARMV8_CE_SHA512) += sha512_ce_glue.o sha512_ce_core.o > > \ No newline at end of file > > diff --git a/arch/arm/cpu/armv8/sha512_ce_core.S > > b/arch/arm/cpu/armv8/sha512_ce_core.S > > new file mode 100644 > > index
[PATCH v1] armv8: crypto: SHA-512 using ARMv8 Crypto Extensions
From: Igor Opaniuk Add support for the SHA-512 Secure Hash Algorithm which uses ARMv8 Crypto Extensions. The CPU should support ARMv8.2 instruction set and implement SHA512H, SHA512H2, SHA512SU0, and SHA512SU1 instructions. This information can be obtained from ID_AA64ISAR0_EL1 (AArch64 Instruction Set Attribute Register 0), bits [15:12] should be 0b0010 [1], that indicates support for SHA512* instructions in AArch64 state. As not all ARMv8-base SoCs support that, ARMV8_CE_SHA512 is left disabled by default for now. Tested in QEMU for ARMv8 with compiled-in SHA-2 support. Even on emulated cpu the hashing speed increase was visible: With CE usage: => time hash sha512 0x4020 0x200 Calculate hash Calculate hash sha512 for 4020 ... 421f ==> 1aeae269f4eb7c37... time: 0.215 seconds Without CE usage: => time hash sha512 0x4020 0x200 sha512 for 4020 ... 421f ==> 1aeae269f4eb7c37... time: 0.356 seconds Real HW tests should provide much more improvement and objective results with 10x speed increase at least. The implementation is based on original implementation from Ard Biesheuvel in Linux kernel [2] [1] https://developer.arm.com/documentation/ddi0601/2023-12/AArch64-Registers/ID-AA64ISAR0-EL1--AArch64-Instruction-Set-Attribute-Register-0 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/crypto/sha2-ce-core.S CC: Ard Biesheuvel CC: Loic Poulain Signed-off-by: Igor Opaniuk --- arch/arm/cpu/armv8/Kconfig | 5 + arch/arm/cpu/armv8/Makefile | 1 + arch/arm/cpu/armv8/sha512_ce_core.S | 210 arch/arm/cpu/armv8/sha512_ce_glue.c | 20 +++ lib/sha512.c| 6 +- 5 files changed, 240 insertions(+), 2 deletions(-) create mode 100644 arch/arm/cpu/armv8/sha512_ce_core.S create mode 100644 arch/arm/cpu/armv8/sha512_ce_glue.c diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig index 9f0fb369f7..fd5c26421b 100644 --- a/arch/arm/cpu/armv8/Kconfig +++ b/arch/arm/cpu/armv8/Kconfig @@ -204,6 +204,11 @@ config ARMV8_CE_SHA256 bool "SHA-256 digest algorithm (ARMv8 Crypto Extensions)" default y if SHA256 +config ARMV8_CE_SHA512 + bool "SHA-512 digest algorithm (ARMv8 Crypto Extensions)" + depends on SHA512 + default n + endif endif diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile index bba4f570db..3894f2bb2a 100644 --- a/arch/arm/cpu/armv8/Makefile +++ b/arch/arm/cpu/armv8/Makefile @@ -45,3 +45,4 @@ obj-$(CONFIG_TARGET_BCMNS3) += bcmns3/ obj-$(CONFIG_XEN) += xen/ obj-$(CONFIG_ARMV8_CE_SHA1) += sha1_ce_glue.o sha1_ce_core.o obj-$(CONFIG_ARMV8_CE_SHA256) += sha256_ce_glue.o sha256_ce_core.o +obj-$(CONFIG_ARMV8_CE_SHA512) += sha512_ce_glue.o sha512_ce_core.o \ No newline at end of file diff --git a/arch/arm/cpu/armv8/sha512_ce_core.S b/arch/arm/cpu/armv8/sha512_ce_core.S new file mode 100644 index 00..906291f35b --- /dev/null +++ b/arch/arm/cpu/armv8/sha512_ce_core.S @@ -0,0 +1,210 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * sha512-ce-core.S - core SHA-384/SHA-512 transform using v8 Crypto + * Extensions + * + * Copyright (C) 2018 Linaro Ltd + * Copyright (C) 2024 Igor Opaniuk + */ + + #include + #include + #include + #include + + .macro adr_l, dst, sym + adrp\dst, \sym + add \dst, \dst, :lo12:\sym + .endm + + .irpb,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19 + .set.Lq\b, \b + .set.Lv\b\().2d, \b + .endr + + .macro sha512h, rd, rn, rm + .inst 0xce608000 | .L\rd | (.L\rn << 5) | (.L\rm << 16) + .endm + + .macro sha512h2, rd, rn, rm + .inst 0xce608400 | .L\rd | (.L\rn << 5) | (.L\rm << 16) + .endm + + .macro sha512su0, rd, rn + .inst 0xcec08000 | .L\rd | (.L\rn << 5) + .endm + + .macro sha512su1, rd, rn, rm + .inst 0xce608800 | .L\rd | (.L\rn << 5) | (.L\rm << 16) + .endm + + /* +* The SHA-512 round constants +*/ + .section".rodata", "a" + .align 4 +.Lsha512_rcon: + .quad 0x428a2f98d728ae22, 0x7137449123ef65cd + .quad 0xb5c0fbcfec4d3b2f, 0xe9b5dba58189dbbc + .quad 0x3956c25bf348b538, 0x59f111f1b605d019 + .quad 0x923f82a4af194f9b, 0xab1c5ed5da6d8118 + .quad 0xd807aa98a3030242, 0x12835b0145706fbe + .quad 0x243185be4ee4b28c, 0x550c7dc3d5ffb4e2 + .quad 0x72be5d74f27b896f, 0x80deb1fe3b1696b1 + .quad 0x9bdc06a725c71235, 0xc19bf174cf692694 + .quad 0xe49b69c19ef14ad2, 0xefbe4786384f25e3 + .quad 0x0fc19dc68b8cd5b5, 0
Re: [PATCH v2 1/3] imx93: Use a header for imx9_probe_mu declaration
On Fri, Feb 9, 2024 at 1:05 PM Mathieu Othacehe wrote:
>
> Put imx9_probe_mu declaration in a new mu.h header file.
>
> Signed-off-by: Mathieu Othacehe
> ---
> arch/arm/include/asm/arch-imx9/mu.h | 13 +
> board/freescale/imx93_evk/spl.c | 2 +-
> board/phytec/phycore_imx93/spl.c| 2 +-
> 3 files changed, 15 insertions(+), 2 deletions(-)
> create mode 100644 arch/arm/include/asm/arch-imx9/mu.h
>
> diff --git a/arch/arm/include/asm/arch-imx9/mu.h
> b/arch/arm/include/asm/arch-imx9/mu.h
> new file mode 100644
> index 000..b8604992914
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-imx9/mu.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2024 Mathieu Othacehe
> + */
> +
> +#ifndef __ARCH_IMX9_MU_H
> +#define __ARCH_IMX9_MU_H
> +
> +#include
> +
> +int imx9_probe_mu(void *ctx, struct event *event);
> +
> +#endif
> diff --git a/board/freescale/imx93_evk/spl.c b/board/freescale/imx93_evk/spl.c
> index be9c24fc0d9..a98ed69db88 100644
> --- a/board/freescale/imx93_evk/spl.c
> +++ b/board/freescale/imx93_evk/spl.c
> @@ -14,6 +14,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -91,7 +92,6 @@ int power_init_board(void)
> }
> #endif
>
> -extern int imx9_probe_mu(void *ctx, struct event *event);
> void board_init_f(ulong dummy)
> {
> int ret;
> diff --git a/board/phytec/phycore_imx93/spl.c
> b/board/phytec/phycore_imx93/spl.c
> index da4b9e53594..dabc5316f33 100644
> --- a/board/phytec/phycore_imx93/spl.c
> +++ b/board/phytec/phycore_imx93/spl.c
> @@ -7,6 +7,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -99,7 +100,6 @@ int power_init_board(void)
> return 0;
> }
>
> -extern int imx9_probe_mu(void *ctx, struct event *event);
> void board_init_f(ulong dummy)
> {
> int ret;
> --
> 2.41.0
>
Reviewed-by: Igor Opaniuk
--
Best regards - Freundliche Grüsse - Meilleures salutations
Igor Opaniuk
Senior Software Engineer, Embedded & Security
E: igor.opan...@foundries.io
W: www.foundries.io
[PATCH v2 7/7] doc: android: avb: sync usage details
From: Igor Opaniuk Sync usage info with the one cmd/avb.c. Signed-off-by: Igor Opaniuk --- Changes in v2: - Address Mattijs comment about usage info, sync with cmd/avb.c doc/android/avb2.rst | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/doc/android/avb2.rst b/doc/android/avb2.rst index a072119574f..4aca7a5c660 100644 --- a/doc/android/avb2.rst +++ b/doc/android/avb2.rst @@ -38,16 +38,22 @@ AVB 2.0 U-Boot shell commands Provides CLI interface to invoke AVB 2.0 verification + misc. commands for different testing purposes:: -avb init - initialize avb 2.0 for -avb verify - run verification process using hash data from vbmeta structure +avb init - initialize avb 2 for avb read_rb - read rollback index at location avb write_rb - write rollback index to avb is_unlocked - returns unlock status of the device -avb get_uuid - read and print uuid of partition +avb get_uuid - read and print uuid of partition avb read_part - read bytes from -partition to buffer +partition to buffer +avb read_part_hex- read bytes from +partition and print to stdout avb write_part - write bytes to - by using data from + by using data from +avb read_pvalue - read a persistent value +avb write_pvalue - write a persistent value +avb verify [slot_suffix] - run verification process using hash data +from vbmeta structure +[slot_suffix] - _a, _b, etc (if vbmeta partition is slotted) Partitions tampering (example) -- -- 2.34.1
[PATCH v2 6/7] cmd: avb: rework do_avb_verify_part
From: Igor Opaniuk
Use existing str_avb_slot_error() function for obtaining
verification fail reason details.
Take into account device lock state for setting correct
androidboot.verifiedbootstate kernel cmdline parameter.
Reviewed-by: Mattijs Korpershoek
Signed-off-by: Igor Opaniuk
---
Changes in v2:
- Mattijs Korpershoek R-b tag applied
cmd/avb.c | 50 +-
1 file changed, 17 insertions(+), 33 deletions(-)
diff --git a/cmd/avb.c b/cmd/avb.c
index 62a3ee18e7f..8fbd48ee5a2 100644
--- a/cmd/avb.c
+++ b/cmd/avb.c
@@ -250,6 +250,7 @@ int do_avb_verify_part(struct cmd_tbl *cmdtp, int flag,
const char * const requested_partitions[] = {"boot", NULL};
AvbSlotVerifyResult slot_result;
AvbSlotVerifyData *out_data;
+ enum avb_boot_state boot_state;
char *cmdline;
char *extra_args;
char *slot_suffix = "";
@@ -287,18 +288,23 @@ int do_avb_verify_part(struct cmd_tbl *cmdtp, int flag,
AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE,
_data);
- switch (slot_result) {
- case AVB_SLOT_VERIFY_RESULT_OK:
- /* Until we don't have support of changing unlock states, we
-* assume that we are by default in locked state.
-* So in this case we can boot only when verification is
-* successful; we also supply in cmdline GREEN boot state
-*/
+ /*
+* LOCKED devices with custom root of trust setup is not supported
(YELLOW)
+*/
+ if (slot_result == AVB_SLOT_VERIFY_RESULT_OK) {
printf("Verification passed successfully\n");
- /* export additional bootargs to AVB_BOOTARGS env var */
+ /*
+* ORANGE state indicates that device may be freely modified.
+* Device integrity is left to the user to verify out-of-band.
+*/
+ if (unlocked)
+ boot_state = AVB_ORANGE;
+ else
+ boot_state = AVB_GREEN;
- extra_args = avb_set_state(avb_ops, AVB_GREEN);
+ /* export boot state to AVB_BOOTARGS env var */
+ extra_args = avb_set_state(avb_ops, boot_state);
if (extra_args)
cmdline = append_cmd_line(out_data->cmdline,
extra_args);
@@ -308,30 +314,8 @@ int do_avb_verify_part(struct cmd_tbl *cmdtp, int flag,
env_set(AVB_BOOTARGS, cmdline);
res = CMD_RET_SUCCESS;
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_VERIFICATION:
- printf("Verification failed\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_IO:
- printf("I/O error occurred during verification\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_OOM:
- printf("OOM error occurred during verification\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA:
- printf("Corrupted dm-verity metadata detected\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_UNSUPPORTED_VERSION:
- printf("Unsupported version of avbtool was used\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_ROLLBACK_INDEX:
- printf("Rollback index check failed\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_PUBLIC_KEY_REJECTED:
- printf("Public key was rejected\n");
- break;
- default:
- printf("Unknown error occurred\n");
+ } else {
+ printf("Verification failed, reason: %s\n",
str_avb_slot_error(slot_result));
}
if (out_data)
--
2.34.1
[PATCH v2 5/7] common: avb_verify: add str_avb_io_error/str_avb_slot_error
From: Igor Opaniuk
Introduce str_avb_io_error() and str_avb_slot_error() functions,
that provide a pointer to AVB runtime error message.
Reviewed-by: Mattijs Korpershoek
Signed-off-by: Igor Opaniuk
---
Changes in v2:
- Mattijs Korpershoek R-b tag applied
common/avb_verify.c | 49
include/avb_verify.h | 3 ++-
2 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/common/avb_verify.c b/common/avb_verify.c
index ed58239cf8a..cff9117d92f 100644
--- a/common/avb_verify.c
+++ b/common/avb_verify.c
@@ -119,6 +119,55 @@ static const unsigned char avb_root_pub[1032] = {
0xd8, 0x7e,
};
+const char *str_avb_io_error(AvbIOResult res)
+{
+ switch (res) {
+ case AVB_IO_RESULT_OK:
+ return "Requested operation was successful";
+ case AVB_IO_RESULT_ERROR_IO:
+ return "Underlying hardware encountered an I/O error";
+ case AVB_IO_RESULT_ERROR_OOM:
+ return "Unable to allocate memory";
+ case AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION:
+ return "Requested partition does not exist";
+ case AVB_IO_RESULT_ERROR_RANGE_OUTSIDE_PARTITION:
+ return "Bytes requested is outside the range of partition";
+ case AVB_IO_RESULT_ERROR_NO_SUCH_VALUE:
+ return "Named persistent value does not exist";
+ case AVB_IO_RESULT_ERROR_INVALID_VALUE_SIZE:
+ return "Named persistent value size is not supported";
+ case AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE:
+ return "Buffer is too small for the requested operation";
+ default:
+ return "Unknown AVB error";
+ }
+}
+
+const char *str_avb_slot_error(AvbSlotVerifyResult res)
+{
+ switch (res) {
+ case AVB_SLOT_VERIFY_RESULT_OK:
+ return "Verification passed successfully";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_OOM:
+ return "Allocation of memory failed";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_IO:
+ return "I/O error occurred while trying to load data";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_VERIFICATION:
+ return "Digest didn't match or signature checks failed";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_ROLLBACK_INDEX:
+ return "Rollback index is less than its stored value";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_PUBLIC_KEY_REJECTED:
+ return "Public keys are not accepted";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA:
+ return "Metadata is invalid or inconsistent";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_UNSUPPORTED_VERSION:
+ return "Metadata requires a newer version of libavb";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT:
+ return "Invalid arguments are used";
+ default:
+ return "Unknown AVB slot verification error";
+ }
+}
/**
*
* Boot states support (GREEN, YELLOW, ORANGE, RED) and dm_verity
diff --git a/include/avb_verify.h b/include/avb_verify.h
index 2fb850044d9..5d998b5a302 100644
--- a/include/avb_verify.h
+++ b/include/avb_verify.h
@@ -52,7 +52,8 @@ char *avb_set_enforce_verity(const char *cmdline);
char *avb_set_ignore_corruption(const char *cmdline);
char *append_cmd_line(char *cmdline_orig, char *cmdline_new);
-
+const char *str_avb_io_error(AvbIOResult res);
+const char *str_avb_slot_error(AvbSlotVerifyResult res);
/**
*
* I/O helper inline functions
--
2.34.1
[PATCH v2 4/7] cmd: avb: rework prints
From: Igor Opaniuk
Simplify and add more context for prints where it's needed.
Signed-off-by: Igor Opaniuk
---
Changes in v2:
- Drop AVB_OPS_CHECK macro and leave previous check
cmd/avb.c | 123 +-
1 file changed, 75 insertions(+), 48 deletions(-)
diff --git a/cmd/avb.c b/cmd/avb.c
index ce8b63873f2..62a3ee18e7f 100644
--- a/cmd/avb.c
+++ b/cmd/avb.c
@@ -11,6 +11,7 @@
#include
#define AVB_BOOTARGS "avb_bootargs"
+
static struct AvbOps *avb_ops;
int do_avb_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
@@ -28,8 +29,10 @@ int do_avb_init(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
avb_ops = avb_ops_alloc(mmc_dev);
if (avb_ops)
return CMD_RET_SUCCESS;
+ else
+ printf("Can't allocate AvbOps");
- printf("Failed to initialize avb2\n");
+ printf("Failed to initialize AVB\n");
return CMD_RET_FAILURE;
}
@@ -41,10 +44,11 @@ int do_avb_read_part(struct cmd_tbl *cmdtp, int flag, int
argc,
s64 offset;
size_t bytes, bytes_read = 0;
void *buffer;
+ int ret;
if (!avb_ops) {
- printf("AVB 2.0 is not initialized, please run 'avb init'\n");
- return CMD_RET_USAGE;
+ printf("AVB is not initialized, please run 'avb init '\n");
+ return CMD_RET_FAILURE;
}
if (argc != 5)
@@ -55,14 +59,15 @@ int do_avb_read_part(struct cmd_tbl *cmdtp, int flag, int
argc,
bytes = hextoul(argv[3], NULL);
buffer = (void *)hextoul(argv[4], NULL);
- if (avb_ops->read_from_partition(avb_ops, part, offset, bytes,
-buffer, _read) ==
-AVB_IO_RESULT_OK) {
+ ret = avb_ops->read_from_partition(avb_ops, part, offset,
+ bytes, buffer, _read);
+ if (ret == AVB_IO_RESULT_OK) {
printf("Read %zu bytes\n", bytes_read);
return CMD_RET_SUCCESS;
}
- printf("Failed to read from partition\n");
+ printf("Failed to read from partition '%s', err = %d\n",
+ part, ret);
return CMD_RET_FAILURE;
}
@@ -74,10 +79,11 @@ int do_avb_read_part_hex(struct cmd_tbl *cmdtp, int flag,
int argc,
s64 offset;
size_t bytes, bytes_read = 0;
char *buffer;
+ int ret;
if (!avb_ops) {
- printf("AVB 2.0 is not initialized, please run 'avb init'\n");
- return CMD_RET_USAGE;
+ printf("AVB is not initialized, please run 'avb init '\n");
+ return CMD_RET_FAILURE;
}
if (argc != 4)
@@ -94,8 +100,9 @@ int do_avb_read_part_hex(struct cmd_tbl *cmdtp, int flag,
int argc,
}
memset(buffer, 0, bytes);
- if (avb_ops->read_from_partition(avb_ops, part, offset, bytes, buffer,
-_read) == AVB_IO_RESULT_OK) {
+ ret = avb_ops->read_from_partition(avb_ops, part, offset,
+ bytes, buffer, _read);
+ if (ret == AVB_IO_RESULT_OK) {
printf("Requested %zu, read %zu bytes\n", bytes, bytes_read);
printf("Data: ");
for (int i = 0; i < bytes_read; i++)
@@ -107,7 +114,8 @@ int do_avb_read_part_hex(struct cmd_tbl *cmdtp, int flag,
int argc,
return CMD_RET_SUCCESS;
}
- printf("Failed to read from partition\n");
+ printf("Failed to read from partition '%s', err = %d\n",
+ part, ret);
free(buffer);
return CMD_RET_FAILURE;
@@ -120,9 +128,10 @@ int do_avb_write_part(struct cmd_tbl *cmdtp, int flag, int
argc,
s64 offset;
size_t bytes;
void *buffer;
+ int ret;
if (!avb_ops) {
- printf("AVB 2.0 is not initialized, run 'avb init' first\n");
+ printf("AVB is not initialized, please run 'avb init '\n");
return CMD_RET_FAILURE;
}
@@ -134,13 +143,15 @@ int do_avb_write_part(struct cmd_tbl *cmdtp, int flag,
int argc,
bytes = hextoul(argv[3], NULL);
buffer = (void *)hextoul(argv[4], NULL);
- if (avb_ops->write_to_partition(avb_ops, part, offset, bytes, buffer) ==
- AVB_IO_RESULT_OK) {
+ ret = avb_ops->write_to_partition(avb_ops, part, offset,
+ bytes, buffer);
+ if (ret == AVB_IO_RESULT_OK) {
printf("Wrote %zu bytes\n", bytes);
return CMD_RET_SUCCESS;
}
- printf("Failed to write in partition\n");
+ printf("Failed to write
[PATCH v2 3/7] common: avb_verify: rework error/debug prints
From: Igor Opaniuk
Make error prints more verbose with additional context.
Also s/print/debug/g for prints, which might be relevant only
for debugging purposes.
Reviewed-by: Mattijs Korpershoek
Signed-off-by: Igor Opaniuk
---
Changes in v2:
- Mattijs Korpershoek R-b tag applied
common/avb_verify.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/common/avb_verify.c b/common/avb_verify.c
index 938a5383b5d..ed58239cf8a 100644
--- a/common/avb_verify.c
+++ b/common/avb_verify.c
@@ -279,9 +279,9 @@ static unsigned long mmc_read_and_flush(struct mmc_part
*part,
* Reading fails on unaligned buffers, so we have to
* use aligned temporary buffer and then copy to destination
*/
-
if (unaligned) {
- printf("Handling unaligned read buffer..\n");
+ debug("%s: handling unaligned read buffer, addr = 0x%p\n",
+ __func__, buffer);
tmp_buf = get_sector_buf();
buf_size = get_sector_buf_size();
if (sectors > buf_size / part->info.blksz)
@@ -320,7 +320,8 @@ static unsigned long mmc_write(struct mmc_part *part,
lbaint_t start,
if (unaligned) {
tmp_buf = get_sector_buf();
buf_size = get_sector_buf_size();
- printf("Handling unaligned wrire buffer..\n");
+ debug("%s: handling unaligned read buffer, addr = 0x%p\n",
+ __func__, buffer);
if (sectors > buf_size / part->info.blksz)
sectors = buf_size / part->info.blksz;
@@ -348,30 +349,35 @@ static struct mmc_part *get_partition(AvbOps *ops, const
char *partition)
dev_num = get_boot_device(ops);
part->mmc = find_mmc_device(dev_num);
if (!part->mmc) {
- printf("No MMC device at slot %x\n", dev_num);
+ printf("%s: no MMC device at slot %x\n", __func__, dev_num);
goto err;
}
- if (mmc_init(part->mmc)) {
- printf("MMC initialization failed\n");
+ ret = mmc_init(part->mmc);
+ if (ret) {
+ printf("%s: MMC initialization failed, err = %d\n",
+ __func__, ret);
goto err;
}
if (IS_MMC(part->mmc)) {
ret = mmc_switch_part(part->mmc, part_num);
- if (ret)
+ if (ret) {
+ printf("%s: MMC part switch failed, err = %d\n",
+ __func__, ret);
goto err;
+ }
}
mmc_blk = mmc_get_blk_desc(part->mmc);
if (!mmc_blk) {
- printf("Error - failed to obtain block descriptor\n");
+ printf("%s: failed to obtain block descriptor\n", __func__);
goto err;
}
ret = part_get_info_by_name(mmc_blk, partition, >info);
if (ret < 0) {
- printf("Can't find partition '%s'\n", partition);
+ printf("%s: can't find partition '%s'\n", __func__, partition);
goto err;
}
@@ -684,7 +690,7 @@ static AvbIOResult read_rollback_index(AvbOps *ops,
{
#ifndef CONFIG_OPTEE_TA_AVB
/* For now we always return 0 as the stored rollback index. */
- printf("%s not supported yet\n", __func__);
+ debug("%s: rollback protection is not implemented\n", __func__);
if (out_rollback_index)
*out_rollback_index = 0;
@@ -730,7 +736,7 @@ static AvbIOResult write_rollback_index(AvbOps *ops,
{
#ifndef CONFIG_OPTEE_TA_AVB
/* For now this is a no-op. */
- printf("%s not supported yet\n", __func__);
+ debug("%s: rollback protection is not implemented\n", __func__);
return AVB_IO_RESULT_OK;
#else
@@ -766,8 +772,7 @@ static AvbIOResult read_is_device_unlocked(AvbOps *ops,
bool *out_is_unlocked)
{
#ifndef CONFIG_OPTEE_TA_AVB
/* For now we always return that the device is unlocked. */
-
- printf("%s not supported yet\n", __func__);
+ debug("%s: device locking is not implemented\n", __func__);
*out_is_unlocked = true;
--
2.34.1
[PATCH v2 2/7] avb: move SPDX license identifiers to the first line
From: Igor Opaniuk Move SPDX license identifiers to the first line, so it conforms to license placement rule [1]: Placement: The SPDX license identifier in kernel files shall be added at the first possible line in a file which can contain a comment. For the majority of files this is the first line, except for scripts which require the '#!PATH_TO_INTERPRETER' in the first line. For those scripts the SPDX identifier goes into the second line. [1] https://www.kernel.org/doc/Documentation/process/license-rules.rst Reviewed-by: Mattijs Korpershoek Signed-off-by: Igor Opaniuk --- Changes in v2: - Fix typo in commit message - Mattijs Korpershoek R-b tag applied cmd/avb.c | 4 +--- common/avb_verify.c| 3 +-- include/avb_verify.h | 4 +--- test/py/tests/test_android/test_avb.py | 3 +-- 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/cmd/avb.c b/cmd/avb.c index 783f51b8169..ce8b63873f2 100644 --- a/cmd/avb.c +++ b/cmd/avb.c @@ -1,8 +1,6 @@ - +// SPDX-License-Identifier: GPL-2.0+ /* * (C) Copyright 2018, Linaro Limited - * - * SPDX-License-Identifier:GPL-2.0+ */ #include diff --git a/common/avb_verify.c b/common/avb_verify.c index 59f2c25e0de..938a5383b5d 100644 --- a/common/avb_verify.c +++ b/common/avb_verify.c @@ -1,7 +1,6 @@ +// SPDX-License-Identifier: GPL-2.0+ /* * (C) Copyright 2018, Linaro Limited - * - * SPDX-License-Identifier:GPL-2.0+ */ #include diff --git a/include/avb_verify.h b/include/avb_verify.h index 1e787ba6668..2fb850044d9 100644 --- a/include/avb_verify.h +++ b/include/avb_verify.h @@ -1,8 +1,6 @@ - +/* SPDX-License-Identifier: GPL-2.0+ */ /* * (C) Copyright 2018, Linaro Limited - * - * SPDX-License-Identifier:GPL-2.0+ */ #ifndef_AVB_VERIFY_H diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py index 238b48c90fa..865efbca4de 100644 --- a/test/py/tests/test_android/test_avb.py +++ b/test/py/tests/test_android/test_avb.py @@ -1,6 +1,5 @@ -# Copyright (c) 2018, Linaro Limited -# # SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018, Linaro Limited # # Android Verified Boot 2.0 Test -- 2.34.1
[PATCH v2 1/7] common: avb_verify: don't call mmc_switch_part for SD
From: Igor Opaniuk
mmc_switch_part() is used for switching between hw partitions
on eMMC (boot0, boot1, user, rpmb).
There is no need to do that for SD card.
Reviewed-by: Mattijs Korpershoek
Signed-off-by: Igor Opaniuk
---
Changes in v2:
- Mattijs Korpershoek R-b tag applied
common/avb_verify.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/common/avb_verify.c b/common/avb_verify.c
index 48ba8db51e5..59f2c25e0de 100644
--- a/common/avb_verify.c
+++ b/common/avb_verify.c
@@ -358,9 +358,11 @@ static struct mmc_part *get_partition(AvbOps *ops, const
char *partition)
goto err;
}
- ret = mmc_switch_part(part->mmc, part_num);
- if (ret)
- goto err;
+ if (IS_MMC(part->mmc)) {
+ ret = mmc_switch_part(part->mmc, part_num);
+ if (ret)
+ goto err;
+ }
mmc_blk = mmc_get_blk_desc(part->mmc);
if (!mmc_blk) {
--
2.34.1
[PATCH v2 0/7] AVB: cosmetic adjustments/improvements
This is the first patch series for incoming major improvements for AVB implementation, that include: - Simplify and add more context for debug/error prints where it's needed. - Move SPDX license identifiers to the first line, so it conforms to license placement rule. - Use mmc_switch_part() only for eMMC. - Rework do_avb_verify_part, take into account device lock state for setting correct androidboot.verifiedbootstate. - Adjust AVB documentation, sync usage info with the one cmd/avb.c. Igor Opaniuk (7): common: avb_verify: don't call mmc_switch_part for SD avb: move SPDX license identifiers to the first line common: avb_verify: rework error/debug prints cmd: avb: rework prints common: avb_verify: add str_avb_io_error/str_avb_slot_error cmd: avb: rework do_avb_verify_part doc: android: avb: sync usage details cmd/avb.c | 173 + common/avb_verify.c| 89 ++--- doc/android/avb2.rst | 16 ++- include/avb_verify.h | 7 +- test/py/tests/test_android/test_avb.py | 3 +- 5 files changed, 178 insertions(+), 110 deletions(-) -- 2.34.1
Re: [AVB/AB] Overhaul plans
Hello Mattijs, On Fri, Feb 9, 2024 at 11:30 AM Mattijs Korpershoek wrote: > > Hi Igor, > > On ven., févr. 09, 2024 at 11:14, Igor Opaniuk wrote: > > > Hi everyone, > > > > I'm currently planning a big overhaul of the current implementation of > > AVB/AB in U-Boot during the 2024 year, which I have barely touched since > > 2019. I used to believe that it was stillborn, but looks like it's > > being actively used > > now by some SoC vendors and Google folks [1][2]. > > This is great news! I am not aware of any development related to the > above but I'm looking forward to this. > > I can't speak for all vendors but I know that TI uses both the AVB and > AB implementation on their AM62x Android solution. Amlogic also uses it. > > > > > This is what I have in my todo list: > > * Backport latest libavb from AOSP upstream and add support for > >Verified Boot 1.3.0 version > > * Sync include/android_bootloader_message.h with AOSP upstream > > * Check and backport fixes for AVB in AOSP U-Boot fork if needed [1] > > * Get acquainted with a current state of A/B support in AOSP and > >backport all needed changes > > * Re-factor libavb, switch to U-Boot existing implementation of > >rsa/sha256/sha512 > > * Add SHA512 implementation that leverage ARMv8 CE > >(pull it from Linux) > > * Enable hw acceleration of SHA256/SHA512 that supports ARMv8 > >Crypto Extensions to speed up verification process on ARMv8-based boards. > > * AVB support for NAND storage > > I know that this has been send but I don't think Alistair has send any > follow-up on this: > https://patchwork.ozlabs.org/project/uboot/patch/20220926220211.868968-1-ade...@google.com/ > > > > > If someone is already working on anything from the above list - > > please feel free to reach out to me, so we can avoid duplication of effort. > > > > Any comments/suggestions are welcome! Thanks! > > From my understanding, the AOSP version of U-Boot has quite a different > bootflow since it relies on the (out-of-tree) boot_android command [3] Correct, but it turned out that they are using some parts of the existing avb implementation in that out-of-tree "boot_android" cmd + the made some adjustments on top of it, that we might be interested in : $ git log --grep=ANDROID --oneline | grep avb ea8f0bb45e ANDROID: Add avb_verify unit tests c9f88bf213 ANDROID: Adding function comments to avb_verify c5599e4a9f Merge "ANDROID: avb_verify: Handle failed malloc in get_partition()" 3aeeae4426 ANDROID: avb_verify: Handle failed malloc in get_partition() 2910c1042c Merge "ANDROID: avb_pubkey: Use bin2c instead of ld" 30fbf100b6 Merge "ANDROID: avb: Extract avb_pubkey_is_trusted()" 296361e80c ANDROID: avb_pubkey: Use bin2c instead of ld 5af2c6d968 ANDROID: avb: Extract avb_pubkey_is_trusted() f74b3f5815 ANDROID: avb_verify: Don't Return ERROR_IO for mismatch in pubkey sizes d6615cd233 Merge "ANDROID: Qualify avb_find_main_pubkey() parameters as const" 9c8470ed6b ANDROID: Qualify avb_find_main_pubkey() parameters as const af808f4b04 ANDROID: avb_find_main_pubkey returns CMD_RET_* 2070f02c75 ANDROID: remove erraneous avb logs Btw, my initial intention (back in 2018) was to make avb implementation boot-command agnostic, as at the time of implementing it different board/SoC vendors used different approaches for booting AOSP; moreover, iirc AOSP-specific cmd (boota/boot_android or whatever it's called now) didn't manage to land to the U-Boot mainline despite multiple attempts by different contributors. > > [3] > https://android.googlesource.com/platform/external/u-boot/+/refs/heads/main/cmd/boot_android.c > > Please keep me in the loop with your progress. If you want, you can > reach me on IRC as well (libera: #u-boot, nick: mkorpershoek) Sure, will keep you posted! > > > > > [1] https://android.googlesource.com/platform/external/u-boot > > [2] https://source.android.com/docs/devices/cuttlefish/bootloader-dev > > [3] > > https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h > > > > -- > > Best regards - Atentamente - Meilleures salutations > > > > Igor Opaniuk > > > > mailto: igor.opan...@gmail.com > > skype: igor.opanyuk > > http://ua.linkedin.com/in/iopaniuk Regards, Igor -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
[AVB/AB] Overhaul plans
Hi everyone, I'm currently planning a big overhaul of the current implementation of AVB/AB in U-Boot during the 2024 year, which I have barely touched since 2019. I used to believe that it was stillborn, but looks like it's being actively used now by some SoC vendors and Google folks [1][2]. This is what I have in my todo list: * Backport latest libavb from AOSP upstream and add support for Verified Boot 1.3.0 version * Sync include/android_bootloader_message.h with AOSP upstream * Check and backport fixes for AVB in AOSP U-Boot fork if needed [1] * Get acquainted with a current state of A/B support in AOSP and backport all needed changes * Re-factor libavb, switch to U-Boot existing implementation of rsa/sha256/sha512 * Add SHA512 implementation that leverage ARMv8 CE (pull it from Linux) * Enable hw acceleration of SHA256/SHA512 that supports ARMv8 Crypto Extensions to speed up verification process on ARMv8-based boards. * AVB support for NAND storage If someone is already working on anything from the above list - please feel free to reach out to me, so we can avoid duplication of effort. Any comments/suggestions are welcome! Thanks! [1] https://android.googlesource.com/platform/external/u-boot [2] https://source.android.com/docs/devices/cuttlefish/bootloader-dev [3] https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
Re: [PATCH v1 4/7] cmd: avb: rework prints
Hi Mattijs,
On Thu, Feb 8, 2024 at 3:00 PM Mattijs Korpershoek
wrote:
>
> Hi Igor,
>
> Thank you for the patch.
>
> On mar., févr. 06, 2024 at 23:31, Igor Opaniuk
> wrote:
>
> > From: Igor Opaniuk
> >
> > Introduce AVB_OPS_CHECK macro for checking AvbOps before using
> > it to avoid code duplication.
> > Simplify and add more context for prints where it's needed.
> >
> > Signed-off-by: Igor Opaniuk
> > ---
> >
> > cmd/avb.c | 156 --
> > 1 file changed, 80 insertions(+), 76 deletions(-)
> >
> > diff --git a/cmd/avb.c b/cmd/avb.c
> > index ce8b63873f2..ae0012c0e79 100644
> > --- a/cmd/avb.c
> > +++ b/cmd/avb.c
> > @@ -11,6 +11,14 @@
> > #include
> >
> > #define AVB_BOOTARGS "avb_bootargs"
> > +
> > +#define AVB_OPS_CHECK(avb_ops) do { \
> > + if (!(avb_ops)) { \
> > + printf("AVB is not initialized, please run 'avb init
> > '\n"); \
> > + return CMD_RET_FAILURE; \
> > + } \
> > +} while (false)
> > +
> checkpatch.pl --u-boot seems to complain about this:
> WARNING: Macros with flow control statements should be avoided
> #25: FILE: cmd/avb.c:15:
>
> This seems documented in the kernel coding style as well.
> https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
>
> I'd argue that in this case, it's not better or worse in terme of
> readability but i'd prefer to stick to the rules in this case.
>
> The error handling (using ret) and the better prints seem fine though.
> Could we drop the AVB_OPS_CHECK macro for v2?
Yeah, I was just trying to find a tradeoff between readability/code duplication
and coding style.
Will drop it in v2. Thanks!
>
> > static struct AvbOps *avb_ops;
> >
> > int do_avb_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const
> > argv[])
> > @@ -28,8 +36,10 @@ int do_avb_init(struct cmd_tbl *cmdtp, int flag, int
> > argc, char *const argv[])
> > avb_ops = avb_ops_alloc(mmc_dev);
> > if (avb_ops)
> > return CMD_RET_SUCCESS;
> > + else
> > + printf("Can't allocate AvbOps");
> >
> > - printf("Failed to initialize avb2\n");
> > + printf("Failed to initialize AVB\n");
> >
> > return CMD_RET_FAILURE;
> > }
> > @@ -41,11 +51,9 @@ int do_avb_read_part(struct cmd_tbl *cmdtp, int flag,
> > int argc,
> > s64 offset;
> > size_t bytes, bytes_read = 0;
> > void *buffer;
> > + int ret;
> >
> > - if (!avb_ops) {
> > - printf("AVB 2.0 is not initialized, please run 'avb init'\n");
> > - return CMD_RET_USAGE;
> > - }
> > + AVB_OPS_CHECK(avb_ops);
> >
> > if (argc != 5)
> > return CMD_RET_USAGE;
> > @@ -55,14 +63,15 @@ int do_avb_read_part(struct cmd_tbl *cmdtp, int flag,
> > int argc,
> > bytes = hextoul(argv[3], NULL);
> > buffer = (void *)hextoul(argv[4], NULL);
> >
> > - if (avb_ops->read_from_partition(avb_ops, part, offset, bytes,
> > - buffer, _read) ==
> > - AVB_IO_RESULT_OK) {
> > + ret = avb_ops->read_from_partition(avb_ops, part, offset,
> > +bytes, buffer, _read);
> > + if (ret == AVB_IO_RESULT_OK) {
> > printf("Read %zu bytes\n", bytes_read);
> > return CMD_RET_SUCCESS;
> > }
> >
> > - printf("Failed to read from partition\n");
> > + printf("Failed to read from partition '%s', err = %d\n",
> > +part, ret);
> >
> > return CMD_RET_FAILURE;
> > }
> > @@ -74,11 +83,9 @@ int do_avb_read_part_hex(struct cmd_tbl *cmdtp, int
> > flag, int argc,
> > s64 offset;
> > size_t bytes, bytes_read = 0;
> > char *buffer;
> > + int ret;
> >
> > - if (!avb_ops) {
> > - printf("AVB 2.0 is not initialized, please run 'avb init'\n");
> > - return CMD_RET_USAGE;
> > - }
> > + AVB_OPS_CHECK(avb_ops);
> >
> > if (argc != 4)
> > return CMD_RET_USAGE;
> > @@ -94,8 +101,9 @@ int do_avb_read_part_hex(struct cmd_tbl *cmdtp, int
> > flag, int argc,
> > }
> > memset(buffer, 0, bytes);
>
Re: [PATCH v1 7/7] doc: android: avb: add slot_suffix param details
Hi Mattijs, On Thu, Feb 8, 2024 at 3:12 PM Mattijs Korpershoek wrote: > > Hi Igor, > > Thank you for the patch. > > On mar., févr. 06, 2024 at 23:31, Igor Opaniuk > wrote: > > > From: Igor Opaniuk > > > > Add info about slot_suffix param for avb verify. > > > > Signed-off-by: Igor Opaniuk > > --- > > > > doc/android/avb2.rst | 8 +--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/doc/android/avb2.rst b/doc/android/avb2.rst > > index a072119574f..c0b2bedb831 100644 > > --- a/doc/android/avb2.rst > > +++ b/doc/android/avb2.rst > > @@ -39,15 +39,17 @@ Provides CLI interface to invoke AVB 2.0 verification + > > misc. commands for > > different testing purposes:: > > > > avb init - initialize avb 2.0 for > > -avb verify - run verification process using hash data from vbmeta > > structure > > +avb verify [slot_suffix] - run verification process using hash data > > +from vbmeta structure. Provide [slot_suffix] if vbmeta partition > > +is slotted > > Any particular reason for this to not be exactly the wording as in cmd/avb.c? > > "avb verify [slot_suffix] - run verification process using hash > data\n" > "from vbmeta structure\n" > "[slot_suffix] - _a, _b, etc (if vbmeta partition is slotted)\n" > > It looks good, but I think it would be better if both are the same for > consistency, since both texts are user facing. Will fix it in v2, thanks! > > > avb read_rb - read rollback index at location > > avb write_rb - write rollback index to > > avb is_unlocked - returns unlock status of the device > > avb get_uuid - read and print uuid of partition > > avb read_part - read bytes from > > -partition to buffer > > +partition to buffer > > avb write_part - write bytes to > > - by using data from > > + by using data from > > > > Partitions tampering (example) > > -- > > -- > > 2.34.1 -- Best regards - Freundliche Grüsse - Meilleures salutations Igor Opaniuk Senior Software Engineer, Embedded & Security E: igor.opan...@foundries.io W: www.foundries.io
Re: [RFC] Drop md5sum, crc32 and sha1 cmds in favor of hash cmd
Hello Tom, Peter, On Wed, Feb 7, 2024 at 3:16 PM Peter Robinson wrote: > > On Wed, 7 Feb 2024 at 13:48, Tom Rini wrote: > > > > On Wed, Feb 07, 2024 at 02:00:16PM +0100, Igor Opaniuk wrote: > > > Hello, > > > > > > I was playing a bit with different hash functions recently, and > > > it turned out that md5sum, crc32, sha1 cmds just duplicate > > > what is already covered by generic `hash` cmd. > > > > > > => sha1 0x6000 0x200 > > > sha1 for 6000 ... 61ff ==> > > > 4ff5ffc91d00a95155518b920f46e2483d0e1437 > > > => hash sha1 0x6000 0x200 > > > sha1 for 6000 ... 61ff ==> > > > 4ff5ffc91d00a95155518b920f46e2483d0e1437 > > > > > > => crc32 0x6000 0x200 > > > crc32 for 6000 ... 61ff ==> 6fe352e8 > > > => hash crc32 0x6000 0x200 > > > crc32 for 6000 ... 61ff ==> 6fe352e8 > > > > > > => md5sum 0x6000 0x200 > > > md5 for 6000 ... 61ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050 > > > => hash md5 0x6000 0x200 > > > md5 for 6000 ... 61ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050 > > > > > > Considering that most of them (besides md5sum) are using the same > > > int hash_command() function under the hood, but have a lot of duplicated > > > code for handling params, does it make sense to do some cleanup and > > > drop all them in favour `hash`? > > > > > > I also plan to extend usage info for `hash` by adding a list > > > compiled-in algos based on hash related compiled flags > > > (CONFIG_SHA1, CONFIG_CRC32 etc), so it's clear what algos > > > are available for hash calculation. > > > > > > Comments/objections are welcome! > > > > It would be good, implementation wise, if each of those commands was > > just a redirect to hash ..., similar to how "load " will call the > > right filesystem calls. Does that make sense? Thanks. > > Yes, I actually have that one my todo list to basically alias the > individual commands to the hash command to remove those commands. My > intention there was a first step to allow us to eventually minimise or > even remove the use of obsolete hashes etc. Thank you for your comments. I'll re-factor as discussed and send a patch series when it's ready (plan to do that before the next merge window opens). Regards, Igor -- Best regards - Atentamente - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk http://ua.linkedin.com/in/iopaniuk
Re: [PATCH v1] cmd: hash: fix param count check
Hi Tom,
On Wed, Feb 7, 2024 at 1:01 AM Igor Opaniuk wrote:
>
> From: Igor Opaniuk
>
> Add correct check for parameter count.
>
> This fixes this issue when `hash` cmd is invoked without params:
>
> => hash
> data abort
> pc : [] lr : []
> reloc pc : [<60019204>]lr : [<5afcffa8>]
> sp : ba6dd9c8 ip : bf7391f0 fp : bf74ec14
> r10: 0001 r9 : ba6dfea0 r8 : bf7ea030
> r7 : r6 : ba6effa8 r5 : r4 :
> r3 : bf7c257c r2 : 0001 r1 : r0 : bf7e6e34
> Flags: nZCv IRQs off FIQs on Mode SVC_32
> Code: e5934004 e1a0e003 e59f3050 e2444001 (e5f4c001)
> Resetting CPU ...
>
> resetting ...
>
> Signed-off-by: Igor Opaniuk
> ---
>
> cmd/hash.c | 19 ++-
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/cmd/hash.c b/cmd/hash.c
> index e163cd67742..5534a735fa7 100644
> --- a/cmd/hash.c
> +++ b/cmd/hash.c
> @@ -14,15 +14,22 @@
> #include
> #include
>
> +#if IS_ENABLED(CONFIG_HASH_VERIFY)
> +#define HARGS 6
> +#else
> +#define HARGS 5
> +#endif
> +
> static int do_hash(struct cmd_tbl *cmdtp, int flag, int argc,
>char *const argv[])
> {
> char *s;
> int flags = HASH_FLAG_ENV;
>
> -#ifdef CONFIG_HASH_VERIFY
> - if (argc < 4)
> + if (argc < (HARGS - 1))
> return CMD_RET_USAGE;
> +
> +#if IS_ENABLED(CONFIG_HASH_VERIFY)
> if (!strcmp(argv[1], "-v")) {
> flags |= HASH_FLAG_VERIFY;
> argc--;
> @@ -37,18 +44,12 @@ static int do_hash(struct cmd_tbl *cmdtp, int flag, int
> argc,
> return hash_command(*argv, flags, cmdtp, flag, argc - 1, argv + 1);
> }
>
> -#ifdef CONFIG_HASH_VERIFY
> -#define HARGS 6
> -#else
> -#define HARGS 5
> -#endif
> -
> U_BOOT_CMD(
> hash, HARGS, 1, do_hash,
> "compute hash message digest",
> "algorithm address count [[*]hash_dest]\n"
> "- compute message digest [save to env var / *address]"
> -#ifdef CONFIG_HASH_VERIFY
> +#if IS_ENABLED(CONFIG_HASH_VERIFY)
> "\nhash -v algorithm address count [*]hash\n"
> "- verify message digest of memory area to immediate
> value, \n"
> " env var or *address"
> --
> 2.34.1
>
This is a fix, however I haven't added the "Fixes: " tag as I could not find
the commit which introduces this cmd (it seems it was added before
U-Boot moved to git).
For the next merge window I'll refactor all hash cmds as we discussed
in my previous email [1]
[1]
https://lore.kernel.org/u-boot/calede9mdjzjtmdhbg-vfis1e5uvfyn4wwd1hopgywxripfj...@mail.gmail.com/T/#t
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
http://ua.linkedin.com/in/iopaniuk
Re: [PATCH] stm32mp: cmd_stm32prog: add dependencies with USB_GADGET_DOWNLOAD
On Wed, Feb 7, 2024 at 2:12 PM Patrick Delaunay wrote: > > This patch avoids compilation issue when CONFIG_USB_GADGET is deactivated > in defconfig, with undefined reference to run_usb_dnl_gadget and to > g_dnl_set_product. > > Signed-off-by: Patrick Delaunay > --- > > arch/arm/mach-stm32mp/cmd_stm32prog/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/mach-stm32mp/cmd_stm32prog/Kconfig > b/arch/arm/mach-stm32mp/cmd_stm32prog/Kconfig > index 8f91db4b46b9..589276282e44 100644 > --- a/arch/arm/mach-stm32mp/cmd_stm32prog/Kconfig > +++ b/arch/arm/mach-stm32mp/cmd_stm32prog/Kconfig > @@ -17,6 +17,7 @@ config CMD_STM32PROG > config CMD_STM32PROG_USB > bool "support stm32prog over USB" > depends on CMD_STM32PROG > + depends on USB_GADGET_DOWNLOAD > default y > help > activate the command "stm32prog usb" for STM32MP soc family > -- > 2.25.1 > Reviewed-by: Igor Opaniuk -- Best regards - Freundliche Grüsse - Meilleures salutations Igor Opaniuk Senior Software Engineer, Embedded & Security E: igor.opan...@foundries.io W: www.foundries.io
Re: [RFC] Drop md5sum, crc32 and sha1 cmds in favor of hash cmd
Hi Tom,
On Wed, Feb 7, 2024 at 2:48 PM Tom Rini wrote:
>
> On Wed, Feb 07, 2024 at 02:00:16PM +0100, Igor Opaniuk wrote:
> > Hello,
> >
> > I was playing a bit with different hash functions recently, and
> > it turned out that md5sum, crc32, sha1 cmds just duplicate
> > what is already covered by generic `hash` cmd.
> >
> > => sha1 0x6000 0x200
> > sha1 for 6000 ... 61ff ==> 4ff5ffc91d00a95155518b920f46e2483d0e1437
> > => hash sha1 0x6000 0x200
> > sha1 for 6000 ... 61ff ==> 4ff5ffc91d00a95155518b920f46e2483d0e1437
> >
> > => crc32 0x6000 0x200
> > crc32 for 6000 ... 61ff ==> 6fe352e8
> > => hash crc32 0x6000 0x200
> > crc32 for 6000 ... 61ff ==> 6fe352e8
> >
> > => md5sum 0x6000 0x200
> > md5 for 6000 ... 61ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050
> > => hash md5 0x6000 0x200
> > md5 for 6000 ... 61ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050
> >
> > Considering that most of them (besides md5sum) are using the same
> > int hash_command() function under the hood, but have a lot of duplicated
> > code for handling params, does it make sense to do some cleanup and
> > drop all them in favour `hash`?
> >
> > I also plan to extend usage info for `hash` by adding a list
> > compiled-in algos based on hash related compiled flags
> > (CONFIG_SHA1, CONFIG_CRC32 etc), so it's clear what algos
> > are available for hash calculation.
> >
> > Comments/objections are welcome!
>
> It would be good, implementation wise, if each of those commands was
> just a redirect to hash ..., similar to how "load " will call the
> right filesystem calls. Does that make sense? Thanks.
Sure. You mean something like this?:
/* cmd/crc32.c */
int do_crc32(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
{
return do_hash(cmdtp, flag, argc, argv);
}
/* cmd/hash.c */
static int do_hash(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[]) {
...
/* if not true, we are called from crc32/sha1/etc alias */
if (!strcmp(argv[0], "hash")) {
/*
* Hash cmd is used directly,
* Move forward to 'algorithm' parameter
*/
argc--;
argv++;
}
...
}
>
> --
> Tom
Regards,
Igor
--
Best regards - Freundliche Grüsse - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
http://ua.linkedin.com/in/iopaniuk
[RFC] Drop md5sum, crc32 and sha1 cmds in favor of hash cmd
Hello, I was playing a bit with different hash functions recently, and it turned out that md5sum, crc32, sha1 cmds just duplicate what is already covered by generic `hash` cmd. => sha1 0x6000 0x200 sha1 for 6000 ... 61ff ==> 4ff5ffc91d00a95155518b920f46e2483d0e1437 => hash sha1 0x6000 0x200 sha1 for 6000 ... 61ff ==> 4ff5ffc91d00a95155518b920f46e2483d0e1437 => crc32 0x6000 0x200 crc32 for 6000 ... 61ff ==> 6fe352e8 => hash crc32 0x6000 0x200 crc32 for 6000 ... 61ff ==> 6fe352e8 => md5sum 0x6000 0x200 md5 for 6000 ... 61ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050 => hash md5 0x6000 0x200 md5 for 6000 ... 61ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050 Considering that most of them (besides md5sum) are using the same int hash_command() function under the hood, but have a lot of duplicated code for handling params, does it make sense to do some cleanup and drop all them in favour `hash`? I also plan to extend usage info for `hash` by adding a list compiled-in algos based on hash related compiled flags (CONFIG_SHA1, CONFIG_CRC32 etc), so it's clear what algos are available for hash calculation. Comments/objections are welcome! Regards, Igor -- Best regards - Freundliche Grüsse - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk +48 666 108 041 http://ua.linkedin.com/in/iopaniuk
Re: [PATCH 2/6] arm: mach-k3: Move disable_linefill_optimization() into R5 directory
Hi Andrew,
On Fri, Feb 2, 2024 at 1:26 AM Andrew Davis wrote:
>
> The disable_linefill_optimization() function is only ever loaded by the
> R5 core, move the code into the R5 directory.
>
> Signed-off-by: Andrew Davis
> ---
> arch/arm/mach-k3/common.c| 25 -
> arch/arm/mach-k3/r5/Makefile | 1 +
> arch/arm/mach-k3/r5/common.c | 35 +++
> 3 files changed, 36 insertions(+), 25 deletions(-)
> create mode 100644 arch/arm/mach-k3/r5/common.c
>
> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
> index f411366778f..5d53efed85b 100644
> --- a/arch/arm/mach-k3/common.c
> +++ b/arch/arm/mach-k3/common.c
> @@ -453,31 +453,6 @@ void board_prep_linux(struct bootm_headers *images)
> }
> #endif
>
> -#ifdef CONFIG_CPU_V7R
> -void disable_linefill_optimization(void)
> -{
> - u32 actlr;
> -
> - /*
> -* On K3 devices there are 2 conditions where R5F can deadlock:
> -* 1.When software is performing series of store operations to
> -* cacheable write back/write allocate memory region and later
> -* on software execute barrier operation (DSB or DMB). R5F may
> -* hang at the barrier instruction.
> -* 2.When software is performing a mix of load and store operations
> -* within a tight loop and store operations are all writing to
> -* cacheable write back/write allocates memory regions, R5F may
> -* hang at one of the load instruction.
> -*
> -* To avoid the above two conditions disable linefill optimization
> -* inside Cortex R5F.
> -*/
> - asm("mrc p15, 0, %0, c1, c0, 1" : "=r" (actlr));
> - actlr |= (1 << 13); /* Set DLFO bit */
> - asm("mcr p15, 0, %0, c1, c0, 1" : : "r" (actlr));
> -}
> -#endif
> -
> static void remove_fwl_regions(struct fwl_data fwl_data, size_t num_regions,
>enum k3_firewall_region_type fwl_type)
> {
> diff --git a/arch/arm/mach-k3/r5/Makefile b/arch/arm/mach-k3/r5/Makefile
> index b99199d3374..8ad86eb2798 100644
> --- a/arch/arm/mach-k3/r5/Makefile
> +++ b/arch/arm/mach-k3/r5/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_SOC_K3_J721S2) += j721s2/
> obj-$(CONFIG_SOC_K3_AM625) += am62x/
> obj-$(CONFIG_SOC_K3_AM62A7) += am62ax/
>
> +obj-y += common.o
> obj-y += lowlevel_init.o
> obj-y += r5_mpu.o
>
> diff --git a/arch/arm/mach-k3/r5/common.c b/arch/arm/mach-k3/r5/common.c
> new file mode 100644
> index 000..ef81f50c6c7
> --- /dev/null
> +++ b/arch/arm/mach-k3/r5/common.c
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * K3: R5 Common Architecture initialization
> + *
> + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include "../common.h"
> +
> +void disable_linefill_optimization(void)
> +{
> + u32 actlr;
> +
> + /*
> +* On K3 devices there are 2 conditions where R5F can deadlock:
> +* 1.When software is performing series of store operations to
> +* cacheable write back/write allocate memory region and later
> +* on software execute barrier operation (DSB or DMB). R5F may
> +* hang at the barrier instruction.
> +* 2.When software is performing a mix of load and store operations
> +* within a tight loop and store operations are all writing to
> +* cacheable write back/write allocates memory regions, R5F may
> +* hang at one of the load instruction.
> + *
> +* To avoid the above two conditions disable linefill optimization
> +* inside Cortex R5F.
> +*/
> + asm("mrc p15, 0, %0, c1, c0, 1" : "=r" (actlr));
> + actlr |= (1 << 13); /* Set DLFO bit */
> + asm("mcr p15, 0, %0, c1, c0, 1" : : "r" (actlr));
> +}
> --
> 2.39.2
>
Reviewed-by: Igor Opaniuk
--
Best regards - Freundliche Grüsse - Meilleures salutations
Igor Opaniuk
Senior Software Engineer, Embedded & Security
E: igor.opan...@foundries.io
W: www.foundries.io
Re: [PATCH 1/6] arm: mach-k3: Move SYS_K3_SPL_ATF definition into R5 Kconfig
Hi Andrew, On Fri, Feb 2, 2024 at 1:25 AM Andrew Davis wrote: > > Loading ATF is only supported from the R5, move the Kconfig symbol > definition to match. > > Signed-off-by: Andrew Davis > --- > arch/arm/mach-k3/Kconfig| 7 --- > arch/arm/mach-k3/r5/Kconfig | 6 ++ > 2 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig > index 03898424c95..45cf740bb1f 100644 > --- a/arch/arm/mach-k3/Kconfig > +++ b/arch/arm/mach-k3/Kconfig > @@ -114,13 +114,6 @@ config K3_EARLY_CONS_IDX > Use this option to set the index of the serial device to be used > for the early console during SPL execution. > > -config SYS_K3_SPL_ATF > - bool "Start Cortex-A from SPL" > - depends on CPU_V7R > - help > - Enabling this will try to start Cortex-A (typically with ATF) > - after SPL from R5. > - > config K3_ATF_LOAD_ADDR > hex "Load address of ATF image" > default 0x7000 > diff --git a/arch/arm/mach-k3/r5/Kconfig b/arch/arm/mach-k3/r5/Kconfig > index ae79f8ff6cd..317a6c4b67e 100644 > --- a/arch/arm/mach-k3/r5/Kconfig > +++ b/arch/arm/mach-k3/r5/Kconfig > @@ -43,3 +43,9 @@ config K3_SYSFW_IMAGE_SPI_OFFS > help > Offset of the combined System Firmware and configuration image tree > blob to be loaded when booting from a SPI flash memory. > + > +config SYS_K3_SPL_ATF > + bool "Start Cortex-A from SPL" > + help > + Enabling this will try to start Cortex-A (typically with ATF) > + after SPL from R5. > -- > 2.39.2 > Reviewed-by: Igor Opaniuk -- Best regards - Freundliche Grüsse - Meilleures salutations Igor Opaniuk Senior Software Engineer, Embedded & Security E: igor.opan...@foundries.io W: www.foundries.io
[PATCH v1] cmd: hash: fix param count check
From: Igor Opaniuk
Add correct check for parameter count.
This fixes this issue when `hash` cmd is invoked without params:
=> hash
data abort
pc : [] lr : []
reloc pc : [<60019204>]lr : [<5afcffa8>]
sp : ba6dd9c8 ip : bf7391f0 fp : bf74ec14
r10: 0001 r9 : ba6dfea0 r8 : bf7ea030
r7 : r6 : ba6effa8 r5 : r4 :
r3 : bf7c257c r2 : 0001 r1 : r0 : bf7e6e34
Flags: nZCv IRQs off FIQs on Mode SVC_32
Code: e5934004 e1a0e003 e59f3050 e2444001 (e5f4c001)
Resetting CPU ...
resetting ...
Signed-off-by: Igor Opaniuk
---
cmd/hash.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/cmd/hash.c b/cmd/hash.c
index e163cd67742..5534a735fa7 100644
--- a/cmd/hash.c
+++ b/cmd/hash.c
@@ -14,15 +14,22 @@
#include
#include
+#if IS_ENABLED(CONFIG_HASH_VERIFY)
+#define HARGS 6
+#else
+#define HARGS 5
+#endif
+
static int do_hash(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
{
char *s;
int flags = HASH_FLAG_ENV;
-#ifdef CONFIG_HASH_VERIFY
- if (argc < 4)
+ if (argc < (HARGS - 1))
return CMD_RET_USAGE;
+
+#if IS_ENABLED(CONFIG_HASH_VERIFY)
if (!strcmp(argv[1], "-v")) {
flags |= HASH_FLAG_VERIFY;
argc--;
@@ -37,18 +44,12 @@ static int do_hash(struct cmd_tbl *cmdtp, int flag, int
argc,
return hash_command(*argv, flags, cmdtp, flag, argc - 1, argv + 1);
}
-#ifdef CONFIG_HASH_VERIFY
-#define HARGS 6
-#else
-#define HARGS 5
-#endif
-
U_BOOT_CMD(
hash, HARGS, 1, do_hash,
"compute hash message digest",
"algorithm address count [[*]hash_dest]\n"
"- compute message digest [save to env var / *address]"
-#ifdef CONFIG_HASH_VERIFY
+#if IS_ENABLED(CONFIG_HASH_VERIFY)
"\nhash -v algorithm address count [*]hash\n"
"- verify message digest of memory area to immediate value,
\n"
" env var or *address"
--
2.34.1
[PATCH v1 7/7] doc: android: avb: add slot_suffix param details
From: Igor Opaniuk Add info about slot_suffix param for avb verify. Signed-off-by: Igor Opaniuk --- doc/android/avb2.rst | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/android/avb2.rst b/doc/android/avb2.rst index a072119574f..c0b2bedb831 100644 --- a/doc/android/avb2.rst +++ b/doc/android/avb2.rst @@ -39,15 +39,17 @@ Provides CLI interface to invoke AVB 2.0 verification + misc. commands for different testing purposes:: avb init - initialize avb 2.0 for -avb verify - run verification process using hash data from vbmeta structure +avb verify [slot_suffix] - run verification process using hash data +from vbmeta structure. Provide [slot_suffix] if vbmeta partition +is slotted avb read_rb - read rollback index at location avb write_rb - write rollback index to avb is_unlocked - returns unlock status of the device avb get_uuid - read and print uuid of partition avb read_part - read bytes from -partition to buffer +partition to buffer avb write_part - write bytes to - by using data from + by using data from Partitions tampering (example) -- -- 2.34.1
[PATCH v1 6/7] cmd: avb: rework do_avb_verify_part
From: Igor Opaniuk
Use existing str_avb_slot_error() function for obtaining
verification fail reason details.
Take into account device lock state for setting correct
androidboot.verifiedbootstate kernel cmdline parameter.
Signed-off-by: Igor Opaniuk
---
cmd/avb.c | 50 +-
1 file changed, 17 insertions(+), 33 deletions(-)
diff --git a/cmd/avb.c b/cmd/avb.c
index ae0012c0e79..e5fc202121f 100644
--- a/cmd/avb.c
+++ b/cmd/avb.c
@@ -239,6 +239,7 @@ int do_avb_verify_part(struct cmd_tbl *cmdtp, int flag,
const char * const requested_partitions[] = {"boot", NULL};
AvbSlotVerifyResult slot_result;
AvbSlotVerifyData *out_data;
+ enum avb_boot_state boot_state;
char *cmdline;
char *extra_args;
char *slot_suffix = "";
@@ -273,18 +274,23 @@ int do_avb_verify_part(struct cmd_tbl *cmdtp, int flag,
AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE,
_data);
- switch (slot_result) {
- case AVB_SLOT_VERIFY_RESULT_OK:
- /* Until we don't have support of changing unlock states, we
-* assume that we are by default in locked state.
-* So in this case we can boot only when verification is
-* successful; we also supply in cmdline GREEN boot state
-*/
+ /*
+* LOCKED devices with custom root of trust setup is not supported
(YELLOW)
+*/
+ if (slot_result == AVB_SLOT_VERIFY_RESULT_OK) {
printf("Verification passed successfully\n");
- /* export additional bootargs to AVB_BOOTARGS env var */
+ /*
+* ORANGE state indicates that device may be freely modified.
+* Device integrity is left to the user to verify out-of-band.
+*/
+ if (unlocked)
+ boot_state = AVB_ORANGE;
+ else
+ boot_state = AVB_GREEN;
- extra_args = avb_set_state(avb_ops, AVB_GREEN);
+ /* export boot state to AVB_BOOTARGS env var */
+ extra_args = avb_set_state(avb_ops, boot_state);
if (extra_args)
cmdline = append_cmd_line(out_data->cmdline,
extra_args);
@@ -294,30 +300,8 @@ int do_avb_verify_part(struct cmd_tbl *cmdtp, int flag,
env_set(AVB_BOOTARGS, cmdline);
res = CMD_RET_SUCCESS;
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_VERIFICATION:
- printf("Verification failed\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_IO:
- printf("I/O error occurred during verification\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_OOM:
- printf("OOM error occurred during verification\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA:
- printf("Corrupted dm-verity metadata detected\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_UNSUPPORTED_VERSION:
- printf("Unsupported version of avbtool was used\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_ROLLBACK_INDEX:
- printf("Rollback index check failed\n");
- break;
- case AVB_SLOT_VERIFY_RESULT_ERROR_PUBLIC_KEY_REJECTED:
- printf("Public key was rejected\n");
- break;
- default:
- printf("Unknown error occurred\n");
+ } else {
+ printf("Verification failed, reason: %s\n",
str_avb_slot_error(slot_result));
}
if (out_data)
--
2.34.1
[PATCH v1 5/7] common: avb_verify: add str_avb_io_error/str_avb_slot_error
From: Igor Opaniuk
Introduce str_avb_io_error() and str_avb_slot_error() functions,
that provide a pointer to AVB runtime error message.
Signed-off-by: Igor Opaniuk
---
common/avb_verify.c | 49
include/avb_verify.h | 3 ++-
2 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/common/avb_verify.c b/common/avb_verify.c
index ed58239cf8a..cff9117d92f 100644
--- a/common/avb_verify.c
+++ b/common/avb_verify.c
@@ -119,6 +119,55 @@ static const unsigned char avb_root_pub[1032] = {
0xd8, 0x7e,
};
+const char *str_avb_io_error(AvbIOResult res)
+{
+ switch (res) {
+ case AVB_IO_RESULT_OK:
+ return "Requested operation was successful";
+ case AVB_IO_RESULT_ERROR_IO:
+ return "Underlying hardware encountered an I/O error";
+ case AVB_IO_RESULT_ERROR_OOM:
+ return "Unable to allocate memory";
+ case AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION:
+ return "Requested partition does not exist";
+ case AVB_IO_RESULT_ERROR_RANGE_OUTSIDE_PARTITION:
+ return "Bytes requested is outside the range of partition";
+ case AVB_IO_RESULT_ERROR_NO_SUCH_VALUE:
+ return "Named persistent value does not exist";
+ case AVB_IO_RESULT_ERROR_INVALID_VALUE_SIZE:
+ return "Named persistent value size is not supported";
+ case AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE:
+ return "Buffer is too small for the requested operation";
+ default:
+ return "Unknown AVB error";
+ }
+}
+
+const char *str_avb_slot_error(AvbSlotVerifyResult res)
+{
+ switch (res) {
+ case AVB_SLOT_VERIFY_RESULT_OK:
+ return "Verification passed successfully";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_OOM:
+ return "Allocation of memory failed";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_IO:
+ return "I/O error occurred while trying to load data";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_VERIFICATION:
+ return "Digest didn't match or signature checks failed";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_ROLLBACK_INDEX:
+ return "Rollback index is less than its stored value";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_PUBLIC_KEY_REJECTED:
+ return "Public keys are not accepted";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA:
+ return "Metadata is invalid or inconsistent";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_UNSUPPORTED_VERSION:
+ return "Metadata requires a newer version of libavb";
+ case AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT:
+ return "Invalid arguments are used";
+ default:
+ return "Unknown AVB slot verification error";
+ }
+}
/**
*
* Boot states support (GREEN, YELLOW, ORANGE, RED) and dm_verity
diff --git a/include/avb_verify.h b/include/avb_verify.h
index 2fb850044d9..5d998b5a302 100644
--- a/include/avb_verify.h
+++ b/include/avb_verify.h
@@ -52,7 +52,8 @@ char *avb_set_enforce_verity(const char *cmdline);
char *avb_set_ignore_corruption(const char *cmdline);
char *append_cmd_line(char *cmdline_orig, char *cmdline_new);
-
+const char *str_avb_io_error(AvbIOResult res);
+const char *str_avb_slot_error(AvbSlotVerifyResult res);
/**
*
* I/O helper inline functions
--
2.34.1
[PATCH v1 4/7] cmd: avb: rework prints
From: Igor Opaniuk
Introduce AVB_OPS_CHECK macro for checking AvbOps before using
it to avoid code duplication.
Simplify and add more context for prints where it's needed.
Signed-off-by: Igor Opaniuk
---
cmd/avb.c | 156 --
1 file changed, 80 insertions(+), 76 deletions(-)
diff --git a/cmd/avb.c b/cmd/avb.c
index ce8b63873f2..ae0012c0e79 100644
--- a/cmd/avb.c
+++ b/cmd/avb.c
@@ -11,6 +11,14 @@
#include
#define AVB_BOOTARGS "avb_bootargs"
+
+#define AVB_OPS_CHECK(avb_ops) do { \
+ if (!(avb_ops)) { \
+ printf("AVB is not initialized, please run 'avb init '\n");
\
+ return CMD_RET_FAILURE; \
+ } \
+} while (false)
+
static struct AvbOps *avb_ops;
int do_avb_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
@@ -28,8 +36,10 @@ int do_avb_init(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
avb_ops = avb_ops_alloc(mmc_dev);
if (avb_ops)
return CMD_RET_SUCCESS;
+ else
+ printf("Can't allocate AvbOps");
- printf("Failed to initialize avb2\n");
+ printf("Failed to initialize AVB\n");
return CMD_RET_FAILURE;
}
@@ -41,11 +51,9 @@ int do_avb_read_part(struct cmd_tbl *cmdtp, int flag, int
argc,
s64 offset;
size_t bytes, bytes_read = 0;
void *buffer;
+ int ret;
- if (!avb_ops) {
- printf("AVB 2.0 is not initialized, please run 'avb init'\n");
- return CMD_RET_USAGE;
- }
+ AVB_OPS_CHECK(avb_ops);
if (argc != 5)
return CMD_RET_USAGE;
@@ -55,14 +63,15 @@ int do_avb_read_part(struct cmd_tbl *cmdtp, int flag, int
argc,
bytes = hextoul(argv[3], NULL);
buffer = (void *)hextoul(argv[4], NULL);
- if (avb_ops->read_from_partition(avb_ops, part, offset, bytes,
-buffer, _read) ==
-AVB_IO_RESULT_OK) {
+ ret = avb_ops->read_from_partition(avb_ops, part, offset,
+ bytes, buffer, _read);
+ if (ret == AVB_IO_RESULT_OK) {
printf("Read %zu bytes\n", bytes_read);
return CMD_RET_SUCCESS;
}
- printf("Failed to read from partition\n");
+ printf("Failed to read from partition '%s', err = %d\n",
+ part, ret);
return CMD_RET_FAILURE;
}
@@ -74,11 +83,9 @@ int do_avb_read_part_hex(struct cmd_tbl *cmdtp, int flag,
int argc,
s64 offset;
size_t bytes, bytes_read = 0;
char *buffer;
+ int ret;
- if (!avb_ops) {
- printf("AVB 2.0 is not initialized, please run 'avb init'\n");
- return CMD_RET_USAGE;
- }
+ AVB_OPS_CHECK(avb_ops);
if (argc != 4)
return CMD_RET_USAGE;
@@ -94,8 +101,9 @@ int do_avb_read_part_hex(struct cmd_tbl *cmdtp, int flag,
int argc,
}
memset(buffer, 0, bytes);
- if (avb_ops->read_from_partition(avb_ops, part, offset, bytes, buffer,
-_read) == AVB_IO_RESULT_OK) {
+ ret = avb_ops->read_from_partition(avb_ops, part, offset,
+ bytes, buffer, _read);
+ if (ret == AVB_IO_RESULT_OK) {
printf("Requested %zu, read %zu bytes\n", bytes, bytes_read);
printf("Data: ");
for (int i = 0; i < bytes_read; i++)
@@ -107,7 +115,8 @@ int do_avb_read_part_hex(struct cmd_tbl *cmdtp, int flag,
int argc,
return CMD_RET_SUCCESS;
}
- printf("Failed to read from partition\n");
+ printf("Failed to read from partition '%s', err = %d\n",
+ part, ret);
free(buffer);
return CMD_RET_FAILURE;
@@ -120,11 +129,9 @@ int do_avb_write_part(struct cmd_tbl *cmdtp, int flag, int
argc,
s64 offset;
size_t bytes;
void *buffer;
+ int ret;
- if (!avb_ops) {
- printf("AVB 2.0 is not initialized, run 'avb init' first\n");
- return CMD_RET_FAILURE;
- }
+ AVB_OPS_CHECK(avb_ops);
if (argc != 5)
return CMD_RET_USAGE;
@@ -134,13 +141,15 @@ int do_avb_write_part(struct cmd_tbl *cmdtp, int flag,
int argc,
bytes = hextoul(argv[3], NULL);
buffer = (void *)hextoul(argv[4], NULL);
- if (avb_ops->write_to_partition(avb_ops, part, offset, bytes, buffer) ==
- AVB_IO_RESULT_OK) {
+ ret = avb_ops->write_to_partition(avb_ops, part, offset,
+ bytes, buffer);
+ if (ret == AVB_IO_RESULT_OK) {
printf("Wrote %zu bytes\n", bytes);
[PATCH v1 3/7] common: avb_verify: rework error/debug prints
From: Igor Opaniuk
Make error prints more verbose with additional context.
Also s/print/debug/g for prints, which might be relevant only
for debugging purposes.
Signed-off-by: Igor Opaniuk
---
common/avb_verify.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/common/avb_verify.c b/common/avb_verify.c
index 938a5383b5d..ed58239cf8a 100644
--- a/common/avb_verify.c
+++ b/common/avb_verify.c
@@ -279,9 +279,9 @@ static unsigned long mmc_read_and_flush(struct mmc_part
*part,
* Reading fails on unaligned buffers, so we have to
* use aligned temporary buffer and then copy to destination
*/
-
if (unaligned) {
- printf("Handling unaligned read buffer..\n");
+ debug("%s: handling unaligned read buffer, addr = 0x%p\n",
+ __func__, buffer);
tmp_buf = get_sector_buf();
buf_size = get_sector_buf_size();
if (sectors > buf_size / part->info.blksz)
@@ -320,7 +320,8 @@ static unsigned long mmc_write(struct mmc_part *part,
lbaint_t start,
if (unaligned) {
tmp_buf = get_sector_buf();
buf_size = get_sector_buf_size();
- printf("Handling unaligned wrire buffer..\n");
+ debug("%s: handling unaligned read buffer, addr = 0x%p\n",
+ __func__, buffer);
if (sectors > buf_size / part->info.blksz)
sectors = buf_size / part->info.blksz;
@@ -348,30 +349,35 @@ static struct mmc_part *get_partition(AvbOps *ops, const
char *partition)
dev_num = get_boot_device(ops);
part->mmc = find_mmc_device(dev_num);
if (!part->mmc) {
- printf("No MMC device at slot %x\n", dev_num);
+ printf("%s: no MMC device at slot %x\n", __func__, dev_num);
goto err;
}
- if (mmc_init(part->mmc)) {
- printf("MMC initialization failed\n");
+ ret = mmc_init(part->mmc);
+ if (ret) {
+ printf("%s: MMC initialization failed, err = %d\n",
+ __func__, ret);
goto err;
}
if (IS_MMC(part->mmc)) {
ret = mmc_switch_part(part->mmc, part_num);
- if (ret)
+ if (ret) {
+ printf("%s: MMC part switch failed, err = %d\n",
+ __func__, ret);
goto err;
+ }
}
mmc_blk = mmc_get_blk_desc(part->mmc);
if (!mmc_blk) {
- printf("Error - failed to obtain block descriptor\n");
+ printf("%s: failed to obtain block descriptor\n", __func__);
goto err;
}
ret = part_get_info_by_name(mmc_blk, partition, >info);
if (ret < 0) {
- printf("Can't find partition '%s'\n", partition);
+ printf("%s: can't find partition '%s'\n", __func__, partition);
goto err;
}
@@ -684,7 +690,7 @@ static AvbIOResult read_rollback_index(AvbOps *ops,
{
#ifndef CONFIG_OPTEE_TA_AVB
/* For now we always return 0 as the stored rollback index. */
- printf("%s not supported yet\n", __func__);
+ debug("%s: rollback protection is not implemented\n", __func__);
if (out_rollback_index)
*out_rollback_index = 0;
@@ -730,7 +736,7 @@ static AvbIOResult write_rollback_index(AvbOps *ops,
{
#ifndef CONFIG_OPTEE_TA_AVB
/* For now this is a no-op. */
- printf("%s not supported yet\n", __func__);
+ debug("%s: rollback protection is not implemented\n", __func__);
return AVB_IO_RESULT_OK;
#else
@@ -766,8 +772,7 @@ static AvbIOResult read_is_device_unlocked(AvbOps *ops,
bool *out_is_unlocked)
{
#ifndef CONFIG_OPTEE_TA_AVB
/* For now we always return that the device is unlocked. */
-
- printf("%s not supported yet\n", __func__);
+ debug("%s: device locking is not implemented\n", __func__);
*out_is_unlocked = true;
--
2.34.1
