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.marq...@arm.com>
With two remarks below:
Acked-by: Julien Grall <jgr...@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
