* Next Xen Arm Community call - Wednesday 22nd November @ 2017-11-16 11:54 Julien Grall 2017-11-16 12:47 ` Artem Mygaiev ` (3 more replies) 0 siblings, 4 replies; 22+ messages in thread From: Julien Grall @ 2017-11-16 11:54 UTC (permalink / raw) To: xen-devel, Edgar E. Iglesias, lars.kurth, scampbel, stewart.Hildebrand, Stefano Stabellini, anastassios.nanos, sgoel, vfachin, Jarvis Roach, volodymyr_babchuk, Artem Mygaiev Hi all, Apologies I was meant to organize the call earlier. I would suggest to have the next community call on Wednesday 22nd November 5pm GMT. Does it sound good? Do you have any specific topic you would like to discuss? Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Next Xen Arm Community call - Wednesday 22nd November 2017-11-16 11:54 Next Xen Arm Community call - Wednesday 22nd November Julien Grall @ 2017-11-16 12:47 ` Artem Mygaiev 2017-11-16 18:58 ` Stefano Stabellini 2017-11-20 18:05 ` Julien Grall ` (2 subsequent siblings) 3 siblings, 1 reply; 22+ messages in thread From: Artem Mygaiev @ 2017-11-16 12:47 UTC (permalink / raw) To: Julien Grall, xen-devel, Edgar E. Iglesias, lars.kurth, scampbel, stewart.Hildebrand, Stefano Stabellini, anastassios.nanos, sgoel, vfachin, Jarvis Roach, Volodymyr Babchuk [-- Attachment #1.1: Type: text/plain, Size: 758 bytes --] Hi Julien 22nd works good for us ________________________________ From: Julien Grall <julien.grall@linaro.org> Sent: Thursday, November 16, 2017 1:54:51 PM To: xen-devel; Edgar E. Iglesias; lars.kurth@citrix.com; scampbel@codeaurora.org; stewart.Hildebrand@dornerworks.com; Stefano Stabellini; anastassios.nanos@onapp.com; sgoel@codeaurora.org; vfachin@de.adit-jv.com; Jarvis Roach; Volodymyr Babchuk; Artem Mygaiev Subject: Next Xen Arm Community call - Wednesday 22nd November Hi all, Apologies I was meant to organize the call earlier. I would suggest to have the next community call on Wednesday 22nd November 5pm GMT. Does it sound good? Do you have any specific topic you would like to discuss? Cheers, -- Julien Grall [-- Attachment #1.2: Type: text/html, Size: 1532 bytes --] [-- Attachment #2: Type: text/plain, Size: 127 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Next Xen Arm Community call - Wednesday 22nd November 2017-11-16 12:47 ` Artem Mygaiev @ 2017-11-16 18:58 ` Stefano Stabellini 0 siblings, 0 replies; 22+ messages in thread From: Stefano Stabellini @ 2017-11-16 18:58 UTC (permalink / raw) To: Artem Mygaiev Cc: Edgar E. Iglesias, lars.kurth, Stefano Stabellini, scampbel, Julien Grall, anastassios.nanos, sgoel, stewart.Hildebrand, xen-devel, vfachin, Volodymyr Babchuk, Jarvis Roach [-- Attachment #1: Type: TEXT/PLAIN, Size: 995 bytes --] For me too, thanks! On Thu, 16 Nov 2017, Artem Mygaiev wrote: > Hi Julien > > > 22nd works good for us > > _______________________________________________________________________________________________________________________________________________________________________________ > From: Julien Grall <julien.grall@linaro.org> > Sent: Thursday, November 16, 2017 1:54:51 PM > To: xen-devel; Edgar E. Iglesias; lars.kurth@citrix.com; scampbel@codeaurora.org; stewart.Hildebrand@dornerworks.com; Stefano Stabellini; anastassios.nanos@onapp.com; > sgoel@codeaurora.org; vfachin@de.adit-jv.com; Jarvis Roach; Volodymyr Babchuk; Artem Mygaiev > Subject: Next Xen Arm Community call - Wednesday 22nd November > Hi all, > > Apologies I was meant to organize the call earlier. > > I would suggest to have the next community call on Wednesday 22nd > November 5pm GMT. Does it sound good? > > Do you have any specific topic you would like to discuss? > > Cheers, > > -- > Julien Grall > > [-- Attachment #2: Type: text/plain, Size: 127 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Next Xen Arm Community call - Wednesday 22nd November 2017-11-16 11:54 Next Xen Arm Community call - Wednesday 22nd November Julien Grall 2017-11-16 12:47 ` Artem Mygaiev @ 2017-11-20 18:05 ` Julien Grall 2017-11-21 14:34 ` Julien Grall 2017-11-27 17:01 ` [RFC] WIP: optee: add OP-TEE mediator Volodymyr Babchuk 3 siblings, 0 replies; 22+ messages in thread From: Julien Grall @ 2017-11-20 18:05 UTC (permalink / raw) To: xen-devel, Edgar E. Iglesias, Lars Kurth, Campbell Sean, stewart.Hildebrand, Stefano Stabellini, anastassios.nanos, Goel, Sameer, vfachin, Jarvis Roach, Volodymyr Babchuk, Artem Mygaiev, Andre Przywara, mirela.simonovic, davorin.mista Answering to myself. On 16 November 2017 at 11:54, Julien Grall <julien.grall@linaro.org> wrote: > Hi all, > > Apologies I was meant to organize the call earlier. > > I would suggest to have the next community call on Wednesday 22nd November > 5pm GMT. Does it sound good? > > Do you have any specific topic you would like to discuss? I would like to discuss about Power Saving when using Xen (e.g suspend, CPUFreq, idling). I will send the details of the call tomorrow. Cheers, _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Next Xen Arm Community call - Wednesday 22nd November 2017-11-16 11:54 Next Xen Arm Community call - Wednesday 22nd November Julien Grall 2017-11-16 12:47 ` Artem Mygaiev 2017-11-20 18:05 ` Julien Grall @ 2017-11-21 14:34 ` Julien Grall 2017-11-27 17:01 ` [RFC] WIP: optee: add OP-TEE mediator Volodymyr Babchuk 3 siblings, 0 replies; 22+ messages in thread From: Julien Grall @ 2017-11-21 14:34 UTC (permalink / raw) To: xen-devel, Edgar E. Iglesias, Lars Kurth, Campbell Sean, stewart.Hildebrand, Stefano Stabellini, anastassios.nanos, Goel, Sameer, vfachin, Jarvis Roach, Volodymyr Babchuk, Artem Mygaiev, Andre Przywara, Mirela Simonović, Davorin Mista Hi all, Quick reminder, the call will be tomorrow (Wednesday 22nd) at 5pm GMT. The details to join the call are: Call +44 1223 406065 (Local dial in) and enter the access code below followed by # key. Participant code: 4915191 Mobile Auto Dial: VoIP: voip://+441223406065;4915191# iOS devices: +44 1223 406065,4915191 and press # Other devices: +44 1223 406065x4915191# Additional Calling Information: UK +44 1142828002 US CA +1 4085761502 US TX +1 5123141073 JP +81 453455355 DE +49 8945604050 NO +47 73187518 SE +46 46313131 FR +33 497235101 TW +886 35657119 HU +36 13275600 IE +353 91337900 Toll Free UK 0800 1412084 US +1 8668801148 CN +86 4006782367 IN 0008009868365 IN +918049282778 TW 08000 22065 HU 0680981587 IE 1800800022 KF +972732558877 Cheers, On 16 November 2017 at 11:54, Julien Grall <julien.grall@linaro.org> wrote: > Hi all, > > Apologies I was meant to organize the call earlier. > > I would suggest to have the next community call on Wednesday 22nd November > 5pm GMT. Does it sound good? > > Do you have any specific topic you would like to discuss? > > Cheers, > > -- > Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC] WIP: optee: add OP-TEE mediator 2017-11-16 11:54 Next Xen Arm Community call - Wednesday 22nd November Julien Grall ` (2 preceding siblings ...) 2017-11-21 14:34 ` Julien Grall @ 2017-11-27 17:01 ` Volodymyr Babchuk 2017-12-01 22:58 ` Stefano Stabellini 3 siblings, 1 reply; 22+ messages in thread From: Volodymyr Babchuk @ 2017-11-27 17:01 UTC (permalink / raw) To: xen-devel, Julien Grall; +Cc: Volodymyr Babchuk This is follow-up to our conversation during community call. You asked me to send OP-TEE mediator as a patch, so we can discuss it in the mailing list. So, there it is. I squashed two patches into one and skipped patches that we already discussed. So, this is basically all what is needed to support OP-TEE in XEN. There are some TODOs left all over the code. But, I don't expect that TODOs implementation would significantly increase codebase. Currently mediator parses requests to perform addresses translation and that's all what is should be done to allow guests to work with OP-TEE. This become possible because I completely revisited virtualization support in OP-TEE. I have found way to enforce complete isolation between different guest states. This lifts many questions like usage quotas, RPC routing, sudden guest death, data isolation, etc. I'm aware that I didn't addressed all comments from previous discussion. Sorry for this. I'm currently busy with OP-TEE, and I think proper mediator implementation will be possible only after I'll stabilize OP-TEE part. So I don't ask anybody to do thorough review. I just want to share my status and discuss this code a whole. --- Add OP-TEE mediator as an example how TEE mediator framework works. 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 successfully passes own tests, while client is running in DomU. Currently it lacks some code for exceptional cases, because this patch was used mostly to debug virtualization in OP-TEE. Nevertheless, it provides all features needed for OP-TEE mediation. WARNING: This is a development patch, it does not cover all corner cases, so, please don't use it in production. It was tested on RCAR Salvator-M3 board. Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> --- xen/arch/arm/tee/Kconfig | 4 + xen/arch/arm/tee/Makefile | 1 + xen/arch/arm/tee/optee.c | 765 +++++++++++++++++++++++++++++++++++++++++++ xen/arch/arm/tee/optee_smc.h | 50 +++ 4 files changed, 820 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..7c6b5c6 100644 --- a/xen/arch/arm/tee/Kconfig +++ b/xen/arch/arm/tee/Kconfig @@ -0,0 +1,4 @@ +config ARM_OPTEE + bool "Enable OP-TEE mediator" + default n + depends on ARM_TEE diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile index c54d479..9d93b42 100644 --- a/xen/arch/arm/tee/Makefile +++ b/xen/arch/arm/tee/Makefile @@ -1 +1,2 @@ obj-y += tee.o +obj-$(CONFIG_ARM_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..59c3600 --- /dev/null +++ b/xen/arch/arm/tee/optee.c @@ -0,0 +1,765 @@ +/* + * xen/arch/arm/tee/optee.c + * + * OP-TEE mediator + * + * 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/domain_page.h> +#include <xen/types.h> +#include <xen/sched.h> + +#include <asm/p2m.h> +#include <asm/tee.h> + +#include "optee_msg.h" +#include "optee_smc.h" + +/* + * Global TODO: + * 1. Create per-domain context, where call and shm will be stored + * 2. Pin pages shared between OP-TEE and guest + */ +/* + * OP-TEE violates SMCCC when it defines own UID. So we need + * to place bytes in correct order. + */ +#define OPTEE_UID (xen_uuid_t){{ \ + (uint8_t)(OPTEE_MSG_UID_0 >> 0), (uint8_t)(OPTEE_MSG_UID_0 >> 8), \ + (uint8_t)(OPTEE_MSG_UID_0 >> 16), (uint8_t)(OPTEE_MSG_UID_0 >> 24), \ + (uint8_t)(OPTEE_MSG_UID_1 >> 0), (uint8_t)(OPTEE_MSG_UID_1 >> 8), \ + (uint8_t)(OPTEE_MSG_UID_1 >> 16), (uint8_t)(OPTEE_MSG_UID_1 >> 24), \ + (uint8_t)(OPTEE_MSG_UID_2 >> 0), (uint8_t)(OPTEE_MSG_UID_2 >> 8), \ + (uint8_t)(OPTEE_MSG_UID_2 >> 16), (uint8_t)(OPTEE_MSG_UID_2 >> 24), \ + (uint8_t)(OPTEE_MSG_UID_3 >> 0), (uint8_t)(OPTEE_MSG_UID_3 >> 8), \ + (uint8_t)(OPTEE_MSG_UID_3 >> 16), (uint8_t)(OPTEE_MSG_UID_3 >> 24), \ + }} + +#define MAX_NONCONTIG_ENTRIES 8 + +/* + * Call context. OP-TEE can issue mulitple 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]; + int optee_thread_id; + int rpc_op; + domid_t domid; +}; +static LIST_HEAD(call_ctx_list); +static DEFINE_SPINLOCK(call_ctx_list_lock); + +/* + * Command buffer shared between OP-TEE and guest. + * Warning! In the proper implementation this SHM buffer *probably* should + * by shadowed by XEN. + * TODO: Reconsider this. + */ +struct shm { + struct list_head list; + struct optee_msg_arg *guest_arg; + struct page *guest_page; + mfn_t guest_mfn; + uint64_t cookie; + domid_t domid; +}; + +static LIST_HEAD(shm_list); +static DEFINE_SPINLOCK(shm_list_lock); + +static int optee_init(void) +{ + printk("OP-TEE mediator init done\n"); + return 0; +} + +static void optee_domain_create(struct domain *d) +{ + register_t resp[4]; + call_smccc_smc(OPTEE_SMC_VM_CREATED, + d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); + if ( resp[0] != OPTEE_SMC_RETURN_OK ) + gprintk(XENLOG_WARNING, "OP-TEE don't want to support domain: %d\n", + (uint32_t)resp[0]); + /* TODO: Change function declaration to be able to retun error */ +} + +static void optee_domain_destroy(struct domain *d) +{ + register_t resp[4]; + call_smccc_smc(OPTEE_SMC_VM_DESTROYED, + d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); + /* TODO: Clean call contexts and SHMs associated with domain */ +} + +static bool forward_call(struct cpu_user_regs *regs) +{ + register_t resp[4]; + + /* TODO: Use separate registers set to prevent leakage to guest */ + 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), + /* VM id 0 is reserved for hypervisor itself */ + current->domain->domain_id + 1, + resp); + + set_user_reg(regs, 0, resp[0]); + set_user_reg(regs, 1, resp[1]); + set_user_reg(regs, 2, resp[2]); + set_user_reg(regs, 3, resp[3]); + + return true; +} + +static struct std_call_ctx *allocate_std_call_ctx(void) +{ + struct std_call_ctx *ret; + + ret = xzalloc(struct std_call_ctx); + if ( !ret ) + return NULL; + + ret->optee_thread_id = -1; + ret->domid = -1; + + spin_lock(&call_ctx_list_lock); + list_add_tail(&ret->list, &call_ctx_list); + spin_unlock(&call_ctx_list_lock); + + return ret; +} + +static void free_std_call_ctx(struct std_call_ctx *ctx) +{ + spin_lock(&call_ctx_list_lock); + list_del(&ctx->list); + spin_unlock(&call_ctx_list_lock); + + if (ctx->xen_arg) + free_xenheap_page(ctx->xen_arg); + + if (ctx->guest_arg) + unmap_domain_page(ctx->guest_arg); + + for (int i = 0; i < MAX_NONCONTIG_ENTRIES; i++) { + if (ctx->non_contig[i]) + free_xenheap_pages(ctx->non_contig[i], ctx->non_contig_order[i]); + } + + xfree(ctx); +} + +static struct std_call_ctx *find_ctx(int thread_id, domid_t domid) +{ + struct std_call_ctx *ctx; + + spin_lock(&call_ctx_list_lock); + list_for_each_entry( ctx, &call_ctx_list, list ) + { + if (ctx->domid == domid && ctx->optee_thread_id == thread_id ) + { + spin_unlock(&call_ctx_list_lock); + return ctx; + } + } + spin_unlock(&call_ctx_list_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 mfn_t lookup_guest_ram_addr(paddr_t gaddr) +{ + mfn_t mfn; + gfn_t gfn; + p2m_type_t t; + gfn = gaddr_to_gfn(gaddr); + mfn = p2m_lookup(current->domain, gfn, &t); + if ( t != p2m_ram_rw || mfn_eq(mfn, INVALID_MFN) ) { + gprintk(XENLOG_INFO, "Domain tries to use invalid gfn\n"); + return INVALID_MFN; + } + return mfn; +} + +static struct shm *allocate_and_map_shm(paddr_t gaddr, uint64_t cookie) +{ + struct shm *ret; + + ret = xzalloc(struct shm); + if ( !ret ) + return NULL; + + ret->guest_mfn = lookup_guest_ram_addr(gaddr); + + if ( mfn_eq(ret->guest_mfn, INVALID_MFN) ) + { + xfree(ret); + return NULL; + } + + ret->guest_arg = map_domain_page(ret->guest_mfn); + if ( !ret->guest_arg ) + { + gprintk(XENLOG_INFO, "Could not map domain page\n"); + xfree(ret); + return NULL; + } + ret->cookie = cookie; + ret->domid = current->domain->domain_id; + + spin_lock(&shm_list_lock); + list_add_tail(&ret->list, &shm_list); + spin_unlock(&shm_list_lock); + return ret; +} + +static void free_shm(uint64_t cookie, domid_t domid) +{ + struct shm *shm, *found = NULL; + spin_lock(&shm_list_lock); + + list_for_each_entry( shm, &shm_list, list ) + { + if (shm->domid == domid && shm->cookie == cookie ) + { + found = shm; + list_del(&found->list); + break; + } + } + spin_unlock(&shm_list_lock); + + if ( !found ) { + return; + } + + if ( found->guest_arg ) + unmap_domain_page(found->guest_arg); + + xfree(found); +} + +static struct shm *find_shm(uint64_t cookie, domid_t domid) +{ + struct shm *shm; + + spin_lock(&shm_list_lock); + list_for_each_entry( shm, &shm_list, list ) + { + if ( shm->domid == domid && shm->cookie == cookie ) + { + spin_unlock(&shm_list_lock); + return shm; + } + } + spin_unlock(&shm_list_lock); + + return NULL; +} + +static bool translate_noncontig(struct std_call_ctx *ctx, + struct optee_msg_param *param, + int idx) +{ + /* + * Refer to OPTEE_MSG_ATTR_NONCONTIG description in optee_msg.h for details. + * + * WARNING: This is test code. It works only with xen page size == 4096 + */ + 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; + + 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; + + gaddr = param->u.tmem.buf_ptr & ~(OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1); + guest_mfn = lookup_guest_ram_addr(gaddr); + 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 ) { + mfn_t entry_mfn = lookup_guest_ram_addr( + pages_data_guest->pages_list[entries_on_page]); + + if ( mfn_eq(entry_mfn, INVALID_MFN) ) + goto err_unmap; + + 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); + guest_mfn = lookup_guest_ram_addr(gaddr); + 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--; + } + + unmap_domain_page(pages_data_guest); + + param->u.tmem.buf_ptr = virt_to_maddr(pages_data_xen_start) | page_offset; + + ctx->non_contig[idx] = pages_data_xen_start; + ctx->non_contig_order[idx] = order; + + unmap_domain_page(pages_data_guest); + return true; + +err_unmap: + unmap_domain_page(pages_data_guest); +err_free: + free_xenheap_pages(pages_data_xen_start, order); + return false; +} + +static bool translate_params(struct std_call_ctx *ctx) +{ + unsigned int i; + uint32_t attr; + + for ( i = 0; i < ctx->xen_arg->num_params; i++ ) { + attr = ctx->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, ctx->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 *ctx) +{ + paddr_t cmd_gaddr, xen_addr; + mfn_t cmd_mfn; + + 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; + + cmd_mfn = lookup_guest_ram_addr(cmd_gaddr); + if ( mfn_eq(cmd_mfn, INVALID_MFN) ) + return false; + + ctx->guest_arg = map_domain_page(cmd_mfn); + if ( !ctx->guest_arg ) + return false; + + ctx->xen_arg = alloc_xenheap_page(); + if ( !ctx->xen_arg ) + return false; + + memcpy(ctx->xen_arg, ctx->guest_arg, OPTEE_MSG_NONCONTIG_PAGE_SIZE); + + xen_addr = virt_to_maddr(ctx->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 cpu_user_regs *regs, + struct std_call_ctx *ctx) +{ + unsigned int i; + uint32_t attr; + + ctx->guest_arg->ret = ctx->xen_arg->ret; + ctx->guest_arg->ret_origin = ctx->xen_arg->ret_origin; + ctx->guest_arg->session = ctx->xen_arg->session; + for ( i = 0; i < ctx->xen_arg->num_params; i++ ) { + attr = ctx->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: + ctx->guest_arg->params[i].u.tmem.size = + ctx->xen_arg->params[i].u.tmem.size; + continue; + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: + ctx->guest_arg->params[i].u.value.a = + ctx->xen_arg->params[i].u.value.a; + ctx->guest_arg->params[i].u.value.b = + ctx->xen_arg->params[i].u.value.b; + continue; + case OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT: + case OPTEE_MSG_ATTR_TYPE_RMEM_INOUT: + ctx->guest_arg->params[i].u.rmem.size = + ctx->xen_arg->params[i].u.rmem.size; + continue; + case OPTEE_MSG_ATTR_TYPE_NONE: + case OPTEE_MSG_ATTR_TYPE_TMEM_INPUT: + case OPTEE_MSG_ATTR_TYPE_RMEM_INPUT: + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: + continue; + } + } + return true; +} + +static bool execute_std_call(struct cpu_user_regs *regs, + struct std_call_ctx *ctx) +{ + register_t optee_ret; + forward_call(regs); + optee_ret = get_user_reg(regs, 0); + + if ( OPTEE_SMC_RETURN_IS_RPC(optee_ret) ) + { + ctx->optee_thread_id = get_user_reg(regs, 3); + ctx->rpc_op = OPTEE_SMC_RETURN_GET_RPC_FUNC(optee_ret); + return true; + } + + copy_std_request_back(regs, ctx); + free_std_call_ctx(ctx); + + return true; +} + +static bool handle_std_call(struct cpu_user_regs *regs) +{ + struct std_call_ctx *ctx; + bool ret; + + ctx = allocate_std_call_ctx(); + + if (!ctx) + return false; + + ctx->domid = current->domain->domain_id; + + ret = copy_std_request(regs, ctx); + if ( !ret ) + goto out; + + /* Now we can safely examine contents of command buffer */ + if ( OPTEE_MSG_GET_ARG_SIZE(ctx->xen_arg->num_params) > + OPTEE_MSG_NONCONTIG_PAGE_SIZE ) { + ret = false; + goto out; + } + + switch ( ctx->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); + break; + default: + ret = false; + } + + if (!ret) + goto out; + + ret = execute_std_call(regs, ctx); + +out: + if (!ret) + free_std_call_ctx(ctx); + + return ret; +} + +static void handle_rpc_cmd_alloc(struct cpu_user_regs *regs, + struct std_call_ctx *ctx, + struct shm *shm) +{ + if ( shm->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 ( ctx->non_contig[MAX_NONCONTIG_ENTRIES - 1] ) + { + free_xenheap_pages(ctx->non_contig[7], + ctx->non_contig_order[MAX_NONCONTIG_ENTRIES - 1]); + ctx->non_contig[7] = NULL; + } + translate_noncontig(ctx, shm->guest_arg->params + 0, + MAX_NONCONTIG_ENTRIES - 1); +} + +static void handle_rpc_cmd(struct cpu_user_regs *regs, struct std_call_ctx *ctx) +{ + struct shm *shm; + uint64_t cookie; + + cookie = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2); + + shm = find_shm(cookie, current->domain->domain_id); + + if ( !shm ) + { + gprintk(XENLOG_ERR, "Can't find SHM with cookie %lx\n", cookie); + return; + } + + switch (shm->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(regs, ctx, shm); + break; + case OPTEE_MSG_RPC_CMD_SHM_FREE: + break; + default: + break; + } +} + +static void handle_rpc_func_alloc(struct cpu_user_regs *regs, + struct std_call_ctx *ctx) +{ + 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 *shm; + + shm = allocate_and_map_shm(ptr, cookie); + if ( !shm ) + { + gprintk(XENLOG_WARNING, "Failed to callocate allocate SHM\n"); + ptr = 0; + } + else + ptr = mfn_to_maddr(shm->guest_mfn); + + set_user_reg(regs, 1, ptr >> 32); + set_user_reg(regs, 2, ptr & 0xFFFFFFFF); + } +} + +static bool handle_rpc(struct cpu_user_regs *regs) +{ + struct std_call_ctx *ctx; + + int optee_thread_id = get_user_reg(regs, 3); + + ctx = find_ctx(optee_thread_id, current->domain->domain_id); + + if (!ctx) + return false; + + switch ( ctx->rpc_op ) { + case OPTEE_SMC_RPC_FUNC_ALLOC: + handle_rpc_func_alloc(regs, ctx); + break; + case OPTEE_SMC_RPC_FUNC_FREE: + { + uint64_t cookie = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2); + free_shm(cookie, current->domain->domain_id); + break; + } + case OPTEE_SMC_RPC_FUNC_FOREIGN_INTR: + break; + case OPTEE_SMC_RPC_FUNC_CMD: + handle_rpc_cmd(regs, ctx); + break; + } + + return execute_std_call(regs, ctx); +} + +static bool handle_get_shm_config(struct cpu_user_regs *regs) +{ + paddr_t shm_start; + size_t shm_size; + int rc; + + /* Give all static SHM region to the hardware domain */ + if ( !is_hardware_domain(current->domain) ) + return false; + + forward_call(regs); + + /* Return error back to the guest */ + if ( get_user_reg(regs, 0) != OPTEE_SMC_RETURN_OK) + return true; + + shm_start = get_user_reg(regs, 1); + shm_size = get_user_reg(regs, 2); + + /* HW dom is mapped 1:1 initially */ + rc = map_regions_p2mt(current->domain, gaddr_to_gfn(shm_start), + shm_size / PAGE_SIZE, + maddr_to_mfn(shm_start), + p2m_ram_rw); + if ( rc < 0 ) + { + gprintk(XENLOG_INFO, "OP-TEE: Can't map static shm for Dom0: %d\n", rc); + set_user_reg(regs, 0, OPTEE_SMC_RETURN_ENOTAVAIL); + } + + return true; +} + +static bool handle_exchange_capabilities(struct cpu_user_regs *regs) +{ + forward_call(regs); + + /* Return error back to the guest */ + if ( get_user_reg(regs, 0) != OPTEE_SMC_RETURN_OK ) + return true; + + /* Don't allow guests to work without dynamic SHM */ + if ( !(get_user_reg(regs, 1) & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM) ) + set_user_reg(regs, 0, OPTEE_SMC_RETURN_ENOTAVAIL); + return true; +} + +static bool optee_handle_smc(struct cpu_user_regs *regs) +{ + + switch ( get_user_reg(regs, 0) ) + { + case OPTEE_SMC_GET_SHM_CONFIG: + return handle_get_shm_config(regs); + case OPTEE_SMC_EXCHANGE_CAPABILITIES: + return handle_exchange_capabilities(regs); + case OPTEE_SMC_CALL_WITH_ARG: + return handle_std_call(regs); + case OPTEE_SMC_CALL_RETURN_FROM_RPC: + return handle_rpc(regs); + default: + return forward_call(regs); + } + return false; +} + +static void optee_remove(void) +{ +} + +static const struct tee_mediator_ops optee_ops = +{ + .init = optee_init, + .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_UID, &optee_ops); + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/tee/optee_smc.h b/xen/arch/arm/tee/optee_smc.h index 92f4d54..2e9df34 100644 --- a/xen/arch/arm/tee/optee_smc.h +++ b/xen/arch/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] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-11-27 17:01 ` [RFC] WIP: optee: add OP-TEE mediator Volodymyr Babchuk @ 2017-12-01 22:58 ` Stefano Stabellini 2017-12-04 14:30 ` Julien Grall ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Stefano Stabellini @ 2017-12-01 22:58 UTC (permalink / raw) To: Volodymyr Babchuk; +Cc: xen-devel, Julien Grall, sstabellini, stuart.yoder On Mon, 27 Nov 2017, Volodymyr Babchuk wrote: > This is follow-up to our conversation during community call. > You asked me to send OP-TEE mediator as a patch, so we can > discuss it in the mailing list. So, there it is. I squashed > two patches into one and skipped patches that we already > discussed. > > So, this is basically all what is needed to support OP-TEE in XEN. > There are some TODOs left all over the code. But, I don't > expect that TODOs implementation would significantly > increase codebase. Currently mediator parses requests to perform > addresses translation and that's all what is should be done > to allow guests to work with OP-TEE. > > This become possible because I completely revisited virtualization > support in OP-TEE. I have found way to enforce complete isolation > between different guest states. This lifts many questions like usage > quotas, RPC routing, sudden guest death, data isolation, etc. > > I'm aware that I didn't addressed all comments from previous > discussion. Sorry for this. I'm currently busy with OP-TEE, > and I think proper mediator implementation will be possible > only after I'll stabilize OP-TEE part. > > So I don't ask anybody to do thorough review. I just want to > share my status and discuss this code a whole. Thank you for sharing! Actually, I think it is not too bad as a starting point. I'll also try to summarize some key concept we have been discussing about OP-TEE support in Xen. = Xen cannot protect the system from a broken/insecure OP-TEE = OP-TEE runs at a higher privilege level than Xen, thus, we can't really expect Xen to protect the system from a broken OP-TEE. Also, Xen cannot really protect OP-TEE from a malicious caller. What we can and should do is to protect Xen, the OP-TEE mediator in Xen specifically, from malicious attackers. In other words, we are not responsible if a call, forwarded to OP-TEE by Xen, ends up crashing OP-TEE, therefore taking down the system. However, we have to pay special care to avoid callers to crash or take over the mediator in Xen. We also have to pay attention so that a caller won't be able to exhaust Xen resources or DOS Xen (allocate too much memory, infinite loops in Xen, etc). This brings me to the next topic. = Error checking / DOS protection = We need powerful checks on arguments passed by the caller and evaluated by the mediator. For example, we cannot expect the guest to actually pass arguments in the format expected by translate_params. ctx->xen_arg could be gibberish. From the resource allocation point of view, it looks like every handle_std_call allocates a new context; every copy_std_request allocates a new Xen page. It would be easy to exhaust Xen resources. Maybe we need a max concurrent request limit or max page allocation per domain or something of the kind. = Locks and Lists = The current lock and list is global. Do you think it should actually be global or per-domain? I do realize that only dom0 is allowed to make calls now so the question for the moment is not too useful. = Xen command forwarding = In the code below, it looks like Xen is forwarding everything to OP-TEE. Are there some commands Xen should avoid forwarding? Should we have a whitelist or a blacklist? = Long running OP-TEE commands and interruptions = I have been told that some OP-TEE RPC commands might take long to complete. Is that right? Like for example one of the OPTEE_MSG_RPC_CMD_*? If so, we need to think what to do in those cases. Specifically, do we need a technique to restart certain commands in Xen, so that when they run for too long and get interrupted by something (such as an interrupt) we know how to restart them? In fact, do we need to setup a timer interrupt to make sure the command doesn't block Xen for too long, consuming the next vcpu's slot as well? = Page pinning = Guest pages passed to OP-TEE need to be pinned (otherwise Xen doesn't guarantee they'll be available). I think the right function to call for that is get_page_from_gfn or get_page. > --- > > Add OP-TEE mediator as an example how TEE mediator framework > works. > > 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 successfully passes own tests, while client is > running in DomU. Currently it lacks some code for exceptional cases, > because this patch was used mostly to debug virtualization in OP-TEE. > Nevertheless, it provides all features needed for OP-TEE mediation. > > WARNING: This is a development patch, it does not cover all corner > cases, so, please don't use it in production. > > It was tested on RCAR Salvator-M3 board. > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > --- > xen/arch/arm/tee/Kconfig | 4 + > xen/arch/arm/tee/Makefile | 1 + > xen/arch/arm/tee/optee.c | 765 +++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/tee/optee_smc.h | 50 +++ > 4 files changed, 820 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..7c6b5c6 100644 > --- a/xen/arch/arm/tee/Kconfig > +++ b/xen/arch/arm/tee/Kconfig > @@ -0,0 +1,4 @@ > +config ARM_OPTEE > + bool "Enable OP-TEE mediator" > + default n > + depends on ARM_TEE > diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile > index c54d479..9d93b42 100644 > --- a/xen/arch/arm/tee/Makefile > +++ b/xen/arch/arm/tee/Makefile > @@ -1 +1,2 @@ > obj-y += tee.o > +obj-$(CONFIG_ARM_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..59c3600 > --- /dev/null > +++ b/xen/arch/arm/tee/optee.c > @@ -0,0 +1,765 @@ > +/* > + * xen/arch/arm/tee/optee.c > + * > + * OP-TEE mediator > + * > + * 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/domain_page.h> > +#include <xen/types.h> > +#include <xen/sched.h> > + > +#include <asm/p2m.h> > +#include <asm/tee.h> > + > +#include "optee_msg.h" > +#include "optee_smc.h" > + > +/* > + * Global TODO: > + * 1. Create per-domain context, where call and shm will be stored > + * 2. Pin pages shared between OP-TEE and guest > + */ > +/* > + * OP-TEE violates SMCCC when it defines own UID. So we need > + * to place bytes in correct order. > + */ > +#define OPTEE_UID (xen_uuid_t){{ \ > + (uint8_t)(OPTEE_MSG_UID_0 >> 0), (uint8_t)(OPTEE_MSG_UID_0 >> 8), \ > + (uint8_t)(OPTEE_MSG_UID_0 >> 16), (uint8_t)(OPTEE_MSG_UID_0 >> 24), \ > + (uint8_t)(OPTEE_MSG_UID_1 >> 0), (uint8_t)(OPTEE_MSG_UID_1 >> 8), \ > + (uint8_t)(OPTEE_MSG_UID_1 >> 16), (uint8_t)(OPTEE_MSG_UID_1 >> 24), \ > + (uint8_t)(OPTEE_MSG_UID_2 >> 0), (uint8_t)(OPTEE_MSG_UID_2 >> 8), \ > + (uint8_t)(OPTEE_MSG_UID_2 >> 16), (uint8_t)(OPTEE_MSG_UID_2 >> 24), \ > + (uint8_t)(OPTEE_MSG_UID_3 >> 0), (uint8_t)(OPTEE_MSG_UID_3 >> 8), \ > + (uint8_t)(OPTEE_MSG_UID_3 >> 16), (uint8_t)(OPTEE_MSG_UID_3 >> 24), \ > + }} > + > +#define MAX_NONCONTIG_ENTRIES 8 > + > +/* > + * Call context. OP-TEE can issue mulitple 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]; > + int optee_thread_id; > + int rpc_op; > + domid_t domid; > +}; > +static LIST_HEAD(call_ctx_list); > +static DEFINE_SPINLOCK(call_ctx_list_lock); > + > +/* > + * Command buffer shared between OP-TEE and guest. > + * Warning! In the proper implementation this SHM buffer *probably* should > + * by shadowed by XEN. > + * TODO: Reconsider this. > + */ > +struct shm { > + struct list_head list; > + struct optee_msg_arg *guest_arg; > + struct page *guest_page; > + mfn_t guest_mfn; > + uint64_t cookie; > + domid_t domid; > +}; > + > +static LIST_HEAD(shm_list); > +static DEFINE_SPINLOCK(shm_list_lock); > + > +static int optee_init(void) > +{ > + printk("OP-TEE mediator init done\n"); > + return 0; > +} > + > +static void optee_domain_create(struct domain *d) > +{ > + register_t resp[4]; > + call_smccc_smc(OPTEE_SMC_VM_CREATED, > + d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); > + if ( resp[0] != OPTEE_SMC_RETURN_OK ) > + gprintk(XENLOG_WARNING, "OP-TEE don't want to support domain: %d\n", > + (uint32_t)resp[0]); > + /* TODO: Change function declaration to be able to retun error */ > +} > + > +static void optee_domain_destroy(struct domain *d) > +{ > + register_t resp[4]; > + call_smccc_smc(OPTEE_SMC_VM_DESTROYED, > + d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); > + /* TODO: Clean call contexts and SHMs associated with domain */ > +} > + > +static bool forward_call(struct cpu_user_regs *regs) > +{ > + register_t resp[4]; > + > + /* TODO: Use separate registers set to prevent leakage to guest */ > + 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), > + /* VM id 0 is reserved for hypervisor itself */ > + current->domain->domain_id + 1, This doesn't look like it would wrap around safely. > + resp); > + > + set_user_reg(regs, 0, resp[0]); > + set_user_reg(regs, 1, resp[1]); > + set_user_reg(regs, 2, resp[2]); > + set_user_reg(regs, 3, resp[3]); > + > + return true; > +} > + > +static struct std_call_ctx *allocate_std_call_ctx(void) > +{ > + struct std_call_ctx *ret; > + > + ret = xzalloc(struct std_call_ctx); > + if ( !ret ) > + return NULL; > + > + ret->optee_thread_id = -1; > + ret->domid = -1; > + > + spin_lock(&call_ctx_list_lock); > + list_add_tail(&ret->list, &call_ctx_list); > + spin_unlock(&call_ctx_list_lock); > + > + return ret; > +} > + > +static void free_std_call_ctx(struct std_call_ctx *ctx) > +{ > + spin_lock(&call_ctx_list_lock); > + list_del(&ctx->list); > + spin_unlock(&call_ctx_list_lock); > + > + if (ctx->xen_arg) > + free_xenheap_page(ctx->xen_arg); > + > + if (ctx->guest_arg) > + unmap_domain_page(ctx->guest_arg); > + > + for (int i = 0; i < MAX_NONCONTIG_ENTRIES; i++) { > + if (ctx->non_contig[i]) > + free_xenheap_pages(ctx->non_contig[i], ctx->non_contig_order[i]); > + } > + > + xfree(ctx); > +} > + > +static struct std_call_ctx *find_ctx(int thread_id, domid_t domid) > +{ > + struct std_call_ctx *ctx; > + > + spin_lock(&call_ctx_list_lock); > + list_for_each_entry( ctx, &call_ctx_list, list ) > + { > + if (ctx->domid == domid && ctx->optee_thread_id == thread_id ) > + { > + spin_unlock(&call_ctx_list_lock); > + return ctx; > + } > + } > + spin_unlock(&call_ctx_list_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 mfn_t lookup_guest_ram_addr(paddr_t gaddr) > +{ > + mfn_t mfn; > + gfn_t gfn; > + p2m_type_t t; > + gfn = gaddr_to_gfn(gaddr); > + mfn = p2m_lookup(current->domain, gfn, &t); > + if ( t != p2m_ram_rw || mfn_eq(mfn, INVALID_MFN) ) { > + gprintk(XENLOG_INFO, "Domain tries to use invalid gfn\n"); > + return INVALID_MFN; > + } > + return mfn; > +} > + > +static struct shm *allocate_and_map_shm(paddr_t gaddr, uint64_t cookie) > +{ > + struct shm *ret; > + > + ret = xzalloc(struct shm); > + if ( !ret ) > + return NULL; > + > + ret->guest_mfn = lookup_guest_ram_addr(gaddr); > + > + if ( mfn_eq(ret->guest_mfn, INVALID_MFN) ) > + { > + xfree(ret); > + return NULL; > + } > + > + ret->guest_arg = map_domain_page(ret->guest_mfn); > + if ( !ret->guest_arg ) > + { > + gprintk(XENLOG_INFO, "Could not map domain page\n"); > + xfree(ret); > + return NULL; > + } > + ret->cookie = cookie; > + ret->domid = current->domain->domain_id; > + > + spin_lock(&shm_list_lock); > + list_add_tail(&ret->list, &shm_list); > + spin_unlock(&shm_list_lock); > + return ret; > +} > + > +static void free_shm(uint64_t cookie, domid_t domid) > +{ > + struct shm *shm, *found = NULL; > + spin_lock(&shm_list_lock); > + > + list_for_each_entry( shm, &shm_list, list ) > + { > + if (shm->domid == domid && shm->cookie == cookie ) > + { > + found = shm; > + list_del(&found->list); > + break; > + } > + } > + spin_unlock(&shm_list_lock); > + > + if ( !found ) { > + return; > + } > + > + if ( found->guest_arg ) > + unmap_domain_page(found->guest_arg); > + > + xfree(found); > +} > + > +static struct shm *find_shm(uint64_t cookie, domid_t domid) > +{ > + struct shm *shm; > + > + spin_lock(&shm_list_lock); > + list_for_each_entry( shm, &shm_list, list ) > + { > + if ( shm->domid == domid && shm->cookie == cookie ) > + { > + spin_unlock(&shm_list_lock); > + return shm; > + } > + } > + spin_unlock(&shm_list_lock); > + > + return NULL; > +} > + > +static bool translate_noncontig(struct std_call_ctx *ctx, > + struct optee_msg_param *param, > + int idx) > +{ > + /* > + * Refer to OPTEE_MSG_ATTR_NONCONTIG description in optee_msg.h for details. > + * > + * WARNING: This is test code. It works only with xen page size == 4096 > + */ > + 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; > + > + 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; > + > + gaddr = param->u.tmem.buf_ptr & ~(OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1); > + guest_mfn = lookup_guest_ram_addr(gaddr); > + 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 ) { > + mfn_t entry_mfn = lookup_guest_ram_addr( > + pages_data_guest->pages_list[entries_on_page]); > + > + if ( mfn_eq(entry_mfn, INVALID_MFN) ) > + goto err_unmap; > + > + 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); > + guest_mfn = lookup_guest_ram_addr(gaddr); > + 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--; > + } > + > + unmap_domain_page(pages_data_guest); > + > + param->u.tmem.buf_ptr = virt_to_maddr(pages_data_xen_start) | page_offset; > + > + ctx->non_contig[idx] = pages_data_xen_start; > + ctx->non_contig_order[idx] = order; > + > + unmap_domain_page(pages_data_guest); > + return true; > + > +err_unmap: > + unmap_domain_page(pages_data_guest); > +err_free: > + free_xenheap_pages(pages_data_xen_start, order); > + return false; > +} > + > +static bool translate_params(struct std_call_ctx *ctx) > +{ > + unsigned int i; > + uint32_t attr; > + > + for ( i = 0; i < ctx->xen_arg->num_params; i++ ) { > + attr = ctx->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, ctx->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 *ctx) > +{ > + paddr_t cmd_gaddr, xen_addr; > + mfn_t cmd_mfn; > + > + 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; > + > + cmd_mfn = lookup_guest_ram_addr(cmd_gaddr); > + if ( mfn_eq(cmd_mfn, INVALID_MFN) ) > + return false; > + > + ctx->guest_arg = map_domain_page(cmd_mfn); > + if ( !ctx->guest_arg ) > + return false; > + > + ctx->xen_arg = alloc_xenheap_page(); > + if ( !ctx->xen_arg ) > + return false; > + > + memcpy(ctx->xen_arg, ctx->guest_arg, OPTEE_MSG_NONCONTIG_PAGE_SIZE); > + > + xen_addr = virt_to_maddr(ctx->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 cpu_user_regs *regs, > + struct std_call_ctx *ctx) > +{ > + unsigned int i; > + uint32_t attr; > + > + ctx->guest_arg->ret = ctx->xen_arg->ret; > + ctx->guest_arg->ret_origin = ctx->xen_arg->ret_origin; > + ctx->guest_arg->session = ctx->xen_arg->session; > + for ( i = 0; i < ctx->xen_arg->num_params; i++ ) { > + attr = ctx->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: > + ctx->guest_arg->params[i].u.tmem.size = > + ctx->xen_arg->params[i].u.tmem.size; > + continue; > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > + ctx->guest_arg->params[i].u.value.a = > + ctx->xen_arg->params[i].u.value.a; > + ctx->guest_arg->params[i].u.value.b = > + ctx->xen_arg->params[i].u.value.b; > + continue; > + case OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT: > + case OPTEE_MSG_ATTR_TYPE_RMEM_INOUT: > + ctx->guest_arg->params[i].u.rmem.size = > + ctx->xen_arg->params[i].u.rmem.size; > + continue; > + case OPTEE_MSG_ATTR_TYPE_NONE: > + case OPTEE_MSG_ATTR_TYPE_TMEM_INPUT: > + case OPTEE_MSG_ATTR_TYPE_RMEM_INPUT: > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > + continue; > + } > + } > + return true; > +} > + > +static bool execute_std_call(struct cpu_user_regs *regs, > + struct std_call_ctx *ctx) > +{ > + register_t optee_ret; > + forward_call(regs); > + optee_ret = get_user_reg(regs, 0); > + > + if ( OPTEE_SMC_RETURN_IS_RPC(optee_ret) ) > + { > + ctx->optee_thread_id = get_user_reg(regs, 3); > + ctx->rpc_op = OPTEE_SMC_RETURN_GET_RPC_FUNC(optee_ret); > + return true; > + } > + > + copy_std_request_back(regs, ctx); > + free_std_call_ctx(ctx); > + > + return true; > +} > + > +static bool handle_std_call(struct cpu_user_regs *regs) > +{ > + struct std_call_ctx *ctx; > + bool ret; > + > + ctx = allocate_std_call_ctx(); > + > + if (!ctx) > + return false; > + > + ctx->domid = current->domain->domain_id; > + > + ret = copy_std_request(regs, ctx); > + if ( !ret ) > + goto out; > + > + /* Now we can safely examine contents of command buffer */ > + if ( OPTEE_MSG_GET_ARG_SIZE(ctx->xen_arg->num_params) > > + OPTEE_MSG_NONCONTIG_PAGE_SIZE ) { > + ret = false; > + goto out; > + } > + > + switch ( ctx->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); > + break; > + default: > + ret = false; > + } > + > + if (!ret) > + goto out; > + > + ret = execute_std_call(regs, ctx); > + > +out: > + if (!ret) > + free_std_call_ctx(ctx); > + > + return ret; > +} > + > +static void handle_rpc_cmd_alloc(struct cpu_user_regs *regs, > + struct std_call_ctx *ctx, > + struct shm *shm) > +{ > + if ( shm->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 ( ctx->non_contig[MAX_NONCONTIG_ENTRIES - 1] ) > + { > + free_xenheap_pages(ctx->non_contig[7], > + ctx->non_contig_order[MAX_NONCONTIG_ENTRIES - 1]); > + ctx->non_contig[7] = NULL; > + } > + translate_noncontig(ctx, shm->guest_arg->params + 0, > + MAX_NONCONTIG_ENTRIES - 1); > +} > + > +static void handle_rpc_cmd(struct cpu_user_regs *regs, struct std_call_ctx *ctx) > +{ > + struct shm *shm; > + uint64_t cookie; > + > + cookie = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2); > + > + shm = find_shm(cookie, current->domain->domain_id); > + > + if ( !shm ) > + { > + gprintk(XENLOG_ERR, "Can't find SHM with cookie %lx\n", cookie); > + return; > + } > + > + switch (shm->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(regs, ctx, shm); > + break; > + case OPTEE_MSG_RPC_CMD_SHM_FREE: > + break; > + default: > + break; > + } > +} > + > +static void handle_rpc_func_alloc(struct cpu_user_regs *regs, > + struct std_call_ctx *ctx) > +{ > + 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 *shm; > + > + shm = allocate_and_map_shm(ptr, cookie); > + if ( !shm ) > + { > + gprintk(XENLOG_WARNING, "Failed to callocate allocate SHM\n"); > + ptr = 0; > + } > + else > + ptr = mfn_to_maddr(shm->guest_mfn); > + > + set_user_reg(regs, 1, ptr >> 32); > + set_user_reg(regs, 2, ptr & 0xFFFFFFFF); > + } > +} > + > +static bool handle_rpc(struct cpu_user_regs *regs) > +{ > + struct std_call_ctx *ctx; > + > + int optee_thread_id = get_user_reg(regs, 3); > + > + ctx = find_ctx(optee_thread_id, current->domain->domain_id); > + > + if (!ctx) > + return false; > + > + switch ( ctx->rpc_op ) { > + case OPTEE_SMC_RPC_FUNC_ALLOC: > + handle_rpc_func_alloc(regs, ctx); > + break; > + case OPTEE_SMC_RPC_FUNC_FREE: > + { > + uint64_t cookie = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2); > + free_shm(cookie, current->domain->domain_id); > + break; > + } > + case OPTEE_SMC_RPC_FUNC_FOREIGN_INTR: > + break; > + case OPTEE_SMC_RPC_FUNC_CMD: > + handle_rpc_cmd(regs, ctx); > + break; > + } > + > + return execute_std_call(regs, ctx); > +} > + > +static bool handle_get_shm_config(struct cpu_user_regs *regs) > +{ > + paddr_t shm_start; > + size_t shm_size; > + int rc; > + > + /* Give all static SHM region to the hardware domain */ > + if ( !is_hardware_domain(current->domain) ) > + return false; > + > + forward_call(regs); > + > + /* Return error back to the guest */ > + if ( get_user_reg(regs, 0) != OPTEE_SMC_RETURN_OK) > + return true; > + > + shm_start = get_user_reg(regs, 1); > + shm_size = get_user_reg(regs, 2); > + > + /* HW dom is mapped 1:1 initially */ > + rc = map_regions_p2mt(current->domain, gaddr_to_gfn(shm_start), > + shm_size / PAGE_SIZE, > + maddr_to_mfn(shm_start), > + p2m_ram_rw); > + if ( rc < 0 ) > + { > + gprintk(XENLOG_INFO, "OP-TEE: Can't map static shm for Dom0: %d\n", rc); > + set_user_reg(regs, 0, OPTEE_SMC_RETURN_ENOTAVAIL); > + } > + > + return true; > +} > + > +static bool handle_exchange_capabilities(struct cpu_user_regs *regs) > +{ > + forward_call(regs); > + > + /* Return error back to the guest */ > + if ( get_user_reg(regs, 0) != OPTEE_SMC_RETURN_OK ) > + return true; > + > + /* Don't allow guests to work without dynamic SHM */ > + if ( !(get_user_reg(regs, 1) & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM) ) > + set_user_reg(regs, 0, OPTEE_SMC_RETURN_ENOTAVAIL); > + return true; > +} > + > +static bool optee_handle_smc(struct cpu_user_regs *regs) > +{ > + > + switch ( get_user_reg(regs, 0) ) > + { > + case OPTEE_SMC_GET_SHM_CONFIG: > + return handle_get_shm_config(regs); > + case OPTEE_SMC_EXCHANGE_CAPABILITIES: > + return handle_exchange_capabilities(regs); > + case OPTEE_SMC_CALL_WITH_ARG: > + return handle_std_call(regs); > + case OPTEE_SMC_CALL_RETURN_FROM_RPC: > + return handle_rpc(regs); > + default: > + return forward_call(regs); > + } > + return false; > +} > + > +static void optee_remove(void) > +{ > +} > + > +static const struct tee_mediator_ops optee_ops = > +{ > + .init = optee_init, > + .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_UID, &optee_ops); > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/arch/arm/tee/optee_smc.h b/xen/arch/arm/tee/optee_smc.h > index 92f4d54..2e9df34 100644 > --- a/xen/arch/arm/tee/optee_smc.h > +++ b/xen/arch/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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-01 22:58 ` Stefano Stabellini @ 2017-12-04 14:30 ` Julien Grall 2017-12-04 16:24 ` Volodymyr Babchuk 2017-12-06 22:39 ` Julien Grall 2017-12-04 14:46 ` Andrew Cooper 2017-12-04 16:15 ` Volodymyr Babchuk 2 siblings, 2 replies; 22+ messages in thread From: Julien Grall @ 2017-12-04 14:30 UTC (permalink / raw) To: Stefano Stabellini, Volodymyr Babchuk; +Cc: xen-devel, stuart.yoder Hi, I am going to answer both e-mails (Stefano and Volodymyr) at once. On 01/12/17 22:58, Stefano Stabellini wrote: > On Mon, 27 Nov 2017, Volodymyr Babchuk wrote: >> This is follow-up to our conversation during community call. >> You asked me to send OP-TEE mediator as a patch, so we can >> discuss it in the mailing list. So, there it is. I squashed >> two patches into one and skipped patches that we already >> discussed. >> >> So, this is basically all what is needed to support OP-TEE in XEN. >> There are some TODOs left all over the code. But, I don't >> expect that TODOs implementation would significantly >> increase codebase. Currently mediator parses requests to perform >> addresses translation and that's all what is should be done >> to allow guests to work with OP-TEE. >> >> This become possible because I completely revisited virtualization >> support in OP-TEE. I have found way to enforce complete isolation >> between different guest states. This lifts many questions like usage >> quotas, RPC routing, sudden guest death, data isolation, etc. I disagree here. You still have to handle sudden guest death in Xen and release any memory allocated in the hypervisor for that guests. >> >> I'm aware that I didn't addressed all comments from previous >> discussion. Sorry for this. I'm currently busy with OP-TEE, >> and I think proper mediator implementation will be possible >> only after I'll stabilize OP-TEE part. >> >> So I don't ask anybody to do thorough review. I just want to >> share my status and discuss this code a whole. > > Thank you for sharing! Actually, I think it is not too bad as a starting > point. > > I'll also try to summarize some key concept we have been discussing > about OP-TEE support in Xen. > > > = Xen cannot protect the system from a broken/insecure OP-TEE = > > OP-TEE runs at a higher privilege level than Xen, thus, we can't really > expect Xen to protect the system from a broken OP-TEE. Also, Xen cannot > really protect OP-TEE from a malicious caller. > > What we can and should do is to protect Xen, the OP-TEE mediator in Xen > specifically, from malicious attackers. > > In other words, we are not responsible if a call, forwarded to OP-TEE by > Xen, ends up crashing OP-TEE, therefore taking down the system. > > However, we have to pay special care to avoid callers to crash or take > over the mediator in Xen. We also have to pay attention so that a caller > won't be able to exhaust Xen resources or DOS Xen (allocate too much > memory, infinite loops in Xen, etc). This brings me to the next topic. > > > = Error checking / DOS protection = > > We need powerful checks on arguments passed by the caller and evaluated > by the mediator. > > For example, we cannot expect the guest to actually pass arguments in > the format expected by translate_params. ctx->xen_arg could be > gibberish. > > From the resource allocation point of view, it looks like every > handle_std_call allocates a new context; every copy_std_request > allocates a new Xen page. It would be easy to exhaust Xen resources. > Maybe we need a max concurrent request limit or max page allocation per > domain or something of the kind. > > > = Locks and Lists = > > The current lock and list is global. Do you think it should actually be > global or per-domain? I do realize that only dom0 is allowed to make > calls now so the question for the moment is not too useful. Hmmm, the commit message says: "With this patch OP-TEE successfully passes own tests, while client is running in DomU.". As said above, we need to make sure that Xen will release memory allocated for SMC requests when a domain dies. So have a list/lock per domain would make more sense (easier to go through). > > > = Xen command forwarding = > > In the code below, it looks like Xen is forwarding everything to OP-TEE. > Are there some commands Xen should avoid forwarding? Should we have a > whitelist or a blacklist? > > > = Long running OP-TEE commands and interruptions = > > I have been told that some OP-TEE RPC commands might take long to > complete. Is that right? Like for example one of the > OPTEE_MSG_RPC_CMD_*? > > If so, we need to think what to do in those cases. Specifically, do we > need a technique to restart certain commands in Xen, so that when they > run for too long and get interrupted by something (such as an > interrupt) we know how to restart them? In fact, do we need to setup a > timer interrupt to make sure the command doesn't block Xen for too long, > consuming the next vcpu's slot as well? I am not sure to understand your suggestion here. Where do you want that timer? In Xen? If so, I don't think this could work. That's OP-TEE job to break down long running operation. This is very similar to when a guest is doing an hypercall. Even if setup a timer, the interrupt will only be received once the hypercall is done (or Xen decided to preempt it). > > > = Page pinning = > > Guest pages passed to OP-TEE need to be pinned (otherwise Xen doesn't > guarantee they'll be available). I think the right function to call for > that is get_page_from_gfn or get_page. No direct use of get_page please. We already have plenty of helper to deal with the translation GFN -> Page or even copying data. I don't want to see more open-coding version because it makes difficult to interaction with other features such as memaccess and PoD. >> --- >> >> Add OP-TEE mediator as an example how TEE mediator framework >> works. >> >> 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 successfully passes own tests, while client is >> running in DomU. Currently it lacks some code for exceptional cases, >> because this patch was used mostly to debug virtualization in OP-TEE. >> Nevertheless, it provides all features needed for OP-TEE mediation. >> >> WARNING: This is a development patch, it does not cover all corner >> cases, so, please don't use it in production. >> >> It was tested on RCAR Salvator-M3 board. >> >> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> >> --- >> xen/arch/arm/tee/Kconfig | 4 + >> xen/arch/arm/tee/Makefile | 1 + >> xen/arch/arm/tee/optee.c | 765 +++++++++++++++++++++++++++++++++++++++++++ >> xen/arch/arm/tee/optee_smc.h | 50 +++ >> 4 files changed, 820 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..7c6b5c6 100644 >> --- a/xen/arch/arm/tee/Kconfig >> +++ b/xen/arch/arm/tee/Kconfig >> @@ -0,0 +1,4 @@ >> +config ARM_OPTEE >> + bool "Enable OP-TEE mediator" >> + default n >> + depends on ARM_TEE >> diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile >> index c54d479..9d93b42 100644 >> --- a/xen/arch/arm/tee/Makefile >> +++ b/xen/arch/arm/tee/Makefile >> @@ -1 +1,2 @@ >> obj-y += tee.o >> +obj-$(CONFIG_ARM_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..59c3600 >> --- /dev/null >> +++ b/xen/arch/arm/tee/optee.c >> @@ -0,0 +1,765 @@ >> +/* >> + * xen/arch/arm/tee/optee.c >> + * >> + * OP-TEE mediator >> + * >> + * 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/domain_page.h> >> +#include <xen/types.h> >> +#include <xen/sched.h> >> + >> +#include <asm/p2m.h> >> +#include <asm/tee.h> >> + >> +#include "optee_msg.h" >> +#include "optee_smc.h" >> + >> +/* >> + * Global TODO: >> + * 1. Create per-domain context, where call and shm will be stored >> + * 2. Pin pages shared between OP-TEE and guest >> + */ >> +/* >> + * OP-TEE violates SMCCC when it defines own UID. So we need >> + * to place bytes in correct order. >> + */ >> +#define OPTEE_UID (xen_uuid_t){{ \ >> + (uint8_t)(OPTEE_MSG_UID_0 >> 0), (uint8_t)(OPTEE_MSG_UID_0 >> 8), \ >> + (uint8_t)(OPTEE_MSG_UID_0 >> 16), (uint8_t)(OPTEE_MSG_UID_0 >> 24), \ >> + (uint8_t)(OPTEE_MSG_UID_1 >> 0), (uint8_t)(OPTEE_MSG_UID_1 >> 8), \ >> + (uint8_t)(OPTEE_MSG_UID_1 >> 16), (uint8_t)(OPTEE_MSG_UID_1 >> 24), \ >> + (uint8_t)(OPTEE_MSG_UID_2 >> 0), (uint8_t)(OPTEE_MSG_UID_2 >> 8), \ >> + (uint8_t)(OPTEE_MSG_UID_2 >> 16), (uint8_t)(OPTEE_MSG_UID_2 >> 24), \ >> + (uint8_t)(OPTEE_MSG_UID_3 >> 0), (uint8_t)(OPTEE_MSG_UID_3 >> 8), \ >> + (uint8_t)(OPTEE_MSG_UID_3 >> 16), (uint8_t)(OPTEE_MSG_UID_3 >> 24), \ >> + }} >> + >> +#define MAX_NONCONTIG_ENTRIES 8 >> + >> +/* >> + * Call context. OP-TEE can issue mulitple 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]; >> + int optee_thread_id; >> + int rpc_op; >> + domid_t domid; >> +}; >> +static LIST_HEAD(call_ctx_list); >> +static DEFINE_SPINLOCK(call_ctx_list_lock); >> + >> +/* >> + * Command buffer shared between OP-TEE and guest. >> + * Warning! In the proper implementation this SHM buffer *probably* should >> + * by shadowed by XEN. >> + * TODO: Reconsider this. >> + */ >> +struct shm { >> + struct list_head list; >> + struct optee_msg_arg *guest_arg; >> + struct page *guest_page; >> + mfn_t guest_mfn; >> + uint64_t cookie; >> + domid_t domid; >> +}; >> + >> +static LIST_HEAD(shm_list); >> +static DEFINE_SPINLOCK(shm_list_lock); >> + >> +static int optee_init(void) >> +{ >> + printk("OP-TEE mediator init done\n"); >> + return 0; >> +} >> + >> +static void optee_domain_create(struct domain *d) >> +{ >> + register_t resp[4]; >> + call_smccc_smc(OPTEE_SMC_VM_CREATED, >> + d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); >> + if ( resp[0] != OPTEE_SMC_RETURN_OK ) >> + gprintk(XENLOG_WARNING, "OP-TEE don't want to support domain: %d\n", >> + (uint32_t)resp[0]); >> + /* TODO: Change function declaration to be able to retun error */ >> +} >> + >> +static void optee_domain_destroy(struct domain *d) >> +{ >> + register_t resp[4]; >> + call_smccc_smc(OPTEE_SMC_VM_DESTROYED, >> + d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); >> + /* TODO: Clean call contexts and SHMs associated with domain */ >> +} >> + >> +static bool forward_call(struct cpu_user_regs *regs) >> +{ >> + register_t resp[4]; >> + >> + /* TODO: Use separate registers set to prevent leakage to guest */ >> + 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), >> + /* VM id 0 is reserved for hypervisor itself */ >> + current->domain->domain_id + 1, > > This doesn't look like it would wrap around safely. Well ordinary domain will always have a domain ID < DOMID_FIRST_RESERVER (0x7FF0). So there are no issue here. Although, we may want a BUILD_BUG_ON() to catch change of DOMID_FIRST_RESERVED. > > >> + resp); >> + >> + set_user_reg(regs, 0, resp[0]); >> + set_user_reg(regs, 1, resp[1]); >> + set_user_reg(regs, 2, resp[2]); >> + set_user_reg(regs, 3, resp[3]); >> + >> + return true; >> +} >> + >> +static struct std_call_ctx *allocate_std_call_ctx(void) >> +{ >> + struct std_call_ctx *ret; >> + >> + ret = xzalloc(struct std_call_ctx); >> + if ( !ret ) >> + return NULL; >> + >> + ret->optee_thread_id = -1; You set it to -1. But no-one is checking that value. So what is the purpose of setting to -1 and not 0? >> + ret->domid = -1; Please use DOMID_INVALID rather than -1. You don't know whether the latter will be used in the future for a domain. >> + >> + spin_lock(&call_ctx_list_lock); >> + list_add_tail(&ret->list, &call_ctx_list); >> + spin_unlock(&call_ctx_list_lock); >> + >> + return ret; >> +} >> + >> +static void free_std_call_ctx(struct std_call_ctx *ctx) >> +{ >> + spin_lock(&call_ctx_list_lock); >> + list_del(&ctx->list); >> + spin_unlock(&call_ctx_list_lock); >> + >> + if (ctx->xen_arg) >> + free_xenheap_page(ctx->xen_arg); >> + >> + if (ctx->guest_arg) >> + unmap_domain_page(ctx->guest_arg); >> + >> + for (int i = 0; i < MAX_NONCONTIG_ENTRIES; i++) { >> + if (ctx->non_contig[i]) >> + free_xenheap_pages(ctx->non_contig[i], ctx->non_contig_order[i]); >> + } >> + >> + xfree(ctx); >> +} >> + >> +static struct std_call_ctx *find_ctx(int thread_id, domid_t domid) >> +{ >> + struct std_call_ctx *ctx; >> + >> + spin_lock(&call_ctx_list_lock); >> + list_for_each_entry( ctx, &call_ctx_list, list ) >> + { >> + if (ctx->domid == domid && ctx->optee_thread_id == thread_id ) >> + { >> + spin_unlock(&call_ctx_list_lock); >> + return ctx; >> + } >> + } >> + spin_unlock(&call_ctx_list_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 mfn_t lookup_guest_ram_addr(paddr_t gaddr) >> +{ >> + mfn_t mfn; >> + gfn_t gfn; >> + p2m_type_t t; >> + gfn = gaddr_to_gfn(gaddr); >> + mfn = p2m_lookup(current->domain, gfn, &t); >> + if ( t != p2m_ram_rw || mfn_eq(mfn, INVALID_MFN) ) { >> + gprintk(XENLOG_INFO, "Domain tries to use invalid gfn\n"); >> + return INVALID_MFN; >> + } >> + return mfn; >> +} >> + >> +static struct shm *allocate_and_map_shm(paddr_t gaddr, uint64_t cookie) >> +{ >> + struct shm *ret; >> + >> + ret = xzalloc(struct shm); >> + if ( !ret ) >> + return NULL; >> + >> + ret->guest_mfn = lookup_guest_ram_addr(gaddr); >> + >> + if ( mfn_eq(ret->guest_mfn, INVALID_MFN) ) >> + { >> + xfree(ret); >> + return NULL; >> + } >> + >> + ret->guest_arg = map_domain_page(ret->guest_mfn); map_domain_page() can never fail, but you use it the wrong way. The purpose of this function is to map the memory for a very short lifetime, and only a the current pCPU (when the all the RAM is not always mapped). Here, you seem to use across SMC call (e.g for RPC). Looking at the usage in the code, you only map it in order to copy the arguments to/from the guest. map_domain_page() will not take a reference on the page and prevent the page to disappear from the guest. So this bits is unsafe. For the arguments, the best is to use guest copy helpers (see access_guest_memory_by_ipa). You might want to look at [1] as it improves the use of access_guest_memory_by_ipa. >> + if ( !ret->guest_arg ) >> + { >> + gprintk(XENLOG_INFO, "Could not map domain page\n"); >> + xfree(ret); >> + return NULL; >> + } >> + ret->cookie = cookie; >> + ret->domid = current->domain->domain_id; >> + >> + spin_lock(&shm_list_lock); >> + list_add_tail(&ret->list, &shm_list); >> + spin_unlock(&shm_list_lock); >> + return ret; >> +} >> + >> +static void free_shm(uint64_t cookie, domid_t domid) >> +{ >> + struct shm *shm, *found = NULL; >> + spin_lock(&shm_list_lock); >> + >> + list_for_each_entry( shm, &shm_list, list ) >> + { >> + if (shm->domid == domid && shm->cookie == cookie ) >> + { >> + found = shm; >> + list_del(&found->list); >> + break; >> + } >> + } >> + spin_unlock(&shm_list_lock); >> + >> + if ( !found ) { >> + return; >> + } >> + >> + if ( found->guest_arg ) >> + unmap_domain_page(found->guest_arg); >> + >> + xfree(found); >> +} >> + >> +static struct shm *find_shm(uint64_t cookie, domid_t domid) >> +{ >> + struct shm *shm; >> + >> + spin_lock(&shm_list_lock); >> + list_for_each_entry( shm, &shm_list, list ) >> + { >> + if ( shm->domid == domid && shm->cookie == cookie ) >> + { >> + spin_unlock(&shm_list_lock); >> + return shm; >> + } >> + } >> + spin_unlock(&shm_list_lock); >> + >> + return NULL; >> +} >> + >> +static bool translate_noncontig(struct std_call_ctx *ctx, >> + struct optee_msg_param *param, >> + int idx) >> +{ >> + /* >> + * Refer to OPTEE_MSG_ATTR_NONCONTIG description in optee_msg.h for details. >> + * >> + * WARNING: This is test code. It works only with xen page size == 4096 That's a call for a BUILD_BUG_ON(). >> + */ >> + 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; >> + >> + 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); What is the limit for num_pages? I can't see anything in the code that prevent any high number and might exhaust Xen memory. >> + >> + 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; >> + >> + gaddr = param->u.tmem.buf_ptr & ~(OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1); >> + guest_mfn = lookup_guest_ram_addr(gaddr); >> + if ( mfn_eq(guest_mfn, INVALID_MFN) ) >> + goto err_free; >> + >> + pages_data_guest = map_domain_page(guest_mfn); Similarly here, you may want to use access_guest_by_ipa. This will do all the safety check for copy from guest memory. Furthermore, I think this is going to simplify a lot this code. >> + if (!pages_data_guest) >> + goto err_free; >> + >> + pages_data_xen = pages_data_xen_start; >> + while ( num_pages ) { >> + mfn_t entry_mfn = lookup_guest_ram_addr( >> + pages_data_guest->pages_list[entries_on_page]); >> + >> + if ( mfn_eq(entry_mfn, INVALID_MFN) ) >> + goto err_unmap; You would need to get a reference on each page, and release it in err_unmap or when the command is done. get_page_from_gfn could do it for you. >> + >> + 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); >> + guest_mfn = lookup_guest_ram_addr(gaddr); >> + 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--; >> + } >> + >> + unmap_domain_page(pages_data_guest); >> + >> + param->u.tmem.buf_ptr = virt_to_maddr(pages_data_xen_start) | page_offset; I am not sure to understand why you are apply the offset of the guest buffer to xen buffer. >> + >> + ctx->non_contig[idx] = pages_data_xen_start; >> + ctx->non_contig_order[idx] = order; >> + >> + unmap_domain_page(pages_data_guest); >> + return true; >> + >> +err_unmap: >> + unmap_domain_page(pages_data_guest); >> +err_free: >> + free_xenheap_pages(pages_data_xen_start, order); >> + return false; >> +} >> + >> +static bool translate_params(struct std_call_ctx *ctx) >> +{ >> + unsigned int i; >> + uint32_t attr; >> + >> + for ( i = 0; i < ctx->xen_arg->num_params; i++ ) { >> + attr = ctx->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, ctx->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 *ctx) >> +{ >> + paddr_t cmd_gaddr, xen_addr; >> + mfn_t cmd_mfn; >> + >> + 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; >> + >> + cmd_mfn = lookup_guest_ram_addr(cmd_gaddr); >> + if ( mfn_eq(cmd_mfn, INVALID_MFN) ) >> + return false; >> + >> + ctx->guest_arg = map_domain_page(cmd_mfn); >> + if ( !ctx->guest_arg ) >> + return false; >> + >> + ctx->xen_arg = alloc_xenheap_page(); >> + if ( !ctx->xen_arg ) >> + return false; >> + >> + memcpy(ctx->xen_arg, ctx->guest_arg, OPTEE_MSG_NONCONTIG_PAGE_SIZE); Have a look a guest copy helpers. Cheers, [1] https://lists.xenproject.org/archives/html/xen-devel/2017-11/msg01481.html -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-04 14:30 ` Julien Grall @ 2017-12-04 16:24 ` Volodymyr Babchuk 2017-12-04 16:29 ` Julien Grall 2017-12-06 22:39 ` Julien Grall 1 sibling, 1 reply; 22+ messages in thread From: Volodymyr Babchuk @ 2017-12-04 16:24 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini, stuart.yoder Hi Julien, On Mon, Dec 04, 2017 at 02:30:32PM +0000, Julien Grall wrote: > Hi, > > I am going to answer both e-mails (Stefano and Volodymyr) at once. > > On 01/12/17 22:58, Stefano Stabellini wrote: > >On Mon, 27 Nov 2017, Volodymyr Babchuk wrote: > >>This is follow-up to our conversation during community call. > >>You asked me to send OP-TEE mediator as a patch, so we can > >>discuss it in the mailing list. So, there it is. I squashed > >>two patches into one and skipped patches that we already > >>discussed. > >> > >>So, this is basically all what is needed to support OP-TEE in XEN. > >>There are some TODOs left all over the code. But, I don't > >>expect that TODOs implementation would significantly > >>increase codebase. Currently mediator parses requests to perform > >>addresses translation and that's all what is should be done > >>to allow guests to work with OP-TEE. > >> > >>This become possible because I completely revisited virtualization > >>support in OP-TEE. I have found way to enforce complete isolation > >>between different guest states. This lifts many questions like usage > >>quotas, RPC routing, sudden guest death, data isolation, etc. > > I disagree here. You still have to handle sudden guest death in Xen and > release any memory allocated in the hypervisor for that guests. I was speaking from OP-TEE point-of-view. From mediator side, there is callback optee_domain_destroy() with comment "TODO: Clean call contexts and SHMs associated with domain". This callback will be called during domain description and it will free any resources that mediator allocated on behalf of the guest. > >> > >>I'm aware that I didn't addressed all comments from previous > >>discussion. Sorry for this. I'm currently busy with OP-TEE, > >>and I think proper mediator implementation will be possible > >>only after I'll stabilize OP-TEE part. > >> > >>So I don't ask anybody to do thorough review. I just want to > >>share my status and discuss this code a whole. > > > >Thank you for sharing! Actually, I think it is not too bad as a starting > >point. > > > >I'll also try to summarize some key concept we have been discussing > >about OP-TEE support in Xen. > > > > > >= Xen cannot protect the system from a broken/insecure OP-TEE = > > > >OP-TEE runs at a higher privilege level than Xen, thus, we can't really > >expect Xen to protect the system from a broken OP-TEE. Also, Xen cannot > >really protect OP-TEE from a malicious caller. > > > >What we can and should do is to protect Xen, the OP-TEE mediator in Xen > >specifically, from malicious attackers. > > > >In other words, we are not responsible if a call, forwarded to OP-TEE by > >Xen, ends up crashing OP-TEE, therefore taking down the system. > > > >However, we have to pay special care to avoid callers to crash or take > >over the mediator in Xen. We also have to pay attention so that a caller > >won't be able to exhaust Xen resources or DOS Xen (allocate too much > >memory, infinite loops in Xen, etc). This brings me to the next topic. > > > > > >= Error checking / DOS protection = > > > >We need powerful checks on arguments passed by the caller and evaluated > >by the mediator. > > > >For example, we cannot expect the guest to actually pass arguments in > >the format expected by translate_params. ctx->xen_arg could be > >gibberish. > > > > From the resource allocation point of view, it looks like every > >handle_std_call allocates a new context; every copy_std_request > >allocates a new Xen page. It would be easy to exhaust Xen resources. > >Maybe we need a max concurrent request limit or max page allocation per > >domain or something of the kind. > > > > > >= Locks and Lists = > > > >The current lock and list is global. Do you think it should actually be > >global or per-domain? I do realize that only dom0 is allowed to make > >calls now so the question for the moment is not too useful. > > Hmmm, the commit message says: "With this patch OP-TEE successfully passes > own tests, while client is running in DomU.". As said above, we need to make > sure that Xen will release memory allocated for SMC requests when a domain > dies. So have a list/lock per domain would make more sense (easier to go > through). You are totaly right. I can't say why I did in a way I did :-) I think, it was easier approach during initial implementation. But I certainly plan to hold separete context with own lists/locks for every domain. This will also ease cleanup, because you are holding all domain data in one place. > > > > > >= Xen command forwarding = > > > >In the code below, it looks like Xen is forwarding everything to OP-TEE. > >Are there some commands Xen should avoid forwarding? Should we have a > >whitelist or a blacklist? > > > > > >= Long running OP-TEE commands and interruptions = > > > >I have been told that some OP-TEE RPC commands might take long to > >complete. Is that right? Like for example one of the > >OPTEE_MSG_RPC_CMD_*? > > > >If so, we need to think what to do in those cases. Specifically, do we > >need a technique to restart certain commands in Xen, so that when they > >run for too long and get interrupted by something (such as an > >interrupt) we know how to restart them? In fact, do we need to setup a > >timer interrupt to make sure the command doesn't block Xen for too long, > >consuming the next vcpu's slot as well? > > I am not sure to understand your suggestion here. Where do you want that > timer? In Xen? If so, I don't think this could work. That's OP-TEE job to > break down long running operation. > > This is very similar to when a guest is doing an hypercall. Even if setup a > timer, the interrupt will only be received once the hypercall is done (or > Xen decided to preempt it). > > > > > > >= Page pinning = > > > >Guest pages passed to OP-TEE need to be pinned (otherwise Xen doesn't > >guarantee they'll be available). I think the right function to call for > >that is get_page_from_gfn or get_page. > > No direct use of get_page please. We already have plenty of helper to deal > with the translation GFN -> Page or even copying data. I don't want to see > more open-coding version because it makes difficult to interaction with > other features such as memaccess and PoD. Okay. Could you please suggest what API should be used in my case? > >>--- > >> > >>Add OP-TEE mediator as an example how TEE mediator framework > >>works. > >> > >>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 successfully passes own tests, while client is > >>running in DomU. Currently it lacks some code for exceptional cases, > >>because this patch was used mostly to debug virtualization in OP-TEE. > >>Nevertheless, it provides all features needed for OP-TEE mediation. > >> > >>WARNING: This is a development patch, it does not cover all corner > >>cases, so, please don't use it in production. > >> > >>It was tested on RCAR Salvator-M3 board. > >> > >>Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > >>--- > >> xen/arch/arm/tee/Kconfig | 4 + > >> xen/arch/arm/tee/Makefile | 1 + > >> xen/arch/arm/tee/optee.c | 765 +++++++++++++++++++++++++++++++++++++++++++ > >> xen/arch/arm/tee/optee_smc.h | 50 +++ > >> 4 files changed, 820 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..7c6b5c6 100644 > >>--- a/xen/arch/arm/tee/Kconfig > >>+++ b/xen/arch/arm/tee/Kconfig > >>@@ -0,0 +1,4 @@ > >>+config ARM_OPTEE > >>+ bool "Enable OP-TEE mediator" > >>+ default n > >>+ depends on ARM_TEE > >>diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile > >>index c54d479..9d93b42 100644 > >>--- a/xen/arch/arm/tee/Makefile > >>+++ b/xen/arch/arm/tee/Makefile > >>@@ -1 +1,2 @@ > >> obj-y += tee.o > >>+obj-$(CONFIG_ARM_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..59c3600 > >>--- /dev/null > >>+++ b/xen/arch/arm/tee/optee.c > >>@@ -0,0 +1,765 @@ > >>+/* > >>+ * xen/arch/arm/tee/optee.c > >>+ * > >>+ * OP-TEE mediator > >>+ * > >>+ * 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/domain_page.h> > >>+#include <xen/types.h> > >>+#include <xen/sched.h> > >>+ > >>+#include <asm/p2m.h> > >>+#include <asm/tee.h> > >>+ > >>+#include "optee_msg.h" > >>+#include "optee_smc.h" > >>+ > >>+/* > >>+ * Global TODO: > >>+ * 1. Create per-domain context, where call and shm will be stored > >>+ * 2. Pin pages shared between OP-TEE and guest > >>+ */ > >>+/* > >>+ * OP-TEE violates SMCCC when it defines own UID. So we need > >>+ * to place bytes in correct order. > >>+ */ > >>+#define OPTEE_UID (xen_uuid_t){{ \ > >>+ (uint8_t)(OPTEE_MSG_UID_0 >> 0), (uint8_t)(OPTEE_MSG_UID_0 >> 8), \ > >>+ (uint8_t)(OPTEE_MSG_UID_0 >> 16), (uint8_t)(OPTEE_MSG_UID_0 >> 24), \ > >>+ (uint8_t)(OPTEE_MSG_UID_1 >> 0), (uint8_t)(OPTEE_MSG_UID_1 >> 8), \ > >>+ (uint8_t)(OPTEE_MSG_UID_1 >> 16), (uint8_t)(OPTEE_MSG_UID_1 >> 24), \ > >>+ (uint8_t)(OPTEE_MSG_UID_2 >> 0), (uint8_t)(OPTEE_MSG_UID_2 >> 8), \ > >>+ (uint8_t)(OPTEE_MSG_UID_2 >> 16), (uint8_t)(OPTEE_MSG_UID_2 >> 24), \ > >>+ (uint8_t)(OPTEE_MSG_UID_3 >> 0), (uint8_t)(OPTEE_MSG_UID_3 >> 8), \ > >>+ (uint8_t)(OPTEE_MSG_UID_3 >> 16), (uint8_t)(OPTEE_MSG_UID_3 >> 24), \ > >>+ }} > >>+ > >>+#define MAX_NONCONTIG_ENTRIES 8 > >>+ > >>+/* > >>+ * Call context. OP-TEE can issue mulitple 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]; > >>+ int optee_thread_id; > >>+ int rpc_op; > >>+ domid_t domid; > >>+}; > >>+static LIST_HEAD(call_ctx_list); > >>+static DEFINE_SPINLOCK(call_ctx_list_lock); > >>+ > >>+/* > >>+ * Command buffer shared between OP-TEE and guest. > >>+ * Warning! In the proper implementation this SHM buffer *probably* should > >>+ * by shadowed by XEN. > >>+ * TODO: Reconsider this. > >>+ */ > >>+struct shm { > >>+ struct list_head list; > >>+ struct optee_msg_arg *guest_arg; > >>+ struct page *guest_page; > >>+ mfn_t guest_mfn; > >>+ uint64_t cookie; > >>+ domid_t domid; > >>+}; > >>+ > >>+static LIST_HEAD(shm_list); > >>+static DEFINE_SPINLOCK(shm_list_lock); > >>+ > >>+static int optee_init(void) > >>+{ > >>+ printk("OP-TEE mediator init done\n"); > >>+ return 0; > >>+} > >>+ > >>+static void optee_domain_create(struct domain *d) > >>+{ > >>+ register_t resp[4]; > >>+ call_smccc_smc(OPTEE_SMC_VM_CREATED, > >>+ d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); > >>+ if ( resp[0] != OPTEE_SMC_RETURN_OK ) > >>+ gprintk(XENLOG_WARNING, "OP-TEE don't want to support domain: %d\n", > >>+ (uint32_t)resp[0]); > >>+ /* TODO: Change function declaration to be able to retun error */ > >>+} > >>+ > >>+static void optee_domain_destroy(struct domain *d) > >>+{ > >>+ register_t resp[4]; > >>+ call_smccc_smc(OPTEE_SMC_VM_DESTROYED, > >>+ d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); > >>+ /* TODO: Clean call contexts and SHMs associated with domain */ > >>+} > >>+ > >>+static bool forward_call(struct cpu_user_regs *regs) > >>+{ > >>+ register_t resp[4]; > >>+ > >>+ /* TODO: Use separate registers set to prevent leakage to guest */ > >>+ 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), > >>+ /* VM id 0 is reserved for hypervisor itself */ > >>+ current->domain->domain_id + 1, > > > >This doesn't look like it would wrap around safely. > > Well ordinary domain will always have a domain ID < DOMID_FIRST_RESERVER > (0x7FF0). So there are no issue here. Although, we may want a BUILD_BUG_ON() > to catch change of DOMID_FIRST_RESERVED. > > > > > > >>+ resp); > >>+ > >>+ set_user_reg(regs, 0, resp[0]); > >>+ set_user_reg(regs, 1, resp[1]); > >>+ set_user_reg(regs, 2, resp[2]); > >>+ set_user_reg(regs, 3, resp[3]); > >>+ > >>+ return true; > >>+} > >>+ > >>+static struct std_call_ctx *allocate_std_call_ctx(void) > >>+{ > >>+ struct std_call_ctx *ret; > >>+ > >>+ ret = xzalloc(struct std_call_ctx); > >>+ if ( !ret ) > >>+ return NULL; > >>+ > >>+ ret->optee_thread_id = -1; > > You set it to -1. But no-one is checking that value. So what is the purpose > of setting to -1 and not 0? > > >>+ ret->domid = -1; > > Please use DOMID_INVALID rather than -1. You don't know whether the latter > will be used in the future for a domain. > > >>+ > >>+ spin_lock(&call_ctx_list_lock); > >>+ list_add_tail(&ret->list, &call_ctx_list); > >>+ spin_unlock(&call_ctx_list_lock); > >>+ > >>+ return ret; > >>+} > >>+ > >>+static void free_std_call_ctx(struct std_call_ctx *ctx) > >>+{ > >>+ spin_lock(&call_ctx_list_lock); > >>+ list_del(&ctx->list); > >>+ spin_unlock(&call_ctx_list_lock); > >>+ > >>+ if (ctx->xen_arg) > >>+ free_xenheap_page(ctx->xen_arg); > >>+ > >>+ if (ctx->guest_arg) > >>+ unmap_domain_page(ctx->guest_arg); > >>+ > >>+ for (int i = 0; i < MAX_NONCONTIG_ENTRIES; i++) { > >>+ if (ctx->non_contig[i]) > >>+ free_xenheap_pages(ctx->non_contig[i], ctx->non_contig_order[i]); > >>+ } > >>+ > >>+ xfree(ctx); > >>+} > >>+ > >>+static struct std_call_ctx *find_ctx(int thread_id, domid_t domid) > >>+{ > >>+ struct std_call_ctx *ctx; > >>+ > >>+ spin_lock(&call_ctx_list_lock); > >>+ list_for_each_entry( ctx, &call_ctx_list, list ) > >>+ { > >>+ if (ctx->domid == domid && ctx->optee_thread_id == thread_id ) > >>+ { > >>+ spin_unlock(&call_ctx_list_lock); > >>+ return ctx; > >>+ } > >>+ } > >>+ spin_unlock(&call_ctx_list_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 mfn_t lookup_guest_ram_addr(paddr_t gaddr) > >>+{ > >>+ mfn_t mfn; > >>+ gfn_t gfn; > >>+ p2m_type_t t; > >>+ gfn = gaddr_to_gfn(gaddr); > >>+ mfn = p2m_lookup(current->domain, gfn, &t); > >>+ if ( t != p2m_ram_rw || mfn_eq(mfn, INVALID_MFN) ) { > >>+ gprintk(XENLOG_INFO, "Domain tries to use invalid gfn\n"); > >>+ return INVALID_MFN; > >>+ } > >>+ return mfn; > >>+} >> + > >>+static struct shm *allocate_and_map_shm(paddr_t gaddr, uint64_t cookie) > >>+{ > >>+ struct shm *ret; > >>+ > >>+ ret = xzalloc(struct shm); > >>+ if ( !ret ) > >>+ return NULL; > >>+ > >>+ ret->guest_mfn = lookup_guest_ram_addr(gaddr); > >>+ > >>+ if ( mfn_eq(ret->guest_mfn, INVALID_MFN) ) > >>+ { > >>+ xfree(ret); > >>+ return NULL; > >>+ } > >>+ > >>+ ret->guest_arg = map_domain_page(ret->guest_mfn); > > map_domain_page() can never fail, but you use it the wrong way. The purpose > of this function is to map the memory for a very short lifetime, and only a > the current pCPU (when the all the RAM is not always mapped). Here, you seem > to use across SMC call (e.g for RPC). > > Looking at the usage in the code, you only map it in order to copy the > arguments to/from the guest. > > map_domain_page() will not take a reference on the page and prevent the page > to disappear from the guest. So this bits is unsafe. > > For the arguments, the best is to use guest copy helpers (see > access_guest_memory_by_ipa). You might want to look at [1] as it improves > the use of access_guest_memory_by_ipa. > > >>+ if ( !ret->guest_arg ) > >>+ { > >>+ gprintk(XENLOG_INFO, "Could not map domain page\n"); > >>+ xfree(ret); > >>+ return NULL; > >>+ } > >>+ ret->cookie = cookie; > >>+ ret->domid = current->domain->domain_id; > >>+ > >>+ spin_lock(&shm_list_lock); > >>+ list_add_tail(&ret->list, &shm_list); > >>+ spin_unlock(&shm_list_lock); > >>+ return ret; > >>+} > >>+ > >>+static void free_shm(uint64_t cookie, domid_t domid) > >>+{ > >>+ struct shm *shm, *found = NULL; > >>+ spin_lock(&shm_list_lock); > >>+ > >>+ list_for_each_entry( shm, &shm_list, list ) > >>+ { > >>+ if (shm->domid == domid && shm->cookie == cookie ) > >>+ { > >>+ found = shm; > >>+ list_del(&found->list); > >>+ break; > >>+ } > >>+ } > >>+ spin_unlock(&shm_list_lock); > >>+ > >>+ if ( !found ) { > >>+ return; > >>+ } > >>+ > >>+ if ( found->guest_arg ) > >>+ unmap_domain_page(found->guest_arg); > >>+ > >>+ xfree(found); > >>+} > >>+ > >>+static struct shm *find_shm(uint64_t cookie, domid_t domid) > >>+{ > >>+ struct shm *shm; > >>+ > >>+ spin_lock(&shm_list_lock); > >>+ list_for_each_entry( shm, &shm_list, list ) > >>+ { > >>+ if ( shm->domid == domid && shm->cookie == cookie ) > >>+ { > >>+ spin_unlock(&shm_list_lock); > >>+ return shm; > >>+ } > >>+ } > >>+ spin_unlock(&shm_list_lock); > >>+ > >>+ return NULL; > >>+} > >>+ > >>+static bool translate_noncontig(struct std_call_ctx *ctx, > >>+ struct optee_msg_param *param, > >>+ int idx) > >>+{ > >>+ /* > >>+ * Refer to OPTEE_MSG_ATTR_NONCONTIG description in optee_msg.h for details. > >>+ * > >>+ * WARNING: This is test code. It works only with xen page size == 4096 > > That's a call for a BUILD_BUG_ON(). > > >>+ */ > >>+ 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; > >>+ > >>+ 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); > > What is the limit for num_pages? I can't see anything in the code that > prevent any high number and might exhaust Xen memory. > > >>+ > >>+ 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; > >>+ > >>+ gaddr = param->u.tmem.buf_ptr & ~(OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1); > >>+ guest_mfn = lookup_guest_ram_addr(gaddr); > >>+ if ( mfn_eq(guest_mfn, INVALID_MFN) ) > >>+ goto err_free; > >>+ > >>+ pages_data_guest = map_domain_page(guest_mfn); > > Similarly here, you may want to use access_guest_by_ipa. This will do all > the safety check for copy from guest memory. > > Furthermore, I think this is going to simplify a lot this code. > > > >>+ if (!pages_data_guest) > >>+ goto err_free; > >>+ > >>+ pages_data_xen = pages_data_xen_start; > >>+ while ( num_pages ) { > >>+ mfn_t entry_mfn = lookup_guest_ram_addr( > >>+ pages_data_guest->pages_list[entries_on_page]); > >>+ > >>+ if ( mfn_eq(entry_mfn, INVALID_MFN) ) > >>+ goto err_unmap; > > You would need to get a reference on each page, and release it in err_unmap > or when the command is done. get_page_from_gfn could do it for you. > > >>+ > >>+ 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); > >>+ guest_mfn = lookup_guest_ram_addr(gaddr); > >>+ 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--; > >>+ } > >>+ > >>+ unmap_domain_page(pages_data_guest); > >>+ > >>+ param->u.tmem.buf_ptr = virt_to_maddr(pages_data_xen_start) | page_offset; > > I am not sure to understand why you are apply the offset of the guest buffer > to xen buffer. > > >>+ > >>+ ctx->non_contig[idx] = pages_data_xen_start; > >>+ ctx->non_contig_order[idx] = order; > >>+ > >>+ unmap_domain_page(pages_data_guest); > >>+ return true; > >>+ > >>+err_unmap: > >>+ unmap_domain_page(pages_data_guest); > >>+err_free: > >>+ free_xenheap_pages(pages_data_xen_start, order); > >>+ return false; > >>+} > >>+ > >>+static bool translate_params(struct std_call_ctx *ctx) > >>+{ > >>+ unsigned int i; > >>+ uint32_t attr; > >>+ > >>+ for ( i = 0; i < ctx->xen_arg->num_params; i++ ) { > >>+ attr = ctx->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, ctx->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 *ctx) > >>+{ > >>+ paddr_t cmd_gaddr, xen_addr; > >>+ mfn_t cmd_mfn; > >>+ > >>+ 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; > >>+ > >>+ cmd_mfn = lookup_guest_ram_addr(cmd_gaddr); > >>+ if ( mfn_eq(cmd_mfn, INVALID_MFN) ) > >>+ return false; > >>+ > >>+ ctx->guest_arg = map_domain_page(cmd_mfn); > >>+ if ( !ctx->guest_arg ) > >>+ return false; > >>+ > >>+ ctx->xen_arg = alloc_xenheap_page(); > >>+ if ( !ctx->xen_arg ) > >>+ return false; > >>+ > >>+ memcpy(ctx->xen_arg, ctx->guest_arg, OPTEE_MSG_NONCONTIG_PAGE_SIZE); > > Have a look a guest copy helpers. > > Cheers, > > [1] > https://lists.xenproject.org/archives/html/xen-devel/2017-11/msg01481.html > > -- > Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-04 16:24 ` Volodymyr Babchuk @ 2017-12-04 16:29 ` Julien Grall 0 siblings, 0 replies; 22+ messages in thread From: Julien Grall @ 2017-12-04 16:29 UTC (permalink / raw) To: Volodymyr Babchuk; +Cc: xen-devel, Stefano Stabellini, stuart.yoder On 04/12/17 16:24, Volodymyr Babchuk wrote: > On Mon, Dec 04, 2017 at 02:30:32PM +0000, Julien Grall wrote: >> On 01/12/17 22:58, Stefano Stabellini wrote: >>> On Mon, 27 Nov 2017, Volodymyr Babchuk wrote: >>> = Page pinning = >>> >>> Guest pages passed to OP-TEE need to be pinned (otherwise Xen doesn't >>> guarantee they'll be available). I think the right function to call for >>> that is get_page_from_gfn or get_page. >> >> No direct use of get_page please. We already have plenty of helper to deal >> with the translation GFN -> Page or even copying data. I don't want to see >> more open-coding version because it makes difficult to interaction with >> other features such as memaccess and PoD. > Okay. Could you please suggest what API should be used in my case? Please read my previous e-mail until the end. I provided suggestions how to handle pinning... > >>>> --- >>>> >>>> Add OP-TEE mediator as an example how TEE mediator framework >>>> works. >>>> >>>> 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 successfully passes own tests, while client is >>>> running in DomU. Currently it lacks some code for exceptional cases, >>>> because this patch was used mostly to debug virtualization in OP-TEE. >>>> Nevertheless, it provides all features needed for OP-TEE mediation. >>>> >>>> WARNING: This is a development patch, it does not cover all corner >>>> cases, so, please don't use it in production. >>>> >>>> It was tested on RCAR Salvator-M3 board. >>>> >>>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> >>>> --- >>>> xen/arch/arm/tee/Kconfig | 4 + >>>> xen/arch/arm/tee/Makefile | 1 + >>>> xen/arch/arm/tee/optee.c | 765 +++++++++++++++++++++++++++++++++++++++++++ >>>> xen/arch/arm/tee/optee_smc.h | 50 +++ >>>> 4 files changed, 820 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..7c6b5c6 100644 >>>> --- a/xen/arch/arm/tee/Kconfig >>>> +++ b/xen/arch/arm/tee/Kconfig >>>> @@ -0,0 +1,4 @@ >>>> +config ARM_OPTEE >>>> + bool "Enable OP-TEE mediator" >>>> + default n >>>> + depends on ARM_TEE >>>> diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile >>>> index c54d479..9d93b42 100644 >>>> --- a/xen/arch/arm/tee/Makefile >>>> +++ b/xen/arch/arm/tee/Makefile >>>> @@ -1 +1,2 @@ >>>> obj-y += tee.o >>>> +obj-$(CONFIG_ARM_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..59c3600 >>>> --- /dev/null >>>> +++ b/xen/arch/arm/tee/optee.c >>>> @@ -0,0 +1,765 @@ >>>> +/* >>>> + * xen/arch/arm/tee/optee.c >>>> + * >>>> + * OP-TEE mediator >>>> + * >>>> + * 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/domain_page.h> >>>> +#include <xen/types.h> >>>> +#include <xen/sched.h> >>>> + >>>> +#include <asm/p2m.h> >>>> +#include <asm/tee.h> >>>> + >>>> +#include "optee_msg.h" >>>> +#include "optee_smc.h" >>>> + >>>> +/* >>>> + * Global TODO: >>>> + * 1. Create per-domain context, where call and shm will be stored >>>> + * 2. Pin pages shared between OP-TEE and guest >>>> + */ >>>> +/* >>>> + * OP-TEE violates SMCCC when it defines own UID. So we need >>>> + * to place bytes in correct order. >>>> + */ >>>> +#define OPTEE_UID (xen_uuid_t){{ \ >>>> + (uint8_t)(OPTEE_MSG_UID_0 >> 0), (uint8_t)(OPTEE_MSG_UID_0 >> 8), \ >>>> + (uint8_t)(OPTEE_MSG_UID_0 >> 16), (uint8_t)(OPTEE_MSG_UID_0 >> 24), \ >>>> + (uint8_t)(OPTEE_MSG_UID_1 >> 0), (uint8_t)(OPTEE_MSG_UID_1 >> 8), \ >>>> + (uint8_t)(OPTEE_MSG_UID_1 >> 16), (uint8_t)(OPTEE_MSG_UID_1 >> 24), \ >>>> + (uint8_t)(OPTEE_MSG_UID_2 >> 0), (uint8_t)(OPTEE_MSG_UID_2 >> 8), \ >>>> + (uint8_t)(OPTEE_MSG_UID_2 >> 16), (uint8_t)(OPTEE_MSG_UID_2 >> 24), \ >>>> + (uint8_t)(OPTEE_MSG_UID_3 >> 0), (uint8_t)(OPTEE_MSG_UID_3 >> 8), \ >>>> + (uint8_t)(OPTEE_MSG_UID_3 >> 16), (uint8_t)(OPTEE_MSG_UID_3 >> 24), \ >>>> + }} >>>> + >>>> +#define MAX_NONCONTIG_ENTRIES 8 >>>> + >>>> +/* >>>> + * Call context. OP-TEE can issue mulitple 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]; >>>> + int optee_thread_id; >>>> + int rpc_op; >>>> + domid_t domid; >>>> +}; >>>> +static LIST_HEAD(call_ctx_list); >>>> +static DEFINE_SPINLOCK(call_ctx_list_lock); >>>> + >>>> +/* >>>> + * Command buffer shared between OP-TEE and guest. >>>> + * Warning! In the proper implementation this SHM buffer *probably* should >>>> + * by shadowed by XEN. >>>> + * TODO: Reconsider this. >>>> + */ >>>> +struct shm { >>>> + struct list_head list; >>>> + struct optee_msg_arg *guest_arg; >>>> + struct page *guest_page; >>>> + mfn_t guest_mfn; >>>> + uint64_t cookie; >>>> + domid_t domid; >>>> +}; >>>> + >>>> +static LIST_HEAD(shm_list); >>>> +static DEFINE_SPINLOCK(shm_list_lock); >>>> + >>>> +static int optee_init(void) >>>> +{ >>>> + printk("OP-TEE mediator init done\n"); >>>> + return 0; >>>> +} >>>> + >>>> +static void optee_domain_create(struct domain *d) >>>> +{ >>>> + register_t resp[4]; >>>> + call_smccc_smc(OPTEE_SMC_VM_CREATED, >>>> + d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); >>>> + if ( resp[0] != OPTEE_SMC_RETURN_OK ) >>>> + gprintk(XENLOG_WARNING, "OP-TEE don't want to support domain: %d\n", >>>> + (uint32_t)resp[0]); >>>> + /* TODO: Change function declaration to be able to retun error */ >>>> +} >>>> + >>>> +static void optee_domain_destroy(struct domain *d) >>>> +{ >>>> + register_t resp[4]; >>>> + call_smccc_smc(OPTEE_SMC_VM_DESTROYED, >>>> + d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); >>>> + /* TODO: Clean call contexts and SHMs associated with domain */ >>>> +} >>>> + >>>> +static bool forward_call(struct cpu_user_regs *regs) >>>> +{ >>>> + register_t resp[4]; >>>> + >>>> + /* TODO: Use separate registers set to prevent leakage to guest */ >>>> + 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), >>>> + /* VM id 0 is reserved for hypervisor itself */ >>>> + current->domain->domain_id + 1, >>> >>> This doesn't look like it would wrap around safely. >> >> Well ordinary domain will always have a domain ID < DOMID_FIRST_RESERVER >> (0x7FF0). So there are no issue here. Although, we may want a BUILD_BUG_ON() >> to catch change of DOMID_FIRST_RESERVED. >> >>> >>> >>>> + resp); >>>> + >>>> + set_user_reg(regs, 0, resp[0]); >>>> + set_user_reg(regs, 1, resp[1]); >>>> + set_user_reg(regs, 2, resp[2]); >>>> + set_user_reg(regs, 3, resp[3]); >>>> + >>>> + return true; >>>> +} >>>> + >>>> +static struct std_call_ctx *allocate_std_call_ctx(void) >>>> +{ >>>> + struct std_call_ctx *ret; >>>> + >>>> + ret = xzalloc(struct std_call_ctx); >>>> + if ( !ret ) >>>> + return NULL; >>>> + >>>> + ret->optee_thread_id = -1; >> >> You set it to -1. But no-one is checking that value. So what is the purpose >> of setting to -1 and not 0? >> >>>> + ret->domid = -1; >> >> Please use DOMID_INVALID rather than -1. You don't know whether the latter >> will be used in the future for a domain. >> >>>> + >>>> + spin_lock(&call_ctx_list_lock); >>>> + list_add_tail(&ret->list, &call_ctx_list); >>>> + spin_unlock(&call_ctx_list_lock); >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static void free_std_call_ctx(struct std_call_ctx *ctx) >>>> +{ >>>> + spin_lock(&call_ctx_list_lock); >>>> + list_del(&ctx->list); >>>> + spin_unlock(&call_ctx_list_lock); >>>> + >>>> + if (ctx->xen_arg) >>>> + free_xenheap_page(ctx->xen_arg); >>>> + >>>> + if (ctx->guest_arg) >>>> + unmap_domain_page(ctx->guest_arg); >>>> + >>>> + for (int i = 0; i < MAX_NONCONTIG_ENTRIES; i++) { >>>> + if (ctx->non_contig[i]) >>>> + free_xenheap_pages(ctx->non_contig[i], ctx->non_contig_order[i]); >>>> + } >>>> + >>>> + xfree(ctx); >>>> +} >>>> + >>>> +static struct std_call_ctx *find_ctx(int thread_id, domid_t domid) >>>> +{ >>>> + struct std_call_ctx *ctx; >>>> + >>>> + spin_lock(&call_ctx_list_lock); >>>> + list_for_each_entry( ctx, &call_ctx_list, list ) >>>> + { >>>> + if (ctx->domid == domid && ctx->optee_thread_id == thread_id ) >>>> + { >>>> + spin_unlock(&call_ctx_list_lock); >>>> + return ctx; >>>> + } >>>> + } >>>> + spin_unlock(&call_ctx_list_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 mfn_t lookup_guest_ram_addr(paddr_t gaddr) >>>> +{ >>>> + mfn_t mfn; >>>> + gfn_t gfn; >>>> + p2m_type_t t; >>>> + gfn = gaddr_to_gfn(gaddr); >>>> + mfn = p2m_lookup(current->domain, gfn, &t); >>>> + if ( t != p2m_ram_rw || mfn_eq(mfn, INVALID_MFN) ) { >>>> + gprintk(XENLOG_INFO, "Domain tries to use invalid gfn\n"); >>>> + return INVALID_MFN; >>>> + } >>>> + return mfn; >>>> +} >> + >>>> +static struct shm *allocate_and_map_shm(paddr_t gaddr, uint64_t cookie) >>>> +{ >>>> + struct shm *ret; >>>> + >>>> + ret = xzalloc(struct shm); >>>> + if ( !ret ) >>>> + return NULL; >>>> + >>>> + ret->guest_mfn = lookup_guest_ram_addr(gaddr); >>>> + >>>> + if ( mfn_eq(ret->guest_mfn, INVALID_MFN) ) >>>> + { >>>> + xfree(ret); >>>> + return NULL; >>>> + } >>>> + >>>> + ret->guest_arg = map_domain_page(ret->guest_mfn); >> >> map_domain_page() can never fail, but you use it the wrong way. The purpose >> of this function is to map the memory for a very short lifetime, and only a >> the current pCPU (when the all the RAM is not always mapped). Here, you seem >> to use across SMC call (e.g for RPC). >> >> Looking at the usage in the code, you only map it in order to copy the >> arguments to/from the guest. >> >> map_domain_page() will not take a reference on the page and prevent the page >> to disappear from the guest. So this bits is unsafe. >> >> For the arguments, the best is to use guest copy helpers (see >> access_guest_memory_by_ipa). You might want to look at [1] as it improves >> the use of access_guest_memory_by_ipa. >> >>>> + if ( !ret->guest_arg ) >>>> + { >>>> + gprintk(XENLOG_INFO, "Could not map domain page\n"); >>>> + xfree(ret); >>>> + return NULL; >>>> + } >>>> + ret->cookie = cookie; >>>> + ret->domid = current->domain->domain_id; >>>> + >>>> + spin_lock(&shm_list_lock); >>>> + list_add_tail(&ret->list, &shm_list); >>>> + spin_unlock(&shm_list_lock); >>>> + return ret; >>>> +} >>>> + >>>> +static void free_shm(uint64_t cookie, domid_t domid) >>>> +{ >>>> + struct shm *shm, *found = NULL; >>>> + spin_lock(&shm_list_lock); >>>> + >>>> + list_for_each_entry( shm, &shm_list, list ) >>>> + { >>>> + if (shm->domid == domid && shm->cookie == cookie ) >>>> + { >>>> + found = shm; >>>> + list_del(&found->list); >>>> + break; >>>> + } >>>> + } >>>> + spin_unlock(&shm_list_lock); >>>> + >>>> + if ( !found ) { >>>> + return; >>>> + } >>>> + >>>> + if ( found->guest_arg ) >>>> + unmap_domain_page(found->guest_arg); >>>> + >>>> + xfree(found); >>>> +} >>>> + >>>> +static struct shm *find_shm(uint64_t cookie, domid_t domid) >>>> +{ >>>> + struct shm *shm; >>>> + >>>> + spin_lock(&shm_list_lock); >>>> + list_for_each_entry( shm, &shm_list, list ) >>>> + { >>>> + if ( shm->domid == domid && shm->cookie == cookie ) >>>> + { >>>> + spin_unlock(&shm_list_lock); >>>> + return shm; >>>> + } >>>> + } >>>> + spin_unlock(&shm_list_lock); >>>> + >>>> + return NULL; >>>> +} >>>> + >>>> +static bool translate_noncontig(struct std_call_ctx *ctx, >>>> + struct optee_msg_param *param, >>>> + int idx) >>>> +{ >>>> + /* >>>> + * Refer to OPTEE_MSG_ATTR_NONCONTIG description in optee_msg.h for details. >>>> + * >>>> + * WARNING: This is test code. It works only with xen page size == 4096 >> >> That's a call for a BUILD_BUG_ON(). >> >>>> + */ >>>> + 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; >>>> + >>>> + 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); >> >> What is the limit for num_pages? I can't see anything in the code that >> prevent any high number and might exhaust Xen memory. >> >>>> + >>>> + 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; >>>> + >>>> + gaddr = param->u.tmem.buf_ptr & ~(OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1); >>>> + guest_mfn = lookup_guest_ram_addr(gaddr); >>>> + if ( mfn_eq(guest_mfn, INVALID_MFN) ) >>>> + goto err_free; >>>> + >>>> + pages_data_guest = map_domain_page(guest_mfn); >> >> Similarly here, you may want to use access_guest_by_ipa. This will do all >> the safety check for copy from guest memory. >> >> Furthermore, I think this is going to simplify a lot this code. >> >> >>>> + if (!pages_data_guest) >>>> + goto err_free; >>>> + >>>> + pages_data_xen = pages_data_xen_start; >>>> + while ( num_pages ) { >>>> + mfn_t entry_mfn = lookup_guest_ram_addr( >>>> + pages_data_guest->pages_list[entries_on_page]); >>>> + >>>> + if ( mfn_eq(entry_mfn, INVALID_MFN) ) >>>> + goto err_unmap; >> >> You would need to get a reference on each page, and release it in err_unmap >> or when the command is done. get_page_from_gfn could do it for you. >> >>>> + >>>> + 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); >>>> + guest_mfn = lookup_guest_ram_addr(gaddr); >>>> + 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--; >>>> + } >>>> + >>>> + unmap_domain_page(pages_data_guest); >>>> + >>>> + param->u.tmem.buf_ptr = virt_to_maddr(pages_data_xen_start) | page_offset; >> >> I am not sure to understand why you are apply the offset of the guest buffer >> to xen buffer. >> >>>> + >>>> + ctx->non_contig[idx] = pages_data_xen_start; >>>> + ctx->non_contig_order[idx] = order; >>>> + >>>> + unmap_domain_page(pages_data_guest); >>>> + return true; >>>> + >>>> +err_unmap: >>>> + unmap_domain_page(pages_data_guest); >>>> +err_free: >>>> + free_xenheap_pages(pages_data_xen_start, order); >>>> + return false; >>>> +} >>>> + >>>> +static bool translate_params(struct std_call_ctx *ctx) >>>> +{ >>>> + unsigned int i; >>>> + uint32_t attr; >>>> + >>>> + for ( i = 0; i < ctx->xen_arg->num_params; i++ ) { >>>> + attr = ctx->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, ctx->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 *ctx) >>>> +{ >>>> + paddr_t cmd_gaddr, xen_addr; >>>> + mfn_t cmd_mfn; >>>> + >>>> + 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; >>>> + >>>> + cmd_mfn = lookup_guest_ram_addr(cmd_gaddr); >>>> + if ( mfn_eq(cmd_mfn, INVALID_MFN) ) >>>> + return false; >>>> + >>>> + ctx->guest_arg = map_domain_page(cmd_mfn); >>>> + if ( !ctx->guest_arg ) >>>> + return false; >>>> + >>>> + ctx->xen_arg = alloc_xenheap_page(); >>>> + if ( !ctx->xen_arg ) >>>> + return false; >>>> + >>>> + memcpy(ctx->xen_arg, ctx->guest_arg, OPTEE_MSG_NONCONTIG_PAGE_SIZE); >> >> Have a look a guest copy helpers. >> >> Cheers, >> >> [1] >> https://lists.xenproject.org/archives/html/xen-devel/2017-11/msg01481.html >> >> -- >> Julien Grall -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-04 14:30 ` Julien Grall 2017-12-04 16:24 ` Volodymyr Babchuk @ 2017-12-06 22:39 ` Julien Grall 1 sibling, 0 replies; 22+ messages in thread From: Julien Grall @ 2017-12-06 22:39 UTC (permalink / raw) To: Stefano Stabellini, Volodymyr Babchuk; +Cc: xen-devel, stuart.yoder Hi, Answering to myself. On 12/04/2017 02:30 PM, Julien Grall wrote: > On 01/12/17 22:58, Stefano Stabellini wrote: >> On Mon, 27 Nov 2017, Volodymyr Babchuk wrote: >> = Xen command forwarding = >> >> In the code below, it looks like Xen is forwarding everything to OP-TEE. >> Are there some commands Xen should avoid forwarding? Should we have a >> whitelist or a blacklist? >> >> >> = Long running OP-TEE commands and interruptions = >> >> I have been told that some OP-TEE RPC commands might take long to >> complete. Is that right? Like for example one of the >> OPTEE_MSG_RPC_CMD_*? >> >> If so, we need to think what to do in those cases. Specifically, do we >> need a technique to restart certain commands in Xen, so that when they >> run for too long and get interrupted by something (such as an >> interrupt) we know how to restart them? In fact, do we need to setup a >> timer interrupt to make sure the command doesn't block Xen for too long, >> consuming the next vcpu's slot as well? > > I am not sure to understand your suggestion here. Where do you want that > timer? In Xen? If so, I don't think this could work. That's OP-TEE job > to break down long running operation. > > This is very similar to when a guest is doing an hypercall. Even if > setup a timer, the interrupt will only be received once the hypercall is > done (or Xen decided to preempt it). So from Stuart's e-mail, I was slightly wrong about this. Most of the time a timer interrupt would get OP-TEE stopping his work and then return to the hypervisor. Although, this is still at the will of OP-TEE :). The scheduler is already setting a timer interrupt to prevent a guest running outside of its slice. So I think this would do the job here to preempt 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] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-01 22:58 ` Stefano Stabellini 2017-12-04 14:30 ` Julien Grall @ 2017-12-04 14:46 ` Andrew Cooper 2017-12-04 16:15 ` Volodymyr Babchuk 2 siblings, 0 replies; 22+ messages in thread From: Andrew Cooper @ 2017-12-04 14:46 UTC (permalink / raw) To: Stefano Stabellini, Volodymyr Babchuk Cc: xen-devel, Julien Grall, stuart.yoder On 01/12/17 22:58, Stefano Stabellini wrote: > > = Xen command forwarding = > > In the code below, it looks like Xen is forwarding everything to OP-TEE. > Are there some commands Xen should avoid forwarding? Should we have a > whitelist or a blacklist? Whitelist everything. At the very minimum, it means that patches to add to the whitelist get a chance for review. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-01 22:58 ` Stefano Stabellini 2017-12-04 14:30 ` Julien Grall 2017-12-04 14:46 ` Andrew Cooper @ 2017-12-04 16:15 ` Volodymyr Babchuk 2017-12-04 16:27 ` Julien Grall 2017-12-04 22:06 ` Stefano Stabellini 2 siblings, 2 replies; 22+ messages in thread From: Volodymyr Babchuk @ 2017-12-04 16:15 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, Julien Grall, stuart.yoder Hi Stefano, On Fri, Dec 01, 2017 at 02:58:57PM -0800, Stefano Stabellini wrote: > On Mon, 27 Nov 2017, Volodymyr Babchuk wrote: > > This is follow-up to our conversation during community call. > > You asked me to send OP-TEE mediator as a patch, so we can > > discuss it in the mailing list. So, there it is. I squashed > > two patches into one and skipped patches that we already > > discussed. > > > > So, this is basically all what is needed to support OP-TEE in XEN. > > There are some TODOs left all over the code. But, I don't > > expect that TODOs implementation would significantly > > increase codebase. Currently mediator parses requests to perform > > addresses translation and that's all what is should be done > > to allow guests to work with OP-TEE. > > > > This become possible because I completely revisited virtualization > > support in OP-TEE. I have found way to enforce complete isolation > > between different guest states. This lifts many questions like usage > > quotas, RPC routing, sudden guest death, data isolation, etc. > > > > I'm aware that I didn't addressed all comments from previous > > discussion. Sorry for this. I'm currently busy with OP-TEE, > > and I think proper mediator implementation will be possible > > only after I'll stabilize OP-TEE part. > > > > So I don't ask anybody to do thorough review. I just want to > > share my status and discuss this code a whole. > > Thank you for sharing! Actually, I think it is not too bad as a starting > point. > > I'll also try to summarize some key concept we have been discussing > about OP-TEE support in Xen. > > > = Xen cannot protect the system from a broken/insecure OP-TEE = > > OP-TEE runs at a higher privilege level than Xen, thus, we can't really > expect Xen to protect the system from a broken OP-TEE. Also, Xen cannot > really protect OP-TEE from a malicious caller. Yes, this is right. > What we can and should do is to protect Xen, the OP-TEE mediator in Xen > specifically, from malicious attackers. > > In other words, we are not responsible if a call, forwarded to OP-TEE by > Xen, ends up crashing OP-TEE, therefore taking down the system. > > However, we have to pay special care to avoid callers to crash or take > over the mediator in Xen. We also have to pay attention so that a caller > won't be able to exhaust Xen resources or DOS Xen (allocate too much > memory, infinite loops in Xen, etc). This brings me to the next topic. Yes, I see where are you going. > > = Error checking / DOS protection = > > We need powerful checks on arguments passed by the caller and evaluated > by the mediator. > > For example, we cannot expect the guest to actually pass arguments in > the format expected by translate_params. ctx->xen_arg could be > gibberish. Yes. The same arguments stands also for OP-TEE itself. OP-TEE checks validity of arguments and mediator should do the same. Actaully, I implemented this checks in mediator. > From the resource allocation point of view, it looks like every > handle_std_call allocates a new context; every copy_std_request > allocates a new Xen page. It would be easy to exhaust Xen resources. > Maybe we need a max concurrent request limit or max page allocation per > domain or something of the kind. This is a very good point. Thanks. Yes, it is currently missing. Is there any mechanism in XEN to provide quotas? I think, this mediator is not the single entity that allocates memory to handle guest calls? Also, this problem is somewhat handled from OP-TEE site: it have limited number of threads, so it can't handle many STD call simultaneously. But I wouldn't rely on OP-TEE there, of course. > > > = Locks and Lists = > > The current lock and list is global. Do you think it should actually be > global or per-domain? I do realize that only dom0 is allowed to make > calls now so the question for the moment is not too useful. Agree lists (and corresponding locks) should be domain-bound. This is in my todo list, which is included in this patch. > > = Xen command forwarding = > > In the code below, it looks like Xen is forwarding everything to OP-TEE. > Are there some commands Xen should avoid forwarding? Should we have a > whitelist or a blacklist? My code implements whitelists (at least, I hope so :-) ). It forwards only known requests. If it does not know type of the request, it returns error back to a caller. > > = Long running OP-TEE commands and interruptions = > > I have been told that some OP-TEE RPC commands might take long to > complete. Is that right? Like for example one of the > OPTEE_MSG_RPC_CMD_*? > > If so, we need to think what to do in those cases. Specifically, do we > need a technique to restart certain commands in Xen, so that when they > run for too long and get interrupted by something (such as an > interrupt) we know how to restart them? In fact, do we need to setup a > timer interrupt to make sure the command doesn't block Xen for too long, > consuming the next vcpu's slot as well? You will rely on OP-TEE there. Mediator (and, thus, XEN) does nothing heavy, it just forwards call to OP-TEE. OP-TEE in turn does RPC return to handle any IRQ that comes to the system. In this way, XEN gets control back and returns control back to guest. This is perfect time for XEN (and for guest) to handle interrupts, switchs running guests/tasks, etc. > > = Page pinning = > > Guest pages passed to OP-TEE need to be pinned (otherwise Xen doesn't > guarantee they'll be available). I think the right function to call for > that is get_page_from_gfn or get_page. Yes, I need to pin pages. I have this in my TODO list. Question is how to do this in a proper way. Julien has objections against get_page() as I can see. > > > > --- > > > > Add OP-TEE mediator as an example how TEE mediator framework > > works. > > > > 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 successfully passes own tests, while client is > > running in DomU. Currently it lacks some code for exceptional cases, > > because this patch was used mostly to debug virtualization in OP-TEE. > > Nevertheless, it provides all features needed for OP-TEE mediation. > > > > WARNING: This is a development patch, it does not cover all corner > > cases, so, please don't use it in production. > > > > It was tested on RCAR Salvator-M3 board. > > > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > > --- > > xen/arch/arm/tee/Kconfig | 4 + > > xen/arch/arm/tee/Makefile | 1 + > > xen/arch/arm/tee/optee.c | 765 +++++++++++++++++++++++++++++++++++++++++++ > > xen/arch/arm/tee/optee_smc.h | 50 +++ > > 4 files changed, 820 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..7c6b5c6 100644 > > --- a/xen/arch/arm/tee/Kconfig > > +++ b/xen/arch/arm/tee/Kconfig > > @@ -0,0 +1,4 @@ > > +config ARM_OPTEE > > + bool "Enable OP-TEE mediator" > > + default n > > + depends on ARM_TEE > > diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile > > index c54d479..9d93b42 100644 > > --- a/xen/arch/arm/tee/Makefile > > +++ b/xen/arch/arm/tee/Makefile > > @@ -1 +1,2 @@ > > obj-y += tee.o > > +obj-$(CONFIG_ARM_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..59c3600 > > --- /dev/null > > +++ b/xen/arch/arm/tee/optee.c > > @@ -0,0 +1,765 @@ > > +/* > > + * xen/arch/arm/tee/optee.c > > + * > > + * OP-TEE mediator > > + * > > + * 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/domain_page.h> > > +#include <xen/types.h> > > +#include <xen/sched.h> > > + > > +#include <asm/p2m.h> > > +#include <asm/tee.h> > > + > > +#include "optee_msg.h" > > +#include "optee_smc.h" > > + > > +/* > > + * Global TODO: > > + * 1. Create per-domain context, where call and shm will be stored > > + * 2. Pin pages shared between OP-TEE and guest > > + */ > > +/* > > + * OP-TEE violates SMCCC when it defines own UID. So we need > > + * to place bytes in correct order. > > + */ > > +#define OPTEE_UID (xen_uuid_t){{ \ > > + (uint8_t)(OPTEE_MSG_UID_0 >> 0), (uint8_t)(OPTEE_MSG_UID_0 >> 8), \ > > + (uint8_t)(OPTEE_MSG_UID_0 >> 16), (uint8_t)(OPTEE_MSG_UID_0 >> 24), \ > > + (uint8_t)(OPTEE_MSG_UID_1 >> 0), (uint8_t)(OPTEE_MSG_UID_1 >> 8), \ > > + (uint8_t)(OPTEE_MSG_UID_1 >> 16), (uint8_t)(OPTEE_MSG_UID_1 >> 24), \ > > + (uint8_t)(OPTEE_MSG_UID_2 >> 0), (uint8_t)(OPTEE_MSG_UID_2 >> 8), \ > > + (uint8_t)(OPTEE_MSG_UID_2 >> 16), (uint8_t)(OPTEE_MSG_UID_2 >> 24), \ > > + (uint8_t)(OPTEE_MSG_UID_3 >> 0), (uint8_t)(OPTEE_MSG_UID_3 >> 8), \ > > + (uint8_t)(OPTEE_MSG_UID_3 >> 16), (uint8_t)(OPTEE_MSG_UID_3 >> 24), \ > > + }} > > + > > +#define MAX_NONCONTIG_ENTRIES 8 > > + > > +/* > > + * Call context. OP-TEE can issue mulitple 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]; > > + int optee_thread_id; > > + int rpc_op; > > + domid_t domid; > > +}; > > +static LIST_HEAD(call_ctx_list); > > +static DEFINE_SPINLOCK(call_ctx_list_lock); > > + > > +/* > > + * Command buffer shared between OP-TEE and guest. > > + * Warning! In the proper implementation this SHM buffer *probably* should > > + * by shadowed by XEN. > > + * TODO: Reconsider this. > > + */ > > +struct shm { > > + struct list_head list; > > + struct optee_msg_arg *guest_arg; > > + struct page *guest_page; > > + mfn_t guest_mfn; > > + uint64_t cookie; > > + domid_t domid; > > +}; > > + > > +static LIST_HEAD(shm_list); > > +static DEFINE_SPINLOCK(shm_list_lock); > > + > > +static int optee_init(void) > > +{ > > + printk("OP-TEE mediator init done\n"); > > + return 0; > > +} > > + > > +static void optee_domain_create(struct domain *d) > > +{ > > + register_t resp[4]; > > + call_smccc_smc(OPTEE_SMC_VM_CREATED, > > + d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); > > + if ( resp[0] != OPTEE_SMC_RETURN_OK ) > > + gprintk(XENLOG_WARNING, "OP-TEE don't want to support domain: %d\n", > > + (uint32_t)resp[0]); > > + /* TODO: Change function declaration to be able to retun error */ > > +} > > + > > +static void optee_domain_destroy(struct domain *d) > > +{ > > + register_t resp[4]; > > + call_smccc_smc(OPTEE_SMC_VM_DESTROYED, > > + d->domain_id + 1, 0, 0, 0, 0, 0, 0, resp); > > + /* TODO: Clean call contexts and SHMs associated with domain */ > > +} > > + > > +static bool forward_call(struct cpu_user_regs *regs) > > +{ > > + register_t resp[4]; > > + > > + /* TODO: Use separate registers set to prevent leakage to guest */ > > + 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), > > + /* VM id 0 is reserved for hypervisor itself */ > > + current->domain->domain_id + 1, > > This doesn't look like it would wrap around safely. > > > > + resp); > > + > > + set_user_reg(regs, 0, resp[0]); > > + set_user_reg(regs, 1, resp[1]); > > + set_user_reg(regs, 2, resp[2]); > > + set_user_reg(regs, 3, resp[3]); > > + > > + return true; > > +} > > + > > +static struct std_call_ctx *allocate_std_call_ctx(void) > > +{ > > + struct std_call_ctx *ret; > > + > > + ret = xzalloc(struct std_call_ctx); > > + if ( !ret ) > > + return NULL; > > + > > + ret->optee_thread_id = -1; > > + ret->domid = -1; > > + > > + spin_lock(&call_ctx_list_lock); > > + list_add_tail(&ret->list, &call_ctx_list); > > + spin_unlock(&call_ctx_list_lock); > > + > > + return ret; > > +} > > + > > +static void free_std_call_ctx(struct std_call_ctx *ctx) > > +{ > > + spin_lock(&call_ctx_list_lock); > > + list_del(&ctx->list); > > + spin_unlock(&call_ctx_list_lock); > > + > > + if (ctx->xen_arg) > > + free_xenheap_page(ctx->xen_arg); > > + > > + if (ctx->guest_arg) > > + unmap_domain_page(ctx->guest_arg); > > + > > + for (int i = 0; i < MAX_NONCONTIG_ENTRIES; i++) { > > + if (ctx->non_contig[i]) > > + free_xenheap_pages(ctx->non_contig[i], ctx->non_contig_order[i]); > > + } > > + > > + xfree(ctx); > > +} > > + > > +static struct std_call_ctx *find_ctx(int thread_id, domid_t domid) > > +{ > > + struct std_call_ctx *ctx; > > + > > + spin_lock(&call_ctx_list_lock); > > + list_for_each_entry( ctx, &call_ctx_list, list ) > > + { > > + if (ctx->domid == domid && ctx->optee_thread_id == thread_id ) > > + { > > + spin_unlock(&call_ctx_list_lock); > > + return ctx; > > + } > > + } > > + spin_unlock(&call_ctx_list_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 mfn_t lookup_guest_ram_addr(paddr_t gaddr) > > +{ > > + mfn_t mfn; > > + gfn_t gfn; > > + p2m_type_t t; > > + gfn = gaddr_to_gfn(gaddr); > > + mfn = p2m_lookup(current->domain, gfn, &t); > > + if ( t != p2m_ram_rw || mfn_eq(mfn, INVALID_MFN) ) { > > + gprintk(XENLOG_INFO, "Domain tries to use invalid gfn\n"); > > + return INVALID_MFN; > > + } > > + return mfn; > > +} > > + > > +static struct shm *allocate_and_map_shm(paddr_t gaddr, uint64_t cookie) > > +{ > > + struct shm *ret; > > + > > + ret = xzalloc(struct shm); > > + if ( !ret ) > > + return NULL; > > + > > + ret->guest_mfn = lookup_guest_ram_addr(gaddr); > > + > > + if ( mfn_eq(ret->guest_mfn, INVALID_MFN) ) > > + { > > + xfree(ret); > > + return NULL; > > + } > > + > > + ret->guest_arg = map_domain_page(ret->guest_mfn); > > + if ( !ret->guest_arg ) > > + { > > + gprintk(XENLOG_INFO, "Could not map domain page\n"); > > + xfree(ret); > > + return NULL; > > + } > > + ret->cookie = cookie; > > + ret->domid = current->domain->domain_id; > > + > > + spin_lock(&shm_list_lock); > > + list_add_tail(&ret->list, &shm_list); > > + spin_unlock(&shm_list_lock); > > + return ret; > > +} > > + > > +static void free_shm(uint64_t cookie, domid_t domid) > > +{ > > + struct shm *shm, *found = NULL; > > + spin_lock(&shm_list_lock); > > + > > + list_for_each_entry( shm, &shm_list, list ) > > + { > > + if (shm->domid == domid && shm->cookie == cookie ) > > + { > > + found = shm; > > + list_del(&found->list); > > + break; > > + } > > + } > > + spin_unlock(&shm_list_lock); > > + > > + if ( !found ) { > > + return; > > + } > > + > > + if ( found->guest_arg ) > > + unmap_domain_page(found->guest_arg); > > + > > + xfree(found); > > +} > > + > > +static struct shm *find_shm(uint64_t cookie, domid_t domid) > > +{ > > + struct shm *shm; > > + > > + spin_lock(&shm_list_lock); > > + list_for_each_entry( shm, &shm_list, list ) > > + { > > + if ( shm->domid == domid && shm->cookie == cookie ) > > + { > > + spin_unlock(&shm_list_lock); > > + return shm; > > + } > > + } > > + spin_unlock(&shm_list_lock); > > + > > + return NULL; > > +} > > + > > +static bool translate_noncontig(struct std_call_ctx *ctx, > > + struct optee_msg_param *param, > > + int idx) > > +{ > > + /* > > + * Refer to OPTEE_MSG_ATTR_NONCONTIG description in optee_msg.h for details. > > + * > > + * WARNING: This is test code. It works only with xen page size == 4096 > > + */ > > + 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; > > + > > + 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; > > + > > + gaddr = param->u.tmem.buf_ptr & ~(OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1); > > + guest_mfn = lookup_guest_ram_addr(gaddr); > > + 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 ) { > > + mfn_t entry_mfn = lookup_guest_ram_addr( > > + pages_data_guest->pages_list[entries_on_page]); > > + > > + if ( mfn_eq(entry_mfn, INVALID_MFN) ) > > + goto err_unmap; > > + > > + 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); > > + guest_mfn = lookup_guest_ram_addr(gaddr); > > + 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--; > > + } > > + > > + unmap_domain_page(pages_data_guest); > > + > > + param->u.tmem.buf_ptr = virt_to_maddr(pages_data_xen_start) | page_offset; > > + > > + ctx->non_contig[idx] = pages_data_xen_start; > > + ctx->non_contig_order[idx] = order; > > + > > + unmap_domain_page(pages_data_guest); > > + return true; > > + > > +err_unmap: > > + unmap_domain_page(pages_data_guest); > > +err_free: > > + free_xenheap_pages(pages_data_xen_start, order); > > + return false; > > +} > > + > > +static bool translate_params(struct std_call_ctx *ctx) > > +{ > > + unsigned int i; > > + uint32_t attr; > > + > > + for ( i = 0; i < ctx->xen_arg->num_params; i++ ) { > > + attr = ctx->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, ctx->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 *ctx) > > +{ > > + paddr_t cmd_gaddr, xen_addr; > > + mfn_t cmd_mfn; > > + > > + 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; > > + > > + cmd_mfn = lookup_guest_ram_addr(cmd_gaddr); > > + if ( mfn_eq(cmd_mfn, INVALID_MFN) ) > > + return false; > > + > > + ctx->guest_arg = map_domain_page(cmd_mfn); > > + if ( !ctx->guest_arg ) > > + return false; > > + > > + ctx->xen_arg = alloc_xenheap_page(); > > + if ( !ctx->xen_arg ) > > + return false; > > + > > + memcpy(ctx->xen_arg, ctx->guest_arg, OPTEE_MSG_NONCONTIG_PAGE_SIZE); > > + > > + xen_addr = virt_to_maddr(ctx->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 cpu_user_regs *regs, > > + struct std_call_ctx *ctx) > > +{ > > + unsigned int i; > > + uint32_t attr; > > + > > + ctx->guest_arg->ret = ctx->xen_arg->ret; > > + ctx->guest_arg->ret_origin = ctx->xen_arg->ret_origin; > > + ctx->guest_arg->session = ctx->xen_arg->session; > > + for ( i = 0; i < ctx->xen_arg->num_params; i++ ) { > > + attr = ctx->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: > > + ctx->guest_arg->params[i].u.tmem.size = > > + ctx->xen_arg->params[i].u.tmem.size; > > + continue; > > + case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT: > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT: > > + ctx->guest_arg->params[i].u.value.a = > > + ctx->xen_arg->params[i].u.value.a; > > + ctx->guest_arg->params[i].u.value.b = > > + ctx->xen_arg->params[i].u.value.b; > > + continue; > > + case OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT: > > + case OPTEE_MSG_ATTR_TYPE_RMEM_INOUT: > > + ctx->guest_arg->params[i].u.rmem.size = > > + ctx->xen_arg->params[i].u.rmem.size; > > + continue; > > + case OPTEE_MSG_ATTR_TYPE_NONE: > > + case OPTEE_MSG_ATTR_TYPE_TMEM_INPUT: > > + case OPTEE_MSG_ATTR_TYPE_RMEM_INPUT: > > + case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT: > > + continue; > > + } > > + } > > + return true; > > +} > > + > > +static bool execute_std_call(struct cpu_user_regs *regs, > > + struct std_call_ctx *ctx) > > +{ > > + register_t optee_ret; > > + forward_call(regs); > > + optee_ret = get_user_reg(regs, 0); > > + > > + if ( OPTEE_SMC_RETURN_IS_RPC(optee_ret) ) > > + { > > + ctx->optee_thread_id = get_user_reg(regs, 3); > > + ctx->rpc_op = OPTEE_SMC_RETURN_GET_RPC_FUNC(optee_ret); > > + return true; > > + } > > + > > + copy_std_request_back(regs, ctx); > > + free_std_call_ctx(ctx); > > + > > + return true; > > +} > > + > > +static bool handle_std_call(struct cpu_user_regs *regs) > > +{ > > + struct std_call_ctx *ctx; > > + bool ret; > > + > > + ctx = allocate_std_call_ctx(); > > + > > + if (!ctx) > > + return false; > > + > > + ctx->domid = current->domain->domain_id; > > + > > + ret = copy_std_request(regs, ctx); > > + if ( !ret ) > > + goto out; > > + > > + /* Now we can safely examine contents of command buffer */ > > + if ( OPTEE_MSG_GET_ARG_SIZE(ctx->xen_arg->num_params) > > > + OPTEE_MSG_NONCONTIG_PAGE_SIZE ) { > > + ret = false; > > + goto out; > > + } > > + > > + switch ( ctx->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); > > + break; > > + default: > > + ret = false; > > + } > > + > > + if (!ret) > > + goto out; > > + > > + ret = execute_std_call(regs, ctx); > > + > > +out: > > + if (!ret) > > + free_std_call_ctx(ctx); > > + > > + return ret; > > +} > > + > > +static void handle_rpc_cmd_alloc(struct cpu_user_regs *regs, > > + struct std_call_ctx *ctx, > > + struct shm *shm) > > +{ > > + if ( shm->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 ( ctx->non_contig[MAX_NONCONTIG_ENTRIES - 1] ) > > + { > > + free_xenheap_pages(ctx->non_contig[7], > > + ctx->non_contig_order[MAX_NONCONTIG_ENTRIES - 1]); > > + ctx->non_contig[7] = NULL; > > + } > > + translate_noncontig(ctx, shm->guest_arg->params + 0, > > + MAX_NONCONTIG_ENTRIES - 1); > > +} > > + > > +static void handle_rpc_cmd(struct cpu_user_regs *regs, struct std_call_ctx *ctx) > > +{ > > + struct shm *shm; > > + uint64_t cookie; > > + > > + cookie = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2); > > + > > + shm = find_shm(cookie, current->domain->domain_id); > > + > > + if ( !shm ) > > + { > > + gprintk(XENLOG_ERR, "Can't find SHM with cookie %lx\n", cookie); > > + return; > > + } > > + > > + switch (shm->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(regs, ctx, shm); > > + break; > > + case OPTEE_MSG_RPC_CMD_SHM_FREE: > > + break; > > + default: > > + break; > > + } > > +} > > + > > +static void handle_rpc_func_alloc(struct cpu_user_regs *regs, > > + struct std_call_ctx *ctx) > > +{ > > + 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 *shm; > > + > > + shm = allocate_and_map_shm(ptr, cookie); > > + if ( !shm ) > > + { > > + gprintk(XENLOG_WARNING, "Failed to callocate allocate SHM\n"); > > + ptr = 0; > > + } > > + else > > + ptr = mfn_to_maddr(shm->guest_mfn); > > + > > + set_user_reg(regs, 1, ptr >> 32); > > + set_user_reg(regs, 2, ptr & 0xFFFFFFFF); > > + } > > +} > > + > > +static bool handle_rpc(struct cpu_user_regs *regs) > > +{ > > + struct std_call_ctx *ctx; > > + > > + int optee_thread_id = get_user_reg(regs, 3); > > + > > + ctx = find_ctx(optee_thread_id, current->domain->domain_id); > > + > > + if (!ctx) > > + return false; > > + > > + switch ( ctx->rpc_op ) { > > + case OPTEE_SMC_RPC_FUNC_ALLOC: > > + handle_rpc_func_alloc(regs, ctx); > > + break; > > + case OPTEE_SMC_RPC_FUNC_FREE: > > + { > > + uint64_t cookie = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2); > > + free_shm(cookie, current->domain->domain_id); > > + break; > > + } > > + case OPTEE_SMC_RPC_FUNC_FOREIGN_INTR: > > + break; > > + case OPTEE_SMC_RPC_FUNC_CMD: > > + handle_rpc_cmd(regs, ctx); > > + break; > > + } > > + > > + return execute_std_call(regs, ctx); > > +} > > + > > +static bool handle_get_shm_config(struct cpu_user_regs *regs) > > +{ > > + paddr_t shm_start; > > + size_t shm_size; > > + int rc; > > + > > + /* Give all static SHM region to the hardware domain */ > > + if ( !is_hardware_domain(current->domain) ) > > + return false; > > + > > + forward_call(regs); > > + > > + /* Return error back to the guest */ > > + if ( get_user_reg(regs, 0) != OPTEE_SMC_RETURN_OK) > > + return true; > > + > > + shm_start = get_user_reg(regs, 1); > > + shm_size = get_user_reg(regs, 2); > > + > > + /* HW dom is mapped 1:1 initially */ > > + rc = map_regions_p2mt(current->domain, gaddr_to_gfn(shm_start), > > + shm_size / PAGE_SIZE, > > + maddr_to_mfn(shm_start), > > + p2m_ram_rw); > > + if ( rc < 0 ) > > + { > > + gprintk(XENLOG_INFO, "OP-TEE: Can't map static shm for Dom0: %d\n", rc); > > + set_user_reg(regs, 0, OPTEE_SMC_RETURN_ENOTAVAIL); > > + } > > + > > + return true; > > +} > > + > > +static bool handle_exchange_capabilities(struct cpu_user_regs *regs) > > +{ > > + forward_call(regs); > > + > > + /* Return error back to the guest */ > > + if ( get_user_reg(regs, 0) != OPTEE_SMC_RETURN_OK ) > > + return true; > > + > > + /* Don't allow guests to work without dynamic SHM */ > > + if ( !(get_user_reg(regs, 1) & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM) ) > > + set_user_reg(regs, 0, OPTEE_SMC_RETURN_ENOTAVAIL); > > + return true; > > +} > > + > > +static bool optee_handle_smc(struct cpu_user_regs *regs) > > +{ > > + > > + switch ( get_user_reg(regs, 0) ) > > + { > > + case OPTEE_SMC_GET_SHM_CONFIG: > > + return handle_get_shm_config(regs); > > + case OPTEE_SMC_EXCHANGE_CAPABILITIES: > > + return handle_exchange_capabilities(regs); > > + case OPTEE_SMC_CALL_WITH_ARG: > > + return handle_std_call(regs); > > + case OPTEE_SMC_CALL_RETURN_FROM_RPC: > > + return handle_rpc(regs); > > + default: > > + return forward_call(regs); > > + } > > + return false; > > +} > > + > > +static void optee_remove(void) > > +{ > > +} > > + > > +static const struct tee_mediator_ops optee_ops = > > +{ > > + .init = optee_init, > > + .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_UID, &optee_ops); > > + > > +/* > > + * Local variables: > > + * mode: C > > + * c-file-style: "BSD" > > + * c-basic-offset: 4 > > + * indent-tabs-mode: nil > > + * End: > > + */ > > diff --git a/xen/arch/arm/tee/optee_smc.h b/xen/arch/arm/tee/optee_smc.h > > index 92f4d54..2e9df34 100644 > > --- a/xen/arch/arm/tee/optee_smc.h > > +++ b/xen/arch/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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-04 16:15 ` Volodymyr Babchuk @ 2017-12-04 16:27 ` Julien Grall 2017-12-04 18:32 ` Volodymyr Babchuk 2017-12-04 22:06 ` Stefano Stabellini 1 sibling, 1 reply; 22+ messages in thread From: Julien Grall @ 2017-12-04 16:27 UTC (permalink / raw) To: Volodymyr Babchuk, Stefano Stabellini; +Cc: xen-devel, stuart.yoder On 04/12/17 16:15, Volodymyr Babchuk wrote: > Hi Stefano, > > On Fri, Dec 01, 2017 at 02:58:57PM -0800, Stefano Stabellini wrote: >> On Mon, 27 Nov 2017, Volodymyr Babchuk wrote: >>> This is follow-up to our conversation during community call. >>> You asked me to send OP-TEE mediator as a patch, so we can >>> discuss it in the mailing list. So, there it is. I squashed >>> two patches into one and skipped patches that we already >>> discussed. >>> >>> So, this is basically all what is needed to support OP-TEE in XEN. >>> There are some TODOs left all over the code. But, I don't >>> expect that TODOs implementation would significantly >>> increase codebase. Currently mediator parses requests to perform >>> addresses translation and that's all what is should be done >>> to allow guests to work with OP-TEE. >>> >>> This become possible because I completely revisited virtualization >>> support in OP-TEE. I have found way to enforce complete isolation >>> between different guest states. This lifts many questions like usage >>> quotas, RPC routing, sudden guest death, data isolation, etc. >>> >>> I'm aware that I didn't addressed all comments from previous >>> discussion. Sorry for this. I'm currently busy with OP-TEE, >>> and I think proper mediator implementation will be possible >>> only after I'll stabilize OP-TEE part. >>> >>> So I don't ask anybody to do thorough review. I just want to >>> share my status and discuss this code a whole. >> >> Thank you for sharing! Actually, I think it is not too bad as a starting >> point. >> >> I'll also try to summarize some key concept we have been discussing >> about OP-TEE support in Xen. >> >> >> = Xen cannot protect the system from a broken/insecure OP-TEE = >> >> OP-TEE runs at a higher privilege level than Xen, thus, we can't really >> expect Xen to protect the system from a broken OP-TEE. Also, Xen cannot >> really protect OP-TEE from a malicious caller. > Yes, this is right. > >> What we can and should do is to protect Xen, the OP-TEE mediator in Xen >> specifically, from malicious attackers. >> >> In other words, we are not responsible if a call, forwarded to OP-TEE by >> Xen, ends up crashing OP-TEE, therefore taking down the system. >> >> However, we have to pay special care to avoid callers to crash or take >> over the mediator in Xen. We also have to pay attention so that a caller >> won't be able to exhaust Xen resources or DOS Xen (allocate too much >> memory, infinite loops in Xen, etc). This brings me to the next topic. > Yes, I see where are you going. > >> >> = Error checking / DOS protection = >> >> We need powerful checks on arguments passed by the caller and evaluated >> by the mediator. >> >> For example, we cannot expect the guest to actually pass arguments in >> the format expected by translate_params. ctx->xen_arg could be >> gibberish. > Yes. The same arguments stands also for OP-TEE itself. OP-TEE checks > validity of arguments and mediator should do the same. Actaully, I > implemented this checks in mediator. > >> From the resource allocation point of view, it looks like every >> handle_std_call allocates a new context; every copy_std_request >> allocates a new Xen page. It would be easy to exhaust Xen resources. >> Maybe we need a max concurrent request limit or max page allocation per >> domain or something of the kind. > This is a very good point. Thanks. Yes, it is currently missing. > Is there any mechanism in XEN to provide quotas? I think, this mediator > is not the single entity that allocates memory to handle guest calls? Most of the time, the memory is either accounted to the guest or only a small amount of memory is allocated for a known period of time (the time of an hypercall for instance). > > Also, this problem is somewhat handled from OP-TEE site: it have limited > number of threads, so it can't handle many STD call simultaneously. But > I wouldn't rely on OP-TEE there, of course. Does it mean OP-TEE will deny the call if it can't handle it? Or will it be put on hold? [..] >> >> = Page pinning = >> >> Guest pages passed to OP-TEE need to be pinned (otherwise Xen doesn't >> guarantee they'll be available). I think the right function to call for >> that is get_page_from_gfn or get_page. > Yes, I need to pin pages. I have this in my TODO list. Question is how > to do this in a proper way. Julien has objections against get_page() > as I can see. If you saw my objection about get_page(), you also saw my suggestions on how to do proper pinning in Xen. 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] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-04 16:27 ` Julien Grall @ 2017-12-04 18:32 ` Volodymyr Babchuk 2017-12-04 22:04 ` Stefano Stabellini [not found] ` <f9fe1cb6-8700-99ba-ee62-2217fb5eb041@linaro.org> 0 siblings, 2 replies; 22+ messages in thread From: Volodymyr Babchuk @ 2017-12-04 18:32 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini, stuart.yoder Hi Julien, On Mon, Dec 04, 2017 at 04:27:14PM +0000, Julien Grall wrote: [...] > >>= Error checking / DOS protection = > >> > >>We need powerful checks on arguments passed by the caller and evaluated > >>by the mediator. > >> > >>For example, we cannot expect the guest to actually pass arguments in > >>the format expected by translate_params. ctx->xen_arg could be > >>gibberish. > >Yes. The same arguments stands also for OP-TEE itself. OP-TEE checks > >validity of arguments and mediator should do the same. Actaully, I > >implemented this checks in mediator. > > > >> From the resource allocation point of view, it looks like every > >>handle_std_call allocates a new context; every copy_std_request > >>allocates a new Xen page. It would be easy to exhaust Xen resources. > >>Maybe we need a max concurrent request limit or max page allocation per > >>domain or something of the kind. > >This is a very good point. Thanks. Yes, it is currently missing. > >Is there any mechanism in XEN to provide quotas? I think, this mediator > >is not the single entity that allocates memory to handle guest calls? > > Most of the time, the memory is either accounted to the guest or only a > small amount of memory is allocated for a known period of time (the time of > an hypercall for instance). Aha, so in my case, I will need to implement own quota mechanism. I think something like "max_pages", initialized with value from xenpolicy will be fine. What do you think? > > > >Also, this problem is somewhat handled from OP-TEE site: it have limited > >number of threads, so it can't handle many STD call simultaneously. But > >I wouldn't rely on OP-TEE there, of course. > > Does it mean OP-TEE will deny the call if it can't handle it? Or will it be > put on hold? OP-TEE will return OPTEE_SMC_RETURN_ETHREAD_LIMIT. Current behavior of optee driver is to wait on a wait queue until another thread will complete its call. So, from OP-TEE OS side - it is a call denial. But from OP-TEE client point of view this is a hold, thanks to mentioned behavior of driver. > [..] > > >> > >>= Page pinning = > >> > >>Guest pages passed to OP-TEE need to be pinned (otherwise Xen doesn't > >>guarantee they'll be available). I think the right function to call for > >>that is get_page_from_gfn or get_page. > >Yes, I need to pin pages. I have this in my TODO list. Question is how > >to do this in a proper way. Julien has objections against get_page() > >as I can see. > > If you saw my objection about get_page(), you also saw my suggestions on how > to do proper pinning in Xen. Yes, I'm sorry, I missed second part of your email. -- Volodymyr Babchuk _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-04 18:32 ` Volodymyr Babchuk @ 2017-12-04 22:04 ` Stefano Stabellini 2017-12-05 11:14 ` Julien Grall [not found] ` <f9fe1cb6-8700-99ba-ee62-2217fb5eb041@linaro.org> 1 sibling, 1 reply; 22+ messages in thread From: Stefano Stabellini @ 2017-12-04 22:04 UTC (permalink / raw) To: Volodymyr Babchuk Cc: xen-devel, Julien Grall, Stefano Stabellini, stuart.yoder On Mon, 4 Dec 2017, Volodymyr Babchuk wrote: > Hi Julien, > > > On Mon, Dec 04, 2017 at 04:27:14PM +0000, Julien Grall wrote: > > [...] > > >>= Error checking / DOS protection = > > >> > > >>We need powerful checks on arguments passed by the caller and evaluated > > >>by the mediator. > > >> > > >>For example, we cannot expect the guest to actually pass arguments in > > >>the format expected by translate_params. ctx->xen_arg could be > > >>gibberish. > > >Yes. The same arguments stands also for OP-TEE itself. OP-TEE checks > > >validity of arguments and mediator should do the same. Actaully, I > > >implemented this checks in mediator. > > > > > >> From the resource allocation point of view, it looks like every > > >>handle_std_call allocates a new context; every copy_std_request > > >>allocates a new Xen page. It would be easy to exhaust Xen resources. > > >>Maybe we need a max concurrent request limit or max page allocation per > > >>domain or something of the kind. > > >This is a very good point. Thanks. Yes, it is currently missing. > > >Is there any mechanism in XEN to provide quotas? I think, this mediator > > >is not the single entity that allocates memory to handle guest calls? > > > > Most of the time, the memory is either accounted to the guest or only a > > small amount of memory is allocated for a known period of time (the time of > > an hypercall for instance). > Aha, so in my case, I will need to implement own quota mechanism. > I think something like "max_pages", initialized with value from > xenpolicy will be fine. What do you think? Yes, that should work. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-04 22:04 ` Stefano Stabellini @ 2017-12-05 11:14 ` Julien Grall 0 siblings, 0 replies; 22+ messages in thread From: Julien Grall @ 2017-12-05 11:14 UTC (permalink / raw) To: Stefano Stabellini, Volodymyr Babchuk; +Cc: xen-devel, stuart.yoder Hi, On 04/12/17 22:04, Stefano Stabellini wrote: > On Mon, 4 Dec 2017, Volodymyr Babchuk wrote: >> Hi Julien, >> >> >> On Mon, Dec 04, 2017 at 04:27:14PM +0000, Julien Grall wrote: >> >> [...] >>>>> = Error checking / DOS protection = >>>>> >>>>> We need powerful checks on arguments passed by the caller and evaluated >>>>> by the mediator. >>>>> >>>>> For example, we cannot expect the guest to actually pass arguments in >>>>> the format expected by translate_params. ctx->xen_arg could be >>>>> gibberish. >>>> Yes. The same arguments stands also for OP-TEE itself. OP-TEE checks >>>> validity of arguments and mediator should do the same. Actaully, I >>>> implemented this checks in mediator. >>>> >>>>> From the resource allocation point of view, it looks like every >>>>> handle_std_call allocates a new context; every copy_std_request >>>>> allocates a new Xen page. It would be easy to exhaust Xen resources. >>>>> Maybe we need a max concurrent request limit or max page allocation per >>>>> domain or something of the kind. >>>> This is a very good point. Thanks. Yes, it is currently missing. >>>> Is there any mechanism in XEN to provide quotas? I think, this mediator >>>> is not the single entity that allocates memory to handle guest calls? >>> >>> Most of the time, the memory is either accounted to the guest or only a >>> small amount of memory is allocated for a known period of time (the time of >>> an hypercall for instance). >> Aha, so in my case, I will need to implement own quota mechanism. >> I think something like "max_pages", initialized with value from >> xenpolicy will be fine. What do you think? > > Yes, that should work. I think "max_pages" will be difficult to size by a user. It would be better to think about another metrics (e.g number of OP-TEE commands in //) and/or limit the use of xmalloc within the code. 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] 22+ messages in thread
[parent not found: <f9fe1cb6-8700-99ba-ee62-2217fb5eb041@linaro.org>]
[parent not found: <20171204185929.GB30163@EPUAKYIW2556.kyiv.epam.com>]
[parent not found: <a6a05ad8-f335-00a2-cdd0-add81deb87bf@linaro.org>]
[parent not found: <2ee9297a-fcb8-c1c0-8ca7-91b9adbcd5b1@epam.com>]
[parent not found: <a7af95e4-09cf-22f8-b8cd-23a0cb5baae8@linaro.org>]
* Re: [RFC] WIP: optee: add OP-TEE mediator [not found] ` <a7af95e4-09cf-22f8-b8cd-23a0cb5baae8@linaro.org> @ 2017-12-05 15:36 ` Stuart Yoder 2017-12-06 22:31 ` Julien Grall 0 siblings, 1 reply; 22+ messages in thread From: Stuart Yoder @ 2017-12-05 15:36 UTC (permalink / raw) To: Julien Grall, Volodymyr Babchuk, Stefano Stabellini; +Cc: xen-devel >> There are limit on pCPUs, though. But this is not a problem, because >> XEN scheduler will decide which guest will access OP-TEE right now. >> OP-TEE don't have own scheduler at all, by the way. It is scheduled >> by normal world. > > Do you mind to give a bit more explanation here? Do you plan to add knowledge of OP-TEE in the scheduler? Regarding scheduling-- OP-TEE runs with interrupts enabled (generally). So when an SMC is in process in OP-TEE and the normal OS or hypervisor timer tick fires, OP-TEE halts the current the in-progress thread, saves states, and returns to the normal world to let the normal world timer interrupt handler and scheduler do its normal thing. Eventually when the normal world thread is re-scheduled and the in-progress thread restarts the SMC. That process continues until the SMC is completely done. So the OS/VMM scheduler needs no awareness of OP-TEE since OP-TEE is cooperating with the normal world. Stuart _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-05 15:36 ` Stuart Yoder @ 2017-12-06 22:31 ` Julien Grall 2017-12-07 16:32 ` Stuart Yoder 0 siblings, 1 reply; 22+ messages in thread From: Julien Grall @ 2017-12-06 22:31 UTC (permalink / raw) To: Stuart Yoder, Volodymyr Babchuk, Stefano Stabellini; +Cc: xen-devel Hi Stuart, On 12/05/2017 03:36 PM, Stuart Yoder wrote: > >>> There are limit on pCPUs, though. But this is not a problem, because >>> XEN scheduler will decide which guest will access OP-TEE right now. >>> OP-TEE don't have own scheduler at all, by the way. It is scheduled >>> by normal world. >> >> Do you mind to give a bit more explanation here? Do you plan to add >> knowledge of OP-TEE in the scheduler? > > Regarding scheduling-- OP-TEE runs with interrupts enabled (generally). > So when an SMC > is in process in OP-TEE and the normal OS or hypervisor timer tick > fires, OP-TEE halts > the current the in-progress thread, saves states, and returns to the > normal world to > let the normal world timer interrupt handler and scheduler do its normal > thing. Eventually > when the normal world thread is re-scheduled and the in-progress thread > restarts the > SMC. That process continues until the SMC is completely done. So the > OS/VMM scheduler > needs no awareness of OP-TEE since OP-TEE is cooperating with the normal > world. Thank you for the explanation. I see you specifically mention the hypervisor timer tick. How about the other interrupts? Will OP-TEE halts the current in-progress thread and then return to Xen/OS? 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] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-06 22:31 ` Julien Grall @ 2017-12-07 16:32 ` Stuart Yoder 0 siblings, 0 replies; 22+ messages in thread From: Stuart Yoder @ 2017-12-07 16:32 UTC (permalink / raw) To: Julien Grall, Volodymyr Babchuk, Stefano Stabellini; +Cc: xen-devel On 12/6/17 4:31 PM, Julien Grall wrote: > Hi Stuart, > > On 12/05/2017 03:36 PM, Stuart Yoder wrote: >> >>>> There are limit on pCPUs, though. But this is not a problem, because >>>> XEN scheduler will decide which guest will access OP-TEE right now. >>>> OP-TEE don't have own scheduler at all, by the way. It is scheduled >>>> by normal world. >>> >>> Do you mind to give a bit more explanation here? Do you plan to add knowledge of OP-TEE in the scheduler? >> >> Regarding scheduling-- OP-TEE runs with interrupts enabled (generally). So when an SMC >> is in process in OP-TEE and the normal OS or hypervisor timer tick fires, OP-TEE halts >> the current the in-progress thread, saves states, and returns to the normal world to >> let the normal world timer interrupt handler and scheduler do its normal thing. Eventually >> when the normal world thread is re-scheduled and the in-progress thread restarts the >> SMC. That process continues until the SMC is completely done. So the OS/VMM scheduler >> needs no awareness of OP-TEE since OP-TEE is cooperating with the normal world. > > Thank you for the explanation. I see you specifically mention the hypervisor timer tick. How about the other interrupts? Will OP-TEE halts the current in-progress thread and then return to Xen/OS? Yes, that is my understanding. There are a few "fast" SMC calls that run with interrupts disabled, but other than that any other normal world interrupt will cause OP-TEE to halt/exit and return to normal world. Stuart _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-04 16:15 ` Volodymyr Babchuk 2017-12-04 16:27 ` Julien Grall @ 2017-12-04 22:06 ` Stefano Stabellini 2017-12-05 13:07 ` Volodymyr Babchuk 1 sibling, 1 reply; 22+ messages in thread From: Stefano Stabellini @ 2017-12-04 22:06 UTC (permalink / raw) To: Volodymyr Babchuk Cc: xen-devel, Stefano Stabellini, stuart.yoder, Julien Grall On Mon, 4 Dec 2017, Volodymyr Babchuk wrote: > > = Xen command forwarding = > > > > In the code below, it looks like Xen is forwarding everything to OP-TEE. > > Are there some commands Xen should avoid forwarding? Should we have a > > whitelist or a blacklist? > My code implements whitelists (at least, I hope so :-) ). It forwards > only known requests. If it does not know type of the request, it > returns error back to a caller. Actually, see below: > > > +static bool optee_handle_smc(struct cpu_user_regs *regs) > > > +{ > > > + > > > + switch ( get_user_reg(regs, 0) ) > > > + { > > > + case OPTEE_SMC_GET_SHM_CONFIG: > > > + return handle_get_shm_config(regs); > > > + case OPTEE_SMC_EXCHANGE_CAPABILITIES: > > > + return handle_exchange_capabilities(regs); > > > + case OPTEE_SMC_CALL_WITH_ARG: > > > + return handle_std_call(regs); > > > + case OPTEE_SMC_CALL_RETURN_FROM_RPC: > > > + return handle_rpc(regs); > > > + default: > > > + return forward_call(regs); > > > + } > > > + return false; > > > +} In the unknown ("default") case the smc is still forwarded. Am I missing anything? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] WIP: optee: add OP-TEE mediator 2017-12-04 22:06 ` Stefano Stabellini @ 2017-12-05 13:07 ` Volodymyr Babchuk 0 siblings, 0 replies; 22+ messages in thread From: Volodymyr Babchuk @ 2017-12-05 13:07 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, Julien Grall, stuart.yoder Hi Stefano, On 05.12.17 00:06, Stefano Stabellini wrote: > On Mon, 4 Dec 2017, Volodymyr Babchuk wrote: >>> = Xen command forwarding = >>> >>> In the code below, it looks like Xen is forwarding everything to OP-TEE. >>> Are there some commands Xen should avoid forwarding? Should we have a >>> whitelist or a blacklist? >> My code implements whitelists (at least, I hope so :-) ). It forwards >> only known requests. If it does not know type of the request, it >> returns error back to a caller. > > Actually, see below: > > >>>> +static bool optee_handle_smc(struct cpu_user_regs *regs) >>>> +{ >>>> + >>>> + switch ( get_user_reg(regs, 0) ) >>>> + { >>>> + case OPTEE_SMC_GET_SHM_CONFIG: >>>> + return handle_get_shm_config(regs); >>>> + case OPTEE_SMC_EXCHANGE_CAPABILITIES: >>>> + return handle_exchange_capabilities(regs); >>>> + case OPTEE_SMC_CALL_WITH_ARG: >>>> + return handle_std_call(regs); >>>> + case OPTEE_SMC_CALL_RETURN_FROM_RPC: >>>> + return handle_rpc(regs); >>>> + default: >>>> + return forward_call(regs); >>>> + } >>>> + return false; >>>> +} > > In the unknown ("default") case the smc is still forwarded. Am I missing > anything? No, you are right. It is I who missed to complete this piece of code. There should be a list of all known OPTEE_SMC_* commands, plus "return false" in "default" case. WBR, -- Volodymyr Babchuk _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-12-07 16:32 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-11-16 11:54 Next Xen Arm Community call - Wednesday 22nd November Julien Grall 2017-11-16 12:47 ` Artem Mygaiev 2017-11-16 18:58 ` Stefano Stabellini 2017-11-20 18:05 ` Julien Grall 2017-11-21 14:34 ` Julien Grall 2017-11-27 17:01 ` [RFC] WIP: optee: add OP-TEE mediator Volodymyr Babchuk 2017-12-01 22:58 ` Stefano Stabellini 2017-12-04 14:30 ` Julien Grall 2017-12-04 16:24 ` Volodymyr Babchuk 2017-12-04 16:29 ` Julien Grall 2017-12-06 22:39 ` Julien Grall 2017-12-04 14:46 ` Andrew Cooper 2017-12-04 16:15 ` Volodymyr Babchuk 2017-12-04 16:27 ` Julien Grall 2017-12-04 18:32 ` Volodymyr Babchuk 2017-12-04 22:04 ` Stefano Stabellini 2017-12-05 11:14 ` Julien Grall [not found] ` <f9fe1cb6-8700-99ba-ee62-2217fb5eb041@linaro.org> [not found] ` <20171204185929.GB30163@EPUAKYIW2556.kyiv.epam.com> [not found] ` <a6a05ad8-f335-00a2-cdd0-add81deb87bf@linaro.org> [not found] ` <2ee9297a-fcb8-c1c0-8ca7-91b9adbcd5b1@epam.com> [not found] ` <a7af95e4-09cf-22f8-b8cd-23a0cb5baae8@linaro.org> 2017-12-05 15:36 ` Stuart Yoder 2017-12-06 22:31 ` Julien Grall 2017-12-07 16:32 ` Stuart Yoder 2017-12-04 22:06 ` Stefano Stabellini 2017-12-05 13:07 ` Volodymyr Babchuk
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.