Re: [PATCH v4 2/5] xen/arm: ffa: Introduce VM to VM support

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi,

> On 24 Mar 2025, at 14:53, Bertrand Marquis <Bertrand.Marquis@xxxxxxx> 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 !!
> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> ---
> 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          |  12 ++
> xen/arch/arm/tee/ffa_partinfo.c | 274 +++++++++++++++++++++-----------
> xen/arch/arm/tee/ffa_private.h  |  12 ++
> 4 files changed, 218 insertions(+), 91 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..e41ab5f8ada6 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -464,6 +464,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.
> diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
> index c0510ceb8338..406c57b95f77 100644
> --- a/xen/arch/arm/tee/ffa_partinfo.c
> +++ b/xen/arch/arm/tee/ffa_partinfo.c
> @@ -63,9 +63,156 @@ static int32_t ffa_partition_info_get(uint32_t *uuid, 
> uint32_t flags,
>     return ret;
> }
> 
> -void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
> +static int32_t ffa_get_sp_count(uint32_t *uuid, uint32_t *sp_count)
> +{
> +    uint32_t src_size;
> +
> +    return ffa_partition_info_get(uuid, FFA_PARTITION_INFO_GET_COUNT_FLAG,
> +                                  sp_count, &src_size);
> +}
> +
> +static int32_t ffa_get_sp_partinfo(uint32_t *uuid, uint32_t *sp_count,
> +                                   void *dst_buf, void *end_buf,
> +                                   uint32_t dst_size)
> {
>     int32_t ret;
> +    uint32_t src_size, real_sp_count;
> +    void *src_buf = ffa_rx;
> +    uint32_t count = 0;
> +
> +    /* Do we have a RX buffer with the SPMC */
> +    if ( !ffa_rx )
> +        return FFA_RET_DENIED;
> +
> +    /* We need to use the RX buffer to receive the list */
> +    spin_lock(&ffa_rx_buffer_lock);
> +
> +    ret = ffa_partition_info_get(uuid, 0, &real_sp_count, &src_size);
> +    if ( ret )
> +        goto out;
> +
> +    /* We now own the RX buffer */
> +
> +    /* We only support a 1.1 firmware version */

This comment should have been removed.
I will fix it on next version of might be possible to do on commit
if there are no further comments here.

Cheers
Bertrand