Message ID | 20231114095217.1142360-2-jens.wiklander@linaro.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b909:0:b0:403:3b70:6f57 with SMTP id t9csp1752001vqg; Tue, 14 Nov 2023 01:52:43 -0800 (PST) X-Google-Smtp-Source: AGHT+IHv/fh/8r0fMZbsTNs/6MkPRcSVIHR1PQPRX4+uerR1U4KoWyUXfA98Krty0LNE0Ua3ICvD X-Received: by 2002:a17:90b:1b0f:b0:27d:2108:af18 with SMTP id nu15-20020a17090b1b0f00b0027d2108af18mr9003692pjb.25.1699955562994; Tue, 14 Nov 2023 01:52:42 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1699955562; cv=none; d=google.com; s=arc-20160816; b=ZU7LOQMJOELcxKFko3N0oFBXpeby+C2CawBifTK9+7Myd0tjsuaw1sd0vUZRPyK4u6 pM9pgT3as0THP+ZOPx+LeBg62r/uehXbFHiKvyClXc/dm0mjZdB3fInUMlEaxPIbnSen YvGEjNDdSggOyQm1cAIvz5yFX81UA1IO7wftt1vpAs+tNm8hzqYpikvm0SHQgjhG8mxm r3ewQSEMEDnbgN1oQUU1hNkoztmRSZsBUxfWA2GF8kCZdtyqHA5EbWH7a7sWVeNz0uEr GLjbGFoldKmatyyMRLy5vnijtsJedEsW6PqQULas4K9oKaBQW1nH+9XhAiMCQKr2ngdS DG9Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=wLRgBSnIsahghCgCHXz9rGutSrnAaUdwkEkt9d4hS6A=; fh=SSmA14+Y2ZA2PO07peKpbI6W7vxD8MB28Dbup/soyB4=; b=AAXkdoHohQkinPp8qcuqhQnD5nf5S3WYO1Yf0du+84sQdOcGXIN2EnB0iSI9GYWZOY Tnlk4lArMCQBqx8lZt+X7G3Q2iU6zG+bncF1qKyr55dBo9EK21dt/4r1q+Sv5xo2e2lz tLvYGbnVDTXXGDb4hjLztD0o3d+MInwa0ZZWSErSxM/6EtcCaAiziNfW84VfKIAo5lGJ bPK0bG02zFyNDP1y4ijDzFtpuJHpmRkLLjQbrLjk0o+LFxBaaPN5KQ5eD1pcpUltuW72 aNvM8stwprKc68c8d5mfha1cOeaYFXUbfJeJy0gtRDGxxbf1RXJ/LRj6HWsnRazZcBfk CV0A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=cKr0w1Ga; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from snail.vger.email (snail.vger.email. [23.128.96.37]) by mx.google.com with ESMTPS id y5-20020a17090aa40500b0027d1ce4ce41si423165pjp.0.2023.11.14.01.52.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Nov 2023 01:52:42 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=cKr0w1Ga; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 9F11880408D9; Tue, 14 Nov 2023 01:52:41 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232538AbjKNJwi (ORCPT <rfc822;lhua1029@gmail.com> + 30 others); Tue, 14 Nov 2023 04:52:38 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60204 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229585AbjKNJwg (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 14 Nov 2023 04:52:36 -0500 Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E4D37191 for <linux-kernel@vger.kernel.org>; Tue, 14 Nov 2023 01:52:32 -0800 (PST) Received: by mail-pf1-x436.google.com with SMTP id d2e1a72fcca58-6c115026985so5570288b3a.1 for <linux-kernel@vger.kernel.org>; Tue, 14 Nov 2023 01:52:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1699955552; x=1700560352; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=wLRgBSnIsahghCgCHXz9rGutSrnAaUdwkEkt9d4hS6A=; b=cKr0w1GaXhCXLGiy4d9VuzTCICQlUl6+Gzg1WfkR7HaCBFBjA1NgEZVPWWIjc2UyGl 5n1jqmPi5yih+aPlXVMK76SF0DKwu9RXulOsw+mBt5Tt/PX+S70xZJu7vLg/6S7yAtiH 1HmeapNBl5h6TcDOZMD20lfQ4sYPQEXrCbIfXOJf2vBKIlI5tiHp77a0qwV557fFgINN rOJxsJnwz8uxoamYEKzMA4tpurUienvc4cIyNZ4wUwFsrWiUEe7TuAtC/VaLNso1JYpW yxQWkJaretPt7jpheMRPcHsQZgw6sElCkWGmDzc+4V1v5rD1MZ+d4gukWOUTs2hvyzc2 bnbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699955552; x=1700560352; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=wLRgBSnIsahghCgCHXz9rGutSrnAaUdwkEkt9d4hS6A=; b=o1dAGDEiR1FXAf5iOWKZ22KtsyQwFvHb7j53q3U5iopXogjYXN9+KbOmJWm5NiFqbF PeBa1TN4AyyPusScPdWZcEXNyWGqOUgrSRCoF1gmRzRxOVV6K+2ljxsvzaUu1TYgxQAI koHXICuhxcnkm6Q4Obwmj3/8PJRtjUEF6W1L4ATKTe0kLxySU5HXfi19Bf6uHS7PGkh0 Y32Kuh4yfvGU3+bTodief6lkWrvZb2l9i0+BNxHKKC95GhyjY3OToWfwE1HsgRrHDAWt Pwnl5DlMctdi3mimc3IM0GDVOFNuInGFtvpdYQzhCxtFc8OH6VPxhMBZzcqDwny/zGzG AQtA== X-Gm-Message-State: AOJu0YzWAvQnM4TpAY/taWuWkolsuOFltNlJiU9Z9ddONE5117Hr/5GW i+FhIAPZgomaqs/pHyQI91C5Y3XbD2InGTessf8= X-Received: by 2002:a05:6a20:1447:b0:15d:b243:6131 with SMTP id a7-20020a056a20144700b0015db2436131mr11396876pzi.44.1699955551955; Tue, 14 Nov 2023 01:52:31 -0800 (PST) Received: from rayden.urgonet (h-217-31-164-171.A175.priv.bahnhof.se. [217.31.164.171]) by smtp.gmail.com with ESMTPSA id h10-20020a056a00218a00b006c34015a8f2sm874749pfi.146.2023.11.14.01.52.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Nov 2023 01:52:31 -0800 (PST) From: Jens Wiklander <jens.wiklander@linaro.org> To: linux-kernel@vger.kernel.org, op-tee@lists.trustedfirmware.org Cc: Sumit Garg <sumit.garg@linaro.org>, Jerome Forissier <jerome.forissier@linaro.org>, Shyam Saini <shyamsaini@linux.microsoft.com>, Jens Wiklander <jens.wiklander@linaro.org> Subject: [PATCH v3 1/2] optee: add page list to kernel private shared memory Date: Tue, 14 Nov 2023 10:52:16 +0100 Message-Id: <20231114095217.1142360-2-jens.wiklander@linaro.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231114095217.1142360-1-jens.wiklander@linaro.org> References: <20231114095217.1142360-1-jens.wiklander@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Tue, 14 Nov 2023 01:52:41 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1782532604396521223 X-GMAIL-MSGID: 1782532604396521223 |
Series |
OP-TEE kernel private shared memory optimizations
|
|
Commit Message
Jens Wiklander
Nov. 14, 2023, 9:52 a.m. UTC
Until now has kernel private shared memory allocated as dynamic shared
memory (not from the static shared memory pool) been returned without a
list of physical pages on allocations via RPC. To support allocations
larger than one page add a list of physical pages.
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
drivers/tee/optee/core.c | 28 +++++++++++++----------
drivers/tee/optee/smc_abi.c | 45 +++++++++++++++++--------------------
2 files changed, 37 insertions(+), 36 deletions(-)
Comments
On Tue, 14 Nov 2023 at 15:22, Jens Wiklander <jens.wiklander@linaro.org> wrote: > > Until now has kernel private shared memory allocated as dynamic shared > memory (not from the static shared memory pool) been returned without a > list of physical pages on allocations via RPC. To support allocations > larger than one page add a list of physical pages. Although this patch looks like a good cleanup, I can't find an ABI change here. Wasn't a list of pages returned earlier too? -Sumit > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> > --- > drivers/tee/optee/core.c | 28 +++++++++++++---------- > drivers/tee/optee/smc_abi.c | 45 +++++++++++++++++-------------------- > 2 files changed, 37 insertions(+), 36 deletions(-) > > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c > index 2a258bd3b6b5..38ea2fecfc2e 100644 > --- a/drivers/tee/optee/core.c > +++ b/drivers/tee/optee/core.c > @@ -27,7 +27,10 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm, > unsigned long start)) > { > unsigned int order = get_order(size); > + unsigned int nr_pages = 1 << order; > + struct page **pages; > struct page *page; > + unsigned int i; > int rc = 0; > > /* > @@ -42,30 +45,29 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm, > shm->paddr = page_to_phys(page); > shm->size = PAGE_SIZE << order; > > - if (shm_register) { > - unsigned int nr_pages = 1 << order, i; > - struct page **pages; > + pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL); > + if (!pages) { > + rc = -ENOMEM; > + goto err; > + } > > - pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL); > - if (!pages) { > - rc = -ENOMEM; > - goto err; > - } > + for (i = 0; i < nr_pages; i++) > + pages[i] = page + i; > > - for (i = 0; i < nr_pages; i++) > - pages[i] = page + i; > + shm->pages = pages; > + shm->num_pages = nr_pages; > > + if (shm_register) { > rc = shm_register(shm->ctx, shm, pages, nr_pages, > (unsigned long)shm->kaddr); > - kfree(pages); > if (rc) > goto err; > } > > return 0; > - > err: > free_pages((unsigned long)shm->kaddr, order); > + shm->kaddr = NULL; > return rc; > } > > @@ -77,6 +79,8 @@ void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm, > shm_unregister(shm->ctx, shm); > free_pages((unsigned long)shm->kaddr, get_order(shm->size)); > shm->kaddr = NULL; > + kfree(shm->pages); > + shm->pages = NULL; > } > > static void optee_bus_scan(struct work_struct *work) > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c > index d5b28fd35d66..b69410c7cc0a 100644 > --- a/drivers/tee/optee/smc_abi.c > +++ b/drivers/tee/optee/smc_abi.c > @@ -678,10 +678,11 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, > struct optee_msg_arg *arg, > struct optee_call_ctx *call_ctx) > { > - phys_addr_t pa; > struct tee_shm *shm; > size_t sz; > size_t n; > + struct page **pages; > + size_t page_count; > > arg->ret_origin = TEEC_ORIGIN_COMMS; > > @@ -716,32 +717,23 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, > return; > } > > - if (tee_shm_get_pa(shm, 0, &pa)) { > - arg->ret = TEEC_ERROR_BAD_PARAMETERS; > - goto bad; > - } > - > - sz = tee_shm_get_size(shm); > - > - if (tee_shm_is_dynamic(shm)) { > - struct page **pages; > + /* > + * If there are pages it's dynamically allocated shared memory (not > + * from the reserved shared memory pool) and needs to be > + * registered. > + */ > + pages = tee_shm_get_pages(shm, &page_count); > + if (pages) { > u64 *pages_list; > - size_t page_num; > - > - pages = tee_shm_get_pages(shm, &page_num); > - if (!pages || !page_num) { > - arg->ret = TEEC_ERROR_OUT_OF_MEMORY; > - goto bad; > - } > > - pages_list = optee_allocate_pages_list(page_num); > + pages_list = optee_allocate_pages_list(page_count); > if (!pages_list) { > arg->ret = TEEC_ERROR_OUT_OF_MEMORY; > goto bad; > } > > call_ctx->pages_list = pages_list; > - call_ctx->num_entries = page_num; > + call_ctx->num_entries = page_count; > > arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT | > OPTEE_MSG_ATTR_NONCONTIG; > @@ -752,17 +744,22 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, > arg->params[0].u.tmem.buf_ptr = virt_to_phys(pages_list) | > (tee_shm_get_page_offset(shm) & > (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1)); > - arg->params[0].u.tmem.size = tee_shm_get_size(shm); > - arg->params[0].u.tmem.shm_ref = (unsigned long)shm; > > - optee_fill_pages_list(pages_list, pages, page_num, > + optee_fill_pages_list(pages_list, pages, page_count, > tee_shm_get_page_offset(shm)); > } else { > + phys_addr_t pa; > + > + if (tee_shm_get_pa(shm, 0, &pa)) { > + arg->ret = TEEC_ERROR_BAD_PARAMETERS; > + goto bad; > + } > + > arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT; > arg->params[0].u.tmem.buf_ptr = pa; > - arg->params[0].u.tmem.size = sz; > - arg->params[0].u.tmem.shm_ref = (unsigned long)shm; > } > + arg->params[0].u.tmem.size = tee_shm_get_size(shm); > + arg->params[0].u.tmem.shm_ref = (unsigned long)shm; > > arg->ret = TEEC_SUCCESS; > return; > -- > 2.34.1 >
On Thu, Nov 16, 2023 at 1:28 PM Sumit Garg <sumit.garg@linaro.org> wrote: > > On Tue, 14 Nov 2023 at 15:22, Jens Wiklander <jens.wiklander@linaro.org> wrote: > > > > Until now has kernel private shared memory allocated as dynamic shared > > memory (not from the static shared memory pool) been returned without a > > list of physical pages on allocations via RPC. To support allocations > > larger than one page add a list of physical pages. > > Although this patch looks like a good cleanup, I can't find an ABI > change here. Wasn't a list of pages returned earlier too? No, because handle_rpc_func_cmd_shm_alloc() didn't take the path where optee_allocate_pages_list() is called. Cheers, Jens > > -Sumit > > > > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> > > --- > > drivers/tee/optee/core.c | 28 +++++++++++++---------- > > drivers/tee/optee/smc_abi.c | 45 +++++++++++++++++-------------------- > > 2 files changed, 37 insertions(+), 36 deletions(-) > > > > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c > > index 2a258bd3b6b5..38ea2fecfc2e 100644 > > --- a/drivers/tee/optee/core.c > > +++ b/drivers/tee/optee/core.c > > @@ -27,7 +27,10 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm, > > unsigned long start)) > > { > > unsigned int order = get_order(size); > > + unsigned int nr_pages = 1 << order; > > + struct page **pages; > > struct page *page; > > + unsigned int i; > > int rc = 0; > > > > /* > > @@ -42,30 +45,29 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm, > > shm->paddr = page_to_phys(page); > > shm->size = PAGE_SIZE << order; > > > > - if (shm_register) { > > - unsigned int nr_pages = 1 << order, i; > > - struct page **pages; > > + pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL); > > + if (!pages) { > > + rc = -ENOMEM; > > + goto err; > > + } > > > > - pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL); > > - if (!pages) { > > - rc = -ENOMEM; > > - goto err; > > - } > > + for (i = 0; i < nr_pages; i++) > > + pages[i] = page + i; > > > > - for (i = 0; i < nr_pages; i++) > > - pages[i] = page + i; > > + shm->pages = pages; > > + shm->num_pages = nr_pages; > > > > + if (shm_register) { > > rc = shm_register(shm->ctx, shm, pages, nr_pages, > > (unsigned long)shm->kaddr); > > - kfree(pages); > > if (rc) > > goto err; > > } > > > > return 0; > > - > > err: > > free_pages((unsigned long)shm->kaddr, order); > > + shm->kaddr = NULL; > > return rc; > > } > > > > @@ -77,6 +79,8 @@ void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm, > > shm_unregister(shm->ctx, shm); > > free_pages((unsigned long)shm->kaddr, get_order(shm->size)); > > shm->kaddr = NULL; > > + kfree(shm->pages); > > + shm->pages = NULL; > > } > > > > static void optee_bus_scan(struct work_struct *work) > > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c > > index d5b28fd35d66..b69410c7cc0a 100644 > > --- a/drivers/tee/optee/smc_abi.c > > +++ b/drivers/tee/optee/smc_abi.c > > @@ -678,10 +678,11 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, > > struct optee_msg_arg *arg, > > struct optee_call_ctx *call_ctx) > > { > > - phys_addr_t pa; > > struct tee_shm *shm; > > size_t sz; > > size_t n; > > + struct page **pages; > > + size_t page_count; > > > > arg->ret_origin = TEEC_ORIGIN_COMMS; > > > > @@ -716,32 +717,23 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, > > return; > > } > > > > - if (tee_shm_get_pa(shm, 0, &pa)) { > > - arg->ret = TEEC_ERROR_BAD_PARAMETERS; > > - goto bad; > > - } > > - > > - sz = tee_shm_get_size(shm); > > - > > - if (tee_shm_is_dynamic(shm)) { > > - struct page **pages; > > + /* > > + * If there are pages it's dynamically allocated shared memory (not > > + * from the reserved shared memory pool) and needs to be > > + * registered. > > + */ > > + pages = tee_shm_get_pages(shm, &page_count); > > + if (pages) { > > u64 *pages_list; > > - size_t page_num; > > - > > - pages = tee_shm_get_pages(shm, &page_num); > > - if (!pages || !page_num) { > > - arg->ret = TEEC_ERROR_OUT_OF_MEMORY; > > - goto bad; > > - } > > > > - pages_list = optee_allocate_pages_list(page_num); > > + pages_list = optee_allocate_pages_list(page_count); > > if (!pages_list) { > > arg->ret = TEEC_ERROR_OUT_OF_MEMORY; > > goto bad; > > } > > > > call_ctx->pages_list = pages_list; > > - call_ctx->num_entries = page_num; > > + call_ctx->num_entries = page_count; > > > > arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT | > > OPTEE_MSG_ATTR_NONCONTIG; > > @@ -752,17 +744,22 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, > > arg->params[0].u.tmem.buf_ptr = virt_to_phys(pages_list) | > > (tee_shm_get_page_offset(shm) & > > (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1)); > > - arg->params[0].u.tmem.size = tee_shm_get_size(shm); > > - arg->params[0].u.tmem.shm_ref = (unsigned long)shm; > > > > - optee_fill_pages_list(pages_list, pages, page_num, > > + optee_fill_pages_list(pages_list, pages, page_count, > > tee_shm_get_page_offset(shm)); > > } else { > > + phys_addr_t pa; > > + > > + if (tee_shm_get_pa(shm, 0, &pa)) { > > + arg->ret = TEEC_ERROR_BAD_PARAMETERS; > > + goto bad; > > + } > > + > > arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT; > > arg->params[0].u.tmem.buf_ptr = pa; > > - arg->params[0].u.tmem.size = sz; > > - arg->params[0].u.tmem.shm_ref = (unsigned long)shm; > > } > > + arg->params[0].u.tmem.size = tee_shm_get_size(shm); > > + arg->params[0].u.tmem.shm_ref = (unsigned long)shm; > > > > arg->ret = TEEC_SUCCESS; > > return; > > -- > > 2.34.1 > >
On Fri, 17 Nov 2023 at 14:00, Jens Wiklander <jens.wiklander@linaro.org> wrote: > > On Thu, Nov 16, 2023 at 1:28 PM Sumit Garg <sumit.garg@linaro.org> wrote: > > > > On Tue, 14 Nov 2023 at 15:22, Jens Wiklander <jens.wiklander@linaro.org> wrote: > > > > > > Until now has kernel private shared memory allocated as dynamic shared > > > memory (not from the static shared memory pool) been returned without a > > > list of physical pages on allocations via RPC. To support allocations > > > larger than one page add a list of physical pages. > > > > Although this patch looks like a good cleanup, I can't find an ABI > > change here. Wasn't a list of pages returned earlier too? > > No, because handle_rpc_func_cmd_shm_alloc() didn't take the path where > optee_allocate_pages_list() is called. Ah I see, so it is tee_shm_is_dynamic() check being removed here that you are referring to. Feel free to add: Reviewed-by: Sumit Garg <sumit.garg@linaro.org> -Sumit > > Cheers, > Jens > > > > > -Sumit > > > > > > > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> > > > --- > > > drivers/tee/optee/core.c | 28 +++++++++++++---------- > > > drivers/tee/optee/smc_abi.c | 45 +++++++++++++++++-------------------- > > > 2 files changed, 37 insertions(+), 36 deletions(-) > > > > > > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c > > > index 2a258bd3b6b5..38ea2fecfc2e 100644 > > > --- a/drivers/tee/optee/core.c > > > +++ b/drivers/tee/optee/core.c > > > @@ -27,7 +27,10 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm, > > > unsigned long start)) > > > { > > > unsigned int order = get_order(size); > > > + unsigned int nr_pages = 1 << order; > > > + struct page **pages; > > > struct page *page; > > > + unsigned int i; > > > int rc = 0; > > > > > > /* > > > @@ -42,30 +45,29 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm, > > > shm->paddr = page_to_phys(page); > > > shm->size = PAGE_SIZE << order; > > > > > > - if (shm_register) { > > > - unsigned int nr_pages = 1 << order, i; > > > - struct page **pages; > > > + pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL); > > > + if (!pages) { > > > + rc = -ENOMEM; > > > + goto err; > > > + } > > > > > > - pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL); > > > - if (!pages) { > > > - rc = -ENOMEM; > > > - goto err; > > > - } > > > + for (i = 0; i < nr_pages; i++) > > > + pages[i] = page + i; > > > > > > - for (i = 0; i < nr_pages; i++) > > > - pages[i] = page + i; > > > + shm->pages = pages; > > > + shm->num_pages = nr_pages; > > > > > > + if (shm_register) { > > > rc = shm_register(shm->ctx, shm, pages, nr_pages, > > > (unsigned long)shm->kaddr); > > > - kfree(pages); > > > if (rc) > > > goto err; > > > } > > > > > > return 0; > > > - > > > err: > > > free_pages((unsigned long)shm->kaddr, order); > > > + shm->kaddr = NULL; > > > return rc; > > > } > > > > > > @@ -77,6 +79,8 @@ void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm, > > > shm_unregister(shm->ctx, shm); > > > free_pages((unsigned long)shm->kaddr, get_order(shm->size)); > > > shm->kaddr = NULL; > > > + kfree(shm->pages); > > > + shm->pages = NULL; > > > } > > > > > > static void optee_bus_scan(struct work_struct *work) > > > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c > > > index d5b28fd35d66..b69410c7cc0a 100644 > > > --- a/drivers/tee/optee/smc_abi.c > > > +++ b/drivers/tee/optee/smc_abi.c > > > @@ -678,10 +678,11 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, > > > struct optee_msg_arg *arg, > > > struct optee_call_ctx *call_ctx) > > > { > > > - phys_addr_t pa; > > > struct tee_shm *shm; > > > size_t sz; > > > size_t n; > > > + struct page **pages; > > > + size_t page_count; > > > > > > arg->ret_origin = TEEC_ORIGIN_COMMS; > > > > > > @@ -716,32 +717,23 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, > > > return; > > > } > > > > > > - if (tee_shm_get_pa(shm, 0, &pa)) { > > > - arg->ret = TEEC_ERROR_BAD_PARAMETERS; > > > - goto bad; > > > - } > > > - > > > - sz = tee_shm_get_size(shm); > > > - > > > - if (tee_shm_is_dynamic(shm)) { > > > - struct page **pages; > > > + /* > > > + * If there are pages it's dynamically allocated shared memory (not > > > + * from the reserved shared memory pool) and needs to be > > > + * registered. > > > + */ > > > + pages = tee_shm_get_pages(shm, &page_count); > > > + if (pages) { > > > u64 *pages_list; > > > - size_t page_num; > > > - > > > - pages = tee_shm_get_pages(shm, &page_num); > > > - if (!pages || !page_num) { > > > - arg->ret = TEEC_ERROR_OUT_OF_MEMORY; > > > - goto bad; > > > - } > > > > > > - pages_list = optee_allocate_pages_list(page_num); > > > + pages_list = optee_allocate_pages_list(page_count); > > > if (!pages_list) { > > > arg->ret = TEEC_ERROR_OUT_OF_MEMORY; > > > goto bad; > > > } > > > > > > call_ctx->pages_list = pages_list; > > > - call_ctx->num_entries = page_num; > > > + call_ctx->num_entries = page_count; > > > > > > arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT | > > > OPTEE_MSG_ATTR_NONCONTIG; > > > @@ -752,17 +744,22 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, > > > arg->params[0].u.tmem.buf_ptr = virt_to_phys(pages_list) | > > > (tee_shm_get_page_offset(shm) & > > > (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1)); > > > - arg->params[0].u.tmem.size = tee_shm_get_size(shm); > > > - arg->params[0].u.tmem.shm_ref = (unsigned long)shm; > > > > > > - optee_fill_pages_list(pages_list, pages, page_num, > > > + optee_fill_pages_list(pages_list, pages, page_count, > > > tee_shm_get_page_offset(shm)); > > > } else { > > > + phys_addr_t pa; > > > + > > > + if (tee_shm_get_pa(shm, 0, &pa)) { > > > + arg->ret = TEEC_ERROR_BAD_PARAMETERS; > > > + goto bad; > > > + } > > > + > > > arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT; > > > arg->params[0].u.tmem.buf_ptr = pa; > > > - arg->params[0].u.tmem.size = sz; > > > - arg->params[0].u.tmem.shm_ref = (unsigned long)shm; > > > } > > > + arg->params[0].u.tmem.size = tee_shm_get_size(shm); > > > + arg->params[0].u.tmem.shm_ref = (unsigned long)shm; > > > > > > arg->ret = TEEC_SUCCESS; > > > return; > > > -- > > > 2.34.1 > > >
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 2a258bd3b6b5..38ea2fecfc2e 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -27,7 +27,10 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm, unsigned long start)) { unsigned int order = get_order(size); + unsigned int nr_pages = 1 << order; + struct page **pages; struct page *page; + unsigned int i; int rc = 0; /* @@ -42,30 +45,29 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm, shm->paddr = page_to_phys(page); shm->size = PAGE_SIZE << order; - if (shm_register) { - unsigned int nr_pages = 1 << order, i; - struct page **pages; + pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL); + if (!pages) { + rc = -ENOMEM; + goto err; + } - pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL); - if (!pages) { - rc = -ENOMEM; - goto err; - } + for (i = 0; i < nr_pages; i++) + pages[i] = page + i; - for (i = 0; i < nr_pages; i++) - pages[i] = page + i; + shm->pages = pages; + shm->num_pages = nr_pages; + if (shm_register) { rc = shm_register(shm->ctx, shm, pages, nr_pages, (unsigned long)shm->kaddr); - kfree(pages); if (rc) goto err; } return 0; - err: free_pages((unsigned long)shm->kaddr, order); + shm->kaddr = NULL; return rc; } @@ -77,6 +79,8 @@ void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm, shm_unregister(shm->ctx, shm); free_pages((unsigned long)shm->kaddr, get_order(shm->size)); shm->kaddr = NULL; + kfree(shm->pages); + shm->pages = NULL; } static void optee_bus_scan(struct work_struct *work) diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c index d5b28fd35d66..b69410c7cc0a 100644 --- a/drivers/tee/optee/smc_abi.c +++ b/drivers/tee/optee/smc_abi.c @@ -678,10 +678,11 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, struct optee_msg_arg *arg, struct optee_call_ctx *call_ctx) { - phys_addr_t pa; struct tee_shm *shm; size_t sz; size_t n; + struct page **pages; + size_t page_count; arg->ret_origin = TEEC_ORIGIN_COMMS; @@ -716,32 +717,23 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, return; } - if (tee_shm_get_pa(shm, 0, &pa)) { - arg->ret = TEEC_ERROR_BAD_PARAMETERS; - goto bad; - } - - sz = tee_shm_get_size(shm); - - if (tee_shm_is_dynamic(shm)) { - struct page **pages; + /* + * If there are pages it's dynamically allocated shared memory (not + * from the reserved shared memory pool) and needs to be + * registered. + */ + pages = tee_shm_get_pages(shm, &page_count); + if (pages) { u64 *pages_list; - size_t page_num; - - pages = tee_shm_get_pages(shm, &page_num); - if (!pages || !page_num) { - arg->ret = TEEC_ERROR_OUT_OF_MEMORY; - goto bad; - } - pages_list = optee_allocate_pages_list(page_num); + pages_list = optee_allocate_pages_list(page_count); if (!pages_list) { arg->ret = TEEC_ERROR_OUT_OF_MEMORY; goto bad; } call_ctx->pages_list = pages_list; - call_ctx->num_entries = page_num; + call_ctx->num_entries = page_count; arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT | OPTEE_MSG_ATTR_NONCONTIG; @@ -752,17 +744,22 @@ static void handle_rpc_func_cmd_shm_alloc(struct tee_context *ctx, arg->params[0].u.tmem.buf_ptr = virt_to_phys(pages_list) | (tee_shm_get_page_offset(shm) & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1)); - arg->params[0].u.tmem.size = tee_shm_get_size(shm); - arg->params[0].u.tmem.shm_ref = (unsigned long)shm; - optee_fill_pages_list(pages_list, pages, page_num, + optee_fill_pages_list(pages_list, pages, page_count, tee_shm_get_page_offset(shm)); } else { + phys_addr_t pa; + + if (tee_shm_get_pa(shm, 0, &pa)) { + arg->ret = TEEC_ERROR_BAD_PARAMETERS; + goto bad; + } + arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT; arg->params[0].u.tmem.buf_ptr = pa; - arg->params[0].u.tmem.size = sz; - arg->params[0].u.tmem.shm_ref = (unsigned long)shm; } + arg->params[0].u.tmem.size = tee_shm_get_size(shm); + arg->params[0].u.tmem.shm_ref = (unsigned long)shm; arg->ret = TEEC_SUCCESS; return;