Re: [PATCH 01/12] xen/arm: ffa: Add endpoint lookup helper

[Jens Wiklander <jens.wiklander@xxxxxxxxxx>]

Hi Bertrand,

On Tue, Feb 3, 2026 at 6:38 PM Bertrand Marquis
<bertrand.marquis@xxxxxxx> wrote:
>
> Direct messaging paths duplicate endpoint validation and RCU domain
> lookup logic across multiple call sites, which makes the checks easy to
> drift and complicates maintenance.
>
> Introduce ffa_endpoint_domain_lookup() to centralize this logic. The
> helper validates the endpoint ID (rejecting ID 0 for the hypervisor),
> performs RCU domain lookup, ensures the domain is live and has an
> initialized FF-A context with a negotiated version, and returns the
> domain locked via RCU.
>
> Switch ffa_msg_send2_vm() to use the helper, replacing its open-coded
> validation sequence. This consolidates approximately 20 lines of
> duplicated checks into a single call.
>
> No functional changes.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> ---
>  xen/arch/arm/tee/ffa.c         | 45 ++++++++++++++++++++++++++++++++++
>  xen/arch/arm/tee/ffa_msg.c     | 24 +++---------------
>  xen/arch/arm/tee/ffa_private.h |  3 +++
>  3 files changed, 51 insertions(+), 21 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 ed18e76080d0..6de2b9f8ac8e 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -433,6 +433,51 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
>      return true;
>  }
>
> +/*
> + * Look up a domain by its FF-A endpoint ID and validate it's ready for FF-A.
> + * Returns FFA_RET_OK on success with domain locked via RCU.
> + * Caller must call rcu_unlock_domain() when done.
> + *
> + * Validates:
> + * - endpoint_id is not 0 (the hypervisor)
> + * - domain exists and is live
> + * - domain has FF-A context initialized
> + * - domain has negotiated an FF-A version
> + */
> +int32_t ffa_endpoint_domain_lookup(uint16_t endpoint_id, struct domain 
> **d_out,
> +                                   struct ffa_ctx **ctx_out)
> +{
> +    struct domain *d;
> +    struct ffa_ctx *ctx;
> +    int err;
> +
> +    if ( endpoint_id == 0 )
> +        return FFA_RET_INVALID_PARAMETERS;
> +
> +    err = rcu_lock_live_remote_domain_by_id(endpoint_id - 1, &d);
> +    if ( err )
> +        return FFA_RET_INVALID_PARAMETERS;
> +
> +    if ( !d->arch.tee )
> +    {
> +        rcu_unlock_domain(d);
> +        return FFA_RET_INVALID_PARAMETERS;
> +    }
> +
> +    ctx = d->arch.tee;
> +    if ( !ACCESS_ONCE(ctx->guest_vers) )
> +    {
> +        rcu_unlock_domain(d);
> +        return FFA_RET_INVALID_PARAMETERS;
> +    }
> +
> +    *d_out = d;
> +    if ( ctx_out )
> +        *ctx_out = ctx;
> +
> +    return FFA_RET_OK;
> +}
> +
>  static int ffa_domain_init(struct domain *d)
>  {
>      struct ffa_ctx *ctx;
> diff --git a/xen/arch/arm/tee/ffa_msg.c b/xen/arch/arm/tee/ffa_msg.c
> index 4e26596461a9..10856fddcbc4 100644
> --- a/xen/arch/arm/tee/ffa_msg.c
> +++ b/xen/arch/arm/tee/ffa_msg.c
> @@ -161,30 +161,12 @@ static int32_t ffa_msg_send2_vm(uint16_t dst_id, const 
> void *src_buf,
>      struct ffa_part_msg_rxtx_1_2 *dst_msg;
>      void *rx_buf;
>      size_t rx_size;
> -    int err;
>      int32_t ret;
>
> -    if ( dst_id == 0 )
> -        /* FF-A ID 0 is the hypervisor, this is not valid */
> -        return FFA_RET_INVALID_PARAMETERS;
> -
>      /* This is also checking that dest is not src */
> -    err = rcu_lock_live_remote_domain_by_id(dst_id - 1, &dst_d);
> -    if ( err )
> -        return FFA_RET_INVALID_PARAMETERS;
> -
> -    if ( dst_d->arch.tee == NULL )
> -    {
> -        ret = FFA_RET_INVALID_PARAMETERS;
> -        goto out_unlock;
> -    }
> -
> -    dst_ctx = dst_d->arch.tee;
> -    if ( !ACCESS_ONCE(dst_ctx->guest_vers) )
> -    {
> -        ret = FFA_RET_INVALID_PARAMETERS;
> -        goto out_unlock;
> -    }
> +    ret = ffa_endpoint_domain_lookup(dst_id, &dst_d, &dst_ctx);
> +    if ( ret )
> +        return ret;
>
>      /* This also checks that destination has set a Rx buffer */
>      ret = ffa_rx_acquire(dst_ctx , &rx_buf, &rx_size);
> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
> index 282c105f3bce..cd7ecabc7eff 100644
> --- a/xen/arch/arm/tee/ffa_private.h
> +++ b/xen/arch/arm/tee/ffa_private.h
> @@ -437,6 +437,9 @@ int32_t ffa_partinfo_domain_init(struct domain *d);
>  bool ffa_partinfo_domain_destroy(struct domain *d);
>  void ffa_handle_partition_info_get(struct cpu_user_regs *regs);
>
> +int32_t ffa_endpoint_domain_lookup(uint16_t endpoint_id, struct domain 
> **d_out,
> +                                   struct ffa_ctx **ctx_out);
> +
>  bool ffa_rxtx_spmc_init(void);
>  void ffa_rxtx_spmc_destroy(void);
>  void *ffa_rxtx_spmc_rx_acquire(void);
> --
> 2.50.1 (Apple Git-155)
>