> On 4 Mar 2026, at 11:44, Jens Wiklander <[email protected]> wrote:
>
> Hi Bertrand,
>
> On Mon, Mar 2, 2026 at 4:44 PM Bertrand Marquis
> <[email protected]> wrote:
>>
>> FF-A v1.2 defines PARTITION_INFO_GET_REGS for register-based partition
>> info retrieval, but Xen currently only supports the buffer-based GET
>> path for guests.
>>
>> Implement GET_REGS using the cached SP list and VM entries, including
>> the register window layout and input validation. Track VM list changes
>> via the partinfo tag and use it to validate GET_REGS tag inputs. Ensure
>> that when a non-Nil UUID is specified, the UUID fields in both GET and
>> GET_REGS results are MBZ as required by the specification.
>>
>> PARTITION_INFO_GET_REGS is available to v1.2 guests, returning cached SP
>> entries and VM entries with UUIDs zeroed for non-Nil UUID queries.
>>
>> Also publish VM membership updates (VM count, ctx list, and partinfo
>> tag) under the same write-locked section so GET_REGS sees coherent state
>> and concurrent changes are reliably reported via RETRY.
>>
>> Signed-off-by: Bertrand Marquis <[email protected]>
>> ---
>> Changes since v1:
>> - ignore x4-x17 not being zero and x3 bits 63-32 not being zero (defined
>> as SBZ in the spec)
>> - detect tag changes during GET_REGS handling and return RETRY
>> - remove strict check of sp_list_entry_size, larger cache entry sizes
>> will now be accepted
>> - publish VM count, ctx list, and partinfo tag updates under
>> ffa_ctx_list_rwlock for coherent visibility
>> ---
>> xen/arch/arm/tee/ffa.c | 23 +++-
>> xen/arch/arm/tee/ffa_partinfo.c | 200 ++++++++++++++++++++++++++++++++
>> xen/arch/arm/tee/ffa_private.h | 4 +-
>> 3 files changed, 223 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>> index aa43ae2595d7..d6cae67e1a48 100644
>> --- a/xen/arch/arm/tee/ffa.c
>> +++ b/xen/arch/arm/tee/ffa.c
>> @@ -44,6 +44,11 @@
>> * - doesn't support signalling the secondary scheduler of pending
>> * notification for secure partitions
>> * - doesn't support notifications for Xen itself
>> + * o FFA_PARTITION_INFO_GET/GET_REGS:
>> + * - v1.0 guests may see duplicate SP IDs when firmware provides UUIDs
>> + * - SP list is cached at init; SPMC tag changes are not tracked
>> + * between calls
>> + * - SP list is capped at FFA_MAX_NUM_SP entries
>> *
>> * There are some large locked sections with ffa_spmc_tx_lock and
>> * ffa_spmc_rx_lock. Especially the ffa_spmc_tx_lock spinlock used
>> @@ -183,10 +188,11 @@ static bool ffa_negotiate_version(struct cpu_user_regs
>> *regs)
>>
>> if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
>> {
>> - /* One more VM with FF-A support available */
>> - inc_ffa_vm_count();
>> write_lock(&ffa_ctx_list_rwlock);
>> + /* Publish VM membership changes atomically with tag updates. */
>> + inc_ffa_vm_count();
>> list_add_tail(&ctx->ctx_list, &ffa_ctx_head);
>> + ffa_partinfo_inc_tag();
>> write_unlock(&ffa_ctx_list_rwlock);
>> }
>>
>> @@ -341,6 +347,12 @@ static void handle_features(struct cpu_user_regs *regs)
>> case FFA_FEATURE_SCHEDULE_RECV_INTR:
>> ffa_set_regs_success(regs, GUEST_FFA_SCHEDULE_RECV_INTR_ID, 0);
>> break;
>> + case FFA_PARTITION_INFO_GET_REGS:
>> + if ( ACCESS_ONCE(ctx->guest_vers) >= FFA_VERSION_1_2 )
>> + ffa_set_regs_success(regs, 0, 0);
>> + else
>> + ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>> + break;
>>
>> case FFA_NOTIFICATION_BIND:
>> case FFA_NOTIFICATION_UNBIND:
>> @@ -402,6 +414,9 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
>> case FFA_PARTITION_INFO_GET:
>> ffa_handle_partition_info_get(regs);
>> return true;
>> + case FFA_PARTITION_INFO_GET_REGS:
>> + ffa_handle_partition_info_get_regs(regs);
>> + return true;
>> case FFA_RX_RELEASE:
>> e = ffa_rx_release(ctx);
>> break;
>> @@ -625,9 +640,11 @@ static int ffa_domain_teardown(struct domain *d)
>>
>> if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) && ACCESS_ONCE(ctx->guest_vers) )
>> {
>> - dec_ffa_vm_count();
>> write_lock(&ffa_ctx_list_rwlock);
>> + /* Publish VM membership changes atomically with tag updates. */
>> + dec_ffa_vm_count();
>> list_del(&ctx->ctx_list);
>> + ffa_partinfo_inc_tag();
>> write_unlock(&ffa_ctx_list_rwlock);
>> }
>>
>> diff --git a/xen/arch/arm/tee/ffa_partinfo.c
>> b/xen/arch/arm/tee/ffa_partinfo.c
>> index 419e19510f6f..16da5ee567db 100644
>> --- a/xen/arch/arm/tee/ffa_partinfo.c
>> +++ b/xen/arch/arm/tee/ffa_partinfo.c
>> @@ -29,10 +29,39 @@ struct ffa_partition_info_1_1 {
>> uint8_t uuid[16];
>> };
>>
>> +/* Registers a3..a17 (15 regs) carry partition descriptors, 3 regs each. */
>> +#define FFA_PARTINFO_REG_MAX_ENTRIES \
>> + ((15 * sizeof(uint64_t)) / sizeof(struct ffa_partition_info_1_1))
>> +
>> /* SP list cache (secure endpoints only); populated at init. */
>> static void *sp_list __read_mostly;
>> static uint32_t sp_list_count __read_mostly;
>> static uint32_t sp_list_entry_size __read_mostly;
>> +
>> +/* SP list is static; tag only moves when VMs are added/removed. */
>> +static atomic_t ffa_partinfo_tag = ATOMIC_INIT(1);
>> +
>> +void ffa_partinfo_inc_tag(void)
>> +{
>> + atomic_inc(&ffa_partinfo_tag);
>> +}
>> +
>> +static inline uint16_t ffa_partinfo_get_tag(void)
>> +{
>> + /*
>> + * Tag moves with VM list changes only.
>> + *
>> + * Limitation: we cannot detect an SPMC tag change between calls
>> because we
>> + * do not retain the previous SPMC tag; we only refresh it via the
>> mandatory
>> + * start_index=0 call and assume it stays stable while combined_tag (our
>> + * VM/SP-count tag) is used for guest validation. This means SPMC tag
>> + * changes alone will not trigger RETRY.
>> + */
>> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
>> + return atomic_read(&ffa_partinfo_tag) & GENMASK(15, 0);
>> + else
>> + return 1;
>> +}
>
> Please add an empty line here.
ack
>
>> static int32_t ffa_partition_info_get(struct ffa_uuid uuid, uint32_t flags,
>> uint32_t *count, uint32_t *fpi_size)
>> {
>> @@ -140,6 +169,7 @@ static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid,
>> uint32_t *sp_count,
>> for ( n = 0; n < sp_list_count; n++ )
>> {
>> void *entry = sp_list + n * sp_list_entry_size;
>> + void *dst_pos;
>>
>> if ( !ffa_sp_entry_matches_uuid(entry, uuid) )
>> continue;
>> @@ -151,11 +181,20 @@ static int32_t ffa_get_sp_partinfo(struct ffa_uuid
>> uuid, uint32_t *sp_count,
>> * This is a non-compliance to the specification but 1.0 VMs should
>> * handle that on their own to simplify Xen implementation.
>> */
>> + dst_pos = *dst_buf;
>> ret = ffa_copy_info(dst_buf, end_buf, entry, dst_size,
>> sp_list_entry_size);
>> if ( ret )
>> return ret;
>>
>> + if ( !ffa_uuid_is_nil(uuid) &&
>> + dst_size >= sizeof(struct ffa_partition_info_1_1) )
>> + {
>> + struct ffa_partition_info_1_1 *fpi = dst_pos;
>> +
>> + memset(fpi->uuid, 0, sizeof(fpi->uuid));
>> + }
>> +
>> count++;
>> }
>>
>> @@ -167,6 +206,38 @@ static int32_t ffa_get_sp_partinfo(struct ffa_uuid
>> uuid, uint32_t *sp_count,
>> return FFA_RET_OK;
>> }
>>
>> +static uint16_t ffa_get_sp_partinfo_regs(struct ffa_uuid uuid,
>> + uint16_t start_index,
>> + uint64_t *out_regs,
>> + uint16_t max_entries)
>> +{
>> + uint32_t idx = 0;
>> + uint16_t filled = 0;
>> + uint32_t n;
>> +
>> + for ( n = 0; n < sp_list_count && filled < max_entries; n++ )
>> + {
>> + void *entry = sp_list + n * sp_list_entry_size;
>> +
>> + if ( !ffa_sp_entry_matches_uuid(entry, uuid) )
>> + continue;
>> +
>> + if ( idx++ < start_index )
>> + continue;
>> +
>> + memcpy(&out_regs[filled * 3], entry,
>> + sizeof(struct ffa_partition_info_1_1));
>> + if ( !ffa_uuid_is_nil(uuid) )
>> + {
>> + out_regs[filled * 3 + 1] = 0;
>> + out_regs[filled * 3 + 2] = 0;
>> + }
>> + filled++;
>> + }
>> +
>> + return filled;
>> +}
>> +
>> static int32_t ffa_get_vm_partinfo(struct ffa_uuid uuid, uint32_t
>> start_index,
>> uint32_t *vm_count, void **dst_buf,
>> void *end_buf, uint32_t dst_size)
>> @@ -383,6 +454,135 @@ out:
>> }
>> }
>>
>> +void ffa_handle_partition_info_get_regs(struct cpu_user_regs *regs)
>> +{
>> + struct domain *d = current->domain;
>> + struct ffa_ctx *ctx = d->arch.tee;
>> + struct ffa_uuid uuid;
>> + uint32_t sp_count = 0, vm_count = 0, total_count;
>> + uint16_t start_index, tag;
>> + uint16_t num_entries = 0;
>> + uint64_t x3 = get_user_reg(regs, 3);
>> + int32_t ret = FFA_RET_OK;
>> + uint64_t out_regs[18] = { 0 };
>> + unsigned int n;
>> + uint16_t tag_out, tag_end;
>> +
>> + if ( ACCESS_ONCE(ctx->guest_vers) < FFA_VERSION_1_2 )
>> + {
>> + ret = FFA_RET_NOT_SUPPORTED;
>> + goto out;
>> + }
>> +
>> + /*
>> + * Registers a3..a17 (15 regs) carry partition descriptors, 3 regs each.
>> + * For FF-A 1.2, that yields a maximum of 5 entries per GET_REGS call.
>> + * Enforce the assumed layout so window sizing stays correct.
>> + */
>> + BUILD_BUG_ON(FFA_PARTINFO_REG_MAX_ENTRIES != 5);
>> +
>> + start_index = x3 & GENMASK(15, 0);
>> + tag = (x3 >> 16) & GENMASK(15, 0);
>> +
>> + /* Start index must allow room for up to 5 entries without 16-bit
>> overflow. */
>
> Nit: The line above is over 80 columns.
Right, how did i miss that.
>
> With or without the line above fixed. Looks good.
> Reviewed-by: Jens Wiklander <[email protected]>
Thanks,
I will check if those can be fixed on commit otherwise i will fix those 2 and
submit a v3 with your R-b on all patches :-)
Cheers
Bertrand
>
> Cheers,
> Jens
>
>> + if ( start_index > (GENMASK(15, 0) - (FFA_PARTINFO_REG_MAX_ENTRIES -
>> 1)) )
>> + {
>> + ret = FFA_RET_INVALID_PARAMETERS;
>> + goto out;
>> + }
>> +
>> + uuid.val[0] = get_user_reg(regs, 1);
>> + uuid.val[1] = get_user_reg(regs, 2);
>> +
>> + tag_out = ffa_partinfo_get_tag();
>> +
>> + if ( start_index == 0 )
>> + {
>> + if ( tag )
>> + {
>> + ret = FFA_RET_INVALID_PARAMETERS;
>> + goto out;
>> + }
>> + }
>> + else if ( tag != tag_out )
>> + {
>> + ret = FFA_RET_RETRY;
>> + goto out;
>> + }
>> +
>> + if ( ffa_uuid_is_nil(uuid) )
>> + {
>> + if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
>> + vm_count = get_ffa_vm_count();
>> + else
>> + vm_count = 1; /* Caller VM only */
>> + }
>> +
>> + ret = ffa_get_sp_count(uuid, &sp_count);
>> + if ( ret )
>> + goto out;
>> +
>> + total_count = sp_count + vm_count;
>> +
>> + if ( total_count == 0 || start_index >= total_count )
>> + {
>> + ret = FFA_RET_INVALID_PARAMETERS;
>> + goto out;
>> + }
>> +
>> + if ( start_index < sp_count )
>> + num_entries = ffa_get_sp_partinfo_regs(uuid, start_index,
>> &out_regs[3],
>> +
>> FFA_PARTINFO_REG_MAX_ENTRIES);
>> +
>> + if ( num_entries < FFA_PARTINFO_REG_MAX_ENTRIES )
>> + {
>> + uint32_t vm_start = start_index > sp_count ?
>> + start_index - sp_count : 0;
>> + uint32_t filled = 0;
>> + void *vm_dst = &out_regs[3 + num_entries * 3];
>> + void *vm_end = &out_regs[18];
>> +
>> + ret = ffa_get_vm_partinfo(uuid, vm_start, &filled, &vm_dst, vm_end,
>> + sizeof(struct ffa_partition_info_1_1));
>> + if ( ret != FFA_RET_OK && ret != FFA_RET_NO_MEMORY )
>> + goto out;
>> +
>> + num_entries += filled;
>> + }
>> +
>> + if ( num_entries == 0 )
>> + {
>> + ret = FFA_RET_INVALID_PARAMETERS;
>> + goto out;
>> + }
>> +
>> + /*
>> + * Detect list changes while building the response so the caller can
>> retry
>> + * with a coherent snapshot tag.
>> + */
>> + tag_end = ffa_partinfo_get_tag();
>> + if ( tag_end != tag_out )
>> + {
>> + ret = FFA_RET_RETRY;
>> + goto out;
>> + }
>> +
>> + out_regs[0] = FFA_SUCCESS_64;
>> + out_regs[2] = ((uint64_t)sizeof(struct ffa_partition_info_1_1) << 48) |
>> + ((uint64_t)tag_end << 32) |
>> + ((uint64_t)(start_index + num_entries - 1) << 16) |
>> + ((uint64_t)(total_count - 1) & GENMASK(15, 0));
>> +
>> + for ( n = 0; n < ARRAY_SIZE(out_regs); n++ )
>> + set_user_reg(regs, n, out_regs[n]);
>> +
>> + return;
>> +
>> +out:
>> + if ( ret )
>> + ffa_set_regs_error(regs, ret);
>> +}
>> +
>> static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
>> uint8_t msg)
>> {
>> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
>> index 1a632983c860..c291f32b56ff 100644
>> --- a/xen/arch/arm/tee/ffa_private.h
>> +++ b/xen/arch/arm/tee/ffa_private.h
>> @@ -289,7 +289,7 @@
>> #define FFA_MSG_SEND2 0x84000086U
>> #define FFA_CONSOLE_LOG_32 0x8400008AU
>> #define FFA_CONSOLE_LOG_64 0xC400008AU
>> -#define FFA_PARTITION_INFO_GET_REGS 0x8400008BU
>> +#define FFA_PARTITION_INFO_GET_REGS 0xC400008BU
>> #define FFA_MSG_SEND_DIRECT_REQ2 0xC400008DU
>> #define FFA_MSG_SEND_DIRECT_RESP2 0xC400008EU
>>
>> @@ -452,6 +452,8 @@ bool ffa_partinfo_init(void);
>> int32_t ffa_partinfo_domain_init(struct domain *d);
>> bool ffa_partinfo_domain_destroy(struct domain *d);
>> void ffa_handle_partition_info_get(struct cpu_user_regs *regs);
>> +void ffa_handle_partition_info_get_regs(struct cpu_user_regs *regs);
>> +void ffa_partinfo_inc_tag(void);
>>
>> int32_t ffa_endpoint_domain_lookup(uint16_t endpoint_id, struct domain
>> **d_out,
>> struct ffa_ctx **ctx_out);
>> --
>> 2.52.0
