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

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hello Julien,

Julien Grall writes:

> On 07/03/2019 21:04, Volodymyr Babchuk wrote:
>> From: Volodymyr Babchuk <vlad.babchuk@xxxxxxxxx>
>>
>> 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?
You see, support for S-EL2 or support for multiple TEEs is a much broader
topic. For me, naming for configuration option is the least important
thing in this case :-)

Right there is no clear vision how it will ever work. I scanned through
SPCI spec and I already have a couple of questions. I need to study it
harder to make serious statements, but I already see that current
mediator framework will hardly fit into SPCI stuff. Frankly, I have
concerns that OP-TEE (or any other existing TEE) will be compatible
with SPCI-enabled systems without major rework. So, we will need to do
big overhaul anyways, when there will appear first SPCI-compatible TEEs and
secure hypervisors.

AFAIK, there is no ARMv8.4 platforms, no S-EL2 hypervisors and so on. It
will take at least couple of years before S-EL2 will hit the market. So
in my opinion it is too early to make any assumptions on how to support
all this in Xen. Lets stick to the current matters.

I'm not insisting on "native". But I can't invent something better right
now. Probably, SPCI-enabled TEE also will be considered "native" as
opposed to, say, "emulated".

As I said, I can't come with anything better than "native". But I'm open
to any suggestions.

> 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@xxxxxxxxx>
>> ---
>>
>>   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?
In v3 thread you asked if I see any use for x86. Because in that version
this option was in common section. Honestly, I don't see any use there,
because I have no idea how this can be implemented on x86. I checked
Open-TEE implementation as an example, but it appeared that it works in
userpsace.

So, I would stick to ARM-only. But I have no strong objections, if x86
folks are willing to support TEEs in the common code.

--
Best regards,Volodymyr Babchuk
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel