Create a CONFIG_FFA_VM_TO_VM parameter to activate FFA communication
between VMs.
When activated list VMs in the system with FF-A support in part_info_get.
When VM to VM is activated, Xen will be tainted as Insecure and a
message is displayed to the user during the boot as there is no
filtering of VMs in FF-A so any VM can communicate or see any other VM
in the system.
WARNING: There is no filtering for now and all VMs are listed !!
This patch is reorganizing the ffa_ctx structure to make clear which
lock is protecting what parts.
This patch is introducing a chain list of the ffa_ctx with a FFA Version
negociated allowing to create the partinfo results for VMs in parallel
by using rwlock which only ensure addition/removal of entries are
protected.
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in v7:
- protect ffa_ctx list with a rw lock to allow several partinfo_get in
parallel but protect adding/removing entries.
Changes in v6:
- remove ACCESS_ONCE for guest_vers access and take the context lock
before modifying it
- move guest_vers in context declaration to fields protected by the
context lock and add a comment to state that lock in only needed when
modifying it
Changes in v5:
- remove invalid comment about 1.1 firmware support
- rename variables from d and dom to curr_d and dest_d (Julien)
- add a TODO in the code for potential holding for long of the CPU
(Julien)
- use an atomic global variable to store the number of VMs instead of
recomputing the value each time (Julien)
- add partinfo information in ffa_ctx (id, cpus and 64bit) and create a
chain list of ctx. Use this chain list to create the partinfo result
without holding a global lock to prevent concurrency issues.
- Move some changes in a preparation patch modifying partinfo for sps to
reduce this patch size and make the review easier
Changes in v4:
- properly handle SPMC version 1.0 header size case in partinfo_get
- switch to local counting variables instead of *pointer += 1 form
- coding style issue with missing spaces in if ()
Changes in v3:
- break partinfo_get in several sub functions to make the implementation
easier to understand and lock handling easier
- rework implementation to check size along the way and prevent previous
implementation limits which had to check that the number of VMs or SPs
did not change
- taint Xen as INSECURE when VM to VM is enabled
Changes in v2:
- Switch ifdef to IS_ENABLED
- dom was not switched to d as requested by Jan because there is already
a variable d pointing to the current domain and it must not be
shadowed.
---
xen/arch/arm/tee/Kconfig | 11 +++
xen/arch/arm/tee/ffa.c | 47 +++++++++++++
xen/arch/arm/tee/ffa_partinfo.c | 100 ++++++++++++++++++++++++---
xen/arch/arm/tee/ffa_private.h | 117 ++++++++++++++++++++++++++------
4 files changed, 245 insertions(+), 30 deletions(-)
diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig
index c5b0f88d7522..88a4c4c99154 100644
--- a/xen/arch/arm/tee/Kconfig
+++ b/xen/arch/arm/tee/Kconfig
@@ -28,5 +28,16 @@ config FFA
[1] https://developer.arm.com/documentation/den0077/latest
+config FFA_VM_TO_VM
+ bool "Enable FF-A between VMs (UNSUPPORTED)" if UNSUPPORTED
+ default n
+ depends on FFA
+ help
+ This option enables to use FF-A between VMs.
+ This is experimental and there is no access control so any
+ guest can communicate with any other guest.
+
+ If unsure, say N.
+
endmenu
diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
index 3bbdd7168a6b..be71eda4869f 100644
--- a/xen/arch/arm/tee/ffa.c
+++ b/xen/arch/arm/tee/ffa.c
@@ -118,6 +118,13 @@ void *ffa_tx __read_mostly;
DEFINE_SPINLOCK(ffa_rx_buffer_lock);
DEFINE_SPINLOCK(ffa_tx_buffer_lock);
+struct list_head ffa_ctx_head;
+/* RW Lock to protect addition/removal and reading in ffa_ctx_head */
+rwlock_t ffa_ctx_list_rwlock;
+
+#ifdef CONFIG_FFA_VM_TO_VM
+atomic_t ffa_vm_count;
+#endif
/* Used to track domains that could not be torn down immediately. */
static struct timer ffa_teardown_timer;
@@ -151,6 +158,7 @@ static void handle_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;
/*
* Guest will use the version it requested if it is our major and minor
@@ -160,10 +168,23 @@ static void handle_version(struct cpu_user_regs *regs)
*/
if ( FFA_VERSION_MAJOR(vers) == FFA_MY_VERSION_MAJOR )
{
+ spin_lock(&ctx->lock);
+ old_vers = ctx->guest_vers;
+
if ( FFA_VERSION_MINOR(vers) > FFA_MY_VERSION_MINOR )
ctx->guest_vers = FFA_MY_VERSION;
else
ctx->guest_vers = vers;
+ spin_unlock(&ctx->lock);
+
+ if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !old_vers )
+ {
+ /* One more VM with FF-A support available */
+ inc_ffa_vm_count();
+ write_lock(&ffa_ctx_list_rwlock);
+ list_add_tail(&ctx->ctx_list, &ffa_ctx_head);
+ write_unlock(&ffa_ctx_list_rwlock);
+ }
}
ffa_set_regs(regs, FFA_MY_VERSION, 0, 0, 0, 0, 0, 0, 0);
}
@@ -345,6 +366,10 @@ static int ffa_domain_init(struct domain *d)
ctx->teardown_d = d;
INIT_LIST_HEAD(&ctx->shm_list);
+ ctx->ffa_id = ffa_get_vm_id(d);
+ ctx->num_vcpus = d->max_vcpus;
+ ctx->is_64bit = is_64bit_domain(d);
+
/*
* ffa_domain_teardown() will be called if ffa_domain_init() returns an
* error, so no need for cleanup in this function.
@@ -421,6 +446,14 @@ static int ffa_domain_teardown(struct domain *d)
if ( !ctx )
return 0;
+ if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && ctx->guest_vers )
+ {
+ dec_ffa_vm_count();
+ write_lock(&ffa_ctx_list_rwlock);
+ list_del(&ctx->ctx_list);
+ write_unlock(&ffa_ctx_list_rwlock);
+ }
+
ffa_rxtx_domain_destroy(d);
ffa_notif_domain_destroy(d);
@@ -464,6 +497,18 @@ static bool ffa_probe(void)
printk(XENLOG_INFO "ARM FF-A Mediator version %u.%u\n",
FFA_MY_VERSION_MAJOR, FFA_MY_VERSION_MINOR);
+ if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
+ {
+ /*
+ * When FFA VM to VM is enabled, the current implementation does not
+ * offer any way to limit which VM can communicate with which VM using
+ * FF-A.
+ * Signal this in the xen console and taint the system as insecure.
+ * TODO: Introduce a solution to limit what a VM can do through FFA.
+ */
+ printk(XENLOG_ERR "ffa: VM to VM is enabled, system is insecure !!\n");
+ add_taint(TAINT_MACHINE_INSECURE);
+ }
/*
* psci_init_smccc() updates this value with what's reported by EL-3
* or secure world.
@@ -538,6 +583,8 @@ static bool ffa_probe(void)
ffa_notif_init();
INIT_LIST_HEAD(&ffa_teardown_head);
+ INIT_LIST_HEAD(&ffa_ctx_head);
+ rwlock_init(&ffa_ctx_list_rwlock);
init_timer(&ffa_teardown_timer, ffa_teardown_timer_callback, NULL, 0);
return true;
diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
index dfa0b23eaf38..fa56b1587e3b 100644
--- a/xen/arch/arm/tee/ffa_partinfo.c
+++ b/xen/arch/arm/tee/ffa_partinfo.c
@@ -150,6 +150,73 @@ out:
return ret;
}
+static int32_t ffa_get_vm_partinfo(uint32_t *vm_count, void *dst_buf,
+ void *end_buf, uint32_t dst_size)
+{
+ struct ffa_ctx *curr_ctx = current->domain->arch.tee;
+ struct ffa_ctx *dest_ctx;
+ uint32_t count = 0;
+ int32_t ret = FFA_RET_OK;
+
+ /*
+ * There could potentially be a lot of VMs in the system and we could
+ * hold the CPU for long here.
+ * Right now there is no solution in FF-A specification to split
+ * the work in this case.
+ * TODO: Check how we could delay the work or have preemption checks.
+ */
+ read_lock(&ffa_ctx_list_rwlock);
+ list_for_each_entry(dest_ctx, &ffa_ctx_head, ctx_list)
+ {
+ /*
+ * Do not include an entry for the caller VM as the spec is not
+ * clearly mandating it and it is not supported by Linux.
+ */
+ if ( dest_ctx != curr_ctx )
+ {
+ /*
+ * We do not have UUID info for VMs so use
+ * the 1.0 structure so that we set UUIDs to
+ * zero using memset
+ */
+ struct ffa_partition_info_1_0 info;
+
+ if ( dst_buf > (end_buf - dst_size) )
+ {
+ ret = FFA_RET_NO_MEMORY;
+ goto out;
+ }
+
+ /*
+ * Context might has been removed since we go it or being removed
+ * right now so we might return information for a VM not existing
+ * anymore. This is acceptable as we return a view of the system
+ * which could change at any time.
+ */
+ info.id = dest_ctx->ffa_id;
+ info.execution_context = dest_ctx->num_vcpus;
+ info.partition_properties = FFA_PART_VM_PROP;
+ if ( dest_ctx->is_64bit )
+ info.partition_properties |= FFA_PART_PROP_AARCH64_STATE;
+
+ memcpy(dst_buf, &info, MIN(sizeof(info), dst_size));
+
+ if ( dst_size > sizeof(info) )
+ memset(dst_buf + sizeof(info), 0,
+ dst_size - sizeof(info));
+
+ dst_buf += dst_size;
+ count++;
+ }
+ }
+ *vm_count = count;
+
+out:
+ read_unlock(&ffa_ctx_list_rwlock);
+
+ return ret;
+}
+
void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
{
int32_t ret = FFA_RET_OK;
@@ -164,7 +231,7 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
};
uint32_t dst_size = 0;
void *dst_buf, *end_buf;
- uint32_t ffa_sp_count = 0;
+ uint32_t ffa_vm_count = 0, ffa_sp_count = 0;
/*
* If the guest is v1.0, he does not get back the entry size so we must
@@ -191,15 +258,18 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
}
if ( ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
+ {
ret = ffa_get_sp_count(uuid, &ffa_sp_count);
+ if ( ret )
+ goto out;
+ }
- goto out;
- }
+ /*
+ * Do not count the caller VM as the spec is not clearly mandating it
+ * and it is not supported by Linux.
+ */
+ ffa_vm_count = get_ffa_vm_count() - 1;
- if ( !ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
- {
- /* Just give an empty partition list to the caller */
- ret = FFA_RET_OK;
goto out;
}
@@ -224,9 +294,19 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
goto out_rx_release;
}
- ret = ffa_get_sp_partinfo(uuid, &ffa_sp_count, dst_buf, end_buf,
- dst_size);
+ if ( ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
+ {
+ ret = ffa_get_sp_partinfo(uuid, &ffa_sp_count, dst_buf, end_buf,
+ dst_size);
+
+ if ( ret )
+ goto out_rx_release;
+
+ dst_buf += ffa_sp_count * dst_size;
+ }
+ if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
+ ret = ffa_get_vm_partinfo(&ffa_vm_count, dst_buf, end_buf, dst_size);
out_rx_release:
if ( ret )
@@ -235,7 +315,7 @@ out:
if ( ret )
ffa_set_regs_error(regs, ret);
else
- ffa_set_regs_success(regs, ffa_sp_count, dst_size);
+ ffa_set_regs_success(regs, ffa_sp_count + ffa_vm_count, dst_size);
}
static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
index 0a9c1082db28..1a1dcabcdc28 100644
--- a/xen/arch/arm/tee/ffa_private.h
+++ b/xen/arch/arm/tee/ffa_private.h
@@ -195,6 +195,18 @@
*/
#define FFA_PARTITION_INFO_GET_COUNT_FLAG BIT(0, U)
+/*
+ * Partition properties we give for a normal world VM:
+ * - can send direct message but not receive them
+ * - can handle indirect messages
+ * - can receive notifications
+ * 32/64 bit flag is set depending on the VM
+ */
+#define FFA_PART_VM_PROP (FFA_PART_PROP_DIRECT_REQ_SEND | \
+ FFA_PART_PROP_INDIRECT_MSGS | \
+ FFA_PART_PROP_RECV_NOTIF | \
+ FFA_PART_PROP_IS_PE_ID)
+
/* Flags used in calls to FFA_NOTIFICATION_GET interface */
#define FFA_NOTIF_FLAG_BITMAP_SP BIT(0, U)
#define FFA_NOTIF_FLAG_BITMAP_VM BIT(1, U)
@@ -297,36 +309,72 @@ struct ffa_ctx_notif {
};
struct ffa_ctx {
- void *rx;
- const void *tx;
- struct page_info *rx_pg;
- struct page_info *tx_pg;
+ /*
+ * Chain list of all FF-A contexts.
+ * As we might have several read from the list of context through parallel
+ * partinfo_get but fewer additions/removal as those happen only during a
+ * version negociation or guest shutdown, access to this list is protected
+ * through a rwlock (addition/removal with write lock, reading through a
+ * read lock).
+ */
+ struct list_head ctx_list; /* chain list of all FF-A contexts */
+
+ /*
+ * Data access unlocked (mainly for part_info_get in VM to VM).
+ * Those should be set before the ctx is added in the list.
+ */
+ /* FF-A Endpoint ID */
+ uint16_t ffa_id;
+ uint16_t num_vcpus;
+ bool is_64bit;
+
+ /*
+ * Global data accessed atomically or using ACCES_ONCE.
+ */
+ struct ffa_ctx_notif notif;
+
+ /*
+ * Global data accessed with lock locked.
+ */
+ spinlock_t lock;
+ /*
+ * FF-A version negociated 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;
- /* FF-A version used by the guest */
- uint32_t guest_vers;
- bool rx_is_free;
- /* Used shared memory objects, struct ffa_shm_mem */
- struct list_head shm_list;
/* Number of allocated shared memory object */
unsigned int shm_count;
- struct ffa_ctx_notif notif;
+ /* Used shared memory objects, struct ffa_shm_mem */
+ struct list_head shm_list;
+
/*
- * tx_lock is used to serialize access to tx
- * rx_lock is used to serialize access to rx_is_free
- * lock is used for the rest in this struct
+ * Rx buffer, accessed with rx_lock locked.
+ * rx_is_free is used to serialize access.
*/
- spinlock_t tx_lock;
spinlock_t rx_lock;
- spinlock_t lock;
- /* Used if domain can't be torn down immediately */
+ bool rx_is_free;
+ void *rx;
+ struct page_info *rx_pg;
+
+ /*
+ * Tx buffer, access with tx_lock locked.
+ */
+ spinlock_t tx_lock;
+ const void *tx;
+ struct page_info *tx_pg;
+
+
+ /*
+ * Domain teardown handling if data shared or used by other domains
+ * do not allow to teardown the domain immediately.
+ */
struct domain *teardown_d;
struct list_head teardown_list;
s_time_t teardown_expire;
- /*
- * Used for ffa_domain_teardown() to keep track of which SPs should be
- * notified that this guest is being destroyed.
- */
+ /* Keep track of SPs that should be notified of VM destruction */
unsigned long *vm_destroy_bitmap;
};
@@ -336,6 +384,12 @@ extern spinlock_t ffa_rx_buffer_lock;
extern spinlock_t ffa_tx_buffer_lock;
extern DECLARE_BITMAP(ffa_fw_abi_supported, FFA_ABI_BITMAP_SIZE);
+extern struct list_head ffa_ctx_head;
+extern rwlock_t ffa_ctx_list_rwlock;
+#ifdef CONFIG_FFA_VM_TO_VM
+extern atomic_t ffa_vm_count;
+#endif
+
bool ffa_shm_domain_destroy(struct domain *d);
void ffa_handle_mem_share(struct cpu_user_regs *regs);
int ffa_handle_mem_reclaim(uint64_t handle, uint32_t flags);
@@ -368,6 +422,29 @@ int ffa_handle_notification_set(struct cpu_user_regs *regs);
void ffa_handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t fid);
int32_t ffa_handle_msg_send2(struct cpu_user_regs *regs);
+#ifdef CONFIG_FFA_VM_TO_VM
+static inline uint16_t get_ffa_vm_count(void)
+{
+ return atomic_read(&ffa_vm_count);
+}
+
+static inline void inc_ffa_vm_count(void)
+{
+ atomic_inc(&ffa_vm_count);
+}
+
+static inline void dec_ffa_vm_count(void)
+{
+ ASSERT(atomic_read(&ffa_vm_count) > 0);
+ atomic_dec(&ffa_vm_count);
+}
+#else
+/* Only count the caller VM */
+#define get_ffa_vm_count() ((uint16_t)1UL)
+#define inc_ffa_vm_count() do {} while(0)
+#define dec_ffa_vm_count() do {} while(0)
+#endif
+
static inline uint16_t ffa_get_vm_id(const struct domain *d)
{
/* +1 since 0 is reserved for the hypervisor in FF-A */
--
2.47.1Hi Bertrand, On 17/07/2025 13:11, Bertrand Marquis wrote: > Create a CONFIG_FFA_VM_TO_VM parameter to activate FFA communication > between VMs. > When activated list VMs in the system with FF-A support in part_info_get. > > When VM to VM is activated, Xen will be tainted as Insecure and a > message is displayed to the user during the boot as there is no > filtering of VMs in FF-A so any VM can communicate or see any other VM > in the system. > > WARNING: There is no filtering for now and all VMs are listed !! > > This patch is reorganizing the ffa_ctx structure to make clear which > lock is protecting what parts. > > This patch is introducing a chain list of the ffa_ctx with a FFA Version > negociated allowing to create the partinfo results for VMs in parallel > by using rwlock which only ensure addition/removal of entries are > protected. > > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> With two remarks below: Acked-by: Julien Grall <jgrall@amazon.com> > --- > Changes in v7: > - protect ffa_ctx list with a rw lock to allow several partinfo_get in > parallel but protect adding/removing entries. > Changes in v6: > - remove ACCESS_ONCE for guest_vers access and take the context lock > before modifying it > - move guest_vers in context declaration to fields protected by the > context lock and add a comment to state that lock in only needed when > modifying it > Changes in v5: > - remove invalid comment about 1.1 firmware support > - rename variables from d and dom to curr_d and dest_d (Julien) > - add a TODO in the code for potential holding for long of the CPU > (Julien) > - use an atomic global variable to store the number of VMs instead of > recomputing the value each time (Julien) > - add partinfo information in ffa_ctx (id, cpus and 64bit) and create a > chain list of ctx. Use this chain list to create the partinfo result > without holding a global lock to prevent concurrency issues. > - Move some changes in a preparation patch modifying partinfo for sps to > reduce this patch size and make the review easier > Changes in v4: > - properly handle SPMC version 1.0 header size case in partinfo_get > - switch to local counting variables instead of *pointer += 1 form > - coding style issue with missing spaces in if () > Changes in v3: > - break partinfo_get in several sub functions to make the implementation > easier to understand and lock handling easier > - rework implementation to check size along the way and prevent previous > implementation limits which had to check that the number of VMs or SPs > did not change > - taint Xen as INSECURE when VM to VM is enabled > Changes in v2: > - Switch ifdef to IS_ENABLED > - dom was not switched to d as requested by Jan because there is already > a variable d pointing to the current domain and it must not be > shadowed. > --- > xen/arch/arm/tee/Kconfig | 11 +++ > xen/arch/arm/tee/ffa.c | 47 +++++++++++++ > xen/arch/arm/tee/ffa_partinfo.c | 100 ++++++++++++++++++++++++--- > xen/arch/arm/tee/ffa_private.h | 117 ++++++++++++++++++++++++++------ > 4 files changed, 245 insertions(+), 30 deletions(-) > > diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig > index c5b0f88d7522..88a4c4c99154 100644 > --- a/xen/arch/arm/tee/Kconfig > +++ b/xen/arch/arm/tee/Kconfig > @@ -28,5 +28,16 @@ config FFA > > [1] https://developer.arm.com/documentation/den0077/latest > > +config FFA_VM_TO_VM > + bool "Enable FF-A between VMs (UNSUPPORTED)" if UNSUPPORTED > + default n > + depends on FFA > + help > + This option enables to use FF-A between VMs. > + This is experimental and there is no access control so any > + guest can communicate with any other guest. > + > + If unsure, say N. > + > endmenu > > diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c > index 3bbdd7168a6b..be71eda4869f 100644 > --- a/xen/arch/arm/tee/ffa.c > +++ b/xen/arch/arm/tee/ffa.c > @@ -118,6 +118,13 @@ void *ffa_tx __read_mostly; > DEFINE_SPINLOCK(ffa_rx_buffer_lock); > DEFINE_SPINLOCK(ffa_tx_buffer_lock); > > +struct list_head ffa_ctx_head; A more common pattern is to use LIST_HEAD(ffa_ctx_head) and ... > +/* RW Lock to protect addition/removal and reading in ffa_ctx_head */ > +rwlock_t ffa_ctx_list_rwlock; ... DEFINE_RWLOCK(ffa_ctx_list_rwlock) which will also initialize list/rwlock for you. So no need for extra code further down and less risk if the variables are used before they are initialized. Cheers, -- Julien Grall
Hi Julien, > On 8 Aug 2025, at 18:53, Julien Grall <julien@xen.org> wrote: > > Hi Bertrand, > > On 17/07/2025 13:11, Bertrand Marquis wrote: >> Create a CONFIG_FFA_VM_TO_VM parameter to activate FFA communication >> between VMs. >> When activated list VMs in the system with FF-A support in part_info_get. >> When VM to VM is activated, Xen will be tainted as Insecure and a >> message is displayed to the user during the boot as there is no >> filtering of VMs in FF-A so any VM can communicate or see any other VM >> in the system. >> WARNING: There is no filtering for now and all VMs are listed !! >> This patch is reorganizing the ffa_ctx structure to make clear which >> lock is protecting what parts. >> This patch is introducing a chain list of the ffa_ctx with a FFA Version >> negociated allowing to create the partinfo results for VMs in parallel >> by using rwlock which only ensure addition/removal of entries are >> protected. >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> > > With two remarks below: > > Acked-by: Julien Grall <jgrall@amazon.com> > >> --- >> Changes in v7: >> - protect ffa_ctx list with a rw lock to allow several partinfo_get in >> parallel but protect adding/removing entries. >> Changes in v6: >> - remove ACCESS_ONCE for guest_vers access and take the context lock >> before modifying it >> - move guest_vers in context declaration to fields protected by the >> context lock and add a comment to state that lock in only needed when >> modifying it >> Changes in v5: >> - remove invalid comment about 1.1 firmware support >> - rename variables from d and dom to curr_d and dest_d (Julien) >> - add a TODO in the code for potential holding for long of the CPU >> (Julien) >> - use an atomic global variable to store the number of VMs instead of >> recomputing the value each time (Julien) >> - add partinfo information in ffa_ctx (id, cpus and 64bit) and create a >> chain list of ctx. Use this chain list to create the partinfo result >> without holding a global lock to prevent concurrency issues. >> - Move some changes in a preparation patch modifying partinfo for sps to >> reduce this patch size and make the review easier >> Changes in v4: >> - properly handle SPMC version 1.0 header size case in partinfo_get >> - switch to local counting variables instead of *pointer += 1 form >> - coding style issue with missing spaces in if () >> Changes in v3: >> - break partinfo_get in several sub functions to make the implementation >> easier to understand and lock handling easier >> - rework implementation to check size along the way and prevent previous >> implementation limits which had to check that the number of VMs or SPs >> did not change >> - taint Xen as INSECURE when VM to VM is enabled >> Changes in v2: >> - Switch ifdef to IS_ENABLED >> - dom was not switched to d as requested by Jan because there is already >> a variable d pointing to the current domain and it must not be >> shadowed. >> --- >> xen/arch/arm/tee/Kconfig | 11 +++ >> xen/arch/arm/tee/ffa.c | 47 +++++++++++++ >> xen/arch/arm/tee/ffa_partinfo.c | 100 ++++++++++++++++++++++++--- >> xen/arch/arm/tee/ffa_private.h | 117 ++++++++++++++++++++++++++------ >> 4 files changed, 245 insertions(+), 30 deletions(-) >> diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig >> index c5b0f88d7522..88a4c4c99154 100644 >> --- a/xen/arch/arm/tee/Kconfig >> +++ b/xen/arch/arm/tee/Kconfig >> @@ -28,5 +28,16 @@ config FFA >> [1] https://developer.arm.com/documentation/den0077/latest >> +config FFA_VM_TO_VM >> + bool "Enable FF-A between VMs (UNSUPPORTED)" if UNSUPPORTED >> + default n >> + depends on FFA >> + help >> + This option enables to use FF-A between VMs. >> + This is experimental and there is no access control so any >> + guest can communicate with any other guest. >> + >> + If unsure, say N. >> + >> endmenu >> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c >> index 3bbdd7168a6b..be71eda4869f 100644 >> --- a/xen/arch/arm/tee/ffa.c >> +++ b/xen/arch/arm/tee/ffa.c >> @@ -118,6 +118,13 @@ void *ffa_tx __read_mostly; >> DEFINE_SPINLOCK(ffa_rx_buffer_lock); >> DEFINE_SPINLOCK(ffa_tx_buffer_lock); >> +struct list_head ffa_ctx_head; > > A more common pattern is to use LIST_HEAD(ffa_ctx_head) and ... > >> +/* RW Lock to protect addition/removal and reading in ffa_ctx_head */ >> +rwlock_t ffa_ctx_list_rwlock; > > ... DEFINE_RWLOCK(ffa_ctx_list_rwlock) which will also initialize list/rwlock for you. So no need for extra code further down and less risk if the variables are used before they are initialized. I will modify those and push a v7 to the mailing list today. Thanks again. Bertrand > > Cheers, > > -- > Julien Grall
Hi Bertrand,
On Thu, Jul 17, 2025 at 1:11 PM Bertrand Marquis
<bertrand.marquis@arm.com> wrote:
>
> Create a CONFIG_FFA_VM_TO_VM parameter to activate FFA communication
> between VMs.
> When activated list VMs in the system with FF-A support in part_info_get.
>
> When VM to VM is activated, Xen will be tainted as Insecure and a
> message is displayed to the user during the boot as there is no
> filtering of VMs in FF-A so any VM can communicate or see any other VM
> in the system.
>
> WARNING: There is no filtering for now and all VMs are listed !!
>
> This patch is reorganizing the ffa_ctx structure to make clear which
> lock is protecting what parts.
>
> This patch is introducing a chain list of the ffa_ctx with a FFA Version
> negociated allowing to create the partinfo results for VMs in parallel
negotiated
> by using rwlock which only ensure addition/removal of entries are
> protected.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in v7:
> - protect ffa_ctx list with a rw lock to allow several partinfo_get in
> parallel but protect adding/removing entries.
> Changes in v6:
> - remove ACCESS_ONCE for guest_vers access and take the context lock
> before modifying it
> - move guest_vers in context declaration to fields protected by the
> context lock and add a comment to state that lock in only needed when
> modifying it
> Changes in v5:
> - remove invalid comment about 1.1 firmware support
> - rename variables from d and dom to curr_d and dest_d (Julien)
> - add a TODO in the code for potential holding for long of the CPU
> (Julien)
> - use an atomic global variable to store the number of VMs instead of
> recomputing the value each time (Julien)
> - add partinfo information in ffa_ctx (id, cpus and 64bit) and create a
> chain list of ctx. Use this chain list to create the partinfo result
> without holding a global lock to prevent concurrency issues.
> - Move some changes in a preparation patch modifying partinfo for sps to
> reduce this patch size and make the review easier
> Changes in v4:
> - properly handle SPMC version 1.0 header size case in partinfo_get
> - switch to local counting variables instead of *pointer += 1 form
> - coding style issue with missing spaces in if ()
> Changes in v3:
> - break partinfo_get in several sub functions to make the implementation
> easier to understand and lock handling easier
> - rework implementation to check size along the way and prevent previous
> implementation limits which had to check that the number of VMs or SPs
> did not change
> - taint Xen as INSECURE when VM to VM is enabled
> Changes in v2:
> - Switch ifdef to IS_ENABLED
> - dom was not switched to d as requested by Jan because there is already
> a variable d pointing to the current domain and it must not be
> shadowed.
> ---
> xen/arch/arm/tee/Kconfig | 11 +++
> xen/arch/arm/tee/ffa.c | 47 +++++++++++++
> xen/arch/arm/tee/ffa_partinfo.c | 100 ++++++++++++++++++++++++---
> xen/arch/arm/tee/ffa_private.h | 117 ++++++++++++++++++++++++++------
> 4 files changed, 245 insertions(+), 30 deletions(-)
>
> diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig
> index c5b0f88d7522..88a4c4c99154 100644
> --- a/xen/arch/arm/tee/Kconfig
> +++ b/xen/arch/arm/tee/Kconfig
> @@ -28,5 +28,16 @@ config FFA
>
> [1] https://developer.arm.com/documentation/den0077/latest
>
> +config FFA_VM_TO_VM
> + bool "Enable FF-A between VMs (UNSUPPORTED)" if UNSUPPORTED
> + default n
> + depends on FFA
> + help
> + This option enables to use FF-A between VMs.
> + This is experimental and there is no access control so any
> + guest can communicate with any other guest.
> +
> + If unsure, say N.
> +
> endmenu
>
> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> index 3bbdd7168a6b..be71eda4869f 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -118,6 +118,13 @@ void *ffa_tx __read_mostly;
> DEFINE_SPINLOCK(ffa_rx_buffer_lock);
> DEFINE_SPINLOCK(ffa_tx_buffer_lock);
>
> +struct list_head ffa_ctx_head;
> +/* RW Lock to protect addition/removal and reading in ffa_ctx_head */
> +rwlock_t ffa_ctx_list_rwlock;
> +
> +#ifdef CONFIG_FFA_VM_TO_VM
> +atomic_t ffa_vm_count;
> +#endif
>
> /* Used to track domains that could not be torn down immediately. */
> static struct timer ffa_teardown_timer;
> @@ -151,6 +158,7 @@ static void handle_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;
>
> /*
> * Guest will use the version it requested if it is our major and minor
> @@ -160,10 +168,23 @@ static void handle_version(struct cpu_user_regs *regs)
> */
> if ( FFA_VERSION_MAJOR(vers) == FFA_MY_VERSION_MAJOR )
> {
> + spin_lock(&ctx->lock);
> + old_vers = ctx->guest_vers;
> +
> if ( FFA_VERSION_MINOR(vers) > FFA_MY_VERSION_MINOR )
> ctx->guest_vers = FFA_MY_VERSION;
> else
> ctx->guest_vers = vers;
> + spin_unlock(&ctx->lock);
> +
> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !old_vers )
> + {
> + /* One more VM with FF-A support available */
> + inc_ffa_vm_count();
> + write_lock(&ffa_ctx_list_rwlock);
> + list_add_tail(&ctx->ctx_list, &ffa_ctx_head);
> + write_unlock(&ffa_ctx_list_rwlock);
> + }
> }
> ffa_set_regs(regs, FFA_MY_VERSION, 0, 0, 0, 0, 0, 0, 0);
> }
> @@ -345,6 +366,10 @@ static int ffa_domain_init(struct domain *d)
> ctx->teardown_d = d;
> INIT_LIST_HEAD(&ctx->shm_list);
>
> + ctx->ffa_id = ffa_get_vm_id(d);
> + ctx->num_vcpus = d->max_vcpus;
> + ctx->is_64bit = is_64bit_domain(d);
> +
> /*
> * ffa_domain_teardown() will be called if ffa_domain_init() returns an
> * error, so no need for cleanup in this function.
> @@ -421,6 +446,14 @@ static int ffa_domain_teardown(struct domain *d)
> if ( !ctx )
> return 0;
>
> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && ctx->guest_vers )
> + {
> + dec_ffa_vm_count();
> + write_lock(&ffa_ctx_list_rwlock);
> + list_del(&ctx->ctx_list);
> + write_unlock(&ffa_ctx_list_rwlock);
> + }
> +
> ffa_rxtx_domain_destroy(d);
> ffa_notif_domain_destroy(d);
>
> @@ -464,6 +497,18 @@ static bool ffa_probe(void)
> printk(XENLOG_INFO "ARM FF-A Mediator version %u.%u\n",
> FFA_MY_VERSION_MAJOR, FFA_MY_VERSION_MINOR);
>
> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
> + {
> + /*
> + * When FFA VM to VM is enabled, the current implementation does not
> + * offer any way to limit which VM can communicate with which VM using
> + * FF-A.
> + * Signal this in the xen console and taint the system as insecure.
> + * TODO: Introduce a solution to limit what a VM can do through FFA.
> + */
> + printk(XENLOG_ERR "ffa: VM to VM is enabled, system is insecure !!\n");
> + add_taint(TAINT_MACHINE_INSECURE);
> + }
> /*
> * psci_init_smccc() updates this value with what's reported by EL-3
> * or secure world.
> @@ -538,6 +583,8 @@ static bool ffa_probe(void)
>
> ffa_notif_init();
> INIT_LIST_HEAD(&ffa_teardown_head);
> + INIT_LIST_HEAD(&ffa_ctx_head);
> + rwlock_init(&ffa_ctx_list_rwlock);
> init_timer(&ffa_teardown_timer, ffa_teardown_timer_callback, NULL, 0);
>
> return true;
> diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
> index dfa0b23eaf38..fa56b1587e3b 100644
> --- a/xen/arch/arm/tee/ffa_partinfo.c
> +++ b/xen/arch/arm/tee/ffa_partinfo.c
> @@ -150,6 +150,73 @@ out:
> return ret;
> }
>
> +static int32_t ffa_get_vm_partinfo(uint32_t *vm_count, void *dst_buf,
> + void *end_buf, uint32_t dst_size)
> +{
> + struct ffa_ctx *curr_ctx = current->domain->arch.tee;
> + struct ffa_ctx *dest_ctx;
> + uint32_t count = 0;
> + int32_t ret = FFA_RET_OK;
> +
> + /*
> + * There could potentially be a lot of VMs in the system and we could
> + * hold the CPU for long here.
> + * Right now there is no solution in FF-A specification to split
> + * the work in this case.
> + * TODO: Check how we could delay the work or have preemption checks.
> + */
> + read_lock(&ffa_ctx_list_rwlock);
> + list_for_each_entry(dest_ctx, &ffa_ctx_head, ctx_list)
> + {
> + /*
> + * Do not include an entry for the caller VM as the spec is not
> + * clearly mandating it and it is not supported by Linux.
> + */
> + if ( dest_ctx != curr_ctx )
> + {
> + /*
> + * We do not have UUID info for VMs so use
> + * the 1.0 structure so that we set UUIDs to
> + * zero using memset
> + */
> + struct ffa_partition_info_1_0 info;
> +
> + if ( dst_buf > (end_buf - dst_size) )
> + {
> + ret = FFA_RET_NO_MEMORY;
> + goto out;
> + }
> +
> + /*
> + * Context might has been removed since we go it or being removed
> + * right now so we might return information for a VM not existing
> + * anymore. This is acceptable as we return a view of the system
> + * which could change at any time.
> + */
> + info.id = dest_ctx->ffa_id;
> + info.execution_context = dest_ctx->num_vcpus;
> + info.partition_properties = FFA_PART_VM_PROP;
> + if ( dest_ctx->is_64bit )
> + info.partition_properties |= FFA_PART_PROP_AARCH64_STATE;
> +
> + memcpy(dst_buf, &info, MIN(sizeof(info), dst_size));
> +
> + if ( dst_size > sizeof(info) )
> + memset(dst_buf + sizeof(info), 0,
> + dst_size - sizeof(info));
> +
> + dst_buf += dst_size;
> + count++;
> + }
> + }
> + *vm_count = count;
> +
> +out:
> + read_unlock(&ffa_ctx_list_rwlock);
> +
> + return ret;
> +}
> +
> void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
> {
> int32_t ret = FFA_RET_OK;
> @@ -164,7 +231,7 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
> };
> uint32_t dst_size = 0;
> void *dst_buf, *end_buf;
> - uint32_t ffa_sp_count = 0;
> + uint32_t ffa_vm_count = 0, ffa_sp_count = 0;
>
> /*
> * If the guest is v1.0, he does not get back the entry size so we must
> @@ -191,15 +258,18 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
> }
>
> if ( ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
> + {
> ret = ffa_get_sp_count(uuid, &ffa_sp_count);
> + if ( ret )
> + goto out;
> + }
>
> - goto out;
> - }
> + /*
> + * Do not count the caller VM as the spec is not clearly mandating it
> + * and it is not supported by Linux.
> + */
> + ffa_vm_count = get_ffa_vm_count() - 1;
>
> - if ( !ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
> - {
> - /* Just give an empty partition list to the caller */
> - ret = FFA_RET_OK;
> goto out;
> }
>
> @@ -224,9 +294,19 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
> goto out_rx_release;
> }
>
> - ret = ffa_get_sp_partinfo(uuid, &ffa_sp_count, dst_buf, end_buf,
> - dst_size);
> + if ( ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
> + {
> + ret = ffa_get_sp_partinfo(uuid, &ffa_sp_count, dst_buf, end_buf,
> + dst_size);
> +
> + if ( ret )
> + goto out_rx_release;
> +
> + dst_buf += ffa_sp_count * dst_size;
> + }
>
> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
> + ret = ffa_get_vm_partinfo(&ffa_vm_count, dst_buf, end_buf, dst_size);
>
> out_rx_release:
> if ( ret )
> @@ -235,7 +315,7 @@ out:
> if ( ret )
> ffa_set_regs_error(regs, ret);
> else
> - ffa_set_regs_success(regs, ffa_sp_count, dst_size);
> + ffa_set_regs_success(regs, ffa_sp_count + ffa_vm_count, dst_size);
> }
>
> static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
> index 0a9c1082db28..1a1dcabcdc28 100644
> --- a/xen/arch/arm/tee/ffa_private.h
> +++ b/xen/arch/arm/tee/ffa_private.h
> @@ -195,6 +195,18 @@
> */
> #define FFA_PARTITION_INFO_GET_COUNT_FLAG BIT(0, U)
>
> +/*
> + * Partition properties we give for a normal world VM:
> + * - can send direct message but not receive them
> + * - can handle indirect messages
> + * - can receive notifications
> + * 32/64 bit flag is set depending on the VM
> + */
> +#define FFA_PART_VM_PROP (FFA_PART_PROP_DIRECT_REQ_SEND | \
> + FFA_PART_PROP_INDIRECT_MSGS | \
> + FFA_PART_PROP_RECV_NOTIF | \
> + FFA_PART_PROP_IS_PE_ID)
> +
> /* Flags used in calls to FFA_NOTIFICATION_GET interface */
> #define FFA_NOTIF_FLAG_BITMAP_SP BIT(0, U)
> #define FFA_NOTIF_FLAG_BITMAP_VM BIT(1, U)
> @@ -297,36 +309,72 @@ struct ffa_ctx_notif {
> };
>
> struct ffa_ctx {
> - void *rx;
> - const void *tx;
> - struct page_info *rx_pg;
> - struct page_info *tx_pg;
> + /*
> + * Chain list of all FF-A contexts.
> + * As we might have several read from the list of context through parallel
> + * partinfo_get but fewer additions/removal as those happen only during a
> + * version negociation or guest shutdown, access to this list is protected
negotiation
With these two typos fixed, please apply:
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Cheers,
Jens
> + * through a rwlock (addition/removal with write lock, reading through a
> + * read lock).
> + */
> + struct list_head ctx_list; /* chain list of all FF-A contexts */
> +
> + /*
> + * Data access unlocked (mainly for part_info_get in VM to VM).
> + * Those should be set before the ctx is added in the list.
> + */
> + /* FF-A Endpoint ID */
> + uint16_t ffa_id;
> + uint16_t num_vcpus;
> + bool is_64bit;
> +
> + /*
> + * Global data accessed atomically or using ACCES_ONCE.
> + */
> + struct ffa_ctx_notif notif;
> +
> + /*
> + * Global data accessed with lock locked.
> + */
> + spinlock_t lock;
> + /*
> + * FF-A version negociated 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;
> - /* FF-A version used by the guest */
> - uint32_t guest_vers;
> - bool rx_is_free;
> - /* Used shared memory objects, struct ffa_shm_mem */
> - struct list_head shm_list;
> /* Number of allocated shared memory object */
> unsigned int shm_count;
> - struct ffa_ctx_notif notif;
> + /* Used shared memory objects, struct ffa_shm_mem */
> + struct list_head shm_list;
> +
> /*
> - * tx_lock is used to serialize access to tx
> - * rx_lock is used to serialize access to rx_is_free
> - * lock is used for the rest in this struct
> + * Rx buffer, accessed with rx_lock locked.
> + * rx_is_free is used to serialize access.
> */
> - spinlock_t tx_lock;
> spinlock_t rx_lock;
> - spinlock_t lock;
> - /* Used if domain can't be torn down immediately */
> + bool rx_is_free;
> + void *rx;
> + struct page_info *rx_pg;
> +
> + /*
> + * Tx buffer, access with tx_lock locked.
> + */
> + spinlock_t tx_lock;
> + const void *tx;
> + struct page_info *tx_pg;
> +
> +
> + /*
> + * Domain teardown handling if data shared or used by other domains
> + * do not allow to teardown the domain immediately.
> + */
> struct domain *teardown_d;
> struct list_head teardown_list;
> s_time_t teardown_expire;
> - /*
> - * Used for ffa_domain_teardown() to keep track of which SPs should be
> - * notified that this guest is being destroyed.
> - */
> + /* Keep track of SPs that should be notified of VM destruction */
> unsigned long *vm_destroy_bitmap;
> };
>
> @@ -336,6 +384,12 @@ extern spinlock_t ffa_rx_buffer_lock;
> extern spinlock_t ffa_tx_buffer_lock;
> extern DECLARE_BITMAP(ffa_fw_abi_supported, FFA_ABI_BITMAP_SIZE);
>
> +extern struct list_head ffa_ctx_head;
> +extern rwlock_t ffa_ctx_list_rwlock;
> +#ifdef CONFIG_FFA_VM_TO_VM
> +extern atomic_t ffa_vm_count;
> +#endif
> +
> bool ffa_shm_domain_destroy(struct domain *d);
> void ffa_handle_mem_share(struct cpu_user_regs *regs);
> int ffa_handle_mem_reclaim(uint64_t handle, uint32_t flags);
> @@ -368,6 +422,29 @@ int ffa_handle_notification_set(struct cpu_user_regs *regs);
> void ffa_handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t fid);
> int32_t ffa_handle_msg_send2(struct cpu_user_regs *regs);
>
> +#ifdef CONFIG_FFA_VM_TO_VM
> +static inline uint16_t get_ffa_vm_count(void)
> +{
> + return atomic_read(&ffa_vm_count);
> +}
> +
> +static inline void inc_ffa_vm_count(void)
> +{
> + atomic_inc(&ffa_vm_count);
> +}
> +
> +static inline void dec_ffa_vm_count(void)
> +{
> + ASSERT(atomic_read(&ffa_vm_count) > 0);
> + atomic_dec(&ffa_vm_count);
> +}
> +#else
> +/* Only count the caller VM */
> +#define get_ffa_vm_count() ((uint16_t)1UL)
> +#define inc_ffa_vm_count() do {} while(0)
> +#define dec_ffa_vm_count() do {} while(0)
> +#endif
> +
> static inline uint16_t ffa_get_vm_id(const struct domain *d)
> {
> /* +1 since 0 is reserved for the hypervisor in FF-A */
> --
> 2.47.1
>
On 7/17/25 08:11, Bertrand Marquis wrote: > Create a CONFIG_FFA_VM_TO_VM parameter to activate FFA communication > between VMs. > When activated list VMs in the system with FF-A support in part_info_get. > > When VM to VM is activated, Xen will be tainted as Insecure and a > message is displayed to the user during the boot as there is no > filtering of VMs in FF-A so any VM can communicate or see any other VM > in the system. > > WARNING: There is no filtering for now and all VMs are listed !! I'm pretty sure that there is already no filtering for things like grant tables and event channels, so this doesn't make things any worse. That said, FF-A is quite tricky to implement without integer overflow/wraparound or denial of service bugs. In particular, code in Hafnium (Secure Partition Monitor running in S-EL2) requires quadratic time because of repeated linear searches. Xen is allowed to use dynamic memory allocation, so it can, should, and must do better. -- Sincerely, Demi Marie Obenour (she/her/hers)
Hi Demi, > On 3 Aug 2025, at 06:24, Demi Marie Obenour <demiobenour@gmail.com> wrote: > > On 7/17/25 08:11, Bertrand Marquis wrote: >> Create a CONFIG_FFA_VM_TO_VM parameter to activate FFA communication >> between VMs. >> When activated list VMs in the system with FF-A support in part_info_get. >> >> When VM to VM is activated, Xen will be tainted as Insecure and a >> message is displayed to the user during the boot as there is no >> filtering of VMs in FF-A so any VM can communicate or see any other VM >> in the system. >> >> WARNING: There is no filtering for now and all VMs are listed !! > I'm pretty sure that there is already no filtering for things like grant > tables and event channels, so this doesn't make things any worse. That > said, FF-A is quite tricky to implement without integer overflow/wraparound > or denial of service bugs. In particular, code in Hafnium (Secure Partition > Monitor running in S-EL2) requires quadratic time because of repeated linear > searches. Xen is allowed to use dynamic memory allocation, so it can, should, > and must do better. I do agree but we still have tricky cases where we could end up in unbounded loops. Some are handled by adding some limits. Dynamic allocation being available in Xen is helping a lot on some cases but for now I try to prevent it when possible but we might have to review this later if we want to increase some capacities (for example the number of shared memories). I plan to do a presentation at the Xen Summit and a design session to discuss how we could define a way to define by configuration or at runtime what secure endpoints or VMs can be contacted by a VM with FF-A enabled. Regards Bertrand > -- > Sincerely, > Demi Marie Obenour (she/her/hers)<OpenPGP_0xB288B55FFF9C22C1.asc>
Hi,
A gentle ping for someone to review this :-)
Thanks
Bertrand
> On 17 Jul 2025, at 14:11, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
>
> Create a CONFIG_FFA_VM_TO_VM parameter to activate FFA communication
> between VMs.
> When activated list VMs in the system with FF-A support in part_info_get.
>
> When VM to VM is activated, Xen will be tainted as Insecure and a
> message is displayed to the user during the boot as there is no
> filtering of VMs in FF-A so any VM can communicate or see any other VM
> in the system.
>
> WARNING: There is no filtering for now and all VMs are listed !!
>
> This patch is reorganizing the ffa_ctx structure to make clear which
> lock is protecting what parts.
>
> This patch is introducing a chain list of the ffa_ctx with a FFA Version
> negociated allowing to create the partinfo results for VMs in parallel
> by using rwlock which only ensure addition/removal of entries are
> protected.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in v7:
> - protect ffa_ctx list with a rw lock to allow several partinfo_get in
> parallel but protect adding/removing entries.
> Changes in v6:
> - remove ACCESS_ONCE for guest_vers access and take the context lock
> before modifying it
> - move guest_vers in context declaration to fields protected by the
> context lock and add a comment to state that lock in only needed when
> modifying it
> Changes in v5:
> - remove invalid comment about 1.1 firmware support
> - rename variables from d and dom to curr_d and dest_d (Julien)
> - add a TODO in the code for potential holding for long of the CPU
> (Julien)
> - use an atomic global variable to store the number of VMs instead of
> recomputing the value each time (Julien)
> - add partinfo information in ffa_ctx (id, cpus and 64bit) and create a
> chain list of ctx. Use this chain list to create the partinfo result
> without holding a global lock to prevent concurrency issues.
> - Move some changes in a preparation patch modifying partinfo for sps to
> reduce this patch size and make the review easier
> Changes in v4:
> - properly handle SPMC version 1.0 header size case in partinfo_get
> - switch to local counting variables instead of *pointer += 1 form
> - coding style issue with missing spaces in if ()
> Changes in v3:
> - break partinfo_get in several sub functions to make the implementation
> easier to understand and lock handling easier
> - rework implementation to check size along the way and prevent previous
> implementation limits which had to check that the number of VMs or SPs
> did not change
> - taint Xen as INSECURE when VM to VM is enabled
> Changes in v2:
> - Switch ifdef to IS_ENABLED
> - dom was not switched to d as requested by Jan because there is already
> a variable d pointing to the current domain and it must not be
> shadowed.
> ---
> xen/arch/arm/tee/Kconfig | 11 +++
> xen/arch/arm/tee/ffa.c | 47 +++++++++++++
> xen/arch/arm/tee/ffa_partinfo.c | 100 ++++++++++++++++++++++++---
> xen/arch/arm/tee/ffa_private.h | 117 ++++++++++++++++++++++++++------
> 4 files changed, 245 insertions(+), 30 deletions(-)
>
> diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig
> index c5b0f88d7522..88a4c4c99154 100644
> --- a/xen/arch/arm/tee/Kconfig
> +++ b/xen/arch/arm/tee/Kconfig
> @@ -28,5 +28,16 @@ config FFA
>
> [1] https://developer.arm.com/documentation/den0077/latest
>
> +config FFA_VM_TO_VM
> + bool "Enable FF-A between VMs (UNSUPPORTED)" if UNSUPPORTED
> + default n
> + depends on FFA
> + help
> + This option enables to use FF-A between VMs.
> + This is experimental and there is no access control so any
> + guest can communicate with any other guest.
> +
> + If unsure, say N.
> +
> endmenu
>
> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> index 3bbdd7168a6b..be71eda4869f 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -118,6 +118,13 @@ void *ffa_tx __read_mostly;
> DEFINE_SPINLOCK(ffa_rx_buffer_lock);
> DEFINE_SPINLOCK(ffa_tx_buffer_lock);
>
> +struct list_head ffa_ctx_head;
> +/* RW Lock to protect addition/removal and reading in ffa_ctx_head */
> +rwlock_t ffa_ctx_list_rwlock;
> +
> +#ifdef CONFIG_FFA_VM_TO_VM
> +atomic_t ffa_vm_count;
> +#endif
>
> /* Used to track domains that could not be torn down immediately. */
> static struct timer ffa_teardown_timer;
> @@ -151,6 +158,7 @@ static void handle_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;
>
> /*
> * Guest will use the version it requested if it is our major and minor
> @@ -160,10 +168,23 @@ static void handle_version(struct cpu_user_regs *regs)
> */
> if ( FFA_VERSION_MAJOR(vers) == FFA_MY_VERSION_MAJOR )
> {
> + spin_lock(&ctx->lock);
> + old_vers = ctx->guest_vers;
> +
> if ( FFA_VERSION_MINOR(vers) > FFA_MY_VERSION_MINOR )
> ctx->guest_vers = FFA_MY_VERSION;
> else
> ctx->guest_vers = vers;
> + spin_unlock(&ctx->lock);
> +
> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !old_vers )
> + {
> + /* One more VM with FF-A support available */
> + inc_ffa_vm_count();
> + write_lock(&ffa_ctx_list_rwlock);
> + list_add_tail(&ctx->ctx_list, &ffa_ctx_head);
> + write_unlock(&ffa_ctx_list_rwlock);
> + }
> }
> ffa_set_regs(regs, FFA_MY_VERSION, 0, 0, 0, 0, 0, 0, 0);
> }
> @@ -345,6 +366,10 @@ static int ffa_domain_init(struct domain *d)
> ctx->teardown_d = d;
> INIT_LIST_HEAD(&ctx->shm_list);
>
> + ctx->ffa_id = ffa_get_vm_id(d);
> + ctx->num_vcpus = d->max_vcpus;
> + ctx->is_64bit = is_64bit_domain(d);
> +
> /*
> * ffa_domain_teardown() will be called if ffa_domain_init() returns an
> * error, so no need for cleanup in this function.
> @@ -421,6 +446,14 @@ static int ffa_domain_teardown(struct domain *d)
> if ( !ctx )
> return 0;
>
> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && ctx->guest_vers )
> + {
> + dec_ffa_vm_count();
> + write_lock(&ffa_ctx_list_rwlock);
> + list_del(&ctx->ctx_list);
> + write_unlock(&ffa_ctx_list_rwlock);
> + }
> +
> ffa_rxtx_domain_destroy(d);
> ffa_notif_domain_destroy(d);
>
> @@ -464,6 +497,18 @@ static bool ffa_probe(void)
> printk(XENLOG_INFO "ARM FF-A Mediator version %u.%u\n",
> FFA_MY_VERSION_MAJOR, FFA_MY_VERSION_MINOR);
>
> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
> + {
> + /*
> + * When FFA VM to VM is enabled, the current implementation does not
> + * offer any way to limit which VM can communicate with which VM using
> + * FF-A.
> + * Signal this in the xen console and taint the system as insecure.
> + * TODO: Introduce a solution to limit what a VM can do through FFA.
> + */
> + printk(XENLOG_ERR "ffa: VM to VM is enabled, system is insecure !!\n");
> + add_taint(TAINT_MACHINE_INSECURE);
> + }
> /*
> * psci_init_smccc() updates this value with what's reported by EL-3
> * or secure world.
> @@ -538,6 +583,8 @@ static bool ffa_probe(void)
>
> ffa_notif_init();
> INIT_LIST_HEAD(&ffa_teardown_head);
> + INIT_LIST_HEAD(&ffa_ctx_head);
> + rwlock_init(&ffa_ctx_list_rwlock);
> init_timer(&ffa_teardown_timer, ffa_teardown_timer_callback, NULL, 0);
>
> return true;
> diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
> index dfa0b23eaf38..fa56b1587e3b 100644
> --- a/xen/arch/arm/tee/ffa_partinfo.c
> +++ b/xen/arch/arm/tee/ffa_partinfo.c
> @@ -150,6 +150,73 @@ out:
> return ret;
> }
>
> +static int32_t ffa_get_vm_partinfo(uint32_t *vm_count, void *dst_buf,
> + void *end_buf, uint32_t dst_size)
> +{
> + struct ffa_ctx *curr_ctx = current->domain->arch.tee;
> + struct ffa_ctx *dest_ctx;
> + uint32_t count = 0;
> + int32_t ret = FFA_RET_OK;
> +
> + /*
> + * There could potentially be a lot of VMs in the system and we could
> + * hold the CPU for long here.
> + * Right now there is no solution in FF-A specification to split
> + * the work in this case.
> + * TODO: Check how we could delay the work or have preemption checks.
> + */
> + read_lock(&ffa_ctx_list_rwlock);
> + list_for_each_entry(dest_ctx, &ffa_ctx_head, ctx_list)
> + {
> + /*
> + * Do not include an entry for the caller VM as the spec is not
> + * clearly mandating it and it is not supported by Linux.
> + */
> + if ( dest_ctx != curr_ctx )
> + {
> + /*
> + * We do not have UUID info for VMs so use
> + * the 1.0 structure so that we set UUIDs to
> + * zero using memset
> + */
> + struct ffa_partition_info_1_0 info;
> +
> + if ( dst_buf > (end_buf - dst_size) )
> + {
> + ret = FFA_RET_NO_MEMORY;
> + goto out;
> + }
> +
> + /*
> + * Context might has been removed since we go it or being removed
> + * right now so we might return information for a VM not existing
> + * anymore. This is acceptable as we return a view of the system
> + * which could change at any time.
> + */
> + info.id = dest_ctx->ffa_id;
> + info.execution_context = dest_ctx->num_vcpus;
> + info.partition_properties = FFA_PART_VM_PROP;
> + if ( dest_ctx->is_64bit )
> + info.partition_properties |= FFA_PART_PROP_AARCH64_STATE;
> +
> + memcpy(dst_buf, &info, MIN(sizeof(info), dst_size));
> +
> + if ( dst_size > sizeof(info) )
> + memset(dst_buf + sizeof(info), 0,
> + dst_size - sizeof(info));
> +
> + dst_buf += dst_size;
> + count++;
> + }
> + }
> + *vm_count = count;
> +
> +out:
> + read_unlock(&ffa_ctx_list_rwlock);
> +
> + return ret;
> +}
> +
> void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
> {
> int32_t ret = FFA_RET_OK;
> @@ -164,7 +231,7 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
> };
> uint32_t dst_size = 0;
> void *dst_buf, *end_buf;
> - uint32_t ffa_sp_count = 0;
> + uint32_t ffa_vm_count = 0, ffa_sp_count = 0;
>
> /*
> * If the guest is v1.0, he does not get back the entry size so we must
> @@ -191,15 +258,18 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
> }
>
> if ( ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
> + {
> ret = ffa_get_sp_count(uuid, &ffa_sp_count);
> + if ( ret )
> + goto out;
> + }
>
> - goto out;
> - }
> + /*
> + * Do not count the caller VM as the spec is not clearly mandating it
> + * and it is not supported by Linux.
> + */
> + ffa_vm_count = get_ffa_vm_count() - 1;
>
> - if ( !ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
> - {
> - /* Just give an empty partition list to the caller */
> - ret = FFA_RET_OK;
> goto out;
> }
>
> @@ -224,9 +294,19 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
> goto out_rx_release;
> }
>
> - ret = ffa_get_sp_partinfo(uuid, &ffa_sp_count, dst_buf, end_buf,
> - dst_size);
> + if ( ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
> + {
> + ret = ffa_get_sp_partinfo(uuid, &ffa_sp_count, dst_buf, end_buf,
> + dst_size);
> +
> + if ( ret )
> + goto out_rx_release;
> +
> + dst_buf += ffa_sp_count * dst_size;
> + }
>
> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
> + ret = ffa_get_vm_partinfo(&ffa_vm_count, dst_buf, end_buf, dst_size);
>
> out_rx_release:
> if ( ret )
> @@ -235,7 +315,7 @@ out:
> if ( ret )
> ffa_set_regs_error(regs, ret);
> else
> - ffa_set_regs_success(regs, ffa_sp_count, dst_size);
> + ffa_set_regs_success(regs, ffa_sp_count + ffa_vm_count, dst_size);
> }
>
> static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
> index 0a9c1082db28..1a1dcabcdc28 100644
> --- a/xen/arch/arm/tee/ffa_private.h
> +++ b/xen/arch/arm/tee/ffa_private.h
> @@ -195,6 +195,18 @@
> */
> #define FFA_PARTITION_INFO_GET_COUNT_FLAG BIT(0, U)
>
> +/*
> + * Partition properties we give for a normal world VM:
> + * - can send direct message but not receive them
> + * - can handle indirect messages
> + * - can receive notifications
> + * 32/64 bit flag is set depending on the VM
> + */
> +#define FFA_PART_VM_PROP (FFA_PART_PROP_DIRECT_REQ_SEND | \
> + FFA_PART_PROP_INDIRECT_MSGS | \
> + FFA_PART_PROP_RECV_NOTIF | \
> + FFA_PART_PROP_IS_PE_ID)
> +
> /* Flags used in calls to FFA_NOTIFICATION_GET interface */
> #define FFA_NOTIF_FLAG_BITMAP_SP BIT(0, U)
> #define FFA_NOTIF_FLAG_BITMAP_VM BIT(1, U)
> @@ -297,36 +309,72 @@ struct ffa_ctx_notif {
> };
>
> struct ffa_ctx {
> - void *rx;
> - const void *tx;
> - struct page_info *rx_pg;
> - struct page_info *tx_pg;
> + /*
> + * Chain list of all FF-A contexts.
> + * As we might have several read from the list of context through parallel
> + * partinfo_get but fewer additions/removal as those happen only during a
> + * version negociation or guest shutdown, access to this list is protected
> + * through a rwlock (addition/removal with write lock, reading through a
> + * read lock).
> + */
> + struct list_head ctx_list; /* chain list of all FF-A contexts */
> +
> + /*
> + * Data access unlocked (mainly for part_info_get in VM to VM).
> + * Those should be set before the ctx is added in the list.
> + */
> + /* FF-A Endpoint ID */
> + uint16_t ffa_id;
> + uint16_t num_vcpus;
> + bool is_64bit;
> +
> + /*
> + * Global data accessed atomically or using ACCES_ONCE.
> + */
> + struct ffa_ctx_notif notif;
> +
> + /*
> + * Global data accessed with lock locked.
> + */
> + spinlock_t lock;
> + /*
> + * FF-A version negociated 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;
> - /* FF-A version used by the guest */
> - uint32_t guest_vers;
> - bool rx_is_free;
> - /* Used shared memory objects, struct ffa_shm_mem */
> - struct list_head shm_list;
> /* Number of allocated shared memory object */
> unsigned int shm_count;
> - struct ffa_ctx_notif notif;
> + /* Used shared memory objects, struct ffa_shm_mem */
> + struct list_head shm_list;
> +
> /*
> - * tx_lock is used to serialize access to tx
> - * rx_lock is used to serialize access to rx_is_free
> - * lock is used for the rest in this struct
> + * Rx buffer, accessed with rx_lock locked.
> + * rx_is_free is used to serialize access.
> */
> - spinlock_t tx_lock;
> spinlock_t rx_lock;
> - spinlock_t lock;
> - /* Used if domain can't be torn down immediately */
> + bool rx_is_free;
> + void *rx;
> + struct page_info *rx_pg;
> +
> + /*
> + * Tx buffer, access with tx_lock locked.
> + */
> + spinlock_t tx_lock;
> + const void *tx;
> + struct page_info *tx_pg;
> +
> +
> + /*
> + * Domain teardown handling if data shared or used by other domains
> + * do not allow to teardown the domain immediately.
> + */
> struct domain *teardown_d;
> struct list_head teardown_list;
> s_time_t teardown_expire;
> - /*
> - * Used for ffa_domain_teardown() to keep track of which SPs should be
> - * notified that this guest is being destroyed.
> - */
> + /* Keep track of SPs that should be notified of VM destruction */
> unsigned long *vm_destroy_bitmap;
> };
>
> @@ -336,6 +384,12 @@ extern spinlock_t ffa_rx_buffer_lock;
> extern spinlock_t ffa_tx_buffer_lock;
> extern DECLARE_BITMAP(ffa_fw_abi_supported, FFA_ABI_BITMAP_SIZE);
>
> +extern struct list_head ffa_ctx_head;
> +extern rwlock_t ffa_ctx_list_rwlock;
> +#ifdef CONFIG_FFA_VM_TO_VM
> +extern atomic_t ffa_vm_count;
> +#endif
> +
> bool ffa_shm_domain_destroy(struct domain *d);
> void ffa_handle_mem_share(struct cpu_user_regs *regs);
> int ffa_handle_mem_reclaim(uint64_t handle, uint32_t flags);
> @@ -368,6 +422,29 @@ int ffa_handle_notification_set(struct cpu_user_regs *regs);
> void ffa_handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t fid);
> int32_t ffa_handle_msg_send2(struct cpu_user_regs *regs);
>
> +#ifdef CONFIG_FFA_VM_TO_VM
> +static inline uint16_t get_ffa_vm_count(void)
> +{
> + return atomic_read(&ffa_vm_count);
> +}
> +
> +static inline void inc_ffa_vm_count(void)
> +{
> + atomic_inc(&ffa_vm_count);
> +}
> +
> +static inline void dec_ffa_vm_count(void)
> +{
> + ASSERT(atomic_read(&ffa_vm_count) > 0);
> + atomic_dec(&ffa_vm_count);
> +}
> +#else
> +/* Only count the caller VM */
> +#define get_ffa_vm_count() ((uint16_t)1UL)
> +#define inc_ffa_vm_count() do {} while(0)
> +#define dec_ffa_vm_count() do {} while(0)
> +#endif
> +
> static inline uint16_t ffa_get_vm_id(const struct domain *d)
> {
> /* +1 since 0 is reserved for the hypervisor in FF-A */
> --
> 2.47.1
>
>
© 2016 - 2025 Red Hat, Inc.