From nobody Fri Oct 31 03:48:32 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail(p=none dis=none) header.from=arm.com Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1754919347414995.6602511361804; Mon, 11 Aug 2025 06:35:47 -0700 (PDT) Received: from list by lists.xenproject.org with outflank-mailman.1077627.1438674 (Exim 4.92) (envelope-from ) id 1ulSgY-0003Zg-BA; Mon, 11 Aug 2025 13:35:34 +0000 Received: by outflank-mailman (output) from mailman id 1077627.1438674; Mon, 11 Aug 2025 13:35:34 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1ulSgY-0003ZZ-6V; Mon, 11 Aug 2025 13:35:34 +0000 Received: by outflank-mailman (input) for mailman id 1077627; Mon, 11 Aug 2025 13:35:32 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1ulSgW-0002rn-Sj for xen-devel@lists.xenproject.org; Mon, 11 Aug 2025 13:35:32 +0000 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by se1-gles-flk1.inumbo.com (Halon) with ESMTP id 0c852d79-76b8-11f0-b898-0df219b8e170; Mon, 11 Aug 2025 15:35:30 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E08022641; Mon, 11 Aug 2025 06:35:21 -0700 (PDT) Received: from C3HXLD123V.arm.com (unknown [10.57.55.159]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C992E3F63F; Mon, 11 Aug 2025 06:35:28 -0700 (PDT) X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 0c852d79-76b8-11f0-b898-0df219b8e170 From: Bertrand Marquis To: xen-devel@lists.xenproject.org Cc: Volodymyr Babchuk , Stefano Stabellini , Julien Grall , Michal Orzel , Jens Wiklander , Julien Grall Subject: [PATCH v8 2/6] xen/arm: ffa: Rework partinfo_get implementation Date: Mon, 11 Aug 2025 15:34:58 +0200 Message-ID: X-Mailer: git-send-email 2.47.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ZM-MESSAGEID: 1754919349313124100 Content-Type: text/plain; charset="utf-8" This patch is in preparation for VM to VM support in order to do the changes on the SP handling part of partinfo_get before adding support for the VM part. This patches is doing the following changes: - split partinfo_get into 3 functions to have the locking handling and proper exit on error handled more clearly - add some potential overflow checks to validate the offset and sizes passed by the VM on partinfo call. - Introduce a maximum number of SPs (for now set to 64) to prevent holding the CPU for too long in case there would be a lot of partitions in the secure world. The limit currently set is thought to be realistic for most use cases as 64 secure partitions is a very high number compared to current seen usage (more 3 or 4). - fix include ordering in ffa_private.h to be in alphabetic order Signed-off-by: Bertrand Marquis Reviewed-by: Jens Wiklander Acked-by: Julien Grall --- Changes in v8: - None Changes in v7: - add Julien A-b Changes in v6: - add missing empty line before ffa_handle_partition_info_get - add Jens R-b Changes in v5: - patch added --- xen/arch/arm/tee/ffa_partinfo.c | 202 +++++++++++++++++++------------- xen/arch/arm/tee/ffa_private.h | 18 ++- 2 files changed, 132 insertions(+), 88 deletions(-) diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinf= o.c index c0510ceb8338..dfa0b23eaf38 100644 --- a/xen/arch/arm/tee/ffa_partinfo.c +++ b/xen/arch/arm/tee/ffa_partinfo.c @@ -63,9 +63,96 @@ static int32_t ffa_partition_info_get(uint32_t *uuid, ui= nt32_t flags, return ret; } =20 -void ffa_handle_partition_info_get(struct cpu_user_regs *regs) +static int32_t ffa_get_sp_count(uint32_t *uuid, uint32_t *sp_count) +{ + uint32_t src_size; + + return ffa_partition_info_get(uuid, FFA_PARTITION_INFO_GET_COUNT_FLAG, + sp_count, &src_size); +} + +static int32_t ffa_get_sp_partinfo(uint32_t *uuid, uint32_t *sp_count, + void *dst_buf, void *end_buf, + uint32_t dst_size) { int32_t ret; + uint32_t src_size, real_sp_count; + void *src_buf =3D ffa_rx; + uint32_t count =3D 0; + + /* Do we have a RX buffer with the SPMC */ + if ( !ffa_rx ) + return FFA_RET_DENIED; + + /* We need to use the RX buffer to receive the list */ + spin_lock(&ffa_rx_buffer_lock); + + ret =3D ffa_partition_info_get(uuid, 0, &real_sp_count, &src_size); + if ( ret ) + goto out; + + /* We now own the RX buffer */ + + /* Validate the src_size we got */ + if ( src_size < sizeof(struct ffa_partition_info_1_0) || + src_size >=3D FFA_PAGE_SIZE ) + { + ret =3D FFA_RET_NOT_SUPPORTED; + goto out_release; + } + + /* + * Limit the maximum time we hold the CPU by limiting the number of SP= s. + * We just ignore the extra ones as this is tested during init in + * ffa_partinfo_init so the only possible reason is SP have been added + * since boot. + */ + if ( real_sp_count > FFA_MAX_NUM_SP ) + real_sp_count =3D FFA_MAX_NUM_SP; + + /* Make sure the data fits in our buffer */ + if ( real_sp_count > (FFA_RXTX_PAGE_COUNT * FFA_PAGE_SIZE) / src_size ) + { + ret =3D FFA_RET_NOT_SUPPORTED; + goto out_release; + } + + for ( uint32_t sp_num =3D 0; sp_num < real_sp_count; sp_num++ ) + { + struct ffa_partition_info_1_1 *fpi =3D src_buf; + + /* filter out SP not following bit 15 convention if any */ + if ( FFA_ID_IS_SECURE(fpi->id) ) + { + if ( dst_buf > (end_buf - dst_size) ) + { + ret =3D FFA_RET_NO_MEMORY; + goto out_release; + } + + memcpy(dst_buf, src_buf, MIN(src_size, dst_size)); + if ( dst_size > src_size ) + memset(dst_buf + src_size, 0, dst_size - src_size); + + dst_buf +=3D dst_size; + count++; + } + + src_buf +=3D src_size; + } + + *sp_count =3D count; + +out_release: + ffa_hyp_rx_release(); +out: + spin_unlock(&ffa_rx_buffer_lock); + return ret; +} + +void ffa_handle_partition_info_get(struct cpu_user_regs *regs) +{ + int32_t ret =3D FFA_RET_OK; struct domain *d =3D current->domain; struct ffa_ctx *ctx =3D d->arch.tee; uint32_t flags =3D get_user_reg(regs, 5); @@ -75,8 +162,8 @@ void ffa_handle_partition_info_get(struct cpu_user_regs = *regs) get_user_reg(regs, 3), get_user_reg(regs, 4), }; - uint32_t src_size, dst_size; - void *dst_buf; + uint32_t dst_size =3D 0; + void *dst_buf, *end_buf; uint32_t ffa_sp_count =3D 0; =20 /* @@ -89,31 +176,26 @@ void ffa_handle_partition_info_get(struct cpu_user_reg= s *regs) else dst_size =3D sizeof(struct ffa_partition_info_1_1); =20 - /* - * FF-A v1.0 has w5 MBZ while v1.1 allows - * FFA_PARTITION_INFO_GET_COUNT_FLAG to be non-zero. - * - * FFA_PARTITION_INFO_GET_COUNT is only using registers and not the - * rxtx buffer so do the partition_info_get directly. - */ - if ( flags =3D=3D FFA_PARTITION_INFO_GET_COUNT_FLAG && - ctx->guest_vers =3D=3D FFA_VERSION_1_1 ) + /* Only count requested */ + if ( flags ) { + /* + * FF-A v1.0 has w5 MBZ while v1.1 allows + * FFA_PARTITION_INFO_GET_COUNT_FLAG to be non-zero. + */ + if ( ctx->guest_vers =3D=3D FFA_VERSION_1_0 || + flags !=3D FFA_PARTITION_INFO_GET_COUNT_FLAG ) + { + ret =3D FFA_RET_INVALID_PARAMETERS; + goto out; + } + if ( ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) ) - ret =3D ffa_partition_info_get(uuid, flags, &ffa_sp_count, - &src_size); - else - ret =3D FFA_RET_OK; + ret =3D ffa_get_sp_count(uuid, &ffa_sp_count); =20 goto out; } =20 - if ( flags ) - { - ret =3D FFA_RET_INVALID_PARAMETERS; - goto out; - } - if ( !ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) ) { /* Just give an empty partition list to the caller */ @@ -121,80 +203,33 @@ void ffa_handle_partition_info_get(struct cpu_user_re= gs *regs) goto out; } =20 + /* Get the RX buffer to write the list of partitions */ ret =3D ffa_rx_acquire(d); if ( ret !=3D FFA_RET_OK ) goto out; =20 dst_buf =3D ctx->rx; + end_buf =3D ctx->rx + ctx->page_count * FFA_PAGE_SIZE; =20 - if ( !ffa_rx ) - { - ret =3D FFA_RET_DENIED; - goto out_rx_release; - } - - spin_lock(&ffa_rx_buffer_lock); - - ret =3D ffa_partition_info_get(uuid, 0, &ffa_sp_count, &src_size); - - if ( ret ) - goto out_rx_hyp_unlock; + /* An entry should be smaller than a page */ + BUILD_BUG_ON(sizeof(struct ffa_partition_info_1_1) > FFA_PAGE_SIZE); =20 /* - * ffa_partition_info_get() succeeded so we now own the RX buffer we - * share with the SPMC. We must give it back using ffa_hyp_rx_release() - * once we've copied the content. + * Check for overflow and that we can at least store one entry. + * page_count cannot be 0 so we have at least one page. */ - - /* we cannot have a size smaller than 1.0 structure */ - if ( src_size < sizeof(struct ffa_partition_info_1_0) ) - { - ret =3D FFA_RET_NOT_SUPPORTED; - goto out_rx_hyp_release; - } - - if ( ctx->page_count * FFA_PAGE_SIZE < ffa_sp_count * dst_size ) + if ( dst_buf >=3D end_buf || dst_buf > (end_buf - dst_size) ) { - ret =3D FFA_RET_NO_MEMORY; - goto out_rx_hyp_release; + ret =3D FFA_RET_INVALID_PARAMETERS; + goto out_rx_release; } =20 - if ( ffa_sp_count > 0 ) - { - uint32_t n, n_limit =3D ffa_sp_count; - void *src_buf =3D ffa_rx; - - /* copy the secure partitions info */ - for ( n =3D 0; n < n_limit; n++ ) - { - struct ffa_partition_info_1_1 *fpi =3D src_buf; - - /* filter out SP not following bit 15 convention if any */ - if ( FFA_ID_IS_SECURE(fpi->id) ) - { - memcpy(dst_buf, src_buf, dst_size); - dst_buf +=3D dst_size; - } - else - ffa_sp_count--; + ret =3D ffa_get_sp_partinfo(uuid, &ffa_sp_count, dst_buf, end_buf, + dst_size); =20 - src_buf +=3D src_size; - } - } =20 -out_rx_hyp_release: - ffa_hyp_rx_release(); -out_rx_hyp_unlock: - spin_unlock(&ffa_rx_buffer_lock); out_rx_release: - /* - * The calling VM RX buffer only contains data to be used by the VM if= the - * call was successful, in which case the VM has to release the buffer - * once it has used the data. - * If something went wrong during the call, we have to release the RX - * buffer back to the SPMC as the VM will not do it. - */ - if ( ret !=3D FFA_RET_OK ) + if ( ret ) ffa_rx_release(d); out: if ( ret ) @@ -353,9 +388,10 @@ bool ffa_partinfo_init(void) goto out; } =20 - if ( count >=3D UINT16_MAX ) + if ( count >=3D FFA_MAX_NUM_SP ) { - printk(XENLOG_ERR "ffa: Impossible number of SPs: %u\n", count); + printk(XENLOG_ERR "ffa: More SPs than the maximum supported: %u - = %u\n", + count, FFA_MAX_NUM_SP); goto out; } =20 diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h index c4cd65538908..0a9c1082db28 100644 --- a/xen/arch/arm/tee/ffa_private.h +++ b/xen/arch/arm/tee/ffa_private.h @@ -6,15 +6,15 @@ #ifndef __FFA_PRIVATE_H__ #define __FFA_PRIVATE_H__ =20 +#include #include -#include -#include -#include #include -#include +#include #include +#include +#include #include -#include +#include =20 /* Error codes */ #define FFA_RET_OK 0 @@ -108,6 +108,14 @@ */ #define FFA_CTX_TEARDOWN_DELAY SECONDS(1) =20 +/* + * The maximum number of Secure partitions we support for partinfo_get. + * This prevents holding the CPU during potentially to long time during + * a partinfo_get call. Value choosen seems realistic for any configuration + * but can be incremented here if needed. + */ +#define FFA_MAX_NUM_SP 64 + /* * We rely on the convention suggested but not mandated by the FF-A * specification that secure world endpoint identifiers have the bit 15 --=20 2.47.1