All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/6] TEE mediator (and OP-TEE) support in XEN
@ 2018-08-22 14:11 Volodymyr Babchuk
  2018-08-22 14:11 ` [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC Volodymyr Babchuk
                   ` (5 more replies)
  0 siblings, 6 replies; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-22 14:11 UTC (permalink / raw)
  To: xen-devel, Stefano Stabellini, Julien Grall
  Cc: tee-dev, Artem Mygaiev, Volodymyr Babchuk, Jens Wiklander

Hello all,

This is follow for patch series [1]. There was lots of discussions
for that series and I tried to address all of them in this new patchset.

Currently, I had a working solution for OP-TEE virtualization and it is being
upstreamed right now ([2]). So, I think it is a good time to introduce support
in XEN as well.

This series include generic TEE mediator framework and full-scale OP-TEE mediator
which is working with mentioned chages in OP-TEE. So, multiple domains can
work simultaneously with OP-TEE.

I added XSM support, so now it is possible to control which domains can work
with TEEs. Also I changed way how TEE discovery is done. Now  it is very
generic and should support any platform.

[1] https://lists.xenproject.org/archives/html/xen-devel/2017-10/msg01451.html
[2] https://github.com/OP-TEE/optee_os/pull/2370

Volodymyr Babchuk (6):
  arm: add SMC wrapper that is compatible with SMCCC
  arm: add generic TEE mediator framework
  arm: tee: add OP-TEE header files
  optee: add OP-TEE mediator
  libxl: create DTS node for OP-TEE if it is enabled
  xsm: add tee access policy support

 MAINTAINERS                                 |   5 +
 tools/flask/policy/modules/dom0.te          |   3 +
 tools/flask/policy/modules/domU_with_tee.te |  23 +
 tools/flask/policy/modules/modules.conf     |   1 +
 tools/flask/policy/modules/xen.if           |  12 +
 tools/libxl/libxl_arm.c                     |  29 +
 tools/libxl/libxl_create.c                  |   1 +
 tools/libxl/libxl_types.idl                 |   1 +
 tools/xl/xl_parse.c                         |   1 +
 xen/arch/arm/Kconfig                        |  10 +
 xen/arch/arm/Makefile                       |   1 +
 xen/arch/arm/arm32/Makefile                 |   1 +
 xen/arch/arm/arm32/smc.S                    |  39 ++
 xen/arch/arm/arm64/Makefile                 |   1 +
 xen/arch/arm/arm64/asm-offsets.c            |   4 +
 xen/arch/arm/arm64/smc.S                    |  30 +
 xen/arch/arm/domain.c                       |   7 +
 xen/arch/arm/setup.c                        |   4 +
 xen/arch/arm/shutdown.c                     |   2 +
 xen/arch/arm/tee/Kconfig                    |   4 +
 xen/arch/arm/tee/Makefile                   |   2 +
 xen/arch/arm/tee/optee.c                    | 972 ++++++++++++++++++++++++++++
 xen/arch/arm/tee/tee.c                      |  89 +++
 xen/arch/arm/vsmc.c                         |   5 +
 xen/arch/arm/xen.lds.S                      |   7 +
 xen/include/asm-arm/processor.h             |  11 +
 xen/include/asm-arm/tee/optee_msg.h         | 444 +++++++++++++
 xen/include/asm-arm/tee/optee_smc.h         | 507 +++++++++++++++
 xen/include/asm-arm/tee/tee.h               | 103 +++
 xen/include/xsm/dummy.h                     |  10 +
 xen/include/xsm/xsm.h                       |  13 +
 xen/xsm/dummy.c                             |   4 +
 xen/xsm/flask/hooks.c                       |  15 +
 xen/xsm/flask/policy/access_vectors         |   7 +
 xen/xsm/flask/policy/security_classes       |   1 +
 35 files changed, 2369 insertions(+)
 create mode 100644 tools/flask/policy/modules/domU_with_tee.te
 create mode 100644 xen/arch/arm/arm32/smc.S
 create mode 100644 xen/arch/arm/arm64/smc.S
 create mode 100644 xen/arch/arm/tee/Kconfig
 create mode 100644 xen/arch/arm/tee/Makefile
 create mode 100644 xen/arch/arm/tee/optee.c
 create mode 100644 xen/arch/arm/tee/tee.c
 create mode 100644 xen/include/asm-arm/tee/optee_msg.h
 create mode 100644 xen/include/asm-arm/tee/optee_smc.h
 create mode 100644 xen/include/asm-arm/tee/tee.h

-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-22 14:11 [PATCH v1 0/6] TEE mediator (and OP-TEE) support in XEN Volodymyr Babchuk
@ 2018-08-22 14:11 ` Volodymyr Babchuk
  2018-08-22 16:46   ` Julien Grall
  2018-08-27  6:44   ` Jan Beulich
  2018-08-22 14:11 ` [PATCH v1 2/6] arm: add generic TEE mediator framework Volodymyr Babchuk
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-22 14:11 UTC (permalink / raw)
  To: xen-devel, Andrew Cooper, George Dunlap, Ian Jackson,
	Jan Beulich, Konrad Rzeszutek Wilk, Stefano Stabellini,
	Tim Deegan, Wei Liu, Julien Grall
  Cc: tee-dev, Edgar E. Iglesias, Volodymyr Babchuk

Existing SMC wrapper call_smc() allows only 4 parameters and
returns only one value. This is enough for existing
use in PSCI code, but TEE mediator will need a call that is
fully compatible with ARM SMCCC.
This patch adds this call for both arm32 and arm64.

There was similar patch by Edgar E. Iglesias ([1]), but looks
like it is abandoned.

[1] https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00636.html

CC: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---

changes from "RFC":
  - response now stored in structure instead of array
  - added comments for arm32 assembly code
  - added offset (instead of magic values) for arm32 asm code

 xen/arch/arm/arm32/Makefile      |  1 +
 xen/arch/arm/arm32/smc.S         | 39 +++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/arm64/Makefile      |  1 +
 xen/arch/arm/arm64/asm-offsets.c |  4 ++++
 xen/arch/arm/arm64/smc.S         | 30 ++++++++++++++++++++++++++++++
 xen/include/asm-arm/processor.h  | 11 +++++++++++
 6 files changed, 86 insertions(+)
 create mode 100644 xen/arch/arm/arm32/smc.S
 create mode 100644 xen/arch/arm/arm64/smc.S

diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 0ac254f..c69f35e 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -10,4 +10,5 @@ obj-y += proc-v7.o proc-caxx.o
 obj-y += smpboot.o
 obj-y += traps.o
 obj-y += vfp.o
+obj-y += smc.o
 
diff --git a/xen/arch/arm/arm32/smc.S b/xen/arch/arm/arm32/smc.S
new file mode 100644
index 0000000..56f7982
--- /dev/null
+++ b/xen/arch/arm/arm32/smc.S
@@ -0,0 +1,39 @@
+/*
+ * xen/arch/arm/arm32/smc.S
+ *
+ * Wrapper for Secure Monitors Calls
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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 <asm/macros.h>
+
+/*
+ * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
+ *                     register_t a3, register_t a4, register_t a5,
+ *                     register_t a6, register_t a7, struct smccc_res *res)
+ */
+ENTRY(call_smccc_smc)
+        mov     r12, sp
+        push    {r4-r7}
+	/*
+	 * According to ARMv7 (or aarch32) ABI, first 4 parameters of
+	 * call_smccc_smc are passed in r0-r3 and other 4 are on stack.
+	 * Load a4-a7 from stack to r4-r7.
+	 */
+        ldm     r12, {r4-r7}
+        smc     #0
+        pop     {r4-r7}
+	/* Load pointer to res (4th parameter on stack) to r12 */
+        ldr     r12, [sp, #(4 * 4)]
+	/* Store returned results from r0-r3 to res */
+        stm     r12, {r0-r3}
+        bx      lr
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index bb5c610..7ecdad7 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -9,6 +9,7 @@ obj-y += entry.o
 obj-y += insn.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o
 obj-y += smpboot.o
+obj-y += smc.o
 obj-y += traps.o
 obj-y += vfp.o
 obj-y += vsysreg.o
diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
index ce24e44..5353fe8 100644
--- a/xen/arch/arm/arm64/asm-offsets.c
+++ b/xen/arch/arm/arm64/asm-offsets.c
@@ -50,6 +50,10 @@ void __dummy__(void)
 
    BLANK();
    OFFSET(INITINFO_stack, struct init_info, stack);
+
+   BLANK();
+   OFFSET(SMCCC_RES_a0, struct smccc_res, a0);
+   OFFSET(SMCCC_RES_a2, struct smccc_res, a2);
 }
 
 /*
diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
new file mode 100644
index 0000000..40843c0
--- /dev/null
+++ b/xen/arch/arm/arm64/smc.S
@@ -0,0 +1,30 @@
+/*
+ * xen/arch/arm/arm64/smc.S
+ *
+ * Wrapper for Secure Monitors Calls
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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 <asm/asm_defns.h>
+#include <asm/macros.h>
+
+/*
+ * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
+ *                     register_t a3, register_t a4, register_t a5,
+ *                     register_t a6, register_t a7, struct smccc_res *res)
+ */
+ENTRY(call_smccc_smc)
+        smc     #0
+        ldr     x4, [sp]
+        stp     x0, x1, [x4, #SMCCC_RES_a0]
+        stp     x2, x3, [x4, #SMCCC_RES_a2]
+        ret
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 222a02d..946a2db 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -650,6 +650,13 @@ union hsr {
 
 
 };
+
+struct smccc_res {
+    register_t a0;
+    register_t a1;
+    register_t a2;
+    register_t a3;
+};
 #endif
 
 /* HSR.EC == HSR_CP{15,14,10}_32 */
@@ -815,6 +822,10 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
 int call_smc(register_t function_id, register_t arg0, register_t arg1,
              register_t arg2);
 
+void call_smccc_smc(register_t a0, register_t a1, register_t a2,
+                    register_t a3, register_t a4, register_t a5,
+                    register_t a6, register_t a7, struct smccc_res *res);
+
 void do_trap_hyp_serror(struct cpu_user_regs *regs);
 
 void do_trap_guest_serror(struct cpu_user_regs *regs);
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v1 2/6] arm: add generic TEE mediator framework
  2018-08-22 14:11 [PATCH v1 0/6] TEE mediator (and OP-TEE) support in XEN Volodymyr Babchuk
  2018-08-22 14:11 ` [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC Volodymyr Babchuk
@ 2018-08-22 14:11 ` Volodymyr Babchuk
  2018-08-22 17:03   ` Julien Grall
  2018-08-22 14:11 ` [PATCH v1 3/6] arm: tee: add OP-TEE header files Volodymyr Babchuk
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-22 14:11 UTC (permalink / raw)
  To: xen-devel, Andrew Cooper, George Dunlap, Ian Jackson,
	Jan Beulich, Konrad Rzeszutek Wilk, Stefano Stabellini,
	Tim Deegan, Wei Liu, Julien Grall
  Cc: tee-dev, Volodymyr Babchuk

This patch adds basic framework for TEE mediators. Guests can't talk
to TEE directly, we need some entity that will intercept request
and decide what to do with them. "TEE mediator" is a such entity.

This is how it works: user can build XEN with multiple TEE mediators
(see the next patches, where OP-TEE mediator is introduced).
TEE mediator register self with REGISTER_TEE_MEDIATOR() macro in the
same way, as device drivers use DT_DEVICE_START()/DT_DEVICE_END()
macros.
In runtime, during initialization, framework calls probe() function
for each available mediator driver to find which TEE is installed
on the platform. Then generic vSMC handler will call selected mediator
when it intercept SMC that belongs to TEE OS or TEE application.

Also, there are hooks for domain construction and destruction, so
TEE mediator can inform TEE about VM lifecycle.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---

changes from "RFC" version:
 - renamed CONFIG_ARM_TEE to CONFIG_TEE
 - changed discovery mechanism: instead of UUID mathing, TEE-specific
   probing is used

 MAINTAINERS                   |   5 ++
 xen/arch/arm/Kconfig          |  10 ++++
 xen/arch/arm/Makefile         |   1 +
 xen/arch/arm/domain.c         |   7 +++
 xen/arch/arm/setup.c          |   4 ++
 xen/arch/arm/shutdown.c       |   2 +
 xen/arch/arm/tee/Kconfig      |   0
 xen/arch/arm/tee/Makefile     |   1 +
 xen/arch/arm/tee/tee.c        |  79 ++++++++++++++++++++++++++++++++
 xen/arch/arm/vsmc.c           |   5 ++
 xen/arch/arm/xen.lds.S        |   7 +++
 xen/include/asm-arm/tee/tee.h | 103 ++++++++++++++++++++++++++++++++++++++++++
 12 files changed, 224 insertions(+)
 create mode 100644 xen/arch/arm/tee/Kconfig
 create mode 100644 xen/arch/arm/tee/Makefile
 create mode 100644 xen/arch/arm/tee/tee.c
 create mode 100644 xen/include/asm-arm/tee/tee.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 15d45d0..b0034a9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -365,6 +365,11 @@ F:	config/Stubdom.mk.in
 F:	m4/stubdom.m4
 F:	stubdom/
 
+TEE MEDIATORS
+M:	Volodymyr Babchuk <volodymyr_babchuk@epam.com>
+S:	Supported
+F:	xen/arch/arm/tee/*
+
 TOOLSTACK
 M:	Ian Jackson <ian.jackson@eu.citrix.com>
 M:	Wei Liu <wei.liu2@citrix.com>
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 2782ee6..09bfc9b 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -57,6 +57,14 @@ config SBSA_VUART_CONSOLE
 	  Allows a guest to use SBSA Generic UART as a console. The
 	  SBSA Generic UART implements a subset of ARM PL011 UART.
 
+config TEE
+	bool "Enable TEE mediators support"
+	default n
+	depends on ARM
+	help
+	  This option enables generic TEE mediators support. It allows guests
+	  to access real TEE via one of TEE mediators implemented in XEN.
+
 endmenu
 
 menu "ARM errata workaround via the alternative framework"
@@ -197,3 +205,5 @@ config ARM32_HARDEN_BRANCH_PREDICTOR
 source "common/Kconfig"
 
 source "drivers/Kconfig"
+
+source "arch/arm/tee/Kconfig"
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 30a2a65..af29aa7 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_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 a74ff1c..41e41b9 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/tee.h>
 #include <asm/vfp.h>
 #include <asm/vgic.h>
 #include <asm/vtimer.h>
@@ -671,6 +672,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);
+
     return 0;
 
 fail:
@@ -878,6 +882,9 @@ int domain_relinquish_resources(struct domain *d)
          */
         domain_vpl011_deinit(d);
 
+        /* Free TEE mediator resources */
+        tee_domain_destroy(d);
+
         d->arch.relmem = RELMEM_xen;
         /* Fallthrough */
 
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 032a6a8..4d31d94 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -47,6 +47,7 @@
 #include <asm/platform.h>
 #include <asm/procinfo.h>
 #include <asm/setup.h>
+#include <asm/tee/tee.h>
 #include <xsm/xsm.h>
 #include <asm/acpi.h>
 
@@ -851,6 +852,9 @@ void __init start_xen(unsigned long boot_phys_offset,
     apply_alternatives_all();
     enable_errata_workarounds();
 
+    /* Initialize TEE mediator */
+    tee_init();
+
     /* Create initial domain 0. */
     /* The vGIC for DOM0 is exactly emulating the hardware GIC */
     config.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
diff --git a/xen/arch/arm/shutdown.c b/xen/arch/arm/shutdown.c
index b32f07e..e1ace18 100644
--- a/xen/arch/arm/shutdown.c
+++ b/xen/arch/arm/shutdown.c
@@ -5,6 +5,7 @@
 #include <xen/smp.h>
 #include <asm/platform.h>
 #include <asm/psci.h>
+#include <asm/tee/tee.h>
 
 static void noreturn halt_this_cpu(void *arg)
 {
@@ -15,6 +16,7 @@ void machine_halt(void)
 {
     int timeout = 10;
 
+    tee_remove();
     watchdog_disable();
     console_start_sync();
     local_irq_enable();
diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig
new file mode 100644
index 0000000..e69de29
diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
new file mode 100644
index 0000000..c54d479
--- /dev/null
+++ b/xen/arch/arm/tee/Makefile
@@ -0,0 +1 @@
+obj-y += tee.o
diff --git a/xen/arch/arm/tee/tee.c b/xen/arch/arm/tee/tee.c
new file mode 100644
index 0000000..6df3b09
--- /dev/null
+++ b/xen/arch/arm/tee/tee.c
@@ -0,0 +1,79 @@
+/*
+ * xen/arch/arm/tee/tee.c
+ *
+ * Generic part of TEE mediator subsystem
+ *
+ * Volodymyr Babchuk <volodymyr_babchuk@epam.com>
+ * 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/errno.h>
+#include <xen/types.h>
+#include <asm/tee/tee.h>
+
+extern const struct tee_mediator_desc _steemediator[], _eteemediator[];
+static const struct tee_mediator_ops *mediator_ops;
+
+void tee_init(void)
+{
+    const struct tee_mediator_desc *desc;
+
+    for ( desc = _steemediator; desc != _eteemediator; desc++ )
+        if ( desc->ops->probe() == 0 )
+        {
+            printk(XENLOG_INFO "Using TEE mediator for %s\n", desc->name);
+            mediator_ops = desc->ops;
+            return;
+        }
+    printk(XENLOG_WARNING "No TEE mediator found\n");
+}
+
+bool tee_handle_smc(struct cpu_user_regs *regs)
+{
+    if ( !mediator_ops )
+        return false;
+
+    return mediator_ops->handle_smc(regs);
+}
+
+int tee_domain_create(struct domain *d)
+{
+    if ( !mediator_ops )
+        return -ENODEV;
+
+    return mediator_ops->domain_create(d);
+}
+
+void tee_domain_destroy(struct domain *d)
+{
+    if ( !mediator_ops )
+        return;
+
+    return mediator_ops->domain_destroy(d);
+}
+
+void tee_remove(void)
+{
+    if ( !mediator_ops )
+        return;
+
+    return mediator_ops->remove();
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index 40a80d5..aacebf9 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -22,6 +22,7 @@
 #include <asm/monitor.h>
 #include <asm/regs.h>
 #include <asm/smccc.h>
+#include <asm/tee/tee.h>
 #include <asm/traps.h>
 #include <asm/vpsci.h>
 
@@ -235,6 +236,10 @@ static bool vsmccc_handle_call(struct cpu_user_regs *regs)
         case ARM_SMCCC_OWNER_STANDARD:
             handled = handle_sssc(regs);
             break;
+        case ARM_SMCCC_OWNER_TRUSTED_APP ... ARM_SMCCC_OWNER_TRUSTED_APP_END:
+        case ARM_SMCCC_OWNER_TRUSTED_OS ... ARM_SMCCC_OWNER_TRUSTED_OS_END:
+            handled = tee_handle_smc(regs);
+            break;
         }
     }
 
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index c9b9546..b78b7f1 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -126,6 +126,13 @@ SECTIONS
       _aedevice = .;
   } :text
 
+  . = ALIGN(8);
+  .teemediator.info : {
+      _steemediator = .;
+      *(.teemediator.info)
+      _eteemediator = .;
+  } :text
+
   . = ALIGN(PAGE_SIZE);             /* Init code and data */
   __init_begin = .;
   .init.text : {
diff --git a/xen/include/asm-arm/tee/tee.h b/xen/include/asm-arm/tee/tee.h
new file mode 100644
index 0000000..ffd25b0
--- /dev/null
+++ b/xen/include/asm-arm/tee/tee.h
@@ -0,0 +1,103 @@
+/*
+ * xen/include/asm-arm/tee.h
+ *
+ * Generic part of TEE mediator subsystem
+ *
+ * Volodymyr Babchuk <volodymyr_babchuk@epam.com>
+ * 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 as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ */
+
+#ifndef __ARCH_ARM_TEE_TEE_H__
+#define __ARCH_ARM_TEE_TEE_H__
+
+#include <xen/lib.h>
+#include <xen/types.h>
+#include <asm/regs.h>
+
+#ifdef CONFIG_TEE
+
+struct tee_mediator_ops {
+    /*
+     * Probe for TEE. Should return 0 if TEE found and
+     * mediator is initialized.
+     */
+    int (*probe)(void);
+
+    /*
+     * Called during domain construction so mediator can inform TEE about new
+     * guest and create own structures for the new domain.
+     */
+    int (*domain_create)(struct domain *d);
+
+    /*
+     * Called during domain destruction to inform TEE that guest is now dead
+     * and to destroy all resources allocated for the domain being destroyed.
+     */
+    void (*domain_destroy)(struct domain *d);
+
+    /* Handle SMC call for current domain. */
+    bool (*handle_smc)(struct cpu_user_regs *regs);
+
+    /* Called during XEN shutdown to free remaining resources. */
+    void (*remove)(void);
+};
+
+struct tee_mediator_desc {
+    /* Name of the TEE. Just for debugging purposes. */
+    const char *name;
+
+    /* Mediator callbacks as described above. */
+    const struct tee_mediator_ops *ops;
+};
+
+void tee_init(void);
+bool tee_handle_smc(struct cpu_user_regs *regs);
+int tee_domain_create(struct domain *d);
+void tee_domain_destroy(struct domain *d);
+void tee_remove(void);
+
+#define REGISTER_TEE_MEDIATOR(_name, _namestr, _ops)          \
+static const struct tee_mediator_desc __tee_desc_##_name __used     \
+__section(".teemediator.info") = {                                  \
+    .name = _namestr,                                               \
+    .ops = _ops                                                     \
+}
+
+#else
+
+static inline void tee_init(void) {}
+static inline bool tee_handle_smc(struct cpu_user_regs *regs)
+{
+    return false;
+}
+
+static inline int tee_domain_create(struct domain *d)
+{
+    return -ENODEV;
+}
+
+static inline void tee_domain_destroy(struct domain *d) {}
+static inline void tee_remove(void) {}
+
+#endif  /* CONFIG_TEE */
+
+#endif /* __ARCH_ARM_TEE_TEE_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v1 3/6] arm: tee: add OP-TEE header files
  2018-08-22 14:11 [PATCH v1 0/6] TEE mediator (and OP-TEE) support in XEN Volodymyr Babchuk
  2018-08-22 14:11 ` [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC Volodymyr Babchuk
  2018-08-22 14:11 ` [PATCH v1 2/6] arm: add generic TEE mediator framework Volodymyr Babchuk
@ 2018-08-22 14:11 ` Volodymyr Babchuk
  2018-08-22 14:11 ` [PATCH v1 4/6] optee: add OP-TEE mediator Volodymyr Babchuk
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-22 14:11 UTC (permalink / raw)
  To: xen-devel, Andrew Cooper, George Dunlap, Ian Jackson,
	Jan Beulich, Konrad Rzeszutek Wilk, Stefano Stabellini,
	Tim Deegan, Wei Liu, Julien Grall, Jens Wiklander
  Cc: tee-dev, Volodymyr Babchuk

This header files describe protocol between OP-TEE and OP-TEE client
driver in Linux. They are needed for upcomient OP-TEE mediator, which
is added in the next patch.
Reason to add those headers in separate patch is to ease up review.
Those files were taken from linux tree (drivers/tee/optee/) and mangled
a bit to compile with XEN.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---

 Changes from "RFC":
  - moved headers to include/asm-arm/tee

 xen/include/asm-arm/tee/optee_msg.h | 444 +++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/tee/optee_smc.h | 457 ++++++++++++++++++++++++++++++++++++
 2 files changed, 901 insertions(+)
 create mode 100644 xen/include/asm-arm/tee/optee_msg.h
 create mode 100644 xen/include/asm-arm/tee/optee_smc.h

diff --git a/xen/include/asm-arm/tee/optee_msg.h b/xen/include/asm-arm/tee/optee_msg.h
new file mode 100644
index 0000000..10747b2
--- /dev/null
+++ b/xen/include/asm-arm/tee/optee_msg.h
@@ -0,0 +1,444 @@
+/*
+ * Copyright (c) 2015-2016, Linaro Limited
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+#ifndef _OPTEE_MSG_H
+#define _OPTEE_MSG_H
+
+#include <xen/bitops.h>
+#include <xen/types.h>
+
+/*
+ * This file defines the OP-TEE message protocol used to communicate
+ * with an instance of OP-TEE running in secure world.
+ *
+ * This file is divided into three sections.
+ * 1. Formatting of messages.
+ * 2. Requests from normal world
+ * 3. Requests from secure world, Remote Procedure Call (RPC), handled by
+ *    tee-supplicant.
+ */
+
+/*****************************************************************************
+ * Part 1 - formatting of messages
+ *****************************************************************************/
+
+#define OPTEE_MSG_ATTR_TYPE_NONE		0x0
+#define OPTEE_MSG_ATTR_TYPE_VALUE_INPUT		0x1
+#define OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT	0x2
+#define OPTEE_MSG_ATTR_TYPE_VALUE_INOUT		0x3
+#define OPTEE_MSG_ATTR_TYPE_RMEM_INPUT		0x5
+#define OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT		0x6
+#define OPTEE_MSG_ATTR_TYPE_RMEM_INOUT		0x7
+#define OPTEE_MSG_ATTR_TYPE_TMEM_INPUT		0x9
+#define OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT		0xa
+#define OPTEE_MSG_ATTR_TYPE_TMEM_INOUT		0xb
+
+#define OPTEE_MSG_ATTR_TYPE_MASK		GENMASK(7, 0)
+
+/*
+ * Meta parameter to be absorbed by the Secure OS and not passed
+ * to the Trusted Application.
+ *
+ * Currently only used with OPTEE_MSG_CMD_OPEN_SESSION.
+ */
+#define OPTEE_MSG_ATTR_META			BIT(8)
+
+/*
+ * Pointer to a list of pages used to register user-defined SHM buffer.
+ * Used with OPTEE_MSG_ATTR_TYPE_TMEM_*.
+ * buf_ptr should point to the beginning of the buffer. Buffer will contain
+ * list of page addresses. OP-TEE core can reconstruct contiguous buffer from
+ * that page addresses list. Page addresses are stored as 64 bit values.
+ * Last entry on a page should point to the next page of buffer.
+ * Every entry in buffer should point to a 4k page beginning (12 least
+ * significant bits must be equal to zero).
+ *
+ * 12 least significant bints of optee_msg_param.u.tmem.buf_ptr should hold page
+ * offset of the user buffer.
+ *
+ * So, entries should be placed like members of this structure:
+ *
+ * struct page_data {
+ *   uint64_t pages_array[OPTEE_MSG_NONCONTIG_PAGE_SIZE/sizeof(uint64_t) - 1];
+ *   uint64_t next_page_data;
+ * };
+ *
+ * Structure is designed to exactly fit into the page size
+ * OPTEE_MSG_NONCONTIG_PAGE_SIZE which is a standard 4KB page.
+ *
+ * The size of 4KB is chosen because this is the smallest page size for ARM
+ * architectures. If REE uses larger pages, it should divide them to 4KB ones.
+ */
+#define OPTEE_MSG_ATTR_NONCONTIG		BIT(9)
+
+/*
+ * Memory attributes for caching passed with temp memrefs. The actual value
+ * used is defined outside the message protocol with the exception of
+ * OPTEE_MSG_ATTR_CACHE_PREDEFINED which means the attributes already
+ * defined for the memory range should be used. If optee_smc.h is used as
+ * bearer of this protocol OPTEE_SMC_SHM_* is used for values.
+ */
+#define OPTEE_MSG_ATTR_CACHE_SHIFT		16
+#define OPTEE_MSG_ATTR_CACHE_MASK		GENMASK(2, 0)
+#define OPTEE_MSG_ATTR_CACHE_PREDEFINED		0
+
+/*
+ * Same values as TEE_LOGIN_* from TEE Internal API
+ */
+#define OPTEE_MSG_LOGIN_PUBLIC			0x00000000
+#define OPTEE_MSG_LOGIN_USER			0x00000001
+#define OPTEE_MSG_LOGIN_GROUP			0x00000002
+#define OPTEE_MSG_LOGIN_APPLICATION		0x00000004
+#define OPTEE_MSG_LOGIN_APPLICATION_USER	0x00000005
+#define OPTEE_MSG_LOGIN_APPLICATION_GROUP	0x00000006
+
+/*
+ * Page size used in non-contiguous buffer entries
+ */
+#define OPTEE_MSG_NONCONTIG_PAGE_SIZE		4096
+
+/**
+ * struct optee_msg_param_tmem - temporary memory reference parameter
+ * @buf_ptr:	Address of the buffer
+ * @size:	Size of the buffer
+ * @shm_ref:	Temporary shared memory reference, pointer to a struct tee_shm
+ *
+ * Secure and normal world communicates pointers as physical address
+ * instead of the virtual address. This is because secure and normal world
+ * have completely independent memory mapping. Normal world can even have a
+ * hypervisor which need to translate the guest physical address (AKA IPA
+ * in ARM documentation) to a real physical address before passing the
+ * structure to secure world.
+ */
+struct optee_msg_param_tmem {
+	u64 buf_ptr;
+	u64 size;
+	u64 shm_ref;
+};
+
+/**
+ * struct optee_msg_param_rmem - registered memory reference parameter
+ * @offs:	Offset into shared memory reference
+ * @size:	Size of the buffer
+ * @shm_ref:	Shared memory reference, pointer to a struct tee_shm
+ */
+struct optee_msg_param_rmem {
+	u64 offs;
+	u64 size;
+	u64 shm_ref;
+};
+
+/**
+ * struct optee_msg_param_value - opaque value parameter
+ *
+ * Value parameters are passed unchecked between normal and secure world.
+ */
+struct optee_msg_param_value {
+	u64 a;
+	u64 b;
+	u64 c;
+};
+
+/**
+ * struct optee_msg_param - parameter used together with struct optee_msg_arg
+ * @attr:	attributes
+ * @tmem:	parameter by temporary memory reference
+ * @rmem:	parameter by registered memory reference
+ * @value:	parameter by opaque value
+ *
+ * @attr & OPTEE_MSG_ATTR_TYPE_MASK indicates if tmem, rmem or value is used in
+ * the union. OPTEE_MSG_ATTR_TYPE_VALUE_* indicates value,
+ * OPTEE_MSG_ATTR_TYPE_TMEM_* indicates @tmem and
+ * OPTEE_MSG_ATTR_TYPE_RMEM_* indicates @rmem,
+ * OPTEE_MSG_ATTR_TYPE_NONE indicates that none of the members are used.
+ */
+struct optee_msg_param {
+	u64 attr;
+	union {
+		struct optee_msg_param_tmem tmem;
+		struct optee_msg_param_rmem rmem;
+		struct optee_msg_param_value value;
+	} u;
+};
+
+/**
+ * struct optee_msg_arg - call argument
+ * @cmd: Command, one of OPTEE_MSG_CMD_* or OPTEE_MSG_RPC_CMD_*
+ * @func: Trusted Application function, specific to the Trusted Application,
+ *	     used if cmd == OPTEE_MSG_CMD_INVOKE_COMMAND
+ * @session: In parameter for all OPTEE_MSG_CMD_* except
+ *	     OPTEE_MSG_CMD_OPEN_SESSION where it's an output parameter instead
+ * @cancel_id: Cancellation id, a unique value to identify this request
+ * @ret: return value
+ * @ret_origin: origin of the return value
+ * @num_params: number of parameters supplied to the OS Command
+ * @params: the parameters supplied to the OS Command
+ *
+ * All normal calls to Trusted OS uses this struct. If cmd requires further
+ * information than what these field holds it can be passed as a parameter
+ * tagged as meta (setting the OPTEE_MSG_ATTR_META bit in corresponding
+ * attrs field). All parameters tagged as meta has to come first.
+ *
+ * Temp memref parameters can be fragmented if supported by the Trusted OS
+ * (when optee_smc.h is bearer of this protocol this is indicated with
+ * OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM). If a logical memref parameter is
+ * fragmented then has all but the last fragment the
+ * OPTEE_MSG_ATTR_FRAGMENT bit set in attrs. Even if a memref is fragmented
+ * it will still be presented as a single logical memref to the Trusted
+ * Application.
+ */
+struct optee_msg_arg {
+	u32 cmd;
+	u32 func;
+	u32 session;
+	u32 cancel_id;
+	u32 pad;
+	u32 ret;
+	u32 ret_origin;
+	u32 num_params;
+
+	/* num_params tells the actual number of element in params */
+	struct optee_msg_param params[0];
+};
+
+/**
+ * OPTEE_MSG_GET_ARG_SIZE - return size of struct optee_msg_arg
+ *
+ * @num_params: Number of parameters embedded in the struct optee_msg_arg
+ *
+ * Returns the size of the struct optee_msg_arg together with the number
+ * of embedded parameters.
+ */
+#define OPTEE_MSG_GET_ARG_SIZE(num_params) \
+	(sizeof(struct optee_msg_arg) + \
+	 sizeof(struct optee_msg_param) * (num_params))
+
+/*****************************************************************************
+ * Part 2 - requests from normal world
+ *****************************************************************************/
+
+/*
+ * Return the following UID if using API specified in this file without
+ * further extensions:
+ * 384fb3e0-e7f8-11e3-af63-0002a5d5c51b.
+ * Represented in 4 32-bit words in OPTEE_MSG_UID_0, OPTEE_MSG_UID_1,
+ * OPTEE_MSG_UID_2, OPTEE_MSG_UID_3.
+ */
+#define OPTEE_MSG_UID_0			0x384fb3e0
+#define OPTEE_MSG_UID_1			0xe7f811e3
+#define OPTEE_MSG_UID_2			0xaf630002
+#define OPTEE_MSG_UID_3			0xa5d5c51b
+#define OPTEE_MSG_FUNCID_CALLS_UID	0xFF01
+
+/*
+ * Returns 2.0 if using API specified in this file without further
+ * extensions. Represented in 2 32-bit words in OPTEE_MSG_REVISION_MAJOR
+ * and OPTEE_MSG_REVISION_MINOR
+ */
+#define OPTEE_MSG_REVISION_MAJOR	2
+#define OPTEE_MSG_REVISION_MINOR	0
+#define OPTEE_MSG_FUNCID_CALLS_REVISION	0xFF03
+
+/*
+ * Get UUID of Trusted OS.
+ *
+ * Used by non-secure world to figure out which Trusted OS is installed.
+ * Note that returned UUID is the UUID of the Trusted OS, not of the API.
+ *
+ * Returns UUID in 4 32-bit words in the same way as
+ * OPTEE_MSG_FUNCID_CALLS_UID described above.
+ */
+#define OPTEE_MSG_OS_OPTEE_UUID_0	0x486178e0
+#define OPTEE_MSG_OS_OPTEE_UUID_1	0xe7f811e3
+#define OPTEE_MSG_OS_OPTEE_UUID_2	0xbc5e0002
+#define OPTEE_MSG_OS_OPTEE_UUID_3	0xa5d5c51b
+#define OPTEE_MSG_FUNCID_GET_OS_UUID	0x0000
+
+/*
+ * Get revision of Trusted OS.
+ *
+ * Used by non-secure world to figure out which version of the Trusted OS
+ * is installed. Note that the returned revision is the revision of the
+ * Trusted OS, not of the API.
+ *
+ * Returns revision in 2 32-bit words in the same way as
+ * OPTEE_MSG_CALLS_REVISION described above.
+ */
+#define OPTEE_MSG_FUNCID_GET_OS_REVISION	0x0001
+
+/*
+ * Do a secure call with struct optee_msg_arg as argument
+ * The OPTEE_MSG_CMD_* below defines what goes in struct optee_msg_arg::cmd
+ *
+ * OPTEE_MSG_CMD_OPEN_SESSION opens a session to a Trusted Application.
+ * The first two parameters are tagged as meta, holding two value
+ * parameters to pass the following information:
+ * param[0].u.value.a-b uuid of Trusted Application
+ * param[1].u.value.a-b uuid of Client
+ * param[1].u.value.c Login class of client OPTEE_MSG_LOGIN_*
+ *
+ * OPTEE_MSG_CMD_INVOKE_COMMAND invokes a command a previously opened
+ * session to a Trusted Application.  struct optee_msg_arg::func is Trusted
+ * Application function, specific to the Trusted Application.
+ *
+ * OPTEE_MSG_CMD_CLOSE_SESSION closes a previously opened session to
+ * Trusted Application.
+ *
+ * OPTEE_MSG_CMD_CANCEL cancels a currently invoked command.
+ *
+ * OPTEE_MSG_CMD_REGISTER_SHM registers a shared memory reference. The
+ * information is passed as:
+ * [in] param[0].attr			OPTEE_MSG_ATTR_TYPE_TMEM_INPUT
+ *					[| OPTEE_MSG_ATTR_FRAGMENT]
+ * [in] param[0].u.tmem.buf_ptr		physical address (of first fragment)
+ * [in] param[0].u.tmem.size		size (of first fragment)
+ * [in] param[0].u.tmem.shm_ref		holds shared memory reference
+ * ...
+ * The shared memory can optionally be fragmented, temp memrefs can follow
+ * each other with all but the last with the OPTEE_MSG_ATTR_FRAGMENT bit set.
+ *
+ * OPTEE_MSG_CMD_UNREGISTER_SHM unregisteres a previously registered shared
+ * memory reference. The information is passed as:
+ * [in] param[0].attr			OPTEE_MSG_ATTR_TYPE_RMEM_INPUT
+ * [in] param[0].u.rmem.shm_ref		holds shared memory reference
+ * [in] param[0].u.rmem.offs		0
+ * [in] param[0].u.rmem.size		0
+ */
+#define OPTEE_MSG_CMD_OPEN_SESSION	0
+#define OPTEE_MSG_CMD_INVOKE_COMMAND	1
+#define OPTEE_MSG_CMD_CLOSE_SESSION	2
+#define OPTEE_MSG_CMD_CANCEL		3
+#define OPTEE_MSG_CMD_REGISTER_SHM	4
+#define OPTEE_MSG_CMD_UNREGISTER_SHM	5
+#define OPTEE_MSG_FUNCID_CALL_WITH_ARG	0x0004
+
+/*****************************************************************************
+ * Part 3 - Requests from secure world, RPC
+ *****************************************************************************/
+
+/*
+ * All RPC is done with a struct optee_msg_arg as bearer of information,
+ * struct optee_msg_arg::arg holds values defined by OPTEE_MSG_RPC_CMD_* below
+ *
+ * RPC communication with tee-supplicant is reversed compared to normal
+ * client communication desribed above. The supplicant receives requests
+ * and sends responses.
+ */
+
+/*
+ * Load a TA into memory, defined in tee-supplicant
+ */
+#define OPTEE_MSG_RPC_CMD_LOAD_TA	0
+
+/*
+ * Reserved
+ */
+#define OPTEE_MSG_RPC_CMD_RPMB		1
+
+/*
+ * File system access, defined in tee-supplicant
+ */
+#define OPTEE_MSG_RPC_CMD_FS		2
+
+/*
+ * Get time
+ *
+ * Returns number of seconds and nano seconds since the Epoch,
+ * 1970-01-01 00:00:00 +0000 (UTC).
+ *
+ * [out] param[0].u.value.a	Number of seconds
+ * [out] param[0].u.value.b	Number of nano seconds.
+ */
+#define OPTEE_MSG_RPC_CMD_GET_TIME	3
+
+/*
+ * Wait queue primitive, helper for secure world to implement a wait queue.
+ *
+ * If secure world need to wait for a secure world mutex it issues a sleep
+ * request instead of spinning in secure world. Conversely is a wakeup
+ * request issued when a secure world mutex with a thread waiting thread is
+ * unlocked.
+ *
+ * Waiting on a key
+ * [in] param[0].u.value.a OPTEE_MSG_RPC_WAIT_QUEUE_SLEEP
+ * [in] param[0].u.value.b wait key
+ *
+ * Waking up a key
+ * [in] param[0].u.value.a OPTEE_MSG_RPC_WAIT_QUEUE_WAKEUP
+ * [in] param[0].u.value.b wakeup key
+ */
+#define OPTEE_MSG_RPC_CMD_WAIT_QUEUE	4
+#define OPTEE_MSG_RPC_WAIT_QUEUE_SLEEP	0
+#define OPTEE_MSG_RPC_WAIT_QUEUE_WAKEUP	1
+
+/*
+ * Suspend execution
+ *
+ * [in] param[0].value	.a number of milliseconds to suspend
+ */
+#define OPTEE_MSG_RPC_CMD_SUSPEND	5
+
+/*
+ * Allocate a piece of shared memory
+ *
+ * Shared memory can optionally be fragmented, to support that additional
+ * spare param entries are allocated to make room for eventual fragments.
+ * The spare param entries has .attr = OPTEE_MSG_ATTR_TYPE_NONE when
+ * unused. All returned temp memrefs except the last should have the
+ * OPTEE_MSG_ATTR_FRAGMENT bit set in the attr field.
+ *
+ * [in]  param[0].u.value.a		type of memory one of
+ *					OPTEE_MSG_RPC_SHM_TYPE_* below
+ * [in]  param[0].u.value.b		requested size
+ * [in]  param[0].u.value.c		required alignment
+ *
+ * [out] param[0].u.tmem.buf_ptr	physical address (of first fragment)
+ * [out] param[0].u.tmem.size		size (of first fragment)
+ * [out] param[0].u.tmem.shm_ref	shared memory reference
+ * ...
+ * [out] param[n].u.tmem.buf_ptr	physical address
+ * [out] param[n].u.tmem.size		size
+ * [out] param[n].u.tmem.shm_ref	shared memory reference (same value
+ *					as in param[n-1].u.tmem.shm_ref)
+ */
+#define OPTEE_MSG_RPC_CMD_SHM_ALLOC	6
+/* Memory that can be shared with a non-secure user space application */
+#define OPTEE_MSG_RPC_SHM_TYPE_APPL	0
+/* Memory only shared with non-secure kernel */
+#define OPTEE_MSG_RPC_SHM_TYPE_KERNEL	1
+
+/*
+ * Free shared memory previously allocated with OPTEE_MSG_RPC_CMD_SHM_ALLOC
+ *
+ * [in]  param[0].u.value.a		type of memory one of
+ *					OPTEE_MSG_RPC_SHM_TYPE_* above
+ * [in]  param[0].u.value.b		value of shared memory reference
+ *					returned in param[0].u.tmem.shm_ref
+ *					above
+ */
+#define OPTEE_MSG_RPC_CMD_SHM_FREE	7
+
+#endif /* _OPTEE_MSG_H */
diff --git a/xen/include/asm-arm/tee/optee_smc.h b/xen/include/asm-arm/tee/optee_smc.h
new file mode 100644
index 0000000..26d100e
--- /dev/null
+++ b/xen/include/asm-arm/tee/optee_smc.h
@@ -0,0 +1,457 @@
+/*
+ * Copyright (c) 2015-2016, Linaro Limited
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+#ifndef OPTEE_SMC_H
+#define OPTEE_SMC_H
+
+#include <asm/smccc.h>
+#include <xen/bitops.h>
+
+#define OPTEE_SMC_STD_CALL_VAL(func_num) \
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_STD_CALL, ARM_SMCCC_CONV_32, \
+			   ARM_SMCCC_OWNER_TRUSTED_OS, (func_num))
+#define OPTEE_SMC_FAST_CALL_VAL(func_num) \
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_CONV_32, \
+			   ARM_SMCCC_OWNER_TRUSTED_OS, (func_num))
+
+/*
+ * Function specified by SMC Calling convention.
+ */
+#define OPTEE_SMC_FUNCID_CALLS_COUNT	0xFF00
+#define OPTEE_SMC_CALLS_COUNT \
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_CONV_32, \
+			   ARM_SMCCC_OWNER_TRUSTED_OS_END, \
+			   OPTEE_SMC_FUNCID_CALLS_COUNT)
+
+/*
+ * Normal cached memory (write-back), shareable for SMP systems and not
+ * shareable for UP systems.
+ */
+#define OPTEE_SMC_SHM_CACHED		1
+
+/*
+ * a0..a7 is used as register names in the descriptions below, on arm32
+ * that translates to r0..r7 and on arm64 to w0..w7. In both cases it's
+ * 32-bit registers.
+ */
+
+/*
+ * Function specified by SMC Calling convention
+ *
+ * Return one of the following UIDs if using API specified in this file
+ * without further extentions:
+ * 65cb6b93-af0c-4617-8ed6-644a8d1140f8
+ * see also OPTEE_SMC_UID_* in optee_msg.h
+ */
+#define OPTEE_SMC_FUNCID_CALLS_UID OPTEE_MSG_FUNCID_CALLS_UID
+#define OPTEE_SMC_CALLS_UID \
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_CONV_32, \
+			   ARM_SMCCC_OWNER_TRUSTED_OS_END, \
+			   OPTEE_SMC_FUNCID_CALLS_UID)
+
+/*
+ * Function specified by SMC Calling convention
+ *
+ * Returns 2.0 if using API specified in this file without further extentions.
+ * see also OPTEE_MSG_REVISION_* in optee_msg.h
+ */
+#define OPTEE_SMC_FUNCID_CALLS_REVISION OPTEE_MSG_FUNCID_CALLS_REVISION
+#define OPTEE_SMC_CALLS_REVISION \
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_CONV_32, \
+			   ARM_SMCCC_OWNER_TRUSTED_OS_END, \
+			   OPTEE_SMC_FUNCID_CALLS_REVISION)
+
+struct optee_smc_calls_revision_result {
+	unsigned long major;
+	unsigned long minor;
+	unsigned long reserved0;
+	unsigned long reserved1;
+};
+
+/*
+ * Get UUID of Trusted OS.
+ *
+ * Used by non-secure world to figure out which Trusted OS is installed.
+ * Note that returned UUID is the UUID of the Trusted OS, not of the API.
+ *
+ * Returns UUID in a0-4 in the same way as OPTEE_SMC_CALLS_UID
+ * described above.
+ */
+#define OPTEE_SMC_FUNCID_GET_OS_UUID OPTEE_MSG_FUNCID_GET_OS_UUID
+#define OPTEE_SMC_CALL_GET_OS_UUID \
+	OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_GET_OS_UUID)
+
+/*
+ * Get revision of Trusted OS.
+ *
+ * Used by non-secure world to figure out which version of the Trusted OS
+ * is installed. Note that the returned revision is the revision of the
+ * Trusted OS, not of the API.
+ *
+ * Returns revision in a0-1 in the same way as OPTEE_SMC_CALLS_REVISION
+ * described above.
+ */
+#define OPTEE_SMC_FUNCID_GET_OS_REVISION OPTEE_MSG_FUNCID_GET_OS_REVISION
+#define OPTEE_SMC_CALL_GET_OS_REVISION \
+	OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_GET_OS_REVISION)
+
+/*
+ * Call with struct optee_msg_arg as argument
+ *
+ * Call register usage:
+ * a0	SMC Function ID, OPTEE_SMC*CALL_WITH_ARG
+ * a1	Upper 32bit of a 64bit physical pointer to a struct optee_msg_arg
+ * a2	Lower 32bit of a 64bit physical pointer to a struct optee_msg_arg
+ * a3	Cache settings, not used if physical pointer is in a predefined shared
+ *	memory area else per OPTEE_SMC_SHM_*
+ * a4-6	Not used
+ * a7	Hypervisor Client ID register
+ *
+ * Normal return register usage:
+ * a0	Return value, OPTEE_SMC_RETURN_*
+ * a1-3	Not used
+ * a4-7	Preserved
+ *
+ * OPTEE_SMC_RETURN_ETHREAD_LIMIT return register usage:
+ * a0	Return value, OPTEE_SMC_RETURN_ETHREAD_LIMIT
+ * a1-3	Preserved
+ * a4-7	Preserved
+ *
+ * RPC return register usage:
+ * a0	Return value, OPTEE_SMC_RETURN_IS_RPC(val)
+ * a1-2	RPC parameters
+ * a3-7	Resume information, must be preserved
+ *
+ * Possible return values:
+ * OPTEE_SMC_RETURN_UNKNOWN_FUNCTION	Trusted OS does not recognize this
+ *					function.
+ * OPTEE_SMC_RETURN_OK			Call completed, result updated in
+ *					the previously supplied struct
+ *					optee_msg_arg.
+ * OPTEE_SMC_RETURN_ETHREAD_LIMIT	Number of Trusted OS threads exceeded,
+ *					try again later.
+ * OPTEE_SMC_RETURN_EBADADDR		Bad physcial pointer to struct
+ *					optee_msg_arg.
+ * OPTEE_SMC_RETURN_EBADCMD		Bad/unknown cmd in struct optee_msg_arg
+ * OPTEE_SMC_RETURN_IS_RPC()		Call suspended by RPC call to normal
+ *					world.
+ */
+#define OPTEE_SMC_FUNCID_CALL_WITH_ARG OPTEE_MSG_FUNCID_CALL_WITH_ARG
+#define OPTEE_SMC_CALL_WITH_ARG \
+	OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_CALL_WITH_ARG)
+
+/*
+ * Get Shared Memory Config
+ *
+ * Returns the Secure/Non-secure shared memory config.
+ *
+ * Call register usage:
+ * a0	SMC Function ID, OPTEE_SMC_GET_SHM_CONFIG
+ * a1-6	Not used
+ * a7	Hypervisor Client ID register
+ *
+ * Have config return register usage:
+ * a0	OPTEE_SMC_RETURN_OK
+ * a1	Physical address of start of SHM
+ * a2	Size of of SHM
+ * a3	Cache settings of memory, as defined by the
+ *	OPTEE_SMC_SHM_* values above
+ * a4-7	Preserved
+ *
+ * Not available register usage:
+ * a0	OPTEE_SMC_RETURN_ENOTAVAIL
+ * a1-3 Not used
+ * a4-7	Preserved
+ */
+#define OPTEE_SMC_FUNCID_GET_SHM_CONFIG	7
+#define OPTEE_SMC_GET_SHM_CONFIG \
+	OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_GET_SHM_CONFIG)
+
+struct optee_smc_get_shm_config_result {
+	unsigned long status;
+	unsigned long start;
+	unsigned long size;
+	unsigned long settings;
+};
+
+/*
+ * Exchanges capabilities between normal world and secure world
+ *
+ * Call register usage:
+ * a0	SMC Function ID, OPTEE_SMC_EXCHANGE_CAPABILITIES
+ * a1	bitfield of normal world capabilities OPTEE_SMC_NSEC_CAP_*
+ * a2-6	Not used
+ * a7	Hypervisor Client ID register
+ *
+ * Normal return register usage:
+ * a0	OPTEE_SMC_RETURN_OK
+ * a1	bitfield of secure world capabilities OPTEE_SMC_SEC_CAP_*
+ * a2-7	Preserved
+ *
+ * Error return register usage:
+ * a0	OPTEE_SMC_RETURN_ENOTAVAIL, can't use the capabilities from normal world
+ * a1	bitfield of secure world capabilities OPTEE_SMC_SEC_CAP_*
+ * a2-7 Preserved
+ */
+/* Normal world works as a uniprocessor system */
+#define OPTEE_SMC_NSEC_CAP_UNIPROCESSOR		BIT(0)
+/* Secure world has reserved shared memory for normal world to use */
+#define OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM	BIT(0)
+/* Secure world can communicate via previously unregistered shared memory */
+#define OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM	BIT(1)
+
+/*
+ * Secure world supports commands "register/unregister shared memory",
+ * secure world accepts command buffers located in any parts of non-secure RAM
+ */
+#define OPTEE_SMC_SEC_CAP_DYNAMIC_SHM		BIT(2)
+
+#define OPTEE_SMC_FUNCID_EXCHANGE_CAPABILITIES	9
+#define OPTEE_SMC_EXCHANGE_CAPABILITIES \
+	OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_EXCHANGE_CAPABILITIES)
+
+struct optee_smc_exchange_capabilities_result {
+	unsigned long status;
+	unsigned long capabilities;
+	unsigned long reserved0;
+	unsigned long reserved1;
+};
+
+/*
+ * Disable and empties cache of shared memory objects
+ *
+ * Secure world can cache frequently used shared memory objects, for
+ * example objects used as RPC arguments. When secure world is idle this
+ * function returns one shared memory reference to free. To disable the
+ * cache and free all cached objects this function has to be called until
+ * it returns OPTEE_SMC_RETURN_ENOTAVAIL.
+ *
+ * Call register usage:
+ * a0	SMC Function ID, OPTEE_SMC_DISABLE_SHM_CACHE
+ * a1-6	Not used
+ * a7	Hypervisor Client ID register
+ *
+ * Normal return register usage:
+ * a0	OPTEE_SMC_RETURN_OK
+ * a1	Upper 32bit of a 64bit Shared memory cookie
+ * a2	Lower 32bit of a 64bit Shared memory cookie
+ * a3-7	Preserved
+ *
+ * Cache empty return register usage:
+ * a0	OPTEE_SMC_RETURN_ENOTAVAIL
+ * a1-7	Preserved
+ *
+ * Not idle return register usage:
+ * a0	OPTEE_SMC_RETURN_EBUSY
+ * a1-7	Preserved
+ */
+#define OPTEE_SMC_FUNCID_DISABLE_SHM_CACHE	10
+#define OPTEE_SMC_DISABLE_SHM_CACHE \
+	OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_DISABLE_SHM_CACHE)
+
+struct optee_smc_disable_shm_cache_result {
+	unsigned long status;
+	unsigned long shm_upper32;
+	unsigned long shm_lower32;
+	unsigned long reserved0;
+};
+
+/*
+ * Enable cache of shared memory objects
+ *
+ * Secure world can cache frequently used shared memory objects, for
+ * example objects used as RPC arguments. When secure world is idle this
+ * function returns OPTEE_SMC_RETURN_OK and the cache is enabled. If
+ * secure world isn't idle OPTEE_SMC_RETURN_EBUSY is returned.
+ *
+ * Call register usage:
+ * a0	SMC Function ID, OPTEE_SMC_ENABLE_SHM_CACHE
+ * a1-6	Not used
+ * a7	Hypervisor Client ID register
+ *
+ * Normal return register usage:
+ * a0	OPTEE_SMC_RETURN_OK
+ * a1-7	Preserved
+ *
+ * Not idle return register usage:
+ * a0	OPTEE_SMC_RETURN_EBUSY
+ * a1-7	Preserved
+ */
+#define OPTEE_SMC_FUNCID_ENABLE_SHM_CACHE	11
+#define OPTEE_SMC_ENABLE_SHM_CACHE \
+	OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_ENABLE_SHM_CACHE)
+
+/*
+ * Resume from RPC (for example after processing a foreign interrupt)
+ *
+ * Call register usage:
+ * a0	SMC Function ID, OPTEE_SMC_CALL_RETURN_FROM_RPC
+ * a1-3	Value of a1-3 when OPTEE_SMC_CALL_WITH_ARG returned
+ *	OPTEE_SMC_RETURN_RPC in a0
+ *
+ * Return register usage is the same as for OPTEE_SMC_*CALL_WITH_ARG above.
+ *
+ * Possible return values
+ * OPTEE_SMC_RETURN_UNKNOWN_FUNCTION	Trusted OS does not recognize this
+ *					function.
+ * OPTEE_SMC_RETURN_OK			Original call completed, result
+ *					updated in the previously supplied.
+ *					struct optee_msg_arg
+ * OPTEE_SMC_RETURN_RPC			Call suspended by RPC call to normal
+ *					world.
+ * OPTEE_SMC_RETURN_ERESUME		Resume failed, the opaque resume
+ *					information was corrupt.
+ */
+#define OPTEE_SMC_FUNCID_RETURN_FROM_RPC	3
+#define OPTEE_SMC_CALL_RETURN_FROM_RPC \
+	OPTEE_SMC_STD_CALL_VAL(OPTEE_SMC_FUNCID_RETURN_FROM_RPC)
+
+#define OPTEE_SMC_RETURN_RPC_PREFIX_MASK	0xFFFF0000
+#define OPTEE_SMC_RETURN_RPC_PREFIX		0xFFFF0000
+#define OPTEE_SMC_RETURN_RPC_FUNC_MASK		0x0000FFFF
+
+#define OPTEE_SMC_RETURN_GET_RPC_FUNC(ret) \
+	((ret) & OPTEE_SMC_RETURN_RPC_FUNC_MASK)
+
+#define OPTEE_SMC_RPC_VAL(func)		((func) | OPTEE_SMC_RETURN_RPC_PREFIX)
+
+/*
+ * Allocate memory for RPC parameter passing. The memory is used to hold a
+ * struct optee_msg_arg.
+ *
+ * "Call" register usage:
+ * a0	This value, OPTEE_SMC_RETURN_RPC_ALLOC
+ * a1	Size in bytes of required argument memory
+ * a2	Not used
+ * a3	Resume information, must be preserved
+ * a4-5	Not used
+ * a6-7	Resume information, must be preserved
+ *
+ * "Return" register usage:
+ * a0	SMC Function ID, OPTEE_SMC_CALL_RETURN_FROM_RPC.
+ * a1	Upper 32bits of 64bit physical pointer to allocated
+ *	memory, (a1 == 0 && a2 == 0) if size was 0 or if memory can't
+ *	be allocated.
+ * a2	Lower 32bits of 64bit physical pointer to allocated
+ *	memory, (a1 == 0 && a2 == 0) if size was 0 or if memory can't
+ *	be allocated
+ * a3	Preserved
+ * a4	Upper 32bits of 64bit Shared memory cookie used when freeing
+ *	the memory or doing an RPC
+ * a5	Lower 32bits of 64bit Shared memory cookie used when freeing
+ *	the memory or doing an RPC
+ * a6-7	Preserved
+ */
+#define OPTEE_SMC_RPC_FUNC_ALLOC	0
+#define OPTEE_SMC_RETURN_RPC_ALLOC \
+	OPTEE_SMC_RPC_VAL(OPTEE_SMC_RPC_FUNC_ALLOC)
+
+/*
+ * Free memory previously allocated by OPTEE_SMC_RETURN_RPC_ALLOC
+ *
+ * "Call" register usage:
+ * a0	This value, OPTEE_SMC_RETURN_RPC_FREE
+ * a1	Upper 32bits of 64bit shared memory cookie belonging to this
+ *	argument memory
+ * a2	Lower 32bits of 64bit shared memory cookie belonging to this
+ *	argument memory
+ * a3-7	Resume information, must be preserved
+ *
+ * "Return" register usage:
+ * a0	SMC Function ID, OPTEE_SMC_CALL_RETURN_FROM_RPC.
+ * a1-2	Not used
+ * a3-7	Preserved
+ */
+#define OPTEE_SMC_RPC_FUNC_FREE		2
+#define OPTEE_SMC_RETURN_RPC_FREE \
+	OPTEE_SMC_RPC_VAL(OPTEE_SMC_RPC_FUNC_FREE)
+
+/*
+ * Deliver foreign interrupt to normal world.
+ *
+ * "Call" register usage:
+ * a0	OPTEE_SMC_RETURN_RPC_FOREIGN_INTR
+ * a1-7	Resume information, must be preserved
+ *
+ * "Return" register usage:
+ * a0	SMC Function ID, OPTEE_SMC_CALL_RETURN_FROM_RPC.
+ * a1-7	Preserved
+ */
+#define OPTEE_SMC_RPC_FUNC_FOREIGN_INTR		4
+#define OPTEE_SMC_RETURN_RPC_FOREIGN_INTR \
+	OPTEE_SMC_RPC_VAL(OPTEE_SMC_RPC_FUNC_FOREIGN_INTR)
+
+/*
+ * Do an RPC request. The supplied struct optee_msg_arg tells which
+ * request to do and the parameters for the request. The following fields
+ * are used (the rest are unused):
+ * - cmd		the Request ID
+ * - ret		return value of the request, filled in by normal world
+ * - num_params		number of parameters for the request
+ * - params		the parameters
+ * - param_attrs	attributes of the parameters
+ *
+ * "Call" register usage:
+ * a0	OPTEE_SMC_RETURN_RPC_CMD
+ * a1	Upper 32bit of a 64bit Shared memory cookie holding a
+ *	struct optee_msg_arg, must be preserved, only the data should
+ *	be updated
+ * a2	Lower 32bit of a 64bit Shared memory cookie holding a
+ *	struct optee_msg_arg, must be preserved, only the data should
+ *	be updated
+ * a3-7	Resume information, must be preserved
+ *
+ * "Return" register usage:
+ * a0	SMC Function ID, OPTEE_SMC_CALL_RETURN_FROM_RPC.
+ * a1-2	Not used
+ * a3-7	Preserved
+ */
+#define OPTEE_SMC_RPC_FUNC_CMD		5
+#define OPTEE_SMC_RETURN_RPC_CMD \
+	OPTEE_SMC_RPC_VAL(OPTEE_SMC_RPC_FUNC_CMD)
+
+/* Returned in a0 */
+#define OPTEE_SMC_RETURN_UNKNOWN_FUNCTION 0xFFFFFFFF
+
+/* Returned in a0 only from Trusted OS functions */
+#define OPTEE_SMC_RETURN_OK		0x0
+#define OPTEE_SMC_RETURN_ETHREAD_LIMIT	0x1
+#define OPTEE_SMC_RETURN_EBUSY		0x2
+#define OPTEE_SMC_RETURN_ERESUME	0x3
+#define OPTEE_SMC_RETURN_EBADADDR	0x4
+#define OPTEE_SMC_RETURN_EBADCMD	0x5
+#define OPTEE_SMC_RETURN_ENOMEM		0x6
+#define OPTEE_SMC_RETURN_ENOTAVAIL	0x7
+#define OPTEE_SMC_RETURN_IS_RPC(ret)	__optee_smc_return_is_rpc((ret))
+
+static inline bool __optee_smc_return_is_rpc(u32 ret)
+{
+	return ret != OPTEE_SMC_RETURN_UNKNOWN_FUNCTION &&
+	       (ret & OPTEE_SMC_RETURN_RPC_PREFIX_MASK) ==
+			OPTEE_SMC_RETURN_RPC_PREFIX;
+}
+
+#endif /* OPTEE_SMC_H */
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v1 4/6] optee: add OP-TEE mediator
  2018-08-22 14:11 [PATCH v1 0/6] TEE mediator (and OP-TEE) support in XEN Volodymyr Babchuk
                   ` (2 preceding siblings ...)
  2018-08-22 14:11 ` [PATCH v1 3/6] arm: tee: add OP-TEE header files Volodymyr Babchuk
@ 2018-08-22 14:11 ` Volodymyr Babchuk
  2018-08-22 17:28   ` Julien Grall
  2018-08-22 14:11 ` [PATCH v1 5/6] libxl: create DTS node for OP-TEE if it is enabled Volodymyr Babchuk
  2018-08-22 14:11 ` [PATCH v1 6/6] xsm: add tee access policy support Volodymyr Babchuk
  5 siblings, 1 reply; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-22 14:11 UTC (permalink / raw)
  To: xen-devel, Andrew Cooper, George Dunlap, Ian Jackson,
	Jan Beulich, Konrad Rzeszutek Wilk, Stefano Stabellini,
	Tim Deegan, Wei Liu, Julien Grall, Jens Wiklander
  Cc: tee-dev, Volodymyr Babchuk

Add OP-TEE mediator, so guests can access OP-TEE services.

OP-TEE mediator support address translation for DomUs.
It tracks execution of STD calls, correctly handles memory-related RPC
requests, tracks buffer allocated for RPCs.

With this patch OP-TEE sucessfully passes own tests, while client is
running in DomU.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---

 Changes from "RFC":
 - Removed special case for Dom0/HwDOM
 - No more support for plain OP-TEE (only OP-TEE with virtualization
   config enabled is supported)
 - Multiple domains is now supported
 - Pages that are shared between OP-TEE and domain are now pinned
 - Renamed CONFIG_ARM_OPTEE to CONFIG_OPTEE
 - Command buffers from domain are now shadowed by XEN
 - Mediator now filters out unknown capabilities and requests
 - call contexts, shared memory object now stored per-domain

 xen/arch/arm/tee/Kconfig            |   4 +
 xen/arch/arm/tee/Makefile           |   1 +
 xen/arch/arm/tee/optee.c            | 972 ++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/tee/optee_smc.h |  50 ++
 4 files changed, 1027 insertions(+)
 create mode 100644 xen/arch/arm/tee/optee.c

diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig
index e69de29..5b829db 100644
--- a/xen/arch/arm/tee/Kconfig
+++ b/xen/arch/arm/tee/Kconfig
@@ -0,0 +1,4 @@
+config OPTEE
+	bool "Enable OP-TEE mediator"
+	default n
+	depends on TEE
diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
index c54d479..982c879 100644
--- a/xen/arch/arm/tee/Makefile
+++ b/xen/arch/arm/tee/Makefile
@@ -1 +1,2 @@
 obj-y += tee.o
+obj-$(CONFIG_OPTEE) += optee.o
diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c
new file mode 100644
index 0000000..7809406
--- /dev/null
+++ b/xen/arch/arm/tee/optee.c
@@ -0,0 +1,972 @@
+/*
+ * xen/arch/arm/tee/optee.c
+ *
+ * OP-TEE mediator
+ *
+ * Volodymyr Babchuk <volodymyr_babchuk@epam.com>
+ * Copyright (c) 2018 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/domain_page.h>
+#include <xen/types.h>
+#include <xen/sched.h>
+
+#include <asm/p2m.h>
+#include <asm/tee/tee.h>
+
+#include <asm/tee/optee_msg.h>
+#include <asm/tee/optee_smc.h>
+
+#define MAX_NONCONTIG_ENTRIES   5
+
+/* * Call context. OP-TEE can issue multiple RPC returns during one call.
+ * We need to preserve context during them.
+ */
+struct std_call_ctx {
+    struct list_head list;
+    struct optee_msg_arg *guest_arg;
+    struct optee_msg_arg *xen_arg;
+    void *non_contig[MAX_NONCONTIG_ENTRIES];
+    int non_contig_order[MAX_NONCONTIG_ENTRIES];
+    mfn_t guest_arg_mfn;
+    int optee_thread_id;
+    int rpc_op;
+};
+
+/* Pre-allocated SHM buffer for RPC commands */
+struct shm_rpc {
+    struct list_head list;
+    struct optee_msg_arg *guest_arg;
+    struct page *guest_page;
+    mfn_t guest_mfn;
+    uint64_t cookie;
+};
+
+/* Shared memory buffer for arbitrary data */
+struct shm_buf {
+    struct list_head list;
+    uint64_t cookie;
+    int page_cnt;
+    struct page_info *pages[];
+};
+
+struct domain_ctx {
+    struct list_head list;
+    struct list_head call_ctx_list;
+    struct list_head shm_rpc_list;
+    struct list_head shm_buf_list;
+    struct domain *domain;
+    spinlock_t lock;
+};
+
+static LIST_HEAD(domain_ctx_list);
+static DEFINE_SPINLOCK(domain_ctx_list_lock);
+
+static int optee_probe(void)
+{
+    struct dt_device_node *node;
+    struct smccc_res resp;
+
+    /* Check for entry in dtb  */
+    node = dt_find_compatible_node(NULL, NULL, "linaro,optee-tz");
+    if ( !node )
+        return -ENODEV;
+
+    /* Check UID */
+    call_smccc_smc(ARM_SMCCC_CALL_UID_FID(TRUSTED_OS_END), 0, 0, 0, 0, 0, 0, 0,
+                   &resp);
+
+    if ( resp.a0 != OPTEE_MSG_UID_0 ||
+         resp.a1 != OPTEE_MSG_UID_1 ||
+         resp.a2 != OPTEE_MSG_UID_2 ||
+         resp.a3 != OPTEE_MSG_UID_3 )
+        return -ENODEV;
+
+    printk("OP-TEE mediator initialized\n");
+    return 0;
+}
+
+static mfn_t lookup_and_pin_guest_ram_addr(paddr_t gaddr,
+                                            struct page_info **pg)
+{
+    mfn_t mfn;
+    gfn_t gfn;
+    p2m_type_t t;
+    struct page_info *page;
+    struct domain *d = current->domain;
+
+    gfn = gaddr_to_gfn(gaddr);
+    mfn = p2m_lookup(d, gfn, &t);
+
+    if ( t != p2m_ram_rw || mfn_eq(mfn, INVALID_MFN) )
+        return INVALID_MFN;
+
+    page = mfn_to_page(mfn);
+    if ( !page )
+        return INVALID_MFN;
+
+    if ( !get_page(page, d) )
+        return INVALID_MFN;
+
+    if ( pg )
+        *pg = page;
+
+    return mfn;
+}
+
+static void unpin_guest_ram_addr(mfn_t mfn)
+{
+    struct page_info *page;
+    page = mfn_to_page(mfn);
+    if ( !page )
+        return;
+
+    put_page(page);
+}
+
+static struct domain_ctx *find_domain_ctx(struct domain* d)
+{
+    struct domain_ctx *ctx;
+
+    spin_lock(&domain_ctx_list_lock);
+
+    list_for_each_entry( ctx, &domain_ctx_list, list )
+    {
+        if ( ctx->domain == d )
+        {
+                spin_unlock(&domain_ctx_list_lock);
+                return ctx;
+        }
+    }
+
+    spin_unlock(&domain_ctx_list_lock);
+    return NULL;
+}
+
+static int optee_domain_create(struct domain *d)
+{
+    struct smccc_res resp;
+    struct domain_ctx *ctx;
+
+    ctx = xzalloc(struct domain_ctx);
+    if ( !ctx )
+        return -ENOMEM;
+
+    call_smccc_smc(OPTEE_SMC_VM_CREATED,
+                   d->domain_id + 1, 0, 0, 0, 0, 0, 0, &resp);
+    if ( resp.a0 != OPTEE_SMC_RETURN_OK ) {
+        gprintk(XENLOG_WARNING, "OP-TEE don't want to support domain: %d\n",
+                (uint32_t)resp.a0);
+        xfree(ctx);
+        return -ENODEV;
+    }
+
+    ctx->domain = d;
+    INIT_LIST_HEAD(&ctx->call_ctx_list);
+    INIT_LIST_HEAD(&ctx->shm_rpc_list);
+    INIT_LIST_HEAD(&ctx->shm_buf_list);
+    spin_lock_init(&ctx->lock);
+
+    spin_lock(&domain_ctx_list_lock);
+    list_add_tail(&ctx->list, &domain_ctx_list);
+    spin_unlock(&domain_ctx_list_lock);
+
+    return 0;
+}
+
+static bool forward_call(struct cpu_user_regs *regs)
+{
+    struct smccc_res resp;
+
+    call_smccc_smc(get_user_reg(regs, 0),
+                   get_user_reg(regs, 1),
+                   get_user_reg(regs, 2),
+                   get_user_reg(regs, 3),
+                   get_user_reg(regs, 4),
+                   get_user_reg(regs, 5),
+                   get_user_reg(regs, 6),
+                   /* client id 0 is reserved for hypervisor itself */
+                   current->domain->domain_id + 1,
+                   &resp);
+
+    set_user_reg(regs, 0, resp.a0);
+    set_user_reg(regs, 1, resp.a1);
+    set_user_reg(regs, 2, resp.a2);
+    set_user_reg(regs, 3, resp.a3);
+    set_user_reg(regs, 4, 0);
+    set_user_reg(regs, 5, 0);
+    set_user_reg(regs, 6, 0);
+    set_user_reg(regs, 7, 0);
+
+    return resp.a0 == OPTEE_SMC_RETURN_OK;
+}
+
+static void set_return(struct cpu_user_regs *regs, uint32_t ret)
+{
+    set_user_reg(regs, 0, ret);
+    set_user_reg(regs, 1, 0);
+    set_user_reg(regs, 2, 0);
+    set_user_reg(regs, 3, 0);
+    set_user_reg(regs, 4, 0);
+    set_user_reg(regs, 5, 0);
+    set_user_reg(regs, 6, 0);
+    set_user_reg(regs, 7, 0);
+}
+
+static struct shm_buf *allocate_shm_buf(struct domain_ctx *ctx,
+                                        uint64_t cookie,
+                                        int page_cnt)
+{
+    struct shm_buf *shm_buf;
+
+    shm_buf = xzalloc_bytes(sizeof(struct shm_buf) +
+                            page_cnt * sizeof(struct page *));
+
+    if ( !shm_buf )
+        return NULL;
+
+    shm_buf->cookie = cookie;
+
+    spin_lock(&ctx->lock);
+    list_add_tail(&shm_buf->list, &ctx->shm_buf_list);
+    spin_unlock(&ctx->lock);
+
+    return shm_buf;
+}
+
+static void free_shm_buf(struct domain_ctx *ctx, uint64_t cookie)
+{
+    struct shm_buf *shm_buf;
+    bool found = false;
+    spin_lock(&ctx->lock);
+
+    list_for_each_entry( shm_buf, &ctx->shm_buf_list, list )
+    {
+        if ( shm_buf->cookie == cookie )
+        {
+            found = true;
+            list_del(&shm_buf->list);
+            break;
+        }
+    }
+    spin_unlock(&ctx->lock);
+
+    if ( !found ) {
+        return;
+    }
+
+    for ( int i = 0; i < shm_buf->page_cnt; i++ )
+        if ( shm_buf->pages[i] )
+            put_page(shm_buf->pages[i]);
+
+    xfree(shm_buf);
+}
+
+static struct std_call_ctx *allocate_std_call_ctx(struct domain_ctx *ctx)
+{
+    struct std_call_ctx *call;
+
+    call = xzalloc(struct std_call_ctx);
+    if ( !call )
+        return NULL;
+
+    call->optee_thread_id = -1;
+
+    spin_lock(&ctx->lock);
+    list_add_tail(&call->list, &ctx->call_ctx_list);
+    spin_unlock(&ctx->lock);
+
+    return call;
+}
+
+static void free_std_call_ctx(struct domain_ctx *ctx, struct std_call_ctx *call)
+{
+    spin_lock(&ctx->lock);
+    list_del(&call->list);
+    spin_unlock(&ctx->lock);
+
+    if ( call->xen_arg )
+        free_xenheap_page(call->xen_arg);
+
+    if ( call->guest_arg ) {
+        unmap_domain_page_global(call->guest_arg);
+        unpin_guest_ram_addr(call->guest_arg_mfn);
+    }
+
+    for ( int i = 0; i < MAX_NONCONTIG_ENTRIES; i++ ) {
+        if ( call->non_contig[i] )
+            free_xenheap_pages(call->non_contig[i], call->non_contig_order[i]);
+    }
+
+    xfree(call);
+}
+
+static struct std_call_ctx *find_call_ctx(struct domain_ctx *ctx, int thread_id)
+{
+    struct std_call_ctx *call;
+
+    spin_lock(&ctx->lock);
+    list_for_each_entry( call, &ctx->call_ctx_list, list )
+    {
+        if ( call->optee_thread_id == thread_id )
+        {
+                spin_unlock(&ctx->lock);
+                return call;
+        }
+    }
+    spin_unlock(&ctx->lock);
+
+    return NULL;
+}
+
+#define PAGELIST_ENTRIES_PER_PAGE                       \
+    ((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(u64)) - 1)
+
+static size_t get_pages_list_size(size_t num_entries)
+{
+    int pages = DIV_ROUND_UP(num_entries, PAGELIST_ENTRIES_PER_PAGE);
+
+    return pages * OPTEE_MSG_NONCONTIG_PAGE_SIZE;
+}
+
+static struct shm_rpc *allocate_and_map_shm_rpc(struct domain_ctx *ctx, paddr_t gaddr,
+                                        uint64_t cookie)
+{
+    struct shm_rpc *shm_rpc;
+
+    shm_rpc = xzalloc(struct shm_rpc);
+    if ( !shm_rpc )
+        return NULL;
+
+    shm_rpc->guest_mfn = lookup_and_pin_guest_ram_addr(gaddr, NULL);
+
+    if ( mfn_eq(shm_rpc->guest_mfn, INVALID_MFN) )
+    {
+        xfree(shm_rpc);
+        return NULL;
+    }
+
+    shm_rpc->guest_arg = map_domain_page_global(shm_rpc->guest_mfn);
+    if ( !shm_rpc->guest_arg )
+    {
+        gprintk(XENLOG_INFO, "Could not map domain page\n");
+        xfree(shm_rpc);
+        return NULL;
+    }
+    shm_rpc->cookie = cookie;
+
+    spin_lock(&ctx->lock);
+    list_add_tail(&shm_rpc->list, &ctx->shm_rpc_list);
+    spin_unlock(&ctx->lock);
+
+    return shm_rpc;
+}
+
+static void free_shm_rpc(struct domain_ctx *ctx, uint64_t cookie)
+{
+    struct shm_rpc *shm_rpc;
+    bool found = false;
+
+    spin_lock(&ctx->lock);
+
+    list_for_each_entry( shm_rpc, &ctx->shm_rpc_list, list )
+    {
+        if ( shm_rpc->cookie == cookie )
+        {
+            found = true;
+            list_del(&shm_rpc->list);
+            break;
+        }
+    }
+    spin_unlock(&ctx->lock);
+
+    if ( !found ) {
+        return;
+    }
+
+    if ( shm_rpc->guest_arg ) {
+        unpin_guest_ram_addr(shm_rpc->guest_mfn);
+        unmap_domain_page_global(shm_rpc->guest_arg);
+    }
+
+    xfree(shm_rpc);
+}
+
+static void optee_domain_destroy(struct domain *d)
+{
+    struct smccc_res resp;
+    struct domain_ctx *ctx;
+    struct std_call_ctx *call, *call_tmp;
+    struct shm_rpc *shm_rpc, *shm_rpc_tmp;
+    struct shm_buf *shm_buf, *shm_buf_tmp;
+    bool found = false;
+
+    /* At this time all domain VCPUs should be stopped */
+
+    /* Inform OP-TEE that domain is shutting down */
+    call_smccc_smc(OPTEE_SMC_VM_DESTROYED,
+                   d->domain_id + 1, 0, 0, 0, 0, 0, 0, &resp);
+
+    /* Remove context from the list */
+    spin_lock(&domain_ctx_list_lock);
+    list_for_each_entry( ctx, &domain_ctx_list, list )
+    {
+        if ( ctx->domain == d )
+        {
+            found = true;
+            list_del(&ctx->list);
+            break;
+        }
+    }
+    spin_unlock(&domain_ctx_list_lock);
+
+    if ( !found )
+        return;
+
+    ASSERT(!spin_is_locked(&ctx->lock));
+
+    list_for_each_entry_safe( call, call_tmp, &ctx->call_ctx_list, list )
+        free_std_call_ctx(ctx, call);
+
+    list_for_each_entry_safe( shm_rpc, shm_rpc_tmp, &ctx->shm_rpc_list, list )
+        free_shm_rpc(ctx, shm_rpc->cookie);
+
+    list_for_each_entry_safe( shm_buf, shm_buf_tmp, &ctx->shm_buf_list, list )
+        free_shm_buf(ctx, shm_buf->cookie);
+
+    xfree(ctx);
+}
+
+static struct shm_rpc *find_shm_rpc(struct domain_ctx *ctx, uint64_t cookie)
+{
+    struct shm_rpc *shm_rpc;
+
+    spin_lock(&ctx->lock);
+    list_for_each_entry( shm_rpc, &ctx->shm_rpc_list, list )
+    {
+        if ( shm_rpc->cookie == cookie )
+        {
+                spin_unlock(&ctx->lock);
+                return shm_rpc;
+        }
+    }
+    spin_unlock(&ctx->lock);
+
+    return NULL;
+}
+
+static bool translate_noncontig(struct domain_ctx *ctx,
+                                struct std_call_ctx *call,
+                                struct optee_msg_param *param,
+                                int idx)
+{
+    /*
+     * Refer to OPTEE_MSG_ATTR_NONCONTIG description in optee_msg.h for details.
+     */
+    uint64_t size;
+    int page_offset;
+    int num_pages;
+    int order;
+    int entries_on_page = 0;
+    paddr_t gaddr;
+    mfn_t guest_mfn;
+    struct {
+        uint64_t pages_list[PAGELIST_ENTRIES_PER_PAGE];
+        uint64_t next_page_data;
+    } *pages_data_guest, *pages_data_xen, *pages_data_xen_start;
+    struct shm_buf *shm_buf;
+
+    page_offset = param->u.tmem.buf_ptr & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1);
+
+    size = ROUNDUP(param->u.tmem.size + page_offset,
+                   OPTEE_MSG_NONCONTIG_PAGE_SIZE);
+
+    num_pages = DIV_ROUND_UP(size, OPTEE_MSG_NONCONTIG_PAGE_SIZE);
+
+    order = get_order_from_bytes(get_pages_list_size(num_pages));
+
+    pages_data_xen_start = alloc_xenheap_pages(order, 0);
+    if ( !pages_data_xen_start )
+        return false;
+
+    shm_buf = allocate_shm_buf(ctx, param->u.tmem.shm_ref, num_pages);
+    if ( !shm_buf )
+        goto err_free;
+
+    gaddr = param->u.tmem.buf_ptr & ~(OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1);
+    guest_mfn = lookup_and_pin_guest_ram_addr(gaddr, NULL);
+    if ( mfn_eq(guest_mfn, INVALID_MFN) )
+        goto err_free;
+
+    pages_data_guest = map_domain_page(guest_mfn);
+    if ( !pages_data_guest )
+        goto err_free;
+
+    pages_data_xen = pages_data_xen_start;
+    while ( num_pages ) {
+        struct page_info *page;
+        mfn_t entry_mfn = lookup_and_pin_guest_ram_addr(
+            pages_data_guest->pages_list[entries_on_page], &page);
+
+        if ( mfn_eq(entry_mfn, INVALID_MFN) )
+            goto err_unmap;
+
+        shm_buf->pages[shm_buf->page_cnt++] = page;
+        pages_data_xen->pages_list[entries_on_page] = mfn_to_maddr(entry_mfn);
+        entries_on_page++;
+
+        if ( entries_on_page == PAGELIST_ENTRIES_PER_PAGE ) {
+            pages_data_xen->next_page_data = virt_to_maddr(pages_data_xen + 1);
+            pages_data_xen++;
+            gaddr = pages_data_guest->next_page_data;
+
+            unmap_domain_page(pages_data_guest);
+            unpin_guest_ram_addr(guest_mfn);
+
+            guest_mfn = lookup_and_pin_guest_ram_addr(gaddr, NULL);
+            if ( mfn_eq(guest_mfn, INVALID_MFN) )
+                goto err_free;
+
+            pages_data_guest = map_domain_page(guest_mfn);
+            if ( !pages_data_guest )
+                goto err_free;
+            /* Roll over to the next page */
+            entries_on_page = 0;
+        }
+        num_pages--;
+    }
+
+    param->u.tmem.buf_ptr = virt_to_maddr(pages_data_xen_start) | page_offset;
+
+    call->non_contig[idx] = pages_data_xen_start;
+    call->non_contig_order[idx] = order;
+
+    unmap_domain_page(pages_data_guest);
+    unpin_guest_ram_addr(guest_mfn);
+    return true;
+
+err_unmap:
+    unmap_domain_page(pages_data_guest);
+    unpin_guest_ram_addr(guest_mfn);
+    free_shm_buf(ctx, shm_buf->cookie);
+
+err_free:
+    free_xenheap_pages(pages_data_xen_start, order);
+
+    return false;
+}
+
+static bool translate_params(struct domain_ctx *ctx,
+                             struct std_call_ctx *call)
+{
+    unsigned int i;
+    uint32_t attr;
+
+    for ( i = 0; i < call->xen_arg->num_params; i++ ) {
+        attr = call->xen_arg->params[i].attr;
+
+        switch ( attr & OPTEE_MSG_ATTR_TYPE_MASK ) {
+        case OPTEE_MSG_ATTR_TYPE_TMEM_INPUT:
+        case OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT:
+        case OPTEE_MSG_ATTR_TYPE_TMEM_INOUT:
+            if ( attr & OPTEE_MSG_ATTR_NONCONTIG ) {
+                if ( !translate_noncontig(ctx, call,
+                                          call->xen_arg->params + i, i) )
+                    return false;
+            }
+            else {
+                gprintk(XENLOG_WARNING, "Guest tries to use old tmem arg\n");
+                return false;
+            }
+            break;
+        case OPTEE_MSG_ATTR_TYPE_NONE:
+        case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT:
+        case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT:
+        case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT:
+        case OPTEE_MSG_ATTR_TYPE_RMEM_INPUT:
+        case OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT:
+        case OPTEE_MSG_ATTR_TYPE_RMEM_INOUT:
+            continue;
+        }
+    }
+    return true;
+}
+
+/*
+ * Copy command buffer into xen memory to:
+ * 1) Hide translated addresses from guest
+ * 2) Make sure that guest wouldn't change data in command buffer during call
+ */
+static bool copy_std_request(struct cpu_user_regs *regs,
+                             struct std_call_ctx *call)
+{
+    paddr_t cmd_gaddr, xen_addr;
+
+    cmd_gaddr = (paddr_t)get_user_reg(regs, 1) << 32 |
+        get_user_reg(regs, 2);
+
+    /*
+     * Command buffer should start at page boundary.
+     * This is OP-TEE ABI requirement.
+     */
+    if ( cmd_gaddr & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1) )
+        return false;
+
+    call->guest_arg_mfn = lookup_and_pin_guest_ram_addr(cmd_gaddr, NULL);
+    if ( mfn_eq(call->guest_arg_mfn, INVALID_MFN) )
+        return false;
+
+    call->guest_arg = map_domain_page_global(call->guest_arg_mfn);
+    if ( !call->guest_arg ) {
+        unpin_guest_ram_addr(call->guest_arg_mfn);
+        return false;
+    }
+
+    call->xen_arg = alloc_xenheap_page();
+    if ( !call->xen_arg ) {
+        unpin_guest_ram_addr(call->guest_arg_mfn);
+        return false;
+    }
+
+    memcpy(call->xen_arg, call->guest_arg, OPTEE_MSG_NONCONTIG_PAGE_SIZE);
+
+    xen_addr = virt_to_maddr(call->xen_arg);
+
+    set_user_reg(regs, 1, xen_addr >> 32);
+    set_user_reg(regs, 2, xen_addr & 0xFFFFFFFF);
+
+    return true;
+}
+
+static bool copy_std_request_back(struct domain_ctx *ctx,
+                                  struct cpu_user_regs *regs,
+                                  struct std_call_ctx *call)
+{
+    unsigned int i;
+    uint32_t attr;
+
+    call->guest_arg->ret = call->xen_arg->ret;
+    call->guest_arg->ret_origin = call->xen_arg->ret_origin;
+    call->guest_arg->session = call->xen_arg->session;
+    for ( i = 0; i < call->xen_arg->num_params; i++ ) {
+        attr = call->xen_arg->params[i].attr;
+
+        switch ( attr & OPTEE_MSG_ATTR_TYPE_MASK ) {
+        case OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT:
+        case OPTEE_MSG_ATTR_TYPE_TMEM_INOUT:
+            call->guest_arg->params[i].u.tmem.size =
+                call->xen_arg->params[i].u.tmem.size;
+            /* fall through */
+        case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT:
+            free_shm_buf(ctx, call->xen_arg->params[i].u.tmem.shm_ref);
+            continue;
+        case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT:
+        case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT:
+            call->guest_arg->params[i].u.value.a =
+                call->xen_arg->params[i].u.value.a;
+            call->guest_arg->params[i].u.value.b =
+                call->xen_arg->params[i].u.value.b;
+            continue;
+        case OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT:
+        case OPTEE_MSG_ATTR_TYPE_RMEM_INOUT:
+            call->guest_arg->params[i].u.rmem.size =
+                call->xen_arg->params[i].u.rmem.size;
+            continue;
+        case OPTEE_MSG_ATTR_TYPE_NONE:
+        case OPTEE_MSG_ATTR_TYPE_RMEM_INPUT:
+        case OPTEE_MSG_ATTR_TYPE_TMEM_INPUT:
+            continue;
+        }
+    }
+
+    return true;
+}
+
+static bool execute_std_call(struct domain_ctx *ctx,
+                             struct cpu_user_regs *regs,
+                             struct std_call_ctx *call)
+{
+    register_t optee_ret;
+    forward_call(regs);
+    optee_ret = get_user_reg(regs, 0);
+
+    if ( OPTEE_SMC_RETURN_IS_RPC(optee_ret) )
+    {
+        call->optee_thread_id = get_user_reg(regs, 3);
+        call->rpc_op = OPTEE_SMC_RETURN_GET_RPC_FUNC(optee_ret);
+        return true;
+    }
+
+    copy_std_request_back(ctx, regs, call);
+
+    if ( call->xen_arg->cmd == OPTEE_MSG_CMD_UNREGISTER_SHM &&
+         call->xen_arg->ret == 0 ) {
+        free_shm_buf(ctx, call->xen_arg->params[0].u.rmem.shm_ref);
+    }
+
+    free_std_call_ctx(ctx, call);
+
+    return true;
+}
+
+static bool handle_std_call(struct domain_ctx *ctx, struct cpu_user_regs *regs)
+{
+    struct std_call_ctx *call;
+    bool ret;
+
+    call = allocate_std_call_ctx(ctx);
+
+    if (!call)
+        return false;
+
+    ret = copy_std_request(regs, call);
+    if ( !ret )
+        goto out;
+
+    /* Now we can safely examine contents of command buffer */
+    if ( OPTEE_MSG_GET_ARG_SIZE(call->xen_arg->num_params) >
+         OPTEE_MSG_NONCONTIG_PAGE_SIZE ) {
+        ret = false;
+        goto out;
+    }
+
+    switch ( call->xen_arg->cmd )
+    {
+    case OPTEE_MSG_CMD_OPEN_SESSION:
+    case OPTEE_MSG_CMD_CLOSE_SESSION:
+    case OPTEE_MSG_CMD_INVOKE_COMMAND:
+    case OPTEE_MSG_CMD_CANCEL:
+    case OPTEE_MSG_CMD_REGISTER_SHM:
+    case OPTEE_MSG_CMD_UNREGISTER_SHM:
+        ret = translate_params(ctx, call);
+        break;
+    default:
+        ret = false;
+    }
+
+    if (!ret)
+        goto out;
+
+    ret = execute_std_call(ctx, regs, call);
+
+out:
+    if (!ret)
+        free_std_call_ctx(ctx, call);
+
+    return ret;
+}
+
+static void handle_rpc_cmd_alloc(struct domain_ctx *ctx,
+                                 struct cpu_user_regs *regs,
+                                 struct std_call_ctx *call,
+                                 struct shm_rpc *shm_rpc)
+{
+    if ( shm_rpc->guest_arg->params[0].attr != (OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT |
+                                            OPTEE_MSG_ATTR_NONCONTIG) )
+    {
+        gprintk(XENLOG_WARNING, "Invalid attrs for shared mem buffer\n");
+        return;
+    }
+
+    /* Last entry in non_contig array is used to hold RPC-allocated buffer */
+    if ( call->non_contig[MAX_NONCONTIG_ENTRIES - 1] )
+    {
+        free_xenheap_pages(call->non_contig[MAX_NONCONTIG_ENTRIES - 1],
+                           call->non_contig_order[MAX_NONCONTIG_ENTRIES - 1]);
+        call->non_contig[MAX_NONCONTIG_ENTRIES - 1] = NULL;
+    }
+    translate_noncontig(ctx, call, shm_rpc->guest_arg->params + 0,
+                        MAX_NONCONTIG_ENTRIES - 1);
+}
+
+static void handle_rpc_cmd(struct domain_ctx *ctx, struct cpu_user_regs *regs,
+                           struct std_call_ctx *call)
+{
+    struct shm_rpc *shm_rpc;
+    uint64_t cookie;
+
+    cookie = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2);
+
+    shm_rpc = find_shm_rpc(ctx, cookie);
+
+    if ( !shm_rpc )
+    {
+        gprintk(XENLOG_ERR, "Can't find SHM-RPC with cookie %lx\n", cookie);
+        return;
+    }
+
+    switch (shm_rpc->guest_arg->cmd) {
+    case OPTEE_MSG_RPC_CMD_GET_TIME:
+        break;
+    case OPTEE_MSG_RPC_CMD_WAIT_QUEUE:
+        break;
+    case OPTEE_MSG_RPC_CMD_SUSPEND:
+        break;
+    case OPTEE_MSG_RPC_CMD_SHM_ALLOC:
+        handle_rpc_cmd_alloc(ctx, regs, call, shm_rpc);
+        break;
+    case OPTEE_MSG_RPC_CMD_SHM_FREE:
+        free_shm_buf(ctx, shm_rpc->guest_arg->params[0].u.value.b);
+        break;
+    default:
+        break;
+    }
+}
+
+static void handle_rpc_func_alloc(struct domain_ctx *ctx,
+                                  struct cpu_user_regs *regs,
+                                  struct std_call_ctx *call)
+{
+    paddr_t ptr = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2);
+
+    if ( ptr & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1) )
+        gprintk(XENLOG_WARNING, "Domain returned invalid RPC command buffer\n");
+
+    if ( ptr ) {
+        uint64_t cookie = get_user_reg(regs, 4) << 32 | get_user_reg(regs, 5);
+        struct shm_rpc *shm_rpc;
+
+        shm_rpc = allocate_and_map_shm_rpc(ctx, ptr, cookie);
+        if ( !shm_rpc )
+        {
+            gprintk(XENLOG_WARNING, "Failed to allocate shm_rpc object\n");
+            ptr = 0;
+        }
+        else
+            ptr = mfn_to_maddr(shm_rpc->guest_mfn);
+
+        set_user_reg(regs, 1, ptr >> 32);
+        set_user_reg(regs, 2, ptr & 0xFFFFFFFF);
+    }
+}
+
+static bool handle_rpc(struct domain_ctx *ctx, struct cpu_user_regs *regs)
+{
+    struct std_call_ctx *call;
+
+    int optee_thread_id = get_user_reg(regs, 3);
+
+    call = find_call_ctx(ctx, optee_thread_id);
+
+    if ( !call )
+        return false;
+
+    switch ( call->rpc_op ) {
+    case OPTEE_SMC_RPC_FUNC_ALLOC:
+        handle_rpc_func_alloc(ctx, regs, call);
+        break;
+    case OPTEE_SMC_RPC_FUNC_FREE:
+    {
+        uint64_t cookie = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2);
+        free_shm_rpc(ctx, cookie);
+        break;
+    }
+    case OPTEE_SMC_RPC_FUNC_FOREIGN_INTR:
+        break;
+    case OPTEE_SMC_RPC_FUNC_CMD:
+        handle_rpc_cmd(ctx, regs, call);
+        break;
+    }
+
+    return execute_std_call(ctx, regs, call);
+}
+
+static bool handle_exchange_capabilities(struct cpu_user_regs *regs)
+{
+    uint32_t caps;
+
+    /* Filter out unknown guest caps */
+    caps = get_user_reg(regs, 1);
+    caps &= OPTEE_SMC_NSEC_CAP_UNIPROCESSOR;
+    set_user_reg(regs, 1, caps);
+
+    /* Forward call and return error (if any) back to the guest */
+    if ( !forward_call(regs) )
+        return true;
+
+    caps = get_user_reg(regs, 1);
+
+    /* Filter out unknown OP-TEE caps */
+    caps &= OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM |
+        OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM |
+        OPTEE_SMC_SEC_CAP_DYNAMIC_SHM;
+
+    /* Drop static SHM_RPC cap */
+    caps &= ~OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM;
+
+    /* Don't allow guests to work without dynamic SHM */
+    if ( !(caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM) ) {
+        set_return(regs, OPTEE_SMC_RETURN_ENOTAVAIL);
+        return true;
+    }
+
+    set_user_reg(regs, 1, caps);
+    return true;
+}
+
+static bool optee_handle_smc(struct cpu_user_regs *regs)
+{
+    struct domain_ctx *ctx;
+
+    ctx = find_domain_ctx(current->domain);
+    if ( !ctx )
+        return false;
+
+    switch ( get_user_reg(regs, 0) )
+    {
+    case OPTEE_SMC_CALLS_COUNT:
+    case OPTEE_SMC_CALLS_UID:
+    case OPTEE_SMC_CALLS_REVISION:
+    case OPTEE_SMC_CALL_GET_OS_UUID:
+    case OPTEE_SMC_FUNCID_GET_OS_REVISION:
+    case OPTEE_SMC_ENABLE_SHM_CACHE:
+    case OPTEE_SMC_DISABLE_SHM_CACHE:
+        forward_call(regs);
+        return true;
+    case OPTEE_SMC_GET_SHM_CONFIG:
+        /* No static SHM available for guests */
+        set_return(regs, OPTEE_SMC_RETURN_ENOTAVAIL);
+        return true;
+    case OPTEE_SMC_EXCHANGE_CAPABILITIES:
+        return handle_exchange_capabilities(regs);
+    case OPTEE_SMC_CALL_WITH_ARG:
+        return handle_std_call(ctx, regs);
+    case OPTEE_SMC_CALL_RETURN_FROM_RPC:
+        return handle_rpc(ctx, regs);
+    default:
+        return false;
+    }
+}
+
+static void optee_remove(void)
+{
+}
+
+static const struct tee_mediator_ops optee_ops =
+{
+    .probe = optee_probe,
+    .domain_create = optee_domain_create,
+    .domain_destroy = optee_domain_destroy,
+    .handle_smc = optee_handle_smc,
+    .remove = optee_remove,
+};
+
+REGISTER_TEE_MEDIATOR(optee, "OP-TEE", &optee_ops);
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/tee/optee_smc.h b/xen/include/asm-arm/tee/optee_smc.h
index 26d100e..1c5a247 100644
--- a/xen/include/asm-arm/tee/optee_smc.h
+++ b/xen/include/asm-arm/tee/optee_smc.h
@@ -305,6 +305,56 @@ struct optee_smc_disable_shm_cache_result {
 	OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_ENABLE_SHM_CACHE)
 
 /*
+ * Inform OP-TEE about a new virtual machine
+ *
+ * Hypervisor issues this call during virtual machine (guest) creation.
+ * OP-TEE records VM_ID of new virtual machine and makes self ready
+ * to receive requests from it.
+ *
+ * Call requests usage:
+ * a0	SMC Function ID, OPTEE_SMC_VM_CREATED
+ * a1	VM_ID of newly created virtual machine
+ * a2-6 Not used
+ * a7	Hypervisor Client ID register. Must be 0, because only hypervisor
+ *      can issue this call
+ *
+ * Normal return register usage:
+ * a0	OPTEE_SMC_RETURN_OK
+ * a1-7	Preserved
+ *
+ * Error return:
+ * a0	OPTEE_SMC_RETURN_ENOTAVAIL	OP-TEE has no resources for
+ *					another VM
+ * a1-7	Preserved
+ *
+ */
+#define OPTEE_SMC_FUNCID_VM_CREATED	13
+#define OPTEE_SMC_VM_CREATED \
+	OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_VM_CREATED)
+
+/*
+ * Inform OP-TEE about shutdown of a virtual machine
+ *
+ * Hypervisor issues this call during virtual machine (guest) destruction.
+ * OP-TEE will clean up all resources associated with this VM.
+ *
+ * Call requests usage:
+ * a0	SMC Function ID, OPTEE_SMC_VM_DESTROYED
+ * a1	VM_ID of virtual machine being shutted down
+ * a2-6 Not used
+ * a7	Hypervisor Client ID register. Must be 0, because only hypervisor
+ *      can issue this call
+ *
+ * Normal return register usage:
+ * a0	OPTEE_SMC_RETURN_OK
+ * a1-7	Preserved
+ *
+ */
+#define OPTEE_SMC_FUNCID_VM_DESTROYED	14
+#define OPTEE_SMC_VM_DESTROYED \
+	OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_VM_DESTROYED)
+
+/*
  * Resume from RPC (for example after processing a foreign interrupt)
  *
  * Call register usage:
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v1 5/6] libxl: create DTS node for OP-TEE if it is enabled
  2018-08-22 14:11 [PATCH v1 0/6] TEE mediator (and OP-TEE) support in XEN Volodymyr Babchuk
                   ` (3 preceding siblings ...)
  2018-08-22 14:11 ` [PATCH v1 4/6] optee: add OP-TEE mediator Volodymyr Babchuk
@ 2018-08-22 14:11 ` Volodymyr Babchuk
  2018-08-22 17:03   ` Wei Liu
  2018-08-22 17:32   ` Julien Grall
  2018-08-22 14:11 ` [PATCH v1 6/6] xsm: add tee access policy support Volodymyr Babchuk
  5 siblings, 2 replies; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-22 14:11 UTC (permalink / raw)
  To: xen-devel, Ian Jackson, Wei Liu, Julien Grall, Stefano Stabellini
  Cc: tee-dev, Volodymyr Babchuk

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---
 tools/libxl/libxl_arm.c     | 29 +++++++++++++++++++++++++++++
 tools/libxl/libxl_create.c  |  1 +
 tools/libxl/libxl_types.idl |  1 +
 tools/xl/xl_parse.c         |  1 +
 4 files changed, 32 insertions(+)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 86f59c0..453ad36 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -429,6 +429,32 @@ static int make_psci_node(libxl__gc *gc, void *fdt)
     return 0;
 }
 
+static int make_optee_node(libxl__gc *gc, void *fdt)
+{
+    int res;
+    LOG(DEBUG, "Creating OP-TEE node in dtb");
+
+    res = fdt_begin_node(fdt, "firmware");
+    if (res) return res;
+
+    res = fdt_begin_node(fdt, "optee");
+    if (res) return res;
+
+    res = fdt_property_compat(gc, fdt, 1, "linaro,optee-tz");
+    if (res) return res;
+
+    res = fdt_property_string(fdt, "method", "smc");
+    if (res) return res;
+
+    res = fdt_end_node(fdt);
+    if (res) return res;
+
+    res = fdt_end_node(fdt);
+    if (res) return res;
+
+    return 0;
+}
+
 static int make_memory_nodes(libxl__gc *gc, void *fdt,
                              const struct xc_dom_image *dom)
 {
@@ -952,6 +978,9 @@ next_resize:
         if (info->arch_arm.vuart == LIBXL_VUART_TYPE_SBSA_UART)
             FDT( make_vpl011_uart_node(gc, fdt, ainfo, dom) );
 
+        if (libxl_defbool_val(info->optee))
+            FDT( make_optee_node(gc, fdt));
+
         if (pfdt)
             FDT( copy_partial_fdt(gc, fdt, pfdt) );
 
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 1fa1d3a..9bab4cc 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -217,6 +217,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
 
     libxl__arch_domain_build_info_acpi_setdefault(b_info);
     libxl_defbool_setdefault(&b_info->dm_restrict, false);
+    libxl_defbool_setdefault(&b_info->optee, false);
 
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index d74fac7..5f4be2e 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -607,6 +607,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     # Alternate p2m is not bound to any architecture or guest type, as it is
     # supported by x86 HVM and ARM support is planned.
     ("altp2m", libxl_altp2m_mode),
+    ("optee",  libxl_defbool),
 
     ], dir=DIR_IN,
        copy_deprecated_fn="libxl__domain_build_info_copy_deprecated",
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index fdfe693..92beeb0 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2160,6 +2160,7 @@ skip_usbdev:
     }
 
     xlu_cfg_get_defbool(config, "dm_restrict", &b_info->dm_restrict, 0);
+    xlu_cfg_get_defbool(config, "optee", &b_info->optee, 0);
 
     if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
         if (!xlu_cfg_get_string (config, "vga", &buf, 0)) {
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v1 6/6] xsm: add tee access policy support
  2018-08-22 14:11 [PATCH v1 0/6] TEE mediator (and OP-TEE) support in XEN Volodymyr Babchuk
                   ` (4 preceding siblings ...)
  2018-08-22 14:11 ` [PATCH v1 5/6] libxl: create DTS node for OP-TEE if it is enabled Volodymyr Babchuk
@ 2018-08-22 14:11 ` Volodymyr Babchuk
  2018-08-23 13:43   ` Julien Grall
  5 siblings, 1 reply; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-22 14:11 UTC (permalink / raw)
  To: xen-devel, Andrew Cooper, George Dunlap, Ian Jackson,
	Jan Beulich, Konrad Rzeszutek Wilk, Stefano Stabellini,
	Tim Deegan, Wei Liu, Daniel De Graaf, Julien Grall
  Cc: Volodymyr Babchuk

As we don't want any guest to access limited resources of TEE,
we need a way to control who can work with it.

Thus, new access vector class "tee" is added with only ony operation
"call" so far. tee framework uses this to check if guest has a right
to work with TEE.

Also, example security context domU_with_tee_t was added.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---
 tools/flask/policy/modules/dom0.te          |  3 +++
 tools/flask/policy/modules/domU_with_tee.te | 23 +++++++++++++++++++++++
 tools/flask/policy/modules/modules.conf     |  1 +
 tools/flask/policy/modules/xen.if           | 12 ++++++++++++
 xen/arch/arm/tee/tee.c                      | 10 ++++++++++
 xen/include/xsm/dummy.h                     | 10 ++++++++++
 xen/include/xsm/xsm.h                       | 13 +++++++++++++
 xen/xsm/dummy.c                             |  4 ++++
 xen/xsm/flask/hooks.c                       | 15 +++++++++++++++
 xen/xsm/flask/policy/access_vectors         |  7 +++++++
 xen/xsm/flask/policy/security_classes       |  1 +
 11 files changed, 99 insertions(+)
 create mode 100644 tools/flask/policy/modules/domU_with_tee.te

diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te
index 1643b40..8bac4f3 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -27,6 +27,9 @@ allow dom0_t xen_t:version {
 	xen_build_id
 };
 
+# Allow dom0 to work with TEE
+allow dom0_t xen_t:tee call;
+
 allow dom0_t xen_t:mmu memorymap;
 
 # Allow dom0 to use these domctls on itself. For domctls acting on other
diff --git a/tools/flask/policy/modules/domU_with_tee.te b/tools/flask/policy/modules/domU_with_tee.te
new file mode 100644
index 0000000..4d856b2
--- /dev/null
+++ b/tools/flask/policy/modules/domU_with_tee.te
@@ -0,0 +1,23 @@
+###############################################################################
+#
+# Domain creation
+#
+###############################################################################
+
+declare_domain(domU_with_tee_t)
+domain_self_comms(domU_t)
+create_domain(dom0_t, domU_with_tee_t)
+manage_domain(dom0_t, domU_with_tee_t)
+domain_comms(dom0_t, domU_with_tee_t)
+domain_comms(domU_with_tee_t, domU_with_tee_t)
+migrate_domain_out(dom0_t, domU_with_tee_t)
+domain_self_comms(domU_with_tee_t)
+
+# This is required for PCI (or other device) passthrough
+delegate_devices(dom0_t, domU_with_tee_t)
+
+# Both of these domain types can be created using the default (system) role
+role system_r types { domU_with_tee_t dm_dom_t };
+
+# Allow to work with TEE
+access_tee(domU_with_tee_t)
diff --git a/tools/flask/policy/modules/modules.conf b/tools/flask/policy/modules/modules.conf
index 6dba0a3..9010d91 100644
--- a/tools/flask/policy/modules/modules.conf
+++ b/tools/flask/policy/modules/modules.conf
@@ -29,6 +29,7 @@ domU = on
 isolated_domU = on
 prot_domU = on
 nomigrate = on
+domU_with_tee = on
 
 # Example device policy.  Also see policy/device_contexts.
 nic_dev = on
diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if
index 5543749..e534179 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -209,3 +209,15 @@ define(`admin_device', `
 define(`delegate_devices', `
     allow $1 $2:resource { add remove };
 ')
+
+################################################################################
+#
+# Miscellaneous services
+#
+################################################################################
+
+# access_tee(domain)
+
+define(`access_tee', `
+    allow $1 xen_t:tee call;
+')
diff --git a/xen/arch/arm/tee/tee.c b/xen/arch/arm/tee/tee.c
index 6df3b09..06f5091 100644
--- a/xen/arch/arm/tee/tee.c
+++ b/xen/arch/arm/tee/tee.c
@@ -19,6 +19,7 @@
 #include <xen/errno.h>
 #include <xen/types.h>
 #include <asm/tee/tee.h>
+#include <xsm/xsm.h>
 
 extern const struct tee_mediator_desc _steemediator[], _eteemediator[];
 static const struct tee_mediator_ops *mediator_ops;
@@ -42,6 +43,9 @@ bool tee_handle_smc(struct cpu_user_regs *regs)
     if ( !mediator_ops )
         return false;
 
+    if ( xsm_tee_op(XSM_PRIV, current->domain) )
+        return false;
+
     return mediator_ops->handle_smc(regs);
 }
 
@@ -50,6 +54,9 @@ int tee_domain_create(struct domain *d)
     if ( !mediator_ops )
         return -ENODEV;
 
+    if ( xsm_tee_op(XSM_PRIV, d) )
+        return -EPERM;
+
     return mediator_ops->domain_create(d);
 }
 
@@ -58,6 +65,9 @@ void tee_domain_destroy(struct domain *d)
     if ( !mediator_ops )
         return;
 
+    if ( xsm_tee_op(XSM_PRIV, d) )
+        return;
+
     return mediator_ops->domain_destroy(d);
 }
 
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index b2cd56c..b8e2fa0 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -724,3 +724,13 @@ static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG uint32_t op)
         return xsm_default_action(XSM_PRIV, current->domain, NULL);
     }
 }
+
+#ifdef CONFIG_TEE
+
+static XSM_INLINE int xsm_tee_op (XSM_DEFAULT_ARG struct domain *d)
+{
+	XSM_ASSERT_ACTION(XSM_PRIV);
+	return 0;
+}
+
+#endif	/* CONFIG_TEE */
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 7f7feff..aa57831 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -180,6 +180,10 @@ struct xsm_operations {
     int (*dm_op) (struct domain *d);
 #endif
     int (*xen_version) (uint32_t cmd);
+
+#ifdef CONFIG_TEE
+    int (*tee_op) (struct domain *d);
+#endif
 };
 
 #ifdef CONFIG_XSM
@@ -692,6 +696,15 @@ static inline int xsm_xen_version (xsm_default_t def, uint32_t op)
     return xsm_ops->xen_version(op);
 }
 
+#ifdef CONFIG_TEE
+
+static inline int xsm_tee_op (xsm_default_t def, struct domain *d)
+{
+	return xsm_ops->tee_op(d);
+}
+
+#endif	/* CONFIG_ARM_TEE */
+
 #endif /* XSM_NO_WRAPPERS */
 
 #ifdef CONFIG_MULTIBOOT
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 479b103..2ef56dd 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -157,4 +157,8 @@ void __init xsm_fixup_ops (struct xsm_operations *ops)
     set_to_dummy_if_null(ops, dm_op);
 #endif
     set_to_dummy_if_null(ops, xen_version);
+
+#ifdef CONFIG_TEE
+    set_to_dummy_if_null(ops, tee_op);
+#endif
 }
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index f01b4cf..73d9c1f 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1718,6 +1718,17 @@ static int flask_xen_version (uint32_t op)
     }
 }
 
+#ifdef CONFIG_TEE
+
+static int flask_tee_op(struct domain *d)
+{
+    u32 dsid = domain_sid(d);
+
+    return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_TEE, TEE__CALL, NULL);
+}
+
+#endif  /* CONFIG_TEE */
+
 long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
 int compat_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
 
@@ -1851,6 +1862,10 @@ static struct xsm_operations flask_ops = {
     .dm_op = flask_dm_op,
 #endif
     .xen_version = flask_xen_version,
+
+#ifdef CONFIG_TEE
+    .tee_op = flask_tee_op,
+#endif
 };
 
 void __init flask_init(const void *policy_buffer, size_t policy_size)
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index 3a2d863..29e20f1 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -535,3 +535,10 @@ class version
 # Xen build id
     xen_build_id
 }
+
+# Class TEE is used to contol access to Trusted Execution Environments
+# on ARM platforms
+class tee
+{
+    call
+}
diff --git a/xen/xsm/flask/policy/security_classes b/xen/xsm/flask/policy/security_classes
index cde4e1a..d4c9482 100644
--- a/xen/xsm/flask/policy/security_classes
+++ b/xen/xsm/flask/policy/security_classes
@@ -19,5 +19,6 @@ class event
 class grant
 class security
 class version
+class tee
 
 # FLASK
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-22 14:11 ` [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC Volodymyr Babchuk
@ 2018-08-22 16:46   ` Julien Grall
  2018-08-23 14:35     ` Volodymyr Babchuk
  2018-08-30 14:48     ` Volodymyr Babchuk
  2018-08-27  6:44   ` Jan Beulich
  1 sibling, 2 replies; 32+ messages in thread
From: Julien Grall @ 2018-08-22 16:46 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu
  Cc: tee-dev, Edgar E. Iglesias

Hi Volodymyr,

On 22/08/18 15:11, Volodymyr Babchuk wrote:
> Existing SMC wrapper call_smc() allows only 4 parameters and
> returns only one value. This is enough for existing
> use in PSCI code, but TEE mediator will need a call that is
> fully compatible with ARM SMCCC.
> This patch adds this call for both arm32 and arm64.
> 
> There was similar patch by Edgar E. Iglesias ([1]), but looks
> like it is abandoned.
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00636.html
> 
> CC: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> ---
> 
> changes from "RFC":
>    - response now stored in structure instead of array
>    - added comments for arm32 assembly code
>    - added offset (instead of magic values) for arm32 asm code

Did you mean arm64?

> 
>   xen/arch/arm/arm32/Makefile      |  1 +
>   xen/arch/arm/arm32/smc.S         | 39 +++++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/arm64/Makefile      |  1 +
>   xen/arch/arm/arm64/asm-offsets.c |  4 ++++
>   xen/arch/arm/arm64/smc.S         | 30 ++++++++++++++++++++++++++++++
>   xen/include/asm-arm/processor.h  | 11 +++++++++++
>   6 files changed, 86 insertions(+)
>   create mode 100644 xen/arch/arm/arm32/smc.S
>   create mode 100644 xen/arch/arm/arm64/smc.S
> 
> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
> index 0ac254f..c69f35e 100644
> --- a/xen/arch/arm/arm32/Makefile
> +++ b/xen/arch/arm/arm32/Makefile
> @@ -10,4 +10,5 @@ obj-y += proc-v7.o proc-caxx.o
>   obj-y += smpboot.o
>   obj-y += traps.o
>   obj-y += vfp.o
> +obj-y += smc.o
>   
> diff --git a/xen/arch/arm/arm32/smc.S b/xen/arch/arm/arm32/smc.S
> new file mode 100644
> index 0000000..56f7982
> --- /dev/null
> +++ b/xen/arch/arm/arm32/smc.S
> @@ -0,0 +1,39 @@
> +/*
> + * xen/arch/arm/arm32/smc.S
> + *
> + * Wrapper for Secure Monitors Calls
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.

Xen is GPLv2 only. Can you please update the copyright accordingly?

Also, the code is pretty much a copy of the Linux part. So it is 
probably worth to keep the copy-right around.

> + *
> + * 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 <asm/macros.h>
> +
> +/*
> + * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
> + *                     register_t a3, register_t a4, register_t a5,
> + *                     register_t a6, register_t a7, struct smccc_res *res)
> + */
> +ENTRY(call_smccc_smc)

I would rather avoid to have 2 function to handle the same convention. 
Can you please either drop call_smc or rework it to handle more parameters?

But it looks like you could re-use arm_smccc_1_1 as it follows the 1.0 
convention for 32-bits.

> +        mov     r12, sp
> +        push    {r4-r7}
> +	/*
> +	 * According to ARMv7 (or aarch32) ABI, first 4 parameters of
> +	 * call_smccc_smc are passed in r0-r3 and other 4 are on stack.
> +	 * Load a4-a7 from stack to r4-r7.
> +	 */
> +        ldm     r12, {r4-r7}
> +        smc     #0
> +        pop     {r4-r7}
> +	/* Load pointer to res (4th parameter on stack) to r12 */
> +        ldr     r12, [sp, #(4 * 4)]
> +	/* Store returned results from r0-r3 to res */
> +        stm     r12, {r0-r3}
> +        bx      lr

Something looks wrong with the indentation. There seem to be a mix of 
hard tab and soft tab.

> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> index bb5c610..7ecdad7 100644
> --- a/xen/arch/arm/arm64/Makefile
> +++ b/xen/arch/arm/arm64/Makefile
> @@ -9,6 +9,7 @@ obj-y += entry.o
>   obj-y += insn.o
>   obj-$(CONFIG_LIVEPATCH) += livepatch.o
>   obj-y += smpboot.o
> +obj-y += smc.o
>   obj-y += traps.o
>   obj-y += vfp.o
>   obj-y += vsysreg.o
> diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
> index ce24e44..5353fe8 100644
> --- a/xen/arch/arm/arm64/asm-offsets.c
> +++ b/xen/arch/arm/arm64/asm-offsets.c
> @@ -50,6 +50,10 @@ void __dummy__(void)
>   
>      BLANK();
>      OFFSET(INITINFO_stack, struct init_info, stack);
> +
> +   BLANK();
> +   OFFSET(SMCCC_RES_a0, struct smccc_res, a0);
> +   OFFSET(SMCCC_RES_a2, struct smccc_res, a2);
>   }
>   
>   /*
> diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
> new file mode 100644
> index 0000000..40843c0
> --- /dev/null
> +++ b/xen/arch/arm/arm64/smc.S
> @@ -0,0 +1,30 @@
> +/*
> + * xen/arch/arm/arm64/smc.S
> + *
> + * Wrapper for Secure Monitors Calls
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.

Xen is GPLv2 only. Can you please update the copyright accordingly?

Also, the code is pretty much a copy of the Linux part. So it is 
probably worth to keep the copy-right around.

> + *
> + * 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 <asm/asm_defns.h>
> +#include <asm/macros.h>
> +
> +/*
> + * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
> + *                     register_t a3, register_t a4, register_t a5,
> + *                     register_t a6, register_t a7, struct smccc_res *res)
> + */
> +ENTRY(call_smccc_smc)
> +        smc     #0
> +        ldr     x4, [sp]
> +        stp     x0, x1, [x4, #SMCCC_RES_a0]
> +        stp     x2, x3, [x4, #SMCCC_RES_a2]
> +        ret
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 222a02d..946a2db 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -650,6 +650,13 @@ union hsr {
>   
>   
>   };
> +
> +struct smccc_res {
> +    register_t a0;
> +    register_t a1;
> +    register_t a2;
> +    register_t a3;
> +};

Any reason to not use arm_smccc_res?

>   #endif
>   
>   /* HSR.EC == HSR_CP{15,14,10}_32 */
> @@ -815,6 +822,10 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
>   int call_smc(register_t function_id, register_t arg0, register_t arg1,
>                register_t arg2);
>   
> +void call_smccc_smc(register_t a0, register_t a1, register_t a2,
> +                    register_t a3, register_t a4, register_t a5,
> +                    register_t a6, register_t a7, struct smccc_res *res);
> +
>   void do_trap_hyp_serror(struct cpu_user_regs *regs);
>   
>   void do_trap_guest_serror(struct cpu_user_regs *regs);
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 2/6] arm: add generic TEE mediator framework
  2018-08-22 14:11 ` [PATCH v1 2/6] arm: add generic TEE mediator framework Volodymyr Babchuk
@ 2018-08-22 17:03   ` Julien Grall
  2018-08-27 19:09     ` Volodymyr Babchuk
  0 siblings, 1 reply; 32+ messages in thread
From: Julien Grall @ 2018-08-22 17:03 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu
  Cc: tee-dev

Hi Volodymyr,

On 22/08/18 15:11, Volodymyr Babchuk wrote:
> This patch adds basic framework for TEE mediators. Guests can't talk
> to TEE directly, we need some entity that will intercept request
> and decide what to do with them. "TEE mediator" is a such entity.
> 
> This is how it works: user can build XEN with multiple TEE mediators
> (see the next patches, where OP-TEE mediator is introduced).
> TEE mediator register self with REGISTER_TEE_MEDIATOR() macro in the
> same way, as device drivers use DT_DEVICE_START()/DT_DEVICE_END()
> macros.
> In runtime, during initialization, framework calls probe() function
> for each available mediator driver to find which TEE is installed
> on the platform. Then generic vSMC handler will call selected mediator
> when it intercept SMC that belongs to TEE OS or TEE application.
> 
> Also, there are hooks for domain construction and destruction, so
> TEE mediator can inform TEE about VM lifecycle.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> ---
> 
> changes from "RFC" version:
>   - renamed CONFIG_ARM_TEE to CONFIG_TEE
>   - changed discovery mechanism: instead of UUID mathing, TEE-specific
>     probing is used
> 
>   MAINTAINERS                   |   5 ++
>   xen/arch/arm/Kconfig          |  10 ++++
>   xen/arch/arm/Makefile         |   1 +
>   xen/arch/arm/domain.c         |   7 +++
>   xen/arch/arm/setup.c          |   4 ++
>   xen/arch/arm/shutdown.c       |   2 +
>   xen/arch/arm/tee/Kconfig      |   0
>   xen/arch/arm/tee/Makefile     |   1 +
>   xen/arch/arm/tee/tee.c        |  79 ++++++++++++++++++++++++++++++++
>   xen/arch/arm/vsmc.c           |   5 ++
>   xen/arch/arm/xen.lds.S        |   7 +++
>   xen/include/asm-arm/tee/tee.h | 103 ++++++++++++++++++++++++++++++++++++++++++
>   12 files changed, 224 insertions(+)
>   create mode 100644 xen/arch/arm/tee/Kconfig
>   create mode 100644 xen/arch/arm/tee/Makefile
>   create mode 100644 xen/arch/arm/tee/tee.c
>   create mode 100644 xen/include/asm-arm/tee/tee.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 15d45d0..b0034a9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -365,6 +365,11 @@ F:	config/Stubdom.mk.in
>   F:	m4/stubdom.m4
>   F:	stubdom/
>   
> +TEE MEDIATORS
> +M:	Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> +S:	Supported
> +F:	xen/arch/arm/tee/*

The star is not necessary here.

> +
>   TOOLSTACK
>   M:	Ian Jackson <ian.jackson@eu.citrix.com>
>   M:	Wei Liu <wei.liu2@citrix.com>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 2782ee6..09bfc9b 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -57,6 +57,14 @@ config SBSA_VUART_CONSOLE
>   	  Allows a guest to use SBSA Generic UART as a console. The
>   	  SBSA Generic UART implements a subset of ARM PL011 UART.
>   
> +config TEE
> +	bool "Enable TEE mediators support"
> +	default n

I think we want this to depends on EXPERT for a few releases. We can Xen 
discuss how we are going to security support this.

> +	depends on ARM

No need to depend on Arm here.

> +	help
> +	  This option enables generic TEE mediators support. It allows guests
> +	  to access real TEE via one of TEE mediators implemented in XEN.
> +
>   endmenu
>   
>   menu "ARM errata workaround via the alternative framework"
> @@ -197,3 +205,5 @@ config ARM32_HARDEN_BRANCH_PREDICTOR
>   source "common/Kconfig"
>   
>   source "drivers/Kconfig"
> +
> +source "arch/arm/tee/Kconfig"
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 30a2a65..af29aa7 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_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 a74ff1c..41e41b9 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/tee.h>
>   #include <asm/vfp.h>
>   #include <asm/vgic.h>
>   #include <asm/vtimer.h>
> @@ -671,6 +672,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);

My concern about domain creation is still not addressed. I would expect 
the toolstack to decide whether TEE should be initialized for a given 
guest and potentially return an error on failure (e.g maximum client ID 
has been reached).

But very likely, you don't need to initialize TEE that early. This could 
be done in a separate DOMCTL as we did for VPL011.

> +
>       return 0;
>   
>   fail:
> @@ -878,6 +882,9 @@ int domain_relinquish_resources(struct domain *d)
>            */
>           domain_vpl011_deinit(d);
>   
> +        /* Free TEE mediator resources */
> +        tee_domain_destroy(d);
> +
>           d->arch.relmem = RELMEM_xen;
>           /* Fallthrough */
>   
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 032a6a8..4d31d94 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -47,6 +47,7 @@
>   #include <asm/platform.h>
>   #include <asm/procinfo.h>
>   #include <asm/setup.h>
> +#include <asm/tee/tee.h>
>   #include <xsm/xsm.h>
>   #include <asm/acpi.h>
>   
> @@ -851,6 +852,9 @@ void __init start_xen(unsigned long boot_phys_offset,
>       apply_alternatives_all();
>       enable_errata_workarounds();
>   
> +    /* Initialize TEE mediator */
> +    tee_init();

Does this really need to be called directly? Can't this be using an init 
call?

> +
>       /* Create initial domain 0. */
>       /* The vGIC for DOM0 is exactly emulating the hardware GIC */
>       config.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
> diff --git a/xen/arch/arm/shutdown.c b/xen/arch/arm/shutdown.c
> index b32f07e..e1ace18 100644
> --- a/xen/arch/arm/shutdown.c
> +++ b/xen/arch/arm/shutdown.c
> @@ -5,6 +5,7 @@
>   #include <xen/smp.h>
>   #include <asm/platform.h>
>   #include <asm/psci.h>
> +#include <asm/tee/tee.h>
>   
>   static void noreturn halt_this_cpu(void *arg)
>   {
> @@ -15,6 +16,7 @@ void machine_halt(void)
>   {
>       int timeout = 10;
>   
> +    tee_remove();

This callback does not seem to be implemented for OP-TEE. I would rather 
only introduce callback that are only necessary at the moment. The other 
can be added later on.

>       watchdog_disable();
>       console_start_sync();
>       local_irq_enable();
> diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig
> new file mode 100644
> index 0000000..e69de29
> diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
> new file mode 100644
> index 0000000..c54d479
> --- /dev/null
> +++ b/xen/arch/arm/tee/Makefile
> @@ -0,0 +1 @@
> +obj-y += tee.o
> diff --git a/xen/arch/arm/tee/tee.c b/xen/arch/arm/tee/tee.c
> new file mode 100644
> index 0000000..6df3b09
> --- /dev/null
> +++ b/xen/arch/arm/tee/tee.c
> @@ -0,0 +1,79 @@
> +/*
> + * xen/arch/arm/tee/tee.c
> + *
> + * Generic part of TEE mediator subsystem
> + *
> + * Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> + * 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/errno.h>
> +#include <xen/types.h>
> +#include <asm/tee/tee.h>
> +
> +extern const struct tee_mediator_desc _steemediator[], _eteemediator[];
> +static const struct tee_mediator_ops *mediator_ops;
> +
> +void tee_init(void)
> +{
> +    const struct tee_mediator_desc *desc;
> +
> +    for ( desc = _steemediator; desc != _eteemediator; desc++ )

{

> +        if ( desc->ops->probe() == 0 )

NIT: !desc->ops->probe()

> +        {
> +            printk(XENLOG_INFO "Using TEE mediator for %s\n", desc->name);
> +            mediator_ops = desc->ops;
> +            return;
> +        }

}

> +    printk(XENLOG_WARNING "No TEE mediator found\n");

Not having a TEE is a valid use case. So printing a warning seems a bit 
too much.

> +}
> +
> +bool tee_handle_smc(struct cpu_user_regs *regs)

How about HVC?

> +{
> +    if ( !mediator_ops )
> +        return false;
> +
> +    return mediator_ops->handle_smc(regs);
> +}
> +
> +int tee_domain_create(struct domain *d)
> +{
> +    if ( !mediator_ops )
> +        return -ENODEV;
> +
> +    return mediator_ops->domain_create(d);
> +}
> +
> +void tee_domain_destroy(struct domain *d)
> +{
> +    if ( !mediator_ops )
> +        return;
> +
> +    return mediator_ops->domain_destroy(d);
> +}
> +
> +void tee_remove(void)
> +{
> +    if ( !mediator_ops )
> +        return;
> +
> +    return mediator_ops->remove();
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> index 40a80d5..aacebf9 100644
> --- a/xen/arch/arm/vsmc.c
> +++ b/xen/arch/arm/vsmc.c
> @@ -22,6 +22,7 @@
>   #include <asm/monitor.h>
>   #include <asm/regs.h>
>   #include <asm/smccc.h>
> +#include <asm/tee/tee.h>
>   #include <asm/traps.h>
>   #include <asm/vpsci.h>
>   
> @@ -235,6 +236,10 @@ static bool vsmccc_handle_call(struct cpu_user_regs *regs)
>           case ARM_SMCCC_OWNER_STANDARD:
>               handled = handle_sssc(regs);
>               break;
> +        case ARM_SMCCC_OWNER_TRUSTED_APP ... ARM_SMCCC_OWNER_TRUSTED_APP_END:
> +        case ARM_SMCCC_OWNER_TRUSTED_OS ... ARM_SMCCC_OWNER_TRUSTED_OS_END:
> +            handled = tee_handle_smc(regs);

I think you want to rename that function "tee_handle_call".

> +            break;
>           }
>       }
>   
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index c9b9546..b78b7f1 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -126,6 +126,13 @@ SECTIONS
>         _aedevice = .;
>     } :text
>   
> +  . = ALIGN(8);
> +  .teemediator.info : {
> +      _steemediator = .;
> +      *(.teemediator.info)
> +      _eteemediator = .;
> +  } :text
> +
>     . = ALIGN(PAGE_SIZE);             /* Init code and data */
>     __init_begin = .;
>     .init.text : {
> diff --git a/xen/include/asm-arm/tee/tee.h b/xen/include/asm-arm/tee/tee.h
> new file mode 100644
> index 0000000..ffd25b0
> --- /dev/null
> +++ b/xen/include/asm-arm/tee/tee.h
> @@ -0,0 +1,103 @@
> +/*
> + * xen/include/asm-arm/tee.h
> + *
> + * Generic part of TEE mediator subsystem
> + *
> + * Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> + * 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 as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.

Xen is GPLv2 only.

> + *
> + * 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.
> + */
> +
> +#ifndef __ARCH_ARM_TEE_TEE_H__
> +#define __ARCH_ARM_TEE_TEE_H__
> +
> +#include <xen/lib.h>
> +#include <xen/types.h>
> +#include <asm/regs.h>
> +
> +#ifdef CONFIG_TEE
> +
> +struct tee_mediator_ops {
> +    /*
> +     * Probe for TEE. Should return 0 if TEE found and
> +     * mediator is initialized.
> +     */
> +    int (*probe)(void);

The caller does not care about the value but knowing there was an error. 
So it looks like to me you want to return a boolean instead.

> +
> +    /*
> +     * Called during domain construction so mediator can inform TEE about new
> +     * guest and create own structures for the new domain.
> +     */
> +    int (*domain_create)(struct domain *d);
> +
> +    /*
> +     * Called during domain destruction to inform TEE that guest is now dead
> +     * and to destroy all resources allocated for the domain being destroyed.
> +     */
> +    void (*domain_destroy)(struct domain *d);
> +
> +    /* Handle SMC call for current domain. */
> +    bool (*handle_smc)(struct cpu_user_regs *regs);
> +
> +    /* Called during XEN shutdown to free remaining resources. */
> +    void (*remove)(void);
> +};
> +
> +struct tee_mediator_desc {
> +    /* Name of the TEE. Just for debugging purposes. */
> +    const char *name;
> +
> +    /* Mediator callbacks as described above. */
> +    const struct tee_mediator_ops *ops;
> +};
> +
> +void tee_init(void);
> +bool tee_handle_smc(struct cpu_user_regs *regs);
> +int tee_domain_create(struct domain *d);
> +void tee_domain_destroy(struct domain *d);
> +void tee_remove(void);
> +
> +#define REGISTER_TEE_MEDIATOR(_name, _namestr, _ops)          \
> +static const struct tee_mediator_desc __tee_desc_##_name __used     \
> +__section(".teemediator.info") = {                                  \
> +    .name = _namestr,                                               \
> +    .ops = _ops                                                     \
> +}
> +
> +#else
> +
> +static inline void tee_init(void) {}
> +static inline bool tee_handle_smc(struct cpu_user_regs *regs)
> +{
> +    return false;
> +}
> +
> +static inline int tee_domain_create(struct domain *d)
> +{
> +    return -ENODEV;
> +}
> +
> +static inline void tee_domain_destroy(struct domain *d) {}
> +static inline void tee_remove(void) {}
> +
> +#endif  /* CONFIG_TEE */
> +
> +#endif /* __ARCH_ARM_TEE_TEE_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 5/6] libxl: create DTS node for OP-TEE if it is enabled
  2018-08-22 14:11 ` [PATCH v1 5/6] libxl: create DTS node for OP-TEE if it is enabled Volodymyr Babchuk
@ 2018-08-22 17:03   ` Wei Liu
  2018-08-22 17:32   ` Julien Grall
  1 sibling, 0 replies; 32+ messages in thread
From: Wei Liu @ 2018-08-22 17:03 UTC (permalink / raw)
  To: Volodymyr Babchuk
  Cc: Stefano Stabellini, Wei Liu, Ian Jackson, xen-devel, tee-dev,
	Julien Grall

On Wed, Aug 22, 2018 at 05:11:33PM +0300, Volodymyr Babchuk wrote:
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> ---
>  tools/libxl/libxl_arm.c     | 29 +++++++++++++++++++++++++++++
>  tools/libxl/libxl_create.c  |  1 +
>  tools/libxl/libxl_types.idl |  1 +
>  tools/xl/xl_parse.c         |  1 +

Please document the new option. See docs/man.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 4/6] optee: add OP-TEE mediator
  2018-08-22 14:11 ` [PATCH v1 4/6] optee: add OP-TEE mediator Volodymyr Babchuk
@ 2018-08-22 17:28   ` Julien Grall
  2018-08-23 14:27     ` Volodymyr Babchuk
  0 siblings, 1 reply; 32+ messages in thread
From: Julien Grall @ 2018-08-22 17:28 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu, Jens Wiklander
  Cc: tee-dev

Hi,

Please only CC relevant people to the patches. This could be done easily 
using the new script add_maintainers.pl.

On 22/08/18 15:11, Volodymyr Babchuk wrote:
> Add OP-TEE mediator, so guests can access OP-TEE services.
> 
> OP-TEE mediator support address translation for DomUs.
> It tracks execution of STD calls, correctly handles memory-related RPC
> requests, tracks buffer allocated for RPCs.
> 
> With this patch OP-TEE sucessfully passes own tests, while client is
> running in DomU.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> ---
> 
>   Changes from "RFC":
>   - Removed special case for Dom0/HwDOM
>   - No more support for plain OP-TEE (only OP-TEE with virtualization
>     config enabled is supported)
>   - Multiple domains is now supported
>   - Pages that are shared between OP-TEE and domain are now pinned
>   - Renamed CONFIG_ARM_OPTEE to CONFIG_OPTEE
>   - Command buffers from domain are now shadowed by XEN
>   - Mediator now filters out unknown capabilities and requests
>   - call contexts, shared memory object now stored per-domain
> 
>   xen/arch/arm/tee/Kconfig            |   4 +
>   xen/arch/arm/tee/Makefile           |   1 +
>   xen/arch/arm/tee/optee.c            | 972 ++++++++++++++++++++++++++++++++++++

This patch is far to big to get a proper review with understanding of 
the code. Can you split it in smaller ones with appropriate commit message?

 From a quick look at it, I would like to understand how the memory 
allocated in Xen is bounded for a given guest? Same question for the time.

I am interested in a normal case but also in the case where someone 
malicious is using that API. How much damage can it do to the hypervisor?

I will give a proper look when it is split.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 5/6] libxl: create DTS node for OP-TEE if it is enabled
  2018-08-22 14:11 ` [PATCH v1 5/6] libxl: create DTS node for OP-TEE if it is enabled Volodymyr Babchuk
  2018-08-22 17:03   ` Wei Liu
@ 2018-08-22 17:32   ` Julien Grall
  2018-08-23 14:03     ` Volodymyr Babchuk
  1 sibling, 1 reply; 32+ messages in thread
From: Julien Grall @ 2018-08-22 17:32 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel, Ian Jackson, Wei Liu, Stefano Stabellini
  Cc: tee-dev

Hi,

On 22/08/18 15:11, Volodymyr Babchuk wrote:
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

I am a bit confused. The DT is only here to tell the guest OP-TEE is 
present. But I don't see anything to tell the hypervisor the guest will 
use OP-TEE.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 6/6] xsm: add tee access policy support
  2018-08-22 14:11 ` [PATCH v1 6/6] xsm: add tee access policy support Volodymyr Babchuk
@ 2018-08-23 13:43   ` Julien Grall
  2018-08-23 13:57     ` Volodymyr Babchuk
  0 siblings, 1 reply; 32+ messages in thread
From: Julien Grall @ 2018-08-23 13:43 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu, Daniel De Graaf

Hi Volodymyr,

On 08/22/2018 03:11 PM, Volodymyr Babchuk wrote:
> As we don't want any guest to access limited resources of TEE,
> we need a way to control who can work with it.
> 
> Thus, new access vector class "tee" is added with only ony operation
> "call" so far. tee framework uses this to check if guest has a right
> to work with TEE.
> 
> Also, example security context domU_with_tee_t was added.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> ---
>   tools/flask/policy/modules/dom0.te          |  3 +++
>   tools/flask/policy/modules/domU_with_tee.te | 23 +++++++++++++++++++++++
>   tools/flask/policy/modules/modules.conf     |  1 +
>   tools/flask/policy/modules/xen.if           | 12 ++++++++++++
>   xen/arch/arm/tee/tee.c                      | 10 ++++++++++
>   xen/include/xsm/dummy.h                     | 10 ++++++++++
>   xen/include/xsm/xsm.h                       | 13 +++++++++++++
>   xen/xsm/dummy.c                             |  4 ++++
>   xen/xsm/flask/hooks.c                       | 15 +++++++++++++++
>   xen/xsm/flask/policy/access_vectors         |  7 +++++++
>   xen/xsm/flask/policy/security_classes       |  1 +
>   11 files changed, 99 insertions(+)
>   create mode 100644 tools/flask/policy/modules/domU_with_tee.te
> 
> diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te
> index 1643b40..8bac4f3 100644
> --- a/tools/flask/policy/modules/dom0.te
> +++ b/tools/flask/policy/modules/dom0.te
> @@ -27,6 +27,9 @@ allow dom0_t xen_t:version {
>   	xen_build_id
>   };
>   
> +# Allow dom0 to work with TEE
> +allow dom0_t xen_t:tee call;
> +
>   allow dom0_t xen_t:mmu memorymap;
>   
>   # Allow dom0 to use these domctls on itself. For domctls acting on other
> diff --git a/tools/flask/policy/modules/domU_with_tee.te b/tools/flask/policy/modules/domU_with_tee.te
> new file mode 100644
> index 0000000..4d856b2
> --- /dev/null
> +++ b/tools/flask/policy/modules/domU_with_tee.te
> @@ -0,0 +1,23 @@
> +###############################################################################
> +#
> +# Domain creation
> +#
> +###############################################################################
> +
> +declare_domain(domU_with_tee_t)
> +domain_self_comms(domU_t)
> +create_domain(dom0_t, domU_with_tee_t)
> +manage_domain(dom0_t, domU_with_tee_t)
> +domain_comms(dom0_t, domU_with_tee_t)
> +domain_comms(domU_with_tee_t, domU_with_tee_t)
> +migrate_domain_out(dom0_t, domU_with_tee_t)
> +domain_self_comms(domU_with_tee_t)
> +
> +# This is required for PCI (or other device) passthrough
> +delegate_devices(dom0_t, domU_with_tee_t)
> +
> +# Both of these domain types can be created using the default (system) role
> +role system_r types { domU_with_tee_t dm_dom_t };
> +
> +# Allow to work with TEE
> +access_tee(domU_with_tee_t)
> diff --git a/tools/flask/policy/modules/modules.conf b/tools/flask/policy/modules/modules.conf
> index 6dba0a3..9010d91 100644
> --- a/tools/flask/policy/modules/modules.conf
> +++ b/tools/flask/policy/modules/modules.conf
> @@ -29,6 +29,7 @@ domU = on
>   isolated_domU = on
>   prot_domU = on
>   nomigrate = on
> +domU_with_tee = on
>   
>   # Example device policy.  Also see policy/device_contexts.
>   nic_dev = on
> diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if
> index 5543749..e534179 100644
> --- a/tools/flask/policy/modules/xen.if
> +++ b/tools/flask/policy/modules/xen.if
> @@ -209,3 +209,15 @@ define(`admin_device', `
>   define(`delegate_devices', `
>       allow $1 $2:resource { add remove };
>   ')
> +
> +################################################################################
> +#
> +# Miscellaneous services
> +#
> +################################################################################
> +
> +# access_tee(domain)
> +
> +define(`access_tee', `
> +    allow $1 xen_t:tee call;
> +')
> diff --git a/xen/arch/arm/tee/tee.c b/xen/arch/arm/tee/tee.c
> index 6df3b09..06f5091 100644
> --- a/xen/arch/arm/tee/tee.c
> +++ b/xen/arch/arm/tee/tee.c
> @@ -19,6 +19,7 @@
>   #include <xen/errno.h>
>   #include <xen/types.h>
>   #include <asm/tee/tee.h>
> +#include <xsm/xsm.h>
>   
>   extern const struct tee_mediator_desc _steemediator[], _eteemediator[];
>   static const struct tee_mediator_ops *mediator_ops;
> @@ -42,6 +43,9 @@ bool tee_handle_smc(struct cpu_user_regs *regs)
>       if ( !mediator_ops )
>           return false;
>   
> +    if ( xsm_tee_op(XSM_PRIV, current->domain) )
> +        return false;
> +
>       return mediator_ops->handle_smc(regs);
>   }
>   
> @@ -50,6 +54,9 @@ int tee_domain_create(struct domain *d)
>       if ( !mediator_ops )
>           return -ENODEV;
>   
> +    if ( xsm_tee_op(XSM_PRIV, d) )
> +        return -EPERM;

I don't think we should use XSM to enforce the use of TEE. This 
contradictory to your next patch where you let the user configure OP-TEE 
for a given guest.

IHMO, XSM should only be used to restrict usage of calls in a fine 
grain. For an overall control, that should be go through a DOMCTL tell 
Xen to initialize OP-TEE for that domain.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 6/6] xsm: add tee access policy support
  2018-08-23 13:43   ` Julien Grall
@ 2018-08-23 13:57     ` Volodymyr Babchuk
  2018-08-23 14:08       ` Julien Grall
  0 siblings, 1 reply; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-23 13:57 UTC (permalink / raw)
  To: Julien Grall, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu, Daniel De Graaf

Hi Julien,

On 23.08.18 16:43, Julien Grall wrote:

> 
> I don't think we should use XSM to enforce the use of TEE. This 
> contradictory to your next patch where you let the user configure OP-TEE 
> for a given guest.
> 
> IHMO, XSM should only be used to restrict usage of calls in a fine 
> grain. For an overall control, that should be go through a DOMCTL tell 
> Xen to initialize OP-TEE for that domain.

Just to be sure. You are proposing to add flag "TEE_ENABLED" for a 
domain and set it during domain construction, based on configuration, right?

What did you mean by "fine grain"?

-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 5/6] libxl: create DTS node for OP-TEE if it is enabled
  2018-08-22 17:32   ` Julien Grall
@ 2018-08-23 14:03     ` Volodymyr Babchuk
  2018-08-23 14:11       ` Julien Grall
  0 siblings, 1 reply; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-23 14:03 UTC (permalink / raw)
  To: Julien Grall, xen-devel, Ian Jackson, Wei Liu, Stefano Stabellini; +Cc: tee-dev

Hi,

On 22.08.18 20:32, Julien Grall wrote:
> Hi,
> 
> On 22/08/18 15:11, Volodymyr Babchuk wrote:
>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> 
> I am a bit confused. The DT is only here to tell the guest OP-TEE is 
> present. But I don't see anything to tell the hypervisor the guest will 
> use OP-TEE.

Yes, this is what I wanted to discuss. I had a felling that it is not 
quite right. I think your idea with DOMCTL is better. I'll rework this 
part in the  following way:

if there is "optee=1" in a config file, then libxl will create DT entry 
and ask hypervisor to enable access to TEE for this domain.

-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 6/6] xsm: add tee access policy support
  2018-08-23 13:57     ` Volodymyr Babchuk
@ 2018-08-23 14:08       ` Julien Grall
  0 siblings, 0 replies; 32+ messages in thread
From: Julien Grall @ 2018-08-23 14:08 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu, Daniel De Graaf



On 08/23/2018 02:57 PM, Volodymyr Babchuk wrote:
> Hi Julien,

Hi Volodymyr,


> On 23.08.18 16:43, Julien Grall wrote:
> 
>>
>> I don't think we should use XSM to enforce the use of TEE. This 
>> contradictory to your next patch where you let the user configure 
>> OP-TEE for a given guest.
>>
>> IHMO, XSM should only be used to restrict usage of calls in a fine 
>> grain. For an overall control, that should be go through a DOMCTL tell 
>> Xen to initialize OP-TEE for that domain.
> 
> Just to be sure. You are proposing to add flag "TEE_ENABLED" for a 
> domain and set it during domain construction, based on configuration, 
> right?

I am suggesting another field xen_arch_domainconfig to tell whether TEE 
needs to be enabled.

> 
> What did you mean by "fine grain"?

XSM is mostly used to decided whether a given hypercall can be used by a 
domain. Here you use it to tell whether the whole TEE can be used for a 
domain.

You probably don't need any XSM for your use case here as you want the 
guest to access, if enabled, all the OP-TEE calls.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 5/6] libxl: create DTS node for OP-TEE if it is enabled
  2018-08-23 14:03     ` Volodymyr Babchuk
@ 2018-08-23 14:11       ` Julien Grall
  2018-08-23 14:16         ` Volodymyr Babchuk
  0 siblings, 1 reply; 32+ messages in thread
From: Julien Grall @ 2018-08-23 14:11 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel, Ian Jackson, Wei Liu, Stefano Stabellini
  Cc: tee-dev

Hi,

On 08/23/2018 03:03 PM, Volodymyr Babchuk wrote:
> Hi,
> 
> On 22.08.18 20:32, Julien Grall wrote:
>> Hi,
>>
>> On 22/08/18 15:11, Volodymyr Babchuk wrote:
>>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>
>> I am a bit confused. The DT is only here to tell the guest OP-TEE is 
>> present. But I don't see anything to tell the hypervisor the guest 
>> will use OP-TEE.
> 
> Yes, this is what I wanted to discuss. I had a felling that it is not 
> quite right. I think your idea with DOMCTL is better. I'll rework this 
> part in the  following way:
> 
> if there is "optee=1" in a config file, then libxl will create DT entry 
> and ask hypervisor to enable access to TEE for this domain.

You probably also want to make the variable TEE agnostic by renaming to 
"tee".

In the future, the toolstack would need to know which TEE is in used in 
the hypervisor. For now, it is fine to assume that it is OP-TEE.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 5/6] libxl: create DTS node for OP-TEE if it is enabled
  2018-08-23 14:11       ` Julien Grall
@ 2018-08-23 14:16         ` Volodymyr Babchuk
  0 siblings, 0 replies; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-23 14:16 UTC (permalink / raw)
  To: Julien Grall, xen-devel, Ian Jackson, Wei Liu, Stefano Stabellini; +Cc: tee-dev

Hi,

On 23.08.18 17:11, Julien Grall wrote:
> Hi,
> 
> On 08/23/2018 03:03 PM, Volodymyr Babchuk wrote:
>> Hi,
>>
>> On 22.08.18 20:32, Julien Grall wrote:
>>> Hi,
>>>
>>> On 22/08/18 15:11, Volodymyr Babchuk wrote:
>>>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>>
>>> I am a bit confused. The DT is only here to tell the guest OP-TEE is 
>>> present. But I don't see anything to tell the hypervisor the guest 
>>> will use OP-TEE.
>>
>> Yes, this is what I wanted to discuss. I had a felling that it is not 
>> quite right. I think your idea with DOMCTL is better. I'll rework this 
>> part in the  following way:
>>
>> if there is "optee=1" in a config file, then libxl will create DT 
>> entry and ask hypervisor to enable access to TEE for this domain.
> 
> You probably also want to make the variable TEE agnostic by renaming to 
> "tee".
> 
> In the future, the toolstack would need to know which TEE is in used in 
> the hypervisor. For now, it is fine to assume that it is OP-TEE.

Yes, sounds good. Will do in this way.
Thank you for review and suggestion.

-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 4/6] optee: add OP-TEE mediator
  2018-08-22 17:28   ` Julien Grall
@ 2018-08-23 14:27     ` Volodymyr Babchuk
  2018-08-23 15:28       ` Julien Grall
  0 siblings, 1 reply; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-23 14:27 UTC (permalink / raw)
  To: Julien Grall, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu, Jens Wiklander
  Cc: tee-dev

Hi,

On 22.08.18 20:28, Julien Grall wrote:
> Hi,
> 
> Please only CC relevant people to the patches. This could be done easily 
> using the new script add_maintainers.pl.
> 
Oh, I'm sorry. I used get_maintainers.pl.

> On 22/08/18 15:11, Volodymyr Babchuk wrote:
>> Add OP-TEE mediator, so guests can access OP-TEE services.
>>
>> OP-TEE mediator support address translation for DomUs.
>> It tracks execution of STD calls, correctly handles memory-related RPC
>> requests, tracks buffer allocated for RPCs.
>>
>> With this patch OP-TEE sucessfully passes own tests, while client is
>> running in DomU.
>>
>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> ---
>>
>>   Changes from "RFC":
>>   - Removed special case for Dom0/HwDOM
>>   - No more support for plain OP-TEE (only OP-TEE with virtualization
>>     config enabled is supported)
>>   - Multiple domains is now supported
>>   - Pages that are shared between OP-TEE and domain are now pinned
>>   - Renamed CONFIG_ARM_OPTEE to CONFIG_OPTEE
>>   - Command buffers from domain are now shadowed by XEN
>>   - Mediator now filters out unknown capabilities and requests
>>   - call contexts, shared memory object now stored per-domain
>>
>>   xen/arch/arm/tee/Kconfig            |   4 +
>>   xen/arch/arm/tee/Makefile           |   1 +
>>   xen/arch/arm/tee/optee.c            | 972 
>> ++++++++++++++++++++++++++++++++++++
> 
> This patch is far to big to get a proper review with understanding of 
> the code. Can you split it in smaller ones with appropriate commit message?
> 
Yes, it is a quite big. But this is a complete feature. I can't remove 
anything from it, because it will not work.
I can split it into series of patches, that will add various pieces of 
code... But this will lead to patches with not-working code until the 
final one. Is this okay?

>  From a quick look at it, I would like to understand how the memory 
> allocated in Xen is bounded for a given guest? Same question for the time.

I store references to allocated pages in per-domain context. But they 
are not accounted as a domain memory. This pages are needed by XEN to 
conceal real PAs from guest. I'm not sure it they should be accounted as 
a memory allocated by domain.

And what about a time? Did you mean time accounting?

> I am interested in a normal case but also in the case where someone 
> malicious is using that API. How much damage can it do to the hypervisor?

Every standard (long-lasting) call requires small amount of memory to 
store context. Every shared buffer requires enough memory to store 
references to shared pages.
OP-TEE has limited resources, so it will not allow you to create, say, 
100 calls and couple of GBs of shared memory. I expect that it will 
limit caller in memory overuse.

Apart from that I can't imagine how malicious user can damage the 
hypervisor.


-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-22 16:46   ` Julien Grall
@ 2018-08-23 14:35     ` Volodymyr Babchuk
  2018-08-23 14:45       ` Julien Grall
  2018-08-30 14:48     ` Volodymyr Babchuk
  1 sibling, 1 reply; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-23 14:35 UTC (permalink / raw)
  To: Julien Grall, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu
  Cc: tee-dev, Edgar E. Iglesias

Hi,

On 22.08.18 19:46, Julien Grall wrote:
> Hi Volodymyr,
> 
> On 22/08/18 15:11, Volodymyr Babchuk wrote:
>> Existing SMC wrapper call_smc() allows only 4 parameters and
>> returns only one value. This is enough for existing
>> use in PSCI code, but TEE mediator will need a call that is
>> fully compatible with ARM SMCCC.
>> This patch adds this call for both arm32 and arm64.
>>
>> There was similar patch by Edgar E. Iglesias ([1]), but looks
>> like it is abandoned.
>>
>> [1] 
>> https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00636.html 
>>
>>
>> CC: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>>
>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> ---
>>
>> changes from "RFC":
>>    - response now stored in structure instead of array
>>    - added comments for arm32 assembly code
>>    - added offset (instead of magic values) for arm32 asm code
> 
> Did you mean arm64?

Yes, you are right.

>>
>>   xen/arch/arm/arm32/Makefile      |  1 +
>>   xen/arch/arm/arm32/smc.S         | 39 
>> +++++++++++++++++++++++++++++++++++++++
>>   xen/arch/arm/arm64/Makefile      |  1 +
>>   xen/arch/arm/arm64/asm-offsets.c |  4 ++++
>>   xen/arch/arm/arm64/smc.S         | 30 ++++++++++++++++++++++++++++++
>>   xen/include/asm-arm/processor.h  | 11 +++++++++++
>>   6 files changed, 86 insertions(+)
>>   create mode 100644 xen/arch/arm/arm32/smc.S
>>   create mode 100644 xen/arch/arm/arm64/smc.S
>>
>> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
>> index 0ac254f..c69f35e 100644
>> --- a/xen/arch/arm/arm32/Makefile
>> +++ b/xen/arch/arm/arm32/Makefile
>> @@ -10,4 +10,5 @@ obj-y += proc-v7.o proc-caxx.o
>>   obj-y += smpboot.o
>>   obj-y += traps.o
>>   obj-y += vfp.o
>> +obj-y += smc.o
>> diff --git a/xen/arch/arm/arm32/smc.S b/xen/arch/arm/arm32/smc.S
>> new file mode 100644
>> index 0000000..56f7982
>> --- /dev/null
>> +++ b/xen/arch/arm/arm32/smc.S
>> @@ -0,0 +1,39 @@
>> +/*
>> + * xen/arch/arm/arm32/smc.S
>> + *
>> + * Wrapper for Secure Monitors Calls
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
> 
> Xen is GPLv2 only. Can you please update the copyright accordingly?
> 
> Also, the code is pretty much a copy of the Linux part. So it is 
> probably worth to keep the copy-right around.
>> + *
>> + * 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 <asm/macros.h>
>> +
>> +/*
>> + * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
>> + *                     register_t a3, register_t a4, register_t a5,
>> + *                     register_t a6, register_t a7, struct smccc_res 
>> *res)
>> + */
>> +ENTRY(call_smccc_smc)
> 
> I would rather avoid to have 2 function to handle the same convention. 
> Can you please either drop call_smc or rework it to handle more parameters?
> 
> But it looks like you could re-use arm_smccc_1_1 as it follows the 1.0 
> convention for 32-bits.
Oops, I missed introduction of arm_smccc_1_1. I think, it will be 
sufficient for my tasks. Looks like, I'll drop this patch at all.

I'm sorry for bothering you.

>> +        mov     r12, sp
>> +        push    {r4-r7}
>> +    /*
>> +     * According to ARMv7 (or aarch32) ABI, first 4 parameters of
>> +     * call_smccc_smc are passed in r0-r3 and other 4 are on stack.
>> +     * Load a4-a7 from stack to r4-r7.
>> +     */
>> +        ldm     r12, {r4-r7}
>> +        smc     #0
>> +        pop     {r4-r7}
>> +    /* Load pointer to res (4th parameter on stack) to r12 */
>> +        ldr     r12, [sp, #(4 * 4)]
>> +    /* Store returned results from r0-r3 to res */
>> +        stm     r12, {r0-r3}
>> +        bx      lr
> 
> Something looks wrong with the indentation. There seem to be a mix of 
> hard tab and soft tab.
> 
>> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
>> index bb5c610..7ecdad7 100644
>> --- a/xen/arch/arm/arm64/Makefile
>> +++ b/xen/arch/arm/arm64/Makefile
>> @@ -9,6 +9,7 @@ obj-y += entry.o
>>   obj-y += insn.o
>>   obj-$(CONFIG_LIVEPATCH) += livepatch.o
>>   obj-y += smpboot.o
>> +obj-y += smc.o
>>   obj-y += traps.o
>>   obj-y += vfp.o
>>   obj-y += vsysreg.o
>> diff --git a/xen/arch/arm/arm64/asm-offsets.c 
>> b/xen/arch/arm/arm64/asm-offsets.c
>> index ce24e44..5353fe8 100644
>> --- a/xen/arch/arm/arm64/asm-offsets.c
>> +++ b/xen/arch/arm/arm64/asm-offsets.c
>> @@ -50,6 +50,10 @@ void __dummy__(void)
>>      BLANK();
>>      OFFSET(INITINFO_stack, struct init_info, stack);
>> +
>> +   BLANK();
>> +   OFFSET(SMCCC_RES_a0, struct smccc_res, a0);
>> +   OFFSET(SMCCC_RES_a2, struct smccc_res, a2);
>>   }
>>   /*
>> diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
>> new file mode 100644
>> index 0000000..40843c0
>> --- /dev/null
>> +++ b/xen/arch/arm/arm64/smc.S
>> @@ -0,0 +1,30 @@
>> +/*
>> + * xen/arch/arm/arm64/smc.S
>> + *
>> + * Wrapper for Secure Monitors Calls
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
> 
> Xen is GPLv2 only. Can you please update the copyright accordingly?
> 
> Also, the code is pretty much a copy of the Linux part. So it is 
> probably worth to keep the copy-right around.
> 
>> + *
>> + * 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 <asm/asm_defns.h>
>> +#include <asm/macros.h>
>> +
>> +/*
>> + * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
>> + *                     register_t a3, register_t a4, register_t a5,
>> + *                     register_t a6, register_t a7, struct smccc_res 
>> *res)
>> + */
>> +ENTRY(call_smccc_smc)
>> +        smc     #0
>> +        ldr     x4, [sp]
>> +        stp     x0, x1, [x4, #SMCCC_RES_a0]
>> +        stp     x2, x3, [x4, #SMCCC_RES_a2]
>> +        ret
>> diff --git a/xen/include/asm-arm/processor.h 
>> b/xen/include/asm-arm/processor.h
>> index 222a02d..946a2db 100644
>> --- a/xen/include/asm-arm/processor.h
>> +++ b/xen/include/asm-arm/processor.h
>> @@ -650,6 +650,13 @@ union hsr {
>>   };
>> +
>> +struct smccc_res {
>> +    register_t a0;
>> +    register_t a1;
>> +    register_t a2;
>> +    register_t a3;
>> +};
> 
> Any reason to not use arm_smccc_res?
> 
>>   #endif
>>   /* HSR.EC == HSR_CP{15,14,10}_32 */
>> @@ -815,6 +822,10 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
>>   int call_smc(register_t function_id, register_t arg0, register_t arg1,
>>                register_t arg2);
>> +void call_smccc_smc(register_t a0, register_t a1, register_t a2,
>> +                    register_t a3, register_t a4, register_t a5,
>> +                    register_t a6, register_t a7, struct smccc_res 
>> *res);
>> +
>>   void do_trap_hyp_serror(struct cpu_user_regs *regs);
>>   void do_trap_guest_serror(struct cpu_user_regs *regs);
>>
> 
> Cheers,
> 

-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-23 14:35     ` Volodymyr Babchuk
@ 2018-08-23 14:45       ` Julien Grall
  2018-08-23 15:16         ` Volodymyr Babchuk
  0 siblings, 1 reply; 32+ messages in thread
From: Julien Grall @ 2018-08-23 14:45 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu
  Cc: tee-dev, Edgar E. Iglesias



On 08/23/2018 03:35 PM, Volodymyr Babchuk wrote:
> Hi,

Hi Volodymyr,


> On 22.08.18 19:46, Julien Grall wrote:
>> Hi Volodymyr,
>>
>> On 22/08/18 15:11, Volodymyr Babchuk wrote:
>>> Existing SMC wrapper call_smc() allows only 4 parameters and
>>> returns only one value. This is enough for existing
>>> use in PSCI code, but TEE mediator will need a call that is
>>> fully compatible with ARM SMCCC.
>>> This patch adds this call for both arm32 and arm64.
>>>
>>> There was similar patch by Edgar E. Iglesias ([1]), but looks
>>> like it is abandoned.
>>>
>>> [1] 
>>> https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00636.html 
>>>
>>>
>>> CC: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>>>
>>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>> ---
>>>
>>> changes from "RFC":
>>>    - response now stored in structure instead of array
>>>    - added comments for arm32 assembly code
>>>    - added offset (instead of magic values) for arm32 asm code
>>
>> Did you mean arm64?
> 
> Yes, you are right.
> 
>>>
>>>   xen/arch/arm/arm32/Makefile      |  1 +
>>>   xen/arch/arm/arm32/smc.S         | 39 
>>> +++++++++++++++++++++++++++++++++++++++
>>>   xen/arch/arm/arm64/Makefile      |  1 +
>>>   xen/arch/arm/arm64/asm-offsets.c |  4 ++++
>>>   xen/arch/arm/arm64/smc.S         | 30 ++++++++++++++++++++++++++++++
>>>   xen/include/asm-arm/processor.h  | 11 +++++++++++
>>>   6 files changed, 86 insertions(+)
>>>   create mode 100644 xen/arch/arm/arm32/smc.S
>>>   create mode 100644 xen/arch/arm/arm64/smc.S
>>>
>>> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
>>> index 0ac254f..c69f35e 100644
>>> --- a/xen/arch/arm/arm32/Makefile
>>> +++ b/xen/arch/arm/arm32/Makefile
>>> @@ -10,4 +10,5 @@ obj-y += proc-v7.o proc-caxx.o
>>>   obj-y += smpboot.o
>>>   obj-y += traps.o
>>>   obj-y += vfp.o
>>> +obj-y += smc.o
>>> diff --git a/xen/arch/arm/arm32/smc.S b/xen/arch/arm/arm32/smc.S
>>> new file mode 100644
>>> index 0000000..56f7982
>>> --- /dev/null
>>> +++ b/xen/arch/arm/arm32/smc.S
>>> @@ -0,0 +1,39 @@
>>> +/*
>>> + * xen/arch/arm/arm32/smc.S
>>> + *
>>> + * Wrapper for Secure Monitors Calls
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>
>> Xen is GPLv2 only. Can you please update the copyright accordingly?
>>
>> Also, the code is pretty much a copy of the Linux part. So it is 
>> probably worth to keep the copy-right around.
>>> + *
>>> + * 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 <asm/macros.h>
>>> +
>>> +/*
>>> + * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
>>> + *                     register_t a3, register_t a4, register_t a5,
>>> + *                     register_t a6, register_t a7, struct 
>>> smccc_res *res)
>>> + */
>>> +ENTRY(call_smccc_smc)
>>
>> I would rather avoid to have 2 function to handle the same convention. 
>> Can you please either drop call_smc or rework it to handle more 
>> parameters?
>>
>> But it looks like you could re-use arm_smccc_1_1 as it follows the 1.0 
>> convention for 32-bits.
> Oops, I missed introduction of arm_smccc_1_1. I think, it will be 
> sufficient for my tasks. Looks like, I'll drop this patch at all.

Are you suggesting that OP-TEE will always use SMCCC 1.1 calling convention?

I would be quite surprised given that you seem to have platform with 
ancient firmware.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-23 14:45       ` Julien Grall
@ 2018-08-23 15:16         ` Volodymyr Babchuk
  2018-08-23 15:31           ` Julien Grall
  0 siblings, 1 reply; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-23 15:16 UTC (permalink / raw)
  To: Julien Grall, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu
  Cc: tee-dev, Edgar E. Iglesias

Hi,

On 23.08.18 17:45, Julien Grall wrote:
> 
> 
> On 08/23/2018 03:35 PM, Volodymyr Babchuk wrote:
>> Hi,
> 
> Hi Volodymyr,
> 
> 
>> On 22.08.18 19:46, Julien Grall wrote:
>>> Hi Volodymyr,
>>>
>>> On 22/08/18 15:11, Volodymyr Babchuk wrote:
>>>> Existing SMC wrapper call_smc() allows only 4 parameters and
>>>> returns only one value. This is enough for existing
>>>> use in PSCI code, but TEE mediator will need a call that is
>>>> fully compatible with ARM SMCCC.
>>>> This patch adds this call for both arm32 and arm64.
>>>>
>>>> There was similar patch by Edgar E. Iglesias ([1]), but looks
>>>> like it is abandoned.
>>>>
>>>> [1] 
>>>> https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00636.html 
>>>>
>>>>
>>>> CC: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>>>>
>>>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>>> ---
>>>>
>>>> changes from "RFC":
>>>>    - response now stored in structure instead of array
>>>>    - added comments for arm32 assembly code
>>>>    - added offset (instead of magic values) for arm32 asm code
>>>
>>> Did you mean arm64?
>>
>> Yes, you are right.
>>
>>>>
>>>>   xen/arch/arm/arm32/Makefile      |  1 +
>>>>   xen/arch/arm/arm32/smc.S         | 39 
>>>> +++++++++++++++++++++++++++++++++++++++
>>>>   xen/arch/arm/arm64/Makefile      |  1 +
>>>>   xen/arch/arm/arm64/asm-offsets.c |  4 ++++
>>>>   xen/arch/arm/arm64/smc.S         | 30 ++++++++++++++++++++++++++++++
>>>>   xen/include/asm-arm/processor.h  | 11 +++++++++++
>>>>   6 files changed, 86 insertions(+)
>>>>   create mode 100644 xen/arch/arm/arm32/smc.S
>>>>   create mode 100644 xen/arch/arm/arm64/smc.S
>>>>
>>>> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
>>>> index 0ac254f..c69f35e 100644
>>>> --- a/xen/arch/arm/arm32/Makefile
>>>> +++ b/xen/arch/arm/arm32/Makefile
>>>> @@ -10,4 +10,5 @@ obj-y += proc-v7.o proc-caxx.o
>>>>   obj-y += smpboot.o
>>>>   obj-y += traps.o
>>>>   obj-y += vfp.o
>>>> +obj-y += smc.o
>>>> diff --git a/xen/arch/arm/arm32/smc.S b/xen/arch/arm/arm32/smc.S
>>>> new file mode 100644
>>>> index 0000000..56f7982
>>>> --- /dev/null
>>>> +++ b/xen/arch/arm/arm32/smc.S
>>>> @@ -0,0 +1,39 @@
>>>> +/*
>>>> + * xen/arch/arm/arm32/smc.S
>>>> + *
>>>> + * Wrapper for Secure Monitors Calls
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or 
>>>> modify
>>>> + * it under the terms of the GNU General Public License as 
>>>> published by
>>>> + * the Free Software Foundation; either version 2 of the License, or
>>>> + * (at your option) any later version.
>>>
>>> Xen is GPLv2 only. Can you please update the copyright accordingly?
>>>
>>> Also, the code is pretty much a copy of the Linux part. So it is 
>>> probably worth to keep the copy-right around.
>>>> + *
>>>> + * 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 <asm/macros.h>
>>>> +
>>>> +/*
>>>> + * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
>>>> + *                     register_t a3, register_t a4, register_t a5,
>>>> + *                     register_t a6, register_t a7, struct 
>>>> smccc_res *res)
>>>> + */
>>>> +ENTRY(call_smccc_smc)
>>>
>>> I would rather avoid to have 2 function to handle the same 
>>> convention. Can you please either drop call_smc or rework it to 
>>> handle more parameters?
>>>
>>> But it looks like you could re-use arm_smccc_1_1 as it follows the 
>>> 1.0 convention for 32-bits.
>> Oops, I missed introduction of arm_smccc_1_1. I think, it will be 
>> sufficient for my tasks. Looks like, I'll drop this patch at all.
> 
> Are you suggesting that OP-TEE will always use SMCCC 1.1 calling 
> convention?

It depends on ARM-TF, I think. At least, on ARMv8 platforms. But you are 
right. I can't be sure that SMCCC 1.1 is supported.
Looks like XEN should determine at boot time which calling convention is 
supported and then use appropriate function in all places where SMCCC 
calls are made. What do you think?

> I would be quite surprised given that you seem to have platform with 
> ancient firmware.
It is not so ancient :) At least it have ARM-TF.
Also, I have found that qemu-armv8 speeds up debugging, as I don't need 
to tinker with real HW most of the time.

-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 4/6] optee: add OP-TEE mediator
  2018-08-23 14:27     ` Volodymyr Babchuk
@ 2018-08-23 15:28       ` Julien Grall
  0 siblings, 0 replies; 32+ messages in thread
From: Julien Grall @ 2018-08-23 15:28 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu, Jens Wiklander
  Cc: tee-dev



On 08/23/2018 03:27 PM, Volodymyr Babchuk wrote:
> Hi,

Hi,


> On 22.08.18 20:28, Julien Grall wrote:
>> Hi,
>>
>> Please only CC relevant people to the patches. This could be done 
>> easily using the new script add_maintainers.pl.
>>
> Oh, I'm sorry. I used get_maintainers.pl.
> 
>> On 22/08/18 15:11, Volodymyr Babchuk wrote:
>>> Add OP-TEE mediator, so guests can access OP-TEE services.
>>>
>>> OP-TEE mediator support address translation for DomUs.
>>> It tracks execution of STD calls, correctly handles memory-related RPC
>>> requests, tracks buffer allocated for RPCs.
>>>
>>> With this patch OP-TEE sucessfully passes own tests, while client is
>>> running in DomU.
>>>
>>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>> ---
>>>
>>>   Changes from "RFC":
>>>   - Removed special case for Dom0/HwDOM
>>>   - No more support for plain OP-TEE (only OP-TEE with virtualization
>>>     config enabled is supported)
>>>   - Multiple domains is now supported
>>>   - Pages that are shared between OP-TEE and domain are now pinned
>>>   - Renamed CONFIG_ARM_OPTEE to CONFIG_OPTEE
>>>   - Command buffers from domain are now shadowed by XEN
>>>   - Mediator now filters out unknown capabilities and requests
>>>   - call contexts, shared memory object now stored per-domain
>>>
>>>   xen/arch/arm/tee/Kconfig            |   4 +
>>>   xen/arch/arm/tee/Makefile           |   1 +
>>>   xen/arch/arm/tee/optee.c            | 972 
>>> ++++++++++++++++++++++++++++++++++++
>>
>> This patch is far to big to get a proper review with understanding of 
>> the code. Can you split it in smaller ones with appropriate commit 
>> message?
>>
> Yes, it is a quite big. But this is a complete feature. I can't remove 
> anything from it, because it will not work.
> I can split it into series of patches, that will add various pieces of 
> code... But this will lead to patches with not-working code until the 
> final one. Is this okay?

This is a new feature so it does not matter if it does not work until 
the end. Although, ideally, this should not break the rest of features.

What I want to avoid is a 900 lines complex patch with very little to 
understand what is done.

> 
>>  From a quick look at it, I would like to understand how the memory 
>> allocated in Xen is bounded for a given guest? Same question for the 
>> time.
> 
> I store references to allocated pages in per-domain context. But they 
> are not accounted as a domain memory. This pages are needed by XEN to 
> conceal real PAs from guest. I'm not sure it they should be accounted as 
> a memory allocated by domain.
Xen heap can be quite limited. As the memory can stay around for a long 
time, would it be possible for a guest to exhaust that pool?

> 
> And what about a time? Did you mean time accounting?

Xen only supports voluntary preemption. This means that long lasting 
operation in Xen may block other vCPUs to run.

Call such p2m_lookup are not cheap to use as it requires to walk the 
page-table is software. From a look at the code, the number of call will 
be bound by guest-controlled parameter.

I can't see anything in the hypervisor sanitizing those values, so the 
guest can control how long the call will take and also the memory 
"reserved" in the hypervisor even if OP-TEE fails afterwards.

>> I am interested in a normal case but also in the case where someone 
>> malicious is using that API. How much damage can it do to the hypervisor?
> 
> Every standard (long-lasting) call requires small amount of memory to 
> store context. Every shared buffer requires enough memory to store 
> references to shared pages.
> OP-TEE has limited resources, so it will not allow you to create, say, 
> 100 calls and couple of GBs of shared memory. I expect that it will 
> limit caller in memory overuse.

Do you mean per Client instance? Or for OP-TEE in general?

In any case, Xen memory allocation is always done before OP-TEE is 
called. So there is still a window where the domain book-keep a big 
chunk of memory that will be release at the end of the call.

>
> Apart from that I can't imagine how malicious user can damage the 
> hypervisor.

See above. I think they are a lot of room for a guest to attack Xen. 
Most likely you want to limit the number of call done in parallel and 
also the shared memory mapped around.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-23 15:16         ` Volodymyr Babchuk
@ 2018-08-23 15:31           ` Julien Grall
  0 siblings, 0 replies; 32+ messages in thread
From: Julien Grall @ 2018-08-23 15:31 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel, Andrew Cooper, George Dunlap,
	Ian Jackson, Jan Beulich, Konrad Rzeszutek Wilk,
	Stefano Stabellini, Tim Deegan, Wei Liu
  Cc: tee-dev, Edgar E. Iglesias

Hi,

On 08/23/2018 04:16 PM, Volodymyr Babchuk wrote:
> Hi,
> 
> On 23.08.18 17:45, Julien Grall wrote:
>>
>>
>> On 08/23/2018 03:35 PM, Volodymyr Babchuk wrote:
>>> Hi,
>>
>> Hi Volodymyr,
>>
>>
>>> On 22.08.18 19:46, Julien Grall wrote:
>>>> Hi Volodymyr,
>>>>
>>>> On 22/08/18 15:11, Volodymyr Babchuk wrote:
>>>>> Existing SMC wrapper call_smc() allows only 4 parameters and
>>>>> returns only one value. This is enough for existing
>>>>> use in PSCI code, but TEE mediator will need a call that is
>>>>> fully compatible with ARM SMCCC.
>>>>> This patch adds this call for both arm32 and arm64.
>>>>>
>>>>> There was similar patch by Edgar E. Iglesias ([1]), but looks
>>>>> like it is abandoned.
>>>>>
>>>>> [1] 
>>>>> https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00636.html 
>>>>>
>>>>>
>>>>> CC: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>>>>>
>>>>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>>>> ---
>>>>>
>>>>> changes from "RFC":
>>>>>    - response now stored in structure instead of array
>>>>>    - added comments for arm32 assembly code
>>>>>    - added offset (instead of magic values) for arm32 asm code
>>>>
>>>> Did you mean arm64?
>>>
>>> Yes, you are right.
>>>
>>>>>
>>>>>   xen/arch/arm/arm32/Makefile      |  1 +
>>>>>   xen/arch/arm/arm32/smc.S         | 39 
>>>>> +++++++++++++++++++++++++++++++++++++++
>>>>>   xen/arch/arm/arm64/Makefile      |  1 +
>>>>>   xen/arch/arm/arm64/asm-offsets.c |  4 ++++
>>>>>   xen/arch/arm/arm64/smc.S         | 30 ++++++++++++++++++++++++++++++
>>>>>   xen/include/asm-arm/processor.h  | 11 +++++++++++
>>>>>   6 files changed, 86 insertions(+)
>>>>>   create mode 100644 xen/arch/arm/arm32/smc.S
>>>>>   create mode 100644 xen/arch/arm/arm64/smc.S
>>>>>
>>>>> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
>>>>> index 0ac254f..c69f35e 100644
>>>>> --- a/xen/arch/arm/arm32/Makefile
>>>>> +++ b/xen/arch/arm/arm32/Makefile
>>>>> @@ -10,4 +10,5 @@ obj-y += proc-v7.o proc-caxx.o
>>>>>   obj-y += smpboot.o
>>>>>   obj-y += traps.o
>>>>>   obj-y += vfp.o
>>>>> +obj-y += smc.o
>>>>> diff --git a/xen/arch/arm/arm32/smc.S b/xen/arch/arm/arm32/smc.S
>>>>> new file mode 100644
>>>>> index 0000000..56f7982
>>>>> --- /dev/null
>>>>> +++ b/xen/arch/arm/arm32/smc.S
>>>>> @@ -0,0 +1,39 @@
>>>>> +/*
>>>>> + * xen/arch/arm/arm32/smc.S
>>>>> + *
>>>>> + * Wrapper for Secure Monitors Calls
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or 
>>>>> modify
>>>>> + * it under the terms of the GNU General Public License as 
>>>>> published by
>>>>> + * the Free Software Foundation; either version 2 of the License, or
>>>>> + * (at your option) any later version.
>>>>
>>>> Xen is GPLv2 only. Can you please update the copyright accordingly?
>>>>
>>>> Also, the code is pretty much a copy of the Linux part. So it is 
>>>> probably worth to keep the copy-right around.
>>>>> + *
>>>>> + * 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 <asm/macros.h>
>>>>> +
>>>>> +/*
>>>>> + * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
>>>>> + *                     register_t a3, register_t a4, register_t a5,
>>>>> + *                     register_t a6, register_t a7, struct 
>>>>> smccc_res *res)
>>>>> + */
>>>>> +ENTRY(call_smccc_smc)
>>>>
>>>> I would rather avoid to have 2 function to handle the same 
>>>> convention. Can you please either drop call_smc or rework it to 
>>>> handle more parameters?
>>>>
>>>> But it looks like you could re-use arm_smccc_1_1 as it follows the 
>>>> 1.0 convention for 32-bits.
>>> Oops, I missed introduction of arm_smccc_1_1. I think, it will be 
>>> sufficient for my tasks. Looks like, I'll drop this patch at all.
>>
>> Are you suggesting that OP-TEE will always use SMCCC 1.1 calling 
>> convention?
> 
> It depends on ARM-TF, I think. At least, on ARMv8 platforms. But you are 
> right. I can't be sure that SMCCC 1.1 is supported.
> Looks like XEN should determine at boot time which calling convention is 
> supported and then use appropriate function in all places where SMCCC 
> calls are made. What do you think?

This may not be entirely trivial to do it as arm_smccc_1_1 is based on 
macros to limit the number of parameters saved. But I would welcome such 
patch.

> 
>> I would be quite surprised given that you seem to have platform with 
>> ancient firmware.
> It is not so ancient :) At least it have ARM-TF.

Well, SMCCC 1.1 has been introduced last January for Meltdown/Spectre 
mitigations. I consider anything before that ancient :).

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-22 14:11 ` [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC Volodymyr Babchuk
  2018-08-22 16:46   ` Julien Grall
@ 2018-08-27  6:44   ` Jan Beulich
  2018-08-27 19:24     ` Volodymyr Babchuk
  1 sibling, 1 reply; 32+ messages in thread
From: Jan Beulich @ 2018-08-27  6:44 UTC (permalink / raw)
  To: Volodymyr Babchuk
  Cc: edgar.iglesias, Tim Deegan, Stefano Stabellini, Wei Liu,
	George Dunlap, Andrew Cooper, Ian Jackson, Lars Kurth, xen-devel,
	tee-dev, Julien Grall

>>> On 22.08.18 at 16:11, <volodymyr_babchuk@epam.com> wrote:
> Existing SMC wrapper call_smc() allows only 4 parameters and
> returns only one value. This is enough for existing
> use in PSCI code, but TEE mediator will need a call that is
> fully compatible with ARM SMCCC.
> This patch adds this call for both arm32 and arm64.
> 
> There was similar patch by Edgar E. Iglesias ([1]), but looks
> like it is abandoned.
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00636.html 
> 
> CC: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> ---
> 
> changes from "RFC":
>   - response now stored in structure instead of array
>   - added comments for arm32 assembly code
>   - added offset (instead of magic values) for arm32 asm code
> 
>  xen/arch/arm/arm32/Makefile      |  1 +
>  xen/arch/arm/arm32/smc.S         | 39 +++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/arm64/Makefile      |  1 +
>  xen/arch/arm/arm64/asm-offsets.c |  4 ++++
>  xen/arch/arm/arm64/smc.S         | 30 ++++++++++++++++++++++++++++++
>  xen/include/asm-arm/processor.h  | 11 +++++++++++
>  6 files changed, 86 insertions(+)
>  create mode 100644 xen/arch/arm/arm32/smc.S
>  create mode 100644 xen/arch/arm/arm64/smc.S

With this diffstat, why did I end up on the _To_ list of this patch?
I shouldn't even have been on the Cc list (and I'm going to blindly
delete all other patches of this series where you've also
apparently put me on the To list). Please remember that patches
are supposed to be sent _To_ the list, with maintainers, reviewers
(and perhaps others) Cc-ed. Together with people replying to
mails often not adjusting To/Cc lists accordingly, I've now ended
up with well over 30 mails in my primary mail folder which
presumably I have no business with at all. 

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 2/6] arm: add generic TEE mediator framework
  2018-08-22 17:03   ` Julien Grall
@ 2018-08-27 19:09     ` Volodymyr Babchuk
  2018-08-28 11:14       ` Julien Grall
  0 siblings, 1 reply; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-27 19:09 UTC (permalink / raw)
  To: Julien Grall, xen-devel, Stefano Stabellini; +Cc: tee-dev

Hi Julien,


On 22.08.18 20:03, Julien Grall wrote:

[...]

>>       if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
>>           goto fail;
>> +    /* Notify TEE that new domain was created */
>> +    tee_domain_create(d);
> 
> My concern about domain creation is still not addressed. I would expect 
> the toolstack to decide whether TEE should be initialized for a given 
> guest and potentially return an error on failure (e.g maximum client ID 
> has been reached).
> 
> But very likely, you don't need to initialize TEE that early. This could 
> be done in a separate DOMCTL as we did for VPL011.

Yes, as we discussed in latter patches, I'll add DOMCTL support. But 
what to do with dom0 construction?
I think, it should be configurable. But how? With commandline option?

[...]

> }
> 
>> +    printk(XENLOG_WARNING "No TEE mediator found\n");
> 
> Not having a TEE is a valid use case. So printing a warning seems a bit 
> too much.

I can change this to INFO. Or it is better to remove this print at all?


Thank you for review, BTW.

-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-27  6:44   ` Jan Beulich
@ 2018-08-27 19:24     ` Volodymyr Babchuk
  2018-08-27 20:19       ` Julien Grall
  2018-08-28  6:09       ` Jan Beulich
  0 siblings, 2 replies; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-27 19:24 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Lars Kurth, xen-devel

Hi Jan,


On 27.08.18 09:44, Jan Beulich wrote:

[...]

>>   xen/arch/arm/arm32/Makefile      |  1 +
>>   xen/arch/arm/arm32/smc.S         | 39 +++++++++++++++++++++++++++++++++++++++
>>   xen/arch/arm/arm64/Makefile      |  1 +
>>   xen/arch/arm/arm64/asm-offsets.c |  4 ++++
>>   xen/arch/arm/arm64/smc.S         | 30 ++++++++++++++++++++++++++++++
>>   xen/include/asm-arm/processor.h  | 11 +++++++++++
>>   6 files changed, 86 insertions(+)
>>   create mode 100644 xen/arch/arm/arm32/smc.S
>>   create mode 100644 xen/arch/arm/arm64/smc.S
> 
> With this diffstat, why did I end up on the _To_ list of this patch?
> I shouldn't even have been on the Cc list (and I'm going to blindly
> delete all other patches of this series where you've also
> apparently put me on the To list). Please remember that patches
> are supposed to be sent _To_ the list, with maintainers, reviewers
> (and perhaps others) Cc-ed. Together with people replying to
> mails often not adjusting To/Cc lists accordingly, I've now ended
> up with well over 30 mails in my primary mail folder which
> presumably I have no business with at all.

My apologies for disturbing you. I really have no intention to upset you.
I used get_maintainer.pl script and it showed your e-mail address. So I 
added you to To: list.

Now I tried add_mandainers.pl and it nevertheless adds you to CCs.
So, I'm really sorry bothering you. But probably you might want to fix 
mentioned scripts/MAINTAINERS  file, to make sure you will never receive 
unwanted emails?

-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-27 19:24     ` Volodymyr Babchuk
@ 2018-08-27 20:19       ` Julien Grall
  2018-08-28  6:09       ` Jan Beulich
  1 sibling, 0 replies; 32+ messages in thread
From: Julien Grall @ 2018-08-27 20:19 UTC (permalink / raw)
  To: Volodymyr Babchuk, Jan Beulich; +Cc: Lars Kurth, nd, xen-devel

Hello,


On 27/08/2018 20:24, Volodymyr Babchuk wrote:
> 
> 
> On 27.08.18 09:44, Jan Beulich wrote:
> 
> [...]
> 
>>>   xen/arch/arm/arm32/Makefile      |  1 +
>>>   xen/arch/arm/arm32/smc.S         | 39 
>>> +++++++++++++++++++++++++++++++++++++++
>>>   xen/arch/arm/arm64/Makefile      |  1 +
>>>   xen/arch/arm/arm64/asm-offsets.c |  4 ++++
>>>   xen/arch/arm/arm64/smc.S         | 30 ++++++++++++++++++++++++++++++
>>>   xen/include/asm-arm/processor.h  | 11 +++++++++++
>>>   6 files changed, 86 insertions(+)
>>>   create mode 100644 xen/arch/arm/arm32/smc.S
>>>   create mode 100644 xen/arch/arm/arm64/smc.S
>>
>> With this diffstat, why did I end up on the _To_ list of this patch?
>> I shouldn't even have been on the Cc list (and I'm going to blindly
>> delete all other patches of this series where you've also
>> apparently put me on the To list). Please remember that patches
>> are supposed to be sent _To_ the list, with maintainers, reviewers
>> (and perhaps others) Cc-ed. Together with people replying to
>> mails often not adjusting To/Cc lists accordingly, I've now ended
>> up with well over 30 mails in my primary mail folder which
>> presumably I have no business with at all.
> 
> My apologies for disturbing you. I really have no intention to upset you.
> I used get_maintainer.pl script and it showed your e-mail address. So I 
> added you to To: list.
> 
> Now I tried add_mandainers.pl and it nevertheless adds you to CCs.
> So, I'm really sorry bothering you. But probably you might want to fix 
> mentioned scripts/MAINTAINERS  file, to make sure you will never receive 
> unwanted emails?

I did try add_maintainers.pl/get_maintainer.pl on that patch and I get
only Stefano and I CCed. So more likely you are not using the scripts correctly.

42sh> ls *.patch
0001-arm-add-SMC-wrapper-that-is-compatible-with-SMCCC.patch

42sh> scripts/get_maintainers.pl 0001-arm-add-SMC-wrapper-that-is-compatible-with-SMCCC.patch
Stefano Stabellini <sstabellini@kernel.org>
Julien Grall <julien.grall@arm.com>
xen-devel@lists.xenproject.org

42sh> scripts/add_maintainers.pl -d .
Processing: 0001-arm-add-SMC-wrapper-that-is-compatible-with-SMCCC.patch
Then perform:
git send-email -to xen-devel@lists.xenproject.org ./*.patch

42sh> ack "Cc|To" 0001-arm-add-SMC-wrapper-that-is-compatible-with-SMCCC.patch
To: xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>

Note that we fixed one issue in add_maintainers.pl that would lead to your
problem. However, this was merged nearly 3 months ago and I would expect
your series to be based on staging...

So you probably want to do upstream development on staging or change
your command line. If the documentation is misleading,
then contribution will be appreciated.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-27 19:24     ` Volodymyr Babchuk
  2018-08-27 20:19       ` Julien Grall
@ 2018-08-28  6:09       ` Jan Beulich
  1 sibling, 0 replies; 32+ messages in thread
From: Jan Beulich @ 2018-08-28  6:09 UTC (permalink / raw)
  To: Volodymyr Babchuk; +Cc: Lars Kurth, xen-devel

>>> On 27.08.18 at 21:24, <volodymyr_babchuk@epam.com> wrote:
> Hi Jan,
> 
> 
> On 27.08.18 09:44, Jan Beulich wrote:
> 
> [...]
> 
>>>   xen/arch/arm/arm32/Makefile      |  1 +
>>>   xen/arch/arm/arm32/smc.S         | 39 
> +++++++++++++++++++++++++++++++++++++++
>>>   xen/arch/arm/arm64/Makefile      |  1 +
>>>   xen/arch/arm/arm64/asm-offsets.c |  4 ++++
>>>   xen/arch/arm/arm64/smc.S         | 30 ++++++++++++++++++++++++++++++
>>>   xen/include/asm-arm/processor.h  | 11 +++++++++++
>>>   6 files changed, 86 insertions(+)
>>>   create mode 100644 xen/arch/arm/arm32/smc.S
>>>   create mode 100644 xen/arch/arm/arm64/smc.S
>> 
>> With this diffstat, why did I end up on the _To_ list of this patch?
>> I shouldn't even have been on the Cc list (and I'm going to blindly
>> delete all other patches of this series where you've also
>> apparently put me on the To list). Please remember that patches
>> are supposed to be sent _To_ the list, with maintainers, reviewers
>> (and perhaps others) Cc-ed. Together with people replying to
>> mails often not adjusting To/Cc lists accordingly, I've now ended
>> up with well over 30 mails in my primary mail folder which
>> presumably I have no business with at all.
> 
> My apologies for disturbing you. I really have no intention to upset you.
> I used get_maintainer.pl script and it showed your e-mail address. So I 
> added you to To: list.

On top of Julien's reply this _still_ leaves the question of why
you've added me to the To list then, rather than the Cc one.

And btw - script and other bugs happen. Please add some salt
to their output.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 2/6] arm: add generic TEE mediator framework
  2018-08-27 19:09     ` Volodymyr Babchuk
@ 2018-08-28 11:14       ` Julien Grall
  0 siblings, 0 replies; 32+ messages in thread
From: Julien Grall @ 2018-08-28 11:14 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel, Stefano Stabellini; +Cc: tee-dev

On 27/08/18 20:09, Volodymyr Babchuk wrote:
> Hi Julien,

Hi,

> 
> On 22.08.18 20:03, Julien Grall wrote:
> 
> [...]
> 
>>>       if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
>>>           goto fail;
>>> +    /* Notify TEE that new domain was created */
>>> +    tee_domain_create(d);
>>
>> My concern about domain creation is still not addressed. I would 
>> expect the toolstack to decide whether TEE should be initialized for a 
>> given guest and potentially return an error on failure (e.g maximum 
>> client ID has been reached).
>>
>> But very likely, you don't need to initialize TEE that early. This 
>> could be done in a separate DOMCTL as we did for VPL011.
> 
> Yes, as we discussed in latter patches, I'll add DOMCTL support.But 
> what to do with dom0 construction?
> I think, it should be configurable. But how? With commandline option?

There are an high chance that OP-TEE will be required by Dom0 for some 
devices. So I think it is fine to have OP-TEE enabled by default for Dom0.

If you want to override the behavior, then a command-line option looks 
the best.

>>> +    printk(XENLOG_WARNING "No TEE mediator found\n");
>>
>> Not having a TEE is a valid use case. So printing a warning seems a 
>> bit too much.
> 
> I can change this to INFO. Or it is better to remove this print at all?

I would prefer the message to be removed. The message "Using TEE 
mediator..." is sufficient as if it is not printed it means there are no 
TEE.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-22 16:46   ` Julien Grall
  2018-08-23 14:35     ` Volodymyr Babchuk
@ 2018-08-30 14:48     ` Volodymyr Babchuk
  2018-08-30 16:43       ` Julien Grall
  1 sibling, 1 reply; 32+ messages in thread
From: Volodymyr Babchuk @ 2018-08-30 14:48 UTC (permalink / raw)
  To: Julien Grall, xen-devel

Hi Julien,

On 22.08.18 19:46, Julien Grall wrote:

[...]
>> +++ b/xen/arch/arm/arm32/smc.S
>> @@ -0,0 +1,39 @@
>> +/*
>> + * xen/arch/arm/arm32/smc.S
>> + *
>> + * Wrapper for Secure Monitors Calls
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
> 
> Xen is GPLv2 only. Can you please update the copyright accordingly?
>
I'll fix this, no problem. But I can see number of files with this 
clause "either version 2 of the License, or (at your option) any later 
version".

Do maintainers/code owners plan to update existing files?
Also, are there any plans to switch to SPDX-Licence-Identifier?

-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
  2018-08-30 14:48     ` Volodymyr Babchuk
@ 2018-08-30 16:43       ` Julien Grall
  0 siblings, 0 replies; 32+ messages in thread
From: Julien Grall @ 2018-08-30 16:43 UTC (permalink / raw)
  To: Volodymyr Babchuk, xen-devel



On 30/08/18 15:48, Volodymyr Babchuk wrote:
> Hi Julien,

Hi,

> 
> On 22.08.18 19:46, Julien Grall wrote:
> 
> [...]
>>> +++ b/xen/arch/arm/arm32/smc.S
>>> @@ -0,0 +1,39 @@
>>> +/*
>>> + * xen/arch/arm/arm32/smc.S
>>> + *
>>> + * Wrapper for Secure Monitors Calls
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>
>> Xen is GPLv2 only. Can you please update the copyright accordingly?
>>
> I'll fix this, no problem. But I can see number of files with this 
> clause "either version 2 of the License, or (at your option) any later 
> version".
> 
> Do maintainers/code owners plan to update existing files?

It is not easy to re-license of a file. You need to get an agreement 
from all the contributors of the file.  Lars did try in the past, but 
this didn't receive any support from the community.

> Also, are there any plans to switch to SPDX-Licence-Identifier?

I am not aware of any plan for this. But that would definitely avoid 
licensing issues because of the wording. This would need to be discussed 
within the community before going forward.

Feel free to kick a thread for it with the committers in CC.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2018-08-30 16:43 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-22 14:11 [PATCH v1 0/6] TEE mediator (and OP-TEE) support in XEN Volodymyr Babchuk
2018-08-22 14:11 ` [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC Volodymyr Babchuk
2018-08-22 16:46   ` Julien Grall
2018-08-23 14:35     ` Volodymyr Babchuk
2018-08-23 14:45       ` Julien Grall
2018-08-23 15:16         ` Volodymyr Babchuk
2018-08-23 15:31           ` Julien Grall
2018-08-30 14:48     ` Volodymyr Babchuk
2018-08-30 16:43       ` Julien Grall
2018-08-27  6:44   ` Jan Beulich
2018-08-27 19:24     ` Volodymyr Babchuk
2018-08-27 20:19       ` Julien Grall
2018-08-28  6:09       ` Jan Beulich
2018-08-22 14:11 ` [PATCH v1 2/6] arm: add generic TEE mediator framework Volodymyr Babchuk
2018-08-22 17:03   ` Julien Grall
2018-08-27 19:09     ` Volodymyr Babchuk
2018-08-28 11:14       ` Julien Grall
2018-08-22 14:11 ` [PATCH v1 3/6] arm: tee: add OP-TEE header files Volodymyr Babchuk
2018-08-22 14:11 ` [PATCH v1 4/6] optee: add OP-TEE mediator Volodymyr Babchuk
2018-08-22 17:28   ` Julien Grall
2018-08-23 14:27     ` Volodymyr Babchuk
2018-08-23 15:28       ` Julien Grall
2018-08-22 14:11 ` [PATCH v1 5/6] libxl: create DTS node for OP-TEE if it is enabled Volodymyr Babchuk
2018-08-22 17:03   ` Wei Liu
2018-08-22 17:32   ` Julien Grall
2018-08-23 14:03     ` Volodymyr Babchuk
2018-08-23 14:11       ` Julien Grall
2018-08-23 14:16         ` Volodymyr Babchuk
2018-08-22 14:11 ` [PATCH v1 6/6] xsm: add tee access policy support Volodymyr Babchuk
2018-08-23 13:43   ` Julien Grall
2018-08-23 13:57     ` Volodymyr Babchuk
2018-08-23 14:08       ` Julien Grall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.