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

[Sudeep Holla]

On Mon, Sep 22, 2025 at 10:12:43AM +0800, Lixiang Mao wrote:
> I compared your fix with mine which is tested internally, looks no obvious
> difference.

Can I take this as tested-by ?

> I think it is good to mainline.
> 

It's too close to v6.17 release, I will send it as fix at v6.18-rc1, must get
backported to all the relevant stable trees. Thanks for reporting and
reviewing.

> 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)

-- 
Regards,
Sudeep