[PATCH v3 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 MMU tables are set up and the BootROM region is not marked as executable, so an attempt to run code from it results in a hang. Mark the BootROM region as executable as suggested by Patrick to prevent the hang.
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 --- V2: - Initialize reenable_dcache variable V3: - Mark BootROM as executable instead --- arch/arm/mach-stm32mp/ecdsa_romapi.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index a2f63ff879f..6156526253c 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -81,6 +81,10 @@ static int romapi_ecdsa_verify(struct udevice *dev, memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions(&rom); + + /* Mark BootROM region as executable. */ + mmu_set_region_dcache_behaviour(0, SZ_2M, DCACHE_DEFAULT_OPTION); + rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
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 --- V2: Avoid #if CONFIG... , use if (CONFIG... instead V3: No change --- 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..ee59866bb73 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; + + 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 6156526253c..12b42b9d59c 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);
Hi,
On 12/7/22 20:24, 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
V2: Avoid #if CONFIG... , use if (CONFIG... instead V3: No change
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(-)
Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
Thanks Patrick
Hi Marek
On 12/7/22 20:24, 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
V2: Avoid #if CONFIG... , use if (CONFIG... instead V3: No change
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..ee59866bb73 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;- 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 6156526253c..12b42b9d59c 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
I am preparing the next STM32 U-Boot pull request, during testing, buidlman is complaining with the following warning:
03: ARM: stm32: Factor out save_boot_params arm: + stm32mp13 +../arch/arm/mach-stm32mp/boot_params.c: In function 'board_fdt_blob_setup': +../arch/arm/mach-stm32mp/boot_params.c:20:35: error: implicit declaration of function 'get_stm32mp_bl2_dtb' [-Werror=implicit-function-declaration] + 20 | unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb(); + | ^~~~~~~~~~~~~~~~~~~ +cc1: all warnings being treated as errors
Same issue with stm32mp15_defconfig and stm32mp15_trusted_defconfig.
Regards Patrice
On 1/4/23 10:00, Patrice CHOTARD wrote:
Hi Marek
On 12/7/22 20:24, 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
V2: Avoid #if CONFIG... , use if (CONFIG... instead V3: No change
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..ee59866bb73 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;- 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 6156526253c..12b42b9d59c 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
On 1/12/23 16:06, Patrice CHOTARD wrote:
Hi Marek
Hi,
I am preparing the next STM32 U-Boot pull request, during testing, buidlman is complaining with the following warning:
03: ARM: stm32: Factor out save_boot_params arm: + stm32mp13 +../arch/arm/mach-stm32mp/boot_params.c: In function 'board_fdt_blob_setup': +../arch/arm/mach-stm32mp/boot_params.c:20:35: error: implicit declaration of function 'get_stm32mp_bl2_dtb' [-Werror=implicit-function-declaration]
- 20 | unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb();
| ^~~~~~~~~~~~~~~~~~~+cc1: all warnings being treated as errors
Same issue with stm32mp15_defconfig and stm32mp15_trusted_defconfig.
Sorry for the inconvenience.
Does the change below help ? If so, can you squash it into 2/4 or do you want v4 ?
diff --git a/arch/arm/mach-stm32mp/boot_params.c b/arch/arm/mach-stm32mp/boot_params.c index e40cca938ef..24d04dcf0f9 100644 --- a/arch/arm/mach-stm32mp/boot_params.c +++ b/arch/arm/mach-stm32mp/boot_params.c @@ -8,6 +8,7 @@ #include <common.h> #include <log.h> #include <linux/libfdt.h> +#include <asm/arch/sys_proto.h> #include <asm/sections.h> #include <asm/system.h>
Hi Marek
On 1/12/23 18:55, Marek Vasut wrote:
On 1/12/23 16:06, Patrice CHOTARD wrote:
Hi Marek
Hi,
I am preparing the next STM32 U-Boot pull request, during testing, buidlman is complaining with the following warning:
03: ARM: stm32: Factor out save_boot_params arm: + stm32mp13 +../arch/arm/mach-stm32mp/boot_params.c: In function 'board_fdt_blob_setup': +../arch/arm/mach-stm32mp/boot_params.c:20:35: error: implicit declaration of function 'get_stm32mp_bl2_dtb' [-Werror=implicit-function-declaration] + 20 | unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb(); + | ^~~~~~~~~~~~~~~~~~~ +cc1: all warnings being treated as errors
Same issue with stm32mp15_defconfig and stm32mp15_trusted_defconfig.
Sorry for the inconvenience.
Does the change below help ? If so, can you squash it into 2/4 or do you want v4 ?
Yes the change you proposed is OK. I saw that you have submitted a v4 including this fix, i will add it to the next U-Boot STM32 PR.
Thanks for your reactivity ;-)
Patrice
diff --git a/arch/arm/mach-stm32mp/boot_params.c b/arch/arm/mach-stm32mp/boot_params.c index e40cca938ef..24d04dcf0f9 100644 --- a/arch/arm/mach-stm32mp/boot_params.c +++ b/arch/arm/mach-stm32mp/boot_params.c @@ -8,6 +8,7 @@ #include <common.h> #include <log.h> #include <linux/libfdt.h> +#include <asm/arch/sys_proto.h> #include <asm/sections.h> #include <asm/system.h>
On 1/13/23 13:55, Patrice CHOTARD wrote:
Hi Marek
Hello Patrice,
On 1/12/23 18:55, Marek Vasut wrote:
On 1/12/23 16:06, Patrice CHOTARD wrote:
Hi Marek
Hi,
I am preparing the next STM32 U-Boot pull request, during testing, buidlman is complaining with the following warning:
03: ARM: stm32: Factor out save_boot_params arm: + stm32mp13 +../arch/arm/mach-stm32mp/boot_params.c: In function 'board_fdt_blob_setup': +../arch/arm/mach-stm32mp/boot_params.c:20:35: error: implicit declaration of function 'get_stm32mp_bl2_dtb' [-Werror=implicit-function-declaration] + 20 | unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb(); + | ^~~~~~~~~~~~~~~~~~~ +cc1: all warnings being treated as errors
Same issue with stm32mp15_defconfig and stm32mp15_trusted_defconfig.
Sorry for the inconvenience.
Does the change below help ? If so, can you squash it into 2/4 or do you want v4 ?
Yes the change you proposed is OK. I saw that you have submitted a v4 including this fix, i will add it to the next U-Boot STM32 PR.
Thanks !
Thanks for your reactivity ;-)
:)
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 --- V2: - Rename image_entry_noargs_t to image_entry_stm32_t - Add missing __noreturn V3: No change --- 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 ee59866bb73..dc4112d5e6c 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 __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) +{ + typedef void __noreturn (*image_entry_stm32_t)(u32 romapi); + uintptr_t romapi = get_stm32mp_rom_api_table(); + + image_entry_stm32_t image_entry = + (image_entry_stm32_t)spl_image->entry_point; + + printf("image entry point: 0x%lx\n", spl_image->entry_point); + image_entry(romapi); +} +#endif
Hi,
On 12/7/22 20:24, 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
V2: - Rename image_entry_noargs_t to image_entry_stm32_t - Add missing __noreturn V3: No change
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 ee59866bb73..dc4112d5e6c 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 __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) +{
- typedef void __noreturn (*image_entry_stm32_t)(u32 romapi);
- uintptr_t romapi = get_stm32mp_rom_api_table();
- image_entry_stm32_t image_entry =
(image_entry_stm32_t)spl_image->entry_point;- printf("image entry point: 0x%lx\n", spl_image->entry_point);
- image_entry(romapi);
+} +#endif
Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
Thanks Patrick
Hi Marek
On 12/7/22 20:24, 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
V2: - Rename image_entry_noargs_t to image_entry_stm32_t - Add missing __noreturn V3: No change
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 ee59866bb73..dc4112d5e6c 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 __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) +{
- typedef void __noreturn (*image_entry_stm32_t)(u32 romapi);
- uintptr_t romapi = get_stm32mp_rom_api_table();
- image_entry_stm32_t image_entry =
(image_entry_stm32_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
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.
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com 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 --- V2: Add RB from Patrice and Patrick V3: No change --- 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
Hi,
On 12/7/22 20:24, Marek Vasut wrote:
In case Dcache is enabled while the ECDSA authentication function is called via BootROM ROM API, the MMU tables are set up and the BootROM region is not marked as executable, so an attempt to run code from it results in a hang. Mark the BootROM region as executable as suggested by Patrick to prevent the hang.
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
V2: - Initialize reenable_dcache variable V3: - Mark BootROM as executable instead
arch/arm/mach-stm32mp/ecdsa_romapi.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index a2f63ff879f..6156526253c 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -81,6 +81,10 @@ static int romapi_ecdsa_verify(struct udevice *dev, memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions(&rom);
/* Mark BootROM region as executable. */
mmu_set_region_dcache_behaviour(0, SZ_2M, DCACHE_DEFAULT_OPTION);
rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
Thanks Patrick
Hi Marek
On 12/7/22 20:24, Marek Vasut wrote:
In case Dcache is enabled while the ECDSA authentication function is called via BootROM ROM API, the MMU tables are set up and the BootROM region is not marked as executable, so an attempt to run code from it results in a hang. Mark the BootROM region as executable as suggested by Patrick to prevent the hang.
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
V2: - Initialize reenable_dcache variable V3: - Mark BootROM as executable instead
arch/arm/mach-stm32mp/ecdsa_romapi.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index a2f63ff879f..6156526253c 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -81,6 +81,10 @@ static int romapi_ecdsa_verify(struct udevice *dev, memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions(&rom);
/* Mark BootROM region as executable. */
mmu_set_region_dcache_behaviour(0, SZ_2M, DCACHE_DEFAULT_OPTION);
rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice
participants (3)
-
Marek Vasut -
Patrice CHOTARD -
Patrick DELAUNAY