Re: [PATCH v2 2/3] nvmem: stm32: add OP-TEE support for STM32MP13x

From: Srinivas Kandagatla
Date: Fri Nov 11 2022 - 12:19:08 EST




On 10/11/2022 15:45, Patrick Delaunay wrote:
For boot with OP-TEE on STM32MP13, the communication with the secure
world no more use STMicroelectronics SMC but communication with the
BSEC TA, for data access (read/write) or lock operation:
- all the request are sent to OP-TEE trusted application,
- for upper OTP with ECC protection and with word programming only
each OTP are permanently locked when programmed to avoid ECC error
on the second write operation

Signed-off-by: Patrick Delaunay <patrick.delaunay@xxxxxxxxxxx>
---

Changes in v2:
- rebase series on linux-next/master
- minor update after V1 revue
- add missing sentinel in stm32_romem_of_match()
- reorder function and remove prototypes for stm32_bsec_pta... functions
- change stm32_bsec_pta_find to static
- add return value in dev_err()
- cleanups some comments, which can be on one line
- remove test on priv->ctx in stm32_bsec_pta_remove
- add missing tee_shm_free(shm) in stm32_bsec_pta_write() when
tee_shm_get_va failed
- return error in stm32_bsec_pta_find when devm_add_action_or_reset failed
- handle driver_register error in stm32_romem_init

drivers/nvmem/stm32-romem.c | 445 +++++++++++++++++++++++++++++++++++-
1 file changed, 441 insertions(+), 4 deletions(-)

diff --git a/drivers/nvmem/stm32-romem.c b/drivers/nvmem/stm32-romem.c
index d1d03c2ad081..0a0e29d09b67 100644
--- a/drivers/nvmem/stm32-romem.c
+++ b/drivers/nvmem/stm32-romem.c
@@ -11,6 +11,7 @@
#include <linux/module.h>
#include <linux/nvmem-provider.h>
#include <linux/of_device.h>
+#include <linux/tee_drv.h>
/* BSEC secure service access from non-secure */
#define STM32_SMC_BSEC 0x82001003
@@ -25,14 +26,401 @@
struct stm32_romem_cfg {
int size;
u8 lower;
+ bool ta;
};
struct stm32_romem_priv {
void __iomem *base;
struct nvmem_config cfg;
u8 lower;
+ struct device *ta;
};
+#if IS_ENABLED(CONFIG_OPTEE)
+/*

...

+
+static const struct tee_client_device_id stm32_bsec_id_table[] = {
+ {
+ UUID_INIT(0x94cf71ad, 0x80e6, 0x40b5,
+ 0xa7, 0xc6, 0x3d, 0xc5, 0x01, 0xeb, 0x28, 0x03)
+ },
+ { }
+};
+
+MODULE_DEVICE_TABLE(tee, stm32_bsec_id_table);
+
+static struct tee_client_driver stm32_bsec_pta_driver = {
+ .id_table = stm32_bsec_id_table,
+ .driver = {
+ .name = "stm32-bsec-pta",
+ .bus = &tee_bus_type,
+ .probe = stm32_bsec_pta_probe,
+ .remove = stm32_bsec_pta_remove,
+ },
+};
+
+static void stm32_bsec_put_device(void *data)
+{
+ put_device(data);
+}
+
+static struct device *stm32_bsec_pta_find(struct device *dev)
+{
+ struct device *pta_dev;
+ int ret;
+
+ pta_dev = driver_find_next_device(&stm32_bsec_pta_driver.driver, NULL);

This is clearly not representing the dependencies in a proper device model.


If the nvmem provider is a TEE client driver lets model it that way.. brining in a additional device and somehow trying to link it with TEE driver is a hack.


+
+ if (pta_dev) {
+ ret = devm_add_action_or_reset(dev, stm32_bsec_put_device, pta_dev);
+ if (ret)
+ dev_err(dev, "devm_add_action_or_reset() failed (%d)\n", ret);
+
+ return ERR_PTR(ret);
+ }
+
+ return pta_dev;
+}
+
+#else
+static int stm32_bsec_pta_read(void *context, unsigned int offset, void *buf,
+ size_t bytes)
+{
+ pr_debug("%s: TA BSEC request without OPTEE support\n", __func__);
+
+ return -ENXIO;
+}
+
+static int stm32_bsec_pta_write(void *context, unsigned int offset, void *buf,
+ size_t bytes)
+{
+ pr_debug("%s: TA BSEC request without OPTEE support\n", __func__);
+
+ return -ENXIO;
+}
+
+static struct device *stm32_bsec_pta_find(struct device *dev)
+{
+ pr_debug("%s: TA BSEC request without OPTEE support\n", __func__);
+
+ return NULL;
+}
+#endif

ifdefing inside the drvier is really ugly, please move this libary functions to a seperate file and add dependecy properly in Kconfig.

+
static int stm32_romem_read(void *context, unsigned int offset, void *buf,
size_t bytes)
{
@@ -173,15 +561,27 @@ static int stm32_romem_probe(struct platform_device *pdev)
} else {
priv->cfg.size = cfg->size;
priv->lower = cfg->lower;
- priv->cfg.reg_read = stm32_bsec_read;
- priv->cfg.reg_write = stm32_bsec_write;
+ if (cfg->ta) {
+ priv->ta = stm32_bsec_pta_find(dev);
+ /* wait for OP-TEE client driver to be up and ready */
+ if (!priv->ta)
+ return -EPROBE_DEFER;
+ if (IS_ERR(priv->ta))
+ return PTR_ERR(priv->ta);
+
+ priv->cfg.reg_read = stm32_bsec_pta_read;
+ priv->cfg.reg_write = stm32_bsec_pta_write;
+ } else {
+ priv->cfg.reg_read = stm32_bsec_read;
+ priv->cfg.reg_write = stm32_bsec_write;
+ }
}
return PTR_ERR_OR_ZERO(devm_nvmem_register(dev, &priv->cfg));
}
/*
- * STM32MP15 BSEC OTP regions: 4096 OTP bits (with 3072 effective bits)
+ * STM32MP15/13 BSEC OTP regions: 4096 OTP bits (with 3072 effective bits)
* => 96 x 32-bits data words
* - Lower: 1K bits, 2:1 redundancy, incremental bit programming
* => 32 (x 32-bits) lower shadow registers = words 0 to 31
@@ -191,6 +591,13 @@ static int stm32_romem_probe(struct platform_device *pdev)
static const struct stm32_romem_cfg stm32mp15_bsec_cfg = {
.size = 384,
.lower = 32,
+ .ta = false,
+};
+
+static const struct stm32_romem_cfg stm32mp13_bsec_cfg = {
+ .size = 384,
+ .lower = 32,
+ .ta = true,
};
static const struct of_device_id stm32_romem_of_match[] = {
@@ -198,7 +605,10 @@ static const struct of_device_id stm32_romem_of_match[] = {
.compatible = "st,stm32mp15-bsec",
.data = (void *)&stm32mp15_bsec_cfg,
}, {
+ .compatible = "st,stm32mp13-bsec",
+ .data = (void *)&stm32mp13_bsec_cfg,
},
+ { /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, stm32_romem_of_match);
@@ -209,7 +619,34 @@ static struct platform_driver stm32_romem_driver = {
.of_match_table = of_match_ptr(stm32_romem_of_match),
},
};
-module_platform_driver(stm32_romem_driver);
+
+static int __init stm32_romem_init(void)
+{
+ int rc;
+
+ rc = platform_driver_register(&stm32_romem_driver);
+ if (rc)
+ return rc;
+
+#if IS_ENABLED(CONFIG_OPTEE)
+ rc = driver_register(&stm32_bsec_pta_driver.driver);
+ if (rc)
+ platform_driver_unregister(&stm32_romem_driver);
+#endif
+
+ return rc;
+}
+
+static void __exit stm32_romem_exit(void)
+{
+ platform_driver_unregister(&stm32_romem_driver);
+#if IS_ENABLED(CONFIG_OPTEE)
+ driver_unregister(&stm32_bsec_pta_driver.driver);
+#endif
+}
+
+module_init(stm32_romem_init);
+module_exit(stm32_romem_exit);
MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@xxxxxx>");
MODULE_DESCRIPTION("STMicroelectronics STM32 RO-MEM");