diff mbox series

[v5,09/10] tools/arm: tee: add "tee" option for xl.cfg

Message ID 20190521212530.12706-10-volodymyr_babchuk@epam.com (mailing list archive)
State Superseded
Headers show
Series TEE mediator (and OP-TEE) support in XEN | expand

Commit Message

Volodymyr Babchuk May 21, 2019, 9:26 p.m. UTC
This enumeration controls TEE type for a domain. Currently there is
two possible options: either 'none' or 'optee'.

'none' is the default value and it basically disables TEE support at
all.

'native' enables access to a "real" OP-TEE installed on a platform.

It is possible to add another types in the future.

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

 All the patches to optee.c should be merged together. They were
 split to ease up review. But they depend heavily on each other.

 Changes from v4:
  - "native" option was replaced with "optee"
  - "tee" property was moved from arch-specific section to the
     global one. Documentation moved inside "Devices" section.

 Changes from v3:
  - tee_enabled renamed to tee_type. Currently two types are supported
    as described in the commit message
  - Add LIBXL_HAVE_BUILDINFO_ARCH_ARM_TEE definition

 Changes from v2:
  - Use arch.tee_enabled instead of separate domctl
---
 docs/man/xl.cfg.5.pod.in    | 19 +++++++++++++++++++
 tools/libxl/libxl.h         |  5 +++++
 tools/libxl/libxl_arm.c     | 13 +++++++++++++
 tools/libxl/libxl_types.idl |  6 ++++++
 tools/xl/xl_parse.c         |  9 +++++++++
 5 files changed, 52 insertions(+)

Comments

Julien Grall June 3, 2019, 12:44 p.m. UTC | #1
Hi Volodymyr,

Some comment on the documentation, the rest looks good to me.


On 21/05/2019 22:26, Volodymyr Babchuk wrote:
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index c7d70e618b..73c64dc896 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -1544,6 +1544,25 @@ Set maximum height for pointer device.
>   
>   =back
>   
> +=item B<tee=["none", "optee"]>

This will become quite difficult to read if we add more TEE. How about:

<tee="STRING">?

> +
> +Set TEE type for the guest. TEE is a Trusted Execution Environment -- separate
> +secuse OS found on some platforms.

s/secuse/secure/

I would also mention this is Arm only so far. Maybe:

B<Arm only> Set TEE...

> +
> +=over 4
> +
> +=item B<"none">
> +
> +Disable TEE support at all. This is the default value.
> +
> +=item B<"optee">
> +
> +Allow guest to access to OP-TEE enabled on the platform. Guest will not be created
> +if platform does not have OP-TEE with virtualization feature or if OP-TEE will
> +deny access.

I have some trouble to read this paragraph. How about:

"Allow a guest to use OP-TEE. Note that a virtualization-aware OP-TEE is 
required for this.".

> +
> +=back
> +
>   =back
>   
>   =head2 Paravirtualised (PV) Guest Specific Options

Cheers,
Ian Jackson June 3, 2019, 2:47 p.m. UTC | #2
Volodymyr Babchuk writes ("[Xen-devel] [PATCH v5 09/10] tools/arm: tee: add "tee" option for xl.cfg"):
> This enumeration controls TEE type for a domain. Currently there is
> two possible options: either 'none' or 'optee'.
> 
> 'none' is the default value and it basically disables TEE support at
> all.
> 
> 'native' enables access to a "real" OP-TEE installed on a platform.
> 
> It is possible to add another types in the future.

Could improve this bit maybe ?

> +=item B<"optee">
> +
> +Allow guest to access to OP-TEE enabled on the platform. Guest will not be created
> +if platform does not have OP-TEE with virtualization feature or if OP-TEE will
> +deny access.

To me (who doesn't really understand this stuff very well) this
doesn't answer a very important question: if I enable this, what (if
any) host/machine/&c resources will this grant the guest access to ?

It sounds like the the answer should be "none", because if I search
for "op-tee" online I would get the impression that it is an
emulator.  Normally granting access to an emulator does not grant
access to host resources.

But in this series you talk about it being a mediator so I suspect
that is not right.

Ian.
diff mbox series

Patch

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index c7d70e618b..73c64dc896 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -1544,6 +1544,25 @@  Set maximum height for pointer device.
 
 =back
 
+=item B<tee=["none", "optee"]>
+
+Set TEE type for the guest. TEE is a Trusted Execution Environment -- separate
+secuse OS found on some platforms.
+
+=over 4
+
+=item B<"none">
+
+Disable TEE support at all. This is the default value.
+
+=item B<"optee">
+
+Allow guest to access to OP-TEE enabled on the platform. Guest will not be created
+if platform does not have OP-TEE with virtualization feature or if OP-TEE will
+deny access.
+
+=back
+
 =back
 
 =head2 Paravirtualised (PV) Guest Specific Options
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 482499a6c0..294a92f645 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -273,6 +273,11 @@ 
  */
 #define LIBXL_HAVE_BUILDINFO_ARM_GIC_VERSION 1
 
+/*
+ * libxl_domain_build_info has the arch_arm.tee field.
+ */
+#define LIBXL_HAVE_BUILDINFO_ARCH_ARM_TEE 1
+
 /*
  * LIBXL_HAVE_SOFT_RESET indicates that libxl supports performing
  * 'soft reset' for domains and there is 'soft_reset' shutdown reason
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 141e159043..6b72c00960 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -89,6 +89,19 @@  int libxl__arch_domain_prepare_config(libxl__gc *gc,
         return ERROR_FAIL;
     }
 
+    switch (d_config->b_info.tee) {
+    case LIBXL_TEE_TYPE_NONE:
+        config->arch.tee_type = XEN_DOMCTL_CONFIG_TEE_NONE;
+        break;
+    case LIBXL_TEE_TYPE_OPTEE:
+        config->arch.tee_type = XEN_DOMCTL_CONFIG_TEE_OPTEE;
+        break;
+    default:
+        LOG(ERROR, "Unknown TEE type %d",
+            d_config->b_info.tee);
+        return ERROR_FAIL;
+    }
+
     return 0;
 }
 
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index cb4702fd7a..4eaccd2cc7 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -460,6 +460,11 @@  libxl_gic_version = Enumeration("gic_version", [
     (0x30, "v3")
     ], init_val = "LIBXL_GIC_VERSION_DEFAULT")
 
+libxl_tee_type = Enumeration("tee_type", [
+    (0, "none"),
+    (1, "optee")
+    ], init_val = "LIBXL_TEE_TYPE_NONE")
+
 libxl_rdm_reserve = Struct("rdm_reserve", [
     ("strategy",    libxl_rdm_reserve_strategy),
     ("policy",      libxl_rdm_reserve_policy),
@@ -537,6 +542,7 @@  libxl_domain_build_info = Struct("domain_build_info",[
     ("nested_hvm",       libxl_defbool),
     ("apic",             libxl_defbool),
     ("dm_restrict",      libxl_defbool),
+    ("tee",              libxl_tee_type),
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 352cd214dd..d98ad0cffb 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2690,6 +2690,15 @@  skip_usbdev:
         }
     }
 
+    if (!xlu_cfg_get_string (config, "tee", &buf, 1)) {
+        e = libxl_tee_type_from_string(buf, &b_info->tee);
+        if (e) {
+            fprintf(stderr,
+                    "Unknown tee \"%s\" specified\n", buf);
+            exit(-ERROR_FAIL);
+        }
+    }
+
     parse_vkb_list(config, d_config);
 
     xlu_cfg_destroy(config);