[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 08/12] xen/arm: ffa: Fix FFA_FEATURES validation


  • To: Bertrand Marquis <bertrand.marquis@xxxxxxx>
  • From: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
  • Date: Thu, 12 Feb 2026 15:42:52 +0100
  • Arc-authentication-results: i=1; mx.google.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=fmsGaSKF5EXtyoY37xtT1UIgrvDD2BADzoVe0lfSOe4=; fh=wNLC6Hyb5Ukz/ErppBRQBwv8vwa/OMsdh6R8bnNsiPU=; b=S/xeuassQH/JaRGGBfttMisg9oKaJ+G8fjiMvx9VtL7YdNLvKyW3NYnq0AgHmf3iQ1 ZgYAfo1NNXGaz+7ZMnxrYQbd5M+5nzXv8JBa4uw2f5+jwPwP3c98CWVQrp3kx5ivLnp6 SqV+FdB/1585Km8CQMdLr2hAnnJ6VfVZi3Suk5zdVLvI0uMbjsPVHNHVH0D4mw4SAWUe VvFz1sz6jflhKy3vchiENlBJ7BIfn468wZ56c1w/xX/nKb4Dr610NT+HeJg2A+55Jp8k QaAoD3jRaOS1UWgCjlpXLP7VlOMdA90FDKKrQNoBu9PS9Pkl6USZGA8DsoFk/DGNVjAt RxQQ==; darn=lists.xenproject.org
  • Arc-seal: i=1; a=rsa-sha256; t=1770907384; cv=none; d=google.com; s=arc-20240605; b=JOS3+mMbhh2Z4KA/izPPzhX107UEQlsz+HSTZ11ou55J9rJqlwY7L/idnHEebDmvF1 hMY1M/dx3Tla/QNC8ioacblDlQDXj5MSvhzo+3fQhXtHpyvbxsiU975mmbbHcXIF7U95 lZIwkjUKqG31HR951xCL67qku3FwAtbe9H4ngGg/KOl/AYA4dG7rsemAcKLAlhm6Vwyr qInDwyYJMWA21zNyPrCVaIzq3mZHHWE/DqvDsXfguL4xXv5XveWWBxzf50Egbsube8YZ /M8Q06GuSHxJR77VH96J5KAxMujIr/RdcuSuuqHsK+6nGFLFp/KgG1sabZAxt7hHvEk0 A8wA==
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>
  • Delivery-date: Thu, 12 Feb 2026 14:43:09 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Bertrand,

On Wed, Feb 11, 2026 at 6:16 PM Bertrand Marquis
<bertrand.marquis@xxxxxxx> wrote:
>
> FFA_FEATURES advertises several ABIs unconditionally, even when firmware
> support is missing or when the ABI is physical-instance-only. This can
> mislead guests about what Xen can actually provide and violates FF-A
> calling conventions. Some SPMCs (Hafnium v2.14 or earlier) also fail to
> report FFA_RX_ACQUIRE despite supporting it.
>
> Update FFA_FEATURES validation to match spec and firmware support:
> - do no check w2-w7 input registers during FFA_FEATURES as they are
>   flagged Should Be Zero by the FF-A specification or are not used by
>   the calls we support.
> - reject 64-bit calling conventions from 32-bit guests with NOT_SUPPORTED
> - return NOT_SUPPORTED for physical-instance-only ABIs
> (FFA_NOTIFICATION_BITMAP_{CREATE,DESTROY}, FFA_RX_ACQUIRE)
> - advertise FFA_INTERRUPT as supported
> - gate message ABIs on firmware support:
> - FFA_MSG_SEND_DIRECT_REQ_{32,64}
> - FFA_MSG_SEND_DIRECT_REQ2 (also requires FF-A 1.2 negotiation)
> - FFA_MSG_SEND2 (or VM-to-VM enabled)
> - report MEM_SHARE_{32,64} only when FFA_MEM_SHARE_64 is supported
> - stop advertising FFA_MSG_YIELD (not implemented)
>
> Update firmware probing: drop FFA_MEM_SHARE_32 checks (deprecated) and
> add FFA_RX_ACQUIRE to the probed set. If FFA_MSG_SEND2 is reported but
> FFA_RX_ACQUIRE is not, assume RX_ACQUIRE support and warn to work
> around the Hafnium bug.
>
> Functional impact: guests now see ABI support that reflects firmware
> capabilities and Xen implementation status. When SEND2 is present but
> RX_ACQUIRE is not reported, Xen assumes RX_ACQUIRE support.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> ---
> Changes since v1:
> - remove completely w2-w7 checking as those are SBZ in the spec or
>   unused by xen
> ---
>  xen/arch/arm/tee/ffa.c | 63 +++++++++++++++++++++++++++++++++---------
>  1 file changed, 50 insertions(+), 13 deletions(-)

Looks good:
Reviewed-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>

Cheers,
Jens

>
> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> index 23e1f408485b..cb8cd95fdafa 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -91,10 +91,10 @@ static const struct ffa_fw_abi ffa_fw_abi_needed[] = {
>      FW_ABI(FFA_PARTITION_INFO_GET),
>      FW_ABI(FFA_NOTIFICATION_INFO_GET_64),
>      FW_ABI(FFA_NOTIFICATION_GET),
> +    FW_ABI(FFA_RX_ACQUIRE),
>      FW_ABI(FFA_RX_RELEASE),
>      FW_ABI(FFA_RXTX_MAP_64),
>      FW_ABI(FFA_RXTX_UNMAP),
> -    FW_ABI(FFA_MEM_SHARE_32),
>      FW_ABI(FFA_MEM_SHARE_64),
>      FW_ABI(FFA_MEM_RECLAIM),
>      FW_ABI(FFA_MSG_SEND_DIRECT_REQ_32),
> @@ -238,21 +238,30 @@ static void handle_features(struct cpu_user_regs *regs)
>      uint32_t a1 = get_user_reg(regs, 1);
>      struct domain *d = current->domain;
>      struct ffa_ctx *ctx = d->arch.tee;
> -    unsigned int n;
>
> -    for ( n = 2; n <= 7; n++ )
> +    /*
> +     * FFA_FEATURES defines w2 as input properties only for specific
> +     * function IDs and reserves w3-w7 as SBZ. Xen does not currently
> +     * implement any feature that consumes w2, so ignore extra inputs.
> +     */
> +    if ( !is_64bit_domain(d) && smccc_is_conv_64(a1) )
>      {
> -        if ( get_user_reg(regs, n) )
> -        {
> -            ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> -            return;
> -        }
> +        /* 32bit guests should only use 32bit convention calls */
> +        ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> +        return;
>      }
>
>      switch ( a1 )
>      {
> +    case FFA_NOTIFICATION_BITMAP_CREATE:
> +    case FFA_NOTIFICATION_BITMAP_DESTROY:
> +    case FFA_RX_ACQUIRE:
> +        /* Physical-instance-only ABIs are not exposed to VMs. */
> +        ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> +        break;
>      case FFA_ERROR:
>      case FFA_VERSION:
> +    case FFA_INTERRUPT:
>      case FFA_SUCCESS_32:
>      case FFA_SUCCESS_64:
>      case FFA_FEATURES:
> @@ -261,16 +270,25 @@ static void handle_features(struct cpu_user_regs *regs)
>      case FFA_RXTX_UNMAP:
>      case FFA_MEM_RECLAIM:
>      case FFA_PARTITION_INFO_GET:
> +        ffa_set_regs_success(regs, 0, 0);
> +        break;
>      case FFA_MSG_SEND_DIRECT_REQ_32:
>      case FFA_MSG_SEND_DIRECT_REQ_64:
> -    case FFA_MSG_SEND2:
>      case FFA_RUN:
> -    case FFA_INTERRUPT:
> -    case FFA_MSG_YIELD:
> -        ffa_set_regs_success(regs, 0, 0);
> +        if ( ffa_fw_supports_fid(a1) )
> +            ffa_set_regs_success(regs, 0, 0);
> +        else
> +            ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> +        break;
> +    case FFA_MSG_SEND2:
> +        if ( ffa_fw_supports_fid(a1) || IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
> +            ffa_set_regs_success(regs, 0, 0);
> +        else
> +            ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>          break;
>      case FFA_MSG_SEND_DIRECT_REQ2:
> -        if ( ACCESS_ONCE(ctx->guest_vers) >= FFA_VERSION_1_2 )
> +        if ( ACCESS_ONCE(ctx->guest_vers) >= FFA_VERSION_1_2 &&
> +             ffa_fw_supports_fid(FFA_MSG_SEND_DIRECT_REQ2) )
>          {
>              ffa_set_regs_success(regs, 0, 0);
>          }
> @@ -281,6 +299,11 @@ static void handle_features(struct cpu_user_regs *regs)
>          break;
>      case FFA_MEM_SHARE_64:
>      case FFA_MEM_SHARE_32:
> +        if ( !ffa_fw_supports_fid(FFA_MEM_SHARE_64) )
> +        {
> +            ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> +            break;
> +        }
>          /*
>           * We currently don't support dynamically allocated buffers. Report
>           * that with 0 in bit[0] of w2.
> @@ -688,6 +711,20 @@ static bool ffa_probe_fw(void)
>                     ffa_fw_abi_needed[i].name);
>      }
>
> +    /*
> +     * Hafnium v2.14 or earlier does not report FFA_RX_ACQUIRE in
> +     * FFA_FEATURES even though it supports it.
> +     */
> +    if ( !ffa_fw_supports_fid(FFA_RX_ACQUIRE) &&
> +         ffa_fw_supports_fid(FFA_MSG_SEND2) )
> +    {
> +        printk(XENLOG_WARNING
> +               "ARM FF-A Firmware reports FFA_MSG_SEND2 without 
> FFA_RX_ACQUIRE\n");
> +        printk(XENLOG_WARNING
> +               "ffa: assuming RX_ACQUIRE support (workaround)\n");
> +        set_bit(FFA_ABI_BITNUM(FFA_RX_ACQUIRE), ffa_fw_abi_supported);
> +    }
> +
>      if ( !ffa_rxtx_spmc_init() )
>      {
>          printk(XENLOG_ERR "ffa: Error during RXTX buffer init\n");
> --
> 2.52.0
>



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.