Re: [Xen-devel] [PATCH v3 10/11] xl: add "tee" option for xl.cfg

[Julien Grall <julien.grall@xxxxxxx>]

Hi Volodymyr,

On 18/12/2018 21:11, Volodymyr Babchuk wrote:
> From: Volodymyr Babchuk <vlad.babchuk@xxxxxxxxx>
>
> This boolean option controls if TEE access is enabled for the domain.
> If access is enabled, xl will set appropriate flag in architecture
> configuration to ask hypervisor to enable TEE support.
>
> Signed-off-by: Volodymyr Babchuk <vlad.babchuk@xxxxxxxxx>
> ---
>   Changes from v2:
>   - Use arch.tee_enabled instead of separate domctl
>
>   docs/man/xl.cfg.pod.5.in    | 10 ++++++++++
>   tools/libxl/libxl_arm.c     |  2 ++
>   tools/libxl/libxl_create.c  |  1 +
>   tools/libxl/libxl_types.idl |  1 +
>   tools/xl/xl_parse.c         |  1 +
>   5 files changed, 15 insertions(+)
>
> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> index b1c0be14cd..9a7064c951 100644
> --- a/docs/man/xl.cfg.pod.5.in
> +++ b/docs/man/xl.cfg.pod.5.in
> @@ -2793,6 +2793,16 @@ Currently, only the "sbsa_uart" model is supported for 
> ARM.
>   
>   =back
>   
> +=over 4
> +
> +=item B<tee=BOOLEAN>
> +
> +Enable TEE support for the guest. Currently only OP-TEE is supported. If this
> +option is enabled, xl will create guest, which can access TEE. Also
> +OP-TEE node will be emitted into guest's device tree.
> +
> +=back
> +
>   =head3 x86
>   
>   =over 4
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 141e159043..f8b4ef55e0 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -89,6 +89,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>           return ERROR_FAIL;
>       }
>   
> +    config->arch.tee_enabled = libxl_defbool_val(d_config->b_info.tee);
> +
>       return 0;
>   }
>   
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index fa573344bc..bc9c4ee9ef 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -219,6 +219,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>   
>       libxl__arch_domain_build_info_setdefault(gc, b_info);
>       libxl_defbool_setdefault(&b_info->dm_restrict, false);
> +    libxl_defbool_setdefault(&b_info->tee, 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 51cf06a3a2..a634f6e704 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -613,6 +613,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),
> +    ("tee",  libxl_defbool),
I would appreciate if you address or at least reply to my comments on v2 [1]. 
For reminder:
The option TEE is described in the Arm section, but the option is declared for 
all architectures here. Do you see any use on x86?
Also, new option should be accompanied with a define (e.g 
LIBXL_HAVE_BUILDINFO_TEE) in libxl.h informing toolstack (e.g libvirt) that the 
option is available.
Lastly, I would consider to introduce an enum here with for now only the 
options: NONE, NATIVE. This would give us some freedom to potentially emulate 
TEE in the future or even choose the TEE (i.e in Secure EL2 case) without 
re-defining a new parameters.
Cheers,

[1] https://lists.xenproject.org/archives/html/xen-devel/2018-09/msg01004.html

--
Julien Grall

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