Re: [PATCH 08/10] xen/arm: ffa: add v1.2 SEND2 header layout

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

HI Jens,

> On 3 Dec 2025, at 17:38, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote:
> 
> Hi Bertrand,
> 
> On Thu, Nov 27, 2025 at 4:53 PM Bertrand Marquis
> <bertrand.marquis@xxxxxxx> wrote:
>> 
>> Teach the SEND2 path about the distinct FF-A v1.1 and v1.2 RX/TX header
>> layouts so we can propagate the 128-bit UUIDs introduced in v1.2.
>> 
>> VM-to-VM SEND2 calls now build the larger v1.2 header, zeroing the UUID
>> fields for v1.1 senders, and the dispatcher validates messages using
>> the v1.1 header layout to keep legacy guests working.
>> 
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
>> ---
>> xen/arch/arm/tee/ffa_msg.c | 55 +++++++++++++++++++++++++++++---------
>> 1 file changed, 42 insertions(+), 13 deletions(-)
>> 
>> diff --git a/xen/arch/arm/tee/ffa_msg.c b/xen/arch/arm/tee/ffa_msg.c
>> index 8bb4bd93f724..472bfad79dd3 100644
>> --- a/xen/arch/arm/tee/ffa_msg.c
>> +++ b/xen/arch/arm/tee/ffa_msg.c
>> @@ -13,7 +13,7 @@
>> #include "ffa_private.h"
>> 
>> /* Encoding of partition message in RX/TX buffer */
>> -struct ffa_part_msg_rxtx {
>> +struct ffa_part_msg_rxtx_1_1 {
>>     uint32_t flags;
>>     uint32_t reserved;
>>     uint32_t msg_offset;
>> @@ -21,6 +21,16 @@ struct ffa_part_msg_rxtx {
>>     uint32_t msg_size;
>> };
>> 
>> +struct ffa_part_msg_rxtx_1_2 {
>> +    uint32_t flags;
>> +    uint32_t reserved;
>> +    uint32_t msg_offset;
>> +    uint32_t send_recv_id;
>> +    uint32_t msg_size;
>> +    uint32_t reserved2;
>> +    uint64_t uuid[2];
>> +};
>> +
>> static void ffa_finish_direct_req_run(struct cpu_user_regs *regs,
>>                                       struct arm_smccc_1_2_regs *req)
>> {
>> @@ -104,12 +114,12 @@ out:
>>     ffa_set_regs_error(regs, ret);
>> }
>> 
>> -static int32_t ffa_msg_send2_vm(uint16_t dst_id, const void *src_buf,
>> -                                struct ffa_part_msg_rxtx *src_msg)
>> +static int32_t ffa_msg_send2_vm(struct ffa_ctx *src_ctx, uint16_t dst_id,
>> +                                struct ffa_part_msg_rxtx_1_2 *src_msg)
>> {
>>     struct domain *dst_d;
>>     struct ffa_ctx *dst_ctx;
>> -    struct ffa_part_msg_rxtx *dst_msg;
>> +    struct ffa_part_msg_rxtx_1_2 *dst_msg;
>>     int err;
>>     int32_t ret;
>> 
>> @@ -142,7 +152,7 @@ static int32_t ffa_msg_send2_vm(uint16_t dst_id, const 
>> void *src_buf,
>> 
>>     /* we need to have enough space in the destination buffer */
>>     if ( (dst_ctx->page_count * FFA_PAGE_SIZE -
>> -          sizeof(struct ffa_part_msg_rxtx)) < src_msg->msg_size )
>> +          sizeof(struct ffa_part_msg_rxtx_1_2)) < src_msg->msg_size )
>>     {
>>         ret = FFA_RET_NO_MEMORY;
>>         ffa_rx_release(dst_d);
>> @@ -154,12 +164,24 @@ static int32_t ffa_msg_send2_vm(uint16_t dst_id, const 
>> void *src_buf,
>>     /* prepare destination header */
>>     dst_msg->flags = 0;
>>     dst_msg->reserved = 0;
>> -    dst_msg->msg_offset = sizeof(struct ffa_part_msg_rxtx);
>> +    dst_msg->msg_offset = sizeof(struct ffa_part_msg_rxtx_1_2);
>>     dst_msg->send_recv_id = src_msg->send_recv_id;
>>     dst_msg->msg_size = src_msg->msg_size;
>> +    dst_msg->reserved2 = 0;
>> 
>> -    memcpy(dst_ctx->rx + sizeof(struct ffa_part_msg_rxtx),
>> -           src_buf + src_msg->msg_offset, src_msg->msg_size);
>> +    if ( src_ctx->guest_vers < FFA_VERSION_1_2 )
>> +    {
>> +        dst_msg->uuid[0] = 0;
>> +        dst_msg->uuid[1] = 0;
>> +    }
>> +    else
>> +    {
>> +        dst_msg->uuid[0] = src_msg->uuid[0];
>> +        dst_msg->uuid[1] = src_msg->uuid[1];
>> +    }
>> +
>> +    memcpy(dst_ctx->rx + sizeof(struct ffa_part_msg_rxtx_1_2),
>> +           src_ctx->tx + src_msg->msg_offset, src_msg->msg_size);
>> 
>>     /* receiver rx buffer will be released by the receiver*/
>> 
>> @@ -175,11 +197,17 @@ int32_t ffa_handle_msg_send2(struct cpu_user_regs 
>> *regs)
>> {
>>     struct domain *src_d = current->domain;
>>     struct ffa_ctx *src_ctx = src_d->arch.tee;
>> -    struct ffa_part_msg_rxtx src_msg;
>> +    /*
>> +     * src_msg is interpreted as v1.2 header, but:
>> +     * - for v1.1 guests, uuid[] is ignored and may contain payload bytes
>> +     * - for v1.2 guests, uuid[] carries the FF-A v1.2 UUID fields
>> +     */
>> +    struct ffa_part_msg_rxtx_1_2 src_msg;
>>     uint16_t dst_id, src_id;
>>     int32_t ret;
>> 
>> -    BUILD_BUG_ON(sizeof(struct ffa_part_msg_rxtx) >= FFA_PAGE_SIZE);
>> +    BUILD_BUG_ON(sizeof(struct ffa_part_msg_rxtx_1_1) >= FFA_PAGE_SIZE);
>> +    BUILD_BUG_ON(sizeof(struct ffa_part_msg_rxtx_1_2) >= FFA_PAGE_SIZE);
>> 
>>     if ( !spin_trylock(&src_ctx->tx_lock) )
>>         return FFA_RET_BUSY;
>> @@ -190,14 +218,15 @@ int32_t ffa_handle_msg_send2(struct cpu_user_regs 
>> *regs)
>>     src_id = src_msg.send_recv_id >> 16;
>>     dst_id = src_msg.send_recv_id & GENMASK(15,0);
>> 
>> -    if ( src_id != ffa_get_vm_id(src_d) )
>> +    if ( src_id != ffa_get_vm_id(src_d) ||
>> +         dst_id == ffa_get_vm_id(src_d) )
> 
> It might be worth mentioning in the commit message that we're making
> it a bit more robust.

Ack i will add this in the commit message.

> 
>>     {
>>         ret = FFA_RET_INVALID_PARAMETERS;
>>         goto out;
>>     }
>> 
>>     /* check source message fits in buffer */
>> -    if ( src_msg.msg_offset < sizeof(struct ffa_part_msg_rxtx) ||
>> +    if ( src_msg.msg_offset < sizeof(struct ffa_part_msg_rxtx_1_1) ||
> 
> This is for FF-A version 1.1; for version 1.2, the minimal offset must
> be larger.

Yes you are right.

I will update the check and optimize things a little bit setting here the
uuid to 0 in src_msg if sender is < 1.1. This will remove the need to 
transfer the context to test the version to ffa_msg_send2_vm.

Thanks for the finding :-)

Cheers
Bertrand