Hi Bertrand, On Tue, Feb 3, 2026 at 6:38 PM Bertrand Marquis <[email protected]> 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 <[email protected]> > --- > 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 <[email protected]> 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) >
