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

Re: [PATCH v3 5/5] xen/arm: ffa: Enable VM to VM without firmware



Hi Bertrand,

On Mon, Mar 24, 2025 at 10:15 AM Bertrand Marquis
<bertrand.marquis@xxxxxxx> wrote:
>
> When VM to VM support is activated and there is no suitable FF-A support
> in the firmware, enable FF-A support for VMs to allow using it for VM to
> VM communications.
> If there is OP-TEE running in the secure world and using the non FF-A
> communication system, having CONFIG_FFA_VM_TO_VM could be non functional
> (if optee is probed first) or Optee could be non functional (if FF-A is

Prefer OP-TEE

> probed first) so it is not recommended to activate the configuration
> option for such systems.
>
> To make buffer full notification work between VMs when there is no
> firmware, rework the notification handling and modify the global flag to
> only be used as check for firmware notification support instead.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> ---
> Changes in v3:
> - fix typos in commit message
> - add spaces around <<
> - move notification id fix back into buffer full patch
> - fix | position in if
> Changes in v2:
> - replace ifdef with IS_ENABLED when possible
> ---
>  xen/arch/arm/tee/ffa.c       |  12 +++-
>  xen/arch/arm/tee/ffa_notif.c | 104 ++++++++++++++++-------------------
>  2 files changed, 59 insertions(+), 57 deletions(-)

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 e41ab5f8ada6..0627625efe60 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -324,8 +324,9 @@ static int ffa_domain_init(struct domain *d)
>      struct ffa_ctx *ctx;
>      int ret;
>
> -    if ( !ffa_fw_version )
> +    if ( !IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !ffa_fw_version )
>          return -ENODEV;
> +
>      /*
>       * We are using the domain_id + 1 as the FF-A ID for VMs as FF-A ID 0 is
>       * reserved for the hypervisor and we only support secure endpoints using
> @@ -561,6 +562,15 @@ err_no_fw:
>      bitmap_zero(ffa_fw_abi_supported, FFA_ABI_BITMAP_SIZE);
>      printk(XENLOG_WARNING "ARM FF-A No firmware support\n");
>
> +    if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
> +    {
> +        INIT_LIST_HEAD(&ffa_teardown_head);
> +        init_timer(&ffa_teardown_timer, ffa_teardown_timer_callback, NULL, 
> 0);
> +
> +        printk(XENLOG_INFO "ARM FF-A only available between VMs\n");
> +        return true;
> +    }
> +
>      return false;
>  }
>
> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
> index f6df2f15bb00..86bef6b3b2ab 100644
> --- a/xen/arch/arm/tee/ffa_notif.c
> +++ b/xen/arch/arm/tee/ffa_notif.c
> @@ -16,7 +16,7 @@
>
>  #include "ffa_private.h"
>
> -static bool __ro_after_init notif_enabled;
> +static bool __ro_after_init fw_notif_enabled;
>  static unsigned int __ro_after_init notif_sri_irq;
>
>  int ffa_handle_notification_bind(struct cpu_user_regs *regs)
> @@ -27,21 +27,17 @@ int ffa_handle_notification_bind(struct cpu_user_regs 
> *regs)
>      uint32_t bitmap_lo = get_user_reg(regs, 3);
>      uint32_t bitmap_hi = get_user_reg(regs, 4);
>
> -    if ( !notif_enabled )
> -        return FFA_RET_NOT_SUPPORTED;
> -
>      if ( (src_dst & 0xFFFFU) != ffa_get_vm_id(d) )
>          return FFA_RET_INVALID_PARAMETERS;
>
>      if ( flags )    /* Only global notifications are supported */
>          return FFA_RET_DENIED;
>
> -    /*
> -     * We only support notifications from SP so no need to check the sender
> -     * endpoint ID, the SPMC will take care of that for us.
> -     */
> -    return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags, bitmap_lo,
> -                           bitmap_hi);
> +    if ( FFA_ID_IS_SECURE(src_dst >> 16) && fw_notif_enabled )
> +        return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags,
> +                               bitmap_lo, bitmap_hi);
> +
> +    return FFA_RET_NOT_SUPPORTED;
>  }
>
>  int ffa_handle_notification_unbind(struct cpu_user_regs *regs)
> @@ -51,18 +47,14 @@ int ffa_handle_notification_unbind(struct cpu_user_regs 
> *regs)
>      uint32_t bitmap_lo = get_user_reg(regs, 3);
>      uint32_t bitmap_hi = get_user_reg(regs, 4);
>
> -    if ( !notif_enabled )
> -        return FFA_RET_NOT_SUPPORTED;
> -
>      if ( (src_dst & 0xFFFFU) != ffa_get_vm_id(d) )
>          return FFA_RET_INVALID_PARAMETERS;
>
> -    /*
> -     * We only support notifications from SP so no need to check the
> -     * destination endpoint ID, the SPMC will take care of that for us.
> -     */
> -    return  ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, bitmap_lo,
> -                            bitmap_hi);
> +    if ( FFA_ID_IS_SECURE(src_dst >> 16) && fw_notif_enabled )
> +        return  ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, 
> bitmap_lo,
> +                                bitmap_hi);
> +
> +    return FFA_RET_NOT_SUPPORTED;
>  }
>
>  void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
> @@ -71,7 +63,7 @@ void ffa_handle_notification_info_get(struct cpu_user_regs 
> *regs)
>      struct ffa_ctx *ctx = d->arch.tee;
>      bool notif_pending;
>
> -    if ( !notif_enabled )
> +    if ( !IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !fw_notif_enabled )
>      {
>          ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>          return;
> @@ -108,7 +100,7 @@ void ffa_handle_notification_get(struct cpu_user_regs 
> *regs)
>      uint32_t w6 = 0;
>      uint32_t w7 = 0;
>
> -    if ( !notif_enabled )
> +    if ( !IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !fw_notif_enabled )
>      {
>          ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>          return;
> @@ -120,7 +112,8 @@ void ffa_handle_notification_get(struct cpu_user_regs 
> *regs)
>          return;
>      }
>
> -    if ( flags & ( FFA_NOTIF_FLAG_BITMAP_SP | FFA_NOTIF_FLAG_BITMAP_SPM ) )
> +    if ( fw_notif_enabled && (flags & ( FFA_NOTIF_FLAG_BITMAP_SP |
> +                                        FFA_NOTIF_FLAG_BITMAP_SPM )) )
>      {
>          struct arm_smccc_1_2_regs arg = {
>              .a0 = FFA_NOTIFICATION_GET,
> @@ -177,15 +170,14 @@ int ffa_handle_notification_set(struct cpu_user_regs 
> *regs)
>      uint32_t bitmap_lo = get_user_reg(regs, 3);
>      uint32_t bitmap_hi = get_user_reg(regs, 4);
>
> -    if ( !notif_enabled )
> -        return FFA_RET_NOT_SUPPORTED;
> -
>      if ( (src_dst >> 16) != ffa_get_vm_id(d) )
>          return FFA_RET_INVALID_PARAMETERS;
>
> -    /* Let the SPMC check the destination of the notification */
> -    return ffa_simple_call(FFA_NOTIFICATION_SET, src_dst, flags, bitmap_lo,
> -                           bitmap_hi);
> +    if ( FFA_ID_IS_SECURE(src_dst >> 16) && fw_notif_enabled )
> +        return ffa_simple_call(FFA_NOTIFICATION_SET, src_dst, flags, 
> bitmap_lo,
> +                               bitmap_hi);
> +
> +    return FFA_RET_NOT_SUPPORTED;
>  }
>
>  #ifdef CONFIG_FFA_VM_TO_VM
> @@ -371,7 +363,7 @@ void ffa_notif_init_interrupt(void)
>  {
>      int ret;
>
> -    if ( notif_enabled && notif_sri_irq < NR_GIC_SGI )
> +    if ( fw_notif_enabled && notif_sri_irq < NR_GIC_SGI )
>      {
>          /*
>           * An error here is unlikely since the primary CPU has already
> @@ -402,41 +394,41 @@ void ffa_notif_init(void)
>      int ret;
>
>      /* Only enable fw notification if all ABIs we need are supported */
> -    if ( !(ffa_fw_supports_fid(FFA_NOTIFICATION_BITMAP_CREATE) &&
> -           ffa_fw_supports_fid(FFA_NOTIFICATION_BITMAP_DESTROY) &&
> -           ffa_fw_supports_fid(FFA_NOTIFICATION_GET) &&
> -           ffa_fw_supports_fid(FFA_NOTIFICATION_INFO_GET_64)) )
> -        return;
> -
> -    arm_smccc_1_2_smc(&arg, &resp);
> -    if ( resp.a0 != FFA_SUCCESS_32 )
> -        return;
> -
> -    irq = resp.a2;
> -    notif_sri_irq = irq;
> -    if ( irq >= NR_GIC_SGI )
> -        irq_set_type(irq, IRQ_TYPE_EDGE_RISING);
> -    ret = request_irq(irq, 0, notif_irq_handler, "FF-A notif", NULL);
> -    if ( ret )
> +    if ( ffa_fw_supports_fid(FFA_NOTIFICATION_BITMAP_CREATE) &&
> +         ffa_fw_supports_fid(FFA_NOTIFICATION_BITMAP_DESTROY) &&
> +         ffa_fw_supports_fid(FFA_NOTIFICATION_GET) &&
> +         ffa_fw_supports_fid(FFA_NOTIFICATION_INFO_GET_64) )
>      {
> -        printk(XENLOG_ERR "ffa: request_irq irq %u failed: error %d\n",
> -               irq, ret);
> -        return;
> -    }
> +        arm_smccc_1_2_smc(&arg, &resp);
> +        if ( resp.a0 != FFA_SUCCESS_32 )
> +            return;
>
> -    notif_enabled = true;
> +        irq = resp.a2;
> +        notif_sri_irq = irq;
> +        if ( irq >= NR_GIC_SGI )
> +            irq_set_type(irq, IRQ_TYPE_EDGE_RISING);
> +        ret = request_irq(irq, 0, notif_irq_handler, "FF-A notif", NULL);
> +        if ( ret )
> +        {
> +            printk(XENLOG_ERR "ffa: request_irq irq %u failed: error %d\n",
> +                   irq, ret);
> +            return;
> +        }
> +        fw_notif_enabled = true;
> +    }
>  }
>
>  int ffa_notif_domain_init(struct domain *d)
>  {
>      int32_t res;
>
> -    if ( !notif_enabled )
> -        return 0;
> +    if ( fw_notif_enabled )
> +    {
>
> -    res = ffa_notification_bitmap_create(ffa_get_vm_id(d), d->max_vcpus);
> -    if ( res )
> -        return -ENOMEM;
> +        res = ffa_notification_bitmap_create(ffa_get_vm_id(d), d->max_vcpus);
> +        if ( res )
> +            return -ENOMEM;
> +    }
>
>      return 0;
>  }
> @@ -447,6 +439,6 @@ void ffa_notif_domain_destroy(struct domain *d)
>       * Call bitmap_destroy even if bitmap create failed as the SPMC will
>       * return a DENIED error that we will ignore.
>       */
> -    if ( notif_enabled )
> +    if ( fw_notif_enabled )
>          ffa_notification_bitmap_destroy(ffa_get_vm_id(d));
>  }
> --
> 2.47.1
>



 


Rackspace

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