Julien Grall writes: > Hi Volodymyr, > > On 8/23/19 7:48 PM, Volodymyr Babchuk wrote: >> We want to limit number of calls to lookup_and_pin_guest_ram_addr() >> per one request. There are two ways to do this: either preempt >> translate_noncontig() or to limit size of one shared buffer size. >> >> It is quite hard to preempt translate_noncontig(), because it is deep >> nested. So we chose second option. We will allow 512 pages per one >> shared buffer. This does not interfere with GP standard, as it >> requires that size limit for shared buffer should be at lest 512kB. > > Do you mean "least" instead of "lest"? Yes
> If so, why 512 pages (i.e 1MB) > is plenty enough for most of the use cases? What does "xtest" consist > on? Bigger buffer xtest tries to allocate is mere 32KB. I believe that 1MB is enough for the most cases, because OP-TEE itself have a very limited resources. But this value is chosen arbitrary. > >> Also, with this limitation OP-TEE still passes own "xtest" test suite, >> so this is okay for now. >> >> Signed-off-by: Volodymyr Babchuk <[email protected]> >> --- >> xen/arch/arm/tee/optee.c | 30 ++++++++++++++++++------------ >> 1 file changed, 18 insertions(+), 12 deletions(-) >> >> diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c >> index ec5402e89b..f4fa8a7758 100644 >> --- a/xen/arch/arm/tee/optee.c >> +++ b/xen/arch/arm/tee/optee.c >> @@ -72,6 +72,17 @@ >> */ >> #define MAX_TOTAL_SMH_BUF_PG 16384 >> +/* >> + * Arbitrary value that limits maximum shared buffer size. It is >> + * merely coincidence that it equals to both default OP-TEE SHM buffer >> + * size limit and to (1 << CONFIG_DOMU_MAX_ORDER). Please note that >> + * this define limits number of pages. But user buffer can be not >> + * aligned to a page boundary. So it is possible that user would not >> + * be able to share exactly MAX_SHM_BUFFER_PG * PAGE_SIZE bytes with >> + * OP-TEE. >> + */ >> +#define MAX_SHM_BUFFER_PG 512 >> + >> #define OPTEE_KNOWN_NSEC_CAPS OPTEE_SMC_NSEC_CAP_UNIPROCESSOR >> #define OPTEE_KNOWN_SEC_CAPS (OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM | \ >> OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM | \ >> @@ -697,15 +708,17 @@ static int translate_noncontig(struct optee_domain >> *ctx, >> size = ROUNDUP(param->u.tmem.size + offset, >> OPTEE_MSG_NONCONTIG_PAGE_SIZE); >> pg_count = DIV_ROUND_UP(size, >> OPTEE_MSG_NONCONTIG_PAGE_SIZE); >> + if ( pg_count > MAX_SHM_BUFFER_PG ) >> + return -ENOMEM; >> + >> order = get_order_from_bytes(get_pages_list_size(pg_count)); >> /* >> - * In the worst case we will want to allocate 33 pages, which is >> - * MAX_TOTAL_SMH_BUF_PG/511 rounded up. This gives order 6 or at >> - * most 64 pages allocated. This buffer will be freed right after >> - * the end of the call and there can be no more than >> + * In the worst case we will want to allocate 2 pages, which is >> + * MAX_SHM_BUFFER_PG/511 rounded up. This buffer will be freed >> + * right after the end of the call and there can be no more than >> * max_optee_threads calls simultaneously. So in the worst case >> - * guest can trick us to allocate 64 * max_optee_threads pages in >> + * guest can trick us to allocate 2 * max_optee_threads pages in >> * total. >> */ >> xen_pgs = alloc_domheap_pages(current->domain, order, 0); >> @@ -747,13 +760,6 @@ static int translate_noncontig(struct optee_domain *ctx, >> xen_data = __map_domain_page(xen_pgs); >> } >> - /* >> - * TODO: That function can pin up to 64MB of guest memory by >> - * calling lookup_and_pin_guest_ram_addr() 16384 times >> - * (assuming that PAGE_SIZE equals to 4096). >> - * This should be addressed before declaring OP-TEE security >> - * supported. >> - */ >> BUILD_BUG_ON(PAGE_SIZE != 4096); > > Without the comment, the BUILD_BUG_ON() looks random. So either you > want to have a different version of the comment or you want to move > the BUILD_BUG_ON() to somewhere else. It is still before get_domain_ram_page() call. But for clarity I can add comment like "Only 4k pages are supported right now". >> page = >> get_domain_ram_page(gaddr_to_gfn(guest_data->pages_list[idx])); >> if ( !page ) >> > > Cheers, -- Volodymyr Babchuk at EPAM _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
