Track FF-A version negotiation per VM and enforce that no FF-A ABI
(other than FFA_VERSION) is processed before a guest has selected a
version.
Each ffa_ctx gains a dedicated guest_vers_lock, a negotiated version
(guest_vers) and a guest_vers_tmp:
- guest_vers is the version negotiated or 0 if no version has been
negotiated. This must be used with ACCESS_ONCE when reading it without
the spinlock taken.
- guest_vers_tmp stores the version currently requested by a VM.
The version requested is the one actually negotiated once a call
different from FFA_VERSION is done to allow several attempts and as
requested by FF-A specification.
We always return our implementation version FFA_MY_VERSION, even if the
version requested was different, as requested by FF-A specification.
Any call other than FFA_VERSION is rejected until a version has been
requested.
Update all places in the code where guest_vers is used to use
ACCESS_ONCE.
This prevents partially initialised contexts from using the mediator
and complies with the FF-A 1.2 FFA_VERSION semantics.
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in v1:
- remove the guest_vers_negotiated and use guest_vers = 0 as condition
for a version being negotiated instead
- introduce guest_vers_tmp to store a requested version until it is
becoming the one negotiated.
- remove not needed if negotiated condition.
- use ACCESS_ONCE when reading guest_vers and use guest_vers == 0 as
condition for a version being negotiated.
- Update FF-A version handling comment in ffa_private.h
---
xen/arch/arm/tee/ffa.c | 101 +++++++++++++++++++++++++-------
xen/arch/arm/tee/ffa_msg.c | 2 +-
xen/arch/arm/tee/ffa_partinfo.c | 4 +-
xen/arch/arm/tee/ffa_private.h | 26 ++++++--
xen/arch/arm/tee/ffa_shm.c | 3 +-
5 files changed, 105 insertions(+), 31 deletions(-)
diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
index 2b4e24750d52..aadd6c21e7f2 100644
--- a/xen/arch/arm/tee/ffa.c
+++ b/xen/arch/arm/tee/ffa.c
@@ -158,31 +158,38 @@ static bool ffa_abi_supported(uint32_t id)
return !ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
}
-static void handle_version(struct cpu_user_regs *regs)
+static bool ffa_negotiate_version(struct cpu_user_regs *regs)
{
struct domain *d = current->domain;
struct ffa_ctx *ctx = d->arch.tee;
- uint32_t vers = get_user_reg(regs, 1);
- uint32_t old_vers;
+ uint32_t fid = get_user_reg(regs, 0);
+ uint32_t in_vers = get_user_reg(regs, 1);
+ uint32_t out_vers = FFA_MY_VERSION;
- /*
- * Guest will use the version it requested if it is our major and minor
- * lower or equals to ours. If the minor is greater, our version will be
- * used.
- * In any case return our version to the caller.
- */
- if ( FFA_VERSION_MAJOR(vers) == FFA_MY_VERSION_MAJOR )
+ spin_lock(&ctx->guest_vers_lock);
+
+ /* If negotiation already published, continue without handling. */
+ if ( ACCESS_ONCE(ctx->guest_vers) )
+ goto out_continue;
+
+ if ( fid != FFA_VERSION )
{
- spin_lock(&ctx->lock);
- old_vers = ctx->guest_vers;
+ if ( !ctx->guest_vers_tmp )
+ {
+ out_vers = 0;
+ goto out_handled;
+ }
- if ( FFA_VERSION_MINOR(vers) > FFA_MY_VERSION_MINOR )
- ctx->guest_vers = FFA_MY_VERSION;
- else
- ctx->guest_vers = vers;
- spin_unlock(&ctx->lock);
+ /*
+ * A successful FFA_VERSION call does not freeze negotiation. Guests
+ * are allowed to issue multiple FFA_VERSION attempts (e.g. probing
+ * several minor versions). Negotiation becomes final only when a
+ * non-VERSION ABI is invoked, as required by the FF-A specification.
+ * Finalize negotiation: publish guest_vers once, then never change.
+ */
+ ACCESS_ONCE(ctx->guest_vers) = ctx->guest_vers_tmp;
- if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !old_vers )
+ if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
{
/* One more VM with FF-A support available */
inc_ffa_vm_count();
@@ -190,8 +197,48 @@ static void handle_version(struct cpu_user_regs *regs)
list_add_tail(&ctx->ctx_list, &ffa_ctx_head);
write_unlock(&ffa_ctx_list_rwlock);
}
+
+ goto out_continue;
}
- ffa_set_regs(regs, FFA_MY_VERSION, 0, 0, 0, 0, 0, 0, 0);
+
+ /*
+ * guest_vers_tmp stores the version selected by the guest (lower minor may
+ * require reduced data structures). However, the value returned to the
+ * guest via FFA_VERSION is always FFA_MY_VERSION, the implementation
+ * version, as required by FF-A. The two values intentionally differ.
+ */
+
+ /*
+ * Return our highest implementation version on request different than our
+ * major and mark negotiated version as our implementation version.
+ */
+ if ( FFA_VERSION_MAJOR(in_vers) != FFA_MY_VERSION_MAJOR )
+ {
+ ctx->guest_vers_tmp = FFA_MY_VERSION;
+ goto out_handled;
+ }
+
+ /*
+ * Use our minor version if a greater minor was requested or the requested
+ * minor if it is lower than ours was requested.
+ */
+ if ( FFA_VERSION_MINOR(in_vers) > FFA_MY_VERSION_MINOR )
+ ctx->guest_vers_tmp = FFA_MY_VERSION;
+ else
+ ctx->guest_vers_tmp = in_vers;
+
+out_handled:
+ spin_unlock(&ctx->guest_vers_lock);
+ if ( out_vers )
+ ffa_set_regs(regs, out_vers, 0, 0, 0, 0, 0, 0, 0);
+ else
+ ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
+ return true;
+
+out_continue:
+ spin_unlock(&ctx->guest_vers_lock);
+
+ return false;
}
static void handle_features(struct cpu_user_regs *regs)
@@ -274,10 +321,17 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
if ( !ctx )
return false;
+ /* A version must be negotiated first */
+ if ( !ACCESS_ONCE(ctx->guest_vers) )
+ {
+ if ( ffa_negotiate_version(regs) )
+ return true;
+ }
+
switch ( fid )
{
case FFA_VERSION:
- handle_version(regs);
+ ffa_set_regs(regs, FFA_MY_VERSION, 0, 0, 0, 0, 0, 0, 0);
return true;
case FFA_ID_GET:
ffa_set_regs_success(regs, ffa_get_vm_id(d), 0);
@@ -371,6 +425,11 @@ static int ffa_domain_init(struct domain *d)
d->arch.tee = ctx;
ctx->teardown_d = d;
INIT_LIST_HEAD(&ctx->shm_list);
+ spin_lock_init(&ctx->lock);
+ spin_lock_init(&ctx->guest_vers_lock);
+ ctx->guest_vers = 0;
+ ctx->guest_vers_tmp = 0;
+ INIT_LIST_HEAD(&ctx->ctx_list);
ctx->ffa_id = ffa_get_vm_id(d);
ctx->num_vcpus = d->max_vcpus;
@@ -452,7 +511,7 @@ static int ffa_domain_teardown(struct domain *d)
if ( !ctx )
return 0;
- if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && ctx->guest_vers )
+ if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && ACCESS_ONCE(ctx->guest_vers) )
{
dec_ffa_vm_count();
write_lock(&ffa_ctx_list_rwlock);
diff --git a/xen/arch/arm/tee/ffa_msg.c b/xen/arch/arm/tee/ffa_msg.c
index c20c5bec0f76..2c2ebc9c5cd6 100644
--- a/xen/arch/arm/tee/ffa_msg.c
+++ b/xen/arch/arm/tee/ffa_msg.c
@@ -113,7 +113,7 @@ static int32_t ffa_msg_send2_vm(uint16_t dst_id, const void *src_buf,
}
dst_ctx = dst_d->arch.tee;
- if ( !dst_ctx->guest_vers )
+ if ( !ACCESS_ONCE(dst_ctx->guest_vers) )
{
ret = FFA_RET_INVALID_PARAMETERS;
goto out_unlock;
diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
index fa56b1587e3b..ec5a53ed1cab 100644
--- a/xen/arch/arm/tee/ffa_partinfo.c
+++ b/xen/arch/arm/tee/ffa_partinfo.c
@@ -238,7 +238,7 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
* use the v1.0 structure size in the destination buffer.
* Otherwise use the size of the highest version we support, here 1.1.
*/
- if ( ctx->guest_vers == FFA_VERSION_1_0 )
+ if ( ACCESS_ONCE(ctx->guest_vers) == FFA_VERSION_1_0 )
dst_size = sizeof(struct ffa_partition_info_1_0);
else
dst_size = sizeof(struct ffa_partition_info_1_1);
@@ -250,7 +250,7 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
* 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 == FFA_VERSION_1_0 ||
+ if ( ACCESS_ONCE(ctx->guest_vers) == FFA_VERSION_1_0 ||
flags != FFA_PARTITION_INFO_GET_COUNT_FLAG )
{
ret = FFA_RET_INVALID_PARAMETERS;
diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
index 8d01da0009d3..4e4ac7fd7bc4 100644
--- a/xen/arch/arm/tee/ffa_private.h
+++ b/xen/arch/arm/tee/ffa_private.h
@@ -355,12 +355,6 @@ struct ffa_ctx {
* Global data accessed with lock locked.
*/
spinlock_t lock;
- /*
- * FF-A version negotiated by the guest, only modifications to
- * this field are done with the lock held as this is expected to
- * be done once at init by a guest.
- */
- uint32_t guest_vers;
/* Number of 4kB pages in each of rx/rx_pg and tx/tx_pg */
unsigned int page_count;
/* Number of allocated shared memory object */
@@ -368,6 +362,26 @@ struct ffa_ctx {
/* Used shared memory objects, struct ffa_shm_mem */
struct list_head shm_list;
+ /*
+ * FF-A version handling
+ * guest_vers is the single published negotiated version. It is 0 until
+ * negotiation completes, after which it is set once and never changes.
+ * Negotiation uses guest_vers_tmp under guest_vers_lock; when a
+ * non-VERSION ABI is invoked, Xen finalizes negotiation by publishing
+ * guest_vers using ACCESS_ONCE() store.
+ * Readers use ACCESS_ONCE(guest_vers) != 0 to detect availability and
+ * can consume guest_vers without barriers because it never changes once
+ * published.
+ */
+ spinlock_t guest_vers_lock;
+ /*
+ * Published negotiated version. Zero means "not negotiated yet".
+ * Once non-zero, it never changes.
+ */
+ uint32_t guest_vers;
+ /* Temporary version used during negotiation under guest_vers_lock */
+ uint32_t guest_vers_tmp;
+
/*
* Rx buffer, accessed with rx_lock locked.
* rx_is_free is used to serialize access.
diff --git a/xen/arch/arm/tee/ffa_shm.c b/xen/arch/arm/tee/ffa_shm.c
index d628c1b70609..dad3da192247 100644
--- a/xen/arch/arm/tee/ffa_shm.c
+++ b/xen/arch/arm/tee/ffa_shm.c
@@ -495,7 +495,8 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs)
if ( frag_len > ctx->page_count * FFA_PAGE_SIZE )
goto out_unlock;
- ret = read_mem_transaction(ctx->guest_vers, ctx->tx, frag_len, &trans);
+ ret = read_mem_transaction(ACCESS_ONCE(ctx->guest_vers), ctx->tx,
+ frag_len, &trans);
if ( ret )
goto out_unlock;
--
2.51.2Hi Bertrand,
On Fri, Dec 5, 2025 at 11:37 AM Bertrand Marquis
<bertrand.marquis@arm.com> wrote:
>
> Track FF-A version negotiation per VM and enforce that no FF-A ABI
> (other than FFA_VERSION) is processed before a guest has selected a
> version.
>
> Each ffa_ctx gains a dedicated guest_vers_lock, a negotiated version
> (guest_vers) and a guest_vers_tmp:
> - guest_vers is the version negotiated or 0 if no version has been
> negotiated. This must be used with ACCESS_ONCE when reading it without
> the spinlock taken.
> - guest_vers_tmp stores the version currently requested by a VM.
>
> The version requested is the one actually negotiated once a call
> different from FFA_VERSION is done to allow several attempts and as
> requested by FF-A specification.
> We always return our implementation version FFA_MY_VERSION, even if the
> version requested was different, as requested by FF-A specification.
>
> Any call other than FFA_VERSION is rejected until a version has been
> requested.
>
> Update all places in the code where guest_vers is used to use
> ACCESS_ONCE.
>
> This prevents partially initialised contexts from using the mediator
> and complies with the FF-A 1.2 FFA_VERSION semantics.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in v1:
> - remove the guest_vers_negotiated and use guest_vers = 0 as condition
> for a version being negotiated instead
> - introduce guest_vers_tmp to store a requested version until it is
> becoming the one negotiated.
> - remove not needed if negotiated condition.
> - use ACCESS_ONCE when reading guest_vers and use guest_vers == 0 as
> condition for a version being negotiated.
> - Update FF-A version handling comment in ffa_private.h
> ---
> xen/arch/arm/tee/ffa.c | 101 +++++++++++++++++++++++++-------
> xen/arch/arm/tee/ffa_msg.c | 2 +-
> xen/arch/arm/tee/ffa_partinfo.c | 4 +-
> xen/arch/arm/tee/ffa_private.h | 26 ++++++--
> xen/arch/arm/tee/ffa_shm.c | 3 +-
> 5 files changed, 105 insertions(+), 31 deletions(-)
>
> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> index 2b4e24750d52..aadd6c21e7f2 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -158,31 +158,38 @@ static bool ffa_abi_supported(uint32_t id)
> return !ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
> }
>
> -static void handle_version(struct cpu_user_regs *regs)
> +static bool ffa_negotiate_version(struct cpu_user_regs *regs)
> {
> struct domain *d = current->domain;
> struct ffa_ctx *ctx = d->arch.tee;
> - uint32_t vers = get_user_reg(regs, 1);
> - uint32_t old_vers;
> + uint32_t fid = get_user_reg(regs, 0);
> + uint32_t in_vers = get_user_reg(regs, 1);
> + uint32_t out_vers = FFA_MY_VERSION;
>
> - /*
> - * Guest will use the version it requested if it is our major and minor
> - * lower or equals to ours. If the minor is greater, our version will be
> - * used.
> - * In any case return our version to the caller.
> - */
> - if ( FFA_VERSION_MAJOR(vers) == FFA_MY_VERSION_MAJOR )
> + spin_lock(&ctx->guest_vers_lock);
> +
> + /* If negotiation already published, continue without handling. */
> + if ( ACCESS_ONCE(ctx->guest_vers) )
> + goto out_continue;
> +
> + if ( fid != FFA_VERSION )
> {
> - spin_lock(&ctx->lock);
> - old_vers = ctx->guest_vers;
> + if ( !ctx->guest_vers_tmp )
> + {
> + out_vers = 0;
> + goto out_handled;
> + }
>
> - if ( FFA_VERSION_MINOR(vers) > FFA_MY_VERSION_MINOR )
> - ctx->guest_vers = FFA_MY_VERSION;
> - else
> - ctx->guest_vers = vers;
> - spin_unlock(&ctx->lock);
> + /*
> + * A successful FFA_VERSION call does not freeze negotiation. Guests
> + * are allowed to issue multiple FFA_VERSION attempts (e.g. probing
> + * several minor versions). Negotiation becomes final only when a
> + * non-VERSION ABI is invoked, as required by the FF-A specification.
> + * Finalize negotiation: publish guest_vers once, then never change.
> + */
> + ACCESS_ONCE(ctx->guest_vers) = ctx->guest_vers_tmp;
>
> - if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !old_vers )
> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
> {
> /* One more VM with FF-A support available */
> inc_ffa_vm_count();
> @@ -190,8 +197,48 @@ static void handle_version(struct cpu_user_regs *regs)
> list_add_tail(&ctx->ctx_list, &ffa_ctx_head);
> write_unlock(&ffa_ctx_list_rwlock);
> }
> +
> + goto out_continue;
> }
> - ffa_set_regs(regs, FFA_MY_VERSION, 0, 0, 0, 0, 0, 0, 0);
> +
> + /*
> + * guest_vers_tmp stores the version selected by the guest (lower minor may
> + * require reduced data structures). However, the value returned to the
> + * guest via FFA_VERSION is always FFA_MY_VERSION, the implementation
> + * version, as required by FF-A. The two values intentionally differ.
> + */
> +
> + /*
> + * Return our highest implementation version on request different than our
> + * major and mark negotiated version as our implementation version.
> + */
> + if ( FFA_VERSION_MAJOR(in_vers) != FFA_MY_VERSION_MAJOR )
> + {
> + ctx->guest_vers_tmp = FFA_MY_VERSION;
> + goto out_handled;
> + }
> +
> + /*
> + * Use our minor version if a greater minor was requested or the requested
> + * minor if it is lower than ours was requested.
> + */
> + if ( FFA_VERSION_MINOR(in_vers) > FFA_MY_VERSION_MINOR )
> + ctx->guest_vers_tmp = FFA_MY_VERSION;
> + else
> + ctx->guest_vers_tmp = in_vers;
> +
> +out_handled:
> + spin_unlock(&ctx->guest_vers_lock);
> + if ( out_vers )
> + ffa_set_regs(regs, out_vers, 0, 0, 0, 0, 0, 0, 0);
> + else
> + ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> + return true;
> +
> +out_continue:
> + spin_unlock(&ctx->guest_vers_lock);
> +
> + return false;
> }
>
> static void handle_features(struct cpu_user_regs *regs)
> @@ -274,10 +321,17 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
> if ( !ctx )
> return false;
>
> + /* A version must be negotiated first */
> + if ( !ACCESS_ONCE(ctx->guest_vers) )
> + {
> + if ( ffa_negotiate_version(regs) )
> + return true;
> + }
> +
> switch ( fid )
> {
> case FFA_VERSION:
> - handle_version(regs);
> + ffa_set_regs(regs, FFA_MY_VERSION, 0, 0, 0, 0, 0, 0, 0);
> return true;
> case FFA_ID_GET:
> ffa_set_regs_success(regs, ffa_get_vm_id(d), 0);
> @@ -371,6 +425,11 @@ static int ffa_domain_init(struct domain *d)
> d->arch.tee = ctx;
> ctx->teardown_d = d;
> INIT_LIST_HEAD(&ctx->shm_list);
> + spin_lock_init(&ctx->lock);
> + spin_lock_init(&ctx->guest_vers_lock);
> + ctx->guest_vers = 0;
> + ctx->guest_vers_tmp = 0;
> + INIT_LIST_HEAD(&ctx->ctx_list);
>
> ctx->ffa_id = ffa_get_vm_id(d);
> ctx->num_vcpus = d->max_vcpus;
> @@ -452,7 +511,7 @@ static int ffa_domain_teardown(struct domain *d)
> if ( !ctx )
> return 0;
>
> - if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && ctx->guest_vers )
> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && ACCESS_ONCE(ctx->guest_vers) )
> {
> dec_ffa_vm_count();
> write_lock(&ffa_ctx_list_rwlock);
> diff --git a/xen/arch/arm/tee/ffa_msg.c b/xen/arch/arm/tee/ffa_msg.c
> index c20c5bec0f76..2c2ebc9c5cd6 100644
> --- a/xen/arch/arm/tee/ffa_msg.c
> +++ b/xen/arch/arm/tee/ffa_msg.c
> @@ -113,7 +113,7 @@ static int32_t ffa_msg_send2_vm(uint16_t dst_id, const void *src_buf,
> }
>
> dst_ctx = dst_d->arch.tee;
> - if ( !dst_ctx->guest_vers )
> + if ( !ACCESS_ONCE(dst_ctx->guest_vers) )
> {
> ret = FFA_RET_INVALID_PARAMETERS;
> goto out_unlock;
> diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
> index fa56b1587e3b..ec5a53ed1cab 100644
> --- a/xen/arch/arm/tee/ffa_partinfo.c
> +++ b/xen/arch/arm/tee/ffa_partinfo.c
> @@ -238,7 +238,7 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
> * use the v1.0 structure size in the destination buffer.
> * Otherwise use the size of the highest version we support, here 1.1.
> */
> - if ( ctx->guest_vers == FFA_VERSION_1_0 )
> + if ( ACCESS_ONCE(ctx->guest_vers) == FFA_VERSION_1_0 )
> dst_size = sizeof(struct ffa_partition_info_1_0);
> else
> dst_size = sizeof(struct ffa_partition_info_1_1);
> @@ -250,7 +250,7 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
> * 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 == FFA_VERSION_1_0 ||
> + if ( ACCESS_ONCE(ctx->guest_vers) == FFA_VERSION_1_0 ||
> flags != FFA_PARTITION_INFO_GET_COUNT_FLAG )
> {
> ret = FFA_RET_INVALID_PARAMETERS;
> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
> index 8d01da0009d3..4e4ac7fd7bc4 100644
> --- a/xen/arch/arm/tee/ffa_private.h
> +++ b/xen/arch/arm/tee/ffa_private.h
> @@ -355,12 +355,6 @@ struct ffa_ctx {
> * Global data accessed with lock locked.
> */
> spinlock_t lock;
> - /*
> - * FF-A version negotiated by the guest, only modifications to
> - * this field are done with the lock held as this is expected to
> - * be done once at init by a guest.
> - */
> - uint32_t guest_vers;
> /* Number of 4kB pages in each of rx/rx_pg and tx/tx_pg */
> unsigned int page_count;
> /* Number of allocated shared memory object */
> @@ -368,6 +362,26 @@ struct ffa_ctx {
> /* Used shared memory objects, struct ffa_shm_mem */
> struct list_head shm_list;
>
> + /*
> + * FF-A version handling
> + * guest_vers is the single published negotiated version. It is 0 until
> + * negotiation completes, after which it is set once and never changes.
> + * Negotiation uses guest_vers_tmp under guest_vers_lock; when a
> + * non-VERSION ABI is invoked, Xen finalizes negotiation by publishing
> + * guest_vers using ACCESS_ONCE() store.
> + * Readers use ACCESS_ONCE(guest_vers) != 0 to detect availability and
> + * can consume guest_vers without barriers because it never changes once
> + * published.
> + */
> + spinlock_t guest_vers_lock;
> + /*
> + * Published negotiated version. Zero means "not negotiated yet".
> + * Once non-zero, it never changes.
> + */
> + uint32_t guest_vers;
It might be worth mentioning that this field must always be accessed
using the ACCESS_ONCE() macro.
Cheers,
Jens
> + /* Temporary version used during negotiation under guest_vers_lock */
> + uint32_t guest_vers_tmp;
> +
> /*
> * Rx buffer, accessed with rx_lock locked.
> * rx_is_free is used to serialize access.
> diff --git a/xen/arch/arm/tee/ffa_shm.c b/xen/arch/arm/tee/ffa_shm.c
> index d628c1b70609..dad3da192247 100644
> --- a/xen/arch/arm/tee/ffa_shm.c
> +++ b/xen/arch/arm/tee/ffa_shm.c
> @@ -495,7 +495,8 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs)
> if ( frag_len > ctx->page_count * FFA_PAGE_SIZE )
> goto out_unlock;
>
> - ret = read_mem_transaction(ctx->guest_vers, ctx->tx, frag_len, &trans);
> + ret = read_mem_transaction(ACCESS_ONCE(ctx->guest_vers), ctx->tx,
> + frag_len, &trans);
> if ( ret )
> goto out_unlock;
>
> --
> 2.51.2
>
Hi Jens,
> On 9 Dec 2025, at 09:45, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> Hi Bertrand,
>
> On Fri, Dec 5, 2025 at 11:37 AM Bertrand Marquis
> <bertrand.marquis@arm.com> wrote:
>>
>> Track FF-A version negotiation per VM and enforce that no FF-A ABI
>> (other than FFA_VERSION) is processed before a guest has selected a
>> version.
>>
>> Each ffa_ctx gains a dedicated guest_vers_lock, a negotiated version
>> (guest_vers) and a guest_vers_tmp:
>> - guest_vers is the version negotiated or 0 if no version has been
>> negotiated. This must be used with ACCESS_ONCE when reading it without
>> the spinlock taken.
>> - guest_vers_tmp stores the version currently requested by a VM.
>>
>> The version requested is the one actually negotiated once a call
>> different from FFA_VERSION is done to allow several attempts and as
>> requested by FF-A specification.
>> We always return our implementation version FFA_MY_VERSION, even if the
>> version requested was different, as requested by FF-A specification.
>>
>> Any call other than FFA_VERSION is rejected until a version has been
>> requested.
>>
>> Update all places in the code where guest_vers is used to use
>> ACCESS_ONCE.
>>
>> This prevents partially initialised contexts from using the mediator
>> and complies with the FF-A 1.2 FFA_VERSION semantics.
>>
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> Changes in v1:
>> - remove the guest_vers_negotiated and use guest_vers = 0 as condition
>> for a version being negotiated instead
>> - introduce guest_vers_tmp to store a requested version until it is
>> becoming the one negotiated.
>> - remove not needed if negotiated condition.
>> - use ACCESS_ONCE when reading guest_vers and use guest_vers == 0 as
>> condition for a version being negotiated.
>> - Update FF-A version handling comment in ffa_private.h
>> ---
>> xen/arch/arm/tee/ffa.c | 101 +++++++++++++++++++++++++-------
>> xen/arch/arm/tee/ffa_msg.c | 2 +-
>> xen/arch/arm/tee/ffa_partinfo.c | 4 +-
>> xen/arch/arm/tee/ffa_private.h | 26 ++++++--
>> xen/arch/arm/tee/ffa_shm.c | 3 +-
>> 5 files changed, 105 insertions(+), 31 deletions(-)
>>
>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>> index 2b4e24750d52..aadd6c21e7f2 100644
>> --- a/xen/arch/arm/tee/ffa.c
>> +++ b/xen/arch/arm/tee/ffa.c
>> @@ -158,31 +158,38 @@ static bool ffa_abi_supported(uint32_t id)
>> return !ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
>> }
>>
>> -static void handle_version(struct cpu_user_regs *regs)
>> +static bool ffa_negotiate_version(struct cpu_user_regs *regs)
>> {
>> struct domain *d = current->domain;
>> struct ffa_ctx *ctx = d->arch.tee;
>> - uint32_t vers = get_user_reg(regs, 1);
>> - uint32_t old_vers;
>> + uint32_t fid = get_user_reg(regs, 0);
>> + uint32_t in_vers = get_user_reg(regs, 1);
>> + uint32_t out_vers = FFA_MY_VERSION;
>>
>> - /*
>> - * Guest will use the version it requested if it is our major and minor
>> - * lower or equals to ours. If the minor is greater, our version will be
>> - * used.
>> - * In any case return our version to the caller.
>> - */
>> - if ( FFA_VERSION_MAJOR(vers) == FFA_MY_VERSION_MAJOR )
>> + spin_lock(&ctx->guest_vers_lock);
>> +
>> + /* If negotiation already published, continue without handling. */
>> + if ( ACCESS_ONCE(ctx->guest_vers) )
>> + goto out_continue;
>> +
>> + if ( fid != FFA_VERSION )
>> {
>> - spin_lock(&ctx->lock);
>> - old_vers = ctx->guest_vers;
>> + if ( !ctx->guest_vers_tmp )
>> + {
>> + out_vers = 0;
>> + goto out_handled;
>> + }
>>
>> - if ( FFA_VERSION_MINOR(vers) > FFA_MY_VERSION_MINOR )
>> - ctx->guest_vers = FFA_MY_VERSION;
>> - else
>> - ctx->guest_vers = vers;
>> - spin_unlock(&ctx->lock);
>> + /*
>> + * A successful FFA_VERSION call does not freeze negotiation. Guests
>> + * are allowed to issue multiple FFA_VERSION attempts (e.g. probing
>> + * several minor versions). Negotiation becomes final only when a
>> + * non-VERSION ABI is invoked, as required by the FF-A specification.
>> + * Finalize negotiation: publish guest_vers once, then never change.
>> + */
>> + ACCESS_ONCE(ctx->guest_vers) = ctx->guest_vers_tmp;
>>
>> - if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !old_vers )
>> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
>> {
>> /* One more VM with FF-A support available */
>> inc_ffa_vm_count();
>> @@ -190,8 +197,48 @@ static void handle_version(struct cpu_user_regs *regs)
>> list_add_tail(&ctx->ctx_list, &ffa_ctx_head);
>> write_unlock(&ffa_ctx_list_rwlock);
>> }
>> +
>> + goto out_continue;
>> }
>> - ffa_set_regs(regs, FFA_MY_VERSION, 0, 0, 0, 0, 0, 0, 0);
>> +
>> + /*
>> + * guest_vers_tmp stores the version selected by the guest (lower minor may
>> + * require reduced data structures). However, the value returned to the
>> + * guest via FFA_VERSION is always FFA_MY_VERSION, the implementation
>> + * version, as required by FF-A. The two values intentionally differ.
>> + */
>> +
>> + /*
>> + * Return our highest implementation version on request different than our
>> + * major and mark negotiated version as our implementation version.
>> + */
>> + if ( FFA_VERSION_MAJOR(in_vers) != FFA_MY_VERSION_MAJOR )
>> + {
>> + ctx->guest_vers_tmp = FFA_MY_VERSION;
>> + goto out_handled;
>> + }
>> +
>> + /*
>> + * Use our minor version if a greater minor was requested or the requested
>> + * minor if it is lower than ours was requested.
>> + */
>> + if ( FFA_VERSION_MINOR(in_vers) > FFA_MY_VERSION_MINOR )
>> + ctx->guest_vers_tmp = FFA_MY_VERSION;
>> + else
>> + ctx->guest_vers_tmp = in_vers;
>> +
>> +out_handled:
>> + spin_unlock(&ctx->guest_vers_lock);
>> + if ( out_vers )
>> + ffa_set_regs(regs, out_vers, 0, 0, 0, 0, 0, 0, 0);
>> + else
>> + ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>> + return true;
>> +
>> +out_continue:
>> + spin_unlock(&ctx->guest_vers_lock);
>> +
>> + return false;
>> }
>>
>> static void handle_features(struct cpu_user_regs *regs)
>> @@ -274,10 +321,17 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
>> if ( !ctx )
>> return false;
>>
>> + /* A version must be negotiated first */
>> + if ( !ACCESS_ONCE(ctx->guest_vers) )
>> + {
>> + if ( ffa_negotiate_version(regs) )
>> + return true;
>> + }
>> +
>> switch ( fid )
>> {
>> case FFA_VERSION:
>> - handle_version(regs);
>> + ffa_set_regs(regs, FFA_MY_VERSION, 0, 0, 0, 0, 0, 0, 0);
>> return true;
>> case FFA_ID_GET:
>> ffa_set_regs_success(regs, ffa_get_vm_id(d), 0);
>> @@ -371,6 +425,11 @@ static int ffa_domain_init(struct domain *d)
>> d->arch.tee = ctx;
>> ctx->teardown_d = d;
>> INIT_LIST_HEAD(&ctx->shm_list);
>> + spin_lock_init(&ctx->lock);
>> + spin_lock_init(&ctx->guest_vers_lock);
>> + ctx->guest_vers = 0;
>> + ctx->guest_vers_tmp = 0;
>> + INIT_LIST_HEAD(&ctx->ctx_list);
>>
>> ctx->ffa_id = ffa_get_vm_id(d);
>> ctx->num_vcpus = d->max_vcpus;
>> @@ -452,7 +511,7 @@ static int ffa_domain_teardown(struct domain *d)
>> if ( !ctx )
>> return 0;
>>
>> - if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && ctx->guest_vers )
>> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && ACCESS_ONCE(ctx->guest_vers) )
>> {
>> dec_ffa_vm_count();
>> write_lock(&ffa_ctx_list_rwlock);
>> diff --git a/xen/arch/arm/tee/ffa_msg.c b/xen/arch/arm/tee/ffa_msg.c
>> index c20c5bec0f76..2c2ebc9c5cd6 100644
>> --- a/xen/arch/arm/tee/ffa_msg.c
>> +++ b/xen/arch/arm/tee/ffa_msg.c
>> @@ -113,7 +113,7 @@ static int32_t ffa_msg_send2_vm(uint16_t dst_id, const void *src_buf,
>> }
>>
>> dst_ctx = dst_d->arch.tee;
>> - if ( !dst_ctx->guest_vers )
>> + if ( !ACCESS_ONCE(dst_ctx->guest_vers) )
>> {
>> ret = FFA_RET_INVALID_PARAMETERS;
>> goto out_unlock;
>> diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
>> index fa56b1587e3b..ec5a53ed1cab 100644
>> --- a/xen/arch/arm/tee/ffa_partinfo.c
>> +++ b/xen/arch/arm/tee/ffa_partinfo.c
>> @@ -238,7 +238,7 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
>> * use the v1.0 structure size in the destination buffer.
>> * Otherwise use the size of the highest version we support, here 1.1.
>> */
>> - if ( ctx->guest_vers == FFA_VERSION_1_0 )
>> + if ( ACCESS_ONCE(ctx->guest_vers) == FFA_VERSION_1_0 )
>> dst_size = sizeof(struct ffa_partition_info_1_0);
>> else
>> dst_size = sizeof(struct ffa_partition_info_1_1);
>> @@ -250,7 +250,7 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
>> * 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 == FFA_VERSION_1_0 ||
>> + if ( ACCESS_ONCE(ctx->guest_vers) == FFA_VERSION_1_0 ||
>> flags != FFA_PARTITION_INFO_GET_COUNT_FLAG )
>> {
>> ret = FFA_RET_INVALID_PARAMETERS;
>> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
>> index 8d01da0009d3..4e4ac7fd7bc4 100644
>> --- a/xen/arch/arm/tee/ffa_private.h
>> +++ b/xen/arch/arm/tee/ffa_private.h
>> @@ -355,12 +355,6 @@ struct ffa_ctx {
>> * Global data accessed with lock locked.
>> */
>> spinlock_t lock;
>> - /*
>> - * FF-A version negotiated by the guest, only modifications to
>> - * this field are done with the lock held as this is expected to
>> - * be done once at init by a guest.
>> - */
>> - uint32_t guest_vers;
>> /* Number of 4kB pages in each of rx/rx_pg and tx/tx_pg */
>> unsigned int page_count;
>> /* Number of allocated shared memory object */
>> @@ -368,6 +362,26 @@ struct ffa_ctx {
>> /* Used shared memory objects, struct ffa_shm_mem */
>> struct list_head shm_list;
>>
>> + /*
>> + * FF-A version handling
>> + * guest_vers is the single published negotiated version. It is 0 until
>> + * negotiation completes, after which it is set once and never changes.
>> + * Negotiation uses guest_vers_tmp under guest_vers_lock; when a
>> + * non-VERSION ABI is invoked, Xen finalizes negotiation by publishing
>> + * guest_vers using ACCESS_ONCE() store.
>> + * Readers use ACCESS_ONCE(guest_vers) != 0 to detect availability and
>> + * can consume guest_vers without barriers because it never changes once
>> + * published.
>> + */
>> + spinlock_t guest_vers_lock;
>> + /*
>> + * Published negotiated version. Zero means "not negotiated yet".
>> + * Once non-zero, it never changes.
>> + */
>> + uint32_t guest_vers;
>
> It might be worth mentioning that this field must always be accessed
> using the ACCESS_ONCE() macro.
yes you are right, the overall comment is only mentioning how to check using
ACCESS_ONCE if version is negotiated but is not very clear.
I will add a comment on top of guest_vers:
guest_vers must always be accessed using ACCESS_ONCE.
Cheers
Bertrand
>
> Cheers,
> Jens
>
>> + /* Temporary version used during negotiation under guest_vers_lock */
>> + uint32_t guest_vers_tmp;
>> +
>> /*
>> * Rx buffer, accessed with rx_lock locked.
>> * rx_is_free is used to serialize access.
>> diff --git a/xen/arch/arm/tee/ffa_shm.c b/xen/arch/arm/tee/ffa_shm.c
>> index d628c1b70609..dad3da192247 100644
>> --- a/xen/arch/arm/tee/ffa_shm.c
>> +++ b/xen/arch/arm/tee/ffa_shm.c
>> @@ -495,7 +495,8 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs)
>> if ( frag_len > ctx->page_count * FFA_PAGE_SIZE )
>> goto out_unlock;
>>
>> - ret = read_mem_transaction(ctx->guest_vers, ctx->tx, frag_len, &trans);
>> + ret = read_mem_transaction(ACCESS_ONCE(ctx->guest_vers), ctx->tx,
>> + frag_len, &trans);
>> if ( ret )
>> goto out_unlock;
>>
>> --
>> 2.51.2
>>
© 2016 - 2025 Red Hat, Inc.