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 <ma...@denx.de>
---
Cc: Alexandru Gagniuc <mr.nuke...@gmail.com>
Cc: Patrice Chotard <patrice.chot...@foss.st.com>
Cc: Patrick Delaunay <patrick.delau...@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 !
