Hi Volodymyr,
Can you clarify whether this is intended for the next release cycle?
On 29/05/2024 21:43, Volodymyr Babchuk wrote:
Allow to provide TEE type for a Dom0less guest via "xen,tee"
property. Create appropriate nodes in the guests' device tree and
initialize tee subsystem for it.
The new property needs to be documented in
docs/misc/arm/device-tree/booting.txt.
Signed-off-by: Volodymyr Babchuk <[email protected]>
---
xen/arch/arm/dom0less-build.c | 69 +++++++++++++++++++++++++++++++
xen/arch/arm/include/asm/kernel.h | 3 ++
2 files changed, 72 insertions(+)
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index fb63ec6fd1..1ea3ecc45c 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -15,6 +15,7 @@
#include <asm/domain_build.h>
#include <asm/static-memory.h>
#include <asm/static-shmem.h>
+#include <asm/tee/tee.h>
bool __init is_dom0less_mode(void)
{
@@ -277,6 +278,42 @@ static int __init make_vpl011_uart_node(struct kernel_info
*kinfo)
}
#endif
+#ifdef CONFIG_OPTEE
+static int __init make_optee_node(struct kernel_info *kinfo)
Please introduce a callback in the TEE framework that will create the
OPTEE node.
[...]
/*
* Scan device tree properties for passthrough specific information.
* Returns < 0 on error
@@ -650,6 +687,15 @@ static int __init prepare_dtb_domU(struct domain *d,
struct kernel_info *kinfo)
if ( ret )
goto err;
+#ifdef CONFIG_OPTEE
+ if ( kinfo->tee_type == XEN_DOMCTL_CONFIG_TEE_OPTEE)
+ {
+ ret = make_optee_node(kinfo);
+ if ( ret )
+ goto err;
+ }
+#endif
+
/*
* domain_handle_dtb_bootmodule has to be called before the rest of
* the device tree is generated because it depends on the value of
@@ -743,6 +789,9 @@ static int __init construct_domU(struct domain *d,
{
struct kernel_info kinfo = {};
const char *dom0less_enhanced;
+#ifdef CONFIG_TEE
+ const char *tee;
+#endif
int rc;
u64 mem;
u32 p2m_mem_mb;
@@ -786,6 +835,18 @@ static int __init construct_domU(struct domain *d,
else if ( rc == 0 && !strcmp(dom0less_enhanced, "no-xenstore") )
kinfo.dom0less_feature = DOM0LESS_ENHANCED_NO_XS;
+#ifdef CONFIG_TEE
I would rather prefer if this code is implemented in tee.c. We also...
+ rc = dt_property_read_string(node, "xen,tee", &tee);
... want to return an error if "xen,tee" exists because CONFIG_TEE is
not set.
+ if ( rc == -EILSEQ ||
+ rc == -ENODATA ||
+ (rc == 0 && !strcmp(tee, "none")) )
+ {
+ if ( !hardware_domain )
I don't understand this check. Why would we require dom0 for OP-TEE? In
any case we should avoid to hide a feature behind the user back. At
minimum, we should print a message, but I would rather prefer a panic()
because the user asks for a feature and we denied it.
+ kinfo.tee_type = XEN_DOMCTL_CONFIG_TEE_NONE;
Why don't we have a else case? Are you assuming that tee_type is
initialized to TEE_NONE (which luckily happens to be 0)? If so, why do
we need the check above?
+ }
+ else if ( rc == 0 && !strcmp(tee, "optee") )
+ kinfo.tee_type = XEN_DOMCTL_CONFIG_TEE_OPTEE;
All the other values should be rejected.
+#endif
if ( vcpu_create(d, 0) == NULL )
return -ENOMEM;
@@ -824,6 +885,14 @@ static int __init construct_domU(struct domain *d,
return rc;
}
+#ifdef CONFIG_TEE
+ if ( kinfo.tee_type )
+ {
+ rc = tee_domain_init(d, kinfo.tee_type);
Can you explain why do you need to call tee_domain_init() rather than
setting d_cfg.arch.tee_type just before domain_create() is called and
rely on the latter to call the former?
+ if ( rc < 0 )
+ return rc;
+ }
+#endif
rc = prepare_dtb_domU(d, &kinfo);
if ( rc < 0 )
return rc;
diff --git a/xen/arch/arm/include/asm/kernel.h
b/xen/arch/arm/include/asm/kernel.h
index 0a23e86c2d..7e7b3f4d56 100644
--- a/xen/arch/arm/include/asm/kernel.h
+++ b/xen/arch/arm/include/asm/kernel.h
@@ -57,6 +57,9 @@ struct kernel_info {
/* Enable pl011 emulation */
bool vpl011;
+ /* TEE type */
+ uint16_t tee_type;
+
/* Enable/Disable PV drivers interfaces */
uint16_t dom0less_feature;
Cheers,
--
Julien Grall
