[PATCH v2 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled

[PATCH v2 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled

[Marek Vasut]

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>
---
V2: - Initialize reenable_dcache variable
---
 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..082178ce83f 100644
--- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
+++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
@@ -63,6 +63,7 @@ static int romapi_ecdsa_verify(struct udevice *dev,
 			       const void *hash, size_t hash_len,
 			       const void *signature, size_t sig_len)
 {
+	bool reenable_dcache = false;
 	struct ecdsa_rom_api rom;
 	uint8_t raw_key[64];
 	uint32_t rom_ret;
@@ -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;
 }
 
-- 
2.35.1

[PATCH v2 2/4] ARM: stm32: Factor out save_boot_params

[Marek Vasut]

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
---
 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 082178ce83f..b1ab49165e7 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);
-- 
2.35.1

[PATCH v2 3/4] ARM: stm32: Pass ROM API table pointer to U-Boot proper

[Marek Vasut]

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
---
 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
-- 
2.35.1

[PATCH v2 4/4] ARM: stm32: Make ECDSA authentication available to U-Boot

[Marek Vasut]

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
---
 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
-- 
2.35.1

Re: [PATCH v2 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled

[Patrick DELAUNAY]

Hi Marek,


Sorry for the delay.

I cross-check with ROM code team to understood this API limitation.


On 12/6/22 23:49, 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>
> ---
> V2: - Initialize reenable_dcache variable
> ---
>   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..082178ce83f 100644
> --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
> +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
> @@ -63,6 +63,7 @@ static int romapi_ecdsa_verify(struct udevice *dev,
>   			       const void *hash, size_t hash_len,
>   			       const void *signature, size_t sig_len)
>   {
> +	bool reenable_dcache = false;
>   	struct ecdsa_rom_api rom;
>   	uint8_t raw_key[64];
>   	uint32_t rom_ret;
> @@ -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;
>   }
>   


In fact, the ecdsa_verify_signature() don't use the HW (no DMA and no 
use of CRYP IP )

It is only a SW library, integrated in ROM code and exported to avoid 
the need

to include the same library in FSBL = TF-A, with size limitation (SYSRAM).


This library don't need to deactivate the data cache, the only impact of 
this deactivation it

is to reduce the execution performance....


After cross-check, I think the only problem today it the U-Boot MMU 
configuration of STM32MP15x

plaform: by default only the DDR is marked executable in U-Boot, all the 
other region are

defined as DEVICE memory/not executable (DCACHE_OFF in mmu_setup).


Deactivate the data cache only avoids the exception which occurs on jump 
to NotExecutable region

because in U-Boot "dcache OFF" imply  "MMU off"  (see cache_enable in 
./arch/arm/lib/cache-cp15.c)

and with MMU deactivated the check on executable MMU tag is also 
deactivated.


I think the next patch is enough:


#define STM32MP_ROM_BASE        U(0x00000000)


static int romapi_ecdsa_verify(struct udevice *dev,
  			       const void *hash, size_t hash_len,
  			       const void *signature, size_t sig_len)
  {
  	struct ecdsa_rom_api rom;
  	uint8_t raw_key[64];
  	uint32_t rom_ret;
@@ -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);
+
+	/* mark executable the exported ROM code function: */
+	mmu_set_region_dcache_behaviour(STM32MP_ROM_BASE, MMU_SECTION_SIZE, DCACHE_DEFAULT_OPTION);
+
  	rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
  
  	return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
  }


Sorry again for the first review, not complete...


Regards


Patrick



Reference in TF-A code: 
arm-trusted-firmware/plat/st/common/stm32mp_crypto_lib.c


uint32_t verify_signature(uint8_t *hash_in, uint8_t *pubkey_in,
               uint8_t *signature, uint32_t ecc_algo)
{
     int ret;

     ret = mmap_add_dynamic_region(STM32MP_ROM_BASE, STM32MP_ROM_BASE,
                       STM32MP_ROM_SIZE_2MB_ALIGNED, MT_CODE | MT_SECURE);

....
     ret = auth_ops.verify_signature(hash_in, pubkey_in, signature, 
ecc_algo);

....
     mmap_remove_dynamic_region(STM32MP_ROM_BASE, 
STM32MP_ROM_SIZE_2MB_ALIGNED);

     return ret;
}

Re: [PATCH v2 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled

[Marek Vasut]

On 12/7/22 11:08, Patrick DELAUNAY wrote:
> Hi Marek,

Hello Patrick,

> Sorry for the delay.

No worries.

> I cross-check with ROM code team to understood this API limitation.

Thank you!

> On 12/6/22 23:49, 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>
>> ---
>> V2: - Initialize reenable_dcache variable
>> ---
>>   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..082178ce83f 100644
>> --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
>> +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
>> @@ -63,6 +63,7 @@ static int romapi_ecdsa_verify(struct udevice *dev,
>>                      const void *hash, size_t hash_len,
>>                      const void *signature, size_t sig_len)
>>   {
>> +    bool reenable_dcache = false;
>>       struct ecdsa_rom_api rom;
>>       uint8_t raw_key[64];
>>       uint32_t rom_ret;
>> @@ -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;
>>   }
> 
> 
> In fact, the ecdsa_verify_signature() don't use the HW (no DMA and no 
> use of CRYP IP )

Hmmm, what does the BootROM use CRYP for then ?
It is necessary to have MP15xC/F for the authenticated boot to work, but 
it seems the only difference there is the presence of CRYP. Or is there 
some BootROM fuse too ?

> It is only a SW library, integrated in ROM code and exported to avoid 
> the need
> 
> to include the same library in FSBL = TF-A, with size limitation (SYSRAM).
> 
> 
> This library don't need to deactivate the data cache, the only impact of 
> this deactivation it
> 
> is to reduce the execution performance....
> 
> 
> After cross-check, I think the only problem today it the U-Boot MMU 
> configuration of STM32MP15x
> 
> plaform: by default only the DDR is marked executable in U-Boot, all the 
> other region are
> 
> defined as DEVICE memory/not executable (DCACHE_OFF in mmu_setup).
> 
> 
> Deactivate the data cache only avoids the exception which occurs on jump 
> to NotExecutable region
> 
> because in U-Boot "dcache OFF" imply  "MMU off"  (see cache_enable in 
> ./arch/arm/lib/cache-cp15.c)
> 
> and with MMU deactivated the check on executable MMU tag is also 
> deactivated.
> 
> 
> I think the next patch is enough:
> 
> 
> #define STM32MP_ROM_BASE        U(0x00000000)
> 
> 
> static int romapi_ecdsa_verify(struct udevice *dev,
>                      const void *hash, size_t hash_len,
>                      const void *signature, size_t sig_len)
>   {
>       struct ecdsa_rom_api rom;
>       uint8_t raw_key[64];
>       uint32_t rom_ret;
> @@ -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);
> +
> +    /* mark executable the exported ROM code function: */
> +    mmu_set_region_dcache_behaviour(STM32MP_ROM_BASE, MMU_SECTION_SIZE, 
> DCACHE_DEFAULT_OPTION);
> +
>       rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
> 
>       return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
>   }

This indeed works, tested and sent V3.

> Sorry again for the first review, not complete...

Thank you for checking !

Re: [PATCH v2 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled

[Patrick DELAUNAY]

Hi,

On 12/7/22 20:32, Marek Vasut wrote:
> On 12/7/22 11:08, Patrick DELAUNAY wrote:
>> Hi Marek,
>
> Hello Patrick,
>
>> Sorry for the delay.
>
> No worries.
>
>> I cross-check with ROM code team to understood this API limitation.
>
> Thank you!
>
>> On 12/6/22 23:49, 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>
>>> ---
>>> V2: - Initialize reenable_dcache variable
>>> ---
>>>   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..082178ce83f 100644
>>> --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
>>> +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
>>> @@ -63,6 +63,7 @@ static int romapi_ecdsa_verify(struct udevice *dev,
>>>                      const void *hash, size_t hash_len,
>>>                      const void *signature, size_t sig_len)
>>>   {
>>> +    bool reenable_dcache = false;
>>>       struct ecdsa_rom_api rom;
>>>       uint8_t raw_key[64];
>>>       uint32_t rom_ret;
>>> @@ -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;
>>>   }
>>
>>
>> In fact, the ecdsa_verify_signature() don't use the HW (no DMA and no 
>> use of CRYP IP )
>
> Hmmm, what does the BootROM use CRYP for then ?


used for SSP = Secure Secret Provisioning

https://wiki.st.com/stm32mpu/wiki/Secure_Secret_Provisioning_(SSP)


> It is necessary to have MP15xC/F for the authenticated boot to work, 
> but it seems the only difference there is the presence of CRYP. Or is 
> there some BootROM fuse too ?


Yes,  the secure boot feature availability is indicated in the security 
field of the chip part number, for STM32MP13 and STM32MP15.

- SSP is not supported

- the associated authentication feature for secure boot is deactivated 
in ROM code


=> the key is burned/locked in OTP on these chips

       and checked by ROM code before to authenticate the FSBL



...



> This indeed works, tested and sent V3.
>
>> Sorry again for the first review, not complete...
>
> Thank you for checking !


Regards

Patrick

Re: [PATCH v2 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled

[Marek Vasut]

On 12/12/22 10:40, Patrick DELAUNAY wrote:
> Hi,

Hello Patrick

[...]

>> Hmmm, what does the BootROM use CRYP for then ?
> 
> 
> used for SSP = Secure Secret Provisioning
> 
> https://wiki.st.com/stm32mpu/wiki/Secure_Secret_Provisioning_(SSP)

Oh, only this part, I see.

>> It is necessary to have MP15xC/F for the authenticated boot to work, 
>> but it seems the only difference there is the presence of CRYP. Or is 
>> there some BootROM fuse too ?
> 
> 
> Yes,  the secure boot feature availability is indicated in the security 
> field of the chip part number, for STM32MP13 and STM32MP15.
> 
> - SSP is not supported
> 
> - the associated authentication feature for secure boot is deactivated 
> in ROM code
> 
> 
> => the key is burned/locked in OTP on these chips
> 
>        and checked by ROM code before to authenticate the FSBL

Thank you for the clarification, this is really useful.