Re: [PATCH v4 09/10] tools/arm: tee: add "tee" option for xl.cfg

[Julien Grall <julien.grall@arm.com>]

(+ Achin)

On 07/03/2019 21:04, Volodymyr Babchuk wrote:
> From: Volodymyr Babchuk <vlad.babchuk@gmail.com>
> 
> This enumeration controls TEE type for a domain. Currently there is
> two possible options: either 'none' or 'native'.
> 
> 'none' is the default value and it basically disables TEE support at
> all.
> 
> 'native' enables access to a "real" TEE installed on a platform.

I am aware I made that suggestion. But I think the naming is not ideal between 
the user and the toolstack. The question is how this is going to fit with the 
S-EL2 feature where multiple TEE can run together?

I have CCed Achin to see he has any vision how this could be interfaced.

> 
> It is possible to add another types in the future.
> 
> Signed-off-by: Volodymyr Babchuk <vlad.babchuk@gmail.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 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    | 12 ++++++++++++
>   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, 45 insertions(+)
> 
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index ad81af1ed8..e15981882b 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -2702,6 +2702,18 @@ Currently, only the "sbsa_uart" model is supported for ARM.
>   
>   =back
>   
> +=over 4
> +
> +=item B<tee=["none", "native"]>
> +
> +Set TEE type for the guest. Currently only OP-TEE is supported. If
> +this option is set to "native", xl will create guest, which can access
> +native TEE on your system (just make sure that you are using OP-TEE
> +with virtualization support endabled). Also OP-TEE node will be
> +emitted into guest's device tree.
> +
> +=back
> +
>   =head3 x86
>   
>   =over 4
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index a38e5cdba2..b24e4141b1 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..6930d0ab3b 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.arch_arm.tee) {
> +    case LIBXL_TEE_TYPE_NONE:
> +        config->arch.tee_type = XEN_DOMCTL_CONFIG_TEE_NONE;
> +        break;
> +    case LIBXL_TEE_TYPE_NATIVE:
> +        config->arch.tee_type = XEN_DOMCTL_CONFIG_TEE_NATIVE;
> +        break;
> +    default:
> +        LOG(ERROR, "Unknown TEE type %d",
> +            d_config->b_info.arch_arm.tee);
> +        return ERROR_FAIL;
> +    }
> +
>       return 0;
>   }
>   
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index b685ac47ac..4f1eb229b8 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -457,6 +457,11 @@ libxl_gic_version = Enumeration("gic_version", [
>       (0x30, "v3")
>       ], init_val = "LIBXL_GIC_VERSION_DEFAULT")
>   
> +libxl_tee_type = Enumeration("tee_type", [
> +    (0, "none"),
> +    (1, "native")
> +    ], init_val = "LIBXL_TEE_TYPE_NONE")
> +
>   libxl_rdm_reserve = Struct("rdm_reserve", [
>       ("strategy",    libxl_rdm_reserve_strategy),
>       ("policy",      libxl_rdm_reserve_policy),
> @@ -615,6 +620,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>   
>       ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
>                                  ("vuart", libxl_vuart_type),
> +                               ("tee",  libxl_tee_type),

AFAICT, TEE also exists on other architecture. So I am wondering whether this 
field should be moved out of arch_arm?

Cheers,

-- 
Julien Grall

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