[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [RFC 2/4] arm: add generic TEE mediator framework
On 17/10/17 17:22, Volodymyr Babchuk wrote:
On Mon, Oct 16, 2017 at 02:00:32PM +0100, Julien Grall wrote:
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index d46b98c..e1f112a 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -50,6 +50,14 @@ config HAS_ITS
prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
depends on HAS_GICV3
+config ARM_TEE
The ARM in the title is a bit pointless. This Kconfig is only used for Arm
architecture.
Just plain TEE then?
Yes please.
+ bool "Enable TEE mediators support"
+ default n
+ depends on ARM
No need for that.
Right.
+ help
+ This option enables generic TEE mediators support. It allows guests
+ to access real TEE via one of TEE mediators implemented in XEN
Missing full stop.
+
endmenu
menu "ARM errata workaround via the alternative framework"
@@ -167,3 +175,5 @@ endmenu
source "common/Kconfig"
source "drivers/Kconfig"
+
+source "arch/arm/tee/Kconfig"
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index ede21fd..2710e0e 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -3,6 +3,7 @@ subdir-$(CONFIG_ARM_64) += arm64
subdir-y += platforms
subdir-$(CONFIG_ARM_64) += efi
subdir-$(CONFIG_ACPI) += acpi
+subdir-$(CONFIG_ARM_TEE) += tee
obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
obj-y += bootfdt.init.o
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 784ae39..3290d39 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -31,6 +31,7 @@
#include <asm/platform.h>
#include <asm/procinfo.h>
#include <asm/regs.h>
+#include <asm/tee.h>
#include <asm/vfp.h>
#include <asm/vgic.h>
#include <asm/vtimer.h>
@@ -673,6 +674,9 @@ int arch_domain_create(struct domain *d, unsigned int
domcr_flags,
if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
goto fail;
+ /* Notify TEE that new domain was created */
+ tee_domain_create(d);
I am not a big fan to see this in arch_domain_create until we see how this
is going to fit with guest. For instance, will TEE be for every guests? What
would be the other necessary information to configure it?...
I think I'll call XSM in tee_domain_create() to check if this domain allowed
to work with TEE. I can't imagine what additional information will be needed.
This interface can be extended in the future, though.
You will never need to inform TEE that a new client (aka domain) is been
created, nor allocated memory for the TEE at domain creation in Xen?
[...]
diff --git a/xen/arch/arm/tee/tee.c b/xen/arch/arm/tee/tee.c
new file mode 100644
index 0000000..7f7a846
--- /dev/null
+++ b/xen/arch/arm/tee/tee.c
@@ -0,0 +1,134 @@
+/*
+ * xen/arch/arm/tee/tee.c
+ *
+ * Generic part of TEE mediator subsystem
+ *
+ * Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
+ * Copyright (c) 2017 EPAM Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/types.h>
+#include <asm/smccc.h>
+#include <asm/tee.h>
+
+/*
+ * According to ARM SMCCC (ARM DEN 0028B, page 17), service owner
+ * for generic TEE queries is 63.
+ */
+#define TRUSTED_OS_GENERIC_API_OWNER 63
+
+#define ARM_SMCCC_FUNC_GET_TEE_UID \
+ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+ ARM_SMCCC_CONV_32, \
+ TRUSTED_OS_GENERIC_API_OWNER, \
+ ARM_SMCCC_FUNC_CALL_UID)
This likely needs to be defined in smccc as AFAIU it is part of the SMCCC.
It only used there. I'm not sure if I should define it globally.
Maybe ARM_SMCCC_FUNC_GET_TEE_UID, but definitely
TRUSTED_OS_GENERIC_API_OWNER should stick with the rest of the subsystem
definition in smccc.h.
[...]
+ printk("No TEE found\n");
+ return;
+ }
+
+ parse_uid(resp, &tee_uid);
+
+ printk("TEE UID:
%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x\n",
+ tee_uid.a[0 ], tee_uid.a[1 ], tee_uid.a[2 ], tee_uid.a[3 ],
Please no space before ]. This is making more confusing to read.
I put it for neat formatting. Probably, I can put double space after commas.
Will be okay?
That is that really important to have them? I mean, ok it is not going
to be neat but the format string is already ugly and it would not be too
difficult to read the arguments.
+ tee_uid.a[4 ], tee_uid.a[5 ], tee_uid.a[6 ], tee_uid.a[7 ],
+ tee_uid.a[8 ], tee_uid.a[9 ], tee_uid.a[10], tee_uid.a[11],
+ tee_uid.a[12], tee_uid.a[13], tee_uid.a[14], tee_uid.a[15]);
+
+ for ( desc = _steemediator; desc != _eteemediator; desc++ )
{
+ if ( memcmp(&desc->uid, &tee_uid, sizeof(xen_uuid_t)) == 0 )
+ {
+ printk("Using TEE mediator for %sp\n", desc->name);
+ mediator_ops = desc->ops;
+ break;
+ }
}
+
+ if ( !mediator_ops )
A warning here would be useful.
Why? Platform is not obligued to have any TEE.
What do you mean? You can only be here because the platform has TEE but
Xen does not have any mediator. You actually print "no TEE found" a bit
above. So why not printing for when Xen is unable to use it?
+ return;
+
+ ret = mediator_ops->init();
+ if ( ret )
+ {
+ printk("TEE mediator failed to initialize :%d\n", ret);
+ mediator_ops = NULL;
+ }
+}
+
+bool tee_handle_smc(struct cpu_user_regs *regs)
+{
+ if ( !mediator_ops )
+ return false;
+
+ return mediator_ops->handle_smc(regs);
+}
+
+void tee_domain_create(struct domain *d)
+{
+ if ( !mediator_ops )
+ return;
+
+ return mediator_ops->domain_create(d);
return here is not necessary. However, I am slightly surprised that
tee_domain_create could never fail.
Good question. Do you want to allow TEE to prevent domain creation?
See my answer a bit above on the call.
+}
+
+void tee_domain_destroy(struct domain *d)
+{
+ if ( !mediator_ops )
+ return;
+
+ return mediator_ops->domain_destroy(d);
Same here.
+}
+
+void tee_remove(void)
What is this callback for? I don't see any use within this series.
Sorry, missed to call it.
[...]
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
