[PATCH 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled
In case Dcache is enabled while the ECDSA authentication function is called via BootROM ROM API, the CRYP DMA might pick stale version of data from DRAM. Disable Dcache around the BootROM call to avoid this issue.
Signed-off-by: Marek Vasut marex@denx.de --- Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com --- arch/arm/mach-stm32mp/ecdsa_romapi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index a2f63ff879f..72b87bf2c64 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -64,6 +64,7 @@ static int romapi_ecdsa_verify(struct udevice *dev, const void *signature, size_t sig_len) { struct ecdsa_rom_api rom; + bool reenable_dcache; uint8_t raw_key[64]; uint32_t rom_ret; int algo; @@ -81,8 +82,21 @@ static int romapi_ecdsa_verify(struct udevice *dev, memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions(&rom); + + /* + * Disable D-cache before calling into BootROM, else CRYP DMA + * may fail to pick up the correct data. + */ + if (dcache_status()) { + dcache_disable(); + reenable_dcache = true; + } + rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
+ if (reenable_dcache) + dcache_enable(); + return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM; }
The STM32MP15xx platform currently comes with two incompatible implementations of save_boot_params() weak function override. Factor the save_boot_params() implementation into common cpu.c code and provide accessors to read out both ROM API table address and DT address from any place in the code instead.
Signed-off-by: Marek Vasut marex@denx.de --- Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com --- arch/arm/mach-stm32mp/boot_params.c | 20 ++--------- arch/arm/mach-stm32mp/cpu.c | 35 +++++++++++++++++++ arch/arm/mach-stm32mp/ecdsa_romapi.c | 20 ++--------- .../arm/mach-stm32mp/include/mach/sys_proto.h | 3 ++ 4 files changed, 42 insertions(+), 36 deletions(-)
diff --git a/arch/arm/mach-stm32mp/boot_params.c b/arch/arm/mach-stm32mp/boot_params.c index e91ef1b2fc7..e40cca938ef 100644 --- a/arch/arm/mach-stm32mp/boot_params.c +++ b/arch/arm/mach-stm32mp/boot_params.c @@ -11,30 +11,14 @@ #include <asm/sections.h> #include <asm/system.h>
-/* - * Force data-section, as .bss will not be valid - * when save_boot_params is invoked. - */ -static unsigned long nt_fw_dtb __section(".data"); - -/* - * Save the FDT address provided by TF-A in r2 at boot time - * This function is called from start.S - */ -void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2, - unsigned long r3) -{ - nt_fw_dtb = r2; - - save_boot_params_ret(); -} - /* * Use the saved FDT address provided by TF-A at boot time (NT_FW_CONFIG = * Non Trusted Firmware configuration file) when the pointer is valid */ void *board_fdt_blob_setup(int *err) { + unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb(); + log_debug("%s: nt_fw_dtb=%lx\n", __func__, nt_fw_dtb);
*err = 0; diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index 855fc755fe0..fa02a11d867 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -378,3 +378,38 @@ int arch_misc_init(void)
return 0; } + +/* + * Without forcing the ".data" section, this would get saved in ".bss". BSS + * will be cleared soon after, so it's not suitable. + */ +static uintptr_t rom_api_table __section(".data"); +static uintptr_t nt_fw_dtb __section(".data"); + +/* + * The ROM gives us the API location in r0 when starting. This is only available + * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot. Save + * the FDT address provided by TF-A in r2 at boot time. This function is called + * from start.S + */ +void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2, + unsigned long r3) +{ +#if IS_ENABLED(CONFIG_STM32_ECDSA_VERIFY) + rom_api_table = r0; +#endif +#if IS_ENABLED(CONFIG_TFABOOT) + nt_fw_dtb = r2; +#endif + save_boot_params_ret(); +} + +uintptr_t get_stm32mp_rom_api_table(void) +{ + return rom_api_table; +} + +uintptr_t get_stm32mp_bl2_dtb(void) +{ + return nt_fw_dtb; +} diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index 72b87bf2c64..32a7357ad56 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -24,26 +24,10 @@ struct ecdsa_rom_api { uint32_t ecc_algo); };
-/* - * Without forcing the ".data" section, this would get saved in ".bss". BSS - * will be cleared soon after, so it's not suitable. - */ -static uintptr_t rom_api_loc __section(".data"); - -/* - * The ROM gives us the API location in r0 when starting. This is only available - * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot. - */ -void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2, - unsigned long r3) -{ - rom_api_loc = r0; - save_boot_params_ret(); -} - static void stm32mp_rom_get_ecdsa_functions(struct ecdsa_rom_api *rom) { - uintptr_t verify_ptr = rom_api_loc + ROM_API_OFFSET_ECDSA_VERIFY; + uintptr_t verify_ptr = get_stm32mp_rom_api_table() + + ROM_API_OFFSET_ECDSA_VERIFY;
rom->ecdsa_verify_signature = *(void **)verify_ptr; } diff --git a/arch/arm/mach-stm32mp/include/mach/sys_proto.h b/arch/arm/mach-stm32mp/include/mach/sys_proto.h index f19a70e53e0..0d39b67178e 100644 --- a/arch/arm/mach-stm32mp/include/mach/sys_proto.h +++ b/arch/arm/mach-stm32mp/include/mach/sys_proto.h @@ -77,3 +77,6 @@ void stm32mp_misc_init(void);
/* helper function: read data from OTP */ u32 get_otp(int index, int shift, int mask); + +uintptr_t get_stm32mp_rom_api_table(void); +uintptr_t get_stm32mp_bl2_dtb(void);
On 12/6/22 03:33, Marek Vasut wrote:
The STM32MP15xx platform currently comes with two incompatible implementations of save_boot_params() weak function override. Factor the save_boot_params() implementation into common cpu.c code and provide accessors to read out both ROM API table address and DT address from any place in the code instead.
Signed-off-by: Marek Vasut marex@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com
arch/arm/mach-stm32mp/boot_params.c | 20 ++--------- arch/arm/mach-stm32mp/cpu.c | 35 +++++++++++++++++++ arch/arm/mach-stm32mp/ecdsa_romapi.c | 20 ++--------- .../arm/mach-stm32mp/include/mach/sys_proto.h | 3 ++ 4 files changed, 42 insertions(+), 36 deletions(-)
diff --git a/arch/arm/mach-stm32mp/boot_params.c b/arch/arm/mach-stm32mp/boot_params.c index e91ef1b2fc7..e40cca938ef 100644 --- a/arch/arm/mach-stm32mp/boot_params.c +++ b/arch/arm/mach-stm32mp/boot_params.c @@ -11,30 +11,14 @@ #include <asm/sections.h> #include <asm/system.h>
-/*
- Force data-section, as .bss will not be valid
- when save_boot_params is invoked.
- */
-static unsigned long nt_fw_dtb __section(".data");
-/*
- Save the FDT address provided by TF-A in r2 at boot time
- This function is called from start.S
- */
-void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
unsigned long r3)-{
- nt_fw_dtb = r2;
- save_boot_params_ret();
-}
/*
- Use the saved FDT address provided by TF-A at boot time (NT_FW_CONFIG =
- Non Trusted Firmware configuration file) when the pointer is valid
*/ void *board_fdt_blob_setup(int *err) {
unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb();
log_debug("%s: nt_fw_dtb=%lx\n", __func__, nt_fw_dtb);
*err = 0;
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index 855fc755fe0..fa02a11d867 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -378,3 +378,38 @@ int arch_misc_init(void)
return 0; }
+/*
- Without forcing the ".data" section, this would get saved in ".bss". BSS
- will be cleared soon after, so it's not suitable.
- */
+static uintptr_t rom_api_table __section(".data"); +static uintptr_t nt_fw_dtb __section(".data");
+/*
- The ROM gives us the API location in r0 when starting. This is only available
- during SPL, as there isn't (yet) a mechanism to pass this on to u-boot. Save
- the FDT address provided by TF-A in r2 at boot time. This function is called
- from start.S
- */
+void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
unsigned long r3)+{ +#if IS_ENABLED(CONFIG_STM32_ECDSA_VERIFY)
- rom_api_table = r0;
+#endif +#if IS_ENABLED(CONFIG_TFABOOT)
- nt_fw_dtb = r2;
+#endif
- save_boot_params_ret();
+}
+uintptr_t get_stm32mp_rom_api_table(void) +{
- return rom_api_table;
+}
+uintptr_t get_stm32mp_bl2_dtb(void) +{
- return nt_fw_dtb;
+} diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index 72b87bf2c64..32a7357ad56 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -24,26 +24,10 @@ struct ecdsa_rom_api { uint32_t ecc_algo); };
-/*
- Without forcing the ".data" section, this would get saved in ".bss". BSS
- will be cleared soon after, so it's not suitable.
- */
-static uintptr_t rom_api_loc __section(".data");
-/*
- The ROM gives us the API location in r0 when starting. This is only available
- during SPL, as there isn't (yet) a mechanism to pass this on to u-boot.
- */
-void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
unsigned long r3)-{
- rom_api_loc = r0;
- save_boot_params_ret();
-}
static void stm32mp_rom_get_ecdsa_functions(struct ecdsa_rom_api *rom) {
- uintptr_t verify_ptr = rom_api_loc + ROM_API_OFFSET_ECDSA_VERIFY;
uintptr_t verify_ptr = get_stm32mp_rom_api_table() +
ROM_API_OFFSET_ECDSA_VERIFY;rom->ecdsa_verify_signature = *(void **)verify_ptr;
} diff --git a/arch/arm/mach-stm32mp/include/mach/sys_proto.h b/arch/arm/mach-stm32mp/include/mach/sys_proto.h index f19a70e53e0..0d39b67178e 100644 --- a/arch/arm/mach-stm32mp/include/mach/sys_proto.h +++ b/arch/arm/mach-stm32mp/include/mach/sys_proto.h @@ -77,3 +77,6 @@ void stm32mp_misc_init(void);
/* helper function: read data from OTP */ u32 get_otp(int index, int shift, int mask);
+uintptr_t get_stm32mp_rom_api_table(void); +uintptr_t get_stm32mp_bl2_dtb(void);
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice
Hi Marek,
On 12/6/22 03:33, Marek Vasut wrote:
The STM32MP15xx platform currently comes with two incompatible implementations of save_boot_params() weak function override. Factor the save_boot_params() implementation into common cpu.c code and provide accessors to read out both ROM API table address and DT address from any place in the code instead.
good idea.
Signed-off-by: Marek Vasut marex@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com
arch/arm/mach-stm32mp/boot_params.c | 20 ++--------- arch/arm/mach-stm32mp/cpu.c | 35 +++++++++++++++++++ arch/arm/mach-stm32mp/ecdsa_romapi.c | 20 ++--------- .../arm/mach-stm32mp/include/mach/sys_proto.h | 3 ++ 4 files changed, 42 insertions(+), 36 deletions(-)
diff --git a/arch/arm/mach-stm32mp/boot_params.c b/arch/arm/mach-stm32mp/boot_params.c index e91ef1b2fc7..e40cca938ef 100644 --- a/arch/arm/mach-stm32mp/boot_params.c +++ b/arch/arm/mach-stm32mp/boot_params.c @@ -11,30 +11,14 @@ #include <asm/sections.h> #include <asm/system.h>
-/*
- Force data-section, as .bss will not be valid
- when save_boot_params is invoked.
- */
-static unsigned long nt_fw_dtb __section(".data");
-/*
- Save the FDT address provided by TF-A in r2 at boot time
- This function is called from start.S
- */
-void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
unsigned long r3)-{
- nt_fw_dtb = r2;
- save_boot_params_ret();
-}
- /*
*/ void *board_fdt_blob_setup(int *err) {
- Use the saved FDT address provided by TF-A at boot time (NT_FW_CONFIG =
- Non Trusted Firmware configuration file) when the pointer is valid
unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb();
log_debug("%s: nt_fw_dtb=%lx\n", __func__, nt_fw_dtb);
*err = 0;
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index 855fc755fe0..fa02a11d867 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -378,3 +378,38 @@ int arch_misc_init(void)
return 0; }
+/*
- Without forcing the ".data" section, this would get saved in ".bss". BSS
- will be cleared soon after, so it's not suitable.
- */
+static uintptr_t rom_api_table __section(".data"); +static uintptr_t nt_fw_dtb __section(".data");
+/*
- The ROM gives us the API location in r0 when starting. This is only available
- during SPL, as there isn't (yet) a mechanism to pass this on to u-boot. Save
- the FDT address provided by TF-A in r2 at boot time. This function is called
- from start.S
- */
+void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
unsigned long r3)+{ +#if IS_ENABLED(CONFIG_STM32_ECDSA_VERIFY)
- rom_api_table = r0;
+#endif +#if IS_ENABLED(CONFIG_TFABOOT)
- nt_fw_dtb = r2;
+#endif
jut avoid #if when it is possible.
if (IS_ENABLED(CONFIG_STM32_ECDSA_VERIFY))
rom_api_table = r0;
if (IS_ENABLED(CONFIG_TFABOOT))
nt_fw_dtb = r2;
- save_boot_params_ret();
+}
+uintptr_t get_stm32mp_rom_api_table(void) +{
- return rom_api_table;
+}
+uintptr_t get_stm32mp_bl2_dtb(void) +{
- return nt_fw_dtb;
+} diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index 72b87bf2c64..32a7357ad56 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -24,26 +24,10 @@ struct ecdsa_rom_api { uint32_t ecc_algo); };
-/*
- Without forcing the ".data" section, this would get saved in ".bss". BSS
- will be cleared soon after, so it's not suitable.
- */
-static uintptr_t rom_api_loc __section(".data");
-/*
- The ROM gives us the API location in r0 when starting. This is only available
- during SPL, as there isn't (yet) a mechanism to pass this on to u-boot.
- */
-void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
unsigned long r3)-{
- rom_api_loc = r0;
- save_boot_params_ret();
-}
- static void stm32mp_rom_get_ecdsa_functions(struct ecdsa_rom_api *rom) {
- uintptr_t verify_ptr = rom_api_loc + ROM_API_OFFSET_ECDSA_VERIFY;
uintptr_t verify_ptr = get_stm32mp_rom_api_table() +
ROM_API_OFFSET_ECDSA_VERIFY;rom->ecdsa_verify_signature = *(void **)verify_ptr; }
diff --git a/arch/arm/mach-stm32mp/include/mach/sys_proto.h b/arch/arm/mach-stm32mp/include/mach/sys_proto.h index f19a70e53e0..0d39b67178e 100644 --- a/arch/arm/mach-stm32mp/include/mach/sys_proto.h +++ b/arch/arm/mach-stm32mp/include/mach/sys_proto.h @@ -77,3 +77,6 @@ void stm32mp_misc_init(void);
/* helper function: read data from OTP */ u32 get_otp(int index, int shift, int mask);
+uintptr_t get_stm32mp_rom_api_table(void); +uintptr_t get_stm32mp_bl2_dtb(void);
regards
Patrick
The ROM API table pointer is no longer accessible from U-Boot, fix this by passing the ROM API pointer through. This makes it possible for U-Boot to call ROM API functions to authenticate payload like signed fitImages.
Signed-off-by: Marek Vasut marex@denx.de --- Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com --- arch/arm/mach-stm32mp/cpu.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index fa02a11d867..9553ecd243c 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -22,6 +22,7 @@ #include <dm/device.h> #include <dm/uclass.h> #include <linux/bitops.h> +#include <spl.h>
/* * early TLB into the .data section so that it not get cleared @@ -413,3 +414,17 @@ uintptr_t get_stm32mp_bl2_dtb(void) { return nt_fw_dtb; } + +#ifdef CONFIG_SPL_BUILD +void jump_to_image_no_args(struct spl_image_info *spl_image) +{ + typedef void __noreturn (*image_entry_noargs_t)(u32 romapi); + uintptr_t romapi = get_stm32mp_rom_api_table(); + + image_entry_noargs_t image_entry = + (image_entry_noargs_t)spl_image->entry_point; + + printf("image entry point: 0x%lx\n", spl_image->entry_point); + image_entry(romapi); +} +#endif
On 12/6/22 03:33, Marek Vasut wrote:
The ROM API table pointer is no longer accessible from U-Boot, fix this by passing the ROM API pointer through. This makes it possible for U-Boot to call ROM API functions to authenticate payload like signed fitImages.
Signed-off-by: Marek Vasut marex@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com
arch/arm/mach-stm32mp/cpu.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index fa02a11d867..9553ecd243c 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -22,6 +22,7 @@ #include <dm/device.h> #include <dm/uclass.h> #include <linux/bitops.h> +#include <spl.h>
/*
- early TLB into the .data section so that it not get cleared
@@ -413,3 +414,17 @@ uintptr_t get_stm32mp_bl2_dtb(void) { return nt_fw_dtb; }
+#ifdef CONFIG_SPL_BUILD +void jump_to_image_no_args(struct spl_image_info *spl_image) +{
- typedef void __noreturn (*image_entry_noargs_t)(u32 romapi);
- uintptr_t romapi = get_stm32mp_rom_api_table();
- image_entry_noargs_t image_entry =
(image_entry_noargs_t)spl_image->entry_point;- printf("image entry point: 0x%lx\n", spl_image->entry_point);
- image_entry(romapi);
+} +#endif
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice
Hi,
minor comment
On 12/6/22 03:33, Marek Vasut wrote:
The ROM API table pointer is no longer accessible from U-Boot, fix this by passing the ROM API pointer through. This makes it possible for U-Boot to call ROM API functions to authenticate payload like signed fitImages.
Signed-off-by: Marek Vasut marex@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com
arch/arm/mach-stm32mp/cpu.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index fa02a11d867..9553ecd243c 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -22,6 +22,7 @@ #include <dm/device.h> #include <dm/uclass.h> #include <linux/bitops.h> +#include <spl.h>
/*
- early TLB into the .data section so that it not get cleared
@@ -413,3 +414,17 @@ uintptr_t get_stm32mp_bl2_dtb(void) { return nt_fw_dtb; }
+#ifdef CONFIG_SPL_BUILD +void jump_to_image_no_args(struct spl_image_info *spl_image)
missing "__noreturn" ?
void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
+{
- typedef void __noreturn (*image_entry_noargs_t)(u32 romapi);
really 'noargs' ?
I propose to replace to 'arg' => image_entry_arg_t
arch/powerpc/lib/spl.c:20
base on arch/arm/lib/spl.c:71
arch/microblaze/cpu/spl.c:37
typedef void __noreturn (*image_entry_noargs_t)(u32 romapi);
or with stm32:
typedef void __noreturn (*image_entry_stm32_t)(u32 romapi);
based on arch/riscv/lib/spl.c:40
- uintptr_t romapi = get_stm32mp_rom_api_table();
- image_entry_noargs_t image_entry =
(image_entry_noargs_t)spl_image->entry_point;- printf("image entry point: 0x%lx\n", spl_image->entry_point);
- image_entry(romapi);
+} +#endif
regards
Patrick
With U-Boot having access to ROM API call table, it is possible to use the ROM API call it authenticate e.g. signed kernel fitImages using the BootROM ECDSA support. Make this available by pulling the ECDSA BootROM call support from SPL-only guard.
Signed-off-by: Marek Vasut marex@denx.de --- Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com --- arch/arm/mach-stm32mp/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-stm32mp/Makefile b/arch/arm/mach-stm32mp/Makefile index 1db9057e049..a19b2797c8b 100644 --- a/arch/arm/mach-stm32mp/Makefile +++ b/arch/arm/mach-stm32mp/Makefile @@ -11,10 +11,10 @@ obj-y += bsec.o obj-$(CONFIG_STM32MP13x) += stm32mp13x.o obj-$(CONFIG_STM32MP15x) += stm32mp15x.o
+obj-$(CONFIG_STM32_ECDSA_VERIFY) += ecdsa_romapi.o ifdef CONFIG_SPL_BUILD obj-y += spl.o obj-y += tzc400.o -obj-$(CONFIG_STM32_ECDSA_VERIFY) += ecdsa_romapi.o else obj-y += cmd_stm32prog/ obj-$(CONFIG_CMD_STM32KEY) += cmd_stm32key.o
On 12/6/22 03:33, Marek Vasut wrote:
With U-Boot having access to ROM API call table, it is possible to use the ROM API call it authenticate e.g. signed kernel fitImages using the BootROM ECDSA support. Make this available by pulling the ECDSA BootROM call support from SPL-only guard.
Signed-off-by: Marek Vasut marex@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com
arch/arm/mach-stm32mp/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-stm32mp/Makefile b/arch/arm/mach-stm32mp/Makefile index 1db9057e049..a19b2797c8b 100644 --- a/arch/arm/mach-stm32mp/Makefile +++ b/arch/arm/mach-stm32mp/Makefile @@ -11,10 +11,10 @@ obj-y += bsec.o obj-$(CONFIG_STM32MP13x) += stm32mp13x.o obj-$(CONFIG_STM32MP15x) += stm32mp15x.o
+obj-$(CONFIG_STM32_ECDSA_VERIFY) += ecdsa_romapi.o ifdef CONFIG_SPL_BUILD obj-y += spl.o obj-y += tzc400.o -obj-$(CONFIG_STM32_ECDSA_VERIFY) += ecdsa_romapi.o else obj-y += cmd_stm32prog/ obj-$(CONFIG_CMD_STM32KEY) += cmd_stm32key.o
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice
Hi,
On 12/6/22 03:33, Marek Vasut wrote:
With U-Boot having access to ROM API call table, it is possible to use the ROM API call it authenticate e.g. signed kernel fitImages using the BootROM ECDSA support. Make this available by pulling the ECDSA BootROM call support from SPL-only guard.
Signed-off-by: Marek Vasut marex@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com
arch/arm/mach-stm32mp/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
Thanks Patrick
On 12/6/22 03:33, Marek Vasut wrote:
In case Dcache is enabled while the ECDSA authentication function is called via BootROM ROM API, the CRYP DMA might pick stale version of data from DRAM. Disable Dcache around the BootROM call to avoid this issue.
Signed-off-by: Marek Vasut marex@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com
arch/arm/mach-stm32mp/ecdsa_romapi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index a2f63ff879f..72b87bf2c64 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -64,6 +64,7 @@ static int romapi_ecdsa_verify(struct udevice *dev, const void *signature, size_t sig_len) { struct ecdsa_rom_api rom;
- bool reenable_dcache; uint8_t raw_key[64]; uint32_t rom_ret; int algo;
@@ -81,8 +82,21 @@ static int romapi_ecdsa_verify(struct udevice *dev, memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions(&rom);
/*
* Disable D-cache before calling into BootROM, else CRYP DMA* may fail to pick up the correct data.*/if (dcache_status()) {
dcache_disable();reenable_dcache = true;}
rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
if (reenable_dcache)
dcache_enable();return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
}
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com Thanks Patrice
just one remark
On 12/6/22 03:33, Marek Vasut wrote:
In case Dcache is enabled while the ECDSA authentication function is called via BootROM ROM API, the CRYP DMA might pick stale version of data from DRAM. Disable Dcache around the BootROM call to avoid this issue.
Signed-off-by: Marek Vasut marex@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com
arch/arm/mach-stm32mp/ecdsa_romapi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index a2f63ff879f..72b87bf2c64 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -64,6 +64,7 @@ static int romapi_ecdsa_verify(struct udevice *dev, const void *signature, size_t sig_len) { struct ecdsa_rom_api rom;
- bool reenable_dcache;
reenable_dcache is used without being initialized
uint8_t raw_key[64]; uint32_t rom_ret; int algo; @@ -81,8 +82,21 @@ static int romapi_ecdsa_verify(struct udevice *dev, memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions(&rom);
/*
* Disable D-cache before calling into BootROM, else CRYP DMA* may fail to pick up the correct data.*/if (dcache_status()) {
dcache_disable();reenable_dcache = true;}
rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
if (reenable_dcache)
dcache_enable();return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
}
Hi Marek,
On 12/6/22 03:33, Marek Vasut wrote:
In case Dcache is enabled while the ECDSA authentication function is called via BootROM ROM API, the CRYP DMA might pick stale version of data from DRAM. Disable Dcache around the BootROM call to avoid this issue.
Signed-off-by: Marek Vasut marex@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com
arch/arm/mach-stm32mp/ecdsa_romapi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index a2f63ff879f..72b87bf2c64 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -64,6 +64,7 @@ static int romapi_ecdsa_verify(struct udevice *dev, const void *signature, size_t sig_len) { struct ecdsa_rom_api rom;
- bool reenable_dcache; uint8_t raw_key[64]; uint32_t rom_ret; int algo;
@@ -81,8 +82,21 @@ static int romapi_ecdsa_verify(struct udevice *dev, memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions(&rom);
- /*
* Disable D-cache before calling into BootROM, else CRYP DMA* may fail to pick up the correct data.*/- if (dcache_status()) {
dcache_disable();reenable_dcache = true;- }
- rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
so the signature verification (the code execution) is done with dcache OFF....
flush the input data should be enought for DMA operation ?
=> call flush_dcache_all() or flush_dcache_range()
for example:
if (dcache_status()) flush_dcache_all();
- if (reenable_dcache)
dcache_enable();- return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM; }
Regards
Patrick
On 12/6/22 10:13, Patrick DELAUNAY wrote:
Hi Marek,
Hi,
[...]
@@ -81,8 +82,21 @@ static int romapi_ecdsa_verify(struct udevice *dev, memcpy(raw_key + 32, pubkey->y, 32); stm32mp_rom_get_ecdsa_functions(&rom);
+ /* + * Disable D-cache before calling into BootROM, else CRYP DMA + * may fail to pick up the correct data. + */ + if (dcache_status()) { + dcache_disable(); + reenable_dcache = true; + }
rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
so the signature verification (the code execution) is done with dcache OFF....
flush the input data should be enought for DMA operation ?
=> call flush_dcache_all() or flush_dcache_range()
for example:
if (dcache_status()) flush_dcache_all();
Wouldn't you then also need to invalidate the dcache after the BootROM call, so that the CPU with dcache enabled could read what the CRYP wrote to DRAM instead of pulling stale data from Dcache ?
That's very much what the enable/disable trick does for you.
participants (3)
-
Marek Vasut -
Patrice CHOTARD -
Patrick DELAUNAY