[PATCH v2] firmware: arm_ffa: Add support for IMPDEF value in the memory access descriptor

[Lixiang Mao]

I compared your fix with mine which is tested internally, looks no 
obvious difference.
I think it is good to mainline.

On 9/19/2025 11:52 PM, Sudeep Holla wrote:
> FF-A v1.2 introduced 16 byte IMPLEMENTATION DEFINED value in the endpoint
> memory access descriptor to allow any sender could to specify an its any
> custom value for each receiver. Also this value must be specified by the
> receiver when retrieving the memory region. The sender must ensure it
> informs the receiver of this value via an IMPLEMENTATION DEFINED mechanism
> such as a partition message.
>
> So the FF-A driver can use the message interfaces to communicate the value
> and set the same in the ffa_mem_region_attributes structures when using
> the memory interfaces.
>
> The driver ensure that the size of the endpoint memory access descriptors
> is set correctly based on the FF-A version.
>
> Fixes: 9fac08d9d985 ("firmware: arm_ffa: Upgrade FF-A version to v1.2 in the driver")
> Reported-by: Lixiang Mao <liximao at qti.qualcomm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla at arm.com>
> ---
>   drivers/firmware/arm_ffa/driver.c | 37 ++++++++++++++++++++++---------
>   include/linux/arm_ffa.h           | 16 ++++++++++++-
>   2 files changed, 42 insertions(+), 11 deletions(-)
>
> v1->v2:
> 	- Fix the size of EMAD in ffa_mem_desc_offset() and the loop
> 	  increment in ffa_setup_and_transmit()
>
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index cf6e44429ecd..b68cd9eb09ee 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -654,6 +654,26 @@ static u16 ffa_memory_attributes_get(u32 func_id)
>   	return FFA_MEM_NORMAL | FFA_MEM_WRITE_BACK | FFA_MEM_INNER_SHAREABLE;
>   }
>   
> +static void ffa_emad_impdef_value_init(u32 version, void *dst, void *src)
> +{
> +	struct ffa_mem_region_attributes *ep_mem_access;
> +
> +	if (version > FFA_VERSION_1_1)
> +		memcpy(dst, src, sizeof(ep_mem_access->impdef_val));
> +}
> +
> +static void
> +ffa_mem_region_additional_setup(u32 version, struct ffa_mem_region *mem_region)
> +{
> +	if (version <= FFA_VERSION_1_0) {
> +		mem_region->ep_mem_size = 0;
> +	} else {
> +		mem_region->ep_mem_size = ffa_emad_size_get(version);
> +		mem_region->ep_mem_offset = sizeof(*mem_region);
> +		memset(mem_region->reserved, 0, 12);
> +	}
> +}
> +
>   static int
>   ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
>   		       struct ffa_mem_ops_args *args)
> @@ -672,27 +692,24 @@ ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
>   	mem_region->flags = args->flags;
>   	mem_region->sender_id = drv_info->vm_id;
>   	mem_region->attributes = ffa_memory_attributes_get(func_id);
> -	ep_mem_access = buffer +
> -			ffa_mem_desc_offset(buffer, 0, drv_info->version);
>   	composite_offset = ffa_mem_desc_offset(buffer, args->nattrs,
>   					       drv_info->version);
>   
> -	for (idx = 0; idx < args->nattrs; idx++, ep_mem_access++) {
> +	for (idx = 0; idx < args->nattrs; idx++) {
> +		ep_mem_access = buffer +
> +			ffa_mem_desc_offset(buffer, idx, drv_info->version);
>   		ep_mem_access->receiver = args->attrs[idx].receiver;
>   		ep_mem_access->attrs = args->attrs[idx].attrs;
>   		ep_mem_access->composite_off = composite_offset;
>   		ep_mem_access->flag = 0;
>   		ep_mem_access->reserved = 0;
> +		ffa_emad_impdef_value_init(drv_info->version,
> +					   ep_mem_access->impdef_val,
> +					   args->attrs[idx].impdef_val);
>   	}
>   	mem_region->handle = 0;
>   	mem_region->ep_count = args->nattrs;
> -	if (drv_info->version <= FFA_VERSION_1_0) {
> -		mem_region->ep_mem_size = 0;
> -	} else {
> -		mem_region->ep_mem_size = sizeof(*ep_mem_access);
> -		mem_region->ep_mem_offset = sizeof(*mem_region);
> -		memset(mem_region->reserved, 0, 12);
> -	}
> +	ffa_mem_region_additional_setup(drv_info->version, mem_region);
>   
>   	composite = buffer + composite_offset;
>   	composite->total_pg_cnt = ffa_get_num_pages_sg(args->sg);
> diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
> index 0a0132c6ae3e..6e180fc0e68e 100644
> --- a/include/linux/arm_ffa.h
> +++ b/include/linux/arm_ffa.h
> @@ -347,6 +347,7 @@ struct ffa_mem_region_attributes {
>   	 * an `struct ffa_mem_region_addr_range`.
>   	 */
>   	u32 composite_off;
> +	u8 impdef_val[16];
>   	u64 reserved;
>   };
>   
> @@ -426,10 +427,23 @@ struct ffa_mem_region {
>   #define CONSTITUENTS_OFFSET(x)	\
>   	(offsetof(struct ffa_composite_mem_region, constituents[x]))
>   
> +static inline u32 ffa_emad_size_get(u32 ffa_version)
> +{
> +	u32 sz;
> +	struct ffa_mem_region_attributes *ep_mem_access;
> +
> +	if (ffa_version <= FFA_VERSION_1_1)
> +		sz = sizeof(*ep_mem_access) - sizeof(ep_mem_access->impdef_val);
> +	else
> +		sz = sizeof(*ep_mem_access);
> +
> +	return sz;
> +}
> +
>   static inline u32
>   ffa_mem_desc_offset(struct ffa_mem_region *buf, int count, u32 ffa_version)
>   {
> -	u32 offset = count * sizeof(struct ffa_mem_region_attributes);
> +	u32 offset = count * ffa_emad_size_get(ffa_version);
>   	/*
>   	 * Earlier to v1.1, the endpoint memory descriptor array started at
>   	 * offset 32(i.e. offset of ep_mem_offset in the current structure)